From 104448e75d87d03d2fb0b4ac96d4da72e1dae50d Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Sun, 22 Jan 2023 22:37:50 +0100 Subject: [PATCH] ssh-ng: Set log-fd for ssh to `4` by default That's expected by `build-remote` and makes sure that errors are correctly forwarded to the user. For instance, let's say that the host-key of `example.org` is unknown and nix-build ../nixpkgs -A hello -j0 --builders 'ssh-ng://example.org' is issued, then you get the following output: cannot build on 'ssh-ng://example.org?&': error: failed to start SSH connection to 'example.org' Failed to find a machine for remote build! derivation: yh46gakxq3kchrbihwxvpn5bmadcw90b-hello-2.12.1.drv required (system, features): (x86_64-linux, []) 2 available machines: [...] The relevant information (`Host key verification failed`) ends up in the daemon's log, but that's not very obvious considering that the daemon isn't very chatty normally. This can be fixed - the same way as its done for legacy-ssh - by passing fd 4 to the SSH wrapper. Now you'd get the following error: cannot build on 'ssh-ng://example.org': error: failed to start SSH connection to 'example.org': Host key verification failed. Failed to find a machine for remote build! [...] ...and now it's clear what's wrong. Please note that this is won't end up in the derivation's log. For previous discussion about this change see https://github.com/NixOS/nix/pull/7659. Change-Id: I5790856dbf58e53ea3e63238b015ea06c347cf92 --- src/libstore/machines.cc | 2 +- src/libstore/ssh-store.cc | 7 ++++++- tests/nixos/remote-builds-ssh-ng.nix | 4 ++++ 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/libstore/machines.cc b/src/libstore/machines.cc index ecae3054e..700c9b3dd 100644 --- a/src/libstore/machines.cc +++ b/src/libstore/machines.cc @@ -69,10 +69,10 @@ ref Machine::openStore() const Store::Params storeParams; if (storeUri.starts_with("ssh://")) { storeParams["max-connections"] = "1"; - storeParams["log-fd"] = "4"; } if (storeUri.starts_with("ssh://") || storeUri.starts_with("ssh-ng://")) { + storeParams["log-fd"] = "4"; if (sshKey != "") storeParams["ssh-key"] = sshKey; if (sshPublicHostKey != "") diff --git a/src/libstore/ssh-store.cc b/src/libstore/ssh-store.cc index 4a6aad449..80d10eb0f 100644 --- a/src/libstore/ssh-store.cc +++ b/src/libstore/ssh-store.cc @@ -32,6 +32,10 @@ struct SSHStoreConfig : virtual RemoteStoreConfig, virtual CommonSSHStoreConfig class SSHStore : public virtual SSHStoreConfig, public virtual RemoteStore { public: + // Hack for getting remote build log output. + // Intentionally not in `SSHStoreConfig` so that it doesn't appear in + // the documentation + const Setting logFD{(StoreConfig*) this, -1, "log-fd", "file descriptor to which SSH's stderr is connected"}; SSHStore(const std::string & scheme, const std::string & host, const Params & params) : StoreConfig(params) @@ -47,7 +51,8 @@ public: sshPublicHostKey, // Use SSH master only if using more than 1 connection. connections->capacity() > 1, - compress) + compress, + logFD) { } diff --git a/tests/nixos/remote-builds-ssh-ng.nix b/tests/nixos/remote-builds-ssh-ng.nix index 5ff471607..8deb9a504 100644 --- a/tests/nixos/remote-builds-ssh-ng.nix +++ b/tests/nixos/remote-builds-ssh-ng.nix @@ -95,6 +95,10 @@ in builder.succeed("mkdir -p -m 700 /root/.ssh") builder.copy_from_host("key.pub", "/root/.ssh/authorized_keys") builder.wait_for_unit("sshd.service") + + out = client.fail("nix-build ${expr nodes.client 1} 2>&1") + assert "error: failed to start SSH connection to 'root@builder': Host key verification failed" in out, f"No host verification error in {out}" + client.succeed(f"ssh -o StrictHostKeyChecking=no {builder.name} 'echo hello world' >&2") # Perform a build