[PATCH] Fix authentication with an empty string domain

Garming Sam garming at catalyst.net.nz
Mon Jan 8 23:12:48 UTC 2018


Hi,

I noticed a a little while back (as it appeared to be required for the
auto-configure tool in the Windows Protocol test-suites) that we no
longer allow '' as our domain, which used to be allowed in Samba 4.6.
Attached is a fix which solved the immediate issue, and appears to
restore the 4.6 behaviour. Both tests I wrote, one using SamLogonEx
against the NETLOGON server and another binding against LDAP failed on
master, but pass on Windows 2012 R2 and with this patch. I also ran the
NETLOGON test against 4.6 to confirm that this indeed did not used to fail.


A bit more discussion on the bug:

https://bugzilla.samba.org/show_bug.cgi?id=13206


Cheers,

Garming

-------------- next part --------------
From 978ea577d258d26b631c5cb7dbf658958e14f276 Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Mon, 8 Jan 2018 13:36:59 +1300
Subject: [PATCH 1/3] tests/py_creds: Add a SamLogonEx test with an empty
 string domain

This test passes against 4.6, but failed against 4.7.5 and master.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13206

Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
 python/samba/tests/py_credentials.py       | 27 +++++++++++++++++++++++++++
 selftest/knownfail.d/empty-domain-samlogon |  1 +
 2 files changed, 28 insertions(+)
 create mode 100644 selftest/knownfail.d/empty-domain-samlogon

diff --git a/python/samba/tests/py_credentials.py b/python/samba/tests/py_credentials.py
index ff017ec..2f5a7d6 100644
--- a/python/samba/tests/py_credentials.py
+++ b/python/samba/tests/py_credentials.py
@@ -129,6 +129,33 @@ class PyCredentialsTests(TestCase):
             else:
                 raise
 
+    def test_SamLogonEx_no_domain(self):
+        c = self.get_netlogon_connection()
+
+        self.user_creds.set_domain('')
+
+        logon = samlogon_logon_info(self.domain,
+                                    self.machine_name,
+                                    self.user_creds)
+
+        logon_level = netlogon.NetlogonNetworkTransitiveInformation
+        validation_level = netlogon.NetlogonValidationSamInfo4
+        netr_flags = 0
+
+        try:
+            c.netr_LogonSamLogonEx(self.server,
+                                   self.user_creds.get_workstation(),
+                                   logon_level,
+                                   logon,
+                                   validation_level,
+                                   netr_flags)
+        except NTSTATUSError as e:
+            enum = ctypes.c_uint32(e[0]).value
+            if enum == ntstatus.NT_STATUS_WRONG_PASSWORD:
+                self.fail("got wrong password error")
+            else:
+                self.fail("got unexpected error" + str(e))
+
     def test_SamLogonExNTLM(self):
         c = self.get_netlogon_connection()
 
diff --git a/selftest/knownfail.d/empty-domain-samlogon b/selftest/knownfail.d/empty-domain-samlogon
new file mode 100644
index 0000000..925a03a
--- /dev/null
+++ b/selftest/knownfail.d/empty-domain-samlogon
@@ -0,0 +1 @@
+^samba.tests.py_credentials.samba.tests.py_credentials.PyCredentialsTests.test_SamLogonEx_no_domain
-- 
1.9.1


From 39789ef62c67eb40514f6799a8b1c7c891b3042b Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Mon, 8 Jan 2018 16:34:02 +1300
Subject: [PATCH 2/3] tests/bind.py: Add a bind test with NTLMSSP with no
 domain

Confirmed to pass against Windows 2012 R2.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13206

Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
 auth/credentials/tests/bind.py         | 26 +++++++++++++++++++++++++-
 selftest/knownfail.d/empty-domain-bind |  1 +
 2 files changed, 26 insertions(+), 1 deletion(-)
 create mode 100644 selftest/knownfail.d/empty-domain-bind

diff --git a/auth/credentials/tests/bind.py b/auth/credentials/tests/bind.py
index 91e493d..4aa4498 100755
--- a/auth/credentials/tests/bind.py
+++ b/auth/credentials/tests/bind.py
@@ -43,6 +43,7 @@ creds_machine = copy.deepcopy(creds)
 creds_user1 = copy.deepcopy(creds)
 creds_user2 = copy.deepcopy(creds)
 creds_user3 = copy.deepcopy(creds)
+creds_user4 = copy.deepcopy(creds)
 
 class BindTests(samba.tests.TestCase):
 
@@ -64,7 +65,7 @@ class BindTests(samba.tests.TestCase):
         self.config_dn = self.info_dc["configurationNamingContext"][0]
         self.computer_dn = "CN=centos53,CN=Computers,%s" % self.domain_dn
         self.password = "P at ssw0rd"
-        self.username = "BindTestUser_" + time.strftime("%s", time.gmtime())
+        self.username = "BindTestUser"
 
     def tearDown(self):
         super(BindTests, self).tearDown()
@@ -113,6 +114,7 @@ unicodePwd:: """ + base64.b64encode("\"P at ssw0rd\"".encode('utf-16-le')) + """
                                       expression="(samAccountName=%s)" % self.username)
         self.assertEquals(len(ldb_res), 1)
         user_dn = ldb_res[0]["dn"]
+        self.addCleanup(delete_force, self.ldb, user_dn)
 
         # do a simple bind and search with the user account in format user at realm
         creds_user1.set_bind_dn(self.username + "@" + creds.get_realm())
@@ -138,5 +140,27 @@ unicodePwd:: """ + base64.b64encode("\"P at ssw0rd\"".encode('utf-16-le')) + """
                                               lp=lp, ldap_only=True)
         res = ldb_user3.search(base="", expression="", scope=SCOPE_BASE, attrs=["*"])
 
+    def test_user_account_bind_no_domain(self):
+        # create user
+        self.ldb.newuser(username=self.username, password=self.password)
+        ldb_res = self.ldb.search(base=self.domain_dn,
+                                      scope=SCOPE_SUBTREE,
+                                      expression="(samAccountName=%s)" % self.username)
+        self.assertEquals(len(ldb_res), 1)
+        user_dn = ldb_res[0]["dn"]
+        self.addCleanup(delete_force, self.ldb, user_dn)
+
+        creds_user4.set_username(self.username)
+        creds_user4.set_password(self.password)
+        creds_user4.set_domain('')
+        creds_user4.set_workstation('')
+        print "BindTest (no domain) with: " + self.username
+        try:
+            ldb_user4 = samba.tests.connect_samdb(host, credentials=creds_user4,
+                                              lp=lp, ldap_only=True)
+        except:
+            self.fail("Failed to connect without the domain set")
+
+        res = ldb_user4.search(base="", expression="", scope=SCOPE_BASE, attrs=["*"])
 
 TestProgram(module=__name__, opts=subunitopts)
diff --git a/selftest/knownfail.d/empty-domain-bind b/selftest/knownfail.d/empty-domain-bind
new file mode 100644
index 0000000..99d71c1
--- /dev/null
+++ b/selftest/knownfail.d/empty-domain-bind
@@ -0,0 +1 @@
+^samba4.ldap.bind\(fl2008r2dc\).__main__.BindTests.test_user_account_bind_no_domain.*
-- 
1.9.1


From c9afec5ec70ec79d285304558c4cf8b094a42685 Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Mon, 18 Dec 2017 00:58:08 +0000
Subject: [PATCH 3/3] auth_sam: Allow domain '' when logging in

Removes the knownfail entries for the tests.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13206

Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
 selftest/knownfail.d/empty-domain-bind     | 1 -
 selftest/knownfail.d/empty-domain-samlogon | 1 -
 source4/auth/ntlm/auth_sam.c               | 2 +-
 3 files changed, 1 insertion(+), 3 deletions(-)
 delete mode 100644 selftest/knownfail.d/empty-domain-bind
 delete mode 100644 selftest/knownfail.d/empty-domain-samlogon

diff --git a/selftest/knownfail.d/empty-domain-bind b/selftest/knownfail.d/empty-domain-bind
deleted file mode 100644
index 99d71c1..0000000
--- a/selftest/knownfail.d/empty-domain-bind
+++ /dev/null
@@ -1 +0,0 @@
-^samba4.ldap.bind\(fl2008r2dc\).__main__.BindTests.test_user_account_bind_no_domain.*
diff --git a/selftest/knownfail.d/empty-domain-samlogon b/selftest/knownfail.d/empty-domain-samlogon
deleted file mode 100644
index 925a03a..0000000
--- a/selftest/knownfail.d/empty-domain-samlogon
+++ /dev/null
@@ -1 +0,0 @@
-^samba.tests.py_credentials.samba.tests.py_credentials.PyCredentialsTests.test_SamLogonEx_no_domain
diff --git a/source4/auth/ntlm/auth_sam.c b/source4/auth/ntlm/auth_sam.c
index 5e2a584..1cb40de 100644
--- a/source4/auth/ntlm/auth_sam.c
+++ b/source4/auth/ntlm/auth_sam.c
@@ -792,7 +792,7 @@ static NTSTATUS authsam_want_check(struct auth_method_context *ctx,
 
 	p = strchr_m(user_info->mapped.account_name, '@');
 	if (p == NULL) {
-		if (effective_domain == NULL) {
+		if (effective_domain == NULL || strequal(effective_domain, "")) {
 			return NT_STATUS_OK;
 		}
 		DEBUG(6,("authsam_check_password: '' without upn not handled (DC)\n"));
-- 
1.9.1



More information about the samba-technical mailing list