The branch, v4-8-test has been updated
       via  03e63dd CVE-2018-1050: s3: RPC: spoolss server. Protect against 
null pointer derefs.
       via  87b10d3 CVE-2018-1057: s4:dsdb/acl: changing dBCSPwd is only 
allowed with a control
       via  5c957af CVE-2018-1057: s4:dsdb: use 
DSDB_CONTROL_PASSWORD_ACL_VALIDATION_OID
       via  6335660 CVE-2018-1057: s4:dsdb/samdb: define 
DSDB_CONTROL_PASSWORD_ACL_VALIDATION_OID control
       via  f8ff72d CVE-2018-1057: s4:dsdb/acl: run password checking only once
       via  4e30547 CVE-2018-1057: s4/dsdb: correctly detect password resets
       via  bd39608 CVE-2018-1057: s4:dsdb/acl: add a NULL check for 
talloc_new() in acl_check_password_rights()
       via  b152db9 CVE-2018-1057: s4:dsdb/acl: add check for 
DSDB_CONTROL_PASSWORD_HASH_VALUES_OID control
       via  93e11c7 CVE-2018-1057: s4:dsdb/acl: check for internal controls 
before other checks
       via  9e7dc49 CVE-2018-1057: s4:dsdb/acl: remove unused else branches in 
acl_check_password_rights()
       via  be3c583 CVE-2018-1057: s4:dsdb/acl: only call dsdb_acl_debug() if 
we checked the acl in acl_check_password_rights()
       via  9a3f754 CVE-2018-1057: s4:dsdb/password_hash: add a helper variable 
for passwordAttr->num_values
       via  231ed98 CVE-2018-1057: s4:dsdb/password_hash: add a helper variable 
for LDB_FLAG_MOD_TYPE
       via  ccb38e9 CVE-2018-1057: s4:dsdb/tests: add a test for password 
change with empty delete
      from  60c7969 WHATSNEW: Domain member setups require winbindd

https://git.samba.org/?p=samba.git;a=shortlog;h=v4-8-test


- Log -----------------------------------------------------------------
commit 03e63dd9841085ee16993d74dff4e62957298bbd
Author: Jeremy Allison <j...@samba.org>
Date:   Tue Jan 2 15:56:03 2018 -0800

    CVE-2018-1050: s3: RPC: spoolss server. Protect against null pointer derefs.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=11343
    
    Signed-off-by: Jeremy Allison <j...@samba.org>
    Reviewed-by: Ralph Boehme <s...@samba.org>
    
    Autobuild-User(v4-8-test): Stefan Metzmacher <me...@samba.org>
    Autobuild-Date(v4-8-test): Tue Mar 13 15:58:25 CET 2018 on sn-devel-144

commit 87b10d37533950abf793f64b43542632b3cb40ae
Author: Ralph Boehme <s...@samba.org>
Date:   Thu Feb 15 23:11:38 2018 +0100

    CVE-2018-1057: s4:dsdb/acl: changing dBCSPwd is only allowed with a control
    
    This is not strictly needed to fig bug 13272, but it makes sense to also
    fix this while fixing the overall ACL checking logic.
    
    Bug: https://bugzilla.samba.org/show_bug.cgi?id=13272
    
    Signed-off-by: Ralph Boehme <s...@samba.org>
    Reviewed-by: Stefan Metzmacher <me...@samba.org>

commit 5c957af090354f678a75cb59861a3a61ef24333e
Author: Ralph Boehme <s...@samba.org>
Date:   Fri Feb 16 15:38:19 2018 +0100

    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 <s...@samba.org>
    Reviewed-by: Stefan Metzmacher <me...@samba.org>

commit 6335660ea218fe59f461658db0be364b8b58b4ca
Author: Ralph Boehme <s...@samba.org>
Date:   Fri Feb 16 15:30:13 2018 +0100

    CVE-2018-1057: s4:dsdb/samdb: define 
