From f06ca34e7dd88a368e058c8f0aca62a4f6005a2b Mon Sep 17 00:00:00 2001 From: Zhi Yan Liu Date: Sat, 29 Mar 2014 03:35:35 +0800 Subject: [PATCH] To prevent remote code injection on Sheepdog store Change-Id: Iae92eaf9eb023f36a1bab7c20ea41c985f2bf51b Signed-off-by: Zhi Yan Liu --- glance/store/sheepdog.py | 61 ++++++++++++++---------- glance/tests/unit/test_sheepdog_store.py | 3 +- glance/tests/unit/test_store_location.py | 13 +++-- 3 files changed, 45 insertions(+), 32 deletions(-) diff --git a/glance/store/sheepdog.py b/glance/store/sheepdog.py index 11293f0e60..7a0133c845 100644 --- a/glance/store/sheepdog.py +++ b/glance/store/sheepdog.py @@ -20,6 +20,7 @@ import hashlib from oslo.config import cfg from glance.common import exception +from glance.common import utils from glance.openstack.common import excutils import glance.openstack.common.log as logging from glance.openstack.common import processutils @@ -31,7 +32,7 @@ import glance.store.location LOG = logging.getLogger(__name__) -DEFAULT_ADDR = 'localhost' +DEFAULT_ADDR = '127.0.0.1' DEFAULT_PORT = 7000 DEFAULT_CHUNKSIZE = 64 # in MiB @@ -62,18 +63,14 @@ class SheepdogImage: self.chunk_size = chunk_size def _run_command(self, command, data, *params): - cmd = ("collie vdi %(command)s -a %(addr)s -p %(port)d %(name)s " - "%(params)s" % - {"command": command, - "addr": self.addr, - "port": self.port, - "name": self.name, - "params": " ".join(map(str, params))}) + cmd = ["collie", "vdi"] + cmd.extend(command) + cmd.extend(["-a", self.addr, "-p", self.port, self.name]) + cmd.extend(params) try: - return processutils.execute( - cmd, process_input=data, shell=True)[0] - except processutils.ProcessExecutionError as exc: + return processutils.execute(*cmd, process_input=data)[0] + except (processutils.ProcessExecutionError, OSError) as exc: LOG.error(exc) raise glance.store.BackendException(exc) @@ -83,7 +80,7 @@ class SheepdogImage: Sheepdog Usage: collie vdi list -r -a address -p port image """ - out = self._run_command("list -r", None) + out = self._run_command(["list", "-r"], None) return long(out.split(' ')[3]) def read(self, offset, count): @@ -93,7 +90,7 @@ class SheepdogImage: Sheepdog Usage: collie vdi read -a address -p port image offset len """ - return self._run_command("read", None, str(offset), str(count)) + return self._run_command(["read"], None, str(offset), str(count)) def write(self, data, offset, count): """ @@ -102,7 +99,7 @@ class SheepdogImage: Sheepdog Usage: collie vdi write -a address -p port image offset len """ - self._run_command("write", data, str(offset), str(count)) + self._run_command(["write"], data, str(offset), str(count)) def create(self, size): """ @@ -110,7 +107,7 @@ class SheepdogImage: Sheepdog Usage: collie vdi create -a address -p port image size """ - self._run_command("create", None, str(size)) + self._run_command(["create"], None, str(size)) def delete(self): """ @@ -118,7 +115,7 @@ class SheepdogImage: Sheepdog Usage: collie vdi delete -a address -p port image """ - self._run_command("delete", None) + self._run_command(["delete"], None) def exist(self): """ @@ -126,7 +123,7 @@ class SheepdogImage: Sheepdog Usage: collie vdi list -r -a address -p port image """ - out = self._run_command("list -r", None) + out = self._run_command(["list", "-r"], None) if not out: return False else: @@ -137,7 +134,7 @@ class StoreLocation(glance.store.location.StoreLocation): """ Class describing a Sheepdog URI. This is of the form: - sheepdog://image + sheepdog://image-id """ @@ -148,10 +145,14 @@ class StoreLocation(glance.store.location.StoreLocation): return "sheepdog://%s" % self.image def parse_uri(self, uri): - if not uri.startswith('sheepdog://'): - raise exception.BadStoreUri(uri, "URI must start with %s://" % - 'sheepdog') - self.image = uri[11:] + valid_schema = 'sheepdog://' + if not uri.startswith(valid_schema): + raise exception.BadStoreUri(_("URI must start with %s://") % + valid_schema) + self.image = uri[len(valid_schema):] + if not utils.is_uuid_like(self.image): + raise exception.BadStoreUri(_("URI must contains well-formated " + "image id")) class ImageIterator(object): @@ -191,7 +192,7 @@ class Store(glance.store.base.Store): try: self.chunk_size = CONF.sheepdog_store_chunk_size * units.Mi - self.addr = CONF.sheepdog_store_address + self.addr = CONF.sheepdog_store_address.strip() self.port = CONF.sheepdog_store_port except cfg.ConfigFileValueError as e: reason = _("Error in store configuration: %s") % e @@ -199,10 +200,18 @@ class Store(glance.store.base.Store): raise exception.BadStoreConfiguration(store_name='sheepdog', reason=reason) + if ' ' in self.addr: + reason = (_("Invalid address configuration of sheepdog store: %s") + % self.addr) + LOG.error(reason) + raise exception.BadStoreConfiguration(store_name='sheepdog', + reason=reason) + try: - processutils.execute("collie", shell=True) - except processutils.ProcessExecutionError as exc: - reason = _("Error in store configuration: %s") % exc + cmd = ["collie", "vdi", "list", "-a", self.addr, "-p", self.port] + processutils.execute(*cmd) + except Exception as e: + reason = _("Error in store configuration: %s") % e LOG.error(reason) raise exception.BadStoreConfiguration(store_name='sheepdog', reason=reason) diff --git a/glance/tests/unit/test_sheepdog_store.py b/glance/tests/unit/test_sheepdog_store.py index 022334971e..7dc57ab2ce 100644 --- a/glance/tests/unit/test_sheepdog_store.py +++ b/glance/tests/unit/test_sheepdog_store.py @@ -56,4 +56,5 @@ class TestStore(base.StoreClearingUnitTest): 'fake_image_id', utils.LimitingReader(six.StringIO('xx'), 1), 2) - self.assertEqual(called_commands, ['list -r', 'create', 'delete']) + self.assertEqual([['list', '-r'], ['create'], ['delete']], + called_commands) diff --git a/glance/tests/unit/test_store_location.py b/glance/tests/unit/test_store_location.py index 898fe8c720..df8d5d7840 100644 --- a/glance/tests/unit/test_store_location.py +++ b/glance/tests/unit/test_store_location.py @@ -66,7 +66,7 @@ class TestStoreLocation(base.StoreClearingUnitTest): 'rbd://imagename', 'rbd://fsid/pool/image/snap', 'rbd://%2F/%2F/%2F/%2F', - 'sheepdog://imagename', + 'sheepdog://244e75f1-9c69-4167-9db7-1aa7d1973f6c', 'cinder://12345678-9012-3455-6789-012345678901', 'vsphere://ip/folder/openstack_glance/2332298?dcPath=dc&dsName=ds', ] @@ -382,15 +382,18 @@ class TestStoreLocation(base.StoreClearingUnitTest): """ Test the specific StoreLocation for the Sheepdog store """ - uri = 'sheepdog://imagename' + uri = 'sheepdog://244e75f1-9c69-4167-9db7-1aa7d1973f6c' loc = glance.store.sheepdog.StoreLocation({}) loc.parse_uri(uri) - self.assertEqual('imagename', loc.image) + self.assertEqual('244e75f1-9c69-4167-9db7-1aa7d1973f6c', loc.image) - bad_uri = 'sheepdog:/image' + bad_uri = 'sheepdog:/244e75f1-9c69-4167-9db7-1aa7d1973f6c' self.assertRaises(exception.BadStoreUri, loc.parse_uri, bad_uri) - bad_uri = 'http://image' + bad_uri = 'http://244e75f1-9c69-4167-9db7-1aa7d1973f6c' + self.assertRaises(exception.BadStoreUri, loc.parse_uri, bad_uri) + + bad_uri = 'image; name' self.assertRaises(exception.BadStoreUri, loc.parse_uri, bad_uri) def test_vmware_store_location(self):