[OSSA 2012-014] Token validation includes revoked roles (CVE-2012-4413)

Bug #1041396 reported by Dolph Mathews on 2012-08-24
268
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Keystone
Critical
Thierry Carrez
Essex
Critical
Thierry Carrez
OpenStack Security Advisory
Undecided
Thierry Carrez
keystone (Ubuntu)
Undecided
Unassigned
Nominated for Precise by Yolanda Robla

Bug Description

To reproduce:
1) Initial setup: http://paste.openstack.org/raw/20477/
2) Authenticate as a normal user, generating a token
3) On the admin API, revoke a role from that user
4) On the admin API, validate the user's generated token

The revoked role is included in the validation response. Ideally, the token should be entirely invalidated and return 404, although at the very least, the revoked role should *NOT* be included in the validation response.

Full example:

Authenticate as a user who has been granted the 'manager' role:

    POST http://127.0.0.1:5000/v2.0/tokens
    ======================================

    Content-Type: application/json

    {
      "auth": {
        "tenantName": "project-x",
        "passwordCredentials": {
          "username": "joe",
          "password": "secret"
        }
      }
    }

    200 OK
    ======

    Date: Fri, 24 Aug 2012 22:43:24 GMT
    Vary: X-Auth-Token
    Content-Length: 448
    Status: 200
    Content-Type: application/json

    {
      "access": {
        "token": {
          "expires": "2012-08-25T22:43:24Z",
          "id": "c0db082bdb7f47e4845d7be551558620",
          "tenant": {
            "id": "10e2a090121748388cf39e915d376f29",
            "enabled": true,
            "description": null,
            "name": "project-x"
          }
        },
        "serviceCatalog": {},
        "user": {
          "username": "joe",
          "roles_links": [],
          "id": "b2a6f8d5dbb249f3b9ac8a46e8cb77e6",
          "roles": [
            {
              "name": "manager"
            }
          ],
          "name": "joe"
        },
        "metadata": {
          "is_admin": 0,
          "roles": [
            "facd80ce22d44eae87375f11295f8e51"
          ]
        }
      }
    }

Validate the user's token (note the presence of the 'manager' role):

    GET http://127.0.0.1:35357/v2.0/tokens/c0db082bdb7f47e4845d7be551558620
    =======================================================================

    X-Auth-Token: ADMIN

    200 OK
    ======

    Status: 200
    Content-Length: 490
    Content-Location: http://127.0.0.1:35357/v2.0/tokens/c0db082bdb7f47e4845d7be551558620
    Vary: X-Auth-Token
    Date: Fri, 24 Aug 2012 22:44:01 GMT
    Content-Type: application/json

    {
      "access": {
        "token": {
          "expires": "2012-08-25T22:43:24Z",
          "id": "c0db082bdb7f47e4845d7be551558620",
          "tenant": {
            "description": null,
            "enabled": true,
            "id": "10e2a090121748388cf39e915d376f29",
            "name": "project-x"
          }
        },
        "serviceCatalog": {},
        "user": {
          "username": "joe",
          "roles_links": [],
          "id": "b2a6f8d5dbb249f3b9ac8a46e8cb77e6",
          "roles": [
            {
              "id": "facd80ce22d44eae87375f11295f8e51",
              "name": "manager"
            }
          ],
          "name": "joe"
        },
        "metadata": {
          "is_admin": 0,
          "roles": [
            "facd80ce22d44eae87375f11295f8e51"
          ]
        }
      }
    }

As admin, revoke the 'manager' role from the user:

    $ keystone user-role-remove --user-id=b2a6f8d5dbb249f3b9ac8a46e8cb77e6 --role-id=facd80ce22d44eae87375f11295f8e51 --tenant-id=10e2a090121748388cf39e915d376f29

