[Freeipa-devel] python-kerberos patch

[Simo Sorce]

On Mon, 2014-01-13 at 14:45 -0500, Rob Crittenden wrote:
> In an effort to be able to to use GSS-Proxy as a client in IPA I've 
> written a patch against python-kerberos to add a call to 
> gss_cred_inquire so we can peek at the current principal name. This will 
> replace the python-krbV call in ipapython/util.py::get_current_principal().
> 
> The patch is pending review upstream at 
> https://trac.calendarserver.org/ticket/830#comment:1 but it hasn't seen 
> any activity yet and I'm impatient.
> 
> Anyone have a few moments to take a look? I'm not super happy with the 
> way one has to call it and some feedback would be helpful.

I do not like their python API, but ... we can;t change that.

One the C code:
you call it yadda_client_yadda but then you user server_creds as the
variable name, that's confusing.

You should simply initialize name and name_token to 0 and
unconditionally free/release them on error, and do that all at the end
based on the return you want.

Also you can simplify string copying..

How I'd rewrite the last 15 lines after gss_display_name()

if (GSS_ERROR(maj_stat)) {
    set_gss_error(maj_stat, min_stat);
    ret = AUTH_GSS_ERROR;
    goto end;
}

state->username = strndup(name_token.value, name_token.length);
if (!state->username) {
    set_gss_error(GSS_S_FAILURE, ENOMEM);
    ret = AUTH_GSS_ERROR;
}

end:
    (void)gss_release_cred(&min_stat, &server_creds);
    (void)gss_release_name(&min_stat, &name);
    (void)gss_release_buffer(&min_state, name_token);

    return ret;
}


You could simplify even more without using ret, and jumping to end on
any maj_stat error and at end
if (maj_stat != GSS_S_COMPLETE) {
    set_gss_error(maj_state, min_stat);
    ...

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York