openstack/deb-glance
Code Issues Proposed changes

Disallow file:// sources on location or copy-from.

Browse Source 
Fixes bug #942118

For security reasons, file:// URIs local to the glance services
should not be supported as external sources (as specified via the
x-image-meta-location or x-glance-api-copy-from headers).

Change-Id: I43763cbefba95153434c7dcdcce3765ed04e05fe
This commit is contained in:
Eoghan Glynn 2012-02-27 22:57:12 +00:00
parent 63f3af7c46
commit e653a0032d
7 changed files with 93 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
glance/api/v1/images.py
View File

@ -226,13 +226,30 @@ class Controller(controller.BaseController):
'image_meta': image_meta
}
@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.
If the above constraint is violated, we reject with 400 "Bad Request".
"""
if source:
for scheme in ['s3', 'swift', 'http']:
if source.lower().startswith(scheme):
return source
msg = _("External sourcing not supported for store %s") % source
logger.error(msg)
raise HTTPBadRequest(msg, request=req, content_type="text/plain")
@staticmethod
def _copy_from(req):
return req.headers.get('x-glance-api-copy-from')
@staticmethod
def _external_source(image_meta, req):
return image_meta.get('location', Controller._copy_from(req))
source = image_meta.get('location', Controller._copy_from(req))
return Controller._validate_source(source, req)
@staticmethod
def _get_from_store(where):

29
glance/tests/functional/__init__.py
View File

@ -92,7 +92,9 @@ class Server(object):
def write_conf(self, **kwargs):
"""
Writes the configuration file for the server to its intended
destination. Returns the name of the configuration file.
destination. Returns the name of the configuration file and
the over-ridden config content (may be useful for populating
error messages).
"""
if self.conf_file_name:
@ -112,22 +114,24 @@ class Server(object):
paste_conf_filepath = conf_filepath.replace(".conf", "-paste.ini")
utils.safe_mkdirs(conf_dir)
def override_conf(filepath, base, override):
def override_conf(filepath, overridden):
with open(filepath, 'wb') as conf_file:
conf_file.write(base % override)
conf_file.write(overridden)
conf_file.flush()
return conf_file.name
self.conf_file_name = override_conf(conf_filepath,
self.conf_base,
conf_override)
overridden_core = self.conf_base % conf_override
self.conf_file_name = override_conf(conf_filepath, overridden_core)
overridden_paste = ''
if self.paste_conf_base:
override_conf(paste_conf_filepath,
self.paste_conf_base,
conf_override)
overridden_paste = self.paste_conf_base % conf_override
override_conf(paste_conf_filepath, overridden_paste)
return self.conf_file_name
overridden = ('==Core config==\n%s\n==Paste config==\n%s' %
(overridden_core, overridden_paste))
return self.conf_file_name, overridden
def start(self, expect_exit=True, expected_exitcode=0, **kwargs):
"""
@ -138,7 +142,7 @@ class Server(object):
"""
# Ensure the configuration file is written
self.write_conf(**kwargs)
overridden = self.write_conf(**kwargs)[1]
cmd = ("%(server_control)s %(server_name)s start "
"%(conf_file_name)s --pid-file=%(pid_file)s "
@ -148,7 +152,8 @@ class Server(object):
no_venv=self.no_venv,
exec_env=self.exec_env,
expect_exit=expect_exit,
expected_exitcode=expected_exitcode)
expected_exitcode=expected_exitcode,
context=overridden)
def stop(self):
"""

2
glance/tests/functional/keystone_utils.py
View File

@ -245,7 +245,7 @@ class KeystoneTests(functional.FunctionalTest):
super(KeystoneTests, self).start_servers(**kwargs)
# Set up the data store
keystone_conf = self.auth_server.write_conf(**kwargs)
keystone_conf = self.auth_server.write_conf(**kwargs)[0]
datafile = os.path.join(os.path.dirname(__file__), 'data',
'keystone_data.py')

13
glance/tests/functional/test_bin_glance.py
View File

@ -43,6 +43,10 @@ class TestBinGlance(functional.FunctionalTest):
# NoAuth strategy.
os.environ['OS_AUTH_STRATEGY'] = 'noauth'
def _assertStartsWith(self, str, prefix):
msg = 'expected "%s" to start with "%s"' % (str, prefix)
self.assertTrue(str.startswith(prefix), msg)
def test_add_with_location(self):
self.cleanup()
self.start_servers(**self.__dict__.copy())
@ -257,16 +261,15 @@ class TestBinGlance(functional.FunctionalTest):
image_file.write("XXX")
image_file.flush()
image_file_name = image_file.name
cmd = minimal_add_command(api_port,
'MyImage',
'< %s' % image_file_name)
suffix = '--silent-upload < %s' % image_file_name
cmd = minimal_add_command(api_port, 'MyImage', suffix)
exitcode, out, err = execute(cmd)
self.assertEqual(0, exitcode)
msg = out.split("\n")
self.assertTrue(msg[0].startswith('Uploading image'))
self.assertTrue(msg[1].startswith('Added new image with ID:'))
self._assertStartsWith(msg[0], 'Added new image with ID:')
# 2. Verify image added as public image
cmd = "bin/glance --port=%d index" % api_port

39
glance/tests/functional/test_copy_to_file.py
View File

