1024: Verify server certificate hostname with OpenSSL

Not tested with nss-compat-ossl.

src/network/ssl/Makefile

@@ -3,6 +3,11 @@ include $(top_builddir)/Makefile.config
 
 INCLUDES += $(GNUTLS_CFLAGS) $(OPENSSL_CFLAGS) $(LIBGCRYPT_CFLAGS)
 
-OBJS = ssl.o socket.o
+SUBDIRS = test
+
+# ELinks uses match-hostname.o only if CONFIG_OPENSSL.
+# However, match-hostname.o has test cases that always need it.
+# The test framework doesn't seem to support conditional tests.
+OBJS = match-hostname.o ssl.o socket.o
 
 include $(top_srcdir)/Makefile.lib

src/network/ssl/match-hostname.c

@@ -0,0 +1,125 @@
+/* Matching a host name to wildcards in SSL certificates */
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
+#include "elinks.h"
+
+#include "intl/charsets.h"
+#include "network/ssl/match-hostname.h"
+#include "util/conv.h"
+#include "util/error.h"
+#include "util/string.h"
+
+/** Checks whether a host name matches a pattern that may contain
+ * wildcards.
+ *
+ * @param[in] hostname
+ *   The host name to which the user wanted to connect.
+ *   Should be in UTF-8 and need not be null-terminated.
+ * @param[in] hostname_length
+ *   The length of @a hostname, in bytes.
+ * @param[in] pattern
+ *   A pattern that the host name might match.
+ *   Should be in UTF-8 and need not be null-terminated.
+ *   The pattern may contain wildcards, as specified in
+ *   RFC 2818 section 3.1.
+ * @param[in] pattern_length
+ *   The length of @a pattern, in bytes.
+ *
+ * @return
+ *   Nonzero if the host name matches.  Zero if it doesn't.
+ *
+ * According to RFC 2818 section 3.1, '*' matches any number of
+ * characters except '.'.  For example, "*r*.example.org" matches
+ * "random.example.org" or "history.example.org" but not
+ * "frozen.fruit.example.org".
+ *
+ * This function does not allocate memory, and consumes at most
+ * O(@a hostname_length * @a pattern_length) time.  */
+int
+match_hostname_pattern(const unsigned char *hostname,
+		       size_t hostname_length,
+		       const unsigned char *pattern,
+		       size_t pattern_length)
+{
+	const unsigned char *const hostname_end = hostname + hostname_length;
+	const unsigned char *const pattern_end = pattern + pattern_length;
+
+	assert(hostname <= hostname_end);
+	assert(pattern <= pattern_end);
+	if_assert_failed return 0;
+
+	while (pattern < pattern_end) {
+		if (*pattern == '*') {
+			const unsigned char *next_wildcard;
+			size_t literal_length;
+
+			++pattern;
+			next_wildcard = memchr(pattern, '*',
+					       pattern_end - pattern);
+			if (next_wildcard == NULL)
+				literal_length = pattern_end - pattern;
+			else
+				literal_length = next_wildcard - pattern;
+
+			for (;;) {
+				size_t hostname_left = hostname_end - hostname;
+				unicode_val_T uni;
+
+				if (hostname_left < literal_length)
+					return 0;
+
+				/* If next_wildcard == NULL, then the
+				 * literal string is at the end of the
+				 * pattern, so anchor the match to the
+				 * end of the hostname.  The end of
+				 * this function can then easily
+				 * verify that the whole hostname was
+				 * matched.
+				 *
+				 * But do not jump directly there;
+				 * first verify that there are no '.'
+				 * characters in between.  */
+				if ((next_wildcard != NULL
+				     || hostname_left == literal_length)
+				    && !c_strlcasecmp(pattern, literal_length,
+						      hostname, literal_length))
+					break;
+
+				/* The literal string doesn't match here.
+				 * Skip one character of the hostname and
+				 * retry.  If the skipped character is '.'
+				 * or one of the equivalent characters
+				 * listed in RFC 3490 section 3.1
+				 * requirement 1, then return 0, because
+				 * '*' must not match such characters.
+				 * Do the same if invalid UTF-8 is found.
+				 * Cast away const.  */
+				uni = utf8_to_unicode((unsigned char **) hostname,
+						      hostname_end);
+				if (uni == 0x002E
+				    || uni == 0x3002
+				    || uni == 0xFF0E
+				    || uni == 0xFF61
+				    || uni == UCS_NO_CHAR)
+					return 0;
+			}
+
+			pattern += literal_length;
+			hostname += literal_length;
+		} else {
+			if (hostname == hostname_end)
+				return 0;
+
+			if (c_toupper(*pattern) != c_toupper(*hostname))
+				return 0;
+
+			++pattern;
+			++hostname;
+		}
+	}
+
+	return hostname == hostname_end;
+}

