User can delete deactivated images

[Niall Bunting]

Overview:

In glance once an admin has marked a image as deactivated a user can no longer download or delete that image. This is so an image can be inspected by the admins without the user interfering.
However, these restrictions can be avoided specifically allowing a user to delete a deactivated image. Meaning an admin would not be able to guarantee the status of a deactivated image.

What should happen: 403 What does happen: 200

How to reproduce:
1. Create an image.
echo test | glance image-create --name 3 --container-format bare --disk-format raw

2. Deactivate the image.
glance image-deactivate 0630d5e4-6009-4723-94e6-1ad056ab649a

3. Check image is deactivated.
glance image-show 0630d5e4-6009-4723-94e6-1ad056ab649a

4. Using the v1 API delete the image.
curl -X DELETE http://localhost:9292/v1/images/0630d5e4-6009-4723-94e6-1ad056ab649a -H 'X-Auth-token: 108322e43f6346ebadb3c2fb72831913'

5. Image is now gone.
glance image-show 0630d5e4-6009-4723-94e6-1ad056ab649a

[Tristan Cacqueray (tristan-cacqueray)]

Since this report concerns a possible security risk, an incomplete security advisory task has been added while the core security reviewers for the affected project or projects confirm the bug and discuss the scope of any vulnerability along with potential solutions.

Can the user delete an image deactivated by the admin ?

[Stuart McLaren (stuart-mclaren)]

