SSH: Use OpenSSH instead of libssh2 for authentication with Git hosts

[bnjmnt4n]

This PR switches to OpenSSH instead of libssh2 for authentication with Git hosts. It allows configuration specified in ~/.ssh/config to be read when connecting via SSH, e.g. specifying custom keys to use for different hosts, or using a different SSH agent. In general, the OpenSSH-based connection seems slightly more robust then the libssh2 version, but I think there are still some issues which do not work well (eg., it didn't work for me when I hadn't connected to the host before, and didn't display the unknown host interaction that the ssh binary does).

I still need to look into whether this affects other workflows (eg. git credentials).

Marking this as a draft since this also depends on upstream PRs to land (rust-lang/git2-rs#1032 which bumps the version of libgit2 (update: landed), and rust-lang/git2-rs#1031 which builds libgit2 using OpenSSH for SSH execution), but I'm creating the PR for improved visibility.

Closes #63.

If you're interested in using this, the following code can be used to build this branch locally:

git clone git@github.com:bnjmnt4n/jj && cd jj
git checkout ssh-openssh
cargo install --path cli --locked --bin jj

Checklist

If applicable:

• I have updated CHANGELOG.md
• I have updated the documentation (README.md, docs/, demos/)
• I have updated the config schema (cli/src/config-schema.json)
• I have added tests to cover my changes

[bnjmnt4n]

Oddly enough, building this as a Nix derivation no longer works correctly with my IdentityAgent SSH config...

[khionu]

Could you include a list of reasons for this switch in the PR description? Would be good for historical purposes, especially for elsewhere in Rust where people may try to make the case for switching.

[khionu]

Could you link said blocking PRs?

[bnjmnt4n]

@khionu updated!

[joyously]

I'm wondering if other SSH implementations are susceptible also.
https://www.wired.com/story/xz-backdoor-everything-you-need-to-know/

Malicious code added to xz Utils versions 5.6.0 and 5.6.1 modified the way the software functions. The backdoor manipulated sshd, the executable file used to make remote SSH connections. Anyone in possession of a predetermined encryption key could stash any code of their choice in an SSH login certificate, upload it, and execute it on the backdoored device.

Wait, How Can a Compression Utility Manipulate a Process as Security-Sensitive as SSH?

Any library can tamper with the inner workings of any executable it is linked against.
OpenSSH, the most popular sshd implementation, doesn’t link the liblzma library, but Debian and many other Linux distributions add a patch to link sshd to systemd, a program that loads a variety of services during the system bootup. Systemd, in turn, links to liblzma, and this allows XZ Utils to exert control over sshd.

[khionu]

I'm wondering if other SSH implementations are susceptible also.

Not relevant to us, the exploit targeted the server-side daemon

[HybridEidolon]

I anticipate that changing to OpenSSH will probably fix the same issue on Windows that #3554 is attempting to address. Hard to imagine it being anything other than strictly better.

[emilazy]

The following patch fixes the Nix build for me:

diff --git a/Cargo.lock b/Cargo.lock
index 4b1d4c56f8...9a1f618ca9 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1846,7 +1846,6 @@
 dependencies = [
  "cc",
  "libc",
- "libssh2-sys",
  "libz-sys",
  "openssl-sys",
  "pkg-config",
@@ -1864,20 +1863,6 @@
 ]
 
 [[package]]
-name = "libssh2-sys"
-version = "0.3.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "2dc8a030b787e2119a731f1951d6a773e2280c660f8ec4b0f5e1505a386e71ee"
-dependencies = [
- "cc",
- "libc",
- "libz-sys",
- "openssl-sys",
- "pkg-config",
- "vcpkg",
-]
-
-[[package]]
 name = "libz-sys"
 version = "1.1.16"
 source = "registry+https://github.com/rust-lang/crates.io-index"
diff --git a/flake.nix b/flake.nix
index 4fba5310ff...c06feae22c 100644
--- a/flake.nix
+++ b/flake.nix
@@ -84,7 +84,7 @@
 
           cargoLock.lockFile = ./Cargo.lock;
           cargoLock.outputHashes = {
-            "git2-0.18.3" = "sha256-Kfg3xWIAarAxeIo2wL30OFni7X4Thf9EzaXbFTWsehE=";
+            "git2-0.18.3" = "sha256-3g7ajPfLfuPWh46rIa70wQRWLZ+jZXBApkyPlJULi/I=";
           };
           nativeBuildInputs = with pkgs; [
             gzip

(An ironically retro way to share the change, I realize!)

[ErichDonGubler]

The CI failure for this PR seems to be the same as for #4080 (comment).

[dbarnett]

Also closes #401.

Looks like one of the two upstream PRs has landed and we're just waiting on rust-lang/git2-rs#1031 to finalize this? Or are there other blockers?

[tim-janik]

Could you include a list of reasons for this switch in the PR description? Would be good for historical purposes, especially for elsewhere in Rust where people may try to make the case for switching.

One scenario I just ran into with JJ is when connections are only possible when ~/.ssh/config must be read to make a connection. With local login sessions, ssh-agent takes care of reading ~/.ssh/config and providing keys, but that is not the case in remote ssh sessions, which have env variables that point to a remote ssh-agent. For example:

• Machine B has a repo, local logins will use the local ssh-agent which reads ~/.ssh/config and thus has the key, so JJ commands from local login sessions work correctly.
• Machine A has its own ssh-agent, when doing ssh B, the ssh session has SSH_* variables that point to the ssh-agent from machine A.
• So using libssh2 on B in an ssh session originating at machine A will never read B:~/.ssh/config and thus not find the needed key.
• To inspect the SSH setup, try env | fgrep SSH in a local session and then in an ssh session.

Examples for scenarios where ~/.ssh/config must be read in order to establish a connection are:

• Network topology forces connections through a proxy, like using ProxyCommand for a corporate VPN
• You have several ssh keys. When ssh connects it tries a default set in some order and then gives up, unless .ssh/config associates a host with a specific key via IdentityFile and has IdentitiesOnly=yes in order to avoid Too many authentication failures