Revert "Support high tag numbers in CBS/CBB."

This reverts commit 66801feb175599a6d1eb3845eb7ce0ca84551fb5. This
turned out to break a lot more than expected. Hopefully we can reland it
soon, but we need to fix up some consumers first.

Note due to work that went in later, this is not a trivial revert and
should be re-reviewed.

(cherry picked from commit 2fc4f362cdaab103241a6a3ca1bf16778944f36b)

Change-Id: I5c26e2f8b65bcabdf4fe2b028486a42aad10ad71
Reviewed-on: https://boringssl-review.googlesource.com/23904
Reviewed-by: Steven Valdez <svaldez@google.com>

6 files changed, 88 insertions, 225 deletions

diff --git a/crypto/bytestring/ber.c b/crypto/bytestring/ber.c
index bb5e17c..4dc94f6 100644
--- a/crypto/bytestring/ber.c
+++ b/crypto/bytestring/ber.c
@@ -29,7 +29,10 @@ static const unsigned kMaxDepth = 2048;
 // is_string_type returns one if |tag| is a string type and zero otherwise. It
 // ignores the constructed bit.
 static int is_string_type(unsigned tag) {
-  switch (tag & ~CBS_ASN1_CONSTRUCTED) {
+  if ((tag & 0xc0) != 0) {
+    return 0;
+  }
+  switch (tag & 0x1f) {
     case CBS_ASN1_BITSTRING:
     case CBS_ASN1_OCTETSTRING:
     case CBS_ASN1_UTF8STRING:
diff --git a/crypto/bytestring/bytestring_test.cc b/crypto/bytestring/bytestring_test.cc
index 6ce7035..e396f15 100644
--- a/crypto/bytestring/bytestring_test.cc
+++ b/crypto/bytestring/bytestring_test.cc
@@ -123,27 +123,27 @@ TEST(CBSTest, GetASN1) {
   uint64_t value;
 
   CBS_init(&data, kData1, sizeof(kData1));
-  EXPECT_FALSE(CBS_peek_asn1_tag(&data, CBS_ASN1_BOOLEAN));
-  EXPECT_TRUE(CBS_peek_asn1_tag(&data, CBS_ASN1_SEQUENCE));
+  EXPECT_FALSE(CBS_peek_asn1_tag(&data, 0x1));
+  EXPECT_TRUE(CBS_peek_asn1_tag(&data, 0x30));
 
-  ASSERT_TRUE(CBS_get_asn1(&data, &contents, CBS_ASN1_SEQUENCE));
+  ASSERT_TRUE(CBS_get_asn1(&data, &contents, 0x30));
   EXPECT_EQ(Bytes("\x01\x02"), Bytes(CBS_data(&contents), CBS_len(&contents)));
 
   CBS_init(&data, kData2, sizeof(kData2));
   // data is truncated
-  EXPECT_FALSE(CBS_get_asn1(&data, &contents, CBS_ASN1_SEQUENCE));
+  EXPECT_FALSE(CBS_get_asn1(&data, &contents, 0x30));
 
   CBS_init(&data, kData3, sizeof(kData3));
   // zero byte length of length
-  EXPECT_FALSE(CBS_get_asn1(&data, &contents, CBS_ASN1_SEQUENCE));
+  EXPECT_FALSE(CBS_get_asn1(&data, &contents, 0x30));
 
   CBS_init(&data, kData4, sizeof(kData4));
   // long form mistakenly used.
-  EXPECT_FALSE(CBS_get_asn1(&data, &contents, CBS_ASN1_SEQUENCE));
+  EXPECT_FALSE(CBS_get_asn1(&data, &contents, 0x30));
 
   CBS_init(&data, kData5, sizeof(kData5));
   // length takes too many bytes.
-  EXPECT_FALSE(CBS_get_asn1(&data, &contents, CBS_ASN1_SEQUENCE));
+  EXPECT_FALSE(CBS_get_asn1(&data, &contents, 0x30));
 
   CBS_init(&data, kData1, sizeof(kData1));
   // wrong tag.
@@ -151,72 +151,56 @@ TEST(CBSTest, GetASN1) {
 
   CBS_init(&data, NULL, 0);
   // peek at empty data.
-  EXPECT_FALSE(CBS_peek_asn1_tag(&data, CBS_ASN1_SEQUENCE));
+  EXPECT_FALSE(CBS_peek_asn1_tag(&data, 0x30));
 
   CBS_init(&data, NULL, 0);
   // optional elements at empty data.
-  ASSERT_TRUE(CBS_get_optional_asn1(
-      &data, &contents, &present,
-      CBS_ASN1_CONTEXT_SPECIFIC | CBS_ASN1_CONSTRUCTED | 0));
+  ASSERT_TRUE(CBS_get_optional_asn1(&data, &contents, &present, 0xa0));
   EXPECT_FALSE(present);
-  ASSERT_TRUE(CBS_get_optional_asn1_octet_string(
-      &data, &contents, &present,
-      CBS_ASN1_CONTEXT_SPECIFIC | CBS_ASN1_CONSTRUCTED | 0));
+  ASSERT_TRUE(
+      CBS_get_optional_asn1_octet_string(&data, &contents, &present, 0xa0));
   EXPECT_FALSE(present);
   EXPECT_EQ(0u, CBS_len(&contents));
-  ASSERT_TRUE(CBS_get_optional_asn1_octet_string(
-      &data, &contents, NULL,
-      CBS_ASN1_CONTEXT_SPECIFIC | CBS_ASN1_CONSTRUCTED | 0));
+  ASSERT_TRUE(CBS_get_optional_asn1_octet_string(&data, &contents, NULL, 0xa0));
   EXPECT_EQ(0u, CBS_len(&contents));
-  ASSERT_TRUE(CBS_get_optional_asn1_uint64(
-      &data, &value, CBS_ASN1_CONTEXT_SPECIFIC | CBS_ASN1_CONSTRUCTED | 0, 42));
+  ASSERT_TRUE(CBS_get_optional_asn1_uint64(&data, &value, 0xa0, 42));
   EXPECT_EQ(42u, value);
 
   CBS_init(&data, kData6, sizeof(kData6));
   // optional element.
-  ASSERT_TRUE(CBS_get_optional_asn1(
-      &data, &contents, &present,
-      CBS_ASN1_CONTEXT_SPECIFIC | CBS_ASN1_CONSTRUCTED | 0));
+  ASSERT_TRUE(CBS_get_optional_asn1(&data, &contents, &present, 0xa0));
   EXPECT_FALSE(present);
-  ASSERT_TRUE(CBS_get_optional_asn1(
-      &data, &contents, &present,
-      CBS_ASN1_CONTEXT_SPECIFIC | CBS_ASN1_CONSTRUCTED | 1));
+  ASSERT_TRUE(CBS_get_optional_asn1(&data, &contents, &present, 0xa1));
   EXPECT_TRUE(present);
   EXPECT_EQ(Bytes("\x04\x01\x01"),
             Bytes(CBS_data(&contents), CBS_len(&contents)));
 
   CBS_init(&data, kData6, sizeof(kData6));
   // optional octet string.
-  ASSERT_TRUE(CBS_get_optional_asn1_octet_string(
-      &data, &contents, &present,
-      CBS_ASN1_CONTEXT_SPECIFIC | CBS_ASN1_CONSTRUCTED | 0));
+  ASSERT_TRUE(
+      CBS_get_optional_asn1_octet_string(&data, &contents, &present, 0xa0));
   EXPECT_FALSE(present);
   EXPECT_EQ(0u, CBS_len(&contents));
-  ASSERT_TRUE(CBS_get_optional_asn1_octet_string(
-      &data, &contents, &present,
-      CBS_ASN1_CONTEXT_SPECIFIC | CBS_ASN1_CONSTRUCTED | 1));
+  ASSERT_TRUE(
+      CBS_get_optional_asn1_octet_string(&data, &contents, &present, 0xa1));
   EXPECT_TRUE(present);
   EXPECT_EQ(Bytes("\x01"), Bytes(CBS_data(&contents), CBS_len(&contents)));
 
   CBS_init(&data, kData7, sizeof(kData7));
   // invalid optional octet string.
-  EXPECT_FALSE(CBS_get_optional_asn1_octet_string(
-      &data, &contents, &present,
-      CBS_ASN1_CONTEXT_SPECIFIC | CBS_ASN1_CONSTRUCTED | 1));
+  EXPECT_FALSE(
+      CBS_get_optional_asn1_octet_string(&data, &contents, &present, 0xa1));
 
   CBS_init(&data, kData8, sizeof(kData8));
   // optional integer.
-  ASSERT_TRUE(CBS_get_optional_asn1_uint64(
-      &data, &value, CBS_ASN1_CONTEXT_SPECIFIC | CBS_ASN1_CONSTRUCTED | 0, 42));
+  ASSERT_TRUE(CBS_get_optional_asn1_uint64(&data, &value, 0xa0, 42));
   EXPECT_EQ(42u, value);
-  ASSERT_TRUE(CBS_get_optional_asn1_uint64(
-      &data, &value, CBS_ASN1_CONTEXT_SPECIFIC | CBS_ASN1_CONSTRUCTED | 1, 42));
+  ASSERT_TRUE(CBS_get_optional_asn1_uint64(&data, &value, 0xa1, 42));
   EXPECT_EQ(1u, value);
 
   CBS_init(&data, kData9, sizeof(kData9));
   // invalid optional integer.
-  EXPECT_FALSE(CBS_get_optional_asn1_uint64(
-      &data, &value, CBS_ASN1_CONTEXT_SPECIFIC | CBS_ASN1_CONSTRUCTED | 1, 42));
+  EXPECT_FALSE(CBS_get_optional_asn1_uint64(&data, &value, 0xa1, 42));
 
   unsigned tag;
   CBS_init(&data, kData1, sizeof(kData1));
@@ -233,54 +217,6 @@ TEST(CBSTest, GetASN1) {
             Bytes(CBS_data(&contents), CBS_len(&contents)));
 }
 
-TEST(CBSTest, ParseASN1Tag) {
-  const struct {
-    bool ok;
-    unsigned tag;
-    std::vector<uint8_t> in;
-  } kTests[] = {
-      {true, CBS_ASN1_SEQUENCE, {0x30, 0}},
-      {true, CBS_ASN1_CONTEXT_SPECIFIC | CBS_ASN1_CONSTRUCTED | 4, {0xa4, 0}},
-      {true, CBS_ASN1_APPLICATION | 30, {0x5e, 0}},
-      {true, CBS_ASN1_APPLICATION | 31, {0x5f, 0x1f, 0}},
-      {true, CBS_ASN1_APPLICATION | 32, {0x5f, 0x20, 0}},
-      {true,
-       CBS_ASN1_PRIVATE | CBS_ASN1_CONSTRUCTED | 0x1fffffff,
-       {0xff, 0x81, 0xff, 0xff, 0xff, 0x7f, 0}},
-      // Tag number fits in unsigned but not |CBS_ASN1_TAG_NUMBER_MASK|.
-      {false, 0, {0xff, 0x82, 0xff, 0xff, 0xff, 0x7f, 0}},
-      // Tag number does not fit in unsigned.
-      {false, 0, {0xff, 0x90, 0x80, 0x80, 0x80, 0, 0}},
-      // Tag number is not minimally-encoded
-      {false, 0, {0x5f, 0x80, 0x1f, 0}},
-      // Tag number should have used short form.
-      {false, 0, {0x5f, 0x80, 0x1e, 0}},
-  };
-  for (const auto &t : kTests) {
-    SCOPED_TRACE(Bytes(t.in));
-    unsigned tag;
-    CBS cbs, child;
-    CBS_init(&cbs, t.in.data(), t.in.size());
-    ASSERT_EQ(t.ok, !!CBS_get_any_asn1(&cbs, &child, &tag));
-    if (t.ok) {
-      EXPECT_EQ(t.tag, tag);
-      EXPECT_EQ(0u, CBS_len(&child));
-      EXPECT_EQ(0u, CBS_len(&cbs));
-
-      CBS_init(&cbs, t.in.data(), t.in.size());
-      EXPECT_TRUE(CBS_peek_asn1_tag(&cbs, t.tag));
-      EXPECT_FALSE(CBS_peek_asn1_tag(&cbs, t.tag + 1));
-
-      EXPECT_TRUE(CBS_get_asn1(&cbs, &child, t.tag));
-      EXPECT_EQ(0u, CBS_len(&child));
-      EXPECT_EQ(0u, CBS_len(&cbs));
-
-      CBS_init(&cbs, t.in.data(), t.in.size());
-      EXPECT_FALSE(CBS_get_asn1(&cbs, &child, t.tag + 1));
-    }
-  }
-}
-
 TEST(CBSTest, GetOptionalASN1Bool) {
   static const uint8_t kTrue[] = {0x0a, 3, CBS_ASN1_BOOLEAN, 1, 0xff};
   static const uint8_t kFalse[] = {0x0a, 3, CBS_ASN1_BOOLEAN, 1, 0x00};
@@ -480,42 +416,15 @@ TEST(CBBTest, Misuse) {
 }
 
 TEST(CBBTest, ASN1) {
-  static const uint8_t kExpected[] = {
-      // SEQUENCE { 1 2 3 }
-      0x30, 3, 1, 2, 3,
-      // [4 CONSTRUCTED] { 4 5 6 }
-      0xa4, 3, 4, 5, 6,
-      // [APPLICATION 30 PRIMITIVE] { 7 8 9 }
-      0x5e, 3, 7, 8, 9,
-      // [APPLICATION 31 PRIMITIVE] { 10 11 12 }
-      0x5f, 0x1f, 3, 10, 11, 12,
-      // [PRIVATE 2^29-1 CONSTRUCTED] { 13 14 15 }
-      0xff, 0x81, 0xff, 0xff, 0xff, 0x7f, 3, 13, 14, 15,
-  };
+  static const uint8_t kExpected[] = {0x30, 3, 1, 2, 3};
   uint8_t *buf;
   size_t buf_len;
   bssl::ScopedCBB cbb;
   CBB contents, inner_contents;
 
   ASSERT_TRUE(CBB_init(cbb.get(), 0));
-  ASSERT_TRUE(CBB_add_asn1(cbb.get(), &contents, CBS_ASN1_SEQUENCE));
+  ASSERT_TRUE(CBB_add_asn1(cbb.get(), &contents, 0x30));
   ASSERT_TRUE(CBB_add_bytes(&contents, (const uint8_t *)"\x01\x02\x03", 3));
-  ASSERT_TRUE(
-      CBB_add_asn1(cbb.get(), &contents,
-                   CBS_ASN1_CONTEXT_SPECIFIC | CBS_ASN1_CONSTRUCTED | 4));
-  ASSERT_TRUE(CBB_add_bytes(&contents, (const uint8_t *)"\x04\x05\x06", 3));
-  ASSERT_TRUE(
-      CBB_add_asn1(cbb.get(), &contents,
-                   CBS_ASN1_APPLICATION | 30));
-  ASSERT_TRUE(CBB_add_bytes(&contents, (const uint8_t *)"\x07\x08\x09", 3));
-  ASSERT_TRUE(
-      CBB_add_asn1(cbb.get(), &contents,
-                   CBS_ASN1_APPLICATION | 31));
-  ASSERT_TRUE(CBB_add_bytes(&contents, (const uint8_t *)"\x0a\x0b\x0c", 3));
-  ASSERT_TRUE(
-      CBB_add_asn1(cbb.get(), &contents,
-                   CBS_ASN1_PRIVATE | CBS_ASN1_CONSTRUCTED | 0x1fffffff));
-  ASSERT_TRUE(CBB_add_bytes(&contents, (const uint8_t *)"\x0d\x0e\x0f", 3));
   ASSERT_TRUE(CBB_finish(cbb.get(), &buf, &buf_len));
   bssl::UniquePtr<uint8_t> scoper(buf);
 
@@ -523,7 +432,7 @@ TEST(CBBTest, ASN1) {
 
   std::vector<uint8_t> test_data(100000, 0x42);
   ASSERT_TRUE(CBB_init(cbb.get(), 0));
-  ASSERT_TRUE(CBB_add_asn1(cbb.get(), &contents, CBS_ASN1_SEQUENCE));
+  ASSERT_TRUE(CBB_add_asn1(cbb.get(), &contents, 0x30));
   ASSERT_TRUE(CBB_add_bytes(&contents, test_data.data(), 130));
   ASSERT_TRUE(CBB_finish(cbb.get(), &buf, &buf_len));
   scoper.reset(buf);
@@ -533,7 +442,7 @@ TEST(CBBTest, ASN1) {
   EXPECT_EQ(Bytes(test_data.data(), 130), Bytes(buf + 3, 130));
 
   ASSERT_TRUE(CBB_init(cbb.get(), 0));
-  ASSERT_TRUE(CBB_add_asn1(cbb.get(), &contents, CBS_ASN1_SEQUENCE));
+  ASSERT_TRUE(CBB_add_asn1(cbb.get(), &contents, 0x30));
   ASSERT_TRUE(CBB_add_bytes(&contents, test_data.data(), 1000));
   ASSERT_TRUE(CBB_finish(cbb.get(), &buf, &buf_len));
   scoper.reset(buf);
@@ -543,8 +452,8 @@ TEST(CBBTest, ASN1) {
   EXPECT_EQ(Bytes(test_data.data(), 1000), Bytes(buf + 4, 1000));
 
   ASSERT_TRUE(CBB_init(cbb.get(), 0));
-  ASSERT_TRUE(CBB_add_asn1(cbb.get(), &contents, CBS_ASN1_SEQUENCE));
-  ASSERT_TRUE(CBB_add_asn1(&contents, &inner_contents, CBS_ASN1_SEQUENCE));
+  ASSERT_TRUE(CBB_add_asn1(cbb.get(), &contents, 0x30));
+  ASSERT_TRUE(CBB_add_asn1(&contents, &inner_contents, 0x30));
   ASSERT_TRUE(CBB_add_bytes(&inner_contents, test_data.data(), 100000));
   ASSERT_TRUE(CBB_finish(cbb.get(), &buf, &buf_len));
   scoper.reset(buf);
@@ -581,12 +490,6 @@ TEST(CBSTest, BerConvert) {
   static const uint8_t kIndefBER[] = {0x30, 0x80, 0x01, 0x01, 0x02, 0x00, 0x00};
   static const uint8_t kIndefDER[] = {0x30, 0x03, 0x01, 0x01, 0x02};
 
-  // kIndefBER2 contains a constructed [APPLICATION 31] with an indefinite
-  // length.
-  static const uint8_t kIndefBER2[] = {0x7f, 0x1f, 0x80, 0x01,
-                                       0x01, 0x02, 0x00, 0x00};
-  static const uint8_t kIndefDER2[] = {0x7f, 0x1f, 0x03, 0x01, 0x01, 0x02};
-
   // kOctetStringBER contains an indefinite length OCTET STRING with two parts.
   // These parts need to be concatenated in DER form.
   static const uint8_t kOctetStringBER[] = {0x24, 0x80, 0x04, 0x02, 0,    1,
@@ -631,8 +534,6 @@ TEST(CBSTest, BerConvert) {
                    sizeof(kSimpleBER));
   ExpectBerConvert("kIndefBER", kIndefDER, sizeof(kIndefDER), kIndefBER,
                    sizeof(kIndefBER));
-  ExpectBerConvert("kIndefBER2", kIndefDER2, sizeof(kIndefDER2), kIndefBER2,
-                   sizeof(kIndefBER2));
   ExpectBerConvert("kOctetStringBER", kOctetStringDER, sizeof(kOctetStringDER),
                    kOctetStringBER, sizeof(kOctetStringBER));
   ExpectBerConvert("kNSSBER", kNSSDER, sizeof(kNSSDER), kNSSBER,
diff --git a/crypto/bytestring/cbb.c b/crypto/bytestring/cbb.c
index 5a1c45b..2853509 100644
--- a/crypto/bytestring/cbb.c
+++ b/crypto/bytestring/cbb.c
@@ -356,20 +356,17 @@ static int add_base128_integer(CBB *cbb, uint32_t v) {
 }
 
 int CBB_add_asn1(CBB *cbb, CBB *out_contents, unsigned tag) {
-  if (!CBB_flush(cbb)) {
+  if (tag > 0xff ||
+      (tag & 0x1f) == 0x1f) {
+    // Long form identifier octets are not supported. Further, all current valid
+    // tag serializations are 8 bits.
+    cbb->base->error = 1;
     return 0;
   }
 
-  // Split the tag into leading bits and tag number.
-  uint8_t tag_bits = (tag >> CBS_ASN1_TAG_SHIFT) & 0xe0;
-  unsigned tag_number = tag & CBS_ASN1_TAG_NUMBER_MASK;
-  if (tag_number >= 0x1f) {
-    // Set all the bits in the tag number to signal high tag number form.
-    if (!CBB_add_u8(cbb, tag_bits | 0x1f) ||
-        !add_base128_integer(cbb, tag_number)) {
-      return 0;
-    }
-  } else if (!CBB_add_u8(cbb, tag_bits | tag_number)) {
+  if (!CBB_flush(cbb) ||
+      // |tag|'s representation matches the DER encoding.
+      !CBB_add_u8(cbb, (uint8_t)tag)) {
     return 0;
   }
 
diff --git a/crypto/bytestring/cbs.c b/crypto/bytestring/cbs.c
index d96371c..ec495d2 100644
--- a/crypto/bytestring/cbs.c
+++ b/crypto/bytestring/cbs.c
@@ -175,53 +175,9 @@ int CBS_get_u24_length_prefixed(CBS *cbs, CBS *out) {
   return cbs_get_length_prefixed(cbs, out, 3);
 }
 
-static int parse_asn1_tag(CBS *cbs, unsigned *out) {
-  uint8_t tag_byte;
-  if (!CBS_get_u8(cbs, &tag_byte)) {
-    return 0;
-  }
-
-  // ITU-T X.690 section 8.1.2.3 specifies the format for identifiers with a tag
-  // number no greater than 30.
-  //
-  // If the number portion is 31 (0x1f, the largest value that fits in the
-  // allotted bits), then the tag is more than one byte long and the
-  // continuation bytes contain the tag number. This parser only supports tag
-  // numbers less than 31 (and thus single-byte tags).
-  unsigned tag = ((unsigned)tag_byte & 0xe0) << CBS_ASN1_TAG_SHIFT;
-  unsigned tag_number = tag_byte & 0x1f;
-  if (tag_number == 0x1f) {
-    tag_number = 0;
-    for (;;) {
-      if (!CBS_get_u8(cbs, &tag_byte) ||
-          ((tag_number << 7) >> 7) != tag_number) {
-        return 0;
-      }
-      tag_number = (tag_number << 7) | (tag_byte & 0x7f);
-      // The tag must be represented in the minimal number of bytes.
-      if (tag_number == 0) {
-        return 0;
-      }
-      if ((tag_byte & 0x80) == 0) {
-        break;
-      }
-    }
-    if (// Check the tag number is within our supported bounds.
-        tag_number > CBS_ASN1_TAG_NUMBER_MASK ||
-        // Small tag numbers should have used low tag number form.
-        tag_number < 0x1f) {
-      return 0;
-    }
-  }
-
-  tag |= tag_number;
-
-  *out = tag;
-  return 1;
-}
-
 static int cbs_get_any_asn1_element(CBS *cbs, CBS *out, unsigned *out_tag,
                                     size_t *out_header_len, int ber_ok) {
+  uint8_t tag, length_byte;
   CBS header = *cbs;
   CBS throwaway;
 
@@ -229,29 +185,34 @@ static int cbs_get_any_asn1_element(CBS *cbs, CBS *out, unsigned *out_tag,
     out = &throwaway;
   }
 
-  unsigned tag;
-  if (!parse_asn1_tag(&header, &tag)) {
+  if (!CBS_get_u8(&header, &tag) ||
+      !CBS_get_u8(&header, &length_byte)) {
     return 0;
   }
-  if (out_tag != NULL) {
-    *out_tag = tag;
-  }
 
-  uint8_t length_byte;
-  if (!CBS_get_u8(&header, &length_byte)) {
+  // ITU-T X.690 section 8.1.2.3 specifies the format for identifiers with a tag
+  // number no greater than 30.
+  //
+  // If the number portion is 31 (0x1f, the largest value that fits in the
+  // allotted bits), then the tag is more than one byte long and the
+  // continuation bytes contain the tag number. This parser only supports tag
+  // numbers less than 31 (and thus single-byte tags).
+  if ((tag & 0x1f) == 0x1f) {
     return 0;
   }
 
-  size_t header_len = CBS_len(cbs) - CBS_len(&header);
+  if (out_tag != NULL) {
+    *out_tag = tag;
+  }
 
   size_t len;
   // The format for the length encoding is specified in ITU-T X.690 section
   // 8.1.3.
   if ((length_byte & 0x80) == 0) {
     // Short form length.
-    len = ((size_t) length_byte) + header_len;
+    len = ((size_t) length_byte) + 2;
     if (out_header_len != NULL) {
-      *out_header_len = header_len;
+      *out_header_len = 2;
     }
   } else {
     // The high bit indicate that this is the long form, while the next 7 bits
@@ -263,9 +224,9 @@ static int cbs_get_any_asn1_element(CBS *cbs, CBS *out, unsigned *out_tag,
     if (ber_ok && (tag & CBS_ASN1_CONSTRUCTED) != 0 && num_bytes == 0) {
       // indefinite length
       if (out_header_len != NULL) {
-        *out_header_len = header_len;
+        *out_header_len = 2;
       }
-      return CBS_get_bytes(cbs, out, header_len);
+      return CBS_get_bytes(cbs, out, 2);
     }
 
     // ITU-T X.690 clause 8.1.3.5.c specifies that the value 0xff shall not be
@@ -288,13 +249,13 @@ static int cbs_get_any_asn1_element(CBS *cbs, CBS *out, unsigned *out_tag,
       return 0;
     }
     len = len32;
-    if (len + header_len + num_bytes < len) {
+    if (len + 2 + num_bytes < len) {
       // Overflow.
       return 0;
     }
-    len += header_len + num_bytes;
+    len += 2 + num_bytes;
     if (out_header_len != NULL) {
-      *out_header_len = header_len + num_bytes;
+      *out_header_len = 2 + num_bytes;
     }
   }
 
@@ -362,10 +323,7 @@ int CBS_peek_asn1_tag(const CBS *cbs, unsigned tag_value) {
   if (CBS_len(cbs) < 1) {
     return 0;
   }
-
-  CBS copy = *cbs;
-  unsigned actual_tag;
-  return parse_asn1_tag(&copy, &actual_tag) && tag_value == actual_tag;
+  return CBS_data(cbs)[0] == tag_value;
 }
 
 int CBS_get_asn1_uint64(CBS *cbs, uint64_t *out) {
diff --git a/crypto/x509/x509_test.cc b/crypto/x509/x509_test.cc
index b4cecca..158fd45 100644
--- a/crypto/x509/x509_test.cc
+++ b/crypto/x509/x509_test.cc
@@ -835,7 +835,7 @@ TEST(X509Test, TestFromBuffer) {
 
   /* This ensures the X509 took a reference to |buf|, otherwise this will be a
    * reference to free memory and ASAN should notice. */
-  ASSERT_EQ(0x30, enc_pointer[0]);
+  ASSERT_EQ(CBS_ASN1_SEQUENCE, enc_pointer[0]);
 }
 
 TEST(X509Test, TestFromBufferWithTrailingData) {
diff --git a/include/openssl/bytestring.h b/include/openssl/bytestring.h
index abf8885..309c6a3 100644
--- a/include/openssl/bytestring.h
+++ b/include/openssl/bytestring.h
@@ -164,36 +164,34 @@ OPENSSL_EXPORT int CBS_get_u24_length_prefixed(CBS *cbs, CBS *out);
 #define CBS_ASN1_UNIVERSALSTRING 0x1cu
 #define CBS_ASN1_BMPSTRING 0x1eu
 
-// CBS_ASN1_TAG_SHIFT is how much the in-memory representation shifts the class
-// and constructed bits from the DER serialization. This allows representing tag
-// numbers beyond 31.
-//
-// Consumers must use the following constants to decompose or assemble tags.
-#define CBS_ASN1_TAG_SHIFT 24
-
 // CBS_ASN1_CONSTRUCTED may be ORed into a tag to toggle the constructed
 // bit. |CBS| and |CBB| APIs consider the constructed bit to be part of the
 // tag.
-#define CBS_ASN1_CONSTRUCTED (0x20u << CBS_ASN1_TAG_SHIFT)
+#define CBS_ASN1_CONSTRUCTED 0x20u
 
-// The following values specify the tag class and may be ORed into a tag number
-// to produce the final tag. If none is used, the tag will be UNIVERSAL.
-#define CBS_ASN1_UNIVERSAL (0u << CBS_ASN1_TAG_SHIFT)
-#define CBS_ASN1_APPLICATION (0x40u << CBS_ASN1_TAG_SHIFT)
-#define CBS_ASN1_CONTEXT_SPECIFIC (0x80u << CBS_ASN1_TAG_SHIFT)
-#define CBS_ASN1_PRIVATE (0xc0u << CBS_ASN1_TAG_SHIFT)
+// The following values specify the constructed bit or tag class and may be ORed
+// into a tag number to produce the final tag. If none is used, the tag will be
+// UNIVERSAL.
+//
+// Note that although they currently match the DER serialization, consumers must
+// use these bits rather than make assumptions about the representation. This is
+// to allow for tag numbers beyond 31 in the future.
+#define CBS_ASN1_APPLICATION 0x40u
+#define CBS_ASN1_CONTEXT_SPECIFIC 0x80u
+#define CBS_ASN1_PRIVATE 0xc0u
 
-// CBS_ASN1_CLASS_MASK may be ANDed with a tag to query its class. This will
-// give one of the four values above.
-#define CBS_ASN1_CLASS_MASK (0xc0u << CBS_ASN1_TAG_SHIFT)
+// CBS_ASN1_CLASS_MASK may be ANDed with a tag to query its class.
+#define CBS_ASN1_CLASS_MASK 0xc0u
 
 // CBS_ASN1_TAG_NUMBER_MASK may be ANDed with a tag to query its number.
-#define CBS_ASN1_TAG_NUMBER_MASK ((1u << (5 + CBS_ASN1_TAG_SHIFT)) - 1)
+#define CBS_ASN1_TAG_NUMBER_MASK 0x1fu
 
 // CBS_get_asn1 sets |*out| to the contents of DER-encoded, ASN.1 element (not
 // including tag and length bytes) and advances |cbs| over it. The ASN.1
 // element must match |tag_value|. It returns one on success and zero
 // on error.
+//
+// Tag numbers greater than 30 are not supported (i.e. short form only).
 OPENSSL_EXPORT int CBS_get_asn1(CBS *cbs, CBS *out, unsigned tag_value);
 
 // CBS_get_asn1_element acts like |CBS_get_asn1| but |out| will include the
@@ -211,12 +209,16 @@ OPENSSL_EXPORT int CBS_peek_asn1_tag(const CBS *cbs, unsigned tag_value);
 // (not including tag and length bytes), sets |*out_tag| to the tag number, and
 // advances |*cbs|. It returns one on success and zero on error. Either of |out|
 // and |out_tag| may be NULL to ignore the value.
+//
+// Tag numbers greater than 30 are not supported (i.e. short form only).
 OPENSSL_EXPORT int CBS_get_any_asn1(CBS *cbs, CBS *out, unsigned *out_tag);
 
 // CBS_get_any_asn1_element sets |*out| to contain the next ASN.1 element from
 // |*cbs| (including header bytes) and advances |*cbs|. It sets |*out_tag| to
 // the tag number and |*out_header_len| to the length of the ASN.1 header. Each
 // of |out|, |out_tag|, and |out_header_len| may be NULL to ignore the value.
+//
+// Tag numbers greater than 30 are not supported (i.e. short form only).
 OPENSSL_EXPORT int CBS_get_any_asn1_element(CBS *cbs, CBS *out,
                                             unsigned *out_tag,
                                             size_t *out_header_len);
@@ -394,7 +396,9 @@ OPENSSL_EXPORT int CBB_add_u24_length_prefixed(CBB *cbb, CBB *out_contents);
 
 // CBB_add_asn1 sets |*out_contents| to a |CBB| into which the contents of an
 // ASN.1 object can be written. The |tag| argument will be used as the tag for
-// the object. It returns one on success or zero on error.
+// the object. Passing in |tag| number 31 will return in an error since only
+// single octet identifiers are supported. It returns one on success or zero
+// on error.
 OPENSSL_EXPORT int CBB_add_asn1(CBB *cbb, CBB *out_contents, unsigned tag);
 
 // CBB_add_bytes appends |len| bytes from |data| to |cbb|. It returns one on