Skip to content

Commit

Permalink
Merge branch '3006.x' into fix_64625
Browse files Browse the repository at this point in the history
  • Loading branch information
dmurphy18 authored Aug 16, 2023
2 parents a9237f5 + 0a53cf3 commit f0bd758
Show file tree
Hide file tree
Showing 14 changed files with 214 additions and 35 deletions.
6 changes: 6 additions & 0 deletions changelog/64193.fixed.md
Original file line number Diff line number Diff line change
@@ -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.
3 changes: 3 additions & 0 deletions pkg/common/salt-common.logrotate
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
rotate 7
compress
notifempty
create 0640 salt salt
}

/var/log/salt/minion {
Expand All @@ -20,6 +21,7 @@
rotate 7
compress
notifempty
create 0640 salt salt
}

/var/log/salt/api {
Expand All @@ -28,6 +30,7 @@
rotate 7
compress
notifempty
create 0640 salt salt
}

/var/log/salt/syndic {
Expand Down
10 changes: 10 additions & 0 deletions pkg/debian/salt-api.postinst
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions pkg/debian/salt-common.install
Original file line number Diff line number Diff line change
@@ -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
Expand Down
4 changes: 4 additions & 0 deletions pkg/debian/salt-common.postinst
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
#!/bin/sh
set -e

/opt/saltstack/salt/bin/python3 -m compileall -qq /opt/saltstack/salt/lib
6 changes: 0 additions & 6 deletions pkg/debian/salt-common.preinst
Original file line number Diff line number Diff line change
Expand Up @@ -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
5 changes: 5 additions & 0 deletions pkg/debian/salt-common.prerm
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions pkg/debian/salt-master.dirs
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,4 @@
/var/cache/salt/master/roots
/var/cache/salt/master/syndics
/var/cache/salt/master/tokens
/var/run/salt/master
10 changes: 9 additions & 1 deletion pkg/debian/salt-master.postinst
Original file line number Diff line number Diff line change
@@ -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
14 changes: 14 additions & 0 deletions pkg/debian/salt-master.preinst
Original file line number Diff line number Diff line change
@@ -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
55 changes: 48 additions & 7 deletions pkg/rpm/salt.spec
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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/
Expand Down Expand Up @@ -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
Expand All @@ -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


Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
17 changes: 12 additions & 5 deletions pkg/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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")])
Expand Down
16 changes: 15 additions & 1 deletion pkg/tests/integration/test_pip.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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)

Expand Down
Loading

0 comments on commit f0bd758

Please sign in to comment.