Cache S4U2Proxy requests by second ticket

krb5_get_credentials() does not know the client principal for an
S4U2Proxy request until the end, because it is in the encrypted part
of the evidence ticket.  However, we can check the cache by second
ticket, since all S4U2Proxy requests in a cache will generally be made
with the same evidence ticket.

In the ccache types, allow mcreds->client and mcreds->server to be
NULL (as Heimdal does) to ignore them for the purpose of matching.  In
krb5int_construct_matching_creds(), set mcreds->client to NULL for
S4U2Proxy requests.  Add a cache check to
k5_get_proxy_cred_from_kdc(), and remove the cache check from
krb5_get_credentials_for_proxy() and the krb5 mech's
get_credentials().

In get_proxy_cred_from_kdc(), fix a bug where cross-realm S4U2Proxy
would cache the evidence ticket used in the final request, rather than
the original evidence ticket.

[ghudson@mit.edu: debugged cache check and cross-realm caching;
switched from new flag to null matching cred principals; wrote commit
message]

ticket: 8931 (new)

5 files changed, 58 insertions, 51 deletions

diff --git a/src/lib/krb5/ccache/cc_retr.c b/src/lib/krb5/ccache/cc_retr.c
index 2c50c9c..4328b7d 100644
--- a/src/lib/krb5/ccache/cc_retr.c
+++ b/src/lib/krb5/ccache/cc_retr.c
@@ -58,15 +58,14 @@ static krb5_boolean
 princs_match(krb5_context context, krb5_flags whichfields,
              const krb5_creds *mcreds, const krb5_creds *creds)
 {
-    krb5_principal_data princ;
-
-    if (!krb5_principal_compare(context, mcreds->client, creds->client))
+    if (mcreds->client != NULL &&
+        !krb5_principal_compare(context, mcreds->client, creds->client))
         return FALSE;
+    if (mcreds->server == NULL)
+        return TRUE;
     if (whichfields & KRB5_TC_MATCH_SRV_NAMEONLY) {
-        /* Ignore the server realm. */
-        princ = *mcreds->server;
-        princ.realm = creds->server->realm;
-        return krb5_principal_compare(context, &princ, creds->server);
+        return krb5_principal_compare_any_realm(context, mcreds->server,
+                                                creds->server);
     } else {
         return krb5_principal_compare(context, mcreds->server, creds->server);
     }
diff --git a/src/lib/krb5/ccache/ccapi/stdcc_util.c b/src/lib/krb5/ccache/ccapi/stdcc_util.c
index c3627fe..b7d728e 100644
--- a/src/lib/krb5/ccache/ccapi/stdcc_util.c
+++ b/src/lib/krb5/ccache/ccapi/stdcc_util.c
@@ -945,8 +945,13 @@ standard_fields_match(context, mcreds, creds)
     krb5_context context;
     const krb5_creds *mcreds, *creds;
 {
-    return (krb5_principal_compare(context, mcreds->client,creds->client) &&
-            krb5_principal_compare(context, mcreds->server,creds->server));
+    if (mcreds->client != NULL &&
+        !krb5_principal_compare(context, mcreds->client, creds->client))
+        return FALSE;
+    if (mcreds->server != NULL &&
+        !krb5_principal_compare(context, mcreds->server,creds->server))
+        return FALSE;
+    return TRUE;
 }
 
 /* only match the server name portion, not the server realm portion */
@@ -956,19 +961,14 @@ srvname_match(context, mcreds, creds)
     krb5_context context;
     const krb5_creds *mcreds, *creds;
 {
-    krb5_boolean retval;
-    krb5_principal_data p1, p2;
-
-    retval = krb5_principal_compare(context, mcreds->client,creds->client);
-    if (retval != TRUE)
-        return retval;
-    /*
-     * Hack to ignore the server realm for the purposes of the compare.
-     */
-    p1 = *mcreds->server;
-    p2 = *creds->server;
-    p1.realm = p2.realm;
-    return krb5_principal_compare(context, &p1, &p2);
+    if (mcreds->client != NULL &&
+        !krb5_principal_compare(context, mcreds->client, creds->client))
+        return FALSE;
+    if (mcreds->server != NULL &&
+        !krb5_principal_compare_any_realm(context, mcreds->server,
+                                          creds->server))
+        return FALSE;
+    return TRUE;
 }
 
 
diff --git a/src/lib/krb5/ccache/ccfns.c b/src/lib/krb5/ccache/ccfns.c
index 62a6983..59982b7 100644
--- a/src/lib/krb5/ccache/ccfns.c
+++ b/src/lib/krb5/ccache/ccfns.c
@@ -96,7 +96,8 @@ krb5_cc_retrieve_cred(krb5_context context, krb5_ccache cache,
     TRACE_CC_RETRIEVE(context, cache, mcreds, ret);
     if (ret != KRB5_CC_NOTFOUND)
         return ret;
-    if (!krb5_is_referral_realm(&mcreds->server->realm))
+    if (mcreds->client == NULL || mcreds->server == NULL ||
+        !krb5_is_referral_realm(&mcreds->server->realm))
         return ret;
 
     /*
diff --git a/src/lib/krb5/krb/get_creds.c b/src/lib/krb5/krb/get_creds.c
index dc0aef6..b3f01be 100644
--- a/src/lib/krb5/krb/get_creds.c
+++ b/src/lib/krb5/krb/get_creds.c
@@ -102,6 +102,11 @@ krb5int_construct_matching_creds(krb5_context context, krb5_flags options,
             return KRB5_NO_2ND_TKT;
     }
 
+    /* For S4U2Proxy requests we don't know the impersonated client in this
+     * API, but matching against the second ticket is good enough. */
+    if (options & KRB5_GC_CONSTRAINED_DELEGATION)
+        mcreds->client = NULL;
+
     return 0;
 }
 
diff --git a/src/lib/krb5/krb/s4u_creds.c b/src/lib/krb5/krb/s4u_creds.c
index 2f12a17..00ff613 100644
--- a/src/lib/krb5/krb/s4u_creds.c
+++ b/src/lib/krb5/krb/s4u_creds.c
@@ -1119,6 +1119,13 @@ get_proxy_cred_from_kdc(krb5_context context, krb5_flags options,
             code = KRB5KRB_AP_WRONG_PRINC;
             goto cleanup;
         }
+
+        /* Put the original evidence ticket in the output creds. */
+        krb5_free_data_contents(context, &tkt->second_ticket);
+        code = krb5int_copy_data_contents(context, &in_creds->second_ticket,
+                                          &tkt->second_ticket);
+        if (code)
+            goto cleanup;
     }
 
     /* Note the authdata we asked for in the output creds. */
