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 <lsleb...@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 <lsleb...@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 <lsleb...@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 _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
