The Samba-Bugzilla – Attachment 8338 Details for
Bug 9414
'samba-tool user add' ignores password complexity settings
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Patches for v4-0-test
tmp40.diff (text/plain), 35.68 KB, created by
Stefan Metzmacher
on 2012-12-11 13:07:17 UTC
(
hide
)
Description:
Patches for v4-0-test
Filename:
MIME Type:
Creator:
Stefan Metzmacher
Created:
2012-12-11 13:07:17 UTC
Size:
35.68 KB
patch
obsolete
>From d5d0f984039b0fa3c5296355139f0f2eedc62d14 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >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 <kseeger@samba.org> >Pair-Programmed-With: Michael Adam <obnox@samba.org> > >Signed-off-by: Stefan Metzmacher <metze@samba.org> >Signed-off-by: Michael Adam <obnox@samba.org> >--- > 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 <obnox@samba.org> >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 <metze@samba.org> > >Signed-off-by: Michael Adam <obnox@samba.org> >Signed-off-by: Stefan Metzmacher <metze@samba.org> >--- > 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 <metze@samba.org> >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 <metze@samba.org> >Reviewed-by: Michael Adam <obnox@samba.org> >--- > 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 <obnox@samba.org> >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 <metze@samba.org> > >Signed-off-by: Michael Adam <obnox@samba.org> >Signed-off-by: Stefan Metzmacher <metze@samba.org> >--- > 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 <obnox@samba.org> >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 <metze@samba.org> > >Signed-off-by: Michael Adam <obnox@samba.org> >Signed-off-by: Stefan Metzmacher <metze@samba.org> >--- > 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 <obnox@samba.org> >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 <metze@samba.org> > >Signed-off-by: Michael Adam <obnox@samba.org> >Signed-off-by: Stefan Metzmacher <metze@samba.org> >--- > 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 <obnox@samba.org> >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 <metze@samba.org> > >Signed-off-by: Michael Adam <obnox@samba.org> >Signed-off-by: Stefan Metzmacher <metze@samba.org> >--- > 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 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Raw
Flags:
obnox
:
review+
metze
:
review+
Actions:
View
Attachments on
bug 9414
:
8299
|
8338
|
8339