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

nixos-rebuild-ng: --sudo --ask-sudo-password fails with sudo-rs #371156

Closed
delliottxyz opened this issue Jan 5, 2025 · 8 comments
Closed

nixos-rebuild-ng: --sudo --ask-sudo-password fails with sudo-rs #371156

delliottxyz opened this issue Jan 5, 2025 · 8 comments
Labels
0.kind: bug Something is broken 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS

Comments

@delliottxyz
Copy link
Contributor

delliottxyz commented Jan 5, 2025

Describe the bug

When using nixos-rebuild-ng with --target-host --sudo --ask-sudo-password the script passes the argument '--prompt= to sudo. This works with the original c implementation of sudo but if the target host is using sudo-rs then this fails with the following error:

'prompt' does not take any arguments

I am unsure if this error is also present in the original nixos-rebuild implementation, but can test if anyone is interested.

Steps To Reproduce

Steps to reproduce the behavior:

  1. On target-host disable c sudo and enable sudo-rs with the following snippet
  security = {
    sudo.enable = false;
    sudo-rs .enable = true;
  };
  1. Run nixos-rebuild --flake . --target-host <host> --sudo --ask-sudo-password switch
  2. Observe error.

Expected behavior

Ideally nixos-rebuild-ng would work without error when using security.sudo-rs.enable = true;.

I appreciate though that it may be decided that this is an upstream issue for sudo-rs, and that is quite fair.

A similar judgement was made with regard to doas in #366527

Screenshots

I do not think they would convey any extra information.

Additional context

It would seem that the issue is that sudo-rs does not yet support the the --prompt argument, or does so in a way divergent from the original sudo.

The sudo-rs usage message does not mention a prompt arg anyway.

usage: sudo -h | -K | -k | -V
usage: sudo -v [-knS] [-g group] [-u user]
usage: sudo -l [-knS] [-g group] [-U user] [-u user] [command [arg ...]]
usage: sudo [-knS] [-D directory] [-g group] [-u user] [-i | -s] [command [arg ...]]
usage: sudo -e [-knS] [-D directory] [-g group] [-u user] file ...

Metadata

 - system: `"x86_64-linux"`
 - host os: `Linux 6.12.8, NixOS, 25.05 (Warbler), 25.05.20250102.6df2492`
 - multi-user?: `yes`
 - sandbox: `yes`
 - version: `nix-env (Nix) 2.24.11`
 - nixpkgs: `/nix/store/apfqvr9kddcl6fscjvb92p4xdrqzcwk0-source`

Notify maintainers

@thiagokokada


Note for maintainers: Please tag this issue in your PR.


Add a 👍 reaction to issues you find important.

@delliottxyz delliottxyz added 0.kind: bug Something is broken 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS labels Jan 5, 2025
@delliottxyz delliottxyz changed the title nixos-rebuild{ng}: nixos-rebuild{ng}: sudo-rs error: 'prompt' does not take any arguments Jan 5, 2025
@FliegendeWurst FliegendeWurst changed the title nixos-rebuild{ng}: sudo-rs error: 'prompt' does not take any arguments nixos-rebuild-ng: --sudo --ask-sudo-password fails with sudo-rs Jan 5, 2025
@thiagokokada
Copy link
Contributor

This is an upstream issue. We need to use --prompt= (an empty prompt) to hide the password prompt from sudo.

sudo-rs needs to implement this flag, or at least offer an alternative.

@delliottxyz
Copy link
Contributor Author

Fair enough, I will bring it up upstream.

@delliottxyz delliottxyz closed this as not planned Won't fix, can't repro, duplicate, stale Jan 5, 2025
@thiagokokada
Copy link
Contributor

thiagokokada commented Jan 5, 2025

To give more context:

It seems upstream thinks this is a security misfeature. I don't think they're correct even if the implementation by sudo is dubious (I concur that doing a regex for the PAM and replacing it is not the best way to do so, but not sure if there is a better solution here), because there are valid use cases where replacing the prompt is necessary like the one used in nixos-rebuild-ng, and also I don't think there is any way that a program is in a position to manipulate a sudo prompt and they couldn't already do other malicious things.

@delliottxyz
Copy link
Contributor Author

delliottxyz commented Jan 5, 2025

Opening another issue about it upstream would be a duplicate in my opinion, so I guess there is no need to do that.

@thiagokokada
Copy link
Contributor

Opening another issue about it upstream would be a duplicate in my opinion, so I guess there is no need to do that.

You can at least add a comment with citing nixos-rebuild-ng use case to show upstream that this needs to be supported.

@squell
Copy link

squell commented Jan 6, 2025

What is the reason that nixos-rebuild-ng needs to disable the password prompt? (I.e., if we simply accept --prompt= in sudo-rs but ignore it, will that fix this problem?)

The reason we are hesitant with this feature in general is, as @thiagokokada points out, that it's kludgy to implement (there might be other authentication modules than passwords, and a PAM module may be more chatty so --prompt= doesn't completely silence all PAM prompts anyway, e.g. pam_pkcs11 will send messages like "Smartcard authentication starting" that are not part of the prompt). And the benefits of this feature did not seem compelling to us a year ago while we do see a (small) potential for abuse.

Note that with original sudo, modifying the sudo prompt doesn't require any particular privileges.

@thiagokokada
Copy link
Contributor

What is the reason that nixos-rebuild-ng needs to disable the password prompt? (I.e., if we simply accept --prompt= in sudo-rs but ignore it, will that fix this problem?)

The reason we need to disable the prompt is because we are injecting the password via stdin. If we don't disable the prompt, it basically shows a very confusing (for the user) message:

[sudo] password for <user>:
[sudo] password for <user>:

Possible multiple times since we call each command in a separate SSH session, and in this case there is no sudo cache.

there might be other authentication modules than passwords, and a PAM module may be more chatty so --prompt= doesn't completely silence all PAM prompts anyway, e.g. pam_pkcs11 will send messages like "Smartcard authentication starting" that are not part of the prompt

I don't think this is what --prompt promises and it is a non-issue. --prompt is about, AFAIK, the password prompt. We can still show other sudo messages without issue.

Note that with original sudo, modifying the sudo prompt doesn't require any particular privileges.

I am not sure why this is an issue. There are multiple ways that if you have the power to run commands for the user that could cause security issues already, e.g.: you can just simulate a sudo prompt and capture the user password (this is what nixos-rebuild-ng does, but with a non-nefarious use case). Having the flag --prompt is not a security issue at all.

@thiagokokada
Copy link
Contributor

@squell, by the way:

       -p prompt, --prompt=prompt
               Use a custom password prompt with optional escape
               sequences.  The following percent (‘%’) escape sequences
               are supported by the sudoers policy:

It is very clear in the documentation that --prompt is about the password prompt, not other prompts. So the case for pam_pkcs11 is a non-issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.kind: bug Something is broken 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS
Projects
None yet
Development

No branches or pull requests

3 participants