Admin rights shouldn't work cross-tenant

Bug #1010547 reported by Alexej Ababilov on 2012-06-08
This bug report is a duplicate of:  Bug #968696: "admin"-ness not properly scoped. Edit Remove
12
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Glance
Wishlist
Unassigned
OpenStack Identity (keystone)
Wishlist
Unassigned

Bug Description

Consider user A is an `admin` in Y tenant and has no roles in Z tenant. Let G image be an image from Z tenant.

User A gets a token for Y tenant and sends a request: "delete G image". Currently, glance allows such malicious action.

That's because glance/common/context.py:RequestContext allows `admin` to mutate any image despite its owner. The A user will be considered as an `admin` by ContextMiddleware since Keystone reports its `admin` role for Y tenant because the token belongs to it.

The problem exists in this branch:
commit fc758a46e77de1746c796cd64228694521ee2b07
Author: Dan Prince <email address hidden>
Date: Thu Jun 7 22:23:48 2012 -0400

The same security hole is found in Keystone: an `admin` user is allowed to do anything, e.g. remove users and their roles for any tenant.

Thierry Carrez (ttx) wrote :

Adding PTL for input. I /think/ this is by design though... admins are not scoped to tenants ?

Brian Waldon (bcwaldon) wrote :

Yeah, this should be fixed. We're treating roles as being globally scoped, when they're actually scoped to a specific tenant like you describe.

@Joe: Your take on the keystone side of the vulnerability ?

summary: - glance allows to delete arbitrary images
+ Admin rights escalate to other tenants (was: glance allows to delete
+ arbitrary images)
Joseph Heck (heckj) wrote :

While I agree this should be fixed, it's not a security bug but how the initial version of authorization was implemented.

In the Diablo and Essex releases of OpenStack, Admin was effectively global and not per-tenant or per-service. That's the entire reason of adding in domains to Keystone, and behind the idea of unifying the role names (which are installation-global) to match up with local service policy.json files. (i.e. move to "nova-admin", "glance-admin", etc instead of a single "admin")

If you want a role that's a global admin, you can still use "admin" and create associated policy.json files that respect that identifier.

Joseph Heck (heckj) wrote :

(tl;dr - to be resolved with Keystone V3 API implementation)

Changed in keystone:
status: New → Confirmed
importance: Undecided → High
Thierry Carrez (ttx) wrote :

Brian, Joe: so you both agree this is not a vulnerability, but by (admittedly weak) design ? And that it should definitely be strengthened in future revisions of the API ?

If yes, I'd suggest that we open this bug as a known and wanted security improvement, rather than keep it embargoed as an exploitable vulnerability.

Alessio: would that work for you ?

Thierry Carrez (ttx) on 2012-06-15
Changed in glance:
status: New → Incomplete
Changed in keystone:
status: Confirmed → Incomplete
Alexej Ababilov (aababilov) wrote :

Sorry for the delay!
Would it be acceptable if admin can access any image in owner_is_tenant mode, otherwise image is modifiable only by users of its tenant.

Here is my patch (https://github.com/aababilov/glance/blob/07c7988d1f8c08aebed789a8b9b5d875cef2fb46/glance/db/simple/api.py), however I should fix unit tests as well:

def is_image_mutable(context, image):
    """Return True if the image is mutable in this context."""
    # Is admin and owner is user == image mutable
    if context.is_admin and not context.owner_is_tenant:
        return True

    # No owner == image not mutable
    if image['owner'] is None or context.owner is None:
        return False

    # Image only mutable by its owner
    return image['owner'] == context.owner

(and so on for another functions)

Thierry Carrez (ttx) wrote :

@Brian, Joe: please answer to comment 6 and 7

Joseph Heck (heckj) wrote :

I'm good for opening it up - it's how the roles are implemented and supported by the individual services. The keystone project has a blueprint that Liem (from HP) is working on now to gather up a recommended set of policy.json files and related roles to provide a layout with explicit per-service administration functions. (https://blueprints.launchpad.net/keystone/+spec/document-deployment-suggestions-policy)

For Alessio, I'd recommend making that a rule in policy.json rather than in the code itself, as that makes it configurable by policy rather than hard coded, but I'll defer to whatever Brian suggests here.

Brian Waldon (bcwaldon) wrote :

I agree with Joe here, this is a future improvement, not a bug. However, we do need to be clear with that the implications of assigning a user an admin-like role entails.

As for future work, we need to separate service-level admin vs tenant-level admin. Right now, we create something that looks like a tenant-level admin, when it has the ability to act as if it were service-level.

Thierry Carrez (ttx) on 2012-06-29
summary: - Admin rights escalate to other tenants (was: glance allows to delete
- arbitrary images)
+ Admin rights shouldn't work cross-tenant
security vulnerability: yes → no
visibility: private → public
Changed in glance:
importance: Undecided → Wishlist
status: Incomplete → Confirmed
Changed in keystone:
status: Incomplete → Confirmed
importance: High → Wishlist
tags: added: security
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers