Re: [sasl] GS2 AUTH48 nits

[Simon Josefsson <[e-mail]>]

Nicolas Williams <[e-mail]> writes:

> On Thu, Jun 17, 2010 at 03:50:54PM +0200, Simon Josefsson wrote:
>> We are close to get GS2 out of AUTH48 (although still waiting for its
>> dependencies...) but I noticed two things I'd like to fix.  Because this
>> is slightly more than editorial (but hopefully not controversial) I
>> wanted to run this by the WG.
>> 
>> 1)
>> 
>> In section 3 about mechanism names it discuss what an implementation do
>> when it cannot derive the SASL name.  It says:
>> 
>>    If the GSS_Inquire_SASLname_for_mech interface is not used, the GS2
>>    implementation needs some other mechanism to map mechanism Object
>>    Identifiers (OIDs) to SASL names internally.  In this case, the
>>    implementation can only support the mechanisms for which it knows the
>>    SASL name.  If the GSS_Inquire_SASLname_for_mech call fails, and the
>>    GS2 implementation cannot map the OID to a SASL mechanism name using
>>    some other means; it cannot use the particular GSS-API mechanism
>>    since it does not know its SASL mechanism name.
>> 
>> I'd like to change "cannot use" into "MUST NOT use" to make sure
>> implementations never attempts to use such a GSS-API mechanism under its
>> hash-derived mechanism name.
>
> ASIDE: GSS_Inquire_SASLname_for_mech() can fail?  (The only error code
> we list that it can return is GSS_S_BAD_MECH, meaning the OID passed in
> is not supported by the mechglue.)

I suppose that is a problem with the document, isn't it?

Looking at RFC 2743, it seems all GSS-API interfaces are documented to
return GSS_S_FAILURE:

   o  GSS_S_FAILURE indicates that the requested operation failed for
   reasons unspecified at the GSS-API level.

Each language binding may define other error situations, so failures are
always _possible_.  For example, C defines calling errors, other
bindings may throw exceptions.

Looking at GNU GSS, it will return GSS_S_FAILURE on out of memory
conditions.

I suggest we add GSS_S_FAILURE error codes to the new GSS-API functions.
Do you agree?  I can propose something separately.

>> OLD:
>>    some other means; it cannot use the particular GSS-API mechanism
>> 
>> OLD:
>>    some other means; it MUST NOT use the particular GSS-API mechanism
>
> I don't think the use of indicative versu normative is a big deal here.
> But the punctuation in this sentence needs help.  I'd remove the comma
> and replace the semi-colon with ", then" and clarify the antecedent of
> "it".
>
> How about:
>
> OLD:
>                If the GSS_Inquire_SASLname_for_mech call fails, and the
>    GS2 implementation cannot map the OID to a SASL mechanism name using
>    some other means; it cannot use the particular GSS-API mechanism
>    since it does not know its SASL mechanism name.
>
> NEW:
>                If GSS_Inquire_SASLname_for_mech() fails and the GS2
>    implementation cannot map the OID to a SASL mechanism name via some
>    other means, then the GS2 implementation cannot use the given GSS-API
>    mechanism.

That is just an editorial change, right?  I don't see any significant
implementation difference between those two paragraphs.

I do see an implementation difference if the "cannot" is replaced by
"MUST NOT": it becomes a testable condition on which to validate
implementations.

Thus, I would like the see something like this:

NEW:
                If GSS_Inquire_SASLname_for_mech() fails and the GS2
    implementation cannot map the OID to a SASL mechanism name via some
    other means, then the GS2 implementation MUST NOT use the given GSS-API
    mechanism.

Otherwise we'll end up in the trap that GS2 implementations break the
"cannot" because, well, the "cannot" is simply not true: implementations
CAN use the GSS-API mechanisms in GS2 under its hash-generated name (or
some random name).  The document does not say that applications
shouldn't use the hash-generated name in this situation.  It is implied,
but a "MUST NOT" would make this clear.

