Skip to main content

Telechat Review of draft-ietf-tcpinc-tcpcrypt-10
review-ietf-tcpinc-tcpcrypt-10-genart-telechat-worley-2017-11-26-00

Request Review of draft-ietf-tcpinc-tcpcrypt
Requested revision No specific revision (document currently at 15)
Type Telechat Review
Team General Area Review Team (Gen-ART) (genart)
Deadline 2017-11-28
Requested 2017-10-25
Authors Andrea Bittau, Daniel B. Giffin , Mark J. Handley , David Mazieres , Quinn Slack , Eric W. Smith
I-D last updated 2017-11-26
Completed reviews Rtgdir Telechat review of -07 by John Drake (diff)
Opsdir Last Call review of -07 by Zitao Wang (diff)
Secdir Last Call review of -07 by Stephen Kent (diff)
Genart Last Call review of -07 by Dale R. Worley (diff)
Secdir Telechat review of -09 by Barry Leiba (diff)
Secdir Telechat review of -10 by Barry Leiba (diff)
Opsdir Telechat review of -10 by Zitao Wang (diff)
Genart Telechat review of -10 by Dale R. Worley (diff)
Assignment Reviewer Dale R. Worley
State Completed
Request Telechat review on draft-ietf-tcpinc-tcpcrypt by General Area Review Team (Gen-ART) Assigned
Reviewed revision 10 (document currently at 15)
Result Ready w/nits
Completed 2017-11-26
review-ietf-tcpinc-tcpcrypt-10-genart-telechat-worley-2017-11-26-00
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]