diff options
| author | Ralph Boehme <slow@samba.org> | 2018-02-22 10:54:37 +0100 |
|---|---|---|
| committer | Karolin Seeger <kseeger@samba.org> | 2018-03-12 10:05:43 +0100 |
| commit | c8aa8ffa40cf2cfb3ed2f295e55778b96418eebd (patch) | |
| tree | 37cfaba9d864b1ac5f76035debc5043cd4640d13 | |
| parent | 7f4fef05c45dfa797a2ab699af32d166df7451ce (diff) | |
| download | samba-c8aa8ffa40cf2cfb3ed2f295e55778b96418eebd.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; |