From 13069a4017d36a549576a21ca3ec5b15c411effc 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 d10aea7..2f75441 100644 --- a/glance/store/sheepdog.py +++ b/glance/store/sheepdog.py @@ -25,6 +25,7 @@ from glance.common import exception from glance.openstack.common import excutils import glance.openstack.common.log as logging from glance.openstack.common import processutils +from glance.openstack.common import uuidutils import glance.store import glance.store.base import glance.store.location @@ -32,7 +33,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 @@ -63,18 +64,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)s %(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) @@ -84,7 +81,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): @@ -94,7 +91,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): """ @@ -103,7 +100,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): """ @@ -111,7 +108,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): """ @@ -119,7 +116,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): """ @@ -127,7 +124,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: @@ -138,7 +135,7 @@ class StoreLocation(glance.store.location.StoreLocation): """ Class describing a Sheepdog URI. This is of the form: - sheepdog://image + sheepdog://image-id """ @@ -149,10 +146,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 uuidutils.is_uuid_like(self.image): + raise exception.BadStoreUri(_("URI must contains well-formated " + "image id")) class ImageIterator(object): @@ -192,7 +193,7 @@ class Store(glance.store.base.Store): try: self.chunk_size = CONF.sheepdog_store_chunk_size * 1024 * 1024 - 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 @@ -200,10 +201,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 8eef86b..bea7e29 100644 --- a/glance/tests/unit/test_sheepdog_store.py +++ b/glance/tests/unit/test_sheepdog_store.py @@ -57,4 +57,5 @@ class TestStore(base.StoreClearingUnitTest): 'fake_image_id', utils.LimitingReader(StringIO.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 7eec171..2464ebb 100644 --- a/glance/tests/unit/test_store_location.py +++ b/glance/tests/unit/test_store_location.py @@ -52,7 +52,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', ] @@ -367,15 +367,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_cinder_store_good_location(self): -- 1.7.9.5