Re: [secdir] secdir review of draft-ietf-sasl-gs2-17

[Simon Josefsson <[e-mail]>]

"Glen Zorn" <[e-mail]> writes:

> I have reviewed this document as part of the security directorate's 
> ongoing effort to review all IETF documents being processed by the 
> IESG.  These comments were written primarily for the benefit of the 
> security area directors.  Document editors and WG chairs should treat 
> these comments just like any other last call comments.

Thanks for your review Glen.  I am now merging your comments into the
document.

> GENERAL COMMENTS
> The reference to a document is not the document; for example, in the
> following excerpt from section 2, there are no names defined in the
> reference to RFC 2743:
>    The document uses many terms and function names defined in [RFC2743]
>    as updated by [RFC5554].
> Please change to something like
>    The document uses many terms and function names defined in RFC 2743
> [RFC2743]
>    as updated by RFC 5554 [RFC5554].
> Note that this comment applies globally to all such usages in the document.

I agree with Alexey, Pasi and Nico on this and we'll let the RFC Editor
handle this.

> ABSTRACT
> The last sentence of the Abstract says: "See
> <http://josefsson.org/sasl-gs2-*/> for more information."  Unless the
> referenced URL is intended to be permanently available (and probably even
> then), this line should be removed before publication as an RFC.

I have removed the link..  It was only to aid during development.

> SECTION 3
> Third paragraph, first sentence.
> Old:
>    If the GSS_Inquire_SASLname_for_mech interface is not used, the GS2
>    implementation need some other mechanism to map mechanism OIDs to
>    SASL name internally.  In this case, the implementation can only
> New:
>    If the GSS_Inquire_SASLname_for_mech interface is not used, the GS2
>    implementation needs some other mechanism to map mechanism OIDs to
>    SASL names internally.  In this case, the implementation can only

Fixed.

> SECTION 3.1
> s/characters outside of the base32 alphabet/characters outside of the Base32
> alphabet/

Fixed.

> SECTION 3.2
> s/This obliterate the need/This obliterates the need/

It now reads 'This eliminates the need'.

> The final sentence seems slightly clumsy; suggest the following change:
> Old:
>    The computed mechanism name can be used
>    directly in the implementation, and the implementation need not
>    concern itself with that the mechanism is part of a mechanism family.
> New:
>    The computed mechanism name can be used
>    directly in the implementation, and the implementation need not
>    be concerned if the mechanism is part of a mechanism family.

Fixed.

> SECTION 4
> In the fourth paragraph, s/As this indicate an error condition/As this
> indicates an error condition/

Fixed.

> In paragraph 5, "The [RFC2743] section 3.1 initial context token header" has
> the sound of GSSAPI "code words" ;-).  Suggest changing to "The GSSAPIv2
> initial context token header (see section 3.1 of RFC 2743 [RFC2743])" or
> some such.

I agree with Nico's reply, so I avoided mentioning GSS*API at all here
and used this text:

    The initial context token header (see section 3.1 of [RFC2743]) MUST
    be removed if present.

> A similar comment pertains to the first paragraph on page 9; suggest:
> Old:
>    When the "gs2-nonstd-flag" flag is present, the client did not find/
>    remove a [RFC2743] section 3.1 token header from the initial token
>    returned by GSS_Init_sec_context.  This signals to the server that it
>    MUST NOT re-add the data that is normally removed by the client.
> New:
>    When the "gs2-nonstd-flag" flag is present, the client did not find/
>    remove a GSSAPIv2 token header (RFC 2743, section 3.1) from the initial
> token
>    returned by GSS_Init_sec_context.  This signals to the server that it
>    MUST NOT re-add the data that is normally removed by the client.

As before, I wanted to avoid using the term GSS*API here.  Instead the
text reads:

   When the "gs2-nonstd-flag" flag is present, the client did not find/
   remove a token header ([RFC2743] section 3.1) from the initial token
   returned by GSS_Init_sec_context.

> s/The NUL characters is forbidden/The NUL character is forbidden/

Fixed.

> s/Upon the receipt/Upon receipt/

Fixed.

> s/results in failed authentication exchange/results in a failed
> authentication exchange/

Fixed.

> SECTION 5
> I find this section rather difficult to understand: not all of the possible
> combinations of gs2-cb-flag and server support for channel bindings seem to
> be covered.  A table might help, if not for that the gs2-cb-flag is
> tri-valued & used to signal two different things.

I agree with Nico that I believe we cover all the cases in the text.
Can you point to which case is not covered?  However, for illustration,
I have added Nico's table.

> SECTION 5.1
> First sentence s/The calls to GSS_Init_sec_context and
> GSS_Accept_sec_context takes a/The calls to GSS_Init_sec_context and
> GSS_Accept_sec_context take a/

Fixed.

> SECTION 6
> This section needs a lot of work if it is expected to be understood by
> non-members of the Illuminati ;->.  Just one example (of many possible):
>
>    Example #3: a two round-trip GSS-API context token exchange, no
>    channel binding, no standard token header.
>
>          C: Request authentication exchange
>          S: Empty Challenge
>          C: F,n,,<initial context token without
>                              standard header>
>          S: Send reply context token as is
>          C: Send reply context token as is
>          S: Send reply context token as is
>          C: Empty message
>          S: Outcome of authentication exchange
>
> What does this mean?  What do 'F' & 'n' stand for?  How is it useful?  It
> might be claimed that these questions could be answered by dissecting the
> ABNF for the gs2 header, but I don't think that's a good answer: examples
> should be self-contained.

I agree with Nico that we shouldn't explain this again in the example
section: the meaning of the protocol messages are explained elsewhere in
the document already.

We could use complete verbatim example exchanges though, but they will
be somewhat large due to the unrelated GSS-API tokens which aren't GS2
specific.

> SECTION 8
>
> The first paragraph says:
>
>    GS2 does not use any GSS-API per-message tokens.  Therefore the
>    setting of req_flags related to per-message tokens is irrelevant.
>
> OK, but what should the client and server behavior should be WRT the flags?

It now reads:

   GS2 does not use any GSS-API per-message tokens.  Therefore the per-
   message token ret_flags from GSS_Init_sec_context() and
   GSS_Accept_sec_context() are irrelevant; implementations SHOULD NOT
   set the per-message req_flags.

> SECTION 14.1
> Old:
>    If a client implement GSS-API mechanism X, potentially negotiated
>    through a GSS-API mechanism Y, and the server also implement GSS-API
>    mechanism X negotiated through a GSS-API mechanism Z, the
>    authentication negotiation will fail.
> New:
>    If a client implements GSS-API mechanism X, potentially negotiated
>    through a GSS-API mechanism Y, and the server also implements GSS-API
>    mechanism X negotiated through a GSS-API mechanism Z, the
>    authentication negotiation will fail.

Fixed.

> SECTION 14.2
>
> s/use of GSS-API mechanisms that negotiate other mechanisms are/use of
> GSS-API mechanisms that negotiate other mechanisms is/

Fixed.

Thanks again,
/Simon