rewrite gss_krb5_ccache_name

[Jeffrey Altman]

Jeffrey Hutzelman wrote:
> --On Friday, November 21, 2008 08:44:04 PM -0500 Jeffrey Altman 
> <jaltman at secure-endpoints.com> wrote:
> 
>> For thread safety reasons the gss library uses thread local storage
>> (TLS) to store a number of values including ccache name and error
>> messages.  The TLS stored value must be a copy in order to prevent
>> corruption when the original source string is deallocated or altered.
> 
> In the example Stephen posted, he does the following:
> 
>     krb5_lock();
>     if( neg_ctx->context == GSS_C_NO_CONTEXT){
>         major_status =gss_krb5_ccache_name(&minor_status,krb5->ccache_name,
> &old_name);
>    :
>    }
>     major_status = gss_init_sec_context(&minor_status,
>   :
>     if(old_name)gss_krb5_ccache_name(&minor_status,old_name, NULL);
>     krb5_unlock();
> 

Stephen wrote that he tried the above code only after

   major_status =gss_krb5_ccache_name(&minor_status,
                                      krb5_ccache_name,
                                      NULL);

resulted in a leak.  When the 'old_name' parameter is provided a NULL
input there is no responsibility on the part of the function caller
to free the memory.

Note that there is a second problem with this api.  When a non-NULL
'old_name' parameter is provided, it must be freed using the same
free() as is linked to the gssapi32.dll library.  Unfortunately,
there is no gss_krb5_free_ccname() function in the API available
to make sure that this is possible.  As a result, passing anything
other than NULL as the 'old_name' parameter is dangerous on Windows.

>> If that is the case, the real bug here is that the gssapi32.dll
>> does not implement a DLL_THREAD_DETACH routine which would free
>> any allocated memory stored under the registered TLS keys.
> 
> Well, if you're not doing that, you certainly should...

Indeed, this should be fixed.

Jeffrey Altman