From 59d2de956ed29aa5565ed5e5947a7abdb27ac013 Mon Sep 17 00:00:00 2001 From: "djm@openbsd.org" Date: Tue, 28 Apr 2020 04:02:29 +0000 Subject: [PATCH] upstream: when signing a challenge using a FIDO toke, perform the hashing in the middleware layer rather than in ssh code. This allows middlewares that call APIs that perform the hashing implicitly (including Microsoft's AFAIK). ok markus@ OpenBSD-Commit-ID: c9fc8630aba26c75d5016884932f08a5a237f37d --- PROTOCOL.u2f | 2 +- sk-api.h | 4 ++-- sk-usbhid.c | 35 ++++++++++++++++++++++++++++++++--- ssh-sk.c | 14 ++------------ 4 files changed, 37 insertions(+), 18 deletions(-) diff --git a/PROTOCOL.u2f b/PROTOCOL.u2f index 459958701..917e669cd 100644 --- a/PROTOCOL.u2f +++ b/PROTOCOL.u2f @@ -236,7 +236,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 0x00040000 /* API version */ + #define SSH_SK_VERSION_MAJOR 0x00050000 /* API version */ #define SSH_SK_VERSION_MAJOR_MASK 0xffff0000 /* Flags */ diff --git a/sk-api.h b/sk-api.h index 170fd4470..1ecaa3537 100644 --- a/sk-api.h +++ b/sk-api.h @@ -1,4 +1,4 @@ -/* $OpenBSD: sk-api.h,v 1.8 2020/01/25 23:13:09 djm Exp $ */ +/* $OpenBSD: sk-api.h,v 1.9 2020/04/28 04:02:29 djm Exp $ */ /* * Copyright (c) 2019 Google LLC * @@ -71,7 +71,7 @@ struct sk_option { uint8_t required; }; -#define SSH_SK_VERSION_MAJOR 0x00040000 /* current API version */ +#define SSH_SK_VERSION_MAJOR 0x00050000 /* 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 index ad83054ad..db6c0057e 100644 --- a/sk-usbhid.c +++ b/sk-usbhid.c @@ -24,6 +24,7 @@ #include #include #include +#include #ifdef WITH_OPENSSL #include @@ -31,6 +32,7 @@ #include #include #include +#include #endif /* WITH_OPENSSL */ #include @@ -710,8 +712,28 @@ check_sign_load_resident_options(struct sk_option **options, char **devicep) return 0; } +/* Calculate SHA256(m) */ +static int +sha256_mem(const void *m, size_t mlen, u_char *d, size_t dlen) +{ +#ifdef WITH_OPENSSL + u_int mdlen; +#endif + + if (dlen != 32) + return -1; +#ifdef WITH_OPENSSL + mdlen = dlen; + if (!EVP_Digest(m, mlen, d, &mdlen, EVP_sha256(), NULL)) + return -1; +#else + SHA256Data(m, mlen, d); +#endif + return 0; +} + int -sk_sign(uint32_t alg, const uint8_t *message, size_t message_len, +sk_sign(uint32_t alg, const uint8_t *data, size_t datalen, const char *application, const uint8_t *key_handle, size_t key_handle_len, uint8_t flags, const char *pin, struct sk_option **options, @@ -721,6 +743,7 @@ sk_sign(uint32_t alg, const uint8_t *message, size_t message_len, char *device = NULL; fido_dev_t *dev = NULL; struct sk_sign_response *response = NULL; + uint8_t message[32]; int ret = SSH_SK_ERR_GENERAL; int r; @@ -735,7 +758,12 @@ sk_sign(uint32_t alg, const uint8_t *message, size_t message_len, *sign_response = NULL; if (check_sign_load_resident_options(options, &device) != 0) goto out; /* error already logged */ - if ((dev = find_device(device, message, message_len, + /* hash data to be signed before it goes to the security key */ + if ((r = sha256_mem(data, datalen, message, sizeof(message))) != 0) { + skdebug(__func__, "hash message failed"); + goto out; + } + if ((dev = find_device(device, message, sizeof(message), application, key_handle, key_handle_len)) == NULL) { skdebug(__func__, "couldn't find device for key handle"); goto out; @@ -745,7 +773,7 @@ sk_sign(uint32_t alg, const uint8_t *message, size_t message_len, goto out; } if ((r = fido_assert_set_clientdata_hash(assert, message, - message_len)) != FIDO_OK) { + sizeof(message))) != FIDO_OK) { skdebug(__func__, "fido_assert_set_clientdata_hash: %s", fido_strerr(r)); goto out; @@ -783,6 +811,7 @@ sk_sign(uint32_t alg, const uint8_t *message, size_t message_len, response = NULL; ret = 0; out: + explicit_bzero(message, sizeof(message)); free(device); if (response != NULL) { free(response->sig_r); diff --git a/ssh-sk.c b/ssh-sk.c index 9aff7639a..1afb205f8 100644 --- a/ssh-sk.c +++ b/ssh-sk.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssh-sk.c,v 1.29 2020/03/06 18:25:48 markus Exp $ */ +/* $OpenBSD: ssh-sk.c,v 1.30 2020/04/28 04:02:29 djm Exp $ */ /* * Copyright (c) 2019 Google LLC * @@ -615,7 +615,6 @@ sshsk_sign(const char *provider_path, struct sshkey *key, int type, alg; 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__, @@ -650,15 +649,7 @@ sshsk_sign(const char *provider_path, struct sshkey *key, goto out; } - /* hash data to be signed before it goes to the security key */ - if ((r = ssh_digest_memory(SSH_DIGEST_SHA256, data, datalen, - message, sizeof(message))) != 0) { - error("%s: hash application failed: %s", __func__, ssh_err(r)); - r = SSH_ERR_INTERNAL_ERROR; - goto out; - } - if ((r = skp->sk_sign(alg, message, sizeof(message), - key->sk_application, + if ((r = skp->sk_sign(alg, data, datalen, key->sk_application, sshbuf_ptr(key->sk_key_handle), sshbuf_len(key->sk_key_handle), key->sk_flags, pin, opts, &resp)) != 0) { debug("%s: sk_sign failed with code %d", __func__, r); @@ -707,7 +698,6 @@ sshsk_sign(const char *provider_path, struct sshkey *key, r = 0; out: sshsk_free_options(opts); - explicit_bzero(message, sizeof(message)); sshsk_free(skp); sshsk_free_sign_response(resp); sshbuf_free(sig);