From bf9e7fdff870c08d06a2d850b34e4a11baeaf55e 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 6d4993ac255e..3bb7995052cd 100644 --- a/python/samba/tests/__init__.py +++ b/python/samba/tests/__init__.py @@ -171,6 +171,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() @@ -184,6 +186,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 67b28ca3f265cc31a935444120640c92318a028b 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 27107f3d130b..98afcb39d1f0 100755 --- a/source4/dsdb/tests/python/passwords.py +++ b/source4/dsdb/tests/python/passwords.py @@ -35,7 +35,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 1146698ca4c0b20eb2bfd0de50917337e3194bc7 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 09f8ea36ec85..4ca142ba376c 100755 --- a/source4/dsdb/tests/python/login_basics.py +++ b/source4/dsdb/tests/python/login_basics.py @@ -47,7 +47,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, @@ -179,6 +180,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 959c64898574..9aa23afd774c 100755 --- a/source4/dsdb/tests/python/password_lockout.py +++ b/source4/dsdb/tests/python/password_lockout.py @@ -68,7 +68,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), @@ -139,7 +140,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) @@ -1421,6 +1422,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 beea26e8e1ae..2ee803a8f6f5 100644 --- a/source4/dsdb/tests/python/rodc_rwdc.py +++ b/source4/dsdb/tests/python/rodc_rwdc.py @@ -204,11 +204,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) @@ -743,12 +745,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 09a0c332f8c8dff142f50753e43b44ba5fdf6d01 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 ad2a2ad16027..b44dd30162b3 100644 --- a/source4/dsdb/tests/python/password_lockout_base.py +++ b/source4/dsdb/tests/python/password_lockout_base.py @@ -247,12 +247,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 246ee60386f4d37d77e629b80ae8318d94886d9b 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 4ca142ba376c..8cbd046468e8 100755 --- a/source4/dsdb/tests/python/login_basics.py +++ b/source4/dsdb/tests/python/login_basics.py @@ -101,7 +101,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, @@ -147,8 +147,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, @@ -162,7 +161,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 b44dd30162b3..d11c64399131 100644 --- a/source4/dsdb/tests/python/password_lockout_base.py +++ b/source4/dsdb/tests/python/password_lockout_base.py @@ -250,7 +250,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 @@ -265,6 +265,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 4f953e937656917cd543e2b73db1d62d28d7e064 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 d11c64399131..5b872980b154 100644 --- a/source4/dsdb/tests/python/password_lockout_base.py +++ b/source4/dsdb/tests/python/password_lockout_base.py @@ -5,6 +5,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 @@ -212,11 +213,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, @@ -247,10 +254,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 @@ -361,10 +368,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 2f743d181e2aec5d4a19fcf2f2fa1534c2ddc7b4 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 8cbd046468e8..c03fef932e0b 100755 --- a/source4/dsdb/tests/python/login_basics.py +++ b/source4/dsdb/tests/python/login_basics.py @@ -55,17 +55,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 @@ -87,7 +94,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), @@ -101,7 +108,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, @@ -132,7 +139,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, @@ -147,8 +154,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, @@ -161,7 +171,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, @@ -178,5 +188,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 6854be4ffd6e374be23c6d172725afb82c511c46 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 28850cd85201..d7b3b032572e 100644 --- a/source3/auth/auth_util.c +++ b/source3/auth/auth_util.c @@ -265,6 +265,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 4010868946fa81c4758deb5c6591b54d41d86c63 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 fc5f1053a785131a41e91700746dc5f7ed075e1c 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 2ee803a8f6f5..53d54807a3de 100644 --- a/source4/dsdb/tests/python/rodc_rwdc.py +++ b/source4/dsdb/tests/python/rodc_rwdc.py @@ -43,7 +43,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) @@ -52,6 +52,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) @@ -1023,10 +1026,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: @@ -1045,19 +1052,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:]): @@ -1066,40 +1077,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) @@ -1109,6 +1120,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 462ac34cb7406928cd27538c846854c4fdace95b 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 049186e5a515..50dae9d7f3eb 100644 --- a/source3/rpc_client/cli_netlogon.c +++ b/source3/rpc_client/cli_netlogon.c @@ -687,6 +687,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 6b5225efe3e7cbe84c8a362b637c13d47ed2da68 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 7fe9f260bb184e10030fe23d2b705cdfd04b8203 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 491538f7bf077238ba0785c45b63f3f45823b107 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 8137980750847a84985431dc1969f4afb201b195 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 5c2d8187255a5e3228b1be054638a8093d0e8fa2 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 348696a895c2..3978d82a79a2 100644 --- a/source4/dsdb/samdb/ldb_modules/password_hash.c +++ b/source4/dsdb/samdb/ldb_modules/password_hash.c @@ -3028,7 +3028,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 b28b156fc87d0dd83c0e9e1bf7d71bb2250e1506 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 04edecf367fbb020264453e54f07b94e70c8ab49 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 2ee0f0552ec3ccec833b8e74c9066b873778b978 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 a66a7f86822f0e02f5b5552930f67156160c38b7 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 5414efb084287d42b53618b66a26c072970f4fc5 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 d7b3b032572e..b60dd2647c86 100644 --- a/source3/auth/auth_util.c +++ b/source3/auth/auth_util.c @@ -137,8 +137,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 8f843bb85d520d13be6d69f540e64522cd7048a1 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 5a90b735777b47d9310d2442104bb4d0241ad4a5 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 a24cef784405..1a2628b50ba5 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 f0b26df42f26c30f5c6642a1cf060dbec8eb8ab9 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 5b0c21a5b15a4fb2bd9a576a106ef29314bbb240 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 d0741d3435ec302198d038b6753f998ac68e4e10 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