diff options
author | Ralph Boehme <slow@samba.org> | 2018-02-22 10:54:37 +0100 |
---|---|---|
committer | Karolin Seeger <kseeger@samba.org> | 2018-03-12 13:06:14 +0100 |
commit | 67fa44aaf2d81001260af85ed803f0ff6444397d (patch) | |
tree | c9ee80505e1e58cde4c565dcb13120d2c5a4fbd9 | |
parent | 6c980a03e54293d3044d32b0715c384903ed2baf (diff) | |
download | samba-67fa44aaf2d81001260af85ed803f0ff6444397d.tar.gz |
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@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
-rw-r--r-- | selftest/knownfail.d/samba4.ldap.passwords.python | 2 | ||||
-rw-r--r-- | source4/dsdb/samdb/ldb_modules/acl.c | 18 |
2 files changed, 17 insertions, 3 deletions
diff --git a/selftest/knownfail.d/samba4.ldap.passwords.python b/selftest/knownfail.d/samba4.ldap.passwords.python deleted file mode 100644 index 343c5a7867d..00000000000 --- a/selftest/knownfail.d/samba4.ldap.passwords.python +++ /dev/null @@ -1,2 +0,0 @@ -samba4.ldap.passwords.python.*.__main__.PasswordTests.test_pw_change_delete_no_value_userPassword -samba4.ldap.passwords.python.*.__main__.PasswordTests.test_pw_change_delete_no_value_unicodePwd diff --git a/source4/dsdb/samdb/ldb_modules/acl.c b/source4/dsdb/samdb/ldb_modules/acl.c index 2c0aee41edf..d27ec80461e 100644 --- a/source4/dsdb/samdb/ldb_modules/acl.c +++ b/source4/dsdb/samdb/ldb_modules/acl.c @@ -966,6 +966,7 @@ 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; @@ -1031,12 +1032,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); } @@ -1066,7 +1070,7 @@ static int acl_check_password_rights(TALLOC_CTX *mem_ctx, goto checked; } - if (add_attr_cnt == 1 && del_attr_cnt == 1) { + if (add_val_cnt == 1 && del_val_cnt == 1) { ret = acl_check_extended_right(tmp_ctx, sd, acl_user_token(module), GUID_DRS_USER_CHANGE_PASSWORD, SEC_ADS_CONTROL_ACCESS, @@ -1078,6 +1082,18 @@ static int acl_check_password_rights(TALLOC_CTX *mem_ctx, goto checked; } + if (add_val_cnt == 1 && del_val_cnt == 0) { + 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; + } + talloc_free(tmp_ctx); return LDB_SUCCESS; |