Reviewer: Dale Worley Review result: Ready with Nits I am the assigned Gen-ART reviewer for this draft. The General Area Review Team (Gen-ART) reviews all IETF documents being processed by the IESG for the IETF Chair. Please wait for direction from your document shepherd or AD before posting a new version of the draft. For more information, please see the FAQ at <https://wiki.tools.ietf.org/area/gen/wiki/GenArtfaq>. Document: review-draft-ietf-tcpinc-tcpcrypt-10 Reviewer: Dale R. Worley Review Date: 2017-11-26 IETF LC End Date: 2017-10-19 IESG Telechat date: 2017-11-30 Summary: This draft is basically ready for publication, but has nits that should be considered before publication. I'm rather impressed that the authors have gone through three revisions in little more than a month after LC. The text prefixed by ">" is extracted from Daniel B Giffin's reply of 21 Oct 2017 to my Gen-Art review of draft-ietf-tcpinc-tcpcrypt-07. [in my review of draft-ietf-tcpinc-tcpcrypt-07:] 6. It might be worth adjusting the rules for how the A and B roles are carried forward during session resumption. Of course, each host should compute the resumption identifier that it expects to receive based on the role it had in the previous session. But it's not clear to me why a host that used k_ab for encryption (i.e., had the A role) in the previous session must also use k_ab for encryption in the resumed session, since the two sequences of k_ab/k_ba are generated from the different session keys of the two sessions. If you made the choice of k_ab/k_ba be dependent on the A/B roles established by TCP-ENO for *this* session, it seems like the specification of the protocol would be a bit simpler. > I'll explain this design choice under separate cover, when I > have a moment ... I can't find the discussion of this design choice. > Yes, underscores are used to mark terms that are being > introduced for the first time and defined. Contra this statement, _..._ is used in two places as an emphatic: 3.2. Protocol Negotiation If a passive opener receives an ENO option including tcpcrypt TEPs it supports, it MAY then attach an ENO option to its SYN-ACK segment, including _solely_ the TEP it wishes to enable. 3.5. Session Resumption If an active opener sends a resumption suboption with a particular TEP and the appropriate half of a resumption identifier and then, in the same TCP handshake, receives a resumption suboption with the same TEP and an identifier-half that does _not_ match that resumption identifier, it MUST ignore that suboption. -- 3.5. Session Resumption Implementations that cache session secrets MUST provide a means for applications to control that caching. In particular, when an application requests a new TCP connection, it must be able to specify that during the connection no session secrets will be cached and all resumption requests will be ignored in favor of fresh key exchange. And for an established connection, an application must be able to cause any cache state that was used in or resulted from establishing the connection to be flushed. A companion document [I-D.ietf-tcpinc-api] describes recommended interfaces for this purpose. The phrase "all resumption requests will be ignored in favor of fresh key exchange" doesn't seem to carry quite the right meaning to me, although what it must mean in this context is clear. That phrase seems to have some implication that it operates when a fresh key exchange is provided along with a resumption request as an alternative... which is implicitly what the situation is, but not explicitly. I would prefer "all resumption requests will be ignored and a fresh key exchange will be required". 3.6. Data Encryption and Authentication ... as above, and provides these and the ciphertext value to the the AEAD ... s/the the/the/ > I've gone through and used something like "negotiated TEP" > in a couple places where the document said that a parameter > depended on the "negotiated key-agreement scheme", and also > added various phrases to make clear that the TEP dictates > all the parameters. Note that the current sections 4.3 and 5 show that the *lengths* are determined by TEP, but the *constants* are fixed by tcpcrypt itself. 3.3. Key exchange o "PK_A", "PK_B": ephemeral public keys for hosts A and B, respectively. [in my review of draft-ietf-tcpinc-tcpcrypt-07:] The use of "PK" for a public key seems to be poorly mnemonic, as it is also the acronym of "private key". There ought to be standard (and distinct!) abbreviations for these phrases, but I can't find any... > Yeah ... at least in this document we don't name private > keys, as they are never transmitted. How about "Pub_A" and "Pub_B"? (This suggests "Prv_A" and "Prv_B" (which wouldn't be used in this document.)) [END]