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 (~~https://github.com/rust-lang/git2-rs/pull/1032 which bumps the version of libgit2~~ (update: landed), and https://github.com/rust-lang/git2-rs/pull/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 [email protected]: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 https://github.com/martinvonz/jj/pull/4080#issuecomment-2227480907.

[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?