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

[Benjamin Kaduk <[e-mail]>]

On Sun, Aug 04, 2019 at 09:51:51AM -0700, Ron Frederick wrote:
> Hi Mark,
> 
> On Aug 3, 2019, at 2:52 PM, Mark Baushke <mdb=[e-mail]> wrote:
> > 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.

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

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

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