From 36c54e9dca26bc1b304933a8db387eff0c361717 Mon Sep 17 00:00:00 2001 From: Ken Raeburn Date: Thu, 29 Jul 2004 02:26:43 +0000 Subject: [PATCH] Add a mutex to the GSSAPI krb5 mechanism credential structure. Lock it while frobbing the contents. Also added krb5_gss_validate_cred_1, which is like krb5_gss_validate_cred but for internal use. It lets the caller supply the krb5_context instead of creating yet another one locally, and leaves the new credential mutex locked on a successful return so that the caller doesn't have to reacquire it. More functions should be changed to use this internally, but it's a performance issue; I don't think it's a correctness or thread-safety issue. * gssapiP_krb5.h (struct _krb5_gss_cred_id_rec): Add a mutex. (krb5_gss_validate_cred_1): Declare. * accept_sec_context.c (rd_and_store_for_creds): Initialize mutex. * acquire_cred.c (krb5_gss_acquire_cred): Initialize mutex. * add_cred.c (krb5_gss_add_cred): Create the krb5 context earlier. Call krb5_gss_validate_cred_1. Make sure the mutex is locked. * copy_ccache.c (gss_krb5_copy_ccache): Lock the mutex in the source credential. * init_sec_context.c (get_credentials, new_connection): Check that the mutex is locked. (mutual_auth): Delete unused credential argument. (krb5_gss_init_sec_context): Lock the mutex. * inq_cred.c (krb5_gss_inquire_cred): Lock the mutex. * rel_cred.c (krb5_gss_release_cred): Destroy the mutex. * set_allowable_enctypes.c (gss_krb5_set_allowable_enctypes): Lock the mutex. * val_cred.c (krb5_gss_validate_cred_1): New function. (krb5_gss_validate_cred): Use it. git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@16630 dc483132-0cff-0310-8789-dd5450dbe970 --- src/lib/gssapi/krb5/ChangeLog | 23 +++++++++ src/lib/gssapi/krb5/accept_sec_context.c | 8 ++++ src/lib/gssapi/krb5/acquire_cred.c | 10 ++++ src/lib/gssapi/krb5/add_cred.c | 26 +++++++---- src/lib/gssapi/krb5/copy_ccache.c | 10 +++- src/lib/gssapi/krb5/gssapiP_krb5.h | 12 +++-- src/lib/gssapi/krb5/init_sec_context.c | 17 +++++-- src/lib/gssapi/krb5/inq_cred.c | 10 ++++ src/lib/gssapi/krb5/rel_cred.c | 3 ++ src/lib/gssapi/krb5/set_allowable_enctypes.c | 8 ++++ src/lib/gssapi/krb5/val_cred.c | 49 ++++++++++++++------ 11 files changed, 146 insertions(+), 30 deletions(-) diff --git a/src/lib/gssapi/krb5/ChangeLog b/src/lib/gssapi/krb5/ChangeLog index 4bc52f823..a9fb83df4 100644 --- a/src/lib/gssapi/krb5/ChangeLog +++ b/src/lib/gssapi/krb5/ChangeLog @@ -1,5 +1,28 @@ 2004-07-28 Ken Raeburn + * gssapiP_krb5.h (struct _krb5_gss_cred_id_rec): Add a mutex. + (krb5_gss_validate_cred_1): Declare. + * accept_sec_context.c (rd_and_store_for_creds): Initialize mutex. + * acquire_cred.c (krb5_gss_acquire_cred): Initialize mutex. + * add_cred.c (krb5_gss_add_cred): Create the krb5 context + earlier. Call krb5_gss_validate_cred_1. Make sure the mutex is + locked. + * copy_ccache.c (gss_krb5_copy_ccache): Lock the mutex in the + source credential. + * init_sec_context.c (get_credentials, new_connection): Check that + the mutex is locked. + (mutual_auth): Delete unused credential argument. + (krb5_gss_init_sec_context): Lock the mutex. + * inq_cred.c (krb5_gss_inquire_cred): Lock the mutex. + * rel_cred.c (krb5_gss_release_cred): Destroy the mutex. + * set_allowable_enctypes.c (gss_krb5_set_allowable_enctypes): Lock + the mutex. + * val_cred.c (krb5_gss_validate_cred_1): New function, most of old + krb5_gss_validate_cred but requires that the krb5 context be + supplied, and returns with the credential mutex still locked if + successful, so the caller needn't re-lock it. + (krb5_gss_validate_cred): Use it. + * set_ccache.c (gss_krb5_ccache_name): Don't make a copy of the string returned by kg_get_ccache_name. Simplify some calls using a temporary error code variable. diff --git a/src/lib/gssapi/krb5/accept_sec_context.c b/src/lib/gssapi/krb5/accept_sec_context.c index 2b7d8494c..219d9da06 100644 --- a/src/lib/gssapi/krb5/accept_sec_context.c +++ b/src/lib/gssapi/krb5/accept_sec_context.c @@ -160,9 +160,17 @@ rd_and_store_for_creds(context, auth_context, inbuf, out_cred) /* zero it out... */ memset(cred, 0, sizeof(krb5_gss_cred_id_rec)); + retval = k5_mutex_init(&cred->lock); + if (retval) { + xfree(cred); + cred = NULL; + goto cleanup; + } + /* copy the client principle into it... */ if ((retval = krb5_copy_principal(context, creds[0]->client, &(cred->princ)))) { + k5_mutex_destroy(&cred->lock); retval = ENOMEM; /* out of memory? */ xfree(cred); /* clean up memory on failure */ cred = NULL; diff --git a/src/lib/gssapi/krb5/acquire_cred.c b/src/lib/gssapi/krb5/acquire_cred.c index 0b0b57a31..b65fc0962 100644 --- a/src/lib/gssapi/krb5/acquire_cred.c +++ b/src/lib/gssapi/krb5/acquire_cred.c @@ -425,6 +425,16 @@ krb5_gss_acquire_cred(minor_status, desired_name, time_req, cred->keytab = NULL; cred->ccache = NULL; + code = k5_mutex_init(&cred->lock); + if (code) { + *minor_status = ret; + krb5_free_context(context); + return GSS_S_FAILURE; + } + /* Note that we don't need to lock this GSSAPI credential record + here, because no other thread can gain access to it until we + return it. */ + if ((cred_usage != GSS_C_INITIATE) && (cred_usage != GSS_C_ACCEPT) && (cred_usage != GSS_C_BOTH)) { diff --git a/src/lib/gssapi/krb5/add_cred.c b/src/lib/gssapi/krb5/add_cred.c index 4ec230a1e..6cb6605d9 100644 --- a/src/lib/gssapi/krb5/add_cred.c +++ b/src/lib/gssapi/krb5/add_cred.c @@ -113,12 +113,21 @@ krb5_gss_add_cred(minor_status, input_cred_handle, return(GSS_S_DUPLICATE_ELEMENT); } - /* verify the credential */ - if (GSS_ERROR(major_status = - krb5_gss_validate_cred(minor_status, input_cred_handle))) - return(major_status); + code = krb5_init_context(&context); + if (code) { + *minor_status = code; + return GSS_S_FAILURE; + } + + major_status = krb5_gss_validate_cred_1(minor_status, input_cred_handle, + context); + if (GSS_ERROR(major_status)) { + krb5_free_context(context); + return major_status; + } cred = (krb5_gss_cred_id_t) input_cred_handle; + k5_mutex_assert_locked(&cred->lock); /* check if the cred_usage is equal or "less" than the passed-in cred if copying */ @@ -127,6 +136,7 @@ krb5_gss_add_cred(minor_status, input_cred_handle, ((cred->usage == GSS_C_BOTH) && (output_cred_handle != NULL)))) { *minor_status = (OM_uint32) G_BAD_USAGE; + krb5_free_context(context); return(GSS_S_FAILURE); } @@ -135,16 +145,14 @@ krb5_gss_add_cred(minor_status, input_cred_handle, if ((g_OID_equal(desired_mech, gss_mech_krb5_old) && cred->prerfc_mech) || (g_OID_equal(desired_mech, gss_mech_krb5) && cred->rfc_mech)) { *minor_status = 0; + krb5_free_context(context); return(GSS_S_DUPLICATE_ELEMENT); } - code = krb5_init_context(&context); - if (code) { - *minor_status = code; + if (GSS_ERROR(kg_sync_ccache_name(context, minor_status))) { + krb5_free_context(context); return GSS_S_FAILURE; } - if (GSS_ERROR(kg_sync_ccache_name(context, minor_status))) - return GSS_S_FAILURE; /* verify the desired_name */ diff --git a/src/lib/gssapi/krb5/copy_ccache.c b/src/lib/gssapi/krb5/copy_ccache.c index d20f72c77..fd408d7fb 100644 --- a/src/lib/gssapi/krb5/copy_ccache.c +++ b/src/lib/gssapi/krb5/copy_ccache.c @@ -19,19 +19,27 @@ gss_krb5_copy_ccache(minor_status, cred_handle, out_ccache) return(stat); k5creds = (krb5_gss_cred_id_t) cred_handle; + code = k5_mutex_lock(&k5creds->lock); + if (code) { + *minor_status = code; + return GSS_S_FAILURE; + } if (k5creds->usage == GSS_C_ACCEPT) { + k5_mutex_unlock(&k5creds->lock); *minor_status = (OM_uint32) G_BAD_USAGE; return(GSS_S_FAILURE); } code = krb5_init_context(&context); if (code) { + k5_mutex_unlock(&k5creds->lock); *minor_status = code; return GSS_S_FAILURE; } code = krb5_cc_start_seq_get(context, k5creds->ccache, &cursor); if (code) { + k5_mutex_unlock(&k5creds->lock); *minor_status = code; krb5_free_context(context); return(GSS_S_FAILURE); @@ -39,7 +47,7 @@ gss_krb5_copy_ccache(minor_status, cred_handle, out_ccache) while (!code && !krb5_cc_next_cred(context, k5creds->ccache, &cursor, &creds)) code = krb5_cc_store_cred(context, out_ccache, &creds); krb5_cc_end_seq_get(context, k5creds->ccache, &cursor); - + k5_mutex_unlock(&k5creds->lock); krb5_free_context(context); if (code) { *minor_status = code; diff --git a/src/lib/gssapi/krb5/gssapiP_krb5.h b/src/lib/gssapi/krb5/gssapiP_krb5.h index 2e5e3292b..cf78b94b8 100644 --- a/src/lib/gssapi/krb5/gssapiP_krb5.h +++ b/src/lib/gssapi/krb5/gssapiP_krb5.h @@ -141,6 +141,9 @@ enum qop { typedef krb5_principal krb5_gss_name_t; typedef struct _krb5_gss_cred_id_rec { + /* protect against simultaneous accesses */ + k5_mutex_t lock; + /* name/type of credential */ gss_cred_usage_t usage; krb5_principal princ; /* this is not interned as a gss_name_t */ @@ -607,9 +610,12 @@ OM_uint32 krb5_gss_validate_cred gss_cred_id_t /* cred */ ); -gss_OID krb5_gss_convert_static_mech_oid -(gss_OID oid - ); +OM_uint32 +krb5_gss_validate_cred_1(OM_uint32 * /* minor_status */, + gss_cred_id_t /* cred_handle */, + krb5_context /* context */); + +gss_OID krb5_gss_convert_static_mech_oid(gss_OID oid); krb5_error_code gss_krb5int_make_seal_token_v3(krb5_context, krb5_gss_ctx_id_rec *, diff --git a/src/lib/gssapi/krb5/init_sec_context.c b/src/lib/gssapi/krb5/init_sec_context.c index 90c3e7d72..adc5ad918 100644 --- a/src/lib/gssapi/krb5/init_sec_context.c +++ b/src/lib/gssapi/krb5/init_sec_context.c @@ -102,6 +102,7 @@ static krb5_error_code get_credentials(context, cred, server, now, krb5_error_code code; krb5_creds in_creds; + k5_mutex_assert_locked(&cred->lock); memset((char *) &in_creds, 0, sizeof(krb5_creds)); if ((code = krb5_copy_principal(context, cred->princ, &in_creds.client))) @@ -262,7 +263,7 @@ make_ap_req_v1(context, ctx, cred, k_cred, chan_bindings, mech_type, token) unsigned char *t; unsigned int tlen; - + k5_mutex_assert_locked(&cred->lock); ap_req.data = 0; /* compute the hash of the channel bindings */ @@ -463,6 +464,7 @@ new_connection( krb5_timestamp now; gss_buffer_desc token; + k5_mutex_assert_locked(&cred->lock); major_status = GSS_S_FAILURE; token.length = 0; token.value = NULL; @@ -647,7 +649,6 @@ fail: static OM_uint32 mutual_auth( OM_uint32 *minor_status, - krb5_gss_cred_id_t cred, gss_ctx_id_t *context_handle, gss_name_t target_name, gss_OID mech_type, @@ -890,6 +891,12 @@ krb5_gss_init_sec_context(minor_status, claimant_cred_handle, } cred = (krb5_gss_cred_id_t) claimant_cred_handle; } + kerr = k5_mutex_lock(&cred->lock); + if (kerr) { + krb5_free_context(context); + *minor_status = kerr; + return GSS_S_FAILURE; + } /* verify the mech_type */ @@ -914,6 +921,7 @@ krb5_gss_init_sec_context(minor_status, claimant_cred_handle, } if (err) { + k5_mutex_unlock(&cred->lock); if (claimant_cred_handle == GSS_C_NO_CREDENTIAL) krb5_gss_release_cred(minor_status, (gss_cred_id_t)cred); *minor_status = 0; @@ -932,12 +940,15 @@ krb5_gss_init_sec_context(minor_status, claimant_cred_handle, input_token, actual_mech_type, output_token, ret_flags, time_rec, context, default_mech); + k5_mutex_unlock(&cred->lock); if (*context_handle == GSS_C_NO_CONTEXT) krb5_free_context(context); else ((krb5_gss_ctx_id_rec *) *context_handle)->k5_context = context; } else { - major_status = mutual_auth(minor_status, cred, context_handle, + /* mutual_auth doesn't care about the credentials */ + k5_mutex_unlock(&cred->lock); + major_status = mutual_auth(minor_status, context_handle, target_name, mech_type, req_flags, time_req, input_chan_bindings, input_token, actual_mech_type, diff --git a/src/lib/gssapi/krb5/inq_cred.c b/src/lib/gssapi/krb5/inq_cred.c index b0a426aa5..4125dd5e4 100644 --- a/src/lib/gssapi/krb5/inq_cred.c +++ b/src/lib/gssapi/krb5/inq_cred.c @@ -129,6 +129,12 @@ krb5_gss_inquire_cred(minor_status, cred_handle, name, lifetime_ret, goto fail; } + code = k5_mutex_lock(&cred->lock); + if (code != 0) { + *minor_status = code; + ret = GSS_S_FAILURE; + goto fail; + } if (cred->tgt_expire > 0) { if ((lifetime = cred->tgt_expire - now) < 0) lifetime = 0; @@ -139,6 +145,7 @@ krb5_gss_inquire_cred(minor_status, cred_handle, name, lifetime_ret, if (name) { if (cred->princ && (code = krb5_copy_principal(context, cred->princ, &ret_name))) { + k5_mutex_unlock(&cred->lock); *minor_status = code; ret = GSS_S_FAILURE; goto fail; @@ -156,6 +163,7 @@ krb5_gss_inquire_cred(minor_status, cred_handle, name, lifetime_ret, GSS_ERROR(ret = generic_gss_add_oid_set_member(minor_status, (gss_OID) gss_mech_krb5, &mechs)))) { + k5_mutex_unlock(&cred->lock); krb5_free_principal(context, ret_name); /* *minor_status set above */ goto fail; @@ -164,6 +172,7 @@ krb5_gss_inquire_cred(minor_status, cred_handle, name, lifetime_ret, if (name) { if (! kg_save_name((gss_name_t) ret_name)) { + k5_mutex_unlock(&cred->lock); (void) gss_release_oid_set(minor_status, &mechs); krb5_free_principal(context, ret_name); *minor_status = (OM_uint32) G_VALIDATE_FAILED; @@ -178,6 +187,7 @@ krb5_gss_inquire_cred(minor_status, cred_handle, name, lifetime_ret, if (cred_usage) *cred_usage = cred->usage; + k5_mutex_unlock(&cred->lock); if (mechanisms) *mechanisms = mechs; diff --git a/src/lib/gssapi/krb5/rel_cred.c b/src/lib/gssapi/krb5/rel_cred.c index fd8bb8945..ed88abb43 100644 --- a/src/lib/gssapi/krb5/rel_cred.c +++ b/src/lib/gssapi/krb5/rel_cred.c @@ -51,6 +51,9 @@ krb5_gss_release_cred(minor_status, cred_handle) cred = *cred_handle; + k5_mutex_destroy(&cred->lock); + /* ignore error destroying mutex */ + if (cred->ccache) code1 = krb5_cc_close(context, cred->ccache); else diff --git a/src/lib/gssapi/krb5/set_allowable_enctypes.c b/src/lib/gssapi/krb5/set_allowable_enctypes.c index aa3636e13..88cae714a 100644 --- a/src/lib/gssapi/krb5/set_allowable_enctypes.c +++ b/src/lib/gssapi/krb5/set_allowable_enctypes.c @@ -95,9 +95,13 @@ gss_krb5_set_allowable_enctypes(OM_uint32 *minor_status, } } } else { + kerr = k5_mutex_lock(&cred->lock); + if (kerr) + goto error_out; if (cred->req_enctypes) free(cred->req_enctypes); cred->req_enctypes = NULL; + k5_mutex_unlock(&cred->lock); return GSS_S_COMPLETE; } @@ -110,9 +114,13 @@ gss_krb5_set_allowable_enctypes(OM_uint32 *minor_status, kerr = ENOMEM; goto error_out; } + kerr = k5_mutex_lock(&cred->lock); + if (kerr) + goto error_out; if (cred->req_enctypes) free(cred->req_enctypes); cred->req_enctypes = new_ktypes; + k5_mutex_unlock(&cred->lock); /* Success! */ return GSS_S_COMPLETE; diff --git a/src/lib/gssapi/krb5/val_cred.c b/src/lib/gssapi/krb5/val_cred.c index fef848044..1fdb3c298 100644 --- a/src/lib/gssapi/krb5/val_cred.c +++ b/src/lib/gssapi/krb5/val_cred.c @@ -31,47 +31,68 @@ */ OM_uint32 -krb5_gss_validate_cred(minor_status, cred_handle) - OM_uint32 *minor_status; - gss_cred_id_t cred_handle; +krb5_gss_validate_cred_1(OM_uint32 *minor_status, gss_cred_id_t cred_handle, + krb5_context context) { - krb5_context context; krb5_gss_cred_id_t cred; krb5_error_code code; krb5_principal princ; - - code = krb5_init_context(&context); - if (code) { - *minor_status = code; - return GSS_S_FAILURE; - } if (!kg_validate_cred_id(cred_handle)) { *minor_status = (OM_uint32) G_VALIDATE_FAILED; - krb5_free_context(context); return(GSS_S_CALL_BAD_STRUCTURE|GSS_S_DEFECTIVE_CREDENTIAL); } cred = (krb5_gss_cred_id_t) cred_handle; + code = k5_mutex_lock(&cred->lock); + if (code) { + *minor_status = code; + return GSS_S_FAILURE; + } + if (cred->ccache) { if ((code = krb5_cc_get_principal(context, cred->ccache, &princ))) { + k5_mutex_unlock(&cred->lock); *minor_status = code; - krb5_free_context(context); return(GSS_S_DEFECTIVE_CREDENTIAL); } if (!krb5_principal_compare(context, princ, cred->princ)) { + k5_mutex_unlock(&cred->lock); *minor_status = KG_CCACHE_NOMATCH; - krb5_free_context(context); return(GSS_S_DEFECTIVE_CREDENTIAL); } (void)krb5_free_principal(context, princ); } - krb5_free_context(context); *minor_status = 0; return GSS_S_COMPLETE; } +OM_uint32 +krb5_gss_validate_cred(minor_status, cred_handle) + OM_uint32 *minor_status; + gss_cred_id_t cred_handle; +{ + krb5_context context; + krb5_error_code code; + OM_uint32 maj; + + code = krb5_init_context(&context); + if (code) { + *minor_status = code; + return GSS_S_FAILURE; + } + + maj = krb5_gss_validate_cred_1(minor_status, cred_handle, context); + if (maj == 0) { + krb5_gss_cred_id_t cred = (krb5_gss_cred_id_t) cred_handle; + k5_mutex_assert_locked(&cred->lock); + k5_mutex_unlock(&cred->lock); + } + krb5_free_context(context); + return maj; +} + -- 2.26.2