From 5ec536b3549edf118d2f88ca8a9265878a7c9fc5 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 8 Oct 2021 18:04:55 +0200 Subject: [PATCH 01/39] selftest/Samba3: remove unused close(USERMAP); calls BUG: https://bugzilla.samba.org/show_bug.cgi?id=14869 BUG: https://bugzilla.samba.org/show_bug.cgi?id=14881 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett [abartlet@samba.org backported from commit d998f7f8df215866ab32e05be772e24fc0b2131c as offline login tests are not in Samba 4.14] --- selftest/target/Samba3.pm | 4 ---- 1 file changed, 4 deletions(-) diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm index b0910433940..914b8d40c89 100755 --- a/selftest/target/Samba3.pm +++ b/selftest/target/Samba3.pm @@ -771,7 +771,6 @@ sub provision_ad_member mkdir($_, 0777) foreach(@dirs); - close(USERMAP); $ret->{DOMAIN} = $dcvars->{DOMAIN}; $ret->{REALM} = $dcvars->{REALM}; $ret->{DOMSID} = $dcvars->{DOMSID}; @@ -920,7 +919,6 @@ sub setup_ad_member_rfc2307 $ret or return undef; - close(USERMAP); $ret->{DOMAIN} = $dcvars->{DOMAIN}; $ret->{REALM} = $dcvars->{REALM}; $ret->{DOMSID} = $dcvars->{DOMSID}; @@ -1018,7 +1016,6 @@ sub setup_ad_member_idmap_rid $ret or return undef; - close(USERMAP); $ret->{DOMAIN} = $dcvars->{DOMAIN}; $ret->{REALM} = $dcvars->{REALM}; $ret->{DOMSID} = $dcvars->{DOMSID}; @@ -1118,7 +1115,6 @@ sub setup_ad_member_idmap_ad $ret or return undef; - close(USERMAP); $ret->{DOMAIN} = $dcvars->{DOMAIN}; $ret->{REALM} = $dcvars->{REALM}; $ret->{DOMSID} = $dcvars->{DOMSID}; -- 2.33.1 From 6ad8a242a682abd7baa1e4d3672bb24c9e65ab0b Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 5 Oct 2021 16:42:00 +0200 Subject: [PATCH 02/39] 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 Reviewed-by: Andrew Bartlett (cherry picked from commit 4dc3c68c9a28f71888e3d6dd3b1f0bcdb8fa45de) --- selftest/target/Samba3.pm | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm index 914b8d40c89..9534d06b597 100755 --- a/selftest/target/Samba3.pm +++ b/selftest/target/Samba3.pm @@ -827,7 +827,7 @@ sub provision_ad_member nmbd => "yes", winbindd => "yes", smbd => "yes")) { - return undef; + return undef; } $ret->{DC_SERVER} = $dcvars->{SERVER}; @@ -1909,7 +1909,7 @@ sub check_or_start($$) { LOG_FILE => $env_vars->{WINBINDD_TEST_LOG}, PCAP_FILE => "env-$ENV{ENVNAME}-winbindd", }; - if ($winbindd ne "yes") { + if ($winbindd ne "yes" and $winbindd ne "offline") { $daemon_ctx->{SKIP_DAEMON} = 1; } @@ -3130,13 +3130,17 @@ sub wait_for_start($$$$$) } } - if ($winbindd eq "yes") { + if ($winbindd eq "yes" or $winbindd eq "offline") { print "checking for winbindd\n"; my $count = 0; $cmd = "SELFTEST_WINBINDD_SOCKET_DIR='$envvars->{SELFTEST_WINBINDD_SOCKET_DIR}' "; $cmd .= "NSS_WRAPPER_PASSWD='$envvars->{NSS_WRAPPER_PASSWD}' "; $cmd .= "NSS_WRAPPER_GROUP='$envvars->{NSS_WRAPPER_GROUP}' "; - $cmd .= Samba::bindir_path($self, "wbinfo") . " --ping-dc"; + if ($winbindd eq "yes") { + $cmd .= Samba::bindir_path($self, "wbinfo") . " --ping-dc"; + } elsif ($winbindd eq "offline") { + $cmd .= Samba::bindir_path($self, "wbinfo") . " --ping"; + } do { $ret = system($cmd); -- 2.33.1 From c25f4625813b0f7f3baf33ce17c3d6691c9fd602 Mon Sep 17 00:00:00 2001 From: Andreas Schneider Date: Tue, 11 May 2021 17:59:51 +0200 Subject: [PATCH 03/39] CVE-2020-25717 selftest: Pass down the machine account name to provision_ad_member Signed-off-by: Andreas Schneider Reviewed-by: Jeremy Allison BUG: https://bugzilla.samba.org/show_bug.cgi?id=14556 (cherry picked from commit fbe68dcbb783409589cdefd8ee551c9971c51f08) Needed as preparation for CVE-2020-25717 --- selftest/target/Samba.pm | 1 + selftest/target/Samba3.pm | 10 ++++++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/selftest/target/Samba.pm b/selftest/target/Samba.pm index 5a7efa9c280..d48ac2c64a1 100644 --- a/selftest/target/Samba.pm +++ b/selftest/target/Samba.pm @@ -588,6 +588,7 @@ sub get_interface($) addcsmb1 => 54, lclnt4dc2smb1 => 55, fipsdc => 56, + fipsadmember => 57, rootdnsforwarder => 64, diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm index 9534d06b597..4686f5d36aa 100755 --- a/selftest/target/Samba3.pm +++ b/selftest/target/Samba3.pm @@ -646,6 +646,7 @@ sub provision_ad_member { my ($self, $prefix, + $machine_account, $dcvars, $trustvars_f, $trustvars_e, @@ -762,7 +763,7 @@ sub provision_ad_member prefix => $prefix, domain => $dcvars->{DOMAIN}, realm => $dcvars->{REALM}, - server => "LOCALADMEMBER", + server => $machine_account, password => "loCalMemberPass", extra_options => $member_options, resolv_conf => $dcvars->{RESOLV_CONF}); @@ -876,7 +877,11 @@ sub setup_ad_member print "PROVISIONING AD MEMBER..."; - return $self->provision_ad_member($prefix, $dcvars, $trustvars_f, $trustvars_e); + return $self->provision_ad_member($prefix, + "LOCALADMEMBER", + $dcvars, + $trustvars_f, + $trustvars_e); } sub setup_ad_member_rfc2307 @@ -1199,6 +1204,7 @@ sub setup_ad_member_fips print "PROVISIONING AD FIPS MEMBER..."; return $self->provision_ad_member($prefix, + "FIPSADMEMBER", $dcvars, $trustvars_f, $trustvars_e, -- 2.33.1 From 449503537d409f5774433ea19f520f560bfff981 Mon Sep 17 00:00:00 2001 From: Andreas Schneider Date: Thu, 10 Jun 2021 16:20:28 +0200 Subject: [PATCH 04/39] CVE-2020-25717 selftest: Only set netbios aliases for the ad_member env The provision_ad_member() function is reused by different setup_ad_member*() functions. Each environment needs to have unique netbios aliases as they are all in the same network. The aliases should only be set for the 'ad_member' environment. Signed-Off-By: Andreas Schneider Reviewed-by: Jeremy Allison Autobuild-User(master): Jeremy Allison Autobuild-Date(master): Fri Jun 11 01:26:36 UTC 2021 on sn-devel-184 BUG: https://bugzilla.samba.org/show_bug.cgi?id=14556 (cherry picked from commit e165dcc770ec58c3749d653d6cb85f6ecf9479d6) --- selftest/target/Samba3.pm | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm index 4686f5d36aa..5c0f71757ef 100755 --- a/selftest/target/Samba3.pm +++ b/selftest/target/Samba3.pm @@ -682,11 +682,17 @@ sub provision_ad_member $substitution_path = "$share_dir/D_$dcvars->{DOMAIN}/u_$dcvars->{DOMAIN}/alice/g_$dcvars->{DOMAIN}/domain users"; push(@dirs, $substitution_path); + + my $netbios_aliases = ""; + if ($machine_account eq "LOCALADMEMBER") { + $netbios_aliases = "netbios aliases = foo bar"; + } + my $member_options = " security = ads workgroup = $dcvars->{DOMAIN} realm = $dcvars->{REALM} - netbios aliases = foo bar + $netbios_aliases template homedir = /home/%D/%G/%U auth event notification = true password server = $dcvars->{SERVER} -- 2.33.1 From 930722757311af95dbd516222e21a4a107f11ce4 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Wed, 14 Apr 2021 10:05:59 +0200 Subject: [PATCH 05/39] CVE-2020-25717 auth3: Simplify check_samba4_security() First set up "server_info" in a local variable and once it's fully set up, assign it to the out parameter "pserver_info". Pointer dereferencing obfuscates the code for me. Signed-off-by: Volker Lendecke Reviewed-by: Jeremy Allison BUG: https://bugzilla.samba.org/show_bug.cgi?id=14556 (cherry picked from commit 062a0c14c6ee0b74e7619af73747df59c5e67672) --- source3/auth/auth_samba4.c | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/source3/auth/auth_samba4.c b/source3/auth/auth_samba4.c index 0a7c67be232..ac141354747 100644 --- a/source3/auth/auth_samba4.c +++ b/source3/auth/auth_samba4.c @@ -108,11 +108,12 @@ static struct server_id *new_server_id_task(TALLOC_CTX *mem_ctx) * services the AD DC. It is tested via pdbtest. */ -static NTSTATUS check_samba4_security(const struct auth_context *auth_context, - void *my_private_data, - TALLOC_CTX *mem_ctx, - const struct auth_usersupplied_info *user_info, - struct auth_serversupplied_info **server_info) +static NTSTATUS check_samba4_security( + const struct auth_context *auth_context, + void *my_private_data, + TALLOC_CTX *mem_ctx, + const struct auth_usersupplied_info *user_info, + struct auth_serversupplied_info **pserver_info) { TALLOC_CTX *frame = talloc_stackframe(); struct netr_SamInfo3 *info3 = NULL; @@ -120,6 +121,7 @@ static NTSTATUS check_samba4_security(const struct auth_context *auth_context, struct auth_user_info_dc *user_info_dc; struct auth4_context *auth4_context; uint8_t authoritative = 0; + struct auth_serversupplied_info *server_info = NULL; nt_status = make_auth4_context_s4(auth_context, mem_ctx, &auth4_context); if (!NT_STATUS_IS_OK(nt_status)) { @@ -161,17 +163,19 @@ static NTSTATUS check_samba4_security(const struct auth_context *auth_context, } if (user_info->flags & USER_INFO_INFO3_AND_NO_AUTHZ) { - *server_info = make_server_info(mem_ctx); - if (*server_info == NULL) { + server_info = make_server_info(mem_ctx); + if (server_info == NULL) { nt_status = NT_STATUS_NO_MEMORY; goto done; } - (*server_info)->info3 = talloc_steal(*server_info, info3); - + server_info->info3 = talloc_move(server_info, &info3); } else { - nt_status = make_server_info_info3(mem_ctx, user_info->client.account_name, - user_info->mapped.domain_name, server_info, - info3); + nt_status = make_server_info_info3( + mem_ctx, + user_info->client.account_name, + user_info->mapped.domain_name, + &server_info, + info3); if (!NT_STATUS_IS_OK(nt_status)) { DEBUG(10, ("make_server_info_info3 failed: %s\n", nt_errstr(nt_status))); @@ -179,6 +183,7 @@ static NTSTATUS check_samba4_security(const struct auth_context *auth_context, } } + *pserver_info = server_info; nt_status = NT_STATUS_OK; done: -- 2.33.1 From bf52e64053a0f4369d005782d2f9a75d03328ddb Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Tue, 13 Apr 2021 15:14:01 +0000 Subject: [PATCH 06/39] CVE-2020-25717 auth: Simplify DEBUG statements in make_auth3_context_for_ntlm() Signed-off-by: Volker Lendecke Reviewed-by: Jeremy Allison BUG: https://bugzilla.samba.org/show_bug.cgi?id=14801 BUG: https://bugzilla.samba.org/show_bug.cgi?id=14556 (cherry picked from commit 8536bf7fce41c43bbed25f7ed4ce5775a1b9c0d5) --- source3/auth/auth.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/source3/auth/auth.c b/source3/auth/auth.c index fa73c4e4db2..6d2a40f0f29 100644 --- a/source3/auth/auth.c +++ b/source3/auth/auth.c @@ -530,28 +530,28 @@ NTSTATUS make_auth3_context_for_ntlm(TALLOC_CTX *mem_ctx, struct auth_context **auth_context) { const char *methods = NULL; + const char *role = NULL; switch (lp_server_role()) { case ROLE_ACTIVE_DIRECTORY_DC: - DEBUG(5,("Making default auth method list for server role = " - "'active directory domain controller'\n")); + role = "'active directory domain controller'"; methods = "samba4"; break; case ROLE_DOMAIN_MEMBER: - DEBUG(5,("Making default auth method list for server role = 'domain member'\n")); + role = "'domain member'"; methods = "anonymous sam winbind sam_ignoredomain"; break; case ROLE_DOMAIN_BDC: case ROLE_DOMAIN_PDC: - DEBUG(5,("Making default auth method list for DC\n")); + role = "'DC'"; methods = "anonymous sam winbind sam_ignoredomain"; break; case ROLE_STANDALONE: - DEBUG(5,("Making default auth method list for server role = 'standalone server', encrypt passwords = yes\n")); if (lp_encrypt_passwords()) { + role = "'standalone server', encrypt passwords = yes"; methods = "anonymous sam_ignoredomain"; } else { - DEBUG(5,("Making default auth method list for server role = 'standalone server', encrypt passwords = no\n")); + role = "'standalone server', encrypt passwords = no"; methods = "anonymous unix"; } break; @@ -560,6 +560,9 @@ NTSTATUS make_auth3_context_for_ntlm(TALLOC_CTX *mem_ctx, return NT_STATUS_UNSUCCESSFUL; } + DBG_INFO("Making default auth method list for server role = %s\n", + role); + return make_auth_context_specific(mem_ctx, auth_context, methods); } -- 2.33.1 From fe1188475e15269321a62b4d11cc520e5b7d0c6e Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Wed, 14 Apr 2021 21:48:32 +0200 Subject: [PATCH 07/39] CVE-2020-25717 auth4: Make auth_anonymous pseudo-async Signed-off-by: Volker Lendecke Reviewed-by: Andrew Bartlett BUG: https://bugzilla.samba.org/show_bug.cgi?id=14556 (cherry picked from commit 759573136876ef2b1b1c7484f99570d7de957e0d) --- source4/auth/ntlm/auth_anonymous.c | 66 ++++++++++++++++++++++++++---- source4/auth/ntlm/wscript_build | 2 +- 2 files changed, 58 insertions(+), 10 deletions(-) diff --git a/source4/auth/ntlm/auth_anonymous.c b/source4/auth/ntlm/auth_anonymous.c index 83aeb431f5f..a25aacaa137 100644 --- a/source4/auth/ntlm/auth_anonymous.c +++ b/source4/auth/ntlm/auth_anonymous.c @@ -20,9 +20,11 @@ */ #include "includes.h" +#include #include "auth/auth.h" #include "auth/ntlm/auth_proto.h" #include "param/param.h" +#include "lib/util/tevent_ntstatus.h" #undef DBGC_CLASS #define DBGC_CLASS DBGC_AUTH @@ -84,19 +86,65 @@ static NTSTATUS anonymous_want_check(struct auth_method_context *ctx, * anonymou logons to be dealt with in one place. Non-anonymou logons 'fail' * and pass onto the next module. **/ -static NTSTATUS anonymous_check_password(struct auth_method_context *ctx, - TALLOC_CTX *mem_ctx, - const struct auth_usersupplied_info *user_info, - struct auth_user_info_dc **_user_info_dc, - bool *authoritative) + +struct anonymous_check_password_state { + struct auth_user_info_dc *user_info_dc; +}; + +static struct tevent_req *anonymous_check_password_send( + TALLOC_CTX *mem_ctx, + struct tevent_context *ev, + struct auth_method_context *ctx, + const struct auth_usersupplied_info *user_info) +{ + struct tevent_req *req = NULL; + struct anonymous_check_password_state *state = NULL; + NTSTATUS status; + + req = tevent_req_create( + mem_ctx, + &state, + struct anonymous_check_password_state); + if (req == NULL) { + return NULL; + } + + status = auth_anonymous_user_info_dc( + state, + lpcfg_netbios_name(ctx->auth_ctx->lp_ctx), + &state->user_info_dc); + if (tevent_req_nterror(req, status)) { + return tevent_req_post(req, ev); + } + tevent_req_done(req); + return tevent_req_post(req, ev); +} + +static NTSTATUS anonymous_check_password_recv( + struct tevent_req *req, + TALLOC_CTX *mem_ctx, + struct auth_user_info_dc **interim_info, + bool *authoritative) { - return auth_anonymous_user_info_dc(mem_ctx, lpcfg_netbios_name(ctx->auth_ctx->lp_ctx), _user_info_dc); + struct anonymous_check_password_state *state = tevent_req_data( + req, struct anonymous_check_password_state); + NTSTATUS status; + + if (tevent_req_is_nterror(req, &status)) { + tevent_req_received(req); + return status; + } + *interim_info = talloc_move(mem_ctx, &state->user_info_dc); + tevent_req_received(req); + return NT_STATUS_OK; } + static const struct auth_operations anonymous_auth_ops = { - .name = "anonymous", - .want_check = anonymous_want_check, - .check_password = anonymous_check_password + .name = "anonymous", + .want_check = anonymous_want_check, + .check_password_send = anonymous_check_password_send, + .check_password_recv = anonymous_check_password_recv, }; _PUBLIC_ NTSTATUS auth4_anonymous_init(TALLOC_CTX *ctx) diff --git a/source4/auth/ntlm/wscript_build b/source4/auth/ntlm/wscript_build index 04a760c3e49..6ea0c4d7e3a 100644 --- a/source4/auth/ntlm/wscript_build +++ b/source4/auth/ntlm/wscript_build @@ -12,7 +12,7 @@ bld.SAMBA_MODULE('auth4_anonymous', source='auth_anonymous.c', subsystem='auth4', init_function='auth4_anonymous_init', - deps='talloc' + deps='tevent' ) -- 2.33.1 From c72dd26cd46ca9dc7e68649f444accf0da9b7d51 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Wed, 14 Apr 2021 22:22:18 +0200 Subject: [PATCH 08/39] CVE-2020-25717 auth4: Make auth_developer pseudo-async This is a simpler approach to really just wrap the code. Signed-off-by: Volker Lendecke Reviewed-by: Andrew Bartlett BUG: https://bugzilla.samba.org/show_bug.cgi?id=14556 (cherry picked from commit 43a1e42815718591faa8d526319b96d089a758fa) --- source4/auth/ntlm/auth_developer.c | 61 +++++++++++++++++++++++++++++- source4/auth/ntlm/wscript_build | 2 +- 2 files changed, 61 insertions(+), 2 deletions(-) diff --git a/source4/auth/ntlm/auth_developer.c b/source4/auth/ntlm/auth_developer.c index 209786b63b2..1823989c68d 100644 --- a/source4/auth/ntlm/auth_developer.c +++ b/source4/auth/ntlm/auth_developer.c @@ -20,9 +20,11 @@ */ #include "includes.h" +#include #include "auth/auth.h" #include "auth/ntlm/auth_proto.h" #include "libcli/security/security.h" +#include "lib/util/tevent_ntstatus.h" #undef DBGC_CLASS #define DBGC_CLASS DBGC_AUTH @@ -137,10 +139,67 @@ static NTSTATUS name_to_ntstatus_check_password(struct auth_method_context *ctx, return nt_status; } +struct name_to_ntstatus_check_password_state { + struct auth_user_info_dc *user_info_dc; + bool authoritative; +}; + +static struct tevent_req *name_to_ntstatus_check_password_send( + TALLOC_CTX *mem_ctx, + struct tevent_context *ev, + struct auth_method_context *ctx, + const struct auth_usersupplied_info *user_info) +{ + struct tevent_req *req = NULL; + struct name_to_ntstatus_check_password_state *state = NULL; + NTSTATUS status; + + req = tevent_req_create( + mem_ctx, + &state, + struct name_to_ntstatus_check_password_state); + if (req == NULL) { + return NULL; + } + + status = name_to_ntstatus_check_password( + ctx, + state, + user_info, + &state->user_info_dc, + &state->authoritative); + if (tevent_req_nterror(req, status)) { + return tevent_req_post(req, ev); + } + tevent_req_done(req); + return tevent_req_post(req, ev); +} + +static NTSTATUS name_to_ntstatus_check_password_recv( + struct tevent_req *req, + TALLOC_CTX *mem_ctx, + struct auth_user_info_dc **interim_info, + bool *authoritative) +{ + struct name_to_ntstatus_check_password_state *state = tevent_req_data( + req, struct name_to_ntstatus_check_password_state); + NTSTATUS status; + + if (tevent_req_is_nterror(req, &status)) { + tevent_req_received(req); + return status; + } + *interim_info = talloc_move(mem_ctx, &state->user_info_dc); + *authoritative = state->authoritative; + tevent_req_received(req); + return NT_STATUS_OK; +} + static const struct auth_operations name_to_ntstatus_auth_ops = { .name = "name_to_ntstatus", .want_check = name_to_ntstatus_want_check, - .check_password = name_to_ntstatus_check_password + .check_password_send = name_to_ntstatus_check_password_send, + .check_password_recv = name_to_ntstatus_check_password_recv, }; _PUBLIC_ NTSTATUS auth4_developer_init(TALLOC_CTX *ctx) diff --git a/source4/auth/ntlm/wscript_build b/source4/auth/ntlm/wscript_build index 6ea0c4d7e3a..1ee8d79563a 100644 --- a/source4/auth/ntlm/wscript_build +++ b/source4/auth/ntlm/wscript_build @@ -28,7 +28,7 @@ bld.SAMBA_MODULE('auth4_developer', source='auth_developer.c', subsystem='auth4', init_function='auth4_developer_init', - deps='talloc' + deps='tevent' ) -- 2.33.1 From a5f3c5d48c490c637cf4e5854dbd01a585c9c0eb Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Wed, 14 Apr 2021 21:59:55 +0200 Subject: [PATCH 09/39] CVE-2020-25717 auth4: Make auth_unix pseudo-async Signed-off-by: Volker Lendecke Reviewed-by: Andrew Bartlett BUG: https://bugzilla.samba.org/show_bug.cgi?id=14556 (cherry picked from commit a6f42ab8a778b9863990da3112c2e868cd006303) --- source4/auth/ntlm/auth_unix.c | 85 ++++++++++++++++++++++++----------- 1 file changed, 59 insertions(+), 26 deletions(-) diff --git a/source4/auth/ntlm/auth_unix.c b/source4/auth/ntlm/auth_unix.c index 67cd5f3dc44..cfe4f1a073f 100644 --- a/source4/auth/ntlm/auth_unix.c +++ b/source4/auth/ntlm/auth_unix.c @@ -27,6 +27,7 @@ #include "lib/tsocket/tsocket.h" #include "../libcli/auth/pam_errors.h" #include "param/param.h" +#include "lib/util/tevent_ntstatus.h" #undef DBGC_CLASS #define DBGC_CLASS DBGC_AUTH @@ -713,46 +714,78 @@ static NTSTATUS authunix_want_check(struct auth_method_context *ctx, return NT_STATUS_OK; } -static NTSTATUS authunix_check_password(struct auth_method_context *ctx, - TALLOC_CTX *mem_ctx, - const struct auth_usersupplied_info *user_info, - struct auth_user_info_dc **user_info_dc, - bool *authoritative) +struct authunix_check_password_state { + struct auth_user_info_dc *user_info_dc; +}; + +static struct tevent_req *authunix_check_password_send( + TALLOC_CTX *mem_ctx, + struct tevent_context *ev, + struct auth_method_context *ctx, + const struct auth_usersupplied_info *user_info) { - TALLOC_CTX *check_ctx; - NTSTATUS nt_status; - struct passwd *pwd; + struct tevent_req *req = NULL; + struct authunix_check_password_state *state = NULL; + struct passwd *pwd = NULL; + NTSTATUS status; - if (user_info->password_state != AUTH_PASSWORD_PLAIN) { - return NT_STATUS_INVALID_PARAMETER; + req = tevent_req_create( + mem_ctx, + &state, + struct authunix_check_password_state); + if (req == NULL) { + return NULL; } - check_ctx = talloc_named_const(mem_ctx, 0, "check_unix_password"); - if (check_ctx == NULL) { - return NT_STATUS_NO_MEMORY; + if (user_info->password_state != AUTH_PASSWORD_PLAIN) { + tevent_req_nterror(req, NT_STATUS_INVALID_PARAMETER); + return tevent_req_post(req, ev); } - nt_status = check_unix_password(check_ctx, ctx->auth_ctx->lp_ctx, user_info, &pwd); - if (!NT_STATUS_IS_OK(nt_status)) { - talloc_free(check_ctx); - return nt_status; + status = check_unix_password( + state, ctx->auth_ctx->lp_ctx, user_info, &pwd); + if (tevent_req_nterror(req, status)) { + return tevent_req_post(req, ev); } - nt_status = authunix_make_user_info_dc(mem_ctx, lpcfg_netbios_name(ctx->auth_ctx->lp_ctx), - user_info, pwd, user_info_dc); - if (!NT_STATUS_IS_OK(nt_status)) { - talloc_free(check_ctx); - return nt_status; + status = authunix_make_user_info_dc( + state, + lpcfg_netbios_name(ctx->auth_ctx->lp_ctx), + user_info, + pwd, + &state->user_info_dc); + if (tevent_req_nterror(req, status)) { + return tevent_req_post(req, ev); } - talloc_free(check_ctx); + tevent_req_done(req); + return tevent_req_post(req, ev); +} + +static NTSTATUS authunix_check_password_recv( + struct tevent_req *req, + TALLOC_CTX *mem_ctx, + struct auth_user_info_dc **interim_info, + bool *authoritative) +{ + struct authunix_check_password_state *state = tevent_req_data( + req, struct authunix_check_password_state); + NTSTATUS status; + + if (tevent_req_is_nterror(req, &status)) { + tevent_req_received(req); + return status; + } + *interim_info = talloc_move(mem_ctx, &state->user_info_dc); + tevent_req_received(req); return NT_STATUS_OK; } static const struct auth_operations unix_ops = { - .name = "unix", - .want_check = authunix_want_check, - .check_password = authunix_check_password + .name = "unix", + .want_check = authunix_want_check, + .check_password_send = authunix_check_password_send, + .check_password_recv = authunix_check_password_recv, }; _PUBLIC_ NTSTATUS auth4_unix_init(TALLOC_CTX *ctx) -- 2.33.1 From e655b408ee2bf4fb4535140c03dd1d795be861dd Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Thu, 15 Apr 2021 10:04:21 +0200 Subject: [PATCH 10/39] CVE-2020-25717 auth4: Make auth_sam pseudo-async Signed-off-by: Volker Lendecke Reviewed-by: Andrew Bartlett BUG: https://bugzilla.samba.org/show_bug.cgi?id=14556 (cherry picked from commit f852fb4cd4e2bcd676a9ea104c5bf00979771eed) --- source4/auth/ntlm/auth_sam.c | 69 ++++++++++++++++++++++++++++++++++-- 1 file changed, 67 insertions(+), 2 deletions(-) diff --git a/source4/auth/ntlm/auth_sam.c b/source4/auth/ntlm/auth_sam.c index c5b27171937..a521bc94bc4 100644 --- a/source4/auth/ntlm/auth_sam.c +++ b/source4/auth/ntlm/auth_sam.c @@ -36,6 +36,7 @@ #include "lib/messaging/irpc.h" #include "libcli/auth/libcli_auth.h" #include "libds/common/roles.h" +#include "lib/util/tevent_ntstatus.h" #undef DBGC_CLASS #define DBGC_CLASS DBGC_AUTH @@ -733,6 +734,68 @@ static NTSTATUS authsam_check_password_internals(struct auth_method_context *ctx return NT_STATUS_OK; } +struct authsam_check_password_state { + struct auth_user_info_dc *user_info_dc; + bool authoritative; +}; + +static struct tevent_req *authsam_check_password_send( + TALLOC_CTX *mem_ctx, + struct tevent_context *ev, + struct auth_method_context *ctx, + const struct auth_usersupplied_info *user_info) +{ + struct tevent_req *req = NULL; + struct authsam_check_password_state *state = NULL; + NTSTATUS status; + + req = tevent_req_create( + mem_ctx, &state, struct authsam_check_password_state); + if (req == NULL) { + return NULL; + } + /* + * authsam_check_password_internals() sets this to false in + * the rodc case, otherwise it leaves it untouched. Default to + * "we're authoritative". + */ + state->authoritative = true; + + status = authsam_check_password_internals( + ctx, + state, + user_info, + &state->user_info_dc, + &state->authoritative); + if (tevent_req_nterror(req, status)) { + return tevent_req_post(req, ev); + } + + tevent_req_done(req); + return tevent_req_post(req, ev); +} + +static NTSTATUS authsam_check_password_recv( + struct tevent_req *req, + TALLOC_CTX *mem_ctx, + struct auth_user_info_dc **interim_info, + bool *authoritative) +{ + struct authsam_check_password_state *state = tevent_req_data( + req, struct authsam_check_password_state); + NTSTATUS status; + + *authoritative = state->authoritative; + + if (tevent_req_is_nterror(req, &status)) { + tevent_req_received(req); + return status; + } + *interim_info = talloc_move(mem_ctx, &state->user_info_dc); + tevent_req_received(req); + return NT_STATUS_OK; +} + static NTSTATUS authsam_ignoredomain_want_check(struct auth_method_context *ctx, TALLOC_CTX *mem_ctx, const struct auth_usersupplied_info *user_info) @@ -888,14 +951,16 @@ static NTSTATUS authsam_get_user_info_dc_principal_wrapper(TALLOC_CTX *mem_ctx, static const struct auth_operations sam_ignoredomain_ops = { .name = "sam_ignoredomain", .want_check = authsam_ignoredomain_want_check, - .check_password = authsam_check_password_internals, + .check_password_send = authsam_check_password_send, + .check_password_recv = authsam_check_password_recv, .get_user_info_dc_principal = authsam_get_user_info_dc_principal_wrapper, }; static const struct auth_operations sam_ops = { .name = "sam", .want_check = authsam_want_check, - .check_password = authsam_check_password_internals, + .check_password_send = authsam_check_password_send, + .check_password_recv = authsam_check_password_recv, .get_user_info_dc_principal = authsam_get_user_info_dc_principal_wrapper, }; -- 2.33.1 From 61e6c5f7c5d1df83350ae4969a1f365b4302a240 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Wed, 14 Apr 2021 22:24:44 +0200 Subject: [PATCH 11/39] CVE-2020-25717 auth4: Remove sync check_password from auth_operations Remove complexity in the data structures, and pushes the async-ness one level down. Signed-off-by: Volker Lendecke Reviewed-by: Andrew Bartlett BUG: https://bugzilla.samba.org/show_bug.cgi?id=14556 (cherry picked from commit 254af19ba89b4c42e5f45ec731e6577d2fcc6736) --- source4/auth/auth.h | 4 ---- source4/auth/ntlm/auth.c | 44 ++++------------------------------------ 2 files changed, 4 insertions(+), 44 deletions(-) diff --git a/source4/auth/auth.h b/source4/auth/auth.h index 51895c9259f..3f9fb1ae3cb 100644 --- a/source4/auth/auth.h +++ b/source4/auth/auth.h @@ -61,10 +61,6 @@ struct auth_operations { /* Given the user supplied info, check a password */ - NTSTATUS (*check_password)(struct auth_method_context *ctx, TALLOC_CTX *mem_ctx, - const struct auth_usersupplied_info *user_info, - struct auth_user_info_dc **interim_info, - bool *authoritative); struct tevent_req *(*check_password_send)(TALLOC_CTX *mem_ctx, struct tevent_context *ev, struct auth_method_context *ctx, diff --git a/source4/auth/ntlm/auth.c b/source4/auth/ntlm/auth.c index 75cf12c5742..e54eb7719f5 100644 --- a/source4/auth/ntlm/auth.c +++ b/source4/auth/ntlm/auth.c @@ -332,7 +332,6 @@ static void auth_check_password_next(struct tevent_req *req) struct auth_check_password_state *state = tevent_req_data(req, struct auth_check_password_state); struct tevent_req *subreq = NULL; - bool authoritative = true; NTSTATUS status; if (state->method == NULL) { @@ -357,47 +356,12 @@ static void auth_check_password_next(struct tevent_req *req) return; } - if (state->method->ops->check_password_send != NULL) { - subreq = state->method->ops->check_password_send(state, - state->ev, - state->method, - state->user_info); - if (tevent_req_nomem(subreq, req)) { - return; - } - tevent_req_set_callback(subreq, - auth_check_password_done, - req); - return; - } - - if (state->method->ops->check_password == NULL) { - tevent_req_nterror(req, NT_STATUS_INTERNAL_ERROR); - return; - } - - status = state->method->ops->check_password(state->method, - state, - state->user_info, - &state->user_info_dc, - &authoritative); - if (!authoritative || - NT_STATUS_EQUAL(status, NT_STATUS_NOT_IMPLEMENTED)) { - DEBUG(11,("auth_check_password_send: " - "%s passes to the next method\n", - state->method->ops->name)); - state->method = state->method->next; - auth_check_password_next(req); - return; - } - - /* the backend has handled the request */ - - if (tevent_req_nterror(req, status)) { + subreq = state->method->ops->check_password_send( + state, state->ev, state->method, state->user_info); + if (tevent_req_nomem(subreq, req)) { return; } - - tevent_req_done(req); + tevent_req_set_callback(subreq, auth_check_password_done, req); } static void auth_check_password_done(struct tevent_req *subreq) -- 2.33.1 From 557c44fe666b758eebb8ffab2b540722d995f3cd Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 4 Oct 2021 17:29:34 +0200 Subject: [PATCH 12/39] CVE-2020-25717: s3:winbindd: make sure we default to r->out.authoritative = true We need to make sure that temporary failures don't trigger a fallback to the local SAM that silently ignores the domain name part for users. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14556 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett --- source3/winbindd/winbindd_dual_srv.c | 7 +++++++ source3/winbindd/winbindd_irpc.c | 7 +++++++ source3/winbindd/winbindd_pam.c | 15 +++++++++++---- source3/winbindd/winbindd_pam_auth_crap.c | 9 ++++++++- source3/winbindd/winbindd_util.c | 7 +++++++ 5 files changed, 40 insertions(+), 5 deletions(-) diff --git a/source3/winbindd/winbindd_dual_srv.c b/source3/winbindd/winbindd_dual_srv.c index 32d11e1fa57..0be5ae5554b 100644 --- a/source3/winbindd/winbindd_dual_srv.c +++ b/source3/winbindd/winbindd_dual_srv.c @@ -941,6 +941,13 @@ NTSTATUS _winbind_SamLogon(struct pipes_struct *p, union netr_Validation *validation = NULL; bool interactive = false; + /* + * Make sure we start with authoritative=true, + * it will only set to false if we don't know the + * domain. + */ + r->out.authoritative = true; + domain = wb_child_domain(); if (domain == NULL) { return NT_STATUS_REQUEST_NOT_ACCEPTED; diff --git a/source3/winbindd/winbindd_irpc.c b/source3/winbindd/winbindd_irpc.c index e419736010b..918393c0827 100644 --- a/source3/winbindd/winbindd_irpc.c +++ b/source3/winbindd/winbindd_irpc.c @@ -142,6 +142,13 @@ static NTSTATUS wb_irpc_SamLogon(struct irpc_message *msg, const char *target_domain_name = NULL; const char *account_name = NULL; + /* + * Make sure we start with authoritative=true, + * it will only set to false if we don't know the + * domain. + */ + req->out.authoritative = true; + switch (req->in.logon_level) { case NetlogonInteractiveInformation: case NetlogonServiceInformation: diff --git a/source3/winbindd/winbindd_pam.c b/source3/winbindd/winbindd_pam.c index f27802ee065..f71eb8197a4 100644 --- a/source3/winbindd/winbindd_pam.c +++ b/source3/winbindd/winbindd_pam.c @@ -1799,7 +1799,7 @@ static NTSTATUS winbindd_dual_pam_auth_samlogon( { fstring name_namespace, name_domain, name_user; NTSTATUS result; - uint8_t authoritative = 0; + uint8_t authoritative = 1; uint32_t flags = 0; uint16_t validation_level = 0; union netr_Validation *validation = NULL; @@ -2453,6 +2453,13 @@ done: result = NT_STATUS_NO_LOGON_SERVERS; } + /* + * Here we don't alter + * state->response->data.auth.authoritative based + * on the servers response + * as we don't want a fallback to the local sam + * for interactive PAM logons + */ set_auth_errors(state->response, result); DEBUG(NT_STATUS_IS_OK(result) ? 5 : 2, ("Plain-text authentication for user %s returned %s (PAM: %d)\n", @@ -2667,7 +2674,7 @@ enum winbindd_result winbindd_dual_pam_auth_crap(struct winbindd_domain *domain, const char *name_domain = NULL; const char *workstation; uint64_t logon_id = 0; - uint8_t authoritative = 0; + uint8_t authoritative = 1; uint32_t flags = 0; uint16_t validation_level; union netr_Validation *validation = NULL; @@ -2740,7 +2747,6 @@ enum winbindd_result winbindd_dual_pam_auth_crap(struct winbindd_domain *domain, &validation_level, &validation); if (!NT_STATUS_IS_OK(result)) { - state->response->data.auth.authoritative = authoritative; goto done; } @@ -2772,7 +2778,6 @@ enum winbindd_result winbindd_dual_pam_auth_crap(struct winbindd_domain *domain, "from firewalled domain [%s]\n", info3->base.account_name.string, info3->base.logon_domain.string); - state->response->data.auth.authoritative = true; result = NT_STATUS_AUTHENTICATION_FIREWALL_FAILED; goto done; } @@ -2794,6 +2799,8 @@ done: } set_auth_errors(state->response, result); + state->response->data.auth.authoritative = authoritative; + /* * Log the winbind pam authentication, the logon_id will tie this to * any of the logons invoked from this request. diff --git a/source3/winbindd/winbindd_pam_auth_crap.c b/source3/winbindd/winbindd_pam_auth_crap.c index dacb6566be6..a6f13806df9 100644 --- a/source3/winbindd/winbindd_pam_auth_crap.c +++ b/source3/winbindd/winbindd_pam_auth_crap.c @@ -26,6 +26,7 @@ struct winbindd_pam_auth_crap_state { struct winbindd_response *response; + bool authoritative; uint32_t flags; }; @@ -47,7 +48,7 @@ struct tevent_req *winbindd_pam_auth_crap_send( if (req == NULL) { return NULL; } - + state->authoritative = true; state->flags = request->flags; if (state->flags & WBFLAG_PAM_AUTH_PAC) { @@ -126,6 +127,11 @@ struct tevent_req *winbindd_pam_auth_crap_send( domain = find_auth_domain(request->flags, auth_domain); if (domain == NULL) { + /* + * We don't know the domain so + * we're not authoritative + */ + state->authoritative = false; tevent_req_nterror(req, NT_STATUS_NO_SUCH_USER); return tevent_req_post(req, ev); } @@ -186,6 +192,7 @@ NTSTATUS winbindd_pam_auth_crap_recv(struct tevent_req *req, if (tevent_req_is_nterror(req, &status)) { set_auth_errors(response, status); + response->data.auth.authoritative = state->authoritative; return status; } diff --git a/source3/winbindd/winbindd_util.c b/source3/winbindd/winbindd_util.c index 414b770a271..ac4cd731c09 100644 --- a/source3/winbindd/winbindd_util.c +++ b/source3/winbindd/winbindd_util.c @@ -2095,6 +2095,13 @@ void winbindd_unset_locator_kdc_env(const struct winbindd_domain *domain) void set_auth_errors(struct winbindd_response *resp, NTSTATUS result) { + /* + * Make sure we start with authoritative=true, + * it will only set to false if we don't know the + * domain. + */ + resp->data.auth.authoritative = true; + resp->data.auth.nt_status = NT_STATUS_V(result); fstrcpy(resp->data.auth.nt_status_string, nt_errstr(result)); -- 2.33.1 From 3733dfeb24e5b58f9d92940573afbcd4a4d418fa Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 4 Oct 2021 17:29:34 +0200 Subject: [PATCH 13/39] CVE-2020-25717: s4:auth/ntlm: make sure auth_check_password() defaults to r->out.authoritative = true We need to make sure that temporary failures don't trigger a fallback to the local SAM that silently ignores the domain name part for users. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14556 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett --- source4/auth/ntlm/auth.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/source4/auth/ntlm/auth.c b/source4/auth/ntlm/auth.c index e54eb7719f5..4c66f2c23cb 100644 --- a/source4/auth/ntlm/auth.c +++ b/source4/auth/ntlm/auth.c @@ -169,6 +169,11 @@ _PUBLIC_ NTSTATUS auth_check_password(struct auth4_context *auth_ctx, /*TODO: create a new event context here! */ ev = auth_ctx->event_ctx; + /* + * We are authoritative by default + */ + *pauthoritative = 1; + subreq = auth_check_password_send(mem_ctx, ev, auth_ctx, -- 2.33.1 From fb466bcff4f42287e2895cf3b1b0dc20776ec607 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 26 Oct 2021 17:42:41 +0200 Subject: [PATCH 14/39] CVE-2020-25717: s4:torture: start with authoritative = 1 This is not strictly needed, but makes it easier to audit that we don't miss important places. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14556 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett --- source4/torture/rpc/samlogon.c | 4 ++-- source4/torture/rpc/schannel.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/source4/torture/rpc/samlogon.c b/source4/torture/rpc/samlogon.c index 76933b8869e..703e25fe3c5 100644 --- a/source4/torture/rpc/samlogon.c +++ b/source4/torture/rpc/samlogon.c @@ -1407,7 +1407,7 @@ static bool test_SamLogon(struct dcerpc_pipe *p, TALLOC_CTX *mem_ctx, union netr_LogonLevel logon; union netr_Validation validation; - uint8_t authoritative = 0; + uint8_t authoritative = 1; uint32_t flags = 0; ZERO_STRUCT(logon); @@ -1520,7 +1520,7 @@ bool test_InteractiveLogon(struct dcerpc_pipe *p, TALLOC_CTX *mem_ctx, union netr_LogonLevel logon; union netr_Validation validation; - uint8_t authoritative = 0; + uint8_t authoritative = 1; struct dcerpc_binding_handle *b = p->binding_handle; ZERO_STRUCT(a); diff --git a/source4/torture/rpc/schannel.c b/source4/torture/rpc/schannel.c index a5755041040..08a5120b66d 100644 --- a/source4/torture/rpc/schannel.c +++ b/source4/torture/rpc/schannel.c @@ -50,7 +50,7 @@ bool test_netlogon_ex_ops(struct dcerpc_pipe *p, struct torture_context *tctx, struct netr_NetworkInfo ninfo; union netr_LogonLevel logon; union netr_Validation validation; - uint8_t authoritative = 0; + uint8_t authoritative = 1; uint32_t _flags = 0; DATA_BLOB names_blob, chal, lm_resp, nt_resp; int i; -- 2.33.1 From 7e3934334f6d532a087db43a308c404d34b81458 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 26 Oct 2021 17:42:41 +0200 Subject: [PATCH 15/39] CVE-2020-25717: s4:smb_server: start with authoritative = 1 This is not strictly needed, but makes it easier to audit that we don't miss important places. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14556 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett --- source4/smb_server/smb/sesssetup.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source4/smb_server/smb/sesssetup.c b/source4/smb_server/smb/sesssetup.c index 68cdd70feff..8428ca3fabb 100644 --- a/source4/smb_server/smb/sesssetup.c +++ b/source4/smb_server/smb/sesssetup.c @@ -102,7 +102,7 @@ static void sesssetup_old_send(struct tevent_req *subreq) struct auth_session_info *session_info; struct smbsrv_session *smb_sess; NTSTATUS status; - uint8_t authoritative = 0; + uint8_t authoritative = 1; uint32_t flags; status = auth_check_password_recv(subreq, req, &user_info_dc, @@ -243,7 +243,7 @@ static void sesssetup_nt1_send(struct tevent_req *subreq) struct auth_user_info_dc *user_info_dc = NULL; struct auth_session_info *session_info; struct smbsrv_session *smb_sess; - uint8_t authoritative = 0; + uint8_t authoritative = 1; uint32_t flags; NTSTATUS status; -- 2.33.1 From c4a595518b453ecc5c0e2bfe7ba2cf531ea37e13 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 26 Oct 2021 17:42:41 +0200 Subject: [PATCH 16/39] CVE-2020-25717: s4:auth_simple: start with authoritative = 1 This is not strictly needed, but makes it easier to audit that we don't miss important places. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14556 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett --- source4/auth/ntlm/auth_simple.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source4/auth/ntlm/auth_simple.c b/source4/auth/ntlm/auth_simple.c index 8df160cefc3..8301aec519c 100644 --- a/source4/auth/ntlm/auth_simple.c +++ b/source4/auth/ntlm/auth_simple.c @@ -150,7 +150,7 @@ static void authenticate_ldap_simple_bind_done(struct tevent_req *subreq) const struct tsocket_address *local_address = user_info->local_host; const char *transport_protection = AUTHZ_TRANSPORT_PROTECTION_NONE; struct auth_user_info_dc *user_info_dc = NULL; - uint8_t authoritative = 0; + uint8_t authoritative = 1; uint32_t flags = 0; NTSTATUS nt_status; -- 2.33.1 From 0e32b75b21d5daf08fffbe25b6c40197ed04cbf8 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 26 Oct 2021 17:42:41 +0200 Subject: [PATCH 17/39] CVE-2020-25717: s3:ntlm_auth: start with authoritative = 1 This is not strictly needed, but makes it easier to audit that we don't miss important places. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14556 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett --- source3/utils/ntlm_auth.c | 4 ++-- source3/utils/ntlm_auth_diagnostics.c | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/source3/utils/ntlm_auth.c b/source3/utils/ntlm_auth.c index 0370803167f..b3ab3a473bf 100644 --- a/source3/utils/ntlm_auth.c +++ b/source3/utils/ntlm_auth.c @@ -1927,7 +1927,7 @@ static void manage_ntlm_server_1_request(enum stdio_helper_mode stdio_helper_mod TALLOC_FREE(mem_ctx); } else { - uint8_t authoritative = 0; + uint8_t authoritative = 1; if (!domain) { domain = smb_xstrdup(get_winbind_domain()); @@ -2443,7 +2443,7 @@ static bool check_auth_crap(void) char *hex_lm_key; char *hex_user_session_key; char *error_string; - uint8_t authoritative = 0; + uint8_t authoritative = 1; setbuf(stdout, NULL); diff --git a/source3/utils/ntlm_auth_diagnostics.c b/source3/utils/ntlm_auth_diagnostics.c index 41591a8de33..fc0fc19bacb 100644 --- a/source3/utils/ntlm_auth_diagnostics.c +++ b/source3/utils/ntlm_auth_diagnostics.c @@ -54,7 +54,7 @@ static bool test_lm_ntlm_broken(enum ntlm_break break_which) DATA_BLOB lm_response = data_blob(NULL, 24); DATA_BLOB nt_response = data_blob(NULL, 24); DATA_BLOB session_key = data_blob(NULL, 16); - uint8_t authoritative = 0; + uint8_t authoritative = 1; uchar lm_key[8]; uchar user_session_key[16]; uchar lm_hash[16]; @@ -177,7 +177,7 @@ static bool test_ntlm_in_lm(void) NTSTATUS nt_status; uint32_t flags = 0; DATA_BLOB nt_response = data_blob(NULL, 24); - uint8_t authoritative = 0; + uint8_t authoritative = 1; uchar lm_key[8]; uchar lm_hash[16]; uchar user_session_key[16]; @@ -245,7 +245,7 @@ static bool test_ntlm_in_both(void) uint32_t flags = 0; DATA_BLOB nt_response = data_blob(NULL, 24); DATA_BLOB session_key = data_blob(NULL, 16); - uint8_t authoritative = 0; + uint8_t authoritative = 1; uint8_t lm_key[8]; uint8_t lm_hash[16]; uint8_t user_session_key[16]; @@ -322,7 +322,7 @@ static bool test_lmv2_ntlmv2_broken(enum ntlm_break break_which) DATA_BLOB lmv2_response = data_blob_null; DATA_BLOB ntlmv2_session_key = data_blob_null; DATA_BLOB names_blob = NTLMv2_generate_names_blob(NULL, get_winbind_netbios_name(), get_winbind_domain()); - uint8_t authoritative = 0; + uint8_t authoritative = 1; uchar user_session_key[16]; DATA_BLOB chall = get_challenge(); char *error_string; @@ -452,7 +452,7 @@ static bool test_plaintext(enum ntlm_break break_which) char *password; smb_ucs2_t *nt_response_ucs2; size_t converted_size; - uint8_t authoritative = 0; + uint8_t authoritative = 1; uchar user_session_key[16]; uchar lm_key[16]; static const uchar zeros[8] = { 0, }; -- 2.33.1 From aa52008a73b9960acbb6b39684c5c460480e365b Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 26 Oct 2021 17:42:41 +0200 Subject: [PATCH 18/39] CVE-2020-25717: s3:torture: start with authoritative = 1 This is not strictly needed, but makes it easier to audit that we don't miss important places. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14556 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett --- source3/torture/pdbtest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source3/torture/pdbtest.c b/source3/torture/pdbtest.c index 5d74aa9ab78..b300504c4cb 100644 --- a/source3/torture/pdbtest.c +++ b/source3/torture/pdbtest.c @@ -277,7 +277,7 @@ static bool test_auth(TALLOC_CTX *mem_ctx, struct samu *pdb_entry) struct netr_SamInfo6 *info6_wbc = NULL; NTSTATUS status; bool ok; - uint8_t authoritative = 0; + uint8_t authoritative = 1; int rc; rc = SMBOWFencrypt(pdb_get_nt_passwd(pdb_entry), challenge_8, -- 2.33.1 From 9c133372ac3687e9316f1b76fdbcc9d95d54db42 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 26 Oct 2021 17:42:41 +0200 Subject: [PATCH 19/39] CVE-2020-25717: s3:rpcclient: start with authoritative = 1 This is not strictly needed, but makes it easier to audit that we don't miss important places. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14556 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett --- source3/rpcclient/cmd_netlogon.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source3/rpcclient/cmd_netlogon.c b/source3/rpcclient/cmd_netlogon.c index d5c1b91f2be..4ea63e40b8d 100644 --- a/source3/rpcclient/cmd_netlogon.c +++ b/source3/rpcclient/cmd_netlogon.c @@ -496,7 +496,7 @@ static NTSTATUS cmd_netlogon_sam_logon(struct rpc_pipe_client *cli, uint32_t logon_param = 0; const char *workstation = NULL; struct netr_SamInfo3 *info3 = NULL; - uint8_t authoritative = 0; + uint8_t authoritative = 1; uint32_t flags = 0; uint16_t validation_level; union netr_Validation *validation = NULL; -- 2.33.1 From 7a2bf7eb4be1c7988bfe14dbbf297f79a05220ad Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 26 Oct 2021 17:42:41 +0200 Subject: [PATCH 20/39] CVE-2020-25717: s3:auth: start with authoritative = 1 This is not strictly needed, but makes it easier to audit that we don't miss important places. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14556 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett --- source3/auth/auth_generic.c | 2 +- source3/auth/auth_samba4.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/source3/auth/auth_generic.c b/source3/auth/auth_generic.c index e62585e919c..57281874b35 100644 --- a/source3/auth/auth_generic.c +++ b/source3/auth/auth_generic.c @@ -416,7 +416,7 @@ NTSTATUS auth_check_password_session_info(struct auth4_context *auth_context, { NTSTATUS nt_status; void *server_info; - uint8_t authoritative = 0; + uint8_t authoritative = 1; struct tevent_context *ev = NULL; struct tevent_req *subreq = NULL; bool ok; diff --git a/source3/auth/auth_samba4.c b/source3/auth/auth_samba4.c index ac141354747..fde78d5e74e 100644 --- a/source3/auth/auth_samba4.c +++ b/source3/auth/auth_samba4.c @@ -120,7 +120,7 @@ static NTSTATUS check_samba4_security( NTSTATUS nt_status; struct auth_user_info_dc *user_info_dc; struct auth4_context *auth4_context; - uint8_t authoritative = 0; + uint8_t authoritative = 1; struct auth_serversupplied_info *server_info = NULL; nt_status = make_auth4_context_s4(auth_context, mem_ctx, &auth4_context); -- 2.33.1 From 97f2108d84017998d2612139b787fdb674c235e2 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 26 Oct 2021 17:42:41 +0200 Subject: [PATCH 21/39] CVE-2020-25717: auth/ntlmssp: start with authoritative = 1 This is not strictly needed, but makes it easier to audit that we don't miss important places. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14556 Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett --- auth/ntlmssp/ntlmssp_server.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/auth/ntlmssp/ntlmssp_server.c b/auth/ntlmssp/ntlmssp_server.c index 001238278d7..939aa0ef4aa 100644 --- a/auth/ntlmssp/ntlmssp_server.c +++ b/auth/ntlmssp/ntlmssp_server.c @@ -799,7 +799,7 @@ static void ntlmssp_server_auth_done(struct tevent_req *subreq) struct gensec_security *gensec_security = state->gensec_security; struct gensec_ntlmssp_context *gensec_ntlmssp = state->gensec_ntlmssp; struct auth4_context *auth_context = gensec_security->auth_context; - uint8_t authoritative = 0; + uint8_t authoritative = 1; NTSTATUS status; status = auth_context->check_ntlm_password_recv(subreq, -- 2.33.1 From 59e3ccaaced1fa3f7db250d44c7e9a94070d583b Mon Sep 17 00:00:00 2001 From: Samuel Cabrero Date: Tue, 28 Sep 2021 10:43:40 +0200 Subject: [PATCH 22/39] 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 Signed-off-by: Samuel Cabrero Signed-off-by: Stefan Metzmacher [abartlet@samba.org Backported from master/4.15 due to conflicts with other new parameters] --- 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 @@ + + + + The integer parameter specifies the minimum uid allowed when mapping a + local account to a domain account. + + + + Note that this option interacts with the configured idmap ranges! + + + +1000 + 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. + + + Note that the low value interacts with the option! @@ -115,4 +118,5 @@ +min domain uid diff --git a/lib/param/loadparm.c b/lib/param/loadparm.c index 3548c47d857..eedfa00bcb0 100644 --- a/lib/param/loadparm.c +++ b/lib/param/loadparm.c @@ -3090,6 +3090,10 @@ struct loadparm_context *loadparm_init(TALLOC_CTX *mem_ctx) "client smb encrypt", "default"); + 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 acb4d149f0b..300b539748b 100644 --- a/source3/param/loadparm.c +++ b/source3/param/loadparm.c @@ -967,6 +967,8 @@ static void init_globals(struct loadparm_context *lp_ctx, bool reinit_globals) Globals.client_smb_encrypt = SMB_ENCRYPTION_DEFAULT; + 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 977bbda9fed9362bfa46e541c8cdb9666d43b57a Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 8 Oct 2021 19:57:18 +0200 Subject: [PATCH 23/39] 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 Reviewed-by: Andrew Bartlett --- 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 57281874b35..f6a501f1df7 100644 --- a/source3/auth/auth_generic.c +++ b/source3/auth/auth_generic.c @@ -166,7 +166,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 ed88e24afbf85cc6a61c236604f3fc43a10f0d97 Mon Sep 17 00:00:00 2001 From: Samuel Cabrero Date: Tue, 28 Sep 2021 10:45:11 +0200 Subject: [PATCH 24/39] 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 Signed-off-by: Samuel Cabrero Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett [abartlet@samba.org Removed knownfail on advice from metze] --- 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 4686b29111e..4de4bc74374 100644 --- a/source3/auth/auth_util.c +++ b/source3/auth/auth_util.c @@ -2103,6 +2103,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 51a78934575b47e6add3110904c5de6c97566fd7 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 8 Oct 2021 17:40:30 +0200 Subject: [PATCH 25/39] 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 Reviewed-by: Andrew Bartlett --- 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 8998f9c8f8a..074e8c7eb71 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 0ef009b205acf1b14f8763cf2e6ce13ec8bea151 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 8 Oct 2021 18:08:20 +0200 Subject: [PATCH 26/39] 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 Reviewed-by: Andrew Bartlett --- 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 4de4bc74374..99b85d47a5f 100644 --- a/source3/auth/auth_util.c +++ b/source3/auth/auth_util.c @@ -1898,7 +1898,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 de475a3eba52cb4599330f6932e98f97c3749ec7 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Fri, 8 Oct 2021 12:33:16 +0200 Subject: [PATCH 27/39] 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 Signed-off-by: Ralph Boehme Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett --- selftest/knownfail.d/ktest | 26 +++++++++++++ source3/auth/auth_util.c | 77 +++++++++++++++++++++----------------- 2 files changed, 68 insertions(+), 35 deletions(-) create mode 100644 selftest/knownfail.d/ktest diff --git a/selftest/knownfail.d/ktest b/selftest/knownfail.d/ktest new file mode 100644 index 00000000000..809612ba0b9 --- /dev/null +++ b/selftest/knownfail.d/ktest @@ -0,0 +1,26 @@ +^samba3.rpc.lsa.lookupsids.krb5.with.old.ccache.ncacn_np.with..smb2...lsa.LookupSidsReply.ktest +^samba3.rpc.lsa.lookupsids.krb5.ncacn_np.with..smb2...lsa.LookupSidsReply.ktest +^samba3.blackbox.rpcclient.krb5.ncacn_np.with..krb5...rpcclient.ktest:local +^samba3.blackbox.rpcclient.krb5.ncacn_np.with..spnego,krb5...rpcclient.ktest:local +^samba3.rpc.lsa.lookupsids.krb5.with.old.ccache.ncacn_np.with..smb2,connect...lsa.LookupSidsReply.ktest +^samba3.rpc.lsa.lookupsids.krb5.ncacn_np.with..smb2,connect...lsa.LookupSidsReply.ktest +^samba3.rpc.lsa.lookupsids.krb5.with.old.ccache.ncacn_np.with..smb2,packet...lsa.LookupSidsReply.ktest +^samba3.rpc.lsa.lookupsids.krb5.ncacn_np.with..smb2,packet...lsa.LookupSidsReply.ktest +^samba3.blackbox.rpcclient.krb5.ncacn_np.with..krb5,packet...rpcclient.ktest:local +^samba3.blackbox.rpcclient.krb5.ncacn_np.with..spnego,krb5,packet...rpcclient.ktest:local +^samba3.rpc.lsa.lookupsids.krb5.with.old.ccache.ncacn_np.with..smb2,sign...lsa.LookupSidsReply.ktest +^samba3.rpc.lsa.lookupsids.krb5.ncacn_np.with..smb2,sign...lsa.LookupSidsReply.ktest +^samba3.blackbox.rpcclient.krb5.ncacn_np.with..krb5,sign...rpcclient.ktest:local +^samba3.blackbox.rpcclient.krb5.ncacn_np.with..spnego,krb5,sign...rpcclient.ktest:local +^samba3.rpc.lsa.lookupsids.krb5.with.old.ccache.ncacn_np.with..smb2,seal...lsa.LookupSidsReply.ktest +^samba3.rpc.lsa.lookupsids.krb5.ncacn_np.with..smb2,seal...lsa.LookupSidsReply.ktest +^samba3.blackbox.rpcclient.krb5.ncacn_np.with..krb5,seal...rpcclient.ktest:local +^samba3.blackbox.rpcclient.krb5.ncacn_np.with..spnego,krb5,seal...rpcclient.ktest:local +^samba3.blackbox.smbclient_krb5.old.ccache..smbclient.ktest:local +^samba3.blackbox.smbclient_krb5.new.ccache..smbclient.ktest:local +^samba3.blackbox.smbclient_large_file..krb5.smbclient.large.posix.write.read.ktest:local +^samba3.blackbox.smbclient_large_file..krb5.cmp.of.read.and.written.files.ktest:local +^samba3.blackbox.smbclient_krb5.old.ccache.--client-protection=encrypt.smbclient.ktest:local +^samba3.blackbox.smbclient_krb5.new.ccache.--client-protection=encrypt.smbclient.ktest:local +^samba3.blackbox.smbclient_large_file.--client-protection=encrypt.krb5.smbclient.large.posix.write.read.ktest:local +^samba3.blackbox.smbclient_large_file.--client-protection=encrypt.krb5.cmp.of.read.and.written.files.ktest:local diff --git a/source3/auth/auth_util.c b/source3/auth/auth_util.c index 99b85d47a5f..d81313a0495 100644 --- a/source3/auth/auth_util.c +++ b/source3/auth/auth_util.c @@ -1933,7 +1933,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 */ @@ -1952,48 +1952,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 debd9501c568ebd536275a240cd796187d1ce39b Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 21 Sep 2021 13:13:52 +0200 Subject: [PATCH 28/39] CVE-2020-25717: s3:lib: add lp_allow_trusted_domains() logic to is_allowed_domain() is_allowed_domain() is a central place we already use to trigger NT_STATUS_AUTHENTICATION_FIREWALL_FAILED, so we can add additional logic there. 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 Reviewed-by: Andrew Bartlett --- source3/lib/util_names.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/source3/lib/util_names.c b/source3/lib/util_names.c index 630a25875c7..876035cbe29 100644 --- a/source3/lib/util_names.c +++ b/source3/lib/util_names.c @@ -200,5 +200,18 @@ bool is_allowed_domain(const char *domain_name) } } - return true; + if (lp_allow_trusted_domains()) { + return true; + } + + if (strequal(lp_workgroup(), domain_name)) { + return true; + } + + if (is_myname(domain_name)) { + return true; + } + + DBG_NOTICE("Not trusted domain '%s'\n", domain_name); + return false; } -- 2.33.1 From d2b7d75eccee7575c3dab25cc8c61e2cf00994d3 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 4 Oct 2021 18:03:55 +0200 Subject: [PATCH 29/39] 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 Reviewed-by: Andrew Bartlett --- 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 d81313a0495..065b525500f 100644 --- a/source3/auth/auth_util.c +++ b/source3/auth/auth_util.c @@ -576,13 +576,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 045a019e452c512224a13cfbd98c592cb7f2e912 Mon Sep 17 00:00:00 2001 From: Alexander Bokovoy Date: Wed, 11 Nov 2020 18:50:45 +0200 Subject: [PATCH 30/39] CVE-2020-25717: Add FreeIPA domain controller role As we want to reduce use of 'classic domain controller' role but FreeIPA relies on it internally, add a separate role to mark FreeIPA domain controller role. It means that role won't result in ROLE_STANDALONE. 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 Signed-off-by: Alexander Bokovoy Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett --- docs-xml/smbdotconf/security/serverrole.xml | 7 ++++ lib/param/loadparm_server_role.c | 2 ++ lib/param/param_table.c | 1 + lib/param/util.c | 1 + libcli/netlogon/netlogon.c | 2 +- libds/common/roles.h | 1 + source3/auth/auth.c | 3 ++ source3/auth/auth_sam.c | 14 ++++---- source3/include/smb_macros.h | 2 +- source3/lib/netapi/joindomain.c | 1 + source3/param/loadparm.c | 4 ++- source3/passdb/lookup_sid.c | 2 +- source3/passdb/machine_account_secrets.c | 7 ++-- source3/registry/reg_backend_prod_options.c | 1 + source3/rpc_server/dssetup/srv_dssetup_nt.c | 1 + source3/smbd/server.c | 2 +- source3/winbindd/winbindd_misc.c | 2 +- source3/winbindd/winbindd_util.c | 40 ++++++++++++++++----- source4/auth/ntlm/auth.c | 1 + source4/kdc/kdc-heimdal.c | 1 + source4/rpc_server/samr/dcesrv_samr.c | 2 ++ 21 files changed, 72 insertions(+), 25 deletions(-) diff --git a/docs-xml/smbdotconf/security/serverrole.xml b/docs-xml/smbdotconf/security/serverrole.xml index 9511c61c96d..b8b83a127b5 100644 --- a/docs-xml/smbdotconf/security/serverrole.xml +++ b/docs-xml/smbdotconf/security/serverrole.xml @@ -78,6 +78,13 @@ url="http://wiki.samba.org/index.php/Samba4/HOWTO">Samba4 HOWTO + SERVER ROLE = IPA DOMAIN CONTROLLER + + This mode of operation runs Samba in a hybrid mode for IPA + domain controller, providing forest trust to Active Directory. + This role requires special configuration performed by IPA installers + and should not be used manually by any administrator. + security diff --git a/lib/param/loadparm_server_role.c b/lib/param/loadparm_server_role.c index 7a6bc770723..a78d1ab9cf3 100644 --- a/lib/param/loadparm_server_role.c +++ b/lib/param/loadparm_server_role.c @@ -42,6 +42,7 @@ static const struct srv_role_tab { { ROLE_DOMAIN_BDC, "ROLE_DOMAIN_BDC" }, { ROLE_DOMAIN_PDC, "ROLE_DOMAIN_PDC" }, { ROLE_ACTIVE_DIRECTORY_DC, "ROLE_ACTIVE_DIRECTORY_DC" }, + { ROLE_IPA_DC, "ROLE_IPA_DC"}, { 0, NULL } }; @@ -140,6 +141,7 @@ bool lp_is_security_and_server_role_valid(int server_role, int security) case ROLE_DOMAIN_PDC: case ROLE_DOMAIN_BDC: case ROLE_ACTIVE_DIRECTORY_DC: + case ROLE_IPA_DC: if (security == SEC_USER) { valid = true; } diff --git a/lib/param/param_table.c b/lib/param/param_table.c index e2f737279dc..3dc5fc59991 100644 --- a/lib/param/param_table.c +++ b/lib/param/param_table.c @@ -111,6 +111,7 @@ static const struct enum_list enum_server_role[] = { {ROLE_ACTIVE_DIRECTORY_DC, "active directory domain controller"}, {ROLE_ACTIVE_DIRECTORY_DC, "domain controller"}, {ROLE_ACTIVE_DIRECTORY_DC, "dc"}, + {ROLE_IPA_DC, "IPA primary domain controller"}, {-1, NULL} }; diff --git a/lib/param/util.c b/lib/param/util.c index cd8e74b9d8f..9a0fc102de8 100644 --- a/lib/param/util.c +++ b/lib/param/util.c @@ -255,6 +255,7 @@ const char *lpcfg_sam_name(struct loadparm_context *lp_ctx) case ROLE_DOMAIN_BDC: case ROLE_DOMAIN_PDC: case ROLE_ACTIVE_DIRECTORY_DC: + case ROLE_IPA_DC: return lpcfg_workgroup(lp_ctx); default: return lpcfg_netbios_name(lp_ctx); diff --git a/libcli/netlogon/netlogon.c b/libcli/netlogon/netlogon.c index 239503e85b6..59af460dc4e 100644 --- a/libcli/netlogon/netlogon.c +++ b/libcli/netlogon/netlogon.c @@ -93,7 +93,7 @@ NTSTATUS pull_netlogon_samlogon_response(DATA_BLOB *data, TALLOC_CTX *mem_ctx, if (ndr->offset < ndr->data_size) { TALLOC_FREE(ndr); /* - * We need to handle a bug in FreeIPA (at least <= 4.1.2). + * We need to handle a bug in IPA (at least <= 4.1.2). * * They include the ip address information without setting * NETLOGON_NT_VERSION_5EX_WITH_IP, while using diff --git a/libds/common/roles.h b/libds/common/roles.h index 4772c8d7d3f..03ba1915b21 100644 --- a/libds/common/roles.h +++ b/libds/common/roles.h @@ -33,6 +33,7 @@ enum server_role { /* not in samr.idl */ ROLE_ACTIVE_DIRECTORY_DC = 4, + ROLE_IPA_DC = 5, /* To determine the role automatically, this is not a valid role */ ROLE_AUTO = 100 diff --git a/source3/auth/auth.c b/source3/auth/auth.c index 6d2a40f0f29..d0036ea6b30 100644 --- a/source3/auth/auth.c +++ b/source3/auth/auth.c @@ -543,6 +543,7 @@ NTSTATUS make_auth3_context_for_ntlm(TALLOC_CTX *mem_ctx, break; case ROLE_DOMAIN_BDC: case ROLE_DOMAIN_PDC: + case ROLE_IPA_DC: role = "'DC'"; methods = "anonymous sam winbind sam_ignoredomain"; break; @@ -574,6 +575,7 @@ NTSTATUS make_auth3_context_for_netlogon(TALLOC_CTX *mem_ctx, switch (lp_server_role()) { case ROLE_DOMAIN_BDC: case ROLE_DOMAIN_PDC: + case ROLE_IPA_DC: methods = "sam_netlogon3 winbind"; break; @@ -595,6 +597,7 @@ NTSTATUS make_auth3_context_for_winbind(TALLOC_CTX *mem_ctx, case ROLE_DOMAIN_MEMBER: case ROLE_DOMAIN_BDC: case ROLE_DOMAIN_PDC: + case ROLE_IPA_DC: methods = "sam"; break; case ROLE_ACTIVE_DIRECTORY_DC: diff --git a/source3/auth/auth_sam.c b/source3/auth/auth_sam.c index e8e0d543f8c..a2ce1013975 100644 --- a/source3/auth/auth_sam.c +++ b/source3/auth/auth_sam.c @@ -143,12 +143,13 @@ static NTSTATUS auth_samstrict_auth(const struct auth_context *auth_context, break; case ROLE_DOMAIN_PDC: case ROLE_DOMAIN_BDC: + case ROLE_IPA_DC: if (!is_local_name && !is_my_domain) { /* If we are running on a DC that has PASSDB module with domain * information, check if DNS forest name is matching the domain - * name. This is the case of FreeIPA domain controller when - * trusted AD DCs attempt to authenticate FreeIPA users using - * the forest root domain (which is the only domain in FreeIPA). + * name. This is the case of IPA domain controller when + * trusted AD DCs attempt to authenticate IPA users using + * the forest root domain (which is the only domain in IPA). */ struct pdb_domain_info *dom_info = NULL; @@ -234,6 +235,7 @@ static NTSTATUS auth_sam_netlogon3_auth(const struct auth_context *auth_context, switch (lp_server_role()) { case ROLE_DOMAIN_PDC: case ROLE_DOMAIN_BDC: + case ROLE_IPA_DC: break; default: DBG_ERR("Invalid server role\n"); @@ -252,9 +254,9 @@ static NTSTATUS auth_sam_netlogon3_auth(const struct auth_context *auth_context, if (!is_my_domain) { /* If we are running on a DC that has PASSDB module with domain * information, check if DNS forest name is matching the domain - * name. This is the case of FreeIPA domain controller when - * trusted AD DCs attempt to authenticate FreeIPA users using - * the forest root domain (which is the only domain in FreeIPA). + * name. This is the case of IPA domain controller when + * trusted AD DCs attempt to authenticate IPA users using + * the forest root domain (which is the only domain in IPA). */ struct pdb_domain_info *dom_info = NULL; dom_info = pdb_get_domain_info(mem_ctx); diff --git a/source3/include/smb_macros.h b/source3/include/smb_macros.h index d9583945c55..def122727f0 100644 --- a/source3/include/smb_macros.h +++ b/source3/include/smb_macros.h @@ -203,7 +203,7 @@ copy an IP address from one buffer to another Check to see if we are a DC for this domain *****************************************************************************/ -#define IS_DC (lp_server_role()==ROLE_DOMAIN_PDC || lp_server_role()==ROLE_DOMAIN_BDC || lp_server_role() == ROLE_ACTIVE_DIRECTORY_DC) +#define IS_DC (lp_server_role()==ROLE_DOMAIN_PDC || lp_server_role()==ROLE_DOMAIN_BDC || lp_server_role() == ROLE_ACTIVE_DIRECTORY_DC || lp_server_role() == ROLE_IPA_DC) #define IS_AD_DC (lp_server_role() == ROLE_ACTIVE_DIRECTORY_DC) /* diff --git a/source3/lib/netapi/joindomain.c b/source3/lib/netapi/joindomain.c index f2d36fc00db..d1710c4b938 100644 --- a/source3/lib/netapi/joindomain.c +++ b/source3/lib/netapi/joindomain.c @@ -375,6 +375,7 @@ WERROR NetGetJoinInformation_l(struct libnetapi_ctx *ctx, case ROLE_DOMAIN_MEMBER: case ROLE_DOMAIN_PDC: case ROLE_DOMAIN_BDC: + case ROLE_IPA_DC: *r->out.name_type = NetSetupDomainName; break; case ROLE_STANDALONE: diff --git a/source3/param/loadparm.c b/source3/param/loadparm.c index 300b539748b..8bcd35f3d88 100644 --- a/source3/param/loadparm.c +++ b/source3/param/loadparm.c @@ -4412,6 +4412,7 @@ int lp_default_server_announce(void) default_server_announce |= SV_TYPE_DOMAIN_MEMBER; break; case ROLE_DOMAIN_PDC: + case ROLE_IPA_DC: default_server_announce |= SV_TYPE_DOMAIN_CTRL; break; case ROLE_DOMAIN_BDC: @@ -4437,7 +4438,8 @@ int lp_default_server_announce(void) bool lp_domain_master(void) { if (Globals._domain_master == Auto) - return (lp_server_role() == ROLE_DOMAIN_PDC); + return (lp_server_role() == ROLE_DOMAIN_PDC || + lp_server_role() == ROLE_IPA_DC); return (bool)Globals._domain_master; } diff --git a/source3/passdb/lookup_sid.c b/source3/passdb/lookup_sid.c index 0e01467b3cb..a551bcfd24a 100644 --- a/source3/passdb/lookup_sid.c +++ b/source3/passdb/lookup_sid.c @@ -121,7 +121,7 @@ bool lookup_name(TALLOC_CTX *mem_ctx, /* If we are running on a DC that has PASSDB module with domain * information, check if DNS forest name is matching the domain - * name. This is the case of FreeIPA domain controller when + * name. This is the case of IPA domain controller when * trusted AD DC looks up users found in a Global Catalog of * the forest root domain. */ if (!check_global_sam && (IS_DC)) { diff --git a/source3/passdb/machine_account_secrets.c b/source3/passdb/machine_account_secrets.c index d81f79c705b..618019b8322 100644 --- a/source3/passdb/machine_account_secrets.c +++ b/source3/passdb/machine_account_secrets.c @@ -198,7 +198,8 @@ bool secrets_fetch_domain_guid(const char *domain, struct GUID *guid) dyn_guid = (struct GUID *)secrets_fetch(key, &size); if (!dyn_guid) { - if (lp_server_role() == ROLE_DOMAIN_PDC) { + if (lp_server_role() == ROLE_DOMAIN_PDC || + lp_server_role() == ROLE_IPA_DC) { new_guid = GUID_random(); if (!secrets_store_domain_guid(domain, &new_guid)) return False; @@ -314,9 +315,7 @@ static const char *trust_keystr(const char *domain) enum netr_SchannelType get_default_sec_channel(void) { - if (lp_server_role() == ROLE_DOMAIN_BDC || - lp_server_role() == ROLE_DOMAIN_PDC || - lp_server_role() == ROLE_ACTIVE_DIRECTORY_DC) { + if (IS_DC) { return SEC_CHAN_BDC; } else { return SEC_CHAN_WKSTA; diff --git a/source3/registry/reg_backend_prod_options.c b/source3/registry/reg_backend_prod_options.c index 655c587ac40..7bd3f324c37 100644 --- a/source3/registry/reg_backend_prod_options.c +++ b/source3/registry/reg_backend_prod_options.c @@ -40,6 +40,7 @@ static int prod_options_fetch_values(const char *key, struct regval_ctr *regvals switch (lp_server_role()) { case ROLE_DOMAIN_PDC: case ROLE_DOMAIN_BDC: + case ROLE_IPA_DC: value_ascii = "LanmanNT"; break; case ROLE_STANDALONE: diff --git a/source3/rpc_server/dssetup/srv_dssetup_nt.c b/source3/rpc_server/dssetup/srv_dssetup_nt.c index 64569382695..932452bc13b 100644 --- a/source3/rpc_server/dssetup/srv_dssetup_nt.c +++ b/source3/rpc_server/dssetup/srv_dssetup_nt.c @@ -63,6 +63,7 @@ static WERROR fill_dsrole_dominfo_basic(TALLOC_CTX *ctx, basic->domain = get_global_sam_name(); break; case ROLE_DOMAIN_PDC: + case ROLE_IPA_DC: basic->role = DS_ROLE_PRIMARY_DC; basic->domain = get_global_sam_name(); break; diff --git a/source3/smbd/server.c b/source3/smbd/server.c index a0456100afe..26961fa6cf4 100644 --- a/source3/smbd/server.c +++ b/source3/smbd/server.c @@ -1979,7 +1979,7 @@ extern void build_options(bool screen); exit_daemon("smbd can not open secrets.tdb", EACCES); } - if (lp_server_role() == ROLE_DOMAIN_BDC || lp_server_role() == ROLE_DOMAIN_PDC) { + if (lp_server_role() == ROLE_DOMAIN_BDC || lp_server_role() == ROLE_DOMAIN_PDC || lp_server_role() == ROLE_IPA_DC) { struct loadparm_context *lp_ctx = loadparm_init_s3(NULL, loadparm_s3_helpers()); if (!open_schannel_session_store(NULL, lp_ctx)) { exit_daemon("ERROR: Samba cannot open schannel store for secured NETLOGON operations.", EACCES); diff --git a/source3/winbindd/winbindd_misc.c b/source3/winbindd/winbindd_misc.c index 451ad6aee14..db7e1c87dee 100644 --- a/source3/winbindd/winbindd_misc.c +++ b/source3/winbindd/winbindd_misc.c @@ -76,7 +76,7 @@ static char *get_trust_type_string(TALLOC_CTX *mem_ctx, case SEC_CHAN_BDC: { int role = lp_server_role(); - if (role == ROLE_DOMAIN_PDC) { + if (role == ROLE_DOMAIN_PDC || role == ROLE_IPA_DC) { s = talloc_strdup(mem_ctx, "PDC"); if (s == NULL) { return NULL; diff --git a/source3/winbindd/winbindd_util.c b/source3/winbindd/winbindd_util.c index ac4cd731c09..42ddbfd2f44 100644 --- a/source3/winbindd/winbindd_util.c +++ b/source3/winbindd/winbindd_util.c @@ -1254,15 +1254,37 @@ bool init_domain_list(void) secure_channel_type = SEC_CHAN_LOCAL; } - status = add_trusted_domain(get_global_sam_name(), - NULL, - get_global_sam_sid(), - LSA_TRUST_TYPE_DOWNLEVEL, - trust_flags, - 0, /* trust_attribs */ - secure_channel_type, - NULL, - &domain); + if ((pdb_domain_info != NULL) && (role == ROLE_IPA_DC)) { + /* This is IPA DC that presents itself as + * an Active Directory domain controller to trusted AD + * forests but in fact is a classic domain controller. + */ + trust_flags = NETR_TRUST_FLAG_PRIMARY; + trust_flags |= NETR_TRUST_FLAG_IN_FOREST; + trust_flags |= NETR_TRUST_FLAG_NATIVE; + trust_flags |= NETR_TRUST_FLAG_OUTBOUND; + trust_flags |= NETR_TRUST_FLAG_TREEROOT; + status = add_trusted_domain(pdb_domain_info->name, + pdb_domain_info->dns_domain, + &pdb_domain_info->sid, + LSA_TRUST_TYPE_UPLEVEL, + trust_flags, + LSA_TRUST_ATTRIBUTE_WITHIN_FOREST, + secure_channel_type, + NULL, + &domain); + TALLOC_FREE(pdb_domain_info); + } else { + status = add_trusted_domain(get_global_sam_name(), + NULL, + get_global_sam_sid(), + LSA_TRUST_TYPE_DOWNLEVEL, + trust_flags, + 0, /* trust_attribs */ + secure_channel_type, + NULL, + &domain); + } if (!NT_STATUS_IS_OK(status)) { DBG_ERR("Failed to add local SAM to " "domain to winbindd's internal list\n"); diff --git a/source4/auth/ntlm/auth.c b/source4/auth/ntlm/auth.c index 4c66f2c23cb..ea9ff70ce80 100644 --- a/source4/auth/ntlm/auth.c +++ b/source4/auth/ntlm/auth.c @@ -737,6 +737,7 @@ const char **auth_methods_from_lp(TALLOC_CTX *mem_ctx, struct loadparm_context * case ROLE_DOMAIN_BDC: case ROLE_DOMAIN_PDC: case ROLE_ACTIVE_DIRECTORY_DC: + case ROLE_IPA_DC: auth_methods = str_list_make(mem_ctx, "anonymous sam winbind sam_ignoredomain", NULL); break; } diff --git a/source4/kdc/kdc-heimdal.c b/source4/kdc/kdc-heimdal.c index a3c357ca1a6..ba74df4f2ec 100644 --- a/source4/kdc/kdc-heimdal.c +++ b/source4/kdc/kdc-heimdal.c @@ -276,6 +276,7 @@ static NTSTATUS kdc_task_init(struct task_server *task) return NT_STATUS_INVALID_DOMAIN_ROLE; case ROLE_DOMAIN_PDC: case ROLE_DOMAIN_BDC: + case ROLE_IPA_DC: task_server_terminate( task, "Cannot start KDC as a 'classic Samba' DC", false); return NT_STATUS_INVALID_DOMAIN_ROLE; diff --git a/source4/rpc_server/samr/dcesrv_samr.c b/source4/rpc_server/samr/dcesrv_samr.c index cda887d45ee..29c509522be 100644 --- a/source4/rpc_server/samr/dcesrv_samr.c +++ b/source4/rpc_server/samr/dcesrv_samr.c @@ -575,6 +575,7 @@ static NTSTATUS dcesrv_samr_info_DomGeneralInformation(struct samr_domain_state break; case ROLE_DOMAIN_PDC: case ROLE_DOMAIN_BDC: + case ROLE_IPA_DC: case ROLE_AUTO: return NT_STATUS_INTERNAL_ERROR; case ROLE_DOMAIN_MEMBER: @@ -723,6 +724,7 @@ static NTSTATUS dcesrv_samr_info_DomInfo7(struct samr_domain_state *state, break; case ROLE_DOMAIN_PDC: case ROLE_DOMAIN_BDC: + case ROLE_IPA_DC: case ROLE_AUTO: return NT_STATUS_INTERNAL_ERROR; case ROLE_DOMAIN_MEMBER: -- 2.33.1 From 4b3a3fa914b79c548100eda1baa5c589fd8f04bf Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 5 Oct 2021 18:11:57 +0200 Subject: [PATCH 31/39] CVE-2020-25719 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 Reviewed-by: Andrew Bartlett [jsutton@samba.org Removed knownfail entries] --- 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 e185acc0c20..694661b53b5 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" #undef DBGC_CLASS #define DBGC_CLASS DBGC_AUTH @@ -46,10 +48,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 5b7e9cc45370601553e464e539d0243eeec98659 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 11 Oct 2021 23:17:19 +0200 Subject: [PATCH 32/39] CVE-2020-25719 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 Reviewed-by: Andrew Bartlett --- source4/auth/auth.h | 8 ------ source4/auth/ntlm/auth.c | 49 ++++-------------------------------- source4/auth/ntlm/auth_sam.c | 12 --------- 3 files changed, 5 insertions(+), 64 deletions(-) diff --git a/source4/auth/auth.h b/source4/auth/auth.h index 3f9fb1ae3cb..6b7db99cbe2 100644 --- a/source4/auth/auth.h +++ b/source4/auth/auth.h @@ -69,14 +69,6 @@ struct auth_operations { TALLOC_CTX *mem_ctx, struct auth_user_info_dc **interim_info, bool *authoritative); - - /* 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; }; struct auth_method_context { diff --git a/source4/auth/ntlm/auth.c b/source4/auth/ntlm/auth.c index ea9ff70ce80..3dd2ffc9276 100644 --- a/source4/auth/ntlm/auth.c +++ b/source4/auth/ntlm/auth.c @@ -86,48 +86,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) @@ -627,8 +585,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 a521bc94bc4..dbbf97665db 100644 --- a/source4/auth/ntlm/auth_sam.c +++ b/source4/auth/ntlm/auth_sam.c @@ -938,22 +938,11 @@ static NTSTATUS authsam_want_check(struct auth_method_context *ctx, return NT_STATUS_OK; } -/* 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_send = authsam_check_password_send, .check_password_recv = authsam_check_password_recv, - .get_user_info_dc_principal = authsam_get_user_info_dc_principal_wrapper, }; static const struct auth_operations sam_ops = { @@ -961,7 +950,6 @@ static const struct auth_operations sam_ops = { .want_check = authsam_want_check, .check_password_send = authsam_check_password_send, .check_password_recv = authsam_check_password_recv, - .get_user_info_dc_principal = authsam_get_user_info_dc_principal_wrapper, }; _PUBLIC_ NTSTATUS auth4_sam_init(TALLOC_CTX *); -- 2.33.1 From 85914f7dbe62cb0092f106ee89a4cff46b91fb3e Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 21 Sep 2021 12:27:28 +0200 Subject: [PATCH 33/39] 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 Reviewed-by: Andrew Bartlett --- 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 b3ab3a473bf..16431771e74 100644 --- a/source3/utils/ntlm_auth.c +++ b/source3/utils/ntlm_auth.c @@ -818,23 +818,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; } } @@ -842,7 +846,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 { @@ -872,7 +877,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 128802ff973fd3a3089e672f7d81c517a779c6be Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 21 Sep 2021 12:44:01 +0200 Subject: [PATCH 34/39] 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 Reviewed-by: Andrew Bartlett --- 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 16431771e74..02a23792890 100644 --- a/source3/utils/ntlm_auth.c +++ b/source3/utils/ntlm_auth.c @@ -790,10 +790,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) { @@ -810,79 +808,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 e9063c114461ac82c5db3fa32f59b57a87be65be Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 4 Oct 2021 19:42:20 +0200 Subject: [PATCH 35/39] 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 Reviewed-by: Andrew Bartlett --- source3/auth/auth_generic.c | 137 ++++++++++++++++++++++++++++-------- 1 file changed, 109 insertions(+), 28 deletions(-) diff --git a/source3/auth/auth_generic.c b/source3/auth/auth_generic.c index f6a501f1df7..a8390b1b156 100644 --- a/source3/auth/auth_generic.c +++ b/source3/auth/auth_generic.c @@ -46,6 +46,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; @@ -54,39 +55,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 + 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(); @@ -99,18 +120,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 @@ -121,22 +214,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, @@ -170,6 +247,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, @@ -179,7 +258,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 a05d4ea28d68fda457c12a66052cb3712939279a Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 5 Oct 2021 17:14:01 +0200 Subject: [PATCH 36/39] 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 Reviewed-by: Andrew Bartlett --- selftest/knownfail.d/ktest | 26 -------------------------- selftest/target/Samba3.pm | 12 +++++------- 2 files changed, 5 insertions(+), 33 deletions(-) delete mode 100644 selftest/knownfail.d/ktest diff --git a/selftest/knownfail.d/ktest b/selftest/knownfail.d/ktest deleted file mode 100644 index 809612ba0b9..00000000000 --- a/selftest/knownfail.d/ktest +++ /dev/null @@ -1,26 +0,0 @@ -^samba3.rpc.lsa.lookupsids.krb5.with.old.ccache.ncacn_np.with..smb2...lsa.LookupSidsReply.ktest -^samba3.rpc.lsa.lookupsids.krb5.ncacn_np.with..smb2...lsa.LookupSidsReply.ktest -^samba3.blackbox.rpcclient.krb5.ncacn_np.with..krb5...rpcclient.ktest:local -^samba3.blackbox.rpcclient.krb5.ncacn_np.with..spnego,krb5...rpcclient.ktest:local -^samba3.rpc.lsa.lookupsids.krb5.with.old.ccache.ncacn_np.with..smb2,connect...lsa.LookupSidsReply.ktest -^samba3.rpc.lsa.lookupsids.krb5.ncacn_np.with..smb2,connect...lsa.LookupSidsReply.ktest -^samba3.rpc.lsa.lookupsids.krb5.with.old.ccache.ncacn_np.with..smb2,packet...lsa.LookupSidsReply.ktest -^samba3.rpc.lsa.lookupsids.krb5.ncacn_np.with..smb2,packet...lsa.LookupSidsReply.ktest -^samba3.blackbox.rpcclient.krb5.ncacn_np.with..krb5,packet...rpcclient.ktest:local -^samba3.blackbox.rpcclient.krb5.ncacn_np.with..spnego,krb5,packet...rpcclient.ktest:local -^samba3.rpc.lsa.lookupsids.krb5.with.old.ccache.ncacn_np.with..smb2,sign...lsa.LookupSidsReply.ktest -^samba3.rpc.lsa.lookupsids.krb5.ncacn_np.with..smb2,sign...lsa.LookupSidsReply.ktest -^samba3.blackbox.rpcclient.krb5.ncacn_np.with..krb5,sign...rpcclient.ktest:local -^samba3.blackbox.rpcclient.krb5.ncacn_np.with..spnego,krb5,sign...rpcclient.ktest:local -^samba3.rpc.lsa.lookupsids.krb5.with.old.ccache.ncacn_np.with..smb2,seal...lsa.LookupSidsReply.ktest -^samba3.rpc.lsa.lookupsids.krb5.ncacn_np.with..smb2,seal...lsa.LookupSidsReply.ktest -^samba3.blackbox.rpcclient.krb5.ncacn_np.with..krb5,seal...rpcclient.ktest:local -^samba3.blackbox.rpcclient.krb5.ncacn_np.with..spnego,krb5,seal...rpcclient.ktest:local -^samba3.blackbox.smbclient_krb5.old.ccache..smbclient.ktest:local -^samba3.blackbox.smbclient_krb5.new.ccache..smbclient.ktest:local -^samba3.blackbox.smbclient_large_file..krb5.smbclient.large.posix.write.read.ktest:local -^samba3.blackbox.smbclient_large_file..krb5.cmp.of.read.and.written.files.ktest:local -^samba3.blackbox.smbclient_krb5.old.ccache.--client-protection=encrypt.smbclient.ktest:local -^samba3.blackbox.smbclient_krb5.new.ccache.--client-protection=encrypt.smbclient.ktest:local -^samba3.blackbox.smbclient_large_file.--client-protection=encrypt.krb5.smbclient.large.posix.write.read.ktest:local -^samba3.blackbox.smbclient_large_file.--client-protection=encrypt.krb5.cmp.of.read.and.written.files.ktest:local diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm index 5c0f71757ef..577e3170daa 100755 --- a/selftest/target/Samba3.pm +++ b/selftest/target/Samba3.pm @@ -1675,7 +1675,6 @@ sub setup_ktest workgroup = KTEST realm = ktest.samba.example.com security = ads - username map = $prefix/lib/username.map server signing = required server min protocol = SMB3_00 client max protocol = SMB3 @@ -1683,6 +1682,10 @@ sub setup_ktest # This disables NTLM auth against the local SAM, which # we use can then test this setting by. ntlm auth = disabled + + idmap config * : backend = autorid + idmap config * : range = 1000000-1999999 + idmap config * : rangesize = 100000 "; my $ret = $self->provision( @@ -1708,12 +1711,6 @@ sub setup_ktest $ret->{KRB5_CONFIG} = $ctx->{krb5_conf}; - open(USERMAP, ">$prefix/lib/username.map") or die("Unable to open $prefix/lib/username.map"); - print USERMAP " -$ret->{USERNAME} = KTEST\\Administrator -"; - close(USERMAP); - #This is the secrets.tdb created by 'net ads join' from Samba3 to a #Samba4 DC with the same parameters as are being used here. The #domain SID is S-1-5-21-1071277805-689288055-3486227160 @@ -1765,6 +1762,7 @@ $ret->{USERNAME} = KTEST\\Administrator if (not $self->check_or_start( env_vars => $ret, nmbd => "yes", + winbindd => "offline", smbd => "yes")) { return undef; } -- 2.33.1 From 0d821434ccdccad38659d76b54159b6335e351c5 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 5 Oct 2021 18:12:49 +0200 Subject: [PATCH 37/39] 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 Reviewed-by: Andrew Bartlett --- 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 a8390b1b156..babda46fc92 100644 --- a/source3/auth/auth_generic.c +++ b/source3/auth/auth_generic.c @@ -48,8 +48,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; @@ -203,19 +201,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); @@ -226,19 +225,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 2292c83cc012473d6729e578ec5a84d6be415fab Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 8 Oct 2021 17:59:59 +0200 Subject: [PATCH 38/39] 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 Reviewed-by: Andrew Bartlett --- 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 babda46fc92..8923aec0a4d 100644 --- a/source3/auth/auth_generic.c +++ b/source3/auth/auth_generic.c @@ -214,7 +214,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 097b17fee44..46fae447347 100644 --- a/source3/auth/proto.h +++ b/source3/auth/proto.h @@ -423,7 +423,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 074e8c7eb71..7b69ca6c222 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 5607df9a4601a89735c1f9a3eaa700310d89c79a Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 8 Oct 2021 18:03:04 +0200 Subject: [PATCH 39/39] 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 Reviewed-by: Andrew Bartlett --- 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 8923aec0a4d..fa22a0b2339 100644 --- a/source3/auth/auth_generic.c +++ b/source3/auth/auth_generic.c @@ -227,7 +227,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 46fae447347..fb7f663512b 100644 --- a/source3/auth/proto.h +++ b/source3/auth/proto.h @@ -434,9 +434,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 7b69ca6c222..b8f37cbeee0 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