Blob Blame History Raw
From 0e179b5f06988c576a1fff505c06920d51fe8ed4 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze@samba.org>
Date: Fri, 12 Nov 2021 15:27:58 +0100
Subject: [PATCH 1/3] CVE-2020-25727: idmap_nss: verify that the name of the
 sid belongs to the configured domain

We already check the sid belongs to the domain, but checking the name
too feels better and make it easier to understand.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
(cherry picked from commit bfd093648b4af51d104096c0cb3535e8706671e5)
---
 source3/winbindd/idmap_nss.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/source3/winbindd/idmap_nss.c b/source3/winbindd/idmap_nss.c
index da50e2b4aa7..2729a0de3f3 100644
--- a/source3/winbindd/idmap_nss.c
+++ b/source3/winbindd/idmap_nss.c
@@ -139,18 +139,21 @@ static NTSTATUS idmap_nss_sids_to_unixids(struct idmap_domain *dom, struct id_ma
 	for (i = 0; ids[i]; i++) {
 		struct group *gr;
 		enum lsa_SidType type;
-		const char *p = NULL;
+		const char *_domain = NULL;
+		const char *_name = NULL;
+		char *domain = NULL;
 		char *name = NULL;
 		bool ret;
 
 		/* by default calls to winbindd are disabled
 		   the following call will not recurse so this is safe */
 		(void)winbind_on();
-		ret = winbind_lookup_sid(talloc_tos(), ids[i]->sid, NULL,
-					 &p, &type);
+		ret = winbind_lookup_sid(talloc_tos(),
+					 ids[i]->sid,
+					 &_domain,
+					 &_name,
+					 &type);
 		(void)winbind_off();
-		name = discard_const_p(char, p);
-
 		if (!ret) {
 			/* TODO: how do we know if the name is really not mapped,
 			 * or something just failed ? */
@@ -158,6 +161,18 @@ static NTSTATUS idmap_nss_sids_to_unixids(struct idmap_domain *dom, struct id_ma
 			continue;
 		}
 
+		domain = discard_const_p(char, _domain);
+		name = discard_const_p(char, _name);
+
+		if (!strequal(domain, dom->name)) {
+			struct dom_sid_buf buf;
+			DBG_ERR("DOMAIN[%s] ignoring SID[%s] belongs to %s [%s\\%s]\n",
+			        dom->name, dom_sid_str_buf(ids[i]->sid, &buf),
+				sid_type_lookup(type), domain, name);
+			ids[i]->status = ID_UNMAPPED;
+			continue;
+		}
+
 		switch (type) {
 		case SID_NAME_USER: {
 			struct passwd *pw;
@@ -190,6 +205,7 @@ static NTSTATUS idmap_nss_sids_to_unixids(struct idmap_domain *dom, struct id_ma
 			ids[i]->status = ID_UNKNOWN;
 			break;
 		}
+		TALLOC_FREE(domain);
 		TALLOC_FREE(name);
 	}
 	return NT_STATUS_OK;
-- 
2.34.1


From 704ae4b8308e9ae6c50e3548f98de65e97ab6aa6 Mon Sep 17 00:00:00 2001
From: Joseph Sutton <josephsutton@catalyst.net.nz>
Date: Fri, 12 Nov 2021 20:53:30 +1300
Subject: [PATCH 2/3] CVE-2020-25717: nsswitch/nsstest.c: Lower 'non existent
 uid' to make room for new accounts

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

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
(cherry picked from commit fdbee5e074ebd76d659613b8b7114d70f938c38a)
---
 nsswitch/nsstest.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/nsswitch/nsstest.c b/nsswitch/nsstest.c
index e2ee9fbf3af..45270cdc459 100644
--- a/nsswitch/nsstest.c
+++ b/nsswitch/nsstest.c
@@ -466,7 +466,7 @@ static void nss_test_errors(void)
 		printf("ERROR Non existent user gave error %d\n", last_error);
 	}
 
-	pwd = getpwuid(0xFFF0);
+	pwd = getpwuid(0xFF00);
 	if (pwd || last_error != NSS_STATUS_NOTFOUND) {
 		total_errors++;
 		printf("ERROR Non existent uid gave error %d\n", last_error);
-- 
2.34.1


From 844723aa82cec67fd863fc327bde9fb04eab438d Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet@samba.org>
Date: Fri, 12 Nov 2021 16:10:31 +1300
Subject: [PATCH 3/3] CVE-2020-25717: s3:auth: Fallback to a SID/UID based
 mapping if the named based lookup fails
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Before the CVE-2020-25717 fixes we had a fallback from
getpwnam('DOMAIN\user') to getpwnam('user') which was very dangerous and
unpredictable.

Now we do the fallback based on sid_to_uid() followed by
getpwuid() on the returned uid.

This obsoletes 'username map [script]' based workaround adviced
for CVE-2020-25717, when nss_winbindd is not used or
idmap_nss is actually used.

In future we may decide to prefer or only do the SID/UID based
lookup, but for now we want to keep this unchanged as much as possible.

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

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

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

[metze@samba.org moved the new logic into the fallback codepath only
 in order to avoid behavior changes as much as possible]
Reviewed-by: Ralph Boehme <slow@samba.org>

Autobuild-User(master): Ralph Böhme <slow@samba.org>
Autobuild-Date(master): Mon Nov 15 19:01:56 UTC 2021 on sn-devel-184

(cherry picked from commit 0a546be05295a7e4a552f9f4f0c74aeb2e9a0d6e)
---
 source3/auth/auth_util.c | 34 +++++++++++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/source3/auth/auth_util.c b/source3/auth/auth_util.c
index 065b525500f..7a97dd45f11 100644
--- a/source3/auth/auth_util.c
+++ b/source3/auth/auth_util.c
@@ -1862,7 +1862,9 @@ const struct auth_session_info *get_session_info_system(void)
 ***************************************************************************/
 
 static NTSTATUS check_account(TALLOC_CTX *mem_ctx, const char *domain,
-			      const char *username, char **found_username,
+			      const char *username,
+			      const struct dom_sid *sid,
+			      char **found_username,
 			      struct passwd **pwd,
 			      bool *username_was_mapped)
 {
@@ -1897,6 +1899,31 @@ static NTSTATUS check_account(TALLOC_CTX *mem_ctx, const char *domain,
 	}
 
 	passwd = smb_getpwnam(mem_ctx, dom_user, &real_username, false);
+	if (!passwd && !*username_was_mapped) {
+		struct dom_sid_buf buf;
+		uid_t uid;
+		bool ok;
+
+		DBG_DEBUG("Failed to find authenticated user %s via "
+			  "getpwnam(), fallback to sid_to_uid(%s).\n",
+			  dom_user, dom_sid_str_buf(sid, &buf));
+
+		ok = sid_to_uid(sid, &uid);
+		if (!ok) {
+			DBG_ERR("Failed to convert SID %s to a UID (dom_user[%s])\n",
+				dom_sid_str_buf(sid, &buf), dom_user);
+			return NT_STATUS_NO_SUCH_USER;
+		}
+		passwd = getpwuid_alloc(mem_ctx, uid);
+		if (!passwd) {
+			DBG_ERR("Failed to find local account with UID %lld for SID %s (dom_user[%s])\n",
+				(long long)uid,
+				dom_sid_str_buf(sid, &buf),
+				dom_user);
+			return NT_STATUS_NO_SUCH_USER;
+		}
+		real_username = talloc_strdup(mem_ctx, passwd->pw_name);
+	}
 	if (!passwd) {
 		DEBUG(3, ("Failed to find authenticated user %s via "
 			  "getpwnam(), denying access.\n", dom_user));
@@ -2042,6 +2069,7 @@ NTSTATUS make_server_info_info3(TALLOC_CTX *mem_ctx,
 	bool username_was_mapped;
 	struct passwd *pwd;
 	struct auth_serversupplied_info *result;
+	struct dom_sid sid;
 	TALLOC_CTX *tmp_ctx = talloc_stackframe();
 
 	/* 
@@ -2088,9 +2116,13 @@ NTSTATUS make_server_info_info3(TALLOC_CTX *mem_ctx,
 
 	/* this call will try to create the user if necessary */
 
+	sid_copy(&sid, info3->base.domain_sid);
+	sid_append_rid(&sid, info3->base.rid);
+
 	nt_status = check_account(tmp_ctx,
 				  nt_domain,
 				  nt_username,
+				  &sid,
 				  &found_username,
 				  &pwd,
 				  &username_was_mapped);
-- 
2.34.1