Subject: Re: [kitten] New Version Notification for draft-kaduk-kitten-gss-loop-02.txt (fwd)

From: Jeffrey Hutzelman <[email-masked]>
Date: Tue, 21 Jan 2014 10:56:49 -0500

On Tue, 2014-01-21 at 02:10 +0100, Martin Rex wrote:
> Jeffrey Hutzelman wrote:
> > On Mon, 2014-01-20 at 20:32 +0100, Martin Rex wrote:
> > > Nico Williams wrote:
> > > > 
> > > > Nonetheless, it seems clear that the intention is that calling a
> > > > gss_release/delete_*() function will modify the input/output parameter
> > > > so that calling it again is safe, so that releasing a zero-length,
> > > > non-zero valued buffer ought to be safe.
> > > 
> > > In theory yes, but practice may differ.
> > > 
> > > What I'm afraid of is wrapper functions that look like this:
> > 
> > Well, if an app has such a wrapper, then the _wrapper_ is not safe to
> > call twice.  That really has nothing to do with whether
> > gss_release_buffer is safe to call twice, and the spec for the latter
> > pretty clearly says it modifies its output.
> 
> I disagree.
> 
> Potentially we're fighting about the difference of
> "call-by-value" vs. "call-by-reference" in C, and what exactly 
> is the meaning of "same foo" when call-by-reference is used.

No, we're arguing over the difference between equal and eq.

> Does "same same" apply when the value is the same. or only when both
> value and reference/address are the same?

Neither.  Both my previous message and the comment, and in fact most of
the discussion, are about what happens when you call gss_release_buffer
on the same gss_buffer_desc structure.  Not two separate gss_buffer_desc
structures that start out with the same contents (i.e. are equal but not
eq).  Not one gss_buffer_desc where after the first call you change it
to look like what it did before that call.

gss_release_buffer operates on the gss_buffer_desc.  It frees the memory
referenced by that structure, and zeroes the length in that structure,
and maybe changes the pointer in that structure to be NULL.  Calling it
twice with pointers to two different objects is not the same as calling
it twice on the same object.

> With that in mind, a comment like
> 
> >
> > /* It is safe to call gss_release_buffer twice on the same buffer. */
> 
> is ambiguous and wrong.

It's not ambiguous, unless you willfully misinterpret it.
It _is_ wrong, which is what started this discussion.

I think we've beat this issue into the ground.
I think we have something resembling consensus that the example code
should avoid double-releasing, even though what it _actually does_
happens to be safe with current major GSS implementations.  Therefore,
we should stop arguing about it.

That leaves open the question of what, if any, changes should be made to
the spec.  There are some on which I suspect there is mostly agreement,
and one which seems to be controversial.  Nico seems to believe it
should be explicitly safe to call gss_release_buffer() on a
gss_buffer_desc that has already been released.  I think I agree.  But
we must be clear here -- we are talking about two calls with _pointers
to the same structure_, not calls with pointers to different structures
that started out with the same value.  In other words, we want to make
the example code's assumption safe.

> What should be safe is calling gss_release_buffer() with an empty
> buffer.  So the important guidance would be to *ALWAYS* create
> gss_buffer_desc structures initialized with {0, 0} in C.

The spec doesn't say this is safe.  In fact, the only sorts of buffers
for which gss_release_buffer is defined are those returned as outputs
from other GSS-API routines.  It is not explicitly safe to use it on
empty buffers, and it is clearly not safe to use it on buffers where the
value is memory allocated by the caller or by some other library.

I agree that the spec should be amened to require gss_release_buffer()
to work on empty buffers.

> The notion that gss_unwrap() could return a zero-length token is
> also somewhat flawed.  gss_unwrap() returns a message (=app data)
> if anything.  There are no such things as zero-length tokens anywhere
> throughout GSS-API.

Find me something in the spec that says that.
It is not axiomatic that app protocols do not contain empty messages.

> And a gssapi mechanism should definitely not
> rely on the application caller to call gss_release_buffer() on a
> zero-length message returned from gss_unwrap().

Agreed.

>   The only sane
> representation of an empty buffer _output_ value from a gssapi mech
> is {0, 0}, i.e. be conservative in what you send (=give out).

Also agreed, though in fact I believe the spec is pretty clear that if
the length is 0 you cannot rely on the pointer to be NULL; it might be
garbage instead.   However, you _can_ rely on gss_release_buffer to
work.

