From 633d8f0f2aa033de7df468a4b01e3c88a3b95194 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 46958c54d3a3..1ccaededc2dd 100644 --- a/source3/auth/auth_sam.c +++ b/source3/auth/auth_sam.c @@ -89,7 +89,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 ) { @@ -98,7 +98,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 60fd6c8b4168f4f95dded2f755263dc5b5ade010 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 1ccaededc2dd..56f28ab94a67 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); } @@ -72,7 +77,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 b66697517e9ded839aeaf3320e60726b749578f5 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 56f28ab94a67..e2c62f94d489 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); @@ -77,6 +83,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); @@ -148,6 +160,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 5e05d3354a221eb89051c0d214450969d4564ea6 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 e2c62f94d489..2948c18df0c1 100644 --- a/source3/auth/auth_sam.c +++ b/source3/auth/auth_sam.c @@ -77,6 +77,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) { @@ -90,11 +91,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 */ @@ -103,7 +104,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; } @@ -113,7 +114,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; } @@ -154,6 +155,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) { @@ -167,7 +169,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 */ @@ -184,7 +186,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 5102cd365f9383f324c40b0c375a1e462f556255 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 93c41ef956df..67e9282459c8 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -242,6 +242,14 @@ for options in ["--option=clientntlmv2auth=no", "--option=clientusespnego=no --o plantestsuite("samba3.blackbox.smbclient_auth.plain (%s) %s" % (env, 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) %s member creds" % (env, 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 6d9b6b27dae2320aa70483e58726e778554016d1 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 a390197fe7f6..6ba8795ae1e4 100644 --- a/python/samba/tests/auth_log_winbind.py +++ b/python/samba/tests/auth_log_winbind.py @@ -322,7 +322,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"]) @@ -425,7 +425,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 2948c18df0c1..cdb8453b3115 100644 --- a/source3/auth/auth_sam.c +++ b/source3/auth/auth_sam.c @@ -90,10 +90,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()); @@ -168,6 +194,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); @@ -183,6 +213,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