Re: [tcpinc] Genart telechat review of draft-ietf-tcpinc-tcpcrypt-10

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@ietf.org
> https://www.ietf.org/mailman/listinfo/tcpinc

Thanks for these comments!

daniel