[SCM] Samba Shared Repository - branch v4-5-stable updated

Karolin Seeger kseeger at samba.org
Tue Mar 13 09:21:53 UTC 2018


The branch, v4-5-stable has been updated
       via  4b43ad8 VERSION: Disable GIT_SNAPSHOT for the 4.6.16 release.
       via  3e0aa75 WHATSNEW: Add release notes for Samba 4.6.16.
       via  3663981 CVE-2018-1057: s4:dsdb/acl: changing dBCSPwd is only allowed with a control
       via  e5b8c81 CVE-2018-1057: s4:dsdb: use DSDB_CONTROL_PASSWORD_ACL_VALIDATION_OID
       via  4adcba5 CVE-2018-1057: s4:dsdb/samdb: define DSDB_CONTROL_PASSWORD_ACL_VALIDATION_OID control
       via  bb43ab0 CVE-2018-1057: s4:dsdb/acl: run password checking only once
       via  67fa44a CVE-2018-1057: s4/dsdb: correctly detect password resets
       via  6c980a0 CVE-2018-1057: s4:dsdb/acl: add a NULL check for talloc_new() in acl_check_password_rights()
       via  54c363e CVE-2018-1057: s4:dsdb/acl: add check for DSDB_CONTROL_PASSWORD_HASH_VALUES_OID control
       via  6d5caff CVE-2018-1057: s4:dsdb/acl: check for internal controls before other checks
       via  99f46aa CVE-2018-1057: s4:dsdb/acl: remove unused else branches in acl_check_password_rights()
       via  d552abe CVE-2018-1057: s4:dsdb/acl: only call dsdb_acl_debug() if we checked the acl in acl_check_password_rights()
       via  abf925c CVE-2018-1057: s4:dsdb/password_hash: add a helper variable for passwordAttr->num_values
       via  7eabe3d CVE-2018-1057: s4:dsdb/password_hash: add a helper variable for LDB_FLAG_MOD_TYPE
       via  e577464 CVE-2018-1057: s4:dsdb/tests: add a test for password change with empty delete
       via  dff5d43 CVE-2018-1050: s3: RPC: spoolss server. Protect against null pointer derefs.
       via  64b6a9f VERSION: Re-enable GIT_SNAPSHOT.
       via  f3ec20f VERSION: Bump version up to 4.5.16.
      from  f333815 VERSION: Disable GIT_SNAPSHOT for the 4.5.15 release.

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


