The Samba-Bugzilla – Attachment 16958 Details for
Bug 14556
CVE-2020-25717 [SECURITY] A user on the domain can become root on domain members
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Patches for v4-6-test
CVE-2020-25717-only-4.6.patch (text/plain), 58.49 KB, created by
Samuel Cabrero
on 2021-11-05 08:23:11 UTC
(
hide
)
Description:
Patches for v4-6-test
Filename:
MIME Type:
Creator:
Samuel Cabrero
Created:
2021-11-05 08:23:11 UTC
Size:
58.49 KB
patch
obsolete
>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(¶ms, &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 >
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=16958">View the attachment on a separate page</a>.</b>
View Attachment As Raw
Actions:
View
Attachments on
bug 14556
:
16845
|
16924
|
16926
|
16934
|
16952
|
16953
|
16954
|
16955
|
16956
| 16958 |
16959
|
16963
|
16969
|
16970
|
16978
|
17449