[PATCH] Check idmap config with testparm
Jeremy Allison
jra at samba.org
Mon Dec 12 19:18:03 UTC 2016
On Mon, Dec 12, 2016 at 10:15:47AM +0100, Andreas Schneider wrote:
> On Monday, 12 December 2016 09:37:07 CET Michael Adam wrote:
> > On 2016-12-08 at 10:46 -0800, Jeremy Allison wrote:
> > > On Thu, Dec 08, 2016 at 09:48:25AM +0100, Volker Lendecke wrote:
> > > > On Thu, Dec 08, 2016 at 07:58:40AM +0000, Rowland Penny wrote:
> > > > > Hi Volker, Could you explain for the idiots amongst us (i.e. me), just
> > > > > how this is supposed to work ?
> > > >
> > > > The AD backend just reads the SFU attributes in Active Directory.
> > > > This is completely controlled by the administrator of the domains. I
> > > > have several customers with a global unix id allocation policy but
> > > > where the unix ids are spread across multiple domains in a more or
> > > > less random fashion. Globally unix ids are guaranteed to be unique,
> > > > but you can't tell from a range assignment which domain they belong
> > > > to. What winbind with overlapping ranges now does is just try all of
> > > > the domains until a mapping is found. There is no guarantee of a
> > > > particular order in which domains are tried, we depend on the AD
> > > > administration to gurantee uniqueness. This is for unixid2sid,
> > > > sid2unixid is simple, there we can just find the domain from the sid.
> > >
> > > It's nice that we can do this, but I would certainly
> > > consider this under "Advanced" usage :-).
> >
> > It may be, but if you think about it, for the AD backend
> > this may actually be a very common and reasonable config.
> >
> > > For most regular users - a blanket "don't have overlapping
> > > ranges" would be very good advice !
> >
> > True. But complaining with "ERROR" about a supported
> > (albeit advanced) config may not be the right thing..
>
> Here is an updated patchset. For imapd_ad overlapping configs it will issue a
> NOTE that they overlap, for all other combnation it will print an error.
>
>
> Michael and I also implemented that winbind fails if an invalid idmap default
> backend is specified.
>
>
> If nobody pushes it earlier or objects, I will push it tomorrow.
../source3/winbindd/winbindd.c: In function ‘main’:
../source3/winbindd/winbindd.c:1699:8: error: ‘i’ undeclared (first use in this function)
for (i = 0; i < ARRAY_SIZE(invalid_backends); i++) {
^
../source3/winbindd/winbindd.c:1699:8: note: each undeclared identifier is reported only once for each function it appears in
../source3/winbindd/winbindd.c:1700:9: error: declaration of ‘ok’ shadows a previous local [-Werror=shadow]
bool ok;
^
../source3/winbindd/winbindd.c:1541:7: error: shadowed declaration is here [-Werror=shadow]
bool ok;
Fixed it up and pushed.. :-).
> Thanks,
>
>
>
> Andreas
>
>
>
> --
> Andreas Schneider GPG-ID: CC014E3D
> Samba Team asn at samba.org
> www.samba.org
> From 1ed5ff38328f42478b4e8eafdb6782872de3df34 Mon Sep 17 00:00:00 2001
> From: Andreas Schneider <asn at samba.org>
> Date: Wed, 7 Dec 2016 17:03:22 +0100
> Subject: [PATCH 1/4] s3-testparm: Fix trailing whitespaces
>
> Signed-off-by: Andreas Schneider <asn at samba.org>
> Reviewed-by: Jeremy Allison <jra at samba.org>
> ---
> source3/utils/testparm.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/source3/utils/testparm.c b/source3/utils/testparm.c
> index e1d66ce68e4..c330ad16959 100644
> --- a/source3/utils/testparm.c
> +++ b/source3/utils/testparm.c
> @@ -1,21 +1,21 @@
> -/*
> +/*
> Unix SMB/CIFS implementation.
> Test validity of smb.conf
> Copyright (C) Karl Auer 1993, 1994-1998
>
> Extensively modified by Andrew Tridgell, 1995
> Converted to popt by Jelmer Vernooij (jelmer at nl.linux.org), 2002
> -
> +
> This program is free software; you can redistribute it and/or modify
> it under the terms of the GNU General Public License as published by
> the Free Software Foundation; either version 3 of the License, or
> (at your option) any later version.
> -
> +
> This program is distributed in the hope that it will be useful,
> but WITHOUT ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> GNU General Public License for more details.
> -
> +
> You should have received a copy of the GNU General Public License
> along with this program. If not, see <http://www.gnu.org/licenses/>.
> */
> @@ -324,7 +324,7 @@ static int do_global_checks(void)
> fprintf(stderr, "ERROR: passdb backend must have a value or be "
> "left out\n\n");
> }
> -
> +
> if (lp_os_level() > 255) {
> fprintf(stderr, "WARNING: Maximum value for 'os level' is "
> "255!\n\n");
> @@ -336,7 +336,7 @@ static int do_global_checks(void)
> }
>
> return ret;
> -}
> +}
>
> /**
> * per-share logic tests
> @@ -491,7 +491,7 @@ static void do_per_share_checks(int s)
> */
> lp_set_cmdline("log level", "2");
>
> - pc = poptGetContext(NULL, argc, argv, long_options,
> + pc = poptGetContext(NULL, argc, argv, long_options,
> POPT_CONTEXT_KEEP_FIRST);
> poptSetOtherOptionHelp(pc, "[OPTION...] <config-file> [host-name] [host-ip]");
>
> @@ -504,7 +504,7 @@ static void do_per_share_checks(int s)
>
> setup_logging(poptGetArg(pc), DEBUG_STDERR);
>
> - if (poptPeekArg(pc))
> + if (poptPeekArg(pc))
> config_file = poptGetArg(pc);
>
> cname = poptGetArg(pc);
> --
> 2.11.0
>
>
> From 76b82579598dd1cca162bf4e280435e7ef772636 Mon Sep 17 00:00:00 2001
> From: Andreas Schneider <asn at samba.org>
> Date: Wed, 7 Dec 2016 17:44:25 +0100
> Subject: [PATCH 2/4] s3-testparm: Print error if the default backend is
> incorrect
>
> Signed-off-by: Andreas Schneider <asn at samba.org>
> Reviewed-by: Michael Adam <obnox at samba.org>
> ---
> source3/utils/testparm.c | 26 ++++++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
>
> diff --git a/source3/utils/testparm.c b/source3/utils/testparm.c
> index c330ad16959..e0cfaf14d9f 100644
> --- a/source3/utils/testparm.c
> +++ b/source3/utils/testparm.c
> @@ -313,6 +313,32 @@ static int do_global_checks(void)
> fprintf(stderr, "'algorithmic rid base' must be even.\n\n");
> }
>
> + if (lp_server_role() != ROLE_STANDALONE) {
> + const char *default_backends[] = {
> + "tdb", "tdb2", "ldap", "autorid", "hash"
> + };
> + const char *idmap_backend;
> + bool valid_backend = false;
> + uint32_t i;
> + bool ok;
> +
> + idmap_backend = lp_idmap_default_backend();
> +
> + for (i = 0; i < ARRAY_SIZE(default_backends); i++) {
> + ok = strequal(idmap_backend, default_backends[i]);
> + if (ok) {
> + valid_backend = true;
> + }
> + }
> +
> + if (!valid_backend) {
> + ret = 1;
> + fprintf(stderr, "ERROR: Do not use the '%s' backend "
> + "as the default idmap backend!\n\n",
> + idmap_backend);
> + }
> + }
> +
> #ifndef HAVE_DLOPEN
> if (lp_preload_modules()) {
> fprintf(stderr, "WARNING: 'preload modules = ' set while loading "
> --
> 2.11.0
>
>
> From d79621499bcd06762dbd0ed01d24ec6ec9173234 Mon Sep 17 00:00:00 2001
> From: Andreas Schneider <asn at samba.org>
> Date: Wed, 7 Dec 2016 18:19:53 +0100
> Subject: [PATCH 3/4] s3-testparm: Print an error if we have overlapping idmap
> config
>
> Except if both backends are 'ad'.
>
> Signed-off-by: Andreas Schneider <asn at samba.org>
> Reviewed-by: Michael Adam <obnox at samba.org>
> ---
> source3/utils/testparm.c | 146 +++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 146 insertions(+)
>
> diff --git a/source3/utils/testparm.c b/source3/utils/testparm.c
> index e0cfaf14d9f..3e80c39cf9d 100644
> --- a/source3/utils/testparm.c
> +++ b/source3/utils/testparm.c
> @@ -36,6 +36,8 @@
> #include "popt_common.h"
> #include "lib/param/loadparm.h"
>
> +#include <regex.h>
> +
> /*******************************************************************
> Check if a directory exists.
> ********************************************************************/
> @@ -57,6 +59,145 @@ static bool directory_exist_stat(const char *dname,SMB_STRUCT_STAT *st)
> return ret;
> }
>
> +struct idmap_config {
> + const char *domain_name;
> + const char *backend;
> + uint32_t high;
> + uint32_t low;
> +};
> +
> +struct idmap_domains {
> + struct idmap_config *c;
> + uint32_t count;
> + uint32_t size;
> +};
> +
> +static bool lp_scan_idmap_found_domain(const char *string,
> + regmatch_t matches[],
> + void *private_data)
> +{
> + bool ok = false;
> +
> + if (matches[1].rm_so == -1) {
> + fprintf(stderr, "Found match, but no name - invalid idmap config");
> + return false;
> + }
> + if (matches[1].rm_eo <= matches[1].rm_so) {
> + fprintf(stderr, "Invalid match - invalid idmap config");
> + return false;
> + }
> +
> + {
> + struct idmap_domains *d = private_data;
> + struct idmap_config *c = &d->c[d->count];
> + regoff_t len = matches[1].rm_eo - matches[1].rm_so;
> + char domname[len + 1];
> +
> + if (d->count >= d->size) {
> + return false;
> + }
> +
> + memcpy(domname, string + matches[1].rm_so, len);
> + domname[len] = '\0';
> +
> + c->domain_name = talloc_strdup_upper(d->c, domname);
> + if (c->domain_name == NULL) {
> + return false;
> + }
> + c->backend = talloc_strdup(d->c, lp_idmap_backend(domname));
> + if (c->backend == NULL) {
> + return false;
> + }
> +
> + ok = lp_idmap_range(domname, &c->low, &c->high);
> + if (!ok) {
> + fprintf(stderr,
> + "ERROR: Invalid idmap range for domain "
> + "%s!\n\n",
> + c->domain_name);
> + return false;
> + }
> +
> + d->count++;
> + }
> +
> + return false; /* Keep scanning */
> +}
> +
> +static bool do_idmap_check(void)
> +{
> + struct idmap_domains *d;
> + uint32_t i;
> + bool ok = false;
> + int rc;
> +
> + d = talloc_zero(talloc_tos(), struct idmap_domains);
> + if (d == NULL) {
> + return false;
> + }
> + d->count = 0;
> + d->size = 32;
> +
> + d->c = talloc_array(d, struct idmap_config, d->size);
> + if (d->c == NULL) {
> + goto done;
> + }
> +
> + rc = lp_wi_scan_global_parametrics("idmapconfig\\(.*\\):backend",
> + 2,
> + lp_scan_idmap_found_domain,
> + d);
> + if (rc != 0) {
> + fprintf(stderr,
> + "FATAL: wi_scan_global_parametrics failed: %d",
> + rc);
> + }
> +
> + for (i = 0; i < d->count; i++) {
> + struct idmap_config *c = &d->c[i];
> + uint32_t j;
> +
> + for (j = 0; j < d->count && j != i; j++) {
> + struct idmap_config *x = &d->c[j];
> +
> + if ((c->low >= x->low && c->low <= x->high) ||
> + (c->high >= x->low && c->high <= x->high)) {
> + /* Allow overlapping ranges for idmap_ad */
> + ok = strequal(c->backend, x->backend);
> + if (ok) {
> + ok = strequal(c->backend, "ad");
> + if (ok) {
> + fprintf(stderr,
> + "NOTE: The idmap_ad "
> + "range for the domain "
> + "%s overlaps with the "
> + "range of %s.\n\n",
> + c->domain_name,
> + x->domain_name);
> + continue;
> + }
> + }
> +
> + fprintf(stderr,
> + "ERROR: The idmap range for the domain "
> + "%s (%s) overlaps with the range of "
> + "%s (%s)!\n\n",
> + c->domain_name,
> + c->backend,
> + x->domain_name,
> + x->backend);
> + ok = false;
> + goto done;
> + }
> + }
> + }
> +
> + ok = true;
> +done:
> + TALLOC_FREE(d);
> + return ok;
> +}
> +
> /***********************************************
> Here we do a set of 'hard coded' checks for bad
> configuration settings.
> @@ -337,6 +478,11 @@ static int do_global_checks(void)
> "as the default idmap backend!\n\n",
> idmap_backend);
> }
> +
> + ok = do_idmap_check();
> + if (!ok) {
> + ret = 1;
> + }
> }
>
> #ifndef HAVE_DLOPEN
> --
> 2.11.0
>
>
> From ca490b24d56774aede66e8975216013cb09bcaa8 Mon Sep 17 00:00:00 2001
> From: Andreas Schneider <asn at samba.org>
> Date: Mon, 12 Dec 2016 10:05:39 +0100
> Subject: [PATCH 4/4] s3:winbind: Do not start with an invalid default idmap
> backend
>
> Pair-Programmed-With: Michael Adam <obnox at samba.org>
> Signed-off-by: Andreas Schneider <asn at samba.org>
> Signed-off-by: Michael Adam <obnox at samba.org>
> ---
> source3/winbindd/winbindd.c | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/source3/winbindd/winbindd.c b/source3/winbindd/winbindd.c
> index 778ee808d0c..e42f0ab4557 100644
> --- a/source3/winbindd/winbindd.c
> +++ b/source3/winbindd/winbindd.c
> @@ -1689,6 +1689,26 @@ int main(int argc, const char **argv)
> exit(1);
> }
>
> + {
> + const char *idmap_backend;
> + const char *invalid_backends[] = {
> + "ad", "rfc2307", "rid",
> + };
> +
> + idmap_backend = lp_idmap_default_backend();
> + for (i = 0; i < ARRAY_SIZE(invalid_backends); i++) {
> + bool ok;
> +
> + ok = strequal(idmap_backend, invalid_backends[i]);
> + if (ok) {
> + DBG_ERR("FATAL: Invalid idmap backend %s "
> + "configured as the default backend!\n",
> + idmap_backend);
> + exit(1);
> + }
> + }
> + }
> +
> ok = directory_create_or_exist(lp_lock_directory(), 0755);
> if (!ok) {
> DEBUG(0, ("Failed to create directory %s for lock files - %s\n",
> --
> 2.11.0
>
More information about the samba-technical
mailing list