From 15c83b4f2180bbe0f886efefcf86e8edda24afd0 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Sat, 18 Jan 2020 08:06:45 +0100 Subject: [PATCH 01/30] s3/auth: use set_current_user_info() in auth3_generate_session_info_pac() This delays reloading config slightly, but I don't see how could affect observable behaviour other then log messages coming from the functions in between the different locations for lp_load_with_shares() like make_session_info_krb5() are sent to a different logfile if "log file" uses %U. Signed-off-by: Ralph Boehme Reviewed-by: Andreas Schneider (cherry picked from commit dc4b1e39ce1f2201a2d6ae2d4cffef2448f69a62) [scabrero@samba.org Prerequisite for CVE-2020-25717 backport] --- source3/auth/auth_generic.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/source3/auth/auth_generic.c b/source3/auth/auth_generic.c index 167d4e00367..0e9c423efef 100644 --- a/source3/auth/auth_generic.c +++ b/source3/auth/auth_generic.c @@ -159,12 +159,6 @@ static NTSTATUS auth3_generate_session_info_pac(struct auth4_context *auth_ctx, } } - /* setup the string used by %U */ - sub_set_smb_name(username); - - /* reload services so that the new %U is taken into account */ - lp_load_with_shares(get_dyn_CONFIGFILE()); - status = make_session_info_krb5(mem_ctx, ntuser, ntdomain, username, pw, info3_copy, is_guest, is_mapped, NULL /* No session key for now, caller will sort it out */, @@ -176,6 +170,14 @@ static NTSTATUS auth3_generate_session_info_pac(struct auth4_context *auth_ctx, goto done; } + /* setup the string used by %U */ + set_current_user_info((*session_info)->unix_info->sanitized_username, + (*session_info)->unix_info->unix_name, + (*session_info)->info->domain_name); + + /* reload services so that the new %U is taken into account */ + lp_load_with_shares(get_dyn_CONFIGFILE()); + DEBUG(5, (__location__ "OK: user: %s domain: %s client: %s\n", ntuser, ntdomain, rhost)); -- 2.33.1 From 6a71f642772177b9574a732908137a0dff15516f Mon Sep 17 00:00:00 2001 From: Samuel Cabrero Date: Thu, 4 Nov 2021 11:51:08 +0100 Subject: [PATCH 02/30] selftest: Fix ktest usermap file The user was not mapped: user_in_list: checking user |KTEST/administrator| against |KTEST\Administrator| The user 'KTEST/administrator' has no mapping. Skip it next time. Signed-off-by: Samuel Cabrero [scabrero@samba.org Once smb_getpswnam() fallbacks are removed the user has to be mapped] --- selftest/target/Samba3.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm index 67e5b0b5291..b07c360c658 100755 --- a/selftest/target/Samba3.pm +++ b/selftest/target/Samba3.pm @@ -1123,7 +1123,7 @@ sub setup_ktest open(USERMAP, ">$prefix/lib/username.map") or die("Unable to open $prefix/lib/username.map"); print USERMAP " -$ret->{USERNAME} = KTEST\\Administrator +$ret->{USERNAME} = KTEST/Administrator "; close(USERMAP); -- 2.33.1 From c674478435452a1d056e5388e37f6e85d7e44a7d Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 5 Oct 2021 16:42:00 +0200 Subject: [PATCH 03/30] 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) (cherry picked from commit 89b9cb8b786c3e4eb8691b5363390b68d8228a2d) [scabrero@samba.org Backported to 4.10] --- selftest/target/Samba3.pm | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm index b07c360c658..e50d4bd2a70 100755 --- a/selftest/target/Samba3.pm +++ b/selftest/target/Samba3.pm @@ -1332,7 +1332,7 @@ sub check_or_start($$$$$) { $ENV{ENVNAME} = "$ENV{ENVNAME}.winbindd"; - if ($winbindd ne "yes") { + if ($winbindd ne "yes" and $winbindd ne "offline") { $SIG{USR1} = $SIG{ALRM} = $SIG{INT} = $SIG{QUIT} = $SIG{TERM} = sub { my $signame = shift; print("Skip winbindd received signal $signame"); @@ -2582,13 +2582,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 { if ($ret != 0) { -- 2.33.1 From 77adecf1b2238dbb1a219ba680e4441a45f2b45a Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 22 Oct 2021 16:20:36 +0200 Subject: [PATCH 04/30] CVE-2020-25719 CVE-2020-25717: selftest: remove "gensec:require_pac" settings BUG: https://bugzilla.samba.org/show_bug.cgi?id=14801 BUG: https://bugzilla.samba.org/show_bug.cgi?id=14556 Signed-off-by: Stefan Metzmacher --- selftest/selftest.pl | 2 -- selftest/target/Samba4.pm | 2 -- 2 files changed, 4 deletions(-) diff --git a/selftest/selftest.pl b/selftest/selftest.pl index f2968139cfd..8c273951ab3 100755 --- a/selftest/selftest.pl +++ b/selftest/selftest.pl @@ -637,8 +637,6 @@ sub write_clientconf($$$) client lanman auth = Yes log level = 1 torture:basedir = $clientdir -#We don't want to pass our self-tests if the PAC code is wrong - gensec:require_pac = true #We don't want to run 'speed' tests for very long torture:timelimit = 1 winbind separator = / diff --git a/selftest/target/Samba4.pm b/selftest/target/Samba4.pm index a7a6c4c9587..0f644661176 100755 --- a/selftest/target/Samba4.pm +++ b/selftest/target/Samba4.pm @@ -777,8 +777,6 @@ sub provision_raw_step1($$) notify:inotify = false ldb:nosync = true ldap server require strong auth = yes -#We don't want to pass our self-tests if the PAC code is wrong - gensec:require_pac = true log file = $ctx->{logdir}/log.\%m log level = $ctx->{server_loglevel} lanman auth = Yes -- 2.33.1 From ec9b9969885220e67596e4be88d94b7364cd39ef Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 4 Oct 2021 17:29:34 +0200 Subject: [PATCH 05/30] 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 [scabrero@samba.org Backported for 4.10 due to no logon_id for log_authentication() neither is_allowed_domain()] --- source3/winbindd/winbindd_dual_srv.c | 7 +++++++ source3/winbindd/winbindd_irpc.c | 7 +++++++ source3/winbindd/winbindd_pam.c | 13 ++++++++++--- source3/winbindd/winbindd_pam_auth_crap.c | 9 ++++++++- source3/winbindd/winbindd_util.c | 7 +++++++ 5 files changed, 39 insertions(+), 4 deletions(-) diff --git a/source3/winbindd/winbindd_dual_srv.c b/source3/winbindd/winbindd_dual_srv.c index 9d3b12ba964..c5dff1cb3ed 100644 --- a/source3/winbindd/winbindd_dual_srv.c +++ b/source3/winbindd/winbindd_dual_srv.c @@ -956,6 +956,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 8cbb0b93086..45615c2dc47 100644 --- a/source3/winbindd/winbindd_irpc.c +++ b/source3/winbindd/winbindd_irpc.c @@ -143,6 +143,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 35018fbe284..deed81d0a79 100644 --- a/source3/winbindd/winbindd_pam.c +++ b/source3/winbindd/winbindd_pam.c @@ -1703,7 +1703,7 @@ static NTSTATUS winbindd_dual_pam_auth_samlogon( unsigned char local_nt_response[24]; 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; union netr_Validation *validation = NULL; @@ -2238,6 +2238,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", @@ -2420,7 +2427,7 @@ enum winbindd_result winbindd_dual_pam_auth_crap(struct winbindd_domain *domain, const char *name_user = NULL; const char *name_domain = NULL; const char *workstation; - uint8_t authoritative = 0; + uint8_t authoritative = 1; uint32_t flags = 0; uint16_t validation_level; union netr_Validation *validation = NULL; @@ -2482,7 +2489,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; } @@ -2526,6 +2532,7 @@ done: } set_auth_errors(state->response, result); + state->response->data.auth.authoritative = authoritative; return NT_STATUS_IS_OK(result) ? WINBINDD_OK : WINBINDD_ERROR; } diff --git a/source3/winbindd/winbindd_pam_auth_crap.c b/source3/winbindd/winbindd_pam_auth_crap.c index b7912db43df..40cab81b5ea 100644 --- a/source3/winbindd/winbindd_pam_auth_crap.c +++ b/source3/winbindd/winbindd_pam_auth_crap.c @@ -24,6 +24,7 @@ struct winbindd_pam_auth_crap_state { struct winbindd_response *response; + bool authoritative; uint32_t flags; }; @@ -45,7 +46,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) { @@ -124,6 +125,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); } @@ -184,6 +190,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 3245c70bb8e..315eb366a52 100644 --- a/source3/winbindd/winbindd_util.c +++ b/source3/winbindd/winbindd_util.c @@ -2062,6 +2062,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 54cb4b7760f503787a7167f6e8ebe28efffe61df Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 4 Oct 2021 17:29:34 +0200 Subject: [PATCH 06/30] 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 --- 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 3a3fa7eaa59..f754bd5cd44 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 8050ca7df5ac9e0172e23df448aafaf28846eb57 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 26 Oct 2021 17:42:41 +0200 Subject: [PATCH 07/30] 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 --- 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 e689dfd5e98..957cb410712 100644 --- a/source4/torture/rpc/samlogon.c +++ b/source4/torture/rpc/samlogon.c @@ -1385,7 +1385,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); @@ -1498,7 +1498,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 c237c82bbe7..72d0bf28fdd 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 cdf4f541fdd0dc59bd9908c791719d35289e3c15 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 26 Oct 2021 17:42:41 +0200 Subject: [PATCH 08/30] 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 --- 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 13f13934412..5e817eecd4b 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 b2b890eec4ed54cf485af6504ca2511a1204ca2c Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 26 Oct 2021 17:42:41 +0200 Subject: [PATCH 09/30] 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 --- 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 fcd9050979d..da8f094a838 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 d7b4a4b568fa357f52fe403601071ce6e71680e5 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 26 Oct 2021 17:42:41 +0200 Subject: [PATCH 10/30] 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 --- 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 6949e778565..4a40bda0b8b 100644 --- a/source3/utils/ntlm_auth.c +++ b/source3/utils/ntlm_auth.c @@ -1766,7 +1766,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()); @@ -2235,7 +2235,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 ed4caeeb1562dc79436fd45099215d26c6623f5c Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 26 Oct 2021 17:42:41 +0200 Subject: [PATCH 11/30] 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 [scabrero@samba.org Backported to 4.10 due to missing commit a5548af018643f2e78c482e33ef0e6073db149e4 to check return value of SMBOWFencrypt()] --- 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 64bc45e6a7c..48190e78bf8 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; SMBOWFencrypt(pdb_get_nt_passwd(pdb_entry), challenge_8, local_nt_response); -- 2.33.1 From e3e56ca5bf867f7bbd7b57040def8893f62fd9f8 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 26 Oct 2021 17:42:41 +0200 Subject: [PATCH 12/30] 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 --- 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 631740562c6..30fa1ed7816 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 ee5aedd988f8eb2b634e008569f70e538abc0d3c Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 26 Oct 2021 17:42:41 +0200 Subject: [PATCH 13/30] 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 [scabrero@samba.org Backported to 4.10 due to missing commits 7f75dec865256049e99f7fcf46317cd2d53e95d1 and 434030ba711e677fdd167a255d05c1cd4db943b7] --- 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 0e9c423efef..4ef2270cb34 100644 --- a/source3/auth/auth_generic.c +++ b/source3/auth/auth_generic.c @@ -415,7 +415,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; if (auth_context->check_ntlm_password_send != NULL) { struct tevent_context *ev = NULL; diff --git a/source3/auth/auth_samba4.c b/source3/auth/auth_samba4.c index a71c75631d7..bf7ccb4348c 100644 --- a/source3/auth/auth_samba4.c +++ b/source3/auth/auth_samba4.c @@ -118,7 +118,7 @@ static NTSTATUS check_samba4_security(const struct auth_context *auth_context, NTSTATUS nt_status; struct auth_user_info_dc *user_info_dc; struct auth4_context *auth4_context; - uint8_t authoritative = 0; + uint8_t authoritative = 1; nt_status = make_auth4_context_s4(auth_context, mem_ctx, &auth4_context); if (!NT_STATUS_IS_OK(nt_status)) { -- 2.33.1 From b3da94faaf4e62864dde44113e63fe122ce36701 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 26 Oct 2021 17:42:41 +0200 Subject: [PATCH 14/30] 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 --- 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 140e89daeb1..eebada670be 100644 --- a/auth/ntlmssp/ntlmssp_server.c +++ b/auth/ntlmssp/ntlmssp_server.c @@ -830,7 +830,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 cfbf6f5206e7fb04ccae6771c1c7e9c415aa9d11 Mon Sep 17 00:00:00 2001 From: Samuel Cabrero Date: Tue, 28 Sep 2021 10:43:40 +0200 Subject: [PATCH 15/30] 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 4c3dfff24f3..4aa91f4d404 100644 --- a/lib/param/loadparm.c +++ b/lib/param/loadparm.c @@ -3015,6 +3015,10 @@ struct loadparm_context *loadparm_init(TALLOC_CTX *mem_ctx) lpcfg_do_global_parameter( lp_ctx, "ldap max search request size", "256000"); + 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 05576e91699..42f491dd18d 100644 --- a/source3/param/loadparm.c +++ b/source3/param/loadparm.c @@ -963,6 +963,8 @@ static void init_globals(struct loadparm_context *lp_ctx, bool reinit_globals) Globals.ldap_max_authenticated_request_size = 16777216; Globals.ldap_max_search_request_size = 256000; + 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 8b92966915775306832e7149dd47433bf40d9b07 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 8 Oct 2021 19:57:18 +0200 Subject: [PATCH 16/30] 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 --- 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 4ef2270cb34..26a38f92b30 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 dd30b6f5fac1c74ce4afad5f086fb2a58440c254 Mon Sep 17 00:00:00 2001 From: Samuel Cabrero Date: Tue, 28 Sep 2021 10:45:11 +0200 Subject: [PATCH 17/30] 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 --- 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 8ff20c33759..8801d3f0f0b 100644 --- a/source3/auth/auth_util.c +++ b/source3/auth/auth_util.c @@ -2078,6 +2078,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 592d1be8ff4545685ee1e4c194f10bb6241d4f05 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 8 Oct 2021 17:40:30 +0200 Subject: [PATCH 18/30] 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 --- 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 424a9f9b124735c614d49d1511bae6da1f16409e Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 8 Oct 2021 18:08:20 +0200 Subject: [PATCH 19/30] 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 --- 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 8801d3f0f0b..6ee500493e6 100644 --- a/source3/auth/auth_util.c +++ b/source3/auth/auth_util.c @@ -1873,7 +1873,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 2b86fc2c17f6427f6bc864b605298c965d7bcd85 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Fri, 8 Oct 2021 12:33:16 +0200 Subject: [PATCH 20/30] 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 --- source3/auth/auth_util.c | 77 ++++++++++++++++++++++------------------ 1 file changed, 42 insertions(+), 35 deletions(-) diff --git a/source3/auth/auth_util.c b/source3/auth/auth_util.c index 6ee500493e6..161e05c2106 100644 --- a/source3/auth/auth_util.c +++ b/source3/auth/auth_util.c @@ -1908,7 +1908,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 */ @@ -1927,48 +1927,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 fd262186108554980600c3c493cd2131a4bf19aa Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 4 Oct 2021 18:03:55 +0200 Subject: [PATCH 21/30] 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 --- 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 161e05c2106..c0e5cfd7fa8 100644 --- a/source3/auth/auth_util.c +++ b/source3/auth/auth_util.c @@ -551,13 +551,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 1775c7a3229f9e242541b2bbee3ce1ec379c11b8 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 5 Oct 2021 18:11:57 +0200 Subject: [PATCH 22/30] 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 --- 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 a4740a46a013c3d13f5a7c18fa9907a5a77e3424 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 11 Oct 2021 23:17:19 +0200 Subject: [PATCH 23/30] 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 [abartlet@samba.org Backported from master/4.15 as check_password is sync in 4.14] --- 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 51895c9259f..f16d0649de2 100644 --- a/source4/auth/auth.h +++ b/source4/auth/auth.h @@ -73,14 +73,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 f754bd5cd44..41c14546b65 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) @@ -663,8 +621,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 fb88cb87f66..a8c7d8b4b85 100644 --- a/source4/auth/ntlm/auth_sam.c +++ b/source4/auth/ntlm/auth_sam.c @@ -854,28 +854,16 @@ 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 = authsam_check_password_internals, - .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, - .get_user_info_dc_principal = authsam_get_user_info_dc_principal_wrapper, }; _PUBLIC_ NTSTATUS auth4_sam_init(TALLOC_CTX *); -- 2.33.1 From 26cf3a0cc0ca0e9400e5e35336984a4d5941bcec Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 21 Sep 2021 12:27:28 +0200 Subject: [PATCH 24/30] 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 --- 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 4a40bda0b8b..63755d97fba 100644 --- a/source3/utils/ntlm_auth.c +++ b/source3/utils/ntlm_auth.c @@ -827,23 +827,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; } } @@ -851,7 +855,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 { @@ -881,7 +886,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 8f0fd0a55f1fcb3877dc3cc1a30df21ba2f317d1 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 21 Sep 2021 12:44:01 +0200 Subject: [PATCH 25/30] 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 --- 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 63755d97fba..b1e1127e771 100644 --- a/source3/utils/ntlm_auth.c +++ b/source3/utils/ntlm_auth.c @@ -799,10 +799,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) { @@ -819,79 +817,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 64999f9e3f0934a28a5a071407099d59e75a534a Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 4 Oct 2021 19:42:20 +0200 Subject: [PATCH 26/30] 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 [abartlet@samba.org Backported due to change in structure initialization with { 0 } to zero ] [abartlet@samba.org backported to 4.12 due to conflict with code not present to reload shared on krb5 login] --- source3/auth/auth_generic.c | 139 ++++++++++++++++++++++++++++-------- 1 file changed, 110 insertions(+), 29 deletions(-) diff --git a/source3/auth/auth_generic.c b/source3/auth/auth_generic.c index 26a38f92b30..3099e8f9057 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 - struct wbcAuthUserParams params = {}; + if (tsocket_address_is_inet(remote_address, "ip")) { + rhost = tsocket_address_inet_addr_string( + remote_address, tmp_ctx); + if (rhost == NULL) { + status = NT_STATUS_NO_MEMORY; + goto done; + } + } else { + rhost = "127.0.0.1"; + } + + if (server_role != ROLE_STANDALONE) { + struct wbcAuthUserParams params = { 0 }; struct wbcAuthUserInfo *info = NULL; struct wbcAuthErrorInfo *err = NULL; + struct auth_serversupplied_info *server_info = NULL; + char *original_user_name = NULL; + char *p = NULL; wbcErr wbc_err; + if (pac_blob == NULL) { + /* + * This should already be catched at the main + * gensec layer, but better check twice + */ + status = NT_STATUS_INTERNAL_ERROR; + goto done; + } + /* * Let winbind decode the PAC. * This will also store the user * data in the netsamlogon cache. * - * We need to do this *before* we - * call get_user_from_kerberos_info() - * as that does a user lookup that - * expects info in the netsamlogon cache. - * - * See BUG: https://bugzilla.samba.org/show_bug.cgi?id=11259 + * This used to be a cache prime + * optimization, but now we delegate + * all logic to winbindd, as we require + * winbindd as domain member anyway. */ params.level = WBC_AUTH_USER_LEVEL_PAC; params.password.pac.data = pac_blob->data; params.password.pac.length = pac_blob->length; + /* we are contacting the privileged pipe */ become_root(); wbc_err = wbcAuthenticateUserEx(¶ms, &info, &err); unbecome_root(); @@ -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 20b9d8cd711a7e6b91024e4d0205caa7c074c434 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 5 Oct 2021 17:14:01 +0200 Subject: [PATCH 27/30] 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 [scabrero@samba.org Backported to 4.11 Run winbindd in offline mode but keep the user name mapping to avoid having to backport fixes for bso#14539] --- selftest/target/Samba3.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm index e50d4bd2a70..58b1933163f 100755 --- a/selftest/target/Samba3.pm +++ b/selftest/target/Samba3.pm @@ -1175,7 +1175,7 @@ $ret->{USERNAME} = KTEST/Administrator # access the share for tests. chmod 0777, "$prefix/share"; - if (not $self->check_or_start($ret, "yes", "no", "yes")) { + if (not $self->check_or_start($ret, "yes", "offline", "yes")) { return undef; } return $ret; -- 2.33.1 From df55440f43709faff492966ec760149e4d60313b Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 5 Oct 2021 18:12:49 +0200 Subject: [PATCH 28/30] 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 [abartlet@samba.org Backported to Samba 4.12 has conflcits as the share reload code is in a different spot] --- source3/auth/auth_generic.c | 29 +++++++++-------------------- 1 file changed, 9 insertions(+), 20 deletions(-) diff --git a/source3/auth/auth_generic.c b/source3/auth/auth_generic.c index 3099e8f9057..23f746c078e 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 9344e71b5adebc6d57f250e8a4fc319b3cb8e06c Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 8 Oct 2021 17:59:59 +0200 Subject: [PATCH 29/30] 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 --- 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 23f746c078e..a11aae713f5 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 fcfd1f36ca2..1ed3f4a2f77 100644 --- a/source3/auth/proto.h +++ b/source3/auth/proto.h @@ -416,7 +416,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 c8cb01d9887286283836c4603a27bae354e68e59 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 8 Oct 2021 18:03:04 +0200 Subject: [PATCH 30/30] 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 --- 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 a11aae713f5..4dd1af784bf 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 1ed3f4a2f77..c00ac70fd3f 100644 --- a/source3/auth/proto.h +++ b/source3/auth/proto.h @@ -427,9 +427,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