Make CBS_get_any_asn1_element() more compliant with DER encoding.

CBS_get_any_asn1_element violates DER encoding by allowing indefinite
form.  All callers except bs_ber.c expect DER encoding.  The callers
must check to see if it was indefinite or not.

Rather than exposing all callers to this behavior,
cbs_get_any_asn1_element_internal() allows specifying whether you want to
allow the normally forbidden indefinite form.  This is used by
CBS_get_any_asn1_element() for strict DER encoding and by a new static
function in bs_ber.c for the relaxed version.

While I was here, I added comments to differentiate between ASN.1
restrictions and CBS limitations.

ok miod@

1 files changed, 51 insertions, 17 deletions

diff --git a/src/lib/libssl/bs_cbs.c b/src/lib/libssl/bs_cbs.c
index c37f81dd60..ba38303c18 100644
--- a/src/lib/libssl/bs_cbs.c
+++ b/src/lib/libssl/bs_cbs.c
@@ -1,4 +1,4 @@
-/*      $OpenBSD: bs_cbs.c,v 1.8 2015/06/13 08:46:00 doug Exp $ */
+/*      $OpenBSD: bs_cbs.c,v 1.9 2015/06/15 07:35:49 doug Exp $ */
 /*
  * Copyright (c) 2014, Google Inc.
  *
@@ -205,24 +205,45 @@ int
 CBS_get_any_asn1_element(CBS *cbs, CBS *out, unsigned *out_tag,
     size_t *out_header_len)
 {
+        return cbs_get_any_asn1_element_internal(cbs, out, out_tag,
+            out_header_len, 1);
+}
+
+/*
+ * Review X.690 for details on ASN.1 DER encoding.
+ *
+ * If non-strict mode is enabled, then DER rules are relaxed
+ * for indefinite constructs (violates DER but a little closer to BER).
+ * Non-strict mode should only be used by bs_ber.c
+ *
+ * Sections 8, 10 and 11 for DER encoding
+ */
+int
+cbs_get_any_asn1_element_internal(CBS *cbs, CBS *out, unsigned *out_tag,
+    size_t *out_header_len, int strict)
+{
         uint8_t tag, length_byte;
         CBS header = *cbs;
         CBS throwaway;
+        size_t len;
 
         if (out == NULL)
                 out = &throwaway;
 
+        /*
+         * Get identifier octet and length octet.  Only 1 octet for each
+         * is a CBS limitation.
+         */
         if (!CBS_get_u8(&header, &tag) || !CBS_get_u8(&header, &length_byte))
                 return 0;
 
+        /* CBS limitation: long form tags are not supported. */
         if ((tag & 0x1f) == 0x1f)
-                /* Long form tags are not supported. */
                 return 0;
 
         if (out_tag != NULL)
                 *out_tag = tag;
 
-        size_t len;
         if ((length_byte & 0x80) == 0) {
                 /* Short form length. */
                 len = ((size_t) length_byte) + 2;
@@ -234,21 +255,40 @@ 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) {
-                        /* indefinite length */
-                        if (out_header_len != NULL)
-                                *out_header_len = 2;
-                        return CBS_get_bytes(cbs, out, 2);
+                /* ASN.1 reserved value for future extensions */
+                if (num_bytes == 0x7f)
+                        return 0;
+
+                /* Handle indefinite form length */
+                if (num_bytes == 0) {
+                        /* DER encoding doesn't allow for indefinite form. */
+                        if (strict) {
+                                return 0;
+
+                        } else {
+                                if ((tag & CBS_ASN1_CONSTRUCTED) != 0 &&
+                                    num_bytes == 0) {
+                                        /* indefinite length */
+                                        if (out_header_len != NULL)
+                                                *out_header_len = 2;
+                                        return CBS_get_bytes(cbs, out, 2);
+                                } else {
+                                        /* Primitive cannot use indefinite. */
+                                        return 0;
+                                }
+                        }
                 }
 
-                if (num_bytes == 0 || num_bytes > 4)
+                /* CBS limitation. */
+                if (num_bytes > 4)
                         return 0;
 
                 if (!cbs_get_u(&header, &len32, num_bytes))
                         return 0;
 
+                /* DER has a minimum length octet requirements. */
                 if (len32 < 128)
-                        /* Length should have used short-form encoding. */
+                        /* Should have used short form instead */
                         return 0;
 
                 if ((len32 >> ((num_bytes - 1) * 8)) == 0)
@@ -279,13 +319,7 @@ cbs_get_asn1(CBS *cbs, CBS *out, unsigned tag_value, int skip_header)
                 out = &throwaway;
 
         if (!CBS_get_any_asn1_element(cbs, out, &tag, &header_len) ||
-            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))
+            tag != tag_value)
                 return 0;
 
         if (skip_header && !CBS_skip(out, header_len)) {