Subject: [tcpinc] Review of draft-ietf-tcpinc-tcpcrypt-01

From: Jana Iyengar <[email-masked]>
Date: Wed, 6 Apr 2016 03:01:50 -0300

High-order thoughts: I enjoyed reading this spec, and I think it's mostly
there. This draft is well-written and is clear enough. There's some amount
of rationale mixed in with the spec which can perhaps be separated out to
ease implementation, but the draft is certainly complete and an independent
implementation can be done from it.

Details:
- Section 3.2: It might be useful to describe why the asymmetry is a
requirement of the protocol.
- Section 3.4: Does this assume that TCP Fast Open is not being used?
Please add a note about interactions with TFO.
- Section 3.4: INIT1_MAGIC (and INIT2_MAGIC) are defined later in the doc,
but add a note here about what they are.
- Section 3.4: Is "eno_transcript" defined precisely somewhere?
- Section 3.5: Can you use odd i's for A's active opens, and even i's for
B's active opens?
- Section 3.6: "the implementation MUST either ignore the frame or
terminate the connection": I think you need to specify one action here. I
think the right course is to terminate the connection, but either ways, I
don't think this should be left to the implementation.
- Section 3.7: This spec changes TCP behavior specified in RFC 793 by
ignoring the URG and FIN bits. I don't know how to capture this... but this
needs to be explicitly called out somewhere. Does this spec need to say
"Updates RFC 793"?
- I actually like this protection... even though other fields cannot be
protected because we want to allow middlebox modification, it would be
GREAT to have the ability to ship the other flags (such as the RST bit, for
instance) for measurement purposes where the endpoint may not change
behavior, but knows that this field was modified/generated by a middlebox.
- Section 4.2.3: What are the hex values in the first 4 bytes? They don't
seem to be described anywhere.
- Section 4.2.3: Can you add a note about the TCP sequence number
wraparound? Specifically, I believe the offset continues monotonically
increasing when the TCP sequence wraps, right?
- Section 10: "Tcpcrypt guarantees that no man-in-the-middle attacks
occurred if Session IDs match...": I think this statement is accurate but a
bit misleading, since tcpcrypt as spec'd doesn't really protect from active
MITM interception. Perhaps add this to the end of the next para about
active eavesdroppers as "However, if the session IDs match, Tcpcrypt
guarantees that ..."

Nits:
- Section 2: "Avoid unnecessary round trips": If they're not necessary, why
have them? :-) Perhaps "Avoid additional round trips", or remove entirely.
- Section 2: "resumed TCP connections": What does this mean? TCP
connections are not "resumed" with a new IP.
- Section 3.5: "... then it uses k_ab for sending segments and k_ba for
receiving." --> "... then it uses k_ab for sending segments and k_ba for
receiving even in subsequent sessions, where it may have the B role."
- Acknowledgments: I don't recall what standard practice allows, but does
it make sense to mention funding sources of the work that led to this IETF
wg document?