[OSSA 2012-013] Update user's default tenant partially succeeds without authz

Bug #1040626 reported by Dolph Mathews
268
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Identity (keystone)
Fix Released
Critical
Dolph Mathews
Essex
Fix Released
Critical
Russell Bryant
OpenStack Security Advisory
Fix Released
Undecided
Russell Bryant
keystone (Ubuntu)
Fix Released
Undecided
Unassigned
Nominated for Precise by Yolanda Robla

Bug Description

Attempting to update a user's default tenant using the following API call returns unauthorized as expected, but the user is still granted "membership" on the newly specified tenant.

As a normal user against the Public API:

    GET http://localhost:5000/v2.0/tenants
    ======================================

    X-Auth-Token: cb81c8ba4cb6428ca21b0e8de8b10ab1

    200 OK
    ======

    Vary: X-Auth-Token
    Content-Type: application/json

    {
      "tenants": [
        {
          "id": "2636bcb347af46c1a00055e9e0ec3cd7",
          "enabled": true,
          "description": null,
          "name": "A"
        }
      ],
      "tenants_links": []
    }

Against the Admin API without an X-Auth-Token:

    PUT http://localhost:35357/v2.0/users/679a2bb1e113473db10827895ae9022f/tenant
    =============================================================================

    Content-Type: application/json

    {
      "user": {
        "tenantId": "77e165b067014483a5503fde1fa235d3"
      }
    }

    401 Not Authorized
    ==================

    Vary: X-Auth-Token
    Content-Type: application/json

    {
      "error": {
        "message": "The request you have made requires authentication.",
        "code": 401,
        "title": "Not Authorized"
      }
    }

And the first call repeated (again, as the user in question against the Public API):

    GET http://localhost:5000/v2.0/tenants
    ======================================

    X-Auth-Token: cb81c8ba4cb6428ca21b0e8de8b10ab1

    200 OK
    ======

    Vary: X-Auth-Token
    Content-Type: application/json

    {
      "tenants": [
        {
          "id": "2636bcb347af46c1a00055e9e0ec3cd7",
          "enabled": true,
          "description": null,
          "name": "A"
        },
        {
          "id": "77e165b067014483a5503fde1fa235d3",
          "enabled": true,
          "description": null,
          "name": "B"
        }
      ],
      "tenants_links": []
    }

Note that the user's default tenant is not actually updated.

Dolph Mathews (dolph)
tags: added: essex-backport
Revision history for this message
Thierry Carrez (ttx) wrote :

So the impact is that you can add yourself (or anyone) as a member of any tenant ? Eek.
Please post patches here and not on gerrit, looks like we'll have to embargo this one.

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

Adding markmc as it looks like that could warrant a stable update release...

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

ttx: already went through gerrit

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

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

Argh. Means we need to speed up and won't be able to embargo it that much.
@Dolph: could you produce a patch for Essex and attach it here ?

Revision history for this message
Dolph Mathews (dolph) wrote :
Joseph Heck (heckj)
Changed in keystone:
status: In Progress → Fix Committed
Revision history for this message
Russell Bryant (russellb) wrote :

Can keystone-core reviewers please confirm that the essex patch is good to go?

We need to get a security advisory out for this ASAP. Please review this vulnerability description. Given how serious the issue is, I still want to give stakeholders advance warning before releasing the advisory even though the patch was already put into the public. However, we should not embargo this as long as usual. So, I propose that we release this advisory and the patch for Essex on Thursday, August 30th.

Title: Lack of authorization for addings users to tenants
Impact: Critical
Reporter: Dolph Mathews <email address hidden>
Products: Keystone
Affects: Essex, Folsom

Description:
Dolph Mathews reported a vulnerability in Keystone. When attempting to update a user's default tenant, Keystone will only partially deny the request when a user is not authorized to complete this action. The API responds with 401 Not Authorized and the user's default tenant is not changed. However, the user is still granted membership to this new tenant. The result is that any user can add any other user to any tenant.

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

I would clarify the last sentence a bit further, as: "The result is that any client that can reach the administrative API (deployed on port 35357, by default) can add any user to any tenant."

As originally stated, there are two different uses of the word "user" (the first being a user of the API, and the second being an entity in the Identity system).

