Skip to content

Commit

Permalink
Merge pull request #2884 from lazka/cygwin-winhello-patches
Browse files Browse the repository at this point in the history
openssh: import winhello patches from cygwin
  • Loading branch information
lazka authored Mar 9, 2022
2 parents 26f6686 + 2df46dd commit da73cbc
Show file tree
Hide file tree
Showing 7 changed files with 334 additions and 3 deletions.
45 changes: 45 additions & 0 deletions openssh/0001-compat-code-for-fido_dev_is_winhello.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
From 35535856b9457cbc6d999bd4ac1a3ef695b3f69e Mon Sep 17 00:00:00 2001
From: Corinna Vinschen <vinschen@redhat.com>
Date: Thu, 10 Feb 2022 18:19:29 +0100
Subject: [PATCH 1/6] compat code for fido_dev_is_winhello()

Signed-off-by: Corinna Vinschen <vinschen@redhat.com>
---
configure.ac | 1 +
sk-usbhid.c | 8 ++++++++
2 files changed, 9 insertions(+)

diff --git a/configure.ac b/configure.ac
index 17fb1e609fbd..473dc38e374a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -3235,6 +3235,7 @@ if test "x$enable_sk" = "xyes" -a "x$enable_sk_internal" = "xyes" ; then
fido_dev_get_touch_begin \
fido_dev_get_touch_status \
fido_dev_supports_cred_prot \
+ fido_dev_is_winhello \
])
LIBS="$saved_LIBS"
AC_CHECK_HEADER([fido.h], [],
diff --git a/sk-usbhid.c b/sk-usbhid.c
index 2d36ac337ffa..721076c7f4a4 100644
--- a/sk-usbhid.c
+++ b/sk-usbhid.c
@@ -381,6 +381,14 @@ fido_assert_set_clientdata(fido_assert_t *assert, const u_char *ptr, size_t len)
}
#endif /* HAVE_FIDO_ASSERT_SET_CLIENTDATA */

