> Hi Ekr,
>
> I did a quick review of draft-rescorla-tcpinc-tls-option-04, as an
individual (not as chair).
>
> Thanks for putting all the explanational text in there; at least for me
that was very helpful. I have a few comments and (potentially stupid)
questions:
>
> 1) Section 2 say that if there is a decryption failure/finished
> failure, endpoint SHOULD signal a connection failure. Does it mean
> that endpoint SHOULD/MUST also send a TCP RST? And do we really,
> really need to close the connection here, or is there maybe a way to
> think about falling back to TCP only…?
I don't know if we *really* need to, but my reasoning here is that
we now know that the middlebox messed with the data and so failing
the connection is safer.
> 2) The current proposal has the possibility that both endpoints
> might not support the same chipper suite and therefore tcpinc will
> not be used. Should we maybe think about picking a default one (or
> set of default ones that can be ordered by preference) that must be
> supported. I know that's hard, but for tcpinc, as the intention is
> anyway ‚just‘ opportunistic encryption, that might make more
> sense. Maybe we can specify something while at the same time saying
> that the server should not expect that a specific chiper suite will
> be available such that we still have a change to deprecate this
> specification later…
Well, there already is an MTI cipher suite (see: S. 3.5), but
someone could always be noncompliant, right?
> 3) Section 3.1.2.3 says
> "Unless the implementation is specifically configured
> otherwise, it SHOULD NOT attempt to validate the certificate, even
> if it is of type X.509 but merely extract the key.“
> Isn’t the intention here that TCP could pass the cert to the app and the
app could validate if it wants…?
Well, maybe.
1. This applies only to the TLS portion of the system, so its
consistent with that usage.
2. There's no requirement for the cert to be valid so absent
out-of-band semantics the application has limited options
for handling it
> 4) Figure 4 (in section 3.1.4 which is wrongly labeled Figure 2)
> indicated that the server cannot send data with the
> ServerHello. That means to actually access data form a server,
> you’ll always have an additional RTT even in 0-RTT mode. Is that
> true?
Diagram fail. In both the 0 and 1-RTT cases the server can send on
the first flight.
> 5) Section 3.2. says „… MUST respond to renegotiation with fatal
> „no_renegotiation“ alert“. What does this mean for the TCP
> connection?
You should fail the connection. Any failure post-handshake should
lead to the same result.
> 6) I’d recommend to specify the ENO sub option at the beginning of
> the doc. And you further need to specify this more detailed, maybe
> with an image, and give all the specfic bits such as the
> application-aware bits, variable-length bit, and the sub option
> type.
Sure, I can do that.
> 7) Similar as in point 1 & 5: section 5 says „If [..] TLS integrity
> fails, the connection MUST be torn down". Should the endpoint send a
> TCP RST here? Further, as TLS integrity checking should effectively
> also be done in the kernel and data should not be delivered to the
> app if the integrity check fails, there is the option to simply drop
> the TCP packet and wait for a retransmit. This does have security
> implications but maybe we can come up with some additional logic
> that e.g. makes sure that if that happens ‚too often‘ we will close
> the connection…
This seems like it's of modest value. The two main ways this can
happen are:
1. Middlebox modification (where retransmits won't help).
2. Bit-errors which are not detected by the TCP checksum.
I'n not aware of much experience that checksum-conforming bit errors
happen often enough to be worthwhile here. Moreover, if there's
any kind of realignment between the boundaries of the packets and
the TCP segments (as can happen with retransmits and MTU reductions)
then it can be hard to determine which segment is bogus.
> 8) Further the draft says in section 6, that there are two options
> for implementation. And I disagree there. tcpinc should always be
> implemented in the kernel and only if there is potentially an app
> that also implements TLS there might be a config option to allow the
> app to do the TLS handling if successfully negotiated in TCP. It is
> not an option to have a tcpinc implementation that relies on the
> possibility that the app might have TLS implemented because then we
> are end up at a the state where we already are.
We're defining a wire protocol here, so specifying particular
implementation tactics seems out of scope. With that said, I
don't think your assessment that we're back at the same state
we always were. As I think I pointed out when I first introduced
this draft, it addresses two problems with the same wire protocol:
1. The basic TCPINC use case.
2. Out-of-band negotiation of application-level TLS.
An application-level implementation is totally consistent with #2.
> 9) Reading section 9, we must be careful that we don’t create a new
> attack here where broken data causes the connection to close,
> e.g. if packet seems to be in a valid window but does not pass the
> the integrity check. Don’t think that’s actually a case but maybe
> worth think about this.
Well, this turns attacks that would involve valid data injection into
attacks on the connection. I think that's an imporovement.
> And some nits/comments:
> - in Figure 1) the packet from the server is 'SYN/ACK + TCP-ENO [ENO]‘
and should be 'SYN/ACK + TCP-ENO [TLS]‘…?
> - second point in 3.1.1 says ‚(see Figure 2‘ which probably should be
‚(see Figure 3)‘ and Figure 3 is actually called Figure 1
Thanks.
> - Can you maybe try and expend (and refer) all acronyms (ECDSA, TOFO,
ECDHE, ALPN, HKDF, AEAD, …)
Sure.
> - In 3.1.2.1.2 text says ‚(see {{server-first-flight)‘. As this reference
is broken (missing closing brackets), the ref also doesn’t show up in the
ref list at the end…
> - Missing reference in 3.3 (on session resumption; can you also provide
more reasoning here) and in 3.4.
> - Another broken reference in 7: {{RFC5246}
> - section 3.1.6 is not really clear: what’s N_MIN, what’s the record seq
number, is ContentType only one byte?
Thanks.
-Ekr
On Thu, Oct 15, 2015 at 5:04 AM, Mirja Kühlewind <
[email protected]> wrote:
> Hi Ekr,
>
> I did a quick review of draft-rescorla-tcpinc-tls-option-04, as an
> individual (not as chair).
>
> Thanks for putting all the explanational text in there; at least for me
> that was very helpful. I have a few comments and (potentially stupid)
> questions:
>
> 1) Section 2 say that if there is a decryption failure/finished failure,
> endpoint SHOULD signal a connection failure. Does it mean that endpoint
> SHOULD/MUST also send a TCP RST? And do we really, really need to close the
> connection here, or is there maybe a way to think about falling back to TCP
> only…?
>
> 2) The current proposal has the possibility that both endpoints might not
> support the same chipper suite and therefore tcpinc will not be used.
> Should we maybe think about picking a default one (or set of default ones
> that can be ordered by preference) that must be supported. I know that's
> hard, but for tcpinc, as the intention is anyway ‚just‘ opportunistic
> encryption, that might make more sense. Maybe we can specify something
> while at the same time saying that the server should not expect that a
> specific chiper suite will be available such that we still have a change to
> deprecate this specification later…
>
> 3) Section 3.1.2.3 says
> "Unless the implementation is specifically configured
> otherwise, it SHOULD NOT attempt to validate the certificate, even
> if it is of type X.509 but merely extract the key.“
> Isn’t the intention here that TCP could pass the cert to the app and the
> app could validate if it wants…?
>
> 4) Figure 4 (in section 3.1.4 which is wrongly labeled Figure 2) indicated
> that the server cannot send data with the ServerHello. That means to
> actually access data form a server, you’ll always have an additional RTT
> even in 0-RTT mode. Is that true?
>
> 5) Section 3.2. says „… MUST respond to renegotiation with fatal
> „no_renegotiation“ alert“. What does this mean for the TCP connection?
> That’s also a general comment that the integration with TCP is probably in
> some cases not well enough specified yet. For my point of view that’s where
> a (kernel) implementation would be helpful!
>
> 6) I’d recommend to specify the ENO sub option at the beginning of the
> doc. And you further need to specify this more detailed, maybe with an
> image, and give all the specfic bits such as the application-aware bits,
> variable-length bit, and the sub option type.
>
> 7) Similar as in point 1 & 5: section 5 says „If [..] TLS integrity fails,
> the connection MUST be torn down". Should the endpoint send a TCP RST here?
> Further, as TLS integrity checking should effectively also be done in the
> kernel and data should not be delivered to the app if the integrity check
> fails, there is the option to simply drop the TCP packet and wait for a
> retransmit. This does have security implications but maybe we can come up
> with some additional logic that e.g. makes sure that if that happens ‚too
> often‘ we will close the connection…
>
> 8) Further the draft says in section 6, that there are two options for
> implementation. And I disagree there. tcpinc should always be implemented
> in the kernel and only if there is potentially an app that also implements
> TLS there might be a config option to allow the app to do the TLS handling
> if successfully negotiated in TCP. It is not an option to have a tcpinc
> implementation that relies on the possibility that the app might have TLS
> implemented because then we are end up at a the state where we already are.
>
> 9) Reading section 9, we must be careful that we don’t create a new attack
> here where broken data causes the connection to close, e.g. if packet seems
> to be in a valid window but does not pass the the integrity check. Don’t
> think that’s actually a case but maybe worth think about this.
>
>
> And some nits/comments:
> - in Figure 1) the packet from the server is 'SYN/ACK + TCP-ENO [ENO]‘ and
> should be 'SYN/ACK + TCP-ENO [TLS]‘…?
> - second point in 3.1.1 says ‚(see Figure 2‘ which probably should be
> ‚(see Figure 3)‘ and Figure 3 is actually called Figure 1…
> - Can you maybe try and expend (and refer) all acronyms (ECDSA, TOFO,
> ECDHE, ALPN, HKDF, AEAD, …)
> - In 3.1.2.1.2 text says ‚(see {{server-first-flight)‘. As this reference
> is broken (missing closing brackets), the ref also doesn’t show up in the
> ref list at the end…
> - Missing reference in 3.3 (on session resumption; can you also provide
> more reasoning here) and in 3.4.
> - Another broken reference in 7: {{RFC5246}
> - section 3.1.6 is not really clear: what’s N_MIN, what’s the record seq
> number, is ContentType only one byte?
>
> Hope that’s helpful!
>
> Mirja
>
>
>
_______________________________________________
Tcpinc mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/tcpinc