As admin, the validation response remains unchanged (including the revoked 'manager' role):

    GET http://127.0.0.1:35357/v2.0/tokens/c0db082bdb7f47e4845d7be551558620
    =======================================================================

    X-Auth-Token: ADMIN

    200 OK
    ======

    Status: 200
    Content-Length: 490
    Content-Location: http://127.0.0.1:35357/v2.0/tokens/c0db082bdb7f47e4845d7be551558620
    Vary: X-Auth-Token
    Date: Fri, 24 Aug 2012 22:44:46 GMT
    Content-Type: application/json

    {
      "access": {
        "token": {
          "expires": "2012-08-25T22:43:24Z",
          "id": "c0db082bdb7f47e4845d7be551558620",
          "tenant": {
            "description": null,
            "enabled": true,
            "id": "10e2a090121748388cf39e915d376f29",
            "name": "project-x"
          }
        },
        "serviceCatalog": {},
        "user": {
          "username": "joe",
          "roles_links": [],
          "id": "b2a6f8d5dbb249f3b9ac8a46e8cb77e6",
          "roles": [
            {
              "id": "facd80ce22d44eae87375f11295f8e51",
              "name": "manager"
            }
          ],
          "name": "joe"
        },
        "metadata": {
          "is_admin": 0,
          "roles": [
            "facd80ce22d44eae87375f11295f8e51"
          ]
        }
      }
    }

Dolph Mathews (dolph) wrote :
Dolph Mathews (dolph) wrote :

Note: I'm suddenly having trouble getting the stable/essex S3 & swift middleware tests to run on my machine (both with and without this patch) -- but I don't think they should be impacted, regardless.

tags: added: essex-backport
Changed in keystone:
status: Confirmed → In Progress
Dolph Mathews (dolph) on 2012-08-27
security vulnerability: yes → no
security vulnerability: no → yes
Russell Bryant (russellb) wrote :

We need a couple of keystone-core reviews on these patches.

Here is a draft description. Right now it's mostly one long awkward sentence. There's probably a more elegant way of describing it ...

Title: Revoking a role does not affect existing tokens
Impact: High
Reporter: Dolph Mathews (Rackspace)
Products: Keystone
Affects: Essex, Folsom

Description:
Dolph Mathews reported a vulnerability in Keystone. If you revoke a role from a user from the admin API and then validate a token that existed before revoking the role, the token validation response will still include that role.

Download full text (6.2 KiB)

I just looked through reviewboard and didn't see any patches pending review.

-joe

On Aug 27, 2012, at 2:16 PM, Russell Bryant <email address hidden> wrote:
> We need a couple of keystone-core reviews on these patches.
>
> Here is a draft description. Right now it's mostly one long awkward
> sentence. There's probably a more elegant way of describing it ...
>
>
> Title: Revoking a role does not affect existing tokens
> Impact: High
> Reporter: Dolph Mathews (Rackspace)
> Products: Keystone
> Affects: Essex, Folsom
>
> Description:
> Dolph Mathews reported a vulnerability in Keystone. If you revoke a role from a user from the admin API and then validate a token that existed before revoking the role, the token validation response will still include that role.
>
> --
> You received this bug notification because you are a member of Keystone
> Bugs, which is subscribed to the bug report.
> https://bugs.launchpad.net/bugs/1041396
>
> Title:
> Token validation includes revoked roles
>
> Status in OpenStack Identity (Keystone):
> In Progress
>
> Bug description:
> To reproduce:
> 1) Initial setup: http://paste.openstack.org/raw/20477/
> 2) Authenticate as a normal user, generating a token
> 3) On the admin API, revoke a role from that user
> 4) On the admin API, validate the user's generated token
>
> The revoked role is included in the validation response. Ideally, the
> token should be entirely invalidated and return 404, although at the
> very least, the revoked role should *NOT* be included in the
> validation response.
>
> Full example:
>
> Authenticate as a user who has been granted the 'manager' role:
>
> POST http://127.0.0.1:5000/v2.0/tokens
> ======================================
>
> Content-Type: application/json
>
> {
> "auth": {
> "tenantName": "project-x",
> "passwordCredentials": {
> "username": "joe",
> "password": "secret"
> }
> }
> }
>
> 200 OK
> ======
>
> Date: Fri, 24 Aug 2012 22:43:24 GMT
> Vary: X-Auth-Token
> Content-Length: 448
> Status: 200
> Content-Type: application/json
>
> {
> "access": {
> "token": {
> "expires": "2012-08-25T22:43:24Z",
> "id": "c0db082bdb7f47e4845d7be551558620",
> "tenant": {
> "id": "10e2a090121748388cf39e915d376f29",
> "enabled": true,
> "description": null,
> "name": "project-x"
> }
> },
> "serviceCatalog": {},
> "user": {
> "username": "joe",
> "roles_links": [],
> "id": "b2a6f8d5dbb249f3b9ac8a46e8cb77e6",
> "roles": [
> {
> "name": "manager"
> }
> ],
> "name": "joe"
> },
> "metadata": {
> "is_admin": 0,
> "roles": [
> "facd80ce22d44eae87375f11295f8e51"
> ]
> }
> }
> }
>
> Validate the user's token (note the presence of the 'manager' role):
>
> GET http://127.0.0.1:35357/v2.0/tokens/c0db082bdb7f47e4...

