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

From: Dale Worley <[email-masked]>
Date: Sun, 26 Nov 2017 15:01:01 -0800

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]

---

From: Daniel B Giffin <[email-masked]>
Date: Tue, 28 Nov 2017 12:59:28 -0800

Hi Dale,

As described below, I've addressed these nits with a few
changes that will appear in the next draft (probably today).

> 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).

>    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.

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.

>    as above, and provides these and the ciphertext value to the the AEAD

Thanks.

> 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.

> (which wouldn't be used in this document.))

Good enough; I've made this change.

> 
> [END]
> 
> 
> _______________________________________________
> Tcpinc mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/tcpinc

Thanks for these comments!

daniel