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
This commit is contained in:
djm@openbsd.org 2022-07-20 03:29:14 +00:00 committed by Damien Miller
parent 5bcfc788b3
commit 9ab929ca2d
4 changed files with 102 additions and 6 deletions

View File

@ -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 */

View File

@ -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;

View File

@ -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) {

View File

@ -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;