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

fix: support to other privilege elevation programs. #92

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

henriquekirchheck
Copy link

This pr adds support for both doas and pkexec, while keeping support for sudo.

For this to work, I decided that the cleanest way to implement this was by adding a extra parameter to the Command struct that specifies if the command should be ran as root or not. When the root parameter is set, it checks for the existence of doas, sudo, and pkexec, in that order, and uses it to run the command by prefixing the command invocation arguments with the respective program when it is constructed.

The specified order was chosen because I believe that if someone has doas installed in their system, it's more likely that they use it as their main privilege escalation program. ´pkexec´ was added more as a fallback in case someone doesn't have doas or sudo.

This was tested in my NixOS Unstable system with both sudo and doas, but I haven't tested pkexec personally.

@henriquekirchheck
Copy link
Author

Forgot to mention that it will Fix #59, but it still uses sudo argv[..]

@Makesesama
Copy link

Makesesama commented Apr 26, 2024

I also tested on NixOs Unstable with doas installed. Works fine for me.

edit: Ok i also tried to clean, which still doesnt work with doas and this pr. Cleaning calls self_elevate which still uses sudo.

@henriquekirchheck
Copy link
Author

I'll look into the clean command and fix it

@henriquekirchheck
Copy link
Author

Seems like the self elevate function is only using sudo but I should be able to just replace it with my get_elevation_program function.

@Makesesama If you could test it I would be very grateful.

@henriquekirchheck henriquekirchheck changed the title Add support to other privilege elevation programs. fix: support to other privilege elevation programs. Apr 27, 2024
@Makesesama
Copy link

Seems to work now

@henriquekirchheck
Copy link
Author

Thank you!

@arthsmn
Copy link

arthsmn commented May 4, 2024

You might also be interested in adding run0 support.

Related post: https://mastodon.social/@pid_eins/112353324518585654

@henriquekirchheck
Copy link
Author

systemd is currently at version 255.4 in NixOS unstable so I'll refrain to add it until it is actually released, but I'll look into it when it is available

src/util.rs Show resolved Hide resolved
@boomshroom
Copy link

boomshroom commented Jun 10, 2024

An alternative is to allow for some way for the user to specify what privilege escalator to use. This would be easier if nh had a config file though.

This would allow for forcing pkexec even if sudo is present, or using run0 (or its predecessor systemd-run) without needing to update nh to support it. And of course it would be ideal in such a scenario to be able to include extra arguments in case the user had particular needs (would be necessary for systemd-run since it doesn't pass through stdio without either -t or -P).

@deviantsemicolon
Copy link

Now that systemd v256 is out and currently being reviewed for eventual merging into nixpkgs, will this be updated to add support for run0

@henriquekirchheck
Copy link
Author

I'll update everything with the suggestions, thank you!

@henriquekirchheck
Copy link
Author

I guess I could add run0 to the list of automatically checked programs, because even if it isn't released yet in nixpkgs unstable it should be fine for people that don't have it because it will just be ignored by the checks.

Now I'm not sure where in that list it should be included, considering that it will be installed in every systemd based system, I think it should be low in the list, maybe above pkexec. I'm open for second opinions.

Also, it should probably check for a environment variable to determine what to do for convenience, just like nh already does with the FLAKE environment variable, but I don't know of any standard environment variable names that specify that.

@henriquekirchheck
Copy link
Author

henriquekirchheck commented Jun 30, 2024

@boomshroom

And of course it would be ideal in such a scenario to be able to include extra arguments in case the user had particular needs (would be necessary for systemd-run since it doesn't pass through stdio without either -t or -P).

Any ideas on how that would look like in the command line? I added a way to specify a custom privilege elevation binary to use, but I cannot think of a way to add the extra command line arguments. Maybe another flag would work, but that seems messy to me.

A config file would be optimal, both for the FLAKE environment variable and the custom privilege elevation program, but I'd probably wait for @viperML to comment on what he wants to do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants