From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Date: Wed, 26 Nov 2025 22:12:21 +0000 (+1300)
Subject: kdc: match implicit dollar without canon affects AS_REQ client only
X-Git-Tag: tdb-1.4.15~78
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e904e8b229bd0fb84803adbb60b6a4b0a62addfc;p=thirdparty%2Fsamba.git

kdc: match implicit dollar without canon affects AS_REQ client only

The smb.conf option

  kdc name match implicit dollar without canonicalization = no

is supposed to avoid the dollar ticket attack by refusing to consider
"foo$" as a match for "foo" unless canonicalization is requested.

This was rather blunt however, as the only time we care about this is for
the client name in an AS_REQ, and we can easily check whether that is the
case.

This makes the option less intrusive, allowing the use of "SERVER" for a
server name rather than "SERVER$". A number of tests no longer fail.

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Gary Lockyer <gary@catalyst.net.nz>
---

diff --git a/python/samba/tests/krb5/alias_tests.py b/python/samba/tests/krb5/alias_tests.py
index 6a517c596e4..84612eb9e23 100755
--- a/python/samba/tests/krb5/alias_tests.py
+++ b/python/samba/tests/krb5/alias_tests.py
@@ -169,14 +169,7 @@ class AliasTests(KDCBaseTest):
                                              ctype=None)
             return [padata], req_body
 
