From: beck <beck@openbsd.org>
Date: Mon, 29 May 2023 14:12:36 +0000 (+0000)
Subject: Correctly catch all return values from X509_NAME_get_index_by_NID
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=9f395b46183ba1c959d4dcfb7f5d79659672b27d;p=openbsd

Correctly catch all return values from X509_NAME_get_index_by_NID

And some comment requests, from jsing@

ok jsing@
---

diff --git a/lib/libtls/tls_verify.c b/lib/libtls/tls_verify.c
index acc034d9c13..a0c39b9dd4f 100644
--- a/lib/libtls/tls_verify.c
+++ b/lib/libtls/tls_verify.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: tls_verify.c,v 1.25 2023/05/28 09:06:34 beck Exp $ */
+/* $OpenBSD: tls_verify.c,v 1.26 2023/05/29 14:12:36 beck Exp $ */
 /*
  * Copyright (c) 2014 Jeremie Courreges-Anglas <jca@openbsd.org>
  *
@@ -224,6 +224,8 @@ tls_check_common_name(struct tls *ctx, X509 *cert, const char *name,
 	    NID_commonName, lastpos);
 	if (lastpos == -1)
 		goto done;
+	if (lastpos < 0)
+		goto err;
 	if (X509_NAME_get_index_by_NID(subject_name, NID_commonName, lastpos)
 	    != -1) {
 		/*
@@ -243,9 +245,7 @@ tls_check_common_name(struct tls *ctx, X509 *cert, const char *name,
 	data = X509_NAME_ENTRY_get_data(X509_NAME_get_entry(subject_name,
 	    lastpos));
 	/*
-	 * Fail if we cannot encode as UTF-8, if the CN is of invalid length, or
-	 * if the UTF-8 encoding of the string contains a 0 byte. We treat any
-	 * certificate with such data in the CN as hostile and fail.
+	 * 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': "
@@ -253,14 +253,19 @@ tls_check_common_name(struct tls *ctx, X509 *cert, const char *name,
 		    "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, "