From 706f9112711ad61cda08a3c075d3fbe87bf2ed9d Mon Sep 17 00:00:00 2001 From: Zhi Yan Liu Date: Sat, 6 Sep 2014 11:57:37 +0800 Subject: [PATCH] Allowing operator to configure a permission for image file in fs store In this way the user other service used, e.g. Nova, who consumes the image could be the exclusive member of the group that owns the files created. Closes-bug: 1264302 Related-Id: I4d543d205b0805fe00dcab1b0872c0a5e0f97a5f Change-Id: Iec8396f92ed11531dccb82957da2455ca333430a Signed-off-by: Zhi Yan Liu --- glance_store/_drivers/filesystem.py | 72 +++++++++++++++++- glance_store/backend.py | 6 +- tests/unit/test_filesystem_store.py | 109 ++++++++++++++++++++++++++++ 3 files changed, 181 insertions(+), 6 deletions(-) diff --git a/glance_store/_drivers/filesystem.py b/glance_store/_drivers/filesystem.py index b020280b..a2f86361 100644 --- a/glance_store/_drivers/filesystem.py +++ b/glance_store/_drivers/filesystem.py @@ -22,6 +22,7 @@ import errno import hashlib import logging import os +import stat import urlparse from oslo.config import cfg @@ -30,7 +31,7 @@ import glance_store from glance_store.common import utils import glance_store.driver from glance_store import exceptions -from glance_store.i18n import _ +from glance_store import i18n import glance_store.location from glance_store.openstack.common import excutils from glance_store.openstack.common import jsonutils @@ -39,6 +40,9 @@ from glance_store.openstack.common import units LOG = logging.getLogger(__name__) +_ = i18n._ +_LE = i18n._LE +_LW = i18n._LW _FILESYSTEM_CONFIGS = [ cfg.StrOpt('filesystem_store_datadir', @@ -51,7 +55,16 @@ _FILESYSTEM_CONFIGS = [ help=_("The path to a file which contains the " "metadata to be returned with any location " "associated with this store. The file must " - "contain a valid JSON dict."))] + "contain a valid JSON dict.")), + cfg.IntOpt('filesystem_store_file_perm', + default=0, + help=_("The required permission for created image file. " + "In this way the user other service used, e.g. Nova, " + "who consumes the image could be the exclusive member " + "of the group that owns the files created. Assigning " + "it less then or equal to zero means don't change the " + "default permission of the file. This value will be " + "decoded as an octal digit."))] class StoreLocation(glance_store.location.StoreLocation): @@ -142,6 +155,35 @@ class Store(glance_store.driver.Store): raise exceptions.BadStoreConfiguration( store_name="filesystem", reason=msg) + def _set_exec_permission(self, datadir): + """ + Set the execution permission of owner-group and/or other-users to + image directory if the image file which contained needs relevant + access permissions. + + :datadir is a directory path in which glance writes image files. + """ + + if self.conf.glance_store.filesystem_store_file_perm <= 0: + return + + try: + mode = os.stat(datadir)[stat.ST_MODE] + perm = int(str(self.conf.glance_store.filesystem_store_file_perm), + 8) + if perm & stat.S_IRWXO > 0: + if not mode & stat.S_IXOTH: + # chmod o+x + mode |= stat.S_IXOTH + os.chmod(datadir, mode) + if perm & stat.S_IRWXG > 0: + if not mode & stat.S_IXGRP: + # chmod g+x + os.chmod(datadir, mode | stat.S_IXGRP) + except (IOError, OSError): + LOG.warn(_LW("Unable to set execution permission of owner-group " + "and/or other-users to datadir: %s") % datadir) + def _create_image_directories(self, directory_paths): """ Create directories to write image files if @@ -153,6 +195,7 @@ class Store(glance_store.driver.Store): for datadir in directory_paths: if os.path.exists(datadir): self._check_write_permission(datadir) + self._set_exec_permission(datadir) else: msg = _("Directory to write image files does not exist " "(%s). Creating.") % datadir @@ -160,6 +203,7 @@ class Store(glance_store.driver.Store): try: os.makedirs(datadir) self._check_write_permission(datadir) + self._set_exec_permission(datadir) except (IOError, OSError): if os.path.exists(datadir): # NOTE(markwash): If the path now exists, some other @@ -167,6 +211,7 @@ class Store(glance_store.driver.Store): # But it doesn't hurt, so we can safely ignore # the error. self._check_write_permission(datadir) + self._set_exec_permission(datadir) continue reason = _("Unable to create datadir: %s") % datadir LOG.error(reason) @@ -197,6 +242,19 @@ class Store(glance_store.driver.Store): raise exceptions.BadStoreConfiguration(store_name="filesystem", reason=reason) + if self.conf.glance_store.filesystem_store_file_perm > 0: + perm = int(str(self.conf.glance_store.filesystem_store_file_perm), + 8) + if not perm & stat.S_IRUSR: + reason = _LE("Specified an invalid " + "'filesystem_store_file_perm' option which " + "could make image file to be unaccessible by " + "glance service.") + LOG.error(reason) + reason = _("Invalid 'filesystem_store_file_perm' option.") + raise exceptions.BadStoreConfiguration(store_name="filesystem", + reason=reason) + self.multiple_datadirs = False directory_paths = set() if self.conf.glance_store.filesystem_store_datadir: @@ -468,6 +526,16 @@ class Store(glance_store.driver.Store): {'bytes_written': bytes_written, 'filepath': filepath, 'checksum_hex': checksum_hex}) + + if self.conf.glance_store.filesystem_store_file_perm > 0: + perm = int(str(self.conf.glance_store.filesystem_store_file_perm), + 8) + try: + os.chmod(filepath, perm) + except (IOError, OSError): + LOG.warn(_LW("Unable to set permission to image: %s") % + filepath) + return ('file://%s' % filepath, bytes_written, checksum_hex, metadata) @staticmethod diff --git a/glance_store/backend.py b/glance_store/backend.py index 5f28aa30..50ac83eb 100644 --- a/glance_store/backend.py +++ b/glance_store/backend.py @@ -140,7 +140,6 @@ class Indexable(object): def _load_store(conf, store_entry, invoke_load=True): - store_cls = None try: LOG.debug("Attempting to import store %s", store_entry) mgr = driver.DriverManager('glance_store.drivers', @@ -148,7 +147,7 @@ def _load_store(conf, store_entry, invoke_load=True): invoke_args=[conf], invoke_on_load=invoke_load) return mgr.driver - except RuntimeError as ex: + except RuntimeError: LOG.warn("Failed to load driver %(driver)s." "The driver will be disabled" % dict(driver=driver)) @@ -176,7 +175,6 @@ def create_stores(conf=CONF): from the given config. Duplicates are not re-registered. """ store_count = 0 - store_classes = set() for (store_entry, store_instance) in _load_stores(conf): schemes = store_instance.get_schemes() @@ -184,7 +182,7 @@ def create_stores(conf=CONF): if not schemes: raise exceptions.BackendException('Unable to register store %s. ' 'No schemes associated with it.' - % store_cls) + % store_entry) else: LOG.debug("Registering store %s with schemes %s", store_entry, schemes) diff --git a/tests/unit/test_filesystem_store.py b/tests/unit/test_filesystem_store.py index e14938c4..334081e4 100644 --- a/tests/unit/test_filesystem_store.py +++ b/tests/unit/test_filesystem_store.py @@ -21,6 +21,7 @@ import hashlib import json import mock import os +import stat import StringIO import uuid @@ -446,3 +447,111 @@ class TestStore(base.StoreBaseTest): self.assertRaises(exceptions.StorageFull, self.store.add, expected_image_id, image_file, expected_file_size) + + def test_configure_add_with_file_perm(self): + """ + Tests filesystem specified by filesystem_store_file_perm + are parsed correctly. + """ + store = self.useFixture(fixtures.TempDir()).path + self.conf.set_override('filesystem_store_datadir', store, + group='glance_store') + self.conf.set_override('filesystem_store_file_perm', 700, # -rwx------ + group='glance_store') + self.store.configure_add() + self.assertEqual(self.store.datadir, store) + + def test_configure_add_with_unaccessible_file_perm(self): + """ + Tests BadStoreConfiguration exception is raised if an invalid + file permission specified in filesystem_store_file_perm. + """ + store = self.useFixture(fixtures.TempDir()).path + self.conf.set_override('filesystem_store_datadir', store, + group='glance_store') + self.conf.set_override('filesystem_store_file_perm', 7, # -------rwx + group='glance_store') + self.assertRaises(exceptions.BadStoreConfiguration, + self.store.configure_add) + + def test_add_with_file_perm_for_group_other_users_access(self): + """ + Test that we can add an image via the filesystem backend with a + required image file permission. + """ + store = self.useFixture(fixtures.TempDir()).path + self.conf.set_override('filesystem_store_datadir', store, + group='glance_store') + self.conf.set_override('filesystem_store_file_perm', 744, # -rwxr--r-- + group='glance_store') + + # -rwx------ + os.chmod(store, 0o700) + self.assertEqual(0o700, stat.S_IMODE(os.stat(store)[stat.ST_MODE])) + + self.store.configure_add() + + Store.WRITE_CHUNKSIZE = 1024 + expected_image_id = str(uuid.uuid4()) + expected_file_size = 5 * units.Ki # 5K + expected_file_contents = "*" * expected_file_size + expected_checksum = hashlib.md5(expected_file_contents).hexdigest() + expected_location = "file://%s/%s" % (store, + expected_image_id) + image_file = six.StringIO(expected_file_contents) + + location, size, checksum, _ = self.store.add(expected_image_id, + image_file, + expected_file_size) + + self.assertEqual(expected_location, location) + self.assertEqual(expected_file_size, size) + self.assertEqual(expected_checksum, checksum) + + # -rwx--x--x for store directory + self.assertEqual(0o711, stat.S_IMODE(os.stat(store)[stat.ST_MODE])) + # -rwxr--r-- for image file + mode = os.stat(expected_location[len('file:/'):])[stat.ST_MODE] + perm = int(str(self.conf.glance_store.filesystem_store_file_perm), 8) + self.assertEqual(perm, stat.S_IMODE(mode)) + + def test_add_with_file_perm_for_owner_users_access(self): + """ + Test that we can add an image via the filesystem backend with a + required image file permission. + """ + store = self.useFixture(fixtures.TempDir()).path + self.conf.set_override('filesystem_store_datadir', store, + group='glance_store') + self.conf.set_override('filesystem_store_file_perm', 600, # -rw------- + group='glance_store') + + # -rwx------ + os.chmod(store, 0o700) + self.assertEqual(0o700, stat.S_IMODE(os.stat(store)[stat.ST_MODE])) + + self.store.configure_add() + + Store.WRITE_CHUNKSIZE = 1024 + expected_image_id = str(uuid.uuid4()) + expected_file_size = 5 * units.Ki # 5K + expected_file_contents = "*" * expected_file_size + expected_checksum = hashlib.md5(expected_file_contents).hexdigest() + expected_location = "file://%s/%s" % (store, + expected_image_id) + image_file = six.StringIO(expected_file_contents) + + location, size, checksum, _ = self.store.add(expected_image_id, + image_file, + expected_file_size) + + self.assertEqual(expected_location, location) + self.assertEqual(expected_file_size, size) + self.assertEqual(expected_checksum, checksum) + + # -rwx------ for store directory + self.assertEqual(0o700, stat.S_IMODE(os.stat(store)[stat.ST_MODE])) + # -rw------- for image file + mode = os.stat(expected_location[len('file:/'):])[stat.ST_MODE] + perm = int(str(self.conf.glance_store.filesystem_store_file_perm), 8) + self.assertEqual(perm, stat.S_IMODE(mode))