fix TLS Certificate CommonName NULL Byte Vulnerability

CVE-2020-7043 TLS Certificate CommonName NULL Byte Vulnerability is fixed
with this commit

with #8 hostname validation for the certificate was introduced
but unfortunately strncasecmp() was used to compare the byte array
against the expected hostname. This does not correctly treat a CN
which contains a NULL byte. In order to fix this vulnerability
the reference implementation from iSECPartners has been included
into the code.

Makefile.am

@@ -5,7 +5,9 @@ openfortivpn_SOURCES = src/config.c src/config.h src/hdlc.c src/hdlc.h \
 		       src/http.c src/http.h src/io.c src/io.h src/ipv4.c \
 		       src/ipv4.h src/log.c src/log.h src/tunnel.c \
 		       src/tunnel.h src/main.c src/ssl.h src/xml.c \
-		       src/xml.h src/userinput.c src/userinput.h
+		       src/xml.h src/userinput.c src/userinput.h \
+		       src/openssl_hostname_validation.c \
+		       src/openssl_hostname_validation.h
 openfortivpn_CFLAGS = -Wall -pedantic -std=gnu99
 openfortivpn_CPPFLAGS = -DSYSCONFDIR=\"$(sysconfdir)\" \
 			-DPPP_PATH=\"@PPP_PATH@\" \

src/openssl_hostname_validation.c

@@ -0,0 +1,165 @@
+/* Obtained from: /~https://github.com/iSECPartners/ssl-conservatory */
+
+/*
+ * Helper functions to perform basic hostname validation using OpenSSL.
+ *
+ * Author:  Alban Diquet
+ *
+ * Copyright (C) 2012, iSEC Partners.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy of
+ * this software and associated documentation files (the "Software"), to deal in
+ * the Software without restriction, including without limitation the rights to
+ * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies
+ * of the Software, and to permit persons to whom the Software is furnished to do
+ * so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in all
+ * copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ *
+ */
+
+
+#include <strings.h>
+#include <openssl/x509v3.h>
+#include <openssl/ssl.h>
+
+#define HOSTNAME_MAX_SIZE 255
+
+#ifndef HAVE_X509_CHECK_HOST
+
+#include "openssl_hostname_validation.h"
+
+
+/**
+* Tries to find a match for hostname in the certificate's Common Name field.
+*
+* Returns MatchFound if a match was found.
+* Returns MatchNotFound if no matches were found.
+* Returns MalformedCertificate if the Common Name had a NUL character embedded in it.
+* Returns Error if the Common Name could not be extracted.
+*/
+static HostnameValidationResult matches_common_name(const char *hostname, const X509 *server_cert) {
+	int common_name_loc = -1;
+	X509_NAME_ENTRY *common_name_entry = NULL;
+	ASN1_STRING *common_name_asn1 = NULL;
+	char *common_name_str = NULL;
+
+	// Find the position of the CN field in the Subject field of the certificate
+	common_name_loc = X509_NAME_get_index_by_NID(X509_get_subject_name((X509 *) server_cert), NID_commonName, -1);
+	if (common_name_loc < 0) {
+		return Error;
+	}
+
+	// Extract the CN field
+	common_name_entry = X509_NAME_get_entry(X509_get_subject_name((X509 *) server_cert), common_name_loc);
+	if (common_name_entry == NULL) {
+		return Error;
+	}
+
+	// Convert the CN field to a C string
+	common_name_asn1 = X509_NAME_ENTRY_get_data(common_name_entry);
+	if (common_name_asn1 == NULL) {
+		return Error;
+	}			
+	common_name_str = (char *) ASN1_STRING_data(common_name_asn1);
+
+	// Make sure there isn't an embedded NUL character in the CN
+	if (ASN1_STRING_length(common_name_asn1) != strlen(common_name_str)) {
+		return MalformedCertificate;
+	}
+
+	// Compare expected hostname with the CN
+	if (strcasecmp(hostname, common_name_str) == 0) {
+		return MatchFound;
+	}
+	else {
+		return MatchNotFound;
+	}
+}
+
+
+/**
+* Tries to find a match for hostname in the certificate's Subject Alternative Name extension.
+*
+* Returns MatchFound if a match was found.
+* Returns MatchNotFound if no matches were found.
+* Returns MalformedCertificate if any of the hostnames had a NUL character embedded in it.
+* Returns NoSANPresent if the SAN extension was not present in the certificate.
+*/
+static HostnameValidationResult matches_subject_alternative_name(const char *hostname, const X509 *server_cert) {
+	HostnameValidationResult result = MatchNotFound;
+	int i;
+	int san_names_nb = -1;
+	STACK_OF(GENERAL_NAME) *san_names = NULL;
+
+	// Try to extract the names within the SAN extension from the certificate
+	san_names = X509_get_ext_d2i((X509 *) server_cert, NID_subject_alt_name, NULL, NULL);
+	if (san_names == NULL) {
+		return NoSANPresent;
+	}
+	san_names_nb = sk_GENERAL_NAME_num(san_names);
+
+	// Check each name within the extension
+	for (i=0; i<san_names_nb; i++) {
+		const GENERAL_NAME *current_name = sk_GENERAL_NAME_value(san_names, i);
+
+		if (current_name->type == GEN_DNS) {
+			// Current name is a DNS name, let's check it
+			char *dns_name = (char *) ASN1_STRING_data(current_name->d.dNSName);
+
+			// Make sure there isn't an embedded NUL character in the DNS name
+			if (ASN1_STRING_length(current_name->d.dNSName) != strlen(dns_name)) {
+				result = MalformedCertificate;
+				break;
+			}
+			else { // Compare expected hostname with the DNS name
+				if (strcasecmp(hostname, dns_name) == 0) {
+					result = MatchFound;
+					break;
+				}
+			}
+		}
+	}
+	sk_GENERAL_NAME_pop_free(san_names, GENERAL_NAME_free);
+
+	return result;
+}
+
+
+/**
+* Validates the server's identity by looking for the expected hostname in the
+* server's certificate. As described in RFC 6125, it first tries to find a match
+* in the Subject Alternative Name extension. If the extension is not present in
+* the certificate, it checks the Common Name instead.
+*
+* Returns MatchFound if a match was found.
+* Returns MatchNotFound if no matches were found.
+* Returns MalformedCertificate if any of the hostnames had a NUL character embedded in it.
+* Returns Error if there was an error.
+*/
+HostnameValidationResult validate_hostname(const char *hostname, const X509 *server_cert) {
+	HostnameValidationResult result;
+
+	if((hostname == NULL) || (server_cert == NULL))
+		return Error;
+
+	// First try the Subject Alternative Names extension
+	result = matches_subject_alternative_name(hostname, server_cert);
+	if (result == NoSANPresent) {
+		// Extension was not found: try the Common Name
+		result = matches_common_name(hostname, server_cert);
+	}
+
+	return result;
+}
+
+#endif

