From 098699613eceff9a1f156d91ded05e247adacd2e Mon Sep 17 00:00:00 2001 From: Josh Patterson Date: Thu, 17 Sep 2026 12:43:37 -0400 Subject: [PATCH] FIX: pip install wheels from a root-owned directory Root ran `pip install --find-links` against directories under /opt/so/conf, which is 939:939 mode 770. Hardening those directories would not have helped: renaming an entry requires write permission on the parent, not on the entry, so uid 939 could move the wheel tree aside and substitute its own between the file.recurse that populates it and the pip install that reads it. clean: True plus the onchanges requisite narrowed that to a race rather than a straight win, but the window is real and the install runs as root. Move both wheel trees to /opt/saltstack, which is created by the salt package and is root-owned, and state the ownership explicitly rather than relying on the files being new: /opt/so/conf/salt/module_packages/docker -> /opt/saltstack/module_packages/docker /opt/so/conf/libvirt/source-packages/libvirt-python -> /opt/saltstack/source-packages/libvirt-python salt.python_modules is included by salt/salt/minion/init.sls, so the docker wheels are staged on every minion in the grid, not just the manager. Upgraded installs keep a now-unused wheel tree in the old socore-writable location, so remove it -- nothing reads it after this change, but leaving a writable staging directory behind serves no purpose. --- salt/libvirt/packages.sls | 15 +++++++++++++-- salt/salt/python_modules.sls | 19 +++++++++++++++++-- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/salt/libvirt/packages.sls b/salt/libvirt/packages.sls index d0de3d81f..a5d41d7c8 100644 --- a/salt/libvirt/packages.sls +++ b/salt/libvirt/packages.sls @@ -29,16 +29,27 @@ install_libvirt-libs: pkg.installed: - name: libvirt-libs +# Root pip-installs these below, so they cannot live under /opt/so/conf (939:939 mode 770): +# write permission on that directory lets uid 939 swap the tree between this state and the +# install. /opt/saltstack is root-owned, so the same trick does not work there. libvirt_python_wheel: file.recurse: - - name: /opt/so/conf/libvirt/source-packages/libvirt-python + - name: /opt/saltstack/source-packages/libvirt-python - source: salt://libvirt/source-packages/libvirt-python + - user: root + - group: root + - dir_mode: 755 + - file_mode: 644 - makedirs: True - clean: True +old_libvirt_python_wheel: + file.absent: + - name: /opt/so/conf/libvirt/source-packages + libvirt_python_module: cmd.run: - - name: /opt/saltstack/salt/bin/python3 -m pip install --no-index --find-links=/opt/so/conf/libvirt/source-packages/libvirt-python libvirt-python + - name: /opt/saltstack/salt/bin/python3 -m pip install --no-index --find-links=/opt/saltstack/source-packages/libvirt-python libvirt-python - onchanges: - file: libvirt_python_wheel diff --git a/salt/salt/python_modules.sls b/salt/salt/python_modules.sls index d6c05a892..efd3dcbe6 100644 --- a/salt/salt/python_modules.sls +++ b/salt/salt/python_modules.sls @@ -3,19 +3,34 @@ # https://securityonion.net/license; you may not use this file except in compliance with the # Elastic License 2.0. +# These wheels are pip-installed by root below, so they cannot live under /opt/so/conf: +# that directory is 939:939 mode 770, and write permission on it is what lets uid 939 +# rename the tree aside and substitute its own wheels between this state and the install. +# Hardening only the files here would not help -- renaming an entry needs write on the +# parent, not on the entry. docker_module_package: file.recurse: - - name: /opt/so/conf/salt/module_packages/docker + - name: /opt/saltstack/module_packages/docker - source: salt://salt/module_packages/docker + - user: root + - group: root + - dir_mode: 755 + - file_mode: 644 - clean: True - makedirs: True +# Installs before this change left wheels in a socore-writable directory; nothing reads +# them now, but leaving them behind leaves a writable staging area lying around. +old_docker_module_package: + file.absent: + - name: /opt/so/conf/salt/module_packages + # fail hard on this state so that soup would be cancelled on a manager (eventhough salt would have already updated) # on a non manager, failing hard here will prevent the minion from upgrading # we want to fail hard here to prevent the minion from upgrading and potetially being able to manager docker containers from a dep mismatch docker_python_module_install: cmd.run: - - name: /opt/saltstack/salt/bin/python3.10 -m pip install docker --no-index --find-links=/opt/so/conf/salt/module_packages/docker/ --upgrade + - name: /opt/saltstack/salt/bin/python3.10 -m pip install docker --no-index --find-links=/opt/saltstack/module_packages/docker/ --upgrade - onchanges: - file: docker_module_package - failhard: True