DSDB_CONTROL_PASSWORD_ACL_VALIDATION_OID control
    
    Will be used to pass "user password change" vs "password reset" from the
    ACL to the password_hash module, ensuring both modules treat the request
    identical.
    
    Bug: https://bugzilla.samba.org/show_bug.cgi?id=13272
    
    Signed-off-by: Ralph Boehme <s...@samba.org>
    Reviewed-by: Stefan Metzmacher <me...@samba.org>

commit f8ff72d75bef5552eb00852a3012db44261d423f
Author: Ralph Boehme <s...@samba.org>
Date:   Wed Feb 14 19:15:49 2018 +0100

    CVE-2018-1057: s4:dsdb/acl: run password checking only once
    
    This is needed, because a later commit will let the acl module add a
    control to the change request msg and we must ensure that this is only
    done once.
    
    Bug: https://bugzilla.samba.org/show_bug.cgi?id=13272
    
    Signed-off-by: Ralph Boehme <s...@samba.org>
    Reviewed-by: Stefan Metzmacher <me...@samba.org>

commit 4e30547371cf9e38cd7a219dd43c9bc5c7a2a7fb
Author: Ralph Boehme <s...@samba.org>
Date:   Thu Feb 22 10:54:37 2018 +0100

    CVE-2018-1057: s4/dsdb: correctly detect password resets
    
    This change ensures we correctly treat the following LDIF
    
      dn: cn=testuser,cn=users,...
      changetype: modify
      delete: userPassword
      add: userPassword
      userPassword: thatsAcomplPASS1
    
    as a password reset. Because delete and add element counts are both
    one, the ACL module wrongly treated this as a password change
    request.
    
    For a password change we need at least one value to delete and one value
    to add. This patch ensures we correctly check attributes and their
    values.
    
    Bug: https://bugzilla.samba.org/show_bug.cgi?id=13272
    
    Signed-off-by: Ralph Boehme <s...@samba.org>
    Reviewed-by: Stefan Metzmacher <me...@samba.org>

commit bd3960888e1f8ef89b35e2075fc17a6ae525cb9e
Author: Ralph Boehme <s...@samba.org>
Date:   Fri Feb 16 15:17:26 2018 +0100

    CVE-2018-1057: s4:dsdb/acl: add a NULL check for talloc_new() in 
acl_check_password_rights()
    
    Bug: https://bugzilla.samba.org/show_bug.cgi?id=13272
    
    Signed-off-by: Ralph Boehme <s...@samba.org>
    Reviewed-by: Stefan Metzmacher <me...@samba.org>

commit b152db938781f6fbd7882617f2c883ee141ae438
Author: Ralph Boehme <s...@samba.org>
Date:   Thu Feb 15 17:43:43 2018 +0100

    CVE-2018-1057: s4:dsdb/acl: add check for 
DSDB_CONTROL_PASSWORD_HASH_VALUES_OID control
    
    Bug: https://bugzilla.samba.org/show_bug.cgi?id=13272
    
    Signed-off-by: Ralph Boehme <s...@samba.org>
    Reviewed-by: Stefan Metzmacher <me...@samba.org>

commit 93e11c7f4919cdea830d5d02b25ceff45cb95b2e
Author: Ralph Boehme <s...@samba.org>
Date:   Thu Feb 15 22:59:24 2018 +0100

    CVE-2018-1057: s4:dsdb/acl: check for internal controls before other checks
    
    Bug: https://bugzilla.samba.org/show_bug.cgi?id=13272
    
    Signed-off-by: Ralph Boehme <s...@samba.org>
    Reviewed-by: Stefan Metzmacher <me...@samba.org>

commit 9e7dc491fe59cb37dac14896af13f878da749876
Author: Ralph Boehme <s...@samba.org>
Date:   Thu Feb 15 17:38:31 2018 +0100

    CVE-2018-1057: s4:dsdb/acl: remove unused else branches in 
acl_check_password_rights()
    
    Bug: https://bugzilla.samba.org/show_bug.cgi?id=13272
    
    Signed-off-by: Ralph Boehme <s...@samba.org>
    Reviewed-by: Stefan Metzmacher <me...@samba.org>

