Allow ASN1_get_object to parse indefinite and universal

justsmth · Nov 14, 2024 · ed62a80 · ed62a80
1 parent 13f467e
commit ed62a80
Show file tree

Hide file tree

Showing 6 changed files with 40 additions and 16 deletions.
diff --git 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 ? 0x21 : ((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
@@ -2303,7 +2303,8 @@ TEST(ASN1Test, GetObject) {
 
   static const uint8_t kIndefinite[] = {0x30, 0x80, 0x00, 0x00};
   ptr = kIndefinite;
-  EXPECT_EQ(0x80, ASN1_get_object(&ptr, &length, &tag, &tag_class,
+  // 0x21 indicates the indefinite form
+  EXPECT_EQ(0x21, ASN1_get_object(&ptr, &length, &tag, &tag_class,
                                   sizeof(kIndefinite)));
 }
 

diff --git a/crypto/asn1/internal.h b/crypto/asn1/internal.h
@@ -217,6 +217,10 @@ void asn1_type_cleanup(ASN1_TYPE *a);
 // ASN.1 PrintableString, and zero otherwise.
 int asn1_is_printable(uint32_t value);
 
+// TODO: Documentation
+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
@@ -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
@@ -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,17 +308,17 @@ 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;
   }
 
   *out = tag;
   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(&copy, &actual_tag) && tag_value == actual_tag;
+  return parse_asn1_tag(&copy, &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
@@ -88,6 +88,10 @@ 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);
 
+// TODO: Documentation
+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