From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Date: Fri, 22 Oct 2021 03:03:18 +0000 (+1300)
Subject: CVE-2020-25722 s4/dsdb/samldb: reject SPN with too few/many components
X-Git-Tag: samba-4.13.14~112
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3a4095aec5eb592d4968465930f7fd7e1435e19f;p=thirdparty%2Fsamba.git

CVE-2020-25722 s4/dsdb/samldb: reject SPN with too few/many components

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

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
---

diff --git a/selftest/knownfail.d/ldap_spn b/selftest/knownfail.d/ldap_spn
index b7eb6f30e7a..63f9fe02ef7 100644
--- a/selftest/knownfail.d/ldap_spn
+++ b/selftest/knownfail.d/ldap_spn
@@ -1,3 +1 @@
 samba.tests.ldap_spn.+LdapSpnTest.test_spn_dodgy_spns
-samba.tests.ldap_spn.+LdapSpnTest.test_spn_one_part_spns_no_slashes_
-samba.tests.ldap_spn.+LdapSpnTest.test_spn_too_many_spn_parts
diff --git a/source4/dsdb/samdb/ldb_modules/samldb.c b/source4/dsdb/samdb/ldb_modules/samldb.c
index f5141936a60..006658d2bce 100644
--- a/source4/dsdb/samdb/ldb_modules/samldb.c
+++ b/source4/dsdb/samdb/ldb_modules/samldb.c
@@ -3868,6 +3868,37 @@ static int check_spn_direct_collision(struct ldb_context *ldb,
 }
 
 
+static int count_spn_components(struct ldb_val val)
+{
+	/*
+	 * a 3 part servicePrincipalName has two slashes, like
+	 * ldap/example.com/DomainDNSZones.example.com.
+	 *
+	 * In krb5_parse_name_flags() we don't count "\/" as a slash (i.e.
+	 * escaped by a backslash), but this is not the behaviour of Windows
+	 * on setting a servicePrincipalName -- slashes are counted regardless
+	 * of backslashes.
+	 *
+	 * Accordingly, here we ignore backslashes. This will reject
+	 * multi-slash SPNs that krb5_parse_name_flags() would accept, and
+	 * allow ones in the form "a\/b" that it won't parse.
+	 */
+	size_t i;
+	int slashes = 0;
+	for (i = 0; i < val.length; i++) {
+		char c = val.data[i];
+		if (c == '/') {
+			slashes++;
+			if (slashes == 3) {
+				/* at this point we don't care */
+				return 4;
+			}
+		}
+	}
+	return slashes + 1;
+}
+
+
 /* Check that "servicePrincipalName" changes do not introduce a collision
  * globally. */
 static int samldb_spn_uniqueness_check(struct samldb_ctx *ac,
@@ -3883,8 +3914,18 @@ static int samldb_spn_uniqueness_check(struct samldb_ctx *ac,
 	}
 
 	for (i = 0; i < spn_el->num_values; i++) {
+		int n_components;
 		spn = (char *)spn_el->values[i].data;
 
+		n_components = count_spn_components(spn_el->values[i]);
+		if (n_components > 3 || n_components < 2) {
+			ldb_asprintf_errstring(ldb,
+					       "samldb: spn[%s] invalid with %u components",
+					       spn, n_components);
+			talloc_free(tmp_ctx);
+			return LDB_ERR_CONSTRAINT_VIOLATION;
+		}
+
 		ret = check_spn_direct_collision(ldb,
 						 tmp_ctx,
 						 spn,