[OSSA 2015-003] Glance image leak when in saving state (CVE-2014-9623)

I see bug 1383973 was switched to private security after being public for more than a month. Is the intent to mark one as a duplicate of the other?

@fungi Yes the idea would have been to discuss security implication here as bug 1383973 didn't have any security tags on it.

Also the former bug lack a description that the behavior overcome quotas settings...

> The image in saving state is not taken into account by global quota enforcement.

Just to clarify, are we talking about this quota setting?

 # Set a system wide quota for every user. This value is the total number
 # of bytes that a user can use across all storage systems. A value of
 # 0 means unlimited.
 #user_storage_quota = 0

Thanks

@Stuart: yes, the attack is not mitigated by this setting.
I guess the reasoning is that most likely vulnerable setups would have set this value and it won't protect against this attack.

I'm seeing this when trying to reproduce with the test_images.py script
attached to the bug:

 2014-12-03 16:21:03.387 30059 TRACE glance.api.v1.upload_utils result = fd.read(*args)
 2014-12-03 16:21:03.387 30059 TRACE glance.api.v1.upload_utils File "glance/common/utils.py", line 191, in read
 2014-12-03 16:21:03.387 30059 TRACE glance.api.v1.upload_utils result = self.data.read(i)
 2014-12-03 16:21:03.387 30059 TRACE glance.api.v1.upload_utils AttributeError: 'ResponseIndexable' object has no attribute 'read'

looks like an issue with copy-from.

I'm using juno stable code.
Anyone else get the reproducer to work on juno?

I had to go back to grizzly to get copy-from to work for me.

Any one else had success with copy-from on a later code base?

Confirmed on Grizzly using reproducer.

Based on zhiyan's update to https://bugs.launchpad.net/glance/+bug/1383973 the underlying inability to delete the bits of an image in a saving state still exists.

Hi,

I have done some testing using attached program by admin and normal user(demo).

In case of admin user, if image is deleted while it is in saving state, image data doesn't get deleted even after data is completely uploaded to the backend.

In case of normal user, if image is deleted while it is in saving state, data gets deleted after image upload completes.
In this case once upload is completed it tries to update the image meta-data, but as image is already deleted it raises ImageNotFound exception and cleans the data for that image from the backend.

user_storage_quota is not considered in the above case as the image is deleted. user_storage_quota doesn't considered 'deleted' or 'killed' images for checking the quota size. So if the image is deleted while the uploading is processing and user tries to add new image, the user_storage_quota will not consider the previous image size as it's status is deleted and doesn't throw 'StorageQuotaFull' exception even if quota size is exceeding.

IMO, image deletion should not be allowed when the image is in saving state.
Please let me know your opinion on the same.

Thanks Abhishek's analysis.

>>> IMO, image deletion should not be allowed when the image is in saving state.

IMO, seems it's a easiest solution as to make a limitation/checking on api level for end user. However I'm a little worried if it reduces usability from end user's perspective. And more, from some cross-project cases, image could dead on 'saving' status, e.g. Nova capture image failed/hang on upload step, then end user or client code e.g. Nova, need a way to delete the dead image from Glance.

OK, my understanding is that this is a valid vulnerability, even if the user is not an admin. Let me know if I got it wrong.

