Enable support for FIDO2/U2F security keys #541

martelletto · 2021-11-19T09:38:12Z

This PR enables client-side support for FIDO2/U2F security keys, allowing the generation and use of ecdsa-sk and ed25519-sk SSH keys on cross-platform and platform authenticators, from PowerShell and WSL.

Build and test instructions can be found at here. I have uploaded Win64 and ARM64 symbols. (GitHub wouldn't let me attach them to the PR.)

Thank you,

-p.

CC @akshayku

requested by akshayku@

only prompt for a FIDO PIN if sshsk_enroll() fails with SSH_ERR_KEY_WRONG_PASSPHRASE, which never happens when sk-usbhid uses webauthn.dll and means we are communicating directly with a security key.

only prompt for a FIDO PIN if sshsk_sign() fails with SSH_ERR_KEY_WRONG_PASSPHRASE, which never happens when sk-usbhid uses webauthn.dll and means we are communicating directly with a security key.

ghost · 2021-11-19T09:38:25Z

All CLA requirements met.

only prompt for a FIDO PIN if sshsig_wrap_sign() fails with SSH_ERR_KEY_WRONG_PASSPHRASE, which never happens when sk-usbhid uses webauthn.dll and means we are communicating directly with a security key.

only prompt for a FIDO PIN if sshkey_certify() fails with SSH_ERR_KEY_WRONG_PASSPHRASE, which never happens when sk-usbhid uses webauthn.dll and means we are communicating directly with a security key.

martelletto · 2021-11-29T15:15:09Z

I have pushed two commits to add support for git commit signatures using FIDO2/U2F security keys. In addition to the two commits, you will need a version of git built with git-for-windows/git#3561.

bagajjal

Appreciate your time for adding this functionality.

contrib/win32/openssh/GetFIDO2.ps1

contrib/win32/openssh/config.h.vs

bagajjal · 2021-12-01T02:15:48Z

contrib/win32/openssh/paths.targets

    <LibreSSL-Path>$(SolutionDir)\LibreSSL\sdk\</LibreSSL-Path>
    <LibreSSL-x86-Path>$(SolutionDir)\LibreSSL\bin\desktop\x86\</LibreSSL-x86-Path>
    <LibreSSL-x64-Path>$(SolutionDir)\LibreSSL\bin\desktop\x64\</LibreSSL-x64-Path>
    <LibreSSL-arm64-Path>$(SolutionDir)\LibreSSL\bin\desktop\arm64\</LibreSSL-arm64-Path>
    <LibreSSL-arm-Path>$(SolutionDir)\LibreSSL\bin\desktop\arm\</LibreSSL-arm-Path>
+    <fido2-Path>$(SolutionDir)\libfido2\</fido2-Path>
+    <fido2-x86-Path>$(SolutionDir)\libfido2\Win32\Release\v143\static\</fido2-x86-Path>


v143 -> I'm guessing it's compiled with visual studio 2022.
Win32-openssh is compiled with visual studio 2015, windows SDK v8.1.

Does it have any compatibility issues?

Can FIDO be compiled with visual studio 2015, windows SDK v10?
We might switch to windows SDK v10 from windows SDK v8.1.

I am not aware of any incompatibility. I had to disable a warning (C4201; nonstandard extension used: nameless struct/union) to build libfido2 with Windows SDK 8.1 (using Visual Studio 2022). I don't have Visual Studio 2015 on my laptop.

Building with Windows SDK 8.1 using Visual Studio 2015 works as well.

Can you please remove the v143 here. Having this makes us change this as we move to different version of visual studio

We can change it once the vendoring process is complete, as it needs to match the SDK version used to build libfido2. CC @anmenaga

sk-usbhid.c

ssh-sk-client.c

sshconnect2.c

sshsig.c

…d by bagajjal@

… by bagajjal@

tavrez · 2021-12-05T14:21:43Z

Have you implemented sk_load_resident_keys?

martelletto · 2021-12-05T14:37:49Z

@tavrez sk_load_resident_keys requires FIDO 2.1 Credential Management, which isn't abstracted by webauthn.h AFAIK. It should work when talking directly to the authenticator (in administrator mode or older versions of Windows), but won't work otherwise.

tavrez · 2021-12-05T14:43:15Z

webauthn.dll inside Windows 11 is version 3, I think it supports FIDO 2.1(according to this commit).
But I haven't found anything related to getting stored keys inside it.

these are needed by sk-usbhid.c when operating in environments with multiple authenticators attached.

unless explicitly specified, don't consider a sk_provider a key constraint, allowing ssh-sk keys using the default internal provider to be added with SSH2_AGENTC_ADD_IDENTITY instead of SSH2_AGENTC_ADD_ID_CONSTRAINED.

a sk_provider is required by ssh-sk-helper. as such, treat ssh-sk keys without a provider as belonging to the "internal" provider.

martelletto · 2021-12-09T15:41:09Z

I have included two commits (7393b48 and b31d77e) adding ssh-agent support for ecdsa-sk/ed25519-sk.

contrib/win32/openssh/OpenSSHBuildHelper.psm1

sk-usbhid.c

.gitignore

no functional change

bagajjal · 2021-12-16T00:44:52Z

sk-usbhid.c

@@ -721,12 +733,31 @@ sk_enroll(uint32_t alg, const uint8_t *challenge, size_t challenge_len,
 		skdebug(__func__, "fido_cred_set_type: %s", fido_strerr(r));
 		goto out;
 	}
-	if ((r = fido_cred_set_clientdata_hash(cred, challenge,
-	    challenge_len)) != FIDO_OK) {
+#ifndef WINDOWS


Is there a reason why you are adding line 737 to 741 as it's new code for Unix?

https://github.com/openssh/openssh-portable/blob/V_8_6/sk-usbhid.c#L724

I used V_8_8 as reference: https://github.com/openssh/openssh-portable/blob/V_8_8/sk-usbhid.c#L724. I could trim that chunk to reduce the difference against V_8_6, but we would have to revisit it in V_8_8 or later. Let me know if you prefer that approach.

sk-usbhid.c

martelletto · 2021-12-16T16:22:14Z

CI is failing with:

Build execution time has reached the maximum allowed time for your plan (60 minutes).

Successful runs seem to take ~54 minutes, so we are very close to the limit. I could keep pushing empty commits until we are lucky and CI passes again, but perhaps it would make sense to bump the limit.

anmenaga · 2021-12-17T22:07:43Z

CI is failing with:

Build execution time has reached the maximum allowed time for your plan (60 minutes).

Successful runs seem to take ~54 minutes, so we are very close to the limit. I could keep pushing empty commits until we are lucky and CI passes again, but perhaps it would make sense to bump the limit.

I've sent them a request to increase it to 90 minutes. Hopefully it gets approved.

martelletto added 12 commits November 11, 2021 10:03

sk-usbhid: use fido_{assert,cred}_set_clientdata()

3c200c2

sk-usbhid: always prefer windows://hello if device == NULL

41c396c

sk-usbhid: randomise user_id by default

bb6503c

requested by akshayku@

ssh-sk-client: use posix_spawn() to run ssh-sk-helper

867d766

ssh-keygen: don't prompt for FIDO PINs by default on Windows

e582f7b

only prompt for a FIDO PIN if sshsk_enroll() fails with SSH_ERR_KEY_WRONG_PASSPHRASE, which never happens when sk-usbhid uses webauthn.dll and means we are communicating directly with a security key.

ssh: don't prompt for FIDO PINs by default on Windows

ba76466

only prompt for a FIDO PIN if sshsk_sign() fails with SSH_ERR_KEY_WRONG_PASSPHRASE, which never happens when sk-usbhid uses webauthn.dll and means we are communicating directly with a security key.

fetch libfido2 using an auxiliary script

3123f25

build definitions for libfido2

fff6e73

build ssh-sk-helper

bd0c9b9

enable ssh-sk

73a3af6

gitignore bits

6de170f

README blurb

98e9c68

martelletto added 2 commits November 29, 2021 13:57

sshsig: don't prompt for FIDO PINs by default on Windows

0d4e6f6

only prompt for a FIDO PIN if sshsig_wrap_sign() fails with SSH_ERR_KEY_WRONG_PASSPHRASE, which never happens when sk-usbhid uses webauthn.dll and means we are communicating directly with a security key.

ssh-keygen -s: don't prompt for FIDO PINs by default on Windows

14e3877

only prompt for a FIDO PIN if sshkey_certify() fails with SSH_ERR_KEY_WRONG_PASSPHRASE, which never happens when sk-usbhid uses webauthn.dll and means we are communicating directly with a security key.

joshbressers mentioned this pull request Nov 29, 2021

Expand Key Usage Guide ossf/great-mfa-project#32

Merged

bagajjal requested changes Dec 1, 2021

View reviewed changes

martelletto added 9 commits December 1, 2021 09:53

sk-usbhid: +#ifdef WINDOWS; requested by bagajjal@

1e3d6d4

sshsig: merge sshsig_sign_fd(), requested by bagajjal@

3dd1a01

ssh-sk-client: merge start_helper(), requested by bagajjal@

dee6c5e

identity_sign: add a clarification comment

783697f

pathnames: adjust _PATH_SSH_SK_HELPER on Windows

ed39b66

fix previous

3e47db8

ssh-sk-client: use GetModuleFileNameW(), requested by bagajjal@

6b54b8d

ssh-keygen: move Windows-specific bits under #ifdef WINDOWS; requeste…

7606c43

…d by bagajjal@

sk-usbhid: move Windows-specific bits under #ifdef WINDOWS; requested…

b6c982f

… by bagajjal@

MrRinkana mentioned this pull request Dec 5, 2021

Enable FIDO support PowerShell/Win32-OpenSSH#1804

Closed

martelletto mentioned this pull request Dec 7, 2021

Missing Windows Install Instuctions Yubico/libfido2#479

Closed

martelletto added 3 commits December 8, 2021 14:10

config.h: define HAVE_FIDO_DEV_GET_TOUCH_{BEGIN,STATUS}

deecf32

these are needed by sk-usbhid.c when operating in environments with multiple authenticators attached.

ssh-add: don't consider a sk_provider a key constraint

7393b48

unless explicitly specified, don't consider a sk_provider a key constraint, allowing ssh-sk keys using the default internal provider to be added with SSH2_AGENTC_ADD_IDENTITY instead of SSH2_AGENTC_ADD_ID_CONSTRAINED.

ssh-agent: promote sk_provider == NULL to "internal"

b31d77e

a sk_provider is required by ssh-sk-helper. as such, treat ssh-sk keys without a provider as belonging to the "internal" provider.

README: mention FIDO2 ssh-agent support

e7b9c09