Fix i2v_GENERAL_NAME to not assume NUL terminated strings

See also 174ba8048a7f2f5e1fca31cfb93b1730d9db8300 from upstream. This
differs from the upstream CL in that:

- We don't silently drop trailing NULs.

- As a NUL-terminated C string, the empty string is a non-NULL pointer
  to an array containing a zero byte. Use the latter consistently.

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

crypto/err/x509v3.errordata

@@ -34,6 +34,7 @@ X509V3,132,INVALID_PROXY_POLICY_SETTING
 X509V3,133,INVALID_PURPOSE
 X509V3,134,INVALID_SECTION
 X509V3,135,INVALID_SYNTAX
+X509V3,163,INVALID_VALUE
 X509V3,136,ISSUER_DECODE_ERROR
 X509V3,137,MISSING_VALUE
 X509V3,138,NEED_ORGANIZATION_AND_NUMBERS

crypto/x509v3/internal.h

@@ -17,6 +17,8 @@
 
 #include <openssl/base.h>
 
+#include <openssl/conf.h>
+
 #if defined(__cplusplus)
 extern "C" {
 #endif
@@ -67,6 +69,13 @@ typedef struct {
   const char *sname;
 } BIT_STRING_BITNAME;
 
+// x509V3_add_value_asn1_string appends a |CONF_VALUE| with the specified name
+// and value to |*extlist|. if |*extlist| is NULL, it sets |*extlist| to a
+// newly-allocated |STACK_OF(CONF_VALUE)| first. It returns one on success and
+// zero on error.
+int x509V3_add_value_asn1_string(const char *name, const ASN1_STRING *value,
+                                 STACK_OF(CONF_VALUE) **extlist);
+
 
 #if defined(__cplusplus)
 }  /* extern C */

crypto/x509v3/v3_alt.c

@@ -139,17 +139,17 @@ STACK_OF(CONF_VALUE) *i2v_GENERAL_NAME(X509V3_EXT_METHOD *method,
         break;
 
     case GEN_EMAIL:
-        if (!X509V3_add_value_uchar("email", gen->d.ia5->data, &ret))
+        if (!x509V3_add_value_asn1_string("email", gen->d.ia5, &ret))
             return NULL;
         break;
 
     case GEN_DNS:
-        if (!X509V3_add_value_uchar("DNS", gen->d.ia5->data, &ret))
+        if (!x509V3_add_value_asn1_string("DNS", gen->d.ia5, &ret))
             return NULL;
         break;
 
     case GEN_URI:
-        if (!X509V3_add_value_uchar("URI", gen->d.ia5->data, &ret))
+        if (!x509V3_add_value_asn1_string("URI", gen->d.ia5, &ret))
             return NULL;
         break;
 

crypto/x509v3/v3_utl.c

@@ -88,27 +88,38 @@ static int ipv6_hex(unsigned char *out, const char *in, int inlen);
 
 /* Add a CONF_VALUE name value pair to stack */
 
-int X509V3_add_value(const char *name, const char *value,
-                     STACK_OF(CONF_VALUE) **extlist)
+static int x509V3_add_len_value(const char *name, const char *value,
+                                size_t value_len, int omit_value,
+                                STACK_OF(CONF_VALUE) **extlist)
 {
     CONF_VALUE *vtmp = NULL;
     char *tname = NULL, *tvalue = NULL;
     if (name && !(tname = OPENSSL_strdup(name)))
-        goto err;
-    if (value && !(tvalue = OPENSSL_strdup(value)))
-        goto err;
+        goto malloc_err;
+    if (omit_value) {
+        /* |CONF_VALUE| cannot represent strings with NULs. */
+        if (OPENSSL_memchr(value, 0, value_len)) {
+            OPENSSL_PUT_ERROR(X509V3, X509V3_R_INVALID_VALUE);
+            goto err;
+        }
+        tvalue = OPENSSL_strndup(value, value_len);
+        if (tvalue == NULL) {
+            goto malloc_err;
+        }
+    }
     if (!(vtmp = CONF_VALUE_new()))
-        goto err;
+        goto malloc_err;
     if (!*extlist && !(*extlist = sk_CONF_VALUE_new_null()))
-        goto err;
+        goto malloc_err;
     vtmp->section = NULL;
     vtmp->name = tname;
     vtmp->value = tvalue;
     if (!sk_CONF_VALUE_push(*extlist, vtmp))
-        goto err;
+        goto malloc_err;
     return 1;
- err:
+ malloc_err:
     OPENSSL_PUT_ERROR(X509V3, ERR_R_MALLOC_FAILURE);
+ err:
     if (vtmp)
         OPENSSL_free(vtmp);
     if (tname)
@@ -118,12 +129,26 @@ int X509V3_add_value(const char *name, const char *value,
     return 0;
 }
 
+int X509V3_add_value(const char *name, const char *value,
+                     STACK_OF(CONF_VALUE) **extlist)
+{
+    return x509V3_add_len_value(name, value, value != NULL ? strlen(value) : 0,
+                                /*omit_value=*/value == NULL, extlist);
+}
+
 int X509V3_add_value_uchar(const char *name, const unsigned char *value,
                            STACK_OF(CONF_VALUE) **extlist)
 {
     return X509V3_add_value(name, (const char *)value, extlist);
 }
 
+int x509V3_add_value_asn1_string(const char *name, const ASN1_STRING *value,
+                                 STACK_OF(CONF_VALUE) **extlist)
+{
+    return x509V3_add_len_value(name, (const char *)value->data, value->length,
+                                /*omit_value=*/0, extlist);
+}
+
 /* Free function for STACK_OF(CONF_VALUE) */
 
 void X509V3_conf_free(CONF_VALUE *conf)

include/openssl/x509v3.h

@@ -974,5 +974,6 @@ BSSL_NAMESPACE_END
 #define X509V3_R_UNSUPPORTED_OPTION 160
 #define X509V3_R_UNSUPPORTED_TYPE 161
 #define X509V3_R_USER_TOO_LONG 162
+#define X509V3_R_INVALID_VALUE 163
 
 #endif