LDAP: changing user_id_attribute bricks group mapping

Bug #1782922 reported by Jakob Englisch on 2018-07-21
28
This bug affects 3 people
Affects Status Importance Assigned to Milestone
OpenStack Identity (keystone)
Medium
Guang Yee
Ubuntu Cloud Archive
Status tracked in Train
Queens
Medium
Unassigned
Rocky
Medium
Unassigned
Stein
Medium
Unassigned
Train
Medium
Unassigned
keystone (Ubuntu)
Status tracked in Eoan
Bionic
Medium
Unassigned
Cosmic
Medium
Unassigned
Disco
Medium
Unassigned
Eoan
Medium
Unassigned

Bug Description

[Impact]
When using the keystone LDAP backend, changing user_id_attribute breaks group mapping. This is because the _dn_to_id() method only calculated the uid to be the first RDN of the DN. _dn_to_id() is updated in the fix to also deal with the case where the uid is set to a different attribute.

[Test Case]
See details in comment #5: https://bugs.launchpad.net/keystone/+bug/1782922/comments/5

[Regression Potential]
The patch takes a minimal approach to the fix and includes unit tests to help ensure the patched code doesn't regress. The patches have landed in all upstream releases back to stable/queens which helps get even more exposure with upstream reviews, gate testing and real deployments.

[Original Description]

Env Details:
Openstack version: Queens (17.0.5)
OS: CentOS 7.5
LDAP: Active Directory, Windows Server 2012R2

We changed the user_id_attribute to sAMAccountName when configuring keystone. [ user_id_attribute = "sAMAccountName" ; group_members_are_ids = False ]. Unfortunately this bricks the group mapping logic in keystone.

The relevant code in keystone:
`list_users_in_group` [1] -> gets all groups from the LDAP server, and then calls `_transform_group_member_ids`. `_transform_group_member_ids` tries to match the user ids (for posixGroups e.g.) or the DN. However DN matching does not match the full DN. It rather takes the first RDN of the DN and computes the keystone user id [2]. The first RDN in Active Directory is the "CN". While the user-create part honors the user_id_attribute and takes "sAMAccountName" in our configuration. The generated user-ids in keystone now do not match anymore and hence group mapping is broken.

A fix could be looking up the user by the DN received from the 'member' attribute of a given group and compare the configured 'user_id_attribute' of the received ldap user id and the in keystone stored user id. A quick fix could also be to mention that behavior in the documentation.

/e: related https://bugs.launchpad.net/keystone/+bug/1231488/comments/19

[1] https://github.com/openstack/keystone/blob/master/keystone/identity/backends/ldap/common.py#L1285

[2] https://github.com/openstack/keystone/blob/master/keystone/identity/backends/ldap/core.py#L126

[3] https://github.com/openstack/keystone/blob/master/keystone/identity/backends/ldap/common.py#L1296

description: updated
Lance Bragstad (lbragstad) wrote :

Hi Jakob,

Do you notice a difference in behavior is you perform a ``keystone-manage mapping_purge``? This should clear out the mapping IDs keystone has locally and regenerate them. Does that have any impact on the issue you're seeing?

Jakob Englisch (jakob-englisch) wrote :

Nope. Did that multiple times while debugging this issue. As described, the group mapping expectes that the first RDN is the same as the user_id_attribute, which should either be stated in the documentation or fixed by changing the group mapping code.

Changed in keystone:
status: New → Triaged
importance: Undecided → Medium
Rafal Ramocki (rafal-ramocki) wrote :

Hi,

I have same issue and I've spent some time debugging it. I'm using OpenLDAP and I've tried to use "entryUUID' as a user_id_attribute. I'm also using groupOfNames as for my groups. I have users that DN looks like:

dn: cn=Rafał Ramocki,ou=Internal,ou=Users,dc=example,dc=com
uid: rafal.ramocki
entryUUID: 123141-3123123-13123123-32131231
(...)

Group looks like:
dn: cn=Users,ou=OpenStack,ou=Groups,dc=example,dc=com
member: cn=Rafał Ramocki,ou=Internal,ou=Users,dc=example,dc=com
(...)

