Draft: Add 'nosetuid' treefile boolean to strip SetUID & SetGID bits #3433
Conversation
Did you mean SetUID? This feels like it'd be more flexible as an allowlist, like: I also think we should debate stripping them out versus erroring. We could instead have e.g.: ? |
rust/src/nosetuid.rs
Outdated
|
|
||
| /// Remove SetUID and SetGID bits from all executables. | ||
| #[context("Removing SetUID & SetGID bits from all executables")] | ||
| fn remove_setuid(_rootfs_dfd: &openat::Dir) -> Result<()> { |
There was a problem hiding this comment.
Hm, we're ignoring the passed dfd...
rust/src/nosetuid.rs
Outdated
| /// Remove SetUID and SetGID bits from all executables. | ||
| #[context("Removing SetUID & SetGID bits from all executables")] | ||
| fn remove_setuid(_rootfs_dfd: &openat::Dir) -> Result<()> { | ||
| for e in WalkDir::new("usr") { |
There was a problem hiding this comment.
So doesn't this actually operate on the process cwd? Which...I don't think we necessarily make be the rootfs.
I think while this walkdir crate is obviously nice code, today it isn't integrating with cap-std, and I've been trying to push things towards that. I am not sure that walkdir is buying us much over a simple recursive traversal function which we have in a few other places.
There was a problem hiding this comment.
travier
changed the title
This implements a post_process step similar to cliwrap that will strip the SetUID and SetGID bits from all executable files found in the compose.
Indeed, this would definitely make this even more useful. I would prefer we do an allow list (that could be empty) and that would keep the bits only from the selected binaries. That might allow user to select which one they want via CoreOS layering maybe? I'll do that + the change to use something cap-std based. |
|
So a more concrete example / use case for this feature: Creating a SetUID/SetGID less setup for rpm-ostree based systems. Instead of using sudo, the user could setup its sshd server config to allow localhost only root login and optionally setup a strong 2 factor authentication for example (via a U2F key, etc.). This would enable us to easily setup rpm-ostree based systems that remove the entire class of vulnerability from SetUID/SetGID apps. |
OK. Hmm, one thing I tried here is: But sadly it doesn't work, because at least sshd directly forks the child process. It's probably doable with a PAM module. The advantage of NO_NEW_PRIVILEGES is that it applies to all setuid binaries, including e.g. any that are outside the OS root, including e.g. NNP can also be done in a more fine grained fashion - there may be a convenient non-PAM-module way to force it on. But there may also be a "big hammer" way to do this globally today as an effective drop-in to all systemd units. |
|
Those are indeed also good ideas. I vaguely remember trying to do things like that a while ago but never really found a nice setup. One would also have to go over each service to make sure that they still work. PAM is also very complex and hard to work with thus I don't think I would be able to write such a module in a secure way (and it's also C). There is also the option of using confined SELinux users, but that's another beast too. I'll have to try it again to make sure it works with rootless podman. The idea behind this PR is that directly removing the SetUID bits avoids all of that complexity and makes it easy to try it out and revert right away if it fails. |
|
@travier: PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
@travier: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
I'm going to close this as this will be more easily done in a container layer and won't need any code changes in rpm-ostree at all. |
Add 'nosetuid' treefile boolean to strip SetUIG & SetGID bits
This implements a post_process step similar to cliwrap that will strip
the SetUID and SetGID bits from all executable files found in the
compose.