Clean up krb5_get_credentials:

  * Use the current coding practice for output parameters.
  * Rename the helper function krb5_get_credentials_core to
    krb5int_construct_matching_creds and document it.
  * Don't fail out if we fail to cache intermediate tgts.
  * Simplify conditional logic and variable handling.  ncreds is now
    always a temporary holder for the resulting credentials.

git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@22787 dc483132-0cff-0310-8789-dd5450dbe970

3 files changed, 59 insertions, 65 deletions

diff --git a/src/lib/krb5/krb/get_creds.c b/src/lib/krb5/krb/get_creds.c
index dad3e1a..771153e 100644
--- a/src/lib/krb5/krb/get_creds.c
+++ b/src/lib/krb5/krb/get_creds.c
@@ -46,10 +46,16 @@
 #include "k5-int.h"
 #include "int-proto.h"
 
+/*
+ * Set *mcreds and *fields to a matching credential and field set for
+ * use with krb5_cc_retrieve_cred, based on a set of input credentials
+ * and options.  The fields of *mcreds will be aliased to the fields
+ * of in_creds, so the contents of *mcreds should not be freed.
+ */
 krb5_error_code
-krb5_get_credentials_core(krb5_context context, krb5_flags options,
-			  krb5_creds *in_creds, krb5_creds *mcreds,
-			  krb5_flags *fields)
+krb5int_construct_matching_creds(krb5_context context, krb5_flags options,
+				 krb5_creds *in_creds, krb5_creds *mcreds,
+				 krb5_flags *fields)
 {
     if (!in_creds || !in_creds->server || !in_creds->client)
         return EINVAL;
@@ -77,7 +83,7 @@ krb5_get_credentials_core(krb5_context context, krb5_flags options,
 	int i;
 
 	*fields |= KRB5_TC_MATCH_KTYPE;
-	ret = krb5_get_tgs_ktypes (context, mcreds->server, &ktypes);
+	ret = krb5_get_tgs_ktypes(context, mcreds->server, &ktypes);
 	for (i = 0; ktypes[i]; i++)
 	    if (ktypes[i] == mcreds->keyblock.enctype)
 		break;
@@ -109,47 +115,43 @@ krb5_get_credentials(krb5_context context, krb5_flags options,
 		     krb5_creds **out_creds)
 {
     krb5_error_code retval;
-    krb5_creds mcreds;
-    krb5_creds *ncreds;
-    krb5_creds **tgts;
+    krb5_creds mcreds, *ncreds, **tgts, **tgts_iter;
     krb5_flags fields;
-    int not_ktype;
-    int kdcopt = 0;
+    int not_ktype, kdcopt = 0;
 
+    *out_creds = NULL;
+
+    /*
+     * See if we already have the ticket cached. To do this usefully
+     * for constrained delegation, we would need to look inside
+     * second_ticket, which we can't do.
+     */
     if ((options & KRB5_GC_CONSTRAINED_DELEGATION) == 0) {
-	retval = krb5_get_credentials_core(context, options,
-					   in_creds,
-					   &mcreds, &fields);
+	retval = krb5int_construct_matching_creds(context, options, in_creds,
+						  &mcreds, &fields);
 
 	if (retval)
 	    return retval;
 
-	if ((ncreds = (krb5_creds *)malloc(sizeof(krb5_creds))) == NULL)
+	ncreds = malloc(sizeof(krb5_creds));
+	if (!ncreds)
 	    return ENOMEM;
 
 	memset(ncreds, 0, sizeof(krb5_creds));
 	ncreds->magic = KV5M_CREDS;
 
-	/* The caller is now responsible for cleaning up in_creds */
-	if ((retval = krb5_cc_retrieve_cred(context, ccache, fields, &mcreds,
-					    ncreds))) {
-	    free(ncreds);
-	    ncreds = in_creds;
-	} else {
+	retval = krb5_cc_retrieve_cred(context, ccache, fields, &mcreds,
+				       ncreds);
+	if (retval == 0) {
 	    *out_creds = ncreds;
+	    return 0;
 	}
-    } else {
-	/*
-	 * To do this usefully for constrained delegation, we would
-	 * need to look inside second_ticket, which we can't do.
-	 */
-	ncreds = in_creds;
-	retval = KRB5_CC_NOTFOUND;
-    }
-
-    if ((retval != KRB5_CC_NOTFOUND && retval != KRB5_CC_NOT_KTYPE)
-	|| options & KRB5_GC_CACHED)
-	return retval;
+	free(ncreds);
+	if ((retval != KRB5_CC_NOTFOUND && retval != KRB5_CC_NOT_KTYPE)
+	    || options & KRB5_GC_CACHED)
+	    return retval;
+    } else if (options & KRB5_GC_CACHED)
+	return KRB5_CC_NOTFOUND;
 
     if (retval == KRB5_CC_NOT_KTYPE)
 	not_ktype = 1;
@@ -168,27 +170,15 @@ krb5_get_credentials(krb5_context context, krb5_flags options,
 	kdcopt |= KDC_OPT_FORWARDABLE | KDC_OPT_CNAME_IN_ADDL_TKT;
     }
 
-    retval = krb5_get_cred_from_kdc_opt(context, ccache, ncreds,
-					out_creds, &tgts, kdcopt);
+    retval = krb5_get_cred_from_kdc_opt(context, ccache, in_creds,
+					&ncreds, &tgts, kdcopt);
     if (tgts) {
-	register int i = 0;
-	krb5_error_code rv2;
-	while (tgts[i]) {
-	    if ((rv2 = krb5_cc_store_cred(context, ccache, tgts[i]))) {
-		retval = rv2;
-		break;
-	    }
-	    i++;
-	}
+	/* Attempt to cache intermediate ticket-granting tickets. */
+	for (tgts_iter = tgts; *tgts_iter; tgts_iter++)
+	    (void) krb5_cc_store_cred(context, ccache, *tgts_iter);
 	krb5_free_tgt_creds(context, tgts);
     }
-    if (!retval && (options & KRB5_GC_CONSTRAINED_DELEGATION)) {
-	if (((*out_creds)->ticket_flags & TKT_FLG_FORWARDABLE) == 0) {
-	    retval = KRB5_TKT_NOT_FORWARDABLE;
-	    krb5_free_creds(context, *out_creds);
-	    *out_creds = NULL;
-	}
-    }
+
     /*
      * Translate KRB5_CC_NOTFOUND if we previously got
      * KRB5_CC_NOT_KTYPE from krb5_cc_retrieve_cred(), in order to
@@ -202,19 +192,23 @@ krb5_get_credentials(krb5_context context, krb5_flags options,
      */
     if ((retval == KRB5_CC_NOTFOUND || retval == KRB5_CC_NOT_KTYPE)
 	&& not_ktype)
-	retval = KRB5_CC_NOT_KTYPE;
-
-    if (!retval && (options & KRB5_GC_NO_STORE) == 0) {
-        /* the purpose of the krb5_get_credentials call is to 
-         * obtain a set of credentials for the caller.  the 
-         * krb5_cc_store_cred() call is to optimize performance
-         * for future calls.  Ignore any errors, since the credentials
-         * are still valid even if we fail to store them in the cache.
-         */
-	krb5_cc_store_cred(context, ccache, *out_creds);
+	return KRB5_CC_NOT_KTYPE;
+    else if (retval)
+	return retval;
+
+    if ((options & KRB5_GC_CONSTRAINED_DELEGATION)
+	&& (ncreds->ticket_flags & TKT_FLG_FORWARDABLE) == 0) {
+	/* This ticket won't work for constrained delegation. */
+	krb5_free_creds(context, ncreds);
+	return KRB5_TKT_NOT_FORWARDABLE;
     }
 
-    return retval;
+    /* Attempt to cache the returned ticket. */
+    if (!(options & KRB5_GC_NO_STORE))
+	(void) krb5_cc_store_cred(context, ccache, ncreds);
+
+    *out_creds = ncreds;
+    return 0;
 }
 
 #define INT_GC_VALIDATE 1
diff --git a/src/lib/krb5/krb/int-proto.h b/src/lib/krb5/krb/int-proto.h
index cc0c9f2..1210e4f 100644
--- a/src/lib/krb5/krb/int-proto.h
+++ b/src/lib/krb5/krb/int-proto.h
@@ -60,9 +60,9 @@ krb5_get_cred_from_kdc_opt(krb5_context context, krb5_ccache ccache,
 			   krb5_creds ***tgts, int kdcopt);
 
 krb5_error_code
-krb5_get_credentials_core(krb5_context context, krb5_flags options,
-			  krb5_creds *in_creds, krb5_creds *mcreds,
-			  krb5_flags *fields);
+krb5_construct_matching_creds(krb5_context context, krb5_flags options,
+			      krb5_creds *in_creds, krb5_creds *mcreds,
+			      krb5_flags *fields);
 
 #define in_clock_skew(date, now) (labs((date)-(now)) < context->clockskew)
 
diff --git a/src/lib/krb5/krb/s4u_creds.c b/src/lib/krb5/krb/s4u_creds.c
index 678a771..883d33c 100644
--- a/src/lib/krb5/krb/s4u_creds.c
+++ b/src/lib/krb5/krb/s4u_creds.c
@@ -766,8 +766,8 @@ krb5_get_credentials_for_proxy(krb5_context context,
         goto cleanup;
     }
 
-    code = krb5_get_credentials_core(context, options, in_creds,
-                                     &mcreds, &fields);
+    code = krb5int_construct_matching_creds(context, options, in_creds,
+                                            &mcreds, &fields);
     if (code != 0)
         goto cleanup;