Re: [Curdle] WGLC draft-ietf-curdle-ssh-kex-sha2

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

Benjamin Kaduk <[e-mail]> writes:

> Hi Mark,
> 
> On Tue, Feb 02, 2021 at 03:13:05PM -0800, Mark D. Baushke wrote:
> > Hi all,
> >
...elided...

> There were a few things that I wanted to mention separately, though:

...elided...
> The table in the IANA considerations disagrees about the implementation
> recommendation for gss-group16-sha512-* with the table in Section 3.3.2
> (SHOULD vs MAY) -- your previous messages to the list suggest that MAY
> is intended, and I mostly assume that the SHOULD crept in during the
> rewrite for the -12, when making the non-GSS group16 method 'SHOULD'.

Yes. MAY is what I intended. The SHOULD is probably from doing a quick
replace for diffie-hellman-group16-sha512 which is marked SHOULD. I think
sticking with MAY is okay here, so I will update it in the -14 edition.

> There's also a line in Section 3.2.3 (ecdh-*, ecmqv-sha2, gss-nistp*)
> that says "this set of methods MAY be implemented" when the tables and
> other prose say "SHOULD" about ecdh-sh2-nistp*.  The "MAY" would line up
> with ecmqv-sha2, that otherwise is not mentioned in this section other
> than the table, but ecmqv-sha2 is not really a "set" of methods.
> I'm not sure what the intent was for that text (maybe it's just stale?);
> my current proposal is to just remove it, but that doesn't do anything
> to address the lack of prose about ecmqv-sha2 in this section.  (It might
> also have been referring to the general set other than the specific named
> curves.)

It was indeed stale text and will be removed in -14.

> I also went through the references, with these comments:
> 
> Section 8.1
> 
> RFC 8031 is just about curve25519/curve448 for IKEv2, and I don't see a
> need for them to be normative references for us -- 8731 is (AFAIK) the
> relevant SSH reference.

Agreed. I will move 8031 to informative and 8731 to normative.

> Section 8.2
> 
> We may get some people asking for RFCs 4419, 4432, 4462, 5656, 8731, and
> 8732 to be moved to normative references.  In my opinion it's debatable,
> so I'm happy to leave them as-is for now -- they're all standards track,
> so there would not be a process issue if we had to move them after the
> IETF LC.

I seem to recall getting feedback from someone that they should be
informative.

> I'll include the editorial notes below for visibility and in case they
> help Mark decipher my proposed changes, but I don't expect anything
> noteworthy that's not (also) mentioned above.

Thanks.

> Thanks,
> 
> Ben
> 
> %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
> %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
> 
> Section 1.1
> 
> > At present, the attacks against SHA-1 are collision attacks that
> > usually rely on human help rather than a pre-image attack.
> 
> nit: I think a comma before "rather than" would help.

Will be fixed in -14.

> > Transcript Collision attacks are documented in [TRANS-COLL]. This
> > paper shows that the man in the middle does not tamper with the
> > Diffie-Hellman values and does not know the connection keys. However,
> > it manages to tamper with both Ic and Is, [...]
> 
> nit: it looks like RFC 4253 writes these as "I_C" and "I_S" (though
> maybe native subscripts are better yet).

Will be fixed in -14.

> Section 1.2.1
> 
> > For ECC, it is recommended to select one with approximately 128 bits
> > of security strength.
> 
> nit: s/one/a curve/

Will be fixed in -14.

> Section 1.2.2
> 
> > For FFC, a modulus 2048 bits (112 bits of security strength).
> 
> editorial: it's not clear what this not-quite-sentence is trying to say.
> If we wanted parallelism with §1.2.1, it would be "it is recommended to
> use a modulus with 2048 bits (112 bits of security strength)", but since
> after the table we go and list an explicit minimum of 2048-bit MODP
> group14 it's not entirely clear that that's the intent.

The -14 draft will use

            For FFC, it is recommended to use a modulus with 2048 bits
            (112 bits of security strength).
 
