-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 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? -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.14 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iEYEARECAAYFAlIM+LYACgkQeiVVYja6o6MNjACfSxhKZIq3nr9YSG3lro9kKQ2A zIIAni3Px6SSQ9CU/x3ltMW2VTJ1Scan =pTjc -----END PGP SIGNATURE-----
>From 8bd0020055574981441d4faf9577d13912d095e0 Mon Sep 17 00:00:00 2001 From: Stephen Gallagher <sgall...@redhat.com> Date: Thu, 15 Aug 2013 11:40:59 -0400 Subject: [PATCH] KRB5: Refactor cc_*_check_existing 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. --- src/providers/krb5/krb5_utils.c | 119 ++++++++++++++++++++-------------------- 1 file changed, 58 insertions(+), 61 deletions(-) diff --git a/src/providers/krb5/krb5_utils.c b/src/providers/krb5/krb5_utils.c index 1b6d57c608e56c995dd4e852093e9e045b2d40a8..d75a0f06d19dea0ce10474222c25c4a6fcde834f 100644 --- a/src/providers/krb5/krb5_utils.c +++ b/src/providers/krb5/krb5_utils.c @@ -704,6 +704,59 @@ done: return ret; } +static errno_t +check_cc_validity(const char *location, + const char *realm, + const char *princ, + bool *_valid) +{ + errno_t ret; + bool valid = false; + krb5_ccache ccache = NULL; + krb5_context context = NULL; + krb5_error_code krberr; + + krberr = krb5_init_context(&context); + if (krberr) { + DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to init kerberos context\n")); + return EIO; + } + + krberr = krb5_cc_resolve(context, location, &ccache); + if (krberr == KRB5_FCC_NOFILE || ccache == NULL) { + /* KRB5_FCC_NOFILE would be returned if the directory components + * of the DIR cache do not exist, which is the case in /run + * after a reboot + */ + DEBUG(SSSDBG_TRACE_FUNC, + ("ccache %s is missing or empty\n", location)); + valid = false; + ret = EOK; + goto done; + } else if (krberr != 0) { + KRB5_DEBUG(SSSDBG_OP_FAILURE, context, krberr); + DEBUG(SSSDBG_CRIT_FAILURE, ("krb5_cc_resolve failed.\n")); + ret = EIO; + goto done; + } + + krberr = check_for_valid_tgt(context, ccache, realm, princ, &valid); + if (krberr != EOK) { + KRB5_DEBUG(SSSDBG_OP_FAILURE, context, krberr); + DEBUG(SSSDBG_CRIT_FAILURE, + ("Could not check if ccache contains a valid principal\n")); + ret = EIO; + goto done; + } + + ret = EOK; + + done: + if (ccache) krb5_cc_close(context, ccache); + krb5_free_context(context); + return ret; +} + /*======== ccache back end utilities ========*/ struct sss_krb5_cc_be * get_cc_be_ops(enum sss_krb5_cc_type type) @@ -852,9 +905,6 @@ cc_file_check_existing(const char *location, uid_t uid, bool active; bool valid; const char *filename; - krb5_ccache ccache = NULL; - krb5_context context = NULL; - krb5_error_code kerr; filename = sss_krb5_residual_check_type(location, SSS_KRB5_TYPE_FILE); if (!filename) { @@ -878,28 +928,9 @@ cc_file_check_existing(const char *location, uid_t uid, return ret; } - kerr = krb5_init_context(&context); - if (kerr != 0) { - DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to init kerberos context\n")); - return EIO; - } - - kerr = krb5_cc_resolve(context, location, &ccache); - if (kerr != 0) { - KRB5_DEBUG(SSSDBG_OP_FAILURE, context, kerr); - krb5_free_context(context); - DEBUG(SSSDBG_CRIT_FAILURE, ("krb5_cc_resolve failed.\n")); - return EIO; - } - - kerr = check_for_valid_tgt(context, ccache, realm, princ, &valid); - krb5_free_context(context); - krb5_cc_close(context, ccache); - if (kerr != EOK) { - KRB5_DEBUG(SSSDBG_OP_FAILURE, context, kerr); - DEBUG(SSSDBG_CRIT_FAILURE, - ("Could not check if ccache contains a valid principal\n")); - return EIO; + ret = check_cc_validity(location, realm, princ, &valid); + if (ret != EOK) { + return ret; } *_active = active; @@ -977,9 +1008,6 @@ cc_dir_check_existing(const char *location, uid_t uid, bool active = false; bool active_primary = false; bool valid = false; - krb5_ccache ccache = NULL; - krb5_context context = NULL; - krb5_error_code krberr; enum sss_krb5_cc_type type; const char *filename; const char *dir; @@ -1062,45 +1090,14 @@ cc_dir_check_existing(const char *location, uid_t uid, goto done; } - krberr = krb5_init_context(&context); - if (krberr) { - DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to init kerberos context\n")); - ret = EIO; - goto done; - } - - krberr = krb5_cc_resolve(context, location, &ccache); - if (krberr == KRB5_FCC_NOFILE || ccache == NULL) { - /* KRB5_FCC_NOFILE would be returned if the directory components - * of the DIR cache do not exist, which is the case in /run - * after a reboot - */ - DEBUG(SSSDBG_TRACE_FUNC, - ("ccache %s is missing or empty\n", location)); - valid = false; - ret = EOK; - goto done; - } else if (krberr != 0) { - KRB5_DEBUG(SSSDBG_OP_FAILURE, context, krberr); - DEBUG(SSSDBG_CRIT_FAILURE, ("krb5_cc_resolve failed.\n")); - ret = EIO; - goto done; - } - - krberr = check_for_valid_tgt(context, ccache, realm, princ, &valid); - if (krberr != EOK) { - KRB5_DEBUG(SSSDBG_OP_FAILURE, context, krberr); - DEBUG(SSSDBG_CRIT_FAILURE, - ("Could not check if ccache contains a valid principal\n")); - ret = EIO; + ret = check_cc_validity(location, realm, princ, &valid); + if (ret != EOK) { goto done; } ret = EOK; done: talloc_free(tmp_ctx); - if (ccache) krb5_cc_close(context, ccache); - krb5_free_context(context); *_active = active; *_valid = valid; return ret; -- 1.8.3.1
0001-KRB5-Refactor-cc_-_check_existing.patch.sig
Description: PGP signature
_______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel