[PATCH] Fix authentication with an empty string domain
Stefan Metzmacher
metze at samba.org
Wed Feb 21 23:53:09 UTC 2018
Hi,
I would like to add more tests, but I don't have the time currently.
I think we should get the attached set into master and 4.8 and 4.7
Please review and push.
metze
Am 09.01.2018 um 20:00 schrieb Andrew Bartlett via samba-technical:
> On Tue, 2018-01-09 at 12:12 +1300, Garming Sam via samba-technical
> wrote:
>> 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
>
> Reviewed-by: Andrew Bartlett <abartlet at samba.org>
>
> Please push when you you have addressed metze's concerns.
>
> Andrew Bartlett
>
-------------- next part --------------
From f72c49fa0c1e2f3795fe09ba7b9c40f46e0403be Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Tue, 9 Jan 2018 08:55:48 +0100
Subject: [PATCH 1/5] s3:libsmb: allow -U"\\administrator" to work
cli_credentials_get_principal() returns NULL in that case.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13206
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
source3/libsmb/cliconnect.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/source3/libsmb/cliconnect.c b/source3/libsmb/cliconnect.c
index 26bf569..f5bf68f 100644
--- a/source3/libsmb/cliconnect.c
+++ b/source3/libsmb/cliconnect.c
@@ -283,8 +283,9 @@ NTSTATUS cli_session_creds_prepare_krb5(struct cli_state *cli,
auth_requested = cli_credentials_authentication_requested(creds);
if (auth_requested) {
+ errno = 0;
user_principal = cli_credentials_get_principal(creds, frame);
- if (user_principal == NULL) {
+ if (errno != 0) {
TALLOC_FREE(frame);
return NT_STATUS_NO_MEMORY;
}
@@ -299,6 +300,10 @@ NTSTATUS cli_session_creds_prepare_krb5(struct cli_state *cli,
try_kerberos = true;
}
+ if (user_principal == NULL) {
+ try_kerberos = false;
+ }
+
if (target_hostname == NULL) {
try_kerberos = false;
} else if (is_ipaddress(target_hostname)) {
--
1.9.1
From 138927061d36b9c29e85f374f818f964c54e92e9 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Tue, 9 Jan 2018 08:57:05 +0100
Subject: [PATCH 2/5] s3:cliconnect.c: remove useless ';'
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13206
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
source3/libsmb/cliconnect.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/source3/libsmb/cliconnect.c b/source3/libsmb/cliconnect.c
index f5bf68f..7689910 100644
--- a/source3/libsmb/cliconnect.c
+++ b/source3/libsmb/cliconnect.c
@@ -1289,7 +1289,7 @@ static struct tevent_req *cli_session_setup_spnego_send(
status = cli_session_creds_prepare_krb5(cli, creds);
if (tevent_req_nterror(req, status)) {
- return tevent_req_post(req, ev);;
+ return tevent_req_post(req, ev);
}
subreq = cli_session_setup_gensec_send(state, ev, cli, creds,
--
1.9.1
From ee38bbf2092ecbdc395457133de9bf0ea70b7278 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 3/5] 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>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
---
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 d727903de7ab7ad4a72dd9c3f023d21dde417dc7 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 4/5] 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>
Reviewed-by: Andrew Bartlett <abartlet at samba.org>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
---
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 8bc5b23dc1640fad3290463244fe9bd46026cfd5 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Tue, 9 Jan 2018 08:54:11 +0100
Subject: [PATCH 5/5] s4:auth_sam: allow logons with an empty domain name
It turns out that an empty domain name maps to the local SAM.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13206
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
selftest/knownfail.d/empty-domain-bind | 1 -
selftest/knownfail.d/empty-domain-samlogon | 1 -
source4/auth/ntlm/auth_sam.c | 16 ++++++++++------
3 files changed, 10 insertions(+), 8 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..8c5ebd7 100644
--- a/source4/auth/ntlm/auth_sam.c
+++ b/source4/auth/ntlm/auth_sam.c
@@ -739,6 +739,10 @@ static NTSTATUS authsam_want_check(struct auth_method_context *ctx,
return NT_STATUS_NOT_IMPLEMENTED;
}
+ if (effective_domain == NULL) {
+ effective_domain = "";
+ }
+
is_local_name = lpcfg_is_myname(ctx->auth_ctx->lp_ctx,
effective_domain);
@@ -784,7 +788,7 @@ static NTSTATUS authsam_want_check(struct auth_method_context *ctx,
return NT_STATUS_NOT_IMPLEMENTED;
}
- if (effective_domain != NULL && !strequal(effective_domain, "")) {
+ if (!strequal(effective_domain, "")) {
DBG_DEBUG("%s is not one domain name (DC)\n",
effective_domain);
return NT_STATUS_NOT_IMPLEMENTED;
@@ -792,11 +796,11 @@ 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) {
- return NT_STATUS_OK;
- }
- DEBUG(6,("authsam_check_password: '' without upn not handled (DC)\n"));
- return NT_STATUS_NOT_IMPLEMENTED;
+ /*
+ * An empty to domain name should be handled
+ * as the local domain name.
+ */
+ return NT_STATUS_OK;
}
effective_domain = p + 1;
--
1.9.1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20180222/d4be6b9a/signature.sig>
More information about the samba-technical
mailing list