[OSSA 2014-015] User gets group auth if same id (CVE-2014-0204)

Bug #1309228 reported by Brant Knudson
264
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Identity (keystone)
High
Brant Knudson
Icehouse
High
Brant Knudson
OpenStack Security Advisory
Medium
Tristan Cacqueray

Bug Description

If a user has the same ID as a group and that group has roles granted to it, the user gets the roles (even if they're not in the group).

Note that Keystone typically assigns IDs and with uuid4 you're not going to get a user with the same ID as a group, but some setups use LDAP so the IDs come from the LDAP entries.

Here's instructions on how to recreate:

1) Start with LDAP system (set up with devstack)

2) Create a user with an id of suspectid

$ ldapadd -D "cn=Manager,dc=openstack,dc=org" -w "ofs5dac"

dn: cn=suspectid,ou=Users,dc=openstack,dc=org
objectclass: inetorgperson
sn: suspect
userPassword: blkpwd

3) Create a group with an id of suspectid

$ ldapadd -D "cn=Manager,dc=openstack,dc=org" -w "ofs5dac"

dn: cn=suspectid,ou=UserGroups,dc=openstack,dc=org
objectclass: groupOfNames
ou: suspect
member: cn=dumb,dc=nonexistent

$ openstack --os-identity-api=3 --os-auth-url http://localhost:5000/v3 group list

4) Grant a role to the group on a project

$ openstack --os-identity-api=3 --os-auth-url http://localhost:5000/v3 role add --group suspect --project demo admin

5) Get a token as the user, notice that the user has the group's access.

$ curl -s \
  -H "Content-Type: application/json" \
  -d '
{ "auth": {
    "passwordCredentials": {
      "username": "suspect",
      "password": "blkpwd"
    },
    "tenantName": "demo"
  }
}' \
  http://localhost:35357/v2.0/tokens | python -m json.tool

---

            "roles": [
                {
                    "name": "admin"
                }
            ],

CVE References

Revision history for this message
Dolph Mathews (dolph) wrote :

I suppose this would also be possible with a SQL identity backend, but the odds of a collision between system-assigned UUIDs is astronomically low. If this is due to the assignment SQL refactor, this should only affect master & icehouse - not havana.

tags: added: icehouse-backport-potential
Changed in keystone:
importance: Undecided → High
Changed in ossa:
status: New → Incomplete
Revision history for this message
Thierry Carrez (ttx) wrote :

Looks valid to me

Changed in ossa:
status: Incomplete → Confirmed
Changed in keystone:
status: New → Confirmed
Thierry Carrez (ttx)
Changed in ossa:
importance: Undecided → High
Revision history for this message
Tristan Cacqueray (tristan-cacqueray) wrote :

Impact description draft #1:

Title: Keystone user and group id mismatch.
Reporter: Brant Knudson (IBM)
Products: Keystone
Versions: 2014.1

Description:
Brant Knudson from IBM reported a vulnerability in Keystone. Someone with write access to the Auth backend may willingly or unwillingly grant additional rights by picking the same IDs for users and groups, resulting in roles assigned to a group being assigned to the affected user even if he is not a member of this group.
Only Keystone setups using LDAP backend are likely affected.

Changed in ossa:
assignee: nobody → Tristan Cacqueray (tristan-cacqueray)
Revision history for this message
Brant Knudson (blk-u) wrote :

Looks like the problem code is keystone.assignment.backends.sql.Assignment:_get_metadata. You pass in a user_id or group_id and it treats them the same: http://git.openstack.org/cgit/openstack/keystone/tree/keystone/assignment/backends/sql.py?id=2fea9c560a6d8c4fc5522795624ac9a84bd40450#n81

The likely fix is to calculate the role type based on the arguments and include that in the query. Here's the table:

mysql> select * from assignment;
+--------------+----------------------------------+----------------------------------+----------------------------------+-----------+
| type | actor_id | target_id | role_id | inherited |
+--------------+----------------------------------+----------------------------------+----------------------------------+-----------+
| UserProject | f4d347ad6e2b46109d3b8c98dc53db61 | ce9a1366f8294daa80e3dc432b416c2c | 9e262162c0c24b73906a8171a787dc10 | 0 |
| GroupProject | suspectid | 79edaf3db5634de681db106103c81b81 | a77bec4d368a4a819e59ad364b793d85 | 0 |
+--------------+----------------------------------+----------------------------------+----------------------------------+-----------+

