Skip to content

Fix SSH connection sharing #506

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.

Sign up for GitHub

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

Merged
edolstra merged 3 commits into from
Jun 15, 2026
Merged

Fix SSH connection sharing #506

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2ff04c4
libstore: Improve SSH connection sharing
pjungkamp Jan 6, 2026
46d1d88
libstore/ssh: Move -S into addCommonSSHOpts
pjungkamp Jan 7, 2026
178c13a
startMaster(): Pass ControlPersist=no
edolstra Jun 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/libstore/include/nix/store/ssh.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,11 @@ private:

Sync<State> state_;

void addCommonSSHOpts(OsStrings & args);
bool isMasterRunning();
void addCommonSSHOpts(OsStrings & args, std::optional<std::filesystem::path> socketPath);
bool isMasterRunning(std::filesystem::path socketPath);

#ifndef _WIN32 // TODO re-enable on Windows, once we can start processes.
std::filesystem::path startMaster();
std::optional<std::filesystem::path> startMaster();
#endif

public:
Expand Down
27 changes: 14 additions & 13 deletions src/libstore/ssh.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ SSHMaster::SSHMaster(
checkValidAuthority(authority);
}

void SSHMaster::addCommonSSHOpts(OsStrings & args)
void SSHMaster::addCommonSSHOpts(OsStrings & args, std::optional<std::filesystem::path> socketPath)
{
auto sshArgs = getNixSshOpts();
args.insert(args.end(), sshArgs.begin(), sshArgs.end());
Expand All @@ -114,12 +114,15 @@ void SSHMaster::addCommonSSHOpts(OsStrings & args)
// the remote session won't be garbled if the local command is slow.
args.push_back(OS_STR("-oPermitLocalCommand=yes"));
args.push_back(OS_STR("-oLocalCommand=echo started"));
args.insert(args.end(), {OS_STR("-S"), socketPath ? socketPath->native() : OS_STR("none")});
}

bool SSHMaster::isMasterRunning()
bool SSHMaster::isMasterRunning(std::filesystem::path socketPath)
{
assert(useMaster);

OsStrings args = {OS_STR("-O"), OS_STR("check"), string_to_os_string(hostnameAndUser)};
addCommonSSHOpts(args);
addCommonSSHOpts(args, socketPath);

auto res = runProgram(RunOptions{.program = "ssh", .args = std::move(args), .mergeStderrToStdout = true});
return res.first == 0;
Expand Down Expand Up @@ -151,7 +154,7 @@ std::unique_ptr<SSHMaster::Connection> SSHMaster::startCommand(OsStrings && comm
#ifdef _WIN32 // TODO re-enable on Windows, once we can start processes.
throw UnimplementedError("cannot yet SSH on windows because spawning processes is not yet implemented");
#else
std::filesystem::path socketPath = startMaster();
auto socketPath = startMaster();

Pipe in, out;
in.create();
Expand Down Expand Up @@ -184,9 +187,7 @@ std::unique_ptr<SSHMaster::Connection> SSHMaster::startCommand(OsStrings && comm

if (!fakeSSH) {
args = {"ssh", hostnameAndUser.c_str(), "-x"};
addCommonSSHOpts(args);
if (!socketPath.empty())
args.insert(args.end(), {"-S", socketPath.string()});
addCommonSSHOpts(args, socketPath);
if (verbosity >= lvlChatty)
args.push_back("-v");
args.splice(args.end(), std::move(extraSshArgs));
Expand All @@ -207,7 +208,7 @@ std::unique_ptr<SSHMaster::Connection> SSHMaster::startCommand(OsStrings && comm

// Wait for the SSH connection to be established,
// So that we don't overwrite the password prompt with our progress bar.
if (!fakeSSH && !useMaster && !isMasterRunning()) {
if (!fakeSSH && !(socketPath && isMasterRunning(*socketPath))) {
std::string reply;
try {
reply = readLine(out.readSide.get());
Expand All @@ -229,10 +230,10 @@ std::unique_ptr<SSHMaster::Connection> SSHMaster::startCommand(OsStrings && comm

#ifndef _WIN32 // TODO re-enable on Windows, once we can start processes.

std::filesystem::path SSHMaster::startMaster()
std::optional<std::filesystem::path> SSHMaster::startMaster()
{
if (!useMaster)
return {};
return std::nullopt;

auto state(state_.lock());

Expand All @@ -249,7 +250,7 @@ std::filesystem::path SSHMaster::startMaster()

auto suspension = logger->suspend();

if (isMasterRunning())
if (isMasterRunning(state->socketPath))
return state->socketPath;

state->sshMaster = startProcess(
Expand All @@ -261,10 +262,10 @@ std::filesystem::path SSHMaster::startMaster()
if (dup2(out.writeSide.get(), STDOUT_FILENO) == -1)
throw SysError("duping over stdout");

OsStrings args = {"ssh", hostnameAndUser.c_str(), "-M", "-N", "-S", state->socketPath.string()};
OsStrings args = {"ssh", hostnameAndUser.c_str(), "-M", "-N", "-oControlPersist=no"};
if (verbosity >= lvlChatty)
args.push_back("-v");
addCommonSSHOpts(args);
addCommonSSHOpts(args, state->socketPath);
auto env = createSSHEnv();
nix::execvpe(args.begin()->c_str(), stringsToCharPtrs(args).data(), stringsToCharPtrs(env).data());

Expand Down
Loading