Subject: [tcpinc] Adam Roach's Yes on draft-ietf-tcpinc-tcpcrypt-09: (with COMMENT)

From: Adam Roach <[email-masked]>
Date: Wed, 08 Nov 2017 16:45:26 -0800

Adam Roach has entered the following ballot position for
draft-ietf-tcpinc-tcpcrypt-09: Yes

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)

Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-tcpinc-tcpcrypt/

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

Thanks for a well-written document. I have some suggestions for improvement
(two minor, and one significant).

This document has several references to tables that are quite distant from the
table being referenced. It might be useful to readers if these were phrased
something like "Table 4 in section 7," so they know where to go looking for the
table.

Section 3.5 says:

   If an active opener receives a resumption suboption for a particular
   TEP and the received identifier-half does not match the "resume[i]"
   value whose other half it previously sent in a resumption suboption
   for the same TEP, it MUST ignore that suboption.  In the typical case
   that this was the only ENO suboption received, this means the host
   MUST disable TCP-ENO and tcpcrypt: that is, it MUST NOT send any more
   ENO options and MUST NOT encrypt the connection.

I think the text here would benefit from pointing out that the client MUST NOT
attempt to resume a session with this host for the next TCP connection attempt.

____

Section 3.6 stipulates:

   When a frame is received, tcpcrypt reconstructs the associated data
   and frame nonce values (the former contains only data sent in the
   clear, and the latter is implicit in the TCP stream), and provides
   these and the ciphertext value to the the AEAD decryption operation.
   The output of this operation is either a plaintext value "P" or the
   special symbol FAIL.  In the latter case, the implementation MUST
   either drop the TCP segment(s) containing the frame or abort the
   connection; but if it aborts, the implementation MUST raise an error
   condition distinct from the end-of-file condition.

In the text above, the choice to "drop the TCP segment(s)" seems a little
underspecified, and both ways that I can interpret it are problematic.

In one interpretation, the TCP stack acts as if those packets were never
received, and so they are never acknowledged. Since retransmissions will
contain the same contents and also fail to decrypt, this is basically just
going to cause a connection failure upon expiration of the retransmission timer
-- in which case an immediate failure is clearly preferable.

The other interpretation is that the TCP packet is processed as received, but
that all of the data that could not be decrypted is elided from the stream of
bytes provided to the receiving application. This is also a problem, since many
applications rely on the in-order delivery aspects of TCP. The prospect that a
sender could provide "Message A", "Message B", and then"Message C" to its TCP
socket and the receiver only get "Message A" followed immediately by "Message
C" is not something that applications will generally be capable of handling. As
before, a connection reset would be preferable to violating the in-order
delivery guarantees of TCP.

Unless my analysis here is confused, I think this text needs to be changed to
conclude:

   In the latter case, the implementation MUST
   abort the connection and raise an error
   condition distinct from the end-of-file condition.

---

From: "Mirja Kuehlewind (IETF)" <[email-masked]>
Date: Thu, 9 Nov 2017 10:45:35 +0900

Hi Adam,

one/two comment(s) below.

> Am 09.11.2017 um 09:45 schrieb Adam Roach <[email-masked]>:
> 

That’s not true. This is to cover the case where the packet got corrupted on the path, thus hopefully the retransmission will decrypt correctly.

The option to abort the connection is if the decryption failure is assumed to be accounted to some kind of interference or attack. If guess we could try to give more advice when to do what. Like if one packet fails to decrypt you should just drop if, if the decryption failure rate is high or the same packet fails multiple time, there might be an attack and you MAY abort. However, I think the default is silent drop. The only important part is that if you decide to abort to also must raise an appropriate error.

> delivery guarantees of TCP.

This can never happen with TCP. All data are delivered in order and cannot be processed if data is missing. There are no messages exposed in TCP; it's all one stream to the application. If a packet would continuously fail to decrypt, the remaining higher order data will not be delivered to the application and at some point the receive window will be full and the sender cannot send any new data anymore. If I remember correctly, I guess if that situation would persist for long, one end should probably reset the connection anyway at some point.

---

From: Adam Roach <[email-masked]>
Date: Wed, 8 Nov 2017 20:11:49 -0600

> That’s not true. This is to cover the case where the packet got corrupted on the path, thus hopefully the retransmission will decrypt correctly.

So, to be clear, you're talking about packet corruption that happens to 
produce a valid checksum, right? If that's the reasoning here, the 
authors probably want to include that rationale in the document.

> This can never happen with TCP.

Right. That's exactly my point.

/a

