Missing x-tenant-id header to registry will return list of all images while using v2 api with registry

This test has been ran with change: https://review.openstack.org/#/c/87726/

Next step is to reproduce on devstack (and also see if downloading images is affected too).

> /home/ubuntu/glance/glance/tests/functional/v2/test_images.py(463)test_permissions()
-> self.assertEqual(201, response.status_code)
(Pdb) p headers
{'X-Tenant-Id': '31e5f2ec-f365-400f-bff1-b2b8702b24ff', 'X-User-Id': 'f9a41d13-0c13-47e9-bee2-ce4e8bfe958e', 'X-Auth-Token': '932c5c84-02ac-4fe5-a9ba-620af0e2bb96', 'X-Identity-Status': 'Confirmed', 'Content-Type': 'application/json', 'X-Roles': 'member'}
(Pdb) p response, response.text
(<Response [201]>, u'{"status": "queued", "name": "image-1", "tags": [], "container_format": "bare", "created_at": "2014-04-16T13:18:18Z", "disk_format": "raw", "updated_at": "2014-04-16T13:18:18Z", "visibility": "private", "self": "/v2/images/463d8efb-b110-4f58-a270-79b733daec8c", "min_disk": 0, "protected": false, "id": "463d8efb-b110-4f58-a270-79b733daec8c", "file": "/v2/images/463d8efb-b110-4f58-a270-79b733daec8c/file", "owner": "31e5f2ec-f365-400f-bff1-b2b8702b24ff", "min_ram": 0, "schema": "/v2/schemas/image"}')
(Pdb) cont
> /home/ubuntu/glance/glance/tests/functional/v2/test_images.py(471)test_permissions()
-> self.assertEqual(204, response.status_code)
(Pdb) p headers
{'X-Tenant-Id': '31e5f2ec-f365-400f-bff1-b2b8702b24ff', 'X-User-Id': 'f9a41d13-0c13-47e9-bee2-ce4e8bfe958e', 'X-Auth-Token': '932c5c84-02ac-4fe5-a9ba-620af0e2bb96', 'X-Identity-Status': 'Confirmed', 'Content-Type': 'application/octet-stream', 'X-Roles': 'member'}
(Pdb) p response, response.text
(<Response [204]>, u'')
(Pdb) cont
> /home/ubuntu/glance/glance/tests/functional/v2/test_images.py(479)test_permissions()
-> self.assertEqual(image_id, images[0]['id'])
(Pdb) p headers
{'X-Tenant-Id': '31e5f2ec-f365-400f-bff1-b2b8702b24ff', 'X-User-Id': 'f9a41d13-0c13-47e9-bee2-ce4e8bfe958e', 'X-Auth-Token': '932c5c84-02ac-4fe5-a9ba-620af0e2bb96', 'X-Identity-Status': 'Confirmed', 'Content-Type': 'application/octet-stream', 'X-Roles': 'member'}
(Pdb) p response, response.text
(<Response [200]>, u'{"images": [{"status": "active", "name": "image-1", "tags": [], "container_format": "bare", "created_at": "2014-04-16T13:18:18Z", "disk_format": "raw", "updated_at": "2014-04-16T13:18:44Z", "visibility": "private", "self": "/v2/images/463d8efb-b110-4f58-a270-79b733daec8c", "min_disk": 0, "protected": false, "id": "463d8efb-b110-4f58-a270-79b733daec8c", "file": "/v2/images/463d8efb-b110-4f58-a270-79b733daec8c/file", "checksum": "8f113e38d28a79a5a451b16048cc2b72", "owner": "31e5f2ec-f365-400f-bff1-b2b8702b24ff", "size": 5, "min_ram": 0, "schema": "/v2/schemas/image"}], "schema": "/v2/schemas/images", "first": "/v2/images"}')
(Pdb) cont
> /home/ubuntu/glance/glance/tests/functional/v2/test_images.py(485)test_permissions()
-> self.assertEqual(200, response.status_code)
(Pdb) p headers
{'X-Tenant-Id': '31e5f2ec-f365-400f-bff1-b2b8702b24ff', 'X-User-Id': 'f9a41d13-0c13-47e9-bee2-ce4e8bfe958e', 'X-Auth-Token': '932c5c84-02ac-4fe5-a9ba-620af0e2bb96', 'X-Identity-Status': 'Confirmed', 'Content-Type': 'application/octet-stream', 'X-Roles': 'member'}
(Pdb) p response, response.text
(<Response [200]>, u'{"status": "active", "name": "image-1", "tags": [], "container_format": "bare", "created_at": "2014-04-16T13:18:18Z", "disk_format": "raw", "updated_at": "2014-04-16T13:1...

I think the token needs to be different in each case -- can we retry the devstack test with separate tokens? Thanks.

I don't think we've exposed a hole on devstack (yet).

Let's close for now and re-open if/when we are confident we have a reproducer.

The exact behavior is there with devstack. Although I do not think this is bug, but rather design thing on "noauth". When keystone is used the server does not behave same way so at least it's not something generic in glance.

Correction to #6:
There is vulnerability in that behavior.

Summary so far:
The test fails as the tenant ID header is not passed down to Registry server from api/noauth.
Without tenant ID, registy+api combination provides all images from the database.

This is rare case as F.E. Keystone provides the tenant id from the token it authenticates, but missing that header, having corruption on it, someone being able to remove the header from the response OR having any auth module issue causing the header missing would grant access to all images on glance.

We used to send all identity headers in the version 1 of the registry client[0][1], but we don't seem to be doing that anymore[2]. This seems to be the cause of this issue and it kinda sounds bad. The RegistryService (like the API service) creates a context based on the headers passed to it and then passes that to the database[3][4] function. I'll work on a fix for it.

[0] https://github.com/openstack/glance/blob/master/glance/registry/client/v1/api.py#L130-L138
[1] https://github.com/openstack/glance/blob/master/glance/registry/client/v1/client.py#L105
[2] https://github.com/openstack/glance/blob/master/glance/registry/client/v2/api.py

[3] https://github.com/openstack/glance/blob/master/glance/context.py#L97
[4] https://github.com/openstack/glance/blob/master/glance/db/sqlalchemy/api.py#L230

To be more precise. What makes me believe this is a bug is that I don't recall an explicit choice of removing the identity headers. @Mark, @Stuart, do you recall any of this?

I'm moving this back to invalid for now.

My 2 cents...

The bug/security issue as described is not reproducible.

My current understanding, based on Erno's update, is that the v2 registry doesn't 'fail securely' in the sense that if no X-Tenant-Id header is present by the time we get to the v2 registry in the pipeline then all images are returned.

We should verify this related behaviour on devstack by removing keystone auth from the registry pipeline and making a request directly to the v2 registry with no X-Tenant-Id header.

If it reproduces we should enter a new, non-security bug. (Non-security as there is no exploit when using keystone.)

I think the fix is to have the v2 registry return an empty list in the absence of a X-Tenant-Id header rather than all images.
We should look at the v1 registry to see how it behaves in the absence of a X-Tenant-Id header.

I'm not sure making the registry client send headers is what we want -- any user could potentially fake those headers with a curl command direct to the registry.

My point of view to the situation is that Security is based on 3 base components:
1) Authentication: Verifying that the requestor is who it/(s)he is claming to be
2) Authorization: What that instance is allowed to see/do
3) Enforcement: Making sure that the previous is followed

