Keystone doesn't handle user_attribute_id mapping

[Haneef Ali]

By default keystone gets the id from first field of DN. It doesn't use user_id_attibute mapping from keystone.conf

In the following code, "id" attribute is always 1 element in DN
---Relevent code---

https://github.com/openstack/keystone/blob/de2c6e15b9f45969c307ac6d1f634d933537aeaa/keystone/common/ldap/core.py#L1277-L1304

  @staticmethod
    def _dn_to_id(dn):
        return utf8_decode(ldap.dn.str2dn(utf8_encode(dn))[0][0][1])

def _ldap_res_to_model(self, res):
        obj = self.model(id=self._dn_to_id(res[0]))
        # LDAP attribute names may be returned in a different case than
        # they are defined in the mapping, so we need to check for keys
        # in a case-insensitive way. We use the case specified in the
        # mapping for the model to ensure we have a predictable way of
        # retrieving values later.
        lower_res = dict((k.lower(), v) for k, v in six.iteritems(res[1]))
        for k in obj.known_keys:
            if k in self.attribute_ignore:
                continue

            try:
                v = lower_res[self.attribute_mapping.get(k, k).lower()]
            except KeyError:
                pass
            else:
                try:
                    obj[k] = v[0]
                except IndexError:
                    obj[k] = None

        return obj

[Haneef Ali (haneef)]

Adding query_mode=sub doesn't help. There is a possibility that mapped attribute can't be in DN too

[Haneef Ali (haneef)]

As a not group_id too have the same problem since it shares the same code

[Guang Yee (guang-yee)]

Confirmed. I have OpenLDAP setup in my local dev vm with the following entry

ldapsearch -x -b ou=users,dc=nodomain -s sub '(cn=Guang*)'
# extended LDIF
#
# LDAPv3
# base <ou=users,dc=nodomain> with scope subtree
# filter: (cn=Guang*)
# requesting: ALL
#

# Guang Yee, users, nodomain
dn: cn=Guang Yee,ou=users,dc=nodomain
objectClass: account
objectClass: posixAccount
cn: Guang Yee
uid: gyee
uidNumber: 10000
gidNumber: 10000
homeDirectory: /home/gyee
loginShell: /bin/bash
gecos: gyee
description: User account

# search result
search: 2
result: 0 Success

# numResponses: 2
# numEntries: 1

In the [ldap] section of keystone.conf I have

user_id_attribute=uid
user_name_attribute=uid

I am expecting both 'id' and 'name' in user ref have the same value as the uid attribute. But here's what I got

curl -s -H 'X-Auth-Token: ADMIN' http://localhost:35357/v3/users | python -mjson.tool
{
    "links": {
        "next": null,
        "previous": null,
        "self": "http://localhost:35357/v3/users"
    },
    "users": [
        {
            "domain_id": "default",
            "id": "Guang Yee",
            "links": {
                "self": "http://localhost:35357/v3/users/Guang Yee"
            },
            "name": "gyee"
        }
    ]
}

[Guang Yee (guang-yee)]

Code in question is here

https://github.com/openstack/keystone/blob/master/keystone/common/ldap/core.py#L1275

Seem like we are always taking the leftmost RDN as the object 'id', regardless of the ID map in keystone.conf. Interesting, we are using the user_id_attribute map to construct the search filter. This may lead to security problems if the leftmost RND is NOT globally unique. Obviously, we'll have id conflicts.

We should alway honor the id attribute maps when constructing the ref.

[Brant Knudson (blk-u)]

I agree this looks like it's not working correctly. Also, looking at the code I don't think it will be worse (performance-wise) to use user_id_attr to get the user ID. This is because the code was always fetching the user anyways.

It also doesn't look like a trivial change, mostly just because the code makes a lot of assumptions.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/117658

[Lance Bragstad (lbragstad)]

Setting this as keystone juno RC1 since it could break existing deployments.

[Adam Young (ayoung)]

What is the actual problem here? What is this breaking?

[Guang Yee (guang-yee)]

The patch I proposed should not break anything. I tested it against live OpenLDAP. Jenkins seem happy as well. If there's a test scenario I need to cover, please let me know.

[Varun Mittal (viral.mutant)]

Is this bug the same as
https://bugs.launchpad.net/keystone/+bug/1231488

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/117658
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=47e09d5dda70b2da87ec5a0fc4608373418870f9
Submitter: Jenkins
Branch: master

commit 47e09d5dda70b2da87ec5a0fc4608373418870f9
Author: Guang Yee <email address hidden>
Date: Thu Aug 28 17:17:25 2014 -0700

    Use id attribute map for read-only LDAP

    For read-only LDAP, we have no control over the DIT. We can only use the
    attribute maps. Therefore, we need to honor the attribute map for all
    attributes.

    This patch also added the missing ID attribute when creating an object in
    LDAP. For lookup, we also need the ID attribute to be returned.

    Change-Id: I57c099eefdca7e756d294c35cf5b70d119c5c955
    closes-bug: #1361306
    closes-bug: #1340041

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/icehouse
Review: https://review.openstack.org/150134

[OpenStack Infra (hudson-openstack)]

Change abandoned by Dolph Mathews (<email address hidden>) on branch: stable/icehouse
Review: https://review.openstack.org/150134
Reason: restore if you'd like to push for this. considering it's nowhere near a clean backport, there's extra risk as well.