upstream: translate and return error codes; retry on bad PIN

Define some well-known error codes in the SK API and pass
them back via ssh-sk-helper.

Use the new "wrong PIN" error code to retry PIN prompting during
ssh-keygen of resident keys.

feedback and ok markus@

OpenBSD-Commit-ID: 9663c6a2bb7a0bc8deaccc6c30d9a2983b481620
This commit is contained in:
djm@openbsd.org 2019-12-30 09:24:45 +00:00 committed by Damien Miller
parent d433596736
commit 43ce96427b
6 changed files with 81 additions and 33 deletions

View File

@ -1,4 +1,4 @@
/* $OpenBSD: sk-api.h,v 1.5 2019/12/30 09:23:28 djm Exp $ */ /* $OpenBSD: sk-api.h,v 1.6 2019/12/30 09:24:45 djm Exp $ */
/* /*
* Copyright (c) 2019 Google LLC * Copyright (c) 2019 Google LLC
* *
@ -32,6 +32,11 @@
#define SSH_SK_ECDSA 0x00 #define SSH_SK_ECDSA 0x00
#define SSH_SK_ED25519 0x01 #define SSH_SK_ED25519 0x01
/* Error codes */
#define SSH_SK_ERR_GENERAL -1
#define SSH_SK_ERR_UNSUPPORTED -2
#define SSH_SK_ERR_PIN_REQUIRED -3
struct sk_enroll_response { struct sk_enroll_response {
uint8_t *public_key; uint8_t *public_key;
size_t public_key_len; size_t public_key_len;

View File

@ -65,6 +65,11 @@
#define SK_ECDSA 0x00 #define SK_ECDSA 0x00
#define SK_ED25519 0x01 #define SK_ED25519 0x01
/* Error codes */
#define SSH_SK_ERR_GENERAL -1
#define SSH_SK_ERR_UNSUPPORTED -2
#define SSH_SK_ERR_PIN_REQUIRED -3
struct sk_enroll_response { struct sk_enroll_response {
uint8_t *public_key; uint8_t *public_key;
size_t public_key_len; size_t public_key_len;
@ -412,6 +417,20 @@ pack_public_key(int alg, const fido_cred_t *cred,
} }
} }
static int
fidoerr_to_skerr(int fidoerr)
{
switch (fidoerr) {
case FIDO_ERR_UNSUPPORTED_OPTION:
return SSH_SK_ERR_UNSUPPORTED;
case FIDO_ERR_PIN_REQUIRED:
case FIDO_ERR_PIN_INVALID:
return SSH_SK_ERR_PIN_REQUIRED;
default:
return -1;
}
}
int int
sk_enroll(int alg, const uint8_t *challenge, size_t challenge_len, sk_enroll(int alg, const uint8_t *challenge, size_t challenge_len,
const char *application, uint8_t flags, const char *pin, const char *application, uint8_t flags, const char *pin,
@ -424,7 +443,7 @@ sk_enroll(int alg, const uint8_t *challenge, size_t challenge_len,
struct sk_enroll_response *response = NULL; struct sk_enroll_response *response = NULL;
size_t len; size_t len;
int cose_alg; int cose_alg;
int ret = -1; int ret = SSH_SK_ERR_GENERAL;
int r; int r;
char *device = NULL; char *device = NULL;
@ -491,8 +510,9 @@ sk_enroll(int alg, const uint8_t *challenge, size_t challenge_len,
skdebug(__func__, "fido_dev_open: %s", fido_strerr(r)); skdebug(__func__, "fido_dev_open: %s", fido_strerr(r));
goto out; goto out;
} }
if ((r = fido_dev_make_cred(dev, cred, NULL)) != FIDO_OK) { if ((r = fido_dev_make_cred(dev, cred, pin)) != FIDO_OK) {
skdebug(__func__, "fido_dev_make_cred: %s", fido_strerr(r)); skdebug(__func__, "fido_dev_make_cred: %s", fido_strerr(r));
ret = fidoerr_to_skerr(r);
goto out; goto out;
} }
if (fido_cred_x5c_ptr(cred) != NULL) { if (fido_cred_x5c_ptr(cred) != NULL) {
@ -657,7 +677,7 @@ sk_sign(int alg, const uint8_t *message, size_t message_len,
fido_assert_t *assert = NULL; fido_assert_t *assert = NULL;
fido_dev_t *dev = NULL; fido_dev_t *dev = NULL;
struct sk_sign_response *response = NULL; struct sk_sign_response *response = NULL;
int ret = -1; int ret = SSH_SK_ERR_GENERAL;
int r; int r;
#ifdef SK_DEBUG #ifdef SK_DEBUG
@ -736,7 +756,7 @@ static int
read_rks(const char *devpath, const char *pin, read_rks(const char *devpath, const char *pin,
struct sk_resident_key ***rksp, size_t *nrksp) struct sk_resident_key ***rksp, size_t *nrksp)
{ {
int r = -1; int ret = SSH_SK_ERR_GENERAL, r = -1;
fido_dev_t *dev = NULL; fido_dev_t *dev = NULL;
fido_credman_metadata_t *metadata = NULL; fido_credman_metadata_t *metadata = NULL;
fido_credman_rp_t *rp = NULL; fido_credman_rp_t *rp = NULL;
@ -747,16 +767,15 @@ read_rks(const char *devpath, const char *pin,
if ((dev = fido_dev_new()) == NULL) { if ((dev = fido_dev_new()) == NULL) {
skdebug(__func__, "fido_dev_new failed"); skdebug(__func__, "fido_dev_new failed");
return -1; return ret;
} }
if ((r = fido_dev_open(dev, devpath)) != FIDO_OK) { if ((r = fido_dev_open(dev, devpath)) != FIDO_OK) {
skdebug(__func__, "fido_dev_open %s failed: %s", skdebug(__func__, "fido_dev_open %s failed: %s",
devpath, fido_strerr(r)); devpath, fido_strerr(r));
fido_dev_free(&dev); fido_dev_free(&dev);
return -1; return ret;
} }
if ((metadata = fido_credman_metadata_new()) == NULL) { if ((metadata = fido_credman_metadata_new()) == NULL) {
r = -1;
skdebug(__func__, "alloc failed"); skdebug(__func__, "alloc failed");
goto out; goto out;
} }
@ -765,7 +784,7 @@ read_rks(const char *devpath, const char *pin,
if (r == FIDO_ERR_INVALID_COMMAND) { if (r == FIDO_ERR_INVALID_COMMAND) {
skdebug(__func__, "device %s does not support " skdebug(__func__, "device %s does not support "
"resident keys", devpath); "resident keys", devpath);
r = 0; ret = 0;
goto out; goto out;
} }
skdebug(__func__, "get metadata for %s failed: %s", skdebug(__func__, "get metadata for %s failed: %s",
@ -776,7 +795,6 @@ read_rks(const char *devpath, const char *pin,
(unsigned long long)fido_credman_rk_existing(metadata), (unsigned long long)fido_credman_rk_existing(metadata),
(unsigned long long)fido_credman_rk_remaining(metadata)); (unsigned long long)fido_credman_rk_remaining(metadata));
if ((rp = fido_credman_rp_new()) == NULL) { if ((rp = fido_credman_rp_new()) == NULL) {
r = -1;
skdebug(__func__, "alloc rp failed"); skdebug(__func__, "alloc rp failed");
goto out; goto out;
} }
@ -801,7 +819,6 @@ read_rks(const char *devpath, const char *pin,
fido_credman_rk_free(&rk); fido_credman_rk_free(&rk);
if ((rk = fido_credman_rk_new()) == NULL) { if ((rk = fido_credman_rk_new()) == NULL) {
r = -1;
skdebug(__func__, "alloc rk failed"); skdebug(__func__, "alloc rk failed");
goto out; goto out;
} }
@ -831,7 +848,6 @@ read_rks(const char *devpath, const char *pin,
fido_cred_id_len(cred))) == NULL || fido_cred_id_len(cred))) == NULL ||
(srk->application = strdup(fido_credman_rp_id(rp, (srk->application = strdup(fido_credman_rp_id(rp,
i))) == NULL) { i))) == NULL) {
r = -1;
skdebug(__func__, "alloc sk_resident_key"); skdebug(__func__, "alloc sk_resident_key");
goto out; goto out;
} }
@ -862,7 +878,6 @@ read_rks(const char *devpath, const char *pin,
/* append */ /* append */
if ((tmp = recallocarray(*rksp, *nrksp, (*nrksp) + 1, if ((tmp = recallocarray(*rksp, *nrksp, (*nrksp) + 1,
sizeof(**rksp))) == NULL) { sizeof(**rksp))) == NULL) {
r = -1;
skdebug(__func__, "alloc rksp"); skdebug(__func__, "alloc rksp");
goto out; goto out;
} }
@ -872,7 +887,7 @@ read_rks(const char *devpath, const char *pin,
} }
} }
/* Success */ /* Success */
r = 0; ret = 0;
out: out:
if (srk != NULL) { if (srk != NULL) {
free(srk->application); free(srk->application);
@ -885,14 +900,14 @@ read_rks(const char *devpath, const char *pin,
fido_dev_close(dev); fido_dev_close(dev);
fido_dev_free(&dev); fido_dev_free(&dev);
fido_credman_metadata_free(&metadata); fido_credman_metadata_free(&metadata);
return r; return ret;
} }
int int
sk_load_resident_keys(const char *pin, sk_load_resident_keys(const char *pin,
struct sk_resident_key ***rksp, size_t *nrksp) struct sk_resident_key ***rksp, size_t *nrksp)
{ {
int r = -1; int ret = SSH_SK_ERR_GENERAL, r = -1;
fido_dev_info_t *devlist = NULL; fido_dev_info_t *devlist = NULL;
size_t i, ndev = 0, nrks = 0; size_t i, ndev = 0, nrks = 0;
const fido_dev_info_t *di; const fido_dev_info_t *di;
@ -924,7 +939,7 @@ sk_load_resident_keys(const char *pin,
} }
} }
/* success */ /* success */
r = 0; ret = 0;
*rksp = rks; *rksp = rks;
*nrksp = nrks; *nrksp = nrks;
rks = NULL; rks = NULL;
@ -938,7 +953,7 @@ sk_load_resident_keys(const char *pin,
} }
free(rks); free(rks);
fido_dev_info_free(&devlist, MAX_FIDO_DEVICES); fido_dev_info_free(&devlist, MAX_FIDO_DEVICES);
return r; return ret;
} }
#endif /* ENABLE_SK_INTERNAL */ #endif /* ENABLE_SK_INTERNAL */

View File

@ -1,4 +1,4 @@
/* $OpenBSD: ssh-keygen.c,v 1.378 2019/12/30 09:23:28 djm Exp $ */ /* $OpenBSD: ssh-keygen.c,v 1.379 2019/12/30 09:24:45 djm Exp $ */
/* /*
* Author: Tatu Ylonen <ylo@cs.hut.fi> * Author: Tatu Ylonen <ylo@cs.hut.fi>
* Copyright (c) 1994 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland * Copyright (c) 1994 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@ -3361,16 +3361,26 @@ main(int argc, char **argv)
switch (type) { switch (type) {
case KEY_ECDSA_SK: case KEY_ECDSA_SK:
case KEY_ED25519_SK: case KEY_ED25519_SK:
if (!quiet) { passphrase1 = NULL;
printf("You may need to touch your security key " for (i = 0 ; i < 3; i++) {
"to authorize key generation.\n"); if (!quiet) {
printf("You may need to touch your security "
"key to authorize key generation.\n");
}
fflush(stdout);
r = sshsk_enroll(type, sk_provider,
cert_key_id == NULL ? "ssh:" : cert_key_id,
sk_flags, passphrase1, NULL, &private, NULL);
if (r == 0)
break;
if (r != SSH_ERR_KEY_WRONG_PASSPHRASE)
exit(1); /* error message already printed */
passphrase1 = read_passphrase("Enter PIN for security "
"key: ", RP_ALLOW_STDIN);
} }
fflush(stdout); if (i > 3)
if (sshsk_enroll(type, sk_provider, fatal("Too many incorrect PINs");
cert_key_id == NULL ? "ssh:" : cert_key_id, break;
sk_flags, NULL, NULL, &private, NULL) != 0)
exit(1); /* error message already printed */
break;
default: default:
if ((r = sshkey_generate(type, bits, &private)) != 0) if ((r = sshkey_generate(type, bits, &private)) != 0)
fatal("sshkey_generate failed"); fatal("sshkey_generate failed");

