[Date Prev][Date Next] [Chronological] [Thread] [Top]

Re: commit: ldap/libraries/libldap gssapi.c

Hallvard B Furuseth wrote:
hyc writes:
	gssapi.c  NONE ->  1.1

ITS#5369 SASL/GSSAPi refactoring from Stefan Metzmacher<metze@samba.org>
and Rafal Szczeniak<mimir@samba.org>, with minor cleanups

This is buggy:

Feel free to patch whatever makes sense. I don't have any SMB servers running at the moment to test against. There were certainly a lot of flaws in the code; my main motivation for committing it was that it helped to compartmentalize the Cyrus-specific code. Since I doubt that anything besides Samba4 will ever use the gssapi.c portion, it's not a big concern at the moment. If it's broken, I expect the Samba guys to let us know.

pkt_len is used uninitialized in sb_sasl_gssapi_decode().

guess_service_principal() is broken: It uses 'ret' uninitialized, as if
a return value from malloc.  It passes (buffer length - 1) instead of
buffer length to snprintf, which I think fails for the (allow_remote&&
givenstr) case.  snprintf() unnecessary anyway since the function can
malloc the exact needed size.  Also it's preferable to only use a string
literal as format argument, so gcc can verify the printf (instead of
warning that it can't).

Format errors: OM_uint32 printed as %u, size_t/ber_len_t as %lu.

Here is a draft patch.  Untested.  Note I've not looked at how this
actually works, just how to get rid of warnings.

Finally, some header files should declare these functions:
   ldap_gssapi_bind_s(), ldap_int_gssapi_close(), ldap_int_gssapi_config(),
   ldap_int_gssapi_get_option(), ldap_int_gssapi_set_option(),
   ldap_pvt_sasl_generic_install(), ldap_pvt_sasl_generic_remove(),
and be #included by bind.c, cyrus.c, gssapi.c, init.c, options.c, request.c.

-- -- Howard Chu CTO, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/