Use newly-sharded ECDH tests.

Also remove some transition step for a recent format change. Together, this
removes the curve hacks in the converter, which can now be purely syntactic.
The RSA ones are still a bit all over the place in terms of sharded vs
combined, so leaving that alone for now.

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

crypto/ecdh_extra/ecdh_test.cc

@@ -126,60 +126,76 @@ TEST(ECDHTest, TestVectors) {
   });
 }
 
-TEST(ECDHTest, Wycheproof) {
-  FileTestGTest("third_party/wycheproof_testvectors/ecdh_test.txt",
-                [](FileTest *t) {
-    t->IgnoreInstruction("curve");  // This is redundant with the per-test one.
-    t->IgnoreInstruction("encoding");
 
-    bssl::UniquePtr<EC_GROUP> group = GetWycheproofCurve(t, "curve", false);
+static void RunWycheproofTest(FileTest *t) {
-    ASSERT_TRUE(group);
+  t->IgnoreInstruction("encoding");
-    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.
+  bssl::UniquePtr<EC_GROUP> group = GetWycheproofCurve(t, "curve", true);
-    // This is non-standard and error-prone.
+  ASSERT_TRUE(group);
-    CBS cbs;
+  bssl::UniquePtr<BIGNUM> priv_key = GetWycheproofBIGNUM(t, "private", false);
-    CBS_init(&cbs, peer_spki.data(), peer_spki.size());
+  ASSERT_TRUE(priv_key);
-    bssl::UniquePtr<EVP_PKEY> peer_evp(EVP_parse_public_key(&cbs));
+  std::vector<uint8_t> peer_spki;
-    if (!peer_evp) {
+  ASSERT_TRUE(t->GetBytes(&peer_spki, "public"));
-      // Note some of Wycheproof's "acceptable" entries are unsupported by
+  WycheproofResult result;
-      // BoringSSL because they test named curves (explicitly forbidden by RFC
+  ASSERT_TRUE(GetWycheproofResult(t, &result));
-      // 5480), while others are supported because they used compressed
+  std::vector<uint8_t> shared;
-      // coordinates. If the peer key fails to parse, we consider it to match
+  ASSERT_TRUE(t->GetBytes(&shared, "shared"));
-      // "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());
+  // Wycheproof stores the peer key in an SPKI to mimic a Java API mistake.
-    ASSERT_TRUE(key);
+  // This is non-standard and error-prone.
-    ASSERT_TRUE(EC_KEY_set_group(key.get(), group.get()));
+  CBS cbs;
-    ASSERT_TRUE(EC_KEY_set_private_key(key.get(), priv_key.get()));
+  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 explicit curves (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);
 
-    std::vector<uint8_t> actual((EC_GROUP_get_degree(group.get()) + 7) / 8);
+  bssl::UniquePtr<EC_KEY> key(EC_KEY_new());
-    int ret =
+  ASSERT_TRUE(key);
-        ECDH_compute_key(actual.data(), actual.size(),
+  ASSERT_TRUE(EC_KEY_set_group(key.get(), group.get()));
-                         EC_KEY_get0_public_key(peer_ec), key.get(), nullptr);
+  ASSERT_TRUE(EC_KEY_set_private_key(key.get(), priv_key.get()));
-    if (result == WycheproofResult::kInvalid) {
+
-      EXPECT_EQ(-1, ret);
+  std::vector<uint8_t> actual((EC_GROUP_get_degree(group.get()) + 7) / 8);
-    } else {
+  int ret =
-      EXPECT_EQ(static_cast<int>(actual.size()), ret);
+      ECDH_compute_key(actual.data(), actual.size(),
-      EXPECT_EQ(Bytes(shared), Bytes(actual.data(), static_cast<size_t>(ret)));
+                       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)));
+  }
+}
+
+TEST(ECDHTest, WycheproofP224) {
+  FileTestGTest("third_party/wycheproof_testvectors/ecdh_secp224r1_test.txt",
+                RunWycheproofTest);
+}
+
+TEST(ECDHTest, WycheproofP256) {
+  FileTestGTest("third_party/wycheproof_testvectors/ecdh_secp256r1_test.txt",
+                RunWycheproofTest);
+}
+
+TEST(ECDHTest, WycheproofP384) {
+  FileTestGTest("third_party/wycheproof_testvectors/ecdh_secp384r1_test.txt",
+                RunWycheproofTest);
+}
+
+TEST(ECDHTest, WycheproofP512) {
+  FileTestGTest("third_party/wycheproof_testvectors/ecdh_secp521r1_test.txt",
+                RunWycheproofTest);
 }
 
 // MakeCustomGroup returns an |EC_GROUP| containing a non-standard group. (P-256

sources.cmake

@@ -67,7 +67,10 @@ set(
   third_party/wycheproof_testvectors/aes_gcm_test.txt
   third_party/wycheproof_testvectors/chacha20_poly1305_test.txt
   third_party/wycheproof_testvectors/dsa_test.txt
-  third_party/wycheproof_testvectors/ecdh_test.txt
+  third_party/wycheproof_testvectors/ecdh_secp224r1_test.txt
+  third_party/wycheproof_testvectors/ecdh_secp256r1_test.txt
+  third_party/wycheproof_testvectors/ecdh_secp384r1_test.txt
+  third_party/wycheproof_testvectors/ecdh_secp521r1_test.txt
   third_party/wycheproof_testvectors/ecdsa_secp224r1_sha224_test.txt
   third_party/wycheproof_testvectors/ecdsa_secp224r1_sha256_test.txt
   third_party/wycheproof_testvectors/ecdsa_secp256r1_sha256_test.txt

util/convert_wycheproof.go

@@ -121,17 +121,6 @@ func printComment(w io.Writer, in string) error {
 	return nil
 }
 
-func isSupportedCurve(curve string) bool {
-	switch curve {
-	case "brainpoolP224r1", "brainpoolP224t1", "brainpoolP256r1", "brainpoolP256t1", "brainpoolP320r1", "brainpoolP320t1", "brainpoolP384r1", "brainpoolP384t1", "brainpoolP512r1", "brainpoolP512t1", "secp256k1":
-		return false
-	case "edwards25519", "curve25519", "secp224r1", "secp256r1", "secp384r1", "secp521r1":
-		return true
-	default:
-		panic("Unknown curve: " + curve)
-	}
-}
-
 func convertWycheproof(f io.Writer, jsonPath string) error {
 	jsonData, err := ioutil.ReadFile(jsonPath)
 	if err != nil {
@@ -154,21 +143,6 @@ func convertWycheproof(f io.Writer, jsonPath string) error {
 	}
 
 	for _, group := range w.TestGroups {
-		// Skip tests with unsupported curves. We filter these out at
-		// conversion time to avoid unnecessarily inflating
-		// crypto_test_data.cc.
-		groupCurve := group["curve"]
-		if groupCurve != nil && !isSupportedCurve(groupCurve.(string)) {
-			continue
-		}
-		if keyI, ok := group["key"]; ok {
-			if key, ok := keyI.(map[string]interface{}); ok {
-				if curve, ok := key["curve"]; ok && !isSupportedCurve(curve.(string)) {
-					continue
-				}
-			}
-		}
-
 		for _, k := range sortedKeys(group) {
 			// Wycheproof files always include both keyPem and
 			// keyDer. Skip keyPem as they contain newlines. We
@@ -184,12 +158,6 @@ func convertWycheproof(f io.Writer, jsonPath string) error {
 		tests := group["tests"].([]interface{})
 		for _, testI := range tests {
 			test := testI.(map[string]interface{})
-
-			curve := test["curve"]
-			// Skip tests with unsupported curves.
-			if curve != nil && !isSupportedCurve(curve.(string)) {
-				continue
-			}
 			if _, err := fmt.Fprintf(f, "# tcId = %d\n", int(test["tcId"].(float64))); err != nil {
 				return err
 			}
@@ -206,13 +174,6 @@ func convertWycheproof(f io.Writer, jsonPath string) error {
 					return err
 				}
 			}
-			// If the curve was only specified at the group level then copy it into
-			// each test.
-			if curve == nil && groupCurve != nil {
-				if err := printAttribute(f, "curve", groupCurve, false); err != nil {
-					return err
-				}
-			}
 			if flags, ok := test["flags"]; ok {
 				for _, flag := range flags.([]interface{}) {
 					if note, ok := w.Notes[flag.(string)]; ok {
@@ -237,7 +198,10 @@ var defaultInputs = []string{
 	"aes_gcm_test.json",
 	"chacha20_poly1305_test.json",
 	"dsa_test.json",
-	"ecdh_test.json",
+	"ecdh_secp224r1_test.json",
+	"ecdh_secp256r1_test.json",
+	"ecdh_secp384r1_test.json",
+	"ecdh_secp521r1_test.json",
 	"ecdsa_secp224r1_sha224_test.json",
 	"ecdsa_secp224r1_sha256_test.json",
 	"ecdsa_secp256r1_sha256_test.json",