[feature request] Enable openssh fido/u2f support

[tavrez]

Is it too soon to ask for this?
libfido2 is available on windows so I think enabling openssh fido support with --with-security-key-builtin will work

[PhilipOakley]

@tavrez Is this something you could checkout/debug using the SDK, then a simple PR if it works for you (i.e. then wrap it into the installer).

As Git-for-Windows is open source your contribution would be welcomed (with the usual quality caveats ;-).

[dscho]

What @PhilipOakley said.

@tavrez you can start by downloading Git for Windows' SDK, running the installer, then sdk cd openssh and see whether the minimal change you suggested in the PKGBUILD makes sdk build work.

[cw789]

I've tried now to build OpenSSH with --with-security-key-builtin with the sdk build - it works.

But that change alone is not enough to use OpenSSH with FIDO/U2F support on Windows.
You'll still need libfido2 on the system.

I'm not aware which libfido2 I could use then.
The Windows one or probably one build for msys2 or mingw ...

[dscho]

I'm not aware which libfido2 I could use then.
The Windows one or probably one build for msys2 or mingw ...

Our OpenSSH build is an MSYS one, so you will need the MSYS version of libfido2. There does not seem to be any at the moment, though: https://github.com/msys2/MSYS2-packages/find/master/.

[cw789]

Thanks Johannes, that's what I was afraid of.

I was thinking about to prepare my first pull request.
But that won't make sense as libfdo2 is not available.

& I'm the wrong for bringing libfido2 to MSYS. Not my terrain.

[tavrez]

Can we just compile it(libfido2 and libcbor) natively and put it inside mingw packages? I mean @cw789 did you test with any of builds?

[cw789]

No I didn't try that.

[dscho]

Can we just compile it

@tavrez sure, by our guest. Download the Git for Windows SDK, then sdk cd MSYS-packages and have a look around for a library that might be similar to libfido2, then copy-edit its PKGBUILD into a new subdirectory libfido2. You should be able to build the package via sdk build libfido2 after that. It may take a couple of cycles until the PKGBUILD is correct and working.

[tavrez]

