diff options
author | djm@openbsd.org <djm@openbsd.org> | 2020-04-28 04:02:29 +0000 |
---|---|---|
committer | Damien Miller <djm@mindrot.org> | 2020-05-01 13:13:29 +1000 |
commit | 59d2de956ed29aa5565ed5e5947a7abdb27ac013 (patch) | |
tree | 983def72c15590c50a0319c8048b9a7d56961edf | |
parent | c9d10dbc0ccfb1c7568bbb784f7aeb7a0b5ded12 (diff) |
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
-rw-r--r-- | PROTOCOL.u2f | 2 | ||||
-rw-r--r-- | sk-api.h | 4 | ||||
-rw-r--r-- | sk-usbhid.c | 35 | ||||
-rw-r--r-- | 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. | |||
236 | 236 | ||
237 | The middleware library need only expose a handful of functions: | 237 | The middleware library need only expose a handful of functions: |
238 | 238 | ||
239 | #define SSH_SK_VERSION_MAJOR 0x00040000 /* API version */ | 239 | #define SSH_SK_VERSION_MAJOR 0x00050000 /* API version */ |
240 | #define SSH_SK_VERSION_MAJOR_MASK 0xffff0000 | 240 | #define SSH_SK_VERSION_MAJOR_MASK 0xffff0000 |
241 | 241 | ||
242 | /* Flags */ | 242 | /* Flags */ |
@@ -1,4 +1,4 @@ | |||
1 | /* $OpenBSD: sk-api.h,v 1.8 2020/01/25 23:13:09 djm Exp $ */ | 1 | /* $OpenBSD: sk-api.h,v 1.9 2020/04/28 04:02:29 djm Exp $ */ |
2 | /* | 2 | /* |
3 | * Copyright (c) 2019 Google LLC | 3 | * Copyright (c) 2019 Google LLC |
4 | * | 4 | * |
@@ -71,7 +71,7 @@ struct sk_option { | |||
71 | uint8_t required; | 71 | uint8_t required; |
72 | }; | 72 | }; |
73 | 73 | ||
74 | #define SSH_SK_VERSION_MAJOR 0x00040000 /* current API version */ | 74 | #define SSH_SK_VERSION_MAJOR 0x00050000 /* current API version */ |
75 | #define SSH_SK_VERSION_MAJOR_MASK 0xffff0000 | 75 | #define SSH_SK_VERSION_MAJOR_MASK 0xffff0000 |
76 | 76 | ||
77 | /* Return the version of the middleware API */ | 77 | /* 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 @@ | |||
24 | #include <stdio.h> | 24 | #include <stdio.h> |
25 | #include <stddef.h> | 25 | #include <stddef.h> |
26 | #include <stdarg.h> | 26 | #include <stdarg.h> |
27 | #include <sha2.h> | ||
27 | 28 | ||
28 | #ifdef WITH_OPENSSL | 29 | #ifdef WITH_OPENSSL |
29 | #include <openssl/opensslv.h> | 30 | #include <openssl/opensslv.h> |
@@ -31,6 +32,7 @@ | |||
31 | #include <openssl/bn.h> | 32 | #include <openssl/bn.h> |
32 | #include <openssl/ec.h> | 33 | #include <openssl/ec.h> |
33 | #include <openssl/ecdsa.h> | 34 | #include <openssl/ecdsa.h> |
35 | #include <openssl/evp.h> | ||
34 | #endif /* WITH_OPENSSL */ | 36 | #endif /* WITH_OPENSSL */ |
35 | 37 | ||
36 | #include <fido.h> | 38 | #include <fido.h> |
@@ -710,8 +712,28 @@ check_sign_load_resident_options(struct sk_option **options, char **devicep) | |||
710 | return 0; | 712 | return 0; |
711 | } | 713 | } |
712 | 714 | ||
715 | /* Calculate SHA256(m) */ | ||
716 | static int | ||
717 | sha256_mem(const void *m, size_t mlen, u_char *d, size_t dlen) | ||
718 | { | ||
719 | #ifdef WITH_OPENSSL | ||
720 | u_int mdlen; | ||
721 | #endif | ||
722 | |||
723 | if (dlen != 32) | ||
724 | return -1; | ||
725 | #ifdef WITH_OPENSSL | ||
726 | mdlen = dlen; | ||
727 | if (!EVP_Digest(m, mlen, d, &mdlen, EVP_sha256(), NULL)) | ||
728 | return -1; | ||
729 | #else | ||
730 | SHA256Data(m, mlen, d); | ||
731 | #endif | ||
732 | return 0; | ||
733 | } | ||
734 | |||
713 | int | 735 | int |
714 | sk_sign(uint32_t alg, const uint8_t *message, size_t message_len, | 736 | sk_sign(uint32_t alg, const uint8_t *data, size_t datalen, |
715 | const char *application, | 737 | const char *application, |
716 | const uint8_t *key_handle, size_t key_handle_len, | 738 | const uint8_t *key_handle, size_t key_handle_len, |
717 | uint8_t flags, const char *pin, struct sk_option **options, | 739 | 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, | |||
721 | char *device = NULL; | 743 | char *device = NULL; |
722 | fido_dev_t *dev = NULL; | 744 | fido_dev_t *dev = NULL; |
723 | struct sk_sign_response *response = NULL; | 745 | struct sk_sign_response *response = NULL; |
746 | uint8_t message[32]; | ||
724 | int ret = SSH_SK_ERR_GENERAL; | 747 | int ret = SSH_SK_ERR_GENERAL; |
725 | int r; | 748 | int r; |
726 | 749 | ||
@@ -735,7 +758,12 @@ sk_sign(uint32_t alg, const uint8_t *message, size_t message_len, | |||
735 | *sign_response = NULL; | 758 | *sign_response = NULL; |
736 | if (check_sign_load_resident_options(options, &device) != 0) | 759 | if (check_sign_load_resident_options(options, &device) != 0) |
737 | goto out; /* error already logged */ | 760 | goto out; /* error already logged */ |
738 | if ((dev = find_device(device, message, message_len, | 761 | /* hash data to be signed before it goes to the security key */ |
762 | if ((r = sha256_mem(data, datalen, message, sizeof(message))) != 0) { | ||
763 | skdebug(__func__, "hash message failed"); | ||
764 | goto out; | ||
765 | } | ||
766 | if ((dev = find_device(device, message, sizeof(message), | ||
739 | application, key_handle, key_handle_len)) == NULL) { | 767 | application, key_handle, key_handle_len)) == NULL) { |
740 | skdebug(__func__, "couldn't find device for key handle"); | 768 | skdebug(__func__, "couldn't find device for key handle"); |
741 | goto out; | 769 | goto out; |
@@ -745,7 +773,7 @@ sk_sign(uint32_t alg, const uint8_t *message, size_t message_len, | |||
745 | goto out; | 773 | goto out; |
746 | } | 774 | } |
747 | if ((r = fido_assert_set_clientdata_hash(assert, message, | 775 | if ((r = fido_assert_set_clientdata_hash(assert, message, |
748 | message_len)) != FIDO_OK) { | 776 | sizeof(message))) != FIDO_OK) { |
749 | skdebug(__func__, "fido_assert_set_clientdata_hash: %s", | 777 | skdebug(__func__, "fido_assert_set_clientdata_hash: %s", |
750 | fido_strerr(r)); | 778 | fido_strerr(r)); |
751 | goto out; | 779 | goto out; |
@@ -783,6 +811,7 @@ sk_sign(uint32_t alg, const uint8_t *message, size_t message_len, | |||
783 | response = NULL; | 811 | response = NULL; |
784 | ret = 0; | 812 | ret = 0; |
785 | out: | 813 | out: |
814 | explicit_bzero(message, sizeof(message)); | ||
786 | free(device); | 815 | free(device); |
787 | if (response != NULL) { | 816 | if (response != NULL) { |
788 | free(response->sig_r); | 817 | free(response->sig_r); |
@@ -1,4 +1,4 @@ | |||
1 | /* $OpenBSD: ssh-sk.c,v 1.29 2020/03/06 18:25:48 markus Exp $ */ | 1 | /* $OpenBSD: ssh-sk.c,v 1.30 2020/04/28 04:02:29 djm Exp $ */ |
2 | /* | 2 | /* |
3 | * Copyright (c) 2019 Google LLC | 3 | * Copyright (c) 2019 Google LLC |
4 | * | 4 | * |
@@ -615,7 +615,6 @@ sshsk_sign(const char *provider_path, struct sshkey *key, | |||
615 | int type, alg; | 615 | int type, alg; |
616 | struct sk_sign_response *resp = NULL; | 616 | struct sk_sign_response *resp = NULL; |
617 | struct sshbuf *inner_sig = NULL, *sig = NULL; | 617 | struct sshbuf *inner_sig = NULL, *sig = NULL; |
618 | uint8_t message[32]; | ||
619 | struct sk_option **opts = NULL; | 618 | struct sk_option **opts = NULL; |
620 | 619 | ||
621 | debug("%s: provider \"%s\", key %s, flags 0x%02x%s", __func__, | 620 | debug("%s: provider \"%s\", key %s, flags 0x%02x%s", __func__, |
@@ -650,15 +649,7 @@ sshsk_sign(const char *provider_path, struct sshkey *key, | |||
650 | goto out; | 649 | goto out; |
651 | } | 650 | } |
652 | 651 | ||
653 | /* hash data to be signed before it goes to the security key */ | 652 | if ((r = skp->sk_sign(alg, data, datalen, key->sk_application, |
654 | if ((r = ssh_digest_memory(SSH_DIGEST_SHA256, data, datalen, | ||
655 | message, sizeof(message))) != 0) { | ||
656 | error("%s: hash application failed: %s", __func__, ssh_err(r)); | ||
657 | r = SSH_ERR_INTERNAL_ERROR; | ||
658 | goto out; | ||
659 | } | ||
660 | if ((r = skp->sk_sign(alg, message, sizeof(message), | ||
661 | key->sk_application, | ||
662 | sshbuf_ptr(key->sk_key_handle), sshbuf_len(key->sk_key_handle), | 653 | sshbuf_ptr(key->sk_key_handle), sshbuf_len(key->sk_key_handle), |
663 | key->sk_flags, pin, opts, &resp)) != 0) { | 654 | key->sk_flags, pin, opts, &resp)) != 0) { |
664 | debug("%s: sk_sign failed with code %d", __func__, r); | 655 | debug("%s: sk_sign failed with code %d", __func__, r); |
@@ -707,7 +698,6 @@ sshsk_sign(const char *provider_path, struct sshkey *key, | |||
707 | r = 0; | 698 | r = 0; |
708 | out: | 699 | out: |
709 | sshsk_free_options(opts); | 700 | sshsk_free_options(opts); |
710 | explicit_bzero(message, sizeof(message)); | ||
711 | sshsk_free(skp); | 701 | sshsk_free(skp); |
712 | sshsk_free_sign_response(resp); | 702 | sshsk_free_sign_response(resp); |
713 | sshbuf_free(sig); | 703 | sshbuf_free(sig); |