Skip to content

Commit

Permalink
Validate image location scheme
Browse files Browse the repository at this point in the history
This fixes the other half of
https://bugs.launchpad.net/glance/+bug/1400966 the directory traversal
bug. In havana, glance would prevent you part of the flaw, by way of
https://bugs.launchpad.net/glance/+bug/942118 but glance would still
allow user to update an image location after an image was created
without validation of the image scheme. This patch closes the loop hole.

This also adds a test to ensure these protected schemes are not allowed
to be used when updating an image location.
  • Loading branch information
omgjlk committed Jan 7, 2015
1 parent 252fe85 commit 7ab98b7
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 4 deletions.
27 changes: 23 additions & 4 deletions glance/store/__init__.py
Expand Up @@ -21,6 +21,7 @@
import time

from oslo.config import cfg
import six.moves.urllib.parse as urlparse

from glance.common import crypt
from glance.common import exception
Expand Down Expand Up @@ -369,6 +370,23 @@ def set_acls(context, location_uri, public=False, read_tenants=[],
LOG.debug(_("Skipping store.set_acls... not implemented."))


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.
"""
pieces = urlparse.urlparse(uri)
valid_schemes = ['s3', 'swift', 'http', 'rbd', 'sheepdog', 'cinder']
return pieces.scheme in valid_schemes


class ImageRepoProxy(glance.domain.proxy.Repo):

def __init__(self, image_repo, context, store_api):
Expand Down Expand Up @@ -402,21 +420,22 @@ def save(self, image):

def _check_location_uri(context, store_api, uri):
"""
Check if an image location uri is valid.
Check if an image location is valid.
:param context: Glance request context
:param store_api: store API module
:param uri: location's uri string
"""
is_ok = True
try:
size = store_api.get_size_from_backend(context, uri)
# NOTE(zhiyan): Some stores return zero when it catch exception
is_ok = size > 0
is_ok = (store_api.validate_external_location(uri) and
store_api.get_size_from_backend(context, uri) > 0)
except (exception.UnknownScheme, exception.NotFound):
is_ok = False
if not is_ok:
raise exception.BadStoreUri(_('Invalid location: %s') % uri)
reason = _('Invalid location')
raise exception.BadStoreUri(message=reason)


def _check_image_location(context, store_api, location):
Expand Down
20 changes: 20 additions & 0 deletions glance/tests/functional/v2/test_images.py
Expand Up @@ -220,6 +220,26 @@ def test_image_lifecycle(self):
self.assertEqual(200, response.status_code)
self.assertEqual(5, json.loads(response.text)['size'])

# update locations should not work on protected schemes
path = self._url('/v2/images/%s' % image_id)
media_type = 'application/openstack-images-v2.1-json-patch'
headers = self._headers({'content-type': media_type})
doc = [{'op': 'replace', 'path': '/locations',
'value': [{'url': 'file:///foo_image',
'metadata': {}}]
}]
data = json.dumps(doc)
response = requests.patch(path, headers=headers, data=data)
self.assertEqual(400, response.status_code, response.text)

doc = [{'op': 'replace', 'path': '/locations',
'value': [{'url': 'swift+config:///foo_image',
'metadata': {}}]
}]
data = json.dumps(doc)
response = requests.patch(path, headers=headers, data=data)
self.assertEqual(400, response.status_code, response.text)

# Deletion should not work on protected images
path = self._url('/v2/images/%s' % image_id)
response = requests.delete(path, headers=self._headers())
Expand Down

0 comments on commit 7ab98b7

Please sign in to comment.