[SSSD] [PATCH] KRB5: Refactor cc_*_check_existing

Stephen Gallagher sgallagh at redhat.com
Thu Aug 22 14:18:36 UTC 2013


-----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.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.14 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iEYEARECAAYFAlIWHbwACgkQeiVVYja6o6PixwCgsTSL37MAYFy15IWeYlXiT3yT
Q2sAoIFeyktu+od/kuTmua9GjvH51KBD
=mn98
-----END PGP SIGNATURE-----
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-KRB5-Refactor-cc_-_check_existing.patch
Type: text/x-patch
Size: 6176 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130822/8d449924/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-KRB5-Only-set-active-and-valid-on-success.patch
Type: text/x-patch
Size: 2115 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130822/8d449924/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-KRB5-Refactor-cc_-_check_existing.patch.sig
Type: application/pgp-signature
Size: 72 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130822/8d449924/attachment.sig>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-KRB5-Only-set-active-and-valid-on-success.patch.sig
Type: application/pgp-signature
Size: 72 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130822/8d449924/attachment-0001.sig>


More information about the sssd-devel mailing list