From 86b41e9c618a8b0455a5ea571a9078fcaddc4eec Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Fri, 16 Feb 2018 15:38:19 +0100 Subject: [PATCH] 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 Reviewed-by: Stefan Metzmacher --- source4/dsdb/samdb/ldb_modules/acl.c | 41 ++++++++++++++++++++++++-- source4/dsdb/samdb/ldb_modules/password_hash.c | 30 ++++++++++++++++++- 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 e8af7e81c19..3a2582106d3 100644 --- a/source4/dsdb/samdb/ldb_modules/password_hash.c +++ b/source4/dsdb/samdb/ldb_modules/password_hash.c @@ -3500,7 +3500,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" */ -- 2.11.4.GIT