From a34fcedba9297dcb3c76f329c5b2d6a4908c0284 Mon Sep 17 00:00:00 2001 From: Aveen Ismail Date: Tue, 19 Mar 2024 22:57:11 +0100 Subject: [PATCH] PKCS11: Fix ECDH derivation with KDF --- pkcs11/tests/ecdh_sp800_test.c | 116 ++++++++++++++++++++++----------- pkcs11/util_pkcs11.c | 76 +++++++++++++++++++++ pkcs11/util_pkcs11.h | 7 +- pkcs11/yubihsm_pkcs11.c | 46 ++++--------- 4 files changed, 172 insertions(+), 73 deletions(-) diff --git a/pkcs11/tests/ecdh_sp800_test.c b/pkcs11/tests/ecdh_sp800_test.c index a322f810..3e46aad3 100644 --- a/pkcs11/tests/ecdh_sp800_test.c +++ b/pkcs11/tests/ecdh_sp800_test.c @@ -21,6 +21,7 @@ #include #include #include +#include #include #include @@ -194,6 +195,42 @@ static bool yh_derive_ecdh(CK_OBJECT_HANDLE priv_key, EVP_PKEY *peer_keypair, return true; } +static size_t do_hash(const EVP_MD *md, uint8_t *hashed, + unsigned char *raw_derived, size_t raw_derived_len) { + + EVP_MD_CTX *mdctx = NULL; + size_t len = 0; + + mdctx = EVP_MD_CTX_create(); + if (mdctx == NULL) { + fail("Failed to create Hash context"); + return 0; + } + + if (EVP_DigestInit_ex(mdctx, md, NULL) == 0) { + fail("Failed to initialize digest"); + len = 0; + goto h_free; + } + + if (EVP_DigestUpdate(mdctx, raw_derived, raw_derived_len) != 1) { + fail("Failed to update digest"); + len = 0; + goto h_free; + } + if (EVP_DigestFinal_ex(mdctx, hashed, (unsigned int *) &len) != 1) { + fail("Failed to finalize digest"); + len = 0; + goto h_free; + } + +h_free: + if (mdctx != NULL) { + EVP_MD_CTX_destroy(mdctx); + } + return len; +} + static size_t openssl_derive(CK_ULONG kdf, EVP_PKEY *private_key, EVP_PKEY *peer_key, unsigned char **ecdh_key, CK_ULONG expected_ecdh_len) { @@ -241,68 +278,73 @@ static size_t openssl_derive(CK_ULONG kdf, EVP_PKEY *private_key, goto c_free; } + *ecdh_key = malloc(BUFSIZE); + if (*ecdh_key == NULL) { + fail("Failed to allocate the buffer to hold the ECDH key derived with " + "openssl"); + len = 0; + goto c_free; + } + + size_t output_bits = 0; const EVP_MD *md; switch (kdf) { case CKD_NULL: - *ecdh_key = malloc(len); - if (*ecdh_key == NULL) { - fail("Failed to allocate the buffer to hold the ECDH key derived with " - "openssl"); - len = 0; - goto c_free; - } memcpy(*ecdh_key, derived, len); goto c_truncate; case CKD_YUBICO_SHA1_KDF_SP800: md = EVP_sha1(); + output_bits = 160; break; case CKD_YUBICO_SHA256_KDF_SP800: md = EVP_sha256(); + output_bits = 256; break; case CKD_YUBICO_SHA384_KDF_SP800: md = EVP_sha384(); + output_bits = 384; break; case CKD_YUBICO_SHA512_KDF_SP800: md = EVP_sha512(); + output_bits = 384; break; - default: - fail("Unsupported KDF"); - len = 0; - goto c_free; } - mdctx = EVP_MD_CTX_create(); - if (mdctx == NULL) { - fail("Failed to create Hash context"); - len = 0; - goto c_free; - } + size_t l = expected_ecdh_len * 8; + size_t reps = ceil((float) l / output_bits); - if (EVP_DigestInit_ex(mdctx, md, NULL) == 0) { - fail("Failed to initialize digest"); - len = 0; - goto c_free; - } + uint8_t res[BUFSIZE] = {0}; + size_t res_len = 0; + size_t k_len = len + 4; + uint8_t *k = malloc(k_len); + memset(k, 0, 4); + memcpy(k + 4, derived, len); - *ecdh_key = malloc(BUFSIZE); - if (*ecdh_key == NULL) { - fail("Failed to allocate the buffer to hold the ECDH key derived with " - "openssl"); - len = 0; - goto c_free; - } + size_t hashed_len = 0; + uint32_t counter = 0; + for (size_t i = 0; i < reps; i++) { + counter++; + memcpy(k, &counter, 4); - if (EVP_DigestUpdate(mdctx, derived, len) != 1) { - fail("Failed to update digest"); - len = 0; - goto c_free; + hashed_len = do_hash(md, res + (i * res_len), k, k_len); + if (hashed_len == 0) { + fail("Failed to apply hash function"); + len = 0; + goto c_free; + } + res_len += hashed_len; } - if (EVP_DigestFinal_ex(mdctx, *ecdh_key, (unsigned int *) &len) != 1) { - fail("Failed to finalize digest"); + + if (expected_ecdh_len > res_len) { + fail("Derived key is too short"); len = 0; goto c_free; } + memcpy(*ecdh_key, res, expected_ecdh_len); + memset((*ecdh_key) + expected_ecdh_len, 0, BUFSIZE - expected_ecdh_len); + len = expected_ecdh_len; + c_truncate: if (expected_ecdh_len < len) { size_t offset = len - expected_ecdh_len; @@ -477,9 +519,9 @@ int main(int argc, char **argv) { run_test(handle, CURVES[i], CKD_YUBICO_SHA1_KDF_SP800, 128, yh_pubkey[i], yh_pubkey[i], true); run_test(handle, CURVES[i], CKD_YUBICO_SHA1_KDF_SP800, 192, yh_pubkey[i], - yh_pubkey[i], false); + yh_pubkey[i], true); run_test(handle, CURVES[i], CKD_YUBICO_SHA1_KDF_SP800, 256, yh_pubkey[i], - yh_pubkey[i], false); + yh_pubkey[i], true); run_test(handle, CURVES[i], CKD_YUBICO_SHA256_KDF_SP800, 128, yh_pubkey[i], yh_pubkey[i], true); diff --git a/pkcs11/util_pkcs11.c b/pkcs11/util_pkcs11.c index dfbb42c6..3d2ee147 100644 --- a/pkcs11/util_pkcs11.c +++ b/pkcs11/util_pkcs11.c @@ -20,6 +20,7 @@ #include #include #include +#include #include "../common/platform-config.h" #include "../common/util.h" #include "../common/time_win.h" @@ -5308,3 +5309,78 @@ bool match_meta_attributes(yubihsm_pkcs11_session *session, } return true; } + +size_t ecdh_with_kdf(ecdh_session_key *shared_secret, size_t shared_secret_len, + CK_ULONG kdf, size_t value_len) { + + size_t out_len = 0; + size_t output_bits = 0; + + hash_t hash = _NONE; + switch (kdf) { + case CKD_NULL: + // Do nothing + break; + case CKD_YUBICO_SHA1_KDF_SP800: + hash = _SHA1; + output_bits = 160; + break; + case CKD_YUBICO_SHA256_KDF_SP800: + hash = _SHA256; + output_bits = 256; + break; + case CKD_YUBICO_SHA384_KDF_SP800: + hash = _SHA384; + output_bits = 384; + break; + case CKD_YUBICO_SHA512_KDF_SP800: + hash = _SHA512; + output_bits = 512; + break; + default: + DBG_ERR("Unsupported KDF"); + return 0; + } + + if (hash == _NONE) { + out_len = shared_secret_len; + } else { + size_t l = value_len * 8; + size_t reps = ceil((float) l / output_bits); + if (reps > INT32_MAX) { + DBG_ERR("Too many repetitions"); + return 0; + } + + uint8_t res[1024] = {0}; + size_t k_len = shared_secret_len + 4; + uint8_t *k = malloc(k_len); + memset(k, 0, 4); + memcpy(k + 4, shared_secret->ecdh_key, shared_secret_len); + + size_t hash_len = sizeof(res); + uint32_t counter = 0; + for (size_t i = 0; i < reps; i++) { + counter++; + memcpy(k, &counter, 4); + + if (!hash_bytes(k, k_len, hash, res + (i * out_len), &hash_len)) { + DBG_ERR("Failed to apply hash function"); + return 0; + } + out_len += hash_len; + } + + if (value_len > out_len) { + DBG_ERR("Derived key is too short"); + return 0; + } + + memcpy(shared_secret->ecdh_key, res, value_len); + memset(shared_secret->ecdh_key + value_len, 0, + sizeof(shared_secret->ecdh_key) - value_len); + out_len = value_len; + } + + return out_len; +} diff --git a/pkcs11/util_pkcs11.h b/pkcs11/util_pkcs11.h index 6a0d4701..d8bcd07b 100644 --- a/pkcs11/util_pkcs11.h +++ b/pkcs11/util_pkcs11.h @@ -21,6 +21,7 @@ #include #include "yubihsm_pkcs11.h" +#include "../common/hash.h" CK_RV set_operation_part(yubihsm_pkcs11_op_info *op_info, yubihsm_pkcs11_part_type part); @@ -122,7 +123,8 @@ bool create_session(yubihsm_pkcs11_slot *slot, CK_FLAGS flags, void release_session(yubihsm_pkcs11_context *ctx, yubihsm_pkcs11_session *session); -CK_RV set_template_attribute(yubihsm_pkcs11_attribute *attribute, CK_BBOOL *value); +CK_RV set_template_attribute(yubihsm_pkcs11_attribute *attribute, + CK_BBOOL *value); CK_RV parse_rsa_template(CK_ATTRIBUTE_PTR pTemplate, CK_ULONG ulCount, yubihsm_pkcs11_object_template *template); CK_RV parse_ec_template(CK_ATTRIBUTE_PTR pTemplate, CK_ULONG ulCount, @@ -185,4 +187,7 @@ CK_RV parse_meta_label_template(yubihsm_pkcs11_object_template *template, pkcs11_meta_object *pkcs11meta, bool public, uint8_t *value, size_t value_len); bool match_byte_array(uint8_t *a, uint16_t a_len, uint8_t *b, uint16_t b_len); + +size_t ecdh_with_kdf(ecdh_session_key *shared_secret, size_t shared_secret_len, + CK_ULONG kdf, size_t value_len); #endif diff --git a/pkcs11/yubihsm_pkcs11.c b/pkcs11/yubihsm_pkcs11.c index 6b859ff7..06589ee6 100644 --- a/pkcs11/yubihsm_pkcs11.c +++ b/pkcs11/yubihsm_pkcs11.c @@ -5685,6 +5685,13 @@ CK_DEFINE_FUNCTION(CK_RV, C_DeriveKey) ecdh_session_key ecdh_key = {0}; size_t out_len = sizeof(ecdh_key.ecdh_key); + + if (value_len > out_len) { + DBG_ERR("Requested derived key is too long"); + rv = CKR_ATTRIBUTE_VALUE_INVALID; + goto c_drv_out; + } + yh_rc rc = yh_util_derive_ecdh(session->slot->device_session, privkey_id, pubkey, in_len, ecdh_key.ecdh_key, &out_len); if (rc != YHR_SUCCESS) { @@ -5693,41 +5700,10 @@ CK_DEFINE_FUNCTION(CK_RV, C_DeriveKey) goto c_drv_out; } - hash_t hash = _NONE; - switch (params->kdf) { - case CKD_NULL: - // Do nothing - break; - case CKD_YUBICO_SHA1_KDF_SP800: - hash = _SHA1; - break; - case CKD_YUBICO_SHA256_KDF_SP800: - hash = _SHA256; - break; - case CKD_YUBICO_SHA384_KDF_SP800: - hash = _SHA384; - break; - case CKD_YUBICO_SHA512_KDF_SP800: - hash = _SHA512; - break; - default: - DBG_ERR("Unsupported KDF"); - rv = CKR_FUNCTION_NOT_SUPPORTED; - goto c_drv_out; - } - - if (hash != _NONE) { - size_t dh_len = out_len; - out_len = sizeof(ecdh_key.ecdh_key); - if (!hash_bytes(ecdh_key.ecdh_key, dh_len, hash, ecdh_key.ecdh_key, - &out_len)) { - DBG_ERR("Failed to apply hash function"); - goto c_drv_out; - } - if (dh_len > out_len) { - // Wipe any remaining bytes of the dh secret - memset(ecdh_key.ecdh_key + out_len, 0, dh_len - out_len); - } + out_len = ecdh_with_kdf(&ecdh_key, out_len, params->kdf, value_len); + if (out_len == 0) { + DBG_ERR("Failed to derive ECDH key with KDF"); + goto c_drv_out; } if (value_len > 0) {