From 6df19847674d59e079782d30ec8a9dbdc77b99e2 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl@samba.org>
Date: Sat, 20 Feb 2021 15:50:12 +0100
Subject: [PATCH] passdb: Simplify sids_to_unixids()

Best reviewed with "git show -b", there's a "continue" statement that
changes subsequent indentation.

Decouple lookup status of ids from ID_TYPE_NOT_SPECIFIED

Bug: https://bugzilla.samba.org/show_bug.cgi?id=14571

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
---
 source3/passdb/lookup_sid.c | 101 +++++++++++++++++++++++++++++++++---
 1 file changed, 95 insertions(+), 6 deletions(-)

diff --git source3/passdb/lookup_sid.c source3/passdb/lookup_sid.c
index 64a181e..cd59aab 100644
--- source3/passdb/lookup_sid.c
+++ source3/passdb/lookup_sid.c
@@ -27,6 +27,7 @@
 #include "idmap_cache.h"
 #include "../libcli/security/security.h"
 #include "lib/winbind_util.h"
+#include "lib/util/bitmap.h"
 
 /*****************************************************************
  Dissect a user-provided name into domain, name, sid and type.
@@ -1400,7 +1401,9 @@ bool sids_to_unix_ids(const struct dom_sid *sids, uint32_t num_sids,
 {
 	struct wbcDomainSid *wbc_sids = NULL;
 	struct wbcUnixId *wbc_ids = NULL;
+	struct bitmap *found = NULL;
 	uint32_t i, num_not_cached;
+	uint32_t wbc_ids_size = 0;
 	wbcErr err;
 	bool ret = false;
 
@@ -1408,6 +1411,20 @@ bool sids_to_unix_ids(const struct dom_sid *sids, uint32_t num_sids,
 	if (wbc_sids == NULL) {
 		return false;
 	}
+	found = bitmap_talloc(wbc_sids, num_sids);
+	if (found == NULL) {
+		goto fail;
+	}
+
+	/*
+	 * We go through the requested SID array three times.
+	 * First time to look for global_sid_Unix_Users
+	 * and global_sid_Unix_Groups SIDS, and to look
+	 * for mappings cached in the idmap_cache.
+	 *
+	 * Use bitmap_set() to mark an ids[] array entry as
+	 * being mapped.
+	 */
 
 	num_not_cached = 0;
 
