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

Lukas Slebodnik lslebodn at redhat.com
Tue Jan 21 11:05:19 UTC 2014


On (20/01/14 18:33), Lukas Slebodnik wrote:
>On (20/01/14 17:51), Sumit Bose wrote:
>>On Wed, Jan 15, 2014 at 03:51:05PM +0100, Lukas Slebodnik wrote:
>>> On (09/01/14 18:58), Sumit Bose wrote:
>>> >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.
>>
>>ok, works for me. I also tested with IPA and AD provider and didn't see
>>an issue.
>>
>>ACK.
>>
>>You have not resend you original first patch. I think the change is
>>still valid, although with you new approach it is not necessary to fix
>>the given issue. Do you think it should be committed as well, or do you
>>have concerns?
>>
>>bye,
>>Sumit
>>
>
>I didn't send patch "Fall back to another method if sid is wrong"
>because patch "LDAP: update id mapping detection for ldap provider"
>solved this problem.
>
>sss_idmap_domain_has_algorithmic_mapping can return error code
>IDMAP_SID_INVALID only if dom_sid is NULL and I am not sure whether if it can
>happen with AD provider.
>
>If you think it is a good idea I can resend all 3 patches.
>
>LS

I am sending all three patches together after IRC discussion with
Sumit.

LS
-------------- next part --------------
>From 551e17167698505ad00a00309e6b29fe553bb1ab 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/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/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 84cbe7ef99af5628bf217016b6a373306fa4af49 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/3] 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

-------------- next part --------------
>From 159b151096397b3a2f7e4e39365ec7ec10761eae 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 3/3] sdap_idamp: Fall back to another method if sid is wrong

sss_idmap_domain_has_algorithmic_mapping can return also
IDMAP_SID_INVALID, 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/2172
---
 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 eed0b02b8e3096d4ac817f8f26f4904b8f4578c7..aad8e34aa99fa0a0d4b28fb968f02957908a4c76 100644
--- a/src/providers/ldap/sdap_idmap.c
+++ b/src/providers/ldap/sdap_idmap.c
@@ -527,9 +527,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 */
+        break;
+    default:
         return false;
     }
 
-- 
1.8.4.2



More information about the sssd-devel mailing list