]> git.ipfire.org Git - thirdparty/samba.git/commitdiff
kdc: remove KRB5SignedPath, to be replaced with PAC
authorIsaac Boukris <iboukris@gmail.com>
Mon, 28 Dec 2020 20:07:10 +0000 (22:07 +0200)
committerAndrew Bartlett <abartlet@samba.org>
Thu, 14 Oct 2021 18:59:31 +0000 (18:59 +0000)
KRB5SignedPath was a Heimdal-specific authorization data element used to
protect the authenticity of evidence tickets when used in constrained
delegation (without a Windows PAC).

Remove this, to be replaced with the Windows PAC which itself now supports
signing the entire ticket in the TGS key.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14642

[jsutton@samba.org Backported from Heimdal commit
 bb1d8f2a8c2545bccdf2c9179ce9259bf1050086
 - Removed tests
 - Removed auditing hook (only present in Heimdal master)
 - Added knownfails
]

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
selftest/knownfail_heimdal_kdc
source4/heimdal/kdc/kerberos5.c
source4/heimdal/kdc/krb5tgs.c
source4/heimdal/lib/asn1/krb5.asn1

index 2854ef0610af473a3d70ca6be8ce7803897922e5..a67621982afe2986c2a56f63f7380cbf5eec797f 100644 (file)
@@ -76,6 +76,7 @@
 #
 ^samba.tests.krb5.s4u_tests.samba.tests.krb5.s4u_tests.S4UKerberosTests.test_bronze_bit_rbcd_old_checksum
 ^samba.tests.krb5.s4u_tests.samba.tests.krb5.s4u_tests.S4UKerberosTests.test_constrained_delegation_missing_client_checksum
+^samba.tests.krb5.s4u_tests.samba.tests.krb5.s4u_tests.S4UKerberosTests.test_constrained_delegation_no_service_pac
 ^samba.tests.krb5.s4u_tests.samba.tests.krb5.s4u_tests.S4UKerberosTests.test_constrained_delegation_unkeyed_client_checksum
 ^samba.tests.krb5.s4u_tests.samba.tests.krb5.s4u_tests.S4UKerberosTests.test_constrained_delegation_unkeyed_service_checksum
 ^samba.tests.krb5.s4u_tests.samba.tests.krb5.s4u_tests.S4UKerberosTests.test_constrained_delegation_zeroed_client_checksum
 ^samba4.rpc.pac on ncacn_np.netr-mem-arcfour.verify-sig-arcfour.fl2003dc
 ^samba4.rpc.pac on ncacn_np.netr-mem-arcfour.verify-sig-arcfour.fl2008dc
 ^samba4.rpc.pac on ncacn_np.netr-mem-arcfour.verify-sig-arcfour.fl2008r2dc
