Re: [tcpinc] Review of draft-rescorla-tcpinc-tls-option-04.txt

[Eric Rescorla <[e-mail]>]

> This is my review of draft-rescorla-tcpinc-tls-option-04.txt.
> Circumstances were such that I had to print it out and review it on
> paper.  Given the tight dependency on TLS1.3, this means my review is
> light on the question of integration with TLS, and more geared towards
> interaction with sockets and TCP-ENO.  Fortunately, the latter is
> probably where I can add the most value.

Thanks for taking the time to review this.


> * 0-RTT
>
> My most serious concern is that of the 0-RTT mode will introduce both
> security and correctness limitations, and therefore that if it is there
> at all it should not be on by default.  The security issue is that 0-RTT
> lacks forward secrecy for the full connection and is vulnerable to
> replay attacks.  Hence, this is not something that should be enabled
> transparently to applications.

Yes, I agree that this shouldn't be on by default (for the PFS reason).
I've been thinking some about the replay issue and it seems like
that's actually pretty easy to address by having the server (side B)
send a nonce in its TCP-ENO response. This isn't possible in the
general TLS case, but it's no problem here. This will cause problems
with TFO, but then TFO is a problem in a lot of places. There's
something like this in the latest draft.


> The functionality issue is that I don't understand how Figure 2 would
> line up with various system calls including requests for the session ID.
> For the sake of concreteness, let's imagine mapping this to a socket
> interface.  On the active opener, there are two main events:
>
>   1. A call to connect(), initiating the connection, and
>
>   2. A point at which connect "finishes."  By default, this event is
>      indicated by the connect system call returning.  However, in the
>      event that fcntl F_GETFD has been used to set the O_NONBLOCK flag,
>      this event is indicated *poll() or select() considering the file
>      descriptor writable.

I assume your thinking here is that you would delay return of
connect() to after the crypto handshake is completed.


> I'm trying to figure out how these two events map to Figure 2 (message
> flow for a zero round trip handshake).  You would only have application
> data to send after event #2.  Yet since the TCP stack doesn't know for a
> fact that the client will write first, it doesn't want to delay sending
> the other parts of the initial message (ClientHello, etc.).

Yeah, I think we're going to need some TFO-ish API to indicate what
you're willing to send in the first flight. This would also have
the side effect of enabling 0-RTT, addressing the default concern
you raise above.


