Subject: [kitten] AD Review: draft-ietf-kitten-rfc5653bis-04

From: Eric Rescorla <[email-masked]>
Date: Sat, 17 Jun 2017 11:23:10 -0700

This document seems generally sound. There are some things about this
API that confused/surprised me and seem perhaps unwise, but given that
this is a bis, I will mostly confine my review to mostly calling them
out and asking you to make sure I understand and that the document is
clear.

OVERALL
1. What is a fatal error?
The document describes exceptions as indicating "fatal errors".
What does this mean for the state of the context. For instance,
if I receive an exception from initSecContext(), does that mean
that it is no longer possible to initiate it? Your example code
seems to treat them as fatal for the context. What happens
if I try to use a context after such an event?

2. How do I enforce properties for received messages?
I see that I can request services for context initialization
(requestConf), and that I can check whether a given message
was encrypted (getPrivacy) but it's not clear to me if this
causes the API to enforce these rules for tokens that I
receive. Is that possible or do I just need to check?

3. Are the request* flags() hard limits? E.g., if I do
requestMutualAuth() do I get it or fail?

4. It's a little unusual to have a structure where you keep
calling initSecContext or acceptContext() repeatedly. In
most APIs you would do like "setRole(Server)" or
"setRoleClient(), and then "Handshake().

DETAIL
S 6.1.16.
Can addProviderAtFront() be used to add new providers which
the API would not normally use at all?

S 6.4.9.
   Successful completion of this call does not guarantee that wrap will
   be able to protect a message of the computed length, since this
   ability may depend on the availability of system resources at the
   time that wrap is called.  However, if the implementation itself
   imposes an upper limit on the length of messages that may be
   processed by wrap, the implementation should not return a value that
   is greater than this length.

This should seems pretty weak. This isn't a hard limit?

S 6.4.10.
   Instance of MessageProp that is used by the
   application to set the desired QOP and privacy
   state.  Set the desired QOP to 0 to request the
   default QOP.  Upon return from this method, this
   object will contain the actual privacy state that
   was applied to the message by the underlying
   mechanism.

Just to be clear: if you ask for a specific QOP you always get it
(or failure). This checking thing is only if you use 0?

It would also be helpful to point out that QOP is mech-specific
as noted in RFC 2743.

S 6.4.21.
What does requestInteg() mean? As far as I can tell the only thing
you can do with a context is wrap() or getMIC(), both of which involve
integrity. So what happens if you set it false?

S 6.5.7.
Does "privacy" here mean "confidentiality"? Can you clarify.

EDITORIAL
S 3.3.
Please do not use the phrase "cryptographic checksum", I recognize
that the terminology in this document is idiosyncratic because of
age, but this isn't really a modern term. Typically
we would now use "authentication tag" or "integrity tag"

S 4.12.3.
So an exception is thrown for an invalid token?

S 5.3.
gss_release_cred() is just eager, right? In any case the data will
be cleaned up on GC? In any case you should make this clear.

S 6.1.15.
I wouldn't say you are "creating a previously exported context". You
are either importing it or creating a new context from a previously
exported one.

S 6.2.1.

   // export and re-import the name
   byte[] exportName = mechName.export();

   // create a new name object from the exported buffer
   GSSName newName = mgr.createName(exportName,
                     GSSName.NT_EXPORT_NAME);

This comment structure is confusing, because the first is just
the export. I would change that.

S 6.2.6.
It's a bit unclear to me under what circumstances you can compare GSS
names. I see you can do .equals() and export/memcmp, but can you
compare strings? Perhaps after you canonicalize them?

S 6.3.9.
Does "union over all mechanisms" mean that if mechanism A supports
INITIATE and B supports ACCEPT you get "INITIATE_AND_ACCEPT"

S 6.4.X
The presentation order here is weird because you show initSecContext()
in the 6.4.1. example but then define it in 6.4.3 and then show
another example that's reduced in 6.4.4. Perhaps you can consolidate
these?

-Ekr

---

From: Benjamin Kaduk <[email-masked]>
Date: Sun, 18 Jun 2017 22:41:52 -0500

Hi Ekr,

[authors: some questions remain, inline]

> out and asking you to make sure I understand and that the document is

There are some bits of the GSSAPI that are strange, surprising, and
confusing, yes ... I hear rumors that there are ideas for a GSSAPIv3
floating around, though nothing concrete yet.

