Merge pull request #277642 from Enzime/fix/nixos-rebuild-remote-sudo

nixos-rebuild: fix issues when using `--target-host`

nixos/tests/all-tests.nix

@@ -604,6 +604,7 @@ in {
   nixos-generate-config = handleTest ./nixos-generate-config.nix {};
   nixos-rebuild-install-bootloader = handleTestOn ["x86_64-linux"] ./nixos-rebuild-install-bootloader.nix {};
   nixos-rebuild-specialisations = handleTestOn ["x86_64-linux"] ./nixos-rebuild-specialisations.nix {};
+  nixos-rebuild-target-host = handleTest ./nixos-rebuild-target-host.nix {};
   nixpkgs = pkgs.callPackage ../modules/misc/nixpkgs/test.nix { inherit evalMinimalConfig; };
   nixseparatedebuginfod = handleTest ./nixseparatedebuginfod.nix {};
   node-red = handleTest ./node-red.nix {};

nixos/tests/nixos-rebuild-target-host.nix

@@ -0,0 +1,136 @@
+import ./make-test-python.nix ({ pkgs, ... }: {
+  name = "nixos-rebuild-target-host";
+
+  nodes = {
+    deployer = { lib, ... }: let
+      inherit (import ./ssh-keys.nix pkgs) snakeOilPrivateKey snakeOilPublicKey;
+    in {
+      imports = [ ../modules/profiles/installation-device.nix ];
+
+      nix.settings = {
+        substituters = lib.mkForce [ ];
+        hashed-mirrors = null;
+        connect-timeout = 1;
+      };
+
+      environment.systemPackages = [ pkgs.passh ];
+
+      system.includeBuildDependencies = true;
+
+      virtualisation = {
+        cores = 2;
+        memorySize = 2048;
+      };
+
+      system.build.privateKey = snakeOilPrivateKey;
+      system.build.publicKey = snakeOilPublicKey;
+    };
+
+    target = { nodes, lib, ... }: let
+      targetConfig = {
+        documentation.enable = false;
+        services.openssh.enable = true;
+
+        users.users.root.openssh.authorizedKeys.keys = [ nodes.deployer.system.build.publicKey ];
+        users.users.alice.openssh.authorizedKeys.keys = [ nodes.deployer.system.build.publicKey ];
+        users.users.bob.openssh.authorizedKeys.keys = [ nodes.deployer.system.build.publicKey ];
+
+        users.users.alice.extraGroups = [ "wheel" ];
+        users.users.bob.extraGroups = [ "wheel" ];
+
+        # Disable sudo for root to ensure sudo isn't called without `--use-remote-sudo`
+        security.sudo.extraRules = lib.mkForce [
+          { groups = [ "wheel" ]; commands = [ { command = "ALL"; } ]; }
+          { users = [ "alice" ]; commands = [ { command = "ALL"; options = [ "NOPASSWD" ]; } ]; }
+        ];
+
+        nix.settings.trusted-users = [ "@wheel" ];
+      };
+    in {
+      imports = [ ./common/user-account.nix ];
+
+      config = lib.mkMerge [
+        targetConfig
+        {
+          system.build = {
+            inherit targetConfig;
+          };
+
+          networking.hostName = "target";
+        }
+      ];
+    };
+  };
+
+  testScript = { nodes, ... }:
+    let
+      sshConfig = builtins.toFile "ssh.conf" ''
+        UserKnownHostsFile=/dev/null
+        StrictHostKeyChecking=no
+      '';
+
+      targetConfigJSON = pkgs.writeText "target-configuration.json"
+        (builtins.toJSON nodes.target.system.build.targetConfig);
+
+      targetNetworkJSON = pkgs.writeText "target-network.json"
+        (builtins.toJSON nodes.target.system.build.networkConfig);
+
+      configFile = hostname: pkgs.writeText "configuration.nix" ''
+        { lib, modulesPath, ... }: {
+          imports = [
+            (modulesPath + "/virtualisation/qemu-vm.nix")
+            (modulesPath + "/testing/test-instrumentation.nix")
+            (modulesPath + "/../tests/common/user-account.nix")
+            (lib.modules.importJSON ./target-configuration.json)
+            (lib.modules.importJSON ./target-network.json)
+            ./hardware-configuration.nix
+          ];
+
+          boot.loader.grub = {
+            enable = true;
+            device = "/dev/vda";
+            forceInstall = true;
+          };
+
+          # this will be asserted
+          networking.hostName = "${hostname}";
+        }
+      '';
+    in
+    ''
+      start_all()
+      target.wait_for_open_port(22)
+
+      deployer.wait_until_succeeds("ping -c1 target")
+      deployer.succeed("install -Dm 600 ${nodes.deployer.system.build.privateKey} ~root/.ssh/id_ecdsa")
+      deployer.succeed("install ${sshConfig} ~root/.ssh/config")
+
+      target.succeed("nixos-generate-config")
+      deployer.succeed("scp alice@target:/etc/nixos/hardware-configuration.nix /root/hardware-configuration.nix")
+
+      deployer.copy_from_host("${configFile "config-1-deployed"}", "/root/configuration-1.nix")
+      deployer.copy_from_host("${configFile "config-2-deployed"}", "/root/configuration-2.nix")
+      deployer.copy_from_host("${configFile "config-3-deployed"}", "/root/configuration-3.nix")
+      deployer.copy_from_host("${targetNetworkJSON}", "/root/target-network.json")
+      deployer.copy_from_host("${targetConfigJSON}", "/root/target-configuration.json")
+
+      # Ensure sudo is disabled for root
+      target.fail("sudo true")
+
+      # This test also ensures that sudo is not called without --use-remote-sudo
+      with subtest("Deploy to root@target"):
+        deployer.succeed("nixos-rebuild switch -I nixos-config=/root/configuration-1.nix --target-host root@target &>/dev/console")
+        target_hostname = deployer.succeed("ssh alice@target cat /etc/hostname").rstrip()
+        assert target_hostname == "config-1-deployed", f"{target_hostname=}"
+
+      with subtest("Deploy to alice@target with passwordless sudo"):
+        deployer.succeed("nixos-rebuild switch -I nixos-config=/root/configuration-2.nix --target-host alice@target --use-remote-sudo &>/dev/console")
+        target_hostname = deployer.succeed("ssh alice@target cat /etc/hostname").rstrip()
+        assert target_hostname == "config-2-deployed", f"{target_hostname=}"
+
+      with subtest("Deploy to bob@target with password based sudo"):
+        deployer.succeed("passh -c 3 -C -p ${nodes.target.users.users.bob.password} -P \"\[sudo\] password\" nixos-rebuild switch -I nixos-config=/root/configuration-3.nix --target-host bob@target --use-remote-sudo &>/dev/console")
+        target_hostname = deployer.succeed("ssh alice@target cat /etc/hostname").rstrip()
+        assert target_hostname == "config-3-deployed", f"{target_hostname=}"
+    '';
+})

pkgs/os-specific/linux/nixos-rebuild/default.nix

@@ -38,6 +38,7 @@ substituteAll {
     install-bootloader = nixosTests.nixos-rebuild-install-bootloader;
     simple-installer = nixosTests.installer.simple;
     specialisations = nixosTests.nixos-rebuild-specialisations;
+    target-host = nixosTests.nixos-rebuild-target-host;
   };
 
   meta = {

pkgs/os-specific/linux/nixos-rebuild/nixos-rebuild.8

@@ -363,11 +363,9 @@ is also set. This is useful when the target-host connection to cache.nixos.org
 is faster than the connection between hosts.
 .
 .It Fl -use-remote-sudo
-When set, nixos-rebuild prefixes remote commands that run on the
-.Fl -build-host
-and
+When set, nixos-rebuild prefixes activation commands that run on the
 .Fl -target-host
-systems with
+system with
 .Ic sudo Ns
 \&. Setting this option allows deploying as a non-root user.
 .

pkgs/os-specific/linux/nixos-rebuild/nixos-rebuild.sh

@@ -157,8 +157,10 @@ while [ "$#" -gt 0 ]; do
     esac
 done
 
-if [[ -n "$SUDO_USER" || -n $remoteSudo ]]; then
-    maybeSudo=(sudo --preserve-env="$preservedSudoVars" --)
+sudoCommand=(sudo --preserve-env="$preservedSudoVars" --)
+
+if [[ -n "$SUDO_USER" ]]; then
+    useSudo=1
 fi
 
 # log the given argument to stderr if verbose mode is on
@@ -178,17 +180,25 @@ buildHostCmd() {
     if [ -z "$buildHost" ]; then
         runCmd "$@"
     elif [ -n "$remoteNix" ]; then
-        runCmd ssh $SSHOPTS "$buildHost" "${maybeSudo[@]}" env PATH="$remoteNix":'$PATH' "$@"
+        runCmd ssh $SSHOPTS "$buildHost" "${useSudo:+${sudoCommand[@]}}" env PATH="$remoteNix":'$PATH' "$@"
     else
-        runCmd ssh $SSHOPTS "$buildHost" "${maybeSudo[@]}" "$@"
+        runCmd ssh $SSHOPTS "$buildHost" "${useSudo:+${sudoCommand[@]}}" "$@"
     fi
 }
 
 targetHostCmd() {
     if [ -z "$targetHost" ]; then
-        runCmd "${maybeSudo[@]}" "$@"
+        runCmd "${useSudo:+${sudoCommand[@]}}" "$@"
     else
-        runCmd ssh $SSHOPTS "$targetHost" "${maybeSudo[@]}" "$@"
+        runCmd ssh $SSHOPTS "$targetHost" "${useSudo:+${sudoCommand[@]}}" "$@"
+    fi
+}
+
+targetHostSudoCmd() {
+    if [ -n "$remoteSudo" ]; then
+        useSudo=1 targetHostCmd "$@"
+    else
+        targetHostCmd "$@"
     fi
 }
 
@@ -426,7 +436,7 @@ if [ "$action" = edit ]; then
     exit 1
 fi
 
-SSHOPTS="$NIX_SSHOPTS -o ControlMaster=auto -o ControlPath=$tmpDir/ssh-%n -o ControlPersist=60"
+SSHOPTS="$NIX_SSHOPTS -o ControlMaster=auto -o ControlPath=$tmpDir/ssh-%n -o ControlPersist=60 -t"
 
 # First build Nix, since NixOS may require a newer version than the
 # current one.
@@ -667,7 +677,7 @@ if [ -z "$rollback" ]; then
             pathToConfig="$(nixFlakeBuild "$flake#$flakeAttr.config.system.build.toplevel" "${extraBuildFlags[@]}" "${lockFlags[@]}")"
         fi
         copyToTarget "$pathToConfig"
-        targetHostCmd nix-env -p "$profile" --set "$pathToConfig"
+        targetHostSudoCmd nix-env -p "$profile" --set "$pathToConfig"
     elif [[ "$action" = test || "$action" = build || "$action" = dry-build || "$action" = dry-activate ]]; then
         if [[ -z $flake ]]; then
             pathToConfig="$(nixBuild '<nixpkgs/nixos>' -A system -k "${extraBuildFlags[@]}")"
@@ -695,7 +705,7 @@ if [ -z "$rollback" ]; then
     fi
 else # [ -n "$rollback" ]
     if [[ "$action" = switch || "$action" = boot ]]; then
-        targetHostCmd nix-env --rollback -p "$profile"
+        targetHostSudoCmd nix-env --rollback -p "$profile"
         pathToConfig="$profile"
     elif [[ "$action" = test || "$action" = build ]]; then
         systemNumber=$(
@@ -740,7 +750,7 @@ if [[ "$action" = switch || "$action" = boot || "$action" = test || "$action" =
     if [[ -n "$NIXOS_SWITCH_USE_DIRTY_ENV" ]]; then
         log "warning: skipping systemd-run since NIXOS_SWITCH_USE_DIRTY_ENV is set. This environment variable will be ignored in the future"
         cmd=()
-    elif ! targetHostCmd "${cmd[@]}" true &>/dev/null; then
+    elif ! targetHostSudoCmd "${cmd[@]}" true; then
         logVerbose "Skipping systemd-run to switch configuration since it is not working in target host."
         cmd=(
             "env"
@@ -762,7 +772,7 @@ if [[ "$action" = switch || "$action" = boot || "$action" = test || "$action" =
         fi
     fi
 
-    if ! targetHostCmd "${cmd[@]}" "$action"; then
+    if ! targetHostSudoCmd "${cmd[@]}" "$action"; then
         log "warning: error(s) occurred while switching to the new configuration"
         exit 1
     fi