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>
fips-20230428
David Benjamin 2 years ago committed by Boringssl LUCI CQ
parent 7ab49bf0af
commit 27e45c4342
  1. 127
      crypto/bio/bio_test.cc
  2. 32
      crypto/fipsmodule/ec/ec_test.cc

@ -154,74 +154,71 @@ TEST(BIOTest, Printf) {
}
}
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},
// 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},
};
class BIOASN1Test : public testing::TestWithParam<ASN1TestParam> {};
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);
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));
TEST(BIOTest, ReadASN1) {
static const size_t kLargeASN1PayloadLen = 8000;
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 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},
};
for (const auto &t : kASN1Tests) {
std::vector<uint8_t> input = t.input;
input.resize(input.size() + t.suffix_len, 0);
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);
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> {};

@ -529,12 +529,12 @@ TEST(ECTest, BrainpoolP256r1) {
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 @@ class ECCurveTest : public testing::TestWithParam<EC_builtin_curve> {
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, SetAffine) {
}
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, IsOnCurve) {
}
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, Compare) {
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_P(ECCurveTest, MulNonMinimal) {
// 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 @@ TEST_P(ECCurveTest, EncodeInfinity) {
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> &params) {
// 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> &params) {
return OBJ_nid2sn(params.param);
}
INSTANTIATE_TEST_SUITE_P(All, ECCurveTest, testing::ValuesIn(AllCurves()),

Loading…
Cancel
Save