+#
+# The lack of KRB5SignedPath means we no longer return
+# KRB5KRB_ERR_RESPONSE_TOO_BIG in this specific case
+#
+^samba4.krb5.kdc with machine account.as-req-pac-request.fl2000dc:local
index a7ca3d934758bbfee604d81d877585a3a672ed09..85511e1660caf4bce9caab37cb42fac123c2bf5b 100644 (file)
@@ -1745,18 +1745,6 @@ _kdc_as_rep(krb5_context context,
     _kdc_log_timestamp(context, config, "AS-REQ", et.authtime, et.starttime,
                       et.endtime, et.renew_till);
 
-    /* do this as the last thing since this signs the EncTicketPart */
-    ret = _kdc_add_KRB5SignedPath(context,
-                                 config,
-                                 server,
-                                 setype,
-                                 client->entry.principal,
-                                 NULL,
-                                 NULL,
-                                 &et);
-    if (ret)
-       goto out;
-
     log_as_req(context, config, reply_key->keytype, setype, b);
 
     ret = _kdc_encode_reply(context, config,
index d143eb739eb2f2af46ab25c2599041361db86d86..c6bab82f5173be3157441940fb121125bfd17ddb 100644 (file)
@@ -47,230 +47,6 @@ get_krbtgt_realm(const PrincipalName *p)
        return NULL;
 }
 
-/*
- * The KDC might add a signed path to the ticket authorization data
- * field. This is to avoid server impersonating clients and the
- * request constrained delegation.
- *
- * This is done by storing a KRB5_AUTHDATA_IF_RELEVANT with a single
- * entry of type KRB5SignedPath.
- */
-
-static krb5_error_code
-find_KRB5SignedPath(krb5_context context,
-                   const AuthorizationData *ad,
-                   krb5_data *data)
-{
-    AuthorizationData child;
-    krb5_error_code ret;
-    int pos;
-
-    if (ad == NULL || ad->len == 0)
-       return KRB5KDC_ERR_PADATA_TYPE_NOSUPP;
-
-    pos = ad->len - 1;
-
-    if (ad->val[pos].ad_type != KRB5_AUTHDATA_IF_RELEVANT)
-       return KRB5KDC_ERR_PADATA_TYPE_NOSUPP;
-
-    ret = decode_AuthorizationData(ad->val[pos].ad_data.data,
-                                  ad->val[pos].ad_data.length,
-                                  &child,
-                                  NULL);
-    if (ret) {
-       krb5_set_error_message(context, ret, "Failed to decode "
-                              "IF_RELEVANT with %d", ret);
-       return ret;
-    }
-
-    if (child.len != 1) {
-       free_AuthorizationData(&child);
-       return KRB5KDC_ERR_PADATA_TYPE_NOSUPP;
-    }
-
-    if (child.val[0].ad_type != KRB5_AUTHDATA_SIGNTICKET) {
-       free_AuthorizationData(&child);
-       return KRB5KDC_ERR_PADATA_TYPE_NOSUPP;
-    }
-
-    if (data)
-       ret = der_copy_octet_string(&child.val[0].ad_data, data);
-    free_AuthorizationData(&child);
-    return ret;
-}
-
-krb5_error_code
-_kdc_add_KRB5SignedPath(krb5_context context,
-                       krb5_kdc_configuration *config,
-                       hdb_entry_ex *krbtgt,
-                       krb5_enctype enctype,
-                       krb5_principal client,
-                       krb5_const_principal server,
-                       krb5_principals principals,
-                       EncTicketPart *tkt)
-{
-    krb5_error_code ret;
-    KRB5SignedPath sp;
-    krb5_data data;
-    krb5_crypto crypto = NULL;
-    size_t size = 0;
-
-    if (server && principals) {
-       ret = add_Principals(principals, server);
-       if (ret)
-           return ret;
-    }
-
-    {
-       KRB5SignedPathData spd;
-
-       spd.client = client;
-       spd.authtime = tkt->authtime;
-       spd.delegated = principals;
-       spd.method_data = NULL;
-
-       ASN1_MALLOC_ENCODE(KRB5SignedPathData, data.data, data.length,
-                          &spd, &size, ret);
-       if (ret)
-           return ret;
-       if (data.length != size)
-           krb5_abortx(context, "internal asn.1 encoder error");
-    }
-
-    {
-       Key *key;
-       ret = hdb_enctype2key(context, &krbtgt->entry, enctype, &key);
-       if (ret == 0)
-           ret = krb5_crypto_init(context, &key->key, 0, &crypto);
-       if (ret) {
-           free(data.data);
-           return ret;
-       }
-    }
-
-    /*
-     * Fill in KRB5SignedPath
-     */
-
-    sp.etype = enctype;
-    sp.delegated = principals;
-    sp.method_data = NULL;
-
-    ret = krb5_create_checksum(context, crypto, KRB5_KU_KRB5SIGNEDPATH, 0,
-                              data.data, data.length, &sp.cksum);
-    krb5_crypto_destroy(context, crypto);
-    free(data.data);
-    if (ret)
-       return ret;
-
-    ASN1_MALLOC_ENCODE(KRB5SignedPath, data.data, data.length, &sp, &size, ret);
-    free_Checksum(&sp.cksum);
-    if (ret)
-       return ret;
-    if (data.length != size)
-       krb5_abortx(context, "internal asn.1 encoder error");
-
-
-    /*
-     * Add IF-RELEVANT(KRB5SignedPath) to the last slot in
-     * authorization data field.
-     */
-
-    ret = _kdc_tkt_add_if_relevant_ad(context, tkt,
-                                     KRB5_AUTHDATA_SIGNTICKET, &data);
-    krb5_data_free(&data);
-
-    return ret;
-}
-
-static krb5_error_code
-check_KRB5SignedPath(krb5_context context,
-                    krb5_kdc_configuration *config,
-                    hdb_entry_ex *krbtgt,
-                    krb5_principal cp,
-                    EncTicketPart *tkt,
-                    krb5_principals *delegated,
-                    int *signedpath)
-{
-    krb5_error_code ret;
-    krb5_data data;
-    krb5_crypto crypto = NULL;
-
-    if (delegated)
-       *delegated = NULL;
-
-    ret = find_KRB5SignedPath(context, tkt->authorization_data, &data);
-    if (ret == 0) {
-       KRB5SignedPathData spd;
-       KRB5SignedPath sp;
-       size_t size = 0;
-
-       ret = decode_KRB5SignedPath(data.data, data.length, &sp, NULL);
-       krb5_data_free(&data);
-       if (ret)
-           return ret;
-
-       spd.client = cp;
-       spd.authtime = tkt->authtime;
-       spd.delegated = sp.delegated;
-       spd.method_data = sp.method_data;
-
-       ASN1_MALLOC_ENCODE(KRB5SignedPathData, data.data, data.length,
-                          &spd, &size, ret);
-       if (ret) {
-           free_KRB5SignedPath(&sp);
-           return ret;
-       }
-       if (data.length != size)
-           krb5_abortx(context, "internal asn.1 encoder error");
-
-       {
-           Key *key;
-           ret = hdb_enctype2key(context, &krbtgt->entry, sp.etype, &key);
-           if (ret == 0)
-               ret = krb5_crypto_init(context, &key->key, 0, &crypto);
-           if (ret) {
-               free(data.data);
-               free_KRB5SignedPath(&sp);
-               return ret;
-           }
-       }
-       ret = krb5_verify_checksum(context, crypto, KRB5_KU_KRB5SIGNEDPATH,
-                                  data.data, data.length,
-                                  &sp.cksum);
-       krb5_crypto_destroy(context, crypto);
-       free(data.data);
-       if (ret) {
-           free_KRB5SignedPath(&sp);
-           kdc_log(context, config, 5,
-                   "KRB5SignedPath not signed correctly, not marking as signed");
-           return 0;
-       }
-
-       if (delegated && sp.delegated) {
-
-           *delegated = malloc(sizeof(*sp.delegated));
-           if (*delegated == NULL) {
-               free_KRB5SignedPath(&sp);
-               return ENOMEM;
-           }
-
-           ret = copy_Principals(*delegated, sp.delegated);
-           if (ret) {
-               free_KRB5SignedPath(&sp);
-               free(*delegated);
-               *delegated = NULL;
-               return ret;
-           }
-       }
-       free_KRB5SignedPath(&sp);
-
-       *signedpath = 1;
-    }
-
-    return 0;
-}
-
 /*
  *
  */
@@ -738,7 +514,6 @@ tgs_make_reply(krb5_context context,
               krb5_principal client_principal,
               hdb_entry_ex *krbtgt,
               krb5_enctype krbtgt_etype,
-              krb5_principals spp,
               const krb5_data *rspac,
               const METHOD_DATA *enc_pa_data,
               const char **e_text,
@@ -903,20 +678,6 @@ tgs_make_reply(krb5_context context,
                goto out;
            }
        }
-
-       /* Filter out type KRB5SignedPath */
-       ret = find_KRB5SignedPath(context, et.authorization_data, NULL);
-       if (ret == 0) {
-           if (et.authorization_data->len == 1) {
-               free_AuthorizationData(et.authorization_data);
-               free(et.authorization_data);
-               et.authorization_data = NULL;
-           } else {
-               AuthorizationData *ad = et.authorization_data;
-               free_AuthorizationDataElement(&ad->val[ad->len - 1]);
-               ad->len--;
-           }
-       }
     }
 
     ret = krb5_copy_keyblock_contents(context, sessionkey, &et.key);