> 1. What is a fatal error?

Fatal errors are specified as a specific list of GSS-API major
status codes in RFC 2743, table 1 (things like GSS_S_BAD_MIC,
GSS_S_FAILURE, GSS_S_UNAUTHORIZED, etc.), as opposed to informatory
status codes like GSS_S_COMPLETE, GSS_S_CONTINUE_NEEDED,
GSS_S_DUPLICATE_TOKEN (which is, confusingly, a fatal error during
context establishment), etc.

> if I try to use a context after such an event?

I think this is also something specified at the GSS level, though
RFC 2743 is unfortunately obscure about it.  In the
context-negotiating routines (GSS_Init_sec_context() and
GSS_Accept_sec_context()), successful return codes are
GSS_S_COMPLETE and GSS_S_CONTINUE_NEEDED, and other return codes (at
the abstract GSS level) abort the context negotiation.  This is
buried in the descriptions for those two functions, though hopefully
I made it a bit more clear in RFC 7546.  (Once a context is
established, then more robust error handling is available and a
context can remain useful after a given message-handling function
returns an error/throws an exception.)  The Java bindings handle the
GSS_S_COMPLETE/GSS_S_CONTINUE_NEEDED distinction differently than
the C bindings do (with init/acceptSecContext returning the token to
send to the peer, if any, and an isEstablished() function to query
whether the local side is complete), but any other GSS-level result
from the context-establishment functions is a GSS-level error, which
presents as a java exception.  Subsequent calls on the failed context are
expected to also throw exceptions (something like the abstract GSS
API GSS_S_NO_CONTEXT).

> receive. Is that possible or do I just need to check?

I think you just need to check.
The GSSAPI is in general a request-and-check sort of affair, and
even when confidentiality protection is enabled on a given context,
a given message could be wrapped without confidentility protection.
In the abstract API, GSS_Unwrap() has an explicit conf_state output
value; the msgProp stuff is essentially specific to the Java
bindings.

> requestMutualAuth() do I get it or fail?

They are not hard limits; this is just how the GSS-API works.
You request things but have to check whether you got them.

> "setRoleClient(), and then "Handshake().

Yes.
The designs floating around for a GSSAPIv3 would most likely have
such a common Handshake() API, but for v2 (and v1) we are stuck with
separtae Init and Accept roles.

> DETAIL

Now we get into areas about which I am less certain; I'll loop in
the authors alias to get a confirmation/correction.

> the API would not normally use at all?

It seems likely.  In the C bindings we generally have mechanisms
globally enabled, and site-local customizations go in
/etc/gss/mechs.d .

> This should seems pretty weak. This isn't a hard limit?

This is telling you what the ciphertext expansion is, but in reverse
-- you put in the ciphertext size and receive back a plaintext
input length that would produce that much ciphertext.

> (or failure). This checking thing is only if you use 0?

I believe so.  GSS QOPs are for practical purposes deprecated and
everyone uses 0.

> It would also be helpful to point out that QOP is mech-specific

Sure.  (Maybe also that their use is disrecommended or something
like that.)

> integrity. So what happens if you set it false?

If you set it to false, then some (hypothetical?) mechanism that
only provides authentication could be used to establish a security
context.  There are also some other GSS functions like
GSS_Pseudo_random() (RFC 4401) that could potentially be used on a
security context that does not require per-message protections,
though I don't know of Java bindings for that one, at least.

Authentication-only is a valid workflow even just limited to this
self-contained spec, using the security context negotiation to
establish a context, query the peer's name with getSrcName(), and
make authorization decisions based on that name.

One might imagine someone trying to shoehorn OAuth2 into a GSS
mechanism using something like that.

> Does "privacy" here mean "confidentiality"? Can you clarify.

That's the only interpretation I can come up with that makes sense,
but let's check with the authors.

> So an exception is thrown for an invalid token?

I think so.

> be cleaned up on GC? In any case you should make this clear.

I think so, though there is an implicit recommendation to destroy
sensitive crypto material immediately after use.

> are either importing it or creating a new context from a previously

"importing" would be more consistent with the language used by the C
bindings.

> compare strings? Perhaps after you canonicalize them?

You have stumbled upon one of the worst warts on the GSS-API ;)