Read more...

The patches are attached to this bug since it's a security vulnerability.

Dolph Mathews (dolph) wrote :

"Granting and revoking roles from a user is not reflected upon token validation for pre-existing tokens. Pre-existing tokens continue to be valid for the original set of roles for the remainder of the token's lifespan, or until explicitly invalidated."

The proposed patch invalidates all tokens held by a user upon role grant/revoke to circumvent the issue.

Joseph Heck (heckj) wrote :

+2 from me on the patches, although we need to be aware that the solution to this vulnerability is *dependent* on the token backend being able to list tokens through the internal token API.

The patches current invoke a WARN log message in that case (i.e. listing tokens returning a NotImplemented exception) - should this be an "ERROR" and is it actionable?

I think WARN is appropriate here, but I thought it worth asking the question.

Russell Bryant (russellb) wrote :

So it looks like memcache is the only token backend that would still have this problem? We should note that in the description. If someone is using the memcache backend, presumably they are accepting this shortcoming. Are they going to get annoyed with getting this warning in their log over and over? Perhaps it should only be logged once?

Updated description:

Title: Revoking a role does not affect existing tokens
Impact: High
Reporter: Dolph Mathews (Rackspace)
Products: Keystone
Affects: Essex, Folsom

Description:
Dolph Mathews reported a vulnerability in Keystone. Granting and revoking roles from a user is not reflected upon token validation for pre-existing tokens. Pre-existing tokens continue to be valid for the original set of roles for the remainder of the token's lifespan, or until explicitly invalidated.

The proposed patch invalidates all tokens held by a user upon role grant/revoke to circumvent the issue. Note that due to how the memcache token backend works, it will still be affected by this issue.

Adam Young (ayoung) wrote :

-2

Please add in code to disable the memcached backend for tokens. Token revocation cannot be done on Memcached, which means that even past changes to deal with revocation do not cover it.

Dolph Mathews (dolph) wrote :

Adam: Not sure exactly what you're asking for here? "code to disable the memcached backend for tokens."

Russell Bryant (russellb) wrote :

I interpreted it as "we should get rid of memcache token backend support completely" ...

Thierry Carrez (ttx) wrote :

@Dolph, Adam: any chance you could work out a patch that would satisfy you both ? I'd like to close this one asap...

Adam Young (ayoung) wrote :

Proposal for Memcached: Each token created gets written into the users token list which is persisted to memcached as well. Upon user disable and revocation events, remove all tokens in the users list.

Dolph Mathews (dolph) wrote :

Applying Adam's patch for bug 1046905 (currently in review: https://review.openstack.org/#/c/12506/ ) *along with* the above patch should solve this issue for the memcache driver.

