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

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  <raeburn@mit.edu>
 
+	* 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;
+}
+