commit be3c583e541e8467090c1e9c2f162c623c60ddb0
Author: Ralph Boehme <s...@samba.org>
Date:   Thu Feb 15 17:38:31 2018 +0100

    CVE-2018-1057: s4:dsdb/acl: only call dsdb_acl_debug() if we checked the 
acl in acl_check_password_rights()
    
    Bug: https://bugzilla.samba.org/show_bug.cgi?id=13272
    
    Signed-off-by: Ralph Boehme <s...@samba.org>
    Reviewed-by: Stefan Metzmacher <me...@samba.org>

commit 9a3f754ec7df9bc587bbb9f8a541a97d9c9b071a
Author: Ralph Boehme <s...@samba.org>
Date:   Thu Feb 15 14:40:59 2018 +0100

    CVE-2018-1057: s4:dsdb/password_hash: add a helper variable for 
passwordAttr->num_values
    
    Bug: https://bugzilla.samba.org/show_bug.cgi?id=13272
    
    Signed-off-by: Ralph Boehme <s...@samba.org>
    Reviewed-by: Stefan Metzmacher <me...@samba.org>

commit 231ed98447dbc310118096b330abf673c5c5d3db
Author: Ralph Boehme <s...@samba.org>
Date:   Thu Feb 15 10:56:06 2018 +0100

    CVE-2018-1057: s4:dsdb/password_hash: add a helper variable for 
LDB_FLAG_MOD_TYPE
    
    Bug: https://bugzilla.samba.org/show_bug.cgi?id=13272
    
    Signed-off-by: Ralph Boehme <s...@samba.org>
    Reviewed-by: Stefan Metzmacher <me...@samba.org>

commit ccb38e965a2bfa875b5988e160924946dd437662
Author: Ralph Boehme <s...@samba.org>
Date:   Thu Feb 15 12:43:09 2018 +0100

    CVE-2018-1057: s4:dsdb/tests: add a test for password change with empty 
delete
    
    Note that the request using the clearTextPassword attribute for the
    password change is already correctly rejected by the server.
    
    Bug: https://bugzilla.samba.org/show_bug.cgi?id=13272
    
    Signed-off-by: Ralph Boehme <s...@samba.org>
    Reviewed-by: Stefan Metzmacher <me...@samba.org>

-----------------------------------------------------------------------

Summary of changes:
 source3/rpc_server/spoolss/srv_spoolss_nt.c    |  13 +++
 source4/dsdb/samdb/ldb_modules/acl.c           | 146 ++++++++++++++++++++++---
 source4/dsdb/samdb/ldb_modules/password_hash.c |  45 ++++++--
 source4/dsdb/samdb/samdb.h                     |   9 ++
 source4/dsdb/tests/python/passwords.py         |  49 +++++++++
 source4/libcli/ldap/ldap_controls.c            |   1 +
 source4/setup/schema_samba4.ldif               |   1 +
 7 files changed, 241 insertions(+), 23 deletions(-)


Changeset truncated at 500 lines:

