From: beck <beck@openbsd.org>
Date: Tue, 26 Oct 2021 09:09:53 +0000 (+0000)
Subject: Validate Subject Alternate Names when they are being added to certificates.
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=ecede11ebf80d5b949654dc4694e771ed95f2abd;p=openbsd

Validate Subject Alternate Names when they are being added to certificates.

With this change we will reject adding SAN DNS, EMAIL, and IP addresses
that are malformed at certificate creation time.

ok jsing@ tb@
---

diff --git a/lib/libcrypto/x509/x509_alt.c b/lib/libcrypto/x509/x509_alt.c
index 5b9f490bae1..02a4a3a3778 100644
--- a/lib/libcrypto/x509/x509_alt.c
+++ b/lib/libcrypto/x509/x509_alt.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: x509_alt.c,v 1.2 2021/08/24 15:23:03 tb Exp $ */
+/* $OpenBSD: x509_alt.c,v 1.3 2021/10/26 09:09:53 beck Exp $ */
 /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL
  * project.
  */
@@ -63,6 +63,8 @@
 #include <openssl/err.h>
 #include <openssl/x509v3.h>
 
+#include "x509_internal.h"
+
 static GENERAL_NAMES *v2i_subject_alt(X509V3_EXT_METHOD *method,
     X509V3_CTX *ctx, STACK_OF(CONF_VALUE) *nval);
 static GENERAL_NAMES *v2i_issuer_alt(X509V3_EXT_METHOD *method,
@@ -612,8 +614,11 @@ GENERAL_NAME *
 v2i_GENERAL_NAME_ex(GENERAL_NAME *out, const X509V3_EXT_METHOD *method,
     X509V3_CTX *ctx, CONF_VALUE *cnf, int is_nc)
 {
-	int type;
+	uint8_t *bytes = NULL;
 	char *name, *value;
+	GENERAL_NAME *ret;
+	size_t len = 0;
+	int type;
 
 	name = cnf->name;
 	value = cnf->value;
@@ -643,7 +648,46 @@ v2i_GENERAL_NAME_ex(GENERAL_NAME *out, const X509V3_EXT_METHOD *method,
 		return NULL;
 	}
 
-	return a2i_GENERAL_NAME(out, method, ctx, type, value, is_nc);
+	ret = a2i_GENERAL_NAME(out, method, ctx, type, value, is_nc);
+
+	/* Validate what we have for sanity */
+	type = x509_constraints_general_to_bytes(ret, &bytes, &len);
+	switch(type) {
+	case GEN_DNS:
+		if (!x509_constraints_valid_sandns(bytes, len)) {
+			X509V3error(X509V3_R_BAD_OBJECT);
+			ERR_asprintf_error_data("name=%s value='%s'", name, bytes);
+			goto err;
+		}
+		break;
+	case GEN_URI:
+		if (!x509_constraints_uri_host(bytes, len, NULL)) {
+			X509V3error(X509V3_R_BAD_OBJECT);
+			ERR_asprintf_error_data("name=%s value='%s'", name, bytes);
+			goto err;
+		}
+		break;
+	case GEN_EMAIL:
+		if (!x509_constraints_parse_mailbox(bytes, len, NULL)) {
+			X509V3error(X509V3_R_BAD_OBJECT);
+			ERR_asprintf_error_data("name=%s value='%s'", name, bytes);
+			goto err;
+		}
+		break;
+	case GEN_IPADD:
+		if (len != 4 && len != 16) {
+			X509V3error(X509V3_R_BAD_IP_ADDRESS);
+			ERR_asprintf_error_data("name=%s len=%zu", name, len);
+			goto err;
+		}
+		break;
+	default:
+		break;
+	}
+	return ret;
+ err:
+	GENERAL_NAME_free(ret);
+	return NULL;
 }
 
 static int
diff --git a/lib/libcrypto/x509/x509_constraints.c b/lib/libcrypto/x509/x509_constraints.c
index db33bf1aa40..f5e1050bb15 100644
--- a/lib/libcrypto/x509/x509_constraints.c
+++ b/lib/libcrypto/x509/x509_constraints.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: x509_constraints.c,v 1.17 2021/09/23 15:49:48 jsing Exp $ */
+/* $OpenBSD: x509_constraints.c,v 1.18 2021/10/26 09:09:53 beck Exp $ */
 /*
  * Copyright (c) 2020 Bob Beck <beck@openbsd.org>
  *
@@ -424,9 +424,14 @@ x509_constraints_parse_mailbox(uint8_t *candidate, size_t len,
 	    strlen(candidate_domain)))
 		goto bad;
 
-	name->local = candidate_local;
-	name->name = candidate_domain;
-	name->type = GEN_EMAIL;
+	if (name != NULL) {
+		name->local = candidate_local;
+		name->name = candidate_domain;
+		name->type = GEN_EMAIL;
+	} else {
+		free(candidate_local);
+		free(candidate_domain);
+	}
 	return 1;
  bad:
 	free(candidate_local);
@@ -511,7 +516,8 @@ x509_constraints_uri_host(uint8_t *uri, size_t len, char **hostpart)
 		host = authority;
 	if (!x509_constraints_valid_host(host, hostlen))
 		return 0;
-	*hostpart = strndup(host, hostlen);
+	if (hostpart != NULL)
+		*hostpart = strndup(host, hostlen);
 	return 1;
 }
 
diff --git a/lib/libcrypto/x509/x509_internal.h b/lib/libcrypto/x509/x509_internal.h
index 8891aecb135..90fafccae0f 100644
--- a/lib/libcrypto/x509/x509_internal.h
+++ b/lib/libcrypto/x509/x509_internal.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: x509_internal.h,v 1.12 2021/09/03 08:58:53 beck Exp $ */
+/* $OpenBSD: x509_internal.h,v 1.13 2021/10/26 09:09:53 beck Exp $ */
 /*
  * Copyright (c) 2020 Bob Beck <beck@openbsd.org>
  *
@@ -106,6 +106,8 @@ struct x509_constraints_names *x509_constraints_names_dup(
     struct x509_constraints_names *names);
 void x509_constraints_names_clear(struct x509_constraints_names *names);
 struct x509_constraints_names *x509_constraints_names_new(size_t names_max);
+int x509_constraints_general_to_bytes(GENERAL_NAME *name, uint8_t **bytes,
+    size_t *len);
 void x509_constraints_names_free(struct x509_constraints_names *names);
 int x509_constraints_valid_host(uint8_t *name, size_t len);
 int x509_constraints_valid_sandns(uint8_t *name, size_t len);