From 43ce96427b76c4918e39af654e2fc9ee18d5d478 Mon Sep 17 00:00:00 2001 From: "djm@openbsd.org" Date: Mon, 30 Dec 2019 09:24:45 +0000 Subject: 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 --- ssherr.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'ssherr.h') diff --git a/ssherr.h b/ssherr.h index 348da5a20..520bd4964 100644 --- a/ssherr.h +++ b/ssherr.h @@ -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 * @@ -80,6 +80,7 @@ #define SSH_ERR_KEY_LENGTH -56 #define SSH_ERR_NUMBER_TOO_LARGE -57 #define SSH_ERR_SIGN_ALG_UNSUPPORTED -58 +#define SSH_ERR_FEATURE_UNSUPPORTED -59 /* Translate a numeric error code to a human-readable error string */ const char *ssh_err(int n); -- cgit v1.2.3 From 59d01f1d720ebede4da42882f592d1093dac7adc Mon Sep 17 00:00:00 2001 From: "djm@openbsd.org" Date: Sat, 25 Jan 2020 23:13:09 +0000 Subject: upstream: improve the error message for u2f enrollment errors by making ssh-keygen be solely responsible for printing the error message and convertint some more common error responses from the middleware to a useful ssherr.h status code. more detail remains visible via -v of course. also remove indepedent copy of sk-api.h declarations in sk-usbhid.c and just include it. feedback & ok markus@ OpenBSD-Commit-ID: a4a8ffa870d9a3e0cfd76544bcdeef5c9fb1f1bb --- PROTOCOL.u2f | 1 + sk-api.h | 3 +- sk-usbhid.c | 99 ++++++++++++++++----------------------------------------- ssh-keygen.c | 4 +-- ssh-sk-helper.c | 4 +-- ssh-sk.c | 6 ++-- ssherr.c | 4 ++- ssherr.h | 3 +- 8 files changed, 43 insertions(+), 81 deletions(-) (limited to 'ssherr.h') diff --git a/PROTOCOL.u2f b/PROTOCOL.u2f index fd0cd0de0..58f75ba28 100644 --- a/PROTOCOL.u2f +++ b/PROTOCOL.u2f @@ -249,6 +249,7 @@ The middleware library need only expose a handful of functions: #define SSH_SK_ERR_GENERAL -1 #define SSH_SK_ERR_UNSUPPORTED -2 #define SSH_SK_ERR_PIN_REQUIRED -3 + #define SSH_SK_ERR_DEVICE_NOT_FOUND -4 struct sk_enroll_response { uint8_t *public_key; diff --git a/sk-api.h b/sk-api.h index 93d6a1229..170fd4470 100644 --- a/sk-api.h +++ b/sk-api.h @@ -1,4 +1,4 @@ -/* $OpenBSD: sk-api.h,v 1.7 2020/01/06 02:00:46 djm Exp $ */ +/* $OpenBSD: sk-api.h,v 1.8 2020/01/25 23:13:09 djm Exp $ */ /* * Copyright (c) 2019 Google LLC * @@ -36,6 +36,7 @@ #define SSH_SK_ERR_GENERAL -1 #define SSH_SK_ERR_UNSUPPORTED -2 #define SSH_SK_ERR_PIN_REQUIRED -3 +#define SSH_SK_ERR_DEVICE_NOT_FOUND -4 struct sk_enroll_response { uint8_t *public_key; diff --git a/sk-usbhid.c b/sk-usbhid.c index cf783e205..2148e1d79 100644 --- a/sk-usbhid.c +++ b/sk-usbhid.c @@ -37,9 +37,19 @@ #include #ifndef SK_STANDALONE -#include "log.h" -#include "xmalloc.h" -#endif +# include "log.h" +# include "xmalloc.h" +/* + * If building as part of OpenSSH, then rename exported functions. + * This must be done before including sk-api.h. + */ +# define sk_api_version ssh_sk_api_version +# define sk_enroll ssh_sk_enroll +# define sk_sign ssh_sk_sign +# define sk_load_resident_keys ssh_sk_load_resident_keys +#endif /* !SK_STANDALONE */ + +#include "sk-api.h" /* #define SK_DEBUG 1 */ @@ -54,63 +64,6 @@ } while (0) #endif -#define SK_VERSION_MAJOR 0x00040000 /* current API version */ - -/* Flags */ -#define SK_USER_PRESENCE_REQD 0x01 -#define SK_USER_VERIFICATION_REQD 0x04 -#define SK_RESIDENT_KEY 0x20 - -/* Algs */ -#define SK_ECDSA 0x00 -#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 { - uint8_t *public_key; - size_t public_key_len; - uint8_t *key_handle; - size_t key_handle_len; - uint8_t *signature; - size_t signature_len; - uint8_t *attestation_cert; - size_t attestation_cert_len; -}; - -struct sk_sign_response { - uint8_t flags; - uint32_t counter; - uint8_t *sig_r; - size_t sig_r_len; - uint8_t *sig_s; - size_t sig_s_len; -}; - -struct sk_resident_key { - uint32_t alg; - size_t slot; - char *application; - struct sk_enroll_response key; -}; - -struct sk_option { - char *name; - char *value; - uint8_t required; -}; - -/* If building as part of OpenSSH, then rename exported functions */ -#if !defined(SK_STANDALONE) -#define sk_api_version ssh_sk_api_version -#define sk_enroll ssh_sk_enroll -#define sk_sign ssh_sk_sign -#define sk_load_resident_keys ssh_sk_load_resident_keys -#endif - /* Return the version of the middleware API */ uint32_t sk_api_version(void); @@ -161,7 +114,7 @@ skdebug(const char *func, const char *fmt, ...) uint32_t sk_api_version(void) { - return SK_VERSION_MAJOR; + return SSH_SK_VERSION_MAJOR; } /* Select the first identified FIDO device attached to the system */ @@ -426,10 +379,10 @@ pack_public_key(uint32_t alg, const fido_cred_t *cred, { switch(alg) { #ifdef WITH_OPENSSL - case SK_ECDSA: + case SSH_SK_ECDSA: return pack_public_key_ecdsa(cred, response); #endif /* WITH_OPENSSL */ - case SK_ED25519: + case SSH_SK_ED25519: return pack_public_key_ed25519(cred, response); default: return -1; @@ -441,6 +394,7 @@ fidoerr_to_skerr(int fidoerr) { switch (fidoerr) { case FIDO_ERR_UNSUPPORTED_OPTION: + case FIDO_ERR_UNSUPPORTED_ALGORITHM: return SSH_SK_ERR_UNSUPPORTED; case FIDO_ERR_PIN_REQUIRED: case FIDO_ERR_PIN_INVALID: @@ -516,11 +470,11 @@ sk_enroll(uint32_t alg, const uint8_t *challenge, size_t challenge_len, *enroll_response = NULL; switch(alg) { #ifdef WITH_OPENSSL - case SK_ECDSA: + case SSH_SK_ECDSA: cose_alg = COSE_ES256; break; #endif /* WITH_OPENSSL */ - case SK_ED25519: + case SSH_SK_ED25519: cose_alg = COSE_EDDSA; break; default: @@ -528,6 +482,7 @@ sk_enroll(uint32_t alg, const uint8_t *challenge, size_t challenge_len, goto out; } if (device == NULL && (device = pick_first_device()) == NULL) { + ret = SSH_SK_ERR_DEVICE_NOT_FOUND; skdebug(__func__, "pick_first_device failed"); goto out; } @@ -546,7 +501,7 @@ sk_enroll(uint32_t alg, const uint8_t *challenge, size_t challenge_len, fido_strerr(r)); goto out; } - if ((r = fido_cred_set_rk(cred, (flags & SK_RESIDENT_KEY) != 0 ? + if ((r = fido_cred_set_rk(cred, (flags & SSH_SK_RESIDENT_KEY) != 0 ? FIDO_OPT_TRUE : FIDO_OPT_OMIT)) != FIDO_OK) { skdebug(__func__, "fido_cred_set_rk: %s", fido_strerr(r)); goto out; @@ -717,10 +672,10 @@ pack_sig(uint32_t alg, fido_assert_t *assert, { switch(alg) { #ifdef WITH_OPENSSL - case SK_ECDSA: + case SSH_SK_ECDSA: return pack_sig_ecdsa(assert, response); #endif /* WITH_OPENSSL */ - case SK_ED25519: + case SSH_SK_ED25519: return pack_sig_ed25519(assert, response); default: return -1; @@ -804,7 +759,7 @@ sk_sign(uint32_t alg, const uint8_t *message, size_t message_len, goto out; } if ((r = fido_assert_set_up(assert, - (flags & SK_USER_PRESENCE_REQD) ? + (flags & SSH_SK_USER_PRESENCE_REQD) ? FIDO_OPT_TRUE : FIDO_OPT_FALSE)) != FIDO_OK) { skdebug(__func__, "fido_assert_set_up: %s", fido_strerr(r)); goto out; @@ -951,15 +906,15 @@ read_rks(const char *devpath, const char *pin, switch (fido_cred_type(cred)) { case COSE_ES256: - srk->alg = SK_ECDSA; + srk->alg = SSH_SK_ECDSA; break; case COSE_EDDSA: - srk->alg = SK_ED25519; + srk->alg = SSH_SK_ED25519; break; default: skdebug(__func__, "unsupported key type %d", fido_cred_type(cred)); - goto out; + goto out; /* XXX free rk and continue */ } if ((r = pack_public_key(srk->alg, cred, diff --git a/ssh-keygen.c b/ssh-keygen.c index 29013a20f..8df55f2c2 100644 --- a/ssh-keygen.c +++ b/ssh-keygen.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssh-keygen.c,v 1.393 2020/01/25 23:02:13 djm Exp $ */ +/* $OpenBSD: ssh-keygen.c,v 1.394 2020/01/25 23:13:09 djm Exp $ */ /* * Author: Tatu Ylonen * Copyright (c) 1994 Tatu Ylonen , Espoo, Finland @@ -3579,7 +3579,7 @@ main(int argc, char **argv) if (r == 0) break; if (r != SSH_ERR_KEY_WRONG_PASSPHRASE) - exit(1); /* error message already printed */ + fatal("Key enrollment failed: %s", ssh_err(r)); if (passphrase != NULL) freezero(passphrase, strlen(passphrase)); passphrase = read_passphrase("Enter PIN for security " diff --git a/ssh-sk-helper.c b/ssh-sk-helper.c index a4be9d369..2f93ad716 100644 --- a/ssh-sk-helper.c +++ b/ssh-sk-helper.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssh-sk-helper.c,v 1.8 2020/01/10 23:43:26 djm Exp $ */ +/* $OpenBSD: ssh-sk-helper.c,v 1.9 2020/01/25 23:13:09 djm Exp $ */ /* * Copyright (c) 2019 Google LLC * @@ -63,7 +63,7 @@ reply_error(int r, char *fmt, ...) va_start(ap, fmt); xvasprintf(&msg, fmt, ap); va_end(ap); - error("%s: %s", __progname, msg); + debug("%s: %s", __progname, msg); free(msg); if (r >= 0) diff --git a/ssh-sk.c b/ssh-sk.c index 3f5eed62d..a8d4de832 100644 --- a/ssh-sk.c +++ b/ssh-sk.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssh-sk.c,v 1.24 2020/01/06 02:00:47 djm Exp $ */ +/* $OpenBSD: ssh-sk.c,v 1.25 2020/01/25 23:13:09 djm Exp $ */ /* * Copyright (c) 2019 Google LLC * @@ -338,6 +338,8 @@ skerr_to_ssherr(int skerr) return SSH_ERR_FEATURE_UNSUPPORTED; case SSH_SK_ERR_PIN_REQUIRED: return SSH_ERR_KEY_WRONG_PASSPHRASE; + case SSH_SK_ERR_DEVICE_NOT_FOUND: + return SSH_ERR_DEVICE_NOT_FOUND; case SSH_SK_ERR_GENERAL: default: return SSH_ERR_INVALID_FORMAT; @@ -490,7 +492,7 @@ sshsk_enroll(int type, const char *provider_path, const char *device, /* enroll key */ if ((r = skp->sk_enroll(alg, challenge, challenge_len, application, flags, pin, opts, &resp)) != 0) { - error("Security key provider \"%s\" returned failure %d", + debug("%s: provider \"%s\" returned failure %d", __func__, provider_path, r); r = skerr_to_ssherr(r); goto out; diff --git a/ssherr.c b/ssherr.c index 38974f51b..bd954aadd 100644 --- a/ssherr.c +++ b/ssherr.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssherr.c,v 1.9 2019/12/30 09:24:45 djm Exp $ */ +/* $OpenBSD: ssherr.c,v 1.10 2020/01/25 23:13:09 djm Exp $ */ /* * Copyright (c) 2011 Damien Miller * @@ -143,6 +143,8 @@ ssh_err(int n) return "signature algorithm not supported"; case SSH_ERR_FEATURE_UNSUPPORTED: return "requested feature not supported"; + case SSH_ERR_DEVICE_NOT_FOUND: + return "device not found"; default: return "unknown error"; } diff --git a/ssherr.h b/ssherr.h index 520bd4964..085e75274 100644 --- a/ssherr.h +++ b/ssherr.h @@ -1,4 +1,4 @@ -/* $OpenBSD: ssherr.h,v 1.7 2019/12/30 09:24:45 djm Exp $ */ +/* $OpenBSD: ssherr.h,v 1.8 2020/01/25 23:13:09 djm Exp $ */ /* * Copyright (c) 2011 Damien Miller * @@ -81,6 +81,7 @@ #define SSH_ERR_NUMBER_TOO_LARGE -57 #define SSH_ERR_SIGN_ALG_UNSUPPORTED -58 #define SSH_ERR_FEATURE_UNSUPPORTED -59 +#define SSH_ERR_DEVICE_NOT_FOUND -60 /* Translate a numeric error code to a human-readable error string */ const char *ssh_err(int n); -- cgit v1.2.3