PKCS11: Fix ECDH derivation with KDF

pkcs11/tests/ecdh_sp800_test.c

@@ -21,6 +21,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <stdbool.h>
+#include <math.h>
 
 #include <openssl/evp.h>
 #include <openssl/ec.h>
@@ -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);

pkcs11/util_pkcs11.c

@@ -20,6 +20,7 @@
 #include <string.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <math.h>
 #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;
+}

pkcs11/util_pkcs11.h

@@ -21,6 +21,7 @@
 #include <pkcs11y.h>
 
 #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

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) {