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

[Benjamin Kaduk <[e-mail]>]

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.

On Tue, 21 Jan 2014, Jeffrey Hutzelman wrote:

> 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:
>>>>>
>> 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.

By the spec it is wrong, yes...

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

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

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

> I agree that the spec should be amened to require gss_release_buffer()
> 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.

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

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