> It could be simplified further:
>
>                If the GS2 implementation uses a GSS-API implementation
>    and GSS_Inquire_SASLname_for_mech() fails then the GS2 implementation
>    cannot use the given GSS-API mechanism as a SASL mechanism.

No -- unfortunately widely used GSS-API libraries still don't have the
GSS_Inquire_SASLname_for_mech function, and it will take a long time
unless it is widely available.

GNU SASL maps the KRB5 OID to "GS2-KRB5" internally when used with an
pre-GS2 GSS-API library, and that has to be permitted.  The GSS-API
interface is just a helper function, it is not required for GS2 to work.

> Also, presumably many GS2 implementations will apply
> GSS_Inquire_SASLname_for_mech() to the OIDs indicated by
> GSS_Indicate_mechs() and then support all the resulting SASL mechanism
> names.  We should have some advice for them, no?  After all, not every
> GSS-API mechanism is suitable for use in SASL via GS2 (must have channel
> binding, must have mutual auth).  But, whatever.  It's getting late and
> I don't care for perfection :)

I noticed that too, but I didn't want to bring it up. :-)

The problem is mitigated since the mutual_req flag MUST be set, and that
it is checked to be true after the context has established.  I don't
know how to query mechanisms whether they support channel bindings?

GNU SASL does not support auto-discovery of unknown GSS-API mechanisms,
intentionally and for several reasons.  I suspect some reasons will be
shared by other implementations as well, thus I'm not sure even the
majority of implementations will support auto-discovery.

If this turns out to be a problem in practice, it can be solved by later
clarifications.

