Checking the config file for validity should not require setuid

[cole-h]

If the file is readable by our user, we should be able to run please --check /etc/please.ini, without needing to have please setuid at that point.

This is especially useful in NixOS, in order to allow us to validate the please.ini file at build time, instead of the user finding out they broke something on accident at run time.

[edneville]

Hello Cole, Thank you for your interest. If I understand what you're doing, you'd like to check the .ini without a setuid binary from your testing infrastructure. In general I want to keep the design such that privs are dropped immediately, but this seems an exception. Generally edit tests of please.ini can be performed with an exitcmd like the one here: <https://github.com/edneville/please/blob/master/please.ini.md#exitcmd> I believe this patch will do what you need though, if you're happy to test and let me know, I'll make it part of the next release. Below the priv actions are moved to just after the ini test, which was very high in the order of things to begin with.

…

---[snip]--- diff --git a/src/bin/please.rs b/src/bin/please.rs index 17933ec..3ef2cc4 100644 --- a/src/bin/please.rs +++ b/src/bin/please.rs @@ -143,6 +143,18 @@ fn general_options( ) as i32); } + let root_uid = nix::unistd::Uid::from_raw(0); + let root_gid = nix::unistd::Gid::from_raw(0); + if nix::unistd::getuid() != root_uid { + if !set_privs("root", root_uid, root_gid) { + std::process::exit(1); + } + + if !drop_privs(ro) { + std::process::exit(1); + } + } + if matches.opt_present("a") { let mut vec = vec![]; @@ -184,13 +196,14 @@ fn main() { let root_uid = nix::unistd::Uid::from_raw(0); let root_gid = nix::unistd::Gid::from_raw(0); + if nix::unistd::getuid() == root_uid { + if !set_privs("root", root_uid, root_gid) { + std::process::exit(1); + } - if !set_privs("root", root_uid, root_gid) { - std::process::exit(1); - } - - if !drop_privs(&ro) { - std::process::exit(1); + if !drop_privs(&ro) { + std::process::exit(1); + } } general_options(&mut ro, args, &service, &mut vec_eo);

---[snip]--- Ed

On 2022-10-08 16:30-0700, Cole Helbling wrote: If the file is readable by our user, we should be able to run `please --check /etc/please.ini`, without needing to have `please` setuid at that point. This is especially useful in NixOS, in order to allow us to validate the `please.ini` file at build time, instead of the user finding out they broke something on accident at run time. -- Reply to this email directly or view it on GitHub: #4 You are receiving this because you are subscribed to this thread. Message ID: ***@***.***>

[cole-h]

Yep, that's exactly what I was attempting. I agree with "dropping privs as early as possible", but thank you for seeing this as a slight exception to that. That patch works perfectly, thank you very much!

[edneville]

Thank you for confirming, it's merged and will be in the next release.

[cole-h]

Thank you for the quick fix and quick responses, I truly appreciate it!