aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRalph Boehme <slow@samba.org>2018-02-16 15:38:19 +0100
committerKarolin Seeger <kseeger@samba.org>2018-03-12 10:05:43 +0100
commitc1de637a37121d0e28d502d8b2ef507e7e8dd57f (patch)
treed71801fdfe2bbfe9c0669695b48204827c5ce7d8
parent06032bffca2352e3e7757214563f6e97d4f162df (diff)
downloadsamba-c1de637a37121d0e28d502d8b2ef507e7e8dd57f.tar.gz
samba-c1de637a37121d0e28d502d8b2ef507e7e8dd57f.tar.xz
samba-c1de637a37121d0e28d502d8b2ef507e7e8dd57f.zip
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>
-rw-r--r--source4/dsdb/samdb/ldb_modules/acl.c41
-rw-r--r--source4/dsdb/samdb/ldb_modules/password_hash.c30
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" */