> Things get even more complicated when you throw in the ENO Session ID
> ("ESID").  What happens when an application requests the ESID between
> points 1 and 2?  I assume you get EAGAIN (or maybe EWOULDBLOCK on
> systems that distinguish and use EWOULDBLOCK for connect).  What happens
> when an application requests the ESID after point 2?  My assumption was
> that the call would return an ESID immediately.  Other possibilities
> include blocking until the ServerHello (highly problematic when
> O_NONBLOCK is set), returning EAGAIN, or returning some other error that
> could be interpreted as TCPINC disabled.  All three of those
> possibilities seem like they could really trip up applications.  Yet if
> you return the ESID before the ServerHello, I don't actually see how the
> chosen ESID can be secure.  (You're vulnerable to replay at that point.)

It seems like if one had a separate way to supply the data, then
there wouldn't be an issue here, right? You would just have to
wait till after connect() completed to ask for the ESID.


> * Negotiation failure
>
> Section 2 states:
>
>         If the TLS handshake fails for non-cryptographic reasons such as
>         failure to negotiate a compatible cipher or the like, endpoints
>         SHOULD behave as if the the TCP-TLS option was not present.
>
> Currently this is in contradiction with TCP-ENO, which states:
>
>         Conversely, if a host receives an ACK segment containing an ENO
>         option, then encryption MUST be enabled.  From this point the
>         host MUST follow the encryption protocol of the negotiated spec
>         and MUST NOT present raw TCP payload data to the application.

I've changed the document to conform to TCP-ENO.


> Now of course we can change TCP-ENO if this is something we want, but
> doing so is unfortunate as it means you might have negotiated something
> else that works with ENO, yet now it's too late to go back and try a
> different encryption spec.  Therefore I'd like to propose something
> different:  requiring all implementations to have some minimum cipher
> suite.

That's already there.


> While normally we wouldn't want to mandate some ciphersuite, I think ENO
> makes this acceptable because of the upgrade path.  This would mean that
> instead of requesting an ENO suboption for "TCP-TLS", you would request
> an ENO suboption for "TCP-TLS with mandatory P256/AES128GCM..." or some
> such.  The other alternative would be to use a variable-length suboption
> and include a bitmask of supported cipher suites.

I'm not excited about that.

BTW, unless I'm missing something, doesn't this same exact property
apply to tcpcrypt, because the symmetric cipher is negotiated in
init1/init2 and you can have a mismatch there.



> * More general comments
>
> This document was very hard to read without the TLS1.3 draft.  A bit
> more context on a lot of questions would greatly have improved
> readability.  For example, how does one distinguish between an X.509
> certificate and a raw public key?

Hmm... I thought this was clear. It's negotiated:

   The server's certificate. If the client offered a "Raw Public Key"
   type in ServerCertTypeExtension this message SHALL contain a
   SubjectPublicKeyInfo value for the server's key as specified in
   {{RFC7250}}.  Otherwise, it SHALL contain one or more X.509
   Certificates, as specified in {{I-D.ietf-tls-tls13}}, Section
   6.3.5. In either case, this message MUST contain a key which is
   consistent with the client's SignatureAlgorithms and NamedGroup
   extensions.

Can you help me understand what's not clear about this text.


> Why do you even need a public key if
> you have ECDH parameters?

Basically, that's the way TLS rolls. We're trying not to have
a situation where we have both pure anonymous and server-authenticated
modes in the protocol logic and so having an unauthenticated
signature key seems like a good compromise.


> I realize that the working group doesn't like APIs creeping into
> protocol documents, but a more concrete discussion of the API changes
> you envision would make it easier to follow the protocol.  I would urge
> that either for the sake of the draft, there be a non-normative section
> on possible API extensions, or that there be some companion informative
> document (along the lines of draft-bittau-tcpinc-api) that we can refer
> to.  After all, configuration comes in to play in, e.g., section
> 3.1.2.3, and it's hard to follow in the abstract.

I'll take a look at what changes to draft-bittau-tcpinc-api would be
helpful here.


> Related to both the public key and API points, I found the mention of
> TOFU uses in section 3.1.1 very vague.  Some more detail would be really
> helpful.

It's vague because I haven't really worked it out. I think there's
a sense that people would like to have some sort of pinning/tofu
mechanism, but I'm not aware that anyone has worked out details
for TCPINC as a whole, much less TLS.


> The document lays out two use cases:  transport-level and application
> level.  I would strongly recommend requesting two different ENO
> suboptions for the two cases.  Only the transport-level suboption would
> really seem to be in scope for the TCPINC working group.  If an extra
> ENO suboption could be of benefit to the TLS working group, then maybe
> that's an ancillary benefit of standardizing ENO.  However, if TCPINC
> chooses TLS, we should go with only simplest most stripped down
> TLS1.3-only protocol, so whatever ENO suboption TCPINC adopts should not
> permit 1.2 as a fallback.

I have had several people suggest two different code points and I'd
be OK with that if the WG wants that. However, I don't think it's
helpful to rule the second case out of scope, since the binding
to TCP is basically the same (which I regard as a feature).


> Section 3.1 states:
>
>         In order to facilitate implementation, this section provides a
>         non-normative description of the parts of TLS 1.3 which are
>         relevant to TCPINC and defines a baseline of algorithms and
>         modes which MUST be supported.
>
> Assuming this sentence is intended to mean what I think it means, I
> would insert the word normative as follows:
>
>         In order to facilitate implementation, this section provides a
>         non-normative description of the parts of TLS 1.3 which are
>         relevant to TCPINC and defines a normative baseline of
>                                          ^^^^^^^^^
>         algorithms and modes which MUST be supported.

Yes, thanks.


> * NITs
>
> ENO in SYN-ACK is already reported.
>
> ** Section 3.1.1
>
> "(see Figure 2" is missing right parenthesis.
>
> "In both case, the server" -> "... cases, ..."
>
> ** Section 3.1.2.1.2
>
> "(see {{server-first-flight)" is clearly some typo intended to be a
> reference.
>
> ** Section 3.1.2.2.1
>
> "The server respond's" -> "... responds"

Thanks.

-Ekr


On Sun, Oct 18, 2015 at 12:04 AM, David Mazieres <
[e-mail]> wrote:

> This is my review of draft-rescorla-tcpinc-tls-option-04.txt.
> Circumstances were such that I had to print it out and review it on
> paper.  Given the tight dependency on TLS1.3, this means my review is
> light on the question of integration with TLS, and more geared towards
> interaction with sockets and TCP-ENO.  Fortunately, the latter is
> probably where I can add the most value.
>
> At a very high level, I think if you are going to combine TCP-ENO and
> TLS, this is basically the natural way to do it.  Use of ENO to enable
> TLS seems good.  Use of an extractor for the session ID with ENO
> transcript seems appropriate.  Assignment of the A and B roles to client
> and server respectively also seems right.  However, I do have some
> concerns with the draft.
>
> * 0-RTT
>
> My most serious concern is that of the 0-RTT mode will introduce both
> security and correctness limitations, and therefore that if it is there
> at all it should not be on by default.  The security issue is that 0-RTT
> lacks forward secrecy for the full connection and is vulnerable to
> replay attacks.  Hence, this is not something that should be enabled
> transparently to applications.
>
> The functionality issue is that I don't understand how Figure 2 would
> line up with various system calls including requests for the session ID.
> For the sake of concreteness, let's imagine mapping this to a socket
> interface.  On the active opener, there are two main events:
>
>   1. A call to connect(), initiating the connection, and
>
>   2. A point at which connect "finishes."  By default, this event is
>      indicated by the connect system call returning.  However, in the
>      event that fcntl F_GETFD has been used to set the O_NONBLOCK flag,
>      this event is indicated *poll() or select() considering the file
>      descriptor writable.
>
> I'm trying to figure out how these two events map to Figure 2 (message
> flow for a zero round trip handshake).  You would only have application
> data to send after event #2.  Yet since the TCP stack doesn't know for a
> fact that the client will write first, it doesn't want to delay sending
> the other parts of the initial message (ClientHello, etc.).
>
> Things get even more complicated when you throw in the ENO Session ID
> ("ESID").  What happens when an application requests the ESID between
> points 1 and 2?  I assume you get EAGAIN (or maybe EWOULDBLOCK on
> systems that distinguish and use EWOULDBLOCK for connect).  What happens
> when an application requests the ESID after point 2?  My assumption was
> that the call would return an ESID immediately.  Other possibilities
> include blocking until the ServerHello (highly problematic when
> O_NONBLOCK is set), returning EAGAIN, or returning some other error that
> could be interpreted as TCPINC disabled.  All three of those
> possibilities seem like they could really trip up applications.  Yet if
> you return the ESID before the ServerHello, I don't actually see how the
> chosen ESID can be secure.  (You're vulnerable to replay at that point.)
>
> * Negotiation failure
>
> Section 2 states:
>
>         If the TLS handshake fails for non-cryptographic reasons such as
>         failure to negotiate a compatible cipher or the like, endpoints
>         SHOULD behave as if the the TCP-TLS option was not present.
>
> Currently this is in contradiction with TCP-ENO, which states:
>
>         Conversely, if a host receives an ACK segment containing an ENO
>         option, then encryption MUST be enabled.  From this point the
>         host MUST follow the encryption protocol of the negotiated spec
>         and MUST NOT present raw TCP payload data to the application.
>
> Now of course we can change TCP-ENO if this is something we want, but
> doing so is unfortunate as it means you might have negotiated something
> else that works with ENO, yet now it's too late to go back and try a
> different encryption spec.  Therefore I'd like to propose something
> different:  requiring all implementations to have some minimum cipher
> suite.
>
> While normally we wouldn't want to mandate some ciphersuite, I think ENO
> makes this acceptable because of the upgrade path.  This would mean that
> instead of requesting an ENO suboption for "TCP-TLS", you would request
> an ENO suboption for "TCP-TLS with mandatory P256/AES128GCM..." or some
> such.  The other alternative would be to use a variable-length suboption
> and include a bitmask of supported cipher suites.
>
> * More general comments
>
> This document was very hard to read without the TLS1.3 draft.  A bit
> more context on a lot of questions would greatly have improved
> readability.  For example, how does one distinguish between an X.509
> certificate and a raw public key?  Why do you even need a public key if
> you have ECDH parameters?
>
> I realize that the working group doesn't like APIs creeping into
> protocol documents, but a more concrete discussion of the API changes
> you envision would make it easier to follow the protocol.  I would urge
> that either for the sake of the draft, there be a non-normative section
> on possible API extensions, or that there be some companion informative
> document (along the lines of draft-bittau-tcpinc-api) that we can refer
> to.  After all, configuration comes in to play in, e.g., section
> 3.1.2.3, and it's hard to follow in the abstract.
>
> Related to both the public key and API points, I found the mention of
> TOFU uses in section 3.1.1 very vague.  Some more detail would be really
> helpful.
>
> The document lays out two use cases:  transport-level and application
> level.  I would strongly recommend requesting two different ENO
> suboptions for the two cases.  Only the transport-level suboption would
> really seem to be in scope for the TCPINC working group.  If an extra
> ENO suboption could be of benefit to the TLS working group, then maybe
> that's an ancillary benefit of standardizing ENO.  However, if TCPINC
> chooses TLS, we should go with only simplest most stripped down
> TLS1.3-only protocol, so whatever ENO suboption TCPINC adopts should not
> permit 1.2 as a fallback.
>
> Section 3.1 states:
>
>         In order to facilitate implementation, this section provides a
>         non-normative description of the parts of TLS 1.3 which are
>         relevant to TCPINC and defines a baseline of algorithms and
>         modes which MUST be supported.
>
> Assuming this sentence is intended to mean what I think it means, I
> would insert the word normative as follows:
>
>         In order to facilitate implementation, this section provides a
>         non-normative description of the parts of TLS 1.3 which are
>         relevant to TCPINC and defines a normative baseline of
>                                          ^^^^^^^^^
>         algorithms and modes which MUST be supported.
>
> * NITs
>
> ENO in SYN-ACK is already reported.
>
> ** Section 3.1.1
>
> "(see Figure 2" is missing right parenthesis.
>
> "In both case, the server" -> "... cases, ..."
>
> ** Section 3.1.2.1.2
>
> "(see {{server-first-flight)" is clearly some typo intended to be a
> reference.
>
> ** Section 3.1.2.2.1
>
> "The server respond's" -> "... responds"
>
>