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

diff --git a/src/lib/krb5/krb/get_creds.c b/src/lib/krb5/krb/get_creds.c
index dad3e1a9175de6c80cac15654da987a1327288ec..771153ea9f4f33f543d2f2fa412e4793139c5129 100644 (file)
--- 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 cc0c9f2de03f04ac292be5d5d5a34c62a4378f4f..1210e4f28524d36a0c3f18a26af5b9c7240e74fb 100644 (file)
--- 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 678a7714ed00d73cb0140a4d8bd286bbe96ef63a..883d33cc777e007bf6cd2a984415c034e2a4719e 100644 (file)
--- 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;