Hi Dale, As described below, I've addressed these nits with a few changes that will appear in the next draft (probably today). Dale Worley wrote: > 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. Sorry about that ... This choice (determining which encryption keys to use by the host's role in the *original* session, not the current one) is mostly arbitrary. I agree it is not the most intuitive choice, but there is a sliver of purpose to justify it: If some weakness in a TCP-ENO implementation allowed attackers to cause two connecting tcpcrypt hosts to play the same role when resuming, and keys were dependent on the roles in this resumed session, they would then go ahead to encrypt using the same keys. That would be quite dangerous. And each instance of session resumption would be an opportunity to perpetrate this role-confusion attack. Using the original session's role to choose the keys means such an attack would have to succeed against the original key exchange, where a confusion of roles should cause an abort due to the asymmetrical Init1/Init2 messages (and if not, would result in unrelated encryption keys and non-matching session IDs). > > 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. Indeed. I've simply removed the emphasis in those two cases, as it doesn't seem necessary to comprehension. > -- > > 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". Okay. I've gone a bit further in order to be very clear: 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 two policies for the duration of the connection: 1) that resumption requests will be ignored, and thus fresh key exchange will be required; and 2) that no session secrets will be cached. (These policies may be specified independently or as a unit.) 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. > > 3.6. Data Encryption and Authentication > > ... > as above, and provides these and the ciphertext value to the the AEAD > ... > > s/the the/the/ Thanks. > > > 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. Yes. I suppose having the constants be parameterized by the TEP would be the ultimate in flexibility, but I can't think of a good enough reason to add this slight complexity to implementations. > > 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.)) Good enough; I've made this change. > > [END] > > > _______________________________________________ > Tcpinc mailing list > Tcpinc@xxxxxxxx > https://www.ietf.org/mailman/listinfo/tcpinc Thanks for these comments! daniel