- Log -----------------------------------------------------------------
commit 4b43ad87039c0e94522b2baa7381255e28935f4e
Author: Karolin Seeger <kseeger at samba.org>
Date:   Mon Mar 12 13:10:30 2018 +0100

    VERSION: Disable GIT_SNAPSHOT for the 4.6.16 release.
    
    CVE-2018-1050 (Denial of Service Attack on external print server.)
    CVE-2018-1057 (Authenticated users can change other users' password.)
    
    Signed-off-by: Karolin Seeger <kseeger at samba.org>

commit 3e0aa7587ccaf061180ea08e089eeb2aa18cf5c0
Author: Karolin Seeger <kseeger at samba.org>
Date:   Mon Mar 12 13:09:35 2018 +0100

    WHATSNEW: Add release notes for Samba 4.6.16.
    
    Signed-off-by: Karolin Seeger <kseeger at samba.org>

commit 36639815f34c374f6d6cf002e19ba1754035e7d1
Author: Ralph Boehme <slow at 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 <slow at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>

commit e5b8c81d2951401691ea6a5e8082edf81354d1a1
Author: Ralph Boehme <slow at 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 <slow at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>

commit 4adcba5f6aecacde5b405bdd1bdc662d303137e3
Author: Ralph Boehme <slow at 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 <slow at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>

commit bb43ab081a539b088afb8c82658d2d8f3715bae8
Author: Ralph Boehme <slow at 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 <slow at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>

commit 67fa44aaf2d81001260af85ed803f0ff6444397d
Author: Ralph Boehme <slow at 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 <slow at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>

commit 6c980a03e54293d3044d32b0715c384903ed2baf
Author: Ralph Boehme <slow at 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 <slow at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>

commit 54c363e01210b478e1da74e446763153a61e56d0
Author: Ralph Boehme <slow at 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 <slow at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>

commit 6d5caffbf5fe546b2f648f957ca8a41b12214f3f
Author: Ralph Boehme <slow at 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 <slow at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>

commit 99f46aabe571a96c3d5334929b069d8b81bd5d5d
Author: Ralph Boehme <slow at 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 <slow at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>

commit d552abe8a2b34e528a3b810f3d1f600495e73466
Author: Ralph Boehme <slow at 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 <slow at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>

commit abf925c2d10d29b9a68017f89b4970c8a2d43310
Author: Ralph Boehme <slow at 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 <slow at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>

commit 7eabe3d477fd9c439ad160f42f537f404019b420
Author: Ralph Boehme <slow at 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 <slow at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>

commit e5774640b0b93f6b03fee0734241192b8a02b73a
Author: Ralph Boehme <slow at 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 <slow at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>

commit dff5d43907fd9825f0ba03dab9aa63c31f5a014c
Author: Jeremy Allison <jra at 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 <jra at samba.org>

commit 64b6a9f6e1c472bb3f0ae052658b4532b81a9242
Author: Karolin Seeger <kseeger at samba.org>
Date:   Mon Mar 12 13:02:04 2018 +0100

    VERSION: Re-enable GIT_SNAPSHOT.
    
    Signed-off-by: Karolin Seeger <kseeger at samba.org>

commit f3ec20fdbea88f9de7731d9566ecce7f2916883a
Author: Karolin Seeger <kseeger at samba.org>
Date:   Wed Nov 22 09:04:28 2017 +0100

    VERSION: Bump version up to 4.5.16.
    
    Signed-off-by: Karolin Seeger <kseeger at samba.org>
    (cherry picked from commit 8376a89e40b82c0b4b365b8daf155159f59945cb)

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

Summary of changes:
 VERSION                                        |   2 +-
 WHATSNEW.txt                                   |  80 +++++++++++++-
 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               |   2 +
 9 files changed, 321 insertions(+), 26 deletions(-)


Changeset truncated at 500 lines:

diff --git a/VERSION b/VERSION
index d874810..ffb776e 100644
--- a/VERSION
+++ b/VERSION
@@ -25,7 +25,7 @@
 ########################################################
 SAMBA_VERSION_MAJOR=4
 SAMBA_VERSION_MINOR=5
-SAMBA_VERSION_RELEASE=15
+SAMBA_VERSION_RELEASE=16
 
 ########################################################
 # If a official release has a serious bug              #
diff --git a/WHATSNEW.txt b/WHATSNEW.txt
index b245e30..a204a54 100644
--- a/WHATSNEW.txt
+++ b/WHATSNEW.txt
@@ -1,4 +1,80 @@
                    ==============================
+                   Release Notes for Samba 4.5.16
+                           March 13, 2018
+                   ==============================
+
+
+This is a security release in order to address the following defects:
+
+o  CVE-2018-1050 (Denial of Service Attack on external print server.)
+o  CVE-2018-1057 (Authenticated users can change other users' password.)
+
+
+=======
+Details
+=======
+
+o  CVE-2018-1050:
+   All versions of Samba from 4.0.0 onwards are vulnerable to a denial of
+   service attack when the RPC spoolss service is configured to be run as
+   an external daemon. Missing input sanitization checks on some of the
+   input parameters to spoolss RPC calls could cause the print spooler
+   service to crash.
+
+   There is no known vulnerability associated with this error, merely a
+   denial of service. If the RPC spoolss service is left by default as an
+   internal service, all a client can do is crash its own authenticated
+   connection.
+
+o  CVE-2018-1057:
+   On a Samba 4 AD DC the LDAP server in all versions of Samba from
+   4.0.0 onwards incorrectly validates permissions to modify passwords
+   over LDAP allowing authenticated users to change any other users'
+   passwords, including administrative users.
+
+   Possible workarounds are described at a dedicated page in the Samba wiki:
+   https://wiki.samba.org/index.php/CVE-2018-1057
+
+
+Changes since 4.5.15:
+---------------------
+
+o  Jeremy Allison <jra at samba.org>
+   * BUG 11343: CVE-2018-1050: Codenomicon crashes in spoolss server code.
+
+o  Ralph Boehme <slow at samba.org>
+   * BUG 13272: CVE-2018-1057: Unprivileged user can change any user (and admin)
+     password.
+
+o  Stefan Metzmacher <metze at samba.org>
+   * BUG 13272: CVE-2018-1057: Unprivileged user can change any user (and admin)
+     password.
+
+
+#######################################
+Reporting bugs & Development Discussion
+#######################################
+
+Please discuss this release on the samba-technical mailing list or by
+joining the #samba-technical IRC channel on irc.freenode.net.
+
+If you do report problems then please try to send high quality
+feedback. If you don't provide vital information to help us track down
+the problem then you will probably be ignored.  All bug reports should
+be filed under the "Samba 4.1 and newer" product in the project's Bugzilla
+database (https://bugzilla.samba.org/).
+
+
+======================================================================
+== Our Code, Our Bugs, Our Responsibility.
+== The Samba Team
+======================================================================
+
+
+Release notes for older releases follow:
+----------------------------------------
+
+                   ==============================
                    Release Notes for Samba 4.5.15
                           November 21, 2017
                    ==============================
@@ -66,8 +142,8 @@ database (https://bugzilla.samba.org/).
 ======================================================================
 
 
-Release notes for older releases follow:
-----------------------------------------
+----------------------------------------------------------------------
+
 
                    ==============================
                    Release Notes for Samba 4.5.14
diff --git a/source3/rpc_server/spoolss/srv_spoolss_nt.c b/source3/rpc_server/spoolss/srv_spoolss_nt.c
index a3c3861..fb56e2b 100644
--- a/source3/rpc_server/spoolss/srv_spoolss_nt.c
+++ b/source3/rpc_server/spoolss/srv_spoolss_nt.c
@@ -178,6 +178,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++;
 	}
@@ -2073,6 +2078,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) {
@@ -2212,6 +2221,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 e5bee6f..06abf66 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" */
@@ -3921,25 +3949,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 586a3bf..6390258 100644
--- a/source4/dsdb/samdb/samdb.h
+++ b/source4/dsdb/samdb/samdb.h
@@ -182,6 +182,15 @@ struct dsdb_control_password_user_account_control {
 /* passed when we want to thoroughly delete linked attributes */
 #define DSDB_CONTROL_REPLMD_VANISH_LINKS "1.3.6.1.4.1.7165.4.3.29"
 
+/*
+ * 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


-- 
Samba Shared Repository



More information about the samba-cvs mailing list