>> 2)
>> 
>> The introduction section contains:
>> 
>>    All GS2 plaintext is protected via the use of GSS-API channel
>>    binding.
>> 
>> This is actually not true anymore, since we discovered that this wasn't
>> possible to implement (see earlier discussions about optional "F,").
>
> The "F," is a compression technique for _GSS-API_ plaintext that is not
> explicitly protected by the GSS-API (the mechanism can and should, but
> doesn't have to).  As such I'd not worry about "F," going unprotected.

Me neither, but it would be useful to talk about it in the document.  An
attacker can insert and remove the "F," without being detected (unless
channel binding is used).  We need to be sure that this only leads to a
Denial-Of-Service situation, and not anything worse.

>> I propose to change this into:
>> 
>>    GS2 plaintext is protected via the use of GSS-API channel binding.
>
> Removing "All" doesn't really help.  I'd rather say that "all" or "most"
> "security-relevant" GS2 plaintext is protected.
>
> I propose instead:
>
> OLD:
>    All GS2 plaintext is protected via the use of GSS-API channel
>    binding.
>
> NEW:
>    Most GS2 plaintext is protected via the use of GSS-API channel
>    binding, with the exception of the "gs2-nonstd-flag" flag.
>
>    GS2 does not protect any plaintext exchanged outside GS2, such as
>    SASL mechanism negotiation plaintext, nor application messages
>    following authentication.  But using channel binding to a secure
>    channel over which all SASL and application plaintext is sent will
>    cause all that plaintext to be authenticated.

This text is in the introduction section...  but your paragraph is quite
informative!  I suggest:

OLD:
   GS2 is designed to be as simple as possible.  It adds to GSS-API
   security context token exchanges only the bare minimum to support
   SASL semantics and negotiation of use of channel binding.
   Specifically, GS2 adds a small header (a few bytes plus the length of
   the client-requested SASL authorization identity) to the initial GSS-
   API context token and to the application channel binding data.  GS2
   uses SASL mechanism negotiation to implement channel binding
   negotiation.  All GS2 plaintext is protected via the use of GSS-API
   channel binding.  Additionally, to simplify the implementation of GS2
   mechanisms for implementors who will not implement a GSS-API
   framework, we compress the initial security context token header
   required by [RFC2743], Section 3.1.

NEW:
   GS2 is designed to be as simple as possible.  It adds to GSS-API
   security context token exchanges only the bare minimum to support
   SASL semantics and negotiation of use of channel binding.
   Specifically, GS2 adds a small header (a few bytes plus the length of
   the client-requested SASL authorization identity) to the initial GSS-
   API context token and to the application channel binding data.  GS2
   uses SASL mechanism negotiation to implement channel binding
   negotiation.  Security-relevant GS2 plaintext is protected via the
   use of GSS-API channel binding.  Additionally, to simplify the
   implementation of GS2 mechanisms for implementors who will not
   implement a GSS-API framework, we compress the initial security
   context token header required by [RFC2743], Section 3.1.

   GS2 does not protect any plaintext exchanged outside GS2, such as
   SASL mechanism negotiation plaintext, nor application messages
   following authentication.  But using channel binding to a secure
   channel over which all SASL and application plaintext is sent will
   cause all that plaintext to be authenticated.

I.e., I've changed "All" to "Security-relevant", and added your last
paragraph.

>> However I believe this prompts a security discussion about non-integrity
>> protected parts.  I suggest adding this paragraph to the security
>> consideration:
>> 
>> 	    <t>Some data that influence the GS2 authentication
>> 	      algorithm is not integrity protected by the GSS-API
>> 	      channel binding mechanism.  This includes the SASL
>> 	      mechanism negotiation (which influences channel binding
>> 	      selection) and the initial [gs2-nonstd-flag ","] part of
>> 	      the GS2 header.  The former problem is discussed in the
>
> I don't think we need to restate the SASL security considerations
> regarding mechanism negotiation, but if we must then we should do so in
> the fourth paragraph of the security considerations section, since that
> one talks about this problem.
>
> In any case, the SASL mechanism negotiation does not constitute "GS2
> plaintext".
>
>> 	      core SASL specification generally.  GS2 confirms the
>> 	      channel binding selection through the GS2 header, which
>> 	      is integrity protected.  The initial part of the GS2
>> 	      header is not integrity protected but this should not
>> 	      allow attackers to influence the negotiation
>> 	      substantially: it merely allows attackers to remove/add
>> 	      a fixed prefix to the initial context token, and it is
>> 	      assumed that this will be detected by any GSS-API
>> 	      mechanism.</t>
>
> I'd rather not touch the security considerations section, except for
> some typos (see below), but if we must then I propose this change
> instead of yours:
>
> OLD:
>    Use of channel binding will also protect the SASL mechanism
>    negotiation -- if there is no MITM, then the external secure channel
>    will have protected the SASL mechanism negotiation.
>
> NEW:
>    Use of channel binding will also protect the SASL mechanism
>    negotiation -- if there is no MITM, then the external secure channel
>    will have protected the SASL mechanism negotiation.  When channel
>    binding is not used there is no protection for the SASL mechanism
>    negotiation; section 5 explains how to protect against some downgrade
>    attacks, and, as stated earlier, it is strongly RECOMMENDED that
>    channel binding be used.

This doesn't address what an attacker can do by inserting/removing "F,"
though.

However maybe this is just too much detail, and it does not really
affect implementations (it only improves security analysis of the
document).  So I'll withdraw my suggestion.  Let's not bias the folks
who do security analysis of the protocol and assert that this is
secure. ;)

> Typos:
>
>    GS2 does not protect against downgrade attacks of channel binding
>    types.  The complexities of negotiation a channel binding type, and
>                                ^^^^^^^^^^^
> 			       negotiating
>    handling downgrade attacks in that negotiation, was intentionally
>                                                    ^^^
> 						   were
>    left out of scope for this document.
>
> Also, "the complexities .. were .. left out of scope" is an odd phrase.
> This would be much better:
>
>    Negotiation of channel binding type was intentionally left out of
>    scope for this document.

I agree.  I've made this change in my local copy, which means I'll
notice it when doing final AUTH48 checks.

/Simon