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

From: Weijun Wang <[email-masked]>
Date: Fri, 7 Jul 2017 16:15:36 +0800

Hi Ekr

I’m about to write another version of this I-D. Except for the rearrangement of example codes in S 6, do you have any other particular parts I need to update?

Thanks
Max

> On Jun 19, 2017, at 3:52 PM, Weijun Wang <[email-masked]> wrote:
> 
> 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.
> 
> On 06/19/2017 11:41 AM, Benjamin Kaduk wrote:
>> On Sat, Jun 17, 2017 at 11:23:10AM -0700, Eric Rescorla wrote:
>>> S 6.1.16.
>>> Can addProviderAtFront() be used to add new providers which
>>> 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 .
> 
> 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.
> 
>>> S 6.5.7.
>>> 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.
> 
> Yes.
> 
>>> 
>>> 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"
> 
> We can use "integrity tag".
> 
>>> 
>>> S 4.12.3.
>>> So an exception is thrown for an invalid token?
>> I think so.
> 
> Correct, if the input token is not well-formed or was not correctly signed/encrypted by the peer, an exception will be thrown.
> 
>>> 
>>> 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.
>> I think so, though there is an implicit recommendation to destroy
>> 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.
> 
>>> 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.
>> "importing" would be more consistent with the language used by the C
>> bindings.
> 
> Yes.
> 
>>> 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.
> 
> Correct.
> 
>>> 
>>> 
>>> 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?
>> 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.
> 
> 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.
> 
>>> 
>>> 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"
>> 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)).
> 
> Same as in Java.
> 
>>> 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?
>> 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
> 
>> -Ben

---

From: Eric Rescorla <[email-masked]>
Date: Sat, 8 Jul 2017 11:42:54 -0700

Well, anything where I had a question probably needs some clarifying text.

-Ekr

On Fri, Jul 7, 2017 at 1:15 AM, Weijun Wang <[email-masked]> wrote:

> Hi Ekr
>
> I’m about to write another version of this I-D. Except for the
> rearrangement of example codes in S 6, do you have any other particular
> parts I need to update?
>
> Thanks
> Max
>
> > 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.
> >
> > 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.
> >
> >>> S 6.5.7.
> > Correct, if the input token is not well-formed or was not correctly
> signed/encrypted by the peer, an exception will be thrown.
> >
> >>>
> >>> S 5.3.
> > 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.
> >
> >>> S 6.1.15.
> > 2. If only one is canonicalized, try to canonicalize the other one using
> this one's mechanism, 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.
> >
> >>>
> >>> S 6.3.9.
> > 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
> >
> >> -Ben
>
>

---

From: Weijun Wang <[email-masked]>
Date: Thu, 13 Jul 2017 11:41:59 +0800

Hi Ekr

Please read my answers below to your original questions.

> On Jun 18, 2017, at 2:23 AM, Eric Rescorla <[email-masked]> wrote:
> 
> 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?

I’ll add a paragraph in "5.12.  Error Reporting” explaining whether the context is useable after an exception is thrown, for context establishment and per-message calls, respectively. Something like

+If an exception is thrown during context establishment, the context
+negotiation has failed and the GSSContext object must be abandoned.
+If it is thrown in a per-message call, the context can remain useful.

> 
> 
> 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?

You would have to check. Even if an established security context already has its getConfState() being true, one can still wrap a message with privacy state set to false and the peer will unwrap it with success. If the peer insists only encrypted messages are allowed, she should always check.

This is already documented in 6.4.10.

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

The method does not fail itself (i.e. does not throws an exception) and you need to check the result with those getXyzState() methods after the context is established.

I can add a paragraph in S 5.9, something like:

+If any retrieved attribute does not match the desired value
+but it is necessary for the application protocol, the application should
+destroy the security context and not use it for application traffic.
+Otherwise, at this point, the context can be used by the application to
+apply cryptographic services to its data.

> 
> 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().

Sorry but this is how GSS-API works now.

> DETAIL

No, 6.1.16 already had

   Only when the indicated provider does not support
   the needed mechanism should the GSSManager move on to a different
   provider.

I think this implies that a new provider added might be used 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?

I think Ben explained this perfectly. This method is mainly used to calculate the source data size limit when you have a output token size 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?

Correct.

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

I’ll add a line saying "QOP is an integer defined by a mechanism".

> 
> 
> 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?

Ben explained it perfectly. Sometimes a mechanism only support authentication but not secure communication.

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

I’ll add a line in 3.5 Confidentiality saying something like “Confidentiality will be applied if the privacy state is set to true". Please forgive me I don’t want to alter the text in S 6 a lot because those were copied as doc comment in implementations and I don’t want them modified.

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

