1024: Verify server certificate hostname with OpenSSL
authorKalle Olavi Niemitalo <kon@iki.fi>
Tue, 3 May 2011 00:52:21 +0000 (3 03:52 +0300)
committerKalle Olavi Niemitalo <Kalle@Pulska.kon.iki.fi>
Thu, 28 Jul 2011 14:23:51 +0000 (28 17:23 +0300)
Not tested with nss-compat-ossl.

src/network/ssl/Makefile
src/network/ssl/match-hostname.c [new file with mode: 0644]
src/network/ssl/match-hostname.h [new file with mode: 0644]
src/network/ssl/socket.c
src/network/ssl/ssl.c
src/network/ssl/ssl.h
src/network/ssl/test/Makefile [new file with mode: 0644]
src/network/ssl/test/match-hostname-test.c [new file with mode: 0644]
src/network/ssl/test/test-match-hostname [new file with mode: 0755]

index 3edb280..d6c8d09 100644 (file)
@@ -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
diff --git a/src/network/ssl/match-hostname.c b/src/network/ssl/match-hostname.c
new file mode 100644 (file)
index 0000000..9a64bb4
--- /dev/null
@@ -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;
+}
diff --git a/src/network/ssl/match-hostname.h b/src/network/ssl/match-hostname.h
new file mode 100644 (file)
index 0000000..60d32b2
--- /dev/null
@@ -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
index b449173..f82bf87 100644 (file)
@@ -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>
 #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;
index bb1ae07..79f11f2 100644 (file)
 #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
index 7f89a51..601fde1 100644 (file)
@@ -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. */
 
diff --git a/src/network/ssl/test/Makefile b/src/network/ssl/test/Makefile
new file mode 100644 (file)
index 0000000..f2196eb
--- /dev/null
@@ -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
diff --git a/src/network/ssl/test/match-hostname-test.c b/src/network/ssl/test/match-hostname-test.c
new file mode 100644 (file)
index 0000000..fbdf6fa
--- /dev/null
@@ -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;
+       
+}
diff --git a/src/network/ssl/test/test-match-hostname b/src/network/ssl/test/test-match-hostname
new file mode 100755 (executable)
index 0000000..01d7173
--- /dev/null
@@ -0,0 +1,3 @@
+#! /bin/sh -e
+
+./match-hostname-test