View File

@ -1,4 +1,4 @@
/* $OpenBSD: ssh-sk.c,v 1.22 2019/12/30 09:24:03 djm Exp $ */ /* $OpenBSD: ssh-sk.c,v 1.23 2019/12/30 09:24:45 djm Exp $ */
/* /*
* Copyright (c) 2019 Google LLC * Copyright (c) 2019 Google LLC
* *
@ -325,6 +325,20 @@ sshsk_key_from_response(int alg, const char *application, uint8_t flags,
return r; return r;
} }
static int
skerr_to_ssherr(int skerr)
{
switch (skerr) {
case SSH_SK_ERR_UNSUPPORTED:
return SSH_ERR_FEATURE_UNSUPPORTED;
case SSH_SK_ERR_PIN_REQUIRED:
return SSH_ERR_KEY_WRONG_PASSPHRASE;
case SSH_SK_ERR_GENERAL:
default:
return SSH_ERR_INVALID_FORMAT;
}
}
int int
sshsk_enroll(int type, const char *provider_path, const char *application, sshsk_enroll(int type, const char *provider_path, const char *application,
uint8_t flags, const char *pin, struct sshbuf *challenge_buf, uint8_t flags, const char *pin, struct sshbuf *challenge_buf,
@ -396,7 +410,7 @@ sshsk_enroll(int type, const char *provider_path, const char *application,
flags, pin, &resp)) != 0) { flags, pin, &resp)) != 0) {
error("Security key provider \"%s\" returned failure %d", error("Security key provider \"%s\" returned failure %d",
provider_path, r); provider_path, r);
r = SSH_ERR_INVALID_FORMAT; /* XXX error codes in API? */ r = skerr_to_ssherr(r);
goto out; goto out;
} }
@ -559,6 +573,7 @@ sshsk_sign(const char *provider_path, struct sshkey *key,
sshbuf_ptr(key->sk_key_handle), sshbuf_len(key->sk_key_handle), sshbuf_ptr(key->sk_key_handle), sshbuf_len(key->sk_key_handle),
key->sk_flags, pin, &resp)) != 0) { key->sk_flags, pin, &resp)) != 0) {
debug("%s: sk_sign failed with code %d", __func__, r); debug("%s: sk_sign failed with code %d", __func__, r);
r = skerr_to_ssherr(r);
goto out; goto out;
} }
/* Assemble signature */ /* Assemble signature */
@ -655,7 +670,7 @@ sshsk_load_resident(const char *provider_path, const char *pin,
if ((r = skp->sk_load_resident_keys(pin, &rks, &nrks)) != 0) { if ((r = skp->sk_load_resident_keys(pin, &rks, &nrks)) != 0) {
error("Security key provider \"%s\" returned failure %d", error("Security key provider \"%s\" returned failure %d",
provider_path, r); provider_path, r);
r = SSH_ERR_INVALID_FORMAT; /* XXX error codes in API? */ r = skerr_to_ssherr(r);
goto out; goto out;
} }
for (i = 0; i < nrks; i++) { for (i = 0; i < nrks; i++) {

View File

@ -1,4 +1,4 @@
/* $OpenBSD: ssherr.c,v 1.8 2018/07/03 11:39:54 djm Exp $ */ /* $OpenBSD: ssherr.c,v 1.9 2019/12/30 09:24:45 djm Exp $ */
/* /*
* Copyright (c) 2011 Damien Miller * Copyright (c) 2011 Damien Miller
* *
@ -141,6 +141,8 @@ ssh_err(int n)
return "number is too large"; return "number is too large";
case SSH_ERR_SIGN_ALG_UNSUPPORTED: case SSH_ERR_SIGN_ALG_UNSUPPORTED:
return "signature algorithm not supported"; return "signature algorithm not supported";
case SSH_ERR_FEATURE_UNSUPPORTED:
return "requested feature not supported";
default: default:
return "unknown error"; return "unknown error";
} }

View File

@ -1,4 +1,4 @@
/* $OpenBSD: ssherr.h,v 1.6 2018/07/03 11:39:54 djm Exp $ */ /* $OpenBSD: ssherr.h,v 1.7 2019/12/30 09:24:45 djm Exp $ */
/* /*
* Copyright (c) 2011 Damien Miller * Copyright (c) 2011 Damien Miller
* *
@ -80,6 +80,7 @@
#define SSH_ERR_KEY_LENGTH -56 #define SSH_ERR_KEY_LENGTH -56
#define SSH_ERR_NUMBER_TOO_LARGE -57 #define SSH_ERR_NUMBER_TOO_LARGE -57
#define SSH_ERR_SIGN_ALG_UNSUPPORTED -58 #define SSH_ERR_SIGN_ALG_UNSUPPORTED -58
#define SSH_ERR_FEATURE_UNSUPPORTED -59
/* Translate a numeric error code to a human-readable error string */ /* Translate a numeric error code to a human-readable error string */
const char *ssh_err(int n); const char *ssh_err(int n);