GSS-API is still using the Checksum word everywhere (RFC 3961’s title is “Encryption and Checksum Specifications for Kerberos 5"). I think the cryptographic word is added like we used in cryptographic random number generator, which means it’s stronger than CRC etc. I would like to continue using this word.

> So an exception is thrown for an invalid token?

Yes, specifically, DEFECTIVE_TOKEN as defined in 4.12.1.

S 6 has not spelt all possible GSSExceptions that a method might throw for brevity.

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

gss_release_cred() is eager, and there is no other guarantee the data can be automatically cleaned up. Even if GC cleaned up the GSSCredential object, there might be unreleased handles underneath.

S 6.3 already has

   When the credential is no longer needed, the application should call the dispose (equivalent to gss_release_cred) method to release any resources held by the credential object and to destroy any cryptographically sensitive information.

Do you think it’s necessary to append something like “An implementation should not rely on garbage control or a finalize() method to dispose a credential”?

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

Accepted.

> the export. I would change that.

Accepted.

> compare strings? Perhaps after you canonicalize them?

As Ben and I explained this is quite complicated. Can we not touch it in this bis?

> INITIATE and B supports ACCEPT you get “INITIATE_AND_ACCEPT"

I’ll add some explanation:

+Specifically, GSSCredential.INITIATE_AND_ACCEPT(0) should be returned
+as long as there exists one credential element allowing context initiation
+and one credential element allowing context acceptance. These two credential
+elements are not necessarily the same one, nor do they need to use the
+same mechanism(s).

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

I’ll remove the current 6.4.2 and 6.4.4. I’ll move the examples for each class to the end of each sub-section.

Thanks
Max

> 
> -Ekr
> 
> _______________________________________________
> Kitten mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/kitten

---

From: Eric Rescorla <[email-masked]>
Date: Fri, 14 Jul 2017 08:57:32 -0700

On Wed, Jul 12, 2017 at 8:41 PM, Weijun Wang <[email-masked]> wrote:

> I’ll add a paragraph in "5.12.  Error Reporting” explaining whether the
> context is useable after an exception is thrown, for context establishment
> and per-message calls, respectively. Something like
>
> You would have to check. Even if an established security context already
> has its getConfState() being true, one can still wrap a message with
> privacy state set to false and the peer will unwrap it with success. If the
> peer insists only encrypted messages are allowed, she should always check.
>
> The method does not fail itself (i.e. does not throws an exception) and
> you need to check the result with those getXyzState() methods after the
> context is established.
>

Sorry, I mean does the handshake fail? Or do you just hace to check.

> I think this implies that a new provider added might be used at all.

That doesn't seem very clear to me. My point is you might have defaults and
then add new omnes.

> GSS-API is still using the Checksum word everywhere (RFC 3961’s title is
> “Encryption and Checksum Specifications for Kerberos 5"). I think the
> cryptographic word is added like we used in cryptographic random number
> generator, which means it’s stronger than CRC etc. I would like to continue
> using this word.
>

At least add a parenthetical, or som other note, because this is not a term
others can understand.

> gss_release_cred() is eager, and there is no other guarantee the data can
> be automatically cleaned up. Even if GC cleaned up the GSSCredential
> object, there might be unreleased handles underneath.
>
> S 6.3 already has
>
>    When the credential is no longer needed, the application should call
> the dispose (equivalent to gss_release_cred) method to release any
> resources held by the credential object and to destroy any
> cryptographically sensitive information.
>
> Do you think it’s necessary to append something like “An implementation
> should not rely on garbage control or a finalize() method to dispose a
> credential”?
>

Yes.

> As Ben and I explained this is quite complicated. Can we not touch it in

The fact that it's complicated seems like more reason to explain it.

-Ekr

---

From: Weijun Wang <[email-masked]>
Date: Sat, 15 Jul 2017 21:21:48 +0800

Hi Eric

I’ll be vacation for the next 2 weeks and I will work on the update when I’m back.

Thanks
Max

> On Jul 14, 2017, at 11:57 PM, Eric Rescorla <[email-masked]> wrote:
...

---

From: Benjamin Kaduk <[email-masked]>
Date: Mon, 17 Jul 2017 19:22:17 -0500

> Sorry, I mean does the handshake fail? Or do you just hace to check.

You have to check.  The GSS-API has always been a request-and-check
type of API.

> That doesn't seem very clear to me. My point is you might have defaults and

I think you could use this to slot in your custom provider that was not
part of the system Java implementation, yes.
But I don't interact with the Java GSS stuff very much.

> At least add a parenthetical, or som other note, because this is not a term

I agree; it's probably worth putting in a "(authentication tag)"
parenthetical.  (The parallel to a Kerberos Checksum remains only a
parallel, as Kerberos is only one of many GSS mechanisms.)

> The fact that it's complicated seems like more reason to explain it.

In some sense, you can always compare GSS names (in that the compare-name
function will not throw an exception), but the results may not always
match the expected behavior.  (It's too bad that RFC 6680 didn't take the
time to summarize the state of affairs when adding naming extensions.)
I don't think we should try to provide a comprehensive review of the
state of affairs in this document, nor any normative statements (that would
only apply to implementations of the Java bindings whereas the issues
apply to all GSS-API implementations).  But perhaps something like the
following would be reasonable:

%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%

A full treatment of the behavior of GSS name comparison is outside the
scope of this work.  However, applications should note that to avoid
surpising behavior, it is best to ensure that the names being compared
are either both mechanism names for the same mechanism, or both internal
names that are not mechanism names.  This holds whether the .equals()
method is used directly, or the .export() method is used to generate byte
strings that are then compared byte-by-byte.

%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%

I considered adding a note that using .toString() and strcmp is
not recommended, but I do not think there is a non-normative statement
to be made that still provides value, and the lack of mention in the
text supplys a small disincentive to do so.

-Ben

---

From: Eric Rescorla <[email-masked]>
Date: Mon, 24 Jul 2017 22:04:09 -0700

> You have to check.  The GSS-API has always been a request-and-check

OK. I think that should be explicitly stated.

> But I don't interact with the Java GSS stuff very much.

OK. This also needs to be explicit.

> > > As Ben and I explained this is quite complicated. Can we not touch it
> in
> > > this bis?
> >
> >

Yes, this seems fine.

-Ekr

---

From: Benjamin Kaduk <[email-masked]>
Date: Tue, 25 Jul 2017 20:11:40 -0500

I set a calendar reminder to ping Max when he gets back from vacation.

-Ben

---

From: Wang Weijun <[email-masked]>
Date: Wed, 26 Jul 2017 20:58:00 -0700

Hi Ben

That's very kind of you. I'm still on vacation but I've flagged this mail thread and will work on it as soon as I'm back.

Thanks
Max

> On Jul 25, 2017, at 18:11, Benjamin Kaduk <[email-masked]> wrote:
>