Re: [tcpinc] Comments on tcpcrypt draft

[Daniel B Giffin <[e-mail]>]

Hi Kyle,

Thanks for your detailed comments back in March.  Most of
them have been addressed in the new draft, as described
below.

Kyle Rose wrote:
> Removing chair hat for this thread.
> 
> 
> Section 3 general comments: There seems to be a lot of exposition here
> beyond what is explicitly required to implement the protocol. I might
> use section 3 to describe the wire protocol without any explanations
> or security analysis of design choices, leaving those explanations for
> a later section. Accordingly, I might bring the specifications of
> INIT1 and INIT2 into section 3. I'm curious to know what experienced
> draft authors think about the options here.

I have moved nearly all of the explanatory language from the
section describing the mechanics of the protocol; most of it
appears now in "Security considerations" or "Design notes".

> 3.1 "An extract function is used to generate a pseudo-random key":
> "Key" or "secret"? TLS 1.3 refers to the output of this key agreement
> as the "ephemeral secret". It might be helpful to harmonize on
> terminology. It's additionally confusing because this key is used to
> derive the "pre-master secret", which is then used to derive further
> keys; and this bit sequence is never used directly as the key for a
> cipher or MAC.

I've stuck with 'PRK' here because that's the name of this
datum in the HKDF RFC 5869, and also because it is indeed a
"key-worthy" value by virtue of its pseudo-randomness, even
if it is not used directly for encryption in tcpcrypt.

But I agree that "ephemeral secret" (for the ECDHE output)
gives a nice naming consistency with the "ephemeral public
keys" used to derive it, so I've adopted your suggestion
there.

