Fix the build with FIPS + NO_ASM.

Setting OPENSSL_NO_ASM skips enabling the “ASM” language in CMake.
However, the FIPS module fundamentally needs to build asm because
delocate works via textual assembly. Thus this combination is currently
broken with CMake.

This change ensures that support for building asm is enabled in CMake
for this combination.

Change-Id: I4516cf3a6f579ee7c72f04ac25d15785926cf125
Reviewed-on: https://boringssl-review.googlesource.com/29884
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_extra/CMakeLists.txt

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

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/CMakeLists.txt

@@ -121,6 +121,13 @@ perlasm(x86_64-mont.${ASM_EXT} bn/asm/x86_64-mont.pl)
 perlasm(x86-mont.${ASM_EXT} bn/asm/x86-mont.pl)
 
 if(FIPS_DELOCATE)
+  if(OPENSSL_NO_ASM)
+    # If OPENSSL_NO_ASM was defined then ASM will not have been enabled, but in
+    # FIPS mode we have to have it because the module build requires going via
+    # textual assembly.
+    enable_language(ASM)
+  endif()
+
   add_library(
     bcm_c_generated_asm
 

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

util/fipstools/break-tests.sh

@@ -0,0 +1,53 @@
+# Copyright (c) 2018, Google Inc.
+#
+# Permission to use, copy, modify, and/or distribute this software for any
+# purpose with or without fee is hereby granted, provided that the above
+# copyright notice and this permission notice appear in all copies.
+#
+# THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+# WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+# MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY
+# SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+# WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION
+# OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
+# CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */
+
+# This script exists to exercise breaking each of the FIPS tests. It builds
+# BoringSSL differently for each test and that can take a long time. Thus it's
+# run twice: once, from a BoringSSL source tree, with "build" as the sole
+# argument to run the builds, and then (from the same location) with no
+# arguments to run each script.
+#
+# Run it with /bin/bash, not /bin/sh, otherwise "read" may fail.
+
+set -x
+
+TESTS="NONE ECDSA_PWCT CRNG RSA_PWCT AES_CBC AES_GCM DES SHA_1 SHA_256 SHA_512 RSA_SIG DRBG ECDSA_SIG"
+
+if [ "x$1" = "xbuild" ]; then
+	for test in $TESTS; do
+		rm -Rf build-$test
+		mkdir build-$test
+		pushd build-$test
+		cmake -GNinja -DCMAKE_TOOLCHAIN_FILE=${HOME}/toolchain -DFIPS=1 -DFIPS_BREAK_TEST=${test} -DCMAKE_BUILD_TYPE=Release ..
+		ninja test_fips
+		popd
+	done
+
+	exit 0
+fi
+
+for test in $TESTS; do
+	pushd build-$test
+	printf "\n\n\\x1b[1m$test\\x1b[0m\n"
+	./fipstools/test_fips
+	echo "Waiting for keypress..."
+	read
+	popd
+done
+
+pushd build-NONE
+printf "\\x1b[1mIntegrity\\x1b[0m\n"
+go run ../util/fipstools/break-hash.go ./fipstools/test_fips ./fipstools/test_fips_broken
+./fipstools/test_fips_broken
+popd