The branch, v4-7-test has been updated via cc04ea1 VERSION: Bump version up to 4.7.7. via 2f57b6d VERSION: Disable GIT_SNAPSHOT for the 4.7.6 release. via f17ddb9 WHATSNEW: Add release notes for Samba 4.7.6. via 49b49f1 CVE-2018-1057: s4:dsdb/acl: changing dBCSPwd is only allowed with a control via 7d8de68 CVE-2018-1057: s4:dsdb: use DSDB_CONTROL_PASSWORD_ACL_VALIDATION_OID via f797e86 CVE-2018-1057: s4:dsdb/samdb: define DSDB_CONTROL_PASSWORD_ACL_VALIDATION_OID control via c5a663f CVE-2018-1057: s4:dsdb/acl: run password checking only once via 7cc3749 CVE-2018-1057: s4/dsdb: correctly detect password resets via a192242 CVE-2018-1057: s4:dsdb/acl: add a NULL check for talloc_new() in acl_check_password_rights() via fd1817c CVE-2018-1057: s4:dsdb/acl: add check for DSDB_CONTROL_PASSWORD_HASH_VALUES_OID control via 0820307 CVE-2018-1057: s4:dsdb/acl: check for internal controls before other checks via 0bb68f5 CVE-2018-1057: s4:dsdb/acl: remove unused else branches in acl_check_password_rights() via b3746a4 CVE-2018-1057: s4:dsdb/acl: only call dsdb_acl_debug() if we checked the acl in acl_check_password_rights() via 7ee55ea CVE-2018-1057: s4:dsdb/password_hash: add a helper variable for passwordAttr->num_values via 43a5d96 CVE-2018-1057: s4:dsdb/password_hash: add a helper variable for LDB_FLAG_MOD_TYPE via d15b66c CVE-2018-1057: s4:dsdb/tests: add a test for password change with empty delete via b59ca4d CVE-2018-1050: s3: RPC: spoolss server. Protect against null pointer derefs. from af47cdb s3:smbd: Do not crash if we fail to init the session table
https://git.samba.org/?p=samba.git;a=shortlog;h=v4-7-test - Log ----------------------------------------------------------------- commit cc04ea177183c054236edd6ab721dc9f36c5dab3 Author: Karolin Seeger <ksee...@samba.org> Date: Tue Mar 13 10:24:24 2018 +0100 VERSION: Bump version up to 4.7.7. Signed-off-by: Karolin Seeger <ksee...@samba.org> commit 2f57b6d9aa381dba7646f919dd7c7dc18fab4979 Author: Karolin Seeger <ksee...@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 <ksee...@samba.org> commit f17ddb96286dbbe45e3aec5f9af1aac4383c0561 Author: Karolin Seeger <ksee...@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 <ksee...@samba.org> commit 49b49f16030858d498e1937d1c81124b65567828 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 7d8de684f0ff3882fdb5db549a85d515bef4391c 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 f797e86896397b9dccccfd3a235f3537ced6cb37 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 c5a663f754148af6fb7e47f5990b93b55abd7342 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 7cc374973406c49130fff019de26bdf0db48f494 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 a192242f81bf21096ee497805cd63ad43ee20515 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 fd1817c8447ec804e1a7f1c85390730a576c9176 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 0820307b1aedbe4078e500b9f9692e404b22b6d3 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 0bb68f5e2629487b358d8b5f53eea6da5fafb351 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 b3746a4ad32f1fae3b0004466bb8b89772207c3d 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 7ee55eab03a7398cba04acb96b3b7f4675d8c017 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 43a5d968f080830677d12fa4def34e03b40d822e 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 d15b66c68af234669930ba2dbf05bd64d37ec854 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> commit b59ca4d1086ab21eec9d111d42cf06afe794b950 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> ----------------------------------------------------------------------- Summary of changes: VERSION | 4 +- 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, 321 insertions(+), 27 deletions(-) Changeset truncated at 500 lines: diff --git a/VERSION b/VERSION index bbc59a4..f9f4813 100644 --- a/VERSION +++ b/VERSION @@ -25,7 +25,7 @@ ######################################################## SAMBA_VERSION_MAJOR=4 SAMBA_VERSION_MINOR=7 -SAMBA_VERSION_RELEASE=6 +SAMBA_VERSION_RELEASE=7 ######################################################## # If a official release has a serious bug # @@ -99,7 +99,7 @@ SAMBA_VERSION_RC_RELEASE= # e.g. SAMBA_VERSION_IS_SVN_SNAPSHOT=yes # # -> "3.0.0-SVN-build-199" # ######################################################## -SAMBA_VERSION_IS_GIT_SNAPSHOT=yes +SAMBA_VERSION_IS_GIT_SNAPSHOT=no ######################################################## # This is for specifying a release nickname # 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 <j...@samba.org> + * BUG 11343: CVE-2018-1050: Codenomicon crashes in spoolss server code. + +o Ralph Boehme <s...@samba.org> + * BUG 13272: CVE-2018-1057: Unprivileged user can change any user (and admin) + password. + +o Stefan Metzmacher <me...@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() -- Samba Shared Repository