Make CBS_get_any_asn1_element accept only DER.

This change makes |CBS_get_any_asn1_element| only handle DER elements.
Another function, |CBS_get_any_ber_asn1_element| is exposed internally
for the cases where we need to process BER data.

Change-Id: I544141a1a3d7913986352a8fd9a6d00b9f282652
Reviewed-on: https://boringssl-review.googlesource.com/4994
Reviewed-by: Adam Langley <agl@google.com>

crypto/bytestring/ber.c

@@ -43,7 +43,7 @@ static int cbs_find_ber(CBS *orig_in, char *ber_found, unsigned depth) {
     unsigned tag;
     size_t header_len;
 
-    if (!CBS_get_any_asn1_element(&in, &contents, &tag, &header_len)) {
+    if (!CBS_get_any_ber_asn1_element(&in, &contents, &tag, &header_len)) {
       return 0;
     }
     if (CBS_len(&contents) == header_len &&
@@ -74,7 +74,7 @@ static char is_primitive_type(unsigned tag) {
 }
 
 /* is_eoc returns true if |header_len| and |contents|, as returned by
- * |CBS_get_any_asn1_element|, indicate an "end of contents" (EOC) value. */
+ * |CBS_get_any_ber_asn1_element|, indicate an "end of contents" (EOC) value. */
 static char is_eoc(size_t header_len, CBS *contents) {
   return header_len == 2 && CBS_len(contents) == 2 &&
          memcmp(CBS_data(contents), "\x00\x00", 2) == 0;
@@ -98,7 +98,7 @@ static int cbs_convert_ber(CBS *in, CBB *out, char squash_header,
     size_t header_len;
     CBB *out_contents, out_contents_storage;
 
-    if (!CBS_get_any_asn1_element(in, &contents, &tag, &header_len)) {
+    if (!CBS_get_any_ber_asn1_element(in, &contents, &tag, &header_len)) {
       return 0;
     }
     out_contents = out;
@@ -129,8 +129,8 @@ static int cbs_convert_ber(CBS *in, CBB *out, char squash_header,
           size_t inner_header_len;
 
           CBS_init(&in_copy, CBS_data(in), CBS_len(in));
-          if (!CBS_get_any_asn1_element(&in_copy, &inner_contents, &inner_tag,
+          if (!CBS_get_any_ber_asn1_element(&in_copy, &inner_contents,
-                                        &inner_header_len)) {
+                                            &inner_tag, &inner_header_len)) {
             return 0;
           }
           if (CBS_len(&inner_contents) > inner_header_len &&

crypto/bytestring/cbs.c

@@ -157,8 +157,8 @@ int CBS_get_u24_length_prefixed(CBS *cbs, CBS *out) {
   return cbs_get_length_prefixed(cbs, out, 3);
 }
 
-int CBS_get_any_asn1_element(CBS *cbs, CBS *out, unsigned *out_tag,
+static int cbs_get_any_asn1_element(CBS *cbs, CBS *out, unsigned *out_tag,
-                             size_t *out_header_len) {
+                                    size_t *out_header_len, int ber_ok) {
   uint8_t tag, length_byte;
   CBS header = *cbs;
   CBS throwaway;
@@ -193,7 +193,7 @@ int CBS_get_any_asn1_element(CBS *cbs, CBS *out, unsigned *out_tag,
     const size_t num_bytes = length_byte & 0x7f;
     uint32_t len32;
 
-    if ((tag & CBS_ASN1_CONSTRUCTED) != 0 && num_bytes == 0) {
+    if (ber_ok && (tag & CBS_ASN1_CONSTRUCTED) != 0 && num_bytes == 0) {
       /* indefinite length */
       if (out_header_len != NULL) {
         *out_header_len = 2;
@@ -229,6 +229,18 @@ int CBS_get_any_asn1_element(CBS *cbs, CBS *out, unsigned *out_tag,
   return CBS_get_bytes(cbs, out, len);
 }
 
+int CBS_get_any_asn1_element(CBS *cbs, CBS *out, unsigned *out_tag,
+                                    size_t *out_header_len) {
+  return cbs_get_any_asn1_element(cbs, out, out_tag, out_header_len,
+                                  0 /* DER only */);
+}
+
+int CBS_get_any_ber_asn1_element(CBS *cbs, CBS *out, unsigned *out_tag,
+                                 size_t *out_header_len) {
+  return cbs_get_any_asn1_element(cbs, out, out_tag, out_header_len,
+                                  1 /* BER allowed */);
+}
+
 static int cbs_get_asn1(CBS *cbs, CBS *out, unsigned tag_value,
                         int skip_header) {
   size_t header_len;
@@ -240,12 +252,7 @@ static int cbs_get_asn1(CBS *cbs, CBS *out, unsigned tag_value,
   }
 
   if (!CBS_get_any_asn1_element(cbs, out, &tag, &header_len) ||
-      tag != tag_value ||
+      tag != tag_value) {
-      (header_len > 0 &&
-       /* This ensures that the tag is either zero length or
-        * indefinite-length. */
-       CBS_len(out) == header_len &&
-       CBS_data(out)[header_len - 1] == 0x80)) {
     return 0;
   }
 

crypto/bytestring/internal.h

@@ -38,6 +38,14 @@ extern "C" {
  * It returns one on success and zero otherwise. */
 OPENSSL_EXPORT int CBS_asn1_ber_to_der(CBS *in, uint8_t **out, size_t *out_len);
 
+/* CBS_get_any_ber_asn1_element acts the same as |CBS_get_any_asn1_element| but
+ * also allows indefinite-length elements to be returned. In that case,
+ * |*out_header_len| and |CBS_len(out)| will both be two as only the header is
+ * returned. */
+OPENSSL_EXPORT int CBS_get_any_ber_asn1_element(CBS *cbs, CBS *out,
+                                                unsigned *out_tag,
+                                                size_t *out_header_len);
+
 
 #if defined(__cplusplus)
 }  /* extern C */

include/openssl/bytestring.h

@@ -150,10 +150,8 @@ OPENSSL_EXPORT int CBS_peek_asn1_tag(const CBS *cbs, unsigned tag_value);
 
 /* 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. If
+ * the tag number and |*out_header_len| to the length of the ASN.1 header. Each
- * the element has indefinite length then |*out| will only contain the
+ * of |out|, |out_tag|, and |out_header_len| may be NULL to ignore the value.
- * 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,