[SSSD] [PATCHES] MAN: Describe change in idmapping with ldap provider

Lukas Slebodnik lslebodn at redhat.com
Wed Jan 15 14:51:05 UTC 2014


On (09/01/14 18:58), Sumit Bose wrote:
>On Thu, Jan 09, 2014 at 06:01:21PM +0100, Lukas Slebodnik wrote:
>> On (09/01/14 14:10), Sumit Bose wrote:
>> >On Tue, Dec 17, 2013 at 05:18:38PM +0100, Lukas Slebodnik wrote:
>> >> ehlo,
>> >> 
>> >> attached patches address ticket #2172
>> >
>> >I think you meant #2175?
>> >
>> I meant #2172.
>> 
>> >> 
>> >> LS
>> >
>> >> From e9bc3fa6bd52afc5108e79182f32bb26d2843609 Mon Sep 17 00:00:00 2001
>> >> From: Lukas Slebodnik <lslebodn at redhat.com>
>> >> Date: Fri, 13 Dec 2013 15:33:23 +0100
>> >> Subject: [PATCH 1/3] sdap_idamp: Fall back to another method if sid is wrong
>> >> 
>> >> sss_idmap_domain_has_algorithmic_mapping can return also
>> >> IDMAP_SID_UNKNOWN, but it does not mean that idmaping is
>> >> unavailable. We should fall back to another method of detection
>> >> (sss_idmap_domain_by_name_has_algorithmic_mapping)
>> >> and do not return false immediately.
>> >> 
>> >> Resolves:
>> >> https://fedorahosted.org/sssd/ticket/2175
>>                                        ^^^^^
>>                                 I changed ticket number.
>> >> ---
>> >>  src/providers/ldap/sdap_idmap.c | 10 ++++++++--
>> >>  1 file changed, 8 insertions(+), 2 deletions(-)
>> >> 
>> >> diff --git a/src/providers/ldap/sdap_idmap.c b/src/providers/ldap/sdap_idmap.c
>> >> index 871b8e0bf2357b1dde3f179bb6dba3d5e7537413..c2c98ae3cc089c5431de072a07403c2b5c3eb273 100644
>> >> --- a/src/providers/ldap/sdap_idmap.c
>> >> +++ b/src/providers/ldap/sdap_idmap.c
>> >> @@ -522,9 +522,15 @@ bool sdap_idmap_domain_has_algorithmic_mapping(struct sdap_idmap_ctx *ctx,
>> >>  
>> >>      err = sss_idmap_domain_has_algorithmic_mapping(ctx->map, dom_sid,
>> >>                                                     &has_algorithmic_mapping);
>> >> -    if (err == IDMAP_SUCCESS) {
>> >> +    switch (err){
>> >> +    case IDMAP_SUCCESS:
>> >>          return has_algorithmic_mapping;
>> >> -    } else if (err != IDMAP_SID_UNKNOWN && err != IDMAP_NO_DOMAIN) {
>> >> +    case IDMAP_SID_INVALID: /* FALLTHROUGH */
>> >> +    case IDMAP_SID_UNKNOWN: /* FALLTHROUGH */
>> >> +    case IDMAP_NO_DOMAIN:   /* FALLTHROUGH */
>> >> +        /* continue with idmap_domain_by_name */
>> >
>> >I agree with the change but in the commit message you say
>> >'IDMAP_SID_UNKNOWN' but you add 'IDMAP_SID_INVALID' here. So the commit
>> >messages should be changed.
>> >
>> Fixed
>> 
>> >> +        break;
>> >> +    default:
>> >>          return false;
>> >>      }
>> >>  
>> >> -- 
>> >> 1.8.4.2
>> >> 
>> >
>> >> From 3c0de1fd3274f70c7ab27d95a55c72b703808c80 Mon Sep 17 00:00:00 2001
>> >> From: Lukas Slebodnik <lslebodn at redhat.com>
>> >> Date: Fri, 13 Dec 2013 18:20:08 +0100
>> >> Subject: [PATCH 2/3] LDAP: Don't fail if subdomain cannot be found by sid
>> >> 
>> >> Domain needn't contain sid if id_provider is ldap.
>> >> With enabled id mapping, user couldn't be stored, because domain
>> >> couldn't be found by sid.
>> >> 
>> >> Resolves:
>> >> https://fedorahosted.org/sssd/ticket/2175
>>                                        ^^^^^
>>                                 I changed ticket number.
>> >
>> >ACK
>> >
>> >> From a374f755b4c1d2fe5860285b973a915b3bacdd23 Mon Sep 17 00:00:00 2001
>> >> From: Lukas Slebodnik <lslebodn at redhat.com>
>> >> Date: Tue, 17 Dec 2013 17:03:50 +0100
>> >> Subject: [PATCH 3/3] MAN: Describe change in id mapping with ldap provider
>> >> 
>> >> ID mapping could be configured with id_provider ldap only with
>> >> enabling option ldap_id_mapping. This behaviour was changed
>> >> in the commit d3e1d88ce7de3216a862b
>> >> "LDAP: Require ID numbers when ID mapping is off"
>> >> 
>> >> ldap_idmap_default_domain_sid need to be also configured since that change.
>> >> 
>> >> Resolves:
>> >> https://fedorahosted.org/sssd/ticket/2172
>> >> ---
>> >>  src/man/sssd-ldap.5.xml | 4 ++++
>> >>  1 file changed, 4 insertions(+)
>> >> 
>> >> diff --git a/src/man/sssd-ldap.5.xml b/src/man/sssd-ldap.5.xml
>> >> index 72586fb1d97037ebd43a737c2a0394bcd652bac9..7a2ab60e067999847795983e1d551cbf62faad9a 100644
>> >> --- a/src/man/sssd-ldap.5.xml
>> >> +++ b/src/man/sssd-ldap.5.xml
>> >> @@ -1428,6 +1428,10 @@
>> >>                              ActiveDirectory objectSID mapping.
>> >>                          </para>
>> >>                          <para>
>> >> +                            The option ldap_idmap_default_domain_sid need to be
>> >> +                            configured for id mapping with ldap provider.
>> >> +                        </para>
>> >> +                        <para>
>> >>                              Default: false
>> >>                          </para>
>> >>                      </listitem>
>> >> -- 
>> >> 1.8.4.2
>> >> 
>> >
>> >I do not understand the reason for this. First I think the ticket number
>> >is wrong. Second ldap_idmap_default_domain_sid is only used to assign a
>> Sorry for mixing ticket numbers.
>> 
>> >slice to the given domain. Why can the slice not be calculated
>> >algorithmically?
>> >
>> Problem occurs with id_provider ldap (Active Directory)
>> Behaviour was changed (regression) in the commit d3e1d88ce7de3216a862b
>>     (in the file src/providers/ldap/ldap_id.c)
>> id_mapping cannot be detected in ldap provider if ldap_idmap_default_domain_sid
>> is not configured *and thus* wrong LDAP filter was used.
>> 
>> >What is needed when using ldap_id_mapping=True with the plain LDAP
>> >provider is ldap_user_primary_group. The reason is that
>> >ldap_id_mapping=True assumes that the POSIX ID of the primary group must
>> >be calculated from a SID as well. To get this SID the numerical value
>> >from the attribute given by ldap_user_primary_group is used as the RID
>> >of the primary group. Then the domain SID from the user SID is
>> >prepended and the POSIX GID is calculated from the resulting SID. By
>> >default ldap_user_primary_group is not set for the LDAP provider and
>> >hence lookup fail.
>> >
>> I confused you with wrong ticket number. Does my solution make sense with
>> ticket #2172? or should I change behaviour in the file
>> src/providers/ldap/ldap_id.c (introduced in the commit d3e1d88ce7de3216a862b)
>> 
>
>Thank you for the clarifications, now all makes sense. If you want
>algorithmic mapping a domain SID is needed and since the plain LDAP
>provider does not know how to read them it has to given by the
>configuration file. Using ldap_idmap_default_domain_sid will give us the
>domain SID with the side-effect of always using slice 0. If the plain
>LDAP provider was used before with this configuration if might have used
>a different slice. The slice number is stored in the cache, but if the
>cache is removed the new allocated slice will be 0 and UIDs and GIDs
>change.
>
>I think it would be better to introduce a new config option to cover
>this case and check this case explicitly in sdap_idmap_init(), i.e. if
>idmapping is requested and neither ldap_idmap_default_domain_sid or the
>new option is available it would be a config error.
>
I don't think we need a new option; we have many options and it is a
regression.

I decided to solve it in another way. Updated paches are attached.

LS
-------------- next part --------------
>From c77347555fb1c8b5142512ca38f881ee9bb05d10 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lslebodn at redhat.com>
Date: Fri, 13 Dec 2013 18:20:08 +0100
Subject: [PATCH 1/2] LDAP: Don't fail if subdomain cannot be found by sid

Domain needn't contain sid if id_provider is ldap.
With enabled id mapping, user couldn't be stored, because domain
couldn't be found by sid.

Resolves:
https://fedorahosted.org/sssd/ticket/2172
---
 src/providers/ldap/sdap_async_users.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/providers/ldap/sdap_async_users.c b/src/providers/ldap/sdap_async_users.c
index ebbc171e49d09d117a3a86f572c69df015e2ab6e..56d5b21467827464063b487b15910e847575ed2b 100644
--- a/src/providers/ldap/sdap_async_users.c
+++ b/src/providers/ldap/sdap_async_users.c
@@ -123,6 +123,7 @@ int sdap_save_user(TALLOC_CTX *memctx,
     bool use_id_mapping;
     char *sid_str;
     char *dom_sid_str = NULL;
+    struct sss_domain_info *subdomain;
 
     DEBUG(SSSDBG_TRACE_FUNC, ("Save user\n"));
 
@@ -161,11 +162,12 @@ int sdap_save_user(TALLOC_CTX *memctx,
     /* If this object has a SID available, we will determine the correct
      * domain by its SID. */
     if (sid_str != NULL) {
-        dom = find_subdomain_by_sid(get_domains_head(dom), sid_str);
-        if (dom == NULL) {
-            DEBUG(SSSDBG_OP_FAILURE, ("SID %s does not belong to any known "
+        subdomain = find_subdomain_by_sid(get_domains_head(dom), sid_str);
+        if (subdomain) {
+            dom = subdomain;
+        } else {
+            DEBUG(SSSDBG_TRACE_FUNC, ("SID %s does not belong to any known "
                                       "domain\n", sid_str));
-            return ERR_DOMAIN_NOT_FOUND;
         }
     }
 
-- 
1.8.4.2

-------------- next part --------------
>From f80bfdc371c3349ee96c87fc291e060fcbcf4f5a Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lslebodn at redhat.com>
Date: Wed, 15 Jan 2014 14:55:10 +0100
Subject: [PATCH 2/2] LDAP: update id mapping detection for ldap provider

For id_provider ldap, it is only necessary to enable option ldap_id_mapping.
It is an regression introduced in the commit d3e1d88ce7de3216a862b

Resolves:
https://fedorahosted.org/sssd/ticket/2172
---
 src/providers/ldap/sdap_idmap.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/providers/ldap/sdap_idmap.c b/src/providers/ldap/sdap_idmap.c
index 871b8e0bf2357b1dde3f179bb6dba3d5e7537413..eed0b02b8e3096d4ac817f8f26f4904b8f4578c7 100644
--- a/src/providers/ldap/sdap_idmap.c
+++ b/src/providers/ldap/sdap_idmap.c
@@ -520,6 +520,11 @@ bool sdap_idmap_domain_has_algorithmic_mapping(struct sdap_idmap_ctx *ctx,
     int ret;
     TALLOC_CTX *tmp_ctx = NULL;
 
+    if (dp_opt_get_bool(ctx->id_ctx->opts->basic, SDAP_ID_MAPPING)
+        && 0 == strcmp("ldap", ctx->id_ctx->be->bet_info[BET_ID].mod_name)) {
+        return true;
+    }
+
     err = sss_idmap_domain_has_algorithmic_mapping(ctx->map, dom_sid,
                                                    &has_algorithmic_mapping);
     if (err == IDMAP_SUCCESS) {
-- 
1.8.4.2



More information about the sssd-devel mailing list