krb5 commit [krb5-1.18]: Allow cross-realm RBCD with PAC and other authdata

https://github.com/krb5/krb5/commit/17570dd94056df70c19108c14d46cd6132509e6a
commit 17570dd94056df70c19108c14d46cd6132509e6a
Author: Isaac Boukris <[email protected]>
Date:   Wed Jan 15 11:14:00 2020 +0100

    Allow cross-realm RBCD with PAC and other authdata
    
    For cross-realm S4U2Proxy requests, require a PAC to be present to
    bypass signedpath verification, but do not require it to be the only
    authdata element.  For within-realm requests, add and verify
    signedpath authdata regardless of the presence of a PAC.
    
    Simplify the test KDB authdata module and the existing RBCD tests as
    we no longer need a way to suppress the test module's KDB authdata.
    
    [[email protected]: rewrote commit message; reordered a condition for
    efficiency]
    
    (cherry picked from commit 94f7c9705879500b1dc8dda8592490efce05688f)
    
    ticket: 8868
    version_fixed: 1.18

 src/include/kdb.h               |    6 +++---
 src/kdc/kdc_authdata.c          |   21 ++++++++-------------
 src/plugins/kdb/test/kdb_test.c |   23 ++++++-----------------
 src/tests/gssapi/t_s4u.py       |    3 ---
 4 files changed, 17 insertions(+), 36 deletions(-)

diff --git a/src/include/kdb.h b/src/include/kdb.h
index 7f1362d..2a85eed 100644
--- a/src/include/kdb.h
+++ b/src/include/kdb.h
@@ -1493,9 +1493,9 @@ 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, set *client_out to the client
-     * name in the PAC; this indicates the requested client principal for a
-     * cross-realm S4U2Proxy request.
+     * 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
+     * requested client principal for a cross-realm S4U2Proxy request.
      *
      * This method is called for TGS requests on the authorization data from
      * the header ticket.  For S4U2Proxy requests it is also called on the
diff --git a/src/kdc/kdc_authdata.c b/src/kdc/kdc_authdata.c
index 29e7205..a18e4b4 100644
--- a/src/kdc/kdc_authdata.c
+++ b/src/kdc/kdc_authdata.c
@@ -658,14 +658,11 @@ free_deleg_path(krb5_context context, krb5_principal *deleg_path)
     free(deleg_path);
 }
 
-/* Return true if the Windows 2000 PAC is the only element in the supplied
- * authorization data. */
+/* Return true if the Windows PAC is present in authorization data. */
 static krb5_boolean
