Add a test that arbitrary curves can be wrapped in EVP_PKEY Conscrypt relies on both this working at the EC_KEY level, but also EVP_PKEY. Make sure this keeps working, even as we mess around with EVP_PKEY's EC bits to separate the curves out. While I'm here, move the common HexToBIGNUM wrapper into test_util.h. Bug: 42290364 Change-Id: I58fa5a8487b8a499000a798040a53fc851b0a732 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/81547 Reviewed-by: Lily Chen <chlily@google.com> Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/crypto/evp/evp_extra_test.cc b/crypto/evp/evp_extra_test.cc index bb1746c..d7e39f5 100644 --- a/crypto/evp/evp_extra_test.cc +++ b/crypto/evp/evp_extra_test.cc
@@ -1170,3 +1170,83 @@ // Calling operations on the |EVP_PKEY| should cleanly fail. EXPECT_EQ(EVP_PKEY_bits(pkey.get()), 0); } + +// Test that |EC_KEY|s using custom curves can be wrapped in |EVP_PKEY|. Callers +// should not follow this code. This is maintained and tested only for +// compatibility with one legacy application, and supported in only a limited +// form. (E.g. such keys cannot be serialized.) +TEST(EVPExtraTest, CustomCurve) { + // Coefficients for brainpoolp256r1, not supported by BoringSSL. + static const char kP[] = + "a9fb57dba1eea9bc3e660a909d838d726e3bf623d52620282013481d1f6e5377"; + static const char kA[] = + "7d5a0975fc2c3057eef67530417affe7fb8055c126dc5c6ce94a4b44f330b5d9"; + static const char kB[] = + "26dc5c6ce94a4b44f330b5d9bbd77cbf958416295cf7e1ce6bccdc18ff8c07b6"; + static const char kX[] = + "8bd2aeb9cb7e57cb2c4b482ffc81b7afb9de27e1e3bd23c23a4453bd9ace3262"; + static const char kY[] = + "547ef835c3dac4fd97f8461a14611dc9c27745132ded8e545c1d54c72f046997"; + static const char kN[] = + "a9fb57dba1eea9bc3e660a909d838d718c397aa3b561a6f7901e0e82974856a7"; + static const char kD[] = + "0da21d76fed40dd82ac3314cce91abb585b5c4246e902b238a839609ea1e7ce1"; + static const char kQX[] = + "3a55e0341cab50452fe27b8a87e4775dec7a9daca94b0d84ad1e9f85b53ea513"; + static const char kQY[] = + "40088146b33bbbe81b092b41146774b35dd478cf056437cfb35ef0df2d269339"; + + // Construct a custom group. + bssl::UniquePtr<BIGNUM> p = HexToBIGNUM(kP), a = HexToBIGNUM(kA), + b = HexToBIGNUM(kB), x = HexToBIGNUM(kX), + y = HexToBIGNUM(kY), n = HexToBIGNUM(kN), + d = HexToBIGNUM(kD), qx = HexToBIGNUM(kQX), + qy = HexToBIGNUM(kQY); + ASSERT_TRUE(p && a && b && x && y && n && d && qx && qy); + bssl::UniquePtr<EC_GROUP> group( + EC_GROUP_new_curve_GFp(p.get(), a.get(), b.get(), nullptr)); + ASSERT_TRUE(group); + bssl::UniquePtr<EC_POINT> g(EC_POINT_new(group.get())); + ASSERT_TRUE(g); + ASSERT_TRUE(EC_POINT_set_affine_coordinates_GFp(group.get(), g.get(), x.get(), + y.get(), nullptr)); + ASSERT_TRUE( + EC_GROUP_set_generator(group.get(), g.get(), n.get(), BN_value_one())); + + // Generate a key with it. + bssl::UniquePtr<EC_KEY> ec_key(EC_KEY_new()); + ASSERT_TRUE(ec_key); + ASSERT_TRUE(EC_KEY_set_group(ec_key.get(), group.get())); + ASSERT_TRUE(EC_KEY_generate_key(ec_key.get())); + + // Wrap it in an |EVP_PKEY|. + bssl::UniquePtr<EVP_PKEY> pkey(EVP_PKEY_new()); + ASSERT_TRUE(pkey); + ASSERT_TRUE(EVP_PKEY_set1_EC_KEY(pkey.get(), ec_key.get())); + + // Signing should work. + bssl::Span<const uint8_t> msg = bssl::StringAsBytes("hello"); + bssl::ScopedEVP_MD_CTX ctx; + ASSERT_TRUE(EVP_DigestSignInit(ctx.get(), nullptr, EVP_sha256(), nullptr, + pkey.get())); + std::vector<uint8_t> sig(EVP_PKEY_size(pkey.get())); + size_t len = sig.size(); + ASSERT_TRUE( + EVP_DigestSign(ctx.get(), sig.data(), &len, msg.data(), msg.size())); + sig.resize(len); + + // Verifying should work. + ctx.Reset(); + ASSERT_TRUE(EVP_DigestVerifyInit(ctx.get(), nullptr, EVP_sha256(), nullptr, + pkey.get())); + ASSERT_TRUE(EVP_DigestVerify(ctx.get(), sig.data(), sig.size(), msg.data(), + msg.size())); + + // We will not serialize arbitrary groups, so serialization should fail. + bssl::ScopedCBB cbb; + ASSERT_TRUE(CBB_init(cbb.get(), 0)); + EXPECT_FALSE(EVP_marshal_public_key(cbb.get(), pkey.get())); + cbb.Reset(); + ASSERT_TRUE(CBB_init(cbb.get(), 0)); + EXPECT_FALSE(EVP_marshal_private_key(cbb.get(), pkey.get())); +}
diff --git a/crypto/fipsmodule/bn/bn_test.cc b/crypto/fipsmodule/bn/bn_test.cc index f6a2622..78ee00f 100644 --- a/crypto/fipsmodule/bn/bn_test.cc +++ b/crypto/fipsmodule/bn/bn_test.cc
@@ -45,7 +45,7 @@ namespace { -static int HexToBIGNUM(bssl::UniquePtr<BIGNUM> *out, const char *in) { +static int HexToBIGNUMWithReturn(bssl::UniquePtr<BIGNUM> *out, const char *in) { BIGNUM *raw = NULL; int ret = BN_hex2bn(&raw, in); out->reset(raw); @@ -89,7 +89,7 @@ } bssl::UniquePtr<BIGNUM> ret; - if (HexToBIGNUM(&ret, hex.c_str()) != static_cast<int>(hex.size())) { + if (HexToBIGNUMWithReturn(&ret, hex.c_str()) != static_cast<int>(hex.size())) { t_->PrintLine("Could not decode '%s'.", hex.c_str()); return nullptr; } @@ -1173,27 +1173,27 @@ TEST_F(BNTest, Hex2BN) { bssl::UniquePtr<BIGNUM> bn; - int ret = HexToBIGNUM(&bn, "0"); + int ret = HexToBIGNUMWithReturn(&bn, "0"); ASSERT_EQ(1, ret); EXPECT_TRUE(BN_is_zero(bn.get())); EXPECT_FALSE(BN_is_negative(bn.get())); - ret = HexToBIGNUM(&bn, "256"); + ret = HexToBIGNUMWithReturn(&bn, "256"); ASSERT_EQ(3, ret); EXPECT_TRUE(BN_is_word(bn.get(), 0x256)); EXPECT_FALSE(BN_is_negative(bn.get())); - ret = HexToBIGNUM(&bn, "-42"); + ret = HexToBIGNUMWithReturn(&bn, "-42"); ASSERT_EQ(3, ret); EXPECT_TRUE(BN_abs_is_word(bn.get(), 0x42)); EXPECT_TRUE(BN_is_negative(bn.get())); - ret = HexToBIGNUM(&bn, "-0"); + ret = HexToBIGNUMWithReturn(&bn, "-0"); ASSERT_EQ(2, ret); EXPECT_TRUE(BN_is_zero(bn.get())); EXPECT_FALSE(BN_is_negative(bn.get())); - ret = HexToBIGNUM(&bn, "abctrailing garbage is ignored"); + ret = HexToBIGNUMWithReturn(&bn, "abctrailing garbage is ignored"); ASSERT_EQ(3, ret); EXPECT_TRUE(BN_is_word(bn.get(), 0xabc)); EXPECT_FALSE(BN_is_negative(bn.get())); @@ -1725,7 +1725,7 @@ bssl::UniquePtr<BIGNUM> bn(BN_new()), expected; ASSERT_TRUE(bn); ASSERT_TRUE(BN_set_u64(bn.get(), test.value)); - ASSERT_TRUE(HexToBIGNUM(&expected, test.hex)); + ASSERT_TRUE(HexToBIGNUMWithReturn(&expected, test.hex)); EXPECT_BIGNUMS_EQUAL("BN_set_u64", expected.get(), bn.get()); uint64_t tmp; @@ -2301,7 +2301,7 @@ }; for (const char *str : kPrimesHex) { SCOPED_TRACE(str); - EXPECT_NE(0, HexToBIGNUM(&p, str)); + EXPECT_NE(0, HexToBIGNUMWithReturn(&p, str)); ASSERT_TRUE(BN_primality_test( &is_probably_prime_1, p.get(), BN_prime_checks_for_generation, ctx(),
diff --git a/crypto/fipsmodule/ec/ec_test.cc b/crypto/fipsmodule/ec/ec_test.cc index 3a5898d..4be8d5b 100644 --- a/crypto/fipsmodule/ec/ec_test.cc +++ b/crypto/fipsmodule/ec/ec_test.cc
@@ -473,12 +473,6 @@ EXPECT_FALSE(EC_KEY_get0_private_key(key.get())); } -static bssl::UniquePtr<BIGNUM> HexToBIGNUM(const char *hex) { - BIGNUM *bn = nullptr; - BN_hex2bn(&bn, hex); - return bssl::UniquePtr<BIGNUM>(bn); -} - // Test that point arithmetic works with custom curves using an arbitrary |a|, // rather than -3, as is common (and more efficient). TEST(ECTest, BrainpoolP256r1) {
diff --git a/crypto/fipsmodule/ecdsa/ecdsa_test.cc b/crypto/fipsmodule/ecdsa/ecdsa_test.cc index 6c9c964..837ceb9 100644 --- a/crypto/fipsmodule/ecdsa/ecdsa_test.cc +++ b/crypto/fipsmodule/ecdsa/ecdsa_test.cc
@@ -32,12 +32,6 @@ #include "../../test/test_util.h" -static bssl::UniquePtr<BIGNUM> HexToBIGNUM(const char *hex) { - BIGNUM *bn = nullptr; - BN_hex2bn(&bn, hex); - return bssl::UniquePtr<BIGNUM>(bn); -} - // Though we do not support secp160r1, it is reachable from the deprecated // custom curve APIs and has some unique properties (n is larger than p with the // difference crossing a word boundary on 32-bit), so test it explicitly.
diff --git a/crypto/test/test_util.cc b/crypto/test/test_util.cc index 78ce8d8..df3c9ed 100644 --- a/crypto/test/test_util.cc +++ b/crypto/test/test_util.cc
@@ -16,6 +16,7 @@ #include <ostream> +#include <openssl/bn.h> #include <openssl/err.h> #include "../internal.h" @@ -82,3 +83,9 @@ sizeof(expected)) << "\""; } + +bssl::UniquePtr<BIGNUM> HexToBIGNUM(const char *hex) { + BIGNUM *bn = nullptr; + BN_hex2bn(&bn, hex); + return bssl::UniquePtr<BIGNUM>(bn); +}
diff --git a/crypto/test/test_util.h b/crypto/test/test_util.h index b7671c1..90253ca 100644 --- a/crypto/test/test_util.h +++ b/crypto/test/test_util.h
@@ -76,5 +76,9 @@ // |reason|. testing::AssertionResult ErrorEquals(uint32_t err, int lib, int reason); +// HexToBignum decodes |hex| as a hexadecimal, big-endian, unsigned integer and +// returns it as a |BIGNUM|, or nullptr on error. +bssl::UniquePtr<BIGNUM> HexToBIGNUM(const char *hex); + #endif // OPENSSL_HEADER_CRYPTO_TEST_TEST_UTIL_H