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

Sumit Bose sbose at redhat.com
Mon Jan 20 16:51:45 UTC 2014


On Wed, Jan 15, 2014 at 03:51:05PM +0100, Lukas Slebodnik wrote:
> 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.

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

> 
> LS



More information about the sssd-devel mailing list