diff options
author | Damien Miller <djm@cvs.openbsd.org> | 2020-01-06 02:00:48 +0000 |
---|---|---|
committer | Damien Miller <djm@cvs.openbsd.org> | 2020-01-06 02:00:48 +0000 |
commit | 84aa6076ea9d5f6c49483cc24487a96e49799ad6 (patch) | |
tree | 1afb196ff4c2182b1f837274fc8d30223cd14be7 | |
parent | 91fd81e04827c6fef6f8e48e28f4e00fdfb3b5f4 (diff) |
Extends the SK API to accept a set of key/value options for all
operations. These are intended to future-proof the API a little by
making it easier to specify additional fields for without having to
change the API version for each.
At present, only two options are defined: one to explicitly specify
the device for an operation (rather than accepting the middleware's
autoselection) and another to specify the FIDO2 username that may
be used when generating a resident key. These new options may be
invoked at key generation time via ssh-keygen -O
This also implements a suggestion from Markus to avoid "int" in favour
of uint32_t for the algorithm argument in the API, to make implementation
of ssh-sk-client/helper a little easier.
feedback, fixes and ok markus@
-rw-r--r-- | usr.bin/ssh/PROTOCOL.u2f | 47 | ||||
-rw-r--r-- | usr.bin/ssh/sk-api.h | 23 | ||||
-rw-r--r-- | usr.bin/ssh/sk-usbhid.c | 194 | ||||
-rw-r--r-- | usr.bin/ssh/ssh-add.c | 5 | ||||
-rw-r--r-- | usr.bin/ssh/ssh-keygen.1 | 23 | ||||
-rw-r--r-- | usr.bin/ssh/ssh-keygen.c | 39 | ||||
-rw-r--r-- | usr.bin/ssh/ssh-sk-client.c | 14 | ||||
-rw-r--r-- | usr.bin/ssh/ssh-sk-helper.c | 45 | ||||
-rw-r--r-- | usr.bin/ssh/ssh-sk.c | 121 | ||||
-rw-r--r-- | usr.bin/ssh/ssh-sk.h | 14 |
10 files changed, 404 insertions, 121 deletions
diff --git a/usr.bin/ssh/PROTOCOL.u2f b/usr.bin/ssh/PROTOCOL.u2f index 5f44c3acc6f..fd0cd0de0ab 100644 --- a/usr.bin/ssh/PROTOCOL.u2f +++ b/usr.bin/ssh/PROTOCOL.u2f @@ -233,7 +233,7 @@ support for the common case of USB HID security keys internally. The middleware library need only expose a handful of functions: - #define SSH_SK_VERSION_MAJOR 0x00030000 /* API version */ + #define SSH_SK_VERSION_MAJOR 0x00040000 /* API version */ #define SSH_SK_VERSION_MAJOR_MASK 0xffff0000 /* Flags */ @@ -245,6 +245,11 @@ The middleware library need only expose a handful of functions: #define SSH_SK_ECDSA 0x00 #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 { uint8_t *public_key; size_t public_key_len; @@ -266,35 +271,63 @@ The middleware library need only expose a handful of functions: }; struct sk_resident_key { - uint8_t alg; + uint32_t alg; size_t slot; char *application; struct sk_enroll_response key; }; + struct sk_option { + char *name; + char *value; + uint8_t important; + }; + /* Return the version of the middleware API */ uint32_t sk_api_version(void); /* Enroll a U2F key (private key generation) */ - int sk_enroll(int alg, const uint8_t *challenge, size_t challenge_len, + int sk_enroll(uint32_t alg, + const uint8_t *challenge, size_t challenge_len, const char *application, uint8_t flags, const char *pin, + struct sk_option **options, struct sk_enroll_response **enroll_response); /* Sign a challenge */ - int sk_sign(int alg, const uint8_t *message, size_t message_len, + int sk_sign(uint32_t alg, const uint8_t *message, size_t message_len, const char *application, const uint8_t *key_handle, size_t key_handle_len, - uint8_t flags, const char *pin, + uint8_t flags, const char *pin, struct sk_option **options, struct sk_sign_response **sign_response); /* Enumerate all resident keys */ - int sk_load_resident_keys(const char *pin, + int sk_load_resident_keys(const char *pin, struct sk_option **options, struct sk_resident_key ***rks, size_t *nrks); The SSH_SK_VERSION_MAJOR should be incremented for each incompatible API change. -In OpenSSH, these will be invoked by using a similar mechanism to +The options may be used to pass miscellaneous options to the middleware +as a NULL-terminated array of pointers to struct sk_option. The middleware +may ignore unsupported or unknown options unless the "important" flag is +set, in which case it should return failure if an unsupported option is +requested. + +At present the following options names are supported: + + "device" + + Specifies a specific FIDO device on which to perform the + operation. The value in this field is interpreted by the + middleware but it would be typical to specify a path to + a /dev node for the device in question. + + "user" + + Specifies the FIDO2 username used when enrolling a key, + overriding OpenSSH's default of using an all-zero username. + +In OpenSSH, the middleware will be invoked by using a similar mechanism to ssh-pkcs11-helper to provide address-space containment of the middleware from ssh-agent. diff --git a/usr.bin/ssh/sk-api.h b/usr.bin/ssh/sk-api.h index 825cb275990..fa7ce628a95 100644 --- a/usr.bin/ssh/sk-api.h +++ b/usr.bin/ssh/sk-api.h @@ -1,4 +1,4 @@ -/* $OpenBSD: sk-api.h,v 1.6 2019/12/30 09:24:45 djm Exp $ */ +/* $OpenBSD: sk-api.h,v 1.7 2020/01/06 02:00:46 djm Exp $ */ /* * Copyright (c) 2019 Google LLC * @@ -56,30 +56,37 @@ struct sk_sign_response { }; struct sk_resident_key { - uint8_t alg; + uint32_t alg; size_t slot; char *application; struct sk_enroll_response key; }; -#define SSH_SK_VERSION_MAJOR 0x00030000 /* current API version */ +struct sk_option { + char *name; + char *value; + uint8_t required; +}; + +#define SSH_SK_VERSION_MAJOR 0x00040000 /* current API version */ #define SSH_SK_VERSION_MAJOR_MASK 0xffff0000 /* Return the version of the middleware API */ uint32_t sk_api_version(void); /* Enroll a U2F key (private key generation) */ -int sk_enroll(int alg, const uint8_t *challenge, size_t challenge_len, +int sk_enroll(uint32_t alg, const uint8_t *challenge, size_t challenge_len, const char *application, uint8_t flags, const char *pin, - struct sk_enroll_response **enroll_response); + struct sk_option **options, struct sk_enroll_response **enroll_response); /* Sign a challenge */ -int sk_sign(int alg, const uint8_t *message, size_t message_len, +int sk_sign(uint32_t alg, const uint8_t *message, size_t message_len, const char *application, const uint8_t *key_handle, size_t key_handle_len, - uint8_t flags, const char *pin, struct sk_sign_response **sign_response); + uint8_t flags, const char *pin, struct sk_option **options, + struct sk_sign_response **sign_response); /* Enumerate all resident keys */ -int sk_load_resident_keys(const char *pin, +int sk_load_resident_keys(const char *pin, struct sk_option **options, struct sk_resident_key ***rks, size_t *nrks); #endif /* _SK_API_H */ diff --git a/usr.bin/ssh/sk-usbhid.c b/usr.bin/ssh/sk-usbhid.c index 878c9980c7f..dc98a7086e3 100644 --- a/usr.bin/ssh/sk-usbhid.c +++ b/usr.bin/ssh/sk-usbhid.c @@ -50,7 +50,7 @@ } while (0) #endif -#define SK_VERSION_MAJOR 0x00030000 /* current API version */ +#define SK_VERSION_MAJOR 0x00040000 /* current API version */ /* Flags */ #define SK_USER_PRESENCE_REQD 0x01 @@ -87,12 +87,18 @@ struct sk_sign_response { }; struct sk_resident_key { - uint8_t alg; + 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 @@ -105,17 +111,18 @@ struct sk_resident_key { uint32_t sk_api_version(void); /* Enroll a U2F key (private key generation) */ -int sk_enroll(int alg, const uint8_t *challenge, size_t challenge_len, +int sk_enroll(uint32_t alg, const uint8_t *challenge, size_t challenge_len, const char *application, uint8_t flags, const char *pin, - struct sk_enroll_response **enroll_response); + struct sk_option **options, struct sk_enroll_response **enroll_response); /* Sign a challenge */ -int sk_sign(int alg, const uint8_t *message, size_t message_len, +int sk_sign(uint32_t alg, const uint8_t *message, size_t message_len, const char *application, const uint8_t *key_handle, size_t key_handle_len, - uint8_t flags, const char *pin, struct sk_sign_response **sign_response); + uint8_t flags, const char *pin, struct sk_option **options, + struct sk_sign_response **sign_response); /* Load resident keys */ -int sk_load_resident_keys(const char *pin, +int sk_load_resident_keys(const char *pin, struct sk_option **options, struct sk_resident_key ***rks, size_t *nrks); static void skdebug(const char *func, const char *fmt, ...) @@ -231,15 +238,27 @@ try_device(fido_dev_t *dev, const uint8_t *message, size_t message_len, /* Iterate over configured devices looking for a specific key handle */ static fido_dev_t * -find_device(const uint8_t *message, size_t message_len, const char *application, - const uint8_t *key_handle, size_t key_handle_len) +find_device(const char *path, const uint8_t *message, size_t message_len, + const char *application, const uint8_t *key_handle, size_t key_handle_len) { fido_dev_info_t *devlist = NULL; fido_dev_t *dev = NULL; size_t devlist_len = 0, i; - const char *path; int r; + if (path != NULL) { + if ((dev = fido_dev_new()) == NULL) { + skdebug(__func__, "fido_dev_new failed"); + return NULL; + } + if ((r = fido_dev_open(dev, path)) != FIDO_OK) { + skdebug(__func__, "fido_dev_open failed"); + fido_dev_free(&dev); + return NULL; + } + return dev; + } + if ((devlist = fido_dev_info_new(MAX_FIDO_DEVICES)) == NULL) { skdebug(__func__, "fido_dev_info_new failed"); goto out; @@ -398,7 +417,7 @@ pack_public_key_ed25519(const fido_cred_t *cred, } static int -pack_public_key(int alg, const fido_cred_t *cred, +pack_public_key(uint32_t alg, const fido_cred_t *cred, struct sk_enroll_response *response) { switch(alg) { @@ -427,10 +446,45 @@ fidoerr_to_skerr(int fidoerr) } } +static int +check_enroll_options(struct sk_option **options, char **devicep, + uint8_t *user_id, size_t user_id_len) +{ + size_t i; + + if (options == NULL) + return 0; + for (i = 0; options[i] != NULL; i++) { + if (strcmp(options[i]->name, "device") == 0) { + if ((*devicep = strdup(options[i]->value)) == NULL) { + skdebug(__func__, "strdup device failed"); + return -1; + } + skdebug(__func__, "requested device %s", *devicep); + } if (strcmp(options[i]->name, "user") == 0) { + if (strlcpy(user_id, options[i]->value, user_id_len) >= + user_id_len) { + skdebug(__func__, "user too long"); + return -1; + } + skdebug(__func__, "requested user %s", + (char *)user_id); + } else { + skdebug(__func__, "requested unsupported option %s", + options[i]->name); + if (options[i]->required) { + skdebug(__func__, "unknown required option"); + return -1; + } + } + } + return 0; +} + int -sk_enroll(int alg, const uint8_t *challenge, size_t challenge_len, +sk_enroll(uint32_t alg, const uint8_t *challenge, size_t challenge_len, const char *application, uint8_t flags, const char *pin, - struct sk_enroll_response **enroll_response) + struct sk_option **options, struct sk_enroll_response **enroll_response) { fido_cred_t *cred = NULL; fido_dev_t *dev = NULL; @@ -450,6 +504,11 @@ sk_enroll(int alg, const uint8_t *challenge, size_t challenge_len, skdebug(__func__, "enroll_response == NULL"); goto out; } + memset(user_id, 0, sizeof(user_id)); + if (check_enroll_options(options, &device, + user_id, sizeof(user_id)) != 0) + goto out; /* error already logged */ + *enroll_response = NULL; switch(alg) { #ifdef WITH_OPENSSL @@ -464,7 +523,7 @@ sk_enroll(int alg, const uint8_t *challenge, size_t challenge_len, skdebug(__func__, "unsupported key type %d", alg); goto out; } - if ((device = pick_first_device()) == NULL) { + if (device == NULL && (device = pick_first_device()) == NULL) { skdebug(__func__, "pick_first_device failed"); goto out; } @@ -473,7 +532,6 @@ sk_enroll(int alg, const uint8_t *challenge, size_t challenge_len, skdebug(__func__, "fido_cred_new failed"); goto out; } - memset(user_id, 0, sizeof(user_id)); if ((r = fido_cred_set_type(cred, cose_alg)) != FIDO_OK) { skdebug(__func__, "fido_cred_set_type: %s", fido_strerr(r)); goto out; @@ -650,7 +708,8 @@ pack_sig_ed25519(fido_assert_t *assert, struct sk_sign_response *response) } static int -pack_sig(int alg, fido_assert_t *assert, struct sk_sign_response *response) +pack_sig(uint32_t alg, fido_assert_t *assert, + struct sk_sign_response *response) { switch(alg) { #ifdef WITH_OPENSSL @@ -664,13 +723,42 @@ pack_sig(int alg, fido_assert_t *assert, struct sk_sign_response *response) } } +/* Checks sk_options for sk_sign() and sk_load_resident_keys() */ +static int +check_sign_load_resident_options(struct sk_option **options, char **devicep) +{ + size_t i; + + if (options == NULL) + return 0; + for (i = 0; options[i] != NULL; i++) { + if (strcmp(options[i]->name, "device") == 0) { + if ((*devicep = strdup(options[i]->value)) == NULL) { + skdebug(__func__, "strdup device failed"); + return -1; + } + skdebug(__func__, "requested device %s", *devicep); + } else { + skdebug(__func__, "requested unsupported option %s", + options[i]->name); + if (options[i]->required) { + skdebug(__func__, "unknown required option"); + return -1; + } + } + } + return 0; +} + int -sk_sign(int alg, const uint8_t *message, size_t message_len, +sk_sign(uint32_t alg, const uint8_t *message, size_t message_len, const char *application, const uint8_t *key_handle, size_t key_handle_len, - uint8_t flags, const char *pin, struct sk_sign_response **sign_response) + uint8_t flags, const char *pin, struct sk_option **options, + struct sk_sign_response **sign_response) { fido_assert_t *assert = NULL; + char *device = NULL; fido_dev_t *dev = NULL; struct sk_sign_response *response = NULL; int ret = SSH_SK_ERR_GENERAL; @@ -685,8 +773,10 @@ sk_sign(int alg, const uint8_t *message, size_t message_len, goto out; } *sign_response = NULL; - if ((dev = find_device(message, message_len, application, key_handle, - key_handle_len)) == NULL) { + if (check_sign_load_resident_options(options, &device) != 0) + goto out; /* error already logged */ + if ((dev = find_device(device, message, message_len, + application, key_handle, key_handle_len)) == NULL) { skdebug(__func__, "couldn't find device for key handle"); goto out; } @@ -733,6 +823,7 @@ sk_sign(int alg, const uint8_t *message, size_t message_len, response = NULL; ret = 0; out: + free(device); if (response != NULL) { free(response->sig_r); free(response->sig_s); @@ -785,6 +876,7 @@ read_rks(const char *devpath, const char *pin, } skdebug(__func__, "get metadata for %s failed: %s", devpath, fido_strerr(r)); + ret = fidoerr_to_skerr(r); goto out; } skdebug(__func__, "existing %llu, remaining %llu", @@ -900,7 +992,7 @@ read_rks(const char *devpath, const char *pin, } int -sk_load_resident_keys(const char *pin, +sk_load_resident_keys(const char *pin, struct sk_option **options, struct sk_resident_key ***rksp, size_t *nrksp) { int ret = SSH_SK_ERR_GENERAL, r = -1; @@ -908,39 +1000,57 @@ sk_load_resident_keys(const char *pin, size_t i, ndev = 0, nrks = 0; const fido_dev_info_t *di; struct sk_resident_key **rks = NULL; + char *device = NULL; *rksp = NULL; *nrksp = 0; - if ((devlist = fido_dev_info_new(MAX_FIDO_DEVICES)) == NULL) { - skdebug(__func__, "fido_dev_info_new failed"); - goto out; - } - if ((r = fido_dev_info_manifest(devlist, - MAX_FIDO_DEVICES, &ndev)) != FIDO_OK) { - skdebug(__func__, "fido_dev_info_manifest failed: %s", - fido_strerr(r)); - goto out; - } - for (i = 0; i < ndev; i++) { - if ((di = fido_dev_info_ptr(devlist, i)) == NULL) { - skdebug(__func__, "no dev info at %zu", i); - continue; - } - skdebug(__func__, "trying %s", fido_dev_info_path(di)); - if ((r = read_rks(fido_dev_info_path(di), pin, - &rks, &nrks)) != 0) { + if (check_sign_load_resident_options(options, &device) != 0) + goto out; /* error already logged */ + if (device != NULL) { + skdebug(__func__, "trying %s", device); + if ((r = read_rks(device, pin, &rks, &nrks)) != 0) { skdebug(__func__, "read_rks failed for %s", fido_dev_info_path(di)); - continue; + ret = r; + goto out; + } + } else { + /* Try all devices */ + if ((devlist = fido_dev_info_new(MAX_FIDO_DEVICES)) == NULL) { + skdebug(__func__, "fido_dev_info_new failed"); + goto out; + } + if ((r = fido_dev_info_manifest(devlist, + MAX_FIDO_DEVICES, &ndev)) != FIDO_OK) { + skdebug(__func__, "fido_dev_info_manifest failed: %s", + fido_strerr(r)); + goto out; + } + for (i = 0; i < ndev; i++) { + if ((di = fido_dev_info_ptr(devlist, i)) == NULL) { + skdebug(__func__, "no dev info at %zu", i); + continue; + } + skdebug(__func__, "trying %s", fido_dev_info_path(di)); + if ((r = read_rks(fido_dev_info_path(di), pin, + &rks, &nrks)) != 0) { + skdebug(__func__, "read_rks failed for %s", + fido_dev_info_path(di)); + /* remember last error */ + ret = r; + continue; + } } } - /* success */ - ret = 0; + /* success, unless we have no keys but a specific error */ + if (nrks > 0 || ret == SSH_SK_ERR_GENERAL) + ret = 0; *rksp = rks; *nrksp = nrks; rks = NULL; nrks = 0; out: + free(device); for (i = 0; i < nrks; i++) { free(rks[i]->application); freezero(rks[i]->key.public_key, rks[i]->key.public_key_len); diff --git a/usr.bin/ssh/ssh-add.c b/usr.bin/ssh/ssh-add.c index d60536fd522..e6c5cd92d54 100644 --- a/usr.bin/ssh/ssh-add.c +++ b/usr.bin/ssh/ssh-add.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssh-add.c,v 1.148 2019/12/30 09:22:49 djm Exp $ */ +/* $OpenBSD: ssh-add.c,v 1.149 2020/01/06 02:00:46 djm Exp $ */ /* * Author: Tatu Ylonen <ylo@cs.hut.fi> * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland @@ -542,7 +542,8 @@ load_resident_keys(int agent_fd, const char *skprovider, int qflag) char *fp; pass = read_passphrase("Enter PIN for security key: ", RP_ALLOW_STDIN); - if ((r = sshsk_load_resident(skprovider, pass, &keys, &nkeys)) != 0) { + if ((r = sshsk_load_resident(skprovider, NULL, pass, + &keys, &nkeys)) != 0) { error("Unable to load resident keys: %s", ssh_err(r)); return r; } diff --git a/usr.bin/ssh/ssh-keygen.1 b/usr.bin/ssh/ssh-keygen.1 index 7b83a224006..92c516588e2 100644 --- a/usr.bin/ssh/ssh-keygen.1 +++ b/usr.bin/ssh/ssh-keygen.1 @@ -1,4 +1,4 @@ -.\" $OpenBSD: ssh-keygen.1,v 1.188 2020/01/03 07:33:33 jmc Exp $ +.\" $OpenBSD: ssh-keygen.1,v 1.189 2020/01/06 02:00:46 djm Exp $ .\" .\" Author: Tatu Ylonen <ylo@cs.hut.fi> .\" Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland @@ -35,7 +35,7 @@ .\" (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF .\" THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. .\" -.Dd $Mdocdate: January 3 2020 $ +.Dd $Mdocdate: January 6 2020 $ .Dt SSH-KEYGEN 1 .Os .Sh NAME @@ -462,8 +462,18 @@ section may be specified. .Pp When generating a key that will be hosted on a FIDO authenticator, this flag may be used to specify key-specific options. -Two FIDO authenticator options are supported at present: -.Pp +The FIDO authenticator options are supported at present are: +.Pp +.Cm application +overrides the default FIDO application/origin string of +.Dq ssh: . +This option may be useful when generating host or domain-specific resident +keys. +.Cm device +explicitly specify a device to generate the key on, rather than accepting +the authenticator middleware's automatic selection. +.Xr fido 4 +device to use, rather than letting the token middleware select one. .Cm no-touch-required indicates that the generated private key should not require touch events (user presence) when making signatures. @@ -478,6 +488,11 @@ Resident keys may be supported on FIDO2 tokens and typically require that a PIN be set on the token prior to generation. Resident keys may be loaded off the token using .Xr ssh-add 1 . +.Cm user +allows specification of a username to be associated with a resident key, +overriding the empty default username. +Specifying a username may be useful when generating multiple resident keys +for the same application name. .Pp The .Fl O diff --git a/usr.bin/ssh/ssh-keygen.c b/usr.bin/ssh/ssh-keygen.c index 3b2ed338024..3d395566e97 100644 --- a/usr.bin/ssh/ssh-keygen.c +++ b/usr.bin/ssh/ssh-keygen.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssh-keygen.c,v 1.381 2020/01/02 22:40:09 djm Exp $ */ +/* $OpenBSD: ssh-keygen.c,v 1.382 2020/01/06 02:00:46 djm Exp $ */ /* * Author: Tatu Ylonen <ylo@cs.hut.fi> * Copyright (c) 1994 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland @@ -2897,7 +2897,7 @@ skip_ssh_url_preamble(const char *s) } static int -do_download_sk(const char *skprovider) +do_download_sk(const char *skprovider, const char *device) { struct sshkey **keys; size_t nkeys, i; @@ -2909,7 +2909,8 @@ do_download_sk(const char *skprovider) fatal("Cannot download keys without provider"); pin = read_passphrase("Enter PIN for security key: ", RP_ALLOW_STDIN); - if ((r = sshsk_load_resident(skprovider, pin, &keys, &nkeys)) != 0) { + if ((r = sshsk_load_resident(skprovider, device, pin, + &keys, &nkeys)) != 0) { freezero(pin, strlen(pin)); error("Unable to load resident keys: %s", ssh_err(r)); return -1; @@ -3049,6 +3050,7 @@ main(int argc, char **argv) int do_gen_candidates = 0, do_screen_candidates = 0, download_sk = 0; unsigned long long cert_serial = 0; char *identity_comment = NULL, *ca_key_path = NULL, **opts = NULL; + char *sk_application = NULL, *sk_device = NULL, *sk_user = NULL; size_t i, nopts = 0; u_int32_t bits = 0; uint8_t sk_flags = SSH_SK_USER_PRESENCE_REQD; @@ -3375,8 +3377,17 @@ main(int argc, char **argv) } if (pkcs11provider != NULL) do_download(pw); - if (download_sk) - return do_download_sk(sk_provider); + if (download_sk) { + for (i = 0; i < nopts; i++) { + if (strncasecmp(opts[i], "device=", 7) == 0) { + sk_device = xstrdup(opts[i] + 7); + } else { + fatal("Option \"%s\" is unsupported for " + "FIDO authenticator download", opts[i]); + } + } + return do_download_sk(sk_provider, sk_device); + } if (print_fingerprint || print_bubblebabble) do_fingerprint(pw); if (change_passphrase) @@ -3463,6 +3474,13 @@ main(int argc, char **argv) sk_flags &= ~SSH_SK_USER_PRESENCE_REQD; } else if (strcasecmp(opts[i], "resident") == 0) { sk_flags |= SSH_SK_RESIDENT_KEY; + } else if (strncasecmp(opts[i], "device=", 7) == 0) { + sk_device = xstrdup(opts[i] + 7); + } else if (strncasecmp(opts[i], "user=", 5) == 0) { + sk_user = xstrdup(opts[i] + 5); + } else if (strncasecmp(opts[i], + "application=", 12) == 0) { + sk_application = xstrdup(opts[i] + 12); } else { fatal("Option \"%s\" is unsupported for " "FIDO authenticator enrollment", opts[i]); @@ -3474,14 +3492,11 @@ main(int argc, char **argv) } passphrase = NULL; for (i = 0 ; i < 3; i++) { - 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, passphrase, NULL, &private, NULL); + r = sshsk_enroll(type, sk_provider, sk_device, + sk_application == NULL ? "ssh:" : sk_application, + sk_user, sk_flags, passphrase, NULL, + &private, NULL); if (r == 0) break; if (r != SSH_ERR_KEY_WRONG_PASSPHRASE) diff --git a/usr.bin/ssh/ssh-sk-client.c b/usr.bin/ssh/ssh-sk-client.c index 08737f1c558..8bb8e48469d 100644 --- a/usr.bin/ssh/ssh-sk-client.c +++ b/usr.bin/ssh/ssh-sk-client.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssh-sk-client.c,v 1.3 2019/12/30 09:23:28 djm Exp $ */ +/* $OpenBSD: ssh-sk-client.c,v 1.4 2020/01/06 02:00:46 djm Exp $ */ /* * Copyright (c) 2019 Google LLC * @@ -276,8 +276,9 @@ sshsk_sign(const char *provider, struct sshkey *key, } int -sshsk_enroll(int type, const char *provider_path, const char *application, - uint8_t flags, const char *pin, struct sshbuf *challenge_buf, +sshsk_enroll(int type, const char *provider_path, const char *device, + const char *application, const char *userid, uint8_t flags, + const char *pin, struct sshbuf *challenge_buf, struct sshkey **keyp, struct sshbuf *attest) { int oerrno, r = SSH_ERR_INTERNAL_ERROR; @@ -301,7 +302,9 @@ sshsk_enroll(int type, const char *provider_path, const char *application, if ((r = sshbuf_put_u32(req, SSH_SK_HELPER_ENROLL)) != 0 || (r = sshbuf_put_u32(req, (u_int)type)) != 0 || (r = sshbuf_put_cstring(req, provider_path)) != 0 || + (r = sshbuf_put_cstring(req, device)) != 0 || (r = sshbuf_put_cstring(req, application)) != 0 || + (r = sshbuf_put_cstring(req, userid)) != 0 || (r = sshbuf_put_u8(req, flags)) != 0 || (r = sshbuf_put_cstring(req, pin)) != 0 || (r = sshbuf_put_stringb(req, challenge_buf)) != 0) { @@ -348,8 +351,8 @@ sshsk_enroll(int type, const char *provider_path, const char *application, } int -sshsk_load_resident(const char *provider_path, const char *pin, - struct sshkey ***keysp, size_t *nkeysp) +sshsk_load_resident(const char *provider_path, const char *device, + const char *pin, struct sshkey ***keysp, size_t *nkeysp) { int oerrno, r = SSH_ERR_INTERNAL_ERROR; struct sshbuf *kbuf = NULL, *req = NULL, *resp = NULL; @@ -368,6 +371,7 @@ sshsk_load_resident(const char *provider_path, const char *pin, if ((r = sshbuf_put_u32(req, SSH_SK_HELPER_LOAD_RESIDENT)) != 0 || (r = sshbuf_put_cstring(req, provider_path)) != 0 || + (r = sshbuf_put_cstring(req, device)) != 0 || (r = sshbuf_put_cstring(req, pin)) != 0) { error("%s: compose: %s", __func__, ssh_err(r)); goto out; diff --git a/usr.bin/ssh/ssh-sk-helper.c b/usr.bin/ssh/ssh-sk-helper.c index 33d7e8e8c33..96fe9700195 100644 --- a/usr.bin/ssh/ssh-sk-helper.c +++ b/usr.bin/ssh/ssh-sk-helper.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssh-sk-helper.c,v 1.6 2019/12/30 09:23:28 djm Exp $ */ +/* $OpenBSD: ssh-sk-helper.c,v 1.7 2020/01/06 02:00:46 djm Exp $ */ /* * Copyright (c) 2019 Google LLC * @@ -74,6 +74,17 @@ reply_error(int r, char *fmt, ...) return resp; } +/* If the specified string is zero length, then free it and replace with NULL */ +static void +null_empty(char **s) +{ + if (s == NULL || *s == NULL || **s != '\0') + return; + + free(*s); + *s = NULL; +} + static struct sshbuf * process_sign(struct sshbuf *req) { @@ -105,10 +116,7 @@ process_sign(struct sshbuf *req) "msg len %zu, compat 0x%lx", __progname, sshkey_type(key), provider, msglen, (u_long)compat); - if (*pin == 0) { - free(pin); - pin = NULL; - } + null_empty(&pin); if ((r = sshsk_sign(provider, key, &sig, &siglen, message, msglen, compat, pin)) != 0) { @@ -135,7 +143,7 @@ process_enroll(struct sshbuf *req) { int r; u_int type; - char *provider, *application, *pin; + char *provider, *application, *pin, *device, *userid; uint8_t flags; struct sshbuf *challenge, *attest, *kbuf, *resp; struct sshkey *key; @@ -146,7 +154,9 @@ process_enroll(struct sshbuf *req) if ((r = sshbuf_get_u32(req, &type)) != 0 || (r = sshbuf_get_cstring(req, &provider, NULL)) != 0 || + (r = sshbuf_get_cstring(req, &device, NULL)) != 0 || (r = sshbuf_get_cstring(req, &application, NULL)) != 0 || + (r = sshbuf_get_cstring(req, &userid, NULL)) != 0 || (r = sshbuf_get_u8(req, &flags)) != 0 || (r = sshbuf_get_cstring(req, &pin, NULL)) != 0 || (r = sshbuf_froms(req, &challenge)) != 0) @@ -160,13 +170,12 @@ process_enroll(struct sshbuf *req) sshbuf_free(challenge); challenge = NULL; } - if (*pin == 0) { - free(pin); - pin = NULL; - } + null_empty(&device); + null_empty(&userid); + null_empty(&pin); - if ((r = sshsk_enroll((int)type, provider, application, flags, pin, - challenge, &key, attest)) != 0) { + if ((r = sshsk_enroll((int)type, provider, device, application, userid, + flags, pin, challenge, &key, attest)) != 0) { resp = reply_error(r, "Enrollment failed: %s", ssh_err(r)); goto out; } @@ -197,7 +206,7 @@ static struct sshbuf * process_load_resident(struct sshbuf *req) { int r; - char *provider, *pin; + char *provider, *pin, *device; struct sshbuf *kbuf, *resp; struct sshkey **keys = NULL; size_t nkeys = 0, i; @@ -206,17 +215,17 @@ process_load_resident(struct sshbuf *req) fatal("%s: sshbuf_new failed", __progname); if ((r = sshbuf_get_cstring(req, &provider, NULL)) != 0 || + (r = sshbuf_get_cstring(req, &device, NULL)) != 0 || (r = sshbuf_get_cstring(req, &pin, NULL)) != 0) fatal("%s: buffer error: %s", __progname, ssh_err(r)); if (sshbuf_len(req) != 0) fatal("%s: trailing data in request", __progname); - if (*pin == 0) { - free(pin); - pin = NULL; - } + null_empty(&device); + null_empty(&pin); - if ((r = sshsk_load_resident(provider, pin, &keys, &nkeys)) != 0) { + if ((r = sshsk_load_resident(provider, device, pin, + &keys, &nkeys)) != 0) { resp = reply_error(r, " sshsk_load_resident failed: %s", ssh_err(r)); goto out; diff --git a/usr.bin/ssh/ssh-sk.c b/usr.bin/ssh/ssh-sk.c index 275a6919dc2..a3c6a8ae7bf 100644 --- a/usr.bin/ssh/ssh-sk.c +++ b/usr.bin/ssh/ssh-sk.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssh-sk.c,v 1.23 2019/12/30 09:24:45 djm Exp $ */ +/* $OpenBSD: ssh-sk.c,v 1.24 2020/01/06 02:00:47 djm Exp $ */ /* * Copyright (c) 2019 Google LLC * @@ -49,29 +49,32 @@ struct sshsk_provider { /* Enroll a U2F key (private key generation) */ int (*sk_enroll)(int alg, const uint8_t *challenge, size_t challenge_len, const char *application, uint8_t flags, - const char *pin, struct sk_enroll_response **enroll_response); + const char *pin, struct sk_option **opts, + struct sk_enroll_response **enroll_response); /* Sign a challenge */ int (*sk_sign)(int alg, const uint8_t *message, size_t message_len, const char *application, const uint8_t *key_handle, size_t key_handle_len, - uint8_t flags, const char *pin, + uint8_t flags, const char *pin, struct sk_option **opts, struct sk_sign_response **sign_response); /* Enumerate resident keys */ - int (*sk_load_resident_keys)(const char *pin, + int (*sk_load_resident_keys)(const char *pin, struct sk_option **opts, struct sk_resident_key ***rks, size_t *nrks); }; /* Built-in version */ int ssh_sk_enroll(int alg, const uint8_t *challenge, size_t challenge_len, const char *application, uint8_t flags, - const char *pin, struct sk_enroll_response **enroll_response); + const char *pin, struct sk_option **opts, + struct sk_enroll_response **enroll_response); int ssh_sk_sign(int alg, const uint8_t *message, size_t message_len, const char *application, const uint8_t *key_handle, size_t key_handle_len, - uint8_t flags, const char *pin, struct sk_sign_response **sign_response); -int ssh_sk_load_resident_keys(const char *pin, + uint8_t flags, const char *pin, struct sk_option **opts, + struct sk_sign_response **sign_response); +int ssh_sk_load_resident_keys(const char *pin, struct sk_option **opts, struct sk_resident_key ***rks, size_t *nrks); static void @@ -331,9 +334,80 @@ skerr_to_ssherr(int skerr) } } +static void +sshsk_free_options(struct sk_option **opts) +{ + size_t i; + + if (opts == NULL) + return; + for (i = 0; opts[i] != NULL; i++) { + free(opts[i]->name); + free(opts[i]->value); + free(opts[i]); + } + free(opts); +} + +static int +sshsk_add_option(struct sk_option ***optsp, size_t *noptsp, + const char *name, const char *value, uint8_t required) +{ + struct sk_option **opts = *optsp; + size_t nopts = *noptsp; + + if ((opts = recallocarray(opts, nopts, nopts + 2, /* extra for NULL */ + sizeof(*opts))) == NULL) { + error("%s: array alloc failed", __func__); + return SSH_ERR_ALLOC_FAIL; + } + *optsp = opts; + *noptsp = nopts + 1; + if ((opts[nopts] = calloc(1, sizeof(**opts))) == NULL) { + error("%s: alloc failed", __func__); + return SSH_ERR_ALLOC_FAIL; + } + if ((opts[nopts]->name = strdup(name)) == NULL || + (opts[nopts]->value = strdup(value)) == NULL) { + error("%s: alloc failed", __func__); + return SSH_ERR_ALLOC_FAIL; + } + opts[nopts]->required = required; + return 0; +} + +static int +make_options(const char *device, const char *user_id, + struct sk_option ***optsp) +{ + struct sk_option **opts = NULL; + size_t nopts = 0; + int r, ret = SSH_ERR_INTERNAL_ERROR; + + if (device != NULL && + (r = sshsk_add_option(&opts, &nopts, "device", device, 0)) != 0) { + ret = r; + goto out; + } + if (user_id != NULL && + (r = sshsk_add_option(&opts, &nopts, "user", user_id, 0)) != 0) { + ret = r; + goto out; + } + /* success */ + *optsp = opts; + opts = NULL; + nopts = 0; + ret = 0; + out: + sshsk_free_options(opts); + return ret; +} + int -sshsk_enroll(int type, const char *provider_path, const char *application, - uint8_t flags, const char *pin, struct sshbuf *challenge_buf, +sshsk_enroll(int type, const char *provider_path, const char *device, + const char *application, const char *userid, uint8_t flags, + const char *pin, struct sshbuf *challenge_buf, struct sshkey **keyp, struct sshbuf *attest) { struct sshsk_provider *skp = NULL; @@ -342,17 +416,23 @@ sshsk_enroll(int type, const char *provider_path, const char *application, const u_char *challenge; size_t challenge_len; struct sk_enroll_response *resp = NULL; + struct sk_option **opts = NULL; int r = SSH_ERR_INTERNAL_ERROR; int alg; - debug("%s: provider \"%s\", application \"%s\", flags 0x%02x, " - "challenge len %zu%s", __func__, provider_path, application, - flags, challenge_buf == NULL ? 0 : sshbuf_len(challenge_buf), + debug("%s: provider \"%s\", device \"%s\", application \"%s\", " + "userid \"%s\", flags 0x%02x, challenge len %zu%s", __func__, + provider_path, device, application, userid, flags, + challenge_buf == NULL ? 0 : sshbuf_len(challenge_buf), (pin != NULL && *pin != '\0') ? " with-pin" : ""); *keyp = NULL; if (attest) sshbuf_reset(attest); + + if ((r = make_options(device, userid, &opts)) != 0) + goto out; + switch (type) { #ifdef WITH_OPENSSL case KEY_ECDSA_SK: @@ -399,7 +479,7 @@ sshsk_enroll(int type, const char *provider_path, const char *application, /* XXX validate flags? */ /* enroll key */ if ((r = skp->sk_enroll(alg, challenge, challenge_len, application, - flags, pin, &resp)) != 0) { + flags, pin, opts, &resp)) != 0) { error("Security key provider \"%s\" returned failure %d", provider_path, r); r = skerr_to_ssherr(r); @@ -429,6 +509,7 @@ sshsk_enroll(int type, const char *provider_path, const char *application, key = NULL; /* transferred */ r = 0; out: + sshsk_free_options(opts); sshsk_free(skp); sshkey_free(key); sshsk_free_enroll_response(resp); @@ -520,6 +601,7 @@ sshsk_sign(const char *provider_path, struct sshkey *key, struct sk_sign_response *resp = NULL; struct sshbuf *inner_sig = NULL, *sig = NULL; uint8_t message[32]; + struct sk_option **opts = NULL; debug("%s: provider \"%s\", key %s, flags 0x%02x%s", __func__, provider_path, sshkey_type(key), key->sk_flags, @@ -563,7 +645,7 @@ sshsk_sign(const char *provider_path, struct sshkey *key, if ((r = skp->sk_sign(alg, message, sizeof(message), key->sk_application, sshbuf_ptr(key->sk_key_handle), sshbuf_len(key->sk_key_handle), - key->sk_flags, pin, &resp)) != 0) { + key->sk_flags, pin, opts, &resp)) != 0) { debug("%s: sk_sign failed with code %d", __func__, r); r = skerr_to_ssherr(r); goto out; @@ -609,6 +691,7 @@ sshsk_sign(const char *provider_path, struct sshkey *key, /* success */ r = 0; out: + sshsk_free_options(opts); explicit_bzero(message, sizeof(message)); sshsk_free(skp); sshsk_free_sign_response(resp); @@ -637,8 +720,8 @@ sshsk_free_sk_resident_keys(struct sk_resident_key **rks, size_t nrks) } int -sshsk_load_resident(const char *provider_path, const char *pin, - struct sshkey ***keysp, size_t *nkeysp) +sshsk_load_resident(const char *provider_path, const char *device, + const char *pin, struct sshkey ***keysp, size_t *nkeysp) { struct sshsk_provider *skp = NULL; int r = SSH_ERR_INTERNAL_ERROR; @@ -646,6 +729,7 @@ sshsk_load_resident(const char *provider_path, const char *pin, size_t i, nrks = 0, nkeys = 0; struct sshkey *key = NULL, **keys = NULL, **tmp; uint8_t flags; + struct sk_option **opts = NULL; debug("%s: provider \"%s\"%s", __func__, provider_path, (pin != NULL && *pin != '\0') ? ", have-pin": ""); @@ -655,11 +739,13 @@ sshsk_load_resident(const char *provider_path, const char *pin, *keysp = NULL; *nkeysp = 0; + if ((r = make_options(device, NULL, &opts)) != 0) + goto out; if ((skp = sshsk_open(provider_path)) == NULL) { r = SSH_ERR_INVALID_FORMAT; /* XXX sshsk_open return code? */ goto out; } - if ((r = skp->sk_load_resident_keys(pin, &rks, &nrks)) != 0) { + if ((r = skp->sk_load_resident_keys(pin, opts, &rks, &nrks)) != 0) { error("Security key provider \"%s\" returned failure %d", provider_path, r); r = skerr_to_ssherr(r); @@ -702,6 +788,7 @@ sshsk_load_resident(const char *provider_path, const char *pin, nkeys = 0; r = 0; out: + sshsk_free_options(opts); sshsk_free(skp); sshsk_free_sk_resident_keys(rks, nrks); sshkey_free(key); diff --git a/usr.bin/ssh/ssh-sk.h b/usr.bin/ssh/ssh-sk.h index 348759a9806..ea9ff6e1a41 100644 --- a/usr.bin/ssh/ssh-sk.h +++ b/usr.bin/ssh/ssh-sk.h @@ -1,4 +1,4 @@ -/* $OpenBSD: ssh-sk.h,v 1.8 2019/12/30 09:23:28 djm Exp $ */ +/* $OpenBSD: ssh-sk.h,v 1.9 2020/01/06 02:00:47 djm Exp $ */ /* * Copyright (c) 2019 Google LLC * @@ -20,9 +20,10 @@ struct sshbuf; struct sshkey; +struct sk_option; /* Version of protocol expected from ssh-sk-helper */ -#define SSH_SK_HELPER_VERSION 3 +#define SSH_SK_HELPER_VERSION 4 /* ssh-sk-helper messages */ #define SSH_SK_HELPER_ERROR 0 /* Only valid H->C */ @@ -40,8 +41,9 @@ struct sshkey; * If successful and the attest_data buffer is not NULL then attestation * information is placed there. */ -int sshsk_enroll(int type, const char *provider_path, const char *application, - uint8_t flags, const char *pin, struct sshbuf *challenge_buf, +int sshsk_enroll(int type, const char *provider_path, const char *device, + const char *application, const char *userid, uint8_t flags, + const char *pin, struct sshbuf *challenge_buf, struct sshkey **keyp, struct sshbuf *attest); /* @@ -60,8 +62,8 @@ int sshsk_sign(const char *provider_path, struct sshkey *key, * * Returns 0 on success or a ssherr.h error code on failure. */ -int sshsk_load_resident(const char *provider_path, const char *pin, - struct sshkey ***keysp, size_t *nkeysp); +int sshsk_load_resident(const char *provider_path, const char *device, + const char *pin, struct sshkey ***keysp, size_t *nkeysp); #endif /* _SSH_SK_H */ |