This repository was archived by the owner on Jan 24, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 41
SEP: Constrained Backends #71
Open
monkz
wants to merge
3
commits into
saltstack:master
Choose a base branch
from
monkz:master
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,122 @@ | ||
- Feature Name: constrained backends | ||
- Start Date: 2023-06-23 | ||
- SEP Status: Draft | ||
- SEP PR: (leave this empty) | ||
- Salt Issue: (leave this empty) | ||
|
||
# Summary | ||
[summary]: #summary | ||
|
||
Apply constraints to individual backends to allow collaboration with "foreign" backends. | ||
|
||
# Motivation | ||
[motivation]: #motivation | ||
|
||
Why are we doing this? What use cases does it support? What is the expected outcome? | ||
|
||
This will open parts of the configuration for other developers (maybe in another department or even external) to modify, but without giving them root rights, even if the minion runs as root. | ||
Currently the publisher_acl is only applied via the salt api - but if a state enters the salt master via a (git) backend, there is no constraint checking, allowing anyone that can change an backend to basically change almost everything, if not everything, within reach of the minion process. | ||
For example: This hinders to allow "foreign" git repositories to be used as backend without extensive merge approval process. (supply chain attacks) | ||
|
||
In the end, an application administrator could contribute states / functions / ... that are specific to a software and enable more complex orchestrations while having the benefits of auto state ordering, without having a merge approver to know everything about every application to spot (unintentional or intentional) malicious changes and an expert keeps being an expert for just one/assigned scope/s, without the need to be trusted with everything. | ||
|
||
While scoping backends inside a host are beneficial for security, they also can have the benefit of being able to use state auto ordering and depending upon each other, maybe even calling functions published by other repositories, without the need of defining them themselves. | ||
|
||
# Design | ||
[design]: #detailed-design | ||
|
||
This is the bulk of the SEP. Explain the design in enough detail for somebody familiar | ||
with the product to understand, and for somebody familiar with the internals to implement. It should include: | ||
|
||
- Definition of any new terminology | ||
- "foreign" backend: a backend that can be modified by other users than the salt master administrator. | ||
- Examples of how the feature is used: | ||
- Target minions on backends | ||
Users able to change webserver.git aren't always the same users that should be able to modify backupservers. | ||
Basically every minion can have a view on an environment based on the targeting. An overlay might not be applied if the conditions aren't met. | ||
- Apply these states constrained to be applied as user x / only with a certain function set. | ||
- This and more could be accomplished by forcing a render pipeline, that acts as gateway that wraps the renderers for unprivileged backends with another renderer, that applies/filters parts by dropping privileges. Or simply raise an exception to stop further processing of a configuration state. | ||
- Allow just a certain list of renderers. | ||
- Some renderers (like py) might have more capabilities than others to try to circumvent scopes. | ||
|
||
|
||
``` | ||
gitfs_remotes: | ||
- https://foo.com/main.git | ||
- https://foo.com/webserver.git: | ||
- target: web-group-a\* | ||
- renderer-prepend: malware_scan | ||
- renderer-append: force_user_x | ||
- renderer-allow: | ||
- jinja|yaml | ||
- https://foo.com/webserver.git: | ||
- target: web-group-b\* | ||
- renderer-append: force_user_y | ||
- renderer-allow: | ||
- jinja|yaml | ||
- py | ||
- https://foo.com/backups.git: | ||
- target: backup\* | ||
``` | ||
|
||
The last part, is still unpolished: | ||
* Render (some) state files in a sandbox. | ||
* Rendering itself can be harmful on the salt minion user, as the renderer (for example: py) might get access to data or write data as the minion user that is not in scope for the targeted user, like the minion keys. Restricting the information that is available/accessable/modifyable. | ||
* The minion forks itself (or starts a subminion process) to render the statefile as a user or even in a container/jail/... environment. The parent process acts as proxy/filter to let subminions gather data from pillars/etc and communicate upwards to the master. | ||
``` | ||
gitfs_remotes: | ||
- https://foo.com/main.git | ||
- https://foo.com/webserver.git: | ||
- renderer-allow: | ||
- py | ||
- renderer-sandbox: | ||
- runas: user_x | ||
- ## TODO: syntax for passing scoped pillar data ## | ||
``` | ||
|
||
- Outline of a test plan for this feature. How do you plan to test it? Can it be automated? | ||
- Create multiple repositories and check if only data in a scope is readable. | ||
- Create a state file for the py renderer and write data as another user. Check if data has been written (it should not). | ||
- Create a state file for the py renderer and run it in a container/jail and write data. Check if data has been permanently written on the minion host (it should not). | ||
- Create functions distributed over multiple repositories and use them together in an orchestrate runner. | ||
- Create states distributed over multiple repositories and use them together with state auto order. | ||
- ... | ||
|
||
|
||
## Alternatives | ||
[alternatives]: #alternatives | ||
|
||
What other designs have been considered? | ||
* Running multiple minions as non root in their respective contexts. | ||
* Hinders state auto ordering. | ||
* Orchestration Runners have to be used, while targeting just one host. | ||
* May have a bigger cpu/memory footprint. | ||
* Not every context on a host needs a distinct communication towards the master. | ||
|
||
What is the impact of not doing this? | ||
* Collaboration via different backends is not possible if anyone does not fit in the scope of salt master trustees. | ||
* Not everyone, who should be able to define a state on a host for a unprivileged scope, needs access to every data or function while depending on states that need to be resolved in privileged contexts. | ||
|
||
## Unresolved questions | ||
[unresolved]: #unresolved-questions | ||
|
||
What parts of the design are still TBD? | ||
|
||
Sandboxing of renderers. | ||
|
||
# Drawbacks | ||
[drawbacks]: #drawbacks | ||
|
||
Why should we *not* do this? Please consider: | ||
|
||
- Implementation cost, both in term of code size and complexity | ||
- Sandboxing and subscoping (pillar)data/functions might have a higher complexity | ||
- Integration of this feature with other existing and planned features | ||
- no | ||
- Cost of migrating existing Salt setups (is it a breaking change?) | ||
- no | ||
- Documentation (would Salt documentation need to be re-organized or altered?) | ||
- no | ||
|
||
|
||
There are tradeoffs to choosing any path. Attempt to identify them here. |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does this help? Every renderer can run arbitrary commands as
root
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A wrapper can ensure that the input (the file from the file backend) and output of a rendering pipeline fits certain criteria. First scanning for arbitrary commands/functions/... in the state file and later checking for arbitrary commands/functions in the returned output.
Limiting the list of renderers should simply allow a salt administrator to concentrate on a limited set of features that should be allowed/denied/enforced - as you said every renderer can run arbitrary commands, but those can be issued with a particular format subset that a renderer can understand. I think, the py renderer would be excluded most of the time.
But you are correct for this, encapsulating a constrained renderer in a sandbox like environment would be favorable. Hence my suggestion to allow an salt administrator to sandbox the rendering within a renderer has only access to scoped data and limited acting room to have an effect on the host/minion.