From 85df7e7ea24b9f7badbcec06a54e144a0cf1baf5 Mon Sep 17 00:00:00 2001 From: Alexander Bantyev Date: Thu, 9 Mar 2023 18:11:01 +0400 Subject: [PATCH 1/3] Logger, ProgressBar: add a way to pause/resume Add new virtual methods pause and resume to the Logger class, and implement them in ProgressBar to allow to pause the bar refreshing. --- src/libmain/progress-bar.cc | 13 +++++++++++++ src/libutil/logging.hh | 3 +++ 2 files changed, 16 insertions(+) diff --git a/src/libmain/progress-bar.cc b/src/libmain/progress-bar.cc index 024259584..882deb169 100644 --- a/src/libmain/progress-bar.cc +++ b/src/libmain/progress-bar.cc @@ -72,6 +72,7 @@ private: uint64_t corruptedPaths = 0, untrustedPaths = 0; bool active = true; + bool paused = false; bool haveUpdate = true; }; @@ -120,6 +121,17 @@ public: updateThread.join(); } + void pause() override { + state_.lock()->paused = true; + writeToStderr("\r\e[K"); + } + + void resume() override { + state_.lock()->paused = false; + writeToStderr("\r\e[K"); + updateCV.notify_one(); + } + bool isVerbose() override { return printBuildLogs; @@ -338,6 +350,7 @@ public: { auto nextWakeup = std::chrono::milliseconds::max(); + if (state.paused) return nextWakeup; state.haveUpdate = false; if (!state.active) return nextWakeup; diff --git a/src/libutil/logging.hh b/src/libutil/logging.hh index 59a707eef..1a37aea9e 100644 --- a/src/libutil/logging.hh +++ b/src/libutil/logging.hh @@ -72,6 +72,9 @@ public: virtual void stop() { }; + virtual void pause() { }; + virtual void resume() { }; + // Whether the logger prints the whole build log virtual bool isVerbose() { return false; } From 5291a82cd9b9d8d7cd6b8338a5224c94c6f23eb7 Mon Sep 17 00:00:00 2001 From: Alexander Bantyev Date: Thu, 9 Mar 2023 18:16:29 +0400 Subject: [PATCH 2/3] SSHMaster: pause logger to show password prompt Pause logger before starting SSH connections, and resume it after the connection is established, so that SSH password prompts are not erased by the logger's updates. --- src/libstore/ssh.cc | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/src/libstore/ssh.cc b/src/libstore/ssh.cc index 69bfe3418..6f6deda51 100644 --- a/src/libstore/ssh.cc +++ b/src/libstore/ssh.cc @@ -1,4 +1,5 @@ #include "ssh.hh" +#include "finally.hh" namespace nix { @@ -35,6 +36,9 @@ void SSHMaster::addCommonSSHOpts(Strings & args) } if (compress) args.push_back("-C"); + + args.push_back("-oPermitLocalCommand=yes"); + args.push_back("-oLocalCommand=echo started"); } std::unique_ptr SSHMaster::startCommand(const std::string & command) @@ -49,6 +53,11 @@ std::unique_ptr SSHMaster::startCommand(const std::string ProcessOptions options; options.dieWithParent = false; + if (!fakeSSH && !useMaster) { + logger->pause(); + } + Finally cleanup = [&]() { logger->resume(); }; + conn->sshPid = startProcess([&]() { restoreProcessContext(); @@ -86,6 +95,18 @@ std::unique_ptr SSHMaster::startCommand(const std::string in.readSide = -1; out.writeSide = -1; + // Wait for the SSH connection to be established, + // So that we don't overwrite the password prompt with our progress bar. + if (!fakeSSH && !useMaster) { + std::string reply; + try { + reply = readLine(out.readSide.get()); + } catch (EndOfFile & e) { } + + if (reply != "started") + throw Error("failed to start SSH connection to '%s'", host); + } + conn->out = std::move(out.readSide); conn->in = std::move(in.writeSide); @@ -109,6 +130,9 @@ Path SSHMaster::startMaster() ProcessOptions options; options.dieWithParent = false; + logger->pause(); + Finally cleanup = [&]() { logger->resume(); }; + state->sshMaster = startProcess([&]() { restoreProcessContext(); @@ -117,11 +141,7 @@ Path SSHMaster::startMaster() if (dup2(out.writeSide.get(), STDOUT_FILENO) == -1) throw SysError("duping over stdout"); - Strings args = - { "ssh", host.c_str(), "-M", "-N", "-S", state->socketPath - , "-o", "LocalCommand=echo started" - , "-o", "PermitLocalCommand=yes" - }; + Strings args = { "ssh", host.c_str(), "-M", "-N", "-S", state->socketPath }; if (verbosity >= lvlChatty) args.push_back("-v"); addCommonSSHOpts(args); From 85a2d1d94fbb682d4ff1e85ee083fac55b6bc9cb Mon Sep 17 00:00:00 2001 From: Alexander Bantyev Date: Thu, 16 Mar 2023 13:34:48 +0400 Subject: [PATCH 3/3] Add a test for nix copy over ssh Check that nix copy can copy stuff, refuses to copy unsigned paths by default, and doesn't hide the ssh password prompt. --- flake.nix | 2 + src/libmain/progress-bar.cc | 4 +- tests/nixos/nix-copy.nix | 85 +++++++++++++++++++++++++++++++++++++ 3 files changed, 89 insertions(+), 2 deletions(-) create mode 100644 tests/nixos/nix-copy.nix diff --git a/flake.nix b/flake.nix index af0ce5022..3c02a7935 100644 --- a/flake.nix +++ b/flake.nix @@ -577,6 +577,8 @@ tests.nix-copy-closure = runNixOSTestFor "x86_64-linux" ./tests/nixos/nix-copy-closure.nix; + tests.nix-copy = runNixOSTestFor "x86_64-linux" ./tests/nixos/nix-copy.nix; + tests.nssPreload = runNixOSTestFor "x86_64-linux" ./tests/nixos/nss-preload.nix; tests.githubFlakes = runNixOSTestFor "x86_64-linux" ./tests/nixos/github-flakes.nix; diff --git a/src/libmain/progress-bar.cc b/src/libmain/progress-bar.cc index 882deb169..6600ec177 100644 --- a/src/libmain/progress-bar.cc +++ b/src/libmain/progress-bar.cc @@ -129,6 +129,7 @@ public: void resume() override { state_.lock()->paused = false; writeToStderr("\r\e[K"); + state_.lock()->haveUpdate = true; updateCV.notify_one(); } @@ -350,9 +351,8 @@ public: { auto nextWakeup = std::chrono::milliseconds::max(); - if (state.paused) return nextWakeup; state.haveUpdate = false; - if (!state.active) return nextWakeup; + if (state.paused || !state.active) return nextWakeup; std::string line; diff --git a/tests/nixos/nix-copy.nix b/tests/nixos/nix-copy.nix new file mode 100644 index 000000000..ee8b77100 --- /dev/null +++ b/tests/nixos/nix-copy.nix @@ -0,0 +1,85 @@ +# Test that ‘nix copy’ works over ssh. + +{ lib, config, nixpkgs, hostPkgs, ... }: + +let + pkgs = config.nodes.client.nixpkgs.pkgs; + + pkgA = pkgs.cowsay; + pkgB = pkgs.wget; + pkgC = pkgs.hello; + pkgD = pkgs.tmux; + +in { + name = "nix-copy"; + + enableOCR = true; + + nodes = + { client = + { config, lib, pkgs, ... }: + { virtualisation.writableStore = true; + virtualisation.additionalPaths = [ pkgA pkgD.drvPath ]; + nix.settings.substituters = lib.mkForce [ ]; + nix.settings.experimental-features = [ "nix-command" ]; + services.getty.autologinUser = "root"; + }; + + server = + { config, pkgs, ... }: + { services.openssh.enable = true; + services.openssh.permitRootLogin = "yes"; + users.users.root.password = "foobar"; + virtualisation.writableStore = true; + virtualisation.additionalPaths = [ pkgB pkgC ]; + }; + }; + + testScript = { nodes }: '' + # fmt: off + import subprocess + + # Create an SSH key on the client. + subprocess.run([ + "${pkgs.openssh}/bin/ssh-keygen", "-t", "ed25519", "-f", "key", "-N", "" + ], capture_output=True, check=True) + + start_all() + + server.wait_for_unit("sshd") + client.wait_for_unit("network.target") + client.wait_for_unit("getty@tty1.service") + client.wait_for_text("]#") + + # Copy the closure of package A from the client to the server using password authentication, + # and check that all prompts are visible + server.fail("nix-store --check-validity ${pkgA}") + client.send_chars("nix copy --to ssh://server ${pkgA} >&2; echo done\n") + client.wait_for_text("continue connecting") + client.send_chars("yes\n") + client.wait_for_text("Password:") + client.send_chars("foobar\n") + client.wait_for_text("done") + server.succeed("nix-store --check-validity ${pkgA}") + + client.copy_from_host("key", "/root/.ssh/id_ed25519") + client.succeed("chmod 600 /root/.ssh/id_ed25519") + + # Install the SSH key on the server. + server.copy_from_host("key.pub", "/root/.ssh/authorized_keys") + server.succeed("systemctl restart sshd") + client.succeed(f"ssh -o StrictHostKeyChecking=no {server.name} 'echo hello world'") + + # Copy the closure of package B from the server to the client, using ssh-ng. + client.fail("nix-store --check-validity ${pkgB}") + # Shouldn't download untrusted paths by default + client.fail("nix copy --from ssh-ng://server ${pkgB} >&2") + client.succeed("nix copy --no-check-sigs --from ssh-ng://server ${pkgB} >&2") + client.succeed("nix-store --check-validity ${pkgB}") + + # Copy the derivation of package D's derivation from the client to the server. + server.fail("nix-store --check-validity ${pkgD.drvPath}") + client.succeed("nix copy --derivation --to ssh://server ${pkgD.drvPath} >&2") + server.succeed("nix-store --check-validity ${pkgD.drvPath}") + ''; +}