Import LibreSSL 3.8.1 upstream

1 files changed, 74 insertions, 27 deletions

diff --git a/tls_verify.c b/tls_verify.c
index 0cb86f6..c588f02 100644
--- a/tls_verify.c
+++ b/tls_verify.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: tls_verify.c,v 1.23 2023/05/11 07:35:27 tb Exp $ */
+/* $OpenBSD: tls_verify.c,v 1.28 2023/06/01 07:32:25 tb Exp $ */
 /*
  * Copyright (c) 2014 Jeremie Courreges-Anglas <jca@openbsd.org>
  *
@@ -92,15 +92,21 @@ tls_check_subject_altname(struct tls *ctx, X509 *cert, const char *name,
 	union tls_addr addrbuf;
 	int addrlen, type;
 	int count, i;
-	int rv = 0;
+	int critical = 0;
+	int rv = -1;
 
 	*alt_match = 0;
 	*alt_exists = 0;
 
-	altname_stack = X509_get_ext_d2i(cert, NID_subject_alt_name,
-	    NULL, NULL);
-	if (altname_stack == NULL)
-		return 0;
+	altname_stack = X509_get_ext_d2i(cert, NID_subject_alt_name, &critical,
+	    NULL);
+	if (altname_stack == NULL) {
+		if (critical != -1) {
+			tls_set_errorx(ctx, "error decoding subjectAltName");
+			goto err;
+		}
+		goto done;
+	}
 
 	if (inet_pton(AF_INET, name, &addrbuf) == 1) {
 		type = GEN_IPADD;
@@ -140,8 +146,7 @@ tls_check_subject_altname(struct tls *ctx, X509 *cert, const char *name,
 					    "NUL byte in subjectAltName, "
 					    "probably a malicious certificate",
 					    name);
-					rv = -1;
-					break;
+					goto err;
 				}
 
 				/*
@@ -154,13 +159,12 @@ tls_check_subject_altname(struct tls *ctx, X509 *cert, const char *name,
 					    "error verifying name '%s': "
 					    "a dNSName of \" \" must not be "
 					    "used", name);
-					rv = -1;
-					break;
+					goto err;
 				}
 
 				if (tls_match_name(data, name) == 0) {
 					*alt_match = 1;
-					break;
+					goto done;
 				}
 			} else {
 #ifdef DEBUG
@@ -181,8 +185,7 @@ tls_check_subject_altname(struct tls *ctx, X509 *cert, const char *name,
 				tls_set_errorx(ctx,
 				    "Unexpected negative length for an "
 				    "IP address: %d", datalen);
-				rv = -1;
-				break;
+				goto err;
 			}
 
 			/*
@@ -192,11 +195,15 @@ tls_check_subject_altname(struct tls *ctx, X509 *cert, const char *name,
 			if (datalen == addrlen &&
 			    memcmp(data, &addrbuf, addrlen) == 0) {
 				*alt_match = 1;
-				break;
+				goto done;
 			}
 		}
 	}
 
+ done:
+	rv = 0;
+
+ err:
 	sk_GENERAL_NAME_pop_free(altname_stack, GENERAL_NAME_free);
 	return rv;
 }
@@ -205,10 +212,13 @@ static int
 tls_check_common_name(struct tls *ctx, X509 *cert, const char *name,
     int *cn_match)
 {
+	unsigned char *utf8_bytes = NULL;
 	X509_NAME *subject_name;
 	char *common_name = NULL;
 	union tls_addr addrbuf;
 	int common_name_len;
+	ASN1_STRING *data;
+	int lastpos = -1;
 	int rv = -1;
 
 	*cn_match = 0;
@@ -217,29 +227,65 @@ tls_check_common_name(struct tls *ctx, X509 *cert, const char *name,
 	if (subject_name == NULL)
 		goto done;
 
-	common_name_len = X509_NAME_get_text_by_NID(subject_name,
-	    NID_commonName, NULL, 0);
-	if (common_name_len < 0)
+	lastpos = X509_NAME_get_index_by_NID(subject_name,
+	    NID_commonName, lastpos);
+	if (lastpos == -1)
 		goto done;
-
-	common_name = calloc(common_name_len + 1, 1);
-	if (common_name == NULL) {
-		tls_set_error(ctx, "out of memory");
+	if (lastpos < 0)
+		goto err;
+	if (X509_NAME_get_index_by_NID(subject_name, NID_commonName, lastpos)
+	    != -1) {
+		/*
+		 * Having multiple CN's is possible, and even happened back in
+		 * the glory days of mullets and Hammer pants. In anything like
+		 * a modern TLS cert, CN is as close to deprecated as it gets,
+		 * and having more than one is bad. We therefore fail if we have
+		 * more than one CN fed to us in the subject, treating the
+		 * certificate as hostile.
+		 */
+		tls_set_errorx(ctx, "error verifying name '%s': "
+		    "Certificate subject contains mutiple Common Name fields, "
+		    "probably a malicious or malformed certificate", name);
 		goto err;
 	}
 
-	X509_NAME_get_text_by_NID(subject_name, NID_commonName, common_name,
-	    common_name_len + 1);
-
-	/* NUL bytes in CN? */
-	if (common_name_len < 0 ||
-	    (size_t)common_name_len != strlen(common_name)) {
+	data = X509_NAME_ENTRY_get_data(X509_NAME_get_entry(subject_name,
+	    lastpos));
+	/*
+	 * Fail if we cannot encode the CN bytes as UTF-8.
+	 */
+	if ((common_name_len = ASN1_STRING_to_UTF8(&utf8_bytes, data)) < 0) {
+		tls_set_errorx(ctx, "error verifying name '%s': "
+		    "Common Name field cannot be encoded as a UTF-8 string, "
+		    "probably a malicious certificate", name);
+		goto err;
+	}
+	/*
+	 * Fail if the CN is of invalid length. RFC 5280 specifies that a CN
+	 * must be between 1 and 64 bytes long.
+	 */
+	if (common_name_len < 1 || common_name_len > 64) {
+		tls_set_errorx(ctx, "error verifying name '%s': "
+		    "Common Name field has invalid length, "
+		    "probably a malicious certificate", name);
+		goto err;
+	}
+	/*
+	 * Fail if the resulting text contains a NUL byte.
+	 */
+	if (memchr(utf8_bytes, 0, common_name_len) != NULL) {
 		tls_set_errorx(ctx, "error verifying name '%s': "
 		    "NUL byte in Common Name field, "
 		    "probably a malicious certificate", name);
 		goto err;
 	}
 
+	common_name = strndup(utf8_bytes, common_name_len);
+	if (common_name == NULL) {
+		tls_set_error(ctx, "out of memory");
+		goto err;
+	}
+
 	/*
 	 * We don't want to attempt wildcard matching against IP addresses,
 	 * so perform a simple comparison here.
@@ -258,6 +304,7 @@ tls_check_common_name(struct tls *ctx, X509 *cert, const char *name,
 	rv = 0;
 
  err:
+	free(utf8_bytes);
 	free(common_name);
 	return rv;
 }