From 3a80eee8eedcee01e106cc9e217b5075b1057892 Mon Sep 17 00:00:00 2001 From: Brian Waldon Date: Sun, 19 Aug 2012 10:49:59 -0400 Subject: [PATCH] Make max image size upload configurable Add conf option image_size_cap to represent what the internal IMAGE_SIZE_CAP constant used to cover in the v1 API. The default is reduced to 1 TB from 50 PB. Thoroughly test this on image create and update through the v1 API. Fixes bug 1038994 Change-Id: Ie0985b62228f8b28c005a8847049d7b68d9b959a --- etc/glance-api.conf | 7 ++ glance/api/v1/images.py | 68 ++++++++++---------- glance/common/config.py | 3 + glance/common/exception.py | 4 ++ glance/common/utils.py | 14 ++++ glance/tests/unit/test_utils.py | 17 +++++ glance/tests/unit/v1/test_api.py | 106 +++++++++++++++++++++++++++++-- 7 files changed, 177 insertions(+), 42 deletions(-) diff --git a/etc/glance-api.conf b/etc/glance-api.conf index 80679664..9262de11 100644 --- a/etc/glance-api.conf +++ b/etc/glance-api.conf @@ -19,6 +19,13 @@ default_store = file # glance.store.s3.Store, # glance.store.swift.Store, + +# Maximum image size (in bytes) that may be uploaded through the +# Glance API server. Defaults to 1 TB. +# WARNING: this value should only be increased after careful consideration +# and must be set to a value under 8 EB (9223372036854775808). +#image_size_cap = 1099511627776 + # Address to bind the API server bind_host = 0.0.0.0 diff --git a/glance/api/v1/images.py b/glance/api/v1/images.py index 9f4fd7db..05c5c2e3 100644 --- a/glance/api/v1/images.py +++ b/glance/api/v1/images.py @@ -57,12 +57,6 @@ SUPPORTED_PARAMS = glance.api.v1.SUPPORTED_PARAMS SUPPORTED_FILTERS = glance.api.v1.SUPPORTED_FILTERS -# 1 PiB, which is a *huge* image by anyone's measure. This is just to protect -# against client programming errors (or DoS attacks) in the image metadata. -# We have a known limit of 1 << 63 in the database -- images.size is declared -# as a BigInteger. -IMAGE_SIZE_CAP = 1 << 50 - # Defined at module level due to _is_opt_registered # identity check (not equality). default_store_opt = cfg.StrOpt('default_store', default='file') @@ -374,15 +368,6 @@ class Controller(controller.BaseController): image_data = req.body_file - if req.content_length: - image_size = int(req.content_length) - elif 'x-image-meta-size' in req.headers: - image_size = int(req.headers['x-image-meta-size']) - else: - LOG.debug(_("Got request with no content-length and no " - "x-image-meta-size header")) - image_size = 0 - scheme = req.headers.get('x-image-meta-store', CONF.default_store) store = self.get_store_or_400(req, scheme) @@ -391,22 +376,15 @@ class Controller(controller.BaseController): LOG.debug(_("Setting image %s to status 'saving'"), image_id) registry.update_image_metadata(req.context, image_id, {'status': 'saving'}) + + LOG.debug(_("Uploading image data for image %(image_id)s " + "to %(scheme)s store"), locals()) + try: - LOG.debug(_("Uploading image data for image %(image_id)s " - "to %(scheme)s store"), locals()) - - if image_size > IMAGE_SIZE_CAP: - max_image_size = IMAGE_SIZE_CAP - msg = _("Denying attempt to upload image larger than " - "%(max_image_size)d. Supplied image size was " - "%(image_size)d") % locals() - LOG.warn(msg) - raise HTTPBadRequest(explanation=msg, request=req) - location, size, checksum = store.add( image_meta['id'], utils.CooperativeReader(image_data), - image_size) + image_meta['size']) # Verify any supplied checksum value matches checksum # returned from store when adding image @@ -468,6 +446,12 @@ class Controller(controller.BaseController): raise HTTPServiceUnavailable(explanation=msg, request=req, content_type='text/plain') + except exception.ImageSizeLimitExceeded, e: + msg = _("Denying attempt to upload image larger than %d.") + self._safe_kill(req, image_id) + raise HTTPBadRequest(explanation=msg % CONF.image_size_cap, + request=req, content_type='text/plain') + except HTTPError, e: self._safe_kill(req, image_id) self.notifier.error('image.upload', e.explanation) @@ -840,17 +824,29 @@ class ImageDeserializer(wsgi.JSONRequestDeserializer): raise HTTPBadRequest(explanation=msg, request=request) image_meta = result['image_meta'] - if 'size' in image_meta: - incoming_image_size = image_meta['size'] - if incoming_image_size > IMAGE_SIZE_CAP: - max_image_size = IMAGE_SIZE_CAP - msg = _("Denying attempt to upload image larger than " - "%(max_image_size)d. Supplied image size was " - "%(incoming_image_size)d") % locals() - LOG.warn(msg) - raise HTTPBadRequest(explanation=msg, request=request) + if request.content_length: + image_size = request.content_length + elif 'size' in image_meta: + image_size = image_meta['size'] + else: + image_size = None data = request.body_file if self.has_body(request) else None + + if image_size is None and data is not None: + data = utils.limiting_iter(data, CONF.image_size_cap) + + #NOTE(bcwaldon): this is a hack to make sure the downstream code + # gets the correct image data + request.body_file = data + + elif image_size > CONF.image_size_cap: + max_image_size = CONF.image_size_cap + msg = _("Denying attempt to upload image larger than %d.") + LOG.warn(msg % max_image_size) + raise HTTPBadRequest(explanation=msg % max_image_size, + request=request) + result['image_data'] = data return result diff --git a/glance/common/config.py b/glance/common/config.py index f1b62dc2..c7a64851 100644 --- a/glance/common/config.py +++ b/glance/common/config.py @@ -51,6 +51,9 @@ common_opts = [ help=_('Whether to include the backend image storage location ' 'in image properties. Revealing storage location can be a ' 'security risk, so use this setting with caution!')), + cfg.IntOpt('image_size_cap', default=1099511627776, + help=_("Maximum size of image a user can upload in bytes. " + "Defaults to 1099511627776 bytes (1 TB).")), ] CONF = cfg.CONF diff --git a/glance/common/exception.py b/glance/common/exception.py index 0e808d75..a05ac1d7 100644 --- a/glance/common/exception.py +++ b/glance/common/exception.py @@ -248,3 +248,7 @@ class UnsupportedHeaderFeature(GlanceException): class InUseByStore(GlanceException): message = _("The image cannot be deleted because it is in use through " "the backend store outside of Glance.") + + +class ImageSizeLimitExceeded(GlanceException): + message = _("The provided image is too large.") diff --git a/glance/common/utils.py b/glance/common/utils.py index 001b539b..db649337 100644 --- a/glance/common/utils.py +++ b/glance/common/utils.py @@ -125,6 +125,20 @@ class CooperativeReader(object): return cooperative_iter(self.fd.__iter__()) +def limiting_iter(iter, limit): + """ + Iterator designed to fail when reading image data past the configured + allowable amount. + """ + bytes_read = 0 + for chunk in iter: + bytes_read += len(chunk) + if bytes_read > limit: + raise exception.ImageSizeLimitExceeded() + else: + yield chunk + + def image_meta_to_http_headers(image_meta): """ Returns a set of image metadata into a dict diff --git a/glance/tests/unit/test_utils.py b/glance/tests/unit/test_utils.py index ec9859ff..c26484e8 100644 --- a/glance/tests/unit/test_utils.py +++ b/glance/tests/unit/test_utils.py @@ -17,6 +17,7 @@ import tempfile +from glance.common import exception from glance.common import utils from glance.tests import utils as test_utils @@ -70,3 +71,19 @@ class TestUtils(test_utils.BaseTestCase): byte = reader.read(1) self.assertEquals(bytes_read, BYTES) + + def test_limited_iter_reads_succeed(self): + fap = 'abcdefghij' + fap = utils.limiting_iter(fap, 10) + bytes_read = 0 + for chunk in fap: + bytes_read += len(chunk) + self.assertEqual(10, bytes_read) + + def test_limited_iter_reads_fail(self): + fap = 'abcdefghij' + fap = utils.limiting_iter(fap, 9) + for i, chunk in enumerate(fap): + if i == 8: + break + self.assertRaises(exception.ImageSizeLimitExceeded, fap.next) diff --git a/glance/tests/unit/v1/test_api.py b/glance/tests/unit/v1/test_api.py index 58a26355..1215eb97 100644 --- a/glance/tests/unit/v1/test_api.py +++ b/glance/tests/unit/v1/test_api.py @@ -19,6 +19,7 @@ import datetime import hashlib import httplib import json +import StringIO import routes from sqlalchemy import exc @@ -29,16 +30,21 @@ import glance.api.middleware.context as context_middleware import glance.api.common from glance.api.v1 import images from glance.api.v1 import router +import glance.common.config from glance.common import utils import glance.context from glance.db.sqlalchemy import api as db_api from glance.db.sqlalchemy import models as db_models +from glance.openstack.common import cfg from glance.openstack.common import timeutils from glance.registry.api import v1 as rserver +import glance.store.filesystem from glance.tests.unit import base from glance.tests import utils as test_utils +CONF = cfg.CONF + _gen_uuid = utils.generate_uuid UUID1 = _gen_uuid() @@ -2146,10 +2152,9 @@ class TestGlanceAPI(base.IsolatedUnitTest): res = req.get_response(self.api) self.assertEquals(res.status_int, 400) - def test_add_image_size_too_big(self): + def test_add_image_size_header_too_big(self): """Tests raises BadRequest for supplied image size that is too big""" - IMAGE_SIZE_CAP = 1 << 50 - fixture_headers = {'x-image-meta-size': IMAGE_SIZE_CAP + 1, + fixture_headers = {'x-image-meta-size': CONF.image_size_cap + 1, 'x-image-meta-name': 'fake image #3'} req = webob.Request.blank("/images") @@ -2157,10 +2162,47 @@ class TestGlanceAPI(base.IsolatedUnitTest): for k, v in fixture_headers.iteritems(): req.headers[k] = v - req.headers['Content-Type'] = 'application/octet-stream' - req.body = "chunk00000remainder" res = req.get_response(self.api) - self.assertEquals(res.status_int, webob.exc.HTTPBadRequest.code) + self.assertEquals(res.status_int, 400) + + def test_add_image_size_chunked_data_too_big(self): + self.config(image_size_cap=512) + fixture_headers = { + 'x-image-meta-name': 'fake image #3', + 'x-image-meta-container_format': 'ami', + 'x-image-meta-disk_format': 'ami', + 'transfer-encoding': 'chunked', + 'content-type': 'application/octet-stream', + } + + req = webob.Request.blank("/images") + req.method = 'POST' + + req.body_file = StringIO.StringIO('X' * (CONF.image_size_cap + 1)) + for k, v in fixture_headers.iteritems(): + req.headers[k] = v + + res = req.get_response(self.api) + self.assertEquals(res.status_int, 400) + + def test_add_image_size_data_too_big(self): + self.config(image_size_cap=512) + fixture_headers = { + 'x-image-meta-name': 'fake image #3', + 'x-image-meta-container_format': 'ami', + 'x-image-meta-disk_format': 'ami', + 'content-type': 'application/octet-stream', + } + + req = webob.Request.blank("/images") + req.method = 'POST' + + req.body = 'X' * (CONF.image_size_cap + 1) + for k, v in fixture_headers.iteritems(): + req.headers[k] = v + + res = req.get_response(self.api) + self.assertEquals(res.status_int, 400) def test_add_image_zero_size(self): """Tests creating an active image with explicitly zero size""" @@ -2510,6 +2552,58 @@ class TestGlanceAPI(base.IsolatedUnitTest): res = req.get_response(self.api) self.assertEquals(res.status_int, 403) + def test_update_image_size_header_too_big(self): + """Tests raises BadRequest for supplied image size that is too big""" + fixture_headers = {'x-image-meta-size': CONF.image_size_cap + 1} + + req = webob.Request.blank("/images/%s" % UUID2) + req.method = 'PUT' + for k, v in fixture_headers.iteritems(): + req.headers[k] = v + + res = req.get_response(self.api) + self.assertEquals(res.status_int, 400) + + def test_update_image_size_data_too_big(self): + self.config(image_size_cap=512) + + fixture_headers = {'content-type': 'application/octet-stream'} + req = webob.Request.blank("/images/%s" % UUID2) + req.method = 'PUT' + + req.body = 'X' * (CONF.image_size_cap + 1) + for k, v in fixture_headers.iteritems(): + req.headers[k] = v + + res = req.get_response(self.api) + self.assertEquals(res.status_int, 400) + + def test_update_image_size_chunked_data_too_big(self): + self.config(image_size_cap=512) + + # Create new image that has no data + req = webob.Request.blank("/images") + req.method = 'POST' + req.headers['x-image-meta-name'] = 'something' + req.headers['x-image-meta-container_format'] = 'ami' + req.headers['x-image-meta-disk_format'] = 'ami' + res = req.get_response(self.api) + image_id = json.loads(res.body)['image']['id'] + + fixture_headers = { + 'content-type': 'application/octet-stream', + 'transfer-encoding': 'chunked', + } + req = webob.Request.blank("/images/%s" % image_id) + req.method = 'PUT' + + req.body_file = StringIO.StringIO('X' * (CONF.image_size_cap + 1)) + for k, v in fixture_headers.iteritems(): + req.headers[k] = v + + res = req.get_response(self.api) + self.assertEquals(res.status_int, 400) + def test_get_index_sort_name_asc(self): """ Tests that the /images registry API returns list of