From 409d29ca737309e51e58483aa0ef0a8d4489d9a1 Mon Sep 17 00:00:00 2001 From: nicoo Date: Thu, 31 Aug 2023 10:33:11 +0000 Subject: [PATCH 01/17] nixos/sudo: Split up `configFile` into individual sections --- nixos/modules/security/sudo.nix | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/nixos/modules/security/sudo.nix b/nixos/modules/security/sudo.nix index d225442773c6..adf83c7a045a 100644 --- a/nixos/modules/security/sudo.nix +++ b/nixos/modules/security/sudo.nix @@ -205,17 +205,20 @@ in } ]; - security.sudo.configFile = + security.sudo.configFile = concatStringsSep "\n" [ '' # Don't edit this file. Set the NixOS options ‘security.sudo.configFile’ # or ‘security.sudo.extraRules’ instead. - + '' + '' # Keep SSH_AUTH_SOCK so that pam_ssh_agent_auth.so can do its magic. Defaults env_keep+=SSH_AUTH_SOCK - + '' + '' # "root" is allowed to do anything. root ALL=(ALL:ALL) SETENV: ALL - + '' + '' # extraRules ${concatStringsSep "\n" ( lists.flatten ( @@ -227,9 +230,12 @@ in ) cfg.extraRules ) )} - + '' + '' + # extraConfig ${cfg.extraConfig} - ''; + '' + ]; security.wrappers = let owner = "root"; From 454151375d626a148fdb4423d577994319d6bd97 Mon Sep 17 00:00:00 2001 From: nicoo Date: Mon, 4 Sep 2023 21:01:09 +0000 Subject: [PATCH 02/17] nixos/sudo: Don't include empty sections This makes the generated sudoers a touch easier to read. --- nixos/modules/security/sudo.nix | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/nixos/modules/security/sudo.nix b/nixos/modules/security/sudo.nix index adf83c7a045a..ad7d43d2682b 100644 --- a/nixos/modules/security/sudo.nix +++ b/nixos/modules/security/sudo.nix @@ -205,7 +205,7 @@ in } ]; - security.sudo.configFile = concatStringsSep "\n" [ + security.sudo.configFile = concatStringsSep "\n" (filter (s: s != "") [ '' # Don't edit this file. Set the NixOS options ‘security.sudo.configFile’ # or ‘security.sudo.extraRules’ instead. @@ -218,7 +218,7 @@ in # "root" is allowed to do anything. root ALL=(ALL:ALL) SETENV: ALL '' - '' + (optionalString (cfg.extraRules != []) '' # extraRules ${concatStringsSep "\n" ( lists.flatten ( @@ -230,12 +230,12 @@ in ) cfg.extraRules ) )} - '' - '' + '') + (optionalString (cfg.extraConfig != "") '' # extraConfig ${cfg.extraConfig} - '' - ]; + '') + ]); security.wrappers = let owner = "root"; From 8742134c80539b3f8e9c7c51b13a225a92e97b9a Mon Sep 17 00:00:00 2001 From: nicoo Date: Mon, 4 Sep 2023 21:06:12 +0000 Subject: [PATCH 03/17] nixos/sudo: Only keep SSH_AUTH_SOCK if used for authentication This will make compatibility with `sudo-rs` easier. --- nixos/modules/security/sudo.nix | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/nixos/modules/security/sudo.nix b/nixos/modules/security/sudo.nix index ad7d43d2682b..eeb2f0dda8b2 100644 --- a/nixos/modules/security/sudo.nix +++ b/nixos/modules/security/sudo.nix @@ -6,6 +6,10 @@ let cfg = config.security.sudo; + enableSSHAgentAuth = + with config.security; + pam.enableSSHAgentAuth && pam.sudo.sshAgentAuth; + inherit (pkgs) sudo; toUserString = user: if (isInt user) then "#${toString user}" else "${user}"; @@ -210,10 +214,10 @@ in # Don't edit this file. Set the NixOS options ‘security.sudo.configFile’ # or ‘security.sudo.extraRules’ instead. '' - '' + (optionalString enableSSHAgentAuth '' # Keep SSH_AUTH_SOCK so that pam_ssh_agent_auth.so can do its magic. Defaults env_keep+=SSH_AUTH_SOCK - '' + '') '' # "root" is allowed to do anything. root ALL=(ALL:ALL) SETENV: ALL From 0365b05f13a4230d75bb63708694ee4692638236 Mon Sep 17 00:00:00 2001 From: nicoo Date: Mon, 4 Sep 2023 23:05:00 +0000 Subject: [PATCH 04/17] nixos/terminfo: Add config option not to add extra sudo config This will be necessary for compatibility with `sudo-rs`. --- .../manual/release-notes/rl-2311.section.md | 6 +++++ nixos/modules/config/terminfo.nix | 25 +++++++++++++------ 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/nixos/doc/manual/release-notes/rl-2311.section.md b/nixos/doc/manual/release-notes/rl-2311.section.md index 9b30a20da162..2fd577864c08 100644 --- a/nixos/doc/manual/release-notes/rl-2311.section.md +++ b/nixos/doc/manual/release-notes/rl-2311.section.md @@ -280,6 +280,12 @@ The module update takes care of the new config syntax and the data itself (user - New `boot.bcache.enable` (default enabled) allows completely removing `bcache` mount support. +- `security.sudo` now provides an extra option, that does not change the + module's default behaviour: + `keepTerminfo` controls whether `TERMINFO` and `TERMINFO_DIRS` are preserved + for `root` and the `wheel` group. + + ## Nixpkgs internals {#sec-release-23.11-nixpkgs-internals} - The use of `sourceRoot = "source";`, `sourceRoot = "source/subdir";`, and similar lines in package derivations using the default `unpackPhase` is deprecated as it requires `unpackPhase` to always produce a directory named "source". Use `sourceRoot = src.name`, `sourceRoot = "${src.name}/subdir";`, or `setSourceRoot = "sourceRoot=$(echo */subdir)";` or similar instead. diff --git a/nixos/modules/config/terminfo.nix b/nixos/modules/config/terminfo.nix index 1ae8e82c471e..ebd1aaea8f04 100644 --- a/nixos/modules/config/terminfo.nix +++ b/nixos/modules/config/terminfo.nix @@ -6,12 +6,23 @@ with lib; { - options.environment.enableAllTerminfo = with lib; mkOption { - default = false; - type = types.bool; - description = lib.mdDoc '' - Whether to install all terminfo outputs - ''; + options = with lib; { + environment.enableAllTerminfo = mkOption { + default = false; + type = types.bool; + description = lib.mdDoc '' + Whether to install all terminfo outputs + ''; + }; + + security.sudo.keepTerminfo = mkOption { + default = true; + type = types.bool; + description = lib.mdDoc '' + Whether to preserve the `TERMINFO` and `TERMINFO_DIRS` + environment variables, for `root` and the `wheel` group. + ''; + }; }; config = { @@ -54,7 +65,7 @@ with lib; export TERM=$TERM ''; - security.sudo.extraConfig = '' + security.sudo.extraConfig = mkIf config.security.sudo.keepTerminfo '' # Keep terminfo database for root and %wheel. Defaults:root,%wheel env_keep+=TERMINFO_DIRS From f5aadb56bed0bba1c5ade776ac49cb1d8a56ecf9 Mon Sep 17 00:00:00 2001 From: nicoo Date: Thu, 7 Sep 2023 11:57:20 +0000 Subject: [PATCH 05/17] nixos/sudo: Refactor option definitions --- nixos/modules/security/sudo.nix | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/nixos/modules/security/sudo.nix b/nixos/modules/security/sudo.nix index eeb2f0dda8b2..0c6b665ec59b 100644 --- a/nixos/modules/security/sudo.nix +++ b/nixos/modules/security/sudo.nix @@ -34,9 +34,9 @@ in ###### interface - options = { + options.security.sudo = { - security.sudo.enable = mkOption { + enable = mkOption { type = types.bool; default = true; description = @@ -46,7 +46,7 @@ in ''; }; - security.sudo.package = mkOption { + package = mkOption { type = types.package; default = pkgs.sudo; defaultText = literalExpression "pkgs.sudo"; @@ -55,7 +55,7 @@ in ''; }; - security.sudo.wheelNeedsPassword = mkOption { + wheelNeedsPassword = mkOption { type = types.bool; default = true; description = @@ -65,7 +65,7 @@ in ''; }; - security.sudo.execWheelOnly = mkOption { + execWheelOnly = mkOption { type = types.bool; default = false; description = lib.mdDoc '' @@ -76,7 +76,7 @@ in ''; }; - security.sudo.configFile = mkOption { + configFile = mkOption { type = types.lines; # Note: if syntax errors are detected in this file, the NixOS # configuration will fail to build. @@ -87,7 +87,7 @@ in ''; }; - security.sudo.extraRules = mkOption { + extraRules = mkOption { description = lib.mdDoc '' Define specific rules to be in the {file}`sudoers` file. More specific rules should come after more general ones in order to @@ -183,7 +183,7 @@ in }); }; - security.sudo.extraConfig = mkOption { + extraConfig = mkOption { type = types.lines; default = ""; description = lib.mdDoc '' From 8b9e867ac83fdc8a3ec4bd7746b455c2b0b79b2d Mon Sep 17 00:00:00 2001 From: nicoo Date: Thu, 7 Sep 2023 12:08:28 +0000 Subject: [PATCH 06/17] nixos/sudo: Refactor checks for Todd C. Miller's implemetation --- nixos/modules/security/sudo.nix | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/nixos/modules/security/sudo.nix b/nixos/modules/security/sudo.nix index 0c6b665ec59b..a7e16c5d6f83 100644 --- a/nixos/modules/security/sudo.nix +++ b/nixos/modules/security/sudo.nix @@ -4,13 +4,15 @@ with lib; let + inherit (pkgs) sudo; + cfg = config.security.sudo; enableSSHAgentAuth = with config.security; pam.enableSSHAgentAuth && pam.sudo.sshAgentAuth; - inherit (pkgs) sudo; + usingMillersSudo = cfg.package.pname == sudo.pname; toUserString = user: if (isInt user) then "#${toString user}" else "${user}"; toGroupString = group: if (isInt group) then "%#${toString group}" else "%${group}"; @@ -197,8 +199,8 @@ in config = mkIf cfg.enable { assertions = [ - { assertion = cfg.package.pname != "sudo-rs"; - message = "The NixOS `sudo` module does not work with `sudo-rs` yet."; } + { assertion = usingMillersSudo; + message = "The NixOS `sudo` module does not yet work with other implementations."; } ]; # We `mkOrder 600` so that the default rule shows up first, but there is From 3a95964fd5ba6240c4d08ee9a5e76faa94c8934a Mon Sep 17 00:00:00 2001 From: nicoo Date: Thu, 7 Sep 2023 12:19:38 +0000 Subject: [PATCH 07/17] nixos/sudo: Drop useless `lib.` qualifiers Also normalise indentation for `mdDoc` to what's prevalent in this file. --- nixos/modules/security/sudo.nix | 49 ++++++++++++++++----------------- 1 file changed, 23 insertions(+), 26 deletions(-) diff --git a/nixos/modules/security/sudo.nix b/nixos/modules/security/sudo.nix index a7e16c5d6f83..46430c220573 100644 --- a/nixos/modules/security/sudo.nix +++ b/nixos/modules/security/sudo.nix @@ -41,18 +41,17 @@ in enable = mkOption { type = types.bool; default = true; - description = - lib.mdDoc '' - Whether to enable the {command}`sudo` command, which - allows non-root users to execute commands as root. - ''; + description = mdDoc '' + Whether to enable the {command}`sudo` command, which + allows non-root users to execute commands as root. + ''; }; package = mkOption { type = types.package; default = pkgs.sudo; defaultText = literalExpression "pkgs.sudo"; - description = lib.mdDoc '' + description = mdDoc '' Which package to use for `sudo`. ''; }; @@ -60,17 +59,16 @@ in wheelNeedsPassword = mkOption { type = types.bool; default = true; - description = - lib.mdDoc '' - Whether users of the `wheel` group must - provide a password to run commands as super user via {command}`sudo`. - ''; + description = mdDoc '' + Whether users of the `wheel` group must + provide a password to run commands as super user via {command}`sudo`. + ''; }; execWheelOnly = mkOption { type = types.bool; default = false; - description = lib.mdDoc '' + description = mdDoc '' Only allow members of the `wheel` group to execute sudo by setting the executable's permissions accordingly. This prevents users that are not members of `wheel` from @@ -82,15 +80,14 @@ in type = types.lines; # Note: if syntax errors are detected in this file, the NixOS # configuration will fail to build. - description = - lib.mdDoc '' - This string contains the contents of the - {file}`sudoers` file. - ''; + description = mdDoc '' + This string contains the contents of the + {file}`sudoers` file. + ''; }; extraRules = mkOption { - description = lib.mdDoc '' + description = mdDoc '' Define specific rules to be in the {file}`sudoers` file. More specific rules should come after more general ones in order to yield the expected behavior. You can use mkBefore/mkAfter to ensure @@ -120,7 +117,7 @@ in options = { users = mkOption { type = with types; listOf (either str int); - description = lib.mdDoc '' + description = mdDoc '' The usernames / UIDs this rule should apply for. ''; default = []; @@ -128,7 +125,7 @@ in groups = mkOption { type = with types; listOf (either str int); - description = lib.mdDoc '' + description = mdDoc '' The groups / GIDs this rule should apply for. ''; default = []; @@ -137,7 +134,7 @@ in host = mkOption { type = types.str; default = "ALL"; - description = lib.mdDoc '' + description = mdDoc '' For what host this rule should apply. ''; }; @@ -145,7 +142,7 @@ in runAs = mkOption { type = with types; str; default = "ALL:ALL"; - description = lib.mdDoc '' + description = mdDoc '' Under which user/group the specified command is allowed to run. A user can be specified using just the username: `"foo"`. @@ -155,7 +152,7 @@ in }; commands = mkOption { - description = lib.mdDoc '' + description = mdDoc '' The commands for which the rule should apply. ''; type = with types; listOf (either str (submodule { @@ -163,7 +160,7 @@ in options = { command = mkOption { type = with types; str; - description = lib.mdDoc '' + description = mdDoc '' A command being either just a path to a binary to allow any arguments, the full command with arguments pre-set or with `""` used as the argument, not allowing arguments to the command at all. @@ -172,7 +169,7 @@ in options = mkOption { type = with types; listOf (enum [ "NOPASSWD" "PASSWD" "NOEXEC" "EXEC" "SETENV" "NOSETENV" "LOG_INPUT" "NOLOG_INPUT" "LOG_OUTPUT" "NOLOG_OUTPUT" ]); - description = lib.mdDoc '' + description = mdDoc '' Options for running the command. Refer to the [sudo manual](https://www.sudo.ws/man/1.7.10/sudoers.man.html). ''; default = []; @@ -188,7 +185,7 @@ in extraConfig = mkOption { type = types.lines; default = ""; - description = lib.mdDoc '' + description = mdDoc '' Extra configuration text appended to {file}`sudoers`. ''; }; From b1eab8ca53dca000ffb5dcb7db62685bc2948215 Mon Sep 17 00:00:00 2001 From: nicoo Date: Thu, 7 Sep 2023 12:46:04 +0000 Subject: [PATCH 08/17] nixos/sudo: Handle `root`'s default rule through `extraRules` This makes things more uniform, and simplifies compatibility with sudo-rs. Moreover, users can not inject rules before this if they need to. --- .../manual/release-notes/rl-2311.section.md | 6 ++++ nixos/modules/security/sudo.nix | 32 ++++++++++++------- 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/nixos/doc/manual/release-notes/rl-2311.section.md b/nixos/doc/manual/release-notes/rl-2311.section.md index 2fd577864c08..98c521b1106b 100644 --- a/nixos/doc/manual/release-notes/rl-2311.section.md +++ b/nixos/doc/manual/release-notes/rl-2311.section.md @@ -200,6 +200,12 @@ - Package `pash` was removed due to being archived upstream. Use `powershell` as an alternative. +- `security.sudo.extraRules` now includes `root`'s default rule, with ordering + priority 400. This is functionally identical for users not specifying rule + order, or relying on `mkBefore` and `mkAfter`, but may impact users calling + `mkOrder n` with n ≤ 400. + + ## Other Notable Changes {#sec-release-23.11-notable-changes} - The Cinnamon module now enables XDG desktop integration by default. If you are experiencing collisions related to xdg-desktop-portal-gtk you can safely remove `xdg.portal.extraPortals = [ pkgs.xdg-desktop-portal-gtk ];` from your NixOS configuration. diff --git a/nixos/modules/security/sudo.nix b/nixos/modules/security/sudo.nix index 46430c220573..04a8e7194064 100644 --- a/nixos/modules/security/sudo.nix +++ b/nixos/modules/security/sudo.nix @@ -200,13 +200,27 @@ in message = "The NixOS `sudo` module does not yet work with other implementations."; } ]; - # We `mkOrder 600` so that the default rule shows up first, but there is - # still enough room for a user to `mkBefore` it. - security.sudo.extraRules = mkOrder 600 [ - { groups = [ "wheel" ]; - commands = [ { command = "ALL"; options = (if cfg.wheelNeedsPassword then [ "SETENV" ] else [ "NOPASSWD" "SETENV" ]); } ]; - } - ]; + security.sudo.extraRules = + let + defaultRule = { users ? [], groups ? [], opts ? [] }: [ { + inherit users groups; + commands = [ { + command = "ALL"; + options = opts ++ [ "SETENV" ]; + } ]; + } ]; + in mkMerge [ + # This is ordered before users' `mkBefore` rules, + # so as not to introduce unexpected changes. + (mkOrder 400 (defaultRule { users = [ "root" ]; })) + + # This is ordered to show before (most) other rules, but + # late-enough for a user to `mkBefore` it. + (mkOrder 600 (defaultRule { + groups = [ "wheel" ]; + opts = (optional (!cfg.wheelNeedsPassword) "NOPASSWD"); + })) + ]; security.sudo.configFile = concatStringsSep "\n" (filter (s: s != "") [ '' @@ -217,10 +231,6 @@ in # Keep SSH_AUTH_SOCK so that pam_ssh_agent_auth.so can do its magic. Defaults env_keep+=SSH_AUTH_SOCK '') - '' - # "root" is allowed to do anything. - root ALL=(ALL:ALL) SETENV: ALL - '' (optionalString (cfg.extraRules != []) '' # extraRules ${concatStringsSep "\n" ( From 717e51a140d6af347b5362ddb149a2c343b947b8 Mon Sep 17 00:00:00 2001 From: nicoo Date: Thu, 7 Sep 2023 12:50:48 +0000 Subject: [PATCH 09/17] nixos/sudo: Make the default rules' options configurable --- nixos/doc/manual/release-notes/rl-2311.section.md | 7 ++++--- nixos/modules/security/sudo.nix | 13 +++++++++++-- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/nixos/doc/manual/release-notes/rl-2311.section.md b/nixos/doc/manual/release-notes/rl-2311.section.md index 98c521b1106b..b7df38e67159 100644 --- a/nixos/doc/manual/release-notes/rl-2311.section.md +++ b/nixos/doc/manual/release-notes/rl-2311.section.md @@ -286,10 +286,11 @@ The module update takes care of the new config syntax and the data itself (user - New `boot.bcache.enable` (default enabled) allows completely removing `bcache` mount support. -- `security.sudo` now provides an extra option, that does not change the +- `security.sudo` now provides two extra options, that do not change the module's default behaviour: - `keepTerminfo` controls whether `TERMINFO` and `TERMINFO_DIRS` are preserved - for `root` and the `wheel` group. + - `defaultOptions` controls the options used for the default rules; + - `keepTerminfo` controls whether `TERMINFO` and `TERMINFO_DIRS` are preserved + for `root` and the `wheel` group. ## Nixpkgs internals {#sec-release-23.11-nixpkgs-internals} diff --git a/nixos/modules/security/sudo.nix b/nixos/modules/security/sudo.nix index 04a8e7194064..882e3d18aa43 100644 --- a/nixos/modules/security/sudo.nix +++ b/nixos/modules/security/sudo.nix @@ -38,6 +38,15 @@ in options.security.sudo = { + defaultOptions = mkOption { + type = with types; listOf str; + default = [ "SETENV" ]; + description = mdDoc '' + Options used for the default rules, granting `root` and the + `wheel` group permission to run any command as any user. + ''; + }; + enable = mkOption { type = types.bool; default = true; @@ -206,8 +215,8 @@ in inherit users groups; commands = [ { command = "ALL"; - options = opts ++ [ "SETENV" ]; - } ]; + options = opts ++ cfg.defaultOptions; + } ]; } ]; in mkMerge [ # This is ordered before users' `mkBefore` rules, From c11da39117871fce949423b3e27da6b796d36957 Mon Sep 17 00:00:00 2001 From: nicoo Date: Thu, 7 Sep 2023 14:36:29 +0000 Subject: [PATCH 10/17] nixos/sudo: Drop the sudoers comment for `extraRules` All rules are now handled through `extraRules`, and it is never empty so `optionalString` isn't needed either. --- nixos/modules/security/sudo.nix | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/nixos/modules/security/sudo.nix b/nixos/modules/security/sudo.nix index 882e3d18aa43..4bf214f73eaf 100644 --- a/nixos/modules/security/sudo.nix +++ b/nixos/modules/security/sudo.nix @@ -240,19 +240,16 @@ in # Keep SSH_AUTH_SOCK so that pam_ssh_agent_auth.so can do its magic. Defaults env_keep+=SSH_AUTH_SOCK '') - (optionalString (cfg.extraRules != []) '' - # extraRules - ${concatStringsSep "\n" ( - lists.flatten ( - map ( - rule: optionals (length rule.commands != 0) [ - (map (user: "${toUserString user} ${rule.host}=(${rule.runAs}) ${toCommandsString rule.commands}") rule.users) - (map (group: "${toGroupString group} ${rule.host}=(${rule.runAs}) ${toCommandsString rule.commands}") rule.groups) - ] - ) cfg.extraRules - ) - )} - '') + (concatStringsSep "\n" ( + lists.flatten ( + map ( + rule: optionals (length rule.commands != 0) [ + (map (user: "${toUserString user} ${rule.host}=(${rule.runAs}) ${toCommandsString rule.commands}") rule.users) + (map (group: "${toGroupString group} ${rule.host}=(${rule.runAs}) ${toCommandsString rule.commands}") rule.groups) + ] + ) cfg.extraRules + ) + ) + "\n") (optionalString (cfg.extraConfig != "") '' # extraConfig ${cfg.extraConfig} From f0107b4f63a70925050954f647d14f6e256362d8 Mon Sep 17 00:00:00 2001 From: nicoo Date: Thu, 7 Sep 2023 14:38:51 +0000 Subject: [PATCH 11/17] nixos/sudo: Check syntax using the configured package This is preferable even for regular `sudo`, but will ensure the check is useful when using `sudo-rs` in the future. Also, dropped antediluvian comment about the syntax check being disabled, when it was clearly not commented out: - introduced in 2007, commit 6d65f0ae03ae14f3e978d89959253d9a8f5e0ec1; - reverted in 2014, commit e68a5b265a96134243a1572f43dfc4ff75dd082b, but without ammending the comments. --- nixos/modules/security/sudo.nix | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/nixos/modules/security/sudo.nix b/nixos/modules/security/sudo.nix index 4bf214f73eaf..528c230686f7 100644 --- a/nixos/modules/security/sudo.nix +++ b/nixos/modules/security/sudo.nix @@ -283,9 +283,7 @@ in src = pkgs.writeText "sudoers-in" cfg.configFile; preferLocalBuild = true; } - # Make sure that the sudoers file is syntactically valid. - # (currently disabled - NIXOS-66) - "${pkgs.buildPackages.sudo}/sbin/visudo -f $src -c && cp $src $out"; + "${cfg.package}/bin/visudo -f $src -c && cp $src $out"; mode = "0440"; }; From 914bf5836974520e6cfd3e687dead3937f6d3db2 Mon Sep 17 00:00:00 2001 From: nicoo Date: Thu, 7 Sep 2023 14:55:33 +0000 Subject: [PATCH 12/17] nixos/{sudo, terminfo}: Adjust defaults for compatibility with `sudo-rs` --- nixos/doc/manual/release-notes/rl-2311.section.md | 10 ++++++++++ nixos/modules/config/terminfo.nix | 5 ++++- nixos/modules/security/sudo.nix | 10 ++++------ 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/nixos/doc/manual/release-notes/rl-2311.section.md b/nixos/doc/manual/release-notes/rl-2311.section.md index b7df38e67159..dd75c8b517ac 100644 --- a/nixos/doc/manual/release-notes/rl-2311.section.md +++ b/nixos/doc/manual/release-notes/rl-2311.section.md @@ -10,6 +10,16 @@ - The `nixos-rebuild` command has been given a `list-generations` subcommand. See `man nixos-rebuild` for more details. +- [`sudo-rs`], a reimplementation of `sudo` in Rust, is now supported. + Switching to it (via `security.sudo.package = pkgs.sudo-rs;`) introduces + slight changes in default behaviour, due to `sudo-rs`' current limitations: + - terminfo-related environment variables aren't preserved for `root` and `wheel`; + - `root` and `wheel` are not given the ability to set (or preserve) + arbitrary environment variables. + +[`sudo-rs`]: https://github.com/memorysafety/sudo-rs/ + + ## New Services {#sec-release-23.11-new-services} - [MCHPRS](https://github.com/MCHPR/MCHPRS), a multithreaded Minecraft server built for redstone. Available as [services.mchprs](#opt-services.mchprs.enable). diff --git a/nixos/modules/config/terminfo.nix b/nixos/modules/config/terminfo.nix index ebd1aaea8f04..d1dbc4e0d059 100644 --- a/nixos/modules/config/terminfo.nix +++ b/nixos/modules/config/terminfo.nix @@ -16,7 +16,10 @@ with lib; }; security.sudo.keepTerminfo = mkOption { - default = true; + default = config.security.sudo.package.pname != "sudo-rs"; + defaultText = literalMD '' + `true` unless using `sudo-rs` + ''; type = types.bool; description = lib.mdDoc '' Whether to preserve the `TERMINFO` and `TERMINFO_DIRS` diff --git a/nixos/modules/security/sudo.nix b/nixos/modules/security/sudo.nix index 528c230686f7..9a018b857469 100644 --- a/nixos/modules/security/sudo.nix +++ b/nixos/modules/security/sudo.nix @@ -40,7 +40,10 @@ in defaultOptions = mkOption { type = with types; listOf str; - default = [ "SETENV" ]; + default = optional usingMillersSudo "SETENV"; + defaultText = literalMD '' + `[ "SETENV" ]` if using the default `sudo` implementation + ''; description = mdDoc '' Options used for the default rules, granting `root` and the `wheel` group permission to run any command as any user. @@ -204,11 +207,6 @@ in ###### implementation config = mkIf cfg.enable { - assertions = [ - { assertion = usingMillersSudo; - message = "The NixOS `sudo` module does not yet work with other implementations."; } - ]; - security.sudo.extraRules = let defaultRule = { users ? [], groups ? [], opts ? [] }: [ { From f66eb0df3b238af9e0f88918a52adfd11db7ac84 Mon Sep 17 00:00:00 2001 From: nicoo Date: Thu, 7 Sep 2023 21:50:28 +0000 Subject: [PATCH 13/17] nixos/sudo: Only wrap `sudoedit` when using Miller's sudo --- nixos/modules/security/sudo.nix | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/nixos/modules/security/sudo.nix b/nixos/modules/security/sudo.nix index 9a018b857469..6c62dd546a1e 100644 --- a/nixos/modules/security/sudo.nix +++ b/nixos/modules/security/sudo.nix @@ -264,7 +264,8 @@ in source = "${cfg.package.out}/bin/sudo"; inherit owner group setuid permissions; }; - sudoedit = { + # sudo-rs does not yet ship a sudoedit (as of v0.2.0) + sudoedit = mkIf usingMillersSudo { source = "${cfg.package.out}/bin/sudoedit"; inherit owner group setuid permissions; }; From 4729358fa5d9a9d817fe4ccff27d8656c17b2075 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Kr=C3=BCger?= Date: Sat, 9 Sep 2023 01:42:49 +0200 Subject: [PATCH 14/17] nixos/test-driver: do not break if the command writes to stderr Capturing `stderr` as part of the return `output` could break existing tests. --- nixos/lib/test-driver/test_driver/machine.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/nixos/lib/test-driver/test_driver/machine.py b/nixos/lib/test-driver/test_driver/machine.py index 809fd690d717..724b5a6a750d 100644 --- a/nixos/lib/test-driver/test_driver/machine.py +++ b/nixos/lib/test-driver/test_driver/machine.py @@ -582,9 +582,7 @@ class Machine: # While sh is bash on NixOS, this is not the case for every distro. # We explicitly call bash here to allow for the driver to boot other distros as well. - out_command = ( - f"{timeout_str} bash -c {shlex.quote(command)} | (base64 -w 0; echo)\n" - ) + out_command = f"{timeout_str} bash -c {shlex.quote(command)} 2>/dev/null | (base64 -w 0; echo)\n" assert self.shell self.shell.send(out_command.encode()) From 7b5b3f5124331ea5679c100e6d58f1493ad4e64d Mon Sep 17 00:00:00 2001 From: nicoo Date: Thu, 7 Sep 2023 15:53:18 +0000 Subject: [PATCH 15/17] nixos/sudo: Add tests for sudo-rs too Duplicated sudo's testsuite for now, as its maintainer does not with to collaborate on testing effors; see #253876. Environment-related tests were removed, as sudo-rs does not support `(NO)SETENV` yet; see memorysafety/sudo-rs#760 --- nixos/tests/all-tests.nix | 1 + nixos/tests/sudo-rs.nix | 97 +++++++++++++++++++++++++ pkgs/tools/security/sudo-rs/default.nix | 6 +- 3 files changed, 103 insertions(+), 1 deletion(-) create mode 100644 nixos/tests/sudo-rs.nix diff --git a/nixos/tests/all-tests.nix b/nixos/tests/all-tests.nix index 30ea7c70026f..d80b9aa8d696 100644 --- a/nixos/tests/all-tests.nix +++ b/nixos/tests/all-tests.nix @@ -739,6 +739,7 @@ in { strongswan-swanctl = handleTest ./strongswan-swanctl.nix {}; stunnel = handleTest ./stunnel.nix {}; sudo = handleTest ./sudo.nix {}; + sudo-rs = handleTest ./sudo-rs.nix {}; swap-file-btrfs = handleTest ./swap-file-btrfs.nix {}; swap-partition = handleTest ./swap-partition.nix {}; swap-random-encryption = handleTest ./swap-random-encryption.nix {}; diff --git a/nixos/tests/sudo-rs.nix b/nixos/tests/sudo-rs.nix new file mode 100644 index 000000000000..334a2d0fa3ea --- /dev/null +++ b/nixos/tests/sudo-rs.nix @@ -0,0 +1,97 @@ +# Some tests to ensure sudo is working properly. +{ pkgs, sudo-rs, ... }: +let + inherit (pkgs.lib) mkIf optionalString; + password = "helloworld"; +in + import ./make-test-python.nix ({ lib, pkgs, ...} : { + name = "sudo"; + meta.maintainers = pkgs.sudo-rs.meta.maintainers; + + nodes.machine = + { lib, ... }: + { + environment.systemPackages = [ pkgs.faketty ]; + users.groups = { foobar = {}; barfoo = {}; baz = { gid = 1337; }; }; + users.users = { + test0 = { isNormalUser = true; extraGroups = [ "wheel" ]; }; + test1 = { isNormalUser = true; password = password; }; + test2 = { isNormalUser = true; extraGroups = [ "foobar" ]; password = password; }; + test3 = { isNormalUser = true; extraGroups = [ "barfoo" ]; }; + test4 = { isNormalUser = true; extraGroups = [ "baz" ]; }; + test5 = { isNormalUser = true; }; + }; + + security.sudo = { + enable = true; + package = sudo-rs; + wheelNeedsPassword = false; + + extraRules = [ + # SUDOERS SYNTAX CHECK (Test whether the module produces a valid output; + # errors being detected by the visudo checks. + + # These should not create any entries + { users = [ "notest1" ]; commands = [ ]; } + { commands = [ { command = "ALL"; options = [ ]; } ]; } + + # Test defining commands with the options syntax, though not setting any options + { users = [ "notest2" ]; commands = [ { command = "ALL"; options = [ ]; } ]; } + + + # CONFIGURATION FOR TEST CASES + { users = [ "test1" ]; groups = [ "foobar" ]; commands = [ "ALL" ]; } + { groups = [ "barfoo" 1337 ]; commands = [ { command = "ALL"; options = [ "NOPASSWD" ]; } ]; } + { users = [ "test5" ]; commands = [ { command = "ALL"; options = [ "NOPASSWD" ]; } ]; runAs = "test1:barfoo"; } + ]; + }; + }; + + nodes.strict = { ... }: { + environment.systemPackages = [ pkgs.faketty ]; + users.users = { + admin = { isNormalUser = true; extraGroups = [ "wheel" ]; }; + noadmin = { isNormalUser = true; }; + }; + + security.sudo = { + package = sudo-rs; + enable = true; + wheelNeedsPassword = false; + execWheelOnly = true; + }; + }; + + testScript = + '' + with subtest("users in wheel group should have passwordless sudo"): + machine.succeed('faketty -- su - test0 -c "sudo -u root true"') + + with subtest("test1 user should have sudo with password"): + machine.succeed('faketty -- su - test1 -c "echo ${password} | sudo -S -u root true"') + + with subtest("test1 user should not be able to use sudo without password"): + machine.fail('faketty -- su - test1 -c "sudo -n -u root true"') + + with subtest("users in group 'foobar' should be able to use sudo with password"): + machine.succeed('faketty -- su - test2 -c "echo ${password} | sudo -S -u root true"') + + with subtest("users in group 'barfoo' should be able to use sudo without password"): + machine.succeed("sudo -u test3 sudo -n -u root true") + + with subtest("users in group 'baz' (GID 1337)"): + machine.succeed("sudo -u test4 sudo -n -u root echo true") + + with subtest("test5 user should be able to run commands under test1"): + machine.succeed("sudo -u test5 sudo -n -u test1 true") + + with subtest("test5 user should not be able to run commands under root"): + machine.fail("sudo -u test5 sudo -n -u root true") + + with subtest("users in wheel should be able to run sudo despite execWheelOnly"): + strict.succeed('faketty -- su - admin -c "sudo -u root true"') + + with subtest("non-wheel users should be unable to run sudo thanks to execWheelOnly"): + strict.fail('faketty -- su - noadmin -c "sudo --help"') + '';; + }) diff --git a/pkgs/tools/security/sudo-rs/default.nix b/pkgs/tools/security/sudo-rs/default.nix index d4621e229225..3cda1cde8322 100644 --- a/pkgs/tools/security/sudo-rs/default.nix +++ b/pkgs/tools/security/sudo-rs/default.nix @@ -4,6 +4,7 @@ , fetchpatch , installShellFiles , nix-update-script +, nixosTests , pam , pandoc , rustPlatform @@ -73,7 +74,10 @@ rustPlatform.buildRustPackage rec { "su::context::tests::invalid_shell" ]; - passthru.updateScript = nix-update-script { }; + passthru = { + updateScript = nix-update-script { }; + tests = nixosTests.sudo-rs; + }; meta = with lib; { description = "A memory safe implementation of sudo and su."; From d63eb55e81ad6a87263c0c94f4362fd64d780e50 Mon Sep 17 00:00:00 2001 From: nicoo Date: Wed, 13 Sep 2023 01:17:09 +0000 Subject: [PATCH 16/17] nixos/sudo: Generate `sudo-i` PAM config for interactive use of `sudo-rs` --- nixos/modules/security/sudo.nix | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/nixos/modules/security/sudo.nix b/nixos/modules/security/sudo.nix index 6c62dd546a1e..f74ccd7ecdc0 100644 --- a/nixos/modules/security/sudo.nix +++ b/nixos/modules/security/sudo.nix @@ -4,7 +4,7 @@ with lib; let - inherit (pkgs) sudo; + inherit (pkgs) sudo sudo-rs; cfg = config.security.sudo; @@ -13,6 +13,7 @@ let pam.enableSSHAgentAuth && pam.sudo.sshAgentAuth; usingMillersSudo = cfg.package.pname == sudo.pname; + usingSudoRs = cfg.package.pname == sudo-rs.pname; toUserString = user: if (isInt user) then "#${toString user}" else "${user}"; toGroupString = group: if (isInt group) then "%#${toString group}" else "%${group}"; @@ -274,6 +275,8 @@ in environment.systemPackages = [ sudo ]; security.pam.services.sudo = { sshAgentAuth = true; usshAuth = true; }; + security.pam.services.sudo-i = mkIf usingSudoRs + { sshAgentAuth = true; usshAuth = true; }; environment.etc.sudoers = { source = From d8d0b8019ff3db7d06e7654c65f8ec41b3a7b535 Mon Sep 17 00:00:00 2001 From: nicoo Date: Wed, 13 Sep 2023 02:01:05 +0000 Subject: [PATCH 17/17] nixos/sudo: Add myself as maintainer --- nixos/modules/security/sudo.nix | 2 ++ 1 file changed, 2 insertions(+) diff --git a/nixos/modules/security/sudo.nix b/nixos/modules/security/sudo.nix index f74ccd7ecdc0..4bdbe9671e6d 100644 --- a/nixos/modules/security/sudo.nix +++ b/nixos/modules/security/sudo.nix @@ -291,4 +291,6 @@ in }; + meta.maintainers = [ lib.maintainers.nicoo ]; + }