Work around some valgrind warnings in GTest. GTest likes to dump the underlying bytes for parameters which, in its fallback paths, tends to hit uninitialized memory. See https://github.com/google/googletest/issues/3805 Work around this. Use the NID, rather than the whole EC_builtin_curve for ECCurveTest, and then don't use TEST_P for one of the BIO tests at all. Change-Id: Ic578d1a1b08294b0cd2f13b3bd17f23f6e5f996d Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55229 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Bob Beck <bbe@google.com>
diff --git a/crypto/bio/bio_test.cc b/crypto/bio/bio_test.cc index 765e962..342325c 100644 --- a/crypto/bio/bio_test.cc +++ b/crypto/bio/bio_test.cc
@@ -154,74 +154,71 @@ } } -static const size_t kLargeASN1PayloadLen = 8000; +TEST(BIOTest, ReadASN1) { + static const size_t kLargeASN1PayloadLen = 8000; -struct ASN1TestParam { - bool should_succeed; - std::vector<uint8_t> input; - // suffix_len is the number of zeros to append to |input|. - size_t suffix_len; - // expected_len, if |should_succeed| is true, is the expected length of the - // ASN.1 element. - size_t expected_len; - size_t max_len; -} kASN1TestParams[] = { - {true, {0x30, 2, 1, 2, 0, 0}, 0, 4, 100}, - {false /* truncated */, {0x30, 3, 1, 2}, 0, 0, 100}, - {false /* should be short len */, {0x30, 0x81, 1, 1}, 0, 0, 100}, - {false /* zero padded */, {0x30, 0x82, 0, 1, 1}, 0, 0, 100}, + struct ASN1Test { + bool should_succeed; + std::vector<uint8_t> input; + // suffix_len is the number of zeros to append to |input|. + size_t suffix_len; + // expected_len, if |should_succeed| is true, is the expected length of the + // ASN.1 element. + size_t expected_len; + size_t max_len; + } kASN1Tests[] = { + {true, {0x30, 2, 1, 2, 0, 0}, 0, 4, 100}, + {false /* truncated */, {0x30, 3, 1, 2}, 0, 0, 100}, + {false /* should be short len */, {0x30, 0x81, 1, 1}, 0, 0, 100}, + {false /* zero padded */, {0x30, 0x82, 0, 1, 1}, 0, 0, 100}, - // Test a large payload. - {true, - {0x30, 0x82, kLargeASN1PayloadLen >> 8, kLargeASN1PayloadLen & 0xff}, - kLargeASN1PayloadLen, - 4 + kLargeASN1PayloadLen, - kLargeASN1PayloadLen * 2}, - {false /* max_len too short */, - {0x30, 0x82, kLargeASN1PayloadLen >> 8, kLargeASN1PayloadLen & 0xff}, - kLargeASN1PayloadLen, - 4 + kLargeASN1PayloadLen, - 3 + kLargeASN1PayloadLen}, + // Test a large payload. + {true, + {0x30, 0x82, kLargeASN1PayloadLen >> 8, kLargeASN1PayloadLen & 0xff}, + kLargeASN1PayloadLen, + 4 + kLargeASN1PayloadLen, + kLargeASN1PayloadLen * 2}, + {false /* max_len too short */, + {0x30, 0x82, kLargeASN1PayloadLen >> 8, kLargeASN1PayloadLen & 0xff}, + kLargeASN1PayloadLen, + 4 + kLargeASN1PayloadLen, + 3 + kLargeASN1PayloadLen}, - // Test an indefinite-length input. - {true, - {0x30, 0x80}, - kLargeASN1PayloadLen + 2, - 2 + kLargeASN1PayloadLen + 2, - kLargeASN1PayloadLen * 2}, - {false /* max_len too short */, - {0x30, 0x80}, - kLargeASN1PayloadLen + 2, - 2 + kLargeASN1PayloadLen + 2, - 2 + kLargeASN1PayloadLen + 1}, -}; + // Test an indefinite-length input. + {true, + {0x30, 0x80}, + kLargeASN1PayloadLen + 2, + 2 + kLargeASN1PayloadLen + 2, + kLargeASN1PayloadLen * 2}, + {false /* max_len too short */, + {0x30, 0x80}, + kLargeASN1PayloadLen + 2, + 2 + kLargeASN1PayloadLen + 2, + 2 + kLargeASN1PayloadLen + 1}, + }; -class BIOASN1Test : public testing::TestWithParam<ASN1TestParam> {}; + for (const auto &t : kASN1Tests) { + std::vector<uint8_t> input = t.input; + input.resize(input.size() + t.suffix_len, 0); -TEST_P(BIOASN1Test, ReadASN1) { - const ASN1TestParam& param = GetParam(); - std::vector<uint8_t> input = param.input; - input.resize(input.size() + param.suffix_len, 0); + bssl::UniquePtr<BIO> bio(BIO_new_mem_buf(input.data(), input.size())); + ASSERT_TRUE(bio); - bssl::UniquePtr<BIO> bio(BIO_new_mem_buf(input.data(), input.size())); - ASSERT_TRUE(bio); + uint8_t *out; + size_t out_len; + int ok = BIO_read_asn1(bio.get(), &out, &out_len, t.max_len); + if (!ok) { + out = nullptr; + } + bssl::UniquePtr<uint8_t> out_storage(out); - uint8_t *out; - size_t out_len; - int ok = BIO_read_asn1(bio.get(), &out, &out_len, param.max_len); - if (!ok) { - out = nullptr; - } - bssl::UniquePtr<uint8_t> out_storage(out); - - ASSERT_EQ(param.should_succeed, (ok == 1)); - if (param.should_succeed) { - EXPECT_EQ(Bytes(input.data(), param.expected_len), Bytes(out, out_len)); + ASSERT_EQ(t.should_succeed, (ok == 1)); + if (t.should_succeed) { + EXPECT_EQ(Bytes(input.data(), t.expected_len), Bytes(out, out_len)); + } } } -INSTANTIATE_TEST_SUITE_P(All, BIOASN1Test, testing::ValuesIn(kASN1TestParams)); - // Run through the tests twice, swapping |bio1| and |bio2|, for symmetry. class BIOPairTest : public testing::TestWithParam<bool> {};
diff --git a/crypto/fipsmodule/ec/ec_test.cc b/crypto/fipsmodule/ec/ec_test.cc index 12430b8..8e144ec 100644 --- a/crypto/fipsmodule/ec/ec_test.cc +++ b/crypto/fipsmodule/ec/ec_test.cc
@@ -529,12 +529,12 @@ EXPECT_EQ(0, BN_cmp(y.get(), qy.get())); } -class ECCurveTest : public testing::TestWithParam<EC_builtin_curve> { +class ECCurveTest : public testing::TestWithParam<int> { public: const EC_GROUP *group() const { return group_.get(); } void SetUp() override { - group_.reset(EC_GROUP_new_by_curve_name(GetParam().nid)); + group_.reset(EC_GROUP_new_by_curve_name(GetParam())); ASSERT_TRUE(group_); } @@ -544,7 +544,7 @@ TEST_P(ECCurveTest, SetAffine) { // Generate an EC_KEY. - bssl::UniquePtr<EC_KEY> key(EC_KEY_new_by_curve_name(GetParam().nid)); + bssl::UniquePtr<EC_KEY> key(EC_KEY_new_by_curve_name(GetParam())); ASSERT_TRUE(key); ASSERT_TRUE(EC_KEY_generate_key(key.get())); @@ -586,7 +586,7 @@ } TEST_P(ECCurveTest, IsOnCurve) { - bssl::UniquePtr<EC_KEY> key(EC_KEY_new_by_curve_name(GetParam().nid)); + bssl::UniquePtr<EC_KEY> key(EC_KEY_new_by_curve_name(GetParam())); ASSERT_TRUE(key); ASSERT_TRUE(EC_KEY_generate_key(key.get())); @@ -610,12 +610,12 @@ } TEST_P(ECCurveTest, Compare) { - bssl::UniquePtr<EC_KEY> key1(EC_KEY_new_by_curve_name(GetParam().nid)); + bssl::UniquePtr<EC_KEY> key1(EC_KEY_new_by_curve_name(GetParam())); ASSERT_TRUE(key1); ASSERT_TRUE(EC_KEY_generate_key(key1.get())); const EC_POINT *pub1 = EC_KEY_get0_public_key(key1.get()); - bssl::UniquePtr<EC_KEY> key2(EC_KEY_new_by_curve_name(GetParam().nid)); + bssl::UniquePtr<EC_KEY> key2(EC_KEY_new_by_curve_name(GetParam())); ASSERT_TRUE(key2); ASSERT_TRUE(EC_KEY_generate_key(key2.get())); const EC_POINT *pub2 = EC_KEY_get0_public_key(key2.get()); @@ -665,13 +665,13 @@ TEST_P(ECCurveTest, GenerateFIPS) { // Generate an EC_KEY. - bssl::UniquePtr<EC_KEY> key(EC_KEY_new_by_curve_name(GetParam().nid)); + bssl::UniquePtr<EC_KEY> key(EC_KEY_new_by_curve_name(GetParam())); ASSERT_TRUE(key); ASSERT_TRUE(EC_KEY_generate_key_fips(key.get())); } TEST_P(ECCurveTest, AddingEqualPoints) { - bssl::UniquePtr<EC_KEY> key(EC_KEY_new_by_curve_name(GetParam().nid)); + bssl::UniquePtr<EC_KEY> key(EC_KEY_new_by_curve_name(GetParam())); ASSERT_TRUE(key); ASSERT_TRUE(EC_KEY_generate_key(key.get())); @@ -840,7 +840,7 @@ // Test that EC_KEY_set_private_key rejects invalid values. TEST_P(ECCurveTest, SetInvalidPrivateKey) { - bssl::UniquePtr<EC_KEY> key(EC_KEY_new_by_curve_name(GetParam().nid)); + bssl::UniquePtr<EC_KEY> key(EC_KEY_new_by_curve_name(GetParam())); ASSERT_TRUE(key); bssl::UniquePtr<BIGNUM> bn(BN_new()); @@ -969,17 +969,19 @@ nullptr, 0, nullptr)); } -static std::vector<EC_builtin_curve> AllCurves() { +static std::vector<int> AllCurves() { const size_t num_curves = EC_get_builtin_curves(nullptr, 0); std::vector<EC_builtin_curve> curves(num_curves); EC_get_builtin_curves(curves.data(), num_curves); - return curves; + std::vector<int> nids; + for (const auto& curve : curves) { + nids.push_back(curve.nid); + } + return nids; } -static std::string CurveToString( - const testing::TestParamInfo<EC_builtin_curve> ¶ms) { - // The comment field contains characters GTest rejects, so use the OBJ name. - return OBJ_nid2sn(params.param.nid); +static std::string CurveToString(const testing::TestParamInfo<int> ¶ms) { + return OBJ_nid2sn(params.param); } INSTANTIATE_TEST_SUITE_P(All, ECCurveTest, testing::ValuesIn(AllCurves()),