Subject: [sasl] AD review comments for draft-ietf-sasl-gs2

From: <[email-masked]>
Date: Mon, 26 Oct 2009 14:05:43 +0100

I've now done my AD review for draft-ietf-sasl-gs2-17. Basically,
it looks good, so I've asked the secretariat to start IETF Last Call.

I do have couple of minor comments that can be considered as the
first last call comments:

- I have two questions about Section 8. First, if the GS2 mechanism
doesn't e.g. support confidentiality, wouldn't setting conf_req_flag
cause things to fail? (so it's not really "irrelevant")

- Second, should Section 8 say something about the flags that are not
related to per-message tokens? (deleg, mutual, anon)

- Section 4 should say either that character case (for things like
"p=" and "a=") must be exactly as shown here, or that they're case
insensitive (if nothing is said, RFC 5234 strings are by default case
insensitive, I think).

- In Section 5, I would suggest adding parenthesis to the "gs2-cb-flag"
('gs2-cbind-flag  = ("p=" cb-name) / "n" / "y"') to make it easier
to parse for the reader..

Best regards,
Pasi

---

From: Nicolas Williams <[email-masked]>
Date: Mon, 26 Oct 2009 11:00:45 -0500

> cause things to fail? (so it's not really "irrelevant")

No because callers of GSS_Init_sec_context() are supposed to check the
ret_flags output argument to ensure that they got what they asked for
(they can get less and more than what they asked for).  Asking for more
than the mechanism can provide does not cause sec context establishment
failure.

> related to per-message tokens? (deleg, mutual, anon)

Hmmm.  Good point.  I think we should REQUIRE that the initiator request
mutual authentication, and that if channel bindings are used then
ret_flags must indicate mutual authentication else GS2 must fail.

But as for anon and deleg, I think those are details of the individual
SASL framework and the GS2 plugin implementations.  I see nothing wrong
with an implementation that provides a SASL interface for requesting
credential delegation / receiving delegated credentials.  Similarly for
anonymity.

> insensitive (if nothing is said, RFC 5234 strings are by default case

Yes, RFC5234, on page 5, says that if you want case-senstive matching
then you must write rules like "rulename = %d97 %d98 %d99" instead of
"rulename = "abc"".  Very annoying, that.

> to parse for the reader..

I agree.

Nico
--

---

From: Simon Josefsson <[email-masked]>
Date: Mon, 26 Oct 2009 17:54:19 +0100

> it looks good, so I've asked the secretariat to start IETF Last Call.

Great!

> cause things to fail? (so it's not really "irrelevant")

I believe discussion indicates no action is required.

> related to per-message tokens? (deleg, mutual, anon)

I'm not sure what to add, but will add text if suggested.

> insensitive (if nothing is said, RFC 5234 strings are by default case

I added this paragraph before the ABNF:

	  <t>The figure below describes the permissible attributes,
	    their use, and the format of their values.  All attribute
	    names are single US-ASCII letters and are
	    case-sensitive.</t>

> to parse for the reader..

Agreed.

Live document available from:
http://josefsson.org/sasl-gs2/draft-ietf-sasl-gs2.txt

Comparison against -17 is also available:
http://josefsson.org/sasl-gs2/draft-ietf-sasl-gs2-from--17.diff.html

Thanks,
/Simon

---

From: Nicolas Williams <[email-masked]>
Date: Mon, 26 Oct 2009 12:01:19 -0500

> I'm not sure what to add, but will add text if suggested.

I think we should add something like this:

   The mutual_req_flag MUST be set.  If channel binding is used then the
   client MUST check that the corresponding ret_flag is set when the
   context is fully establish, else authentication MUST fail.

The first requirement is important, while the rest is, IMO, redundant --
channel binding necessarily implies mutual authentication in the
Kerberos sense.  Note that we don't require checking that mutual
authentication actually happened.

SCRAM, incidentally, provides mutual authentication in the Kerberos
sense.  We should state that in the SCRAM doc (oops; but we can still
make that change).  SCRAM does not provide mutual authen. in the sense
that both, the client and server have names that are each authenticated
to the other (SCRAM has no notion of server naming).

And we should add something like this too:

   Use or non-use of deleg_req_flag and anon_req_flag is an
   implementation-specific detail.  SASL and GS2 implementors are
   encouraged to provide programming interfaces by which clients may
   choose to delegate credentials and by which servers may receive them.
   SASL and GS2 implementors are encouraged to provide programming
   interfaces which provide a good mapping of GSS-API naming options.

> 	    names are single US-ASCII letters and are

For the record, I don't mind if they're really case-insensitive.

Nico
--

---

From: Alexey Melnikov <[email-masked]>
Date: Tue, 27 Oct 2009 13:02:11 +0000

> On Mon, Oct 26, 2009 at 02:05:43PM +0100, [email-masked] wrote:
 [...]
> ret_flags must indicate mutual authentication else GS2 must fail.
+1.
> credential delegation / receiving delegated credentials.  Similarly for
+1.
> "rulename = "abc"".  Very annoying, that.
Yes, it is annoying.
I think the intent was to have all attribute names to be case-sensitive.
>> to parse for the reader..
Fine with me.

---

From: Simon Josefsson <[email-masked]>
Date: Thu, 29 Oct 2009 15:31:58 +0100

> authentication actually happened.

Seems fine to me, added in my local copy.

>    interfaces which provide a good mapping of GSS-API naming options.

Added too.

> For the record, I don't mind if they're really case-insensitive.

I would prefer to keep SCRAM and GS2 in sync here.

Thanks,
/Simon