[PATCH] Simplify idmap initialization for xid2sid

Volker Lendecke Volker.Lendecke at SerNet.DE
Thu Aug 20 19:59:53 UTC 2015


Hi!

The last patch in this patchset changes idmap
initialization. The goal is to remove the domain name
dependency in finding an idmap domain.

I've looked at this code while working on a bulk xids2sids
call, which would be greatl simplified when it is able to
just send xids down to the idmap child instead of requiring
names for everything.

A second effect is that we remove one dependency on the
unreliable domain list in the winbind parent.

Review appreciated!

Thanks,

Volker

P.S: In case you wondered: regcomp and regexec are listed in
the opengroup spec.

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kontakt at sernet.de
-------------- next part --------------
From f4d285967841dc04d2e9e79834a5844547f40df1 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 18 Aug 2015 13:18:33 +0200
Subject: [PATCH 1/3] loadparm3: Add scan_global_parametrics()

This routine takes a regex and goes through all parametric parameters
in [global], matching the regex. It can easily be extended to also
look at shares, but right now it will only be used to list all idmap
config domain names.

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/include/proto.h  |  9 +++++++
 source3/param/loadparm.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 73 insertions(+)

diff --git a/source3/include/proto.h b/source3/include/proto.h
index 0858289..11bd2b2 100644
--- a/source3/include/proto.h
+++ b/source3/include/proto.h
@@ -23,6 +23,9 @@
 #ifndef _PROTO_H_
 #define _PROTO_H_
 
+#include <sys/types.h>
+#include <regex.h>
+
 /* The following definitions come from lib/access.c  */
 
 bool client_match(const char *tok, const void *item);
@@ -986,6 +989,12 @@ int lp_smb2_max_credits(void);
 int lp_cups_encrypt(void);
 bool lp_widelinks(int );
 
+int wi_scan_global_parametrics(
+	const char *regex, size_t max_matches,
+	bool (*cb)(const char *string, regmatch_t matches[],
+		   void *private_data),
+	void *private_data);
+
 char *lp_parm_talloc_string(TALLOC_CTX *ctx, int snum, const char *type, const char *option, const char *def);
 const char *lp_parm_const_string(int snum, const char *type, const char *option, const char *def);
 struct loadparm_service;
diff --git a/source3/param/loadparm.c b/source3/param/loadparm.c
index 87e63e2..d9344fb 100644
--- a/source3/param/loadparm.c
+++ b/source3/param/loadparm.c
@@ -1066,6 +1066,70 @@ static struct parmlist_entry *get_parametrics(int snum, const char *type,
 	}
 }
 