@@ -1417,34 +1434,40 @@ bool sids_to_unix_ids(const struct dom_sid *sids, uint32_t num_sids,
 
 		if (fetch_uid_from_cache(&ids[i].id.uid, &sids[i])) {
 			ids[i].type = WBC_ID_TYPE_UID;
+			bitmap_set(found, i);	
 			continue;
 		}
 		if (fetch_gid_from_cache(&ids[i].id.gid, &sids[i])) {
 			ids[i].type = WBC_ID_TYPE_GID;
+			bitmap_set(found, i);	
 			continue;
 		}
 		if (sid_peek_check_rid(&global_sid_Unix_Users,
 				       &sids[i], &rid)) {
 			ids[i].type = WBC_ID_TYPE_UID;
 			ids[i].id.uid = rid;
+			bitmap_set(found, i);	
 			continue;
 		}
 		if (sid_peek_check_rid(&global_sid_Unix_Groups,
 				       &sids[i], &rid)) {
 			ids[i].type = WBC_ID_TYPE_GID;
 			ids[i].id.gid = rid;
+			bitmap_set(found, i);	
 			continue;
 		}
 		if (idmap_cache_find_sid2uid(&sids[i], &ids[i].id.uid,
 					     &expired)
 		    && !expired && ids[i].id.uid != (uid_t)-1) {
 			ids[i].type = WBC_ID_TYPE_UID;
+			bitmap_set(found, i);	
 			continue;
 		}
 		if (idmap_cache_find_sid2gid(&sids[i], &ids[i].id.gid,
 					     &expired)
 		    && !expired && ids[i].id.gid != (gid_t)-1) {
 			ids[i].type = WBC_ID_TYPE_GID;
+			bitmap_set(found, i);	
 			continue;
 		}
 		ids[i].type = WBC_ID_TYPE_NOT_SPECIFIED;
@@ -1455,43 +1478,109 @@ bool sids_to_unix_ids(const struct dom_sid *sids, uint32_t num_sids,
 	if (num_not_cached == 0) {
 		goto done;
 	}
-	wbc_ids = TALLOC_ARRAY(talloc_tos(), struct wbcUnixId, num_not_cached);
+
+	/*
+	 * For the ones that we couldn't map in the loop above, query winbindd
+	 * via wbcSidsToUnixIds().
+	 */
+
+	wbc_ids_size = num_not_cached;
+	wbc_ids = talloc_array(talloc_tos(), struct wbcUnixId, wbc_ids_size);
+
 	if (wbc_ids == NULL) {
 		goto fail;
 	}
-	for (i=0; i<num_not_cached; i++) {
+	for (i=0; i<wbc_ids_size; i++) {
 		wbc_ids[i].type = WBC_ID_TYPE_NOT_SPECIFIED;
+		wbc_ids[i].id.gid = (uint32_t)-1;
 	}
-	err = wbcSidsToUnixIds(wbc_sids, num_not_cached, wbc_ids);
+	err = wbcSidsToUnixIds(wbc_sids, wbc_ids_size, wbc_ids);
 	if (!WBC_ERROR_IS_OK(err)) {
 		DEBUG(10, ("wbcSidsToUnixIds returned %s\n",
 			   wbcErrorString(err)));
 	}
 
+	/*
+	 * Second time through the SID array, replace
+	 * the ids[] entries that wbcSidsToUnixIds() was able to
+	 * map.
+	 *
+	 * Use bitmap_set() to mark an ids[] array entry as
+	 * being mapped.
+	 */
+
 	num_not_cached = 0;
 
 	for (i=0; i<num_sids; i++) {
-		if (ids[i].type == WBC_ID_TYPE_NOT_SPECIFIED) {
+		if (bitmap_query(found, i)) {
+			continue;
+		}
+
+		SMB_ASSERT(num_not_cached < wbc_ids_size);
+
+		switch(wbc_ids[num_not_cached].type) {
+		case WBC_ID_TYPE_UID:
+		case WBC_ID_TYPE_GID:
 			ids[i] = wbc_ids[num_not_cached];
-			num_not_cached += 1;
+			bitmap_set(found, i);
+			break;
+		case WBC_ID_TYPE_NOT_SPECIFIED:
+			/*
+			 * wbcSidsToUnixIds() wasn't able to map this
+			 * so we still need to check legacy_sid_to_XXX()
+			 * below. Don't mark the bitmap entry
+			 * as being found so the final loop knows
+			 * to try and map this entry.
+			 */
+ 			ids[i].type = WBC_ID_TYPE_NOT_SPECIFIED;
+			ids[i].id.gid = (uint32_t)-1;
+			break;
+		default:
+			/*
+			 * A successful return from wbcSidsToUnixIds()
+			 * cannot return anything other than the values
+			 * checked for above. Ensure this is so.
+			 */
+			smb_panic(__location__);
+			break;
 		}
+		num_not_cached += 1;
 	}
 
+	/*
+	 * Third and final time through the SID array,
+	 * try legacy_sid_to_gid()/legacy_sid_to_uid()
+	 * for entries we haven't already been able to
+	 * map.
+	 *
+	 * Use bitmap_set() to mark an ids[] array entry as
+	 * being mapped.
+	 */	
+
 	for (i=0; i<num_sids; i++) {
-		if (ids[i].type != WBC_ID_TYPE_NOT_SPECIFIED) {
+		if (bitmap_query(found, i)) {
 			continue;
 		}
+
 		if (legacy_sid_to_gid(&sids[i], &ids[i].id.gid)) {
 			ids[i].type = WBC_ID_TYPE_GID;
+			bitmap_set(found, i);
 			continue;
 		}
 		if (legacy_sid_to_uid(&sids[i], &ids[i].id.uid)) {
 			ids[i].type = WBC_ID_TYPE_UID;
+			bitmap_set(found, i);
 			continue;
 		}
 	}
 
 done:
+	/*
+	 * Pass through the return array for consistency.
+	 * Any ids[].id mapped to (uint32_t)-1 must be returned
+	 * as ID_TYPE_NOT_SPECIFIED.
+	 */
+
 	for (i=0; i<num_sids; i++) {
 		switch(ids[i].type) {
 		case WBC_ID_TYPE_GID:
-- 
2.26.2