I successfully compiled everything and tested library with both my U2F and FIDO2 keys, I will package them properly and push a PR ASAP(I probably need your help since I'm not familiar with progress).
I'm also writing a module for using instead of openssh internal library, my module use native windows hello api(like browsers) and bring more ability like using internal TPM and biometrics for key generation.

[dscho]

@tavrez you managed to surprise me! I will gladly help you with whatever process hurdles you face.

[tavrez]

At first I thought since libcbor and libfido2 can be compiled natively on Windows, it's better to compile them under mingw, but then I realised openssh can't use them so I went back to msys.

Now the problem is both libcbor and libfido2 only have cmake files, and I do not know how to build PKGBUILD for them. Should I download cmake in their PKGBUILD or you have better approach to this?

I should also mention that using fido2 in Windows 10 version 1903 and higher requires administrator privileges, this needs to be noted somewhere for end users

[rimrul]

You'll probably want a compile time dependency (makedepends in PKGBUILD) on cmake. See the PKGBUILD of jsoncpp or task for an example.

[dscho]

You'll probably want a compile time dependency (makedepends in PKGBUILD) on cmake. See the PKGBUILD of jsoncpp or task for an example.

For lurkers: https://github.com/git-for-windows/MSYS2-packages/blob/450628337752150d35673af52f18102994257a9a/task/PKGBUILD#L18 and https://github.com/git-for-windows/MSYS2-packages/blob/450628337752150d35673af52f18102994257a9a/task/PKGBUILD#L55 are probably the most relevant parts of task's PKGBUILD file.

[tavrez]

First try pushed on git-for-windows/MSYS2-packages#34

[dscho]

This is my favorite ticket of the week.

[dscho]

I'm also writing a module for using instead of openssh internal library, my module use native windows hello api(like browsers) and bring more ability like using internal TPM and biometrics for key generation.

@tavrez I am curious... Does this mean that we will be able to SSH into a remote server via Windows Hello? If so, how will this be configured, and what is your progress?

[tavrez]

From OpenSSH release notes

FIDO tokens are most commonly connected via USB but may be attached
via other means such as Bluetooth or NFC. In OpenSSH, communication
with the token is managed via a middleware library, specified by the
SecurityKeyProvider directive in ssh/sshd_config(5) or the
$SSH_SK_PROVIDER environment variable for ssh-keygen(1) and
ssh-add(1). The API for this middleware is documented in the sk-api.h
and PROTOCOL.u2f files in the source distribution.

I'm writing a module to be used in this form, and it calls Windows Hello to connect to keys instead of direct connecting to a key, it will invoke a dialog like this:

what is your progress?

Well, slowly going on, I can call windows APIs but need to learn how to wrap them in OpenSSH format.

[tavrez]

Current shipped version of git for windows (2.25.1) support external modules, as a test:

$ ssh-keygen -t ecdsa-sk -w test.dll
Generating public/private ecdsa-sk key pair.
You may need to touch your authenticator to authorize key generation.
Provider "test.dll" dlopen failed: No such file or directory
Key enrollment failed: invalid format

-w will open module and tries to use it for connecting to security key

[dscho]

I also found this very nice blog post that explains more about this feature: https://duo.com/labs/tech-notes/u2f-key-support-in-openssh

[dscho]

OpenSSH v8.2's Release Notes also have a nice section on FIDO/U2F Support, and I found yet another good blog post about this subject: https://foremostlist.com/openssh-now-supports-fido-u2f-security-keys-for-2-factor-authentication/.

[tavrez]

Thanks for all the link, I've completely read all the man pages and related codes on OpenSSH.

After couple of tests on yesterday and today I think I learned how I should wrap things up, need sometime to polish and test things, then I will push things inside my repo for tests, after that we can pull it inside msys as optional module if needed.

[tavrez]

Sadly I have to say with the current implementations I cannot make this module :-(

OpenSSH is hashing the random challenge before giving it to the sk module, see:
https://github.com/openssh/openssh-portable/blob/master/ssh-sk.c#L653-L667
However Windows API calls require the challenge itself, and it will hash the data itself , see:
https://github.com/microsoft/webauthn/blob/master/webauthn.h#L133-L153
so the challenge will be hashed two times and verification will fail.

Either OpenSSH should gives the module the original challenge or Windows API should have an option to not hash the input data. So for now, I can't do anything but sending mails to developers

[dscho]

😞

I have to say, however, that I do not quite understand the problem... whether we let the Windows API hash the original data or a hashed version thereof should not matter, as long as we're consistent about double-hashing, no (and as long as SHA-256 is safe)?

[rimrul]

If I understand it correctly, the authentication module is supposed to generate a response out of the hash of the challenge. If it generates that response out of the hash of the hash of the challenge the remote will (rightfully) reject that response as invalid.

[dscho]

Oh right, the hash is generated both on the client and on the server. My bad. Thanks for screwing on my head right @rimrul!

[tavrez]

Good news: I contacted with both Microsoft and OpenSSH team, no answer from MS till now but OpenSSH dev responded me that my suggestion about changes in API format is "reasonable". I'll try to finish my implementation and give it to them so they can change what's needed in future releases.
I'll release my work in a .dll and a ssh-sk-helper, will post them here for tests.

[tavrez]

I finished writing my module on tavrez/openssh-sk-winhello
It need some polishes as well as makefile and readme files, I'll do them as soon as possible
If you are interested test it and give me feedback :)

[dscho]

Nice! I'll check it out as soon as I'll get a chance, and hope that others will do the same.

[cw789]

@tavrez
I've just tested your OpenSSH SK WinHello implementation.
Works wonderful. Thanks a lot.

[dscho]

@tavrez do you think it'd be time to bundle this with Git for Windows, or do you think it needs more testing?

[tavrez]

I’ll do couple more tests at the weekend and will share the results

[JohnVillalovos]

I’ll do couple more tests at the weekend and will share the results

Any news on this? This looks very cool to have this feature ☺

[tavrez]

Any news on this? This looks very cool to have this feature ☺

Sorry I forgot to mention it here, due to a weird bug some users reported in tavrez/openssh-sk-winhello/issues/1 I've decided to contact Microsoft to see if they can help, it usually takes a long time to get an answer from them. If you are interested, you can download and install it manually from my repo, I'll help if you have any problems with it.

[tavrez]

Ok I'm ready to discuss about what I should do to make my module bundled inside git-for-windows :-)

[dscho]

Ok I'm ready to discuss about what I should do to make my module bundled inside git-for-windows :-)

Perfect! What about tavrez/openssh-sk-winhello#1, though?

Also, I cloned, built and installed it, tried ssh-keygen -t ecdsa-sk -w winhello.dll but it asked me to insert my security key into the USB slot. I do not (yet) have a USB security key, but hoped that I could use Windows Hello with the usual biometric verification (or a PIN). Does that work already, or is it expected not to work?

[tavrez]

Perfect! What about tavrez/openssh-sk-winhello#1, though?

Well, in short, it's ok.
Long answer: this is about a check function(IsUserVerifyingPlatformAuthenticatorAvailable) which should be called at the beginning, it's return value shows if the system supports WinHello or not. This function returned failure for some of the testers, I asked Microsoft about this and they said it should only fail during remote or vm scenarios and they asked me if I found anything else which result in failure of this test, report it back. I reported back but got no answer :D
But despite what they said users can continue and use WinHello even when this return failure, so for now I only show a warning in logs and skip this.

but hoped that I could use Windows Hello with the usual biometric verification (or a PIN). Does that work already, or is it expected not to work?

OpenSSH supports ECDSA or Ed25519 key types, if your internal TPM module support generation of these type of keys, it lets you use it, but I haven't seen any TPM module supporting these kind of keys :/

[dscho]

but hoped that I could use Windows Hello with the usual biometric verification (or a PIN). Does that work already, or is it expected not to work?

OpenSSH supports ECDSA or Ed25519 key types, if your internal TPM module support generation of these type of keys, it lets you use it, but I haven't seen any TPM module supporting these kind of keys :/

I guess that's my TPM's fault, then.

So... next step: open a PR over at https://github.com/git-for-windows/MSYS2-packages to add the openssh-sk-winhello package, loosely modeled after https://github.com/git-for-windows/MSYS2-packages/blob/HEAD/windows-default-manifest/PKGBUILD.

I am looking forward to it!

[tavrez]

Before I create PR, let's talk about these things :D

1. OpenSSH interface to FIDO modules may change with every new release, my module will stop working and I have to rewrite it for new version. As an example, let's think a new version of OpenSSH came out and you get that into repo, and before I finish a new version you start publishing new version of git-for-windows, now my module won't work and shows error. Is there anyway to automate this checking and don't make more troubles for you? As an idea which I'm not sure if it's working because I don't know the automation progress of this repo:
   I can tag my releases based on supporting OpenSSH version(e.g. v8.2-latest, v8.3-latest), so when you wanna build a new release, you check for current version of OpenSSH and get proper library from me if it exists.

2. Should I make my library enabled by default? Enabling it requires adding an environment variable and modification inside git_config. If I should, how can I do it in PKGBUILD? :D If there is an example, point it to me

3. Winhello nags about exe files not being signed, can you do me a favor and sign /usr/lib/ssh/ssh-sk-helper.exe and my dll and check if it's going away? (even if you can't/don't want to do it in final release, please test it for me, need to talk about it with MS)

4. totaly unrelevant: when I update git-sdk with pacman, is there any point on doing git pull in root? :D

5. another unrelevant: is there any reason for not having pacman on final release? There are many good packages in MSYS-repo of git-for-windows which are not available in final release(I really want man)

[dscho]

OpenSSH interface to FIDO modules may change with every new release, my module will stop working and I have to rewrite it for new version.

How about adding some automated build to check for compatibility? Is there a way to test-load your helper in a headless environment, i.e. without actually authenticating anything? Sort of a dry run?

Should I make my library enabled by default?

I think that would be neat.

Enabling it requires adding an environment variable and modification inside git_config.

Oh. Can we somehow introduce a good default if that environment variable is not set? And what config setting is required? I thought this was purely on the SSH side...

Winhello nags about exe files not being signed, can you do me a favor and sign /usr/lib/ssh/ssh-sk-helper.exe and my dll and check if it's going away?

In my test, that nag indeed goes away.

How about editing openssh/PKGBUILD to include something like this?

test -z "$SIGNTOOL" ||
eval "$SIGNTOOL" ssh-sk-helper.exe

I can then easily set it up in the automated builds of OpenSSH.

when I update git-sdk with pacman, is there any point on doing git pull in root?

That is how I like to update my SDK, the biggest problem is it does not track all the packages, so if you have installed, say, msys2-runtime-devel, that does not get updated. You'll still have to run pacman -Su after the pull. And to make the git status output clean, you will also have to run /usr/src/build-extra/commit-msys2.sh ignore -a.

is there any reason for not having pacman on final release?

Yes. The end user installation is intended to live in C:\Program Files\Git, which is write-protected, and very much so on purpose. Therefore having pacman.exe would be rather pointless.

It does not seem to me as if you really wanted Git for Windows, you're more interested in something like MSYS2 (or Git for Windows' SDK, which is a close cousin of MSYS2).

