[SSSD] [PATCH] ldap_opts: Get rid on 389ds specific values in rfc2307bis schema

Jakub Hrozek jhrozek at redhat.com
Fri Jul 25 08:40:58 UTC 2014


On Tue, Jul 22, 2014 at 01:28:17PM +0200, Lukas Slebodnik wrote:
> On (21/07/14 11:10), Stephen Gallagher wrote:
> >On 07/20/2014 03:20 PM, Jakub Hrozek wrote:
> >> On Fri, Jul 18, 2014 at 05:32:09PM +0200, Lukas Slebodnik wrote:
> >>> On (18/07/14 16:34), Jakub Hrozek wrote:
> >>>> On Thu, Jul 17, 2014 at 04:35:31PM +0200, Lukas Slebodnik
> >>>> wrote:
> >>>>> ehlo,
> >>>>> 
> >>>>> There is problem with OpenLDAP server and dereferencing of
> >>>>> attributes that is not in the schema  of the server?
> >>>>> 
> >>>>> sh-4.2$ ldapsearch -x -LLL -h openldap.server.test -b
> >>>>> 'dc=example,dc=com' \ -E 'deref=member:uid,dummy_attr'
> >>>>> cn=ref_grp Protocol error (2) Additional information:
> >>>>> Dereference control: attribute decoding error sh-4.2$ echo
> >>>>> $? 2
> >>>>> 
> >>>>> The attribute nsUniqueID is a 389-only, non-standard
> >>>>> attribute. It is an operational attribute that is not in the
> >>>>> rfc2307bis nor inetOrgPerson nor posixAccount schema.
> >>>>> OpenLDAP supports the standard entryUUID attribute, which is
> >>>>> basically the same (uniquely identifies an entry throughout a
> >>>>> replication topology), but uses the standard UUID format
> >>>>> rather than the non-standard format used by 389.
> >>>>> 
> >>>>> 4x FIXME removed :-)
> >>>>> 
> >>>>> 
> >>>>> Any comments are welcomed.
> >>>>> 
> >>>>> LS
> >>>> 
> >>>> Thanks for the detective work on finding the root cause of the
> >>>> problem!
> >>>> 
> >>>> I wonder if we could remove the attribute completely, though.
> >>>> It appears to be completely unused now:
> >>>> 
> >>>> $ git grep -l SYSDB_UUID src/db/sysdb.h:#define SYSDB_UUID
> >>>> "uniqueID" src/providers/ad/ad_opts.h:    { "ldap_user_uuid",
> >>>> "objectGUID", SYSDB_UUID, NULL }, src/providers/ad/ad_opts.h:
> >>>> { "ldap_group_uuid", "objectGUID", SYSDB_UUID, NULL }, 
> >>>> src/providers/ad/ad_opts.h:    { "ldap_netgroup_uuid",
> >>>> "nsUniqueId", SYSDB_UUID, NULL }, src/providers/ipa/ipa_opts.h:
> >>>> { "ldap_user_uuid", "nsUniqueId", SYSDB_UUID, NULL }, 
> >>>> src/providers/ipa/ipa_opts.h:    { "ldap_group_uuid",
> >>>> "nsUniqueId", SYSDB_UUID, NULL }, src/providers/ipa/ipa_opts.h:
> >>>> { "ipa_netgroup_uuid", "ipaUniqueID", SYSDB_UUID, NULL }, 
> >>>> src/providers/ipa/ipa_opts.h:    { "ipa_host_uuid",
> >>>> "ipaUniqueID", SYSDB_UUID, NULL}, src/providers/ipa/ipa_opts.h:
> >>>> { "ipa_hostgroup_uuid", "ipaUniqueID", SYSDB_UUID, NULL}, 
> >>>> src/providers/ipa/ipa_opts.h:    { "ipa_selinux_usermap_uuid",
> >>>> "ipaUniqueID", SYSDB_UUID, NULL}, 
> >>>> src/providers/ldap/ldap_opts.h:    { "ldap_user_uuid", NULL,
> >>>> SYSDB_UUID, NULL }, src/providers/ldap/ldap_opts.h:    {
> >>>> "ldap_group_uuid", NULL, SYSDB_UUID, NULL }, 
> >>>> src/providers/ldap/ldap_opts.h:    { "ldap_user_uuid",
> >>>> "nsUniqueId", SYSDB_UUID, NULL }, 
> >>>> src/providers/ldap/ldap_opts.h:    { "ldap_group_uuid",
> >>>> "nsUniqueId", SYSDB_UUID, NULL }, 
> >>>> src/providers/ldap/ldap_opts.h:    { "ldap_user_uuid",
> >>>> "objectGUID", SYSDB_UUID, NULL }, 
> >>>> src/providers/ldap/ldap_opts.h:    { "ldap_group_uuid",
> >>>> "objectGUID", SYSDB_UUID, NULL }, 
> >>>> src/providers/ldap/ldap_opts.h:    { "ldap_netgroup_uuid",
> >>>> "nsUniqueId", SYSDB_UUID, NULL },
> >>>> 
> >>>> And according to "git log -S" it was actually never used.
> >>>> 
> >>>> So my proposal is to just remove the attribute along with
> >>>> SYSDB_UUID.
> >>> I was thinking about this change but I thought that we need to
> >>> deprecate options before removing. I don't have a problem with
> >>> removing them. and it should not be a problem in this case. I
> >>> would like to know opinion of older developers (Sumit, Stephen,
> >>> ...) Why were these options introduced? Do they help with ldap
> >>> search?
> >>> 
> >>> LS
> >> 
> >> Sure, being defensive when removing anything is a good idea. But
> >> since the option doesn't do anything and never did, I think we're
> >> fine. You're right that removing an option requires two acks,
> >> though. So far you have mine :-)
> >
> >I'll ack it as well. I suspect that this was planned for a future use
> >that we never ended up with. I don't remember any reason for it.
> >
> I prepared separate patches for different *_uuid options.
> I thought it would be simple change. There is small complication with
> IPA_UNIQUE_ID. I am not sure why it was used for storing HBAC rules.
> (0005-Remove-macro-IPA_UNIQUE_ID.patch). HBAC works with 5th patch.
> I hope I tested all possible ways. If we agree with this version it can be
> squashed to 4th patch (0004-LDAP-Remove-unused-option-ipa_hostgroup_uuid)