Thierry Carrez (ttx) wrote :

Keystone-core: please confirm that patches above are OK

Dolph: do they need any rebasing now that Adam's patch landed ?

All: How does the complete fix affect the proposed description in comment 8 ?

Adam Young (ayoung) wrote :

Dolph's patch should not require rebasing. It changes:

keystone/identity/core.py

The other patch only changed
 keystone/token/backends/memcache.py
 tests/test_backend.py
 tests/test_backend_memcache.py

Dolph's patch was no explicitly skipping the memcached backend, the problem was entirely in memcached.

This completely addresses the issue in comment 8. The whole thing now needs testing together.

Thierry Carrez (ttx) wrote :

Patches could use a bit of refactoring, to avoid the copy-paste of code.

Adding Mark to make sure that behavior change (invalidate all the user tokens whenever a role is granted or revoked) would be acceptable for stable/essex.

Fixed proposed description:
"""
Title: Revoking a role does not affect existing tokens
Impact: High
Reporter: Dolph Mathews (Rackspace)
Products: Keystone
Affects: Essex, Folsom

Description:
Dolph Mathews reported a vulnerability in Keystone. Granting and revoking roles from a user is not reflected upon token validation for pre-existing tokens. Pre-existing tokens continue to be valid for the original set of roles for the remainder of the token's lifespan, or until explicitly invalidated. This fix invalidates all tokens held by a user upon role grant/revoke to circumvent the issue.
"""

Mark McLoughlin (markmc) wrote :

I don't think leaving the current behaviour is acceptable for stable/essex

Ideally, the behaviour change to fix this wouldn't be so violent, but I think we're unlikely to break any clients here ... clients can't really assume their token won't be invalidated at any point and removing roles etc. doesn't happen often

So, yeah - fine by me for stable/essex

Dolph Mathews (dolph) wrote :

Refactored implementation into token API.

Dolph Mathews (dolph) wrote :
Thierry Carrez (ttx) wrote :

Keystone core, please ack latest patch

Adam Young (ayoung) wrote :

We should drop the catch block in revoke_tokens long term, as not supporting list is not acceptable any more. Failure to support list should make things blow up.

Joseph Heck (heckj) wrote :

agreed with Adam - not support token_list() is opening a security hole

patches are good with https://review.openstack.org/#/c/12506/ merged in place (which it is in master)

we'll need a https://review.openstack.org/#/c/12506/ equiv against stable-essex as well.

Russell Bryant (russellb) wrote :

So would you guys like to update the patch so it makes not having token_list an error?

Also, Adam, can you prepare an Essex version of the memcache changes? We will need that before we can send out the advance notification of the advisory. If we can get it out today, we'll be able to release all of this next week and have it in folsom-rc1.

Adam Young (ayoung) wrote :
Adam Young (ayoung) wrote :

I get a "Patch format detection failed" with the patches above. They should be generated using git format-patch.

Dolph Mathews (dolph) wrote :

Agree; dropped the 'try.' An implementation of list_tokens is now *required* in the token driver in order to grant and revoke roles.

Dolph Mathews (dolph) wrote :
Dolph Mathews (dolph) wrote :

Alternative version of above patches produced with format-patch

Adam Young (ayoung) wrote :
Dolph Mathews (dolph) wrote :

I missed a LOG reference that was no longer used in patch v3 -- updated here.

Dolph Mathews (dolph) wrote :
Adam Young (ayoung) wrote :

I can approve both patches.

Note that the essex patch requires https://review.openstack.org/#/c/12590/

Russell Bryant (russellb) wrote :

Cool, thanks guys! Based on heckj's ack earlier and agreement that token_list should be mandatory, I'm going to go ahead and push out the advance notification. If any further changes are needed, I can push out updates.

Russell Bryant (russellb) wrote :

Advance notification has been sent. Thanks for working on this today, guys!

