From 10a3b240d11024abe14cb73a0a799d184c1fa8e4 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 23 Jan 2020 15:48:39 +0100 Subject: [PATCH 1/6] s3:auth_sam: replace confusing FALL_THROUGH; with break; There's no real logic change here, but is makes it easier to understand. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14247 Signed-off-by: Stefan Metzmacher Reviewed-by: Andreas Schneider (cherry picked from commit 85b168c6dac88f5065c0ec6e925937439f2c12ed) --- source3/auth/auth_sam.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source3/auth/auth_sam.c b/source3/auth/auth_sam.c index 9310ef0e1f05..bf93f19aa774 100644 --- a/source3/auth/auth_sam.c +++ b/source3/auth/auth_sam.c @@ -92,7 +92,7 @@ static NTSTATUS auth_samstrict_auth(const struct auth_context *auth_context, return NT_STATUS_NOT_IMPLEMENTED; } - FALL_THROUGH; + break; case ROLE_DOMAIN_PDC: case ROLE_DOMAIN_BDC: if ( !is_local_name && !is_my_domain ) { @@ -101,7 +101,7 @@ static NTSTATUS auth_samstrict_auth(const struct auth_context *auth_context, return NT_STATUS_NOT_IMPLEMENTED; } - FALL_THROUGH; + break; default: /* name is ok */ break; } -- 2.17.1 From c2a95edf75ee28b22c7e1ac98e0b96bc7381ecee Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 23 Jan 2020 16:13:59 +0100 Subject: [PATCH 2/6] s3:auth_sam: unify the debug messages of all auth_sam*_auth() functions BUG: https://bugzilla.samba.org/show_bug.cgi?id=14247 Signed-off-by: Stefan Metzmacher Reviewed-by: Andreas Schneider (cherry picked from commit 72ef8d3a52c1ab07c079a4c014ba8ac7bff528f7) --- source3/auth/auth_sam.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/source3/auth/auth_sam.c b/source3/auth/auth_sam.c index bf93f19aa774..91cc85544be6 100644 --- a/source3/auth/auth_sam.c +++ b/source3/auth/auth_sam.c @@ -35,6 +35,11 @@ static NTSTATUS auth_sam_ignoredomain_auth(const struct auth_context *auth_conte if (!user_info || !auth_context) { return NT_STATUS_UNSUCCESSFUL; } + + DBG_DEBUG("Check auth for: [%s]\\[%s]\n", + user_info->mapped.domain_name, + user_info->mapped.account_name); + return check_sam_security(&auth_context->challenge, mem_ctx, user_info, server_info); } @@ -75,7 +80,9 @@ static NTSTATUS auth_samstrict_auth(const struct auth_context *auth_context, return NT_STATUS_LOGON_FAILURE; } - DEBUG(10, ("Check auth for: [%s]\n", user_info->mapped.account_name)); + DBG_DEBUG("Check auth for: [%s]\\[%s]\n", + user_info->mapped.domain_name, + user_info->mapped.account_name); is_local_name = is_myname(user_info->mapped.domain_name); is_my_domain = strequal(user_info->mapped.domain_name, lp_workgroup()); -- 2.17.1 From f3fd84f0d42b17941b87c6195532c25cbe7c85e2 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 23 Jan 2020 16:17:30 +0100 Subject: [PATCH 3/6] s3:auth_sam: make sure we never handle empty usernames BUG: https://bugzilla.samba.org/show_bug.cgi?id=14247 Signed-off-by: Stefan Metzmacher Reviewed-by: Andreas Schneider (cherry picked from commit 01b8374e7942141e7f6cbdec7623c981a008e4c1) --- source3/auth/auth_sam.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/source3/auth/auth_sam.c b/source3/auth/auth_sam.c index 91cc85544be6..da38bf4caed8 100644 --- a/source3/auth/auth_sam.c +++ b/source3/auth/auth_sam.c @@ -36,6 +36,12 @@ static NTSTATUS auth_sam_ignoredomain_auth(const struct auth_context *auth_conte return NT_STATUS_UNSUCCESSFUL; } + if (user_info->mapped.account_name == NULL || + user_info->mapped.account_name[0] == '\0') + { + return NT_STATUS_NOT_IMPLEMENTED; + } + DBG_DEBUG("Check auth for: [%s]\\[%s]\n", user_info->mapped.domain_name, user_info->mapped.account_name); @@ -80,6 +86,12 @@ static NTSTATUS auth_samstrict_auth(const struct auth_context *auth_context, return NT_STATUS_LOGON_FAILURE; } + if (user_info->mapped.account_name == NULL || + user_info->mapped.account_name[0] == '\0') + { + return NT_STATUS_NOT_IMPLEMENTED; + } + DBG_DEBUG("Check auth for: [%s]\\[%s]\n", user_info->mapped.domain_name, user_info->mapped.account_name); @@ -154,6 +166,12 @@ static NTSTATUS auth_sam_netlogon3_auth(const struct auth_context *auth_context, return NT_STATUS_LOGON_FAILURE; } + if (user_info->mapped.account_name == NULL || + user_info->mapped.account_name[0] == '\0') + { + return NT_STATUS_NOT_IMPLEMENTED; + } + DBG_DEBUG("Check auth for: [%s]\\[%s]\n", user_info->mapped.domain_name, user_info->mapped.account_name); -- 2.17.1 From 6cc637214f7c0c2dc1540d5a1f65ea8f8187c111 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 23 Jan 2020 16:21:43 +0100 Subject: [PATCH 4/6] s3:auth_sam: introduce effective_domain helper variables BUG: https://bugzilla.samba.org/show_bug.cgi?id=14247 Signed-off-by: Stefan Metzmacher Reviewed-by: Andreas Schneider (cherry picked from commit a63e2a312c761093fedb09bd234b6736485a930a) --- source3/auth/auth_sam.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/source3/auth/auth_sam.c b/source3/auth/auth_sam.c index da38bf4caed8..fa40a5c8f3eb 100644 --- a/source3/auth/auth_sam.c +++ b/source3/auth/auth_sam.c @@ -80,6 +80,7 @@ static NTSTATUS auth_samstrict_auth(const struct auth_context *auth_context, const struct auth_usersupplied_info *user_info, struct auth_serversupplied_info **server_info) { + const char *effective_domain = user_info->mapped.domain_name; bool is_local_name, is_my_domain; if (!user_info || !auth_context) { @@ -93,11 +94,11 @@ static NTSTATUS auth_samstrict_auth(const struct auth_context *auth_context, } DBG_DEBUG("Check auth for: [%s]\\[%s]\n", - user_info->mapped.domain_name, + effective_domain, user_info->mapped.account_name); - is_local_name = is_myname(user_info->mapped.domain_name); - is_my_domain = strequal(user_info->mapped.domain_name, lp_workgroup()); + is_local_name = is_myname(effective_domain); + is_my_domain = strequal(effective_domain, lp_workgroup()); /* check whether or not we service this domain/workgroup name */ @@ -106,7 +107,7 @@ static NTSTATUS auth_samstrict_auth(const struct auth_context *auth_context, case ROLE_DOMAIN_MEMBER: if ( !is_local_name ) { DEBUG(6,("check_samstrict_security: %s is not one of my local names (%s)\n", - user_info->mapped.domain_name, (lp_server_role() == ROLE_DOMAIN_MEMBER + effective_domain, (lp_server_role() == ROLE_DOMAIN_MEMBER ? "ROLE_DOMAIN_MEMBER" : "ROLE_STANDALONE") )); return NT_STATUS_NOT_IMPLEMENTED; } @@ -116,7 +117,7 @@ static NTSTATUS auth_samstrict_auth(const struct auth_context *auth_context, case ROLE_DOMAIN_BDC: if ( !is_local_name && !is_my_domain ) { DEBUG(6,("check_samstrict_security: %s is not one of my local names or domain name (DC)\n", - user_info->mapped.domain_name)); + effective_domain)); return NT_STATUS_NOT_IMPLEMENTED; } @@ -160,6 +161,7 @@ static NTSTATUS auth_sam_netlogon3_auth(const struct auth_context *auth_context, const struct auth_usersupplied_info *user_info, struct auth_serversupplied_info **server_info) { + const char *effective_domain = user_info->mapped.domain_name; bool is_my_domain; if (!user_info || !auth_context) { @@ -173,7 +175,7 @@ static NTSTATUS auth_sam_netlogon3_auth(const struct auth_context *auth_context, } DBG_DEBUG("Check auth for: [%s]\\[%s]\n", - user_info->mapped.domain_name, + effective_domain, user_info->mapped.account_name); /* check whether or not we service this domain/workgroup name */ @@ -190,7 +192,7 @@ static NTSTATUS auth_sam_netlogon3_auth(const struct auth_context *auth_context, is_my_domain = strequal(user_info->mapped.domain_name, lp_workgroup()); if (!is_my_domain) { DBG_INFO("%s is not our domain name (DC for %s)\n", - user_info->mapped.domain_name, lp_workgroup()); + effective_domain, lp_workgroup()); return NT_STATUS_NOT_IMPLEMENTED; } -- 2.17.1 From 4d404b407395af07cb5a7cceb21b9f770544699a Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 4 Feb 2020 11:32:05 +0100 Subject: [PATCH 5/6] s3:selftest: test authentication with an empty userdomain and upn names BUG: https://bugzilla.samba.org/show_bug.cgi?id=14247 Signed-off-by: Stefan Metzmacher Reviewed-by: Andreas Schneider (cherry picked from commit a9eeea6ef78cc44c8423c7125fa1376921060018) --- selftest/knownfail.d/empty-domain-name | 7 +++++++ source3/selftest/tests.py | 8 ++++++++ 2 files changed, 15 insertions(+) create mode 100644 selftest/knownfail.d/empty-domain-name diff --git a/selftest/knownfail.d/empty-domain-name b/selftest/knownfail.d/empty-domain-name new file mode 100644 index 000000000000..9855e4d58c28 --- /dev/null +++ b/selftest/knownfail.d/empty-domain-name @@ -0,0 +1,7 @@ +^samba3.blackbox.smbclient_auth.empty_domain.domain_creds.smbclient.*as.user.*nt4_member +^samba3.blackbox.smbclient_auth.empty_domain.member_creds.smbclient.*as.user.*ad_member +^samba3.blackbox.smbclient_auth.dot_domain.domain_creds.smbclient.*as.user.*nt4_member +^samba3.blackbox.smbclient_auth.dot_domain.domain_creds.smbclient.*as.user.*ad_member +^samba3.blackbox.smbclient_auth.upn.domain_creds.smbclient.*as.*user.*nt4_member +^samba3.blackbox.smbclient_auth.upn.member_creds.smbclient.*as.*user.*nt4_member +^samba3.blackbox.smbclient_auth.upn.member_creds.smbclient.*as.*user.*ad_member diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py index ca427cc0c7c2..b67512489966 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -258,6 +258,14 @@ for options in ["--option=clientntlmv2auth=no", "--option=clientusespnego=no --o plantestsuite("samba3.blackbox.smbclient_auth.plain.%s" % (options), env, [os.path.join(samba3srcdir, "script/tests/test_smbclient_auth.sh"), '$SERVER', '$SERVER_IP', '$DC_USERNAME', '$DC_PASSWORD', smbclient3, configuration, options]) plantestsuite("samba3.blackbox.smbclient_auth.plain.%s.member_creds" % (options), env, [os.path.join(samba3srcdir, "script/tests/test_smbclient_auth.sh"), '$SERVER', '$SERVER_IP', '$SERVER/$USERNAME', '$PASSWORD', smbclient3, configuration, options]) +for env in ["nt4_member", "ad_member"]: + plantestsuite("samba3.blackbox.smbclient_auth.empty_domain.domain_creds", env, [os.path.join(samba3srcdir, "script/tests/test_smbclient_auth.sh"), '$SERVER', '$SERVER_IP', '/$DC_USERNAME', '$DC_PASSWORD', smbclient3, configuration, options]) + plantestsuite("samba3.blackbox.smbclient_auth.empty_domain.member_creds", env, [os.path.join(samba3srcdir, "script/tests/test_smbclient_auth.sh"), '$SERVER', '$SERVER_IP', '/$USERNAME', '$PASSWORD', smbclient3, configuration, options]) + plantestsuite("samba3.blackbox.smbclient_auth.dot_domain.domain_creds", env, [os.path.join(samba3srcdir, "script/tests/test_smbclient_auth.sh"), '$SERVER', '$SERVER_IP', './$DC_USERNAME', '$DC_PASSWORD', smbclient3, configuration, options]) + plantestsuite("samba3.blackbox.smbclient_auth.dot_domain.member_creds", env, [os.path.join(samba3srcdir, "script/tests/test_smbclient_auth.sh"), '$SERVER', '$SERVER_IP', './$USERNAME', '$PASSWORD', smbclient3, configuration, options]) + plantestsuite("samba3.blackbox.smbclient_auth.upn.domain_creds", env, [os.path.join(samba3srcdir, "script/tests/test_smbclient_auth.sh"), '$SERVER', '$SERVER_IP', '$DC_USERNAME@$REALM', '$DC_PASSWORD', smbclient3, configuration, options]) + plantestsuite("samba3.blackbox.smbclient_auth.upn.member_creds", env, [os.path.join(samba3srcdir, "script/tests/test_smbclient_auth.sh"), '$SERVER', '$SERVER_IP', '$USERNAME@$SERVER', '$PASSWORD', smbclient3, configuration, options]) + env = "ad_dc" plantestsuite("samba3.blackbox.smbspool", env, [os.path.join(samba3srcdir, "script/tests/test_smbspool.sh"), '$SERVER', '$SERVER_IP', '$DC_USERNAME', '$DC_PASSWORD', env]) -- 2.17.1 From 3637e34a7740152585881bdfc20a4d63f9bf9027 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 23 Jan 2020 16:21:43 +0100 Subject: [PATCH 6/6] s3:auth_sam: map an empty domain or '.' to the local SAM name When a domain member gets an empty domain name or '.', it should not forward the authentication to domain controllers of the primary domain. But we need to keep passing UPN account names with an empty domain to the DCs as a domain member. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14247 Signed-off-by: Stefan Metzmacher Reviewed-by: Andreas Schneider (cherry picked from commit 590df382bea44eec2dbfd2a28c659b0a29188bca) --- python/samba/tests/auth_log_winbind.py | 4 +-- selftest/knownfail.d/empty-domain-name | 2 +- source3/auth/auth_sam.c | 38 ++++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 3 deletions(-) diff --git a/python/samba/tests/auth_log_winbind.py b/python/samba/tests/auth_log_winbind.py index 4f4beff1e32f..4c68187aef85 100644 --- a/python/samba/tests/auth_log_winbind.py +++ b/python/samba/tests/auth_log_winbind.py @@ -321,7 +321,7 @@ class AuthLogTestsWinbind(AuthLogTestBase, BlackboxTestCase): self.assertEquals("unix:", msg["Authentication"]["localAddress"]) self.assertEquals('', msg["Authentication"]["clientDomain"]) # This is what the existing winbind implementation returns. - self.assertEquals("NT_STATUS_INVALID_HANDLE", + self.assertEquals("NT_STATUS_NO_SUCH_USER", msg["Authentication"]["status"]) self.assertEquals(self.credentials.get_username(), msg["Authentication"]["clientAccount"]) @@ -424,7 +424,7 @@ class AuthLogTestsWinbind(AuthLogTestBase, BlackboxTestCase): self.assertEquals("unix:", msg["Authentication"]["localAddress"]) self.assertEquals('', msg["Authentication"]["clientDomain"]) # This is what the existing winbind implementation returns. - self.assertEquals("NT_STATUS_INVALID_HANDLE", + self.assertEquals("NT_STATUS_NO_SUCH_USER", msg["Authentication"]["status"]) self.assertEquals(self.credentials.get_username(), msg["Authentication"]["clientAccount"]) diff --git a/selftest/knownfail.d/empty-domain-name b/selftest/knownfail.d/empty-domain-name index 9855e4d58c28..a1ffcaf7e3cb 100644 --- a/selftest/knownfail.d/empty-domain-name +++ b/selftest/knownfail.d/empty-domain-name @@ -1,5 +1,5 @@ ^samba3.blackbox.smbclient_auth.empty_domain.domain_creds.smbclient.*as.user.*nt4_member -^samba3.blackbox.smbclient_auth.empty_domain.member_creds.smbclient.*as.user.*ad_member +^samba3.blackbox.smbclient_auth.empty_domain.domain_creds.smbclient.*as.user.*ad_member ^samba3.blackbox.smbclient_auth.dot_domain.domain_creds.smbclient.*as.user.*nt4_member ^samba3.blackbox.smbclient_auth.dot_domain.domain_creds.smbclient.*as.user.*ad_member ^samba3.blackbox.smbclient_auth.upn.domain_creds.smbclient.*as.*user.*nt4_member diff --git a/source3/auth/auth_sam.c b/source3/auth/auth_sam.c index fa40a5c8f3eb..c6357c696eac 100644 --- a/source3/auth/auth_sam.c +++ b/source3/auth/auth_sam.c @@ -93,10 +93,36 @@ static NTSTATUS auth_samstrict_auth(const struct auth_context *auth_context, return NT_STATUS_NOT_IMPLEMENTED; } + if (lp_server_role() == ROLE_DOMAIN_MEMBER) { + const char *p = NULL; + + p = strchr_m(user_info->mapped.account_name, '@'); + if (p != NULL) { + /* + * This needs to go to the DC, + * even if @ is the last character + */ + return NT_STATUS_NOT_IMPLEMENTED; + } + } + + if (effective_domain == NULL) { + effective_domain = ""; + } + DBG_DEBUG("Check auth for: [%s]\\[%s]\n", effective_domain, user_info->mapped.account_name); + + if (strequal(effective_domain, "") || strequal(effective_domain, ".")) { + /* + * An empty domain name or '.' should be handled + * as the local SAM name. + */ + effective_domain = lp_netbios_name(); + } + is_local_name = is_myname(effective_domain); is_my_domain = strequal(effective_domain, lp_workgroup()); @@ -174,6 +200,10 @@ static NTSTATUS auth_sam_netlogon3_auth(const struct auth_context *auth_context, return NT_STATUS_NOT_IMPLEMENTED; } + if (effective_domain == NULL) { + effective_domain = ""; + } + DBG_DEBUG("Check auth for: [%s]\\[%s]\n", effective_domain, user_info->mapped.account_name); @@ -189,6 +219,14 @@ static NTSTATUS auth_sam_netlogon3_auth(const struct auth_context *auth_context, return NT_STATUS_INVALID_SERVER_STATE; } + if (strequal(effective_domain, "") || strequal(effective_domain, ".")) { + /* + * An empty domain name or '.' should be handled + * as the local SAM name. + */ + effective_domain = lp_workgroup(); + } + is_my_domain = strequal(user_info->mapped.domain_name, lp_workgroup()); if (!is_my_domain) { DBG_INFO("%s is not our domain name (DC for %s)\n", -- 2.17.1