I've tried to use "uid" as user name and entryUUID as unique record identifier. At the time of comparing group members with users code constructs query with following parameters:

(entryUUID=Rafał Ramocki)

And it obviously is is failing to find that user as it used "cn" insteed entryUUID. In fact this code works OK only if "user_id_attribute" is is first parameter in DN.

PS: This have another drownback. Since "Rafał" is an UTF string it's usage causes some other errors in Nova (SQL Alhemy fails to convert UTF8 to LATIN1 as some tables are still using it), and nova-placement (as UTF-u string is not URL-safe, and causes error in /usr/lib/python2.7/site-packages/nova/api/openstack/placement/requestlog.py (line 38) and maybe some other places.

Colleen Murphy (krinkle) on 2018-10-10
tags: added: ldap

Fix proposed to branch: master
Review: https://review.opendev.org/666575

Changed in keystone:
assignee: nobody → Corey Bryant (corey.bryant)
status: Triaged → In Progress
Changed in keystone (Ubuntu):
status: New → Triaged
importance: Undecided → Medium
Changed in keystone (Ubuntu Bionic):
status: New → Triaged
Changed in keystone (Ubuntu Cosmic):
status: New → Triaged
Changed in keystone (Ubuntu Disco):
status: New → Triaged
Changed in keystone (Ubuntu Cosmic):
importance: Undecided → Medium
Changed in keystone (Ubuntu Bionic):
importance: Undecided → Medium
Changed in keystone (Ubuntu Disco):
importance: Undecided → Medium
Corey Bryant (corey.bryant) wrote :

Here's what I'm currently testing with. Deployment-wise it's openstack charms specific but the other details should be meaningful to others.

Changed in keystone:
assignee: Corey Bryant (corey.bryant) → Guang Yee (guang-yee)

Reviewed: https://review.opendev.org/649177
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=a1dc21f3d34ae34bc6a5c9acebc0eb752495ae7a
Submitter: Zuul
Branch: master

commit a1dc21f3d34ae34bc6a5c9acebc0eb752495ae7a
Author: Raildo Mascena <email address hidden>
Date: Mon Apr 1 16:48:07 2019 -0300

    Fixing dn_to_id function for cases were id is not in the DN

    The more common scenario to return the uid as part of the RDN in a DN,
    However, it's a valid case to not have the uid in the RDN, so we need to
    search in the LDAP based on the DN and return the uid in the entire object.

    Also, we do not support multivalued attribute id on DN, so the test case
    covering this case, it was adjusted for raise NotFound.

    Closes-Bug: 1782922
    Change-Id: I87a3bfa94b5907ce4c6b4eb8e124ec948b390bf2

Changed in keystone:
status: In Progress → Fix Released

Change abandoned by Corey Bryant (<email address hidden>) on branch: master
Review: https://review.opendev.org/666575
Reason: Abandoned in favor of https://review.opendev.org/649177

Chris Sanders (chris.sanders) wrote :

Subscribing field-high which wasn't carried over from the other duplicate bug https://bugs.launchpad.net/keystone/+bug/1832766

Corey Bryant (corey.bryant) wrote :

Note that the patch that was merged had a bug which is being fixed in https://review.opendev.org/#/c/672519/ so both patches will need to be merged and backported.

Reviewed: https://review.opendev.org/672350
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=909cc9fa8380a03dfdb808db7fb863400fa36054
Submitter: Zuul
Branch: stable/stein

commit 909cc9fa8380a03dfdb808db7fb863400fa36054
Author: Raildo Mascena <email address hidden>
Date: Mon Apr 1 16:48:07 2019 -0300

    Fixing dn_to_id function for cases were id is not in the DN

    The more common scenario to return the uid as part of the RDN in a DN,
    However, it's a valid case to not have the uid in the RDN, so we need to
    search in the LDAP based on the DN and return the uid in the entire object.

    Also, we do not support multivalued attribute id on DN, so the test case
    covering this case, it was adjusted for raise NotFound.

    Closes-Bug: 1782922
    Change-Id: I87a3bfa94b5907ce4c6b4eb8e124ec948b390bf2
    (cherry picked from commit a1dc21f3d34ae34bc6a5c9acebc0eb752495ae7a)