---

From: "Mirja Kuehlewind (IETF)" <[email-masked]>
Date: Thu, 9 Nov 2017 12:02:16 +0900

Hi Adam,

> Am 09.11.2017 um 11:11 schrieb Adam Roach <[email-masked]>:
> 
> So, to be clear, you're talking about packet corruption that happens to produce a valid checksum, right? If that's the reasoning here, the authors probably want to include that rationale in the document.
> 

True. Actually I let the author jump in at this point to make sure they don’t have any other cases in mind. In any case it’s probably good to the explicitly as possible about these thing we know here.

Thanks!
Mirja

---

From: Kyle Rose <[email-masked]>
Date: Thu, 9 Nov 2017 22:16:00 +0800

On Thu, Nov 9, 2017 at 10:11 AM, Adam Roach <[email-masked]> wrote:
> > That’s not true. This is to cover the case where the packet got corrupted on
> > the path, thus hopefully the retransmission will decrypt correctly.
>
> produce a valid checksum, right? If that's the reasoning here, the authors
> probably want to include that rationale in the document.

Mirja's is my interpretation, as well.

Off-path attackers wouldn't be able to sent segments with the right
sequence number with high probability, so it's unlikely that this is a
DoS vector; but giving implementations the option of simply dropping
segments for which the authenticity check fails is not likely to cause
recurring timeout problems for correct implementations.

Kyle

---

From: Adam Roach <[email-masked]>
Date: Thu, 9 Nov 2017 09:37:06 -0600

> recurring timeout problems for correct implementations.

Yes, and you're giving implementors options here without really 
explaining why -- which means they're probably just going to pick one 
randomly. Adding text that gives some notion about why one might choose 
one option over the other would allow them to make an informed choice 
rather than a random one.

Again, this remains a suggestion and not a blocking comment. You can 
feel free to proceed with the document as-is.

/a

---

From: Daniel B Giffin <[email-masked]>
Date: Thu, 16 Nov 2017 19:14:49 -0800

Adam Roach wrote:
> Yes, and you're giving implementors options here without really explaining
> why -- which means they're probably just going to pick one randomly. Adding
> text that gives some notion about why one might choose one option over the
> other would allow them to make an informed choice rather than a random one.

That makes sense.  How about the following text instead?
(It is very similar to how we treat spurious FIN in the
following section, 3.7.)

  [...] The output of this operation is either a plaintext
  value `P` or the special symbol FAIL.  In the latter case,
  the implementation SHOULD abort the connection and raise
  an error condition distinct from the end-of-file
  condition.  But if none of the TCP segment(s) containing
  the frame have been acknowledged and retransmission could
  potentially result in a valid frame, an implementation MAY
  instead drop these segments.

This doesn't speculate on how the corruption came about (I
agree it's difficult to find a reasonable case), but it
gives a reasonable default via SHOULD and also leaves a
little room for implementations with some sort of clever
anti-DOS mitigation.

daniel

> 
> Again, this remains a suggestion and not a blocking comment. You can feel
> free to proceed with the document as-is.
> 
> /a

---

From: Daniel B Giffin <[email-masked]>
Date: Thu, 16 Nov 2017 19:17:36 -0800

Thanks for the review, Adam ...

Good idea.  I've gone and added the section to all table
references.

> attempt to resume a session with this host for the next TCP connection attempt.

Is your concern that these two peers can get "stuck" in a
state where they repeatedly fail a mutual attempt at
resumption, thus forever falling back to no encryption?

In the case of a hash-collision of resumption identifiers,
it seems we don't get stuck because the active opener will
advance to the next session secret on the next connection:

	When proposing resumption, the active opener MUST use the lowest value
	of `i` that has not already been used (successfully or not) to
	negotiate resumption with the same host and for the same pre-session
	key `ss[0]`.

Maybe you're seeing a different problem?

> Section 3.6 stipulates:
[truncate]

(I've responded to this concern in a separate thread.)

daniel

---

From: Adam Roach <[email-masked]>
Date: Fri, 17 Nov 2017 11:31:20 +0800

> 	negotiate resumption with the same host and for the same pre-session

Ah, you're right -- this will prevent issues with hash collisions. I 
suppose there's still a small chance of this situation arising due to 
implementation errors, but I suspect it's not worth decreasing the 
incidence of resumption to account for those.

/a

---

From: Adam Roach <[email-masked]>
Date: Fri, 17 Nov 2017 11:35:31 +0800

> little room for implementations with some sort of clever

Thanks -- this seems like a good approach.

/a