-only_pac_p(krb5_context context, krb5_authdata **authdata)
+has_pac(krb5_context context, krb5_authdata **authdata)
 {
-    return has_kdc_issued_authdata(context, authdata,
-                                   KRB5_AUTHDATA_WIN2K_PAC) &&
-        authdata[1] == NULL;
+    return has_kdc_issued_authdata(context, authdata, KRB5_AUTHDATA_WIN2K_PAC);
 }
 
 /* Verify AD-SIGNTICKET authdata if we need to, and insert an AD-SIGNEDPATH
@@ -685,12 +682,11 @@ handle_signticket(krb5_context context, unsigned int flags,
 
     s4u2proxy = isflagset(flags, KRB5_KDB_FLAG_CONSTRAINED_DELEGATION);
 
-    /*
-     * The Windows PAC fulfils the same role as the signed path
-     * if it is the only authorization data element.
-     */
+    /* For cross-realm the Windows PAC must have been verified, and it
+     * fulfills the same role as the signed path. */
     if (req->msg_type == KRB5_TGS_REQ &&
-        !only_pac_p(context, enc_tkt_req->authorization_data)) {
+        (!isflagset(flags, KRB5_KDB_FLAG_CROSS_REALM) ||
+         !has_pac(context, enc_tkt_req->authorization_data))) {
         ret = verify_signedpath(context, local_tgt, local_tgt_key, enc_tkt_req,
                                 &deleg_path, &signed_path);
         if (ret)
@@ -705,8 +701,7 @@ handle_signticket(krb5_context context, unsigned int flags,
     /* No point in including signedpath authdata for a cross-realm TGT, since
      * it will be presented to a different KDC. */
     if (!isflagset(server->attributes, KRB5_KDB_NO_AUTH_DATA_REQUIRED) &&
-        !is_cross_tgs_principal(server->princ) &&
-        !only_pac_p(context, enc_tkt_reply->authorization_data)) {
+        !is_cross_tgs_principal(server->princ)) {
         ret = make_signedpath(context, for_user_princ,
                               s4u2proxy ? subject_server->princ : NULL,
                               local_tgt_key, deleg_path, enc_tkt_reply);
diff --git a/src/plugins/kdb/test/kdb_test.c b/src/plugins/kdb/test/kdb_test.c
index 69a4663..7f62c9c 100644
--- a/src/plugins/kdb/test/kdb_test.c
+++ b/src/plugins/kdb/test/kdb_test.c
@@ -66,7 +66,6 @@
  *                 # intermediate_service may be in a different realm.
  *                 target_service = intermediate_service
  *             }
- *             ad_type = mspac
  *         }
  *
  * Key values are generated using a hash of the kvno, enctype, salt type,
@@ -882,30 +881,20 @@ test_sign_authdata(krb5_context context, unsigned int flags,
                    void *ad_info, krb5_data ***auth_indicators,
                    krb5_authdata ***signed_auth_data)
 {
-    testhandle h = context->dal_handle->db_context;
     krb5_authdata *pac_ad = NULL, *test_ad = NULL, **list;
     krb5_data **inds, d;
     int i, val;
-    char *ad_type;
 
     generate_pac(context, flags, client_princ, server_princ, client,
                  header_server, local_tgt, server_key, header_key,
                  local_tgt_key, authtime, ad_info, &pac_ad);
 
-    /*
-     * Omit test_ad if ad_type is mspac (only), as handle_signticket() fails in
-     * constrained delegation if the PAC is not the only authorization data
-     * element.
-     */
-    ad_type = get_string(h, "ad_type", NULL, NULL);
-    if (ad_type == NULL || strcmp(ad_type, "mspac") != 0) {
-        test_ad = ealloc(sizeof(*test_ad));
-        test_ad->magic = KV5M_AUTHDATA;
-        test_ad->ad_type = TEST_AD_TYPE;
-        test_ad->contents = (uint8_t *)estrdup("db-authdata-test");
-        test_ad->length = strlen((char *)test_ad->contents);
-    }
-    free(ad_type);
+    /* Add our TEST_AD_TYPE authdata */
+    test_ad = ealloc(sizeof(*test_ad));
+    test_ad->magic = KV5M_AUTHDATA;
+    test_ad->ad_type = TEST_AD_TYPE;
+    test_ad->contents = (uint8_t *)estrdup("db-authdata-test");
+    test_ad->length = strlen((char *)test_ad->contents);
 
     list = ealloc(3 * sizeof(*list));
     list[0] = (test_ad != NULL) ? test_ad : pac_ad;
diff --git a/src/tests/gssapi/t_s4u.py b/src/tests/gssapi/t_s4u.py
index 1a395c3..711612d 100755
--- a/src/tests/gssapi/t_s4u.py
+++ b/src/tests/gssapi/t_s4u.py
@@ -289,7 +289,6 @@ a_princs = {'krbtgt/A': {'keys': 'aes128-cts'},
             'rb': {'keys': 'aes128-cts'}}
 a_kconf = {'realms': {'$realm': {'database_module': 'test'}},
            'dbmodules': {'test': {'db_library': 'test',
-                                  'ad_type': 'mspac',
                                   'princs': a_princs,
                                   'rbcd': {'rb@A': 'impersonator@A'},
                                   'alias': {'rb@A': 'rb',
@@ -306,7 +305,6 @@ b_princs = {'krbtgt/B': {'keys': 'aes128-cts'},
             'rb': {'keys': 'aes128-cts'}}
 b_kconf = {'realms': {'$realm': {'database_module': 'test'}},
            'dbmodules': {'test': {'db_library': 'test',
-                                  'ad_type': 'mspac',
                                   'princs': b_princs,
                                   'rbcd': {'rb@B': 'impersonator@A'},
                                   'alias': {'rb@B': 'rb',
@@ -321,7 +319,6 @@ c_princs = {'krbtgt/C': {'keys': 'aes128-cts'},
 c_kconf = {'realms': {'$realm': {'database_module': 'test'}},
            'capaths': { 'A' : { 'C' : 'B' }},
            'dbmodules': {'test': {'db_library': 'test',
-                                  'ad_type': 'mspac',
                                   'princs': c_princs,
                                   'rbcd': {'rb@C': 'impersonator@A'},
                                   'alias': {'rb@C': 'rb',