[tavrez]

How about adding some automated build to check for compatibility? Is there a way to test-load your helper in a headless environment, i.e. without actually authenticating anything? Sort of a dry run?

Yes,I can create a mini app to load the dll and write its API version, but you also need to check API version of ssh, I'm not sure if it's possible.

Oh. Can we somehow introduce a good default if that environment variable is not set? And what config setting is required? I thought this was purely on the SSH side...

If we do not set env variable, ssh will use its internal implementation(libfido2 one which requires admin privileges), but if we set something and it doesn't work(e.g. version mismatch), it will fail and won't fallback to it's internal build.
In ssh_config we should have something like this:

Host *
	SecurityKeyProvider winhello.dll

In my test, that nag indeed goes away.

Perfect! Thank you, if signing is available in auto build, it is gonna be so great.

It does not seem to me as if you really wanted Git for Windows

I love Git for Windows, you have many great packages in git-for-windows/MSYS-packages , just wanted to see them in action! I can continue to add packages manually into it

[dscho]

How about adding some automated build to check for compatibility? Is there a way to test-load your helper in a headless environment, i.e. without actually authenticating anything? Sort of a dry run?

Yes,I can create a mini app to load the dll and write its API version, but you also need to check API version of ssh, I'm not sure if it's possible.

TBH I was kind of hoping for some sort of built-in functionality where you could, say, query a "module version" or some such. I'd rather not reimplement DLL loading in a separate tool because it's not what we want to verify, we want to verify that ssh-keygen.exe can use the .dll.