Reviewed: https://review.opendev.org/672351
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=7b84e9fcf1c259fe32def301f5e94a2ded845533
Submitter: Zuul
Branch: stable/rocky

commit 7b84e9fcf1c259fe32def301f5e94a2ded845533
Author: Raildo Mascena <email address hidden>
Date: Mon Apr 1 16:48:07 2019 -0300

    Fixing dn_to_id function for cases were id is not in the DN

    The more common scenario to return the uid as part of the RDN in a DN,
    However, it's a valid case to not have the uid in the RDN, so we need to
    search in the LDAP based on the DN and return the uid in the entire object.

    Also, we do not support multivalued attribute id on DN, so the test case
    covering this case, it was adjusted for raise NotFound.

    Closes-Bug: 1782922
    Change-Id: I87a3bfa94b5907ce4c6b4eb8e124ec948b390bf2
    (cherry picked from commit a1dc21f3d34ae34bc6a5c9acebc0eb752495ae7a)

Reviewed: https://review.opendev.org/674030
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=9d9451e13c8e7a1835d721be7b8a4a5c6dff2b95
Submitter: Zuul
Branch: stable/queens

commit 9d9451e13c8e7a1835d721be7b8a4a5c6dff2b95
Author: Raildo Mascena <email address hidden>
Date: Mon Apr 1 16:48:07 2019 -0300

    Fixing dn_to_id function for cases were id is not in the DN

    The more common scenario to return the uid as part of the RDN in a DN,
    However, it's a valid case to not have the uid in the RDN, so we need to
    search in the LDAP based on the DN and return the uid in the entire object.

    Also, we do not support multivalued attribute id on DN, so the test case
    covering this case, it was adjusted for raise NotFound.

    Closes-Bug: 1782922
    Change-Id: I87a3bfa94b5907ce4c6b4eb8e124ec948b390bf2
    (cherry picked from commit a1dc21f3d34ae34bc6a5c9acebc0eb752495ae7a)

tags: added: sts-sru-needed
Changed in keystone (Ubuntu Eoan):
status: Triaged → Fix Released
Corey Bryant (corey.bryant) wrote :

Cosmic is EOL. will fix direction in Rocky cloud archive

Changed in keystone (Ubuntu Cosmic):
status: Triaged → Won't Fix
Corey Bryant (corey.bryant) wrote :

Ubuntu SRU Details:

[Impact]
When using the keystone LDAP backend, changing user_id_attribute breaks group mapping. This is because the _dn_to_id() method only calculated the uid to be the first RDN of the DN. _dn_to_id() is updated in the fix to also deal with the case where the uid is set to a different attribute.

[Test Case]
See details in comment #5: https://bugs.launchpad.net/keystone/+bug/1782922/comments/5

[Regression Potential]
The patch takes a minimal approach to the fix and includes unit tests to help ensure the patched code doesn't regress. The patches have landed in all upstream releases back to stable/queens which helps get even more exposure with upstream reviews, gate testing and real deployments.

This issue was fixed in the openstack/keystone 16.0.0.0rc1 release candidate.

Steve Langasek (vorlon) wrote :

The SRU diff looks good, but the SRU template is missing.

Since I see that a new test is added as part of the patch, for test case I'll accept an answer that the tests are run either at build time or via autopkgtest.

Changed in keystone (Ubuntu Disco):
status: Triaged → Incomplete
Steve Langasek (vorlon) wrote :

(It should be possible to confirm from a log that this newly added test did in fact run.)

Łukasz Zemczak (sil2100) wrote :

I see Corey added the template in bug comment #15, possibly because he does not have the powers to modify the bug description in this case? Anyway, I'll copy it over to the description field and accept.

description: updated
Changed in keystone (Ubuntu Disco):
status: Incomplete → Fix Committed
tags: added: verification-needed verification-needed-disco

Hello Jakob, or anyone else affected,

Accepted keystone into disco-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/keystone/2:15.0.0-0ubuntu1.2 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-disco to verification-done-disco. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-disco. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Duplicates of this bug

Other bug subscribers