Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged

Conversation

barneysowood
Copy link
Contributor

@barneysowood barneysowood commented Apr 29, 2023

What does this PR do?

Updates Debian and RPM packages to improve isolation offered by running the salt-master processes under the salt user.

Implementation of running salt-master as a non-root user by default introduced in #64037, gives ownership of shared salt directories to the salt user to mitigate a number of issues with running the salt-master as a non-root user. This PR aims to reduce the scope of those permission grants to increase isolation of the salt-master processes running under the salt user - see #64193 for more detail.

What issues does this PR fix or reference?

Fixes: #64193

Previous Behavior

The salt user that the salt-master process runs as was able to:

  • Write to /opt/saltstack/salt modifying the salt install, including python, shared libs and python modules used by both the salt master and other salt process (eg salt-api and salt-minion) which run as root, compromising the isolation offered by running the salt-master as a non-root user.
  • Read and write configs in /etc/salt for other salt daemons (that run as root) compromising the isolation offered by running as a non-root user.
  • Reading private data from /etc/salt/pki/minion and /var/cache/salt/minion again compromising isolation.

New Behavior

Debian and RPM packages will now:

  • Keep ownership of /opt/saltstack/salt hierarchy as root:root to prevent modification by salt-master process
  • Bytecompile python packages under /opt/saltstack/salt/lib/ on install so that performance isn't compromised and the bytecompiled modules are owned by root
  • Clean up *.pyc files and __pycache__ dirs under /opt/saltstack/salt/lib on uninstall
  • Under /etc/salt limit ownership to /etc/salt/pki/master and /etc/salt/master.d
  • Under /var/cache/salt and /var/run/salt only give ownership on master directories
  • Under /var/log/salt, ensure /var/log/salt/master exists and give ownership of that. Also update logrotate config to create that with correct ownership and perms and install that on debian packages.

This should ensure that a compromised salt-master process cannot:

  • Modify the salt installation used by it or other salt-daemons
  • Modify the config of other salt daemons or tools
  • Read pki or other private data via the local filesystem for a salt-minion running on the same host

Merge requirements satisfied?

Commits signed with GPG?

No

@barneysowood barneysowood requested a review from a team as a code owner April 29, 2023 19:45
@barneysowood barneysowood requested review from dwoz and removed request for a team April 29, 2023 19:45
@salt-project-bot-prod-environment salt-project-bot-prod-environment bot changed the title Update 3006 packaging to reduce permissions given to salt user for running salt-master [master] Update 3006 packaging to reduce permissions given to salt user for running salt-master Apr 29, 2023
@barneysowood barneysowood temporarily deployed to ci April 29, 2023 20:02 — with GitHub Actions Inactive
@barneysowood barneysowood temporarily deployed to ci April 29, 2023 20:02 — with GitHub Actions Inactive
@barneysowood barneysowood temporarily deployed to ci April 29, 2023 20:02 — with GitHub Actions Inactive
@dwoz
Copy link
Contributor

dwoz commented Apr 29, 2023

This is going to be affected by #64174

@barneysowood barneysowood temporarily deployed to ci April 29, 2023 20:59 — with GitHub Actions Inactive
@barneysowood barneysowood temporarily deployed to ci April 29, 2023 20:59 — with GitHub Actions Inactive
@barneysowood barneysowood temporarily deployed to ci April 29, 2023 20:59 — with GitHub Actions Inactive
@barneysowood barneysowood temporarily deployed to ci April 29, 2023 22:03 — with GitHub Actions Inactive
@barneysowood barneysowood temporarily deployed to ci April 29, 2023 22:03 — with GitHub Actions Inactive
@barneysowood barneysowood temporarily deployed to ci April 29, 2023 22:03 — with GitHub Actions Inactive
twangboy
twangboy previously approved these changes May 1, 2023
@twangboy twangboy requested a review from dmurphy18 May 1, 2023 23:49
@barneysowood barneysowood marked this pull request as draft May 2, 2023 15:17
@barneysowood
Copy link
Contributor Author

As discussed in #64174, will wait for that to be merged then re-base this and integrate on top of those changes.
Also need to add some tests.

@barneysowood
Copy link
Contributor Author

Note that issues in #64219 will affect this.

@barneysowood barneysowood changed the base branch from master to 3006.x May 12, 2023 13:07
@salt-project-bot-prod-environment salt-project-bot-prod-environment bot changed the title [master] Update 3006 packaging to reduce permissions given to salt user for running salt-master [3006.x] Update 3006 packaging to reduce permissions given to salt user for running salt-master May 12, 2023
@barneysowood barneysowood temporarily deployed to ci May 12, 2023 13:18 — with GitHub Actions Inactive
@barneysowood barneysowood temporarily deployed to ci May 12, 2023 13:18 — with GitHub Actions Inactive
@barneysowood barneysowood temporarily deployed to ci May 12, 2023 13:19 — with GitHub Actions Inactive
@barneysowood barneysowood temporarily deployed to ci May 12, 2023 14:23 — with GitHub Actions Inactive
@barneysowood barneysowood temporarily deployed to ci August 16, 2023 20:07 — with GitHub Actions Inactive
@barneysowood barneysowood temporarily deployed to ci August 16, 2023 20:07 — with GitHub Actions Inactive
@barneysowood barneysowood temporarily deployed to ci August 16, 2023 20:07 — with GitHub Actions Inactive
@barneysowood barneysowood temporarily deployed to ci August 16, 2023 20:07 — with GitHub Actions Inactive
@barneysowood barneysowood temporarily deployed to ci August 16, 2023 20:07 — with GitHub Actions Inactive
@barneysowood barneysowood temporarily deployed to ci August 16, 2023 20:07 — with GitHub Actions Inactive
@Ch3LL Ch3LL requested a review from s0undt3ch August 16, 2023 20:25
@MKLeb MKLeb temporarily deployed to ci August 16, 2023 20:43 — with GitHub Actions Inactive
@MKLeb MKLeb temporarily deployed to ci August 16, 2023 20:43 — with GitHub Actions Inactive
@MKLeb MKLeb temporarily deployed to ci August 16, 2023 20:43 — with GitHub Actions Inactive
@MKLeb MKLeb temporarily deployed to ci August 16, 2023 20:43 — with GitHub Actions Inactive
@MKLeb MKLeb temporarily deployed to ci August 16, 2023 21:01 — with GitHub Actions Inactive
@MKLeb MKLeb temporarily deployed to ci August 16, 2023 21:04 — with GitHub Actions Inactive
@MKLeb MKLeb temporarily deployed to ci August 16, 2023 21:47 — with GitHub Actions Inactive
@MKLeb MKLeb temporarily deployed to ci August 16, 2023 21:47 — with GitHub Actions Inactive
@MKLeb MKLeb temporarily deployed to ci August 16, 2023 21:47 — with GitHub Actions Inactive
@MKLeb MKLeb temporarily deployed to ci August 16, 2023 21:47 — with GitHub Actions Inactive
@MKLeb MKLeb temporarily deployed to ci August 16, 2023 21:47 — with GitHub Actions Inactive
@MKLeb MKLeb temporarily deployed to ci August 16, 2023 21:47 — with GitHub Actions Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] 3006.0 Debian and RPM salt-master package creates insecure permissions for salt user
8 participants