Subject: Secdir early review of draft-ietf-taps-transport-security-04

From: Paul Wouters <[email-masked]>
Date: Sun, 25 Nov 2018 21:40:38 -0800

Reviewer: Paul Wouters
Review result: Has Issues

Review of draft-ietf-taps-transport-security-04

I have reviewed this document as part of the security directorate's
ongoing effort to review all IETF documents being processed by the
IESG.  These comments were written primarily for the benefit of the
security area directors.  Document editors and WG chairs should treat
these comments just like any other last call comments.

The summary of the review is HAS ISSUES.

SecDir tools note: The secdir review link refers to an abstract containing the
text:

        This draft summarizes a number of IETF transport security protocols a

Note the word "IETF ... protocols". I don't actually see that in any of the
revisions 00 to 04? Where did this comment/text come from?

Reading the introduction, this draft's goal is:

   This document provides a survey of commonly used or notable network
   security protocols, with a focus on how they interact and integrate
   with applications and transport protocols.  Its goal is to supplement
   efforts to define and catalog transport services [RFC8095] by
   describing the interfaces required to add security protocols.

My first comment is regarding "commonly used or notable". Especially
the latter one is odd. Why should the IETF reader be aware of
"notable" but "not commonly used" protocols? And the reason I
say this is because it lists CurveCP and MinimalT, which as far
as I know are not deployed at all. While openvpn, which has a
serious userbase, is not mentioned at all. And openconnect, an open
specification compatible with Cisco Anyconnect, which even has a draft,
draft-mavrogiannopoulos-openconnect-02, is not mentioned at all. In my
opinion, the document would be improved by adding Openvpn and Openconnect,
and removing CurveCP and MinimalT.

My second comment is regarding IETF vs non-IETF protocols. Or even
"specified in a draft or RFC" versus "there is an academic paper and some
implementation and it's not clear what's what". The reason I am saying
this is that for instance the protocol "specification" of WireGuard keeps
changing. Their website lists a "whitepaper" that is changing over time
with no changelog, and wild claims that I've spend time in the last few
years to counter, upon which it gets rewritten with new misleading or
wrong claims.

It is kind of hard to do an IETF style summary of protocols when the
"specification" includes phrases like:

        the tunnel simply works. Key exchanges, connections, disconnections,
        reconnections, discovery, and so forth happen behind the scenes
        transparently and reliably, and the administrator does not need to
        worry about these details. In other words, from the perspective of
        administration, the WireGuard interface appears to be stateless.

This is not a white paper but a Public Relations document.

None of the other discussed protocols in this document require
administrators to "worry" about those "details" either.

At the IETF, we recommend IETF protocols. If someone comes up with an
alternative protocol that accomplishes the same as an existing one, we
tend to tell people to use the existing IETF protocol instead. Or, we
think their new protocol merits further standarization, and we ask them
to produce either a ISE or IETF stream document that people can reference,
comment, improve and discuss openly on its technical merits.

It is also clear the white paper is not a good complete formal
specification like we do at IETF. For example, if there is packetloss,
the "specification" does not at all specify which of the parties (or
both?) are responsible for retransmission. Can one spoofed packet lead
a WireGuard endpoint to retransmit its response repeatedly?

This further weakens their "stateless" claim. (and if it sounds like I
am being harsh, I know I am. It is because previously, I had to counter
false claims about IPsec speed, and IKE "being noisy" where as having
looked again at "the whitepaper" (which keeps changing every time I look
at it) it seems to now have a new hobby horse in the word "stateless". the
openvpn tun0: is also "stateless", so is the VTI IPsec based interface vti0:

Because of these reasons, I am a bit worried that an IETF document is
making any claims about WireGuard features, as there is no way to know
what the protocol will be like by the time this document is published,
and this document cannot even point to a stable reference of te specification
at the time of review.

If this document wants to mention WireGuard as a protocol to take note of,
I would like to see at least some text there urging WireGuard to write up
(and version) their protocol in an IETF draft/RFC or other proper/formal
specification.

Now, having said all of this about WireGuard, I do agree with mentioning
WireGuard in this document as it does have an actual userbase. Let's
just urge and help them with their specification.  (and If the author(s)
of WireGuard are reading this, I am more than happy to help you with this)

As for CurveCP and MinimalT, as far as I can see, these  are just academic
exercises with no serious deployment. While the IRTF might be discussing
the these, I don't think an IETF document should reference these two
proof of concept ideas until they have matured more.

Introduction / Abstract:

Expand/explain TAPS on first use?

Section 4.4.1.1:

        IKEv2 is a control protocol that runs on UDP port 500

Change to:

IKEv2 is a control protocol that runs on UDP or TCP port 4500 and UDP port 500.

Note you don't mention the one big drawback, that it cannot be run on any UDP
port (although at least now we can run on any TCP port) which is an important
problem when networks block IKE/IPsec on purpose compared to non-IETF VPN
protocols.

        It then
        goes through a phase of authentication in which both peers present
        blobs signed by a shared secret or private key, after which another
        set of keys is derived, referred to as the "Child SA".  These Child
        SA keys are used by ESP.

This text makes it appear only the blob's are authenticated, but the entire
IKE exchange is authenticated. Also sessions keys are not all that is a
Child SA. So perhaps:

IKE then
performs a phase of authentication in which both peers present
blobs signed by a shared secret or private key that authenticates
the entire IKE exchange and the IKE identities. IKE then derives
further sets of keys on demand, which together with traffic policies
are referred to as the "Child SA". These Child SA keys are used by ESP.

        Each proposal may contain an encryption algorithm, an authentication
        algorithm

This is a bit misleading with both the "may" and the lack of AEAD discussion?
Perhaps:

Each proposal contains an encryption with authentication algorithm or an AEAD
algorithm,

        Any SA used by IKEv2 can be rekeyed upon expiration,

Rekeying happens before expiration, not upon (which would imply trafficflow
interuption) Similarly to how it is described for tcpcrypt in the document. So
perhaps just change "upon" to "before" ? Or go in a little more detail with:

Any SA used by IKE/IPsec can be rekeyed before expiration. It does so by
negotiating a second SA, and once confirmed up and running, the old SA is
deleted. This guarantees there is no slowdown or halt of traffic flow during
rekeying.

        that allows a set of Security Associations to migrate over different
        addresses and interfaces

I would say "different outer IP addresses and interfaces"

        Each SA is
        identified by a Security Parameter Index (SPI), which is marked on
        each encrypted ESP packet.

I would avoid the word "mark" here, since to me that more presents internal
state inside a kernel (eg iptables MARK) and not something that goes over the
wire. So perhaps:

The SA of an encrypted packet is identified by its Security Parameter Index
(SPI) [which is send as part of the ESP header)

        an anti-replay service (a form of partial sequence integrity)

What is partial about this? Perhaps you meant to say ESP, like UDP, does not
provide guaranteed delivery? Anyway, a "partial" security function reads odd :)

        and limited traffic flow confidentiality.

