]> git.ipfire.org Git - thirdparty/krb5.git/commitdiff
Cache S4U2Proxy requests by second ticket
authorIsaac Boukris <iboukris@gmail.com>
Mon, 20 Jul 2020 22:40:06 +0000 (00:40 +0200)
committerGreg Hudson <ghudson@mit.edu>
Fri, 7 Aug 2020 17:50:01 +0000 (13:50 -0400)
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)

src/lib/gssapi/krb5/init_sec_context.c
src/lib/krb5/ccache/cc_retr.c
src/lib/krb5/ccache/ccapi/stdcc_util.c
src/lib/krb5/ccache/ccfns.c
src/lib/krb5/krb/get_creds.c
src/lib/krb5/krb/s4u_creds.c
src/tests/s4u2proxy.c

index 3f77157c98eaf9d8634916c8493de51e93cc456a..8236560377fda35ccb52f97aa417e353848d850b 100644 (file)
@@ -165,44 +165,37 @@ static krb5_error_code get_credentials(context, cred, server, now,
             goto cleanup;
     }
 
-    /*
-     * For IAKERB or constrained delegation, only check the cache in this step.
-     * For IAKERB we will ask the server to make any necessary TGS requests;
-     * for constrained delegation we will adjust in_creds and make an S4U2Proxy
-     * request below if the cache lookup fails.
-     */
-    if (cred->impersonator != NULL || cred->iakerb_mech)
+    /* Try constrained delegation if we have proxy credentials. */
+    if (cred->impersonator != NULL) {
+        /* If we are trying to get a ticket to ourselves, we should use the
+         * the evidence ticket directly from cache. */
+        if (krb5_principal_compare(context, cred->impersonator,
+                                   server->princ)) {
+            flags |= KRB5_GC_CACHED;
+        } else {
+            memset(&mcreds, 0, sizeof(mcreds));
+            mcreds.magic = KV5M_CREDS;
+            mcreds.server = cred->impersonator;
+            mcreds.client = cred->name->princ;
+            code = krb5_cc_retrieve_cred(context, cred->ccache,
+                                         KRB5_TC_MATCH_AUTHDATA, &mcreds,
+                                         &evidence_creds);
+            if (code)
+                goto cleanup;
+
+            in_creds.client = cred->impersonator;
+            in_creds.second_ticket = evidence_creds.ticket;
+            flags = KRB5_GC_CANONICALIZE | KRB5_GC_CONSTRAINED_DELEGATION;
+        }
+    }
+
+    /* For IAKERB, only check the cache in this step.  We will ask the server
+     * to make any necessary TGS requests. */
+    if (cred->iakerb_mech)
         flags |= KRB5_GC_CACHED;
 
     code = krb5_get_credentials(context, flags, cred->ccache,
                                 &in_creds, &result_creds);
-
-    /*
-     * Try constrained delegation if we have proxy credentials, unless
-     * we are trying to get a ticket to ourselves (in which case we could
-     * just use the evidence ticket directly from cache).
-     */
-    if (code == KRB5_CC_NOTFOUND && cred->impersonator != NULL &&
-        !cred->iakerb_mech &&
-        !krb5_principal_compare(context, cred->impersonator, server->princ)) {
-
-        memset(&mcreds, 0, sizeof(mcreds));
-        mcreds.magic = KV5M_CREDS;
-        mcreds.server = cred->impersonator;
-        mcreds.client = cred->name->princ;
-        code = krb5_cc_retrieve_cred(context, cred->ccache,
-                                     KRB5_TC_MATCH_AUTHDATA, &mcreds,
-                                     &evidence_creds);
-        if (code)
-            goto cleanup;
-
-        in_creds.client = cred->impersonator;
-        in_creds.second_ticket = evidence_creds.ticket;
-        flags = KRB5_GC_CANONICALIZE | KRB5_GC_CONSTRAINED_DELEGATION;
-        code = krb5_get_credentials(context, flags, cred->ccache,
-                                    &in_creds, &result_creds);
-    }
-
     if (code)
         goto cleanup;
 
index 2c50c9cce9b00d1e985d6e1bdc66ece2e14987fb..4328b7d6f9f3fa48a7590b49aa6cb6a889826a7f 100644 (file)
@@ -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);
     }
index c3627fe0162d082db70c0da90dd46b9ecbfc324e..b7d728e03cd6863cae6bce108277de9d9570175a 100644 (file)
@@ -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;
 }
 
 
index 62a6983d8de75fc79908e7f9eb810a30da6180e2..59982b7527939e730a8dd3e486e18fa69dc293bc 100644 (file)
@@ -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;
 
     /*
index dc0aef667234acb338aeaa74e968078006cf8937..b3f01be9b8d2faca3bb4991e13bcc58424979fed 100644 (file)
@@ -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;
 }
 
index 2f12a17c8bf08b2c5a3a4fe4f9ab45c953c57dcb..00ff613e8b658cb4819a4a211148d7d7c105d916 100644 (file)
@@ -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;
index 4adf6aca5fa0b027de6c17d36aad099fbb6d7e01..3786bad2ca7d51bc81f79b6b18d8a69603ab0783 100644 (file)
@@ -124,6 +124,9 @@ main(int argc, char **argv)
                                          KRB5_GC_CANONICALIZE, defcc,
                                          &mcred, ev_ticket, &new_cred));
 
+    assert(data_eq(new_cred->second_ticket, ev_cred.ticket));
+    assert(new_cred->second_ticket.length != 0);
+
     /* Store the new cred in the default ccache. */
     check(krb5_cc_store_cred(context, defcc, new_cred));