From ca052eadd5590e9d7feafc2b7b805a2e1c577c92 Mon Sep 17 00:00:00 2001 From: Viktor Dukhovni Date: Wed, 4 Mar 2015 02:24:54 +0000 Subject: [PATCH] Fix gss_inquire_cred_by_mech. Delegated or other explicit credentials were mishandled, the code only worked correctly when processing default credentials. In particular this caused root's default credential cache to be accessed when accepting delegated credentials in SSH: ssh_gssapi_accept_ctx() -> ssh_gssapi_getclient() -> gss_inquire_cred_by_mech() When /tmp/krb5cc_0 contained expired tickets, cascaded credentials stopped working for non-root users! --- lib/gssapi/krb5/inquire_cred.c | 255 +++++++++++++++++++-------------- lib/gssapi/krb5/inquire_cred_by_mech.c | 4 + 2 files changed, 153 insertions(+), 106 deletions(-) diff --git a/lib/gssapi/krb5/inquire_cred.c b/lib/gssapi/krb5/inquire_cred.c index a63ffea4f..93d165343 100644 --- a/lib/gssapi/krb5/inquire_cred.c +++ b/lib/gssapi/krb5/inquire_cred.c @@ -45,136 +45,179 @@ OM_uint32 GSSAPI_CALLCONV _gsskrb5_inquire_cred krb5_context context; gss_cred_id_t aqcred_init = GSS_C_NO_CREDENTIAL; gss_cred_id_t aqcred_accept = GSS_C_NO_CREDENTIAL; - gsskrb5_cred acred = NULL, icred = NULL; + gsskrb5_cred cred = (gsskrb5_cred)cred_handle; + gss_OID_set amechs = GSS_C_NO_OID_SET; + gss_OID_set imechs = GSS_C_NO_OID_SET; + OM_uint32 junk; + OM_uint32 aminor; OM_uint32 ret; + OM_uint32 aret; + OM_uint32 alife = GSS_C_INDEFINITE; + OM_uint32 ilife = GSS_C_INDEFINITE; + + /* + * XXX This function is more complex than it has to be. It should call + * _gsskrb5_inquire_cred_by_mech() twice and merge the results in the + * cred_handle == GSS_C_NO_CREDENTIAL case, but since + * _gsskrb5_inquire_cred_by_mech() is implemented in terms of this + * function, first we must fix _gsskrb5_inquire_cred_by_mech(). + */ *minor_status = 0; if (output_name) - *output_name = NULL; + *output_name = GSS_C_NO_NAME; + if (cred_usage) + *cred_usage = GSS_C_BOTH; /* There's no NONE */ if (mechanisms) - *mechanisms = GSS_C_NO_OID_SET; + *mechanisms = GSS_C_NO_OID_SET; GSSAPI_KRB5_INIT (&context); if (cred_handle == GSS_C_NO_CREDENTIAL) { - ret = _gsskrb5_acquire_cred(minor_status, - GSS_C_NO_NAME, - GSS_C_INDEFINITE, - GSS_C_NO_OID_SET, - GSS_C_ACCEPT, - &aqcred_accept, - NULL, - NULL); - if (ret == GSS_S_COMPLETE) - acred = (gsskrb5_cred)aqcred_accept; - - ret = _gsskrb5_acquire_cred(minor_status, - GSS_C_NO_NAME, - GSS_C_INDEFINITE, - GSS_C_NO_OID_SET, - GSS_C_INITIATE, - &aqcred_init, - NULL, - NULL); - if (ret == GSS_S_COMPLETE) - icred = (gsskrb5_cred)aqcred_init; - - if (icred == NULL && acred == NULL) { - *minor_status = 0; - return GSS_S_NO_CRED; - } - } else - acred = (gsskrb5_cred)cred_handle; + /* + * From here to the end of this if we should refactor into a separate + * function. + */ + /* Get the info for the default ACCEPT credential */ + aret = _gsskrb5_acquire_cred(&aminor, + GSS_C_NO_NAME, + GSS_C_INDEFINITE, + GSS_C_NO_OID_SET, + GSS_C_ACCEPT, + &aqcred_accept, + NULL, + NULL); + if (aret == GSS_S_COMPLETE) { + aret = _gsskrb5_inquire_cred(&aminor, + aqcred_accept, + output_name, + &alife, + NULL, + &amechs); + (void) _gsskrb5_release_cred(&junk, &aqcred_accept); + if (aret == GSS_S_COMPLETE) { + output_name = NULL; /* Can't merge names; output only one */ + if (cred_usage) + *cred_usage = GSS_C_ACCEPT; + if (lifetime) + *lifetime = alife; + if (mechanisms) { + *mechanisms = amechs; + amechs = GSS_C_NO_OID_SET; + } + (void) gss_release_oid_set(&junk, &amechs); + } else if (aret != GSS_S_NO_CRED) { + *minor_status = aminor; + return aret; + } else { + alife = GSS_C_INDEFINITE; + } + } + + /* Get the info for the default INITIATE credential */ + ret = _gsskrb5_acquire_cred(minor_status, + GSS_C_NO_NAME, + GSS_C_INDEFINITE, + GSS_C_NO_OID_SET, + GSS_C_INITIATE, + &aqcred_init, + NULL, + NULL); + if (ret == GSS_S_COMPLETE) { + ret = _gsskrb5_inquire_cred(minor_status, + aqcred_init, + output_name, + &ilife, + NULL, + &imechs); + (void) _gsskrb5_release_cred(&junk, &aqcred_init); + if (ret == GSS_S_COMPLETE) { + /* + * Merge results for INITIATE with ACCEPT if we had ACCEPT and + * for those outputs that are desired. + */ + if (cred_usage) { + *cred_usage = (*cred_usage == GSS_C_ACCEPT) ? + GSS_C_BOTH : GSS_C_INITIATE; + } + if (lifetime) + *lifetime = min(alife, ilife); + if (mechanisms) { + /* + * This is just one mechanism (IAKERB and such would live + * elsewhere). imechs will be equal to amechs, though not + * ==. + */ + if (aret != GSS_S_COMPLETE) { + *mechanisms = imechs; + imechs = GSS_C_NO_OID_SET; + } + } + (void) gss_release_oid_set(&junk, &amechs); + } else if (ret != GSS_S_NO_CRED) { + *minor_status = aminor; + return aret; + } + } + + if (aret != GSS_S_COMPLETE && ret != GSS_S_COMPLETE) { + *minor_status = aminor; + return aret; + } + *minor_status = 0; /* Even though 0 is not specified to be special */ + return GSS_S_COMPLETE; + } - if (acred) - HEIMDAL_MUTEX_lock(&acred->cred_id_mutex); - if (icred) - HEIMDAL_MUTEX_lock(&icred->cred_id_mutex); + HEIMDAL_MUTEX_lock(&cred->cred_id_mutex); if (output_name != NULL) { - if (icred && icred->principal != NULL) { - gss_name_t name; - - if (acred && acred->principal) - name = (gss_name_t)acred->principal; - else - name = (gss_name_t)icred->principal; - + if (cred->principal != NULL) { + gss_name_t name = (gss_name_t)cred->principal; ret = _gsskrb5_duplicate_name(minor_status, name, output_name); if (ret) - goto out; - } else if (acred && acred->usage == GSS_C_ACCEPT) { - krb5_principal princ; - *minor_status = krb5_sname_to_principal(context, NULL, - NULL, KRB5_NT_SRV_HST, - &princ); - if (*minor_status) { - ret = GSS_S_FAILURE; - goto out; - } - *output_name = (gss_name_t)princ; - } else { - krb5_principal princ; - *minor_status = krb5_get_default_principal(context, - &princ); - if (*minor_status) { - ret = GSS_S_FAILURE; - goto out; - } - *output_name = (gss_name_t)princ; - } + goto out; + } else if (cred->usage == GSS_C_ACCEPT) { + /* + * Keytab case, princ may not be set (yet, ever, whatever). + * + * We used to unconditionally output the krb5_sname_to_principal() + * of the host service for the hostname, but we didn't know if we + * had keytab entries for it, so it was incorrect. We can't be + * breaking anything in tree by outputting GSS_C_NO_NAME, but we + * might be breaking other callers. + */ + *output_name = GSS_C_NO_NAME; + } else { + /* This shouldn't happen */ + *minor_status = KRB5_NOCREDS_SUPPLIED; /* XXX */ + ret = GSS_S_NO_CRED; + goto out; + } } if (lifetime != NULL) { - OM_uint32 alife = GSS_C_INDEFINITE, ilife = GSS_C_INDEFINITE; - - if (acred) alife = acred->lifetime; - if (icred) ilife = icred->lifetime; - - ret = _gsskrb5_lifetime_left(minor_status, - context, - min(alife,ilife), - lifetime); - if (ret) - goto out; - } - if (cred_usage != NULL) { - if (acred && icred) - *cred_usage = GSS_C_BOTH; - else if (acred) - *cred_usage = GSS_C_ACCEPT; - else if (icred) - *cred_usage = GSS_C_INITIATE; - else - abort(); + ret = _gsskrb5_lifetime_left(minor_status, + context, + cred->lifetime, + lifetime); + if (ret) + goto out; } - + if (cred_usage != NULL) + *cred_usage = cred->usage; if (mechanisms != NULL) { ret = gss_create_empty_oid_set(minor_status, mechanisms); if (ret) - goto out; - if (acred) - ret = gss_add_oid_set_member(minor_status, - &acred->mechanisms->elements[0], - mechanisms); - if (ret == GSS_S_COMPLETE && icred) - ret = gss_add_oid_set_member(minor_status, - &icred->mechanisms->elements[0], - mechanisms); + goto out; + ret = gss_add_oid_set_member(minor_status, + &cred->mechanisms->elements[0], + mechanisms); if (ret) - goto out; + goto out; } ret = GSS_S_COMPLETE; -out: - if (acred) - HEIMDAL_MUTEX_unlock(&acred->cred_id_mutex); - if (icred) - HEIMDAL_MUTEX_unlock(&icred->cred_id_mutex); - - if (aqcred_init != GSS_C_NO_CREDENTIAL) - ret = _gsskrb5_release_cred(minor_status, &aqcred_init); - if (aqcred_accept != GSS_C_NO_CREDENTIAL) - ret = _gsskrb5_release_cred(minor_status, &aqcred_accept); +out: + HEIMDAL_MUTEX_unlock(&cred->cred_id_mutex); return ret; } diff --git a/lib/gssapi/krb5/inquire_cred_by_mech.c b/lib/gssapi/krb5/inquire_cred_by_mech.c index 2b60ea840..6ce4994eb 100644 --- a/lib/gssapi/krb5/inquire_cred_by_mech.c +++ b/lib/gssapi/krb5/inquire_cred_by_mech.c @@ -47,6 +47,10 @@ OM_uint32 GSSAPI_CALLCONV _gsskrb5_inquire_cred_by_mech ( OM_uint32 maj_stat; OM_uint32 lifetime; + /* + * XXX This is busted. _gsskrb5_inquire_cred() should be implemented in + * terms of _gsskrb5_inquire_cred_by_mech(), NOT the other way around. + */ maj_stat = _gsskrb5_inquire_cred (minor_status, cred_handle, name, &lifetime, &usage, NULL); -- 2.11.4.GIT