diff --git a/changelog/64193.fixed.md b/changelog/64193.fixed.md new file mode 100644 index 000000000000..d7b6ebaff81d --- /dev/null +++ b/changelog/64193.fixed.md @@ -0,0 +1,6 @@ +Fixes permissions created by the Debian and RPM packages for the salt user. + +The salt user created by the Debian and RPM packages to run the salt-master process, was previously given ownership of various directories in a way which compromised the benefits of running the salt-master process as a non-root user. + +This fix sets the salt user to only have write access to those files and +directories required for the salt-master process to run. diff --git a/pkg/common/salt-common.logrotate b/pkg/common/salt-common.logrotate index a0306ff37024..1bc063ebfdb9 100644 --- a/pkg/common/salt-common.logrotate +++ b/pkg/common/salt-common.logrotate @@ -4,6 +4,7 @@ rotate 7 compress notifempty + create 0640 salt salt } /var/log/salt/minion { @@ -20,6 +21,7 @@ rotate 7 compress notifempty + create 0640 salt salt } /var/log/salt/api { @@ -28,6 +30,7 @@ rotate 7 compress notifempty + create 0640 salt salt } /var/log/salt/syndic { diff --git a/pkg/debian/salt-api.postinst b/pkg/debian/salt-api.postinst new file mode 100644 index 000000000000..9345d72bf2aa --- /dev/null +++ b/pkg/debian/salt-api.postinst @@ -0,0 +1,10 @@ +case "$1" in + configure) + if [ ! -e "/var/log/salt/api" ]; then + touch /var/log/salt/api + chmod 640 /var/log/salt/api + fi + chown salt:salt /var/log/salt/api + if command -v systemctl; then systemctl enable salt-api; fi + ;; +esac diff --git a/pkg/debian/salt-common.install b/pkg/debian/salt-common.install index 7e88a78ee3ad..0671a987c207 100644 --- a/pkg/debian/salt-common.install +++ b/pkg/debian/salt-common.install @@ -1,5 +1,6 @@ conf/roster /etc/salt conf/cloud /etc/salt +pkg/common/salt-common.logrotate /etc/logrotate.d/salt pkg/common/fish-completions/salt-cp.fish /usr/share/fish/vendor_completions.d pkg/common/fish-completions/salt-call.fish /usr/share/fish/vendor_completions.d pkg/common/fish-completions/salt-syndic.fish /usr/share/fish/vendor_completions.d diff --git a/pkg/debian/salt-common.postinst b/pkg/debian/salt-common.postinst new file mode 100644 index 000000000000..c5a8d969b450 --- /dev/null +++ b/pkg/debian/salt-common.postinst @@ -0,0 +1,4 @@ +#!/bin/sh +set -e + +/opt/saltstack/salt/bin/python3 -m compileall -qq /opt/saltstack/salt/lib diff --git a/pkg/debian/salt-common.preinst b/pkg/debian/salt-common.preinst index 4c2b576cc98c..0e4ce9c59f7f 100644 --- a/pkg/debian/salt-common.preinst +++ b/pkg/debian/salt-common.preinst @@ -31,11 +31,5 @@ case "$1" in -s $SALT_SHELL \ -g $SALT_GROUP \ $SALT_USER - # 5. adjust file and directory permissions - if ! dpkg-statoverride --list $SALT_HOME >/dev/null - then - chown -R $SALT_USER:$SALT_GROUP $SALT_HOME - chmod u=rwx,g=rwx,o=rx $SALT_HOME - fi ;; esac diff --git a/pkg/debian/salt-common.prerm b/pkg/debian/salt-common.prerm new file mode 100644 index 000000000000..236c2bd3d12d --- /dev/null +++ b/pkg/debian/salt-common.prerm @@ -0,0 +1,5 @@ +#!/bin/sh +set -e + +dpkg -L salt-common | perl -ne 's,/([^/]*)\.py$,/__pycache__/\1.*, or next; unlink $_ or die $! foreach glob($_)' +find /opt/saltstack/salt -type d -name __pycache__ -empty -print0 | xargs --null --no-run-if-empty rmdir diff --git a/pkg/debian/salt-master.dirs b/pkg/debian/salt-master.dirs index cffed208e63a..542db04259fb 100644 --- a/pkg/debian/salt-master.dirs +++ b/pkg/debian/salt-master.dirs @@ -13,3 +13,4 @@ /var/cache/salt/master/roots /var/cache/salt/master/syndics /var/cache/salt/master/tokens +/var/run/salt/master diff --git a/pkg/debian/salt-master.postinst b/pkg/debian/salt-master.postinst index 1c78ee734788..4f7686d8ed9c 100644 --- a/pkg/debian/salt-master.postinst +++ b/pkg/debian/salt-master.postinst @@ -1,6 +1,14 @@ case "$1" in configure) - chown -R salt:salt /etc/salt /var/log/salt /opt/saltstack/salt/ /var/cache/salt/ /var/run/salt + if [ ! -e "/var/log/salt/master" ]; then + touch /var/log/salt/master + chmod 640 /var/log/salt/master + fi + if [ ! -e "/var/log/salt/key" ]; then + touch /var/log/salt/key + chmod 640 /var/log/salt/key + fi + chown -R salt:salt /etc/salt/pki/master /etc/salt/master.d /var/log/salt/master /var/log/salt/key /var/cache/salt/master /var/run/salt/master if command -v systemctl; then systemctl enable salt-master; fi ;; esac diff --git a/pkg/debian/salt-master.preinst b/pkg/debian/salt-master.preinst new file mode 100644 index 000000000000..3a00b757eeb7 --- /dev/null +++ b/pkg/debian/salt-master.preinst @@ -0,0 +1,14 @@ +case "$1" in + install|upgrade) + [ -z "$SALT_HOME" ] && SALT_HOME=/opt/saltstack/salt + [ -z "$SALT_USER" ] && SALT_USER=salt + [ -z "$SALT_NAME" ] && SALT_NAME="Salt" + [ -z "$SALT_GROUP" ] && SALT_GROUP=salt + PY_VER=$(/opt/saltstack/salt/bin/python3 -c "import sys; sys.stdout.write('{}.{}'.format(*sys.version_info)); sys.stdout.flush();") + + # Reset permissions to fix previous installs + find ${SALT_HOME} /etc/salt /var/log/salt /var/cache/salt /var/run/salt \ + \! \( -path /etc/salt/cloud.deploy.d\* -o -path /var/log/salt/cloud -o -path /opt/saltstack/salt/lib/python${PY_VER}/site-packages/salt/cloud/deploy\* \) -a \ + \( -user ${SALT_USER} -o -group ${SALT_GROUP} \) -exec chown root:root \{\} \; + ;; +esac diff --git a/pkg/rpm/salt.spec b/pkg/rpm/salt.spec index f3cae2816050..1d48c30a7bff 100644 --- a/pkg/rpm/salt.spec +++ b/pkg/rpm/salt.spec @@ -194,6 +194,7 @@ cp -R $RPM_BUILD_DIR/build/salt %{buildroot}/opt/saltstack/ # Add some directories install -d -m 0755 %{buildroot}%{_var}/log/salt install -d -m 0755 %{buildroot}%{_var}/run/salt +install -d -m 0755 %{buildroot}%{_var}/run/salt/master install -d -m 0755 %{buildroot}%{_var}/cache/salt install -Dd -m 0750 %{buildroot}%{_var}/cache/salt/master install -Dd -m 0750 %{buildroot}%{_var}/cache/salt/minion @@ -328,6 +329,7 @@ rm -rf %{buildroot} %dir %attr(0750, salt, salt) %{_sysconfdir}/salt/pki/master/minions_denied/ %dir %attr(0750, salt, salt) %{_sysconfdir}/salt/pki/master/minions_pre/ %dir %attr(0750, salt, salt) %{_sysconfdir}/salt/pki/master/minions_rejected/ +%dir %attr(0750, salt, salt) %{_var}/run/salt/master/ %dir %attr(0750, salt, salt) %{_var}/cache/salt/master/ %dir %attr(0750, salt, salt) %{_var}/cache/salt/master/jobs/ %dir %attr(0750, salt, salt) %{_var}/cache/salt/master/proc/ @@ -406,8 +408,14 @@ usermod -c "%{_SALT_NAME}" \ -d %{_SALT_HOME} \ -g %{_SALT_GROUP} \ %{_SALT_USER} -# 5. adjust file and directory permissions -chown -R %{_SALT_USER}:%{_SALT_GROUP} %{_SALT_HOME} + +%pre master +# Reset permissions to fix previous installs +PY_VER=$(/opt/saltstack/salt/bin/python3 -c "import sys; sys.stdout.write('{}.{}'.format(*sys.version_info)); sys.stdout.flush();") +find /etc/salt /opt/saltstack/salt /var/log/salt /var/cache/salt /var/run/salt \ + \! \( -path /etc/salt/cloud.deploy.d\* -o -path /var/log/salt/cloud -o -path /opt/saltstack/salt/lib/python${PY_VER}/site-packages/salt/cloud/deploy\* \) -a \ + \( -user salt -o -group salt \) -exec chown root:root \{\} \; + # assumes systemd for RHEL 7 & 8 & 9 %preun master @@ -424,15 +432,12 @@ chown -R %{_SALT_USER}:%{_SALT_GROUP} %{_SALT_HOME} %post -chown -R %{_SALT_USER}:%{_SALT_GROUP} %{_SALT_HOME} -chmod u=rwx,g=rwx,o=rx %{_SALT_HOME} ln -s -f /opt/saltstack/salt/spm %{_bindir}/spm ln -s -f /opt/saltstack/salt/salt-pip %{_bindir}/salt-pip +/opt/saltstack/salt/bin/python3 -m compileall -qq /opt/saltstack/salt/lib %post cloud -chown -R salt:salt /etc/salt/cloud.deploy.d -chown -R salt:salt /opt/saltstack/salt/lib/python3.10/site-packages/salt/cloud/deploy ln -s -f /opt/saltstack/salt/salt-cloud %{_bindir}/salt-cloud @@ -452,7 +457,6 @@ if [ $1 -lt 2 ]; then /bin/openssl sha256 -r -hmac orboDeJITITejsirpADONivirpUkvarP /opt/saltstack/salt/lib/libcrypto.so.1.1 | cut -d ' ' -f 1 > /opt/saltstack/salt/lib/.libcrypto.so.1.1.hmac || : fi fi -chown -R salt:salt /etc/salt /var/log/salt /opt/saltstack/salt/ /var/cache/salt/ /var/run/salt/ %post syndic %systemd_post salt-syndic.service @@ -480,6 +484,43 @@ ln -s -f /opt/saltstack/salt/salt-ssh %{_bindir}/salt-ssh %systemd_post salt-api.service ln -s -f /opt/saltstack/salt/salt-api %{_bindir}/salt-api + +%posttrans cloud +PY_VER=$(/opt/saltstack/salt/bin/python3 -c "import sys; sys.stdout.write('{}.{}'.format(*sys.version_info)); sys.stdout.flush();") +if [ ! -e "/var/log/salt/cloud" ]; then + touch /var/log/salt/cloud + chmod 640 /var/log/salt/cloud +fi +chown -R %{_SALT_USER}:%{_SALT_GROUP} /etc/salt/cloud.deploy.d /var/log/salt/cloud /opt/saltstack/salt/lib/python${PY_VER}/site-packages/salt/cloud/deploy + + +%posttrans master +if [ ! -e "/var/log/salt/master" ]; then + touch /var/log/salt/master + chmod 640 /var/log/salt/master +fi +if [ ! -e "/var/log/salt/key" ]; then + touch /var/log/salt/key + chmod 640 /var/log/salt/key +fi +chown -R %{_SALT_USER}:%{_SALT_GROUP} /etc/salt/pki/master /etc/salt/master.d /var/log/salt/master /var/log/salt/key /var/cache/salt/master /var/run/salt/master + + +%posttrans api +if [ ! -e "/var/log/salt/api" ]; then + touch /var/log/salt/api + chmod 640 /var/log/salt/api +fi +chown %{_SALT_USER}:%{_SALT_GROUP} /var/log/salt/api + + +%preun +if [ $1 -eq 0 ]; then + # Uninstall + find /opt/saltstack/salt -type f -name \*\.pyc -print0 | xargs --null --no-run-if-empty rm + find /opt/saltstack/salt -type d -name __pycache__ -empty -print0 | xargs --null --no-run-if-empty rmdir +fi + %postun master %systemd_postun_with_restart salt-master.service if [ $1 -eq 0 ]; then diff --git a/pkg/tests/conftest.py b/pkg/tests/conftest.py index b7d908f32d58..257f4615fb32 100644 --- a/pkg/tests/conftest.py +++ b/pkg/tests/conftest.py @@ -352,10 +352,10 @@ def salt_master(salt_factories, install_salt, state_tree, pillar_tree): config_overrides["api_pidfile"] = salt.config.DEFAULT_API_OPTS.get( "api_pidfile" ) - # verify files where set with correct owner/group + # verify files were set with correct owner/group verify_files = [ - pathlib.Path("/var", "log", "salt"), - pathlib.Path("/etc", "salt", "master"), + pathlib.Path("/etc", "salt", "pki", "master"), + pathlib.Path("/etc", "salt", "master.d"), pathlib.Path("/var", "cache", "salt", "master"), ] for _file in verify_files: @@ -410,10 +410,17 @@ def salt_master(salt_factories, install_salt, state_tree, pillar_tree): factory.after_terminate(pytest.helpers.remove_stale_master_key, factory) if test_user: # Salt factories calls salt.utils.verify.verify_env - # which sets root perms on /var/log/salt since we are running + # which sets root perms on /etc/salt/pki/master since we are running # the test suite as root, but we want to run Salt master as salt # We ensure those permissions where set by the package earlier - shutil.chown(pathlib.Path("/var", "log", "salt"), "salt", "salt") + subprocess.run( + [ + "chown", + "-R", + "salt:salt", + str(pathlib.Path("/etc", "salt", "pki", "master")), + ] + ) # The engines_dirs is created in .nox path. We need to set correct perms # for the user running the Salt Master subprocess.run(["chown", "-R", "salt:salt", str(CODE_DIR.parent / ".nox")]) diff --git a/pkg/tests/integration/test_pip.py b/pkg/tests/integration/test_pip.py index 7037763064ec..eb4b5c4896e3 100644 --- a/pkg/tests/integration/test_pip.py +++ b/pkg/tests/integration/test_pip.py @@ -29,7 +29,19 @@ def wipe_pydeps(shell, install_salt, extras_pypath): shell.run( *(install_salt.binary_paths["pip"] + ["uninstall", "-y", dep]), ) - shutil.rmtree(extras_pypath, ignore_errors=True) + # Let's remove everything under the extras directory, uninstalling doesn't get dependencies + dirs = [] + files = [] + for filename in extras_pypath.glob("**/**"): + if filename != extras_pypath and filename.exists(): + if filename.is_dir(): + dirs.append(filename) + else: + files.append(filename) + for fp in files: + fp.unlink() + for dirname in dirs: + shutil.rmtree(dirname, ignore_errors=True) def test_pip_install(salt_call_cli): @@ -88,6 +100,8 @@ def test_pip_install_extras(shell, install_salt, extras_pypath_bin): def demote(user_uid, user_gid): def result(): + # os.setgid does not remove group membership, so we remove them here so they are REALLY non-root + os.setgroups([]) os.setgid(user_gid) os.setuid(user_uid) diff --git a/pkg/tests/integration/test_salt_user.py b/pkg/tests/integration/test_salt_user.py index 8b6b0dcd42c8..5e1ee05e70d6 100644 --- a/pkg/tests/integration/test_salt_user.py +++ b/pkg/tests/integration/test_salt_user.py @@ -1,3 +1,4 @@ +import os import pathlib import subprocess import sys @@ -13,6 +14,53 @@ ] +@pytest.fixture +def pkg_paths(): + """ + Paths created by package installs + """ + paths = [ + "/etc/salt", + "/var/cache/salt", + "/var/log/salt", + "/var/run/salt", + "/opt/saltstack/salt", + ] + return paths + + +@pytest.fixture +def pkg_paths_salt_user(): + """ + Paths created by package installs and owned by salt user + """ + return [ + "/etc/salt/cloud.deploy.d", + "/var/log/salt/cloud", + "/opt/saltstack/salt/lib/python{}.{}/site-packages/salt/cloud/deploy".format( + *sys.version_info + ), + "/etc/salt/pki/master", + "/etc/salt/master.d", + "/var/log/salt/master", + "/var/log/salt/api", + "/var/log/salt/key", + "/var/cache/salt/master", + "/var/run/salt/master", + ] + + +@pytest.fixture +def pkg_paths_salt_user_exclusions(): + """ + Exclusions from paths created by package installs and owned by salt user + """ + paths = [ + "/var/cache/salt/master/.root_key" # written by salt, salt-run and salt-key as root + ] + return paths + + def test_salt_user_master(salt_master, install_salt): """ Test the correct user is running the Salt Master @@ -27,7 +75,7 @@ def test_salt_user_master(salt_master, install_salt): def test_salt_user_home(install_salt): """ - Test the correct user is running the Salt Master + Test the salt user's home is /opt/saltstack/salt """ proc = subprocess.run( ["getent", "passwd", "salt"], check=False, capture_output=True @@ -43,7 +91,7 @@ def test_salt_user_home(install_salt): def test_salt_user_group(install_salt): """ - Test the salt user is the salt group + Test the salt user is in the salt group """ proc = subprocess.run(["id", "salt"], check=False, capture_output=True) assert proc.returncode == 0 @@ -75,18 +123,41 @@ def test_salt_user_shell(install_salt): assert shell_exists is True -def test_salt_cloud_dirs(install_salt): +def test_pkg_paths( + install_salt, pkg_paths, pkg_paths_salt_user, pkg_paths_salt_user_exclusions +): """ - Test the correct user is running the Salt Master + Test package paths ownership """ - paths = [ - "/opt/saltstack/salt/lib/python{}.{}/site-packages/salt/cloud/deploy".format( - *sys.version_info - ), - "/etc/salt/cloud.deploy.d", - ] - for name in paths: - path = pathlib.Path(name) - assert path.exists() - assert path.owner() == "salt" - assert path.group() == "salt" + salt_user_subdirs = [] + for _path in pkg_paths: + pkg_path = pathlib.Path(_path) + assert pkg_path.exists() + for dirpath, sub_dirs, files in os.walk(pkg_path): + path = pathlib.Path(dirpath) + # Directories owned by salt:salt or their subdirs/files + if ( + str(path) in pkg_paths_salt_user or str(path) in salt_user_subdirs + ) and str(path) not in pkg_paths_salt_user_exclusions: + assert path.owner() == "salt" + assert path.group() == "salt" + salt_user_subdirs.extend( + [str(path.joinpath(sub_dir)) for sub_dir in sub_dirs] + ) + # Individual files owned by salt user + for file in files: + file_path = path.joinpath(file) + if str(file_path) not in pkg_paths_salt_user_exclusions: + assert file_path.owner() == "salt" + # Directories owned by root:root + else: + assert path.owner() == "root" + assert path.group() == "root" + for file in files: + file_path = path.joinpath(file) + # Individual files owned by salt user + if str(file_path) in pkg_paths_salt_user: + assert file_path.owner() == "salt" + else: + assert file_path.owner() == "root" + assert file_path.group() == "root"