It is confusing no matter whether you look at the abstract spec or
language bindings.  When you first create a name you end up with a
generic "internal name", and certain operations can cause that to be
transformed into a "mechanism name" that contains additional
mechanism-specific information.  The offically recommended way to
compare GSS names is to GSS_Export_name() and use memcmp(), noting
that you must GSS_Canonicalize_name() between GSS_Import_name() and
GSS_Export_name().

>From RFC 2743:

   Note that the results obtained by using GSS_Compare_name() will in
   general be different from those obtained by invoking
   GSS_Canonicalize_name() and GSS_Export_name(), and then comparing the
   exported names.  The first series of operations determines whether
   two (unauthenticated) names identify the same principal; the second
   whether a particular mechanism would authenticate them as the same
   principal.  These two operations will in general give the same
   results only for MNs.

Sadly, lots of applications use GSS_Display_name() and strcmp(),
which is something of a security vulnerability waiting to happen.

I'm not entirely sure that I've answered your question, though.

> INITIATE and B supports ACCEPT you get "INITIATE_AND_ACCEPT"

Yes.

Again quoting RFC 2743:

   GSS_Inquire_cred() should indicate INITIATE-AND-ACCEPT for
   "cred_usage" if both of the following conditions hold:

      (1) there exists in the credential an element which allows context
      initiation using some mechanism

      (2) there exists in the credential an element which allows context
      acceptance using some mechanism (allowably, but not necessarily,
      one of the same mechanism(s) qualifying for (1)).

> another example that's reduced in 6.4.4. Perhaps you can consolidate

I'll leave that for the authors.

-Ben

---

From: Weijun Wang <[email-masked]>
Date: Mon, 19 Jun 2017 15:52:37 +0800

Hi Ekr,

Thanks for the review.

Below I keep questions that I can answer. For other general GSS-API 
questions, I could not answer better than Ben. Thanks a lot, Ben.

Yes.

A Java security provider advertises what service(s) it provides (for 
GSS-API, GssApiMechanism). If someone adds a provider that does not 
contains this service, it will be ignored.

Yes.

>> we would now use "authentication tag" or "integrity tag"

We can use "integrity tag".

>> So an exception is thrown for an invalid token?

Correct, if the input token is not well-formed or was not correctly 
signed/encrypted by the peer, an exception will be thrown.

> sensitive crypto material immediately after use.

Well...

Java has a mechanism to provide a callback method called finalize() and 
hope GC will call it, and Oracle's SunNativeGSS provider (as a bridge to 
a native GSS-API lib) does provide one to release the native cred handle 
explicitly. That said, everyone is saying finalize() is unreliable and 
it usage is already deprecated.

Even if finalize() is reliable, whether it should dispose the cred is 
not documented and up to the implementation.

Yes.

>> the export. I would change that.

Correct.

> I'm not entirely sure that I've answered your question, though.

Oracle's Java implementation looks like this:

1. If they are both canonicalized, compare the canonicalized mech 
element inside.

2. If only one is canonicalized, try to canonicalize the other one using 
this one's mechanism, and compare the result.

3. Otherwise, canonicalize both to Kerberos 5 and compare the result.

By comparing 2 canonicalized mech elements, I mean comparing the type 
and the string or byte array name, which is equivalent to comparing the 
exported form.

>        one of the same mechanism(s) qualifying for (1)).

Same as in Java.

> I'll leave that for the authors.

Yes, it's confused.

Basically, 6.4.4 and 6.4.6 are meant to demonstrate initSecContext() and 
acceptSecContext(), and 6.4.1 is meant to demonstrate all methods in 
GSSContext (here, from the initiator's perspective).

For this bis, I am OK with just removing 6.4.4 and 6.4.6. The examples 
shown here are only of the most common usage and can be read again in S 7.

Or, we can indent 6.4.4 and 6.4.6 (plus 6.1.17 and 6.1.19) one level 
deeper to become 6.4.3.1 etc and/or rename the title to "initSecContext 
Example code" etc.

Or maybe the order looks weird because every class has an example as the 
first sub-section (see 6.1.1, 6.2.1, and 6.3.1) before talking about 
what this class is about. If this is the major reason, we can move these 
examples (6.1.1, 6.2.1, 6.3.1, and 6.4.1) to the end of each section. 
But then we will also need to indent/rename 6.1.17 and 6.1.19 to avoid 
two "Example code" in a row.

Thanks,
Weijun