Subject: Re: [Curdle] Second AD Review: draft-ietf-curdle-ssh-curves.

From: Mark Baushke <[email-masked]>
Date: Sat, 3 Aug 2019 21:52:36 +0000

Hi Daniel,

I have updated and uploaded the draft. Please let me know of any issues.

        -- Mark

---

From: Ron Frederick <[email-masked]>
Date: Sun, 4 Aug 2019 09:51:51 -0700

Hi Mark,

> I have updated and uploaded the draft. Please let me know of any issues.

Here are a few thoughts on version 9 of the draft:

In section 3, you say “Key-agreement schemes Curve25519 and Curve448”, but these algorithms are not key-agreement schemes by themselves. I’d recommend going with the proposed SSH names here if you are discussing the full key agreement scheme and not the underlying elliptic curve algorithms. In other words, I’d suggest “Key-agreement schemes curve25519-sha256 and curve448-sha512” (possibly with quotes around each), as done earlier in this section.
When detecting the all-zeroes value, the new text says it should “abort if so, as described in Section 6” of RFC 7748. However, the original RFC doesn’t actually define what “abort” means. It says “abort if so (see below)”, but I don’t know what that is referring to. The only mention of “abort” is in sections 6.1 and 6.2, and nothing in section 7 seems to really expand on what should be done if the all-zeroes check matches. This is more a fault of RFC 7748 than it is of this doc, but since you are now recommending to “abort” in this doc, it would be good to be more specific about what that means for an SSH client/server.
I’m also not sure the sentence about “Alternative implementations” of these functions. There isn’t a lot of clarity on how one would determine in an alternate implementation when “either input forces the shared secret to one of a small set of values”. Since this point is covered already in RFC 7748 and this doc says “No further validation is required beyond what is discusses in RFC 7748”, though, I think it might be better to just leave this out.
-- 
Ron Frederick
[email-masked]

---

From: Benjamin Kaduk <[email-masked]>
Date: Mon, 12 Aug 2019 17:27:55 -0500

> In section 3, you say “Key-agreement schemes Curve25519 and Curve448”, but these algorithms are not key-agreement schemes by themselves. I’d recommend going with the proposed SSH names here if you…

That seems like a good suggestion, thanks.

> When detecting the all-zeroes value, the new text says it should “abort if so, as described in Section 6” of RFC 7748. However, the original RFC doesn’t actually define what “abort” means. It says…

My read of 7748 is that the "(see below)" refers to the following
paragraph, and is intended to expound on the all-zero check, not the act of
aborting.  Note that the second usage of "abort" does not have a "see
below" note, which leads one to suspect that the referenced content is
between the two mentions.  It's probably okay for 7748 to be fairly generic
about what "abort" means, since it's just defining the curves and not a
protocol using them; for our usage in ssh it may suffice to make a brief
reference to normal SSH error handling procedures.

> I’m also not sure the sentence about “Alternative implementations” of these functions. There isn’t a lot of clarity on how one would determine in an alternate implementation when “either input forc…

This is a recurring point of mild contention, whether to insist on a
specific algorithm for performing group operations or just describing the
group operations needed and the security considerations of using
implementation algorithms other than the one specified.  The mathematician
in me is fine with specifying the curves using just the abstract
mathematical description and noting that small-subgroup attacks exist, but
I recognize that there are protocol-design considerations in nailing the
procedure down tightly (at the cost of reducing implementation
flexibility).  While I'm pretty sure we have precedent in the IETF for both
general approaches, I'm inclined to accept the "proven language from
tcpcrypt" for now; there's always a chance the IETF LC and/or IESG will
want to chime in, too.

Given all that, I think it's okay to start the IETF LC, and we can wrap any
changes in response to these comments up along with any other IETF LC
comments.

-Ben

---

From: "Mark D. Baushke" <[email-masked]>
Date: Mon, 12 Aug 2019 22:05:23 -0700

Hi Ron & Ben,

Modulo the date stamp changes and the line break changes, here is what
the modified text looks like after trying to address the comments you
both made.

Let me know if I captured your intended changes correctly or not.

I can either upload a new version, or just roll these changes into the
next version after addressing any other comments during LC.

	Thanks,
	-- Mark

