Add an ECDH Wycheproof driver.

Unfortunately, this driver suffers a lot from Wycheproof's Java
heritgate, but so it goes. Their test formats bake in a lot of Java API
mistakes.

Change-Id: I3299e85efb58e99e4fa34841709c3bea6518968d
Reviewed-on: https://boringssl-review.googlesource.com/27865
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>

crypto/ecdh/ecdh_test.cc

@@ -20,15 +20,18 @@
 #include <gtest/gtest.h>
 
 #include <openssl/bn.h>
+#include <openssl/bytestring.h>
 #include <openssl/crypto.h>
 #include <openssl/ec.h>
 #include <openssl/ec_key.h>
 #include <openssl/ecdh.h>
 #include <openssl/err.h>
+#include <openssl/evp.h>
 #include <openssl/nid.h>
 
 #include "../test/file_test.h"
 #include "../test/test_util.h"
+#include "../test/wycheproof_util.h"
 
 
 static bssl::UniquePtr<EC_GROUP> GetCurve(FileTest *t, const char *key) {
@@ -115,6 +118,60 @@ TEST(ECDHTest, TestVectors) {
   });
 }
 
+TEST(ECDHTest, Wycheproof) {
+  FileTestGTest("third_party/wycheproof/ecdh_test.txt", [](FileTest *t) {
+    t->IgnoreInstruction("curve");  // This is redundant with the per-test one.
+
+    bssl::UniquePtr<EC_GROUP> group = GetWycheproofCurve(t, "curve", false);
+    ASSERT_TRUE(group);
+    bssl::UniquePtr<BIGNUM> priv_key = GetWycheproofBIGNUM(t, "private", false);
+    ASSERT_TRUE(priv_key);
+    std::vector<uint8_t> peer_spki;
+    ASSERT_TRUE(t->GetBytes(&peer_spki, "public"));
+    WycheproofResult result;
+    ASSERT_TRUE(GetWycheproofResult(t, &result));
+    std::vector<uint8_t> shared;
+    ASSERT_TRUE(t->GetBytes(&shared, "shared"));
+
+    // Wycheproof stores the peer key in an SPKI to mimic a Java API mistake.
+    // This is non-standard and error-prone.
+    CBS cbs;
+    CBS_init(&cbs, peer_spki.data(), peer_spki.size());
+    bssl::UniquePtr<EVP_PKEY> peer_evp(EVP_parse_public_key(&cbs));
+    if (!peer_evp) {
+      // Note some of Wycheproof's "acceptable" entries are unsupported by
+      // BoringSSL because they test named curves (explicitly forbidden by RFC
+      // 5480), while others are supported because they used compressed
+      // coordinates. If the peer key fails to parse, we consider it to match
+      // "acceptable", but if the resulting shared secret matches below, it too
+      // matches "acceptable".
+      //
+      // TODO(davidben): Use the flags field to disambiguate these. Possibly
+      // first get the Wycheproof folks to use flags more consistently.
+      EXPECT_NE(WycheproofResult::kValid, result);
+      return;
+    }
+    EC_KEY *peer_ec = EVP_PKEY_get0_EC_KEY(peer_evp.get());
+    ASSERT_TRUE(peer_ec);
+
+    bssl::UniquePtr<EC_KEY> key(EC_KEY_new());
+    ASSERT_TRUE(key);
+    ASSERT_TRUE(EC_KEY_set_group(key.get(), group.get()));
+    ASSERT_TRUE(EC_KEY_set_private_key(key.get(), priv_key.get()));
+
+    std::vector<uint8_t> actual((EC_GROUP_get_degree(group.get()) + 7) / 8);
+    int ret =
+        ECDH_compute_key(actual.data(), actual.size(),
+                         EC_KEY_get0_public_key(peer_ec), key.get(), nullptr);
+    if (result == WycheproofResult::kInvalid) {
+      EXPECT_EQ(-1, ret);
+    } else {
+      EXPECT_EQ(static_cast<int>(actual.size()), ret);
+      EXPECT_EQ(Bytes(shared), Bytes(actual.data(), static_cast<size_t>(ret)));
+    }
+  });
+}
+
 // MakeCustomGroup returns an |EC_GROUP| containing a non-standard group. (P-256
 // with the wrong generator.)
 static bssl::UniquePtr<EC_GROUP> MakeCustomGroup() {

crypto/test/wycheproof_util.cc

@@ -14,6 +14,7 @@
 
 #include "./wycheproof_util.h"
 
+#include <openssl/bn.h>
 #include <openssl/digest.h>
 #include <openssl/ec.h>
 #include <openssl/nid.h>
@@ -89,3 +90,35 @@ bssl::UniquePtr<EC_GROUP> GetWycheproofCurve(FileTest *t, const char *key,
   }
   return bssl::UniquePtr<EC_GROUP>(EC_GROUP_new_by_curve_name(nid));
 }
