Bug 1379475

Summary: Convert Binary LDAP Attributes to Base64
Product: Red Hat OpenStack Reporter: Sean Lee <slee>
Component: openstack-keystoneAssignee: John Dennis <jdennis>
Status: CLOSED WONTFIX QA Contact: nlevinki <nlevinki>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: 8.0 (Liberty)CC: byount, jdennis, nkinder, panbalag, srevivo
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2017-01-13 16:38:02 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:

Description Sean Lee 2016-09-26 22:11:22 UTC
Keystone does not currently properly handle binary attributes from ldap/AD. Instead, it simply ignores them and throws "Unable to decode value for attribute ..." errors in the log.

If possible, I would like to propose that we convert binary attributes to base64 like ldapsearch does. 

--- /usr/lib/python2.7/site-packages/keystone/common/ldap/core.py.orig
+++ /usr/lib/python2.7/site-packages/keystone/common/ldap/core.py.new
@@ -19,6 +19,7 @@
 import re
 import sys
 import weakref
+import base64
 
 import ldap.filter
 import ldappool
@@ -166,7 +167,8 @@
                 val2py = enabled2py if kind == 'enabled' else ldap2py
                 ldap_attrs[kind] = [val2py(x) for x in values]
             except UnicodeDecodeError:
-                LOG.debug('Unable to decode value for attribute %s', kind)
+                # convert binary attributes to base64 like ldapsearch does
+                ldap_attrs[kind] = [base64.b64encode(x) for x in values]
 
         py_result.append((utf8_decode(dn), ldap_attrs))
     if at_least_one_referral:

This patch is related to BZ#1378254. After setting "disable_domain_id_mapping=true" in keystone.conf, this patch allows us to use objectGUID/objectSid as unique ID for user_id_attribute/group_id_attribute.

This proposed patch will help the following BZs as well:

BZ 1128926 - keystone is unable to handle 'objectGUID' and 'objectSID' active directory attributes. 
BZ 1138684 - Keystone LDAP identity driver crashes on binary attributes

Comment 1 John Dennis 2016-09-27 19:15:45 UTC
-1

I'm not a big fan of heuristics like this, there is no definitive way to determine the data type given an arbitrary blob of data. Just because something can or cannot be decoded from UTF-8 is *not* a reliable indicator of whether the data type is text or not. It's entirely possible to construct binary data that will decode without errors into garbage text.

Every LDAP attribute has a well known type that is unambiguous defined the schema. Being able to know the type of an LDAP attribute is one of the primary reasons LDAP schema exists. The right way to do this is to load the schema from the LDAP serve and then convert to a Python object based the the attributes schema type. A compromise solution is to have a hard coded table of attributes and their types. Essentially this boils down to an exception table, if the attribute is not in the table it's assumed to be text, otherwise the table identifies the Python constructor to use to convert the value into a Python type. Hard coded table are not ideal because they have to be maintained, but thats still better than randomly guessing.

I could not recommend introducing a change like this in a downstream only implementation. First I would want to see buy-in from the upstream developers on this topic.

The existing implementation of convert_ldap_result() is poor and also depends on hueristics, but let's not make it worse than it already is. I'd probably be willing to support a patch that utilized a hardcoded lookup table approach in lieu of schema loading, but lets not introduce more guessing because that only opens us up to more problems.