@ -43,6 +43,7 @@ established, then the relevant test case is skipped.
import hashlib
import httplib2
import json
import tempfile
from glance.tests import functional
from glance.tests.utils import skip_if_disabled, requires
@ -97,7 +98,7 @@ class TestCopyToFile(functional.FunctionalTest):
copy_from = get_uri(self, original_image_id)
# POST /images with public image copied from_store (to Swift)
# POST /images with public image copied from_store (to file)
headers = {'X-Image-Meta-Name': 'copied',
'X-Image-Meta-disk_format': 'raw',
'X-Image-Meta-container_format': 'ovf',
@ -187,7 +188,7 @@ class TestCopyToFile(functional.FunctionalTest):
copy_from = get_http_uri(self, 'foobar')
# POST /images with public image copied HTTP (to Swift)
# POST /images with public image copied from HTTP (to file)
headers = {'X-Image-Meta-Name': 'copied',
'X-Image-Meta-disk_format': 'raw',
'X-Image-Meta-container_format': 'ovf',
@ -223,3 +224,37 @@ class TestCopyToFile(functional.FunctionalTest):
self.assertEqual(response.status, 200)
self.stop_servers()
@skip_if_disabled
def test_copy_from_file(self):
"""
Ensure we can't copy from file
"""
self.cleanup()
self.start_servers(**self.__dict__.copy())
api_port = self.api_port
registry_port = self.registry_port
with tempfile.NamedTemporaryFile() as image_file:
image_file.write("XXX")
image_file.flush()
copy_from = 'file://' + image_file.name
# POST /images with public image copied from file (to file)
headers = {'X-Image-Meta-Name': 'copied',
'X-Image-Meta-disk_format': 'raw',
'X-Image-Meta-container_format': 'ovf',
'X-Image-Meta-Is-Public': 'True',
'X-Glance-API-Copy-From': copy_from}
path = "http://%s:%d/v1/images" % ("0.0.0.0", self.api_port)
http = httplib2.Http()
response, content = http.request(path, 'POST', headers=headers)
self.assertEqual(response.status, 400, content)
expected = 'External sourcing not supported for store ' + copy_from
msg = 'expected "%s" in "%s"' % (expected, content)
self.assertTrue(expected in content, msg)
self.stop_servers()

14
glance/tests/unit/test_clients.py
View File

@ -1616,7 +1616,7 @@ class TestClient(base.IsolatedUnitTest):
'container_format': 'bare',
'status': 'active',
'size': 19,
'location': "file:///tmp/glance-tests/3",
'location': "http://localhost/glance-tests/3",
'properties': {}}
new_image = self.client.add_image(fixture)
@ -1653,7 +1653,7 @@ class TestClient(base.IsolatedUnitTest):
'disk_format': 'vhd',
'container_format': 'ovf',
'size': 19,
'location': "file:///tmp/glance-tests/2",
'location': "http://localhost/glance-tests/2",
}
new_image = self.client.add_image(fixture)
new_image_id = new_image['id']
@ -1676,7 +1676,7 @@ class TestClient(base.IsolatedUnitTest):
'disk_format': 'vhd',
'container_format': 'ovf',
'size': 19,
'location': "file:///tmp/glance-tests/2",
'location': "http://localhost/glance-tests/2",
'properties': {'distro': 'Ubuntu 10.04 LTS'},
}
new_image = self.client.add_image(fixture)
@ -1700,7 +1700,7 @@ class TestClient(base.IsolatedUnitTest):
'disk_format': 'iso',
'container_format': 'bare',
'size': 19,
'location': "file:///tmp/glance-tests/2",
'location': "http://localhost/glance-tests/2",
'properties': {'install': 'Bindows Heaven'},
}
new_image = self.client.add_image(fixture)
@ -1728,7 +1728,7 @@ class TestClient(base.IsolatedUnitTest):
'disk_format': 'iso',
'container_format': 'vhd',
'size': 19,
'location': "file:///tmp/glance-tests/3",
'location': "http://localhost/glance-tests/3",
'properties': {'install': 'Bindows Heaven'},
}
@ -1745,7 +1745,7 @@ class TestClient(base.IsolatedUnitTest):
'container_format': 'ovf',
'status': 'bad status',
'size': 19,
'location': "file:///tmp/glance-tests/2",
'location': "http://localhost/glance-tests/2",
}
self.assertRaises(exception.Duplicate,
@ -1760,7 +1760,7 @@ class TestClient(base.IsolatedUnitTest):
'container_format': 'ovf',
'status': 'bad status',
'size': 19,
'location': "file:///tmp/glance-tests/2",
'location': "http://localhost/glance-tests/2",
}
new_image = self.client.add_image(fixture)

6
glance/tests/utils.py
View File

@ -190,7 +190,8 @@ def execute(cmd,
no_venv=False,
exec_env=None,
expect_exit=True,
expected_exitcode=0):
expected_exitcode=0,
context=None):
"""
Executes a command in a subprocess. Returns a tuple
of (exitcode, out, err), where out is the string output
@ -207,6 +208,7 @@ def execute(cmd,
environment variable
:param expect_exit: Optional flag true iff timely exit is expected
:param expected_exitcode: expected exitcode from the launcher
:param context: additional context for error message
"""
env = os.environ.copy()
@ -254,6 +256,8 @@ def execute(cmd,
"code of %(exitcode)d."\
"\n\nSTDOUT: %(out)s"\
"\n\nSTDERR: %(err)s" % locals()
if context:
msg += "\n\nCONTEXT: %s" % context
raise RuntimeError(msg)
return exitcode, out, err