Represent unknown universal types with V_ASN1_OTHER
OpenSSL's ASN1_STRING representation has many cases. There's a grab-bag V_ASN1_OTHER cases that can represent any element. But it is currently only used for non-universal tags. Unknown universal tags go into the type field directly. This has a few problems: - Certain high values, V_ASN1_NEG_INTEGER and V_ASN1_NEG_ENUMERATED, are treated special. This was one of the two causes behind CVE-2016-2108 and had to be worked around with V_ASN1_MAX_UNIVERSAL. - OpenSSL can never compatibly support a new universal type in a non-ASN1_STRING form. Otherwise ASN1_TYPE's union changes its in-memory representation. - It is a bit ambiguous when OpenSSL does or doesn't know the type. - This is broadly implemented by having a default in all the switch/cases, which is a little awkward. - It's yet another "unknown tag" case when V_ASN1_OTHER covers such cases just fine. Remove this representation and use V_ASN1_OTHER. This more unambiguously resolves CVE-2016-2108. ASN1_STRING's and ASN1_TYPE's respective type fields are now a closed set. Update the documenthation accordingly. Formally allowing universal types in ASN1_STRING also opens the door to clearing the ASN1_PRINTABLE mess (https://crbug.com/boringssl/412). BoringSSL currently rejects X.509 names that are actually valid, because the OpenSSL X509_NAME representation cannot represent them. This allows us to introduce an ASN1_STRING-based ANY representation, which just represents all non-ASN1_STRING types in an V_ASN1_OTHER. The implementation is a little clumsy (the way things tasn_dec.c is written, I had to introduce yet another check), but I'm hoping that, when the parser is rewritten with CBS, this can be integrated into a single type dispatch. Update-Note: This does not change the set of inputs accepted or rejected by the ASN.1 parser. It does, however, change the in-memory representation in edge cases. Unless the application was specifically inspecting the in-memory representation for these unknown types, we expect this to have no impact. Fixed: 561 Change-Id: Ibf9550e285ce50b11c7609d28b139354b9dd41dc Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/58148 Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: David Benjamin <davidben@google.com>
This commit is contained in:
parent
92de195169
commit
8c8629bfd8
@ -40,45 +40,6 @@
|
||||
#endif
|
||||
|
||||
|
||||
// kTag128 is an ASN.1 structure with a universal tag with number 128.
|
||||
static const uint8_t kTag128[] = {
|
||||
0x1f, 0x81, 0x00, 0x01, 0x00,
|
||||
};
|
||||
|
||||
// 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.");
|
||||
|
||||
// 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,
|
||||
};
|
||||
|
||||
TEST(ASN1Test, LargeTags) {
|
||||
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();
|
||||
|
||||
p = kTagOverflow;
|
||||
obj.reset(d2i_ASN1_TYPE(NULL, &p, sizeof(kTagOverflow)));
|
||||
EXPECT_FALSE(obj) << "Parsed value with tag overflow" << obj->type;
|
||||
ERR_clear_error();
|
||||
|
||||
p = kTag128;
|
||||
obj.reset(d2i_ASN1_TYPE(NULL, &p, sizeof(kTag128)));
|
||||
ASSERT_TRUE(obj);
|
||||
EXPECT_EQ(128, obj->type);
|
||||
const uint8_t kZero = 0;
|
||||
EXPECT_EQ(Bytes(&kZero, 1), Bytes(obj->value.asn1_string->data,
|
||||
obj->value.asn1_string->length));
|
||||
}
|
||||
|
||||
// |obj| and |i2d_func| require different template parameters because C++ may
|
||||
// deduce, say, |ASN1_STRING*| via |obj| and |const ASN1_STRING*| via
|
||||
// |i2d_func|. Template argument deduction then fails. The language is not able
|
||||
@ -107,6 +68,67 @@ void TestSerialize(T obj, int (*i2d_func)(U a, uint8_t **pp),
|
||||
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);
|
||||
|
||||
// The historical in-memory representation of |kTag128| was for both
|
||||
// |obj->type| and |obj->value.asn1_string->type| to be NULL. This is no
|
||||
// longer used and should be rejected by the encoder.
|
||||
obj.reset(ASN1_TYPE_new());
|
||||
ASSERT_TRUE(obj);
|
||||
obj->type = 128;
|
||||
obj->value.asn1_string = ASN1_STRING_type_new(128);
|
||||
ASSERT_TRUE(obj->value.asn1_string);
|
||||
const uint8_t zero = 0;
|
||||
ASSERT_TRUE(ASN1_STRING_set(obj->value.asn1_string, &zero, sizeof(zero)));
|
||||
EXPECT_EQ(-1, i2d_ASN1_TYPE(obj.get(), nullptr));
|
||||
|
||||
// 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 ||
|
||||
|
@ -136,6 +136,23 @@ unsigned long ASN1_tag2bit(int tag) {
|
||||
return tag2bit[tag];
|
||||
}
|
||||
|
||||
static int is_supported_universal_type(int tag, int aclass) {
|
||||
if (aclass != V_ASN1_UNIVERSAL) {
|
||||
return 0;
|
||||
}
|
||||
return tag == V_ASN1_OBJECT || tag == V_ASN1_NULL || tag == V_ASN1_BOOLEAN ||
|
||||
tag == V_ASN1_BIT_STRING || tag == V_ASN1_INTEGER ||
|
||||
tag == V_ASN1_ENUMERATED || tag == V_ASN1_OCTET_STRING ||
|
||||
tag == V_ASN1_NUMERICSTRING || tag == V_ASN1_PRINTABLESTRING ||
|
||||
tag == V_ASN1_T61STRING || tag == V_ASN1_VIDEOTEXSTRING ||
|
||||
tag == V_ASN1_IA5STRING || tag == V_ASN1_UTCTIME ||
|
||||
tag == V_ASN1_GENERALIZEDTIME || tag == V_ASN1_GRAPHICSTRING ||
|
||||
tag == V_ASN1_VISIBLESTRING || tag == V_ASN1_GENERALSTRING ||
|
||||
tag == V_ASN1_UNIVERSALSTRING || tag == V_ASN1_BMPSTRING ||
|
||||
tag == V_ASN1_UTF8STRING || tag == V_ASN1_SET ||
|
||||
tag == V_ASN1_SEQUENCE;
|
||||
}
|
||||
|
||||
// Macro to initialize and invalidate the cache
|
||||
|
||||
// Decode an ASN1 item, this currently behaves just like a standard 'd2i'
|
||||
@ -677,7 +694,7 @@ static int asn1_d2i_ex_primitive(ASN1_VALUE **pval, const unsigned char **in,
|
||||
OPENSSL_PUT_ERROR(ASN1, ASN1_R_NESTED_ASN1_ERROR);
|
||||
return 0;
|
||||
}
|
||||
if (oclass != V_ASN1_UNIVERSAL) {
|
||||
if (!is_supported_universal_type(utype, oclass)) {
|
||||
utype = V_ASN1_OTHER;
|
||||
}
|
||||
}
|
||||
@ -820,8 +837,7 @@ static int asn1_ex_c2i(ASN1_VALUE **pval, const unsigned char *cont, int len,
|
||||
case V_ASN1_UTF8STRING:
|
||||
case V_ASN1_OTHER:
|
||||
case V_ASN1_SET:
|
||||
case V_ASN1_SEQUENCE:
|
||||
default: {
|
||||
case V_ASN1_SEQUENCE: {
|
||||
CBS cbs;
|
||||
CBS_init(&cbs, cont, (size_t)len);
|
||||
if (utype == V_ASN1_BMPSTRING) {
|
||||
@ -884,6 +900,9 @@ static int asn1_ex_c2i(ASN1_VALUE **pval, const unsigned char *cont, int len,
|
||||
}
|
||||
break;
|
||||
}
|
||||
default:
|
||||
OPENSSL_PUT_ERROR(ASN1, ASN1_R_BAD_TEMPLATE);
|
||||
goto err;
|
||||
}
|
||||
// If ASN1_ANY and NULL type fix up value
|
||||
if (typ && (utype == V_ASN1_NULL)) {
|
||||
|
@ -691,13 +691,17 @@ static int asn1_ex_i2c(ASN1_VALUE **pval, unsigned char *cout, int *out_omit,
|
||||
case V_ASN1_UTF8STRING:
|
||||
case V_ASN1_SEQUENCE:
|
||||
case V_ASN1_SET:
|
||||
default:
|
||||
// This is not a valid |ASN1_ITEM| type, but it appears in |ASN1_TYPE|.
|
||||
case V_ASN1_OTHER:
|
||||
// All based on ASN1_STRING and handled the same
|
||||
strtmp = (ASN1_STRING *)*pval;
|
||||
cont = strtmp->data;
|
||||
len = strtmp->length;
|
||||
|
||||
break;
|
||||
|
||||
default:
|
||||
OPENSSL_PUT_ERROR(ASN1, ASN1_R_BAD_TEMPLATE);
|
||||
return -1;
|
||||
}
|
||||
if (cout && len) {
|
||||
OPENSSL_memcpy(cout, cont, len);
|
||||
|
@ -467,31 +467,39 @@ DECLARE_ASN1_ITEM(ASN1_FBOOLEAN)
|
||||
// |ASN1_STRING|, to represent most values.
|
||||
|
||||
// An asn1_string_st (aka |ASN1_STRING|) represents a value of a string-like
|
||||
// ASN.1 type. It contains a type field, and a byte string data field with a
|
||||
// ASN.1 type. It contains a |type| field, and a byte string |data| field with a
|
||||
// type-specific representation.
|
||||
//
|
||||
// When representing a string value, the type field is one of
|
||||
// |V_ASN1_OCTET_STRING|, |V_ASN1_UTF8STRING|, |V_ASN1_NUMERICSTRING|,
|
||||
// |V_ASN1_PRINTABLESTRING|, |V_ASN1_T61STRING|, |V_ASN1_VIDEOTEXSTRING|,
|
||||
// |V_ASN1_IA5STRING|, |V_ASN1_GRAPHICSTRING|, |V_ASN1_ISO64STRING|,
|
||||
// |V_ASN1_VISIBLESTRING|, |V_ASN1_GENERALSTRING|, |V_ASN1_UNIVERSALSTRING|, or
|
||||
// |V_ASN1_BMPSTRING|. The data contains the byte representation of of the
|
||||
// If |type| is one of |V_ASN1_OCTET_STRING|, |V_ASN1_UTF8STRING|,
|
||||
// |V_ASN1_NUMERICSTRING|, |V_ASN1_PRINTABLESTRING|, |V_ASN1_T61STRING|,
|
||||
// |V_ASN1_VIDEOTEXSTRING|, |V_ASN1_IA5STRING|, |V_ASN1_GRAPHICSTRING|,
|
||||
// |V_ASN1_ISO64STRING|, |V_ASN1_VISIBLESTRING|, |V_ASN1_GENERALSTRING|,
|
||||
// |V_ASN1_UNIVERSALSTRING|, or |V_ASN1_BMPSTRING|, the object represents an
|
||||
// ASN.1 string type. The data contains the byte representation of the
|
||||
// string.
|
||||
//
|
||||
// When representing a BIT STRING value, the type field is |V_ASN1_BIT_STRING|.
|
||||
// See bit string documentation below for how the data and flags are used.
|
||||
// If |type| is |V_ASN1_BIT_STRING|, the object represents a BIT STRING value.
|
||||
// See bit string documentation below for the data and flags.
|
||||
//
|
||||
// When representing an INTEGER or ENUMERATED value, the type field is one of
|
||||
// |V_ASN1_INTEGER|, |V_ASN1_NEG_INTEGER|, |V_ASN1_ENUMERATED|, or
|
||||
// |V_ASN1_NEG_ENUMERATED|. See integer documentation below for details.
|
||||
// If |type| is one of |V_ASN1_INTEGER|, |V_ASN1_NEG_INTEGER|,
|
||||
// |V_ASN1_ENUMERATED|, or |V_ASN1_NEG_ENUMERATED|, the object represents an
|
||||
// INTEGER or ENUMERATED value. See integer documentation below for details.
|
||||
//
|
||||
// When representing a GeneralizedTime or UTCTime value, the type field is
|
||||
// |V_ASN1_GENERALIZEDTIME| or |V_ASN1_UTCTIME|, respectively. The data contains
|
||||
// the DER encoding of the value. For example, the UNIX epoch would be
|
||||
// If |type| is |V_ASN1_GENERALIZEDTIME| or |V_ASN1_UTCTIME|, the object
|
||||
// represents a GeneralizedTime or UTCTime value, respectively. The data
|
||||
// contains the DER encoding of the value. For example, the UNIX epoch would be
|
||||
// "19700101000000Z" for a GeneralizedTime and "700101000000Z" for a UTCTime.
|
||||
//
|
||||
// |ASN1_STRING|, when stored in an |ASN1_TYPE|, may also represent an element
|
||||
// with tag not directly supported by this library. See |ASN1_TYPE| for details.
|
||||
// If |type| is |V_ASN1_SEQUENCE|, |V_ASN1_SET|, or |V_ASN1_OTHER|, the object
|
||||
// represents a SEQUENCE, SET, or arbitrary ASN.1 value, respectively. Unlike
|
||||
// the above cases, the data contains the DER encoding of the entire structure,
|
||||
// including the header. If the value is explicitly or implicitly tagged, this
|
||||
// too will be reflected in the data field. As this case handles unknown types,
|
||||
// the contents are not checked when parsing or serializing.
|
||||
//
|
||||
// Other values of |type| do not represent a valid ASN.1 value, though
|
||||
// default-constructed objects may set |type| to -1. Such objects cannot be
|
||||
// serialized.
|
||||
//
|
||||
// |ASN1_STRING| additionally has the following typedefs: |ASN1_BIT_STRING|,
|
||||
// |ASN1_BMPSTRING|, |ASN1_ENUMERATED|, |ASN1_GENERALIZEDTIME|,
|
||||
@ -508,15 +516,14 @@ DECLARE_ASN1_ITEM(ASN1_FBOOLEAN)
|
||||
// |ASN1_STRING_length|.
|
||||
//
|
||||
// If a function returns an |ASN1_STRING| where the typedef or ASN.1 structure
|
||||
// implies constraints on the type field, callers may assume that the type field
|
||||
// is correct. However, if a function takes an |ASN1_STRING| as input, callers
|
||||
// must ensure the type field matches. These invariants are not captured by the
|
||||
// C type system and may not be checked at runtime. For example, callers may
|
||||
// assume the output of |X509_get0_serialNumber| has type |V_ASN1_INTEGER| or
|
||||
// |V_ASN1_NEG_INTEGER|. Callers must not pass a string of type
|
||||
// |V_ASN1_OCTET_STRING| to |X509_set_serialNumber|. Doing so may break
|
||||
// invariants on the |X509| object and break the |X509_get0_serialNumber|
|
||||
// invariant.
|
||||
// implies constraints on |type|, callers may assume that |type| is correct.
|
||||
// However, if a function takes an |ASN1_STRING| as input, callers must ensure
|
||||
// |type| matches. These invariants are not captured by the C type system and
|
||||
// may not be checked at runtime. For example, callers may assume the output of
|
||||
// |X509_get0_serialNumber| has type |V_ASN1_INTEGER| or |V_ASN1_NEG_INTEGER|.
|
||||
// Callers must not pass a string of type |V_ASN1_OCTET_STRING| to
|
||||
// |X509_set_serialNumber|. Doing so may break invariants on the |X509| object
|
||||
// and break the |X509_get0_serialNumber| invariant.
|
||||
//
|
||||
// TODO(https://crbug.com/boringssl/445): This is very unfriendly. Getting the
|
||||
// type field wrong should not cause memory errors, but it may do strange
|
||||
@ -1492,15 +1499,14 @@ DECLARE_ASN1_ITEM(ASN1_OBJECT)
|
||||
// |ASN1_BOOLEAN|.
|
||||
//
|
||||
// If |type| is |V_ASN1_SEQUENCE|, |V_ASN1_SET|, or |V_ASN1_OTHER|, the tag is
|
||||
// SEQUENCE, SET, or some non-universal tag, respectively. |value| is an
|
||||
// |ASN1_STRING| containing the entire element, including the tag and length.
|
||||
// The |ASN1_STRING|'s |type| field matches the containing |ASN1_TYPE|'s |type|.
|
||||
// SEQUENCE, SET, or some arbitrary tag, respectively. |value| uses the
|
||||
// corresponding |ASN1_STRING| representation. Although any type may be
|
||||
// represented in |V_ASN1_OTHER|, the parser will always return the more
|
||||
// specific encoding when available.
|
||||
//
|
||||
// Other positive values of |type|, up to |V_ASN1_MAX_UNIVERSAL|, correspond to
|
||||
// universal primitive tags not directly supported by this library. |value| is
|
||||
// an |ASN1_STRING| containing the body of the element, excluding the tag
|
||||
// and length. The |ASN1_STRING|'s |type| field matches the containing
|
||||
// |ASN1_TYPE|'s |type|.
|
||||
// Other values of |type| do not represent a valid ASN.1 value, though
|
||||
// default-constructed objects may set |type| to -1. Such objects cannot be
|
||||
// serialized.
|
||||
struct asn1_type_st {
|
||||
int type;
|
||||
union {
|
||||
|
Loading…
x
Reference in New Issue
Block a user