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

[Simo Sorce <[e-mail]>]

Answers inline.

On Wed, 2019-05-08 at 10:06 -0500, 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.

See Hubert's question

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

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" ?

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

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