Re: [Curdle] Genart last call review of draft-ietf-curdle-ssh-curves-09

["Mark D. Baushke" <[e-mail]>]

Hi Christer,

Christer Holmberg via Datatracker <[e-mail]> writes:

> Reviewer: Christer Holmberg
> Review result: Ready with Nits
>
> I am the assigned Gen-ART reviewer for this draft. The General Area
> Review Team (Gen-ART) reviews all IETF documents being processed
> by the IESG for the IETF Chair.  Please treat these comments just
> like any other last call comments.

Okay.

> For more information, please see the FAQ at
>
> https://trac.ietf.org/trac/gen/wiki/GenArtfaq
>
> Document: draft-ietf-curdle-ssh-curves-09
> Reviewer: Christer Holmberg
> Review Date: 2019-08-22
> IETF LC End Date: 2019-08-26
> IESG Telechat date: Not scheduled for a telechat
>
> Summary:
>
> The document is almost ready for publication. I have no technical issues, but
> there are some issues in Section 1 that I'd like to authors to address.
>
> Major issues: N/A
>
> Minor issues: N/A
>
> 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.

I understand.

Only occurance of "as discussed in" is found in this text:

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

Section 7 of RFC7748 is the Security Considerations section and is a
discussion of the elements which are potentially a security issue during
implementation. This draft provides a SHOULD recommendation to abort the
connection when one of these raised security considerations is found.

That said, I am okay with changing the text to

    "Alternative implementations of these functions SHOULD abort when
    either input forces the shared secret to one of a small set of
    values, as described in Section 7 of [RFC7748]."

The only other time the word discussed is used is in

    No further validation is required beyond what is discussed in <xref
    target="RFC7748"/>.

which may be replaced with

    No further validation is required beyond what is described in <xref
    target="RFC7748"/>.

I was unable to find any "as defined in" statements. I found declarative
statements "is defined in" "are defined in" "is defined as" which were
references to normative protocol elements.

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.

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

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

> Q1_2:
>
> - I don't think you should use “we” terminology (“we describe”, “we chose”,
> etc..). Please talk about the document, and if you want to refer to a choice
> made by the WG please indicate that.

Fixed.

> Q1_3:
>
> - Instead of “currently”, I suggest to say something like “at the time of
> publication”. Because, the meaning of “currently” changes every second :)

Fixed.

> 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
        "[e-mail]", 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.

Would you rather that I use 'This document defines ...' here and in the
abstract rather than 'This document describes ...' ? Please advise.


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

> - Also, is there anything preventing unforeseen analytical advances
> against Curve448?

There is nothing preventing any unforeseen analytical advances against
any of the current public key implementations. This is why
Post-Quantum-Cryptographic (PQC) is such a host topic today.

However, the same tools which may be able to exploit EcDSA may not be as
quickly pointed at these curves. Cryptographic agility is the key here.

Let me know if you which to have me remove the entire paragraph or not.

        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. Due
        to different implementation status of these two curves
        (high-quality free implementations of Curve25519 has
        been in deployed use for several years, while Curve448
        implementations are slowly appearing), it is accepted
        that adoption of Curve448 will be slower.

Should I upload my updated draft-ietf-curdle-ssh-curves-10.xml or
do you have additional suggestions?

	Thank you,
	-- Mark