upstream: 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 OpenBSD-Commit-ID: e45e9f1bf2b2f32d9850669e7a8dbd64acc5fca4
openssh · Jul 20, 2022 · 9ab929c · 9ab929c
1 parent 5bcfc78
commit 9ab929c
Show file tree

Hide file tree

Showing 4 changed files with 102 additions and 6 deletions.
diff --git a/sk-api.h b/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
  *
@@ -26,6 +26,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 */
@@ -37,6 +38,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;
@@ -77,7 +79,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/sk-usbhid.c b/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
@@ -398,7 +398,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;
 	}
@@ -764,6 +764,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,
@@ -817,6 +871,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/ssh-keygen.c b/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
@@ -3216,6 +3216,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)
 {
@@ -3803,6 +3821,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/ssh-sk.c b/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
  *
@@ -354,6 +354,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;