Attachment 7667 Details for Bug 9002 – Patches for v3-6-test

[patch] Patches for v3-6-test

tmp36.diff (text/plain), 7.17 KB, created by Stefan Metzmacher on 2012-06-21 13:52:32 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Stefan Metzmacher

Created: 2012-06-21 13:52:32 UTC

Size: 7.17 KB

patch

obsolete

>From 826f83966ac03868072f64978763e129dc1c849b Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Fri, 2 Mar 2012 05:08:17 +0100
>Subject: [PATCH 1/4] s3:winbindd: discard the expired gid cache if we're online (bug #9002)
>
>This matches the uid case...
>
>metze
>---
> source3/winbindd/winbindd_sids_to_xids.c |    2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
>diff --git a/source3/winbindd/winbindd_sids_to_xids.c b/source3/winbindd/winbindd_sids_to_xids.c
>index 9c38b5a..b3699d0 100644
>--- a/source3/winbindd/winbindd_sids_to_xids.c
>+++ b/source3/winbindd/winbindd_sids_to_xids.c
>@@ -135,7 +135,7 @@ static bool winbindd_sids_to_xids_in_cache(struct dom_sid *sid,
> 	 * sids. So we check groups before users.
> 	 */
> 	if (idmap_cache_find_sid2gid(sid, &gid, &expired)) {
>-		if (expired && is_domain_offline(find_our_domain())) {
>+		if (expired && is_domain_online(find_our_domain())) {
> 			return false;
> 		}
> 		map->sid = sid;
>-- 
>1.7.4.1
>
>
>From 680fb7321347adb56625456ffdd7a9aad7c6d19e Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Thu, 24 May 2012 09:08:21 +0200
>Subject: [PATCH 2/4] s3:winbindd: do not expose negative cache idmap entries as valid mappings (bug #9002)
>
>metze
>---
> source3/winbindd/winbindd_sids_to_xids.c |   22 ++++++++++++++++------
> 1 files changed, 16 insertions(+), 6 deletions(-)
>
>diff --git a/source3/winbindd/winbindd_sids_to_xids.c b/source3/winbindd/winbindd_sids_to_xids.c
>index b3699d0..62f09f0 100644
>--- a/source3/winbindd/winbindd_sids_to_xids.c
>+++ b/source3/winbindd/winbindd_sids_to_xids.c
>@@ -256,21 +256,26 @@ NTSTATUS winbindd_sids_to_xids_recv(struct tevent_req *req,
> 
> 	for (i=0; i<state->num_sids; i++) {
> 		char type;
>-		uint64_t unix_id = (uint64_t)-1;
>+		uint32_t unix_id = UINT32_MAX;
> 		bool found = true;
> 
> 		if (state->cached[i].sid != NULL) {
> 			unix_id = state->cached[i].xid.id;
>-			if (state->cached[i].xid.type == ID_TYPE_UID) {
>+
>+			switch (state->cached[i].xid.type) {
>+			case ID_TYPE_UID:
> 				type = 'U';
>-			} else {
>+				break;
>+			case ID_TYPE_GID:
> 				type = 'G';
>+				break;
>+			default:
>+				found = false;
>+				break;
> 			}
> 		} else {
> 			unix_id = state->ids.ids[num_non_cached].unix_id;
>-			if (unix_id == -1) {
>-				found = false;
>-			}
>+
> 			switch(state->ids.ids[num_non_cached].type) {
> 			case WBC_ID_TYPE_UID:
> 				type = 'U';
>@@ -286,10 +291,15 @@ NTSTATUS winbindd_sids_to_xids_recv(struct tevent_req *req,
> 				break;
> 			default:
> 				found = false;
>+				break;
> 			}
> 			num_non_cached += 1;
> 		}
> 
>+		if (unix_id == UINT32_MAX) {
>+			found = false;
>+		}
>+
> 		if (found) {
> 			result = talloc_asprintf_append_buffer(
> 				result, "%c%lu\n", type,
>-- 
>1.7.4.1
>
>
>From cdf3a24dac8eee2dde63c0ce5870740f02941c75 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Tue, 19 Jun 2012 17:57:19 +0200
>Subject: [PATCH 3/4] s3:passdb: don't turn negative cache entries into valid idmappings (bug #9002)
>
>It's typical that some file operations set a NTACL, which tries
>sid2uid() before sid2gid(), this will create a negative cache entry.
>
>Negative SID2UID entries cause that a valid SID2GID mapping is ignored
>and the group is ignored in the UNIX Token.
>
>metze
>---
> source3/passdb/lookup_sid.c |    4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/source3/passdb/lookup_sid.c b/source3/passdb/lookup_sid.c
>index 2afa86e..64a181e 100644
>--- a/source3/passdb/lookup_sid.c
>+++ b/source3/passdb/lookup_sid.c
>@@ -1437,13 +1437,13 @@ bool sids_to_unix_ids(const struct dom_sid *sids, uint32_t num_sids,
> 		}
> 		if (idmap_cache_find_sid2uid(&sids[i], &ids[i].id.uid,
> 					     &expired)
>-		    && !expired) {
>+		    && !expired && ids[i].id.uid != (uid_t)-1) {
> 			ids[i].type = WBC_ID_TYPE_UID;
> 			continue;
> 		}
> 		if (idmap_cache_find_sid2gid(&sids[i], &ids[i].id.gid,
> 					     &expired)
>-		    && !expired) {
>+		    && !expired && ids[i].id.gid != (gid_t)-1) {
> 			ids[i].type = WBC_ID_TYPE_GID;
> 			continue;
> 		}
>-- 
>1.7.4.1
>
>
>From 2a2e651030060fe5529bc11342bf3e635f2b5802 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Tue, 19 Jun 2012 17:57:19 +0200
>Subject: [PATCH 4/4] s3:winbindd: don't turn negative cache entries into valid idmappings (bug #9002)
>
>It's typical that some file operations set a NTACL, which tries
>sid2uid() before sid2gid(), this will create a negative cache entry.
>
>Negative SID2UID entries cause that a valid SID2GID mapping is ignored
>and the group is ignored in the UNIX Token.
>
>metze
>---
> source3/winbindd/winbindd_sids_to_xids.c |   72 ++++++++++++++++++++++++-----
> 1 files changed, 59 insertions(+), 13 deletions(-)
>
>diff --git a/source3/winbindd/winbindd_sids_to_xids.c b/source3/winbindd/winbindd_sids_to_xids.c
>index 62f09f0..7487f8f 100644
>--- a/source3/winbindd/winbindd_sids_to_xids.c
>+++ b/source3/winbindd/winbindd_sids_to_xids.c
>@@ -123,41 +123,87 @@ struct tevent_req *winbindd_sids_to_xids_send(TALLOC_CTX *mem_ctx,
> static bool winbindd_sids_to_xids_in_cache(struct dom_sid *sid,
> 					   struct id_map *map)
> {
>-	uid_t uid;
>-	gid_t gid;
>-	bool expired;
>+	bool is_online = is_domain_online(find_our_domain());
>+	gid_t gid = (gid_t)-1;
>+	bool gid_expired = false;
>+	bool gid_cached = false;
>+	bool gid_negative = false;
>+	uid_t uid = (uid_t)-1;
>+	bool uid_expired = false;
>+	bool uid_cached = false;
>+	bool uid_negative = false;
> 
> 	if (!winbindd_use_idmap_cache()) {
> 		return false;
> 	}
>+
> 	/*
> 	 * SIDS_TO_XIDS is primarily used to resolve the user's group
> 	 * sids. So we check groups before users.
> 	 */
>-	if (idmap_cache_find_sid2gid(sid, &gid, &expired)) {
>-		if (expired && is_domain_online(find_our_domain())) {
>-			return false;
>+	gid_cached = idmap_cache_find_sid2gid(sid, &gid, &gid_expired);
>+	if (!is_online) {
>+		gid_expired = false;
>+	}
>+	if (gid_cached && !gid_expired) {
>+		if (gid != (gid_t)-1) {
>+			map->sid = sid;
>+			map->xid.id = gid;
>+			map->xid.type = ID_TYPE_GID;
>+			map->status = ID_MAPPED;
>+			return true;
>+		}
>+		gid_negative = true;
>+	}
>+	uid_cached = idmap_cache_find_sid2uid(sid, &uid, &uid_expired);
>+	if (!is_online) {
>+		uid_expired = false;
>+	}
>+	if (uid_cached && !uid_expired) {
>+		if (uid != (uid_t)-1) {
>+			map->sid = sid;
>+			map->xid.id = uid;
>+			map->xid.type = ID_TYPE_UID;
>+			map->status = ID_MAPPED;
>+			return true;
> 		}
>+		uid_negative = true;
>+	}
>+
>+	/*
>+	 * Here we know that we only have negative
>+	 * or no entries.
>+	 *
>+	 * All valid cases already returned to the caller.
>+	 */
>+
>+	if (gid_negative && uid_negative) {
> 		map->sid = sid;
>-		map->xid.id = gid;
>+		map->xid.id = UINT32_MAX;
>+		map->xid.type = ID_TYPE_NOT_SPECIFIED;
>+		map->status = ID_MAPPED;
>+		return true;
>+	}
>+
>+	if (gid_negative) {
>+		map->sid = sid;
>+		map->xid.id = gid; /* this is (gid_t)-1 */
> 		map->xid.type = ID_TYPE_GID;
> 		map->status = ID_MAPPED;
> 		return true;
> 	}
>-	if (idmap_cache_find_sid2uid(sid, &uid, &expired)) {
>-		if (expired && is_domain_online(find_our_domain())) {
>-			return false;
>-		}
>+
>+	if (uid_negative) {
> 		map->sid = sid;
>-		map->xid.id = uid;
>+		map->xid.id = uid; /* this is (uid_t)-1 */
> 		map->xid.type = ID_TYPE_UID;
> 		map->status = ID_MAPPED;
> 		return true;
> 	}
>+
> 	return false;
> }
> 
>-
> static void winbindd_sids_to_xids_lookupsids_done(struct tevent_req *subreq)
> {
> 	struct tevent_req *req = tevent_req_callback_data(
>-- 
>1.7.4.1
>

Flags:

obnox: review+

Actions: View

Attachments on bug 9002: 7661 | 7662 | 7663 | 7667