From 3b115fffb132ee3a247079e8b6043c33707d20c5 Mon Sep 17 00:00:00 2001 From: Greg Becker Date: Tue, 20 Aug 2019 23:08:02 -0700 Subject: permissions: fix file permissions on intermediate install directories (#12399) - mkdirp now takes arguments to allow it to properly set permissions on created directories. - Two arguments (group and mode) set permissions for the leaf directory. - Intermediate directories can inherit permissions from either the topmost existing directory (the parent) or the leaf. --- lib/spack/llnl/util/filesystem.py | 74 +++++++++++++++++++++++++++++-------- lib/spack/spack/directory_layout.py | 20 +++------- 2 files changed, 64 insertions(+), 30 deletions(-) (limited to 'lib') diff --git a/lib/spack/llnl/util/filesystem.py b/lib/spack/llnl/util/filesystem.py index 19feb1686f..752aa0138e 100644 --- a/lib/spack/llnl/util/filesystem.py +++ b/lib/spack/llnl/util/filesystem.py @@ -226,7 +226,10 @@ def group_ids(uid=None): def chgrp(path, group): """Implement the bash chgrp function on a single path""" - gid = grp.getgrnam(group).gr_gid + if isinstance(group, six.string_types): + gid = grp.getgrnam(group).gr_gid + else: + gid = group os.chown(path, -1, gid) @@ -424,6 +427,13 @@ def get_filetype(path_name): return output.strip() +def chgrp_if_not_world_writable(path, group): + """chgrp path to group if path is not world writable""" + mode = os.stat(path).st_mode + if not mode & stat.S_IWOTH: + chgrp(path, group) + + def mkdirp(*paths, **kwargs): """Creates a directory, as well as parent directories if needed. @@ -431,27 +441,38 @@ def mkdirp(*paths, **kwargs): paths (str): paths to create with mkdirp Keyword Aguments: - mode (permission bits or None, optional): optional permissions to - set on the created directory -- use OS default if not provided - mode_intermediate (permission bits or None, optional): - same as mode, but for newly-created intermediate directories + mode (permission bits or None, optional): optional permissions to set + on the created directory -- use OS default if not provided + group (group name or None, optional): optional group for permissions of + final created directory -- use OS default if not provided. Only + used if world write permissions are not set + default_perms ('parents' or 'args', optional): The default permissions + that are set for directories that are not themselves an argument + for mkdirp. 'parents' means intermediate directories get the + permissions of their direct parent directory, 'args' means + intermediate get the same permissions specified in the arguments to + mkdirp -- default value is 'args' """ mode = kwargs.get('mode', None) - mode_intermediate = kwargs.get('mode_intermediate', None) + group = kwargs.get('group', None) + default_perms = kwargs.get('default_perms', 'args') + for path in paths: if not os.path.exists(path): try: + # detect missing intermediate folders intermediate_folders = [] - if mode_intermediate is not None: - # detect missing intermediate folders - intermediate_path = os.path.dirname(path) + last_parent = '' + + intermediate_path = os.path.dirname(path) - while intermediate_path: - if os.path.exists(intermediate_path): - break + while intermediate_path: + if os.path.exists(intermediate_path): + last_parent = intermediate_path + break - intermediate_folders.append(intermediate_path) - intermediate_path = os.path.dirname(intermediate_path) + intermediate_folders.append(intermediate_path) + intermediate_path = os.path.dirname(intermediate_path) # create folders os.makedirs(path) @@ -459,13 +480,36 @@ def mkdirp(*paths, **kwargs): # leaf folder permissions if mode is not None: os.chmod(path, mode) + if group: + chgrp_if_not_world_writable(path, group) + if mode is not None: + os.chmod(path, mode) # reset sticky grp bit post chgrp # for intermediate folders, change mode just for newly created # ones and if mode_intermediate has been specified, otherwise # intermediate folders list is not populated at all and default # OS mode will be used + if default_perms == 'args': + intermediate_mode = mode + intermediate_group = group + elif default_perms == 'parents': + stat_info = os.stat(last_parent) + intermediate_mode = stat_info.st_mode + intermediate_group = stat_info.st_gid + else: + msg = "Invalid value: '%s'. " % default_perms + msg += "Choose from 'args' or 'parents'." + raise ValueError(msg) + for intermediate_path in reversed(intermediate_folders): - os.chmod(intermediate_path, mode_intermediate) + if intermediate_mode is not None: + os.chmod(intermediate_path, intermediate_mode) + if intermediate_group is not None: + chgrp_if_not_world_writable(intermediate_path, + intermediate_group) + os.chmod(intermediate_path, + intermediate_mode) # reset sticky bit after + 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 35a8e4dcc5..01ddd0e1f7 100644 --- a/lib/spack/spack/directory_layout.py +++ b/lib/spack/spack/directory_layout.py @@ -12,7 +12,7 @@ from contextlib import contextmanager import ruamel.yaml as yaml -from llnl.util.filesystem import mkdirp, chgrp +from llnl.util.filesystem import mkdirp import spack.config import spack.spec @@ -256,23 +256,13 @@ class YamlDirectoryLayout(DirectoryLayout): from spack.package_prefs import get_package_group # Each package folder can have its own specific permissions, while - # intermediate folders (arch/compiler) are set with full access to - # everyone (0o777) and install_tree root folder is the chokepoint - # for restricting global access. - # So, whoever has access to the install_tree is allowed to install - # packages for same arch/compiler and since no data is stored in - # intermediate folders, it does not represent a security threat. + # intermediate folders (arch/compiler) are set with access permissions + # equivalent to the root permissions of the layout. group = get_package_group(spec) perms = get_package_dir_permissions(spec) - perms_intermediate = 0o777 - mkdirp(spec.prefix, mode=perms, mode_intermediate=perms_intermediate) - 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) + mkdirp(spec.prefix, mode=perms, group=group, default_perms='parents') + mkdirp(self.metadata_path(spec), mode=perms, group=group) # in prefix self.write_spec(spec, self.spec_file_path(spec)) -- cgit v1.2.3-60-g2f50