Revision history for this message
Russell Bryant (russellb) wrote :

Thanks for the quick feedback! Here is an updated version:

Title: Lack of authorization for addings users to tenants
Impact: Critical
Reporter: Dolph Mathews (Rackspace)
Products: Keystone
Affects: Essex, Folsom

Description:
Dolph Mathews reported a vulnerability in Keystone. When attempting to update a user's default tenant, Keystone will only partially deny the request when a user is not authorized to complete this action. The API responds with 401 Not Authorized and the user's default tenant is not changed. However, the user is still granted membership to this new tenant.The result is that any client that can reach the administrative API (deployed on port 35357, by default) can add any user to any tenant.

visibility: private → public
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to keystone (stable/essex)

Fix proposed to branch: stable/essex
Review: https://review.openstack.org/12194

Revision history for this message
Russell Bryant (russellb) wrote : Re: Update user's default tenant partially succeeds without authz

Essex backport posted: c13d0ba606f7b2bdc609a7f388334e5efec3f3aa

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

Reviewed: https://review.openstack.org/12194
Committed: http://github.com/openstack/keystone/commit/5438d3b5a219d7c8fa67e66e538d325a61617155
Submitter: Jenkins
Branch: stable/essex

commit 5438d3b5a219d7c8fa67e66e538d325a61617155
Author: Dolph Mathews <email address hidden>
Date: Thu Aug 23 07:39:20 2012 -0500

    Require authz to update user's tenant (bug 1040626)

    Change-Id: I82f80b84af2bc4db00b3dcb87a2ec338816a82e9

Revision history for this message
Russell Bryant (russellb) wrote : Re: Update user's default tenant partially succeeds without authz
Revision history for this message
Steve Beattie (sbeattie) wrote :

This was fixed in Ubuntu 12.04 LTS in http://www.ubuntu.com/usn/usn-1552-1/ but still needs to be fixed in quantal (ubuntu 12.10). Attached is a debdiff to do so.

Steve Beattie (sbeattie)
Changed in keystone (Ubuntu):
status: New → Triaged
Revision history for this message
Ubuntu Foundations Team Bug Bot (crichton) wrote :

The attachment "1040626-stable-essex.patch" of this bug report has been identified as being a patch. The ubuntu-reviewers team has been subscribed to the bug report so that they can review the patch. In the event that this is in fact not a patch you can resolve this situation by removing the tag 'patch' from the bug report and editing the attachment so that it is not flagged as a patch. Additionally, if you are member of the ubuntu-reviewers team please also unsubscribe the team from this bug report.

[This is an automated message performed by a Launchpad user owned by Brian Murray. Please contact him regarding any issues with the action taken in this bug report.]

tags: added: patch
Revision history for this message
Steve Beattie (sbeattie) wrote : Re: Update user's default tenant partially succeeds without authz

Addressed in Ubuntu 12.10 with keystone 2012.2~rc1~20120906.2517-0ubuntu2.

Changed in keystone (Ubuntu):
status: Triaged → Fix Released
Thierry Carrez (ttx)
Changed in keystone:
status: Fix Committed → Fix Released
Thierry Carrez (ttx)
Changed in keystone:
milestone: folsom-rc1 → 2012.2
Revision history for this message
Neela Shah (neela) wrote :

Is there a corresponding fix similar to this for when using commands? This fix seems to be targeted to the API path.

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

@Neela: The CLI client uses the HTTP API, so this fix applies to both.

Revision history for this message
Brian Murray (brian-murray) wrote : Please test proposed package

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
Revision history for this message
Yolanda Robla (yolanda.robla) wrote : Verification report.

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

Stable review: https://review.openstack.org/12194

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

Revision history for this message
Yolanda Robla (yolanda.robla) wrote : Re: Update user's default tenant partially succeeds without authz

Test coverage log.

tags: added: verification-done
removed: verification-needed
Revision history for this message
Scott Kitterman (kitterman) wrote : Update Released

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)
summary: - Update user's default tenant partially succeeds without authz
+ [OSSA 2012-013] Update user's default tenant partially succeeds without
+ authz
Changed in ossa:
assignee: nobody → Russell Bryant (russellb)
status: New → Fix Released
To post a comment you must log in.
This report contains Public Security information  
Everyone can see this security related information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.