From: Kalle Olavi Niemitalo Date: Tue, 3 May 2011 00:52:21 +0000 (+0300) Subject: 1024: Verify server certificate hostname with OpenSSL X-Git-Tag: CONLY~129 X-Git-Url: https://repo.or.cz/w/elinks.git/commitdiff_plain/0c3f3e09 1024: Verify server certificate hostname with OpenSSL Not tested with nss-compat-ossl. --- diff --git a/src/network/ssl/Makefile b/src/network/ssl/Makefile index 3edb2800..d6c8d09b 100644 --- a/src/network/ssl/Makefile +++ b/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 diff --git a/src/network/ssl/match-hostname.c b/src/network/ssl/match-hostname.c new file mode 100644 index 00000000..9a64bb40 --- /dev/null +++ b/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; +} diff --git a/src/network/ssl/match-hostname.h b/src/network/ssl/match-hostname.h new file mode 100644 index 00000000..60d32b2d --- /dev/null +++ b/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 diff --git a/src/network/ssl/socket.c b/src/network/ssl/socket.c index b449173e..f82bf871 100644 --- a/src/network/ssl/socket.c +++ b/src/network/ssl/socket.c @@ -6,6 +6,7 @@ #ifdef CONFIG_OPENSSL #include +#include #define USE_OPENSSL #elif defined(CONFIG_NSS_COMPAT_OSSL) #include @@ -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 +#endif #include +#ifdef HAVE_NETINET_IN_H +#include +#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; diff --git a/src/network/ssl/ssl.c b/src/network/ssl/ssl.c index bb1ae073..79f11f20 100644 --- a/src/network/ssl/ssl.c +++ b/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 diff --git a/src/network/ssl/ssl.h b/src/network/ssl/ssl.h index 7f89a511..601fde1f 100644 --- a/src/network/ssl/ssl.h +++ b/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. */ diff --git a/src/network/ssl/test/Makefile b/src/network/ssl/test/Makefile new file mode 100644 index 00000000..f2196eb6 --- /dev/null +++ b/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 diff --git a/src/network/ssl/test/match-hostname-test.c b/src/network/ssl/test/match-hostname-test.c new file mode 100644 index 00000000..fbdf6fa6 --- /dev/null +++ b/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 +#include + +#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 index 00000000..01d7173a --- /dev/null +++ b/src/network/ssl/test/test-match-hostname @@ -0,0 +1,3 @@ +#! /bin/sh -e + +./match-hostname-test