Test ASN1_TYPE parsing more extensively
We should be able to parse a variety of valid types, and also reject
syntax errors in any types that we recognize. Also double-check that
nothing went wrong in the translation from i2d_ASN1_TYPE to X509_ALGOR
(basically the only use of ASN1_TYPE in the library).
Change-Id: I825f44c35a7e8ea6374641cf5e0ea5daaf471ad7
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/81727
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/crypto/asn1/asn1_test.cc b/crypto/asn1/asn1_test.cc
index e2cf0d3..54d2cb4 100644
--- a/crypto/asn1/asn1_test.cc
+++ b/crypto/asn1/asn1_test.cc
@@ -71,55 +71,6 @@
EXPECT_EQ(Bytes(expected), Bytes(buf));
}
-// Historically, unknown universal tags were represented in |ASN1_TYPE| as
-// |ASN1_STRING|s with the type matching the tag number. This can collide with
-// |V_ASN_NEG|, which was one of the causes of CVE-2016-2108. We now represent
-// unsupported values with |V_ASN1_OTHER|, but retain the |V_ASN1_MAX_UNIVERSAL|
-// limit.
-TEST(ASN1Test, UnknownTags) {
- // kTag258 is an ASN.1 structure with a universal tag with number 258.
- static const uint8_t kTag258[] = {0x1f, 0x82, 0x02, 0x01, 0x00};
- static_assert(
- V_ASN1_NEG_INTEGER == 258,
- "V_ASN1_NEG_INTEGER changed. Update kTag258 to collide with it.");
- const uint8_t *p = kTag258;
- bssl::UniquePtr<ASN1_TYPE> obj(d2i_ASN1_TYPE(NULL, &p, sizeof(kTag258)));
- EXPECT_FALSE(obj) << "Parsed value with illegal tag" << obj->type;
- ERR_clear_error();
-
- // kTagOverflow is an ASN.1 structure with a universal tag with number 2^35-1,
- // which will not fit in an int.
- static const uint8_t kTagOverflow[] = {0x1f, 0xff, 0xff, 0xff,
- 0xff, 0x7f, 0x01, 0x00};
- p = kTagOverflow;
- obj.reset(d2i_ASN1_TYPE(NULL, &p, sizeof(kTagOverflow)));
- EXPECT_FALSE(obj) << "Parsed value with tag overflow" << obj->type;
- ERR_clear_error();
-
- // kTag128 is an ASN.1 structure with a universal tag with number 128. It
- // should be parsed as |V_ASN1_OTHER|.
- static const uint8_t kTag128[] = {0x1f, 0x81, 0x00, 0x01, 0x00};
- p = kTag128;
- obj.reset(d2i_ASN1_TYPE(NULL, &p, sizeof(kTag128)));
- ASSERT_TRUE(obj);
- EXPECT_EQ(V_ASN1_OTHER, obj->type);
- EXPECT_EQ(Bytes(kTag128), Bytes(obj->value.asn1_string->data,
- obj->value.asn1_string->length));
- TestSerialize(obj.get(), i2d_ASN1_TYPE, kTag128);
-
- // If a tag is known, but has the wrong constructed bit, it should be
- // rejected, not placed in |V_ASN1_OTHER|.
- static const uint8_t kConstructedOctetString[] = {0x24, 0x00};
- p = kConstructedOctetString;
- obj.reset(d2i_ASN1_TYPE(nullptr, &p, sizeof(kConstructedOctetString)));
- EXPECT_FALSE(obj);
-
- static const uint8_t kPrimitiveSequence[] = {0x10, 0x00};
- p = kPrimitiveSequence;
- obj.reset(d2i_ASN1_TYPE(nullptr, &p, sizeof(kPrimitiveSequence)));
- EXPECT_FALSE(obj);
-}
-
static bssl::UniquePtr<BIGNUM> BIGNUMPow2(unsigned bit) {
bssl::UniquePtr<BIGNUM> bn(BN_new());
if (!bn || //
@@ -599,6 +550,22 @@
TestSerialize(val.get(), i2d_BASIC_CONSTRAINTS, kCA);
}
+static std::vector<uint8_t> EmbedParamInAlgorithmIdentifier(
+ bssl::Span<const uint8_t> param) {
+ bssl::ScopedCBB cbb;
+ CBB seq;
+ BSSL_CHECK(CBB_init(cbb.get(), 64));
+ BSSL_CHECK(CBB_add_asn1(cbb.get(), &seq, CBS_ASN1_SEQUENCE));
+ static const uint8_t kTestOID[] = {0x2a, 0x86, 0x48, 0x86, 0xf7, 0x12,
+ 0x04, 0x01, 0x84, 0xb7, 0x09, 0x02};
+ BSSL_CHECK(
+ CBB_add_asn1_element(&seq, CBS_ASN1_OBJECT, kTestOID, sizeof(kTestOID)));
+ BSSL_CHECK(CBB_add_bytes(&seq, param.data(), param.size()));
+ BSSL_CHECK(CBB_flush(cbb.get()));
+ return std::vector(CBB_data(cbb.get()),
+ CBB_data(cbb.get()) + CBB_len(cbb.get()));
+}
+
TEST(ASN1Test, ASN1Type) {
const struct {
int type;
@@ -616,6 +583,12 @@
{V_ASN1_BIT_STRING, {0x03, 0x02, 0x01, 0x00}},
// INTEGER { -1 }
{V_ASN1_INTEGER, {0x02, 0x01, 0xff}},
+ // INTEGER { 1 }
+ {V_ASN1_INTEGER, {0x02, 0x01, 0x01}},
+ // ENUMERATED { -1 }
+ {V_ASN1_ENUMERATED, {0x0a, 0x01, 0xff}},
+ // ENUMERATED { 1 }
+ {V_ASN1_ENUMERATED, {0x0a, 0x01, 0x01}},
// OBJECT_IDENTIFIER { 1.2.840.113554.4.1.72585.2 }
{V_ASN1_OBJECT,
{0x06, 0x0c, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x12, 0x04, 0x01, 0x84, 0xb7,
@@ -626,20 +599,114 @@
{V_ASN1_SEQUENCE, {0x30, 0x00}},
// SET {}
{V_ASN1_SET, {0x31, 0x00}},
+ // IA5String { "a" }
+ {V_ASN1_IA5STRING, {0x16, 0x01, 0x61}},
+ // UTF8String { "a" }
+ {V_ASN1_UTF8STRING, {0x0c, 0x01, 0x61}},
+ // BMPString { u"a" }
+ {V_ASN1_BMPSTRING, {0x1e, 0x02, 0x00, 0x61}},
+ // UniversalString { U"a" }
+ {V_ASN1_UNIVERSALSTRING, {0x1c, 0x04, 0x00, 0x00, 0x00, 0x61}},
// [0] { UTF8String { "a" } }
{V_ASN1_OTHER, {0xa0, 0x03, 0x0c, 0x01, 0x61}},
+ // [UNIVERSAL 128] { `00` }
+ // Unknown universal tags are parsed as |V_ASN1_OTHER|.
+ {V_ASN1_OTHER, {0x1f, 0x81, 0x00, 0x01, 0x00}},
};
for (const auto &t : kTests) {
SCOPED_TRACE(Bytes(t.der));
+ auto check_asn1_type = [&](const ASN1_TYPE *typ) {
+ EXPECT_EQ(ASN1_TYPE_get(typ), t.type);
+ EXPECT_EQ(typ->type, t.type);
+ // If we parsed a string type, check that is consistent.
+ if (t.type != V_ASN1_NULL && t.type != V_ASN1_OBJECT &&
+ t.type != V_ASN1_BOOLEAN) {
+ int str_type = ASN1_STRING_type(typ->value.asn1_string);
+ if (str_type == V_ASN1_NEG_INTEGER ||
+ str_type == V_ASN1_NEG_ENUMERATED) {
+ str_type &= ~V_ASN1_NEG;
+ }
+ EXPECT_EQ(str_type, t.type);
+ }
+ };
+
// The input should successfully parse.
const uint8_t *ptr = t.der.data();
bssl::UniquePtr<ASN1_TYPE> val(d2i_ASN1_TYPE(nullptr, &ptr, t.der.size()));
ASSERT_TRUE(val);
- EXPECT_EQ(ASN1_TYPE_get(val.get()), t.type);
- EXPECT_EQ(val->type, t.type);
+ check_asn1_type(val.get());
TestSerialize(val.get(), i2d_ASN1_TYPE, t.der);
+
+ // Test the same thing wrapped in an AlgorithmIdentifier.
+ std::vector<uint8_t> alg_der = EmbedParamInAlgorithmIdentifier(t.der);
+ ptr = alg_der.data();
+ bssl::UniquePtr<X509_ALGOR> alg(
+ d2i_X509_ALGOR(nullptr, &ptr, alg_der.size()));
+ ASSERT_TRUE(alg);
+
+ check_asn1_type(alg->parameter);
+ TestSerialize(alg.get(), i2d_X509_ALGOR, alg_der);
+ }
+
+ static_assert(V_ASN1_NEG_INTEGER == 258,
+ "V_ASN1_NEG_INTEGER changed. Update first test below to "
+ "collide with it.");
+ const std::vector<uint8_t> kInvalidTests[] = {
+ // Tag [UNIVERSAL 258] should be rejected.
+ //
+ // Historically, unknown universal tags were represented in |ASN1_TYPE| as
+ // |ASN1_STRING|s with the type matching the tag number. This can collide
+ // with |V_ASN_NEG|, which was one of the causes of CVE-2016-2108. (258
+ // matches |V_ASN1_NEG_INTEGER|.) We now represent unsupported values with
+ // |V_ASN1_OTHER|, but retain the |V_ASN1_MAX_UNIVERSAL| limit.
+ {0x1f, 0x82, 0x02, 0x01, 0x00},
+ // Tag [UNIVERSAL 2^35-1] will not fit in an int and should not
+ // overflow.
+ {0x1f, 0xff, 0xff, 0xff, 0xff, 0x7f, 0x01, 0x00},
+ // EOC is not a value.
+ {0x00, 0x00},
+ // Supported primitive types should be rejected if constructed.
+ {0x21, 0x00}, // [BOOLEAN CONSTRUCTED] {}
+ {0x22, 0x00}, // [INTEGER CONSTRUCTED] {}
+ {0x23, 0x00}, // [BIT STRING CONSTRUCTED] {}
+ {0x24, 0x00}, // [OCTET STRING CONSTRUCTED] {}
+ {0x25, 0x00}, // [NULL CONSTRUCTED] {}
+ {0x26, 0x00}, // [OBJECT IDENTIFIER CONSTRUCTED] {}
+ {0x2a, 0x00}, // [ENUMERATED CONSTRUCTED] {}
+ {0x2c, 0x00}, // [UTF8String CONSTRUCTED] {}
+ {0x3c, 0x00}, // [UniversalString CONSTRUCTED] {}
+ {0x3e, 0x00}, // [BMPString CONSTRUCTED] {}
+ // Supported constructed types should be rejected if primitive.
+ {0x10, 0x00}, // [SEQUENCE PRIMITIVE] {}
+ {0x11, 0x00}, // [SET PRIMITIVE] {}
+ // Invalid recognized types should be rejected.
+ {0x0c, 0x01, 0xff}, // UTF8String { `ff` }
+ {0x1c, 0x01, 0xff}, // UniversalString { `ff` }
+ {0x1e, 0x01, 0xff}, // BMPString { `ff` }
+ {0x02, 0x02, 0x00, 0x00}, // INTEGER { `0000` }
+ {0x0a, 0x02, 0x00, 0x00}, // ENUMERATED { `0000` }
+ {0x06, 0x00}, // OBJECT_IDENTIFIER { }
+ {0x06, 0x01, 0xff}, // OBJECT_IDENTIFIER { `ff` }
+ {0x03, 0x01, 0xff}, // BIT_STRING { `ff` }
+ {0x05, 0x01, 0xff}, // NULL { `ff` }
+ {0x01, 0x00}, // BOOLEAN {}
+ {0x01, 0x02, 0x00, 0x00}, // BOOLEAN { `0000` }
+ };
+ for (const auto &t : kInvalidTests) {
+ SCOPED_TRACE(Bytes(t));
+ const uint8_t *ptr = t.data();
+ bssl::UniquePtr<ASN1_TYPE> val(d2i_ASN1_TYPE(nullptr, &ptr, t.size()));
+ EXPECT_FALSE(val);
+ ERR_clear_error();
+
+ std::vector<uint8_t> alg_der = EmbedParamInAlgorithmIdentifier(t);
+ ptr = alg_der.data();
+ bssl::UniquePtr<X509_ALGOR> alg(
+ d2i_X509_ALGOR(nullptr, &ptr, alg_der.size()));
+ EXPECT_FALSE(alg);
+ ERR_clear_error();
}
}
@@ -1977,7 +2044,7 @@
// Encoding invalid |ASN1_TYPE|s should fail. |ASN1_TYPE|s are
// default-initialized to an invalid type.
-TEST(ASN1Test, InvalidASN1Type) {
+TEST(ASN1Test, EncodeInvalidASN1Type) {
bssl::UniquePtr<ASN1_TYPE> obj(ASN1_TYPE_new());
ASSERT_TRUE(obj);
EXPECT_EQ(-1, obj->type);