Hi Mark,
Please see inline. I have removed the comments where your suggested solution is fine and I have nothing further to say.
Nits/editorial comments:
General:
---------
QGEN_1:
>> - The document uses “as discussed in”, “as defined in”, “as described”
>> in terminology. It might be justified to use different terminology in
>> some cases, but in general I suggest trying to use consistent terminology.
…
>The use of "as described in" is found in the standard boilerplate
>copyright notice and the boilerplate Requirements language sections.
>So, I have migrated from "discussed" to described" for consistentcy.
That's good.
Section 1:
-----------
Q1_1:
>> The text says:
>>
>> ”[RFC5656] describes how elliptic curves are integrated in SSH, and this document reuses those protocol messages.”
>>
>> …and:
>>
>> “This document describes how to implement key exchange based on Curve25519 and Curve448 [RFC7748] in SSH.”
>>
>> - It is unclear to me what “integrated in SSH” means.
>
> The title of RFC 5656 is "Elliptic Curve Algorithm Integration in the Secure Shell Transport Layer" and this Draft intents to augment the
> Elliptic Curve methods defined there.
>
>>
>> Does it mean that RFC 5656 describes the generic procedures for performing SSH key exchanges using elliptic curves, or does it also
>> cover other things?
>
> RFC5656 covers three specific constructions:
>
> a) Elliptic Curve Diffie-Hellman (ECDH),>
> b) Elliptic Curve Menezes-Qu-Vanstone (ECMQV) key agreement, and
> c) Elliptic Curve Digital Signature Algorithm (ECDSA).
>
> This draft does not cover the use of a digital signature algoirthm or apply the Curve25519 or Curve448 constructions to the use of ECMQV and
> focuses entirely on ECDH key exchange extensions for a different construction of elliptic curves.
Would it be good to indicate that in a note?
>> - I think the “this document reuses those protocol messages” sounds a
>> little confusing because I don't know what “those protocol message”
>>refers to. Perhaps say something like “reuses the protocol messages
>> defined in that specification”.
>
> This draft resuses the SSH_MSG_KEX_ECDH_INIT and SSH_MSG_KEX_ECDH_REPLY
> messages which apply to the ECDH construction as in in section 4 of this draft.
>
> Does this paragraph work as a better replacement for the first paragraph
> of the introduction? Or, is it too detailed for a summary introduction?
>
> Secure Shell (SSH) <xref target="RFC4251"/> is a secure remote
> login protocol. The key exchange protocol described in <xref
> target="RFC4253"/> supports an extensible set of methods. <xref
> target="RFC5656"/> defines how elliptic curves are integrated
> into this extensible SSH framework, and this document reuses the
> Elliptic Curve Diffie-Hellman (ECDH) key exchange protocol
> messages defined in section 7.1 "ECDH Message Numbers" <xref
> target="RFC5656"/>.
>
> It seems a bit detailed to me for an introduction. Let me know if you
> have any suggestions on a revision.
I think the text looks good, and it is for sure a good clarification for a non-security person like myself :)
---
Q1_4:
>> The text says:
>>
>> “The Curve448 key exchange method is novel but similar in spirit,”
>>
>> - I don't know what this means, since there is now further explanation.
>
> The paragraph now reads:
>
>This document describes how to implement key exchange based on
>Curve25519 and Curve448 <xref target="RFC7748"/> in SSH. For
>Curve25519 with SHA-256 <xref target="RFC6234"/>, the algorithm
>described is equivalent to the privately defined algorithm
>"curve25519-sha256@xxxxxxxxxx", which at the time of publication
> was implemented and widely deployed in libssh and OpenSSH. The
> Curve448 key exchange method is similar but uses SHA-512 <xref
> target="RFC6234"/> to further separate it from the Curve25519
> alternative.
Looks good.
> Would you rather that I use 'This document defines ...' here and in the
> abstract rather than 'This document describes ...' ? Please advise.
Personally I would use "describe", but my main issue is being consistent - no matter what word is used :)
---
Q1_5:
>> The text says:
>>
>>
>> “This document provide Curve25519 as the preferred choice, but
>> suggests that the fall back option Curve448 is implemented to provide
>> an hedge against unforeseen analytical advances against Curve25519
>> and SHA-256.”
>>
>> - Is the only reason why one should implement Curve448 that something
>> MAY happen to Curve25519 in the future?
>
>No, the Curve448 also has a stronger cryptographic security strength. If
>it becomes a requirement to use a minimum of 128 bits of security
>strength, then Curve25519 may be rejected by some and thus the need to
> provide for something stronger.
Wouldn't it be enough to say that, instead of talking about unforeseen analytical advances etc? I noted that the sec-dir reviewer had some comments on that too.
Please see below for suggested modified text.
> Let me know if you which to have me remove the entire paragraph or not.
I think you could keep the paragraph, but instead say something like:
“This document provide Curve25519 as the preferred choice, but
suggests that the Curve448 is implemented in order to provide
128 bits of security strength, should that become a requirement.
At the time of writing this specification high-quality free implementations
of Curve25519 had been in deployed use for several years, while Curve448
implementations were slowly appearing, so it was accepted that adoption
of Curve448 would be slower."
> Should I upload my updated draft-ietf-curdle-ssh-curves-10.xml or
> do you have additional suggestions?
I haven't seen the update draft yet, but if you are ok with my suggestions you can go ahead to upload. If you are not ok with my suggestions, then let me know :)
Regards,
Christer
