Add ECDH_compute_key_fips inside the module.

This change adds a function so that an ECDH and the hashing of the
resulting 'x' coordinate can occur inside the FIPS boundary.

Change-Id: If93c20a70dc9dcbca49056f10915d3ce064f641f
Reviewed-on: https://boringssl-review.googlesource.com/30104
Reviewed-by: Adam Langley <agl@google.com>

crypto/CMakeLists.txt

@@ -109,7 +109,7 @@ add_subdirectory(dh)
 add_subdirectory(dsa)
 add_subdirectory(rsa_extra)
 add_subdirectory(ec_extra)
-add_subdirectory(ecdh)
+add_subdirectory(ecdh_extra)
 add_subdirectory(ecdsa_extra)
 
 # Level 3
@@ -192,7 +192,7 @@ add_library(
   $<TARGET_OBJECTS:dsa>
   $<TARGET_OBJECTS:rsa_extra>
   $<TARGET_OBJECTS:ec_extra>
-  $<TARGET_OBJECTS:ecdh>
+  $<TARGET_OBJECTS:ecdh_extra>
   $<TARGET_OBJECTS:ecdsa_extra>
   $<TARGET_OBJECTS:cmac>
   $<TARGET_OBJECTS:evp>
@@ -234,7 +234,7 @@ add_executable(
   curve25519/ed25519_test.cc
   curve25519/spake25519_test.cc
   curve25519/x25519_test.cc
-  ecdh/ecdh_test.cc
+  ecdh_extra/ecdh_test.cc
   dh/dh_test.cc
   digest_extra/digest_test.cc
   dsa/dsa_test.cc

crypto/ecdh/CMakeLists.txt → crypto/ecdh_extra/CMakeLists.txt

@@ -1,9 +1,9 @@
 include_directories(../../include)
 
 add_library(
-  ecdh
+  ecdh_extra
 
   OBJECT
 
-  ecdh.c
+  ecdh_extra.c
 )

crypto/ecdh/ecdh_test.cc → crypto/ecdh_extra/ecdh_test.cc

@@ -28,6 +28,7 @@
 #include <openssl/err.h>
 #include <openssl/evp.h>
 #include <openssl/nid.h>
+#include <openssl/sha.h>
 
 #include "../test/file_test.h"
 #include "../test/test_util.h"
@@ -68,7 +69,7 @@ static bssl::UniquePtr<BIGNUM> GetBIGNUM(FileTest *t, const char *key) {
 }
 
 TEST(ECDHTest, TestVectors) {
-  FileTestGTest("crypto/ecdh/ecdh_tests.txt", [](FileTest *t) {
+  FileTestGTest("crypto/ecdh_extra/ecdh_tests.txt", [](FileTest *t) {
     bssl::UniquePtr<EC_GROUP> group = GetCurve(t, "Curve");
     ASSERT_TRUE(group);
     bssl::UniquePtr<BIGNUM> priv_key = GetBIGNUM(t, "Private");
@@ -115,6 +116,13 @@ TEST(ECDHTest, TestVectors) {
     ASSERT_GE(ret, 0);
     EXPECT_EQ(Bytes(z.data(), z.size() - 1),
               Bytes(actual_z.data(), static_cast<size_t>(ret)));
+
+    // Test that |ECDH_compute_key_fips| hashes as expected.
+    uint8_t digest[SHA256_DIGEST_LENGTH], expected_digest[SHA256_DIGEST_LENGTH];
+    ASSERT_TRUE(ECDH_compute_key_fips(digest, sizeof(digest),
+                                      peer_pub_key.get(), key.get()));
+    SHA256(z.data(), z.size(), expected_digest);
+    EXPECT_EQ(Bytes(digest), Bytes(expected_digest));
   });
 }
 

crypto/err/ecdh.errordata

@@ -1,3 +1,4 @@
 ECDH,100,KDF_FAILED
 ECDH,101,NO_PRIVATE_VALUE
 ECDH,102,POINT_ARITHMETIC_FAILURE
+ECDH,103,UNKNOWN_DIGEST_LENGTH

crypto/fipsmodule/bcm.c

@@ -57,6 +57,7 @@
 #include "des/des.c"
 #include "digest/digest.c"
 #include "digest/digests.c"
+#include "ecdh/ecdh.c"
 #include "ecdsa/ecdsa.c"
 #include "ec/ec.c"
 #include "ec/ec_key.c"

crypto/fipsmodule/ecdh/ecdh.c

@@ -0,0 +1,161 @@
+/* ====================================================================
+ * Copyright 2002 Sun Microsystems, Inc. ALL RIGHTS RESERVED.
+ *
+ * The Elliptic Curve Public-Key Crypto Library (ECC Code) included
+ * herein is developed by SUN MICROSYSTEMS, INC., and is contributed
+ * to the OpenSSL project.
+ *
+ * The ECC Code is licensed pursuant to the OpenSSL open source
+ * license provided below.
+ *
+ * The ECDH software is originally written by Douglas Stebila of
+ * Sun Microsystems Laboratories.
+ *
+ */
+/* ====================================================================
+ * Copyright (c) 2000-2002 The OpenSSL Project.  All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ *
+ * 3. All advertising materials mentioning features or use of this
+ *    software must display the following acknowledgment:
+ *    "This product includes software developed by the OpenSSL Project
+ *    for use in the OpenSSL Toolkit. (http://www.OpenSSL.org/)"
+ *
+ * 4. The names "OpenSSL Toolkit" and "OpenSSL Project" must not be used to
+ *    endorse or promote products derived from this software without
+ *    prior written permission. For written permission, please contact
+ *    licensing@OpenSSL.org.
+ *
+ * 5. Products derived from this software may not be called "OpenSSL"
+ *    nor may "OpenSSL" appear in their names without prior written
+ *    permission of the OpenSSL Project.
+ *
+ * 6. Redistributions of any form whatsoever must retain the following
+ *    acknowledgment:
+ *    "This product includes software developed by the OpenSSL Project
+ *    for use in the OpenSSL Toolkit (http://www.OpenSSL.org/)"
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE OpenSSL PROJECT ``AS IS'' AND ANY
+ * EXPRESSED OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL THE OpenSSL PROJECT OR
+ * ITS CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
+ * OF THE POSSIBILITY OF SUCH DAMAGE.
+ * ====================================================================
+ *
+ * This product includes cryptographic software written by Eric Young
+ * (eay@cryptsoft.com).  This product includes software written by Tim
+ * Hudson (tjh@cryptsoft.com). */
+
+#include <openssl/ecdh.h>
+
+#include <limits.h>
+#include <string.h>
+
+#include <openssl/bn.h>
+#include <openssl/ec.h>
+#include <openssl/ec_key.h>
+#include <openssl/err.h>
+#include <openssl/mem.h>
+#include <openssl/sha.h>
+
+#include "../ec/internal.h"
+
+
+int ECDH_compute_key_fips(uint8_t *out, size_t out_len, const EC_POINT *pub_key,
+                          const EC_KEY *priv_key) {
+  if (priv_key->priv_key == NULL) {
+    OPENSSL_PUT_ERROR(ECDH, ECDH_R_NO_PRIVATE_VALUE);
+    return 0;
+  }
+  const EC_SCALAR *const priv = &priv_key->priv_key->scalar;
+
+  BN_CTX *ctx = BN_CTX_new();
+  if (ctx == NULL) {
+    return 0;
+  }
+  BN_CTX_start(ctx);
+
+  int ret = 0;
+  size_t buflen = 0;
+  uint8_t *buf = NULL;
+
+  const EC_GROUP *const group = EC_KEY_get0_group(priv_key);
+  EC_POINT *shared_point = EC_POINT_new(group);
+  if (shared_point == NULL) {
+    OPENSSL_PUT_ERROR(ECDH, ERR_R_MALLOC_FAILURE);
+    goto err;
+  }
+
+  if (!ec_point_mul_scalar(group, shared_point, NULL, pub_key, priv, ctx)) {
+    OPENSSL_PUT_ERROR(ECDH, ECDH_R_POINT_ARITHMETIC_FAILURE);
+    goto err;
+  }
+
+  BIGNUM *x = BN_CTX_get(ctx);
+  if (!x) {
+    OPENSSL_PUT_ERROR(ECDH, ERR_R_MALLOC_FAILURE);
+    goto err;
+  }
+
+  if (!EC_POINT_get_affine_coordinates_GFp(group, shared_point, x, NULL, ctx)) {
+    OPENSSL_PUT_ERROR(ECDH, ECDH_R_POINT_ARITHMETIC_FAILURE);
+    goto err;
+  }
+
+  buflen = (EC_GROUP_get_degree(group) + 7) / 8;
+  buf = OPENSSL_malloc(buflen);
+  if (buf == NULL) {
+    OPENSSL_PUT_ERROR(ECDH, ERR_R_MALLOC_FAILURE);
+    goto err;
+  }
+
+  if (!BN_bn2bin_padded(buf, buflen, x)) {
+    OPENSSL_PUT_ERROR(ECDH, ERR_R_INTERNAL_ERROR);
+    goto err;
+  }
+
+  switch (out_len) {
+    case SHA224_DIGEST_LENGTH:
+      SHA224(buf, buflen, out);
+      break;
+    case SHA256_DIGEST_LENGTH:
+      SHA256(buf, buflen, out);
+      break;
+    case SHA384_DIGEST_LENGTH:
+      SHA384(buf, buflen, out);
+      break;
+    case SHA512_DIGEST_LENGTH:
+      SHA512(buf, buflen, out);
+      break;
+    default:
+      OPENSSL_PUT_ERROR(ECDH, ECDH_R_UNKNOWN_DIGEST_LENGTH);
+      goto err;
+  }
+
+  ret = 1;
+
+err:
+  OPENSSL_free(buf);
+  EC_POINT_free(shared_point);
+  BN_CTX_end(ctx);
+  BN_CTX_free(ctx);
+  return ret;
+}

fipstools/cavp_kas_test.cc

@@ -25,6 +25,7 @@
 #include <openssl/ec_key.h>
 #include <openssl/err.h>
 #include <openssl/nid.h>
+#include <openssl/sha.h>
 
 #include "../crypto/internal.h"
 #include "../crypto/test/file_test.h"
@@ -35,20 +36,20 @@ static bool TestKAS(FileTest *t, void *arg) {
   const bool validate = *reinterpret_cast<bool *>(arg);
 
   int nid = NID_undef;
-  const EVP_MD *md = nullptr;
+  size_t digest_len = 0;
 
   if (t->HasInstruction("EB - SHA224")) {
     nid = NID_secp224r1;
-    md = EVP_sha224();
+    digest_len = SHA224_DIGEST_LENGTH;
   } else if (t->HasInstruction("EC - SHA256")) {
     nid = NID_X9_62_prime256v1;
-    md = EVP_sha256();
+    digest_len = SHA256_DIGEST_LENGTH;
   } else if (t->HasInstruction("ED - SHA384")) {
     nid = NID_secp384r1;
-    md = EVP_sha384();
+    digest_len = SHA384_DIGEST_LENGTH;
   } else if (t->HasInstruction("EE - SHA512")) {
     nid = NID_secp521r1;
-    md = EVP_sha512();
+    digest_len = SHA512_DIGEST_LENGTH;
   } else {
     return false;
   }
@@ -86,17 +87,9 @@ static bool TestKAS(FileTest *t, void *arg) {
     return false;
   }
 
-  constexpr size_t kMaxCurveFieldBits = 521;
-  uint8_t shared_bytes[(kMaxCurveFieldBits + 7)/8];
-  const int shared_bytes_len =
-      ECDH_compute_key(shared_bytes, sizeof(shared_bytes), their_point.get(),
-                       ec_key.get(), nullptr);
-
   uint8_t digest[EVP_MAX_MD_SIZE];
-  unsigned digest_len;
-  if (shared_bytes_len < 0 ||
-      !EVP_Digest(shared_bytes, shared_bytes_len, digest, &digest_len, md,
-                  nullptr)) {
+  if (!ECDH_compute_key_fips(digest, digest_len, their_point.get(),
+                             ec_key.get())) {
     return false;
   }
 

include/openssl/ecdh.h

@@ -89,6 +89,22 @@ OPENSSL_EXPORT int ECDH_compute_key(
     void *out, size_t outlen, const EC_POINT *pub_key, const EC_KEY *priv_key,
     void *(*kdf)(const void *in, size_t inlen, void *out, size_t *outlen));
 
+// ECDH_compute_key_fips calculates the shared key between |pub_key| and
+// |priv_key| and hashes it with the appropriate SHA function for |out_len|. The
+// only value values for |out_len| are thus 24 (SHA-224), 32 (SHA-256), 48
+// (SHA-384), and 64 (SHA-512). It returns one on success and zero on error.
+//
+// Note that the return value is different to |ECDH_compute_key|: it returns an
+// error flag (as is common for BoringSSL) rather than the number of bytes
+// written.
+//
+// This function allows the FIPS module to compute an ECDH and KDF within the
+// module boundary without taking an arbitrary function pointer for the KDF,
+// which isn't very FIPSy.
+OPENSSL_EXPORT int ECDH_compute_key_fips(uint8_t *out, size_t out_len,
+                                         const EC_POINT *pub_key,
+                                         const EC_KEY *priv_key);
+
 
 #if defined(__cplusplus)
 }  // extern C
@@ -97,5 +113,6 @@ OPENSSL_EXPORT int ECDH_compute_key(
 #define ECDH_R_KDF_FAILED 100
 #define ECDH_R_NO_PRIVATE_VALUE 101
 #define ECDH_R_POINT_ARITHMETIC_FAILURE 102
+#define ECDH_R_UNKNOWN_DIGEST_LENGTH 103
 
 #endif  // OPENSSL_HEADER_ECDH_H

sources.cmake

@@ -40,7 +40,7 @@ set(
   crypto/cmac/cavp_aes128_cmac_tests.txt
   crypto/cmac/cavp_aes192_cmac_tests.txt
   crypto/cmac/cavp_aes256_cmac_tests.txt
-  crypto/ecdh/ecdh_tests.txt
+  crypto/ecdh_extra/ecdh_tests.txt
   crypto/evp/evp_tests.txt
   crypto/evp/scrypt_tests.txt
   crypto/fipsmodule/aes/aes_tests.txt