-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
In addition to validating the keytab everytime a TGT is requested, we also validate the keytab on back end startup to give early warning that the keytab is not usable.
Fixes: #556
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 07/21/2010 11:51 AM, Jakub Hrozek wrote:
In addition to validating the keytab everytime a TGT is requested, we also validate the keytab on back end startup to give early warning that the keytab is not usable.
Fixes: #556
Nack.
sss_krb5_verify_keytab() should not be passed a memctx. No memory created in this function is being passed back to the caller. It would be much better to create a tmp_ctx (at the top-level) for the function and talloc_free() that in the done: label. This will make it easier to detect memory leaks with valgrind, as well.
There is a bug with memory handling of the realm_name variable. If we populate this value with krb5_get_default_realm(), it needs to be freed with krb5_free_string(). Right now we would be attempting to call talloc_free() on it, and that would fail. The safest thing to do would be to pass a temporary string into krb5_get_default_realm() and then talloc_strdup() the returned value into realm_name and immediately krb5_free_string() the temporary variable.
- -- Stephen Gallagher RHCE 804006346421761
Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 07/22/2010 02:06 PM, Stephen Gallagher wrote:
Nack.
sss_krb5_verify_keytab() should not be passed a memctx. No memory created in this function is being passed back to the caller. It would be much better to create a tmp_ctx (at the top-level) for the function and talloc_free() that in the done: label. This will make it easier to detect memory leaks with valgrind, as well.
Fixed
There is a bug with memory handling of the realm_name variable. If we populate this value with krb5_get_default_realm(), it needs to be freed with krb5_free_string(). Right now we would be attempting to call talloc_free() on it, and that would fail. The safest thing to do would be to pass a temporary string into krb5_get_default_realm() and then talloc_strdup() the returned value into realm_name and immediately krb5_free_string() the temporary variable.
Thanks, fixed too in the patch as well as in the ldap_child itself where we had the same problem (that's patch 0001).
We also discussed getting the default realm with Simo and Sumit on #freeipa, that discussion is now recorded in #570.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 07/22/2010 11:39 AM, Jakub Hrozek wrote:
On 07/22/2010 02:06 PM, Stephen Gallagher wrote:
Nack.
sss_krb5_verify_keytab() should not be passed a memctx. No memory created in this function is being passed back to the caller. It would be much better to create a tmp_ctx (at the top-level) for the function and talloc_free() that in the done: label. This will make it easier to detect memory leaks with valgrind, as well.
Fixed
Nack. If an error is returned from krb5_init_context, the memory for tmp_ctx is leaked.
Otherwise it looks good.
- -- Stephen Gallagher RHCE 804006346421761
Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 07/26/2010 06:34 PM, Stephen Gallagher wrote:
Nack. If an error is returned from krb5_init_context, the memory for tmp_ctx is leaked.
Otherwise it looks good.
Thanks, new patch attached
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 07/27/2010 10:53 AM, Jakub Hrozek wrote:
On 07/26/2010 06:34 PM, Stephen Gallagher wrote:
Nack. If an error is returned from krb5_init_context, the memory for tmp_ctx is leaked.
Otherwise it looks good.
Thanks, new patch attached
Ack.
- -- Stephen Gallagher RHCE 804006346421761
Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
On 07/27/2010 10:57 AM, Stephen Gallagher wrote:
On 07/27/2010 10:53 AM, Jakub Hrozek wrote:
On 07/26/2010 06:34 PM, Stephen Gallagher wrote:
Nack. If an error is returned from krb5_init_context, the memory for tmp_ctx is leaked.
Otherwise it looks good.
Thanks, new patch attached
Ack.
Pushed both to master.
- -- Stephen Gallagher RHCE 804006346421761
Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/
sssd-devel@lists.fedorahosted.org
-
Jakub Hrozek -
Stephen Gallagher