1.7.4 keystone middleware allows operator_roles to delete accounts

Bug #1177526 reported by Alejandro Comisario
10
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Identity (keystone)
Invalid
Undecided
Unassigned
Essex
Invalid
Undecided
Unassigned
OpenStack Object Storage (swift)
Fix Released
Undecided
Chmouel Boudjnah

Bug Description

Hi, we are using swift 1.7.4 with keystone auth, and we think we might found a bug.
Our proxy-server.conf for kesytone is as follow :

[filter:keystoneauth]
use = egg:swift#keystoneauth
operator_roles = admin, swiftoperator
is_admin = true

And every user that has one of the operator_roles roles, are able to directly delete an account despite it has or not containers/objects.

As long as we understood, only the roles contained in reseller_admin_role are able to delete accounts despite there is data in it or not.

information type: Private Security → Public Security
Revision history for this message
Thierry Carrez (ttx) wrote :

I'm not sure I would count that as a vulnerability...

Let's see what the Swift devs say about it, it might just be a misconception due to lack of documentation.

Revision history for this message
Alejandro Comisario (alejandro-comisario) wrote :

Thierry, we query into de IRC channel, and swifterdarrel, creiht & clayg encourage us to issue this bug since they think it is a bug, hoping for chmouel lead us the way.

kindest regards

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

Great, adding swift-core for comment and impact confirmation.

Revision history for this message
Chmouel Boudjnah (chmouel) wrote :

Alejandro, you are indeed correct people in operator_roles should not be able to issue a DELETE on the account, but i can't seem to reproduce this as I am getting a 405 when trying to do that, can you please show us how do you do that ? :

# Making sure the admin has the admin role
stack@devstack:~/devstack$ keystone user-role-list --user admin
+----------------------------------+-------+----------------------------------+----------------------------------+
| id | name | user_id | tenant_id |
+----------------------------------+-------+----------------------------------+----------------------------------+
| 06e77ba20bc0406e91da2b01a87e84c3 | admin | 058d18862233418bb55b0fee730e22e9 | 19cae1368377431bb6a3bb610b9ec37e |
+----------------------------------+-------+----------------------------------+----------------------------------+
stack@devstack:~/devstack$ keystone role-get 06e77ba20bc0406e91da2b01a87e84c3
+----------+----------------------------------+
| Property | Value |
+----------+----------------------------------+
| id | 06e77ba20bc0406e91da2b01a87e84c3 |
| name | admin |
+----------+----------------------------------+

# operator_roles has admin as role to authorize
stack@devstack:~/devstack$ grep -w operator_roles /etc/swift/proxy-server.conf
operator_roles = Member, admin

# Using http://p.chmouel.com/ks script, getting a token for admin user
stack@devstack:~/devstack$ ks -s localhost admin:admin ADMIN
TOKEN='4a22807037e1465a8c3c0918fca62a47'
STORAGE_URL='http://46.231.128.140:8080/v1/AUTH_19cae1368377431bb6a3bb610b9ec37e'
KEYSTONE_URL='http://localhost:5000/v2.0/tokens'
# curl -H "X-Auth-Token: ${TOKEN}" ${STORAGE_URL}

