Deprecated but still supported "idmap backend" actually is broken

Dmitry Butskoy buc at odusz.so-cdu.ru
Wed Oct 10 17:00:35 GMT 2007


simo wrote:
> On Wed, 2007-10-10 at 18:37 +0400, Dmitry Butskoy wrote:
>   
>> The "idmap backend" parameter is now deprecated, but it seems to be 
>> supported for a while.
>>
>> Actually, for 3.0.26a, it is broken.
>>
>>
>> Consider nsswitch/idmap.c:idmap_init() :
>>
>> If "idmap domains" config is not used, then "dom_list = 
>> idmap_default_domain", but the last is just "default domain" string. As 
>> a result, when I specify "idmap backend = rid:FOO=1000-2000" (and leave 
>> "idmap domains" empty), the correspond domain name appears as "default 
>> domain", not "FOO" ... Then "getent passwd <uidnumber>" does not work etc...
>>     
>
>
>   
[snip]
> I don't see any problem in the code, if you see it please be more
> specific (point at specific lines in the code that you think are wrong,
> or post logs that show evidence please).
>   

Well,

See nsswitch/idmap.c:idmap_init() :

> NTSTATUS idmap_init(void)
> {
>     NTSTATUS ret;
>     static NTSTATUS idmap_init_status = NT_STATUS_UNSUCCESSFUL;
>     struct idmap_domain *dom;
>     char *compat_backend = NULL;
>     char *compat_params = NULL;
>     const char **dom_list = NULL;
>     char *alloc_backend = NULL;
>     BOOL default_already_defined = False;
>     BOOL pri_dom_is_in_list = False;
>     int compat = 0;
>     int i;
>
>     ret = idmap_init_cache();
>     if (!NT_STATUS_IS_OK(ret))
>         return ret;
>
>     if (NT_STATUS_IS_OK(idmap_init_status))
>         return NT_STATUS_OK;
>
>     static_init_idmap;
>
>     dom_list = lp_idmap_domains();
it should be NULL, as I don't use "idmap domains" in smb.conf
>
>     if ( lp_idmap_backend() ) {
yes...
>                const char **compat_list = lp_idmap_backend();
>         char *p = NULL;
>         const char *q = NULL;
>
>         if (dom_list) {
>             DEBUG(0, ("WARNING: idmap backend and idmap domains "
>                   "are mutually excusive!\n"));
>             DEBUGADD(0,("idmap backend option will be IGNORED!\n"));
>         } else {
>             compat = 1;
>
>             compat_backend = talloc_strdup(idmap_ctx, *compat_list);
>             if (compat_backend == NULL) {
>                 ret = NT_STATUS_NO_MEMORY;
>                 goto done;
>             }
>
>             /* strip any leading idmap_ prefix of */
>             if (strncmp(*compat_list, "idmap_", 6) == 0 ) {
>                 q = *compat_list += 6;
>                 DEBUG(0, ("WARNING: idmap backend uses obsolete"
>                       " and deprecated 'idmap_' prefix.\n"
>                       "Please replace 'idmap_%s' by '%s' in"
>                       " %s\n", q, q, dyn_CONFIGFILE));
>                 compat_backend = talloc_strdup(idmap_ctx, q);
>             } else {
>                 compat_backend = talloc_strdup(idmap_ctx,
>                                 *compat_list);
>             }
>
>             /* separate the backend and module arguements */
>             if ((p = strchr(compat_backend, ':')) != NULL) {
>                 *p = '\0';
>                 compat_params = p + 1;
>             }
>         }
>     } else if ( !dom_list ) {
>         /* Back compatible: without idmap domains and explicit
>            idmap backend.  Taking default idmap backend: tdb */
>
>         compat = 1;
>         compat_backend = talloc_strdup( idmap_ctx, "tdb");
>         compat_params = compat_backend;
>     }
>
>     if ( ! dom_list) {
Yes, dom_list is still NULL ...
>         dom_list = idmap_default_domain;
...and now dom_list == { "default domain", NULL };
(see the "idmap_default_domain" definition at the beginning of the file)
>     }
>
>     /***************************
>      * initialize idmap domains
>      */
>     DEBUG(1, ("Initializing idmap domains\n"));
>
>     for (i = 0; dom_list[i]; i++) {
>                const char *parm_backend;
>         char *config_option;
>
>         /* ignore BUILTIN and local MACHINE domains */
>         if (strequal(dom_list[i], "BUILTIN")
>              || strequal(dom_list[i], get_global_sam_name()))
>         {
>             DEBUG(0,("idmap_init: Ignoring invalid domain %s\n",
>                  dom_list[i]));
>             continue;
>         }
>
>         if (strequal(dom_list[i], lp_workgroup())) {
>             pri_dom_is_in_list = True;
>         }
>         /* init domain */
>
>         dom = TALLOC_ZERO_P(idmap_ctx, struct idmap_domain);
>         IDMAP_CHECK_ALLOC(dom);
>
>         dom->name = talloc_strdup(dom, dom_list[i]);
Oops! dom->name is "defailt domain" now, but should be "FOO"  :(
>         IDMAP_CHECK_ALLOC(dom->name);
>
>         config_option = talloc_asprintf(dom, "idmap config %s",
>                         dom->name);
>         IDMAP_CHECK_ALLOC(config_option);
>
>         /* default or specific ? */
>
>         dom->default_domain = lp_parm_bool(-1, config_option,
>                            "default", False);
>
>         if (dom->default_domain ||
>             strequal(dom_list[i], idmap_default_domain[0])) {
>
>             /* make sure this is set even when we match
>              * idmap_default_domain[0] */
>             dom->default_domain = True;
>
>             if (default_already_defined) {
>                 DEBUG(1, ("ERROR: Multiple domains defined as"
>                       " default!\n"));
>                 ret = NT_STATUS_INVALID_PARAMETER;
>                 goto done;
>             }
>
>             default_already_defined = True;
>
>         }
>
>         dom->readonly = lp_parm_bool(-1, config_option,
>                          "readonly", False);
>
>         /* find associated backend (default: tdb) */
>         if (compat) {
>             parm_backend = talloc_strdup(idmap_ctx, compat_backend);
>         } else {
>             char *backend = lp_parm_const_string(-1, config_option,
>                                  "backend", "tdb");
>             parm_backend = talloc_strdup(idmap_ctx,    backend);
>         }
>         IDMAP_CHECK_ALLOC(parm_backend);
>
>         /* get the backend methods for this domain */
>         dom->methods = get_methods(backends, parm_backend);
>
>         if ( ! dom->methods) {
>             ret = smb_probe_module("idmap", parm_backend);
>             if (NT_STATUS_IS_OK(ret)) {
>                 dom->methods = get_methods(backends,
>                                parm_backend);
>             }
>         }
>         if ( ! dom->methods) {
>             DEBUG(0, ("ERROR: Could not get methods for "
>                   "backend %s\n", parm_backend));
>             ret = NT_STATUS_UNSUCCESSFUL;
>             goto done;
>         }
>
>         /* check the set_mapping function exists otherwise mark the
>          * module as readonly */
>         if ( ! dom->methods->set_mapping) {
>             DEBUG(5, ("Forcing to readonly, as this module can't"
>                   " store arbitrary mappings.\n"));
>             dom->readonly = True;
>         }
>
>         /* now that we have methods,
>          * set the destructor for this domain */
>         talloc_set_destructor(dom, close_domain_destructor);
>
>         if (compat_params) {
>             dom->params = talloc_strdup(dom, compat_params);
>             IDMAP_CHECK_ALLOC(dom->params);
>         } else {
>             dom->params = NULL;
>         }
>
>         /* Finally instance a backend copy for this domain */
>         ret = dom->methods->init(dom);
>         if ( ! NT_STATUS_IS_OK(ret)) {
>             DEBUG(0, ("ERROR: Initialization failed for backend "
>                   "%s (domain %s), deferred!\n",
>                   parm_backend, dom->name));
>         }
>         idmap_domains = talloc_realloc(idmap_ctx, idmap_domains,
>                         struct idmap_domain *, i+1);
>         if ( ! idmap_domains) {
>             DEBUG(0, ("Out of memory!\n"));
>             ret = NT_STATUS_NO_MEMORY;
>             goto done;
>         }
>         idmap_domains[i] = dom;
>
>         /* save default domain position for future uses */
>         if (dom->default_domain) {
>             def_dom_num = i;
>         }
>
>         DEBUG(10, ("Domain %s - Backend %s - %sdefault - %sreadonly\n",
>                 dom->name, parm_backend,
>                 dom->default_domain?"":"not ",
>                 dom->readonly?"":"not "));
Here we can see that domain name is not "FOO" , but "default domaiin" ...
BTW, you can see this in the log I posted before (with log level 10 -- 
search for "nsswitch/idmap.c:idmap_init(491)" ...
>
>         talloc_free(config_option);
>     }
>
[snip]


>> P.S. Can the new complicated "idmap config DOMAIN ...." be edited under 
>> SWAT?
>>     
>
> I am not sure it does, perhaps not.
>   

After the specifying "idmap domains = FOO" and commit changes, we do not 
see any "idmap config FOO" appeared. And it seems we cannot see such at 
all, by design...

Anyway, a lot of (production) users use SWAT to configure Samba, and it 
is really bad idea to cause them to edit smb.conf manually just for some 
specific parameters. At least, save old "idmap backend" until "idmap 
config" will be supported properly by SWAT.


~buc



More information about the samba-technical mailing list