From 41411bdcc737092cfea0c3266371c9b52d07b26c Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Sat, 18 Jan 2020 08:06:45 +0100
Subject: [PATCH 01/20] s3/auth: use set_current_user_info() in
 auth3_generate_session_info_pac()

This delays reloading config slightly, but I don't see how could affect
observable behaviour other then log messages coming from the functions in
between the different locations for lp_load_with_shares() like
make_session_info_krb5() are sent to a different logfile if "log file" uses %U.

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>
(cherry picked from commit dc4b1e39ce1f2201a2d6ae2d4cffef2448f69a62)

[scabrero@samba.org Prerequisite for CVE-2020-25717 backport]
---
 source3/auth/auth_generic.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/source3/auth/auth_generic.c b/source3/auth/auth_generic.c
index f9b918496fd..7987d800f5a 100644
--- a/source3/auth/auth_generic.c
+++ b/source3/auth/auth_generic.c
@@ -157,12 +157,6 @@ static NTSTATUS auth3_generate_session_info_pac(struct auth4_context *auth_ctx,
 		}
 	}
 
-	/* setup the string used by %U */
-	sub_set_smb_name(username);
-
-	/* reload services so that the new %U is taken into account */
-	lp_load_with_shares(get_dyn_CONFIGFILE());
-
 	status = make_session_info_krb5(mem_ctx,
 					ntuser, ntdomain, username, pw,
 					info3_copy, is_guest, is_mapped, NULL /* No session key for now, caller will sort it out */,
@@ -174,6 +168,14 @@ static NTSTATUS auth3_generate_session_info_pac(struct auth4_context *auth_ctx,
 		goto done;
 	}
 
+	/* setup the string used by %U */
+	set_current_user_info((*session_info)->unix_info->sanitized_username,
+			      (*session_info)->unix_info->unix_name,
+			      (*session_info)->info->domain_name);
+
+	/* reload services so that the new %U is taken into account */
+	lp_load_with_shares(get_dyn_CONFIGFILE());
+
 	DEBUG(5, (__location__ "OK: user: %s domain: %s client: %s\n",
 		  ntuser, ntdomain, rhost));
 
-- 
2.33.1


From 2a2b3b6157d5c8c740227b88352f02bce543aaaf Mon Sep 17 00:00:00 2001
From: Samuel Cabrero <scabrero@suse.de>
Date: Thu, 4 Nov 2021 11:51:08 +0100
Subject: [PATCH 02/20] selftest: Fix ktest usermap file

The user was not mapped:

user_in_list: checking user |KTEST/administrator| against |KTEST\Administrator|
The user 'KTEST/administrator' has no mapping. Skip it next time.

Signed-off-by: Samuel Cabrero <scabrero@samba.org>

[scabrero@samba.org Once smb_getpswnam() fallbacks are removed the user
 has to be mapped]
---
 selftest/target/Samba3.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index dc8f216213b..5c2d1ac1087 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -956,7 +956,7 @@ sub setup_ktest($$$)
 
 	open(USERMAP, ">$prefix/lib/username.map") or die("Unable to open $prefix/lib/username.map");
 	print USERMAP "
-$ret->{USERNAME} = KTEST\\Administrator
+$ret->{USERNAME} = KTEST/Administrator
 ";
 	close(USERMAP);
 
-- 
2.33.1


From 9d243d645522deef770b917626751053a011a3bb Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Tue, 5 Oct 2021 16:42:00 +0200
Subject: [PATCH 03/20] selftest/Samba3: replace (winbindd => "yes", skip_wait
 => 1) with (winbindd => "offline")

This is much more flexible and concentrates the logic in a single place.

We'll use winbindd => "offline" in other places soon.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
(cherry picked from commit 4dc3c68c9a28f71888e3d6dd3b1f0bcdb8fa45de)
(cherry picked from commit 89b9cb8b786c3e4eb8691b5363390b68d8228a2d)

[scabrero@samba.org Backported to 4.7]
---
 selftest/target/Samba3.pm | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index 5c2d1ac1087..3a6426fabf3 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -1165,7 +1165,7 @@ sub check_or_start($$$$$) {
 
 		$ENV{ENVNAME} = "$ENV{ENVNAME}.winbindd";
 
-		if ($winbindd ne "yes") {
+		if ($winbindd ne "yes" and $winbindd ne "offline") {
 			$SIG{USR1} = $SIG{ALRM} = $SIG{INT} = $SIG{QUIT} = $SIG{TERM} = sub {
 				my $signame = shift;
 				print("Skip winbindd received signal $signame");
@@ -2239,11 +2239,15 @@ sub wait_for_start($$$$$)
 		}
 	}
 
-	if ($winbindd eq "yes") {
+	if ($winbindd eq "yes" or $winbindd eq "offline") {
 	    print "checking for winbindd\n";
 	    my $count = 0;
 	    do {
-		$ret = system("SELFTEST_WINBINDD_SOCKET_DIR=" . $envvars->{SELFTEST_WINBINDD_SOCKET_DIR} . " " . Samba::bindir_path($self, "wbinfo") . " --ping-dc");
+                if ($winbindd eq "yes") {
+		    $ret = system("SELFTEST_WINBINDD_SOCKET_DIR=" . $envvars->{SELFTEST_WINBINDD_SOCKET_DIR} . " " . Samba::bindir_path($self, "wbinfo") . " --ping-dc");
+                } elsif ($winbindd eq "offline") {
+		    $ret = system("SELFTEST_WINBINDD_SOCKET_DIR=" . $envvars->{SELFTEST_WINBINDD_SOCKET_DIR} . " " . Samba::bindir_path($self, "wbinfo") . " --ping");
+		}
 		if ($ret != 0) {
 		    sleep(1);
 		}
-- 
2.33.1


From e860f921c0965119553d1ec947c61ec1596cb317 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Fri, 22 Oct 2021 16:20:36 +0200
Subject: [PATCH 04/20] CVE-2020-25719 CVE-2020-25717: selftest: remove
 "gensec:require_pac" settings

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
---
 selftest/selftest.pl      | 2 --
 selftest/target/Samba4.pm | 2 --
 2 files changed, 4 deletions(-)

diff --git a/selftest/selftest.pl b/selftest/selftest.pl
index c4a546472b5..8c569fcc943 100755
--- a/selftest/selftest.pl
+++ b/selftest/selftest.pl
@@ -587,8 +587,6 @@ sub write_clientconf($$$)
 	client lanman auth = Yes
 	log level = 1
 	torture:basedir = $clientdir
-#We don't want to pass our self-tests if the PAC code is wrong
-	gensec:require_pac = true
 #We don't want to run 'speed' tests for very long
         torture:timelimit = 1
         winbind separator = /
diff --git a/selftest/target/Samba4.pm b/selftest/target/Samba4.pm
index 6df017fcd3e..24a7623d2cf 100755
--- a/selftest/target/Samba4.pm
+++ b/selftest/target/Samba4.pm
@@ -597,8 +597,6 @@ sub provision_raw_step1($$)
 	notify:inotify = false
 	ldb:nosync = true
 	ldap server require strong auth = yes
-#We don't want to pass our self-tests if the PAC code is wrong
-	gensec:require_pac = true
 	log file = $ctx->{logdir}/log.\%m
 	log level = $ctx->{server_loglevel}
 	lanman auth = Yes
-- 
2.33.1


From 943e90de8cc9782ee8679633696584ec61b49861 Mon Sep 17 00:00:00 2001
From: Samuel Cabrero <scabrero@samba.org>
Date: Tue, 28 Sep 2021 10:43:40 +0200
Subject: [PATCH 05/20] CVE-2020-25717: loadparm: Add new parameter "min domain
 uid"

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

Pair-Programmed-With: Stefan Metzmacher <metze@samba.org>

Signed-off-by: Samuel Cabrero <scabrero@samba.org>
Signed-off-by: Stefan Metzmacher <metze@samba.org>

[abartlet@samba.org Backported from master/4.15 due to
 conflicts with other new parameters]
[scabrero@samba.org Backported to 4.6]
---
 docs-xml/smbdotconf/security/mindomainuid.xml | 17 +++++++++++++++++
 docs-xml/smbdotconf/winbind/idmapconfig.xml   |  4 ++++
 lib/param/loadparm.c                          |  4 ++++
 source3/param/loadparm.c                      |  2 ++
 4 files changed, 27 insertions(+)
 create mode 100644 docs-xml/smbdotconf/security/mindomainuid.xml

diff --git a/docs-xml/smbdotconf/security/mindomainuid.xml b/docs-xml/smbdotconf/security/mindomainuid.xml
new file mode 100644
index 00000000000..46ae795d730
--- /dev/null
+++ b/docs-xml/smbdotconf/security/mindomainuid.xml
@@ -0,0 +1,17 @@
+<samba:parameter name="min domain uid"
+                 type="integer"
+                 context="G"
+                 xmlns:samba="http://www.samba.org/samba/DTD/samba-doc">
+<description>
+  <para>
+    The integer parameter specifies the minimum uid allowed when mapping a
+    local account to a domain account.
+  </para>
+
+  <para>
+    Note that this option interacts with the configured <emphasis>idmap ranges</emphasis>!
+  </para>
+</description>
+
+<value type="default">1000</value>
+</samba:parameter>
diff --git a/docs-xml/smbdotconf/winbind/idmapconfig.xml b/docs-xml/smbdotconf/winbind/idmapconfig.xml
index 1374040fb29..f70f11df757 100644
--- a/docs-xml/smbdotconf/winbind/idmapconfig.xml
+++ b/docs-xml/smbdotconf/winbind/idmapconfig.xml
@@ -80,6 +80,9 @@
 		authoritative for a unix ID to SID mapping, so it must be set
 		for each individually configured domain and for the default
 		configuration. The configured ranges must be mutually disjoint.
+		</para>
+		<para>
+		Note that the low value interacts with the <smbconfoption name="min domain uid"/> option!
 		</para></listitem>
 		</varlistentry>
 
@@ -115,4 +118,5 @@
 	</programlisting>
 	
 </description>
+<related>min domain uid</related>
 </samba:parameter>
diff --git a/lib/param/loadparm.c b/lib/param/loadparm.c
index 516c14b7685..e183d49246f 100644
--- a/lib/param/loadparm.c
+++ b/lib/param/loadparm.c
@@ -2916,6 +2916,10 @@ struct loadparm_context *loadparm_init(TALLOC_CTX *mem_ctx)
 		}
 	}
 
+	lpcfg_do_global_parameter(lp_ctx,
+				  "min domain uid",
+				  "1000");
+
 	for (i = 0; parm_table[i].label; i++) {
 		if (!(lp_ctx->flags[i] & FLAG_CMDLINE)) {
 			lp_ctx->flags[i] |= FLAG_DEFAULT;
diff --git a/source3/param/loadparm.c b/source3/param/loadparm.c
index 752ab5f498c..a3ac538e2a8 100644
--- a/source3/param/loadparm.c
+++ b/source3/param/loadparm.c
@@ -939,6 +939,8 @@ static void init_globals(struct loadparm_context *lp_ctx, bool reinit_globals)
 
 	Globals.aio_max_threads = 100;
 
+	Globals.min_domain_uid = 1000;
+
 	/* Now put back the settings that were set with lp_set_cmdline() */
 	apply_lp_set_cmdline();
 }
-- 
2.33.1


From 689bdf92a759f71b36bf9d2b6a6a5c4567f04066 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Fri, 8 Oct 2021 19:57:18 +0200
Subject: [PATCH 06/20] CVE-2020-25717: s3:auth: let
 auth3_generate_session_info_pac() forward the low level errors

Mapping everything to ACCESS_DENIED makes it hard to debug problems,
which may happen because of our more restrictive behaviour in future.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
---
 source3/auth/auth_generic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source3/auth/auth_generic.c b/source3/auth/auth_generic.c
index 7987d800f5a..191fe424aea 100644
--- a/source3/auth/auth_generic.c
+++ b/source3/auth/auth_generic.c
@@ -164,7 +164,7 @@ static NTSTATUS auth3_generate_session_info_pac(struct auth4_context *auth_ctx,
 	if (!NT_STATUS_IS_OK(status)) {
 		DEBUG(1, ("Failed to map kerberos pac to server info (%s)\n",
 			  nt_errstr(status)));
-		status = NT_STATUS_ACCESS_DENIED;
+		status = nt_status_squash(status);
 		goto done;
 	}
 
-- 
2.33.1


From a62adaac929f86a3f7a073d9965c4726f00fda1e Mon Sep 17 00:00:00 2001
From: Samuel Cabrero <scabrero@samba.org>
Date: Tue, 28 Sep 2021 10:45:11 +0200
Subject: [PATCH 07/20] CVE-2020-25717: s3:auth: Check minimum domain uid

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

Pair-Programmed-With: Stefan Metzmacher <metze@samba.org>

Signed-off-by: Samuel Cabrero <scabrero@samba.org>
Signed-off-by: Stefan Metzmacher <metze@samba.org>
---
 source3/auth/auth_util.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/source3/auth/auth_util.c b/source3/auth/auth_util.c
index ae6bfb3de41..2cc26a69c15 100644
--- a/source3/auth/auth_util.c
+++ b/source3/auth/auth_util.c
@@ -1432,6 +1432,22 @@ NTSTATUS make_server_info_info3(TALLOC_CTX *mem_ctx,
 			}
 		}
 		goto out;
+	} else if ((lp_security() == SEC_ADS || lp_security() == SEC_DOMAIN) &&
+		   !is_myname(domain) && pwd->pw_uid < lp_min_domain_uid()) {
+		/*
+		 * !is_myname(domain) because when smbd starts tries to setup
+		 * the guest user info, calling this function with nobody
+		 * username. Nobody is usually uid 65535 but it can be changed
+		 * to a regular user with 'guest account' parameter
+		 */
+		nt_status = NT_STATUS_INVALID_TOKEN;
+		DBG_NOTICE("Username '%s%s%s' is invalid on this system, "
+			   "it does not meet 'min domain uid' "
+			   "restriction (%u < %u): %s\n",
+			   nt_domain, lp_winbind_separator(), nt_username,
+			   pwd->pw_uid, lp_min_domain_uid(),
+			   nt_errstr(nt_status));
+		goto out;
 	}
 
 	result = make_server_info(tmp_ctx);
-- 
2.33.1


From 76556189974e7cf0855579e36e12998059f1bc67 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Fri, 8 Oct 2021 17:40:30 +0200
Subject: [PATCH 08/20] CVE-2020-25717: s3:auth: we should not try to
 autocreate the guest account

We should avoid autocreation of users as much as possible.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
---
 source3/auth/user_krb5.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source3/auth/user_krb5.c b/source3/auth/user_krb5.c
index 2b009baa58d..c7bf2f2b9db 100644
--- a/source3/auth/user_krb5.c
+++ b/source3/auth/user_krb5.c
@@ -155,7 +155,7 @@ NTSTATUS get_user_from_kerberos_info(TALLOC_CTX *mem_ctx,
 			if (!fuser) {
 				return NT_STATUS_NO_MEMORY;
 			}
-			pw = smb_getpwnam(mem_ctx, fuser, &unixuser, true);
+			pw = smb_getpwnam(mem_ctx, fuser, &unixuser, false);
 		}
 
 		/* extra sanity check that the guest account is valid */
-- 
2.33.1


From e32156e9492775bfdab8e1310457dca97461dba6 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Fri, 8 Oct 2021 18:08:20 +0200
Subject: [PATCH 09/20] CVE-2020-25717: s3:auth: no longer let check_account()
 autocreate local users

So far we autocreated local user accounts based on just the
account_name (just ignoring any domain part).

This only happens via a possible 'add user script',
which is not typically defined on domain members
and on NT4 DCs local users already exist in the
local passdb anyway.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
---
 source3/auth/auth_util.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source3/auth/auth_util.c b/source3/auth/auth_util.c
index 2cc26a69c15..10e7ba72745 100644
--- a/source3/auth/auth_util.c
+++ b/source3/auth/auth_util.c
@@ -1227,7 +1227,7 @@ static NTSTATUS check_account(TALLOC_CTX *mem_ctx, const char *domain,
 		return NT_STATUS_NO_MEMORY;
 	}
 
-	passwd = smb_getpwnam(mem_ctx, dom_user, &real_username, true );
+	passwd = smb_getpwnam(mem_ctx, dom_user, &real_username, false);
 	if (!passwd) {
 		DEBUG(3, ("Failed to find authenticated user %s via "
 			  "getpwnam(), denying access.\n", dom_user));
-- 
2.33.1


From 62333b8c99687a79f132bf880d385a48c50be206 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow@samba.org>
Date: Fri, 8 Oct 2021 12:33:16 +0200
Subject: [PATCH 10/20] CVE-2020-25717: s3:auth: remove fallbacks in
 smb_getpwnam()

So far we tried getpwnam("DOMAIN\account") first and
always did a fallback to getpwnam("account") completely
ignoring the domain part, this just causes problems
as we mix "DOMAIN1\account", "DOMAIN2\account",
and "account"!

As we require a running winbindd for domain member setups
we should no longer do a fallback to just "account" for
users served by winbindd!

For users of the local SAM don't use this code path,
as check_sam_security() doesn't call check_account().

The only case where smb_getpwnam("account") happens is
when map_username() via ("username map [script]")  mapped
"DOMAIN\account" to something without '\', but that is
explicitly desired by the admin.

Note: use 'git show -w'

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

Pair-Programmed-With: Stefan Metzmacher <metze@samba.org>

Signed-off-by: Ralph Boehme <slow@samba.org>
Signed-off-by: Stefan Metzmacher <metze@samba.org>

[scabrero@samba.org Backported to 4.7 removing
 selftest/knownfail.d/ktest after fixing user mapping in ktest
 environment]
---
 source3/auth/auth_util.c | 77 ++++++++++++++++++++++------------------
 1 file changed, 42 insertions(+), 35 deletions(-)

diff --git a/source3/auth/auth_util.c b/source3/auth/auth_util.c
index 10e7ba72745..7c11f05f039 100644
--- a/source3/auth/auth_util.c
+++ b/source3/auth/auth_util.c
@@ -1262,7 +1262,7 @@ struct passwd *smb_getpwnam( TALLOC_CTX *mem_ctx, const char *domuser,
 {
 	struct passwd *pw = NULL;
 	char *p = NULL;
-	char *username = NULL;
+	const char *username = NULL;
 
 	/* we only save a copy of the username it has been mangled 
 	   by winbindd use default domain */
@@ -1281,48 +1281,55 @@ struct passwd *smb_getpwnam( TALLOC_CTX *mem_ctx, const char *domuser,
 	/* code for a DOMAIN\user string */
 
 	if ( p ) {
-		pw = Get_Pwnam_alloc( mem_ctx, domuser );
-		if ( pw ) {
-			/* make sure we get the case of the username correct */
-			/* work around 'winbind use default domain = yes' */
-
-			if ( lp_winbind_use_default_domain() &&
-			     !strchr_m( pw->pw_name, *lp_winbind_separator() ) ) {
-				char *domain;
-
-				/* split the domain and username into 2 strings */
-				*p = '\0';
-				domain = username;
-
-				*p_save_username = talloc_asprintf(mem_ctx,
-								"%s%c%s",
-								domain,
-								*lp_winbind_separator(),
-								pw->pw_name);
-				if (!*p_save_username) {
-					TALLOC_FREE(pw);
-					return NULL;
-				}
-			} else {
-				*p_save_username = talloc_strdup(mem_ctx, pw->pw_name);
-			}
+		const char *domain = NULL;
 
-			/* whew -- done! */
-			return pw;
+		/* split the domain and username into 2 strings */
+		*p = '\0';
+		domain = username;
+		p++;
+		username = p;
+
+		if (strequal(domain, get_global_sam_name())) {
+			/*
+			 * This typically don't happen
+			 * as check_sam_Security()
+			 * don't call make_server_info_info3()
+			 * and thus check_account().
+			 *
+			 * But we better keep this.
+			 */
+			goto username_only;
 		}
 
-		/* setup for lookup of just the username */
-		/* remember that p and username are overlapping memory */
-
-		p++;
-		username = talloc_strdup(mem_ctx, p);
-		if (!username) {
+		pw = Get_Pwnam_alloc( mem_ctx, domuser );
+		if (pw == NULL) {
 			return NULL;
 		}
+		/* make sure we get the case of the username correct */
+		/* work around 'winbind use default domain = yes' */
+
+		if ( lp_winbind_use_default_domain() &&
+		     !strchr_m( pw->pw_name, *lp_winbind_separator() ) ) {
+			*p_save_username = talloc_asprintf(mem_ctx,
+							"%s%c%s",
+							domain,
+							*lp_winbind_separator(),
+							pw->pw_name);
+			if (!*p_save_username) {
+				TALLOC_FREE(pw);
+				return NULL;
+			}
+		} else {
+			*p_save_username = talloc_strdup(mem_ctx, pw->pw_name);
+		}
+
+		/* whew -- done! */
+		return pw;
+
 	}
 
 	/* just lookup a plain username */
-
+username_only:
 	pw = Get_Pwnam_alloc(mem_ctx, username);
 
 	/* Create local user if requested but only if winbindd
-- 
2.33.1


From d5437dbc46c14f1cf70e2b428a03dff1da27ab42 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Mon, 4 Oct 2021 18:03:55 +0200
Subject: [PATCH 11/20] CVE-2020-25717: s3:auth: don't let create_local_token
 depend on !winbind_ping()

We always require a running winbindd on a domain member, so
we should better fail a request instead of silently alter
the behaviour, which results in a different unix token, just
because winbindd might be restarted.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
---
 source3/auth/auth_util.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/source3/auth/auth_util.c b/source3/auth/auth_util.c
index 7c11f05f039..d69259ac2cd 100644
--- a/source3/auth/auth_util.c
+++ b/source3/auth/auth_util.c
@@ -553,13 +553,11 @@ NTSTATUS create_local_token(TALLOC_CTX *mem_ctx,
 	}
 
 	/*
-	 * If winbind is not around, we can not make much use of the SIDs the
-	 * domain controller provided us with. Likewise if the user name was
-	 * mapped to some local unix user.
+	 * If the user name was mapped to some local unix user,
+	 * we can not make much use of the SIDs the
+	 * domain controller provided us with.
 	 */
-
-	if (((lp_server_role() == ROLE_DOMAIN_MEMBER) && !winbind_ping()) ||
-	    (server_info->nss_token)) {
+	if (server_info->nss_token) {
 		char *found_username = NULL;
 		status = create_token_from_username(session_info,
 						    server_info->unix_name,
-- 
2.33.1


From b13b36bd864db9d0c60533b609454cc52552771a Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Tue, 5 Oct 2021 18:11:57 +0200
Subject: [PATCH 12/20] CVE-2020-25717: auth/gensec: always require a PAC in
 domain mode (DC or member)

AD domains always provide a PAC unless UF_NO_AUTH_DATA_REQUIRED is set
on the service account, which can only be explicitly configured,
but that's an invalid configuration!

We still try to support standalone servers in an MIT realm,
as legacy setup.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>

[scabrero@samba.org Backported to 4.7 due to lack of debug class macro]
---
 auth/gensec/gensec_util.c | 27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/auth/gensec/gensec_util.c b/auth/gensec/gensec_util.c
index da2e2e5f4b8..92c66cc8468 100644
--- a/auth/gensec/gensec_util.c
+++ b/auth/gensec/gensec_util.c
@@ -25,6 +25,8 @@
 #include "auth/gensec/gensec_internal.h"
 #include "auth/common_auth.h"
 #include "../lib/util/asn1.h"
+#include "param/param.h"
+#include "libds/common/roles.h"
 
 NTSTATUS gensec_generate_session_info_pac(TALLOC_CTX *mem_ctx,
 					  struct gensec_security *gensec_security,
@@ -43,10 +45,27 @@ NTSTATUS gensec_generate_session_info_pac(TALLOC_CTX *mem_ctx,
 	session_info_flags |= AUTH_SESSION_INFO_DEFAULT_GROUPS;
 
 	if (!pac_blob) {
-		if (gensec_setting_bool(gensec_security->settings, "gensec", "require_pac", false)) {
-			DEBUG(1, ("Unable to find PAC in ticket from %s, failing to allow access\n",
-				  principal_string));
-			return NT_STATUS_ACCESS_DENIED;
+		enum server_role server_role =
+			lpcfg_server_role(gensec_security->settings->lp_ctx);
+
+		/*
+		 * For any domain setup (DC or member) we require having
+		 * a PAC, as the service ticket comes from an AD DC,
+		 * which will always provide a PAC, unless
+		 * UF_NO_AUTH_DATA_REQUIRED is configured for our
+		 * account, but that's just an invalid configuration,
+		 * the admin configured for us!
+		 *
+		 * As a legacy case, we still allow kerberos tickets from an MIT
+		 * realm, but only in standalone mode. In that mode we'll only
+		 * ever accept a kerberos authentication with a keytab file
+		 * being explicitly configured via the 'keytab method' option.
+		 */
+		if (server_role != ROLE_STANDALONE) {
+			DBG_WARNING("Unable to find PAC in ticket from %s, "
+				    "failing to allow access\n",
+				    principal_string);
+			return NT_STATUS_NO_IMPERSONATION_TOKEN;
 		}
 		DBG_NOTICE("Unable to find PAC for %s, resorting to local "
 			   "user lookup\n", principal_string);
-- 
2.33.1


From 5695a3072afe6416c803243999adb74d914f903d Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Mon, 11 Oct 2021 23:17:19 +0200
Subject: [PATCH 13/20] CVE-2020-25717: s4:auth: remove unused
 auth_generate_session_info_principal()

We'll require a PAC at the main gensec layer already.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>

[abartlet@samba.org Backported from master/4.15 as
 check_password is sync in 4.14]
[scabrero@samba.org Backported to 4.7 due to different return code]
[scabrero@samba.org Backported to 4.6, flags field in struct
 auth_operations is used]
---
 source4/auth/auth.h          |  6 -----
 source4/auth/ntlm/auth.c     | 49 ++++--------------------------------
 source4/auth/ntlm/auth_sam.c | 13 ----------
 3 files changed, 5 insertions(+), 63 deletions(-)

diff --git a/source4/auth/auth.h b/source4/auth/auth.h
index fb486941697..38db52b4ecc 100644
--- a/source4/auth/auth.h
+++ b/source4/auth/auth.h
@@ -66,12 +66,6 @@ struct auth_operations {
 				   const struct auth_usersupplied_info *user_info,
 				   struct auth_user_info_dc **interim_info);
 
-	/* Lookup a 'session info interim' return based only on the principal or DN */
-	NTSTATUS (*get_user_info_dc_principal)(TALLOC_CTX *mem_ctx,
-						       struct auth4_context *auth_context,
-						       const char *principal,
-						       struct ldb_dn *user_dn,
-						       struct auth_user_info_dc **interim_info);
 	uint32_t flags;
 };
 
diff --git a/source4/auth/ntlm/auth.c b/source4/auth/ntlm/auth.c
index eeb23367e7e..ba72e0c50e0 100644
--- a/source4/auth/ntlm/auth.c
+++ b/source4/auth/ntlm/auth.c
@@ -83,48 +83,6 @@ _PUBLIC_ NTSTATUS auth_get_challenge(struct auth4_context *auth_ctx, uint8_t cha
 	return NT_STATUS_OK;
 }
 
-/****************************************************************************
-Used in the gensec_gssapi and gensec_krb5 server-side code, where the
-PAC isn't available, and for tokenGroups in the DSDB stack.
-
- Supply either a principal or a DN
-****************************************************************************/
-static NTSTATUS auth_generate_session_info_principal(struct auth4_context *auth_ctx,
-						  TALLOC_CTX *mem_ctx,
-						  const char *principal,
-						  struct ldb_dn *user_dn,
-                                                  uint32_t session_info_flags,
-                                                  struct auth_session_info **session_info)
-{
-	NTSTATUS nt_status;
-	struct auth_method_context *method;
-	struct auth_user_info_dc *user_info_dc;
-
-	for (method = auth_ctx->methods; method; method = method->next) {
-		if (!method->ops->get_user_info_dc_principal) {
-			continue;
-		}
-
-		nt_status = method->ops->get_user_info_dc_principal(mem_ctx, auth_ctx, principal, user_dn, &user_info_dc);
-		if (NT_STATUS_EQUAL(nt_status, NT_STATUS_NOT_IMPLEMENTED)) {
-			continue;
-		}
-		if (!NT_STATUS_IS_OK(nt_status)) {
-			return nt_status;
-		}
-
-		nt_status = auth_generate_session_info_wrapper(auth_ctx, mem_ctx, 
-							       user_info_dc,
-							       user_info_dc->info->account_name,
-							       session_info_flags, session_info);
-		talloc_free(user_info_dc);
-
-		return nt_status;
-	}
-
-	return NT_STATUS_NOT_IMPLEMENTED;
-}
-
 /**
  * Check a user's Plaintext, LM or NTLM password.
  * (sync version)
@@ -498,8 +456,11 @@ static NTSTATUS auth_generate_session_info_pac(struct auth4_context *auth_ctx,
 	TALLOC_CTX *tmp_ctx;
 
 	if (!pac_blob) {
-		return auth_generate_session_info_principal(auth_ctx, mem_ctx, principal_name,
-						       NULL, session_info_flags, session_info);
+		/*
+		 * This should already be catched at the main
+		 * gensec layer, but better check twice
+		 */
+		return NT_STATUS_INTERNAL_ERROR;
 	}
 
 	tmp_ctx = talloc_named(mem_ctx, 0, "gensec_gssapi_session_info context");
diff --git a/source4/auth/ntlm/auth_sam.c b/source4/auth/ntlm/auth_sam.c
index 90eabcaf506..0d96d2a13a7 100644
--- a/source4/auth/ntlm/auth_sam.c
+++ b/source4/auth/ntlm/auth_sam.c
@@ -684,22 +684,10 @@ static NTSTATUS authsam_want_check(struct auth_method_context *ctx,
 	return NT_STATUS_NOT_IMPLEMENTED;
 }
 
-				   
-/* Wrapper for the auth subsystem pointer */
-static NTSTATUS authsam_get_user_info_dc_principal_wrapper(TALLOC_CTX *mem_ctx,
-							  struct auth4_context *auth_context,
-							  const char *principal,
-							  struct ldb_dn *user_dn,
-							  struct auth_user_info_dc **user_info_dc)
-{
-	return authsam_get_user_info_dc_principal(mem_ctx, auth_context->lp_ctx, auth_context->sam_ctx,
-						 principal, user_dn, user_info_dc);
-}
 static const struct auth_operations sam_ignoredomain_ops = {
 	.name		           = "sam_ignoredomain",
 	.want_check	           = authsam_ignoredomain_want_check,
 	.check_password	           = authsam_check_password_internals,
-	.get_user_info_dc_principal = authsam_get_user_info_dc_principal_wrapper,
 	.flags                     = AUTH_METHOD_LOCAL_SAM
 };
 
@@ -707,7 +695,6 @@ static const struct auth_operations sam_ops = {
 	.name		           = "sam",
 	.want_check	           = authsam_want_check,
 	.check_password	           = authsam_check_password_internals,
-	.get_user_info_dc_principal = authsam_get_user_info_dc_principal_wrapper,
 	.flags                     = AUTH_METHOD_LOCAL_SAM
 };
 
-- 
2.33.1


From 4be8d97eb025ca85f853521365ba29143571b835 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Tue, 21 Sep 2021 12:27:28 +0200
Subject: [PATCH 14/20] CVE-2020-25717: s3:ntlm_auth: fix memory leaks in
 ntlm_auth_generate_session_info_pac()

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
---
 source3/utils/ntlm_auth.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/source3/utils/ntlm_auth.c b/source3/utils/ntlm_auth.c
index aed6a85dd27..41095ec0b06 100644
--- a/source3/utils/ntlm_auth.c
+++ b/source3/utils/ntlm_auth.c
@@ -816,23 +816,27 @@ static NTSTATUS ntlm_auth_generate_session_info_pac(struct auth4_context *auth_c
 	if (!p) {
 		DEBUG(3, ("[%s] Doesn't look like a valid principal\n",
 			  princ_name));
-		return NT_STATUS_LOGON_FAILURE;
+		status = NT_STATUS_LOGON_FAILURE;
+		goto done;
 	}
 
 	user = talloc_strndup(mem_ctx, princ_name, p - princ_name);
 	if (!user) {
-		return NT_STATUS_NO_MEMORY;
+		status = NT_STATUS_NO_MEMORY;
+		goto done;
 	}
 
 	realm = talloc_strdup(talloc_tos(), p + 1);
 	if (!realm) {
-		return NT_STATUS_NO_MEMORY;
+		status = NT_STATUS_NO_MEMORY;
+		goto done;
 	}
 
 	if (!strequal(realm, lp_realm())) {
 		DEBUG(3, ("Ticket for foreign realm %s@%s\n", user, realm));
 		if (!lp_allow_trusted_domains()) {
-			return NT_STATUS_LOGON_FAILURE;
+			status = NT_STATUS_LOGON_FAILURE;
+			goto done;
 		}
 	}
 
@@ -840,7 +844,8 @@ static NTSTATUS ntlm_auth_generate_session_info_pac(struct auth4_context *auth_c
 		domain = talloc_strdup(mem_ctx,
 					logon_info->info3.base.logon_domain.string);
 		if (!domain) {
-			return NT_STATUS_NO_MEMORY;
+			status = NT_STATUS_NO_MEMORY;
+			goto done;
 		}
 		DEBUG(10, ("Domain is [%s] (using PAC)\n", domain));
 	} else {
@@ -870,7 +875,8 @@ static NTSTATUS ntlm_auth_generate_session_info_pac(struct auth4_context *auth_c
 			domain = talloc_strdup(mem_ctx, realm);
 		}
 		if (!domain) {
-			return NT_STATUS_NO_MEMORY;
+			status = NT_STATUS_NO_MEMORY;
+			goto done;
 		}
 		DEBUG(10, ("Domain is [%s] (using Winbind)\n", domain));
 	}
-- 
2.33.1


From 27e4608473ed7a5547689b36cd097f6311d865c5 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Tue, 21 Sep 2021 12:44:01 +0200
Subject: [PATCH 15/20] CVE-2020-25717: s3:ntlm_auth: let
 ntlm_auth_generate_session_info_pac() base the name on the PAC LOGON_INFO
 only

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
---
 source3/utils/ntlm_auth.c | 91 ++++++++++++---------------------------
 1 file changed, 28 insertions(+), 63 deletions(-)

diff --git a/source3/utils/ntlm_auth.c b/source3/utils/ntlm_auth.c
index 41095ec0b06..38660a0699f 100644
--- a/source3/utils/ntlm_auth.c
+++ b/source3/utils/ntlm_auth.c
@@ -788,10 +788,8 @@ static NTSTATUS ntlm_auth_generate_session_info_pac(struct auth4_context *auth_c
 	struct PAC_LOGON_INFO *logon_info = NULL;
 	char *unixuser;
 	NTSTATUS status;
-	char *domain = NULL;
-	char *realm = NULL;
-	char *user = NULL;
-	char *p;
+	const char *domain = "";
+	const char *user = "";
 
 	tmp_ctx = talloc_new(mem_ctx);
 	if (!tmp_ctx) {
@@ -808,79 +806,46 @@ static NTSTATUS ntlm_auth_generate_session_info_pac(struct auth4_context *auth_c
 		if (!NT_STATUS_IS_OK(status)) {
 			goto done;
 		}
-	}
-
-	DEBUG(3, ("Kerberos ticket principal name is [%s]\n", princ_name));
-
-	p = strchr_m(princ_name, '@');
-	if (!p) {
-		DEBUG(3, ("[%s] Doesn't look like a valid principal\n",
-			  princ_name));
-		status = NT_STATUS_LOGON_FAILURE;
+	} else {
+		status = NT_STATUS_ACCESS_DENIED;
+		DBG_WARNING("Kerberos ticket for[%s] has no PAC: %s\n",
+			    princ_name, nt_errstr(status));
 		goto done;
 	}
 
-	user = talloc_strndup(mem_ctx, princ_name, p - princ_name);
-	if (!user) {
-		status = NT_STATUS_NO_MEMORY;
-		goto done;
+	if (logon_info->info3.base.account_name.string != NULL) {
+		user = logon_info->info3.base.account_name.string;
+	} else {
+		user = "";
+	}
+	if (logon_info->info3.base.logon_domain.string != NULL) {
+		domain = logon_info->info3.base.logon_domain.string;
+	} else {
+		domain = "";
 	}
 
-	realm = talloc_strdup(talloc_tos(), p + 1);
-	if (!realm) {
-		status = NT_STATUS_NO_MEMORY;
+	if (strlen(user) == 0 || strlen(domain) == 0) {
+		status = NT_STATUS_ACCESS_DENIED;
+		DBG_WARNING("Kerberos ticket for[%s] has invalid "
+			    "account_name[%s]/logon_domain[%s]: %s\n",
+			    princ_name,
+			    logon_info->info3.base.account_name.string,
+			    logon_info->info3.base.logon_domain.string,
+			    nt_errstr(status));
 		goto done;
 	}
 
-	if (!strequal(realm, lp_realm())) {
-		DEBUG(3, ("Ticket for foreign realm %s@%s\n", user, realm));
+	DBG_NOTICE("Kerberos ticket principal name is [%s] "
+		   "account_name[%s]/logon_domain[%s]\n",
+		   princ_name, user, domain);
+
+	if (!strequal(domain, lp_workgroup())) {
 		if (!lp_allow_trusted_domains()) {
 			status = NT_STATUS_LOGON_FAILURE;
 			goto done;
 		}
 	}
 
-	if (logon_info && logon_info->info3.base.logon_domain.string) {
-		domain = talloc_strdup(mem_ctx,
-					logon_info->info3.base.logon_domain.string);
-		if (!domain) {
-			status = NT_STATUS_NO_MEMORY;
-			goto done;
-		}
-		DEBUG(10, ("Domain is [%s] (using PAC)\n", domain));
-	} else {
-
-		/* If we have winbind running, we can (and must) shorten the
-		   username by using the short netbios name. Otherwise we will
-		   have inconsistent user names. With Kerberos, we get the
-		   fully qualified realm, with ntlmssp we get the short
-		   name. And even w2k3 does use ntlmssp if you for example
-		   connect to an ip address. */
-
-		wbcErr wbc_status;
-		struct wbcDomainInfo *info = NULL;
-
-		DEBUG(10, ("Mapping [%s] to short name using winbindd\n",
-			   realm));
-
-		wbc_status = wbcDomainInfo(realm, &info);
-
-		if (WBC_ERROR_IS_OK(wbc_status)) {
-			domain = talloc_strdup(mem_ctx,
-						info->short_name);
-			wbcFreeMemory(info);
-		} else {
-			DEBUG(3, ("Could not find short name: %s\n",
-				  wbcErrorString(wbc_status)));
-			domain = talloc_strdup(mem_ctx, realm);
-		}
-		if (!domain) {
-			status = NT_STATUS_NO_MEMORY;
-			goto done;
-		}
-		DEBUG(10, ("Domain is [%s] (using Winbind)\n", domain));
-	}
-
 	unixuser = talloc_asprintf(tmp_ctx, "%s%c%s", domain, winbind_separator(), user);
 	if (!unixuser) {
 		status = NT_STATUS_NO_MEMORY;
-- 
2.33.1


From 99652d2966d8267db0e0ca3b5c593e7c6ab0b539 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Mon, 4 Oct 2021 19:42:20 +0200
Subject: [PATCH 16/20] CVE-2020-25717: s3:auth: let
 auth3_generate_session_info_pac() delegate everything to
 make_server_info_wbcAuthUserInfo()

This consolidates the code paths used for NTLMSSP and Kerberos!

I checked what we were already doing for NTLMSSP, which is this:

a) source3/auth/auth_winbind.c calls wbcAuthenticateUserEx()
b) as a domain member we require a valid response from winbindd,
   otherwise we'll return NT_STATUS_NO_LOGON_SERVERS
c) we call make_server_info_wbcAuthUserInfo(), which internally
   calls make_server_info_info3()
d) auth_check_ntlm_password() calls
   smb_pam_accountcheck(unix_username, rhost), where rhost
   is only an ipv4 or ipv6 address (without reverse dns lookup)
e) from auth3_check_password_send/auth3_check_password_recv()
   server_returned_info will be passed to auth3_generate_session_info(),
   triggered by gensec_session_info(), which means we'll call into
   create_local_token() in order to transform auth_serversupplied_info
   into auth_session_info.

For Kerberos gensec_session_info() will call
auth3_generate_session_info_pac() via the gensec_generate_session_info_pac()
helper function. The current logic is this:

a) gensec_generate_session_info_pac() is the function that
   evaluates the 'gensec:require_pac', which defaulted to 'no'
   before.
b) auth3_generate_session_info_pac() called
   wbcAuthenticateUserEx() in order to pass the PAC blob
   to winbindd, but only to prime its cache, e.g. netsamlogon cache
   and others. Most failures were just ignored.
c) If the PAC blob is available, it extracted the PAC_LOGON_INFO
   from it.
d) Then we called the horrible get_user_from_kerberos_info() function:
   - It uses a first part of the tickets principal name (before the @)
     as username and combines that with the 'logon_info->base.logon_domain'
     if the logon_info (PAC) is present.
   - As a fallback without a PAC it's tries to ask winbindd for a mapping
     from realm to netbios domain name.
   - Finally is falls back to using the realm as netbios domain name
   With this information is builds 'userdomain+winbind_separator+useraccount'
   and calls map_username() followed by smb_getpwnam() with create=true,
   Note this is similar to the make_server_info_info3() => check_account()
   => smb_getpwnam() logic under 3.
   - It also calls smb_pam_accountcheck(), but may pass the reverse DNS lookup name
     instead of the ip address as rhost.
   - It does some MAP_TO_GUEST_ON_BAD_UID logic and auto creates the
     guest account.
e) We called create_info3_from_pac_logon_info()
f) make_session_info_krb5() calls gets called and triggers this:
   - If get_user_from_kerberos_info() mapped to guest, it calls
     make_server_info_guest()
   - If create_info3_from_pac_logon_info() created a info3 from logon_info,
     it calls make_server_info_info3()
   - Without a PAC it tries pdb_getsampwnam()/make_server_info_sam() with
     a fallback to make_server_info_pw()
   From there it calls create_local_token()

I tried to change auth3_generate_session_info_pac() to behave similar
to auth_winbind.c together with auth3_generate_session_info() as
a domain member, as we now rely on a PAC:

a) As domain member we require a PAC and always call wbcAuthenticateUserEx()
   and require a valid response!
b) we call make_server_info_wbcAuthUserInfo(), which internally
   calls make_server_info_info3(). Note make_server_info_info3()
   handles MAP_TO_GUEST_ON_BAD_UID and make_server_info_guest()
   internally.
c) Similar to auth_check_ntlm_password() we now call
   smb_pam_accountcheck(unix_username, rhost), where rhost
   is only an ipv4 or ipv6 address (without reverse dns lookup)
d) From there it calls create_local_token()

As standalone server (in an MIT realm) we continue
with the already existing code logic, which works without a PAC:
a) we keep smb_getpwnam() with create=true logic as it
   also requires an explicit 'add user script' option.
b) In the following commits we assert that there's
   actually no PAC in this mode, which means we can
   remove unused and confusing code.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>

[abartlet@samba.org Backported due to change in structure
 initialization with { 0 } to zero ]
[abartlet@samba.org backported to 4.12 due to conflict
 with code not present to reload shared on krb5 login]
---
 source3/auth/auth_generic.c | 139 ++++++++++++++++++++++++++++--------
 1 file changed, 110 insertions(+), 29 deletions(-)

diff --git a/source3/auth/auth_generic.c b/source3/auth/auth_generic.c
index 191fe424aea..10db34306f6 100644
--- a/source3/auth/auth_generic.c
+++ b/source3/auth/auth_generic.c
@@ -44,6 +44,7 @@ static NTSTATUS auth3_generate_session_info_pac(struct auth4_context *auth_ctx,
 						uint32_t session_info_flags,
 						struct auth_session_info **session_info)
 {
+	enum server_role server_role = lp_server_role();
 	TALLOC_CTX *tmp_ctx;
 	struct PAC_LOGON_INFO *logon_info = NULL;
 	struct netr_SamInfo3 *info3_copy = NULL;
@@ -52,39 +53,59 @@ static NTSTATUS auth3_generate_session_info_pac(struct auth4_context *auth_ctx,
 	char *ntuser;
 	char *ntdomain;
 	char *username;
-	char *rhost;
+	const char *rhost;
 	struct passwd *pw;
 	NTSTATUS status;
-	int rc;
 
 	tmp_ctx = talloc_new(mem_ctx);
 	if (!tmp_ctx) {
 		return NT_STATUS_NO_MEMORY;
 	}
 
-	if (pac_blob) {
-#ifdef HAVE_KRB5
-		struct wbcAuthUserParams params = {};
+	if (tsocket_address_is_inet(remote_address, "ip")) {
+		rhost = tsocket_address_inet_addr_string(
+			remote_address, tmp_ctx);
+		if (rhost == NULL) {
+			status = NT_STATUS_NO_MEMORY;
+			goto done;
+		}
+	} else {
+		rhost = "127.0.0.1";
+	}
+
+	if (server_role != ROLE_STANDALONE) {
+		struct wbcAuthUserParams params = { 0 };
 		struct wbcAuthUserInfo *info = NULL;
 		struct wbcAuthErrorInfo *err = NULL;
+		struct auth_serversupplied_info *server_info = NULL;
+		char *original_user_name = NULL;
+		char *p = NULL;
 		wbcErr wbc_err;
 
+		if (pac_blob == NULL) {
+			/*
+			 * This should already be catched at the main
+			 * gensec layer, but better check twice
+			 */
+			status = NT_STATUS_INTERNAL_ERROR;
+			goto done;
+		}
+
 		/*
 		 * Let winbind decode the PAC.
 		 * This will also store the user
 		 * data in the netsamlogon cache.
 		 *
-		 * We need to do this *before* we
-		 * call get_user_from_kerberos_info()
-		 * as that does a user lookup that
-		 * expects info in the netsamlogon cache.
-		 *
-		 * See BUG: https://bugzilla.samba.org/show_bug.cgi?id=11259
+		 * This used to be a cache prime
+		 * optimization, but now we delegate
+		 * all logic to winbindd, as we require
+		 * winbindd as domain member anyway.
 		 */
 		params.level = WBC_AUTH_USER_LEVEL_PAC;
 		params.password.pac.data = pac_blob->data;
 		params.password.pac.length = pac_blob->length;
 
+		/* we are contacting the privileged pipe */
 		become_root();
 		wbc_err = wbcAuthenticateUserEx(&params, &info, &err);
 		unbecome_root();
@@ -97,18 +118,90 @@ static NTSTATUS auth3_generate_session_info_pac(struct auth4_context *auth_ctx,
 		 */
 
 		switch (wbc_err) {
-			case WBC_ERR_WINBIND_NOT_AVAILABLE:
 			case WBC_ERR_SUCCESS:
 				break;
+			case WBC_ERR_WINBIND_NOT_AVAILABLE:
+				status = NT_STATUS_NO_LOGON_SERVERS;
+				DBG_ERR("winbindd not running - "
+					"but required as domain member: %s\n",
+					nt_errstr(status));
+				goto done;
 			case WBC_ERR_AUTH_ERROR:
 				status = NT_STATUS(err->nt_status);
 				wbcFreeMemory(err);
 				goto done;
+			case WBC_ERR_NO_MEMORY:
+				status = NT_STATUS_NO_MEMORY;
+				goto done;
 			default:
 				status = NT_STATUS_LOGON_FAILURE;
 				goto done;
 		}
 
+		status = make_server_info_wbcAuthUserInfo(tmp_ctx,
+							  info->account_name,
+							  info->domain_name,
+							  info, &server_info);
+		if (!NT_STATUS_IS_OK(status)) {
+			DEBUG(10, ("make_server_info_wbcAuthUserInfo failed: %s\n",
+				   nt_errstr(status)));
+			goto done;
+		}
+
+		/* We skip doing this step if the caller asked us not to */
+		if (!(server_info->guest)) {
+			const char *unix_username = server_info->unix_name;
+
+			/* We might not be root if we are an RPC call */
+			become_root();
+			status = smb_pam_accountcheck(unix_username, rhost);
+			unbecome_root();
+
+			if (!NT_STATUS_IS_OK(status)) {
+				DEBUG(3, ("check_ntlm_password:  PAM Account for user [%s] "
+					  "FAILED with error %s\n",
+					  unix_username, nt_errstr(status)));
+				goto done;
+			}
+
+			DEBUG(5, ("check_ntlm_password:  PAM Account for user [%s] "
+				  "succeeded\n", unix_username));
+		}
+
+		DEBUG(3, ("Kerberos ticket principal name is [%s]\n", princ_name));
+
+		p = strchr_m(princ_name, '@');
+		if (!p) {
+			DEBUG(3, ("[%s] Doesn't look like a valid principal\n",
+				  princ_name));
+			status = NT_STATUS_LOGON_FAILURE;
+			goto done;
+		}
+
+		original_user_name = talloc_strndup(tmp_ctx, princ_name, p - princ_name);
+		if (original_user_name == NULL) {
+			status = NT_STATUS_NO_MEMORY;
+			goto done;
+		}
+
+		status = create_local_token(mem_ctx,
+					    server_info,
+					    NULL,
+					    original_user_name,
+					    session_info);
+		if (!NT_STATUS_IS_OK(status)) {
+			DEBUG(10, ("create_local_token failed: %s\n",
+				   nt_errstr(status)));
+			goto done;
+		}
+
+		goto session_info_ready;
+	}
+
+	/* This is the standalone legacy code path */
+
+	if (pac_blob != NULL) {
+#ifdef HAVE_KRB5
 		status = kerberos_pac_logon_info(tmp_ctx, *pac_blob, NULL, NULL,
 						 NULL, NULL, 0, &logon_info);
 #else
@@ -119,22 +212,6 @@ static NTSTATUS auth3_generate_session_info_pac(struct auth4_context *auth_ctx,
 		}
 	}
 
-	rc = get_remote_hostname(remote_address,
-				 &rhost,
-				 tmp_ctx);
-	if (rc < 0) {
-		status = NT_STATUS_NO_MEMORY;
-		goto done;
-	}
-	if (strequal(rhost, "UNKNOWN")) {
-		rhost = tsocket_address_inet_addr_string(remote_address,
-							 tmp_ctx);
-		if (rhost == NULL) {
-			status = NT_STATUS_NO_MEMORY;
-			goto done;
-		}
-	}
-
 	status = get_user_from_kerberos_info(tmp_ctx, rhost,
 					     princ_name, logon_info,
 					     &is_mapped, &is_guest,
@@ -168,6 +245,8 @@ static NTSTATUS auth3_generate_session_info_pac(struct auth4_context *auth_ctx,
 		goto done;
 	}
 
+session_info_ready:
+
 	/* setup the string used by %U */
 	set_current_user_info((*session_info)->unix_info->sanitized_username,
 			      (*session_info)->unix_info->unix_name,
@@ -177,7 +256,9 @@ static NTSTATUS auth3_generate_session_info_pac(struct auth4_context *auth_ctx,
 	lp_load_with_shares(get_dyn_CONFIGFILE());
 
 	DEBUG(5, (__location__ "OK: user: %s domain: %s client: %s\n",
-		  ntuser, ntdomain, rhost));
+		  (*session_info)->info->account_name,
+		  (*session_info)->info->domain_name,
+		  rhost));
 
 	status = NT_STATUS_OK;
 
-- 
2.33.1


From 59a6794b829a7fa30ea6ecc45b2a05e8e0ca3756 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Tue, 5 Oct 2021 17:14:01 +0200
Subject: [PATCH 17/20] CVE-2020-25717: selftest: configure 'ktest' env with
 winbindd and idmap_autorid

The 'ktest' environment was/is designed to test kerberos in an active
directory member setup. It was created at a time we wanted to test
smbd/winbindd with kerberos without having the source4 ad dc available.

This still applies to testing the build with system krb5 libraries
but without relying on a running ad dc.

As a domain member setup requires a running winbindd, we should test it
that way, in order to reflect a valid setup.

As a side effect it provides a way to demonstrate that we can accept
smb connections authenticated via kerberos, but no connection to
a domain controller! In order get this working offline, we need an
idmap backend with ID_TYPE_BOTH support, so we use 'autorid', which
should be the default choice.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>

[scabrero@samba.org Backported to 4.11 Run winbindd in offline mode
 but keep the user name mapping to avoid having to backport fixes
 for bso#14539]
---
 selftest/target/Samba3.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index 3a6426fabf3..707b05514a2 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -1006,7 +1006,7 @@ $ret->{USERNAME} = KTEST/Administrator
 	# access the share for tests.
 	chmod 0777, "$prefix/share";
 
-	if (not $self->check_or_start($ret, "yes", "no", "yes")) {
+	if (not $self->check_or_start($ret, "yes", "offline", "yes")) {
 	       return undef;
 	}
 	return $ret;
-- 
2.33.1


From 4a8e1449cd47ca10664acc31478b5b9d6095ff6c Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Tue, 5 Oct 2021 18:12:49 +0200
Subject: [PATCH 18/20] CVE-2020-25717: s3:auth: let
 auth3_generate_session_info_pac() reject a PAC in standalone mode

We should be strict in standalone mode, that we only support MIT realms
without a PAC in order to keep the code sane.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>

[abartlet@samba.org Backported to Samba 4.12 has conflcits
 as the share reload code is in a different spot]
---
 source3/auth/auth_generic.c | 29 +++++++++--------------------
 1 file changed, 9 insertions(+), 20 deletions(-)

diff --git a/source3/auth/auth_generic.c b/source3/auth/auth_generic.c
index 10db34306f6..fc15dd2b863 100644
--- a/source3/auth/auth_generic.c
+++ b/source3/auth/auth_generic.c
@@ -46,8 +46,6 @@ static NTSTATUS auth3_generate_session_info_pac(struct auth4_context *auth_ctx,
 {
 	enum server_role server_role = lp_server_role();
 	TALLOC_CTX *tmp_ctx;
-	struct PAC_LOGON_INFO *logon_info = NULL;
-	struct netr_SamInfo3 *info3_copy = NULL;
 	bool is_mapped;
 	bool is_guest;
 	char *ntuser;
@@ -201,19 +199,20 @@ static NTSTATUS auth3_generate_session_info_pac(struct auth4_context *auth_ctx,
 	/* This is the standalone legacy code path */
 
 	if (pac_blob != NULL) {
-#ifdef HAVE_KRB5
-		status = kerberos_pac_logon_info(tmp_ctx, *pac_blob, NULL, NULL,
-						 NULL, NULL, 0, &logon_info);
-#else
-		status = NT_STATUS_ACCESS_DENIED;
-#endif
+		/*
+		 * In standalone mode we don't expect a PAC!
+		 * we only support MIT realms
+		 */
+		status = NT_STATUS_BAD_TOKEN_TYPE;
+		DBG_WARNING("Unexpected PAC for [%s] in standalone mode - %s\n",
+			    princ_name, nt_errstr(status));
 		if (!NT_STATUS_IS_OK(status)) {
 			goto done;
 		}
 	}
 
 	status = get_user_from_kerberos_info(tmp_ctx, rhost,
-					     princ_name, logon_info,
+					     princ_name, NULL,
 					     &is_mapped, &is_guest,
 					     &ntuser, &ntdomain,
 					     &username, &pw);
@@ -224,19 +223,9 @@ static NTSTATUS auth3_generate_session_info_pac(struct auth4_context *auth_ctx,
 		goto done;
 	}
 
-	/* Get the info3 from the PAC data if we have it */
-	if (logon_info) {
-		status = create_info3_from_pac_logon_info(tmp_ctx,
-					logon_info,
-					&info3_copy);
-		if (!NT_STATUS_IS_OK(status)) {
-			goto done;
-		}
-	}
-
 	status = make_session_info_krb5(mem_ctx,
 					ntuser, ntdomain, username, pw,
-					info3_copy, is_guest, is_mapped, NULL /* No session key for now, caller will sort it out */,
+					NULL, is_guest, is_mapped, NULL /* No session key for now, caller will sort it out */,
 					session_info);
 	if (!NT_STATUS_IS_OK(status)) {
 		DEBUG(1, ("Failed to map kerberos pac to server info (%s)\n",
-- 
2.33.1


From e367f0d0439df1dc535e134b2b19d10a97c11eed Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Fri, 8 Oct 2021 17:59:59 +0200
Subject: [PATCH 19/20] CVE-2020-25717: s3:auth: simplify
 get_user_from_kerberos_info() by removing the unused logon_info argument

This code is only every called in standalone mode on a MIT realm,
it means we never have a PAC and we also don't have winbindd arround.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
---
 source3/auth/auth_generic.c |  2 +-
 source3/auth/proto.h        |  1 -
 source3/auth/user_krb5.c    | 57 +++++++------------------------------
 3 files changed, 11 insertions(+), 49 deletions(-)

diff --git a/source3/auth/auth_generic.c b/source3/auth/auth_generic.c
index fc15dd2b863..1b09730e573 100644
--- a/source3/auth/auth_generic.c
+++ b/source3/auth/auth_generic.c
@@ -212,7 +212,7 @@ static NTSTATUS auth3_generate_session_info_pac(struct auth4_context *auth_ctx,
 	}
 
 	status = get_user_from_kerberos_info(tmp_ctx, rhost,
-					     princ_name, NULL,
+					     princ_name,
 					     &is_mapped, &is_guest,
 					     &ntuser, &ntdomain,
 					     &username, &pw);
diff --git a/source3/auth/proto.h b/source3/auth/proto.h
index 5fd315846d8..7950522fac8 100644
--- a/source3/auth/proto.h
+++ b/source3/auth/proto.h
@@ -364,7 +364,6 @@ struct PAC_LOGON_INFO;
 NTSTATUS get_user_from_kerberos_info(TALLOC_CTX *mem_ctx,
 				     const char *cli_name,
 				     const char *princ_name,
-				     struct PAC_LOGON_INFO *logon_info,
 				     bool *is_mapped,
 				     bool *mapped_to_guest,
 				     char **ntuser,
diff --git a/source3/auth/user_krb5.c b/source3/auth/user_krb5.c
index c7bf2f2b9db..e3ea583b61f 100644
--- a/source3/auth/user_krb5.c
+++ b/source3/auth/user_krb5.c
@@ -31,7 +31,6 @@
 NTSTATUS get_user_from_kerberos_info(TALLOC_CTX *mem_ctx,
 				     const char *cli_name,
 				     const char *princ_name,
-				     struct PAC_LOGON_INFO *logon_info,
 				     bool *is_mapped,
 				     bool *mapped_to_guest,
 				     char **ntuser,
@@ -40,8 +39,8 @@ NTSTATUS get_user_from_kerberos_info(TALLOC_CTX *mem_ctx,
 				     struct passwd **_pw)
 {
 	NTSTATUS status;
-	char *domain = NULL;
-	char *realm = NULL;
+	const char *domain = NULL;
+	const char *realm = NULL;
 	char *user = NULL;
 	char *p;
 	char *fuser = NULL;
@@ -62,55 +61,16 @@ NTSTATUS get_user_from_kerberos_info(TALLOC_CTX *mem_ctx,
 		return NT_STATUS_NO_MEMORY;
 	}
 
-	realm = talloc_strdup(talloc_tos(), p + 1);
-	if (!realm) {
-		return NT_STATUS_NO_MEMORY;
-	}
+	realm = p + 1;
 
 	if (!strequal(realm, lp_realm())) {
 		DEBUG(3, ("Ticket for foreign realm %s@%s\n", user, realm));
 		if (!lp_allow_trusted_domains()) {
 			return NT_STATUS_LOGON_FAILURE;
 		}
-	}
-
-	if (logon_info && logon_info->info3.base.logon_domain.string) {
-		domain = talloc_strdup(mem_ctx,
-					logon_info->info3.base.logon_domain.string);
-		if (!domain) {
-			return NT_STATUS_NO_MEMORY;
-		}
-		DEBUG(10, ("Domain is [%s] (using PAC)\n", domain));
+		domain = realm;
 	} else {
-
-		/* If we have winbind running, we can (and must) shorten the
-		   username by using the short netbios name. Otherwise we will
-		   have inconsistent user names. With Kerberos, we get the
-		   fully qualified realm, with ntlmssp we get the short
-		   name. And even w2k3 does use ntlmssp if you for example
-		   connect to an ip address. */
-
-		wbcErr wbc_status;
-		struct wbcDomainInfo *info = NULL;
-
-		DEBUG(10, ("Mapping [%s] to short name using winbindd\n",
-			   realm));
-
-		wbc_status = wbcDomainInfo(realm, &info);
-
-		if (WBC_ERROR_IS_OK(wbc_status)) {
-			domain = talloc_strdup(mem_ctx,
-						info->short_name);
-			wbcFreeMemory(info);
-		} else {
-			DEBUG(3, ("Could not find short name: %s\n",
-				  wbcErrorString(wbc_status)));
-			domain = talloc_strdup(mem_ctx, realm);
-		}
-		if (!domain) {
-			return NT_STATUS_NO_MEMORY;
-		}
-		DEBUG(10, ("Domain is [%s] (using Winbind)\n", domain));
+		domain = lp_workgroup();
 	}
 
 	fuser = talloc_asprintf(mem_ctx,
@@ -175,7 +135,11 @@ NTSTATUS get_user_from_kerberos_info(TALLOC_CTX *mem_ctx,
 		return NT_STATUS_NO_MEMORY;
 	}
 	*ntuser = user;
-	*ntdomain = domain;
+	*ntdomain = talloc_strdup(mem_ctx, domain);
+	if (*ntdomain == NULL) {
+		return NT_STATUS_NO_MEMORY;
+	}
+
 	*_pw = pw;
 
 	return NT_STATUS_OK;
@@ -282,7 +246,6 @@ NTSTATUS make_session_info_krb5(TALLOC_CTX *mem_ctx,
 NTSTATUS get_user_from_kerberos_info(TALLOC_CTX *mem_ctx,
 				     const char *cli_name,
 				     const char *princ_name,
-				     struct PAC_LOGON_INFO *logon_info,
 				     bool *is_mapped,
 				     bool *mapped_to_guest,
 				     char **ntuser,
-- 
2.33.1


From 3f59f5003edd0a605a983fc9fc5d85e8625ae71f Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Fri, 8 Oct 2021 18:03:04 +0200
Subject: [PATCH 20/20] CVE-2020-25717: s3:auth: simplify
 make_session_info_krb5() by removing unused arguments

This is only ever be called in standalone mode with an MIT realm,
so we don't have a PAC/info3 structure.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
---
 source3/auth/auth_generic.c |  2 +-
 source3/auth/proto.h        |  2 --
 source3/auth/user_krb5.c    | 20 +-------------------
 3 files changed, 2 insertions(+), 22 deletions(-)

diff --git a/source3/auth/auth_generic.c b/source3/auth/auth_generic.c
index 1b09730e573..30171fa8ee4 100644
--- a/source3/auth/auth_generic.c
+++ b/source3/auth/auth_generic.c
@@ -225,7 +225,7 @@ static NTSTATUS auth3_generate_session_info_pac(struct auth4_context *auth_ctx,
 
 	status = make_session_info_krb5(mem_ctx,
 					ntuser, ntdomain, username, pw,
-					NULL, is_guest, is_mapped, NULL /* No session key for now, caller will sort it out */,
+					is_guest, is_mapped,
 					session_info);
 	if (!NT_STATUS_IS_OK(status)) {
 		DEBUG(1, ("Failed to map kerberos pac to server info (%s)\n",
diff --git a/source3/auth/proto.h b/source3/auth/proto.h
index 7950522fac8..57c03eb9705 100644
--- a/source3/auth/proto.h
+++ b/source3/auth/proto.h
@@ -375,9 +375,7 @@ NTSTATUS make_session_info_krb5(TALLOC_CTX *mem_ctx,
 				char *ntdomain,
 				char *username,
 				struct passwd *pw,
-				const struct netr_SamInfo3 *info3,
 				bool mapped_to_guest, bool username_was_mapped,
-				DATA_BLOB *session_key,
 				struct auth_session_info **session_info);
 
 /* The following definitions come from auth/auth_samba4.c  */
diff --git a/source3/auth/user_krb5.c b/source3/auth/user_krb5.c
index e3ea583b61f..9f7da831adc 100644
--- a/source3/auth/user_krb5.c
+++ b/source3/auth/user_krb5.c
@@ -150,9 +150,7 @@ NTSTATUS make_session_info_krb5(TALLOC_CTX *mem_ctx,
 				char *ntdomain,
 				char *username,
 				struct passwd *pw,
-				const struct netr_SamInfo3 *info3,
 				bool mapped_to_guest, bool username_was_mapped,
-				DATA_BLOB *session_key,
 				struct auth_session_info **session_info)
 {
 	NTSTATUS status;
@@ -166,20 +164,6 @@ NTSTATUS make_session_info_krb5(TALLOC_CTX *mem_ctx,
 			return status;
 		}
 
-	} else if (info3) {
-		/* pass the unmapped username here since map_username()
-		   will be called again in make_server_info_info3() */
-
-		status = make_server_info_info3(mem_ctx,
-						ntuser, ntdomain,
-						&server_info,
-						info3);
-		if (!NT_STATUS_IS_OK(status)) {
-			DEBUG(1, ("make_server_info_info3 failed: %s!\n",
-				  nt_errstr(status)));
-			return status;
-		}
-
 	} else {
 		/*
 		 * We didn't get a PAC, we have to make up the user
@@ -231,7 +215,7 @@ NTSTATUS make_session_info_krb5(TALLOC_CTX *mem_ctx,
 
 	server_info->nss_token |= username_was_mapped;
 
-	status = create_local_token(mem_ctx, server_info, session_key, ntuser, session_info);
+	status = create_local_token(mem_ctx, server_info, NULL, ntuser, session_info);
 	talloc_free(server_info);
 	if (!NT_STATUS_IS_OK(status)) {
 		DEBUG(10,("failed to create local token: %s\n",
@@ -261,9 +245,7 @@ NTSTATUS make_session_info_krb5(TALLOC_CTX *mem_ctx,
 				char *ntdomain,
 				char *username,
 				struct passwd *pw,
-				const struct netr_SamInfo3 *info3,
 				bool mapped_to_guest, bool username_was_mapped,
-				DATA_BLOB *session_key,
 				struct auth_session_info **session_info)
 {
 	return NT_STATUS_NOT_IMPLEMENTED;
-- 
2.33.1