From a1a56cafb5fcccfed686584a57162c3444047ec5 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 38ae1462a80d27c1acc62ba7c6a7e84b4228ea11 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 7f99c80f99c3073cc548d793a66ed2534111635b 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 91cc3c928f79..d6cac6aa0585 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 43905c8a47dbdc7c257729a138b5126155ec1118 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 43461c4713f73b7895cc9b5101d9ca2f81a5fd99 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 d6cac6aa0585..bf2e484dd9a9 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 5a9e151293289841211e0a3f4da34a49e53d995d 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 a37d9e0e3503ef4d187a7e9506f436b8ba971613 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 bf2e484dd9a9..b186e723f399 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 73f7ea3a27683a393b22953091a70dc3ac158947 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 8cf89c99c168708402070f20e29e1e010b6ca031 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 53467df8585c5665e882f26403ea5257b3bbc978 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 99e89dd547254aa5bdd3c0ef8542c31a68204fb0 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 483f7f301bf001becff0c1f188f2753010d96f35 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 b1bb18b9cf4abcb597e55ab3d4de8d5043e9382b 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 819d549dd8da8cdef0d2c91640eba5ad81354b5f 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 acf89f38c4ba4a508d88525d250908cce6fb24ff 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 cfcf88315c561d66ddd5d02af9493f43d40beab5 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 7238e004152a97bcf7764e6c1d122d3e5effaa4e 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 3f573f297f89..b10cc37e6082 100644 --- a/source4/kdc/hdb-samba4.c +++ b/source4/kdc/hdb-samba4.c @@ -527,7 +527,6 @@ static krb5_error_code hdb_samba4_audit(krb5_context context, * logs are consistent, even if some elements are always NULL. */ ui = (struct auth_usersupplied_info) { - .mapped_state = true, .was_mapped = true, .client = { .account_name = r->cname, -- 2.25.1 From 8eeca012a34794ad0d19b7bbaa3852708eae7d6c 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 b490723c08bb..c0fbaacd4d8d 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 d0a0874f30fa4acef25f11c335b1cd28cd1a5c74 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 f881d17e5e435d6af6e05d68f3d1ed4a2e378094 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 cdc6a1f48cae0de7235aebdfc7fb809ca46d8dc5 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 e68e5b6e8f8111f95d2343a0c3cb991d4b69f6a2 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 dea4c4652e2398aff0323d68e311e3057b4084cb 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 ca89d48cb491..c2fcc399ab8b 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 cb5b619ebcce64cecebbf342894529c2c94231b6 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 84529ab4f9ffebe6e12629b2d8eeacf30b677f89 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 339255ece7fba789b8e2053ebf2d97e6809a6c85 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