diff --git a/source3/rpc_server/spoolss/srv_spoolss_nt.c 
b/source3/rpc_server/spoolss/srv_spoolss_nt.c
index f0226ba..f6e680e 100644
--- a/source3/rpc_server/spoolss/srv_spoolss_nt.c
+++ b/source3/rpc_server/spoolss/srv_spoolss_nt.c
@@ -142,6 +142,11 @@ static void prune_printername_cache(void);
 static const char *canon_servername(const char *servername)
 {
        const char *pservername = servername;
+
+       if (servername == NULL) {
+               return "";
+       }
+
        while (*pservername == '\\') {
                pservername++;
        }
@@ -2042,6 +2047,10 @@ WERROR _spoolss_DeletePrinterDriver(struct pipes_struct 
*p,
                return WERR_ACCESS_DENIED;
        }
 
+       if (r->in.architecture == NULL || r->in.driver == NULL) {
+               return WERR_INVALID_ENVIRONMENT;
+       }
+
        /* check that we have a valid driver name first */
 
        if ((version = get_version_id(r->in.architecture)) == -1) {
@@ -2181,6 +2190,10 @@ WERROR _spoolss_DeletePrinterDriverEx(struct 
pipes_struct *p,
                return WERR_ACCESS_DENIED;
        }
 
+       if (r->in.architecture == NULL || r->in.driver == NULL) {
+               return WERR_INVALID_ENVIRONMENT;
+       }
+
        /* check that we have a valid driver name first */
        if (get_version_id(r->in.architecture) == -1) {
                /* this is what NT returns */
diff --git a/source4/dsdb/samdb/ldb_modules/acl.c 
b/source4/dsdb/samdb/ldb_modules/acl.c
index 27d4e76..d750362 100644
--- a/source4/dsdb/samdb/ldb_modules/acl.c
+++ b/source4/dsdb/samdb/ldb_modules/acl.c
@@ -966,11 +966,79 @@ static int acl_check_password_rights(TALLOC_CTX *mem_ctx,
 {
        int ret = LDB_SUCCESS;
        unsigned int del_attr_cnt = 0, add_attr_cnt = 0, rep_attr_cnt = 0;
+       unsigned int del_val_cnt = 0, add_val_cnt = 0, rep_val_cnt = 0;
        struct ldb_message_element *el;
        struct ldb_message *msg;
+       struct ldb_control *c = NULL;
        const char *passwordAttrs[] = { "userPassword", "clearTextPassword",
-                                       "unicodePwd", "dBCSPwd", NULL }, **l;
+                                       "unicodePwd", 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
+                * looks like. In it's value blob it contains the NT and/or LM
+                * hash of the old password specified by the user.  This control
+                * is used by the SAMR and "kpasswd" password change mechanisms.
+                *
+                * This control can't be used by real LDAP clients,
+                * the only caller is samdb_set_password_internal(),
+                * so we don't have to strict verification of the input.
+                */
+               ret = acl_check_extended_right(tmp_ctx,
+                                              sd,
+                                              acl_user_token(module),
+                                              GUID_DRS_USER_CHANGE_PASSWORD,
+                                              SEC_ADS_CONTROL_ACCESS,
+                                              sid);
+               goto checked;
+       }
+
+       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
+                * have a force password set.
+                * This control is used by the SAMR/NETLOGON/LSA password
+                * reset mechanisms.
+                *
+                * This control can't be used by real LDAP clients,
+                * the only caller is samdb_set_password_internal(),
+                * so we don't have to strict verification of the input.
+                */
+               ret = acl_check_extended_right(tmp_ctx, sd, 
acl_user_token(module),
+                                              GUID_DRS_FORCE_CHANGE_PASSWORD,
+                                              SEC_ADS_CONTROL_ACCESS,
+                                              sid);
+               goto checked;
+       }
+
+       el = ldb_msg_find_element(req->op.mod.message, "dBCSPwd");
+       if (el != NULL) {
+               /*
+                * dBCSPwd is only allowed with a control.
+                */
+               talloc_free(tmp_ctx);
+               return LDB_ERR_UNWILLING_TO_PERFORM;
+       }
 
        msg = ldb_msg_copy_shallow(tmp_ctx, req->op.mod.message);
        if (msg == NULL) {
@@ -984,12 +1052,15 @@ static int acl_check_password_rights(TALLOC_CTX *mem_ctx,
                while ((el = ldb_msg_find_element(msg, *l)) != NULL) {
                        if (LDB_FLAG_MOD_TYPE(el->flags) == 
LDB_FLAG_MOD_DELETE) {
                                ++del_attr_cnt;
+                               del_val_cnt += el->num_values;
                        }
                        if (LDB_FLAG_MOD_TYPE(el->flags) == LDB_FLAG_MOD_ADD) {
                                ++add_attr_cnt;
+                               add_val_cnt += el->num_values;
                        }
                        if (LDB_FLAG_MOD_TYPE(el->flags) == 
LDB_FLAG_MOD_REPLACE) {
                                ++rep_attr_cnt;
+                               rep_val_cnt += el->num_values;
                        }
                        ldb_msg_remove_element(msg, el);
                }
@@ -1002,26 +1073,30 @@ static int acl_check_password_rights(TALLOC_CTX 
*mem_ctx,
                return LDB_SUCCESS;
        }
 
-       if (ldb_request_get_control(req,
-                                   DSDB_CONTROL_PASSWORD_CHANGE_OID) != NULL) {
-               /* The "DSDB_CONTROL_PASSWORD_CHANGE_OID" control means that we
-                * have a user password change and not a set as the message
-                * looks like. In it's value blob it contains the NT and/or LM
-                * hash of the old password specified by the user.
-                * This control is used by the SAMR and "kpasswd" password
-                * change mechanisms. */
+
+       if (rep_attr_cnt > 0) {
+               pav->pwd_reset = true;
+
                ret = acl_check_extended_right(tmp_ctx, sd, 
acl_user_token(module),
-                                              GUID_DRS_USER_CHANGE_PASSWORD,
+                                              GUID_DRS_FORCE_CHANGE_PASSWORD,
                                               SEC_ADS_CONTROL_ACCESS,
                                               sid);
+               goto checked;
        }
-       else if (rep_attr_cnt > 0 || (add_attr_cnt != del_attr_cnt)) {
+
+       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,
                                               sid);
+               goto checked;
        }
-       else if (add_attr_cnt == 1 && del_attr_cnt == 1) {
+
+       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,
@@ -1030,17 +1105,53 @@ static int acl_check_password_rights(TALLOC_CTX 
*mem_ctx,
                if (ret == LDB_ERR_INSUFFICIENT_ACCESS_RIGHTS) {
                        ret = LDB_ERR_CONSTRAINT_VIOLATION;
                }
+               goto checked;
+       }
+
+       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,
+                                              sid);
+               /* Very strange, but we get constraint violation in this case */
+               if (ret == LDB_ERR_INSUFFICIENT_ACCESS_RIGHTS) {
+                       ret = LDB_ERR_CONSTRAINT_VIOLATION;
+               }
+               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;
+
+checked:
        if (ret != LDB_SUCCESS) {
                dsdb_acl_debug(sd, acl_user_token(module),
                               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)
 {
@@ -1055,6 +1166,7 @@ static int acl_modify(struct ldb_module *module, struct 
ldb_request *req)
        struct ldb_control *as_system;
        struct ldb_control *is_undelete;
        bool userPassword;
+       bool password_rights_checked = false;
        TALLOC_CTX *tmp_ctx;
        const struct ldb_message *msg = req->op.mod.message;
        static const char *acl_attrs[] = {
@@ -1200,6 +1312,9 @@ static int acl_modify(struct ldb_module *module, struct 
ldb_request *req)
                } else if (ldb_attr_cmp("unicodePwd", el->name) == 0 ||
                           (userPassword && ldb_attr_cmp("userPassword", 
el->name) == 0) ||
                           ldb_attr_cmp("clearTextPassword", el->name) == 0) {
+                       if (password_rights_checked) {
+                               continue;
+                       }
                        ret = acl_check_password_rights(tmp_ctx,
                                                        module,
                                                        req,
@@ -1210,6 +1325,7 @@ static int acl_modify(struct ldb_module *module, struct 
ldb_request *req)
                        if (ret != LDB_SUCCESS) {
                                goto fail;
                        }
+                       password_rights_checked = true;
                } else if (ldb_attr_cmp("servicePrincipalName", el->name) == 0) 
{
                        ret = acl_check_spn(tmp_ctx,
                                            module,
diff --git a/source4/dsdb/samdb/ldb_modules/password_hash.c 
b/source4/dsdb/samdb/ldb_modules/password_hash.c
index c428ff7..1ddafb3 100644
--- 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" */
@@ -4246,25 +4274,26 @@ static int password_hash_modify(struct ldb_module 
*module, struct ldb_request *r
                }
 
                while ((passwordAttr = ldb_msg_find_element(msg, *l)) != NULL) {
-                       if (LDB_FLAG_MOD_TYPE(passwordAttr->flags) == 
LDB_FLAG_MOD_DELETE) {
+                       unsigned int mtype = 
LDB_FLAG_MOD_TYPE(passwordAttr->flags);
+                       unsigned int nvalues = passwordAttr->num_values;
+
+                       if (mtype == LDB_FLAG_MOD_DELETE) {
                                ++del_attr_cnt;
                        }
-                       if (LDB_FLAG_MOD_TYPE(passwordAttr->flags) == 
LDB_FLAG_MOD_ADD) {
+                       if (mtype == LDB_FLAG_MOD_ADD) {
                                ++add_attr_cnt;
                        }
-                       if (LDB_FLAG_MOD_TYPE(passwordAttr->flags) == 
LDB_FLAG_MOD_REPLACE) {
+                       if (mtype == LDB_FLAG_MOD_REPLACE) {
                                ++rep_attr_cnt;
                        }
-                       if ((passwordAttr->num_values != 1) &&
-                           (LDB_FLAG_MOD_TYPE(passwordAttr->flags) == 
LDB_FLAG_MOD_ADD)) {
+                       if ((nvalues != 1) && (mtype == LDB_FLAG_MOD_ADD)) {
                                talloc_free(ac);
                                ldb_asprintf_errstring(ldb,
                                                       "'%s' attribute must 
have exactly one value on add operations!",
                                                       *l);
                                return LDB_ERR_CONSTRAINT_VIOLATION;
                        }
-                       if ((passwordAttr->num_values > 1) &&
-                           (LDB_FLAG_MOD_TYPE(passwordAttr->flags) == 
LDB_FLAG_MOD_DELETE)) {
+                       if ((nvalues > 1) && (mtype == LDB_FLAG_MOD_DELETE)) {
                                talloc_free(ac);
                                ldb_asprintf_errstring(ldb,
                                                       "'%s' attribute must 
have zero or one value(s) on delete operations!",
diff --git a/source4/dsdb/samdb/samdb.h b/source4/dsdb/samdb/samdb.h
index 6a4820c..eb52740 100644
--- a/source4/dsdb/samdb/samdb.h
+++ b/source4/dsdb/samdb/samdb.h
@@ -195,6 +195,15 @@ struct dsdb_control_password_user_account_control {
 
 #define DSDB_CONTROL_INVALID_NOT_IMPLEMENTED "1.3.6.1.4.1.7165.4.3.32"
 
+/*
+ * Used to pass "user password change" vs "password reset" from the ACL to the
+ * password_hash module, ensuring both modules treat the request identical.
+ */
+#define DSDB_CONTROL_PASSWORD_ACL_VALIDATION_OID "1.3.6.1.4.1.7165.4.3.33"
+struct dsdb_control_password_acl_validation {
+       bool pwd_reset;
+};
+
 #define DSDB_EXTENDED_REPLICATED_OBJECTS_OID "1.3.6.1.4.1.7165.4.4.1"
 struct dsdb_extended_replicated_object {
        struct ldb_message *msg;
diff --git a/source4/dsdb/tests/python/passwords.py 
b/source4/dsdb/tests/python/passwords.py
index db013ea..be1f34d 100755
--- a/source4/dsdb/tests/python/passwords.py
+++ b/source4/dsdb/tests/python/passwords.py
@@ -1020,6 +1020,55 @@ userPassword: thatsAcomplPASS4
         # Reset the "minPwdLength" as it was before
         self.ldb.set_minPwdLength(minPwdLength)
 
+    def test_pw_change_delete_no_value_userPassword(self):
+        """Test password change with userPassword where the delete attribute 
doesn't have a value"""
+
+        try:
+            self.ldb2.modify_ldif("""
+dn: cn=testuser,cn=users,""" + self.base_dn + """
+changetype: modify
+delete: userPassword
+add: userPassword
+userPassword: thatsAcomplPASS1
+""")
+        except LdbError, (num, msg):
+            self.assertEquals(num, ERR_CONSTRAINT_VIOLATION)
+        else:
+            self.fail()
+
+    def test_pw_change_delete_no_value_clearTextPassword(self):
+        """Test password change with clearTextPassword where the delete 
attribute doesn't have a value"""
+
+        try:
+            self.ldb2.modify_ldif("""
+dn: cn=testuser,cn=users,""" + self.base_dn + """
+changetype: modify
+delete: clearTextPassword
+add: clearTextPassword
+clearTextPassword: thatsAcomplPASS2
+""")
+        except LdbError, (num, msg):
+            self.assertTrue(num == ERR_CONSTRAINT_VIOLATION or
+                            num == ERR_NO_SUCH_ATTRIBUTE) # for Windows
+        else:
+            self.fail()
+
+    def test_pw_change_delete_no_value_unicodePwd(self):
+        """Test password change with unicodePwd where the delete attribute 
doesn't have a value"""
+
+        try:
+            self.ldb2.modify_ldif("""
+dn: cn=testuser,cn=users,""" + self.base_dn + """
+changetype: modify
+delete: unicodePwd
+add: unicodePwd
+unicodePwd:: """ + 
base64.b64encode("\"thatsAcomplPASS3\"".encode('utf-16-le')) + """
+""")
+        except LdbError, (num, msg):
+            self.assertEquals(num, ERR_CONSTRAINT_VIOLATION)
+        else:
+            self.fail()
+
     def tearDown(self):
         super(PasswordTests, self).tearDown()
         delete_force(self.ldb, "cn=testuser,cn=users," + self.base_dn)
diff --git a/source4/libcli/ldap/ldap_controls.c 
b/source4/libcli/ldap/ldap_controls.c
index 9df95c3..7ecc908 100644
--- a/source4/libcli/ldap/ldap_controls.c
+++ b/source4/libcli/ldap/ldap_controls.c
@@ -1262,6 +1262,7 @@ static const struct ldap_control_handler 
ldap_known_controls[] = {
        { DSDB_CONTROL_PASSWORD_CHANGE_STATUS_OID, NULL, NULL },
        { DSDB_CONTROL_PASSWORD_HASH_VALUES_OID, NULL, NULL },
        { DSDB_CONTROL_PASSWORD_CHANGE_OID, NULL, NULL },
+       { DSDB_CONTROL_PASSWORD_ACL_VALIDATION_OID, NULL, NULL },
        { DSDB_CONTROL_APPLY_LINKS, NULL, NULL },
        { LDB_CONTROL_BYPASS_OPERATIONAL_OID, NULL, NULL },
        { DSDB_CONTROL_CHANGEREPLMETADATA_OID, NULL, NULL },
diff --git a/source4/setup/schema_samba4.ldif b/source4/setup/schema_samba4.ldif
index ebc1ae6..6aafc9e 100644
--- a/source4/setup/schema_samba4.ldif
+++ b/source4/setup/schema_samba4.ldif
@@ -226,6 +226,7 @@
 #Allocated: LDB_CONTROL_RECALCULATE_RDN_OID 1.3.6.1.4.1.7165.4.3.30
 #Allocated: DSDB_CONTROL_FORCE_RODC_LOCAL_CHANGE 1.3.6.1.4.1.7165.4.3.31
 #Allocated: DSDB_CONTROL_INVALID_NOT_IMPLEMENTED 1.3.6.1.4.1.7165.4.3.32
+#Allocated: DSDB_CONTROL_PASSWORD_ACL_VALIDATION_OID 1.3.6.1.4.1.7165.4.3.33
 
 
 # Extended 1.3.6.1.4.1.7165.4.4.x


-- 
Samba Shared Repository

Reply via email to