Change KDC constrained-delegation precedence order

MS-SFU errata from 2019/12/09 indicates that legacy constrained
delegation should be prefered over resource-based constrained
delegation, which results slight diferences.

Also clarify that in the get_authdata_info KDB method, the PAC must be
verified and checked for user sensitivity for S4U2Proxy.  Document
that the client name should only be provided in the cross-realm
S4U2Proxy case.

[ghudson@mit.edu: clarified comments and commit message]

(cherry picked from commit cf6b710518bd6da8c491ee4020a9ad8ded321d66)

ticket: 8884
version_fixed: 1.18.1

4 files changed, 107 insertions, 103 deletions

diff --git a/src/include/kdb.h b/src/include/kdb.h
index 2a85eed..c56947a 100644
--- a/src/include/kdb.h
+++ b/src/include/kdb.h
@@ -1493,8 +1493,13 @@ typedef struct _kdb_vftabl {
      * such as a Windows PAC, based on the request client lookup flags.  Return
      * 0 if all checks have passed.  Optionally return a representation of the
      * authdata in *ad_info_out, to be consumed by allowed_to_delegate_from and
-     * sign_authdata.  If client_out is not NULL and the PAC has been verified,
-     * set *client_out to the client name in the PAC; this indicates the
+     * sign_authdata.  Returning *ad_info_out is required to support
+     * resource-based constrained delegation.
+     *
+     * If the KRB5_KDB_FLAG_CONSTRAINED_DELEGATION bit is set, a PAC must be
+     * provided and verified, and an error should be returned if the client is
+     * not allowed to delegate.  If the KRB5_KDB_FLAG_CROSS_REALM bit is also
+     * set, set *client_out to the client name in the PAC; this indicates the
      * requested client principal for a cross-realm S4U2Proxy request.
      *
      * This method is called for TGS requests on the authorization data from
diff --git a/src/kdc/kdc_util.c b/src/kdc/kdc_util.c
index 221bde1..e5898ea 100644
--- a/src/kdc/kdc_util.c
+++ b/src/kdc/kdc_util.c
@@ -1672,65 +1672,6 @@ kdc_process_s4u2self_req(kdc_realm_t *kdc_active_realm,
     return 0;
 }
 
-static krb5_error_code
-check_allowed_to_delegate_to(krb5_context context, krb5_const_principal client,
-                             const krb5_db_entry *server,
-                             krb5_const_principal proxy)
-{
-    /* Must be in same realm */
-    if (!krb5_realm_compare(context, server->princ, proxy))
-        return KRB5KDC_ERR_POLICY;
-
-    return krb5_db_check_allowed_to_delegate(context, client, server, proxy);
-}
-
-static krb5_error_code
-check_rbcd_policy(kdc_realm_t *kdc_active_realm, unsigned int flags,
-                  const krb5_principal stkt_client_princ,
-                  krb5_principal stkt_authdata_client,
-                  const krb5_db_entry *stkt_server,
-                  krb5_const_principal header_client_princ,
-                  void *header_ad_info, const krb5_db_entry *proxy)
-{
-    krb5_principal client_princ = stkt_client_princ;
-
-    /* Ensure that either the evidence ticket server or the client matches the
-     * TGT client. */
-    if (isflagset(flags, KRB5_KDB_FLAG_CROSS_REALM)) {
-        /*
-         * Check that the proxy server is local, that the second ticket is a
-         * cross-realm TGT for us, and that the second ticket client matches
-         * the header ticket client.
-         */
-        if (isflagset(flags, KRB5_KDB_FLAG_ISSUING_REFERRAL) ||
-            !is_cross_tgs_principal(stkt_server->princ) ||
-            !krb5_principal_compare_any_realm(kdc_context, stkt_server->princ,
-                                              tgs_server) ||
-            !krb5_principal_compare(kdc_context, stkt_client_princ,
-                                    header_client_princ)) {
-            return KRB5KDC_ERR_BADOPTION;
-        }
-        /* The KDB module must be able to recover the reply ticket client name
-         * from the evidence ticket authorization data. */
-        if (stkt_authdata_client == NULL ||
-            stkt_authdata_client->realm.length == 0)
-            return KRB5KDC_ERR_BADOPTION;
-        client_princ = stkt_authdata_client;
-    } else if (!krb5_principal_compare(kdc_context, stkt_server->princ,
-                                       header_client_princ)) {
-        return KRB5KDC_ERR_BADOPTION;
-    }
-
-    /* If we are issuing a referral, the KDC in the resource realm will check
-     * if delegation is allowed. */
-    if (isflagset(flags, KRB5_KDB_FLAG_ISSUING_REFERRAL))
-        return 0;
-
-    return krb5_db_allowed_to_delegate_from(kdc_context, client_princ,
-                                            header_client_princ,
-                                            header_ad_info, proxy);
-}
-
 krb5_error_code
 kdc_process_s4u2proxy_req(kdc_realm_t *kdc_active_realm, unsigned int flags,
                           krb5_kdc_req *request,
@@ -1747,6 +1688,7 @@ kdc_process_s4u2proxy_req(kdc_realm_t *kdc_active_realm, unsigned int flags,
 {
     krb5_error_code errcode;
     krb5_boolean support_rbcd;
+    krb5_principal client_princ = t2enc->client;
 
     /*
      * Constrained delegation is mutually exclusive with renew/forward/etc.
@@ -1764,59 +1706,102 @@ kdc_process_s4u2proxy_req(kdc_realm_t *kdc_active_realm, unsigned int flags,
         return KRB5KDC_ERR_POLICY;
     }
 
+    /* Check if the client supports resource-based constrained delegation. */
+    errcode = kdc_get_pa_pac_rbcd(kdc_context, request->padata, &support_rbcd);
+    if (errcode)
+        return errcode;
+
     errcode = krb5_db_get_authdata_info(kdc_context, flags,
                                         t2enc->authorization_data,
                                         t2enc->client, proxy_princ, server_key,
                                         krbtgt_key, krbtgt,
                                         t2enc->times.authtime, stkt_ad_info,
                                         stkt_authdata_client);
-    if (errcode && errcode != KRB5_PLUGIN_OP_NOTSUPP) {
+    if (errcode != 0 && errcode != KRB5_PLUGIN_OP_NOTSUPP) {
         *status = "NOT_ALLOWED_TO_DELEGATE";
         return errcode;
     }
 
-    errcode = kdc_get_pa_pac_rbcd(kdc_context, request->padata, &support_rbcd);
-    if (errcode)
-        return errcode;
+    /* For RBCD we require that both client and impersonator's authdata have
+     * been verified. */
+    if (errcode != 0 || ad_info == NULL)
+        support_rbcd = FALSE;
 
-    if (support_rbcd && ad_info != NULL) {
-        errcode = check_rbcd_policy(kdc_active_realm, flags, t2enc->client,
-                                    *stkt_authdata_client, server,
-                                    server_princ, ad_info, proxy);
-        if (errcode == 0)
-            return 0;
-        if (errcode != KRB5KDC_ERR_POLICY &&
-            errcode != KRB5_PLUGIN_OP_NOTSUPP) {
-            *status = "INVALID_S4U2PROXY_XREALM_REQUEST";
-            return errcode;
+    /* Ensure that either the evidence ticket server or the client matches the
+     * TGT client. */
+    if (isflagset(flags, KRB5_KDB_FLAG_CROSS_REALM)) {
+        /*
+         * Check that the proxy server is local, that the second ticket is a
+         * cross-realm TGT for us, and that the second ticket client matches
+         * the header ticket client.
+         */
+        if (isflagset(flags, KRB5_KDB_FLAG_ISSUING_REFERRAL) ||
+            !is_cross_tgs_principal(server->princ) ||
+            !krb5_principal_compare_any_realm(kdc_context, server->princ,
+                                              tgs_server) ||
+            !krb5_principal_compare(kdc_context, client_princ, server_princ)) {
+            *status = "XREALM_EVIDENCE_TICKET_MISMATCH";
+            return KRB5KDC_ERR_BADOPTION;
+        }
+        /* The KDB module must be able to recover the reply ticket client name
+         * from the evidence ticket authorization data. */
+        if (*stkt_authdata_client == NULL ||
+            (*stkt_authdata_client)->realm.length == 0) {
+            *status = "UNSUPPORTED_S4U2PROXY_REQUEST";
+            return KRB5KDC_ERR_BADOPTION;
         }
-        /* Fall back to old constrained delegation. */
-    }
 
-    /* Ensure that evidence ticket server matches TGT client */
-    if (!krb5_principal_compare(kdc_context,
-                                server->princ, /* after canon */
-                                server_princ)) {
+        client_princ = *stkt_authdata_client;
+    } else if (!krb5_principal_compare(kdc_context,
+                                       server->princ, /* after canon */
+                                       server_princ)) {
         *status = "EVIDENCE_TICKET_MISMATCH";
         return KRB5KDC_ERR_SERVER_NOMATCH;
     }
 
-    if (!isflagset(t2enc->flags, TKT_FLG_FORWARDABLE)) {
-        *status = "EVIDENCE_TKT_NOT_FORWARDABLE";
-        return KRB5_TKT_NOT_FORWARDABLE;
+    /* If both are in the same realm, try allowed_to_delegate first. */
+    if (krb5_realm_compare(kdc_context, server->princ, proxy_princ)) {
+
+        errcode = krb5_db_check_allowed_to_delegate(kdc_context, client_princ,
+                                                    server, proxy_princ);
+        if (errcode != 0 && errcode != KRB5KDC_ERR_POLICY &&
+            errcode != KRB5_PLUGIN_OP_NOTSUPP)
+            return errcode;
+
+        if (errcode == 0) {
+
+            /*
+             * In legacy constrained-delegation, the evidence ticket must be
+             * forwardable.  This check deliberately causes an error response
+             * even if the delegation is also authorized by resource-based
+             * constrained delegation (which does not require a forwardable
+             * evidence ticket).  Windows KDCs behave the same way.
+             */
+            if (!isflagset(t2enc->flags, TKT_FLG_FORWARDABLE)) {
+                *status = "EVIDENCE_TKT_NOT_FORWARDABLE";
+                return KRB5KDC_ERR_BADOPTION;
+            }
+
+            return 0;
+        }
+        /* Fall back to resource-based constrained-delegation. */
     }
 
-    /* Backend policy check */
-    errcode = check_allowed_to_delegate_to(kdc_context,
-                                           t2enc->client,
-                                           server,
-                                           proxy_princ);
-    if (errcode) {
-        *status = "NOT_ALLOWED_TO_DELEGATE";
-        return errcode;
+    if (!support_rbcd) {
+        *status = "UNSUPPORTED_S4U2PROXY_REQUEST";
+        return KRB5KDC_ERR_BADOPTION;
     }
 
-    return 0;
+    /* If we are issuing a referral, the KDC in the resource realm will check
+     * if delegation is allowed. */
+    if (isflagset(flags, KRB5_KDB_FLAG_ISSUING_REFERRAL))
+        return 0;
+
+    errcode = krb5_db_allowed_to_delegate_from(kdc_context, client_princ,
+                                               server_princ, ad_info, proxy);
+    if (errcode)
+        *status = "NOT_ALLOWED_TO_DELEGATE";
+    return errcode;
 }
 
 krb5_error_code
diff --git a/src/tests/gssapi/t_s4u.py b/src/tests/gssapi/t_s4u.py
index 711612d..4e0e446 100755
--- a/src/tests/gssapi/t_s4u.py
+++ b/src/tests/gssapi/t_s4u.py
@@ -36,7 +36,7 @@ realm.kinit(realm.user_princ, password('user'), ['-f', '-c', usercache])
 output = realm.run(['./t_s4u2proxy_krb5', usercache, storagecache, '-',
                     pservice1, pservice2], expected_code=1)
 if ('auth1: ' + realm.user_princ not in output or
-    'NOT_ALLOWED_TO_DELEGATE' not in output):
+    'KDC can\'t fulfill requested option' not in output):
     fail('krb5 -> s4u2proxy')
 
 # Again with SPNEGO.
@@ -44,7 +44,7 @@ output = realm.run(['./t_s4u2proxy_krb5', '--spnego', usercache, storagecache,
                     '-', pservice1, pservice2],
                    expected_code=1)
 if ('auth1: ' + realm.user_princ not in output or
-    'NOT_ALLOWED_TO_DELEGATE' not in output):
+    'KDC can\'t fulfill requested option' not in output):
     fail('krb5 -> s4u2proxy (SPNEGO)')
 
 # Try krb5 -> S4U2Proxy without forwardable user creds.
@@ -52,28 +52,28 @@ realm.kinit(realm.user_princ, password('user'), ['-c', usercache])
 output = realm.run(['./t_s4u2proxy_krb5', usercache, storagecache, pservice1,
                    pservice1, pservice2], expected_code=1)
 if ('auth1: ' + realm.user_princ not in output or
-    'EVIDENCE_TKT_NOT_FORWARDABLE' not in output):
+    'KDC can\'t fulfill requested option' not in output):
     fail('krb5 -> s4u2proxy not-forwardable')
 
 # Try S4U2Self.  Ask for an S4U2Proxy step; this won't succeed because
 # service/1 isn't allowed to get a forwardable S4U2Self ticket.
 realm.run(['./t_s4u', puser, pservice2], expected_code=1,
-          expected_msg='EVIDENCE_TKT_NOT_FORWARDABLE')
+          expected_msg='KDC can\'t fulfill requested option')
 realm.run(['./t_s4u', '--spnego', puser, pservice2], expected_code=1,
-          expected_msg='EVIDENCE_TKT_NOT_FORWARDABLE')
+          expected_msg='KDC can\'t fulfill requested option')
 
 # Correct that problem and try again.  As above, the S4U2Proxy step
 # won't actually succeed since we don't support that in DB2.
 realm.run([kadminl, 'modprinc', '+ok_to_auth_as_delegate', service1])
 realm.run(['./t_s4u', puser, pservice2], expected_code=1,
-          expected_msg='NOT_ALLOWED_TO_DELEGATE')
+          expected_msg='KDC can\'t fulfill requested option')
 
 # Again with SPNEGO.  This uses SPNEGO for the initial authentication,
 # but still uses krb5 for S4U2Proxy--the delegated cred is returned as
 # a krb5 cred, not a SPNEGO cred, and t_s4u uses the delegated cred
 # directly rather than saving and reacquiring it.
 realm.run(['./t_s4u', '--spnego', puser, pservice2], expected_code=1,
-          expected_msg='NOT_ALLOWED_TO_DELEGATE')
+          expected_msg='KDC can\'t fulfill requested option')
 
 realm.stop()
 
@@ -286,14 +286,20 @@ a_princs = {'krbtgt/A': {'keys': 'aes128-cts'},
             'sensitive': {'keys': 'aes128-cts',
                           'flags': '+disallow_forwardable'},
             'impersonator': {'keys': 'aes128-cts'},
+            'service1': {'keys': 'aes128-cts',
+                         'flags': '+ok_to_auth_as_delegate'},
+            'rb2': {'keys': 'aes128-cts'},
             'rb': {'keys': 'aes128-cts'}}
 a_kconf = {'realms': {'$realm': {'database_module': 'test'}},
            'dbmodules': {'test': {'db_library': 'test',
                                   'princs': a_princs,
-                                  'rbcd': {'rb@A': 'impersonator@A'},
+                                  'rbcd': {'rb@A': 'impersonator@A',
+                                           'rb2@A': 'service1@A'},
+                                  'delegation': {'service1': 'rb2'},
                                   'alias': {'rb@A': 'rb',
                                             'rb@B': '@B',
                                             'rb@C': '@B',
+                                            'rb2_alias': 'rb2',
                                             'service/rb.a': 'rb',
                                             'service/rb.b': '@B',
                                             'service/rb.c': '@B' }}}}
@@ -338,7 +344,7 @@ domain_realm = {'domain_realm': {'.a':'A', '.b':'B', '.c':'C'}}
 domain_conf = ra.special_env('domain_conf', False, krb5_conf=domain_realm)
 
 ra.extract_keytab('impersonator@A', ra.keytab)
-ra.kinit('impersonator@A', None, ['-k', '-t', ra.keytab])
+ra.kinit('impersonator@A', None, ['-F', '-k', '-t', ra.keytab])
 
 mark('Local-realm RBCD')
 ra.run(['./t_s4u', 'p:' + ra.user_princ, 'p:rb'])
@@ -370,6 +376,14 @@ ra.run(['./t_s4u', 'p:' + ra.user_princ, 'h:service@rb.c'], env=domain_conf)
 ra.run(['./t_s4u', 'p:' + 'sensitive@A', 'h:service@rb.c'], expected_code=1)
 ra.run(['./t_s4u', 'p:' + rb.user_princ, 'h:service@rb.c'])
 
+mark('With both delegation types, 2nd ticket must be forwardable')
+ra.extract_keytab('service1@A', ra.keytab)
+ra.kinit('service1@A', None, ['-F', '-k', '-t', ra.keytab])
+ra.run(['./t_s4u', 'p:' + ra.user_princ, 'p:rb2'], expected_code=1)
+ra.run(['./t_s4u', 'p:' + ra.user_princ, 'p:rb2_alias'])
+ra.kinit('service1@A', None, ['-f', '-k', '-t', ra.keytab])
+ra.run(['./t_s4u', 'p:' + ra.user_princ, 'p:rb2'])
+
 ra.stop()
 rb.stop()
 rc.stop()
diff --git a/src/tests/t_audit.py b/src/tests/t_audit.py
index 0f880ed..e48f4eb 100755
--- a/src/tests/t_audit.py
+++ b/src/tests/t_audit.py
@@ -14,7 +14,7 @@ realm.run([kvno, 'target'])
 # Make S4U2Self and S4U2Proxy requests so they will be audited.  The
 # S4U2Proxy request is expected to fail.
 realm.run([kvno, '-k', realm.keytab, '-U', 'user', '-P', 'target'],
-          expected_code=1, expected_msg='NOT_ALLOWED_TO_DELEGATE')
+          expected_code=1, expected_msg='KDC can\'t fulfill requested option')
 
 # Make a U2U request so it will be audited.
 uuserver = os.path.join(buildtop, 'appl', 'user_user', 'uuserver')