commit b319e3b89f08961ab27599ef2101b3361ef01e5a
author David Benjamin <davidben@google.com> Mon Aug 09 01:07:10 2021 -0400
committer Boringssl LUCI CQ <boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com> Wed Aug 11 19:39:07 2021 +0000
tree 06a5812b845182936c9b80df21070c36654f5f25
parent e3a365554293d2629f5eeb901db811c8c0213de8

Fix ASN1_STRING_print_ex with negative integers.

ASN1_STRING and ASN1_TYPE type values almost line up, but not quite.
Negative INTEGERs are not possible in X509_NAME (tag2bit maps INTEGER to
0), but negative ENUMERATEDs are (tag2bit maps ENUMERATED to
B_ASN1_UNKNOWN). See https://crbug.com/boringssl/412 for some notes on
this mess. Either way, the library will freely produce ASN1_STRING
INTEGERs and ENUMERATEDs in non-MSTRING contexts, so get this case
right.

Change-Id: Ica537f4d683e7a6becc96e2eee3cb66e53372124
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48785
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>

crypto/asn1/a_strex.c

diff --git a/crypto/asn1/a_strex.c b/crypto/asn1/a_strex.c
index b8f6626..4329493 100644
--- a/crypto/asn1/a_strex.c
+++ b/crypto/asn1/a_strex.c
@@ -303,6 +303,20 @@
      */
     ASN1_TYPE t;
     t.type = str->type;
+    /* Negative INTEGER and ENUMERATED values are the only case where
+     * |ASN1_STRING| and |ASN1_TYPE| types do not match.
+     *
+     * TODO(davidben): There are also some type fields which, in |ASN1_TYPE|, do
+     * not correspond to |ASN1_STRING|. It is unclear whether those are allowed
+     * in |ASN1_STRING| at all, or what the space of allowed types is.
+     * |ASN1_item_ex_d2i| will never produce such a value so, for now, we say
+     * this is an invalid input. But this corner of the library in general
+     * should be more robust. */
+    if (t.type == V_ASN1_NEG_INTEGER) {
+      t.type = V_ASN1_INTEGER;
+    } else if (t.type == V_ASN1_NEG_ENUMERATED) {
+      t.type = V_ASN1_ENUMERATED;
+    }
     t.value.asn1_string = (ASN1_STRING *)str;
     unsigned char *der_buf = NULL;
     int der_len = i2d_ASN1_TYPE(&t, &der_buf);

crypto/asn1/asn1_test.cc

diff --git a/crypto/asn1/asn1_test.cc b/crypto/asn1/asn1_test.cc
index 0d9c43c..76f87bf 100644
--- a/crypto/asn1/asn1_test.cc
+++ b/crypto/asn1/asn1_test.cc
@@ -721,6 +721,30 @@
        ASN1_STRING_FLAG_BITS_LEFT | 4,
        ASN1_STRFLGS_DUMP_ALL | ASN1_STRFLGS_DUMP_DER,
        "#03020480"},
+      // INTEGER { 1 }
+      {V_ASN1_INTEGER,
+       {0x01},
+       0,
+       ASN1_STRFLGS_DUMP_ALL | ASN1_STRFLGS_DUMP_DER,
+       "#020101"},
+      // INTEGER { -1 }
+      {V_ASN1_NEG_INTEGER,
+       {0x01},
+       0,
+       ASN1_STRFLGS_DUMP_ALL | ASN1_STRFLGS_DUMP_DER,
+       "#0201FF"},
+      // ENUMERATED { 1 }
+      {V_ASN1_ENUMERATED,
+       {0x01},
+       0,
+       ASN1_STRFLGS_DUMP_ALL | ASN1_STRFLGS_DUMP_DER,
+       "#0A0101"},
+      // ENUMERATED { -1 }
+      {V_ASN1_NEG_ENUMERATED,
+       {0x01},
+       0,
+       ASN1_STRFLGS_DUMP_ALL | ASN1_STRFLGS_DUMP_DER,
+       "#0A01FF"},
   };
   for (const auto &t : kTests) {
     SCOPED_TRACE(t.type);