s3-idmap: Rework idmap_cache to store ID_TYPE_BOTH values

This required that the lower level cache store a UID/GID and a type, and that
we operate on struct unixid rather than just uid/gid.

The ID_TYPE_BOTH is then handled as being a positive mapping for both
a UID and GID value.  Wrapper functions are provided so that callers are not
changed in this patch.

Andrew Bartlett

Signed-off-by: Michael Adam <obnox@samba.org>

2 files changed, 240 insertions, 176 deletions

diff --git a/source3/lib/idmap_cache.c b/source3/lib/idmap_cache.c
index 413029c7a1..6bba5b0551 100644
--- a/source3/lib/idmap_cache.c
+++ b/source3/lib/idmap_cache.c
@@ -20,150 +20,155 @@
 #include "includes.h"
 #include "idmap_cache.h"
 #include "../libcli/security/security.h"
+#include "../librpc/gen_ndr/idmap.h"
 
 /**
- * Find a sid2uid mapping
+ * Find a sid2xid mapping
  * @param[in] sid		the sid to map
- * @param[out] puid		where to put the result
+ * @param[out] id		where to put the result
  * @param[out] expired		is the cache entry expired?
  * @retval Was anything in the cache at all?
  *
- * If *puid == -1 this was a negative mapping.
+ * If id->id == -1 this was a negative mapping.
  */
 