@@ -945,24 +706,6 @@ tgs_make_reply(krb5_context context,
     _kdc_log_timestamp(context, config, "TGS-REQ", et.authtime, et.starttime,
                       et.endtime, et.renew_till);
 
-    /* Don't sign cross realm tickets, they can't be checked anyway */
-    {
-       char *r = get_krbtgt_realm(&ek.sname);
-
-       if (r == NULL || strcmp(r, ek.srealm) == 0) {
-           ret = _kdc_add_KRB5SignedPath(context,
-                                         config,
-                                         krbtgt,
-                                         krbtgt_etype,
-                                         client_principal,
-                                         NULL,
-                                         spp,
-                                         &et);
-           if (ret)
-               goto out;
-       }
-    }
-
     if (enc_pa_data->len) {
        rep.padata = calloc(1, sizeof(*rep.padata));
        if (rep.padata == NULL) {
@@ -1517,7 +1260,6 @@ tgs_build_reply(krb5_context context,
     HDB *clientdb, *s4u2self_impersonated_clientdb;
     krb5_realm ref_realm = NULL;
     EncTicketPart *tgt = &ticket->ticket;
-    krb5_principals spp = NULL;
     const EncryptionKey *ekey;
     krb5_keyblock sessionkey;
     krb5_kvno kvno;
@@ -1891,23 +1633,6 @@ server_lookup:
        goto out;
     }
 
-    /* also check the krbtgt for signature */
-    ret = check_KRB5SignedPath(context,
-                              config,
-                              krbtgt,
-                              cp,
-                              tgt,
-                              &spp,
-                              &signedpath);
-    if (ret) {
-       const char *msg = krb5_get_error_message(context, ret);
-       kdc_log(context, config, 0,
-               "KRB5SignedPath check failed for %s (%s) from %s with %s",
-               spn, cpn, from, msg);
-       krb5_free_error_message(context, msg);
-       goto out;
-    }
-
     /*
      * Process request
      */
@@ -2200,27 +1925,6 @@ server_lookup:
            goto out;
        }
 
-       /*
-        * Check that the KDC issued the user's ticket.
-        */
-       ret = check_KRB5SignedPath(context,
-                                  config,
-                                  krbtgt,
-                                  cp,
-                                  &adtkt,
-                                  NULL,
-                                  &ad_signedpath);
-       if (ret) {
-           const char *msg = krb5_get_error_message(context, ret);
-           kdc_log(context, config, 0,
-                   "KRB5SignedPath check from service %s failed "
-                   "for delegation to %s for client %s (%s)"
-                   "from %s failed with %s",
-                   spn, tpn, dpn, cpn, from, msg);
-           krb5_free_error_message(context, msg);
-           goto out;
-       }
-
        if (!ad_signedpath) {
            ret = KRB5KDC_ERR_BADOPTION;
            kdc_log(context, config, 0,
@@ -2318,7 +2022,6 @@ server_lookup:
                         cp,
                         krbtgt_out,
                         krbtgt_etype,
-                        spp,
                         &rspac,
                         &enc_pa_data,
                         e_text,
index c2f40c0ecd6cca18bd2c0d8501456ab6827c94be..155d51d4ac875271702ffcb39c2b340269c0d02e 100644 (file)
@@ -43,9 +43,6 @@ EXPORTS
        KRB-PRIV,
        KRB-SAFE,
        KRB-SAFE-BODY,
-       KRB5SignedPath,
-       KRB5SignedPathData,
-       KRB5SignedPathPrincipals,
        KerberosString,
        KerberosTime,
        KrbCredInfo,
@@ -713,24 +710,6 @@ PA-S4U2Self ::= SEQUENCE {
         auth[3]                GeneralString
 }
 
--- never encoded on the wire, just used to checksum over
-KRB5SignedPathData ::= SEQUENCE {
-       client[0]       Principal OPTIONAL,
-       authtime[1]     KerberosTime,
-       delegated[2]    Principals OPTIONAL,
-       method_data[3]  METHOD-DATA OPTIONAL
-}
-
-KRB5SignedPath ::= SEQUENCE {
-       -- DERcoded KRB5SignedPathData
-       -- krbtgt key (etype), KeyUsage = XXX
-       etype[0]        ENCTYPE,
-       cksum[1]        Checksum,
-       -- srvs delegated though
-       delegated[2]    Principals OPTIONAL,
-       method_data[3]  METHOD-DATA OPTIONAL
-}
-
 AD-LoginAlias ::= SEQUENCE { -- ad-type number TBD --
        login-alias     [0] PrincipalName,
        checksum        [1] Checksum