On Thu, Aug 22, 2013 at 10:18:36AM -0400, Stephen Gallagher wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > On 08/22/2013 10:11 AM, Stephen Gallagher wrote: > > On 08/15/2013 11:50 AM, Stephen Gallagher wrote: > >> There was duplicated code in cc_file_check_existing() and in > >> cc_dir_check_existing(). I pulled them into the same function. > > > >> There are two changes made to the original code here: 1) Fixes a > >> use-after-free bug in cc_file_check_existing(). In the original > >> code, we called krb5_free_context() and then used that context > >> immediately after that in krb5_cc_close(). This patch corrects > >> the ordering > > > >> 2) The krb5_cc_resolve() call handles KRB5_FCC_NOFILE for all > >> cache types. Previously, this was only handled for DIR caches. > > > >> This second part I need someone with Kerberos knowledge to > >> verify. Is there a risk of receiving this error for the FILE or > >> KEYRING types, and if so is this handling still acceptable or > >> should they be special-cased? > > > > > > > > Self-nack. New patch attached (I was never actually returning the > > validity result). > > > > Interdiff: > > > > diff --git a/src/providers/krb5/krb5_utils.c > > b/src/providers/krb5/krb5_utils.c index 47c45d0..8166435 100644 --- > > a/src/providers/krb5/krb5_utils.c +++ > > b/src/providers/krb5/krb5_utils.c @@ -750,6 +750,7 @@ > > check_cc_validity(const char *location, } > > > > ret = EOK; + *_valid = valid; > > > > done: if (ccache) krb5_cc_close(context, ccache); > > > > > Adding another consistency patch that goes along with this: > > > Only set active and valid on success > > The FILE cache only sets the return values of _active and _bool if the > entire function succeeds. The DIR cache was setting it even on failure. > This patch makes both consistent. This will benefit static analysis > tools which would be able to detect if the variable is ever used > uninitialized anywhere. > > > Both patches are attached for simplicity.
ACK to both. I tested that the ccache is reused if exists and recreated if doesn't exist for both DIR and FILE ccaches. _______________________________________________ sssd-devel mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
