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:
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. http://folk.uio.no/hbf/OpenLDAP/gssapi.txt
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.
Hallvard B Furuseth wrote:
hyc writes:
gssapi.c NONE -> 1.1
ITS#5369 SASL/GSSAPi refactoring from Stefan Metzmachermetze@samba.org and Rafal Szczeniakmimir@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. http://folk.uio.no/hbf/OpenLDAP/gssapi.txt
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 writes:
Feel free to patch whatever makes sense. I don't have any SMB servers running at the moment to test against.
Me neither, nor have I ever used gssapi (knowingly:-). So I'll wait and see if someone else does something first.
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.
OK. I'll just refer the ITS to this thread then.
Howard Chu wrote:
Since I doubt that anything besides Samba4 will ever use the gssapi.c portion,
I'd like to implement support for binding with SASL/GSSAPI to a LDAP server with a forwardable ticket received from the web browser via SPNEGO/Kerberos. Since web2ldap is multi-threaded I'd prefer to have an API to explicitly setup the GSSAPI context and pass this smoehow to ldap_sasl_interactive_bind_s() (all via python-ldap).
My C programming knowledge is limited so I can't tell how to achieve this and I can't provide patches for libldap either.
Ciao, Michael.