@@ -1145,11 +1152,33 @@ k5_get_proxy_cred_from_kdc(krb5_context context, krb5_flags options,
 {
     krb5_error_code code;
     krb5_const_principal canonprinc;
-    krb5_creds copy, *creds;
+    krb5_creds mcreds, copy, *creds, *ncreds;
+    krb5_flags fields;
     struct canonprinc iter = { in_creds->server, .no_hostrealm = TRUE };
 
     *out_creds = NULL;
 
+    code = krb5int_construct_matching_creds(context, options, in_creds,
+                                            &mcreds, &fields);
+    if (code != 0)
+        return code;
+
+    ncreds = calloc(1, sizeof(*ncreds));
+    if (ncreds == NULL)
+        return ENOMEM;
+    ncreds->magic = KV5M_CRED;
+
+    code = krb5_cc_retrieve_cred(context, ccache, fields, &mcreds, ncreds);
+    if (code) {
+        free(ncreds);
+    } else {
+        *out_creds = ncreds;
+    }
+
+    if ((code != KRB5_CC_NOTFOUND && code != KRB5_CC_NOT_KTYPE) ||
+        options & KRB5_GC_CACHED)
+        return code;
+
     copy = *in_creds;
     while ((code = k5_canonprinc(context, &iter, &canonprinc)) == 0 &&
            canonprinc != NULL) {
@@ -1195,9 +1224,6 @@ krb5_get_credentials_for_proxy(krb5_context context,
                                krb5_creds **out_creds)
 {
     krb5_error_code code;
-    krb5_creds mcreds;
-    krb5_creds *ncreds = NULL;
-    krb5_flags fields;
     krb5_data *evidence_tkt_data = NULL;
     krb5_creds s4u_creds;
 
@@ -1219,30 +1245,6 @@ krb5_get_credentials_for_proxy(krb5_context context,
         goto cleanup;
     }
 
-    code = krb5int_construct_matching_creds(context, options, in_creds,
-                                            &mcreds, &fields);
-    if (code != 0)
-        goto cleanup;
-
-    ncreds = calloc(1, sizeof(*ncreds));
-    if (ncreds == NULL) {
-        code = ENOMEM;
-        goto cleanup;
-    }
-    ncreds->magic = KV5M_CRED;
-
-    code = krb5_cc_retrieve_cred(context, ccache, fields, &mcreds, ncreds);
-    if (code != 0) {
-        free(ncreds);
-        ncreds = in_creds;
-    } else {
-        *out_creds = ncreds;
-    }
-
-    if ((code != KRB5_CC_NOTFOUND && code != KRB5_CC_NOT_KTYPE)
-        || options & KRB5_GC_CACHED)
-        goto cleanup;
-
     code = encode_krb5_ticket(evidence_tkt, &evidence_tkt_data);
     if (code != 0)
         goto cleanup;