Subject: Re: [Curdle] new-AD review comments on draft-ietf-curdle-gss-keyex-sha2-08

From: Benjamin Kaduk <[email-masked]>
Date: Fri, 7 Jun 2019 17:35:15 -0500

Sorry for the slow reply -- it's been a busy month in my personal
situation.

On Thu, May 09, 2019 at 12:34:40PM +0200, Hubert Kario wrote:
> On Wednesday, 8 May 2019 17:06:04 CEST Benjamin Kaduk wrote:
> > Hi all,
> > 
> > I'm almost ready to send draft-ietf-curdle-gss-keyex-sha2 to the IESG for
> > evaluation, but would like to see a new revision posted first, mostly for
> > the Abstract changes.  In addition to the changes mentioned below, please
> > also go ahead and make the editorial changes mentioned in the last
> > call/directorate reviews.
> > 
> > -Ben
> > 
> > Abstract
> > 
> >    This document specifies additions and amendments to RFC4462.  It
> >    defines a new key exchange method that uses SHA-2 for integrity and
> >    deprecates weak DH groups.  The purpose of this specification is to
> > 
> > I think we need to remove this statement about "deprecates weak DH groups";
> > I didn't see any further discussion of such deprecations in any revision of
> > the document.
> 
> 
> we've done that by not listing them, would you prefer to list them explicitly 
> in section 6 and Section 4 with a "MUST NOT" in "Implementation Support" and 
> "Implementation Recommendations"?

Since we are only Update:ing 4462 and not Obsolete:ing it, we do need to
explicitly say what is deprecated.  The sections you list seem like fine
places (note that since the IANA registry doesn't list the "implementation
support" column, it seems that our request would just be to add this
document as a reference for the affected entries).

-Ben

> > Section 2
> > 
> >                                                            Following the
> >    rationale of [RFC8268] only SHA-256 and SHA-512 hashes are used for
> >    DH groups.  For NIST curves the same curve-to-hashing algorithm
> >    pairing used in [RFC5656] is adopted for consistency.
> > 
> > 8268 doesn't provide a whole lot of rationale for using SHA-256, rather,
> > the closest thing to guidance would be in Section 1 where it cites the NSA
> > IAD FAQ that wants to *avoid* using SHA-256.  So perhaps it's better to say
> > that this just follows the practice of 8268, rather than the rationale from
> > it, especially since we are just using a "consistency" argument for the
> > NIST curves' hashes.
> 
> yes, "practice" would work better in this context
> 
> -- 
> Regards,
> Hubert Kario
> Senior Quality Engineer, QE BaseOS Security team
> Web: www.cz.redhat.com
> Red Hat Czech s.r.o., Purkyňova 115, 612 00  Brno, Czech Republic

---

From: Benjamin Kaduk <[email-masked]>
Date: Fri, 7 Jun 2019 17:36:59 -0500