-        if self.uncanonicalized_implicit_dollar:
-            expected_error_mode = KDC_ERR_TGT_REVOKED
-        else:
-            # These are machine accounts, but we aren't explicitly
-            # adding the '$', so the ntvfs test will not find the
-            # principal.
-            expected_error_mode = KDC_ERR_C_PRINCIPAL_UNKNOWN
-
+        expected_error_mode = KDC_ERR_TGT_REVOKED
 
         # Make a request using S4U2Self. The request should fail.
         kdc_exchange_dict = self.tgs_exchange_dict(
diff --git a/selftest/expectedfail.d/krb5-no-dollar b/selftest/expectedfail.d/krb5-no-dollar
deleted file mode 100644
index 53acc75dab1..00000000000
--- a/selftest/expectedfail.d/krb5-no-dollar
+++ /dev/null
@@ -1,14 +0,0 @@
-# We expect some Kerberos name matching tests to fail in the
-# ad_dc_ntvfs environment, because it has
-#
-#  kdc name match implicit dollar without canonicalization = no
-#
-# set, meaning tests that assert 'foo' matches 'foo$' in principal
-# lookups will fail.
-
-^samba4\.krb5\.kdc\ with\ machine\ account\ no\ dollar\ extension\.canon\.canon\.enterprise\.lc\-user\.no\-win2k\.removedollar\.s4u2self\.canon\.enterprise\.lc\-user\.no\-win2k\.removedollar\.s4u2self\(ad_dc_ntvfs:local\)
-^samba4\.krb5\.kdc\ with\ machine\ account\ no\ dollar\ extension\.canon\.canon\.enterprise\.uc\-user\.no\-win2k\.removedollar\.s4u2self\.canon\.enterprise\.uc\-user\.no\-win2k\.removedollar\.s4u2self\(ad_dc_ntvfs:local\)
-
-
-^samba4\.krb5\.kdc\ with\ machine\ account\ no\ dollar\ extension\.canon\.canon\.enterprise\.lc\-user\.win2k\.removedollar\.s4u2self\.canon\.enterprise\.lc\-user\.win2k\.removedollar\.s4u2self\(ad_dc_ntvfs:local\)
-^samba4\.krb5\.kdc\ with\ machine\ account\ no\ dollar\ extension\.canon\.canon\.enterprise\.uc\-user\.win2k\.removedollar\.s4u2self\.canon\.enterprise\.uc\-user\.win2k\.removedollar\.s4u2self\(ad_dc_ntvfs:local\)
diff --git a/selftest/knownfail_mit_kdc.d/as-canonicalization b/selftest/knownfail_mit_kdc.d/as-canonicalization
index 0eccbeebabd..ca39dbc3ca4 100644
--- a/selftest/knownfail_mit_kdc.d/as-canonicalization
+++ b/selftest/knownfail_mit_kdc.d/as-canonicalization
@@ -52,28 +52,28 @@ samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_
 samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_Enterprise_UpperUserName_UPN_RemoveDollar\(
 samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_Enterprise_UpperUserName_UPN_RemoveDollar_AsReqSelf\(
 samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_NetbiosRealm\(
-samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_NetbiosRealm_RemoveDollar\(
+samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_NetbiosRealm_RemoveDollar\((?!ad_dc_ntvfs)
 samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_NetbiosRealm_UPN\(
-samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_NetbiosRealm_UPN_RemoveDollar\(
-samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_RemoveDollar\(
+samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_NetbiosRealm_UPN_RemoveDollar\((?!ad_dc_ntvfs)
+samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_RemoveDollar\((?!ad_dc_ntvfs)
 samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UPN\(
-samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UPN_RemoveDollar\(
+samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UPN_RemoveDollar\((?!ad_dc_ntvfs)
 samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperRealm_NetbiosRealm\(
-samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperRealm_NetbiosRealm_RemoveDollar\(
+samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperRealm_NetbiosRealm_RemoveDollar\((?!ad_dc_ntvfs)
 samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperRealm_NetbiosRealm_UPN\(
-samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperRealm_NetbiosRealm_UPN_RemoveDollar\(
+samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperRealm_NetbiosRealm_UPN_RemoveDollar\((?!ad_dc_ntvfs)
 samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperRealm_UpperUserName_NetbiosRealm\(
-samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperRealm_UpperUserName_NetbiosRealm_RemoveDollar\(
+samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperRealm_UpperUserName_NetbiosRealm_RemoveDollar\((?!ad_dc_ntvfs)
 samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperRealm_UpperUserName_NetbiosRealm_UPN\(
-samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperRealm_UpperUserName_NetbiosRealm_UPN_RemoveDollar\(
+samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperRealm_UpperUserName_NetbiosRealm_UPN_RemoveDollar\((?!ad_dc_ntvfs)
 samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperUserName\(
 samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperUserName_NetbiosRealm\(
-samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperUserName_NetbiosRealm_RemoveDollar\(
+samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperUserName_NetbiosRealm_RemoveDollar\((?!ad_dc_ntvfs)
 samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperUserName_NetbiosRealm_UPN\(
-samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperUserName_NetbiosRealm_UPN_RemoveDollar\(
-samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperUserName_RemoveDollar\(
+samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperUserName_NetbiosRealm_UPN_RemoveDollar\((?!ad_dc_ntvfs)
+samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperUserName_RemoveDollar\((?!ad_dc_ntvfs)
 samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperUserName_UPN\(
-samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperUserName_UPN_RemoveDollar\(
+samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperUserName_UPN_RemoveDollar\((?!ad_dc_ntvfs)
 samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_UserCredentials\(
 samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_UserCredentials_Enterprise\(
 samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_UserCredentials_Enterprise_NetbiosRealm\(
@@ -162,25 +162,25 @@ samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_
 ^samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_Canonicalize_UpperUserName_UPN_AsReqSelf\(
 ^samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_Canonicalize_UpperUserName_UPN_RemoveDollar_AsReqSelf\(
 ^samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_NetbiosRealm_AsReqSelf\(
-^samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_NetbiosRealm_RemoveDollar_AsReqSelf\(
+^samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_NetbiosRealm_RemoveDollar_AsReqSelf\((?!ad_dc_ntvfs)
 ^samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_NetbiosRealm_UPN_AsReqSelf\(
-^samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_NetbiosRealm_UPN_RemoveDollar_AsReqSelf\(
-^samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_RemoveDollar_AsReqSelf\(
+^samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_NetbiosRealm_UPN_RemoveDollar_AsReqSelf\((?!ad_dc_ntvfs)
+^samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_RemoveDollar_AsReqSelf\((?!ad_dc_ntvfs)
 ^samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UPN_AsReqSelf\(
-^samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UPN_RemoveDollar_AsReqSelf\(
+^samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UPN_RemoveDollar_AsReqSelf\((?!ad_dc_ntvfs)
 ^samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperRealm_NetbiosRealm_AsReqSelf\(
-^samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperRealm_NetbiosRealm_RemoveDollar_AsReqSelf\(
+^samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperRealm_NetbiosRealm_RemoveDollar_AsReqSelf\((?!ad_dc_ntvfs)
 ^samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperRealm_NetbiosRealm_UPN_AsReqSelf\(
-^samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperRealm_NetbiosRealm_UPN_RemoveDollar_AsReqSelf\(
+^samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperRealm_NetbiosRealm_UPN_RemoveDollar_AsReqSelf\((?!ad_dc_ntvfs)
 ^samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperRealm_UpperUserName_NetbiosRealm_AsReqSelf\(
-^samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperRealm_UpperUserName_NetbiosRealm_RemoveDollar_AsReqSelf\(
+^samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperRealm_UpperUserName_NetbiosRealm_RemoveDollar_AsReqSelf\((?!ad_dc_ntvfs)
 ^samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperRealm_UpperUserName_NetbiosRealm_UPN_AsReqSelf\(
-^samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperRealm_UpperUserName_NetbiosRealm_UPN_RemoveDollar_AsReqSelf\(
+^samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperRealm_UpperUserName_NetbiosRealm_UPN_RemoveDollar_AsReqSelf\((?!ad_dc_ntvfs)
 ^samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperUserName_AsReqSelf\(
 ^samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperUserName_NetbiosRealm_AsReqSelf\(
-^samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperUserName_NetbiosRealm_RemoveDollar_AsReqSelf\(
+^samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperUserName_NetbiosRealm_RemoveDollar_AsReqSelf\((?!ad_dc_ntvfs)
 ^samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperUserName_NetbiosRealm_UPN_AsReqSelf\(
-^samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperUserName_NetbiosRealm_UPN_RemoveDollar_AsReqSelf\(
-^samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperUserName_RemoveDollar_AsReqSelf\(
+^samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperUserName_NetbiosRealm_UPN_RemoveDollar_AsReqSelf\((?!ad_dc_ntvfs)
+^samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperUserName_RemoveDollar_AsReqSelf\((?!ad_dc_ntvfs)
 ^samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperUserName_UPN_AsReqSelf\(
-^samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperUserName_UPN_RemoveDollar_AsReqSelf\(
+^samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_tests.KerberosASCanonicalizationTests.test_MachineCredentials_UpperUserName_UPN_RemoveDollar_AsReqSelf\((?!ad_dc_ntvfs)
diff --git a/source4/kdc/db-glue.c b/source4/kdc/db-glue.c
index 32886336339..23feecc6dc0 100644
--- a/source4/kdc/db-glue.c
+++ b/source4/kdc/db-glue.c
@@ -3452,7 +3452,9 @@ static krb5_error_code samba_kdc_lookup_client(krb5_context context,
 			return ret;
 		}
 
-		if (! (sdb_flags & SDB_F_CANON)) {
+		if ((sdb_flags & SDB_F_GET_CLIENT) &&
+		    (sdb_flags & SDB_F_FOR_AS_REQ) &&
+		    ! (sdb_flags & SDB_F_CANON)) {
 			/*
 			 * The client has not requested canonicalisation,
 			 * and the principal has not been found.