U2F support in OpenSSH HEAD

[Damien Miller]

On Thu, 14 Nov 2019, Michael Forney wrote:

> On 2019-11-14, Damien Miller <djm at mindrot.org> wrote:
> > Please give this a try - security key support is a substantial change and
> > it really needs testing ahead of the next release.
> 
> Hi Damien,
> 
> Thanks for working on security key support, this is a really nice
> feature to have in openssh.
> 
> My non-FIDO2 security key (YubiKey NEO) doesn't work with the latest
> changes to openssh and libfido2, failing with `try_device:
> fido_dev_get_assert: FIDO_ERR_USER_PRESENCE_REQUIRED`. I'm not sure if
> this is a problem in libfido2 or sk-usbhid.c (I also reported this
> issue at https://github.com/Yubico/libfido2/issues/73).
> 
> Is try_device incompatible with U2F keys? It seems to me to be trying
> to detect the presence of a key handle using an assert with up=0, but
> that causes the U2F codepath in libfido2 to return an error
> FIDO_ERR_USER_PRESENCE_REQUIRED.
> 
> I believe that since try_device is only trying to find the device with
> the key, FIDO_ERR_USER_PRESENCE_REQUIRED should be ignored here, since
> that seems to indicate that the key lookup succeeded, but
> authentication was not attempted. I attached a diff that makes this
> change and it seems to fix my issue.

Thanks for testing this!

Does this patch help? If you're able to test multiple U2F-only keys in
a host then that would be ideal - you'll be able to see whether ssh is
trying each device if you run it in verbose mode (i.e. ssh -vvv ...)

Basically, I want to make sure that FIDO_ERR_USER_PRESENCE_REQUIRED is
returned only when a token actually claims a key handle, and not all the
time...

diff --git a/sk-usbhid.c b/sk-usbhid.c
index 63c7cb2..8758e2d 100644
--- a/sk-usbhid.c
+++ b/sk-usbhid.c
@@ -197,6 +197,10 @@ try_device(fido_dev_t *dev, const uint8_t *message, size_t message_len,
 	}
 	r = fido_dev_get_assert(dev, assert, NULL);
 	skdebug(__func__, "fido_dev_get_assert: %s", fido_strerr(r));
+	if (r == FIDO_ERR_USER_PRESENCE_REQUIRED) {
+		/* U2F tokens may return this */
+		r = FIDO_OK;
+	}
  out:
 	fido_assert_free(&assert);