From d1a5113cfed76fb8d4935bb90f83560f231942cb Mon Sep 17 00:00:00 2001
From: Greg Becker <becker33@llnl.gov>
Date: Thu, 11 Oct 2018 14:29:07 -0700
Subject: [PATCH] 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
---
etc/spack/defaults/packages.yaml | 4 +-
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 +-
.../spack/test/concretize_preferences.py | 75 +++++++++++++++++-
10 files changed, 338 insertions(+), 9 deletions(-)
create mode 100644 lib/spack/spack/hooks/permissions_setters.py
diff --git a/etc/spack/defaults/packages.yaml b/etc/spack/defaults/packages.yaml
index 961fb157f0..5c38ddb683 100644
--- a/etc/spack/defaults/packages.yaml
+++ b/etc/spack/defaults/packages.yaml
@@ -40,4 +40,6 @@ packages:
szip: [libszip, libaec]
tbb: [intel-tbb]
unwind: [libunwind]
-
+ permissions:
+ read: world
+ write: user
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 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 @@ def create_install_directory(self, spec):
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.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.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 @@ def build_process():
# 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.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 @@
'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 @@ def create(self):
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 @@ def test_external_mpi(self):
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)
--
GitLab