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

From: "Glen Zorn" <[email-masked]>
Date: Fri, 20 Nov 2009 14:39:59 -0500

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.

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.

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.

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

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

SECTION 3.2
s/This obliterate the need/This obliterates 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.

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

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.

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.

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

s/Upon the receipt/Upon receipt/

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

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.

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/

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.

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?

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.

SECTION 14.2

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

---

From: Nicolas Williams <[email-masked]>
Date: Mon, 23 Nov 2009 10:52:34 -0600

> Note that this comment applies globally to all such usages in the document.

I'm with Alexey on this.  The RFC Editor might even re-write this by
replacing the reference with the full title and reference.

(Personally I very much dislike unnecessary redundancy, as in "defined
in RFC 2743 [RFC2743]", but whatever style the RFC Editor thinks is
appropriate and consistent with current RFC Editor practice is what we
should go with.

> then), this line should be removed before publication as an RFC.

I think that's clearly not intended to remain in the final RFC, but to
help reviewers understand the progression.  I agree that this is best
placed in a changelog-type section, but at this point it does not
matter.

> initial context token header (see section 3.1 of RFC 2743 [RFC2743])" or

My attitude is that different technologies tend to have different
terminology, and we can't forever be explaining it when we have
documents that deal with more than one technology.  In this case the
reader is expected to, and indeed must have some familiarity with both,
SASL and the GSS-API.  I'm not sure how we could avoid that.

The text in qquestion is very specific as to where you'll find the
definition of "initial context token header": RFC2743, section 3.1.
That should suffice; it's certainly better than writing a lengthy
exposition on what a context token is, what an initial context token is,
what an "initial context token header" is, and why it's there.

(Also, FYI it's GSS-API, or GSS-API v2, update 1 -- not "GSSAPIv2".  One
of the many _annoying_ terminology bridging problems of the past is that
the original SASL/GSS-API bridge wasn't named properly.  As a result
"GSS-API" means one thing, and "GSSAPI" (without the hyphen) can mean
another, and you have to know the context in order to tell which it is.
Better to always be careful to include the hyphen.  This is really,
really annoying.  I hope this document will put that problem to bed for
good once GS2 is widely adopted..)

> tri-valued & used to signal two different things.

I believe we handled all cases.  There are three flag values and two
server support alternatives.  I suppose we can add a table, something
like:

    FLAG	SERVER CB SUPPORT	DISPOSITION
    ----	-----------------	-----------

    n		Irrelevant		If server disallows non-channel-
                                        bound authentication, then fail

    y		CB not supported	Authentication may succeed

    y		CB supported		Authentication must fail

    p		CB supported		Authentication may succeed, with
                                        CB used

    p		CB not supported	Authentication will fail

    <none>	CB not supported	Client does not even try because
                                        it insists on CB

Note the use of 'may', 'must' and 'will' in the above.

"Authentication must fail" means that the server must fail
authentication.  "Authentication will fail" means that the GS2 message
exchange will proceed but authentication will necessarily fail as a
result of CB inputs differing on the client and server side.

> should be self-contained.

I don't agree.  Example protocol exchanges need to be simplified --
we're trying show what the flows look like, not to be exacting in
detail.  The more explanation the example needs, the less clear it
obviously is.  In the example above the "<initial context token
withoutstandard header>" makes it clear, if you've read the preceding
sections, that the "F,n,," part is the GS2 header.  We could add quotes
to the F,n,, part, if you think that will help.

> OK, but what should the client and server behavior should be WRT the flags?

There's no actual behavior w.r.t. req_flags and ret_flags.  The GSS-API
mechanism is free to enable per-msg token features not requested by the
caller, but they are truly irrelevant: the application won't be using
per-msg tokens.  It may make things simpler if we explicitly require
that req_flags not be set:

   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.

Thanks!

Nico
--

---

From: Alexey Melnikov <[email-masked]>
Date: Mon, 23 Nov 2009 21:46:00 +0000

>   set the per-message req_flags.
I agree this would be an improvement.