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

[Weijun Wang <[e-mail]>]

> On Aug 4, 2017, at 8:40 AM, Benjamin Kaduk <[e-mail]> wrote:
> 
> Hi Max,
> 
> I hope your vacation was relaxing!

Mentally yes, physically not really.

> 
> On Thu, Aug 03, 2017 at 10:45:11AM +0800, Weijun Wang wrote:
>> Hi Ekr and Ben
>> 
>> Thanks a lot for you careful review and great suggestions. I'll make the following changes:
>> 
>> 1. Append "(authentication tag)" after the first appearance of "cryptographic checksum".
>> 
>> 2. In S 4.9, append "and might not match the initiator-requested values" after "These will reflect the actual attributes of the established context."
>> 
>> 3. Add Ben's suggested text on GSSName comparison into S 4.13, at the end of the "GSSName objects can be compared..." paragraph.
>> 
>> 4. And a clarification text "When neither of the methods is called, the implementation should choose a default provider for each mechanism it supports" to the introduction of addProviderAtFront() and addProviderAtEnd(), at the end of S 6.1.
> 
> I'm not entirely sure that this addresses Ekr's concern.  Laying it out (hopefully)
> more clearly, suppose that I have a java standard library that provides GSSAPI
> support, and supports the krb5 and ntlm mechanisms.  It's clear that my
> application code could supply an alternate provider for the krb5 mech and
> have that used preferentially.  But could my application code also supply
> a provider for, say, the IAKERB mech via addProviderAtFront, and have that used?

Yes, you can.

That said, it's not easy. The Java bindings of GSS-API do not contain public available interfaces for mechanism providers, and each GSS-API implementation defines its own interfaces. So if you want to write a IAKERB provider for Oracle's Java (or OpenJDK), you need to extend Oracle's interfaces. If you want to write one for IBM's Java, you extend IBM's interfaces. At least for Oracle JDK, these interfaces are internal so you end up releasing your own fork of JDK.

> 
> Regardless, thanks for these edits -- I look forward to the new revision.

Working on it.

Thanks
Max

> 
> -Ben
> 
> 
> 
>> I'll post a new draft soon.
>> 
>> Thanks
>> Max
>> 
>> 
>>> On Jul 25, 2017, at 1:04 PM, Eric Rescorla <[e-mail]> wrote:
>>> 
>>> 
>>> 
>>> On Mon, Jul 17, 2017 at 5:22 PM, Benjamin Kaduk <[e-mail]> wrote:
>>> On Fri, Jul 14, 2017 at 08:57:32AM -0700, Eric Rescorla wrote:
>>>> On Wed, Jul 12, 2017 at 8:41 PM, Weijun Wang <[e-mail]> wrote:
>>>> 
>>>>> Hi Ekr
>>>>> 
>>>>> Please read my answers below to your original questions.
>>>>> 
>>>>>> On Jun 18, 2017, at 2:23 AM, Eric Rescorla <[e-mail]> 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.
>>>>> 
>>>> 
>>>> 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.
>>> 
>>> OK. I think that should be explicitly stated.
>>> 
>>> 
>>>> 
>>>>> 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
>>>>>> S 6.1.16.
>>>>>> Can addProviderAtFront() be used to add new providers which
>>>>>> the API would not normally use at all?
>>>>> 
>>>>> 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.
>>>>> 
>>>> 
>>>> That doesn't seem very clear to me. My point is you might have defaults and
>>>> then add new omnes.
>>> 
>>> 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.
>>> 
>>> OK. This also needs to be explicit. 
>>> 
>>>>>> 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.
>>>>> 
>>>>> 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.
>>>> 
>>>> 
>>>> 
>>>>> 
>>>>>> 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.
>>>>> 
>>>>> Accepted.
>>>>> 
>>>>>> 
>>>>>> 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.
>>>>> 
>>>>> Accepted.
>>>>> 
>>>>>> 
>>>>>> 
>>>>>> 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?
>>>>> 
>>>>> As Ben and I explained this is quite complicated. Can we not touch it in
>>>>> this bis?
>>>> 
>>>> 
>>>> 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.
>>> 
>>> %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
>>> 
>>> Yes, this seems fine.
>>> 
>>> -Ekr