LDAP: changing user_id_attribute bricks group mapping

Bug #1782922 reported by Jakob Englisch on 2018-07-21
26
This bug affects 3 people
Affects Status Importance Assigned to Milestone
OpenStack Identity (keystone)
Medium
Guang Yee
Ubuntu Cloud Archive
Medium
Unassigned
Queens
Medium
Unassigned
Rocky
Medium
Unassigned
Stein
Medium
Unassigned
Train
Medium
Unassigned
keystone (Ubuntu)
Medium
Unassigned
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 #25: https://bugs.launchpad.net/keystone/+bug/1782922/comments/25

[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
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.

Corey Bryant (corey.bryant) wrote :

Hello Jakob, or anyone else affected,

Accepted keystone into rocky-proposed. The package will build now and be available in the Ubuntu Cloud Archive in a few hours, and then in the -proposed repository.

Please help us by testing this new package. To enable the -proposed repository:

  sudo add-apt-repository cloud-archive:rocky-proposed
  sudo apt-get update

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-rocky-needed to verification-rocky-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-rocky-failed. In either case, details of your testing will help us make a better decision.

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

tags: added: verification-rocky-needed
Corey Bryant (corey.bryant) wrote :

Hello Jakob, or anyone else affected,

Accepted keystone into stein-proposed. The package will build now and be available in the Ubuntu Cloud Archive in a few hours, and then in the -proposed repository.

Please help us by testing this new package. To enable the -proposed repository:

  sudo add-apt-repository cloud-archive:stein-proposed
  sudo apt-get update

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-stein-needed to verification-stein-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-stein-failed. In either case, details of your testing will help us make a better decision.

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

tags: added: verification-stein-needed
Felipe Reyes (freyes) wrote :
Download full text (13.3 KiB)

Hello Corey,

I was trying to verify the SRU that it's in disco-proposed without success.
IIUC, the commands "openstack user list" and "openstack group list" should fail
when the package installed is 2:15.0.0-0ubuntu1.1 , here is the output of my
terminal, could you help me understand if I'm doing something wrong?

$ juju add-model lp1782922 && sleep 5 && tox -e func-smoke
Added 'lp1782922' model on stsstack/stsstack with credential 'laptop' for user 'laptop'
func-smoke installed: DEPRECATION: Python 2.7 will reach the end of its life on January 1st, 2020. Please upgrade your Python as Python 2.7 won't be maintained after that date. A future version of pip will drop support for Python 2.7. More details about Python 2 support in pip, can be found at https://pip.pypa.io/en/latest/development/release-process/#python-2-support,amulet==1.21.0,aodhclient==1.3.0,appdirs==1.4.3,Babel==2.7.0,backports.os==0.1.1,blessings==1.6,bundletester==0.12.2,certifi==2019.9.11,cffi==1.13.1,chardet==3.0.4,charm-tools==2.7.2,charmhelpers==0.20.4,Cheetah3==3.2.4,cliff==2.16.0,cmd2==0.8.9,colander==1.7.0,configparser==4.0.2,contextlib2==0.6.0.post1,coverage==4.5.4,cryptography==2.8,debtcollector==1.22.0,decorator==4.4.0,dict2colander==0.2,distro==1.4.0,distro-info==0.0.0,dogpile.cache==0.8.0,entrypoints==0.3,enum34==1.1.6,extras==1.0.0,fasteners==0.15,fixtures==3.0.0,flake8==2.4.1,funcsigs==1.0.2,functools32==3.2.3.post2,future==0.18.1,futures==3.3.0,futurist==1.9.0,gnocchiclient==3.1.1,httplib2==0.14.0,idna==2.8,importlib-metadata==0.23,ipaddress==1.0.23,iso8601==0.1.12,Jinja2==2.10.3,jmespath==0.9.4,jsonpatch==1.24,jsonpointer==2.0,jsonschema==2.5.1,juju-deployer==0.11.0,juju-wait==2.5.0,jujubundlelib==0.5.6,jujuclient==0.54.0,keyring==18.0.1,keystoneauth1==3.18.0,launchpadlib==1.10.7,lazr.authentication==0.1.3,lazr.restfulclient==0.14.2,lazr.uri==1.0.3,libcharmstore==0.0.9,linecache2==1.0.0,macaroonbakery==1.2.3,MarkupSafe==1.1.1,mccabe==0.3.1,mock==3.0.5,monotonic==1.5,more-itertools==5.0.0,msgpack==0.6.2,munch==2.3.2,netaddr==0.7.19,netifaces==0.10.9,nose==1.3.7,oauth==1.0.1,oauthlib==3.1.0,openstacksdk==0.36.0,os-client-config==1.33.0,os-service-types==1.7.0,osc-lib==1.14.1,oslo.concurrency==3.30.0,oslo.config==6.11.1,oslo.context==2.23.0,oslo.i18n==3.24.0,oslo.log==3.44.1,oslo.serialization==2.29.2,oslo.utils==3.41.2,osprofiler==2.8.2,otherstuf==1.1.0,parse==1.12.1,path.py==11.5.2,pathlib2==2.3.5,pathspec==0.3.4,pbr==5.4.3,pep8==1.7.1,pika==0.13.1,pkg-resources==0.0.0,prettytable==0.7.2,protobuf==3.10.0,pycparser==2.19,pyflakes==0.8.1,pyinotify==0.9.6,pymacaroons==0.13.0,PyNaCl==1.3.0,pyOpenSSL==19.0.0,pyparsing==2.4.2,pyperclip==1.7.0,pyRFC3339==1.1,python-barbicanclient==4.9.0,python-ceilometerclient==2.9.0,python-cinderclient==4.3.0,python-dateutil==2.8.0,python-designateclient==3.0.0,python-glanceclient==2.17.0,python-heatclient==1.18.0,python-keystoneclient==3.22.0,python-manilaclient==1.29.0,python-mimeparse==1.6.0,python-neutronclient==6.14.0,python-novaclient==16.0.0,python-openstackclient==4.0.0,python-subunit==1.3.0,python-swiftclient==3.8.1,pytz==2019.3,pyudev==0.21.0,PyYAML==3.13,requests==2.22.0,requestsexceptions==1.4.0,rfc3986==1.3.2,ruamel.or...

Timo Aaltonen (tjaalton) wrote :

Hello Jakob, or anyone else affected,

Accepted keystone into bionic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/keystone/2:13.0.2-0ubuntu2 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-bionic to verification-done-bionic. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-bionic. 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.

Changed in keystone (Ubuntu Bionic):
status: Triaged → Fix Committed
tags: added: verification-needed-bionic
description: updated
tags: added: verification-done-disco
removed: verification-needed-disco
Corey Bryant (corey.bryant) wrote :

Please see the attached document for testing details.

Corey Bryant (corey.bryant) wrote :

This has been tested successfully on disco-proposed, stein-proposed, and rocky-proposed using the steps in https://bugs.launchpad.net/keystone/+bug/1782922/comments/28.

Note: The current package version in bionic-proposed (keystone 2:13.0.2-0ubuntu2) has a regression that is being fixed via https://bugs.launchpad.net/bugs/1850634.

tags: added: verification-failed-bionic verification-rocky-done verification-stein-done
removed: verification-needed-bionic verification-rocky-needed verification-stein-needed

This issue was fixed in the openstack/keystone 13.0.3 release.

The verification of the Stable Release Update for keystone has completed successfully and the package is now being released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

Launchpad Janitor (janitor) wrote :

This bug was fixed in the package keystone - 2:15.0.0-0ubuntu1.2

---------------
keystone (2:15.0.0-0ubuntu1.2) disco; urgency=medium

  * d/p/000*-fixing-dn-to-id.patch: Fix LDAP backend's dn_to_id function
    for cases were id is not in the DN (LP: #1782922).

 -- Corey Bryant <email address hidden> Wed, 18 Sep 2019 11:08:09 +0200

Changed in keystone (Ubuntu Disco):
status: Fix Committed → Fix Released
Corey Bryant (corey.bryant) wrote :

The verification of the Stable Release Update for keystone has completed successfully and the package has now been released to -updates. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

Corey Bryant (corey.bryant) wrote :

This bug was fixed in the package keystone - 2:15.0.0-0ubuntu1.2~cloud0
---------------

 keystone (2:15.0.0-0ubuntu1.2~cloud0) bionic-stein; urgency=medium
 .
   * New update for the Ubuntu Cloud Archive.
 .
 keystone (2:15.0.0-0ubuntu1.2) disco; urgency=medium
 .
   * d/p/000*-fixing-dn-to-id.patch: Fix LDAP backend's dn_to_id function
     for cases were id is not in the DN (LP: #1782922).

Corey Bryant (corey.bryant) wrote :

 This bug was fixed in the package keystone - 2:14.1.0-0ubuntu1.1~cloud1
 ---------------

  keystone (2:14.1.0-0ubuntu1.1~cloud1) bionic-rocky; urgency=medium
  .
    * d/p/000*-fixing-dn-to-id.patch: Fix LDAP backend's dn_to_id function
      for cases were id is not in the DN (LP: #1782922).

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