[SCM] Samba Shared Repository - branch v4-7-stable updated
Karolin Seeger
kseeger at samba.org
Tue Mar 13 09:21:10 UTC 2018
The branch, v4-7-stable has been updated
via 5cfa947 VERSION: Disable GIT_SNAPSHOT for the 4.7.6 release.
via 4119137 WHATSNEW: Add release notes for Samba 4.7.6.
via 11fbafc CVE-2018-1057: s4:dsdb/acl: changing dBCSPwd is only allowed with a control
via 86b41e9 CVE-2018-1057: s4:dsdb: use DSDB_CONTROL_PASSWORD_ACL_VALIDATION_OID
via f11f3cc CVE-2018-1057: s4:dsdb/samdb: define DSDB_CONTROL_PASSWORD_ACL_VALIDATION_OID control
via 32d65d8 CVE-2018-1057: s4:dsdb/acl: run password checking only once
via 946bab0 CVE-2018-1057: s4/dsdb: correctly detect password resets
via bb2ab8e CVE-2018-1057: s4:dsdb/acl: add a NULL check for talloc_new() in acl_check_password_rights()
via a6221ea CVE-2018-1057: s4:dsdb/acl: add check for DSDB_CONTROL_PASSWORD_HASH_VALUES_OID control
via 32384ea CVE-2018-1057: s4:dsdb/acl: check for internal controls before other checks
via 31088fa CVE-2018-1057: s4:dsdb/acl: remove unused else branches in acl_check_password_rights()
via 50eb427 CVE-2018-1057: s4:dsdb/acl: only call dsdb_acl_debug() if we checked the acl in acl_check_password_rights()
via e2acd0d CVE-2018-1057: s4:dsdb/password_hash: add a helper variable for passwordAttr->num_values
via 5ad58a9 CVE-2018-1057: s4:dsdb/password_hash: add a helper variable for LDB_FLAG_MOD_TYPE
via d8de52b CVE-2018-1057: s4:dsdb/tests: add a test for password change with empty delete
via 9f9db58 CVE-2018-1050: s3: RPC: spoolss server. Protect against null pointer derefs.
via a572eed VERSION: Bump version up to 4.7.6...
from c15b477 VERSION: Disable GIT_SNAPSHOT for the 4.7.5 release.
https://git.samba.org/?p=samba.git;a=shortlog;h=v4-7-stable
- Log -----------------------------------------------------------------
commit 5cfa947e5098bc361ff13fdad1b4fe3211a39154
Author: Karolin Seeger <kseeger at samba.org>
Date: Sun Mar 11 22:03:58 2018 +0100
VERSION: Disable GIT_SNAPSHOT for the 4.7.6 release.
CVE-2018-1050 (Denial of Service Attack on external print server.)
CVE-2018-1057 (Authenticated users can change other user's password.)
Signed-off-by: Karolin Seeger <kseeger at samba.org>
commit 4119137555557da17e5ec6e49236e3c3aab2e377
Author: Karolin Seeger <kseeger at samba.org>
Date: Sun Mar 11 22:02:30 2018 +0100
WHATSNEW: Add release notes for Samba 4.7.6.
CVE-2018-1050 (Denial of Service Attack on external print server.)
CVE-2018-1057 (Authenticated users can change other user's password.)
Signed-off-by: Karolin Seeger <kseeger at samba.org>
commit 11fbafc9169c0f8442e84c3f4a48ad3ced84a550
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 86b41e9c618a8b0455a5ea571a9078fcaddc4eec
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 f11f3ccb776f9055d8911326903c93a04dc1ca44
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 32d65d8ba602cc426da668a99d027db954f26cfe
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 946bab0ad8311912433b873501b877c334586f09
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 bb2ab8e3b86fed1c284919fec1b5455de8d36b91
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 a6221ea75e4d333e704d3df2f5df37f394faf19f
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 32384ea708c3e529c2f3d83169aba4945aa577fd
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 31088faf09fd5dc71d0dcc050628f68c43d0981e
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 50eb427844493630f835de0cddb4f9d3c83d20aa
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 e2acd0d259d7200486d06c16abdc3232c8f1c3f2
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 5ad58a9f0d8826e14b7acd82c133c3579d53639b
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 d8de52b979bbe071e6cc57405d5bc428f2b4ea6f
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 9f9db5815d380bcb3ed9509cfb3b448600e93b48
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 a572eed8bfb3effb5a2308336ae64db90a1d8d5e
Author: Karolin Seeger <kseeger at samba.org>
Date: Wed Feb 7 09:27:59 2018 +0100
VERSION: Bump version up to 4.7.6...
and re-enable GIT_SNAPSHOT.
Signed-off-by: Karolin Seeger <kseeger at samba.org>
(cherry picked from commit 5c782d5f7670d7855990cd359b919706d584ac4b)
-----------------------------------------------------------------------
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 | 1 +
9 files changed, 320 insertions(+), 26 deletions(-)
Changeset truncated at 500 lines:
diff --git a/VERSION b/VERSION
index 05e61bf..57dfd64 100644
--- a/VERSION
+++ b/VERSION
@@ -25,7 +25,7 @@
########################################################
SAMBA_VERSION_MAJOR=4
SAMBA_VERSION_MINOR=7
-SAMBA_VERSION_RELEASE=5
+SAMBA_VERSION_RELEASE=6
########################################################
# If a official release has a serious bug #
diff --git a/WHATSNEW.txt b/WHATSNEW.txt
index 2914f57..021f2e7 100644
--- a/WHATSNEW.txt
+++ b/WHATSNEW.txt
@@ -1,4 +1,80 @@
=============================
+ Release Notes for Samba 4.7.6
+ 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.7.5:
+--------------------
+
+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.7.5
February 7, 2018
=============================
@@ -82,8 +158,8 @@ database (https://bugzilla.samba.org/).
======================================================================
-Release notes for older releases follow:
-----------------------------------------
+----------------------------------------------------------------------
+
=============================
Release Notes for Samba 4.7.4
diff --git a/source3/rpc_server/spoolss/srv_spoolss_nt.c b/source3/rpc_server/spoolss/srv_spoolss_nt.c
index fcc491e..3d99470b 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 96113b5..3a25821 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" */
@@ -4234,25 +4262,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 894df0f..8790ff5 100644
--- a/source4/dsdb/samdb/samdb.h
+++ b/source4/dsdb/samdb/samdb.h
@@ -194,6 +194,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
--
Samba Shared Repository
More information about the samba-cvs
mailing list