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

[Brant Knudson]

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"
                }
            ],

[Dolph Mathews (dolph)]

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.

[Thierry Carrez (ttx)]

Looks valid to me

[Tristan Cacqueray (tristan-cacqueray)]

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.

[Brant Knudson (blk-u)]

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)]

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

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

[Brant Knudson (blk-u)]

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

[Brant Knudson (blk-u)]

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.

[Brant Knudson (blk-u)]

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

[Brant Knudson (blk-u)]

My opinion is that this doesn't affect havana.

The SQL query used different tables: http://git.openstack.org/cgit/openstack/keystone/tree/keystone/assignment/backends/sql.py?h=stable/havana#n70

Apparently LDAP didn't support groups: http://git.openstack.org/cgit/openstack/keystone/tree/keystone/assignment/backends/ldap.py?h=stable/havana

[Brant Knudson (blk-u)]

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.

[Brant Knudson (blk-u)]

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

[Thierry Carrez (ttx)]

keystone coresec please review proposed patch.

[Thierry Carrez (ttx)]

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.

[Brant Knudson (blk-u)]

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.

[Tristan Cacqueray (tristan-cacqueray)]

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

[Brant Knudson (blk-u)]

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

[Tristan Cacqueray (tristan-cacqueray)]

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.

[Thierry Carrez (ttx)]

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

[Brant Knudson (blk-u)]

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.

[Tristan Cacqueray (tristan-cacqueray)]

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.

[Brant Knudson (blk-u)]

The text in comment 22 looks good to me.

[Thierry Carrez (ttx)]

+1

[Adam Young (ayoung)]

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

[Dolph Mathews (dolph)]

+1 for impact description in comment 22

[Dolph Mathews (dolph)]

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

[Jeremy Stanley (fungi)]

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.

[Tristan Cacqueray (tristan-cacqueray)]

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

[Brant Knudson (blk-u)]

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

[Tristan Cacqueray (tristan-cacqueray)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[Thierry Carrez (ttx)]

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

[Matthew Thode (prometheanfire)]

still missing stable/icehouse :(

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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