Comment 30 for bug 1546507

Revision history for this message
Mike Fedosin (mfedosin) wrote : Re: Regular user can delete any image file

Hey! I was busy last week, so didn't pay to much attention for this bug... I think I should be more precise here.

First of all, that's for sure that operator can disable setting custom locations with policies. And it was the first thing I tried to do. Unfortunately it leads to the fact that several tempest tests fail - all of them from Nova:
tempest.api.compute.images.test_images.ImagesTestJSON.test_create_image_from_stopped_server
tempest.api.compute.images.test_list_image_filters.ListImageFiltersTestJSON
tempest.api.compute.images.test_images_oneserver.ImagesOneServerTestJSON.test_create_delete_image
tempest.api.compute.images.test_images_oneserver.ImagesOneServerTestJSON.test_create_image_specify_multibyte_character_image_name
tempest.api.compute.servers.test_server_actions.ServerActionsTestJSON.test_create_backup
tempest.api.compute.servers.test_delete_server.DeleteServersTestJSON.test_delete_server_while_in_shelved_state
tempest.scenario.test_snapshot_pattern.TestSnapshotPattern.test_snapshot_pattern
tempest.scenario.test_shelve_instance.TestShelveInstance.test_shelve_instance
tempest.api.compute.servers.test_servers_negative.ServersNegativeTestJSON.test_shelve_shelved_server
tempest.api.compute.servers.test_server_actions.ServerActionsTestJSON.test_shelve_unshelve_server

It made me say that disabling custom locations breaks Nova. All of these tests fail only for Ceph deployments, Swift pass.

My second though was to add requirement of uniqueness for locations, but it doesn't work (at least for ceph), because there're several ways to define url for an image. For example, "rbd://647f7ae8-648a-44f5-83ad-f7bd2299274e/images/2e4b6dca-9700-4715-b81d-4463cd7038de/snap" and "rbd://2e4b6dca-9700-4715-b81d-4463cd7038de" are equal. Adding workarounds for these cases with regexp is not an option, I suppose.

My final thought was to restrict what type of locations user may set - at first it was only http(s), but then I found a pattern, where each glance location contains image identifier. It allowed to smooth the requirement and also permit users to set locations that contain original image id.

Let's see how it works:
1. Assume we have good public image with id "good-public-image". Image file has a link "rbd://good-public-image"
2. Attacker wants to destroy the data and creates his own private image with id "malicious-image".
3. Then he tries to set custom location to his image: "rbd://good-public-image", but it fails, because this url doesn't contain id of his own private image, i.e. "malicious-image".

About the solution - it completely eliminates the possibility of this type of attack, but adds some restrictions for honest users. I think we can consider it as a good workaround, but in Newton we have to define a better one. I suppose we can implement a solution that was proposed for Glare service - define what type of locations (custom or not) we have. You can read about it here https://review.openstack.org/#/c/283136/2/specs/newton/glare-api.rst LL 136-142.