[3006.x] Update 3006 packaging to reduce permissions given to salt user for running salt-master

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.

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 {

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

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

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

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

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

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

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

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

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

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")])

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)