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

[[e-mail] (Martin Rex)]

Benjamin Kaduk wrote:
> Benjamin Kaduk wrote:
>>
>> I submitted an -01 of the gss-loop document, with sample code.
>> There's a duplicate line of source XML in section 2.5 which causes some 
>> duplicate text, it's fixed in my git already.  Sorry about that.
>>
>> Comments welcome :)
> 
> We had some discussion on IRC, with Nico, Simo, Tom, Greg, and me.
> 
> It's unclear whether the src_name output of gss_accept_sec_context() is 
> only intended to be populated on a GSS_S_COMPLETE return; if not, it is 
> very easy to leak storage from earlier iterations when multiple calls are 
> needed.

It is up to the mechanism implementation at which point during the
handshake the src_name output parameter from gss_accept_sec_context()
is populated.  The application caller MUST provide the exact same
parameter as src_name to *ALL* iteration calls of gss_accept_sec_context()
of a single handshake, and the gssapi mechanism implementation ought
to actually "create" output values at most once during a handshake,
independent of when it is returned.

I actually had to look at the source to figure out what I had implemented
in my gsskrb5.dll wrapper for the Microsoft Kerberos SSP.  My implementation
clears all output parameters on entry to gss_accept_sec_context(),
returns scalars (ints) whenever they become available and memory-managed
objects (src_name, delegated_cred) only on the final interation that
returns GSS_S_COMPLETE.

I believe that to be conservative and simple behaviour with the least
potential for problems.

An application caller to the context iterator functions ought to
allocate the storage for all necessary/desired input and output
parameters _before_ the first iterator call, and use that exact
same storage for the entire handshake, i.e. until the iterator
either completes (GSS_S_COMPLETE)
or fails (everything else than COMPLETE and CONTINUE_NEEDED).

Creating the storage for input or output parameters only temporarily
for a singular invocation of either of the context iterator calls
is dangerous, unsafe and will result in undefined behaviour
and might result in unexpected handshakes or crashes.


> 
> We discussed whether (either at the abstract level or the C bindings) it 
> is guaranteed to be safe to call gss_release_buffer() twice on the same 
> buffer.  It is required to set the length of the buffer to zero (but not 
> to zero the value field), but also "Any buffer object returned by a 
> GSS-API routine may be passed to gss_release_buffer (even if there is no 
> storage associated with the buffer)."  What exactly constitutes a "buffer
> object returned by a GSS-API routine" seems subject to interpretation.


What in particular causes your notion "seems subject to interprettion"?

IMO, the specifications (rfc2743,rfc2744) are perfectly clear on this point.

In the high-level GSS-API spec (rfc2743), output parameters
are those parameters listed under "Outputs:",
e.g. GSS_Init_sec_context:

   http://tools.ietf.org/html/rfc2743#page-42

In the GSS-API C-Bindings spec (rfc2744), output parameters
are those that are tagged as "modify", whereas input parameters
are those tagged as "read" in the first line of each parameter description,
e.g. for gss_init_sec_context:

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


>
> (There are many things that could have been done to make this situation 
> better, but we don't hvae a time machine.)  Buffers with length zero can 
> be generated in lots of ways other than gss_release_buffer() 
> (gss_init_sec_context(), gss_accept_sec_context(), gss_unwrap()).

Where do you see the problem?
An output buffer with a zero length is allowed to have a NULL value pointer,
and as an alternative, the gssapi mechanism could use a pointer to
one central, static empty string.  The one thing that a gssapi mechanism
obviously ought to NOT do is invoking the implementation-specific behaviour
of malloc(0).

When the gssapi mechanism does not allocate memory for zero-length output
buffers, then robust gss_release_buffer() is trivial to achieve,
if the length is 0, do nothing and return GSS_S_COMPLETE.



> 
> We're pretty sure that GSS_S_CONTINUE_NEEDED will not be set with any 
> other major status bits (in the C API).

This is my understanding as well (Microsoft's SSPI might differ,
they have an additional COMPLETE status beyond GSS-API for DCE interop).


-Martin