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
_______________________________________________
Freeipa-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/freeipa-devel