So the query should also have q.filter_by(type='UserProject') (group project because the args are user_id=xxx,tenant_id=yyy rather than group_id=xxx,tenant_id=yyy.

I tried writing a quick test in test_auth but that uses the kvs backend which apparently handles this correctly.

It should be good enough to write a test for get_roles_for_user_and_project, since that's what's called when the token is created.

Brant Knudson (blk-u)
Changed in keystone:
assignee: nobody → Brant Knudson (blk-u)
Revision history for this message
Brant Knudson (blk-u) wrote :

Here's a patch with a test showing the problem.

Looks like the SQL and LDAP assignment backends are not doing this correctly.

Revision history for this message
Brant Knudson (blk-u) wrote :

Here's a follow-on patch with the fix for the SQL backend.

Revision history for this message
Brant Knudson (blk-u) wrote :

The fix for the LDAP backend is trickier... the entry for the role assignments has a member attribute containing the DNs for both users and groups. If it was easy to compare DNs then this would be an easy check, just see if the entry is in the user tree or the group tree. Unfortunately, LDAP DNs cannot be compared with a simple string comparison, or even case-insensitive comparison.

Revision history for this message
Brant Knudson (blk-u) wrote :

Here's the patches for adding the test, the SQL fix, and the LDAP fix. I fixed a comment error in the test patch.

Revision history for this message
Brant Knudson (blk-u) wrote :
Revision history for this message
Brant Knudson (blk-u) wrote :
Revision history for this message
Brant Knudson (blk-u) wrote :
Revision history for this message
Brant Knudson (blk-u) wrote :

I think I'll re-do these patches so that they apply cleanly on stable/icehouse. All it should is moving the test_backend_ldap code so that it's not at the end of the class where it causes a conflict with new tests.

Revision history for this message
Brant Knudson (blk-u) wrote :

These patches applied cleanly to both master and stable/icehouse (as they are now).

Revision history for this message
Thierry Carrez (ttx) wrote :

keystone coresec please review proposed patch.

Changed in ossa:
importance: High → Medium
Revision history for this message
Thierry Carrez (ttx) wrote :

Impact desc looks good, just remove the trailing dot in the Title line, and put the "Only..." sentence on the same paragraph as the rest.

Revision history for this message
Brant Knudson (blk-u) wrote :

Please change the reporter in the impact description from "Brant Knudson" to "Michael Stancampiano". I just opened the bug, it was Mike who told me about it.

Otherwise the impact descrption in comment 3 looks good to me.

Revision history for this message
Tristan Cacqueray (tristan-cacqueray) wrote :

@Brant Thanks for the review! Do you know what, if any, company should I credit too ?

Revision history for this message
Brant Knudson (blk-u) wrote :

@Tristan - IBM is correct for the company. It was found during internal testing.

Revision history for this message
Tristan Cacqueray (tristan-cacqueray) wrote :

Alright, so here is impact description draft #2:

Title: Keystone user and group id mismatch
Reporter: Michael Stancampiano (IBM)
Products: Keystone
Versions: 2014.1

Description:
Michael Stancampiano from IBM reported a vulnerability in Keystone. Someone with write access to the Auth backend may willingly or unwillingly grant additional rights by picking the same IDs for users and groups, resulting in roles assigned to a group being assigned to the affected user even if he is not a member of this group. Only Keystone setups using LDAP backend are likely affected.

Revision history for this message
Thierry Carrez (ttx) wrote :

I would remove "likely" since it sounds like we think it may impact other backends but have no clue. Otherwise looks good.

Changed in ossa:
status: Confirmed → Triaged
Revision history for this message
Brant Knudson (blk-u) wrote :

A couple of nits

 - change "Auth backend" to "user and group repository (such as the LDAP directory server)" -- we have auth plugins so someone might be confused by this.

 - change "LDAP backend" to "LDAP for the Identity driver" -- the setting in the config file is like [identity].driver = keystone.identity.backends.ldap.Identity ( http://git.openstack.org/cgit/openstack/keystone/tree/etc/keystone.conf.sample#n754 ), so maybe this would be more consistent with how we refer to things.

Revision history for this message
Tristan Cacqueray (tristan-cacqueray) wrote :

Thanks you for the comments!
Here is impact description draft #3:

Title: Keystone user and group id mismatch
Reporter: Michael Stancampiano (IBM)
Products: Keystone
Versions: 2014.1

Description:
Michael Stancampiano from IBM reported a vulnerability in Keystone. Someone with write access to the user and group repository (such as the LDAP directory server) may willingly or unwillingly grant additional rights by picking the same IDs for users and groups, resulting in roles assigned to a group being assigned to the affected user even if he is not a member of this group. Only Keystone setups using LDAP for the Identity driver are affected.

Revision history for this message
Brant Knudson (blk-u) wrote :

The text in comment 22 looks good to me.

Revision history for this message
Thierry Carrez (ttx) wrote :

+1

Revision history for this message
Adam Young (ayoung) wrote :

There is a way to do Comparisons with DN's. John Dennis wrote up a fairly long treatise on it.

Revision history for this message
Dolph Mathews (dolph) wrote :

+1 for impact description in comment 22

Revision history for this message
Dolph Mathews (dolph) wrote :

+1 for the patches in comment #13.

I squashed all three patches into a single .patch (attached), which should be quicker to merge when this is opened. The only change here is the combining of Brant's commit messages. I'd encourage other reviewers to review comment 13 first and then commit message here.

Revision history for this message
Jeremy Stanley (fungi) wrote :

I'm good with Tristan's impact description in comment #22. We should plan a disclosure date and send downstream advance notifications at the earliest opportunity.

summary: - User gets group auth if same id
+ User gets group auth if same id (CVE-2014-0204)
Thierry Carrez (ttx)
Changed in ossa:
status: Triaged → In Progress
Revision history for this message
Tristan Cacqueray (tristan-cacqueray) wrote : Re: User gets group auth if same id (CVE-2014-0204)

@dolph, running ./run_tests.sh with the patch attached to comment #27 produces this error with stable/icehouse branch:

AssertionError: There is no script for 45 version

see http://paste.openstack.org/show/YTbHqEMnSmjSzPBUwx4R/

This may be related to my devstack or something...

If it's ok for you, we will send the pre-OSSA with a disclosure date set to:
2014-05-20, 1500UTC

Thanks.

Revision history for this message
Brant Knudson (blk-u) wrote :

Tristan - that error happens if you don't clean up the .pyc files, so the migration process finds a pyc file for migration 45 and the py isn't there anymore. Clean out the pyc files when checking out a branch with different migrations -- find keystone -name "*.pyc" | xargs rm

Revision history for this message
Tristan Cacqueray (tristan-cacqueray) wrote :

Thanks you Brant, that was it!
Tests succeed for both master and stable/icehouse branch.

The pre-OSSA have been sent.

Proposed public disclosure date/time:
2014-05-20, 15:00 UTC

Changed in ossa:
status: In Progress → Fix Committed
Thierry Carrez (ttx)
Changed in keystone:
status: Confirmed → In Progress
information type: Private Security → Public Security
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to keystone (master)

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to keystone (stable/icehouse)

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

summary: - User gets group auth if same id (CVE-2014-0204)
+ [OSSA 2014-015] User gets group auth if same id (CVE-2014-0204)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to keystone (master)

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to keystone (master)

Reviewed: https://review.openstack.org/94396
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=97dfd55ad1b40365754dcbfce856f7ffae280a44
Submitter: Jenkins
Branch: master

commit 97dfd55ad1b40365754dcbfce856f7ffae280a44
Author: Brant Knudson <email address hidden>
Date: Fri Apr 18 11:18:42 2014 -0500

    SQL fix for get_roles_for_user_and_project user=group ID

    When there was a role assigned to a group with the same ID as a
    user, the SQL assignment backend would incorrectly return the
    assignment to the group when requesting roles for the user via
    the get_roles_for_user_and_project method.

    With this change, assignments to a group with the same ID are not
    returned for the user when calling get_roles_for_user_and_project.

    Change-Id: Id3d6f66c995e65e37d909359420d71ecdde86b69
    Partial-Bug: #1309228

Changed in keystone:
assignee: Brant Knudson (blk-u) → Adam Young (ayoung)
Changed in keystone:
assignee: Adam Young (ayoung) → Brant Knudson (blk-u)
Alan Pevec (apevec)
tags: removed: icehouse-backport-potential
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to keystone (stable/icehouse)

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to keystone (master)

Reviewed: https://review.openstack.org/94470
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=729dcad7384ba66ee7494154969cdd7ae90d86ee
Submitter: Jenkins
Branch: master

commit 729dcad7384ba66ee7494154969cdd7ae90d86ee
Author: Brant Knudson <email address hidden>
Date: Wed May 21 17:30:42 2014 -0500

    LDAP fix for get_roles_for_user_and_project user=group ID

    When there was a role assigned to a group with the same ID as a
    user, the LDAP assignment backend would incorrectly return the
    assignment to the group when requesting roles for the user via
    the get_roles_for_user_and_project method.

    With this change, assignments to a group with the same ID are not
    returned for the user when calling get_roles_for_user_and_project.

    Functions were added to compare DNs more accurately based on the
    LDAP RFCs.

    The fakeldap code was changed to normalize the values when
    comparing values for checking if the values match the filter.

    Co-Authored By: Nathan Kinder <email address hidden>
    Co-Authored By: Adam Young <email address hidden>

    Change-Id: Ia6f1ae2e3af1e968f1a393bd4f2f38812a88a5d0
    Closes-Bug: #1309228

Changed in keystone:
status: In Progress → Fix Committed
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to keystone (master)

Related fix proposed to branch: master
Review: https://review.openstack.org/95263

Revision history for this message
Thierry Carrez (ttx) wrote :

Only missing stable/icehouse:
https://review.openstack.org/#/c/94397/

Revision history for this message
Matthew Thode (prometheanfire) wrote :

still missing stable/icehouse :(

https://review.openstack.org/#/c/94397/

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to keystone (stable/icehouse)

Reviewed: https://review.openstack.org/94397
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=786af9829c5329a982e3451f77afebbfb21850bd
Submitter: Jenkins
Branch: stable/icehouse

commit 786af9829c5329a982e3451f77afebbfb21850bd
Author: Brant Knudson <email address hidden>
Date: Fri Apr 18 11:18:42 2014 -0500

    SQL and LDAP fixes for get_roles_for_user_and_project user=group ID

    When there was a role assigned to a group with the same ID as a user,
    the SQL and LDAP assignment backends would incorrectly return the
    assignment to the group when requesting roles for the user via the
    get_roles_for_user_and_project method.

    With this change, assignments to a group with the same ID are not
    returned for the user when calling get_roles_for_user_and_project.

    Functions were added to compare DNs more accurately based on the
    LDAP RFCs.

    The fakeldap code was changed to normalize the values when
    comparing values for checking if the values match the filter.

    Co-Authored By: Nathan Kinder <email address hidden>
    Co-Authored By: Adam Young <email address hidden>

    Change-Id: Id3d6f66c995e65e37d909359420d71ecdde86b69
    Closes-Bug: #1309228

Thierry Carrez (ttx)
Changed in ossa:
status: Fix Committed → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to keystone (master)

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

commit f0eee2f3b48dd0cffb9f75e396da2d914925cba5
Author: Brant Knudson <email address hidden>
Date: Fri May 23 15:44:05 2014 -0500

    Remove obsolete note from ldap

    There was a note in the ldap assignment backend about normalizing
    a DN using .upper(). This note is obsolete with the recent change
    to use a DN compare function.

    Change-Id: I34c69573498b5f2d2103ea485160df421c6f2b03
    Related-Bug: #1309228

Thierry Carrez (ttx)
Changed in keystone:
milestone: none → juno-1
status: Fix Committed → Fix Released
Thierry Carrez (ttx)
Changed in keystone:
milestone: juno-1 → 2014.2
To post a comment you must log in.