Ticket History #5936: gss_accept_sec_context() returns minor_status == 0 if arg3 == GSS_C_NO_CREDENTIAL

Near the end of gss_accept_sec_context(), we have:

if (!verifier_cred_handle && cred_handle) {
krb5_gss_release_cred(minor_status, &cred_handle);
}

Unfortunately, minor_status is a passed in ref and this call will
clear the error that we expect to be returning to the caller. This
same class may also affect the my proposed fix to the memory leak
if arg3 == GSS_C_NO_CREDENTIAL in the non-error case in the opposite
sense, that is that code may return non-zero minor_status when
major_status is zero. That appears to be be less problematic but
should also be fixed once that patch or something like it is adopted.

Hi,

I've started to use krb5's (krb-1.6.3) gss API and it happened quite
often in the first time, that this function failed for various reasons
(which is not a problem so far).

The function returned GSS_S_FAILURE and according to the documentation a
more specific error code should be in minor_status. But in my case
minor_status was always 0.

I've digged a little bit in the implementation in
krb5/src/lib/gssapi/krb5/accept_sec_context.c and it looks like in line
928 the minor_status is correctly set to code, which is the return value
of most krb5 functions:

*minor_status = code;

So far this would work perfectly.

Unfortunately, at the end of this function it will be overwritten:

if (!verifier_cred_handle && cred_handle) {
krb5_gss_release_cred(minor_status, &cred_handle);
}

At least in my case, the condition was always true (because I've called
accept_sec_contect with verifier_cred_handle=GSS_C_NO_CREDENTIAL) and so
the real error was always hidden.

Because this is not very convenient (and usually the return code of
krb5_gss_release_cred is much less helpful than the real error code of a
previous failed function), I'd suggest to change the code like this:

--- src/lib/gssapi/krb5/accept_sec_context.c
+++ src/lib/gssapi/krb5/accept_sec_context.c
@@ -991,7 +991,8 @@
*output_token = token;
}
if (!verifier_cred_handle && cred_handle) {
- krb5_gss_release_cred(minor_status, &cred_handle);
+ int release_minor_status;
+ krb5_gss_release_cred(&release_minor_status, &cred_handle);
}
krb5_free_context(context);
return (major_status);


It would be great if you could review this patch and consider to apply
the it.

Thank you very much in advance!


Best regards,
Christian

Hi Jeffrey,

Jeffrey Altman via RT wrote:
Sorry for opening this new bug - I've searched in RT before I've created
it, but without success.
I've checked now #5442 and some referred bugs, and I think it would be
the best to set this one (#6051) as a duplicate of #5936.

Good news that it is already fixed- I'd like to try it out. Is there a
public access to the krb SVN server?

Best regards,
Christian

Fixed by patches in #5442.