On (25/07/14 10:40), Jakub Hrozek wrote:
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?
Attached patches remove only options ldap_*_uuid. They can be applied on master or sssd-1-11.
Now, you should decide which patch set you want to push to which branch :-)
LS