Considering the former report does not apply to normal user (see comment #9), the severity is lower than expected: The quota is overran only until the upload is completed and images are cleaned after.
I also did not consider malicious administrator.

Here is impact description draft #1

Title: Glance user storage quota bypass
Reporter: Tushar Patil (NTT)
Products: Glance
Versions: up to 2014.1.3 and 2014.2 version up to 2014.2.1

Description:
Tushar Patil from NTT reported a vulnerability in Glance. By deleting images that are being uploaded, a malicious user can overcome the storage quota and thus may overrun the backend. Images in deleted state are not taken into account by quota and won't be effectively deleted until the upload is completed. Only Glance setups configured with user_storage_quota are affected.

+1 impact description

+1 impact desc

Fix for master trunk proposed at https://review.openstack.org/#/c/144464/

In the future, please do not make any public mention of embargoed (private) security vulnerabilities before their coordinated publication by the OpenStack Vulnerability Management Team in the form of an official OpenStack Security Advisory. This includes discussion of the bug or associated fixes in public forums such as mailing lists, code review systems and bug trackers. Please also avoid private disclosure to other individuals not already approved for access to this information, and provide this same reminder to those who are made aware of the issue prior to publication. All discussion should remain confined to this private bug report, and any proposed fixes should be added as to the bug as attachments.

Since this bug is now mentioned in a public code review change, we should go ahead and end the embargo by switching it from private security to public security.

Oh, I see where the bug description could have been misinterpreted--the expectation on this particular issue (since there's a corresponding public bug which doesn't mention any security implications) was that it could be fixed in public without discussing this private bug report. However the commit message unfortunately mentions both bug numbers rather than just the public bug.

Re @fungi, about bug number mentioning within patch's commit message, I think it's not a big deal due to this report is under private status, improper users couldn't even know the background and deeper affect from security perspective.

Reviewers who see that bug number and attempt to view the resultant bug report will get a message which says "...you may not have permission to see it..." which is generally known to indicate a private bug (especially if they check that the next bug number following it is already allocated). This is then a hint to more closely scrutinize the review and the other public bug report for an indication of why it might indicate a vulnerable condition.

It's not a disaster, just an indication that we should perhaps go ahead and switch this bug to public as well so as to avoid any further confusion.

Have left some comments on https://review.openstack.org/#/c/144464/ ; let's wait for Zhi Yan a bit. If he is away tomorrow, we can fix the commit message from the web UI as needed.

I open this bug since its pretty much already disclosed and I'll request a CVE with impact description from comment #12.

@Tristan: Sounds good.

Reviewed: https://review.openstack.org/144464
Committed: https://git.openstack.org/cgit/openstack/glance/commit/?id=0dc8fbb3479a53c5bba8475d14f4c7206904c5ea
Submitter: Jenkins
Branch: master

commit 0dc8fbb3479a53c5bba8475d14f4c7206904c5ea
Author: Zhi Yan Liu <email address hidden>
Date: Tue Dec 30 22:25:50 2014 +0800

    Cleanup chunks for deleted image that was 'saving'

    Currently image data cannot be removed synchronously for an image that
    is in saving state. And when, the upload operation for such an image is
    completed the operator configured quota can be exceeded.

    This patch fixes the issue of left over chunks for an image which was
    deleted from saving status. However, by the limitation of the design we
    cannot enforce a global quota check for the image in saving status.

    This change introduces a inconsonance between http response codes of
    v1 and v2 APIs. The status codes which we will now see after the upload
    process completes on an image which was deleted mid way are:

    v1: 412 Precondition Failed
    v2: 410 Gone

    SecurityImpact
    UpgradeImpact
    APIImpact

    Closes-Bug: 1383973
    Closes-Bug: 1398830
    Closes-Bug: 1188532

    Change-Id: I47229b366c25367ec1bd48aec684e0880f3dfe60
    Signed-off-by: Zhi Yan Liu <email address hidden>

Fix proposed to branch: stable/juno
Review: https://review.openstack.org/149387

Reviewed: https://review.openstack.org/149387
Committed: https://git.openstack.org/cgit/openstack/glance/commit/?id=7d5d8657fd70b20518610b3c6f8e41e16c72fa31
Submitter: Jenkins
Branch: stable/juno

commit 7d5d8657fd70b20518610b3c6f8e41e16c72fa31
Author: Zhi Yan Liu <email address hidden>
Date: Tue Dec 30 22:25:50 2014 +0800

    Cleanup chunks for deleted image that was 'saving'

    Currently image data cannot be removed synchronously for an image that
    is in saving state. And when, the upload operation for such an image is
    completed the operator configured quota can be exceeded.

    This patch fixes the issue of left over chunks for an image which was
    deleted from saving status. However, by the limitation of the design we
    cannot enforce a global quota check for the image in saving status.

    This change introduces a inconsonance between http response codes of
    v1 and v2 APIs. The status codes which we will now see after the upload
    process completes on an image which was deleted mid way are:

    v1: 412 Precondition Failed
    v2: 410 Gone

    SecurityImpact
    UpgradeImpact
    APIImpact

    Closes-Bug: 1383973
    Closes-Bug: 1398830
    Closes-Bug: 1188532

    Conflicts:
     glance/api/v1/upload_utils.py
     glance/api/v2/image_data.py
     glance/tests/unit/test_domain_proxy.py
     glance/tests/unit/v1/test_api.py

    Change-Id: I47229b366c25367ec1bd48aec684e0880f3dfe60
    Signed-off-by: Zhi Yan Liu <email address hidden>
    (cherry picked from commit 0dc8fbb3479a53c5bba8475d14f4c7206904c5ea)

Change abandoned by nikhil komawar (<email address hidden>) on branch: stable/icehouse
Review: https://review.openstack.org/149397
Reason: https://review.openstack.org/#/c/149646

(confirming) stable/icehouse: https://review.openstack.org/#/c/149646

Reviewed: https://review.openstack.org/149646
Committed: https://git.openstack.org/cgit/openstack/glance/commit/?id=f1260cc771ee068651aa62b972bef49d9af81eb0
Submitter: Jenkins
Branch: stable/icehouse

commit f1260cc771ee068651aa62b972bef49d9af81eb0
Author: Zhi Yan Liu <email address hidden>
Date: Tue Dec 30 22:25:50 2014 +0800

    Cleanup chunks for deleted image that was 'saving'

    Currently image data cannot be removed synchronously for an image that
    is in saving state. And when, the upload operation for such an image is
    completed the operator configured quota can be exceeded.
    This patch fixes the issue of left over chunks for an image which was
    deleted from saving status. However, by the limitation of the design we
    cannot enforce a global quota check for the image in saving status.
    This change introduces a inconsonance between http response codes of
    v1 and v2 APIs. The status codes which we will now see after the upload
    process completes on an image which was deleted mid way are:

    v1: 412 Precondition Failed
    v2: 410 Gone

    SecurityImpact
    UpgradeImpact
    APIImpact

    Closes-Bug: 1383973
    Closes-Bug: 1398830
    Closes-Bug: 1188532

    Conflicts:
            glance/api/v1/upload_utils.py
            glance/api/v2/image_data.py
            glance/quota/__init__.py
            glance/tests/unit/test_domain_proxy.py
            glance/tests/unit/test_quota.py
            glance/tests/unit/v1/test_api.py

    Signed-off-by: Zhi Yan Liu <email address hidden>
    (cherry picked from commit 0dc8fbb3479a53c5bba8475d14f4c7206904c5ea)

    Change-Id: I47229b366c25367ec1bd48aec684e0880f3dfe60