[Freeipa-devel] python-kerberos patch

[Rob Crittenden]

Simo Sorce wrote:
> 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.
>

Thanks for the detailed review. I applied your changes and updated the 
attachment on the upstream ticket.

If you think it is acceptable, I'd like to add this patch to at least 
rawhide if not F-20.

rob