+static void discard_whitespace(char *str)
+{
+	size_t len = strlen(str);
+	size_t i = 0;
+
+	while (i < len) {
+		if (isspace(str[i])) {
+			memmove(&str[i], &str[i+1], len-i);
+			len -= 1;
+			continue;
+		}
+		i += 1;
+	}
+}
+
+/*
+ * Go through all [global] parametric parameters, match the regex
+ * against it and call "cb" for a match.
+ */
+
+int wi_scan_global_parametrics(
+	const char *regex_str, size_t max_matches,
+	bool (*cb)(const char *string, regmatch_t matches[],
+		   void *private_data),
+	void *private_data)
+{
+	struct parmlist_entry *data;
+	regex_t regex;
+	int ret;
+
+	ret = regcomp(&regex, regex_str, REG_ICASE);
+	if (ret != 0) {
+		return ret;
+	}
+
+	for (data = Globals.param_opt; data != NULL; data = data->next) {
+		size_t keylen = strlen(data->key);
+		char key[keylen+1];
+		regmatch_t matches[max_matches];
+		bool stop;
+
+		memcpy(key, data->key, sizeof(key));
+		discard_whitespace(key);
+
+		ret = regexec(&regex, key, max_matches, matches, 0);
+		if (ret == REG_NOMATCH) {
+			continue;
+		}
+		if (ret != 0) {
+			goto fail;
+		}
+
+		stop = cb(key, matches, private_data);
+		if (stop) {
+			break;
+		}
+	}
+
+	ret = 0;
+fail:
+	regfree(&regex);
+	return ret;
+}
+
 
 #define MISSING_PARAMETER(name) \
     DEBUG(0, ("%s(): value is NULL or empty!\n", #name))
-- 
2.4.6


From ebd9c1c9b817cb5cf05b4af6ef8adf3b57a6f063 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Tue, 18 Aug 2015 16:58:02 +0200
Subject: [PATCH 2/3] idmap: Move idmap_init() under the static vars

Just moving code, idmap_init will need to reference the variables

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/winbindd/idmap.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/source3/winbindd/idmap.c b/source3/winbindd/idmap.c
index 1e2feb9..0ba8fda 100644
--- a/source3/winbindd/idmap.c
+++ b/source3/winbindd/idmap.c
@@ -32,21 +32,6 @@
 
 static_decl_idmap;
 
-static void idmap_init(void)
-{
-	static bool initialized;
-
-	if (initialized) {
-		return;
-	}
-
-	DEBUG(10, ("idmap_init(): calling static_init_idmap\n"));
-
-	static_init_idmap;
-
-	initialized = true;
-}
-
 /**
  * Pointer to the backend methods. Modules register themselves here via
  * smb_register_idmap.
@@ -79,6 +64,21 @@ static struct idmap_domain *passdb_idmap_domain;
 static struct idmap_domain **idmap_domains = NULL;
 static int num_domains = 0;
 
+static void idmap_init(void)
+{
+	static bool initialized;
+
+	if (initialized) {
+		return;
+	}
+
+	DEBUG(10, ("idmap_init(): calling static_init_idmap\n"));
+
+	static_init_idmap;
+
+	initialized = true;
+}
+
 static struct idmap_methods *get_methods(const char *name)
 {
 	struct idmap_backend *b;
-- 
2.4.6


From d90b1fe1d3ab1cc17ded108ec2c4b227c433cea1 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Wed, 19 Aug 2015 17:00:46 +0200
Subject: [PATCH 3/3] idmap: Initialize all idmap domains at startup

So far we have initialized idmap domains on demand indexed by name.
For sid2xid this works okay, because we could do lookupsids before
and thus get the name. For xid2sid this is more problematic. We
have to rely on enumtrustdoms to work completely, and we have to
look at the list of winbind domains in the parent to get the domain
name. Relying on domain->have_idmap_config is not particularly nice.

This patch re-works initialization of idmap domains by scanning all
parametric parameters, scanning for :backend configuration settings.
This way we get a complete list of :range definitions. This means
we can rely on the idmap domain array to be complete. This in turn
means we can live without the domain name to find a domain, we can
do a range search by uid or gid.

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/winbindd/idmap.c | 198 ++++++++++++++++++++++++++---------------------
 1 file changed, 108 insertions(+), 90 deletions(-)

diff --git a/source3/winbindd/idmap.c b/source3/winbindd/idmap.c
index 0ba8fda..100774a 100644
--- a/source3/winbindd/idmap.c
+++ b/source3/winbindd/idmap.c
@@ -64,12 +64,22 @@ static struct idmap_domain *passdb_idmap_domain;
 static struct idmap_domain **idmap_domains = NULL;
 static int num_domains = 0;
 
-static void idmap_init(void)
+static struct idmap_domain *idmap_init_named_domain(TALLOC_CTX *mem_ctx,
+						    const char *domname);
+static struct idmap_domain *idmap_init_domain(TALLOC_CTX *mem_ctx,
+					      const char *domainname,
+					      const char *modulename,
+					      bool check_range);
+static bool idmap_found_domain_backend(
+	const char *string, regmatch_t matches[], void *private_data);
+
+static bool idmap_init(void)
 {
 	static bool initialized;
+	int ret;
 
 	if (initialized) {
-		return;
+		return true;
 	}
 
 	DEBUG(10, ("idmap_init(): calling static_init_idmap\n"));
@@ -77,6 +87,79 @@ static void idmap_init(void)
 	static_init_idmap;
 
 	initialized = true;
+
+	if (!pdb_is_responsible_for_everything_else()) {
+		default_idmap_domain = idmap_init_named_domain(NULL, "*");
+		if (default_idmap_domain == NULL) {
+			return false;
+		}
+	}
+
+	passdb_idmap_domain = idmap_init_domain(
+		NULL, get_global_sam_name(), "passdb", false);
+	if (passdb_idmap_domain == NULL) {
+		TALLOC_FREE(default_idmap_domain);
+		return false;
+	}
+
+	idmap_domains = talloc_array(NULL, struct idmap_domain *, 0);
+	if (idmap_domains == NULL) {
+		TALLOC_FREE(passdb_idmap_domain);
+		TALLOC_FREE(default_idmap_domain);
+		return false;
+	}
+
+	ret = wi_scan_global_parametrics(
+		"idmapconfig\\(.*\\):backend", 2,
+		idmap_found_domain_backend, NULL);
+	if (ret != 0) {
+		DBG_WARNING("wi_scan_global_parametrics returned %d\n", ret);
+	}
+
+	return true;
+}
+
+static bool idmap_found_domain_backend(
+	const char *string, regmatch_t matches[], void *private_data)
+{
+	if (matches[1].rm_so == -1) {
+		DBG_WARNING("Found match, but no name??\n");
+		return false;
+	}
+
+	{
+		struct idmap_domain *dom, **tmp;
+		regoff_t len = matches[1].rm_eo - matches[1].rm_so;
+		char domname[len+1];
+
+		memcpy(domname, string + matches[1].rm_so, len);
+		domname[len] = '\0';
+
+		DBG_DEBUG("Found idmap domain \"%s\"\n", domname);
+
+		if (strcmp(domname, "*") == 0) {
+			return false;
+		}
+
+		dom = idmap_init_named_domain(idmap_domains, domname);
+		if (dom == NULL) {
+			DBG_NOTICE("Could not init idmap domain %s\n",
+				   domname);
+		}
+
+		tmp = talloc_realloc(idmap_domains, idmap_domains,
+				     struct idmap_domain *, num_domains + 1);
+		if (tmp == NULL) {
+			DBG_WARNING("talloc_realloc failed\n");
+			TALLOC_FREE(dom);
+			return false;
+		}
+		idmap_domains = tmp;
+		idmap_domains[num_domains] = dom;
+		num_domains += 1;
+	}
+
+	return false;
 }
 
 static struct idmap_methods *get_methods(const char *name)
@@ -280,8 +363,12 @@ static struct idmap_domain *idmap_init_named_domain(TALLOC_CTX *mem_ctx,
 	struct idmap_domain *result = NULL;
 	char *config_option;
 	const char *backend;
+	bool ok;
 
-	idmap_init();
+	ok = idmap_init();
+	if (!ok) {
+		return NULL;
+	}
 
 	config_option = talloc_asprintf(talloc_tos(), "idmap config %s",
 					domname);
@@ -312,57 +399,6 @@ fail:
 }
 
 /**
- * Initialize the default domain structure
- * @param[in] mem_ctx		memory context for the result
- * @result The default domain structure
- *
- * This routine takes the module name from the "idmap backend" parameter,
- * passing a possible parameter like ldap:ldap://ldap-url/ to the module.
- */
-
-static struct idmap_domain *idmap_init_default_domain(TALLOC_CTX *mem_ctx)
-{
-	return idmap_init_named_domain(mem_ctx, "*");
-}
-
-/**
- * Initialize the passdb domain structure
- * @param[in] mem_ctx		memory context for the result
- * @result The default domain structure
- *
- * No config, passdb has its own configuration.
- */
-
-static struct idmap_domain *idmap_passdb_domain(TALLOC_CTX *mem_ctx)
-{
-	idmap_init();
-
-	if (!pdb_is_responsible_for_everything_else()) {
-		/*
-		 * Always init the default domain, we can't go without one
-		 */
-		if (default_idmap_domain == NULL) {
-			default_idmap_domain = idmap_init_default_domain(NULL);
-		}
-		if (default_idmap_domain == NULL) {
-			return NULL;
-		}
-	}
-
-	if (passdb_idmap_domain != NULL) {
-		return passdb_idmap_domain;
-	}
-
-	passdb_idmap_domain = idmap_init_domain(mem_ctx, get_global_sam_name(),
-						"passdb", false);
-	if (passdb_idmap_domain == NULL) {
-		DEBUG(1, ("Could not init passdb idmap domain\n"));
-	}
-
-	return passdb_idmap_domain;
-}
-
-/**
  * Find a domain struct according to a domain name
  * @param[in] domname		Domain name to get the config for
  * @result The default domain structure that fits
@@ -379,21 +415,14 @@ static struct idmap_domain *idmap_passdb_domain(TALLOC_CTX *mem_ctx)
 
 static struct idmap_domain *idmap_find_domain(const char *domname)
 {
-	struct idmap_domain *result;
+	bool ok;
 	int i;
 
 	DEBUG(10, ("idmap_find_domain called for domain '%s'\n",
 		   domname?domname:"NULL"));
 
-	idmap_init();
-
-	/*
-	 * Always init the default domain, we can't go without one
-	 */
-	if (default_idmap_domain == NULL) {
-		default_idmap_domain = idmap_init_default_domain(NULL);
-	}
-	if (default_idmap_domain == NULL) {
+	ok = idmap_init();
+	if (!ok) {
 		return NULL;
 	}
 
@@ -407,38 +436,21 @@ static struct idmap_domain *idmap_find_domain(const char *domname)
 		}
 	}
 
-	if (idmap_domains == NULL) {
-		/*
-		 * talloc context for all idmap domains
-		 */
-		idmap_domains = talloc_array(NULL, struct idmap_domain *, 1);
-	}
-
-	if (idmap_domains == NULL) {
-		DEBUG(0, ("talloc failed\n"));
-		return NULL;
-	}
-
-	result = idmap_init_named_domain(idmap_domains, domname);
-	if (result == NULL) {
-		/*
-		 * Could not init that domain -- try the default one
-		 */
-		return default_idmap_domain;
-	}
-
-	ADD_TO_ARRAY(idmap_domains, struct idmap_domain *, result,
-		     &idmap_domains, &num_domains);
-	return result;
+	return default_idmap_domain;
 }
 
 struct idmap_domain *idmap_find_domain_with_sid(const char *domname,
 						const struct dom_sid *sid)
 {
-	idmap_init();
+	bool ok;
+
+	ok = idmap_init();
+	if (!ok) {
+		return NULL;
+	}
 
 	if (sid_check_is_for_passdb(sid)) {
-		return idmap_passdb_domain(NULL);
+		return passdb_idmap_domain;
 	}
 
 	return idmap_find_domain(domname);
@@ -493,6 +505,12 @@ NTSTATUS idmap_backends_unixid_to_sid(const char *domname, struct id_map *id)
 {
 	struct idmap_domain *dom;
 	struct id_map *maps[2];
+	bool ok;
+
+	ok = idmap_init();
+	if (!ok) {
+		return NT_STATUS_NONE_MAPPED;
+	}
 
 	 DEBUG(10, ("idmap_backend_unixid_to_sid: domain = '%s', xid = %d "
 		    "(type %d)\n",
@@ -505,7 +523,7 @@ NTSTATUS idmap_backends_unixid_to_sid(const char *domname, struct id_map *id)
 	 * Always give passdb a chance first
 	 */
 
-	dom = idmap_passdb_domain(NULL);
+	dom = passdb_idmap_domain;
 	if ((dom != NULL)
 	    && NT_STATUS_IS_OK(dom->methods->unixids_to_sids(dom, maps))
 	    && id->status == ID_MAPPED) {
-- 
2.4.6



More information about the samba-technical mailing list