CVE-2018-1057: s4:dsdb: use DSDB_CONTROL_PASSWORD_ACL_VALIDATION_OID

This is used to pass information about which password change operation (change
or reset) the acl module validated, down to the password_hash module.

It's very important that both modules treat the request identical.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13272

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>

2 files changed, 67 insertions, 4 deletions

diff --git a/source4/dsdb/samdb/ldb_modules/acl.c b/source4/dsdb/samdb/ldb_modules/acl.c
index f2259261588..9b4be7b6909 100644
--- a/source4/dsdb/samdb/ldb_modules/acl.c
+++ b/source4/dsdb/samdb/ldb_modules/acl.c
@@ -973,13 +973,22 @@ static int acl_check_password_rights(TALLOC_CTX *mem_ctx,
 	const char *passwordAttrs[] = { "userPassword", "clearTextPassword",
 					"unicodePwd", "dBCSPwd", NULL }, **l;
 	TALLOC_CTX *tmp_ctx = talloc_new(mem_ctx);
+	struct dsdb_control_password_acl_validation *pav = NULL;
 
 	if (tmp_ctx == NULL) {
 		return LDB_ERR_OPERATIONS_ERROR;
 	}
 
+	pav = talloc_zero(req, struct dsdb_control_password_acl_validation);
+	if (pav == NULL) {
+		talloc_free(tmp_ctx);
+		return LDB_ERR_OPERATIONS_ERROR;
+	}
+
 	c = ldb_request_get_control(req, DSDB_CONTROL_PASSWORD_CHANGE_OID);
 	if (c != NULL) {
+		pav->pwd_reset = false;
+
 		/*
 		 * The "DSDB_CONTROL_PASSWORD_CHANGE_OID" control means that we
 		 * have a user password change and not a set as the message
@@ -1002,6 +1011,8 @@ static int acl_check_password_rights(TALLOC_CTX *mem_ctx,
 
 	c = ldb_request_get_control(req, DSDB_CONTROL_PASSWORD_HASH_VALUES_OID);
 	if (c != NULL) {
+		pav->pwd_reset = true;
+
 		/*
 		 * The "DSDB_CONTROL_PASSWORD_HASH_VALUES_OID" control, without
 		 * "DSDB_CONTROL_PASSWORD_CHANGE_OID" control means that we
@@ -1055,6 +1066,8 @@ static int acl_check_password_rights(TALLOC_CTX *mem_ctx,
 
 
 	if (rep_attr_cnt > 0) {
+		pav->pwd_reset = true;
+
 		ret = acl_check_extended_right(tmp_ctx, sd, acl_user_token(module),
 					       GUID_DRS_FORCE_CHANGE_PASSWORD,
 					       SEC_ADS_CONTROL_ACCESS,
@@ -1063,6 +1076,8 @@ static int acl_check_password_rights(TALLOC_CTX *mem_ctx,
 	}
 
 	if (add_attr_cnt != del_attr_cnt) {
+		pav->pwd_reset = true;
+
 		ret = acl_check_extended_right(tmp_ctx, sd, acl_user_token(module),
 					       GUID_DRS_FORCE_CHANGE_PASSWORD,
 					       SEC_ADS_CONTROL_ACCESS,
@@ -1071,6 +1086,8 @@ static int acl_check_password_rights(TALLOC_CTX *mem_ctx,
 	}
 
 	if (add_val_cnt == 1 && del_val_cnt == 1) {
+		pav->pwd_reset = false;
+
 		ret = acl_check_extended_right(tmp_ctx, sd, acl_user_token(module),
 					       GUID_DRS_USER_CHANGE_PASSWORD,
 					       SEC_ADS_CONTROL_ACCESS,
@@ -1083,6 +1100,8 @@ static int acl_check_password_rights(TALLOC_CTX *mem_ctx,
 	}
 
 	if (add_val_cnt == 1 && del_val_cnt == 0) {
+		pav->pwd_reset = true;
+
 		ret = acl_check_extended_right(tmp_ctx, sd, acl_user_token(module),
 					       GUID_DRS_FORCE_CHANGE_PASSWORD,
 					       SEC_ADS_CONTROL_ACCESS,
@@ -1094,6 +1113,14 @@ static int acl_check_password_rights(TALLOC_CTX *mem_ctx,
 		goto checked;
 	}
 
+	/*
+	 * Everything else is handled by the password_hash module where it will
+	 * fail, but with the correct error code when the module is again
+	 * checking the attributes. As the change request will lack the
+	 * DSDB_CONTROL_PASSWORD_ACL_VALIDATION_OID control, we can be sure that
+	 * any modification attempt that went this way will be rejected.
+	 */
+
 	talloc_free(tmp_ctx);
 	return LDB_SUCCESS;
 
@@ -1103,11 +1130,19 @@ checked:
 			       req->op.mod.message->dn,
 			       true,
 			       10);
+		talloc_free(tmp_ctx);
+		return ret;
 	}
-	talloc_free(tmp_ctx);
-	return ret;
-}
 
+	ret = ldb_request_add_control(req,
+		DSDB_CONTROL_PASSWORD_ACL_VALIDATION_OID, false, pav);
+	if (ret != LDB_SUCCESS) {
+		ldb_debug(ldb_module_get_ctx(module), LDB_DEBUG_ERROR,
+			  "Unable to register ACL validation control!\n");
+		return ret;
+	}
+	return LDB_SUCCESS;
+}
 
 static int acl_modify(struct ldb_module *module, struct ldb_request *req)
 {
diff --git a/source4/dsdb/samdb/ldb_modules/password_hash.c b/source4/dsdb/samdb/ldb_modules/password_hash.c
index 1e7081b997a..3063d80f63b 100644
--- a/source4/dsdb/samdb/ldb_modules/password_hash.c
+++ b/source4/dsdb/samdb/ldb_modules/password_hash.c
@@ -3188,7 +3188,35 @@ static int setup_io(struct ph_context *ac,
 		/* On "add" we have only "password reset" */
 		ac->pwd_reset = true;
 	} else if (ac->req->operation == LDB_MODIFY) {
-		if (io->og.cleartext_utf8 || io->og.cleartext_utf16
+		struct ldb_control *pav_ctrl = NULL;
+		struct dsdb_control_password_acl_validation *pav = NULL;
+
+		pav_ctrl = ldb_request_get_control(ac->req,
+				DSDB_CONTROL_PASSWORD_ACL_VALIDATION_OID);
+		if (pav_ctrl != NULL) {
+			pav = talloc_get_type_abort(pav_ctrl->data,
+				struct dsdb_control_password_acl_validation);
+		}
+
+		if (pav == NULL && ac->update_password) {
+			bool ok;
+
+			/*
+			 * If the DSDB_CONTROL_PASSWORD_ACL_VALIDATION_OID
+			 * control is missing, we require system access!
+			 */
+			ok = dsdb_module_am_system(ac->module);
+			if (!ok) {
+				return ldb_module_operr(ac->module);
+			}
+		}
+
+		if (pav != NULL) {
+			/*
+			 * We assume what the acl module has validated.
+			 */
+			ac->pwd_reset = pav->pwd_reset;
+		} else if (io->og.cleartext_utf8 || io->og.cleartext_utf16
 		    || io->og.nt_hash || io->og.lm_hash) {
 			/* If we have an old password specified then for sure it
 			 * is a user "password change" */