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>
diff --git a/crypto/err/x509v3.errordata b/crypto/err/x509v3.errordata
index e53b780..492259c 100644
--- a/crypto/err/x509v3.errordata
+++ b/crypto/err/x509v3.errordata
@@ -34,6 +34,7 @@
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
diff --git a/crypto/x509v3/internal.h b/crypto/x509v3/internal.h
index 1b932c1..df7d813 100644
--- a/crypto/x509v3/internal.h
+++ b/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 @@
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 */
diff --git a/crypto/x509v3/v3_alt.c b/crypto/x509v3/v3_alt.c
index 4d54075..9ea3ed0 100644
--- a/crypto/x509v3/v3_alt.c
+++ b/crypto/x509v3/v3_alt.c
@@ -139,17 +139,17 @@
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;
diff --git a/crypto/x509v3/v3_utl.c b/crypto/x509v3/v3_utl.c
index 695e224..ab1f36e 100644
--- a/crypto/x509v3/v3_utl.c
+++ b/crypto/x509v3/v3_utl.c
@@ -88,27 +88,38 @@
/* 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 @@
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)
diff --git a/include/openssl/x509v3.h b/include/openssl/x509v3.h
index 31422f4..84fa108 100644
--- a/include/openssl/x509v3.h
+++ b/include/openssl/x509v3.h
@@ -974,5 +974,6 @@
#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