-bool idmap_cache_find_sid2uid(const struct dom_sid *sid, uid_t *puid,
-			      bool *expired)
+bool idmap_cache_find_sid2unixid(const struct dom_sid *sid, struct unixid *id,
+				 bool *expired)
 {
 	fstring sidstr;
 	char *key;
 	char *value;
 	char *endptr;
 	time_t timeout;
-	uid_t uid;
 	bool ret;
+	struct unixid tmp_id;
 
-	key = talloc_asprintf(talloc_tos(), "IDMAP/SID2UID/%s",
+	key = talloc_asprintf(talloc_tos(), "IDMAP/SID2XID/%s",
 			      sid_to_fstring(sidstr, sid));
 	if (key == NULL) {
 		return false;
 	}
 	ret = gencache_get(key, &value, &timeout);
-	TALLOC_FREE(key);
 	if (!ret) {
+		TALLOC_FREE(key);
 		return false;
 	}
-	uid = strtol(value, &endptr, 10);
-	ret = (*endptr == '\0');
-	SAFE_FREE(value);
+	tmp_id.id = strtol(value, &endptr, 10);
+	DEBUG(0, ("Parsing result of %s, endptr=%s, id=%llu\n", key, endptr, (unsigned long long)tmp_id.id));
+
+	ret = (*endptr == ':');
 	if (ret) {
-		*puid = uid;
+		switch (endptr[1]) {
+		case 'U':
+			tmp_id.type = ID_TYPE_UID;
+			break;
+
+		case 'G':
+			tmp_id.type = ID_TYPE_GID;
+			break;
+
+		case 'B':
+			tmp_id.type = ID_TYPE_BOTH;
+			break;
+
+		case '\0':
+		default:
+			TALLOC_FREE(key);
+			SAFE_FREE(value);
+			DEBUG(0, ("FAILED Parsing result of %s, endptr=%s, id=%llu\n", key, endptr, (unsigned long long)tmp_id.id));
+			return false;
+		}
+		if (endptr[2] != '\0') {
+			TALLOC_FREE(key);
+			SAFE_FREE(value);
+			DEBUG(0, ("FAILED (2) Parsing result of %s, endptr=%s, id=%llu\n", key, endptr, (unsigned long long)tmp_id.id));
+			return false;
+		}
+
+		*id = tmp_id;
 		*expired = (timeout <= time(NULL));
+	} else {
+		DEBUG(0, ("FAILED (3) Parsing result of %s, value=%s\n", key, value));
 	}
+	TALLOC_FREE(key);
+	SAFE_FREE(value);
 	return ret;
 }
 
 /**
- * Find a uid2sid mapping
- * @param[in] uid		the uid to map
- * @param[out] sid		where to put the result
+ * Find a sid2uid mapping
+ * @param[in] sid		the sid to map
+ * @param[out] puid		where to put the result
  * @param[out] expired		is the cache entry expired?
  * @retval Was anything in the cache at all?
  *
- * If "is_null_sid(sid)", this was a negative mapping.
+ * If *puid == -1 this was a negative mapping.
  */
 
-bool idmap_cache_find_uid2sid(uid_t uid, struct dom_sid *sid, bool *expired)
+bool idmap_cache_find_sid2uid(const struct dom_sid *sid, uid_t *puid,
+			      bool *expired)
 {
-	char *key;
-	char *value;
-	time_t timeout;
-	bool ret = true;
-
-	key = talloc_asprintf(talloc_tos(), "IDMAP/UID2SID/%d", (int)uid);
-	if (key == NULL) {
-		return false;
-	}
-	ret = gencache_get(key, &value, &timeout);
-	TALLOC_FREE(key);
+	bool ret;
+	struct unixid id;
+	ret = idmap_cache_find_sid2unixid(sid, &id, expired);
 	if (!ret) {
 		return false;
 	}
-	ZERO_STRUCTP(sid);
-	if (value[0] != '-') {
-		ret = string_to_sid(sid, value);
-	}
-	SAFE_FREE(value);
-	if (ret) {
-		*expired = (timeout <= time(NULL));
+
+	if (id.type == ID_TYPE_BOTH || id.type == ID_TYPE_UID) {
+		*puid = id.id;
+	} else {
+		*puid = -1;
 	}
-	return ret;
+	return true;
 }
 
 /**
- * Store a mapping in the idmap cache
+ * Find a sid2gid mapping
  * @param[in] sid		the sid to map
- * @param[in] uid		the uid to map
+ * @param[out] pgid		where to put the result
+ * @param[out] expired		is the cache entry expired?
+ * @retval Was anything in the cache at all?
  *
- * If both parameters are valid values, then a positive mapping in both
- * directions is stored. If "is_null_sid(sid)" is true, then this will be a
- * negative mapping of uid, we want to cache that for this uid we could not
- * find anything. Likewise if "uid==-1", then we want to cache that we did not
- * find a mapping for the sid passed here.
+ * If *pgid == -1 this was a negative mapping.
  */
 
-void idmap_cache_set_sid2uid(const struct dom_sid *sid, uid_t uid)
+bool idmap_cache_find_sid2gid(const struct dom_sid *sid, gid_t *pgid,
+			      bool *expired)
 {
-	time_t now = time(NULL);
-	time_t timeout;
-	fstring sidstr, key, value;
-
-	if (!is_null_sid(sid)) {
-		fstr_sprintf(key, "IDMAP/SID2UID/%s",
-			     sid_to_fstring(sidstr, sid));
-		fstr_sprintf(value, "%d", (int)uid);
-		timeout = (uid == -1)
-			? lp_idmap_negative_cache_time()
-			: lp_idmap_cache_time();
-		gencache_set(key, value, now + timeout);
+	bool ret;
+	struct unixid id;
+	ret = idmap_cache_find_sid2unixid(sid, &id, expired);
+	if (!ret) {
+		return false;
 	}
-	if (uid != -1) {
-		fstr_sprintf(key, "IDMAP/UID2SID/%d", (int)uid);
-		if (is_null_sid(sid)) {
-			/* negative uid mapping */
-			fstrcpy(value, "-");
-			timeout = lp_idmap_negative_cache_time();
-		}
-		else {
-			sid_to_fstring(value, sid);
-			timeout = lp_idmap_cache_time();
-		}
-		gencache_set(key, value, now + timeout);
+
+	if (id.type == ID_TYPE_BOTH || id.type == ID_TYPE_GID) {
+		*pgid = id.id;
+	} else {
+		*pgid = -1;
 	}
+	return true;
 }
 
 /**
- * Find a sid2gid mapping
- * @param[in] sid		the sid to map
- * @param[out] pgid		where to put the result
+ * Find a uid2sid mapping
+ * @param[in] uid		the uid to map
+ * @param[out] sid		where to put the result
  * @param[out] expired		is the cache entry expired?
  * @retval Was anything in the cache at all?
  *
- * If *pgid == -1 this was a negative mapping.
+ * If "is_null_sid(sid)", this was a negative mapping.
  */
 
-bool idmap_cache_find_sid2gid(const struct dom_sid *sid, gid_t *pgid,
-			      bool *expired)
+bool idmap_cache_find_uid2sid(uid_t uid, struct dom_sid *sid, bool *expired)
 {
-	fstring sidstr;
 	char *key;
 	char *value;
-	char *endptr;
 	time_t timeout;
-	gid_t gid;
-	bool ret;
+	bool ret = true;
 
-	key = talloc_asprintf(talloc_tos(), "IDMAP/SID2GID/%s",
-			      sid_to_fstring(sidstr, sid));
+	key = talloc_asprintf(talloc_tos(), "IDMAP/UID2SID/%d", (int)uid);
 	if (key == NULL) {
 		return false;
 	}
@@ -172,11 +177,12 @@ bool idmap_cache_find_sid2gid(const struct dom_sid *sid, gid_t *pgid,
 	if (!ret) {
 		return false;
 	}
-	gid = strtol(value, &endptr, 10);
-	ret = (*endptr == '\0');
+	ZERO_STRUCTP(sid);
+	if (value[0] != '-') {
+		ret = string_to_sid(sid, value);
+	}
 	SAFE_FREE(value);
 	if (ret) {
-		*pgid = gid;
 		*expired = (timeout <= time(NULL));
 	}
 	return ret;
@@ -231,23 +237,34 @@ bool idmap_cache_find_gid2sid(gid_t gid, struct dom_sid *sid, bool *expired)
  * find a mapping for the sid passed here.
  */
 
-void idmap_cache_set_sid2gid(const struct dom_sid *sid, gid_t gid)
+void idmap_cache_set_sid2unixid(const struct dom_sid *sid, struct unixid *unix_id)
 {
 	time_t now = time(NULL);
 	time_t timeout;
 	fstring sidstr, key, value;
 
 	if (!is_null_sid(sid)) {
-		fstr_sprintf(key, "IDMAP/SID2GID/%s",
+		fstr_sprintf(key, "IDMAP/SID2XID/%s",
 			     sid_to_fstring(sidstr, sid));
-		fstr_sprintf(value, "%d", (int)gid);
-		timeout = (gid == -1)
+		switch (unix_id->type) {
+		case ID_TYPE_UID:
+			fstr_sprintf(value, "%d:U", (int)unix_id->id);
+			break;
+		case ID_TYPE_GID:
+			fstr_sprintf(value, "%d:G", (int)unix_id->id);
+			break;
+		case ID_TYPE_BOTH:
+			fstr_sprintf(value, "%d:B", (int)unix_id->id);
+			break;
+		default:
+			return;
+		}
+		timeout = (unix_id->id == -1)
 			? lp_idmap_negative_cache_time()
 			: lp_idmap_cache_time();
 		gencache_set(key, value, now + timeout);
 	}
-	if (gid != -1) {
-		fstr_sprintf(key, "IDMAP/GID2SID/%d", (int)gid);
+	if (unix_id->id != -1) {
 		if (is_null_sid(sid)) {
 			/* negative gid mapping */
 			fstrcpy(value, "-");
@@ -257,58 +274,126 @@ void idmap_cache_set_sid2gid(const struct dom_sid *sid, gid_t gid)
 			sid_to_fstring(value, sid);
 			timeout = lp_idmap_cache_time();
 		}
+		switch (unix_id->type) {
+		case ID_TYPE_BOTH:
+			fstr_sprintf(key, "IDMAP/UID2SID/%d", (int)unix_id->id);
+			gencache_set(key, value, now + timeout);
+			fstr_sprintf(key, "IDMAP/GID2SID/%d", (int)unix_id->id);
+			gencache_set(key, value, now + timeout);
+			return;
+
+		case ID_TYPE_UID:
+			fstr_sprintf(key, "IDMAP/UID2SID/%d", (int)unix_id->id);
+			break;
+
+		case ID_TYPE_GID:
+			fstr_sprintf(key, "IDMAP/GID2SID/%d", (int)unix_id->id);
+			break;
+
+		default:
+			return;
+		}
 		gencache_set(key, value, now + timeout);
 	}
 }
 
-
 /**
  * Store a mapping in the idmap cache
  * @param[in] sid		the sid to map
- * @param[in] uid/gid		the uid/gid to map
+ * @param[in] uid		the uid to map
  *
  * If both parameters are valid values, then a positive mapping in both
  * directions is stored. If "is_null_sid(sid)" is true, then this will be a
- * negative mapping of gid, we want to cache that for this id we could not
- * find anything. Likewise if "id==-1", then we want to cache that we did not
+ * negative mapping of uid, we want to cache that for this uid we could not
+ * find anything. Likewise if "uid==-1", then we want to cache that we did not
  * find a mapping for the sid passed here.
  */
 
-void idmap_cache_set_sid2both(const struct dom_sid *sid, uid_t id)
+void idmap_cache_set_sid2uid(const struct dom_sid *sid, uid_t uid)
 {
-	time_t now = time(NULL);
-	time_t timeout;
-	fstring sidstr, key, value;
-
-	if (!is_null_sid(sid)) {
-		fstr_sprintf(key, "IDMAP/SID2BOTH/%s",
-			     sid_to_fstring(sidstr, sid));
-		fstr_sprintf(value, "%d", (int)id);
-		timeout = (id == -1)
-			? lp_idmap_negative_cache_time()
-			: lp_idmap_cache_time();
-		gencache_set(key, value, now + timeout);
-	}
-	if (id != -1) {
-		fstr_sprintf(key, "IDMAP/BOTH2SID/%d", (int)id);
-		if (is_null_sid(sid)) {
-			/* negative id mapping */
-			fstrcpy(value, "-");
-			timeout = lp_idmap_negative_cache_time();
+	struct unixid id;
+	id.type = ID_TYPE_UID;
+	id.id = uid;
+
+	if (uid == -1) {
+		uid_t tmp_gid;
+		bool expired;
+		/* If we were asked to invalidate this SID -> UID
+		 * mapping, it was because we found out that this was
+		 * not a UID at all.  Do not overwrite a valid GID or
+		 * BOTH mapping */
+		if (idmap_cache_find_sid2gid(sid, &tmp_gid, &expired)) {
+			if (!expired) {
+				return;
+			}
 		}
-		else {
-			sid_to_fstring(value, sid);
-			timeout = lp_idmap_cache_time();
+	}
+
+	idmap_cache_set_sid2unixid(sid, &id);
+	return;
+}
+
+/**
+ * Store a mapping in the idmap cache
+ * @param[in] sid		the sid to map
+ * @param[in] gid		the gid to map
+ *
+ * If both parameters are valid values, then a positive mapping in both
+ * directions is stored. If "is_null_sid(sid)" is true, then this will be a
+ * negative mapping of gid, we want to cache that for this gid we could not
+ * find anything. Likewise if "gid==-1", then we want to cache that we did not
+ * find a mapping for the sid passed here.
+ */
+
+void idmap_cache_set_sid2gid(const struct dom_sid *sid, gid_t gid)
+{
+	struct unixid id;
+	id.type = ID_TYPE_GID;
+	id.id = gid;
+
+	if (gid == -1) {
+		uid_t tmp_uid;
+		bool expired;
+		/* If we were asked to invalidate this SID -> GID
+		 * mapping, it was because we found out that this was
+		 * not a GID at all.  Do not overwrite a valid UID or
+		 * BOTH mapping */
+		if (idmap_cache_find_sid2uid(sid, &tmp_uid, &expired)) {
+			if (!expired) {
+				return;
+			}
 		}
-		gencache_set(key, value, now + timeout);
 	}
+
+	idmap_cache_set_sid2unixid(sid, &id);
+	return;
+}
+
+
+/**
+ * Store a mapping in the idmap cache
+ * @param[in] sid		the sid to map
+ * @param[in] gid		the gid to map
+ *
+ * If both parameters are valid values, then a positive mapping in both
+ * directions is stored. If "is_null_sid(sid)" is true, then this will be a
+ * negative mapping of gid, we want to cache that for this gid we could not
+ * find anything. Likewise if "gid==-1", then we want to cache that we did not
+ * find a mapping for the sid passed here.
+ */
+
+void idmap_cache_set_sid2both(const struct dom_sid *sid, gid_t gid)
+{
+	struct unixid id;
+	id.type = ID_TYPE_BOTH;
+	id.id = gid;
+
+	idmap_cache_set_sid2unixid(sid, &id);
+	return;
 }
 
 
 static char* key_xid2sid_str(TALLOC_CTX* mem_ctx, char t, const char* id) {
-	if (t == 'B') {
-		return talloc_asprintf(mem_ctx, "IDMAP/BOTH2SID/%s", id);
-	}
 	return talloc_asprintf(mem_ctx, "IDMAP/%cID2SID/%s", t, id);
 }
 
@@ -318,21 +403,10 @@ static char* key_xid2sid(TALLOC_CTX* mem_ctx, char t, int id) {
 	return key_xid2sid_str(mem_ctx, t, str);
 }
 
-static char* key_sid2xid_str(TALLOC_CTX* mem_ctx, char t, const char* sid) {
-	if (t == 'B') {
-		return talloc_asprintf(mem_ctx, "IDMAP/SID2BOTH/%s", sid);
-	}
-	return talloc_asprintf(mem_ctx, "IDMAP/SID2%cID/%s", t, sid);
+static char* key_sid2xid_str(TALLOC_CTX* mem_ctx, const char* id) {
+	return talloc_asprintf(mem_ctx, "IDMAP/SID2XID/%s", id);
 }
 
-/* static char* key_sid2xid(TALLOC_CTX* mem_ctx, char t, const struct dom_sid* sid) */
-/* { */
-/* 	char* sid_str = sid_string_talloc(mem_ctx, sid); */
-/* 	char* key = key_sid2xid_str(mem_ctx, t, sid_str); */
-/* 	talloc_free(sid_str); */
-/* 	return key; */
-/* } */
-
 static bool idmap_cache_del_xid(char t, int xid)
 {
 	TALLOC_CTX* mem_ctx = talloc_stackframe();
@@ -348,7 +422,7 @@ static bool idmap_cache_del_xid(char t, int xid)
 	}
 
 	if (sid_str[0] != '-') {
-		const char* sid_key = key_sid2xid_str(mem_ctx, t, sid_str);
+		const char* sid_key = key_sid2xid_str(mem_ctx, sid_str);
 		if (!gencache_del(sid_key)) {
 			DEBUG(2, ("failed to delete: %s\n", sid_key));
 			ret = false;
@@ -378,56 +452,43 @@ bool idmap_cache_del_gid(gid_t gid) {
 	return idmap_cache_del_xid('G', gid);
 }
 
-bool idmap_cache_del_both(uid_t id) {
-	return idmap_cache_del_xid('B', id);
-}
-
-static bool idmap_cache_del_sid2xid(TALLOC_CTX* mem_ctx, char t, const char* sid)
+bool idmap_cache_del_sid(const struct dom_sid *sid)
 {
-	const char* sid_key = key_sid2xid_str(mem_ctx, t, sid);
-	char* xid_str;
-	time_t timeout;
+	TALLOC_CTX* mem_ctx = talloc_stackframe();
 	bool ret = true;
+	bool expired;
+	struct unixid id;
+	const char *sid_key;
 
-	if (!gencache_get(sid_key, &xid_str, &timeout)) {
+	if (!idmap_cache_find_sid2unixid(sid, &id, &expired)) {
 		ret = false;
 		goto done;
 	}
 
-	if (atoi(xid_str) != -1) {
-		const char* xid_key = key_xid2sid_str(mem_ctx, t, xid_str);
-		if (!gencache_del(xid_key)) {
-			DEBUG(2, ("failed to delete: %s\n", xid_key));
-			ret = false;
-		} else {
-			DEBUG(5, ("delete: %s\n", xid_key));
+	if (id.id != -1) {
+		switch (id.type) {
+		case ID_TYPE_BOTH:
+			idmap_cache_del_xid('U', id.id);
+			idmap_cache_del_xid('G', id.id);
+			break;
+		case ID_TYPE_UID:
+			idmap_cache_del_xid('U', id.id);
+			break;
+		case ID_TYPE_GID:
+			idmap_cache_del_xid('G', id.id);
+			break;
+		default:
+			break;
 		}
 	}
 
-	if (!gencache_del(sid_key)) {
-		DEBUG(2, ("failed to delete: %s\n", sid_key));
-		ret = false;
-	} else {
-		DEBUG(5, ("delete: %s\n", sid_key));
+	sid_key = key_sid2xid_str(mem_ctx, dom_sid_string(mem_ctx, sid));
+	if (sid_key == NULL) {
+		return false;
 	}
+	/* If the mapping was symmetric, then this should fail */
+	gencache_del(sid_key);
 done:
-	return ret;
-}
-
-bool idmap_cache_del_sid(const struct dom_sid *sid)
-{
-	TALLOC_CTX* mem_ctx = talloc_stackframe();
-	const char* sid_str = sid_string_talloc(mem_ctx, sid);
-	bool ret = true;
-
-	if (!idmap_cache_del_sid2xid(mem_ctx, 'U', sid_str) &&
-	    !idmap_cache_del_sid2xid(mem_ctx, 'G', sid_str) &&
-	    !idmap_cache_del_sid2xid(mem_ctx, 'B', sid_str))
-	{
-		DEBUG(3, ("no entry: %s\n", key_xid2sid_str(mem_ctx, '?', sid_str)));
-		ret = false;
-	}
-
 	talloc_free(mem_ctx);
 	return ret;
 }
diff --git a/source3/lib/idmap_cache.h b/source3/lib/idmap_cache.h
index 4c87139770..afb8c16940 100644
--- a/source3/lib/idmap_cache.h
+++ b/source3/lib/idmap_cache.h
@@ -22,16 +22,19 @@
 #define _LIB_IDMAP_CACHE_H_
 
 /* The following definitions come from lib/idmap_cache.c  */
-
+struct unixid;
+bool idmap_cache_find_sid2unixid(const struct dom_sid *sid, struct unixid *id,
+				  bool *expired);
 bool idmap_cache_find_sid2uid(const struct dom_sid *sid, uid_t *puid,
 			      bool *expired);
-bool idmap_cache_find_uid2sid(uid_t uid, struct dom_sid *sid, bool *expired);
-void idmap_cache_set_sid2uid(const struct dom_sid *sid, uid_t uid);
 bool idmap_cache_find_sid2gid(const struct dom_sid *sid, gid_t *pgid,
 			      bool *expired);
+bool idmap_cache_find_uid2sid(uid_t uid, struct dom_sid *sid, bool *expired);
 bool idmap_cache_find_gid2sid(gid_t gid, struct dom_sid *sid, bool *expired);
+void idmap_cache_set_sid2unixid(const struct dom_sid *sid, struct unixid *unix_id);
+void idmap_cache_set_sid2uid(const struct dom_sid *sid, uid_t uid);
 void idmap_cache_set_sid2gid(const struct dom_sid *sid, gid_t gid);
-void idmap_cache_set_sid2both(const struct dom_sid *sid, uid_t id);
+void idmap_cache_set_sid2both(const struct dom_sid *sid, uid_t uid);
 
 bool idmap_cache_del_uid(uid_t uid);
 bool idmap_cache_del_gid(gid_t gid);