Private key with CRLF line endings causes "error in libcrypto" with bundled OpenSSH

[richardebeling]

• I was not able to find an open or closed issue matching what I'm seeing

Setup

I installed Git-2.43.0-64-bit.exe on Windows 10 Pro (64-bit) with the default "Use bundled OpenSSH" selection

$ git --version --build-options
git version 2.43.0.windows.1
cpu: x86_64
built from commit: 4b968f3ea3b32a7bc50846bab49f3f381841d297
sizeof-long: 4
sizeof-size_t: 8
shell-path: /bin/sh
feature: fsmonitor--daemon

$ cmd.exe /c ver
Microsoft Windows [Version 10.0.19045.3693]

$ cat /etc/install-options.txt
Editor Option: VIM
Custom Editor Path:
Default Branch Option:
Path Option: Cmd
SSH Option: OpenSSH
Tortoise Option: false
CURL Option: OpenSSL
CRLF Option: CRLFAlways
Bash Terminal Option: MinTTY
Git Pull Behavior Option: Merge
Use Credential Manager: Disabled
Performance Tweaks FSCache: Enabled
Enable Symlinks: Enabled
Enable Pseudo Console Support: Disabled
Enable FSMonitor: Disabled

Details

I want to connect to an SSH server using pubkey authentication. The private key file was created automatically by vagrant and can be read by Windows' bundled OpenSSH as well as PuttyGen. It uses CRLF as end-of-line markers.

The bundled SSH client doesn't load this private key, but instead prints

Load key "path/to/key": error in libcrypto

When changing the line endings to LF, loading it succeeds.

$ file -k .vagrant/machines/default/virtualbox/private_key
.vagrant/machines/default/virtualbox/private_key: OpenSSH private key\012- , ASCII text, with CRLF line terminators

$ ssh vagrant@127.0.0.1 -p 2222 -i .vagrant/machines/default/virtualbox/private_key
Load key ".vagrant/machines/default/virtualbox/private_key": error in libcrypto
vagrant@127.0.0.1: Permission denied (publickey).

$ dos2unix .vagrant/machines/default/virtualbox/private_key
dos2unix: converting file .vagrant/machines/default/virtualbox/private_key to Unix format...

$ ssh vagrant@127.0.0.1 -p 2222 -i .vagrant/machines/default/virtualbox/private_key
Welcome to Ubuntu 22.04.3 LTS (GNU/Linux 5.15.0-87-generic x86_64)
# [...]

[dscho]

The private key file was [...] can be read by Windows' bundled OpenSSH [... and it ...] uses CRLF as end-of-line markers.

The bundled SSH client doesn't load this private key [...]

The reason is most likely PowerShell/openssh-portable@41e4e89#diff-911b87e474316fdc8c48074a84d40562d1b0e7f06fa587d9b8f13926c539d81c which has not made it into Git for Windows' bundled version of OpenSSH.

[dscho]

@richardebeling how urgent do you need this? Urgently enough to backport the patch yourself and to open a PR?

[richardebeling]

I can try to look into it. I haven't done any development for msys, cygwin or similar environments, so I'm not sure if I fully understand the impact of potential changes. From my understanding, git-for-windows maintains a fork of the msys2 packages, including the openssh package, which ends up in the installer. I don't think the upstreams (both msys2/MSYS2-packages and openssh/openssh-portable) would want to merge CRLF support since they focus on unix/posix environments.

To locally fix this issue, it seems sane to me to add a new 0006-allow-crlf-in-ssh-keys.patch to "our" MSYS2-packages/openssh that adds the changes you linked. I will try to prepare a PR for that.

Where would a test for this go? git-for-windows/MSYS2-packages doesn't seem to run tests in CI, but it's not related to git, so the tests in this repo (./t/) also seem inappropriate to me.

Could it, in the long run, make sense to use PowerShell/openssh-portable for the bundled SSH instead? I'd imagine that they improve other aspects of windows interoperability as well. This might have negative side effects in an msys environment that I'm currently unaware of, though.

I'm a bit confused by the git-for-windows/MSYS2-packages fork diverging from its upstream (same for git-for-windows/MINGW-packages) -- is the workflow documented somewhere?

[rimrul]

I can try to look into it. I haven't done any development for msys, cygwin or similar environments, so I'm not sure if I fully understand the impact of potential changes. From my understanding, git-for-windows maintains a fork of the msys2 packages,

That's pretty correct so far. Though we do pick up some packages directly from MSYS2.

including the openssh package, which ends up in the installer. I don't think the upstreams (both msys2/MSYS2-packages and openssh/openssh-portable) would want to merge CRLF support since they focus on unix/posix environments.

MSYS2 might care about the increased interoperability with native Windows tools, but we can always upstream the patch later.

To locally fix this issue, it seems sane to me to add a new 0006-allow-crlf-in-ssh-keys.patch to "our" MSYS2-packages/openssh that adds the changes you linked.

Yes, that's a good aproach.

I will try to prepare a PR for that.

Where would a test for this go?

Your patch could add a test to regress/. The corresponding make tests is already part of the check() function of the PKGBUILD file.

git-for-windows/MSYS2-packages doesn't seem to run tests in CI,

We do run check() as part of build-and-deploy.yml

but it's not related to git, so the tests in this repo (./t/) also seem inappropriate to me.

Could it, in the long run, make sense to use PowerShell/openssh-portable for the bundled SSH instead?

Probably not. Most of those changes don't make sense for an Msys2 executable.

I'd imagine that they improve other aspects of windows interoperability as well. This might have negative side effects in an msys environment that I'm currently unaware of, though.

I'm a bit confused by the git-for-windows/MSYS2-packages fork diverging from its upstream (same for git-for-windows/MINGW-packages) -- is the workflow documented somewhere?

There isn't much of a workflow. The histories diverge at some point and we update the package definitions for the packages we build ourselves when/if needed.

[dscho]

I agree with all that @rimrul said.

Could it, in the long run, make sense to use PowerShell/openssh-portable for the bundled SSH instead?

Probably not. Most of those changes don't make sense for an Msys2 executable.

Here is an interesting thought: we could establish mingw-w64-openssh (i.e. a non-MSYS variant of SSH), and bundle that in Git for Windows.

The only downside that I can think of is that interactive SSH usage might change behavior because it no longer can use the pseudo terminal support provided by the MSYS2 runtime. However, interactive SSH support is not exactly included in Git for Windows' "contract".

[rimrul]

Here is an interesting thought: we could establish mingw-w64-openssh (i.e. a non-MSYS variant of SSH), and bundle that in Git for Windows.

That could work.

[dscho]

I can try to look into it.

@richardebeling please note that I won't have time to get to this, so if you need this, please open a PR.