Glance images are loaded into memory

[Stephen Finucane]

Nova appears to be loading entire responses from glance into memory [1]. This is generally not an issue but these responses could be an entire images [2]. Given a large enough image, this seems like a potential avenue for DoS, not to mention being highly inefficient.

[1] https://github.com/openstack/nova/blob/16.0.0/nova/image/glance.py#L167-L170
[2] https://github.com/openstack/nova/blob/16.0.0/nova/image/glance.py#L292-L295

[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.

[Matthew Booth (mbooth-9)]

Already discussed this with Stephen, but for posterity: this bug seems to have been introduced in change Ibc84f159, which was a fix for bug 1557584. We also need to ensure that the fix to this bug doesn't re-break GlanceClientWrapper's retry mechanism.

Ideally, I think we would continue to return an iterator from that method, but we should ensure that at least the initial iteration would hit the retry code. It would be better still if we could switch glance endpoint on hitting a failure half way through a bulk data transfer, but that would require a more substantial change. Glance would have to support partial fetching of an image (does it already?) and we'd have to change the caller to track how much data has been transferred already in case of a failure.

In practice, though, I think it's sufficient that we ensure we're talking to a glance endpoint which is up when we start the transfer. If it goes down during the transfer the operation will fail, which seems acceptable to me.

[Jeremy Stanley (fungi)]

Just to clarify, the described behavior/regression was introduced by openstack/nova commit ae6d868e2f13f90d9f97c982fdbbccdc6fb8b9c9 during the Newton development cycle (and also backported to stable/mitaka and stable/liberty)?

[Matthew Booth (mbooth-9)]

Yes, that's correct.

[Jeremy Stanley (fungi)]

In keeping with recent OpenStack vulnerability management policy changes, no report should remain under private embargo for more than 90 days. Because this report predates the change in policy, the deadline for public disclosure is being set to 90 days from today. If the report is not resolved within the next 90 days, it will revert to our public workflow as of 2020-05-27. Please see http://lists.openstack.org/pipermail/openstack-discuss/2020-February/012721.html for further details.

[Jeremy Stanley (fungi)]

It doesn't look like this report has seen any activity since my update two months ago, so consider this a friendly reminder:

The embargo for this report is due to expire one month from today, on May 27, and will be switched public on or shortly after that day if it is not already resolved sooner.

Thanks!

[Jeremy Stanley (fungi)]

The embargo for this report has expired and is now lifted, so it's acceptable to discuss further in public.

[Stephen Finucane (stephenfinucane)]

I finally got around to investigating this today. tl;dr: there does not appear to be an issue here.

The return value of 'glanceclient.Client.images.data' is 'glanceclient.common.utils.RequestIdProxy', owing to the use of the 'add_req_id_to_object' decorator [2]. This is *not* a generator, which means the 'inspect.isgenerator' conditional at [1] is False and we will never convert these large images to a list. In fact, there appears to be only one case that does trigger this: the 'glanceclient.Client.images.list' case, which returns a 'glanceclient.common.utils.GeneratorProxy' object due to the use of the 'add_req_id_to_generator' decorator. This is the function at the root of bug #1557584. As such, the fix is correct and there's nothing to do here besides possibly documenting things better in the code.

[1] https://github.com/openstack/nova/blob/16.0.0/nova/image/glance.py#L167
[2] https://github.com/openstack/python-glanceclient/blob/3.1.1/glanceclient/v2/images.py#L200
[3] https://github.com/openstack/python-glanceclient/blob/3.1.1/glanceclient/v2/images.py#L85

[sean mooney (sean-k-mooney)]

yep i have checked over Stephens assessment and i believe he is correct in comment 9 above
https://bugs.launchpad.net/nova/+bug/1736920/comments/9