> > > > The other places where the API can output a zero-length token in
> > > > non-error conditions are gss_init/accept_sec_context() and
> > > > gss_delete_sec_context,
> > > 
> > > Nope, neither of these three calls can return a zero-length output token,
> > 
> > You know, we've discussed that question on several occasions, and I
> > can't remember ever being able to find something in RFC2743 that says
> > that.  The C bindings happen to make it impossible to tell the
> > difference between no token and an empty token, which makes using empty
> > tokens impractical, but if you can find text that prohibits them, please
> > do point it out.
> 
> As mentioned above, the GSS-API does not have the concept of
> zero-length _tokens_.

You keep saying that.  I asked you to point to _text in the spec_ that
says that.  Your assertion in email does not carry the same weight as
text in a published standards-track RFC.

>   Either there is a token, or there is no token.
> The reason is simple: you can not securely convey a zero-length token
> over an insecure communication channel.

Um, yes I can?  Every app protocol I've seen conveys GSS-API tokens as a
length followed by that many bytes.  A length of 0, followed by 0 bytes,
certainly conveys an empty token.  Apps certainly cannot rely on tokens
to be self-describing, since they are in fact opaque.

-- Jeff

---

From: [email-masked] (Martin Rex)
Date: Tue, 21 Jan 2014 19:10:49 +0100 (CET)

> we should stop arguing about it.
 
OK.

> the example code's assumption safe.
 
There is an explicit notion of an empty buffer in the C-Bindings
Header file:  GSS_C_EMPTY_BUFFER
IMHO, it would be difficult for a gssapi mechanism to argue to
be compliant to rfc2744 when breaking on a gss_release_buffer()
call that is passed an empty buffer, because the semantics of
a gss_buffer_desc with {0, 0} contents is well-defined in the
GSS-API C-Bindings.

http://tools.ietf.org/html/rfc2744#page-6

   3.2. String and similar data

   Many of the GSS-API routines take arguments and return values that
   describe contiguous octet-strings.  All such data is passed between
   the GSS-API and the caller using the gss_buffer_t data type.  This
   data type is a pointer to a buffer descriptor, which consists of a
   length field that contains the total number of bytes in the datum,
   and a value field which contains a pointer to the actual datum:

   typedef struct gss_buffer_desc_struct {
      size_t    length;
      void      *value;
   } gss_buffer_desc, *gss_buffer_t;

   Storage for data returned to the application by a GSS-API routine
   using the gss_buffer_t conventions is allocated by the GSS-API
   routine.  The application may free this storage by invoking the
   gss_release_buffer routine.  Allocation of the gss_buffer_desc object
   is always the responsibility of the application;  unused
   gss_buffer_desc objects may be initialized to the value
   GSS_C_EMPTY_BUFFER.

-Martin

---

From: Benjamin Kaduk <[email-masked]>
Date: Tue, 21 Jan 2014 15:34:21 -0500 (EST)

Sorry for the long silence from me; I was doing a lot of non-computer 
things over the weekend and didn't really catch up all the way.

I think we've basically found agreement, more inline.

> It _is_ wrong, which is what started this discussion.

By the spec it is wrong, yes...

> we should stop arguing about it.

...even though it is safe in all known implementations.
So, I agree that we should stop arguing about it.

> value is memory allocated by the caller or by some other library.

I had previously had the impression that Martin conveyed in his reply, 
namely that GSS_C_EMPTY_BUFFER was special and was required to be handled 
correctly by gss_release_buffer(); however, a quick search through rfc 
2744 does not find anything to support such a claim, so I cannot make it.

As such, I think that the easiest way forward for the sample code is to 
set the buffer to GSS_C_EMPTY_BUFFER after releasing it and check for 
NULL-ness before releasing it...

> to work on empty buffers.

... which is a pretty silly thing for application code to have to do, so I 
agree with this proposal as well.

> garbage instead.   However, you _can_ rely on gss_release_buffer to

I agree.  (As a side note, though 0 is a perfectly fine null pointer 
constant in C99, it is probably nicer to always spell it NULL.)

In summary:
(1) the sample code in -02 is wrong
(2) the sample code in -02 is safe with all known implementations
(3) It would be nice to update 2743/2744 to (a) require that 
gss_release_buffer(GSS_C_EMPTY_BUFFER) is safe, and (b) have 
gss_release_buffer() set the buffer to GSS_C_EMPTY_BUFFER on output 
(instead of just setting the length to 0).  This is probably out of scope 
for draft-kaduk-kitten-gss-loop.
(4) There is ongoing disagreement about empty tokens, zero-length tokens, 
absent tokens, and whether applications (are allowed to) use them.

-Ben