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>

diff --git a/source4/dsdb/samdb/ldb_modules/acl.c b/source4/dsdb/samdb/ldb_modules/acl.c
index f22592615880a204a5853ac84d7a9d8588f68343..9b4be7b6909e4ad205a47207a83326308a9b83b6 100644 (file)
--- 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 315d7d13719426c97c2ca3e31b025d07ebf23aff..1c1de09de77306ee8795a8afb0d5a480f6927feb 100644 (file)
--- a/source4/dsdb/samdb/ldb_modules/password_hash.c
+++ b/source4/dsdb/samdb/ldb_modules/password_hash.c
@@ -3512,7 +3512,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" */