diff --git a/crypto/asn1/asn1_lib.c b/crypto/asn1/asn1_lib.c index 462eb2e9f3..1089d7efd2 100644 --- a/crypto/asn1/asn1_lib.c +++ b/crypto/asn1/asn1_lib.c @@ -65,6 +65,7 @@ #include "../internal.h" #include "internal.h" +#include "../bytestring/internal.h" // Cross-module errors from crypto/x509/i2d_pr.c. @@ -113,8 +114,9 @@ OPENSSL_DECLARE_ERROR_REASON(ASN1, UNSUPPORTED_TYPE) static void asn1_put_length(unsigned char **pp, int length); -int ASN1_get_object(const unsigned char **inp, long *out_len, int *out_tag, - int *out_class, long in_len) { +int asn1_get_object_maybe_indefinite(const unsigned char **inp, long *out_len, + int *out_tag, int *out_class, long in_len, + int indefinite_ok) { if (in_len < 0) { OPENSSL_PUT_ERROR(ASN1, ASN1_R_HEADER_TOO_LONG); return 0x80; @@ -131,9 +133,12 @@ int ASN1_get_object(const unsigned char **inp, long *out_len, int *out_tag, int indefinite; CBS cbs, body; CBS_init(&cbs, *inp, (size_t)in_len); - if (!CBS_get_any_ber_asn1_element(&cbs, &body, &tag, &header_len, - /*out_ber_found=*/NULL, &indefinite) || - indefinite || !CBS_skip(&body, header_len) || + + int ber_found_temp; + if (!cbs_get_any_asn1_element(&cbs, &body, &tag, &header_len, &ber_found_temp, + &indefinite, /*ber_ok=*/1, + /*universal_tag_ok=*/indefinite_ok) || + (indefinite && !indefinite_ok) || !CBS_skip(&body, header_len) || // Bound the length to comfortably fit in an int. Lengths in this // module often switch between int and long without overflow checks. CBS_len(&body) > INT_MAX / 2) { @@ -143,22 +148,32 @@ int ASN1_get_object(const unsigned char **inp, long *out_len, int *out_tag, // Convert between tag representations. int tag_class = (tag & CBS_ASN1_CLASS_MASK) >> CBS_ASN1_TAG_SHIFT; - int constructed = (tag & CBS_ASN1_CONSTRUCTED) >> CBS_ASN1_TAG_SHIFT; + int constructed = (indefinite ? 1 : 0) | + ((tag & CBS_ASN1_CONSTRUCTED) >> CBS_ASN1_TAG_SHIFT); int tag_number = tag & CBS_ASN1_TAG_NUMBER_MASK; // To avoid ambiguity with V_ASN1_NEG, impose a limit on universal tags. if (tag_class == V_ASN1_UNIVERSAL && tag_number > V_ASN1_MAX_UNIVERSAL) { + // Ruby's OpenSSL::test_basic_asn1data has an assertion that violates this OPENSSL_PUT_ERROR(ASN1, ASN1_R_HEADER_TOO_LONG); return 0x80; } *inp = CBS_data(&body); + // If |indefinite|, |header_len| and |CBS_len(&body)| were equal prior to the + // |CBS_skip(&body, header_len)| call above and |CBS_len(&body)| is now zero. *out_len = CBS_len(&body); *out_tag = tag_number; *out_class = tag_class; return constructed; } +int ASN1_get_object(const unsigned char **inp, long *out_len, int *out_tag, + int *out_class, long in_len) { + return asn1_get_object_maybe_indefinite(inp, out_len, out_tag, out_class, in_len, + /*indefinite_ok=*/1); +} + // class 0 is constructed constructed == 2 for indefinite length constructed void ASN1_put_object(unsigned char **pp, int constructed, int length, int tag, int xclass) { diff --git a/crypto/asn1/asn1_test.cc b/crypto/asn1/asn1_test.cc index 0858bee1cd..91ba691e33 100644 --- a/crypto/asn1/asn1_test.cc +++ b/crypto/asn1/asn1_test.cc @@ -2291,20 +2291,69 @@ TEST(ASN1Test, PrintASN1Object) { std::string(reinterpret_cast(bio_data), bio_len)); } -TEST(ASN1Test, GetObject) { - // The header is valid, but there are not enough bytes for the length. - static const uint8_t kTruncated[] = {0x30, 0x01}; - const uint8_t *ptr = kTruncated; +const struct GetObjectTestData { + std::vector in; + const int expected_return; + const int expected_tag; + const int expected_class; + const int expected_length; +} kGetObjectTests[] = { + // OpenSSL succeeds for all test cases below. + // The header is valid, but there are not enough bytes for the length. + {{0x30, 0x01}, 0x80, 0, 0, 0}, + {{0x30, 0x80, 0x00, 0x00}, 0x21, 0x10, 0, 0}, + {{0x00, 0x00}, 0x00, 0x00, 0x00, 0}, + {{0x01, 0x00}, 0x00, 0x01, 0x00, 0}, + {{0x41, 0x00}, 0x00, 0x01, 0x40, 0}, + {{0x81, 0x00}, 0x00, 0x01, 0x80, 0}, + {{0xC1, 0x00}, 0x00, 0x01, 0xC0, 0}, + {{0x1F, 0x20, 0x00}, 0x00, 0x20, 0x00, 0}, + // Rejected to avoid ambiguity with V_ASN1_NEG. Ruby has a test case expecting this to succeed. + {{0x1F, 0xC0, 0x20, 0x00}, 0x80, 0x00, 0x00, 0}, + {{0x41, 0x02, 0xAB, 0xCD}, 0x00, 0x01, 0x40, 2}, + {{0x61, 0x00}, 0x20, 0x01, 0x40, 0}, + {{0x61, 0x80, 0xC2, 0x02, 0xAB, 0xCD, 0x00, 0x00}, 0x21, 0x01, 0x40, 0}, +}; + +static void verifyGetObject(GetObjectTestData t) { long length; int tag; int tag_class; - EXPECT_EQ(0x80, ASN1_get_object(&ptr, &length, &tag, &tag_class, - sizeof(kTruncated))); - static const uint8_t kIndefinite[] = {0x30, 0x80, 0x00, 0x00}; - ptr = kIndefinite; - EXPECT_EQ(0x80, ASN1_get_object(&ptr, &length, &tag, &tag_class, - sizeof(kIndefinite))); + SCOPED_TRACE(Bytes(t.in)); + const uint8_t *ptr = t.in.data(); + EXPECT_EQ(t.expected_return, + ASN1_get_object(&ptr, &length, &tag, &tag_class, t.in.size())); + if (!(t.expected_return & 0x80)) { + EXPECT_EQ(t.expected_length, length); + EXPECT_EQ(t.expected_tag, tag); + EXPECT_EQ(t.expected_class, tag_class); + } +} + +TEST(ASN1Test, GetObject) { + for (const auto &t : kGetObjectTests) { + verifyGetObject(t); + } + + { + GetObjectTestData test_case{ {0x41, 0x81, 0x80}, 0x00, 0x01, 0x40, 128 }; + for(int i = 0; i < 64; i++) { + test_case.in.push_back(0xAB); + test_case.in.push_back(0xCD); + } + verifyGetObject(test_case); + } + + { + GetObjectTestData test_case{ {0x41, 0x82, 0x01, 0x00}, 0x00, 0x01, 0x40, 256 }; + for(int i = 0; i < 128; i++) { + test_case.in.push_back(0xAB); + test_case.in.push_back(0xCD); + } + verifyGetObject(test_case); + } + } template diff --git a/crypto/asn1/internal.h b/crypto/asn1/internal.h index 6a632c2e01..304b4b5a26 100644 --- a/crypto/asn1/internal.h +++ b/crypto/asn1/internal.h @@ -217,6 +217,21 @@ void asn1_type_cleanup(ASN1_TYPE *a); // ASN.1 PrintableString, and zero otherwise. int asn1_is_printable(uint32_t value); +// asn1_get_object_maybe_indefinite parses an ASN.1 header, including tag, class, +// and length information. The tag number is written to |*out_tag|. The class is +// written to |*out_class|. If the tag is not indefinite, the content length is +// written to |*out_len|. |inp| is advanced past the header in the input buffer. +// +// If |indefinite_ok| is non-zero, indefinite-length encoding and universal tags +// are allowed, otherwise these will produce errors. +// +// The return value may have the following bits set: +// * 0x80: error occurred while parsing. +// * 0x20: the encoding is constructed, not primitive. +// * 0x01: indefinite-length constructed encoding. +int asn1_get_object_maybe_indefinite(const unsigned char **inp, long *out_len, int *out_tag, + int *out_class, long in_len, int indefinite_ok); + // asn1_bit_string_length returns the number of bytes in |str| and sets // |*out_padding_bits| to the number of padding bits. // diff --git a/crypto/asn1/tasn_dec.c b/crypto/asn1/tasn_dec.c index e7baf635e3..97214b5986 100644 --- a/crypto/asn1/tasn_dec.c +++ b/crypto/asn1/tasn_dec.c @@ -867,7 +867,7 @@ static int asn1_check_tlen(long *olen, int *otag, unsigned char *oclass, const unsigned char *p; p = *in; - i = ASN1_get_object(&p, &plen, &ptag, &pclass, len); + i = asn1_get_object_maybe_indefinite(&p, &plen, &ptag, &pclass, len, /*indefinite_ok=*/0); if (i & 0x80) { OPENSSL_PUT_ERROR(ASN1, ASN1_R_BAD_OBJECT_HEADER); return 0; diff --git a/crypto/bytestring/cbs.c b/crypto/bytestring/cbs.c index 5110be4321..5bb5f2e793 100644 --- a/crypto/bytestring/cbs.c +++ b/crypto/bytestring/cbs.c @@ -277,7 +277,7 @@ static int parse_base128_integer(CBS *cbs, uint64_t *out) { return 1; } -static int parse_asn1_tag(CBS *cbs, CBS_ASN1_TAG *out) { +static int parse_asn1_tag(CBS *cbs, CBS_ASN1_TAG *out, int universal_tag_ok) { uint8_t tag_byte; if (!CBS_get_u8(cbs, &tag_byte)) { return 0; @@ -308,7 +308,7 @@ static int parse_asn1_tag(CBS *cbs, CBS_ASN1_TAG *out) { // Tag [UNIVERSAL 0] is reserved for use by the encoding. Reject it here to // avoid some ambiguity around ANY values and BER indefinite-length EOCs. See // https://crbug.com/boringssl/455. - if ((tag & ~CBS_ASN1_CONSTRUCTED) == 0) { + if (!universal_tag_ok && (tag & ~CBS_ASN1_CONSTRUCTED) == 0) { return 0; } @@ -316,9 +316,9 @@ static int parse_asn1_tag(CBS *cbs, CBS_ASN1_TAG *out) { return 1; } -static int cbs_get_any_asn1_element(CBS *cbs, CBS *out, CBS_ASN1_TAG *out_tag, +int cbs_get_any_asn1_element(CBS *cbs, CBS *out, CBS_ASN1_TAG *out_tag, size_t *out_header_len, int *out_ber_found, - int *out_indefinite, int ber_ok) { + int *out_indefinite, int ber_ok, int universal_tag_ok) { CBS header = *cbs; CBS throwaway; @@ -334,7 +334,7 @@ static int cbs_get_any_asn1_element(CBS *cbs, CBS *out, CBS_ASN1_TAG *out_tag, } CBS_ASN1_TAG tag; - if (!parse_asn1_tag(&header, &tag)) { + if (!parse_asn1_tag(&header, &tag, universal_tag_ok)) { return 0; } if (out_tag != NULL) { @@ -434,7 +434,7 @@ int CBS_get_any_asn1(CBS *cbs, CBS *out, CBS_ASN1_TAG *out_tag) { int CBS_get_any_asn1_element(CBS *cbs, CBS *out, CBS_ASN1_TAG *out_tag, size_t *out_header_len) { return cbs_get_any_asn1_element(cbs, out, out_tag, out_header_len, NULL, NULL, - /*ber_ok=*/0); + /*ber_ok=*/0, /*universal_tag_ok=*/0); } int CBS_get_any_ber_asn1_element(CBS *cbs, CBS *out, CBS_ASN1_TAG *out_tag, @@ -444,7 +444,7 @@ int CBS_get_any_ber_asn1_element(CBS *cbs, CBS *out, CBS_ASN1_TAG *out_tag, return cbs_get_any_asn1_element( cbs, out, out_tag, out_header_len, out_ber_found ? out_ber_found : &ber_found_temp, out_indefinite, - /*ber_ok=*/1); + /*ber_ok=*/1, /*universal_tag_ok=*/0); } static int cbs_get_asn1(CBS *cbs, CBS *out, CBS_ASN1_TAG tag_value, @@ -481,7 +481,7 @@ int CBS_get_asn1_element(CBS *cbs, CBS *out, CBS_ASN1_TAG tag_value) { int CBS_peek_asn1_tag(const CBS *cbs, CBS_ASN1_TAG tag_value) { CBS copy = *cbs; CBS_ASN1_TAG actual_tag; - return parse_asn1_tag(©, &actual_tag) && tag_value == actual_tag; + return parse_asn1_tag(©, &actual_tag, /*universal_tag_ok=*/0) && tag_value == actual_tag; } int CBS_get_asn1_uint64(CBS *cbs, uint64_t *out) { diff --git a/crypto/bytestring/internal.h b/crypto/bytestring/internal.h index ace43f5152..7465e24373 100644 --- a/crypto/bytestring/internal.h +++ b/crypto/bytestring/internal.h @@ -88,6 +88,26 @@ OPENSSL_EXPORT int cbb_add_latin1(CBB *cbb, uint32_t u); OPENSSL_EXPORT int cbb_add_ucs2_be(CBB *cbb, uint32_t u); OPENSSL_EXPORT int cbb_add_utf32_be(CBB *cbb, uint32_t u); +// cbs_get_any_asn1_element parses an ASN.1 element from |cbs|. |*out_indefinite| +// is set to one if the length was indefinite and zero otherwise. On success, +// if the length is indefinite |out| will only contain the ASN.1 header, +// otherwise is will contain both the header and the content. If |out_tag| is +// not NULL, |*out_tag| is set to the element's tag number. If |out_header_len| +// is not NULL, |*out_header_len| is set to the length of the header. +// +// If |ber_ok| is one, BER encoding is permitted. In this case, if +// |out_ber_found| is not NULL and BER-specific encoding was found, +// |*out_ber_found| is set to one. If |out_indefinite| is not NULL and the +// element has indefinite-length, |*out_indefinite| is set to one. +// If |ber_ok| is zero, both |out_ber_found| and |out_indefinite| must be NULL. +// +// If |universal_tag_ok| is 1, universal tags are permitted. Otherwise, only +// context-specific tags are accepted. +// +// It returns one on success and zero on failure. +int cbs_get_any_asn1_element(CBS *cbs, CBS *out, CBS_ASN1_TAG *out_tag, + size_t *out_header_len, int *out_ber_found, + int *out_indefinite, int ber_ok, int universal_tag_ok); #if defined(__cplusplus) } // extern C