I can't see a reason either. The only benefit of storing the uniqueID,
as far as I can see, is to be able to match objects with their server
side counterparts. I also tested some HBAC combinations, even offline
auth with cache using the old RDN, but with patched code -- everything
worked fine, because the code doesn't care about either the DN format or
the presence of UUID. We don't need to upgrade sysdb either.

I wonder if, with some very old IPA versions, the rules could have no
name? The following code in hbac_attrs_to_rule() seems to suggest it:

274     ret = sysdb_attrs_get_el(hbac_ctx->rules[idx],
275                              IPA_CN, &el);
276     if (ret != EOK || el->num_values == 0) {
277         DEBUG(SSSDBG_CONF_SETTINGS, "rule has no name, assuming '(none)'.\n");
278         new_rule->name = talloc_strdup(new_rule, "(none)");
279     } else {
280         new_rule->name = talloc_strndup(new_rule,
281                                         (const char*) el->values[0].data,
282                                         el->values[0].length);
283     }
284                                                          

Your patches make 'cn' mandatory because it's used as an RDN value,
so additional change to patch 5/7 should also change the code above to
require CN.  BTW I also tested that IPA wouldn't let you define two rules
with the same CN..

So I think we should push these patches to master, if we can't justify
the presence of some code, it should be removed..but at the same time,
sssd-1-11 should only contain minimal and safe changes at this point.

Can you also prepare patches #1, #2 so that they apply on sssd-1-11 and
send a version of #3 that only removes the LDAP part?



More information about the sssd-devel mailing list