# Trying to do a delete of the account
stack@devstack:~/devstack$ curl -v -X DELETE -H "X-Auth-Token: ${TOKEN}" ${STORAGE_URL}
* About to connect() to 46.231.128.140 port 8080 (#0)
* Trying 46.231.128.140... connected
> DELETE /v1/AUTH_19cae1368377431bb6a3bb610b9ec37e HTTP/1.1
> User-Agent: curl/7.22.0 (x86_64-pc-linux-gnu) libcurl/7.22.0 OpenSSL/1.0.1 zlib/1.2.3.4 libidn/1.23 librtmp/2.3
> Host: 46.231.128.140:8080
> Accept: */*
> X-Auth-Token: 4a22807037e1465a8c3c0918fca62a47
>
< HTTP/1.1 405 Method Not Allowed
< Content-Length: 91
< Content-Type: text/html; charset=UTF-8
< Allow: HEAD, GET, POST, OPTIONS
< X-Trans-Id: tx408c29ef0e0940dea6f04-005190e04a
< Date: Mon, 13 May 2013 12:44:58 GMT
<
* Connection #0 to host 46.231.128.140 left intact
* Closing connection #0
<html><h1>Method Not Allowed</h1><p>The method is not allowed for this resource.</p></html>stack@devstack:~/devstack$

Thierry Carrez (ttx)
Changed in swift:
status: New → Incomplete
Revision history for this message
Alejandro Comisario (alejandro-comisario) wrote :
Download full text (5.1 KiB)

Thanks for the quick response Chmouel.
Heres how we miss-achieve the deletion of and account being in the operator_roles.
Again, we are using SWIFT 1.7.4 (Folsom release) with the essex keystone middleware 2012.1.4 since we are hiting a keystone/essex service.

1 - Our proxy-server.conf file, were you can see in the keystoneauth section, the operator_roles line composed by admin, and swiftoperator roles.

cat /etc/swift/proxy-server.conf

[DEFAULT]
bind_port = 8080
workers = 16
user = swift
log_name = swift-proxy-server
log_facility = LOG_LOCAL0
log_level = DEBUG
log_headers = True
log_address = /dev/log

[pipeline:main]
pipeline = catch_errors healthcheck cache authtoken keystoneauth proxy-logging proxy-server

[app:proxy-server]
use = egg:swift#proxy
allow_account_management = true
account_autocreate = true
set log_name = swift-proxy-server
set log_facility = LOG_LOCAL0
set log_level = DEBUG
set access_log_name = swift-proxy-server
set access_log_facility = LOG_LOCAL0
set access_log_level = DEBUG
set log_headers = True

[filter:healthcheck]
use = egg:swift#healthcheck

[filter:catch_errors]
use = egg:swift#catch_errors

[filter:cache]
use = egg:swift#memcache
set log_name = cache
memcache_servers = 172.16.177.253:11211,172.16.177.254:11211

[filter:keystoneauth]
use = egg:swift#keystoneauth
operator_roles = admin, swiftoperator
is_admin = true

[filter:authtoken]
paste.filter_factory = keystone.middleware.auth_token:filter_factory
service_protocol = http
service_host = essexkeystone.melicloud.com
service_port = 5000
auth_protocol = http
auth_host = essexkeystone.melicloud.com
auth_port = 35357
admin_tenant_name = swift
admin_user = swiftAdmin
admin_password = xxxxxxxxx
delay_auth_decision = 1
token_cache_time = 43200
memcache_servers = 172.16.177.253:11211,172.16.177.254:11211

[filter:proxy-logging]
use = egg:swift#proxy_logging

2 - In the token and user section from the result of a "get token" operation you can see that the user mvenesio has the swiftoperator role, as well as the swift url at the endpoints section.

            {
                "endpoints": [
                    {
                        "adminURL": "http://172.16.1.84:8080/",
                        "internalURL": "http://172.16.1.84:8080/v1/AUTH_1bf1f1b69a864abb84ed8a1bc82cff21",
                        "publicURL": "http://172.16.1.84:8080/v1/AUTH_1bf1f1b69a864abb84ed8a1bc82cff21",
                        "region": "SwiftRegion"
                    }
                ],
                "endpoints_links": [],
                "name": "swift",
                "type": "object-store"
            },
        ],
        "token": {
            "expires": "2013-05-14T16:53:22Z",
            "id": "8ff060e2e2d54cfc97602692096d5e98",
            "tenant": {
                "description": null,
                "enabled": true,
                "id": "1bf1f1b69a864abb84ed8a1bc82cff21",
                "name": "cloudbuilders"
            }
        },
        "user": {
            "id": "28db445c87aa48cea5b7b33cd9c18adf",
            "name": "mvenesio",
            "roles": [
                {
                    "id": "6626d07a39e7415fbad7d51d99b130a8",
           ...

Read more...

Revision history for this message
Chmouel Boudjnah (chmouel) wrote :

Ah I didn't realize you are using essex keystone middleware, this should be affected to keystone then since it was belonging there by the time of essex.

affects: swift → keystone
Revision history for this message
Chmouel Boudjnah (chmouel) wrote :

I can confirm this is reproducibe with essex :

$ curl -X DELETE -H 'X-Auth-Token: d10f07ca70514c0b9d2fc4ffed2a1e3d' http://46.231.128.140:8080/v1/AUTH_297272c5c1214887a5081d59df978da1
$ curl -H 'X-Auth-Token: d10f07ca70514c0b9d2fc4ffed2a1e3d' http://46.231.128.140:8080/v1/AUTH_297272c5c1214887a5081d59df978da1
<html>
 <head>
  <title>500 Internal Server Error</title>
 </head>
 <body>
  <h1>500 Internal Server Error</h1>
  The server has either erred or is incapable of performing the requested operation.<br /><br />

 </body>

swift ERROR Unhandled exception in request:
Traceback (most recent call last):
  File "/opt/stack/swift/swift/proxy/server.py", line 1888, in handle_request
    return handler(req)
  File "/opt/stack/swift/swift/proxy/server.py", line 93, in wrapped
    return func(*a, **kw)
  File "/opt/stack/swift/swift/proxy/server.py", line 645, in GET
    return self.GETorHEAD(req)
  File "/opt/stack/swift/swift/proxy/server.py", line 1629, in GETorHEAD
    self.account_name)

Revision history for this message
Alejandro Comisario (alejandro-comisario) wrote :

Ok so, now is within the keystone scope ??
Should we fix it ourselves? or wait for a patch / backport ?

Since the keystone middleware is essex, but the swift version is folsom.
Cheers

Revision history for this message
Chmouel Boudjnah (chmouel) wrote :

Hi Alexjandro, Looking more to reproduce this, it actually affect as well swift keystoneauth in master.

It only allow the user to delete its own account not the others.
It only works when enabling the setting allow_account_management to true.

I will be issuing a fix for this but I don't know if this should be considered as a security issue since it really affect its own user, i let the other to decide.

Revision history for this message
Chmouel Boudjnah (chmouel) wrote :

FYI: this is documented as such in https://github.com/openstack/swift/blob/master/etc/proxy-server.conf-sample#L65 but it seems that tempauth only allow deleting account for user who has .reseller_admin group.

Revision history for this message
Alejandro Comisario (alejandro-comisario) wrote :

Perfect, while this matter is decided, do you advice us to configure :

allow_account_management = false
and set explicitly the roles for reseller_admin ??

if that's ok.
Cause we dont want a normal user to delete its own account, but yes, the possibility for us to delete one if we need to.

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

Agreed with Chmouel in comment 9 (not a vulnerability), will open this bug publicly tomorrow if nobody complains

Changed in swift:
status: New → Incomplete
Changed in keystone:
status: Incomplete → Invalid
no longer affects: swift/essex
Revision history for this message
Chmouel Boudjnah (chmouel) wrote :

well you need allow_account_management = true to delete an account even with a reseller account token.

it's definitively a bug to allow a user to delete their own account (but not a security bug).

 I am going to fix this in master, feel free to backport it for essex.

Thanks.

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

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

Changed in swift:
assignee: nobody → Chmouel Boudjnah (chmouel)
status: Incomplete → In Progress
Thierry Carrez (ttx)
information type: Public Security → Public
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to swift (master)

Reviewed: https://review.openstack.org/29212
Committed: http://github.com/openstack/swift/commit/6f722f7320c42415407587974ac9100a2cb8d08f
Submitter: Jenkins
Branch: master

commit 6f722f7320c42415407587974ac9100a2cb8d08f
Author: Chmouel Boudjnah <email address hidden>
Date: Wed May 15 10:48:31 2013 +0200

    Don't allow users to delete their own account.

    - In keystoneauth we allowed authenticated users to delete their own
      account we are disallowing that and only allow users with reseller
      admin to do that for its own or for the others.
    - Fixes bug 1177526.

    Change-Id: I825c5a968e8eae0991915056825fe0e0c195647e

Changed in swift:
status: In Progress → Fix Committed
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to swift (feature/ec)

Fix proposed to branch: feature/ec
Review: https://review.openstack.org/38552

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to swift (feature/ec)
Download full text (15.3 KiB)

Reviewed: https://review.openstack.org/38552
Committed: http://github.com/openstack/swift/commit/f3ba55039c4caa91b71b453f2dc1f5310c2ac801
Submitter: Jenkins
Branch: feature/ec

commit f2c3e1af9cf0ad1fbf7753cd2e5caef9bd297cb5
Author: Samuel Merritt <email address hidden>
Date: Mon Jul 22 16:14:59 2013 -0700

    Fix bulk's unit tests on Mac OS.

    Turns out if your $TMPDIR is really long
    (e.g. /var/folders/vq/n32yszdn4s76z6l8dxklmwdh0000gn/T/, which is 50
    characters), then the test that drops stuff into $TMPDIR and adds it
    to a tarball and uploads it will fail due to every added file's name
    being too long.

    Change-Id: I8fcab2d95091f72b831b906bfc87a36d8be12631

commit 5c1a7871d9173db9fbd855b72a98ecd8ff163800
Author: Newptone <email address hidden>
Date: Sun Jul 21 12:18:24 2013 +0800

    Unified format of boolean params in conf files

    In swift conf files, boolean options use different
    format: some use true/false, and some use True/False.
    This patch is aim to using lowcase true/false to unify
    boolean params formats in swift conf files.

    Fix Bug #1203421

    Change-Id: I3e1bfc6e43231f51e0710aa54869f3774ee896b1

commit 63061e37ed098bf1ad509177484037544eb6b089
Author: Kun Huang <email address hidden>
Date: Tue Jul 23 13:46:51 2013 +0800

    Add notes for /srv/node in swift-object-info

    'devices' is set in object-server.conf on each node, not in ring data,
    and the things printed here is just for watching not for running, so
    just leave a note here. (this https://review.openstack.org/#/c/23951/
    is used for running, so just a note is not enough)

    mark this commit as bug fixing is because this script is the last place
    using /srv/node but not from conf as Chmouel said.

    fixes import change on read_metadata
    fixes bug #885006
    Change-Id: I727ec2d01c093af61fd3895e5701d87ef67cd9ff

commit 13bbe4b7c31b17efbebd91bd01586757600a082b
Author: Michael Barton <email address hidden>
Date: Mon Jul 22 21:12:22 2013 -0700

    fix unit tests in 2.6 by using closing(GzipFile)

    Python 2.6 doesn't support using GzipFile as a context manager.

    Change-Id: Ic12b5a916bc89ae8d4480879723201c1715285af

commit 6384b192b55b823d86078588f22b8bc847954aad
Author: Alex Gaynor <email address hidden>
Date: Mon Jul 22 14:59:30 2013 -0700

    Ensure that files are always closed explicitly.

    This is needed on Pythons without reference
    counting garbage collectors (e.g. PyPy).

    Change-Id: Ieb563ace9f65a4ad204b01be32bf7a9d5f226005

commit 0e3103c0dd3690f5e69ec7953c270430d0564d12
Author: Alex Gaynor <email address hidden>
Date: Mon Jul 22 15:27:54 2013 -0700

    Corrected a number of style violations in the tests.

    Change-Id: Ib5e81ad0476c56cf84d222d67f55b8db3eb0249e

commit fc293a750c8a83ec99d3b462b4689741c1e66a9b
Author: Peter Portante <email address hidden>
Date: Wed Jul 17 17:41:41 2013 -0400

    Move the mount checking into DiskFile constructor

    Move the mount checking into the DiskFile object constructor. The
    REPLICATE method does not use the DiskFile object currently, so that
    method st...

Thierry Carrez (ttx)
Changed in swift:
milestone: none → 1.9.1
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

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