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

[[e-mail] (Martin Rex)]

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:

OM_uint32 app_wrapper_gss_release_cred(gss_ctx_id_t cred)
{
    OM_uint32      maj_stat;
    OM_uint32      min_stat;
    gss_cred_id_t  l_cred = cred;

    maj_stat = gss_release_cred(&min_stat, &l_cred);
    if ( GSS_S_COMPLETE!=maj_stat ) {
        app_show_error( maj_stat, min_stat);
    }

    return(maj_stat); 
}

And if that app_wrapper_* release function is called twice,
then a non-zero copy of the original cred will be passed on the
second call, and that may result in the undefined behaviour
of double-free().

>
> The API can output zero-length buffers from gss_unwrap(),
> so the question comes up of whether such buffers can have
> non-zero values -- the RFC doesn't say.

correct -- gss_unwrap() may return a zero-length output buffer.
The length of the buffer must be zero for obvious reasons.
The value of the pointer is then irrelevant, and the gssapi
mechanism must be able to cope safely with whatever it returns
along with a zero length.


>
> 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,
they either return a token or do not return a token (which means that
a gssapi wrapper mechanism that would blindly copy output parameters
from underlying implementations and use an implementation specific
malloc(0) would be buggy and leak memory for these three functions.


>
> and there the text is not entirely
> dispositive...  but it is highly suggestive that zero-length tokens
> are empty, and this, together with the general design of
> gss_release/delete_*() strongly implies that it should be safe to call
> any of those twice with the same input/output parameter.

No, NEVER.

The only thing that _might_ be safe, when the gssapi mechanism designer
didn't mess up, is superfluous release calls with a NULL handle,
i.e. calling it once with a handle that gets zeroed, and then calling
it again with the zero-ed handle.  The big difference here is that
the second call is *NOT* with the same handle, but with a handle
that was zero-ed by a prior release call.  This is why I don't like
the current language, that would suggest the wrapper describe above
could be safely called as well (which it can not).


> 
> The only reason to assume that releasing a zero-length/non-zero value
> buffer is that an implementation might have called malloc(0) in the
> process of outputting a zero-length buffer from gss_unwrap()

malloc(0) is something that no reasonable gssapi mechanism
implementation should ever do.


-Martin