File _service:tar_scm_kernel_repo:backport-0005-CVE-... of Package samba

File _service:tar_scm_kernel_repo:backport-0005-CVE-2020-25722-dsdb-Add-restrictions-on-computer-acc.patch of Package samba

From a76d5d6202379c1f38f7f91df1cce006754463b2 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet@samba.org>
Date: Wed, 22 Sep 2021 11:29:02 +1200
Subject: [PATCH 070/266] CVE-2020-25722 dsdb: Add restrictions on computer
 accounts without a trailing $

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

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

Conflict:remove test
Reference:https://gitlab.com/samba-team/samba/-/commit/a76d5d6202379c1f38f7f91df1cce006754463b2
---
 source4/dsdb/samdb/ldb_modules/samldb.c | 172 +++++++++++++++++++++---
 1 file changed, 155 insertions(+), 17 deletions(-)

diff --git a/source4/dsdb/samdb/ldb_modules/samldb.c b/source4/dsdb/samdb/ldb_modules/samldb.c
index 283a2de..7881dd0 100644
--- a/source4/dsdb/samdb/ldb_modules/samldb.c
+++ b/source4/dsdb/samdb/ldb_modules/samldb.c
@@ -68,6 +68,13 @@ struct samldb_ctx {
 	/* used for add operations */
 	enum samldb_add_type type;
 
+	/*
+	 * should we apply the need_trailing_dollar restriction to
+	 * samAccountName
+	 */
+
+	bool need_trailing_dollar;
+
 	/* the resulting message */
 	struct ldb_message *msg;
 
@@ -232,12 +239,86 @@ static int samldb_unique_attr_check(struct samldb_ctx *ac, const char *attr,
 
 static int samldb_sam_accountname_valid_check(struct samldb_ctx *ac)
 {
-	int ret = samldb_unique_attr_check(ac, "samAccountName", NULL,
-					   ldb_get_default_basedn(
-						   ldb_module_get_ctx(ac->module)));
-	if (ret == LDB_ERR_OBJECT_CLASS_VIOLATION) {
+	int ret = 0;
+	bool is_admin;
+	struct security_token *user_token = NULL;
+	struct ldb_context *ldb = ldb_module_get_ctx(ac->module);
+	struct ldb_message_element *el = dsdb_get_single_valued_attr(ac->msg, "samAccountName",
+					 ac->req->operation);
+	if (el == NULL || el->num_values == 0) {
+		ldb_asprintf_errstring(ldb,
+			"%08X: samldb: 'samAccountName' can't be deleted/empty!",
+			W_ERROR_V(WERR_DS_ILLEGAL_MOD_OPERATION));
+		if (ac->req->operation == LDB_ADD) {
+			return LDB_ERR_CONSTRAINT_VIOLATION;
+		} else {
+			return LDB_ERR_UNWILLING_TO_PERFORM;
+		}
+	}
+
+	ret = samldb_unique_attr_check(ac, "samAccountName", NULL,
+				       ldb_get_default_basedn(
+					       ldb_module_get_ctx(ac->module)));
+
+	/*
+	 * Error code munging to try and match what must be some quite
+	 * strange code-paths in Windows
+	 */
+	if (ret == LDB_ERR_CONSTRAINT_VIOLATION
+	    && ac->req->operation == LDB_MODIFY) {
+		ret = LDB_ERR_ATTRIBUTE_OR_VALUE_EXISTS;
+	} else if (ret == LDB_ERR_OBJECT_CLASS_VIOLATION) {
 		ret = LDB_ERR_CONSTRAINT_VIOLATION;
 	}
+
+	if (ret != LDB_SUCCESS) {
+		return ret;
+	}
+
+	if (!ac->need_trailing_dollar) {
+		return LDB_SUCCESS;
+	}
+
+	/* This does not permit a single $ */
+	if (el->values[0].length < 2) {
+		ldb_asprintf_errstring(ldb,
+				       "%08X: samldb: 'samAccountName' "
+				       "can't just be one character!",
+			W_ERROR_V(WERR_DS_ILLEGAL_MOD_OPERATION));
+		return LDB_ERR_UNWILLING_TO_PERFORM;
+	}
+
+	user_token = acl_user_token(ac->module);
+	if (user_token == NULL) {
+		return LDB_ERR_INSUFFICIENT_ACCESS_RIGHTS;
+	}
+
+	is_admin
+		= security_token_has_builtin_administrators(user_token);
+
+	if (is_admin) {
+		/*
+		 * Administrators are allowed to select strange names.
+		 * This is poor practice but not prevented.
+		 */
+		return false;
+	}
+
+	if (el->values[0].data[el->values[0].length - 1] != '$') {
+		ldb_asprintf_errstring(ldb,
+				       "%08X: samldb: 'samAccountName' "
+				       "must have a trailing $!",
+			W_ERROR_V(WERR_DS_ILLEGAL_MOD_OPERATION));
+		return LDB_ERR_UNWILLING_TO_PERFORM;
+	}
+	if (el->values[0].data[el->values[0].length - 2] == '$') {
+		ldb_asprintf_errstring(ldb,
+				       "%08X: samldb: 'samAccountName' "
+				       "must not have a double trailing $!",
+			W_ERROR_V(WERR_DS_ILLEGAL_MOD_OPERATION));
+		return LDB_ERR_UNWILLING_TO_PERFORM;
+	}
+
 	return ret;
 }
 
@@ -554,17 +635,32 @@ static int samldb_schema_add_handle_mapiid(struct samldb_ctx *ac)
 }
 
 /* sAMAccountName handling */
-static int samldb_generate_sAMAccountName(struct ldb_context *ldb,
+static int samldb_generate_sAMAccountName(struct samldb_ctx *ac,
 					  struct ldb_message *msg)
 {
+	struct ldb_context *ldb = ldb_module_get_ctx(ac->module);
 	char *name;
 
-	/* Format: $000000-000000000000 */
+        /*
+	 * This is currently a Samba-only behaviour, to add a trailing
+	 * $ even for the generated accounts.
+	 */
+
+	if (ac->need_trailing_dollar) {
+		/* Format: $000000-00000000000$ */
+		name = talloc_asprintf(msg, "$%.6X-%.6X%.5X$",
+				       (unsigned int)generate_random(),
+				       (unsigned int)generate_random(),
+				       (unsigned int)generate_random());
+	} else {
+		/* Format: $000000-000000000000 */
+               
+	       name = talloc_asprintf(msg, "$%.6X-%.6X%.6X",
+				      (unsigned int)generate_random(),
+				      (unsigned int)generate_random(),
+				      (unsigned int)generate_random());
+        }
 
-	name = talloc_asprintf(msg, "$%.6X-%.6X%.6X",
-				(unsigned int)generate_random(),
-				(unsigned int)generate_random(),
-				(unsigned int)generate_random());
 	if (name == NULL) {
 		return ldb_oom(ldb);
 	}
@@ -573,11 +669,10 @@ static int samldb_generate_sAMAccountName(struct ldb_context *ldb,
 
 static int samldb_check_sAMAccountName(struct samldb_ctx *ac)
 {
-	struct ldb_context *ldb = ldb_module_get_ctx(ac->module);
 	int ret;
 
 	if (ldb_msg_find_element(ac->msg, "sAMAccountName") == NULL) {
-		ret = samldb_generate_sAMAccountName(ldb, ac->msg);
+		ret = samldb_generate_sAMAccountName(ac, ac->msg);
 		if (ret != LDB_SUCCESS) {
 			return ret;
 		}
@@ -1473,6 +1568,20 @@ static int samldb_objectclass_trigger(struct samldb_ctx *ac)
 			if (ret != LDB_SUCCESS) {
 				return ret;
 			}
+                        
+			/*
+		         * Require, for non-admin modifications, a trailing $
+		         * for either objectclass=computer or a trust account
+		         * type in userAccountControl
+		         */
+		        if ((user_account_control
+		             & UF_TRUST_ACCOUNT_MASK) != 0) {
+			        ac->need_trailing_dollar = true;
+		        }
+
+		        if (is_computer_objectclass) {
+			        ac->need_trailing_dollar = true;
+		        }
 
 			/* add "sAMAccountType" attribute */
 			ret = dsdb_user_obj_set_account_type(ldb, ac->msg, user_account_control, NULL);
@@ -3989,12 +4098,41 @@ static int samldb_modify(struct ldb_module *module, struct ldb_request *req)
 
 	el = ldb_msg_find_element(ac->msg, "sAMAccountName");
 	if (el != NULL) {
+		uint32_t user_account_control;
+		struct ldb_result *res = NULL;
+		const char * const attrs[] = { "userAccountControl",
+					       "objectclass",
+					       NULL };
+		ret = dsdb_module_search_dn(ac->module,
+					    ac,
+					    &res,
+					    ac->msg->dn,
+					    attrs,
+					    DSDB_FLAG_NEXT_MODULE | DSDB_SEARCH_SHOW_DELETED,
+					    ac->req);
+		if (ret != LDB_SUCCESS) {
+			return ret;
+		}
+
+		user_account_control
+			= ldb_msg_find_attr_as_uint(res->msgs[0],
+						    "userAccountControl",
+						    0);
+
+		if ((user_account_control
+		     & UF_TRUST_ACCOUNT_MASK) != 0) {
+			ac->need_trailing_dollar = true;
+
+		} else if (samdb_find_attribute(ldb,
+						res->msgs[0],
+						"objectclass",
+						"computer")
+			   != NULL) {
+			ac->need_trailing_dollar = true;
+		}
+
 		ret = samldb_sam_accountname_valid_check(ac);
-		/*
-		 * Other errors are checked for elsewhere, we just
-		 * want to prevent duplicates
-		 */
-		if (ret == LDB_ERR_ENTRY_ALREADY_EXISTS) {
+		if (ret != LDB_SUCCESS) {
 			return ret;
 		}
 	}
-- 
2.27.0