On Fri, May 10, 2019 at 10:02:36AM -0400, Simo Sorce wrote:
> Answers inline.
> 
> On Wed, 2019-05-08 at 10:06 -0500, Benjamin Kaduk wrote:
> > Hi all,
> > 
> See Hubert's question
> Fixed, replaced "rationale" with "practice"
> 
> > Section 3
> > 
> > RFC 8174 has an updated boilerplate text to use for the BCP 14 keywords.
> 
> Fixed
> 
> > Section 4
> > 
> >                The method name for each method is the concatenation of
> >    the family method name with the Base64 encoding of the MD5 hash
> >    [RFC1321] of the ASN.1 DER encoding [ISO-IEC-8825-1] of the
> >    underlying GSS-API mechanism's OID.  [...]
> > 
> > nit: I'd suggest using the "family name prefix" to match the table, rather
> > than "family method name".
> 
> Fixed
> 
> > Section 5
> > 
> >    In [RFC5656] new SSH key exchange algorithms based on Elliptic Curve
> >    Cryptography are introduced.  We reuse much of section 4 to define
> >    GSS-API-authenticated ECDH Key Exchanges.
> > 
> > nit: I think this is intended to refer to Section 4 of RFC 5656, but the
> > current text reads like we are referring to Section 4 of the current
> > document.
> 
> Fixed
> 
> > Section 5.1
> > 
> >    For NIST Curves keys use uncompressed point representation and must
> > 
> > nit: "the"
> > 
> >    be converted using the algorithm in Section 2.3.4 of [SEC1v2].  If
> >    the conversion fails or the point is trasmitted using compressed
> > 
> > nit: "the"
> 
> both nits Fixed
> 
> >    representation, the key exchange MUST fail.
> > 
> >    A GSS Context is established according to Section 4 of [RFC5656]; The
> >    client initiates the establishment using GSS_Init_sec_context() and
> >    the server completes it using GSS_Accept_sec_context().  For the
> > 
> > I'm not sure what this reference was intended to be; the current one
> > doesn't really seem right, though.
> > 
> > Also, in a fully general GSS negotiation loop, the server does not
> > necessarily make the last step, so "completes" may be slightly misleading.
> 
> Would it be ok to replace "completes" with "responds to" ?

I think so.  (And sorry for nitpicking so much on this; you have the
misfortune of having an AD who is a bit of a subject matter expert.)

> >                                                               This key
> >    exchange process will exchange only a single token once the context
> >    has been established, therefore the replay_det_req_flag and
> >    sequence_req_flag SHOULD be set to "false".
> > 
> > nit: "single message token", since we are not making a statement about
> > context establishment tokens, and the previous text was about the context
> > establishment process which might cause the reader to jump towards context
> > tokens here.
> 
> I am not sure what you are suggesting/requesting here, can you clarify?

I'm suggesting s/single token/single message token/, to reiterate that
these are not context tokens.

> >                  For curve25519 and curve448 the algorithm in Section 6
> >    of [RFC7748] is used instead.
> > 
> > nits: "algorithms", "are" (since there are different procedures in
> > subsections 6.1 and 6.2 for the two curves).
> 
> Fixed
> 
> > Section 6
> > 
> > It's not clear to me why the "Implementation Support" column is here -- it
> > duplicates content from tables elsewhere in the document, and does not seem
> > to be part of the registration template.
> 
> Ok, I'll drop the column.
> 
> > Section 7.3
> > 
> >    Some GSSAPI mechanisms can optionally delegate credentials to the
> >    target host by setting the deleg_ret_flag.  In this case extra care
> >    must be taken to ensure that the acceptor being authenticated matches
> >    the target the user intended.  [...]
> > 
> > This is a bit weird to read, since the setting of the deleg_ret_flag is not
> > what effectuates the delegation of credentials; that happens by virtue of
> > the exchange of context tokens.  (Also, RFC 2743 only lists a
> > deleg_req_flag; the acceptor-side state is passed in the deleg_state
> > boolean.)  A simple replacement with deleg_req_flag doesn't work, either,
> > as that's not something the mechanism does, but rather the GSSAPI consumer.
> > So I think we may need a broader change, like:
> > 
> > % Some GSSAPI mechanisms can act on a request to delegate credentials to
> > % the target host when the deleg_req_flag is set.  In this case, extra care
> > % must be taken to ensure that the acceptor being authenticated matches the
> > % target the user intended.  [...]
> > 
> > (And yes, I know that I'm basically responsible for this text being
> > present; it may well have been my error originally.)
> 
> I like your wording better too, and nice catch on the flag name typo.
> 
> Once I get answers on the remaining open questions I will include the
> results of the discussion in the document[1] and issue a new draft.

Thanks, and sorry again for the slow response!

-Ben

> Thanks,
> Simo.
> 
> [1] https://github.com/simo5/ietf/blob/master/draft-ietf-curdle-gss-keyex-sha2.xml
> 
> -- 
> Simo Sorce
> Sr. Principal Software Engineer
> Red Hat, Inc
> 
>