To prevent client use v2 patch api to handle file and swift location

The change will be used to restrict client to download and delete any
file in glance-api server. The same resone and logic as what we did in
v1:
https://github.com/openstack/glance/blob/master/glance/api/v1/images.py#L429

Closes-Bug: bug/1400966
DocImpact

Note: Even this change could fully resolve the problem for Glance, but
we still need to fix this issue from glance_store perspective
separatelly due to other projects can use the lib directly.

Change-Id: I72dbead3cb2dcb87f52658ddb880e26880cc229b
Signed-off-by: Zhi Yan Liu <zhiyanl@cn.ibm.com>

glance/api/v1/images.py

@@ -25,7 +25,6 @@
 from oslo.config import cfg
 from oslo.utils import excutils
 from oslo.utils import strutils
-import six.moves.urllib.parse as urlparse
 from webob.exc import HTTPBadRequest
 from webob.exc import HTTPConflict
 from webob.exc import HTTPForbidden
@@ -42,6 +41,7 @@
 from glance.api.v1 import upload_utils
 from glance.common import exception
 from glance.common import property_utils
+from glance.common import store_utils
 from glance.common import utils
 from glance.common import wsgi
 from glance import i18n
@@ -416,26 +416,19 @@ def meta(self, req, id):
     @staticmethod
     def _validate_source(source, req):
         """
-        External sources (as specified via the location or copy-from headers)
-        are supported only over non-local store types, i.e. S3, Swift, HTTP.
-        Note the absence of 'file://' for security reasons, see LP bug #942118.
-        'swift+config://' is also absent for security reasons, see LP bug
-        #1334196.
-        If the above constraint is violated, we reject with 400 "Bad Request".
+        To validate if external sources (as specified via the location
+        or copy-from headers) are supported. Otherwise we reject
+        with 400 "Bad Request".
         """
         if source:
-            pieces = urlparse.urlparse(source)
-            schemes = [scheme for scheme in store.get_known_schemes()
-                       if scheme != 'file' and scheme != 'swift+config']
-            for scheme in schemes:
-                if pieces.scheme == scheme:
-                    return source
-            msg = ("External sourcing not supported for "
-                   "store '%s'" % pieces.scheme)
-            LOG.debug(msg)
-            raise HTTPBadRequest(explanation=msg,
-                                 request=req,
-                                 content_type="text/plain")
+            if store_utils.validate_external_location(source):
+                return source
+            else:
+                msg = _("External source are not supported: '%s'") % source
+                LOG.debug(msg)
+                raise HTTPBadRequest(explanation=msg,
+                                     request=req,
+                                     content_type="text/plain")
 
     @staticmethod
     def _copy_from(req):

glance/common/store_utils.py

@@ -16,6 +16,7 @@
 
 import glance_store as store_api
 from oslo.config import cfg
+import six.moves.urllib.parse as urlparse
 
 from glance.common import utils
 import glance.db as db_api
@@ -119,3 +120,24 @@ def delete_image_location_from_backend(context, image_id, location):
         # such as uploading process failure then we can't use
         # location status mechanism to support image pending delete.
         safe_delete_from_backend(context, image_id, location)
+
+
+def validate_external_location(uri):
+    """
+    Validate if URI of external location are supported.
+
+    Only over non-local store types are OK, i.e. S3, Swift,
+    HTTP. Note the absence of 'file://' for security reasons,
+    see LP bug #942118, 1400966, 'swift+config://' is also
+    absent for security reasons, see LP bug #1334196.
+
+    :param uri: The URI of external image location.
+    :return: Whether given URI of external image location are OK.
+    """
+
+    # TODO(zhiyan): This function could be moved to glance_store.
+
+    pieces = urlparse.urlparse(uri)
+    valid_schemes = [scheme for scheme in store_api.get_known_schemes()
+                     if scheme != 'file' and scheme != 'swift+config']
+    return pieces.scheme in valid_schemes

glance/location.py

@@ -26,6 +26,7 @@
 from glance import i18n
 import glance.openstack.common.log as logging
 
+
 _ = i18n._
 _LE = i18n._LE
 
@@ -66,27 +67,29 @@ def save(self, image):
         return result
 
 