What is "limited" about it? You can generate bogus traffic and pad traffic to
any size (most common max mtu). What would you want in additional to make it
not "limited" ?

One thing I would add to the ESP section is mentioning it acts like UDP. It
cannot be reset by a single TCP RST packet, and you don't have two layers of
netflows doing retransmission. This feature of ESP (and some UDP based
protocols in this document) is fairly important.

Section 4.4.2.1:

        Mutual Authentication

It is possible to do server-only or opportunistic/anonymous IKE as well. Maybe
add "Usually" ?

4.6.1:

        Tcpcrypt exposes a universally-unique connection-specific session ID
        to the application, suitable for application-level endpoint
        authentication either in-band or out-of-band.

This kind of conflicts with the introduction that claims this is only an
opportunistic encryption protocol? Maybe merge this part into the description
text?

4.6.2:

I might say here something about only passive attacker protection? That seems
like an important difference compared to the other protocols. Or perhaps make
this clear in the introduction text when mentioning opportunistic encryption.

4.7:

        WireGuard is a layer 3 protocol designed to complement or replace
        IPsec [WireGuard] for certain use cases.

I am confused about how it "complements" IPsec? Perhaps you mean it is an
"alternative" (and perhaps a non-IETF alternative) ?

I think "replace" is very misleading, as IPsec has a vast number of different
use cases compared to WireGuard's "static VPN configuration".

        WireGuard is designed to be entirely stateless, modulo the CryptoKey
        routing table,

Wouldn't you be able to say the same about the ESP SPD/SAD table? I'm not sure
how different the ESP/wireguard statelessness is on a scale or truly stateless
to NFS

You don't mention anything about port preconfiguartion and the "port knocking"
thing?

           o  Record replay prevention (Stateful, timestamp-based).

I thought it was stateless module CryptoKey routing? :)

You do not mention mobility or session resumption for WireGuard. Since it is
all about static inner IP addresses over arbitrary outer addresses, it has
mobility. And I guess the 1RTT means it has session resumption?

Can you add a description of rekeying for WireGuard similar to IKEv2/ESP? I
thought there was an issue there that during rekey, there is no continued data
flow possible, so connections briefly slow down or halt when the channel is
being rekeyed.

Does WireGuard not offer padding or any other kind of Traffic Confidentiality
options ?

4.8:

The MinimalT section seems to lack some information bullet points compared to
the other sections? eg shouldn't it have things like:  o Datagram Transport ?

Misc:

Should tor be added to the list of protocols?

Why are MinimalT and CurveCP part of this list? Is there any actual deployment
out there? I thought this document described actual transport security
protocols people can pick. I don't see CurveCP as a real candidate for this. I
don't know about MinimalT.

5:

I'm a little confused by the term Application in this section. Is this the
enduser application or the userland part of the security protocol (eg IKE). I
think you mean the application of the enduser?

5.1:

Listed as mandatory feature is:

   o  Public-key certificate based authentication.

Yet you have mentioned that neither WireGuard or CurveCP can do authentication
based on certificates?

5.2:

        o  Length-hiding padding (LHP):

            *  Application dependency: Knowledge of desired padding policies.

This is not (always?) true? For example ESP can do padding after IKE negotiated
it, and do it based on MTU ? The (end user) application does not have to be
aware of anything?

5.3:

The definition of "Mandatory" within this section is confusing. Maybe use
another word like "Core" to indicate it is part of the core of the protocol and
cannot be disabled?

NITS:

spelling error: defailt