If we do not set env variable, ssh will use its internal implementation(libfido2 one which requires admin privileges), but if we set something and it doesn't work(e.g. version mismatch), it will fail and won't fallback to it's internal build.

Hmm. Maybe it is time to add a patch to git-for-windows/MSYS2-packages' openssh/, to make this configurable via a config file.

I am really wary of requiring an environment variable because there are so many ways to call into Git, and we do not want to prevent users from overriding config settings via their own environment variables.

I mean, we could potentially teach git-wrapper.c and compat/mingw.c to set that environment variable (unless unset), as we do here and here. But it is not very elegant, and it does not allow the user to force-unset that environment variable.

[tavrez]

we want to verify that ssh-keygen.exe can use the .dll

It's possible from their source files, sk-api.h is available on both openssh and my code, they should define same (SSH_SK_VERSION_MAJOR & SSH_SK_VERSION_MAJOR_MASK)

I am really wary of requiring an environment variable because there are so many ways to call into Git, and we do not want to prevent users from overriding config settings via their own environment variables.

How about a new file in /etc/profile.d ? exporting env we need in it. and if users define same in .bashrc, it can overwrite ours

[dscho]

we want to verify that ssh-keygen.exe can use the .dll

It's possible from their source files, sk-api.h is available on both openssh and my code, they should define same (SSH_SK_VERSION_MAJOR & SSH_SK_VERSION_MAJOR_MASK)

I am more worried about having a binary .dll lying around and not knowing whether it will work correctly or not. If OpenSSH would have a command to test that (i.e. to verify that the .dll implements the expected protocol version), that would be really helpful.

I am really wary of requiring an environment variable because there are so many ways to call into Git, and we do not want to prevent users from overriding config settings via their own environment variables.

How about a new file in /etc/profile.d ? exporting env we need in it. and if users define same in .bashrc, it can overwrite ours

When I said:

there are so many ways to call into Git

I did mean it. Your /etc/profile.d/ idea would work only for users of Git Bash. But not for users who call git.exe in their CMD or PowerShell.

[klausenbusk]

FWIW: FIDO2/U2F support was just merged into PowerShell's fork of openssh-portable: PowerShell/openssh-portable#541

[dscho]

I think we have some FIDO2 support in our current OpenSSH, but it would need for someone other than me to drive this forward.

[klausenbusk]

I just tested the latest version of MSYS2 and my FIDO/U2F key works perfectly with Windows Hello. Looking at the PRs to upstream's MSYS2-packages repository, I guess msys2/MSYS2-packages#2875 and msys2/MSYS2-packages#2884 must be added to https://github.com/git-for-windows/MSYS2-packages, for FIDO/U2F keys to work with Windows Hello in Git for Windows.

I may have time to send a few PRs, but anyone else is also free to do it :)

[dscho]

I may have time to send a few PR

Please do.

We already link libfido2 and libcbor, but when I recently tried to upgrade to a new-enough libfido2 version to have support for Windows Hello, I ran into a segfault.

If you can resolve all that, I will be happy to merge the PR(s) and build the packages.