Our default policy seems to allow users to deactivate their own images. (I'm a bit surprised -- I thought that would have been an admin only operation.)

Can you try updating the policy (role:admin) and trying again.

Also, can you compare v1 and v2 behaviour? Thanks.

[Niall Bunting (niall-bunting)]

Updated the policy on my machine and still works. It seems to happen on both v1 and v2.

[Niall Bunting (niall-bunting)]

Can the user delete an image deactivated by the admin ? - Yup i think that is the case

[Stuart McLaren (stuart-mclaren)]

Thanks Niall.

Good catch.

I don't think you should be able to delete an image that an admin has deactivated before they've had a chance to download/ inspect it.

[Niall Bunting (niall-bunting)]

This patch fixes the problem by checking the image is in the deactivated state. It also allows admins to continue o download these images.

[Stuart McLaren (stuart-mclaren)]

Thanks Niall.

Couple of minor comments:

Why does the v2 admin test return HTTPBadRequest?

v1 has log level warn, v2 has debug - they should probably be the same. (The msg might as well be the same also.)

[Niall Bunting (niall-bunting)]

This contains a patch that addresses the issues raised by Stuart.

[Grant Murphy (gmurphy)]

First draft of impact description -

Title: Lack of ACL on deactivated image deletion request
Reporter: Niall Bunting (HPE)
Products: Glance
Affects: >=2015.1.0

Description:
Niall Bunting of Hewlett Packard Enterprise (HPE) reported a
vulnerability in Glance. Due to a failure to properly restrict
access controls a user may delete images that have been deactivated
by an administrator. A tenant may abuse this flaw to hide malicious
activities from an administrator. All Glance deployments are affected.

[Stuart McLaren (stuart-mclaren)]

Note that this behaviour is implemented as described in the relevant spec.

That said, I wonder if we should consider changing the behaviour to prevent deletion.

[Niall Bunting (niall-bunting)]

Just to add to what Stuart said it is here: https://github.com/openstack/glance-specs/blame/master/specs/kilo/deactivate-image.rst#L36-L37

However I personally consider it a design flaw.

[Stuart McLaren (stuart-mclaren)]

Seeing the behaviour is as per the (public) spec we can probably make this bug publically viewable.

[Tristan Cacqueray (tristan-cacqueray)]

Grant, I think the affect line should be "<= 2015.1.2, ==11.0.0" (because after the fix is merged, the proposed statement won't be true). Also it is worth noting that user can only delete deactivated images he owns right ?

I agree with Niall, it's a design flaw since the intent of that spec is to 'put the image "on hold" preventing instances from being built with it until it has be properly examined'. If the user can delete and prevent examination, then it's an issue.

However I also think this could be made public and fixed on gerrit directly.

[Niall Bunting (niall-bunting)]

@Tristan Yeah they can only delete their own images.

[OpenStack Infra (hudson-openstack)]

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

[Brian Rosmaita (brian-rosmaita)]

Just to provide some context here ...

The 'deactivate' command was designed as a means of preventing consumption of an image someone had reported as malicious while an operator investigated the image. If the user deletes the image, this accomplishes that goal.

Someone remarked somewhere that this is like flushing the drugs down the toilet when the police knock on the door, and there's something to that point of view, but in this case an administrator could download a copy of the image for analysis either before or quickly after deactivation, if it's important to preserve the image bits.

Part of the reason for allowing image operations such as modifying metadata or sharing while an image is 'deactivated' is that we didn't want to make image deactivation equivalent to an accusation that the image owner was intentionally doing something malicious; we just wanted a way to put the image in suspended animation while an accusation was being looked into.

So I don't see this as a bug as much as something for an administrator to be aware of. As Stuart pointed out somewhere above, the spec never said that the image owner would not be allowed to delete the image. In fact, I'd argue that it's the right of the owner of an image to be able to delete it.

[Stuart McLaren (stuart-mclaren)]

> I'd argue that it's the right of the owner of an image to be able to delete it.

Seems reasonable, but:

From policy.json:

    "delete_image": "not user_id:pleadthefifth",

admins can already prevent users deleting images via policy.

[Brian Rosmaita (brian-rosmaita)]

Sure, but that's not the default, and it's configurable.

My point is simply that this is not a bug because it's reasonable behavior that is in accord with the image deactivation spec.

[Niall Bunting (niall-bunting)]

"The 'deactivate' command was designed as a means of preventing consumption of an image someone had reported as malicious while an operator investigated the image. If the user deletes the image, this accomplishes that goal."

But the secondary goal is to investigate this is not accomplished. Therefore the user with a copy of the image could just upload it again and you would be non the wiser.

"Someone remarked somewhere that this is like flushing the drugs down the toilet when the police knock on the door, and there's something to that point of view, but in this case an administrator could download a copy of the image for analysis either before or quickly after deactivation, if it's important to preserve the image bits."

This assumes that the admin has the ability to download a copy of the image, when he notices a problem and deactivates it.

"Part of the reason for allowing image operations such as modifying metadata or sharing while an image is 'deactivated' is that we didn't want to make image deactivation equivalent to an accusation that the image owner was intentionally doing something malicious; we just wanted a way to put the image in suspended animation while an accusation was being looked into."

Well the user cannot use the image to create new instances, that to me looks like the image is suspected of being malicious as its pretty useless. Then they can delete it so you cannot even look into the accusation at all.

"So I don't see this as a bug as much as something for an administrator to be aware of. As Stuart pointed out somewhere above, the spec never said that the image owner would not be allowed to delete the image. In fact, I'd argue that it's the right of the owner of an image to be able to delete it."

In my opinion the spec features a design flaw so the fact that this does not match makes sense. Why should a owner have the right to flush their drugs down the toilet an you put it?

[Niall Bunting (niall-bunting)]

"If we decide that we want to make this change, I'd argue that (1) it's an enhancement, and (2) it needs to be controlled by either configuration or policy, because allowing deletion of a deactivated image is up to the deployer, not up to Glance. For example, depending upon how billing is done in a cloud, a deployer may not want it to be the case that a user is being charged for an image that the user cannot use."

In regards to this, If a user is being billed for an image they cannot use I think that is a problem with the billing rather than making sure they can get rid of potentially malicious images.

[Brian Rosmaita (brian-rosmaita)]

These are all reasonable points of view, but the key point is that it's not a bug. The point of image deactivation was to give operators a way to disable builds from an image while the image was being investigated, and that's exactly what happens.

If an admin wants to prevent deletion, or wants to retain a copy of the image, that's possible by various other means. So I still think that this is an enhancement request, not a bug, and I'm not convinced that it's an enhancement that every deployer would want, so I think it should be configurable.

[Jeremy Stanley (fungi)]

On the point of "this bug allows the user to delete the malicious disabled image and then upload a replacement," what's to stop them from uploading a replacement under a new image name anyway even with this "bug" fixed?

[Niall Bunting (niall-bunting)]

I agree Brian this should probably be configurable if people hold different opinions. I think it would be beneficial to have this option.

[Tristan Cacqueray (tristan-cacqueray)]

Until a clear consensus about whenever this bug caused an actual security vulnerability, the OSSA task is now Won't Fix.