-def _check_location_uri(context, store_api, uri):
+def _check_location_uri(context, store_api, store_utils, uri):
     """Check if an image location is valid.
 
     :param context: Glance request context
     :param store_api: store API module
+    :param store_utils: store utils module
     :param uri: location's uri string
     """
+
     is_ok = True
     try:
-        size = store_api.get_size_from_backend(uri, context=context)
         # NOTE(zhiyan): Some stores return zero when it catch exception
-        is_ok = size > 0
+        is_ok = (store_utils.validate_external_location(uri) and
+                 store_api.get_size_from_backend(uri, context=context) > 0)
     except (store.UnknownScheme, store.NotFound):
         is_ok = False
     if not is_ok:
         reason = _('Invalid location')
         raise exception.BadStoreUri(message=reason)
 
 
-def _check_image_location(context, store_api, location):
-    _check_location_uri(context, store_api, location['url'])
+def _check_image_location(context, store_api, store_utils, location):
+    _check_location_uri(context, store_api, store_utils, location['url'])
     store_api.check_location_metadata(location['metadata'])
 
 
@@ -122,6 +125,7 @@ class ImageFactoryProxy(glance.domain.proxy.ImageFactory):
     def __init__(self, factory, context, store_api, store_utils):
         self.context = context
         self.store_api = store_api
+        self.store_utils = store_utils
         proxy_kwargs = {'context': context, 'store_api': store_api,
                         'store_utils': store_utils}
         super(ImageFactoryProxy, self).__init__(factory,
@@ -131,7 +135,10 @@ def __init__(self, factory, context, store_api, store_utils):
     def new_image(self, **kwargs):
         locations = kwargs.get('locations', [])
         for loc in locations:
-            _check_image_location(self.context, self.store_api, loc)
+            _check_image_location(self.context,
+                                  self.store_api,
+                                  self.store_utils,
+                                  loc)
             loc['status'] = 'active'
             if _count_duplicated_locations(locations, loc) > 1:
                 raise exception.DuplicateLocation(location=loc['url'])
@@ -169,7 +176,9 @@ def extend(self, other):
 
     def insert(self, i, location):
         _check_image_location(self.image_proxy.context,
-                              self.image_proxy.store_api, location)
+                              self.image_proxy.store_api,
+                              self.image_proxy.store_utils,
+                              location)
         location['status'] = 'active'
         if _count_duplicated_locations(self.value, location) > 0:
             raise exception.DuplicateLocation(location=location['url'])
@@ -214,7 +223,9 @@ def __getitem__(self, i):
 
     def __setitem__(self, i, location):
         _check_image_location(self.image_proxy.context,
-                              self.image_proxy.store_api, location)
+                              self.image_proxy.store_api,
+                              self.image_proxy.store_utils,
+                              location)
         location['status'] = 'active'
         self.value.__setitem__(i, location)
         _set_image_size(self.image_proxy.context,
@@ -303,7 +314,9 @@ def set_attr(self, value):
                                           '%s') % ori_value)
             # NOTE(zhiyan): Check locations are all valid.
             for location in value:
-                _check_image_location(self.context, self.store_api,
+                _check_image_location(self.context,
+                                      self.store_api,
+                                      self.store_utils,
                                       location)
                 location['status'] = 'active'
                 if _count_duplicated_locations(value, location) > 1:

glance/tests/functional/v1/test_copy_to_file.py

@@ -250,7 +250,7 @@ def test_copy_from_file(self):
         response, content = http.request(path, 'POST', headers=headers)
         self.assertEqual(400, response.status, content)
 
-        expected = 'External sourcing not supported for store \'file\''
+        expected = 'External source are not supported: \'%s\'' % copy_from
         msg = 'expected "%s" in "%s"' % (expected, content)
         self.assertTrue(expected in content, msg)
 
@@ -276,7 +276,7 @@ def test_copy_from_swift_config(self):
         response, content = http.request(path, 'POST', headers=headers)
         self.assertEqual(400, response.status, content)
 
-        expected = 'External sourcing not supported for store \'swift+config\''
+        expected = 'External source are not supported: \'swift+config://xxx\''
         msg = 'expected "%s" in "%s"' % (expected, content)
         self.assertTrue(expected in content, msg)