src/network/ssl/match-hostname.h

@@ -0,0 +1,10 @@
+
+#ifndef EL__NETWORK_SSL_MATCH_HOSTNAME_H
+#define EL__NETWORK_SSL_MATCH_HOSTNAME_H
+
+int match_hostname_pattern(const unsigned char *hostname,
+			   size_t hostname_length,
+			   const unsigned char *pattern,
+			   size_t pattern_length);
+
+#endif

src/network/ssl/socket.c

@@ -6,6 +6,7 @@
 
 #ifdef CONFIG_OPENSSL
 #include <openssl/ssl.h>
+#include <openssl/x509v3.h>
 #define USE_OPENSSL
 #elif defined(CONFIG_NSS_COMPAT_OSSL)
 #include <nss_compat_ossl/nss_compat_ossl.h>
@@ -17,7 +18,13 @@
 #error "Huh?! You have SSL enabled, but not OPENSSL nor GNUTLS!! And then you want exactly *what* from me?"
 #endif
 
+#ifdef HAVE_ARPA_INET_H
+#include <arpa/inet.h>
+#endif
 #include <errno.h>
+#ifdef HAVE_NETINET_IN_H
+#include <netinet/in.h>
+#endif
 
 #include "elinks.h"
 
@@ -25,6 +32,7 @@
 #include "main/select.h"
 #include "network/connection.h"
 #include "network/socket.h"
+#include "network/ssl/match-hostname.h"
 #include "network/ssl/socket.h"
 #include "network/ssl/ssl.h"
 #include "protocol/uri.h"
@@ -143,7 +151,12 @@ verify_certificates(struct socket *socket)
 		gnutls_x509_crt_deinit(cert);
 		return -5;
 	}
-	hostname = get_uri_string(conn->uri, URI_HOST);
+
+	/* Because RFC 5280 defines dNSName as an IA5String, it can
+	 * only contain ASCII characters.  Internationalized domain
+	 * names must thus be in Punycode form.  Because GnuTLS 2.8.6
+	 * does not itself support IDN, ELinks must convert.  */
+	hostname = get_uri_string(conn->uri, URI_HOST | URI_IDN);
 	if (!hostname) return -6;
 
 	ret = !gnutls_x509_crt_check_hostname(cert, hostname);
@@ -153,6 +166,203 @@ verify_certificates(struct socket *socket)
 }
 #endif	/* CONFIG_GNUTLS */
 
