From da4970c43ef909306ad6c3f3815d749504e3a58b Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 4 Mar 2022 21:50:15 +0100 Subject: [PATCH 01/26] python:tests: let insta_creds() also copy the bind_dn from the template BUG: https://bugzilla.samba.org/show_bug.cgi?id=13879 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit a30a7626254c863f95b98c97ea46ff54b98078ad) --- python/samba/tests/__init__.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/python/samba/tests/__init__.py b/python/samba/tests/__init__.py index 3812f5bad4a4..d0cf59e46154 100644 --- a/python/samba/tests/__init__.py +++ b/python/samba/tests/__init__.py @@ -172,6 +172,8 @@ class TestCase(unittest.TestCase): username = template.get_username() userpass = template.get_password() + simple_bind_dn = template.get_bind_dn() + if kerberos_state is None: kerberos_state = template.get_kerberos_state() @@ -185,6 +187,8 @@ class TestCase(unittest.TestCase): c.set_gensec_features(c.get_gensec_features() | gensec.FEATURE_SEAL) c.set_kerberos_state(kerberos_state) + if simple_bind_dn: + c.set_bind_dn(simple_bind_dn) return c def assertStringsEqual(self, a, b, msg=None, strip=False): -- 2.25.1 From a4496f4bf5dbb4e5cd4eb7005afbfe2a05005ea5 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Sat, 5 Mar 2022 01:36:50 +0100 Subject: [PATCH 02/26] dsdb/tests: passwords.py don't need to import BasePasswordTestCase BUG: https://bugzilla.samba.org/show_bug.cgi?id=13879 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 90754591a7e4d5a3af70c01425930f4ec063c516) --- source4/dsdb/tests/python/passwords.py | 1 - 1 file changed, 1 deletion(-) diff --git a/source4/dsdb/tests/python/passwords.py b/source4/dsdb/tests/python/passwords.py index 5025962e6ea3..0a2b0fd57174 100755 --- a/source4/dsdb/tests/python/passwords.py +++ b/source4/dsdb/tests/python/passwords.py @@ -36,7 +36,6 @@ from samba import gensec from samba.samdb import SamDB import samba.tests from samba.tests import delete_force -from password_lockout_base import BasePasswordTestCase parser = optparse.OptionParser("passwords.py [options] ") sambaopts = options.SambaOptions(parser) -- 2.25.1 From c5195ec7318bcc3f7179f325816ecbf5ebd5ff65 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Sat, 5 Mar 2022 00:09:17 +0100 Subject: [PATCH 03/26] dsdb/tests: let all BasePasswordTestCase tests provide self.host_url[_ldaps] This will make further changes easier. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13879 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 5a3214c99048a88b0a9f509e3b5b38326529b02c) --- source4/dsdb/tests/python/login_basics.py | 5 ++--- source4/dsdb/tests/python/password_lockout.py | 7 +++---- source4/dsdb/tests/python/rodc_rwdc.py | 4 ++++ 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/source4/dsdb/tests/python/login_basics.py b/source4/dsdb/tests/python/login_basics.py index d265441826de..6b1d04ad8cee 100755 --- a/source4/dsdb/tests/python/login_basics.py +++ b/source4/dsdb/tests/python/login_basics.py @@ -48,7 +48,8 @@ class BasicUserAuthTests(BasePasswordTestCase): def setUp(self): self.host = host - self.host_url = host_url + self.host_url = "ldap://%s" % host + self.host_url_ldaps = "ldaps://%s" % host self.lp = lp self.global_creds = global_creds self.ldb = SamDB(url=self.host_url, credentials=self.global_creds, @@ -180,6 +181,4 @@ userPassword: %s self._test_login_basics(self.lockout1ntlm_creds) -host_url = "ldap://%s" % host - TestProgram(module=__name__, opts=subunitopts) diff --git a/source4/dsdb/tests/python/password_lockout.py b/source4/dsdb/tests/python/password_lockout.py index 445944862b8a..e162f4e037d0 100755 --- a/source4/dsdb/tests/python/password_lockout.py +++ b/source4/dsdb/tests/python/password_lockout.py @@ -69,7 +69,8 @@ import password_lockout_base class PasswordTests(password_lockout_base.BasePasswordTestCase): def setUp(self): self.host = host - self.host_url = host_url + self.host_url = "ldap://%s" % host + self.host_url_ldaps = "ldaps://%s" % host self.lp = lp self.global_creds = global_creds self.ldb = SamDB(url=self.host_url, session_info=system_session(self.lp), @@ -140,7 +141,7 @@ lockoutTime: 0 cmd = cmd_sambatool.subcommands['user'].subcommands['unlock'] result = cmd._run("samba-tool user unlock", username, - "-H%s" % host_url, + "-H%s" % self.host_url, "-U%s%%%s" % (global_creds.get_username(), global_creds.get_password())) self.assertEqual(result, None) @@ -1422,6 +1423,4 @@ class PasswordTestsWithDefaults(PasswordTests): self._test_login_lockout(self.lockout1ntlm_creds, wait_lockout_duration=False) -host_url = "ldap://%s" % host - TestProgram(module=__name__, opts=subunitopts) diff --git a/source4/dsdb/tests/python/rodc_rwdc.py b/source4/dsdb/tests/python/rodc_rwdc.py index c4507869fd17..d32169a3042a 100644 --- a/source4/dsdb/tests/python/rodc_rwdc.py +++ b/source4/dsdb/tests/python/rodc_rwdc.py @@ -205,11 +205,13 @@ class RodcRwdcCachedTests(password_lockout_base.BasePasswordTestCase): self.global_creds = CREDS self.host = RWDC self.host_url = 'ldap://%s' % RWDC + self.host_url_ldaps = 'ldaps://%s' % RWDC self.ldb = SamDB(url='ldap://%s' % RWDC, session_info=system_session(self.lp), credentials=self.global_creds, lp=self.lp) super(RodcRwdcCachedTests, self).setUp() self.host_url = 'ldap://%s' % RODC + self.host_url_ldaps = 'ldaps://%s' % RODC self.samr = samr.samr("ncacn_ip_tcp:%s[seal]" % self.host, self.lp, self.global_creds) self.samr_handle = self.samr.Connect2(None, security.SEC_FLAG_MAXIMUM_ALLOWED) @@ -744,12 +746,14 @@ class RodcRwdcTests(password_lockout_base.BasePasswordTestCase): self.global_creds = CREDS self.host = RWDC self.host_url = 'ldap://%s' % RWDC + self.host_url_ldaps = 'ldaps://%s' % RWDC self.ldb = SamDB(url='ldap://%s' % RWDC, session_info=system_session(self.lp), credentials=self.global_creds, lp=self.lp) super(RodcRwdcTests, self).setUp() self.host = RODC self.host_url = 'ldap://%s' % RODC + self.host_url_ldaps = 'ldaps://%s' % RODC self.ldb = SamDB(url='ldap://%s' % RODC, session_info=system_session(self.lp), credentials=self.global_creds, lp=self.lp) -- 2.25.1 From b5904118b952fe72e5bd8d7fad9ad7c2234f584b Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 4 Mar 2022 23:35:26 +0100 Subject: [PATCH 04/26] dsdb/tests: make use of assertLoginFailure helper BUG: https://bugzilla.samba.org/show_bug.cgi?id=13879 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 03ba5af3d9eaeb5f0c7c1a1a61ef2ac454eb8392) --- source4/dsdb/tests/python/password_lockout_base.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/source4/dsdb/tests/python/password_lockout_base.py b/source4/dsdb/tests/python/password_lockout_base.py index 0f9617da1e6f..913e8304a8ae 100644 --- a/source4/dsdb/tests/python/password_lockout_base.py +++ b/source4/dsdb/tests/python/password_lockout_base.py @@ -248,12 +248,7 @@ userPassword: """ + userpass + """ self._check_account_initial(userdn) # Fail once to get a badPasswordTime - try: - ldb = SamDB(url=self.host_url, credentials=fail_creds, lp=self.lp) - self.fail() - except LdbError as e: - (num, msg) = e.args - self.assertEqual(num, ERR_INVALID_CREDENTIALS) + self.assertLoginFailure(self.host_url, fail_creds, self.lp) # Succeed to reset everything to 0 ldb = SamDB(url=self.host_url, credentials=creds, lp=self.lp) -- 2.25.1 From 746152ef1dcd0ce68bc9f69b4a16b4ae4ae1ee80 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 4 Mar 2022 23:35:26 +0100 Subject: [PATCH 05/26] dsdb/tests: introduce assertLoginSuccess This makes it possible to catch failures with knownfail entries. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13879 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 751ce671a4af32bc1c56433a5a1c8161377856c5) --- source4/dsdb/tests/python/login_basics.py | 7 +++---- source4/dsdb/tests/python/password_lockout_base.py | 13 ++++++++++++- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/source4/dsdb/tests/python/login_basics.py b/source4/dsdb/tests/python/login_basics.py index 6b1d04ad8cee..e43673456f50 100755 --- a/source4/dsdb/tests/python/login_basics.py +++ b/source4/dsdb/tests/python/login_basics.py @@ -102,7 +102,7 @@ class BasicUserAuthTests(BasePasswordTestCase): # check logging in with the correct password succeeds test_creds.set_password(userpass) - user_ldb = SamDB(url=self.host_url, credentials=test_creds, lp=self.lp) + user_ldb = self.assertLoginSuccess(self.host_url, test_creds, self.lp) res = self._check_account(userdn, badPwdCount=0, badPasswordTime=badPasswordTime, @@ -148,8 +148,7 @@ userPassword: %s badPasswordTime = int(res[0]["badPasswordTime"][0]) else: # for NTLM, logging in with the old password succeeds - user_ldb = SamDB(url=self.host_url, credentials=test_creds, - lp=self.lp) + user_ldb = self.assertLoginSuccess(self.host_url, test_creds, self.lp) info_msg = 'Test NTLM login with old password succeeds' res = self._check_account(userdn, badPwdCount=0, @@ -163,7 +162,7 @@ userPassword: %s # check logging in with the new password succeeds test_creds.set_password(new_password) - user_ldb = SamDB(url=self.host_url, credentials=test_creds, lp=self.lp) + user_ldb = self.assertLoginSuccess(self.host_url, test_creds, self.lp) res = self._check_account(userdn, badPwdCount=0, badPasswordTime=badPasswordTime, diff --git a/source4/dsdb/tests/python/password_lockout_base.py b/source4/dsdb/tests/python/password_lockout_base.py index 913e8304a8ae..0951ed8594f5 100644 --- a/source4/dsdb/tests/python/password_lockout_base.py +++ b/source4/dsdb/tests/python/password_lockout_base.py @@ -251,7 +251,7 @@ userPassword: """ + userpass + """ self.assertLoginFailure(self.host_url, fail_creds, self.lp) # Succeed to reset everything to 0 - ldb = SamDB(url=self.host_url, credentials=creds, lp=self.lp) + ldb = self.assertLoginSuccess(self.host_url, creds, self.lp) return ldb @@ -266,6 +266,17 @@ userPassword: """ + userpass + """ "(got err %d, expected %d)" % (num, errno))) + def assertLoginSuccess(self, url, creds, lp): + try: + ldb = SamDB(url=url, credentials=creds, lp=lp) + return ldb + except LdbError as e1: + (num, msg) = e1.args + self.assertEqual(num, LDB_SUCCESS, + ("Login failed - %d - %s" % ( + num, msg))) + + def setUp(self): super(BasePasswordTestCase, self).setUp() -- 2.25.1 From f7bda7316f1d089a9df92f681739ec578cfe4147 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 4 Mar 2022 21:53:06 +0100 Subject: [PATCH 06/26] dsdb/tests: prepare BasePasswordTestCase for simple bind tests BUG: https://bugzilla.samba.org/show_bug.cgi?id=13879 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 0b1fbc9d56e2a25e3f1527ee5bc54880bdc65fc6) --- .../tests/python/password_lockout_base.py | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/source4/dsdb/tests/python/password_lockout_base.py b/source4/dsdb/tests/python/password_lockout_base.py index 0951ed8594f5..f311502da3c0 100644 --- a/source4/dsdb/tests/python/password_lockout_base.py +++ b/source4/dsdb/tests/python/password_lockout_base.py @@ -6,6 +6,7 @@ from samba.credentials import Credentials, DONT_USE_KERBEROS, MUST_USE_KERBEROS from ldb import SCOPE_BASE, LdbError from ldb import ERR_CONSTRAINT_VIOLATION from ldb import ERR_INVALID_CREDENTIALS +from ldb import SUCCESS as LDB_SUCCESS from ldb import Message, MessageElement, Dn from ldb import FLAG_MOD_REPLACE from samba import gensec, dsdb @@ -213,11 +214,17 @@ class BasePasswordTestCase(PasswordTestCase): FLAG_MOD_REPLACE, "lockOutObservationWindow") self.ldb.modify(m) - def _readd_user(self, creds, lockOutObservationWindow=0): + def _readd_user(self, creds, lockOutObservationWindow=0, simple=False): username = creds.get_username() userpass = creds.get_password() userdn = "cn=%s,cn=users,%s" % (username, self.base_dn) + if simple: + creds.set_bind_dn(userdn) + ldap_url = self.host_url_ldaps + else: + ldap_url = self.host_url + delete_force(self.ldb, userdn) self.ldb.add({ "dn": userdn, @@ -248,10 +255,10 @@ userPassword: """ + userpass + """ self._check_account_initial(userdn) # Fail once to get a badPasswordTime - self.assertLoginFailure(self.host_url, fail_creds, self.lp) + self.assertLoginFailure(ldap_url, fail_creds, self.lp) # Succeed to reset everything to 0 - ldb = self.assertLoginSuccess(self.host_url, creds, self.lp) + ldb = self.assertLoginSuccess(ldap_url, creds, self.lp) return ldb @@ -362,10 +369,17 @@ lockoutThreshold: """ + str(lockoutThreshold) + """ userpass="thatsAcomplPASS0", kerberos_state=DONT_USE_KERBEROS) self.lockout1ntlm_ldb = self._readd_user(self.lockout1ntlm_creds) + self.lockout1simple_creds = self.insta_creds(self.template_creds, + username="lockout1simple", + userpass="thatsAcomplPASS0", + kerberos_state=DONT_USE_KERBEROS) + self.lockout1simple_ldb = self._readd_user(self.lockout1simple_creds, + simple=True) def delete_ldb_connections(self): del self.lockout1krb5_ldb del self.lockout1ntlm_ldb + del self.lockout1simple_ldb del self.ldb def tearDown(self): -- 2.25.1 From 0ecdeef69be11f85fd11723593ad24869e98452f Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 4 Mar 2022 21:53:06 +0100 Subject: [PATCH 07/26] dsdb/tests: add test_login_basics_simple() This demonstrates that 'old password allowed period' also applies to LDAP simple binds and not only to GSS-SPNEGO/NTLMSSP binds. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13879 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15001 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 3625d1381592f7af8ec14715c6c2dfa4d9f02676) --- selftest/knownfail.d/samba4.ldap.login_basics | 1 + source4/dsdb/tests/python/login_basics.py | 26 ++++++++++++++----- 2 files changed, 20 insertions(+), 7 deletions(-) create mode 100644 selftest/knownfail.d/samba4.ldap.login_basics diff --git a/selftest/knownfail.d/samba4.ldap.login_basics b/selftest/knownfail.d/samba4.ldap.login_basics new file mode 100644 index 000000000000..9854b5ce76f3 --- /dev/null +++ b/selftest/knownfail.d/samba4.ldap.login_basics @@ -0,0 +1 @@ +^samba4.ldap.login_basics.python.*.__main__.BasicUserAuthTests.test_login_basics_simple diff --git a/source4/dsdb/tests/python/login_basics.py b/source4/dsdb/tests/python/login_basics.py index e43673456f50..d28b56c60653 100755 --- a/source4/dsdb/tests/python/login_basics.py +++ b/source4/dsdb/tests/python/login_basics.py @@ -56,17 +56,24 @@ class BasicUserAuthTests(BasePasswordTestCase): session_info=system_session(self.lp), lp=self.lp) super(BasicUserAuthTests, self).setUp() - def _test_login_basics(self, creds): + def _test_login_basics(self, creds, simple=False): username = creds.get_username() userpass = creds.get_password() userdn = "cn=%s,cn=users,%s" % (username, self.base_dn) if creds.get_kerberos_state() == MUST_USE_KERBEROS: logoncount_relation = 'greater' lastlogon_relation = 'greater' + ldap_url = self.host_url print("Performs a lockout attempt against LDAP using Kerberos") + elif simple: + logoncount_relation = 'equal' + lastlogon_relation = 'equal' + ldap_url = self.host_url_ldaps + print("Performs a lockout attempt against LDAP using Simple") else: logoncount_relation = 'equal' lastlogon_relation = 'equal' + ldap_url = self.host_url print("Performs a lockout attempt against LDAP using NTLM") # get the intial logon values for this user @@ -88,7 +95,7 @@ class BasicUserAuthTests(BasePasswordTestCase): # check logging in with the wrong password fails test_creds.set_password("thatsAcomplPASS1xBAD") - self.assertLoginFailure(self.host_url, test_creds, self.lp) + self.assertLoginFailure(ldap_url, test_creds, self.lp) res = self._check_account(userdn, badPwdCount=1, badPasswordTime=("greater", badPasswordTime), @@ -102,7 +109,7 @@ class BasicUserAuthTests(BasePasswordTestCase): # check logging in with the correct password succeeds test_creds.set_password(userpass) - user_ldb = self.assertLoginSuccess(self.host_url, test_creds, self.lp) + user_ldb = self.assertLoginSuccess(ldap_url, test_creds, self.lp) res = self._check_account(userdn, badPwdCount=0, badPasswordTime=badPasswordTime, @@ -133,7 +140,7 @@ userPassword: %s # for Kerberos, logging in with the old password fails if creds.get_kerberos_state() == MUST_USE_KERBEROS: - self.assertLoginFailure(self.host_url, test_creds, self.lp) + self.assertLoginFailure(ldap_url, test_creds, self.lp) info_msg = 'Test Kerberos login with old password fails' expectBadPwdTime = ("greater", badPasswordTime) res = self._check_account(userdn, @@ -148,8 +155,11 @@ userPassword: %s badPasswordTime = int(res[0]["badPasswordTime"][0]) else: # for NTLM, logging in with the old password succeeds - user_ldb = self.assertLoginSuccess(self.host_url, test_creds, self.lp) - info_msg = 'Test NTLM login with old password succeeds' + user_ldb = self.assertLoginSuccess(ldap_url, test_creds, self.lp) + if simple: + info_msg = 'Test simple-bind login with old password succeeds' + else: + info_msg = 'Test NTLM login with old password succeeds' res = self._check_account(userdn, badPwdCount=0, badPasswordTime=badPasswordTime, @@ -162,7 +172,7 @@ userPassword: %s # check logging in with the new password succeeds test_creds.set_password(new_password) - user_ldb = self.assertLoginSuccess(self.host_url, test_creds, self.lp) + user_ldb = self.assertLoginSuccess(ldap_url, test_creds, self.lp) res = self._check_account(userdn, badPwdCount=0, badPasswordTime=badPasswordTime, @@ -179,5 +189,7 @@ userPassword: %s def test_login_basics_ntlm(self): self._test_login_basics(self.lockout1ntlm_creds) + def test_login_basics_simple(self): + self._test_login_basics(self.lockout1simple_creds, simple=True) TestProgram(module=__name__, opts=subunitopts) -- 2.25.1 From 9f0217af18f059151038fc3875492720faa2374c Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 8 Mar 2022 15:14:09 +0100 Subject: [PATCH 08/26] s3:auth: let make_user_info_netlogon_interactive() set USER_INFO_INTERACTIVE_LOGON This is not really relevant for now, as USER_INFO_INTERACTIVE_LOGON is not evaluated in the source3/auth stack. But better add it to be consistent. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15001 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 012bd9f5b780f7a90cf3bd918f044ea67fae7017) --- source3/auth/auth_util.c | 1 + 1 file changed, 1 insertion(+) diff --git a/source3/auth/auth_util.c b/source3/auth/auth_util.c index 7a97dd45f11e..9d9f2234f210 100644 --- a/source3/auth/auth_util.c +++ b/source3/auth/auth_util.c @@ -264,6 +264,7 @@ bool make_user_info_netlogon_interactive(TALLOC_CTX *mem_ctx, if (NT_STATUS_IS_OK(nt_status)) { (*user_info)->logon_parameters = logon_parameters; + (*user_info)->flags |= USER_INFO_INTERACTIVE_LOGON; } ret = NT_STATUS_IS_OK(nt_status) ? true : false; -- 2.25.1 From 332b0ce74c1c780ec05c0b24bd3eb41418024c35 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 4 Mar 2022 19:09:41 +0100 Subject: [PATCH 09/26] s4:auth_sam: use USER_INFO_INTERACTIVE_LOGON as inducation for an interactive logon Using != AUTH_PASSWORD_RESPONSE is not the correct indication due to the local mappings from AUTH_PASSWORD_PLAIN via AUTH_PASSWORD_HASH to AUTH_PASSWORD_RESPONSE. It means an LDAP simble bind will now honour 'old password allowed period'. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13879 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15001 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 2ad44686229ba02f98de5769c26a3dfeaf5ada2b) --- selftest/knownfail.d/samba4.ldap.login_basics | 1 - source4/auth/ntlm/auth_sam.c | 5 +++-- 2 files changed, 3 insertions(+), 3 deletions(-) delete mode 100644 selftest/knownfail.d/samba4.ldap.login_basics diff --git a/selftest/knownfail.d/samba4.ldap.login_basics b/selftest/knownfail.d/samba4.ldap.login_basics deleted file mode 100644 index 9854b5ce76f3..000000000000 --- a/selftest/knownfail.d/samba4.ldap.login_basics +++ /dev/null @@ -1 +0,0 @@ -^samba4.ldap.login_basics.python.*.__main__.BasicUserAuthTests.test_login_basics_simple diff --git a/source4/auth/ntlm/auth_sam.c b/source4/auth/ntlm/auth_sam.c index dbbf97665db3..ddde4363d926 100644 --- a/source4/auth/ntlm/auth_sam.c +++ b/source4/auth/ntlm/auth_sam.c @@ -410,10 +410,11 @@ static NTSTATUS authsam_password_check_and_record(struct auth4_context *auth_con return NT_STATUS_WRONG_PASSWORD; } - if (user_info->password_state != AUTH_PASSWORD_RESPONSE) { + if (user_info->flags & USER_INFO_INTERACTIVE_LOGON) { /* * The authentication was OK against the previous password, - * but it's not a NTLM network authentication. + * but it's not a NTLM network authentication, + * LDAP simple bind or something similar. * * We just return the original wrong password. * This skips the update of the bad pwd count, -- 2.25.1 From 8b85a8a03c84cb18f984cd23236bb23210a55e86 Mon Sep 17 00:00:00 2001 From: Garming Sam Date: Mon, 1 Apr 2019 15:46:48 +1300 Subject: [PATCH 10/26] rodc: Add tests for simple BIND alongside NTLMSSP binds BUG: https://bugzilla.samba.org/show_bug.cgi?id=13879 Signed-off-by: Garming Sam Reviewed-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 62fb6c1dc8527db6cf0f08d4d06e8813707f767a) --- selftest/knownfail.d/rodc_rwdc | 1 + source4/dsdb/tests/python/rodc_rwdc.py | 59 ++++++++++++++++---------- 2 files changed, 38 insertions(+), 22 deletions(-) create mode 100644 selftest/knownfail.d/rodc_rwdc diff --git a/selftest/knownfail.d/rodc_rwdc b/selftest/knownfail.d/rodc_rwdc new file mode 100644 index 000000000000..c148d035f5e2 --- /dev/null +++ b/selftest/knownfail.d/rodc_rwdc @@ -0,0 +1 @@ +^samba4.ldap.rodc_rwdc.*test_ldap_change_password_simple_bind diff --git a/source4/dsdb/tests/python/rodc_rwdc.py b/source4/dsdb/tests/python/rodc_rwdc.py index d32169a3042a..6cd0e50e47b6 100644 --- a/source4/dsdb/tests/python/rodc_rwdc.py +++ b/source4/dsdb/tests/python/rodc_rwdc.py @@ -44,7 +44,7 @@ class RodcRwdcTestException(Exception): pass -def make_creds(username, password, kerberos_state=None): +def make_creds(username, password, kerberos_state=None, simple_dn=None): # use the global CREDS as a template c = Credentials() c.set_username(username) @@ -53,6 +53,9 @@ def make_creds(username, password, kerberos_state=None): c.set_realm(CREDS.get_realm()) c.set_workstation(CREDS.get_workstation()) + if simple_dn is not None: + c.set_bind_dn(simple_dn) + if kerberos_state is None: kerberos_state = CREDS.get_kerberos_state() c.set_kerberos_state(kerberos_state) @@ -1024,10 +1027,14 @@ class RodcRwdcTests(password_lockout_base.BasePasswordTestCase): "add: userPassword\n" "userPassword: %s\n" % (user_dn, old_password, new_password)) - def try_ldap_logon(self, server, creds, errno=None): + def try_ldap_logon(self, server, creds, errno=None, simple=False): try: - tmpdb = SamDB('ldap://%s' % server, credentials=creds, - session_info=system_session(LP), lp=LP) + if simple: + tmpdb = SamDB('ldaps://%s' % server, credentials=creds, + session_info=system_session(LP), lp=LP) + else: + tmpdb = SamDB('ldap://%s' % server, credentials=creds, + session_info=system_session(LP), lp=LP) if errno is not None: self.fail("logon failed to fail with ldb error %s" % errno) except ldb.LdbError as e10: @@ -1046,19 +1053,23 @@ class RodcRwdcTests(password_lockout_base.BasePasswordTestCase): if min_pwd_age != 0: self.rwdc_db.set_minPwdAge('0') - def _test_ldap_change_password(self, errno=None): + def _test_ldap_change_password(self, errno=None, simple=False): self.zero_min_password_age() dn, username, password = self._new_user() - creds1 = make_creds(username, password) + + simple_dn = dn if simple else None + + creds1 = make_creds(username, password, simple_dn=simple_dn) # With NTLM, this should fail on RODC before replication, # because the user isn't known. - self.try_ldap_logon(RODC, creds1, ldb.ERR_INVALID_CREDENTIALS) + self.try_ldap_logon(RODC, creds1, ldb.ERR_INVALID_CREDENTIALS, + simple=simple) self.force_replication() # Now the user is replicated to RODC, so logon should work - self.try_ldap_logon(RODC, creds1) + self.try_ldap_logon(RODC, creds1, simple=simple) passwords = ['password#%s' % i for i in range(1, 6)] for prev, password in zip(passwords[:-1], passwords[1:]): @@ -1067,40 +1078,40 @@ class RodcRwdcTests(password_lockout_base.BasePasswordTestCase): # The password has changed enough times to make the old # password invalid (though with kerberos that doesn't matter). # For NTLM, the old creds should always fail - self.try_ldap_logon(RODC, creds1, errno) - self.try_ldap_logon(RWDC, creds1, errno) + self.try_ldap_logon(RODC, creds1, errno, simple=simple) + self.try_ldap_logon(RWDC, creds1, errno, simple=simple) - creds2 = make_creds(username, password) + creds2 = make_creds(username, password, simple_dn=simple_dn) # new creds work straight away with NTLM, because although it # doesn't have the password, it knows the user and forwards # the query. - self.try_ldap_logon(RODC, creds2) - self.try_ldap_logon(RWDC, creds2) + self.try_ldap_logon(RODC, creds2, simple=simple) + self.try_ldap_logon(RWDC, creds2, simple=simple) self.force_replication() # After another replication check RODC still works and fails, # as appropriate to various creds - self.try_ldap_logon(RODC, creds2) - self.try_ldap_logon(RODC, creds1, errno) + self.try_ldap_logon(RODC, creds2, simple=simple) + self.try_ldap_logon(RODC, creds1, errno, simple=simple) prev = password password = 'password#6' self._change_password(dn, prev, password) - creds3 = make_creds(username, password) + creds3 = make_creds(username, password, simple_dn=simple_dn) # previous password should still work. - self.try_ldap_logon(RWDC, creds2) - self.try_ldap_logon(RODC, creds2) + self.try_ldap_logon(RWDC, creds2, simple=simple) + self.try_ldap_logon(RODC, creds2, simple=simple) # new password should still work. - self.try_ldap_logon(RWDC, creds3) - self.try_ldap_logon(RODC, creds3) + self.try_ldap_logon(RWDC, creds3, simple=simple) + self.try_ldap_logon(RODC, creds3, simple=simple) # old password should still fail (but not on kerberos). - self.try_ldap_logon(RWDC, creds1, errno) - self.try_ldap_logon(RODC, creds1, errno) + self.try_ldap_logon(RWDC, creds1, errno, simple=simple) + self.try_ldap_logon(RODC, creds1, errno, simple=simple) def test_ldap_change_password_kerberos(self): CREDS.set_kerberos_state(MUST_USE_KERBEROS) @@ -1110,6 +1121,10 @@ class RodcRwdcTests(password_lockout_base.BasePasswordTestCase): CREDS.set_kerberos_state(DONT_USE_KERBEROS) self._test_ldap_change_password(ldb.ERR_INVALID_CREDENTIALS) + def test_ldap_change_password_simple_bind(self): + CREDS.set_kerberos_state(DONT_USE_KERBEROS) + self._test_ldap_change_password(ldb.ERR_INVALID_CREDENTIALS, simple=True) + def _test_ldap_change_password_reveal_on_demand(self, errno=None): self.zero_min_password_age() -- 2.25.1 From 99c3cb8f32a40c6c9feb63e0af4174cd3304dcf3 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 4 Mar 2022 11:41:20 +0100 Subject: [PATCH 11/26] s3:rpc_client: let rpccli_netlogon_network_logon() fallback to workstation = lp_netbios_name() BUG: https://bugzilla.samba.org/show_bug.cgi?id=13879 BUG: https://bugzilla.samba.org/show_bug.cgi?id=14641 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 5c04c01354944fc3a64bb109bf3e9bf89086cc6f) --- source3/rpc_client/cli_netlogon.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/source3/rpc_client/cli_netlogon.c b/source3/rpc_client/cli_netlogon.c index 175f83d67501..efaa6f9ead6d 100644 --- a/source3/rpc_client/cli_netlogon.c +++ b/source3/rpc_client/cli_netlogon.c @@ -656,6 +656,10 @@ NTSTATUS rpccli_netlogon_network_logon( return NT_STATUS_NO_MEMORY; } + if (workstation == NULL) { + workstation = lp_netbios_name(); + } + if (workstation[0] != '\\' && workstation[1] != '\\') { workstation_name_slash = talloc_asprintf(mem_ctx, "\\\\%s", workstation); } else { -- 2.25.1 From aecc9a67968358f7f716b1b8b2e17e003b0bf6c1 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 2 Mar 2022 14:32:41 +0100 Subject: [PATCH 12/26] s4:auth: a simple bind uses the DCs name as workstation I've seen that in LogonSamLogonEx request triggered by a simple bind with a user of a trusted domain within the same forest. Note simple binds don't work with users for another forest/external domain, as the DsCrackNames call on the bind_dn fails. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13879 BUG: https://bugzilla.samba.org/show_bug.cgi?id=14641 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 31db704882bbcd569c2abb764ac1d3691ee0a267) --- source4/auth/ntlm/auth_simple.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/source4/auth/ntlm/auth_simple.c b/source4/auth/ntlm/auth_simple.c index b2e763813953..f767adb36960 100644 --- a/source4/auth/ntlm/auth_simple.c +++ b/source4/auth/ntlm/auth_simple.c @@ -26,6 +26,7 @@ #include "lib/util/tevent_ntstatus.h" #include "auth/auth.h" #include "dsdb/samdb/samdb.h" +#include "lib/param/param.h" #undef DBGC_CLASS #define DBGC_CLASS DBGC_AUTH @@ -80,7 +81,7 @@ _PUBLIC_ struct tevent_req *authenticate_ldap_simple_bind_send(TALLOC_CTX *mem_c /* No client.domain_name, use account_name instead */ /* user_info->mapped.* will be filled below */ - user_info->workstation_name = NULL; + user_info->workstation_name = lpcfg_netbios_name(lp_ctx); user_info->remote_host = remote_address; user_info->local_host = local_address; -- 2.25.1 From 710bef6ffd9bb1d3d4bf2d0174f2800835f7bc87 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 3 Mar 2022 23:14:38 +0100 Subject: [PATCH 13/26] s4:auth: encrypt_user_info() should set password_state instead of mapped_state user_info->mapped_state has nothing to do with enum auth_password_state, user_info->password_state is the one that holds the auth_password_state value. Luckily user_info->password_state was never referenced in the encrypt_user_info() callers. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13879 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit a6fb598d9dcbfe21ef285b5f30fabcb88a259c93) --- source4/auth/ntlm/auth_util.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source4/auth/ntlm/auth_util.c b/source4/auth/ntlm/auth_util.c index a0d061dca2a6..58e97fb4a771 100644 --- a/source4/auth/ntlm/auth_util.c +++ b/source4/auth/ntlm/auth_util.c @@ -73,7 +73,7 @@ NTSTATUS encrypt_user_info(TALLOC_CTX *mem_ctx, struct auth4_context *auth_conte return NT_STATUS_NO_MEMORY; } *user_info_temp = *user_info_in; - user_info_temp->mapped_state = to_state; + user_info_temp->password_state = to_state; nt_status = auth_get_challenge(auth_context, chal); if (!NT_STATUS_IS_OK(nt_status)) { @@ -147,7 +147,7 @@ NTSTATUS encrypt_user_info(TALLOC_CTX *mem_ctx, struct auth4_context *auth_conte return NT_STATUS_NO_MEMORY; } *user_info_temp = *user_info_in; - user_info_temp->mapped_state = to_state; + user_info_temp->password_state = to_state; if (E_deshash(user_info_in->password.plaintext, lanman.hash)) { user_info_temp->password.hash.lanman = talloc(user_info_temp, -- 2.25.1 From c521d4612ade684b24a73804879d4e86c053078a Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 3 Mar 2022 23:15:31 +0100 Subject: [PATCH 14/26] auth/ntlmssp: don't set mapped_state explicitly in auth_usersupplied_info We already use talloc_zero() and mapped_state will be removed in the next commits. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13879 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 9a4ac8ab2e2c8ee48f6bf5a6ecf7988c435ba1c6) --- auth/ntlmssp/ntlmssp_server.c | 1 - 1 file changed, 1 deletion(-) diff --git a/auth/ntlmssp/ntlmssp_server.c b/auth/ntlmssp/ntlmssp_server.c index ce78af1d32d0..e077c2f7379a 100644 --- a/auth/ntlmssp/ntlmssp_server.c +++ b/auth/ntlmssp/ntlmssp_server.c @@ -771,7 +771,6 @@ static NTSTATUS ntlmssp_server_preauth(struct gensec_security *gensec_security, user_info->logon_parameters = MSV1_0_ALLOW_SERVER_TRUST_ACCOUNT | MSV1_0_ALLOW_WORKSTATION_TRUST_ACCOUNT; user_info->flags = 0; - user_info->mapped_state = false; user_info->client.account_name = ntlmssp_state->user; user_info->client.domain_name = ntlmssp_state->domain; user_info->workstation_name = ntlmssp_state->client.netbios_name; -- 2.25.1 From e40c28405327d58813bb6a6bb8c68855de72ee59 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 3 Mar 2022 23:16:13 +0100 Subject: [PATCH 15/26] s4:smb_server: don't set mapped_state explicitly in auth_usersupplied_info We already use talloc_zero() and mapped_state will be removed in the next commits. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13879 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 859c7817350553259eb09c889bc40afebb60064a) --- source4/smb_server/smb/sesssetup.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/source4/smb_server/smb/sesssetup.c b/source4/smb_server/smb/sesssetup.c index 8428ca3fabb9..07a7e7ea46cb 100644 --- a/source4/smb_server/smb/sesssetup.c +++ b/source4/smb_server/smb/sesssetup.c @@ -188,7 +188,6 @@ static void sesssetup_old(struct smbsrv_request *req, union smb_sesssetup *sess) user_info->service_description = "SMB"; - user_info->mapped_state = false; user_info->logon_parameters = 0; user_info->flags = 0; user_info->client.account_name = sess->old.in.user; @@ -375,7 +374,6 @@ static void sesssetup_nt1(struct smbsrv_request *req, union smb_sesssetup *sess) user_info->service_description = "SMB"; user_info->auth_description = "bare-NTLM"; - user_info->mapped_state = false; user_info->logon_parameters = 0; user_info->flags = 0; user_info->client.account_name = sess->nt1.in.user; -- 2.25.1 From fdb744d4dba01a22cb002cd8c34755b70c1bb61a Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 3 Mar 2022 23:16:13 +0100 Subject: [PATCH 16/26] s4:dsdb: don't set mapped_state in auth_usersupplied_info for audit logging mapped_state is completely irrelevant for audit logging and will also be removed in the next commits. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13879 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 99efe5f4e9ce426b28cef94d858849707ce15739) --- source4/dsdb/samdb/ldb_modules/password_hash.c | 1 - 1 file changed, 1 deletion(-) diff --git a/source4/dsdb/samdb/ldb_modules/password_hash.c b/source4/dsdb/samdb/ldb_modules/password_hash.c index 0e08f4889ee0..1b8a713ecf59 100644 --- a/source4/dsdb/samdb/ldb_modules/password_hash.c +++ b/source4/dsdb/samdb/ldb_modules/password_hash.c @@ -3011,7 +3011,6 @@ static int check_password_restrictions_and_log(struct setup_password_fields_io * * logs are consistent, even if some elements are always NULL. */ struct auth_usersupplied_info ui = { - .mapped_state = true, .was_mapped = true, .client = { .account_name = io->u.sAMAccountName, -- 2.25.1 From 5beda8cd8235b95cdbd674083c586a6e6d298ea5 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 3 Mar 2022 23:16:13 +0100 Subject: [PATCH 17/26] s4:kdc: don't set mapped_state in auth_usersupplied_info for audit logging mapped_state is completely irrelevant for audit logging and will also be removed in the next commits. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13879 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit ca6948642bc2ff821ec4ca8ab24902b1ba9e8397) --- source4/kdc/hdb-samba4.c | 1 - 1 file changed, 1 deletion(-) diff --git a/source4/kdc/hdb-samba4.c b/source4/kdc/hdb-samba4.c index 76bafcee73b8..5f94ea3b55c3 100644 --- a/source4/kdc/hdb-samba4.c +++ b/source4/kdc/hdb-samba4.c @@ -331,7 +331,6 @@ static krb5_error_code hdb_samba4_auth_status(krb5_context context, HDB *db, * logs are consistent, even if some elements are always NULL. */ struct auth_usersupplied_info ui = { - .mapped_state = true, .was_mapped = true, .client = { .account_name = original_client_name, -- 2.25.1 From b71b972d623c74ef0b8b22ae002ef46e08e7206f Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 3 Mar 2022 23:16:13 +0100 Subject: [PATCH 18/26] s4:rpc_server/samr: don't set mapped_state in auth_usersupplied_info for audit logging mapped_state is completely irrelevant for audit logging and will also be removed in the next commits. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13879 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 52787b9c1e9370133ff4481c62c2e7b9393c2439) --- source4/rpc_server/samr/samr_password.c | 1 - 1 file changed, 1 deletion(-) diff --git a/source4/rpc_server/samr/samr_password.c b/source4/rpc_server/samr/samr_password.c index 437e8f662759..858c7b040648 100644 --- a/source4/rpc_server/samr/samr_password.c +++ b/source4/rpc_server/samr/samr_password.c @@ -56,7 +56,6 @@ static void log_password_change_event(struct imessaging_context *msg_ctx, * logs are consistent, even if some elements are always NULL. */ struct auth_usersupplied_info ui = { - .mapped_state = true, .was_mapped = true, .client = { .account_name = original_client_name, -- 2.25.1 From 29279e253018f92c4b9c37d1632dff7efaeb8f66 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 3 Mar 2022 23:14:10 +0100 Subject: [PATCH 19/26] s4:auth: check for user_info->mapped.account_name if it needs to be filled mapped_state is a special hack for authenticate_ldap_simple_bind_send() in order to avoid some additional work in authsam_check_password_internals(). But that code will be changed in the next commits, so we can simplify the logic and only check for user_info->mapped.account_name being NULL. As it's the important factor that user_info->mapped.account_name is non-NULL down in the auth stack. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13879 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit c7b8c71b2b71bb9d95c33d403c4204376f443852) --- source4/auth/ntlm/auth.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/source4/auth/ntlm/auth.c b/source4/auth/ntlm/auth.c index 3dd2ffc92765..09d660a392b1 100644 --- a/source4/auth/ntlm/auth.c +++ b/source4/auth/ntlm/auth.c @@ -220,17 +220,12 @@ _PUBLIC_ struct tevent_req *auth_check_password_send(TALLOC_CTX *mem_ctx, state->user_info = user_info; state->authoritative = 1; - if (!user_info->mapped_state) { + if (user_info->mapped.account_name == NULL) { struct auth_usersupplied_info *user_info_tmp; /* * We don't really do any mapping here. * - * So we don't set user_info->mapped_state, - * but we set mapped.domain_name and - * mapped.account_name to the client - * provided values. - * * It's up to the backends to do mappings * for their authentication. */ -- 2.25.1 From 4e12dea78119cf1e0baaf4a9498b40939fe984a9 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 7 Mar 2022 21:16:51 +0100 Subject: [PATCH 20/26] s4:auth: fix confusing DEBUG message in authsam_want_check() BUG: https://bugzilla.samba.org/show_bug.cgi?id=13879 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit a12683bd1206df4d4d87a3842d92e34a69e172b7) --- source4/auth/ntlm/auth_sam.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source4/auth/ntlm/auth_sam.c b/source4/auth/ntlm/auth_sam.c index ddde4363d926..673f900b0c6b 100644 --- a/source4/auth/ntlm/auth_sam.c +++ b/source4/auth/ntlm/auth_sam.c @@ -871,13 +871,13 @@ static NTSTATUS authsam_want_check(struct auth_method_context *ctx, /* * The caller already did a cracknames call. */ - DBG_DEBUG("%s is not one domain name (DC)\n", + DBG_DEBUG("%s is not own domain name (DC)\n", effective_domain); return NT_STATUS_NOT_IMPLEMENTED; } if (!strequal(effective_domain, "")) { - DBG_DEBUG("%s is not one domain name (DC)\n", + DBG_DEBUG("%s is not own domain name (DC)\n", effective_domain); return NT_STATUS_NOT_IMPLEMENTED; } -- 2.25.1 From fca07e2ce8e815e9faac287474049f23c68ee89e Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 3 Mar 2022 23:24:25 +0100 Subject: [PATCH 21/26] s3:auth: make_user_info_map() should not set mapped_state mapped_state is only evaluated in authsam_check_password_internals() of auth_sam.c in source4, so setting it in the auth3 code doesn't make any difference. I've proved that with an SMB_ASSERT() and a full pipeline not triggering it. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13879 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit c56cb12f347b7582290ce1d4dfe3959d69050bd9) --- source3/auth/auth_util.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/source3/auth/auth_util.c b/source3/auth/auth_util.c index 9d9f2234f210..a0260b67c193 100644 --- a/source3/auth/auth_util.c +++ b/source3/auth/auth_util.c @@ -136,8 +136,6 @@ NTSTATUS make_user_info_map(TALLOC_CTX *mem_ctx, lm_interactive_pwd, nt_interactive_pwd, plaintext, password_state); if (NT_STATUS_IS_OK(result)) { - /* We have tried mapping */ - (*user_info)->mapped_state = true; /* did we actually map the user to a different name? */ (*user_info)->was_mapped = was_mapped; } -- 2.25.1 From 4bbfb694fb9d3cb923e706046528788d592621d9 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 7 Mar 2022 20:57:52 +0100 Subject: [PATCH 22/26] nsswitch: let test_wbinfo.sh also test wbinfo -a $USERNAME@$DOMAIN When winbindd forwards wbinfo -a via netrLogonSamLogon* to a remote DC work fine for upn names, e.g. administrator@DOMAIN. But it currently fails locally on a DC against the local sam. For the RODC only work because it forwards the request to an RWDC. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13879 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15003 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit e1d2c59d360fb4e72dafe788b5d9dbb0572bf811) --- nsswitch/tests/test_wbinfo.sh | 2 ++ selftest/knownfail.d/samba.blackbox.wbinfo | 2 ++ 2 files changed, 4 insertions(+) create mode 100644 selftest/knownfail.d/samba.blackbox.wbinfo diff --git a/nsswitch/tests/test_wbinfo.sh b/nsswitch/tests/test_wbinfo.sh index 2ac83828a0e0..198918494cfc 100755 --- a/nsswitch/tests/test_wbinfo.sh +++ b/nsswitch/tests/test_wbinfo.sh @@ -294,6 +294,8 @@ testit "wbinfo --user-sids against $TARGET" $wbinfo --user-sids $admin_sid || fa testit "wbinfo -a against $TARGET with domain creds" $wbinfo -a "$DOMAIN/$USERNAME"%"$PASSWORD" || failed=`expr $failed + 1` +testit "wbinfo -a against $TARGET with domain upn creds" $wbinfo -a "$USERNAME@$DOMAIN"%"$PASSWORD" || failed=$(expr $failed + 1) + testit "wbinfo --getdcname against $TARGET" $wbinfo --getdcname=$DOMAIN testit "wbinfo -p against $TARGET" $wbinfo -p || failed=`expr $failed + 1` diff --git a/selftest/knownfail.d/samba.blackbox.wbinfo b/selftest/knownfail.d/samba.blackbox.wbinfo new file mode 100644 index 000000000000..fa71377ffde1 --- /dev/null +++ b/selftest/knownfail.d/samba.blackbox.wbinfo @@ -0,0 +1,2 @@ +^samba.blackbox.wbinfo.ad_dc.*.wbinfo.-a.against.*.with.domain.upn.creds +^samba.blackbox.wbinfo.promoted_dc.*.wbinfo.-a.against.*.with.domain.upn.creds -- 2.25.1 From 9f57982eb83781925d70c54207995c3b94b748ad Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 3 Mar 2022 23:23:21 +0100 Subject: [PATCH 23/26] winbindd: don't set mapped_state in winbindd_dual_auth_passdb() mapped_state is a special hack for authenticate_ldap_simple_bind_send() in order to avoid some additional work in authsam_check_password_internals() This doesn't apply here. We should also handle wbinfo -a authentication UPN names, e.g. administrator@DOMAIN, even if the account belongs to the local sam. With this change the behavior is consistent also locally on DCs and also an RODC can handle these requests locally for cached accounts. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13879 BUG: https://bugzilla.samba.org/show_bug.cgi?id=15003 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 8dfdbe095a4c8a7bedd29341656a7c3164517713) --- selftest/knownfail.d/samba.blackbox.wbinfo | 2 -- source3/winbindd/winbindd_pam.c | 3 --- 2 files changed, 5 deletions(-) delete mode 100644 selftest/knownfail.d/samba.blackbox.wbinfo diff --git a/selftest/knownfail.d/samba.blackbox.wbinfo b/selftest/knownfail.d/samba.blackbox.wbinfo deleted file mode 100644 index fa71377ffde1..000000000000 --- a/selftest/knownfail.d/samba.blackbox.wbinfo +++ /dev/null @@ -1,2 +0,0 @@ -^samba.blackbox.wbinfo.ad_dc.*.wbinfo.-a.against.*.with.domain.upn.creds -^samba.blackbox.wbinfo.promoted_dc.*.wbinfo.-a.against.*.with.domain.upn.creds diff --git a/source3/winbindd/winbindd_pam.c b/source3/winbindd/winbindd_pam.c index d50439de4245..4f371bebeee7 100644 --- a/source3/winbindd/winbindd_pam.c +++ b/source3/winbindd/winbindd_pam.c @@ -1430,9 +1430,6 @@ static NTSTATUS winbindd_dual_auth_passdb(TALLOC_CTX *mem_ctx, return NT_STATUS_NO_MEMORY; } - /* We don't want any more mapping of the username */ - user_info->mapped_state = True; - /* We don't want to come back to winbindd or to do PAM account checks */ user_info->flags |= USER_INFO_INFO3_AND_NO_AUTHZ; -- 2.25.1 From 5c62849f640ff5baca32bf32a87432a9d273ef3b Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 3 Mar 2022 11:10:00 +0100 Subject: [PATCH 24/26] s4:auth: rename user_info->mapped_state to user_info->cracknames_called This makes it much clearer what it is used for and it is a special hack for authenticate_ldap_simple_bind_send() in order to avoid some additional work in authsam_check_password_internals(). BUG: https://bugzilla.samba.org/show_bug.cgi?id=13879 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 427125d182252d8aee3dd906ee34a909cdbb8ef3) --- auth/common_auth.h | 2 +- source4/auth/ntlm/auth_sam.c | 4 ++-- source4/auth/ntlm/auth_simple.c | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/auth/common_auth.h b/auth/common_auth.h index 0452c673ebce..9d51ea69719b 100644 --- a/auth/common_auth.h +++ b/auth/common_auth.h @@ -49,7 +49,7 @@ struct auth_usersupplied_info uint32_t logon_parameters; - bool mapped_state; + bool cracknames_called; bool was_mapped; uint64_t logon_id; /* the values the client gives us */ diff --git a/source4/auth/ntlm/auth_sam.c b/source4/auth/ntlm/auth_sam.c index 673f900b0c6b..cf0656ae0da1 100644 --- a/source4/auth/ntlm/auth_sam.c +++ b/source4/auth/ntlm/auth_sam.c @@ -658,7 +658,7 @@ static NTSTATUS authsam_check_password_internals(struct auth_method_context *ctx * really, really want to get back to exactly the same account * we got the DN for. */ - if (user_info->mapped_state == false) { + if (!user_info->cracknames_called) { p = strchr_m(account_name, '@'); } else { /* @@ -867,7 +867,7 @@ static NTSTATUS authsam_want_check(struct auth_method_context *ctx, return NT_STATUS_OK; } - if (user_info->mapped_state) { + if (user_info->cracknames_called) { /* * The caller already did a cracknames call. */ diff --git a/source4/auth/ntlm/auth_simple.c b/source4/auth/ntlm/auth_simple.c index f767adb36960..4f8267e92857 100644 --- a/source4/auth/ntlm/auth_simple.c +++ b/source4/auth/ntlm/auth_simple.c @@ -123,7 +123,7 @@ _PUBLIC_ struct tevent_req *authenticate_ldap_simple_bind_send(TALLOC_CTX *mem_c user_info->mapped.account_name = nt4_username; user_info->mapped.domain_name = nt4_domain; - user_info->mapped_state = true; + user_info->cracknames_called = true; subreq = auth_check_password_send(state, ev, state->auth_context, -- 2.25.1 From 616180002438ff9a8763829af8a2d4ed3f8e9448 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 3 Mar 2022 11:10:00 +0100 Subject: [PATCH 25/26] auth: let auth logging prefer user_info->orig_client.{account,domain}_name if available The optional user_info->orig_client.{account,domain}_name are the once really used by the client and should be used in audit logging. But we still fallback to user_info->client.{account,domain}_name. This will be important for the next commit. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13879 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett (cherry picked from commit 24b580cae23860a0fe6c9d3a285d60564057043d) --- auth/auth_log.c | 20 ++++++++++++++++---- auth/common_auth.h | 2 +- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/auth/auth_log.c b/auth/auth_log.c index 60bc63345918..dc1cea12390c 100644 --- a/auth/auth_log.c +++ b/auth/auth_log.c @@ -152,6 +152,12 @@ static void log_authentication_event_json( char negotiate_flags[11]; char logon_id[19]; int rc = 0; + const char *clientDomain = ui->orig_client.domain_name ? + ui->orig_client.domain_name : + ui->client.domain_name; + const char *clientAccount = ui->orig_client.account_name ? + ui->orig_client.account_name : + ui->client.account_name; authentication = json_new_object(); if (json_is_invalid(&authentication)) { @@ -203,12 +209,12 @@ static void log_authentication_event_json( goto failure; } rc = json_add_string( - &authentication, "clientDomain", ui->client.domain_name); + &authentication, "clientDomain", clientDomain); if (rc != 0) { goto failure; } rc = json_add_string( - &authentication, "clientAccount", ui->client.account_name); + &authentication, "clientAccount", clientAccount); if (rc != 0) { goto failure; } @@ -594,6 +600,12 @@ static void log_authentication_event_human_readable( char *trust_account_name = NULL; char *logon_line = NULL; const char *password_type = NULL; + const char *clientDomain = ui->orig_client.domain_name ? + ui->orig_client.domain_name : + ui->client.domain_name; + const char *clientAccount = ui->orig_client.account_name ? + ui->orig_client.account_name : + ui->client.account_name; frame = talloc_stackframe(); @@ -640,8 +652,8 @@ static void log_authentication_event_human_readable( " %s\n", ui->service_description, ui->auth_description, - log_escape(frame, ui->client.domain_name), - log_escape(frame, ui->client.account_name), + log_escape(frame, clientDomain), + log_escape(frame, clientAccount), ts, password_type, nt_errstr(status), diff --git a/auth/common_auth.h b/auth/common_auth.h index 9d51ea69719b..d922b66ab4dc 100644 --- a/auth/common_auth.h +++ b/auth/common_auth.h @@ -56,7 +56,7 @@ struct auth_usersupplied_info struct { const char *account_name; const char *domain_name; - } client, mapped; + } client, mapped, orig_client; enum auth_password_state password_state; -- 2.25.1 From b480b01c0cc867c967a007a4354700d7555c7e13 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 3 Mar 2022 11:10:00 +0100 Subject: [PATCH 26/26] s4:auth: let authenticate_ldap_simple_bind() pass down the mapped nt4names authenticate_ldap_simple_bind*() needs to pass the result of the cracknames operation into the auth stack as user_info->client.{account,domain}_name, because user_info->client.{account,domain}_name is also used when forwarding the request via netrLogonSamLogon* to a remote server, for exactly that the values are also used in order to map a AUTH_PASSWORD_PLAIN into AUTH_PASSWORD_RESPONSE, where the NTLMv2 response contains the account and domain names passed in the netr_IdentityInfo value. Otherwise it would not be possible to forward the LDAP simple bind authentication request to a remote DC. Currently this only applies to an RODC that forwards the request to an RWDC. But note that LDAP simple binds (as on Windows) only work for users in the DCs forest, as the DsCrackNames need to work and it can't work for users of remote forests. I tested that in a DC of a forest root domain, if rejected the LDAP simple bind against a different forest, but allowed it for a users of a child domain in the same forest. The NTLMSSP bind worked in both cases. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13879 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett Autobuild-User(master): Andrew Bartlett Autobuild-Date(master): Thu Mar 10 04:10:54 UTC 2022 on sn-devel-184 (cherry picked from commit 40f2070d3b2b1b13cc08f7844bfe4945e9f0cd86) --- selftest/knownfail.d/rodc_rwdc | 1 - source4/auth/ntlm/auth_simple.c | 5 +++-- 2 files changed, 3 insertions(+), 3 deletions(-) delete mode 100644 selftest/knownfail.d/rodc_rwdc diff --git a/selftest/knownfail.d/rodc_rwdc b/selftest/knownfail.d/rodc_rwdc deleted file mode 100644 index c148d035f5e2..000000000000 --- a/selftest/knownfail.d/rodc_rwdc +++ /dev/null @@ -1 +0,0 @@ -^samba4.ldap.rodc_rwdc.*test_ldap_change_password_simple_bind diff --git a/source4/auth/ntlm/auth_simple.c b/source4/auth/ntlm/auth_simple.c index 4f8267e92857..006e4d8279ae 100644 --- a/source4/auth/ntlm/auth_simple.c +++ b/source4/auth/ntlm/auth_simple.c @@ -121,8 +121,9 @@ _PUBLIC_ struct tevent_req *authenticate_ldap_simple_bind_send(TALLOC_CTX *mem_c return tevent_req_post(req, ev); } - user_info->mapped.account_name = nt4_username; - user_info->mapped.domain_name = nt4_domain; + user_info->orig_client = user_info->client; + user_info->client.account_name = nt4_username; + user_info->client.domain_name = nt4_domain; user_info->cracknames_called = true; subreq = auth_check_password_send(state, ev, -- 2.25.1