when enrolling a resident key on a security token, check if a

credential with matching application and user ID strings already
exists. if so, prompt the user for confirmation before overwriting
the credential.

patch from Pedro Martelletto via GHPR329

NB. cranks SSH_SK_VERSION_MAJOR, so any third-party FIDO middleware
implementations will need to adjust

diff --git a/usr.bin/ssh/sk-api.h b/usr.bin/ssh/sk-api.h
index cf71b60..32d0f11 100644 (file)
--- a/usr.bin/ssh/sk-api.h
+++ b/usr.bin/ssh/sk-api.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: sk-api.h,v 1.14 2021/11/02 22:56:40 djm Exp $ */
+/* $OpenBSD: sk-api.h,v 1.15 2022/07/20 03:29:14 djm Exp $ */
 /*
  * Copyright (c) 2019 Google LLC
  *
@@ -24,6 +24,7 @@
 /* Flags */
 #define SSH_SK_USER_PRESENCE_REQD      0x01
 #define SSH_SK_USER_VERIFICATION_REQD  0x04
+#define SSH_SK_FORCE_OPERATION         0x10
 #define SSH_SK_RESIDENT_KEY            0x20
 
 /* Algs */
@@ -35,6 +36,7 @@
 #define SSH_SK_ERR_UNSUPPORTED         -2
 #define SSH_SK_ERR_PIN_REQUIRED                -3
 #define SSH_SK_ERR_DEVICE_NOT_FOUND    -4