(This replaces the previous term "pre-master secret", which
isn't a very helpful in the absence of a "master secret".
And we've got the unrelated "master keys" later.)

Does this seem any clearer now?

> 3.1 "A collision-resistant function is one on which, for sufficiently
> large L, an attacker cannot find...": I think this is unclear: L is
> not a function of the function chosen, but if "F"="family" might be a
> function of the family from which the function is chosen (though .
> Alternatively, is there a normative reference for this so the
> definition of CPRF can simply be dropped?

I can't quite understand your objection to the language here
(perhaps on account of some truncated text), but anyway I
have moved it to the "Security considerations" section.
Because the CPRF to use is completely specified in this
document, I'm hoping it's not necessary to cite an external
definition.

> 3.3 "when and only when": Precise and clear to me, but academic
> language-ish? Would be interested in others' opinions on this.

Okay; that phrase has fallen out during a rework of the
language there.

> 3.3 "This document reserves and associates four 'cs' values with
> tcpcrypt, as listed in Table 1": Registry?

Table 1 lives in "IANA Considerations"; let me know if the
language there could be improved.

> 3.4 "ephemeral public keys...SHOULD NOT ever be written to persistent
> storage": I agree for the private keys, but as written this states
> that the public keys shouldn't be written out either, which doesn't
> matter at all.

Thanks, fixed.

> 3.4 For harmonization with TLS terminology, I might suggest:
> 
> ECDHE output = "ephemeral secret"
> PRK = "master secret"

(The terms have been adjusted as discussed above.)

> 3.4 "param := ...": Why this notation, where param is separate from PMS?

Okay, I've removed this intermediate term for simplicity.

> 3.4 "PRK := Extract (N_A, ...": Why is N_A used as the salt here
> instead of N_A || N_B? I see that N_B is included in INIT2, but this
> just seems like an arbitrary choice without any explanation anywhere I
> can find.

It is perhaps somewhat arbitrary, but the explanation is
that "this is how the cryptographer designed it."  This
document doesn't explain the design considerations behind
most of the other crypto stuff either, but let us know if
this bit seems distractingly inscrutable.

> 3.4 "SID[0]...will with overwhelming probability be unique for each
> individual TCP connection": It feels like this belongs in a later
> security analysis section: for one thing, the uniqueness doesn't seem
> to be used elsewhere in section 3, and (as I said earlier) it feels
> like the doc would be better structured to separate the protocol
> description from any statements about its security properties, which
> qualify simply as "noise" to someone trying to implement it.

I was about to agree and move this language elsewhere, but I
think this fact could be useful to an implementor and don't
want to bury it in later explanatory text.

> 3.4 "There is no 'key confirmation' step in tcpcrypt...": Again, I'd
> move this later.

Done.

> 3.4 "If key negotiation is compromised and yields two different
> keys,...": This means collisions in SID[i](0..8) (72 bits) will result
> in failed connections, which will lead to a frequency of something
> like 1 out of every 2^36 connections wherever the session cache on the
> passive opener cannot distinguish between different active openers. Is
> this actually a problem? Not clear: should there be requirements
> around how active and passive openers partition their session caches?
> If naïve clients or servers use one big pool, you can expect to see
> this problem quite frequently at internet scale, with billions of
> connections per second.

Well the frequency of collision depends on the size of your
cache.  I guess you're saying that if you manage a cache for
2^36 active clients, then each new connection has
probability of collision (2^36)/(2^72) = 2^(-36).
Apparently we are supposed to have around 2^33
internet-connected things this year, so perhaps this is a
reasonable concern.

The motivation for the nine-byte SID prefix is to yield a
multiple-of-four ENO length in the common case.  More
comment on this tradeoff is welcome!

> 3.5 The session cache section feels like it should also be split up
> into descriptions of protocol requirements and digressions (e.g., "the
> only cost is the additional ten bytes...").
> 
> 3.5 "The only cost is the additional ten bytes...": And the key exchange.
> 
> 3.5 "The active opener MUST use the lowest value of 'i' that has not
> already appeared in a resumption suboption...": 'i' doesn't appear in
> the resumption suboption: SID[i] does.
> 
> 3.6 "Each chunk is placed in the 'data' portion of a frame's
> 'plaintext' value, which is then encrypted to yield the frame's
> 'ciphertext' field.": This language is confusing. I envision a "frame"
> as a segment of communication, not as an intermediate value in a
> computation, which is all the plaintext is in this case.
> 
> 3.6 "Chunks must be small enough that the ciphertext (slightly longer
> than the plaintext)...": Maybe "(as defined by the AEAD cipher used,
> but typically slightly longer than the plaintext)"?

Thanks, all the above points are addressed in the new draft.

> 3.6 "An 'associated data' value...": Is it worth looking at Damien
> Miller et al.'s construction for encrypted lengths?
> https://tools.ietf.org/html/draft-josefsson-ssh-chacha20-poly1305-openssh-00
>  Tcpcrypt is still in the draft stage, so it may be worth looking at
> this now to see if there's any value there.

Huh, yes that is an interesting possibility.  TCP segment
sizes will give quite a lot of info about how application
data is batched even if frame-lengths are encrypted, but
encrypting the lengths would certainly muddy the signal a
bit.  I think there was discussion of this for TLS; it would
be great to hear a summary of that if someone happens to
have followed it.

We'll look into what tradeoffs there might be
(extensibility, etc.) for tcpcrypt.

> 3.8 "more than one concurrent re-key operation": What does it mean for
> two re-key operations to be concurrent?

Ok, I've clarified that.

> 4.1 "The four-byte field 'message_len'": This is to accommodate really
> long keys (e.g., PQC)?

Perhaps: two bytes may not be enough.  Three is just
awkward.

> 4.1 Any particular reason for INIT1 to have both message_len and N_A?
> If the format changes in the future, INIT1_MAGIC could just be revved.

(Are you misinterpreting N_A as a length?  It is a nonce.)

> 4.2.1 "an integrity 'tag'": This is never defined.

Ok, I've changed the language to avoid introducing this
term.

> 4.2.2 Should the associated data of the first data frame include the
> INIT1 message? It's included in the PRK, but is key derivation the
> right place to put what amounts to authentication of the handshake
> transcript?

ENO's requirements for session IDs, which tcpcrypt fulfills,
make it infeasible to manipulate the transcript without
perturbing the session ID.
 
> 4.2.3 Magic numbers are specified here, but not in INIT1_MAGIC, which
> is in the lonely appendix. Inconsistent layout.
> 
> 4.2.3 Clear from this that a cipher MUST re-key at least every 2^64 bytes.

Thanks, I've addressed the above.

> Section 5 general comments: I wonder if this belongs in this document
> at all, or whether it should be moved to a separate API document.

For now, "API extensions" remains in the tcpcrypt document.
Discussion welcome.

> Section 6: "A tcpcrypt implementation MUST support at least the
> schemes ECDHE-P256 and ECDHE-P521": I wonder if, now that the safe
> curves stuff has moved through CFRG, whether we should MTI curve25519
> and curve448 instead.

I think the idea here was to avoid making mandatory anything
that doesn't have widespread implementations.  It looks like
X25519 support is in OpenSSL 1.1.0 (which is just now being
released?), but I don't see mention of X448.  Is there some
kind of IETF best-practice we can refer to on this?

> Section 9: I wonder whether these should be a la carte or suites:
> should ENO choose the combination of KX and AEAD cipher at the same
> time? One way, you potentially get combinatorial explosion, which
> might be a problem with limited ENO option space as algorithms get
> replaced; the other, you get weird combinations of algorithms as
> judged by security margin, and a more complicated handshake. I'd love
> to hear opinions on this.

The current document represents our best stab at this
tradeoff, so I guess we'd have to hear very specific and
well-motivated arguments to change it.

Thanks again for all the helpful input!

daniel