Re: [tcpinc] Eric Rescorla's Discuss on draft-ietf-tcpinc-tcpcrypt-09: (with DISCUSS and COMMENT)

Daniel B Giffin <dbg@scs.stanford.edu> Fri, 17 November 2017 12:17 UTC

Return-Path: <dbg@scs.stanford.edu>
X-Original-To: tcpinc@ietfa.amsl.com
Delivered-To: tcpinc@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 328DD127275; Fri, 17 Nov 2017 04:17:07 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.9
X-Spam-Level:
X-Spam-Status: No, score=-1.9 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, SPF_PASS=-0.001, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 4BSnNALKHeT9; Fri, 17 Nov 2017 04:17:04 -0800 (PST)
Received: from market.scs.stanford.edu (www.scs.stanford.edu [IPv6:2001:470:806d:1::9]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 5E155126CB6; Fri, 17 Nov 2017 04:17:04 -0800 (PST)
Received: from market.scs.stanford.edu (localhost [127.0.0.1]) by market.scs.stanford.edu (8.15.2/8.15.2) with ESMTP id vAHCH3hu034955; Fri, 17 Nov 2017 04:17:03 -0800 (PST)
Received: (from dbg@localhost) by market.scs.stanford.edu (8.15.2/8.15.2/Submit) id vAHCH3Pb094991; Fri, 17 Nov 2017 04:17:03 -0800 (PST)
Date: Fri, 17 Nov 2017 04:17:03 -0800
From: Daniel B Giffin <dbg@scs.stanford.edu>
To: Eric Rescorla <ekr@rtfm.com>
Cc: The IESG <iesg@ietf.org>, draft-ietf-tcpinc-tcpcrypt@ietf.org, Kyle Rose <krose@krose.org>, tcpinc-chairs@ietf.org, tcpinc@ietf.org
Message-ID: <20171117121703.GE57159@scs.stanford.edu>
References: <151036992713.398.18032326140786383584.idtracker@ietfa.amsl.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <151036992713.398.18032326140786383584.idtracker@ietfa.amsl.com>
Archived-At: <https://mailarchive.ietf.org/arch/msg/tcpinc/KYX68w0eE9OllOi_MymX44Z8uIk>
Subject: Re: [tcpinc] Eric Rescorla's Discuss on draft-ietf-tcpinc-tcpcrypt-09: (with DISCUSS and COMMENT)
X-BeenThere: tcpinc@ietf.org
X-Mailman-Version: 2.1.22
Precedence: list
List-Id: "Working group mailing list for TCP Increased Security \(tcpinc\)" <tcpinc.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/tcpinc>, <mailto:tcpinc-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/tcpinc/>
List-Post: <mailto:tcpinc@ietf.org>
List-Help: <mailto:tcpinc-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/tcpinc>, <mailto:tcpinc-request@ietf.org?subject=subscribe>
X-List-Received-Date: Fri, 17 Nov 2017 12:17:07 -0000

Thanks for the helpful review, Ekr!

Eric Rescorla wrote:
> ----------------------------------------------------------------------
> DISCUSS:
> ----------------------------------------------------------------------
> 
> https://mozphab-ietf.devsvcdev.mozaws.net/D3970
> 
>    2^64 bytes in the underlying TCP datastream (which would cause the
>    "offset" field to wrap) before re-keying.
> 
> In TLS and other WGs, we have adopted the practice of
> salting the nonce with a secret per-connection value to avoid
> large-scale surveillance attacks. Why did you opt to use a weaker
> construction. See:
> https://tlswg.github.io/tls13-spec/draft-ietf-tls-tls13.html#security-record-layer
> and https://eprint.iacr.org/2016/564.

Thanks again for pointing this out.  The next draft
implements a nonce-randomizing countermeasure.  I'll excerpt
the relevant changes.  First, what was before the frame
nonce is now called the frame ID:

   Lastly, a "frame ID" (used to construct the nonce for the AEAD
   algorithm) has this format:

                     byte
                        +------+------+------+------+
                      0 |       FRAME_ID_MAGIC      |
                        +------+------+------+------+
                      4 |                           |
                        +           offset          +
                      8 |                           |
                        +------+------+------+------+


Next, the traffic keys are extended by 12 bytes to provide
bits for nonce randomization.  This bit shows where the keys
come from:

   Given a session secret "ss[i]", the two sides compute a series of
   master keys as follows:

                 mk[0] = CPRF(ss[i], CONST_REKEY, K_LEN)
                 mk[j] = CPRF(mk[j-1], CONST_REKEY, K_LEN)

   The process of advancing through the series of master keys is
   described in Section 3.8.

   Finally, each master key "mk[j]" is used to generate traffic keys for
   protecting application data using authenticated encryption:

            k_ab[j] = CPRF(mk[j], CONST_KEY_A, ae_keylen + 12)
            k_ba[j] = CPRF(mk[j], CONST_KEY_B, ae_keylen + 12)

As before, "ae_keylen" depends on the negotiated AEAD
algorithm, e.g. 16 bytes for AES128-GCM.

The encryption operation is detailed in Section 3.6.  After
explaining the construction of the "plaintext" and
"associated data" values, it explains how the frame nonce is
formed:

   A "frame ID" value (see Section 4.2.3) is also constructed for the
   frame but not explicitly transmitted.  It contains an "offset" field
   whose integer value is the zero-indexed byte offset of the beginning
   of the current encryption frame in the underlying TCP datastream.
   (That is, the offset in the framing stream, not the plaintext
   application stream.)  Because it is strictly necessary for the
   security of the AEAD algorithms specified in this document, an
   implementation MUST NOT ever transmit distinct frames with the same
   frame ID value under the same encryption key.  In particular, a
   retransmitted TCP segment MUST contain the same payload bytes for the
   same TCP sequence numbers, and a host MUST NOT transmit more than
   2^64 bytes in the underlying TCP datastream (which would cause the
   "offset" field to wrap) before re-keying.

   With reference to the "AEAD Interface" described in Section 2 of
   [RFC5116], tcpcrypt invokes the AEAD algorithm with values taken from
   the traffic key "k_ab[j]" or "k_ba[j]" for some "j", according to the
   host's role as described in Section 3.3.

   First, the traffic key is divided into two parts:

        byte   0                        ae_keylen    ae_keylen + 11
               |                           |            |
               v                           v            v
             +----+----+--...--+----+----+----+--...--+----+
             |             K             |        NR       |
             +----+----+--...--+----+----+----+--...--+----+

             \_____________________________________________/
                              traffic key

   The first "ae_keylen" bytes of the traffic key provide the AEAD key
   "K", while the remaining 12 bytes provide a "nonce randomizer" value
   "NR".  The frame ID is then combined via bitwise exclusive-or with
   the nonce randomizer to yield "N", the AEAD nonce for the frame:

                            N = frame_ID xor NR

   The plaintext value serves as "P", and the associated data as "A".
   The output of the encryption operation, "C", is transmitted in the
   frame's "ciphertext" field.

This packaging of the key and nonce bits into one value is a
little bit awkward in the above excerpts (versus say the TLS
approach of doing two separate extractions to get the two
distinct values), but elsewhere in the spec it saves a lot
of complexity because the handling of a traffic key can be
explained without reference to the nonce randomizer.

Still, I'm open to other ways of describing this.

> 
>    FIN flag set, it MUST immediately send a frame (with empty
>    application data if necessary) with "rekey = 1".
> 
> I don't think that the algorithm in this section
> necessarily works properly, because you have to handle rekeys in
> sequence:
> 
> Frame 1 [0:999]
> Frame 2 [1000:1999, rekey=1]
> Frame 3 [2000:2999, rekey=1]
> 
> Now what happens if the frames are re-ordered so you get Frame 3 and
> then Frame 2. You will try to decrypt Frame 3 with generation 2 and
> Frame 2 with generation 3, neither of which will work (though you
> might be able to interpret the text loosely to have you try to decrypt
> Frame 2 with generation 2). Note that if you were to resequence before
> processing, this wouldn't happen.
> 
> At minimum I think some clarification is needed here.

As Kyle explained, it works because tcpcrypt is now
contained by the TCP datastream.  But you may be remembering
the good old days when it was simply a segment rewriter!

> Given that you are allowing P-256 and point reuse, you
> should be requiring point validation. See:
> https://tlswg.github.io/tls13-spec/draft-ietf-tls-tls13.html#rfc.section.4.2.8.2
> https://tlswg.github.io/tls13-spec/draft-ietf-tls-tls13.html#elliptic-curve-diffie-hellman

Yes, thank you.  For the NIST curves, I have added:

	Implementations MUST validate these "pubkey" values according to the
  algorithm in [ieee1363] Section A.16.10.

But I see that TLS refers to ANSI X9.62 for validation, even
though it refers to IEEE1363 for DH computation.  Is there a
good reason not to stick with the one source?

Also, I guess there's no need to check on-the-curve if we
allow only compressed format, and it's not perfectly clear
to me whether we need to check group membership, but I'd
really rather not have all these details in this document if
there's a good way to cite it out.

Lastly, I've added this to section 3.3:

   If a host receives an ephemeral public key from its peer and a
   required key-validation step fails (see Section 5), it MUST abort the
   connection and raise an error condition distinct from the end-of-file
   condition.
 
> You should probably also be requiring Curve25519 output validation.

I think you're saying we should check that the DH result is
not zero?

No harm, I suppose, but I'm going to try to get guidance on
whether it's necessary.

> You still seem to need to specify an MTI symmetric algorithm.

There's a table of the requirements in "6. AEAD Algorithms."
But you're not the only one who had trouble finding it, so I
wonder what's going on ...

> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
> 
> 
> The design of session resumption here essentially precludes doing
> tcpcrypt resumption across servers (as one does with TLS) because you
> need extremely tight control of ss[i] or you have catastrophic
> results. Was this a deliberate choice by the WG?

I think it's a choice to keep key material in the kernel and
to have control over forward secrecy.

> 
>    suboption containing the TEP identifier and "v = 0".  In order to
>    propose session resumption (described further below) with a
>    particular TEP, a host sends a variable-length suboption containing
> 
> It would be clearer if you explained resumption here.
> 
>           PRK = Extract(N_A, eno-transcript | Init1 | Init2 | ES)
> What is the rationale for providing N_A as the salt to HKDF-Extract, given that
> you also supply N_A in the Init1 message?

I'm not qualified to say.  I agree it seems "redundant", but
it appears to achieve what it needs to.  Perhaps we can find
a more satisfying answer for you ...

>    session resumption such that a given pre-session key "ss[0]" is only
>    used for either passive or active opens at the same host, not both.
> This seems like it probably needs some more emphasis, as failure to follow this
> instruction results in GCM compromise.

For our reference, here's the paragraph in question:

   A session secret may not be used to secure more than one TCP
   connection.  To prevent this, a host MUST NOT resume with a session
   secret if it has ever enabled encryption in the past with the same
   secret, in either role.  In the event that two hosts simultaneously
   send SYN segments to each other that propose resumption with the same
   session secret but the two segments are not part of a simultaneous
   open, both connections will have to revert to fresh key-exchange.  To
   avoid this limitation, implementations MAY choose to implement
   session resumption such that a given pre-session key "ss[0]" is only
   used for either passive or active opens at the same host, not both.

It seems like the straightforward implementation is to mark
a session secret "used" as soon as a resume succeeds.  The
only downside is that you could be forced to do fresh key
exchange in the case of bad timing (if you try to connect to
the host at the same time it connects to you), so the last
sentence here gives you a way to avoid that ever happening
(with I guess the downside of having to store two session
secrets for the same host).  Does it seem that this
paragraph is somehow risking a dangerous implementation?

> It would probably be useful to explain why you opted for this design rather
> than having nonces, which would remove the need for such strict ss[i]
> management. I assume the reason is that you want to save the round trip?

If I understand the question, I think the choice here is
mostly to prioritize forward secrecy.  Perhaps we can find a
way to make this more clear in Security Considerations.

>    connection; but if it aborts, the implementation MUST raise an error
>    condition distinct from the end-of-file condition.
> 
> Note that this interacts badly with the rekey issue I raise below.

It's not clear exactly what you mean, but perhaps this is
another concern that is moot because of the reliable-ordered
receipt of frames?  In any case (and with relevance to some
other discussion in this thread), due to other comments we
have improved the guidance on what to do in the case of
encryption failure:

                     [...] The output of this operation is either a
   plaintext value "P" or the special symbol FAIL.  In the latter case,
   the implementation SHOULD abort the connection and raise an error
   condition distinct from the end-of-file condition.  But if none of
   the TCP segment(s) containing the frame have been acknowledged and
   retransmission could potentially result in a valid frame, an
   implementation MAY instead drop these segments.

> 
>    format", and MUST accept values encoded in "compressed",
>    "uncompressed" or "hybrid" formats.
> 
> Why are you allowing multiple formats here? Generally, if you're going to
> encourage compressed, you want to just require it.

That makes sense.  I'm changing this to:

   Implementations MUST encode these "pubkey" values in "compressed
   format".

Thanks again for all the useful comments.

daniel