src/openssl_hostname_validation.h

@@ -0,0 +1,53 @@
+/* Obtained from: /~https://github.com/iSECPartners/ssl-conservatory */
+
+/*
+ * Helper functions to perform basic hostname validation using OpenSSL.
+ *
+ * Author:  Alban Diquet
+ *
+ * Copyright (C) 2012, iSEC Partners.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy of
+ * this software and associated documentation files (the "Software"), to deal in
+ * the Software without restriction, including without limitation the rights to
+ * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies
+ * of the Software, and to permit persons to whom the Software is furnished to do
+ * so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in all
+ * copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ *
+ */
+
+#ifndef HAVE_X509_CHECK_HOST
+
+typedef enum {
+	MatchFound,
+	MatchNotFound,
+	NoSANPresent,
+	MalformedCertificate,
+	Error
+} HostnameValidationResult;
+
+/**
+* Validates the server's identity by looking for the expected hostname in the
+* server's certificate. As described in RFC 6125, it first tries to find a match
+* in the Subject Alternative Name extension. If the extension is not present in
+* the certificate, it checks the Common Name instead.
+*
+* Returns MatchFound if a match was found.
+* Returns MatchNotFound if no matches were found.
+* Returns MalformedCertificate if any of the hostnames had a NUL character embedded in it.
+* Returns Error if there was an error.
+*/
+HostnameValidationResult validate_hostname(const char *hostname, const X509 *server_cert);
+
+#endif

src/tunnel.c

@@ -29,6 +29,9 @@
 #include "tunnel.h"
 #include "http.h"
 #include "log.h"
+#ifndef HAVE_X509_CHECK_HOST
+#include "openssl_hostname_validation.h"
+#endif
 
 #include <unistd.h>
 #include <fcntl.h>
@@ -671,15 +674,10 @@ static int ssl_verify_cert(struct tunnel *tunnel)
 	                    0, 0, NULL) == 1)
 		cert_valid = 1;
 #else
-	char common_name[FIELD_SIZE + 1];
-	// Use explicit Common Name check if native validation not available.
-	// Note: this will ignore Subject Alternative Name fields.
-	if (subj
-	    && X509_NAME_get_text_by_NID(subj, NID_commonName, common_name,
-	                                 FIELD_SIZE) > 0
-	    && strncasecmp(common_name, tunnel->config->gateway_host,
-	                   FIELD_SIZE) == 0)
-		cert_valid = 1;
+	// Use validate_hostname form iSECPartners if native validation not available
+	// in order to avoid TLS Certificate CommonName NULL Byte Vulnerability
+	if (validate_hostname(tunnel->config->gateway_host, cert) == MatchFound)
+ 		cert_valid = 1;
 #endif
 
 	// Try to validate certificate using local PKI

tests/lint/run.sh

@@ -3,10 +3,10 @@
 
 rc=0
 
-./tests/lint/eol-at-eof.sh $(git ls-files) || rc=1
+./tests/lint/eol-at-eof.sh $(git ls-files | grep -v openssl_hostname_validation) || rc=1
 
-./tests/lint/line_length.py $(git ls-files '*.[ch]') || rc=1
+./tests/lint/line_length.py $(git ls-files '*.[ch]' | grep -v openssl_hostname_validation) || rc=1
 
-./tests/lint/astyle.sh $(git ls-files '*.[ch]') || rc=1
+./tests/lint/astyle.sh $(git ls-files '*.[ch]' | grep -v openssl_hostname_validation) || rc=1
 
 exit $rc