+#ifdef USE_OPENSSL
+
+/** Checks whether the host component of a URI matches a host name in
+ * the server certificate.
+ *
+ * @param[in] uri_host
+ *   The host name (or IP address) to which the user wanted to connect.
+ *   Should be in UTF-8.
+ * @param[in] cert_host_asn1
+ *   A host name found in the server certificate: either as commonName
+ *   in the subject field, or as a dNSName in the subjectAltName
+ *   extension.  This may contain wildcards, as specified in RFC 2818
+ *   section 3.1.
+ *
+ * @return
+ *   Nonzero if the host matches.  Zero if it doesn't, or on error.
+ *
+ * If @a uri_host is an IP address literal rather than a host name,
+ * then this function returns 0, meaning that the host name does not match.
+ * According to RFC 2818, if the certificate is intended to match an
+ * IP address, then it must have that IP address as an iPAddress
+ * SubjectAltName, rather than in commonName.  For comparing those,
+ * match_uri_host_ip() must be used instead of this function.  */
+static int
+match_uri_host_name(const unsigned char *uri_host,
+		    ASN1_STRING *cert_host_asn1)
+{
+	const size_t uri_host_len = strlen(uri_host);
+	unsigned char *cert_host = NULL;
+	int cert_host_len;
+	int matched = 0;
+
+	if (is_ip_address(uri_host, uri_host_len))
+		goto mismatch;
+
+	/* This function is used for both dNSName and commonName.
+	 * Although dNSName is always an IA5 string, commonName allows
+	 * many other encodings too.  Normalize all to UTF-8.  */
+	cert_host_len = ASN1_STRING_to_UTF8(&cert_host,
+					    cert_host_asn1);
+	if (cert_host_len < 0)
+		goto mismatch;
+
+	matched = match_hostname_pattern(uri_host, uri_host_len,
+					 cert_host, cert_host_len);
+
+mismatch:
+	if (cert_host)
+		OPENSSL_free(cert_host);
+	return matched;
+}
+
+/** Checks whether the host component of a URI matches an IP address
+ * in the server certificate.
+ *
+ * @param[in] uri_host
+ *   The IP address (or host name) to which the user wanted to connect.
+ *   Should be in UTF-8.
+ * @param[in] cert_host_asn1
+ *   An IP address found as iPAddress in the subjectAltName extension
+ *   of the server certificate.  According to RFC 5280 section 4.2.1.6,
+ *   that is an octet string in network byte order.  According to
+ *   RFC 2818 section 3.1, wildcards are not allowed.
+ *
+ * @return
+ *   Nonzero if the host matches.  Zero if it doesn't, or on error.
+ *
+ * If @a uri_host is a host name rather than an IP address literal,
+ * then this function returns 0, meaning that the address does not match.
+ * This function does not try to resolve the host name to an IP address
+ * and compare that to @a cert_host_asn1, because such an approach would
+ * be vulnerable to DNS spoofing.
+ *
+ * This function does not support the address-and-netmask format used
+ * in the name constraints extension of a CA certificate (RFC 5280
+ * section 4.2.1.10).  */
+static int
+match_uri_host_ip(const unsigned char *uri_host,
+		  ASN1_OCTET_STRING *cert_host_asn1)
+{
+	const unsigned char *cert_host_addr = ASN1_STRING_data(cert_host_asn1);
+	struct in_addr uri_host_in;
+#ifdef CONFIG_IPV6
+	struct in6_addr uri_host_in6;
+#endif
+
+	/* RFC 5280 defines the iPAddress alternative of GeneralName
+	 * as an OCTET STRING.  Verify that the type is indeed that.
+	 * This is an assertion because, if someone puts the wrong
+	 * type of data there, then it will not even be recognized as
+	 * an iPAddress, and this function will not be called.
+	 *
+	 * (Because GeneralName is defined in an implicitly tagged
+	 * ASN.1 module, the OCTET STRING tag is not part of the DER
+	 * encoding.  BER also allows a constructed encoding where
+	 * each substring begins with the OCTET STRING tag; but ITU-T
+	 * Rec. X.690 (07/2002) subclause 8.21 says those would be
+	 * OCTET STRING even if the outer string were of some other
+	 * type.  "A Layman's Guide to a Subset of ASN.1, BER, and
+	 * DER" (Kaliski, 1993) claims otherwise, though.)  */
+	assert(ASN1_STRING_type(cert_host_asn1) == V_ASN1_OCTET_STRING);
+	if_assert_failed return 0;
+
+	/* cert_host_addr, url_host_in, and url_host_in6 are all in
+	 * network byte order.  */
+	switch (ASN1_STRING_length(cert_host_asn1)) {
+	case 4:
+		return inet_aton(uri_host, &uri_host_in) != 0
+		    && memcmp(cert_host_addr, &uri_host_in.s_addr, 4) == 0;
+
+#ifdef CONFIG_IPV6
+	case 16:
+		return inet_pton(AF_INET6, uri_host, &uri_host_in6) == 1
+		    && memcmp(cert_host_addr, &uri_host_in6.s6_addr, 16) == 0;
+#endif
+
+	default:
+		return 0;
+	}
+}
+
+/** Verify one certificate in the server certificate chain.
+ * This callback is documented in SSL_set_verify(3).  */
+static int
+verify_callback(int preverify_ok, X509_STORE_CTX *ctx)
+{
+	X509 *cert;
+	SSL *ssl;
+	struct socket *socket;
+	struct connection *conn;
+	unsigned char *host_in_uri;
+	GENERAL_NAMES *alts;
+	int saw_dns_name = 0;
+	int matched = 0;
+
+	/* If OpenSSL already found a problem, keep that.  */
+	if (!preverify_ok)
+		return 0;
+
+	/* Examine only the server certificate, not CA certificates.  */
+	if (X509_STORE_CTX_get_error_depth(ctx) != 0)
+		return preverify_ok;
+
+	cert = X509_STORE_CTX_get_current_cert(ctx);
+	ssl = X509_STORE_CTX_get_ex_data(ctx, SSL_get_ex_data_X509_STORE_CTX_idx());
+	socket = SSL_get_ex_data(ssl, socket_SSL_ex_data_idx);
+	conn = socket->conn;
+	host_in_uri = get_uri_string(conn->uri, URI_HOST | URI_IDN);
+	if (!host_in_uri)
+		return 0;
+
+	/* RFC 5280 section 4.2.1.6 describes the subjectAltName extension.
+	 * RFC 2818 section 3.1 says Common Name must not be used
+	 * if dNSName is present.  */
+	alts = X509_get_ext_d2i(cert, NID_subject_alt_name, NULL, NULL);
+	if (alts != NULL) {
+		int alt_count;
+		int alt_pos;
+		GENERAL_NAME *alt;
+
+		alt_count = sk_GENERAL_NAME_num(alts);
+		for (alt_pos = 0; !matched && alt_pos < alt_count; ++alt_pos) {
+			alt = sk_GENERAL_NAME_value(alts, alt_pos);
+			if (alt->type == GEN_DNS) {
+				saw_dns_name = 1;
+				matched = match_uri_host_name(host_in_uri,
+							      alt->d.dNSName);
+			} else if (alt->type == GEN_IPADD) {
+				matched = match_uri_host_ip(host_in_uri,
+							    alt->d.iPAddress);
+			}
+		}
+
+		/* Free the GENERAL_NAMES list and each element.  */
+		sk_GENERAL_NAME_pop_free(alts, GENERAL_NAME_free);
+	}
+
+	if (!matched && !saw_dns_name) {
+		X509_NAME *name;
+		int cn_index;
+		X509_NAME_ENTRY *entry = NULL;
+
+		name = X509_get_subject_name(cert);
+		cn_index = X509_NAME_get_index_by_NID(name, NID_commonName, -1);
+		if (cn_index >= 0)
+			entry = X509_NAME_get_entry(name, cn_index);
+		if (entry != NULL)
+			matched = match_uri_host_name(host_in_uri,
+						      X509_NAME_ENTRY_get_data(entry));
+	}
+
+	mem_free(host_in_uri);
+	return matched;
+}
+
+#endif	/* USE_OPENSSL */
+
 static void
 ssl_want_read(struct socket *socket)
 {
@@ -219,7 +429,7 @@ ssl_connect(struct socket *socket)
 	if (get_opt_bool("connection.ssl.cert_verify", NULL))
 		SSL_set_verify(socket->ssl, SSL_VERIFY_PEER
 					  | SSL_VERIFY_FAIL_IF_NO_PEER_CERT,
-				NULL);
+			       verify_callback);
 
 	if (get_opt_bool("connection.ssl.client_cert.enable", NULL)) {
 		unsigned char *client_cert;

src/network/ssl/ssl.c

@@ -45,7 +45,35 @@
 #define	PATH_MAX	256 /* according to my /usr/include/bits/posix1_lim.h */
 #endif
 
-SSL_CTX *context = NULL;
+static SSL_CTX *context = NULL;
+int socket_SSL_ex_data_idx = -1;
+
+/** Prevent SSL_dup() if the SSL is associated with struct socket.
+ * We cannot copy struct socket and it doesn't have a reference count
+ * either.  */
+static int
+socket_SSL_ex_data_dup(CRYPTO_EX_DATA *to, CRYPTO_EX_DATA *from,
+		       void *from_d, int idx, long argl, void *argp)
+{
+	/* The documentation of from_d in RSA_get_ex_new_index(3)
+	 * is a bit unclear.  The caller does something like:
+	 *
+	 * void *data = CRYPTO_get_ex_data(from, idx);
+	 * socket_SSL_dup(to, from, &data, idx, argl, argp);
+	 * CRYPTO_set_ex_data(to, idx, data);
+	 *
+	 * i.e., from_d always points to a pointer, even though
+	 * it is just a void * in the prototype.  */
+	struct socket *socket = *(void **) from_d;
+
+	assert(idx == socket_SSL_ex_data_idx);
+	if_assert_failed return 0;
+
+	if (socket)
+		return 0;	/* prevent SSL_dup() */
+	else
+		return 1;	/* allow SSL_dup() */
+}
 
 static void
 init_openssl(struct module *module)
@@ -66,12 +94,17 @@ init_openssl(struct module *module)
 	context = SSL_CTX_new(SSLv23_client_method());
 	SSL_CTX_set_options(context, SSL_OP_ALL);
 	SSL_CTX_set_default_verify_paths(context);
+	socket_SSL_ex_data_idx = SSL_get_ex_new_index(0, NULL,
+						      NULL,
+						      socket_SSL_ex_data_dup,
+						      NULL);
 }
 
 static void
 done_openssl(struct module *module)
 {
 	if (context) SSL_CTX_free(context);
+	/* There is no function that undoes SSL_get_ex_new_index.  */
 }
 
 static struct option_info openssl_options[] = {
@@ -256,6 +289,12 @@ init_ssl_connection(struct socket *socket,
 	socket->ssl = SSL_new(context);
 	if (!socket->ssl) return S_SSL_ERROR;
 
+	if (!SSL_set_ex_data(socket->ssl, socket_SSL_ex_data_idx, socket)) {
+		SSL_free(socket->ssl);
+		socket->ssl = NULL;
+		return S_SSL_ERROR;
+	}
+
 	/* If the server name is known, pass it to OpenSSL.
 	 *
 	 * The return value of SSL_set_tlsext_host_name is not

src/network/ssl/ssl.h

@@ -29,6 +29,9 @@ void done_ssl_connection(struct socket *socket);
 
 unsigned char *get_ssl_connection_cipher(struct socket *socket);
 
+#if defined(CONFIG_OPENSSL) || defined(CONFIG_NSS_COMPAT_OSSL)
+extern int socket_SSL_ex_data_idx;
+#endif
 
 /* Internal type used in ssl module. */
 

src/network/ssl/test/Makefile

@@ -0,0 +1,9 @@
+top_builddir=../../../..
+include $(top_builddir)/Makefile.config
+
+SUBDIRS = 
+TEST_PROGS = match-hostname-test
+TESTDEPS += \
+ $(top_builddir)/src/network/ssl/match-hostname.o
+
+include $(top_srcdir)/Makefile.lib

src/network/ssl/test/match-hostname-test.c

@@ -0,0 +1,123 @@
+/* Test match_hostname_pattern() */
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
+#include <stdio.h>
+#include <stdlib.h>
+
+#include "elinks.h"
+
+#include "network/ssl/match-hostname.h"
+#include "util/string.h"
+
+struct match_hostname_pattern_test_case
+{
+	const unsigned char *pattern;
+	const unsigned char *hostname;
+	int match;
+};
+
+static const struct match_hostname_pattern_test_case match_hostname_pattern_test_cases[] = {
+	{ "*r*.example.org", "random.example.org", 1 },
+	{ "*r*.example.org", "history.example.org", 1 },
+	{ "*r*.example.org", "frozen.fruit.example.org", 0 },
+	{ "*r*.example.org", "steamed.fruit.example.org", 0 },
+
+	{ "ABC.def.Ghi", "abc.DEF.gHI", 1 },
+
+	{ "*", "localhost", 1 },
+	{ "*", "example.org", 0 },
+	{ "*.*", "example.org", 1 },
+	{ "*.*.*", "www.example.org", 1 },
+	{ "*.*.*", "example.org", 0 },
+
+	{ "assign", "assignee", 0 },
+	{ "*peg", "arpeggiator", 0 },
+	{ "*peg*", "arpeggiator", 1 },
+	{ "*r*gi*", "arpeggiator", 1 },
+	{ "*r*git*", "arpeggiator", 0 },
+
+	{ NULL, NULL, 0 }
+};
+
+int
+main(void)
+{
+	const struct match_hostname_pattern_test_case *test;
+	int count_ok = 0;
+	int count_fail = 0;
+	struct string hostname_str = NULL_STRING;
+	struct string pattern_str = NULL_STRING;
+
+	if (!init_string(&hostname_str) || !init_string(&pattern_str)) {
+		fputs("Out of memory.\n", stderr);
+		done_string(&hostname_str);
+		done_string(&pattern_str);
+		return EXIT_FAILURE;
+	}
+
+	for (test = match_hostname_pattern_test_cases; test->pattern; test++) {
+		int match;
+
+		match = match_hostname_pattern(
+			test->hostname,
+			strlen(test->hostname),
+			test->pattern,
+			strlen(test->pattern));
+		if (!match == !test->match) {
+			/* Test OK */
+			count_ok++;
+		} else {
+			fprintf(stderr, "match_hostname_pattern() test failed\n"
+				"\tHostname: %s\n"
+				"\tPattern: %s\n"
+				"\tActual result: %d\n"
+				"\tCorrect result: %d\n",
+				test->hostname,
+				test->pattern,
+				match,
+				test->match);
+			count_fail++;
+		}
+
+		/* Try with strings that are not null-terminated.  */
+		hostname_str.length = 0;
+		add_to_string(&hostname_str, test->hostname);
+		add_to_string(&hostname_str, "ZZZ");
+		pattern_str.length = 0;
+		add_to_string(&pattern_str, test->pattern);
+		add_to_string(&hostname_str, "______");
+
+		match = match_hostname_pattern(
+			hostname_str.source,
+			strlen(test->hostname),
+			pattern_str.source,
+			strlen(test->pattern));
+		if (!match == !test->match) {
+			/* Test OK */
+			count_ok++;
+		} else {
+			fprintf(stderr, "match_hostname_pattern() test failed\n"
+				"\tVariant: Strings were not null-terminated.\n"
+				"\tHostname: %s\n"
+				"\tPattern: %s\n"
+				"\tActual result: %d\n"
+				"\tCorrect result: %d\n",
+				test->hostname,
+				test->pattern,
+				match,
+				test->match);
+			count_fail++;
+		}
+	}
+
+	printf("Summary of match_hostname_pattern() tests: %d OK, %d failed.\n",
+	       count_ok, count_fail);
+
+	done_string(&hostname_str);
+	done_string(&pattern_str);
+	return count_fail ? EXIT_FAILURE : EXIT_SUCCESS;
+	
+}

src/network/ssl/test/test-match-hostname

@@ -0,0 +1,3 @@
+#! /bin/sh -e
+
+./match-hostname-test