From a3f31271cd7e5083a5bae359609a0e81232bbb91 Mon Sep 17 00:00:00 2001 From: Samuel Cabrero Date: Mon, 8 Nov 2021 20:22:01 +0100 Subject: [PATCH 1/8] CVE-2020-25717: libcli/util: Add NT_STATUS_INVALID_TOKEN error code Signed-off-by: Samuel Cabrero --- libcli/util/ntstatus.h | 1 + source3/libsmb/nterr.c | 1 + 2 files changed, 2 insertions(+) diff --git a/libcli/util/ntstatus.h b/libcli/util/ntstatus.h index 3e7c6296954..3b6f9d46dd4 100644 --- a/libcli/util/ntstatus.h +++ b/libcli/util/ntstatus.h @@ -623,6 +623,7 @@ typedef uint32_t NTSTATUS; #define NT_STATUS_RPC_PIPE_EMPTY NT_STATUS(0xC0000000 | 0x30061) #define NT_STATUS_ERROR_DS_OBJ_STRING_NAME_EXISTS NT_STATUS(0xC0000000 | 0x2071) #define NT_STATUS_ERROR_DS_INCOMPATIBLE_VERSION NT_STATUS(0xC0000000 | 0x00002177) +#define NT_STATUS_INVALID_TOKEN NT_STATUS(0xC0000000 | 0x0465) /* I use NT_STATUS_FOOBAR when I have no idea what error code to use - * this means we need a torture test */ diff --git a/source3/libsmb/nterr.c b/source3/libsmb/nterr.c index e48f221be0d..f1b1d64c303 100644 --- a/source3/libsmb/nterr.c +++ b/source3/libsmb/nterr.c @@ -571,6 +571,7 @@ static const nt_err_code_struct nt_errs[] = { "NT_STATUS_INVALID_LOCK_RANGE", NT_STATUS_INVALID_LOCK_RANGE }, { "NT_STATUS_ERROR_DS_OBJ_STRING_NAME_EXISTS", NT_STATUS_ERROR_DS_OBJ_STRING_NAME_EXISTS }, { "NT_STATUS_ERROR_DS_INCOMPATIBLE_VERSION", NT_STATUS_ERROR_DS_INCOMPATIBLE_VERSION }, + { "NT_STATUS_INVALID_TOKEN", NT_STATUS_INVALID_TOKEN }, { "STATUS_MORE_ENTRIES", STATUS_MORE_ENTRIES }, { "STATUS_SOME_UNMAPPED", STATUS_SOME_UNMAPPED }, { "STATUS_NOTIFY_CLEANUP", STATUS_NOTIFY_CLEANUP }, -- 2.33.1 From 989793e5e7d625aecf17dd631441cfeaed73ba3f Mon Sep 17 00:00:00 2001 From: Samuel Cabrero Date: Tue, 28 Sep 2021 10:43:40 +0200 Subject: [PATCH 2/8] 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] [scabrero@samba.org Backported to 4.6] [scabrero@samba.org Backported to 3.6] --- docs-xml/smbdotconf/security/mindomainuid.xml | 17 +++++++++++++++++ docs-xml/smbdotconf/winbind/idmapconfig.xml | 4 ++++ source3/include/proto.h | 1 + source3/param/loadparm.c | 13 +++++++++++++ 4 files changed, 35 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 69bddf0ebf7..5aa4ab7bee2 100644 --- a/docs-xml/smbdotconf/winbind/idmapconfig.xml +++ b/docs-xml/smbdotconf/winbind/idmapconfig.xml @@ -90,6 +90,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! @@ -125,4 +128,5 @@ +min domain uid diff --git a/source3/include/proto.h b/source3/include/proto.h index 7303e76c961..ceae1c457fd 100644 --- a/source3/include/proto.h +++ b/source3/include/proto.h @@ -1668,6 +1668,7 @@ bool lp_afs_share(int ); bool lp_acl_check_permissions(int ); bool lp_acl_group_control(int ); bool lp_acl_map_full_control(int ); +int lp_min_domain_uid(void); int lp_create_mask(int ); int lp_force_create_mode(int ); int lp_security_mask(int ); diff --git a/source3/param/loadparm.c b/source3/param/loadparm.c index dd633399a05..5dfbd4c84a8 100644 --- a/source3/param/loadparm.c +++ b/source3/param/loadparm.c @@ -385,6 +385,7 @@ struct global { int ismb2_max_trans; int ismb2_max_credits; char *ncalrpc_dir; + int min_domain_uid; }; static struct global Globals; @@ -4798,6 +4799,15 @@ static struct parm_struct parm_table[] = { .enum_list = NULL, .flags = FLAG_ADVANCED, }, + { + .label = "min domain uid", + .type = P_INTEGER, + .p_class = P_GLOBAL, + .ptr = &Globals.min_domain_uid, + .special = NULL, + .enum_list = NULL, + .flags = FLAG_ADVANCED, + }, {NULL, P_BOOL, P_NONE, NULL, NULL, NULL, 0} }; @@ -5495,6 +5505,8 @@ static void init_globals(bool reinit_globals) string_set(&Globals.ncalrpc_dir, get_dyn_NCALRPCDIR()); + Globals.min_domain_uid = 1000; + /* Now put back the settings that were set with lp_set_cmdline() */ apply_lp_set_cmdline(); } @@ -6024,6 +6036,7 @@ FN_LOCAL_BOOL(lp_afs_share, bAfs_Share) FN_LOCAL_BOOL(lp_acl_check_permissions, bAclCheckPermissions) FN_LOCAL_BOOL(lp_acl_group_control, bAclGroupControl) FN_LOCAL_BOOL(lp_acl_map_full_control, bAclMapFullControl) +FN_GLOBAL_INTEGER(lp_min_domain_uid, &Globals.min_domain_uid) FN_LOCAL_INTEGER(lp_create_mask, iCreate_mask) FN_LOCAL_INTEGER(lp_force_create_mode, iCreate_force_mode) FN_LOCAL_INTEGER(lp_security_mask, iSecurity_mask) -- 2.33.1 From 38d63d8dac4966bbabf0e2af19d967ba00bc5148 Mon Sep 17 00:00:00 2001 From: Samuel Cabrero Date: Tue, 28 Sep 2021 10:45:11 +0200 Subject: [PATCH 3/8] 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 [scabrero@samba.org Backported to 3.6] --- 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 288f46142e7..f82947eed0d 100644 --- a/source3/auth/auth_util.c +++ b/source3/auth/auth_util.c @@ -1311,6 +1311,22 @@ NTSTATUS make_server_info_info3(TALLOC_CTX *mem_ctx, if (!NT_STATUS_IS_OK(nt_status)) { return nt_status; + } 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; + DEBUG(3, ("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))); + return nt_status; } result = make_server_info(NULL); -- 2.33.1 From 2eec87d2736cb5404271f76cd9376f1a5c0a66e7 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 8 Oct 2021 17:40:30 +0200 Subject: [PATCH 4/8] 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 e52149afd7e..c75a8b550ef 100644 --- a/source3/auth/user_krb5.c +++ b/source3/auth/user_krb5.c @@ -153,7 +153,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 f5a41c51bf4e660818e640cad96366bfab5e069b Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 8 Oct 2021 18:08:20 +0200 Subject: [PATCH 5/8] 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 [scabrero@samba.org Backported to 3.6] --- 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 f82947eed0d..29c897e5344 100644 --- a/source3/auth/auth_util.c +++ b/source3/auth/auth_util.c @@ -1121,7 +1121,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 165607fa787ae1226e985dbe4dd87b4dbc24b438 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Fri, 8 Oct 2021 12:33:16 +0200 Subject: [PATCH 6/8] 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 [scabrero@samba.org Backported to 4.7 removing selftest/knownfail.d/ktest after fixing user mapping in ktest environment] --- source3/auth/auth_util.c | 77 ++++++++++++++++++++++------------------ 1 file changed, 42 insertions(+), 35 deletions(-) diff --git a/source3/auth/auth_util.c b/source3/auth/auth_util.c index 29c897e5344..ba397c82638 100644 --- a/source3/auth/auth_util.c +++ b/source3/auth/auth_util.c @@ -1156,7 +1156,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 */ @@ -1175,48 +1175,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 d71810ec82f81390047012a9fdfa047eb56224c1 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 4 Oct 2021 18:03:55 +0200 Subject: [PATCH 7/8] 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 [scabrero@samba.org Backported to 3.6] --- source3/auth/auth_util.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/source3/auth/auth_util.c b/source3/auth/auth_util.c index ba397c82638..329b341d164 100644 --- a/source3/auth/auth_util.c +++ b/source3/auth/auth_util.c @@ -451,13 +451,12 @@ NTSTATUS create_local_token(struct auth_serversupplied_info *server_info) struct wbcUnixId *ids; /* - * 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) { status = create_token_from_username(server_info, server_info->unix_name, server_info->guest, -- 2.33.1 From 41d7efb3555fef5fc7ec112b2ebe7aa9cd28dc23 Mon Sep 17 00:00:00 2001 From: Samuel Cabrero Date: Fri, 5 Nov 2021 13:51:33 +0100 Subject: [PATCH 8/8] CVE-2020-25717: s3:auth: Always require a PAC in domain mode 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: Samuel Cabrero --- source3/auth/user_krb5.c | 17 ++++++++++++----- source3/smbd/sesssetup.c | 8 ++++++++ source3/smbd/smb2_sesssetup.c | 7 +++++++ source3/utils/ntlm_auth.c | 9 +++++++++ 4 files changed, 36 insertions(+), 5 deletions(-) diff --git a/source3/auth/user_krb5.c b/source3/auth/user_krb5.c index c75a8b550ef..74e23aff840 100644 --- a/source3/auth/user_krb5.c +++ b/source3/auth/user_krb5.c @@ -213,16 +213,23 @@ NTSTATUS make_server_info_krb5(TALLOC_CTX *mem_ctx, } } else { - /* - * We didn't get a PAC, we have to make up the user - * ourselves. Try to ask the pdb backend to provide - * SID consistency with ntlmssp session setup - */ struct samu *sampass; /* The stupid make_server_info_XX functions here don't take a talloc context. */ struct auth_serversupplied_info *tmp = NULL; + if (lp_server_role() != ROLE_STANDALONE) { + status = NT_STATUS_ACCESS_DENIED; + DEBUG(2, ("make_server_info_krb5: Kerberos ticket " + "has no PAC: %s\n", nt_errstr(status))); + return status; + } + + /* + * We didn't get a PAC, we have to make up the user + * ourselves. Try to ask the pdb backend to provide + * SID consistency with ntlmssp session setup + */ sampass = samu_new(talloc_tos()); if (sampass == NULL) { return NT_STATUS_NO_MEMORY; diff --git a/source3/smbd/sesssetup.c b/source3/smbd/sesssetup.c index 75c2a1551d1..9e4370539df 100644 --- a/source3/smbd/sesssetup.c +++ b/source3/smbd/sesssetup.c @@ -345,6 +345,14 @@ static void reply_spnego_kerberos(struct smb_request *req, return; } + if (lp_server_role() != ROLE_STANDALONE && logon_info == NULL) { + DEBUG(2, ("Unable to find PAC in ticket from %s, failing " + "to allow access\n", principal)); + ret = NT_STATUS_NO_IMPERSONATION_TOKEN; + reply_nterror(req, nt_status_squash(ret)); + return; + } + ret = get_user_from_kerberos_info(talloc_tos(), sconn->client_id.name, principal, logon_info, diff --git a/source3/smbd/smb2_sesssetup.c b/source3/smbd/smb2_sesssetup.c index 1f48e332e85..0673e498867 100644 --- a/source3/smbd/smb2_sesssetup.c +++ b/source3/smbd/smb2_sesssetup.c @@ -207,6 +207,13 @@ static NTSTATUS smbd_smb2_session_setup_krb5(struct smbd_smb2_session *session, goto fail; } + if (lp_server_role() != ROLE_STANDALONE && logon_info == NULL) { + DEBUG(2, ("Unable to find PAC in ticket from %s, failing " + "to allow access\n", principal)); + status = NT_STATUS_NO_IMPERSONATION_TOKEN; + goto fail; + } + status = get_user_from_kerberos_info(talloc_tos(), smb2req->sconn->client_id.name, principal, logon_info, diff --git a/source3/utils/ntlm_auth.c b/source3/utils/ntlm_auth.c index 73f41a7fe66..84119afbc12 100644 --- a/source3/utils/ntlm_auth.c +++ b/source3/utils/ntlm_auth.c @@ -1530,6 +1530,15 @@ static void manage_gss_spnego_request(struct ntlm_auth_state *state, if (NT_STATUS_IS_OK(status)) { + if (lp_server_role() != ROLE_STANDALONE && logon_info == NULL) { + DEBUG(2, ("Unable to find PAC in ticket from %s, failing " + "to allow access\n", principal)); + x_fprintf(x_stdout, "BH Unable to find PAC " + "in ticket from %s, failing " + "to allow access\n", principal); + return; + } + domain = strchr_m(principal, '@'); if (domain == NULL) { -- 2.33.1