> Section 1.2.3
> 
> > The only IFC algorithm for key exchange is the RSA algorithm via
> > [RFC4432].
> 
> nit: s/via/specified in/

Will be fixed in -14.

> Section 3.1
> 
> > diffie-hellman-group1-sha1 and diffie-hellman-group14-sha1 are
> > currently mandatory to implement (MTI). [...]
> 
> I'd suggest avoiding "currently" (due to the ambiguity of whether it
> applies to the state of affairs before or after the publication of this
> document, when the reader will be reading it after publication) and
> favoring "prior to the changes made by this document, [...] were
> mandatory to implement".

Will be fixed in -14 to use "Prior to the changes made by this document,"
at the start of the paragraph.
 
> > SHA-1 has security concerns provided in [RFC6194].
> 
> nit: s/provided/documented/ or something like that

Will be fixed in -14.

> Section 3.2.1
> 
> > These key exchange methods are described in [RFC8731] and [RFC8732]
> > and is similar to the IKEv2 Key Agreement described in [RFC8031].
> 
> nit: singular/plural mismatch "are described"/"is similar".

Will be fixed in -14.

> Section 3.2.2
> 
> > This Key Exchange Method is described in [RFC8731] and is similar
> > [...]
> 
> nit: since we added gss-curve448-sha512-* to this section we have to go
> to "methods are described" and "are similar" just as in §3.2.1.

Ack.

> > This method MAY be implemented.
> 
> nit: may also need to reword this to just cover curve448-sha512.

Ack.

> Section 3.2.3
> 
> > At present, there are three named curves in this name-space which
> > SHOULD be supported. They appear in [RFC5656] in section 10.1 Required
> > Curves all of the NISTP curves named are mandatory to implement if any
> > of this RFC is implemented.
> 
> nit: "at present" may not age as well as "at the time of this writing".

Will be fixed in -14.

> nit: I think the intent was maybe to format this as """section 10.1
> ("Required Curves").  All of the NISTP curves named therein are
> mandatory to implement if any of that RFC is implemented".

Will be fixed in -14.

> > This set of methods MAY be implemented.
> 
> Is this sentence stale text?  AFAICT it refers to the same
> ecdh-sha2-nistp* that are SHOULD elsewhere in the section.
> But if it referred to ecmqv-sha2 then the "MAY" would make more sense.

The intent was ecmqv-sha2 would be MAY. Also, any OID-based curved MAY
be implemented. This section may need better editing to get the point
across.

> > The GSS-API name-space with gss-nistp*-sha* mirrors the algorithms
> > used by ecdh-sha2-* names. The table provides guidance for
> > implementation.
> 
> This would be a good place to put a reference to RFC 8732.

Agreed. Will be fixed in -14.

> Section 3.3.1
> 
> > [RFC8270] mandates implementations avoid any MODP group
> 
> nit: "mandates that"

Will be fixed in -14.

> Section 3.3.2
> 
> > diffie-hellman-group14-sha256 method MUST be implemented.
> 
> nit: "The".

Will be fixed in -14.

> Section 3.4
> 
> > Uses an RSA 2048-bit modulus with a SHA2-256 hash.
> 
> nit: "It uses"

Will be fixed in -14.

> Section 6
> 
> > It is desirable to deprecate or remove key exchange method name that
> > are considered weak. A key exchange method may be weak because too few
> > bits are used, or the hashing algorithm is considered too weak.
> 
> nit: this is not an exhaustive list, so we could tweak the wording.
> 

Good point. I guess your suggestion of ", or for other reasons" like

        It is desirable to deprecate or remove key exchange method name
        that are considered weak. A key exchange method may be weak
        because too few bits are used, or the hashing algorithm is
        considered too weak, or for other reasons.

at the end of the paragraph is one way to address this.

Other suggestions from the list greatfully considered.

	Be safe, stay healthy,
	-- Mark