From d1a5113cfed76fb8d4935bb90f83560f231942cb Mon Sep 17 00:00:00 2001 From: Greg Becker Date: Thu, 11 Oct 2018 14:29:07 -0700 Subject: permissions: add permission configuration to packages.yaml (#8773) Spack can now be configured to assign permissions to the files installed by a package. In the `packages.yaml` file under `permissions`, the attributes `read`, `write`, and `group` control the package permissions. These attributes can be set per-package, or for all packages under `all`. If permissions are set under `all` and for a specific package, the package-specific settings take precedence. The `read` and `write` attributes take one of `user`, `group`, and `world`. packages: all: permissions: write: group group: spack my_app: permissions: read: group group: my_team --- lib/spack/docs/build_settings.rst | 49 ++++++++++++++++ lib/spack/llnl/util/filesystem.py | 25 ++++++++- lib/spack/spack/directory_layout.py | 16 +++++- lib/spack/spack/hooks/permissions_setters.py | 64 +++++++++++++++++++++ lib/spack/spack/package.py | 15 ++++- lib/spack/spack/package_prefs.py | 77 +++++++++++++++++++++++++- lib/spack/spack/schema/packages.py | 17 ++++++ lib/spack/spack/stage.py | 5 +- lib/spack/spack/test/concretize_preferences.py | 75 ++++++++++++++++++++++++- 9 files changed, 335 insertions(+), 8 deletions(-) create mode 100644 lib/spack/spack/hooks/permissions_setters.py (limited to 'lib') diff --git a/lib/spack/docs/build_settings.rst b/lib/spack/docs/build_settings.rst index 0f935aa1ce..e46d52cc52 100644 --- a/lib/spack/docs/build_settings.rst +++ b/lib/spack/docs/build_settings.rst @@ -166,3 +166,52 @@ The syntax for the ``provider`` section differs slightly from other concretization rules. A provider lists a value that packages may ``depend_on`` (e.g, mpi) and a list of rules for fulfilling that dependency. + +.. _package_permissions: + +------------------- +Package Permissions +------------------- + +Spack can be configured to assign permissions to the files installed +by a package. + +In the ``packages.yaml`` file under ``permissions``, the attributes +``read``, ``write``, and ``group`` control the package +permissions. These attributes can be set per-package, or for all +packages under ``all``. If permissions are set under ``all`` and for a +specific package, the package-specific settings take precedence. + +The ``read`` and ``write`` attributes take one of ``user``, ``group``, +and ``world``. + +.. code-block:: yaml + + packages: + all: + permissions: + write: group + group: spack + my_app: + permissions: + read: group + group: my_team + +The permissions settings describe the broadest level of access to +installations of the specified packages. The execute permissions of +the file are set to the same level as read permissions for those files +that are executable. The default setting for ``read`` is ``world``, +and for ``write`` is ``user``. In the example above, installations of +``my_app`` will be installed with user and group permissions but no +world permissions, and owned by the group ``my_team``. All other +packages will be installed with user and group write privileges, and +world read privileges. Those packages will be owned by the group +``spack``. + +The ``group`` attribute assigns a unix-style group to a package. All +files installed by the package will be owned by the assigned group, +and the sticky group bit will be set on the install prefix and all +directories inside the install prefix. This will ensure that even +manually placed files within the install prefix are owned by the +assigned group. If no group is assigned, Spack will allow the OS +default behavior to go as expected. diff --git a/lib/spack/llnl/util/filesystem.py b/lib/spack/llnl/util/filesystem.py index 7dacb9804f..599d0b95e1 100644 --- a/lib/spack/llnl/util/filesystem.py +++ b/lib/spack/llnl/util/filesystem.py @@ -242,6 +242,25 @@ def group_ids(uid=None): return [g.gr_gid for g in grp.getgrall() if user in g.gr_mem] +def chgrp(path, group): + """Implement the bash chgrp function on a single path""" + gid = grp.getgrnam(group).gr_gid + os.chown(path, -1, gid) + + +def chmod_x(entry, perms): + """Implements chmod, treating all executable bits as set using the chmod + utility's `+X` option. + """ + mode = os.stat(entry).st_mode + if os.path.isfile(entry): + if not mode & (stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH): + perms &= ~stat.S_IXUSR + perms &= ~stat.S_IXGRP + perms &= ~stat.S_IXOTH + os.chmod(entry, perms) + + def copy_mode(src, dest): """Set the mode of dest to that of src unless it is a link. """ @@ -413,12 +432,14 @@ def get_filetype(path_name): return output.strip() -def mkdirp(*paths): +def mkdirp(*paths, **kwargs): """Creates a directory, as well as parent directories if needed.""" + mode = kwargs.get('mode', stat.S_IRWXU | stat.S_IRWXG | stat.S_IRWXO) for path in paths: if not os.path.exists(path): try: - os.makedirs(path) + os.makedirs(path, mode) + os.chmod(path, mode) # For systems that ignore makedirs mode except OSError as e: if e.errno != errno.EEXIST or not os.path.isdir(path): raise e diff --git a/lib/spack/spack/directory_layout.py b/lib/spack/spack/directory_layout.py index 20e6f7d4ed..b8da44ca2a 100644 --- a/lib/spack/spack/directory_layout.py +++ b/lib/spack/spack/directory_layout.py @@ -30,7 +30,7 @@ import re import ruamel.yaml as yaml -from llnl.util.filesystem import mkdirp +from llnl.util.filesystem import mkdirp, chgrp import spack.config import spack.spec @@ -263,7 +263,19 @@ class YamlDirectoryLayout(DirectoryLayout): if prefix: raise InstallDirectoryAlreadyExistsError(prefix) - mkdirp(self.metadata_path(spec)) + # Create install directory with properly configured permissions + # Cannot import at top of file + from spack.package_prefs import get_package_dir_permissions + from spack.package_prefs import get_package_group + group = get_package_group(spec) + perms = get_package_dir_permissions(spec) + mkdirp(spec.prefix, mode=perms) + if group: + chgrp(spec.prefix, group) + # Need to reset the sticky group bit after chgrp + os.chmod(spec.prefix, perms) + + mkdirp(self.metadata_path(spec), mode=perms) self.write_spec(spec, self.spec_file_path(spec)) def check_installed(self, spec): diff --git a/lib/spack/spack/hooks/permissions_setters.py b/lib/spack/spack/hooks/permissions_setters.py new file mode 100644 index 0000000000..c5084652de --- /dev/null +++ b/lib/spack/spack/hooks/permissions_setters.py @@ -0,0 +1,64 @@ +############################################################################## +# Copyright (c) 2013-2018, Lawrence Livermore National Security, LLC. +# Produced at the Lawrence Livermore National Laboratory. +# +# This file is part of Spack. +# Created by Todd Gamblin, tgamblin@llnl.gov, All rights reserved. +# LLNL-CODE-647188 +# +# For details, see https://github.com/spack/spack +# Please also see the NOTICE and LICENSE files for our notice and the LGPL. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU Lesser General Public License (as +# published by the Free Software Foundation) version 2.1, February 1999. +# +# This program is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the IMPLIED WARRANTY OF +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the terms and +# conditions of the GNU Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this program; if not, write to the Free Software +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA +############################################################################## +import os + +from llnl.util.filesystem import chmod_x, chgrp + +from spack.package_prefs import get_package_permissions, get_package_group +from spack.package_prefs import get_package_dir_permissions + + +def forall_files(path, fn, args, dir_args=None): + """Apply function to all files in directory, with file as first arg. + + Does not apply to the root dir. Does not apply to links""" + for root, dirs, files in os.walk(path): + for d in dirs: + if not os.path.islink(os.path.join(root, d)): + if dir_args: + fn(os.path.join(root, d), *dir_args) + else: + fn(os.path.join(root, d), *args) + for f in files: + if not os.path.islink(os.path.join(root, d)): + fn(os.path.join(root, f), *args) + + +def chmod_real_entries(path, perms): + # Don't follow links so we don't change things outside the prefix + if not os.path.islink(path): + chmod_x(path, perms) + + +def post_install(spec): + if not spec.external: + perms = get_package_permissions(spec) + dir_perms = get_package_dir_permissions(spec) + group = get_package_group(spec) + + forall_files(spec.prefix, chmod_real_entries, [perms], [dir_perms]) + + if group: + forall_files(spec.prefix, chgrp, [group]) diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py index 6aa07ed0b9..77c69cbdab 100644 --- a/lib/spack/spack/package.py +++ b/lib/spack/spack/package.py @@ -66,7 +66,7 @@ import spack.util.web import spack.multimethod import spack.binary_distribution as binary_distribution -from llnl.util.filesystem import mkdirp, touch +from llnl.util.filesystem import mkdirp, touch, chgrp from llnl.util.filesystem import working_dir, install_tree, install from llnl.util.lang import memoized from llnl.util.link_tree import LinkTree @@ -78,6 +78,7 @@ from spack.stage import Stage, ResourceStage, StageComposite from spack.util.environment import dump_environment from spack.util.package_hash import package_hash from spack.version import Version +from spack.package_prefs import get_package_dir_permissions, get_package_group """Allowed URL schemes for spack packages.""" _ALLOWED_URL_SCHEMES = ["http", "https", "ftp", "file", "git"] @@ -1527,6 +1528,18 @@ class PackageBase(with_metaclass(PackageMeta, PackageViewMixin, object)): # Create the install prefix and fork the build process. if not os.path.exists(self.prefix): spack.store.layout.create_install_directory(self.spec) + else: + # Set the proper group for the prefix + group = get_package_group(self.spec) + if group: + chgrp(self.prefix, group) + # Set the proper permissions. + # This has to be done after group because changing groups blows + # away the sticky group bit on the directory + mode = os.stat(self.prefix).st_mode + perms = get_package_dir_permissions(self.spec) + if mode != perms: + os.chmod(self.prefix, perms) # Fork a child to do the actual installation # we preserve verbosity settings across installs. diff --git a/lib/spack/spack/package_prefs.py b/lib/spack/spack/package_prefs.py index f8e2499bb1..5768c5700c 100644 --- a/lib/spack/spack/package_prefs.py +++ b/lib/spack/spack/package_prefs.py @@ -22,6 +22,7 @@ # License along with this program; if not, write to the Free Software # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA ############################################################################## +import stat from six import string_types from six import iteritems @@ -31,7 +32,7 @@ import spack.repo import spack.error from spack.util.path import canonicalize_path from spack.version import VersionList - +from spack.config import ConfigError _lesser_spec_types = {'compiler': spack.spec.CompilerSpec, 'version': VersionList} @@ -252,5 +253,79 @@ def is_spec_buildable(spec): return allpkgs[spec.name]['buildable'] +def get_package_dir_permissions(spec): + """Return the permissions configured for the spec. + + Include the GID bit if group permissions are on. This makes the group + attribute sticky for the directory. Package-specific settings take + precedent over settings for ``all``""" + perms = get_package_permissions(spec) + if perms & stat.S_IRWXG: + perms |= stat.S_ISGID + return perms + + +def get_package_permissions(spec): + """Return the permissions configured for the spec. + + Package-specific settings take precedence over settings for ``all``""" + + # Get read permissions level + for name in (spec.name, 'all'): + try: + readable = spack.config.get('packages:%s:permissions:read' % name, + '') + if readable: + break + except AttributeError: + readable = 'world' + + # Get write permissions level + for name in (spec.name, 'all'): + try: + writable = spack.config.get('packages:%s:permissions:write' % name, + '') + if writable: + break + except AttributeError: + writable = 'user' + + perms = stat.S_IRWXU + if readable in ('world', 'group'): # world includes group + perms |= stat.S_IRGRP | stat.S_IXGRP + if readable == 'world': + perms |= stat.S_IROTH | stat.S_IXOTH + + if writable in ('world', 'group'): + if readable == 'user': + raise ConfigError('Writable permissions may not be more' + + ' permissive than readable permissions.\n' + + ' Violating package is %s' % spec.name) + perms |= stat.S_IWGRP + if writable == 'world': + if readable != 'world': + raise ConfigError('Writable permissions may not be more' + + ' permissive than readable permissions.\n' + + ' Violating package is %s' % spec.name) + perms |= stat.S_IWOTH + + return perms + + +def get_package_group(spec): + """Return the unix group associated with the spec. + + Package-specific settings take precedence over settings for ``all``""" + for name in (spec.name, 'all'): + try: + group = spack.config.get('packages:%s:permissions:group' % name, + '') + if group: + break + except AttributeError: + group = '' + return group + + class VirtualInPackagesYAMLError(spack.error.SpackError): """Raised when a disallowed virtual is found in packages.yaml""" diff --git a/lib/spack/spack/schema/packages.py b/lib/spack/spack/schema/packages.py index 56949b7267..f9153a837f 100644 --- a/lib/spack/spack/schema/packages.py +++ b/lib/spack/spack/schema/packages.py @@ -59,6 +59,23 @@ schema = { 'type': 'boolean', 'default': True, }, + 'permissions': { + 'type': 'object', + 'additionalProperties': False, + 'properties': { + 'read': { + 'type': 'string', + 'enum': ['user', 'group', 'world'], + }, + 'write': { + 'type': 'string', + 'enum': ['user', 'group', 'world'], + }, + 'group': { + 'type': 'string', + }, + }, + }, 'modules': { 'type': 'object', 'default': {}, diff --git a/lib/spack/spack/stage.py b/lib/spack/spack/stage.py index 82363f31ee..807a83e5aa 100644 --- a/lib/spack/spack/stage.py +++ b/lib/spack/spack/stage.py @@ -23,6 +23,7 @@ # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA ############################################################################## import os +import stat import sys import errno import hashlib @@ -488,11 +489,13 @@ class Stage(object): if self._need_to_create_path(): tmp_root = get_tmp_root() if tmp_root is not None: + # tempfile.mkdtemp already sets mode 0700 tmp_dir = tempfile.mkdtemp('', _stage_prefix, tmp_root) tty.debug('link %s -> %s' % (self.path, tmp_dir)) os.symlink(tmp_dir, self.path) else: - mkdirp(self.path) + # emulate file permissions for tempfile.mkdtemp + mkdirp(self.path, mode=stat.S_IRWXU) # Make sure we can actually do something with the stage we made. ensure_access(self.path) self.created = True diff --git a/lib/spack/spack/test/concretize_preferences.py b/lib/spack/spack/test/concretize_preferences.py index 6156b53ef0..bcb1f0cc21 100644 --- a/lib/spack/spack/test/concretize_preferences.py +++ b/lib/spack/spack/test/concretize_preferences.py @@ -23,11 +23,12 @@ # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA ############################################################################## import pytest +import stat import spack.package_prefs import spack.repo import spack.util.spack_yaml as syaml -from spack.config import ConfigScope +from spack.config import ConfigScope, ConfigError from spack.spec import Spec @@ -45,6 +46,31 @@ def concretize_scope(config, tmpdir): spack.repo.path._provider_index = None +@pytest.fixture() +def configure_permissions(): + conf = syaml.load("""\ +all: + permissions: + read: group + write: group + group: all +mpich: + permissions: + read: user + write: user +mpileaks: + permissions: + write: user + group: mpileaks +callpath: + permissions: + write: world +""") + spack.config.set('packages', conf, scope='concretize') + + yield + + def concretize(abstract_spec): return Spec(abstract_spec).concretized() @@ -174,3 +200,50 @@ mpich: spec = Spec('mpi') spec.concretize() assert spec['mpich'].external_path == '/dummy/path' + + def test_config_permissions_from_all(self, configure_permissions): + # Although these aren't strictly about concretization, they are + # configured in the same file and therefore convenient to test here. + # Make sure we can configure readable and writable + + # Test inheriting from 'all' + spec = Spec('zmpi') + perms = spack.package_prefs.get_package_permissions(spec) + assert perms == stat.S_IRWXU | stat.S_IRWXG + + dir_perms = spack.package_prefs.get_package_dir_permissions(spec) + assert dir_perms == stat.S_IRWXU | stat.S_IRWXG | stat.S_ISGID + + group = spack.package_prefs.get_package_group(spec) + assert group == 'all' + + def test_config_permissions_from_package(self, configure_permissions): + # Test overriding 'all' + spec = Spec('mpich') + perms = spack.package_prefs.get_package_permissions(spec) + assert perms == stat.S_IRWXU + + dir_perms = spack.package_prefs.get_package_dir_permissions(spec) + assert dir_perms == stat.S_IRWXU + + group = spack.package_prefs.get_package_group(spec) + assert group == 'all' + + def test_config_permissions_differ_read_write(self, configure_permissions): + # Test overriding group from 'all' and different readable/writable + spec = Spec('mpileaks') + perms = spack.package_prefs.get_package_permissions(spec) + assert perms == stat.S_IRWXU | stat.S_IRGRP | stat.S_IXGRP + + dir_perms = spack.package_prefs.get_package_dir_permissions(spec) + expected = stat.S_IRWXU | stat.S_IRGRP | stat.S_IXGRP | stat.S_ISGID + assert dir_perms == expected + + group = spack.package_prefs.get_package_group(spec) + assert group == 'mpileaks' + + def test_config_perms_fail_write_gt_read(self, configure_permissions): + # Test failure for writable more permissive than readable + spec = Spec('callpath') + with pytest.raises(ConfigError): + spack.package_prefs.get_package_permissions(spec) -- cgit v1.2.3-60-g2f50