From d5d0f984039b0fa3c5296355139f0f2eedc62d14 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 23 Nov 2012 11:49:05 +0100 Subject: [PATCH 1/7] s4:dsdb/password_hash: Honor password complexity settings. Honor password complexity settings when creating new users. Without this patch, you could set simple passwords although the complexity settings were enabled. This was an issue with 'samba-tool user add' and also when adding new users via Windows' "Active Directory Users and Computers" MMC Snap-In. The following scenarios were tested successfully after applying the patch: -'samba-tool user add' against s4 -'samba-tool user add -H' against a Windows DC -Adding a new user on a s4 DC using Windows' "Active Directory Users and Computers" MMC Snap-In. Please note that this bug was caused by a mistake in the documentation. Fix bug #9414 - 'samba-tool user add' ignores password complexity settings. Pair-programmed-with: Karolin Seeger Pair-Programmed-With: Michael Adam Signed-off-by: Stefan Metzmacher Signed-off-by: Michael Adam --- source4/dsdb/samdb/ldb_modules/password_hash.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/password_hash.c b/source4/dsdb/samdb/ldb_modules/password_hash.c index 620de75..4644628 100644 --- a/source4/dsdb/samdb/ldb_modules/password_hash.c +++ b/source4/dsdb/samdb/ldb_modules/password_hash.c @@ -2188,11 +2188,6 @@ static int setup_io(struct ph_context *ac, & (UF_INTERDOMAIN_TRUST_ACCOUNT | UF_WORKSTATION_TRUST_ACCOUNT | UF_SERVER_TRUST_ACCOUNT)); - if ((io->u.userAccountControl & UF_PASSWD_NOTREQD) != 0) { - /* see [MS-ADTS] 2.2.15 */ - io->u.restrictions = 0; - } - if (ac->userPassword) { ret = msg_find_old_and_new_pwd_val(orig_msg, "userPassword", ac->req->operation, -- 1.7.9.5 From 507abbb2776523921278007981f94d9aa13262fc Mon Sep 17 00:00:00 2001 From: Michael Adam Date: Tue, 11 Dec 2012 11:42:11 +0100 Subject: [PATCH 2/7] s4:torture:rpc:samr: add debugging of result of (many) dcerpc_samr_* calls Pair-Programmed-With: Stefan Metzmacher Signed-off-by: Michael Adam Signed-off-by: Stefan Metzmacher --- source4/torture/rpc/samr.c | 107 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 106 insertions(+), 1 deletion(-) diff --git a/source4/torture/rpc/samr.c b/source4/torture/rpc/samr.c index 7f50ce9..ba0f20b 100644 --- a/source4/torture/rpc/samr.c +++ b/source4/torture/rpc/samr.c @@ -665,6 +665,9 @@ static bool test_SetUserPass(struct dcerpc_pipe *p, struct torture_context *tctx torture_assert_ntstatus_ok(tctx, dcerpc_samr_SetUserInfo_r(b, tctx, &s), "SetUserInfo failed"); + torture_comment(tctx, "(%s:%s) new_password[%s] status[%s]\n", + __location__, __FUNCTION__, + newpass, nt_errstr(s.out.result)); if (!NT_STATUS_IS_OK(s.out.result)) { torture_warning(tctx, "SetUserInfo level %u failed - %s\n", s.in.level, nt_errstr(s.out.result)); @@ -724,6 +727,9 @@ static bool test_SetUserPass_23(struct dcerpc_pipe *p, struct torture_context *t torture_assert_ntstatus_ok(tctx, dcerpc_samr_SetUserInfo_r(b, tctx, &s), "SetUserInfo failed"); + torture_comment(tctx, "(%s:%s) new_password[%s] status[%s]\n", + __location__, __FUNCTION__, + newpass, nt_errstr(s.out.result)); if (!NT_STATUS_IS_OK(s.out.result)) { torture_warning(tctx, "SetUserInfo level %u failed - %s\n", s.in.level, nt_errstr(s.out.result)); @@ -749,6 +755,9 @@ static bool test_SetUserPass_23(struct dcerpc_pipe *p, struct torture_context *t torture_assert_ntstatus_ok(tctx, dcerpc_samr_SetUserInfo_r(b, tctx, &s), "SetUserInfo failed"); + torture_comment(tctx, "(%s:%s) new_password[%s] status[%s]\n", + __location__, __FUNCTION__, + newpass, nt_errstr(s.out.result)); if (!NT_STATUS_EQUAL(s.out.result, NT_STATUS_WRONG_PASSWORD)) { torture_warning(tctx, "SetUserInfo level %u should have failed with WRONG_PASSWORD- %s\n", s.in.level, nt_errstr(s.out.result)); @@ -818,6 +827,9 @@ static bool test_SetUserPassEx(struct dcerpc_pipe *p, struct torture_context *tc torture_assert_ntstatus_ok(tctx, dcerpc_samr_SetUserInfo_r(b, tctx, &s), "SetUserInfo failed"); + torture_comment(tctx, "(%s:%s) new_password[%s] status[%s]\n", + __location__, __FUNCTION__, + newpass, nt_errstr(s.out.result)); if (!NT_STATUS_IS_OK(s.out.result)) { torture_warning(tctx, "SetUserInfo level %u failed - %s\n", s.in.level, nt_errstr(s.out.result)); @@ -836,6 +848,9 @@ static bool test_SetUserPassEx(struct dcerpc_pipe *p, struct torture_context *tc torture_assert_ntstatus_ok(tctx, dcerpc_samr_SetUserInfo_r(b, tctx, &s), "SetUserInfo failed"); + torture_comment(tctx, "(%s:%s) new_password[%s] status[%s]\n", + __location__, __FUNCTION__, + newpass, nt_errstr(s.out.result)); if (!NT_STATUS_EQUAL(s.out.result, NT_STATUS_WRONG_PASSWORD)) { torture_warning(tctx, "SetUserInfo level %u should have failed with WRONG_PASSWORD: %s\n", s.in.level, nt_errstr(s.out.result)); @@ -905,6 +920,9 @@ static bool test_SetUserPass_25(struct dcerpc_pipe *p, struct torture_context *t torture_assert_ntstatus_ok(tctx, dcerpc_samr_SetUserInfo_r(b, tctx, &s), "SetUserInfo failed"); + torture_comment(tctx, "(%s:%s) new_password[%s] status[%s]\n", + __location__, __FUNCTION__, + newpass, nt_errstr(s.out.result)); if (!NT_STATUS_IS_OK(s.out.result)) { torture_warning(tctx, "SetUserInfo level %u failed - %s\n", s.in.level, nt_errstr(s.out.result)); @@ -923,6 +941,9 @@ static bool test_SetUserPass_25(struct dcerpc_pipe *p, struct torture_context *t torture_assert_ntstatus_ok(tctx, dcerpc_samr_SetUserInfo_r(b, tctx, &s), "SetUserInfo failed"); + torture_comment(tctx, "(%s:%s) new_password[%s] status[%s]\n", + __location__, __FUNCTION__, + newpass, nt_errstr(s.out.result)); if (!NT_STATUS_EQUAL(s.out.result, NT_STATUS_WRONG_PASSWORD)) { torture_warning(tctx, "SetUserInfo level %u should have failed with WRONG_PASSWORD- %s\n", s.in.level, nt_errstr(s.out.result)); @@ -1312,10 +1333,16 @@ static bool test_SetUserPass_level_ex(struct dcerpc_pipe *p, if (use_setinfo2) { torture_assert_ntstatus_ok(tctx, dcerpc_samr_SetUserInfo2_r(b, tctx, &s2), "SetUserInfo2 failed"); - status = s2.out.result; + torture_comment(tctx, "(%s:%s) new_password[%s] status[%s]\n", + __location__, __FUNCTION__, + newpass, nt_errstr(s2.out.result)); + status = s2.out.result; } else { torture_assert_ntstatus_ok(tctx, dcerpc_samr_SetUserInfo_r(b, tctx, &s), "SetUserInfo failed"); + torture_comment(tctx, "(%s:%s) new_password[%s] status[%s]\n", + __location__, __FUNCTION__, + newpass, nt_errstr(s.out.result)); status = s.out.result; } @@ -1729,6 +1756,9 @@ static bool test_ChangePasswordUser(struct dcerpc_binding_handle *b, torture_assert_ntstatus_ok(tctx, dcerpc_samr_ChangePasswordUser_r(b, tctx, &r), "ChangePasswordUser failed"); + torture_comment(tctx, "(%s:%s) old_password[%s] new_password[%s] status[%s]\n", + __location__, __FUNCTION__, + oldpass, newpass, nt_errstr(r.out.result)); torture_assert_ntstatus_equal(tctx, r.out.result, NT_STATUS_WRONG_PASSWORD, "ChangePasswordUser failed: expected NT_STATUS_WRONG_PASSWORD because we broke the LM hash"); @@ -1751,6 +1781,9 @@ static bool test_ChangePasswordUser(struct dcerpc_binding_handle *b, torture_assert_ntstatus_ok(tctx, dcerpc_samr_ChangePasswordUser_r(b, tctx, &r), "ChangePasswordUser failed"); + torture_comment(tctx, "(%s:%s) old_password[%s] new_password[%s] status[%s]\n", + __location__, __FUNCTION__, + oldpass, newpass, nt_errstr(r.out.result)); torture_assert_ntstatus_equal(tctx, r.out.result, NT_STATUS_WRONG_PASSWORD, "expected NT_STATUS_WRONG_PASSWORD because we broke the NT hash"); @@ -1773,6 +1806,9 @@ static bool test_ChangePasswordUser(struct dcerpc_binding_handle *b, torture_assert_ntstatus_ok(tctx, dcerpc_samr_ChangePasswordUser_r(b, tctx, &r), "ChangePasswordUser failed"); + torture_comment(tctx, "(%s:%s) old_password[%s] new_password[%s] status[%s]\n", + __location__, __FUNCTION__, + oldpass, newpass, nt_errstr(r.out.result)); if (!NT_STATUS_EQUAL(r.out.result, NT_STATUS_WRONG_PASSWORD)) { torture_warning(tctx, "ChangePasswordUser failed: expected NT_STATUS_WRONG_PASSWORD because we broke the LM cross-hash, got %s\n", nt_errstr(r.out.result)); ret = false; @@ -1797,6 +1833,9 @@ static bool test_ChangePasswordUser(struct dcerpc_binding_handle *b, torture_assert_ntstatus_ok(tctx, dcerpc_samr_ChangePasswordUser_r(b, tctx, &r), "ChangePasswordUser failed"); + torture_comment(tctx, "(%s:%s) old_password[%s] new_password[%s] status[%s]\n", + __location__, __FUNCTION__, + oldpass, newpass, nt_errstr(r.out.result)); if (!NT_STATUS_EQUAL(r.out.result, NT_STATUS_WRONG_PASSWORD)) { torture_warning(tctx, "ChangePasswordUser failed: expected NT_STATUS_WRONG_PASSWORD because we broke the NT cross-hash, got %s\n", nt_errstr(r.out.result)); ret = false; @@ -1828,6 +1867,9 @@ static bool test_ChangePasswordUser(struct dcerpc_binding_handle *b, torture_assert_ntstatus_ok(tctx, dcerpc_samr_ChangePasswordUser_r(b, tctx, &r), "ChangePasswordUser failed"); + torture_comment(tctx, "(%s:%s) old_password[%s] new_password[%s] status[%s]\n", + __location__, __FUNCTION__, + oldpass, newpass, nt_errstr(r.out.result)); if (NT_STATUS_IS_OK(r.out.result)) { changed = true; *password = newpass; @@ -1867,6 +1909,9 @@ static bool test_ChangePasswordUser(struct dcerpc_binding_handle *b, torture_assert_ntstatus_ok(tctx, dcerpc_samr_ChangePasswordUser_r(b, tctx, &r), "ChangePasswordUser failed"); + torture_comment(tctx, "(%s:%s) old_password[%s] new_password[%s] status[%s]\n", + __location__, __FUNCTION__, + oldpass, newpass, nt_errstr(r.out.result)); if (NT_STATUS_IS_OK(r.out.result)) { changed = true; *password = newpass; @@ -1906,6 +1951,9 @@ static bool test_ChangePasswordUser(struct dcerpc_binding_handle *b, torture_assert_ntstatus_ok(tctx, dcerpc_samr_ChangePasswordUser_r(b, tctx, &r), "ChangePasswordUser failed"); + torture_comment(tctx, "(%s:%s) old_password[%s] new_password[%s] status[%s]\n", + __location__, __FUNCTION__, + oldpass, newpass, nt_errstr(r.out.result)); if (NT_STATUS_EQUAL(r.out.result, NT_STATUS_PASSWORD_RESTRICTION)) { torture_comment(tctx, "ChangePasswordUser returned: %s perhaps min password age? (not fatal)\n", nt_errstr(r.out.result)); } else if (!NT_STATUS_IS_OK(r.out.result)) { @@ -1931,6 +1979,9 @@ static bool test_ChangePasswordUser(struct dcerpc_binding_handle *b, if (changed) { torture_assert_ntstatus_ok(tctx, dcerpc_samr_ChangePasswordUser_r(b, tctx, &r), "ChangePasswordUser failed"); + torture_comment(tctx, "(%s:%s) old_password[%s] new_password[%s] status[%s]\n", + __location__, __FUNCTION__, + oldpass, newpass, nt_errstr(r.out.result)); if (NT_STATUS_EQUAL(r.out.result, NT_STATUS_PASSWORD_RESTRICTION)) { torture_comment(tctx, "ChangePasswordUser returned: %s perhaps min password age? (not fatal)\n", nt_errstr(r.out.result)); } else if (!NT_STATUS_EQUAL(r.out.result, NT_STATUS_WRONG_PASSWORD)) { @@ -2008,6 +2059,9 @@ static bool test_OemChangePasswordUser2(struct dcerpc_pipe *p, torture_assert_ntstatus_ok(tctx, dcerpc_samr_OemChangePasswordUser2_r(b, tctx, &r), "OemChangePasswordUser2 failed"); + torture_comment(tctx, "(%s:%s) old_password[%s] new_password[%s] status[%s]\n", + __location__, __FUNCTION__, + oldpass, newpass, nt_errstr(r.out.result)); if (!NT_STATUS_EQUAL(r.out.result, NT_STATUS_PASSWORD_RESTRICTION) && !NT_STATUS_EQUAL(r.out.result, NT_STATUS_WRONG_PASSWORD)) { @@ -2031,6 +2085,9 @@ static bool test_OemChangePasswordUser2(struct dcerpc_pipe *p, torture_assert_ntstatus_ok(tctx, dcerpc_samr_OemChangePasswordUser2_r(b, tctx, &r), "OemChangePasswordUser2 failed"); + torture_comment(tctx, "(%s:%s) old_password[%s] new_password[%s] status[%s]\n", + __location__, __FUNCTION__, + oldpass, newpass, nt_errstr(r.out.result)); if (!NT_STATUS_EQUAL(r.out.result, NT_STATUS_PASSWORD_RESTRICTION) && !NT_STATUS_EQUAL(r.out.result, NT_STATUS_WRONG_PASSWORD)) { @@ -2049,6 +2106,9 @@ static bool test_OemChangePasswordUser2(struct dcerpc_pipe *p, torture_assert_ntstatus_ok(tctx, dcerpc_samr_OemChangePasswordUser2_r(b, tctx, &r), "OemChangePasswordUser2 failed"); + torture_comment(tctx, "(%s:%s) old_password[%s] new_password[%s] status[%s]\n", + __location__, __FUNCTION__, + oldpass, newpass, nt_errstr(r.out.result)); if (!NT_STATUS_EQUAL(r.out.result, NT_STATUS_PASSWORD_RESTRICTION) && !NT_STATUS_EQUAL(r.out.result, NT_STATUS_INVALID_PARAMETER)) { @@ -2063,6 +2123,9 @@ static bool test_OemChangePasswordUser2(struct dcerpc_pipe *p, torture_assert_ntstatus_ok(tctx, dcerpc_samr_OemChangePasswordUser2_r(b, tctx, &r), "OemChangePasswordUser2 failed"); + torture_comment(tctx, "(%s:%s) old_password[%s] new_password[%s] status[%s]\n", + __location__, __FUNCTION__, + oldpass, newpass, nt_errstr(r.out.result)); if (!NT_STATUS_EQUAL(r.out.result, NT_STATUS_INVALID_PARAMETER)) { torture_warning(tctx, "OemChangePasswordUser2 failed, should have returned INVALID_PARAMETER for no supplied validation hash and invalid user - %s\n", @@ -2078,6 +2141,9 @@ static bool test_OemChangePasswordUser2(struct dcerpc_pipe *p, torture_assert_ntstatus_ok(tctx, dcerpc_samr_OemChangePasswordUser2_r(b, tctx, &r), "OemChangePasswordUser2 failed"); + torture_comment(tctx, "(%s:%s) old_password[%s] new_password[%s] status[%s]\n", + __location__, __FUNCTION__, + oldpass, newpass, nt_errstr(r.out.result)); if (!NT_STATUS_EQUAL(r.out.result, NT_STATUS_WRONG_PASSWORD)) { torture_warning(tctx, "OemChangePasswordUser2 failed, should have returned WRONG_PASSWORD for invalid user - %s\n", @@ -2093,6 +2159,9 @@ static bool test_OemChangePasswordUser2(struct dcerpc_pipe *p, torture_assert_ntstatus_ok(tctx, dcerpc_samr_OemChangePasswordUser2_r(b, tctx, &r), "OemChangePasswordUser2 failed"); + torture_comment(tctx, "(%s:%s) old_password[%s] new_password[%s] status[%s]\n", + __location__, __FUNCTION__, + oldpass, newpass, nt_errstr(r.out.result)); if (!NT_STATUS_EQUAL(r.out.result, NT_STATUS_INVALID_PARAMETER)) { torture_warning(tctx, "OemChangePasswordUser2 failed, should have returned INVALID_PARAMETER for no supplied password and invalid user - %s\n", @@ -2114,6 +2183,9 @@ static bool test_OemChangePasswordUser2(struct dcerpc_pipe *p, torture_assert_ntstatus_ok(tctx, dcerpc_samr_OemChangePasswordUser2_r(b, tctx, &r), "OemChangePasswordUser2 failed"); + torture_comment(tctx, "(%s:%s) old_password[%s] new_password[%s] status[%s]\n", + __location__, __FUNCTION__, + oldpass, newpass, nt_errstr(r.out.result)); if (NT_STATUS_EQUAL(r.out.result, NT_STATUS_PASSWORD_RESTRICTION)) { torture_comment(tctx, "OemChangePasswordUser2 returned: %s perhaps min password age? (not fatal)\n", nt_errstr(r.out.result)); @@ -2196,6 +2268,9 @@ static bool test_ChangePasswordUser2(struct dcerpc_pipe *p, struct torture_conte torture_assert_ntstatus_ok(tctx, dcerpc_samr_ChangePasswordUser2_r(b, tctx, &r), "ChangePasswordUser2 failed"); + torture_comment(tctx, "(%s:%s) old_password[%s] new_password[%s] status[%s]\n", + __location__, __FUNCTION__, + oldpass, newpass, nt_errstr(r.out.result)); if (allow_password_restriction && NT_STATUS_EQUAL(r.out.result, NT_STATUS_PASSWORD_RESTRICTION)) { torture_comment(tctx, "ChangePasswordUser2 returned: %s perhaps min password age? (not fatal)\n", nt_errstr(r.out.result)); @@ -2282,6 +2357,9 @@ bool test_ChangePasswordUser3(struct dcerpc_pipe *p, struct torture_context *tct torture_assert_ntstatus_ok(tctx, dcerpc_samr_ChangePasswordUser3_r(b, tctx, &r), "ChangePasswordUser3 failed"); + torture_comment(tctx, "(%s:%s) old_password[%s] new_password[%s] status[%s]\n", + __location__, __FUNCTION__, + oldpass, newpass, nt_errstr(r.out.result)); if (!NT_STATUS_EQUAL(r.out.result, NT_STATUS_PASSWORD_RESTRICTION) && (!NT_STATUS_EQUAL(r.out.result, NT_STATUS_WRONG_PASSWORD))) { torture_warning(tctx, "ChangePasswordUser3 failed, should have returned WRONG_PASSWORD (or at least 'PASSWORD_RESTRICTON') for invalid password verifier - %s\n", @@ -2314,6 +2392,9 @@ bool test_ChangePasswordUser3(struct dcerpc_pipe *p, struct torture_context *tct torture_assert_ntstatus_ok(tctx, dcerpc_samr_ChangePasswordUser3_r(b, tctx, &r), "ChangePasswordUser3 failed"); + torture_comment(tctx, "(%s:%s) old_password[%s] new_password[%s] status[%s]\n", + __location__, __FUNCTION__, + oldpass, newpass, nt_errstr(r.out.result)); if (!NT_STATUS_EQUAL(r.out.result, NT_STATUS_PASSWORD_RESTRICTION) && (!NT_STATUS_EQUAL(r.out.result, NT_STATUS_WRONG_PASSWORD))) { torture_warning(tctx, "ChangePasswordUser3 failed, should have returned WRONG_PASSWORD (or at least 'PASSWORD_RESTRICTON') for invalidly encrpted password - %s\n", @@ -2327,6 +2408,9 @@ bool test_ChangePasswordUser3(struct dcerpc_pipe *p, struct torture_context *tct r.in.account = &account_bad; torture_assert_ntstatus_ok(tctx, dcerpc_samr_ChangePasswordUser3_r(b, tctx, &r), "ChangePasswordUser3 failed"); + torture_comment(tctx, "(%s:%s) old_password[%s] new_password[%s] status[%s]\n", + __location__, __FUNCTION__, + oldpass, newpass, nt_errstr(r.out.result)); if (!NT_STATUS_EQUAL(r.out.result, NT_STATUS_WRONG_PASSWORD)) { torture_warning(tctx, "ChangePasswordUser3 failed, should have returned WRONG_PASSWORD for invalid username - %s\n", nt_errstr(r.out.result)); @@ -2362,6 +2446,18 @@ bool test_ChangePasswordUser3(struct dcerpc_pipe *p, struct torture_context *tct torture_assert_ntstatus_ok(tctx, dcerpc_samr_ChangePasswordUser3_r(b, tctx, &r), "ChangePasswordUser3 failed"); + torture_comment(tctx, "(%s:%s) old_password[%s] new_password[%s] status[%s]\n", + __location__, __FUNCTION__, + oldpass, newpass, nt_errstr(r.out.result)); + + torture_comment(tctx, "(%s): dominfo[%s], reject[%s], handle_reject_reason[%s], " + "last_password_change[%s], dominfo->min_password_age[%lld]\n", + __location__, + (dominfo == NULL)? "NULL" : "present", + reject ? "true" : "false", + handle_reject_reason ? "true" : "false", + null_nttime(last_password_change) ? "null" : "not null", + dominfo ? (long long)dominfo->min_password_age : (long long)0); if (NT_STATUS_EQUAL(r.out.result, NT_STATUS_PASSWORD_RESTRICTION) && dominfo @@ -2514,6 +2610,9 @@ bool test_ChangePasswordRandomBytes(struct dcerpc_pipe *p, struct torture_contex torture_assert_ntstatus_ok(tctx, dcerpc_samr_SetUserInfo_r(b, tctx, &s), "SetUserInfo failed"); + torture_comment(tctx, "(%s:%s) old_password[%s] new_password[%s] status[%s]\n", + __location__, __FUNCTION__, + oldpass, "RANDOM", nt_errstr(s.out.result)); if (!NT_STATUS_IS_OK(s.out.result)) { torture_warning(tctx, "SetUserInfo level %u failed - %s\n", s.in.level, nt_errstr(s.out.result)); @@ -2547,6 +2646,9 @@ bool test_ChangePasswordRandomBytes(struct dcerpc_pipe *p, struct torture_contex torture_assert_ntstatus_ok(tctx, dcerpc_samr_ChangePasswordUser3_r(b, tctx, &r), "ChangePasswordUser3 failed"); + torture_comment(tctx, "(%s:%s) old_password[%s] new_password[%s] status[%s]\n", + __location__, __FUNCTION__, + oldpass, "RANDOM", nt_errstr(r.out.result)); if (NT_STATUS_EQUAL(r.out.result, NT_STATUS_PASSWORD_RESTRICTION)) { if (reject && reject->extendedFailureReason != SAM_PWD_CHANGE_NO_ERROR) { @@ -2586,6 +2688,9 @@ bool test_ChangePasswordRandomBytes(struct dcerpc_pipe *p, struct torture_contex torture_assert_ntstatus_ok(tctx, dcerpc_samr_ChangePasswordUser3_r(b, tctx, &r), "ChangePasswordUser3 failed"); + torture_comment(tctx, "(%s:%s) old_password[%s] new_password[%s] status[%s]\n", + __location__, __FUNCTION__, + oldpass, newpass, nt_errstr(r.out.result)); if (NT_STATUS_EQUAL(r.out.result, NT_STATUS_PASSWORD_RESTRICTION)) { if (reject && reject->extendedFailureReason != SAM_PWD_CHANGE_NO_ERROR) { -- 1.7.9.5 From a272ff40d6e447880cc362b338dbc84c37cbda01 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 10 Dec 2012 23:56:47 +0100 Subject: [PATCH 3/7] s4:dsdb/common: only pass the DSDB_CONTROL_PASSWORD_HASH_VALUES_OID if required This should give the password_hash module a chance to detect if the called was the cleartext password or not. Signed-off-by: Stefan Metzmacher Reviewed-by: Michael Adam --- source4/dsdb/common/util.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/source4/dsdb/common/util.c b/source4/dsdb/common/util.c index 632d5bfa..4543003 100644 --- a/source4/dsdb/common/util.c +++ b/source4/dsdb/common/util.c @@ -1978,6 +1978,7 @@ NTSTATUS samdb_set_password(struct ldb_context *ldb, TALLOC_CTX *mem_ctx, struct ldb_request *req; struct dsdb_control_password_change_status *pwd_stat = NULL; int ret; + bool hash_values = false; NTSTATUS status = NT_STATUS_OK; #define CHECK_RET(x) \ @@ -2013,6 +2014,7 @@ NTSTATUS samdb_set_password(struct ldb_context *ldb, TALLOC_CTX *mem_ctx, el = ldb_msg_find_element(msg, "unicodePwd"); el->flags = LDB_FLAG_MOD_REPLACE; } + hash_values = true; } else { /* the password wasn't specified correctly */ talloc_free(msg); @@ -2050,13 +2052,15 @@ NTSTATUS samdb_set_password(struct ldb_context *ldb, TALLOC_CTX *mem_ctx, return NT_STATUS_NO_MEMORY; } } - ret = ldb_request_add_control(req, - DSDB_CONTROL_PASSWORD_HASH_VALUES_OID, - true, NULL); - if (ret != LDB_SUCCESS) { - talloc_free(req); - talloc_free(msg); - return NT_STATUS_NO_MEMORY; + if (hash_values) { + ret = ldb_request_add_control(req, + DSDB_CONTROL_PASSWORD_HASH_VALUES_OID, + true, NULL); + if (ret != LDB_SUCCESS) { + talloc_free(req); + talloc_free(msg); + return NT_STATUS_NO_MEMORY; + } } ret = ldb_request_add_control(req, DSDB_CONTROL_PASSWORD_CHANGE_STATUS_OID, -- 1.7.9.5 From a9ff05351ba1b44890bb62fc2e7a71cd0a180c12 Mon Sep 17 00:00:00 2001 From: Michael Adam Date: Tue, 11 Dec 2012 13:04:22 +0100 Subject: [PATCH 4/7] s4:dsdb/password_hash: do the min password age checks first Pair-Programmed-With: Stefan Metzmacher Signed-off-by: Michael Adam Signed-off-by: Stefan Metzmacher --- source4/dsdb/samdb/ldb_modules/password_hash.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/source4/dsdb/samdb/ldb_modules/password_hash.c b/source4/dsdb/samdb/ldb_modules/password_hash.c index 4644628..9bf596c 100644 --- a/source4/dsdb/samdb/ldb_modules/password_hash.c +++ b/source4/dsdb/samdb/ldb_modules/password_hash.c @@ -1954,6 +1954,19 @@ static int check_password_restrictions(struct setup_password_fields_io *io) return LDB_SUCCESS; } + /* Password minimum age: yes, this is a minus. The ages are in negative 100nsec units! */ + if ((io->u.pwdLastSet - io->ac->status->domain_data.minPwdAge > io->g.last_set) && + !io->ac->pwd_reset) + { + ret = LDB_ERR_CONSTRAINT_VIOLATION; + ldb_asprintf_errstring(ldb, + "%08X: %s - check_password_restrictions: " + "password is too young to change!", + W_ERROR_V(WERR_PASSWORD_RESTRICTION), + ldb_strerror(ret)); + return ret; + } + /* * Fundamental password checks done by the call * "samdb_check_password". @@ -2064,17 +2077,6 @@ static int check_password_restrictions(struct setup_password_fields_io *io) return ret; } - /* Password minimum age: yes, this is a minus. The ages are in negative 100nsec units! */ - if (io->u.pwdLastSet - io->ac->status->domain_data.minPwdAge > io->g.last_set) { - ret = LDB_ERR_CONSTRAINT_VIOLATION; - ldb_asprintf_errstring(ldb, - "%08X: %s - check_password_restrictions: " - "password is too young to change!", - W_ERROR_V(WERR_PASSWORD_RESTRICTION), - ldb_strerror(ret)); - return ret; - } - return LDB_SUCCESS; } -- 1.7.9.5 From 987fe0f269b254e8465d98d2132e62e91196d4da Mon Sep 17 00:00:00 2001 From: Michael Adam Date: Tue, 11 Dec 2012 13:18:00 +0100 Subject: [PATCH 5/7] s4:rpc_server/samr: do WRONG_PASSWORD checks after the complexity checks This matches the windows behavior. Pair-Programmed-With: Stefan Metzmacher Signed-off-by: Michael Adam Signed-off-by: Stefan Metzmacher --- source4/rpc_server/samr/samr_password.c | 112 ++++++++++++++++++------------- 1 file changed, 65 insertions(+), 47 deletions(-) diff --git a/source4/rpc_server/samr/samr_password.c b/source4/rpc_server/samr/samr_password.c index 8963b04..5caf4b9 100644 --- a/source4/rpc_server/samr/samr_password.c +++ b/source4/rpc_server/samr/samr_password.c @@ -88,34 +88,22 @@ NTSTATUS dcesrv_samr_ChangePasswordUser(struct dcesrv_call_state *dce_call, if (lm_pwd) { D_P16(lm_pwd->hash, r->in.new_lm_crypted->hash, new_lmPwdHash.hash); D_P16(new_lmPwdHash.hash, r->in.old_lm_crypted->hash, checkHash.hash); - if (memcmp(checkHash.hash, lm_pwd, 16) != 0) { - return NT_STATUS_WRONG_PASSWORD; - } } /* decrypt and check the new nt hash */ D_P16(nt_pwd->hash, r->in.new_nt_crypted->hash, new_ntPwdHash.hash); D_P16(new_ntPwdHash.hash, r->in.old_nt_crypted->hash, checkHash.hash); - if (memcmp(checkHash.hash, nt_pwd, 16) != 0) { - return NT_STATUS_WRONG_PASSWORD; - } /* The NT Cross is not required by Win2k3 R2, but if present check the nt cross hash */ if (r->in.cross1_present && r->in.nt_cross && lm_pwd) { D_P16(lm_pwd->hash, r->in.nt_cross->hash, checkHash.hash); - if (memcmp(checkHash.hash, new_ntPwdHash.hash, 16) != 0) { - return NT_STATUS_WRONG_PASSWORD; - } } /* The LM Cross is not required by Win2k3 R2, but if present check the lm cross hash */ if (r->in.cross2_present && r->in.lm_cross && lm_pwd) { D_P16(nt_pwd->hash, r->in.lm_cross->hash, checkHash.hash); - if (memcmp(checkHash.hash, new_lmPwdHash.hash, 16) != 0) { - return NT_STATUS_WRONG_PASSWORD; - } } /* Start a SAM with user privileges for the password change */ @@ -148,6 +136,37 @@ NTSTATUS dcesrv_samr_ChangePasswordUser(struct dcesrv_call_state *dce_call, return status; } + /* decrypt and check the new lm hash */ + if (lm_pwd) { + if (memcmp(checkHash.hash, lm_pwd, 16) != 0) { + ldb_transaction_cancel(sam_ctx); + return NT_STATUS_WRONG_PASSWORD; + } + } + + if (memcmp(checkHash.hash, nt_pwd, 16) != 0) { + ldb_transaction_cancel(sam_ctx); + return NT_STATUS_WRONG_PASSWORD; + } + + /* The NT Cross is not required by Win2k3 R2, but if present + check the nt cross hash */ + if (r->in.cross1_present && r->in.nt_cross && lm_pwd) { + if (memcmp(checkHash.hash, new_ntPwdHash.hash, 16) != 0) { + ldb_transaction_cancel(sam_ctx); + return NT_STATUS_WRONG_PASSWORD; + } + } + + /* The LM Cross is not required by Win2k3 R2, but if present + check the lm cross hash */ + if (r->in.cross2_present && r->in.lm_cross && lm_pwd) { + if (memcmp(checkHash.hash, new_lmPwdHash.hash, 16) != 0) { + ldb_transaction_cancel(sam_ctx); + return NT_STATUS_WRONG_PASSWORD; + } + } + /* And this confirms it in a transaction commit */ ret = ldb_transaction_commit(sam_ctx); if (ret != LDB_SUCCESS) { @@ -256,9 +275,6 @@ NTSTATUS dcesrv_samr_OemChangePasswordUser2(struct dcesrv_call_state *dce_call, E_deshash(new_pass, new_lm_hash); E_old_pw_hash(new_lm_hash, lm_pwd->hash, lm_verifier.hash); - if (memcmp(lm_verifier.hash, r->in.hash->hash, 16) != 0) { - return NT_STATUS_WRONG_PASSWORD; - } /* Connect to a SAMDB with user privileges for the password change */ sam_ctx = samdb_connect(mem_ctx, dce_call->event_ctx, @@ -290,6 +306,11 @@ NTSTATUS dcesrv_samr_OemChangePasswordUser2(struct dcesrv_call_state *dce_call, return status; } + if (memcmp(lm_verifier.hash, r->in.hash->hash, 16) != 0) { + ldb_transaction_cancel(sam_ctx); + return NT_STATUS_WRONG_PASSWORD; + } + /* And this confirms it in a transaction commit */ ret = ldb_transaction_commit(sam_ctx); if (ret != LDB_SUCCESS) { @@ -379,8 +400,33 @@ NTSTATUS dcesrv_samr_ChangePasswordUser3(struct dcesrv_call_state *dce_call, goto failed; } - if (r->in.nt_verifier == NULL) { - status = NT_STATUS_WRONG_PASSWORD; + /* Connect to a SAMDB with user privileges for the password change */ + sam_ctx = samdb_connect(mem_ctx, dce_call->event_ctx, + dce_call->conn->dce_ctx->lp_ctx, + dce_call->conn->auth_state.session_info, 0); + if (sam_ctx == NULL) { + return NT_STATUS_INVALID_SYSTEM_SERVICE; + } + + ret = ldb_transaction_start(sam_ctx); + if (ret != LDB_SUCCESS) { + DEBUG(1, ("Failed to start transaction: %s\n", ldb_errstring(sam_ctx))); + return NT_STATUS_TRANSACTION_ABORTED; + } + + /* Performs the password modification. We pass the old hashes read out + * from the database since they were already checked against the user- + * provided ones. */ + status = samdb_set_password(sam_ctx, mem_ctx, + user_dn, NULL, + &new_password, + NULL, NULL, + lm_pwd, nt_pwd, /* this is a user password change */ + &reason, + &dominfo); + + if (!NT_STATUS_IS_OK(status)) { + ldb_transaction_cancel(sam_ctx); goto failed; } @@ -389,6 +435,7 @@ NTSTATUS dcesrv_samr_ChangePasswordUser3(struct dcesrv_call_state *dce_call, E_old_pw_hash(new_nt_hash, nt_pwd->hash, nt_verifier.hash); if (memcmp(nt_verifier.hash, r->in.nt_verifier->hash, 16) != 0) { + ldb_transaction_cancel(sam_ctx); status = NT_STATUS_WRONG_PASSWORD; goto failed; } @@ -408,42 +455,13 @@ NTSTATUS dcesrv_samr_ChangePasswordUser3(struct dcesrv_call_state *dce_call, E_deshash(new_pass, new_lm_hash); E_old_pw_hash(new_nt_hash, lm_pwd->hash, lm_verifier.hash); if (memcmp(lm_verifier.hash, r->in.lm_verifier->hash, 16) != 0) { + ldb_transaction_cancel(sam_ctx); status = NT_STATUS_WRONG_PASSWORD; goto failed; } } } - /* Connect to a SAMDB with user privileges for the password change */ - sam_ctx = samdb_connect(mem_ctx, dce_call->event_ctx, - dce_call->conn->dce_ctx->lp_ctx, - dce_call->conn->auth_state.session_info, 0); - if (sam_ctx == NULL) { - return NT_STATUS_INVALID_SYSTEM_SERVICE; - } - - ret = ldb_transaction_start(sam_ctx); - if (ret != LDB_SUCCESS) { - DEBUG(1, ("Failed to start transaction: %s\n", ldb_errstring(sam_ctx))); - return NT_STATUS_TRANSACTION_ABORTED; - } - - /* Performs the password modification. We pass the old hashes read out - * from the database since they were already checked against the user- - * provided ones. */ - status = samdb_set_password(sam_ctx, mem_ctx, - user_dn, NULL, - &new_password, - NULL, NULL, - lm_pwd, nt_pwd, /* this is a user password change */ - &reason, - &dominfo); - - if (!NT_STATUS_IS_OK(status)) { - ldb_transaction_cancel(sam_ctx); - goto failed; - } - /* And this confirms it in a transaction commit */ ret = ldb_transaction_commit(sam_ctx); if (ret != LDB_SUCCESS) { -- 1.7.9.5 From 14fdbc5c58a551f60573257041aeafcb954401ea Mon Sep 17 00:00:00 2001 From: Michael Adam Date: Tue, 11 Dec 2012 13:21:11 +0100 Subject: [PATCH 6/7] s4:torture/samr: allow STATUS_PASSWORD_RESTRICTIONS from ChangePasswordUser Pair-Programmed-With: Stefan Metzmacher Signed-off-by: Michael Adam Signed-off-by: Stefan Metzmacher --- source4/torture/rpc/samr.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/source4/torture/rpc/samr.c b/source4/torture/rpc/samr.c index ba0f20b..01f4c0f 100644 --- a/source4/torture/rpc/samr.c +++ b/source4/torture/rpc/samr.c @@ -1759,8 +1759,10 @@ static bool test_ChangePasswordUser(struct dcerpc_binding_handle *b, torture_comment(tctx, "(%s:%s) old_password[%s] new_password[%s] status[%s]\n", __location__, __FUNCTION__, oldpass, newpass, nt_errstr(r.out.result)); - torture_assert_ntstatus_equal(tctx, r.out.result, NT_STATUS_WRONG_PASSWORD, - "ChangePasswordUser failed: expected NT_STATUS_WRONG_PASSWORD because we broke the LM hash"); + if (!NT_STATUS_EQUAL(r.out.result, NT_STATUS_PASSWORD_RESTRICTION)) { + torture_assert_ntstatus_equal(tctx, r.out.result, NT_STATUS_WRONG_PASSWORD, + "ChangePasswordUser failed: expected NT_STATUS_WRONG_PASSWORD because we broke the LM hash"); + } /* Unbreak the LM hash */ hash1.hash[0]--; @@ -1784,8 +1786,10 @@ static bool test_ChangePasswordUser(struct dcerpc_binding_handle *b, torture_comment(tctx, "(%s:%s) old_password[%s] new_password[%s] status[%s]\n", __location__, __FUNCTION__, oldpass, newpass, nt_errstr(r.out.result)); - torture_assert_ntstatus_equal(tctx, r.out.result, NT_STATUS_WRONG_PASSWORD, - "expected NT_STATUS_WRONG_PASSWORD because we broke the NT hash"); + if (!NT_STATUS_EQUAL(r.out.result, NT_STATUS_PASSWORD_RESTRICTION)) { + torture_assert_ntstatus_equal(tctx, r.out.result, NT_STATUS_WRONG_PASSWORD, + "expected NT_STATUS_WRONG_PASSWORD because we broke the NT hash"); + } /* Unbreak the NT hash */ hash3.hash[0]--; @@ -1809,8 +1813,10 @@ static bool test_ChangePasswordUser(struct dcerpc_binding_handle *b, torture_comment(tctx, "(%s:%s) old_password[%s] new_password[%s] status[%s]\n", __location__, __FUNCTION__, oldpass, newpass, nt_errstr(r.out.result)); - if (!NT_STATUS_EQUAL(r.out.result, NT_STATUS_WRONG_PASSWORD)) { - torture_warning(tctx, "ChangePasswordUser failed: expected NT_STATUS_WRONG_PASSWORD because we broke the LM cross-hash, got %s\n", nt_errstr(r.out.result)); + if (!NT_STATUS_EQUAL(r.out.result, NT_STATUS_WRONG_PASSWORD) && + !NT_STATUS_EQUAL(r.out.result, NT_STATUS_PASSWORD_RESTRICTION)) + { + torture_warning(tctx, "ChangePasswordUser failed: expected NT_STATUS_WRONG_PASSWORD or NT_STATUS_PASSWORD_RESTRICTION because we broke the LM cross-hash, got %s\n", nt_errstr(r.out.result)); ret = false; } @@ -1836,8 +1842,10 @@ static bool test_ChangePasswordUser(struct dcerpc_binding_handle *b, torture_comment(tctx, "(%s:%s) old_password[%s] new_password[%s] status[%s]\n", __location__, __FUNCTION__, oldpass, newpass, nt_errstr(r.out.result)); - if (!NT_STATUS_EQUAL(r.out.result, NT_STATUS_WRONG_PASSWORD)) { - torture_warning(tctx, "ChangePasswordUser failed: expected NT_STATUS_WRONG_PASSWORD because we broke the NT cross-hash, got %s\n", nt_errstr(r.out.result)); + if (!NT_STATUS_EQUAL(r.out.result, NT_STATUS_WRONG_PASSWORD) && + !NT_STATUS_EQUAL(r.out.result, NT_STATUS_PASSWORD_RESTRICTION)) + { + torture_warning(tctx, "ChangePasswordUser failed: expected NT_STATUS_WRONG_PASSWORD or NT_STATUS_PASSWORD_RESTRICTION because we broke the NT cross-hash, got %s\n", nt_errstr(r.out.result)); ret = false; } -- 1.7.9.5 From bb89a7bd4ae8d5be6d227defeddcd4df51211b53 Mon Sep 17 00:00:00 2001 From: Michael Adam Date: Tue, 11 Dec 2012 13:34:49 +0100 Subject: [PATCH 7/7] s4:torture:rpc:samr: fix password age calculation in test_ChangePasswordUser3() The min_password_age field is the negative of the age. Pair-Programmed-With: Stefan Metzmacher Signed-off-by: Michael Adam Signed-off-by: Stefan Metzmacher --- source4/torture/rpc/samr.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source4/torture/rpc/samr.c b/source4/torture/rpc/samr.c index 01f4c0f..f17f0d7 100644 --- a/source4/torture/rpc/samr.c +++ b/source4/torture/rpc/samr.c @@ -2490,8 +2490,8 @@ bool test_ChangePasswordUser3(struct dcerpc_pipe *p, struct torture_context *tct Guenther */ - if ((dominfo->min_password_age > 0) && !null_nttime(last_password_change) && - (last_password_change + dominfo->min_password_age > t)) { + if ((dominfo->min_password_age < 0) && !null_nttime(last_password_change) && + (last_password_change - dominfo->min_password_age > t)) { if (reject->extendedFailureReason != SAM_PWD_CHANGE_NO_ERROR) { torture_warning(tctx, "expected SAM_PWD_CHANGE_NO_ERROR (%d), got %d\n", -- 1.7.9.5