For us the Keystone is taking care of the first part. If for any reason Keystone or what ever is handling our Authentication does not provide us the details needed to fulfill the two last points we should not assume that we will give everything out. After all Glance is responsible for the Authorization and Enforcement.

I do not think the operators of OpenStack clouds would be too pleased if potential bug in our Authentication would expose all images to the requestor. Ofcourse this is decision that has been/needs to be made if we want to default the behavior to allow all and communicated accordingly. Then the behavior should also be consistent regardless if API server is using sqlalchemy or registry as back end. This is not the case currently.

We should also double check that we can't use an unscoped token when keystone is in the pipeline to list all images.

@Stuart

I don't think we should make the registry client send headers. After some more digging I definitely agree there's no security issue here. Keystone's middleware doesn't accept external `X-Tenant-Id` to begin with, which means that whatever we'd send to the registry server could be gathered by the keystone middleware again in the registry side.

I don't think this is a security issue at all. Furthermore, I don't think there's a bug here. AFAICT, it can't be reproduced.

i suspect Flavio meant "Incomplete" rather than "Invalid" in comment #9

OK, so I discussed this with various fellows on the Glance and VMT side. Our take is that the Registry server, as it currently stands, is designed to be operated on a trusted network, which feeds it proper headers. If you don't run Keystone, the header should be set by whatever you use. If you operate in noAuth mode, security should be handled higher in the stack. So this is not an exploitable vulnerability, and shall result in no OSSA.

That said, there certainly seems to be room for improvement here, which would allow the Registry server to be operated over untrusted networks. That includes (but probably is not limited to) defaulting to list less things if you fail to provide it the proper headers. That's security strengthening, and we encourage it in future versions of OpenStack, and shall be developed in the open.

Therefore I suggest that this bug is kept open so that improvements in this area can continue to be discussed. It shall be publicly opened so that everyone can participate in the discussion. Unless someone involved complains, this bug shall therefore be open in a few days.

Closing due no interest.