+
+bssl::UniquePtr<BIGNUM> GetWycheproofBIGNUM(FileTest *t, const char *key,
+                                            bool instruction) {
+  std::string value;
+  bool ok = instruction ? t->GetInstruction(&value, key)
+                        : t->GetAttribute(&value, key);
+  if (!ok) {
+    return nullptr;
+  }
+  BIGNUM *bn = nullptr;
+  if (BN_hex2bn(&bn, value.c_str()) != static_cast<int>(value.size())) {
+    BN_free(bn);
+    t->PrintLine("Could not decode value '%s'", value.c_str());
+    return nullptr;
+  }
+  bssl::UniquePtr<BIGNUM> ret(bn);
+  if (!value.empty()) {
+    // If the high bit is one, this is a negative number in Wycheproof.
+    // Wycheproof's tests generally mimic Java APIs, including all their
+    // mistakes. See
+    // https://github.com/google/wycheproof/blob/0329f5b751ef102bd6b7b7181b6e049522a887f5/java/com/google/security/wycheproof/JsonUtil.java#L62.
+    if ('0' > value[0] || value[0] > '7') {
+      bssl::UniquePtr<BIGNUM> tmp(BN_new());
+      if (!tmp ||
+          !BN_set_bit(tmp.get(), value.size() * 4) ||
+          !BN_sub(ret.get(), ret.get(), tmp.get())) {
+        return nullptr;
+      }
+    }
+  }
+  return ret;
+}

crypto/test/wycheproof_util.h

@@ -41,5 +41,10 @@ const EVP_MD *GetWycheproofDigest(FileTest *t, const char *key,
 bssl::UniquePtr<EC_GROUP> GetWycheproofCurve(FileTest *t, const char *key,
                                              bool instruction);
 
+// GetWycheproofBIGNUM returns a BIGNUM in the Wycheproof format, or nullptr on
+// error.
+bssl::UniquePtr<BIGNUM> GetWycheproofBIGNUM(FileTest *t, const char *key,
+                                            bool instruction);
+
 
 #endif  // OPENSSL_HEADER_CRYPTO_TEST_WYCHEPROOF_UTIL_H

sources.cmake

@@ -63,6 +63,7 @@ set(
   third_party/wycheproof/aes_gcm_test.txt
   third_party/wycheproof/chacha20_poly1305_test.txt
   third_party/wycheproof/dsa_test.txt
+  third_party/wycheproof/ecdh_test.txt
   third_party/wycheproof/ecdsa_secp224r1_sha224_test.txt
   third_party/wycheproof/ecdsa_secp224r1_sha256_test.txt
   third_party/wycheproof/ecdsa_secp256r1_sha256_test.txt

third_party/wycheproof/convert_wycheproof.go

@@ -231,6 +231,7 @@ func main() {
 		"aes_gcm_test.json",
 		"chacha20_poly1305_test.json",
 		"dsa_test.json",
+		"ecdh_test.json",
 		"ecdsa_secp224r1_sha224_test.json",
 		"ecdsa_secp224r1_sha256_test.json",
 		"ecdsa_secp256r1_sha256_test.json",
@@ -243,7 +244,6 @@ func main() {
 
 		// TODO(davidben): The following tests still need test drivers.
 		// "aes_cbc_pkcs5_test.json",
-		// "ecdh_test.json",
 	}
 	for _, jsonPath := range jsonPaths {
 		if !strings.HasSuffix(jsonPath, ".json") {