Thierry Carrez (ttx) on 2012-09-10
summary: - Token validation includes revoked roles
+ Token validation includes revoked roles (CVE-2012-4413)

Proposed public disclosure date/time:
Wednesday, September 12th, 1500 UTC

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

Changed in keystone:
assignee: Dolph Mathews (dolph) → Thierry Carrez (ttx)

Pushed both patches as draft in preparation for release today at 1500 UTC. Feel free to test.

Thierry Carrez (ttx) on 2012-09-12
visibility: private → public
Thierry Carrez (ttx) wrote :

Patches public now.

Reviewed: https://review.openstack.org/12868
Committed: http://github.com/openstack/keystone/commit/efb6b3fca0ba0ad768b3e803a324043095d326e2
Submitter: Jenkins
Branch: master

commit efb6b3fca0ba0ad768b3e803a324043095d326e2
Author: Dolph Mathews <email address hidden>
Date: Fri Sep 7 14:35:21 2012 -0500

    Delete user tokens after role grant/revoke

    Delete user tokens when a new role is granted or revoked, in order to
    prevent old tokens to continue to be valid for the original set of
    roles for the remainder of the token's lifespan.

    Addresses CVE-2012-4413.
    Fixes bug 1041396.

    Change-Id: Iecf891f274b67408f568b949a7028362c4c30312

Changed in keystone:
status: In Progress → Fix Committed

Reviewed: https://review.openstack.org/12870
Committed: http://github.com/openstack/keystone/commit/58ac6691a21675be9e2ffb0f84a05fc3cd4d2e2e
Submitter: Jenkins
Branch: stable/essex

commit 58ac6691a21675be9e2ffb0f84a05fc3cd4d2e2e
Author: Dolph Mathews <email address hidden>
Date: Fri Sep 7 14:55:31 2012 -0500

    Delete user tokens after role grant/revoke

    Delete user tokens when a new role is granted or revoked, in order to
    prevent old tokens to continue to be valid for the original set of
    roles for the remainder of the token's lifespan.

    Addresses CVE-2012-4413.
    Fixes bug 1041396.

    Change-Id: Ib11b5b3a933c6000afe0c875c3f71f1f101bb202

Thierry Carrez (ttx) on 2012-09-14
Changed in keystone:
status: Fix Committed → Fix Released
Thierry Carrez (ttx) on 2012-09-27
Changed in keystone:
milestone: folsom-rc1 → 2012.2
Changed in keystone (Ubuntu):
status: New → Fix Released

Hello Dolph, or anyone else affected,

Accepted keystone into precise-proposed. The package will build now and be available at http://launchpad.net/ubuntu/+source/keystone/2012.1.3+stable-20130423-f48dd0fc-0ubuntu1 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 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 to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-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-needed

Please find the attached test log from the Ubuntu Server Team's CI infrastructure. As part of the verification process for this bug, Keystone has been deployed and configured across multiple nodes using precise-proposed as an installation source. After successful bring-up and configuration of the cluster, a number of exercises and smoke tests have be invoked to ensure the updated package did not introduce any regressions. A number of test iterations were carried out to catch any possible transient errors.

Please Note the list of installed packages at the top and bottom of the report.

For records of upstream test coverage of this update, please see the Jenkins links in the comments of the relevant upstream code-review(s):

Trunk review: https://review.openstack.org/12868
Stable review: https://review.openstack.org/12870

As per the provisional Micro Release Exception granted to this package by the Technical Board, we hope this contributes toward verification of this update.

Test coverage log.

tags: added: verification-done
removed: verification-needed

The verification of this Stable Release Update has completed successfully and the package has now been 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 regresssions.

Thierry Carrez (ttx) on 2013-06-07
summary: - Token validation includes revoked roles (CVE-2012-4413)
+ [OSSA 2012-014] Token validation includes revoked roles (CVE-2012-4413)
Changed in ossa:
assignee: nobody → Thierry Carrez (ttx)
status: New → Fix Released
To post a comment you must log in.