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

diff --git a/PROTOCOL.u2f b/PROTOCOL.u2f
index 45995870128fc4123571daca3b4f516395df9f2d..917e669cddaa344409e793046b8f8a690820599b 100644 (file)
--- 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 170fd4470b0fa6e40025ec9114b67e8f08d7d6ad..1ecaa353730c485b96e0723630c7ee976b587c8b 100644 (file)
--- 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 ad83054ad1641fcd34000472ba961425ce0b7c2f..db6c0057e5f3d90d21ca3eae7fb8da1ec49baf3e 100644 (file)
--- a/sk-usbhid.c
+++ b/sk-usbhid.c
@@ -24,6 +24,7 @@
 #include <stdio.h>
 #include <stddef.h>
 #include <stdarg.h>
+#include <sha2.h>
 
 #ifdef WITH_OPENSSL
 #include <openssl/opensslv.h>
@@ -31,6 +32,7 @@
 #include <openssl/bn.h>
 #include <openssl/ec.h>
 #include <openssl/ecdsa.h>
+#include <openssl/evp.h>
 #endif /* WITH_OPENSSL */
 
 #include <fido.h>
@@ -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 9aff7639aa5753512756796f2d7d4f6306c52359..1afb205f86b09ac31c1b6a5ff276da2e9fd95657 100644 (file)
--- 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);