+#ifndef HAVE_FIDO_DEV_IS_WINHELLO
+static bool
+fido_dev_is_winhello(const fido_dev_t *)
+{
+ return false;
+}
+#endif /* HAVE_FIDO_DEV_IS_WINHELLO */
+
/* Check if the specified key handle exists on a given sk. */
static int
sk_try(const struct sk_usbhid *sk, const char *application,
--
2.35.1

Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
From b9f7ac867f89d2bd3ca2c4cdd99db0a259e4b7b3 Mon Sep 17 00:00:00 2001
From: Corinna Vinschen <vinschen@redhat.com>
Date: Tue, 15 Feb 2022 11:28:08 +0100
Subject: [PATCH 2/6] check_sk_options: add temporary WinHello workaround

Up to libfido 1.10.0, WinHello advertises "clientPin" rather
than "uv" capability. This is fixed in 1.11.0. For the time
being, workaround it here.

Signed-off-by: Corinna Vinschen <vinschen@redhat.com>
---
sk-usbhid.c | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/sk-usbhid.c b/sk-usbhid.c
index 721076c7f4a4..caa18216bab5 100644
--- a/sk-usbhid.c
+++ b/sk-usbhid.c
@@ -450,6 +450,15 @@ check_sk_options(fido_dev_t *dev, const char *opt, int *ret)
skdebug(__func__, "device is not fido2");
return 0;
}
+ /*
+ * Workaround required up to libfido2 1.10.0. As soon as 1.11.0
+ * is released and updated in the Cygwin release, we can drop this.
+ */
+ if (fido_dev_is_winhello(dev) && strcmp (opt, "uv") == 0) {
+ skdebug(__func__, "device is winhello");
+ *ret = 1;
+ return 0;
+ }
if ((info = fido_cbor_info_new()) == NULL) {
skdebug(__func__, "fido_cbor_info_new failed");
return -1;
--
2.35.1

Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
From 1664911f85dfa5474a320097665752083cbbef40 Mon Sep 17 00:00:00 2001
From: Corinna Vinschen <vinschen@redhat.com>
Date: Tue, 15 Feb 2022 11:48:54 +0100
Subject: [PATCH 3/6] sk_enroll: don't drop SSH_SK_USER_VERIFICATION_REQD flag
from response

Generating two different keys with -O verify-required may have different
results on different systems. The resulting public key might become
unusable with "uv" devices. The decision whether to use "uv" or
"clientPin" method should be performed when signing, not when enrolling.

Signed-off-by: Corinna Vinschen <vinschen@redhat.com>
---
sk-usbhid.c | 7 -------
1 file changed, 7 deletions(-)

diff --git a/sk-usbhid.c b/sk-usbhid.c
index caa18216bab5..58c6a775959b 100644
--- a/sk-usbhid.c
+++ b/sk-usbhid.c
@@ -913,13 +913,6 @@ sk_enroll(uint32_t alg, const uint8_t *challenge, size_t challenge_len,
goto out;
}
response->flags = flags;
- if ((flags & SSH_SK_USER_VERIFICATION_REQD)) {
- if (check_sk_options(sk->dev, "uv", &internal_uv) == 0 &&
- internal_uv != -1) {
- /* user verification handled by token */
- response->flags &= ~SSH_SK_USER_VERIFICATION_REQD;
- }
- }
if (pack_public_key(alg, cred, response) != 0) {
skdebug(__func__, "pack_public_key failed");
goto out;
--
2.35.1

Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
From e7f401c7d0e60762b47b9bf801b17043a0ad724e Mon Sep 17 00:00:00 2001
From: Corinna Vinschen <vinschen@redhat.com>
Date: Fri, 11 Feb 2022 14:33:41 +0100
Subject: [PATCH 4/6] sk_sign: set FIDO2 uv attribute explicitely for WinHello

WinHello via libfido2 performs user verification by default.
However, if we stick to that, there's no way to differentiate
between keys created with or without "-O verify-required".
Set FIDO2 uv attribute explicitely to FIDO_OPT_FALSE, then check
if user verification has been requested.

Signed-off-by: Corinna Vinschen <vinschen@redhat.com>
---
sk-usbhid.c | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/sk-usbhid.c b/sk-usbhid.c
index 58c6a775959b..e906eb3d4999 100644
--- a/sk-usbhid.c
+++ b/sk-usbhid.c
@@ -1142,6 +1142,14 @@ sk_sign(uint32_t alg, const uint8_t *data, size_t datalen,
skdebug(__func__, "fido_assert_set_up: %s", fido_strerr(r));
goto out;
}
+ /*
+ * WinHello requests the PIN by default. Make "uv" request explicit
+ * to allow keys with and without -O verify-required to make sense.
+ */
+ if (pin == NULL && fido_dev_is_winhello (sk->dev) &&
+ (r = fido_assert_set_uv(assert, FIDO_OPT_FALSE)) != FIDO_OK) {
+ skdebug(__func__, "fido_assert_set_uv: %s", fido_strerr(r));
+ }
if (pin == NULL && (flags & SSH_SK_USER_VERIFICATION_REQD)) {
if (check_sk_options(sk->dev, "uv", &internal_uv) < 0 ||
internal_uv != 1) {
--
2.35.1

Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
From 0ef134636e09340278c8cac6cf402dbfdce3ccfc Mon Sep 17 00:00:00 2001
From: Corinna Vinschen <vinschen@redhat.com>
Date: Sat, 12 Feb 2022 17:59:04 +0100
Subject: [PATCH 5/6] if WinHello device is present, use it exclusively

Disable usage of direct USB device access for admin users on systems
supporting WinHello. The actual device is the same anyway, just handled
via the WinHello abstraction layer. This also avoids calling sk_try if
WinHello is present.

Signed-off-by: Corinna Vinschen <vinschen@redhat.com>
---
sk-usbhid.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)

diff --git a/sk-usbhid.c b/sk-usbhid.c
index e906eb3d4999..98b69ad4e362 100644
--- a/sk-usbhid.c
+++ b/sk-usbhid.c
@@ -504,6 +504,13 @@ sk_select_by_cred(const fido_dev_info_t *devlist, size_t ndevs,
skv[0] = NULL;
goto out;
}
+ for (i = 0; i < skvcnt; i++) {
+ if (fido_dev_is_winhello (skv[i]->dev)) {
+ sk = skv[i];
+ skv[i] = NULL;
+ goto out;
+ }
+ }
sk = NULL;
for (i = 0; i < skvcnt; i++) {
if (sk_try(skv[i], application, key_handle,
@@ -540,6 +547,13 @@ sk_select_by_touch(const fido_dev_info_t *devlist, size_t ndevs)
}
goto out;
}
+ for (idx = 0; idx < skvcnt; idx++) {
+ if (fido_dev_is_winhello (skv[idx]->dev)) {
+ sk = skv[idx];
+ skv[idx] = NULL;
+ goto out;
+ }
+ }
#ifndef HAVE_FIDO_DEV_GET_TOUCH_STATUS
skdebug(__func__, "libfido2 version does not support a feature needed for multiple tokens. Please upgrade to >=1.5.0");
goto out;
--
2.35.1

Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
From f0a037193883a8d3dbedf8a363682c90a28435d2 Mon Sep 17 00:00:00 2001
From: Corinna Vinschen <vinschen@redhat.com>
Date: Fri, 11 Feb 2022 14:46:08 +0100
Subject: [PATCH 6/6] Defer token PIN prompt to handle "uv" as well as
"clientPin" tokens gracefully

Allow to support middlewares that handle PIN/UV gestures internally
or OOB (like Windows Hello), as well as situations where it is
preferable to have OpenSSH prompt for the PIN.

To do that, first attempt the operation (sshsk_enroll, sshsk_sign)
without passphrase. Only if the middleware responds with
SSH_ERR_KEY_WRONG_PASSPHRASE, prompt for one.

Signed-off-by: Corinna Vinschen <vinschen@redhat.com>
---
ssh-keygen.c | 11 ++---------
sshconnect2.c | 23 ++++++++++++-----------
2 files changed, 14 insertions(+), 20 deletions(-)

diff --git a/ssh-keygen.c b/ssh-keygen.c
index d4b7f4dcf800..da4027e9bc15 100644
--- a/ssh-keygen.c
+++ b/ssh-keygen.c
@@ -3241,7 +3241,7 @@ usage(void)
int
main(int argc, char **argv)
{
- char comment[1024], *passphrase;
+ char comment[1024], *passphrase = NULL;
char *rr_hostname = NULL, *ep, *fp, *ra;
struct sshkey *private, *public;
struct passwd *pw;
@@ -3751,13 +3751,6 @@ main(int argc, char **argv)
}
if ((attest = sshbuf_new()) == NULL)
fatal("sshbuf_new failed");
- if ((sk_flags &
- (SSH_SK_USER_VERIFICATION_REQD|SSH_SK_RESIDENT_KEY))) {
- passphrase = read_passphrase("Enter PIN for "
- "authenticator: ", RP_ALLOW_STDIN);
- } else {
- passphrase = NULL;
- }
for (i = 0 ; ; i++) {
fflush(stdout);
r = sshsk_enroll(type, sk_provider, sk_device,
@@ -3773,7 +3766,7 @@ main(int argc, char **argv)
freezero(passphrase, strlen(passphrase));
passphrase = NULL;
}
- if (i >= 3)
+ if (i > 3)
fatal("Too many incorrect PINs");
passphrase = read_passphrase("Enter PIN for "
"authenticator: ", RP_ALLOW_STDIN);
diff --git a/sshconnect2.c b/sshconnect2.c
index b25225e645cb..f0610011e15c 100644
--- a/sshconnect2.c
+++ b/sshconnect2.c
@@ -1262,10 +1262,20 @@ identity_sign(struct identity *id, u_char **sigp, size_t *lenp,
goto out;
}
sign_key = prv;
- if (sshkey_is_sk(sign_key)) {
+ if (sshkey_is_sk(sign_key) &&
+ (sign_key->sk_flags & SSH_SK_USER_PRESENCE_REQD)) {
+ fmprintf(stdout, "You may need to touch your "
+ "authenticator.\n");
+ }
+ }
+ retry_pin:
+ if ((r = sshkey_sign(sign_key, sigp, lenp, data, datalen,
+ alg, options.sk_provider, pin, compat)) != 0) {
+ debug_fr(r, "sshkey_sign");
+ if (pin == NULL && !retried && sshkey_is_sk(sign_key) &&
+ r == SSH_ERR_KEY_WRONG_PASSPHRASE) {
if ((sign_key->sk_flags &
SSH_SK_USER_VERIFICATION_REQD)) {
- retry_pin:
xasprintf(&prompt, "Enter PIN for %s key %s: ",
sshkey_type(sign_key), id->filename);
pin = read_passphrase(prompt, 0);
@@ -1281,15 +1291,6 @@ identity_sign(struct identity *id, u_char **sigp, size_t *lenp,
sshkey_type(sign_key), fp);
free(fp);
}
- }
- }
- if ((r = sshkey_sign(sign_key, sigp, lenp, data, datalen,
- alg, options.sk_provider, pin, compat)) != 0) {
- debug_fr(r, "sshkey_sign");
- if (pin == NULL && !retried && sshkey_is_sk(sign_key) &&
- r == SSH_ERR_KEY_WRONG_PASSPHRASE) {
- notify_complete(notifier, NULL);
- notifier = NULL;
retried = 1;
goto retry_pin;
}
--
2.35.1

28 changes: 25 additions & 3 deletions openssh/PKGBUILD
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

pkgname=openssh
pkgver=8.9p1
pkgrel=2
pkgrel=3
pkgdesc='Free version of the SSH connectivity tools'
url='https://www.openssh.com/portable.html'
license=('custom:BSD')
Expand All @@ -13,12 +13,24 @@ makedepends=('heimdal-devel' 'libedit-devel' 'libcrypt-devel' 'libfido2-devel' '
source=("https://ftp.openbsd.org/pub/OpenBSD/OpenSSH/portable/${pkgname}-${pkgver}.tar.gz"{,.asc}
openssh-7.3p1-msys2.patch
openssh-7.3p1-msys2-setkey.patch
openssh-7.3p1-msys2-drive-name-in-path.patch)
openssh-7.3p1-msys2-drive-name-in-path.patch
0001-compat-code-for-fido_dev_is_winhello.patch
0002-check_sk_options-add-temporary-WinHello-workaround.patch
0003-sk_enroll-don-t-drop-SSH_SK_USER_VERIFICATION_REQD-f.patch
0004-sk_sign-set-FIDO2-uv-attribute-explicitely-for-WinHe.patch
0005-if-WinHello-device-is-present-use-it-exclusively.patch
0006-Defer-token-PIN-prompt-to-handle-uv-as-well-as-clien.patch)
sha256sums=('fd497654b7ab1686dac672fb83dfb4ba4096e8b5ffcdaccd262380ae58bec5e7'
'SKIP'
'd03aabd023ddb655f3c7fe82df489e73d01c0311e8e3fa055e6c91f9cb0f35d0'
'25079cf4a10c1ab70d60302bccaabee513762520dffd7c35285f7aae3ea36087'
'903b3eee51e492a125cab9c724ad967450307d53e457f025e4432b81cb145af5')
'903b3eee51e492a125cab9c724ad967450307d53e457f025e4432b81cb145af5'
'61579f2a0550863fad9ae0dac1ffdee6019eb104c15a710e4acc0f335bff79ba'
'8236ef3fc6a7367f60d9f63ea537c5ea01b007756d039b77837e664db6148647'
'5671b81770125aae1298928d64fa5055a2b3114cfc8546e37685fbbcb28e23bd'
'5876afa65f2456b200429a73f49461cdebf4b60d034a4c8da8a3905de5ba94ea'
'e35425d67cb830190e4dcf46e7dc16ddb0670bef0e4cbf5f0fe8c9af121fd722'
'1e480ed27950ab7c276181007d855490b1754ee5620d8fd19472dedea754cb94')
validpgpkeys=('7168B983815A5EEF59A4ADFD2A3F414E736060BA') # Damien Miller <djm@mindrot.org>

backup=('etc/ssh/ssh_config' 'etc/ssh/sshd_config')
Expand All @@ -28,6 +40,16 @@ prepare() {
patch -p1 -i ${srcdir}/openssh-7.3p1-msys2.patch
patch -p1 -i ${srcdir}/openssh-7.3p1-msys2-setkey.patch
patch -p1 -i ${srcdir}/openssh-7.3p1-msys2-drive-name-in-path.patch

# patches from cygwin:
# https://cygwin.com/git-cygwin-packages/?p=git/cygwin-packages/openssh.git;a=tree
patch -p1 -i ${srcdir}/0001-compat-code-for-fido_dev_is_winhello.patch
patch -p1 -i ${srcdir}/0002-check_sk_options-add-temporary-WinHello-workaround.patch
patch -p1 -i ${srcdir}/0003-sk_enroll-don-t-drop-SSH_SK_USER_VERIFICATION_REQD-f.patch
patch -p1 -i ${srcdir}/0004-sk_sign-set-FIDO2-uv-attribute-explicitely-for-WinHe.patch
patch -p1 -i ${srcdir}/0005-if-WinHello-device-is-present-use-it-exclusively.patch
patch -p1 -i ${srcdir}/0006-Defer-token-PIN-prompt-to-handle-uv-as-well-as-clien.patch

autoreconf -fvi
}

Expand Down

0 comments on commit da73cbc

Please sign in to comment.