+#define SSH_SK_ERR_CREDENTIAL_EXISTS   -5
 
 struct sk_enroll_response {
        uint8_t flags;
@@ -75,7 +77,7 @@ struct sk_option {
        uint8_t required;
 };
 
-#define SSH_SK_VERSION_MAJOR           0x00090000 /* current API version */
+#define SSH_SK_VERSION_MAJOR           0x000a0000 /* current API version */
 #define SSH_SK_VERSION_MAJOR_MASK      0xffff0000
 
 /* Return the version of the middleware API */

diff --git a/usr.bin/ssh/sk-usbhid.c b/usr.bin/ssh/sk-usbhid.c
index a8186ca..cbf7d8b 100644 (file)
--- a/usr.bin/ssh/sk-usbhid.c
+++ b/usr.bin/ssh/sk-usbhid.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: sk-usbhid.c,v 1.39 2022/04/29 03:16:48 dtucker Exp $ */
+/* $OpenBSD: sk-usbhid.c,v 1.40 2022/07/20 03:29:14 djm Exp $ */
 /*
  * Copyright (c) 2019 Markus Friedl
  * Copyright (c) 2020 Pedro Martelletto
@@ -290,7 +290,7 @@ sk_try(const struct sk_usbhid *sk, const char *application,
        /* generate an invalid signature on FIDO2 tokens */
        if ((r = fido_assert_set_clientdata(assert, message,
            sizeof(message))) != FIDO_OK) {
-               skdebug(__func__, "fido_assert_set_clientdata_hash: %s",
+               skdebug(__func__, "fido_assert_set_clientdata: %s",
                    fido_strerr(r));
                goto out;
        }
@@ -651,6 +651,60 @@ check_enroll_options(struct sk_option **options, char **devicep,
        return 0;
 }
 
+static int
+key_lookup(fido_dev_t *dev, const char *application, const uint8_t *user_id,
+    size_t user_id_len, const char *pin)
+{
+       fido_assert_t *assert = NULL;
+       uint8_t message[32];
+       int r = FIDO_ERR_INTERNAL;
+       size_t i;
+
+       memset(message, '\0', sizeof(message));
+       if (pin == NULL) {
+               skdebug(__func__, "NULL pin");
+               goto out;
+       }
+       if ((assert = fido_assert_new()) == NULL) {
+               skdebug(__func__, "fido_assert_new failed");
+               goto out;
+       }
+       /* generate an invalid signature on FIDO2 tokens */
+       if ((r = fido_assert_set_clientdata(assert, message,
+           sizeof(message))) != FIDO_OK) {
+               skdebug(__func__, "fido_assert_set_clientdata: %s",
+                   fido_strerr(r));
+               goto out;
+       }
+       if ((r = fido_assert_set_rp(assert, application)) != FIDO_OK) {
+               skdebug(__func__, "fido_assert_set_rp: %s", fido_strerr(r));
+               goto out;
+       }
+       if ((r = fido_assert_set_up(assert, FIDO_OPT_FALSE)) != FIDO_OK) {
+               skdebug(__func__, "fido_assert_up: %s", fido_strerr(r));
+               goto out;
+       }
+       if ((r = fido_dev_get_assert(dev, assert, pin)) != FIDO_OK) {
+               skdebug(__func__, "fido_dev_get_assert: %s", fido_strerr(r));
+               goto out;
+       }
+       r = FIDO_ERR_NO_CREDENTIALS;
+       skdebug(__func__, "%zu signatures returned", fido_assert_count(assert));
+       for (i = 0; i < fido_assert_count(assert); i++) {
+               if (fido_assert_user_id_len(assert, i) == user_id_len &&
+                   memcmp(fido_assert_user_id_ptr(assert, i), user_id,
+                   user_id_len) == 0) {
+                       skdebug(__func__, "credential exists");
+                       r = FIDO_OK;
+                       goto out;
+               }
+       }
+ out:
+       fido_assert_free(&assert);
+
+       return r;
+}
+
 int
 sk_enroll(uint32_t alg, const uint8_t *challenge, size_t challenge_len,
     const char *application, uint8_t flags, const char *pin,
@@ -704,6 +758,19 @@ sk_enroll(uint32_t alg, const uint8_t *challenge, size_t challenge_len,
                goto out;
        }
        skdebug(__func__, "using device %s", sk->path);
+       if ((flags & SSH_SK_RESIDENT_KEY) != 0 &&
+           (flags & SSH_SK_FORCE_OPERATION) == 0 &&
+           (r = key_lookup(sk->dev, application, user_id, sizeof(user_id),
+           pin)) != FIDO_ERR_NO_CREDENTIALS) {
+               if (r != FIDO_OK) {
+                       ret = SSH_SK_ERR_GENERAL;
+                       skdebug(__func__, "key_lookup failed");
+               } else {
+                       ret = SSH_SK_ERR_CREDENTIAL_EXISTS;
+                       skdebug(__func__, "key exists");
+               }
+               goto out;
+       }
        if ((cred = fido_cred_new()) == NULL) {
                skdebug(__func__, "fido_cred_new failed");
                goto out;

diff --git a/usr.bin/ssh/ssh-keygen.c b/usr.bin/ssh/ssh-keygen.c
index 02ef18a..d518c78 100644 (file)
--- a/usr.bin/ssh/ssh-keygen.c
+++ b/usr.bin/ssh/ssh-keygen.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-keygen.c,v 1.455 2022/07/20 03:13:04 djm Exp $ */
+/* $OpenBSD: ssh-keygen.c,v 1.456 2022/07/20 03:29:14 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1994 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -3193,6 +3193,24 @@ save_attestation(struct sshbuf *attest, const char *path)
                    "%s\n", path);
 }
 
+static int
+confirm_sk_overwrite(const char *application, const char *user)
+{
+       char yesno[3];
+
+       printf("A resident key scoped to '%s' with user id '%s' already "
+           "exists.\n", application == NULL ? "ssh:" : application,
+           user == NULL ? "null" : user);
+       printf("Overwrite key in token (y/n)? ");
+       fflush(stdout);
+       if (fgets(yesno, sizeof(yesno), stdin) == NULL)
+               return 0;
+       if (yesno[0] != 'y' && yesno[0] != 'Y')
+               return 0;
+       printf("Touch your authenticator to authorize key generation.\n");
+       return 1;
+}
+
 static void
 usage(void)
 {
@@ -3777,6 +3795,13 @@ main(int argc, char **argv)
                            &private, attest);
                        if (r == 0)
                                break;
+                       if (r == SSH_ERR_KEY_BAD_PERMISSIONS &&
+                           (sk_flags & SSH_SK_RESIDENT_KEY) != 0 &&
+                           (sk_flags & SSH_SK_FORCE_OPERATION) == 0 &&
+                           confirm_sk_overwrite(sk_application, sk_user)) {
+                               sk_flags |= SSH_SK_FORCE_OPERATION;
+                               continue;
+                       }
                        if (r != SSH_ERR_KEY_WRONG_PASSPHRASE)
                                fatal_r(r, "Key enrollment failed");
                        else if (passphrase != NULL) {

diff --git a/usr.bin/ssh/ssh-sk.c b/usr.bin/ssh/ssh-sk.c
index ad5e6e9..e5e72f2 100644 (file)
--- a/usr.bin/ssh/ssh-sk.c
+++ b/usr.bin/ssh/ssh-sk.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-sk.c,v 1.38 2022/01/14 03:35:10 djm Exp $ */
+/* $OpenBSD: ssh-sk.c,v 1.39 2022/07/20 03:29:14 djm Exp $ */
 /*
  * Copyright (c) 2019 Google LLC
  *
@@ -334,6 +334,8 @@ skerr_to_ssherr(int skerr)
                return SSH_ERR_KEY_WRONG_PASSPHRASE;
        case SSH_SK_ERR_DEVICE_NOT_FOUND:
                return SSH_ERR_DEVICE_NOT_FOUND;
+       case SSH_SK_ERR_CREDENTIAL_EXISTS:
+               return SSH_ERR_KEY_BAD_PERMISSIONS;
        case SSH_SK_ERR_GENERAL:
        default:
                return SSH_ERR_INVALID_FORMAT;