--- draft-ietf-curdle-ssh-curves-09.txt	2019-08-03 14:44:28.000000000 -0700
+++ draft-ietf-curdle-ssh-curves-10.txt	2019-08-12 21:55:24.000000000 -0700
@@ -133,21 +133,25 @@
    are generated as described therein.  Public keys are defined as
    strings of 32 bytes for Curve25519 and 56 bytes for Curve448.
 
-   Key-agreement schemes Curve25519 and Curve448 perform the Diffie-
-   Helman protocol using the functions X25519 and X448, respectively.
-   Implementations SHOULD compute these functions using the algorithms
-   described in [RFC7748].  When they do so, implementations MUST check
-   whether the computed Diffie-Hellman shared secret is the all-zero
-   value and abort if so, as described in Section 6 of [RFC7748].
-   Alternative implementations of these functions SHOULD abort when
-   either input forces the shared secret to one of a small set of
-   values, as discussed in Section 7 of [RFC7748].  Clients and servers
-   MUST fail the key exchange if the length of the received public keys
-   are not the expected lengths.  No further validation is required
-   beyond what is discussed in [RFC7748].  The derived shared secret is
-   32 bytes when Curve25519 is used and 56 bytes when Curve448 is used.
-   The encodings of all values are defined in [RFC7748].  The hash used
-   is SHA-256 for Curve25519 and SHA-512 for Curve448.
+   Key-agreement schemes "curve25519-sha256" and "curve448-sha512"
+   perform the Diffie-Helman protocol using the functions X25519 and
+   X448, respectively.  Implementations SHOULD compute these functions
+   using the algorithms described in [RFC7748].  When they do so,
+   implementations MUST check whether the computed Diffie-Hellman shared
+   secret is the all-zero value and abort if so, as described in
+   Section 6 of [RFC7748].  Alternative implementations of these
+   functions SHOULD abort when either input forces the shared secret to
+   one of a small set of values, as discussed in Section 7 of [RFC7748].
+   Clients and servers MUST fail the key exchange if the length of the
+   received public keys are not the expected lengths.  An abort for
+   these purposes is defined as a disconnect of the session with an
+   appropriate SSH "protocol error" for the fault provided to or by the
+   client.  No further validation is required beyond what is discussed
+   in [RFC7748].  The derived shared secret is 32 bytes when
+   "curve25519-sha256" is used and 56 bytes when "curve448-sha512" is
+   used.  The encodings of all values are defined in [RFC7748].  The
+   hash used is SHA-256 for "curve25519-sha256" and SHA-512 for
+   "curve448-sha512".
 
 3.1.  Shared Secret Encoding
 
@@ -158,9 +162,9 @@
 
    The shared secret, K, is defined in [RFC4253] and [RFC5656] as an
    integer encoded as a multiple precision integer (mpint).
-   Curve25519/448 outputs a binary string X, which is the 32 or 56 byte
-   point obtained by scalar multiplication of the other side's public
-   key and the local private key scalar.  The 32 or 56 bytes of X are
+   Curve25519/448 outputs a binary string X, which is the 32 or 56 byte
+   point obtained by scalar multiplication of the other side's public
+   key and the local private key scalar.  The 32 or 56 bytes of X are
    converted into K by interpreting the octets as an unsigned fixed-
    length integer encoded in network byte order.
 
@@ -215,8 +218,8 @@
    The way the derived binary secret string is encoded into a mpint
    before it is hashed (i.e., adding or removing zero-bytes for
    encoding) raises the potential for a side-channel attack which could
-   determine the length of what is hashed.  This would leak the most
-   significant bit of the derived secret, and/or allow detection of when
+   determine the length of what is hashed.  This would leak the most
+   significant bit of the derived secret, and/or allow detection of when
    the most significant bytes are zero.  For backwards compatibility
    reasons it was decided not to address this potential problem.

---

From: Benjamin Kaduk <[email-masked]>
Date: Wed, 14 Aug 2019 22:58:49 -0500

> Let me know if I captured your intended changes correctly or not.

Yes, that works for me.

> next version after addressing any other comments during LC.

Either way is fine; personally, I would wait until more comments come in.

Thanks!

-Ben