Here is a patch for both openssl and gnutls. Please comment, I'll push it tomorrow.
BTW, when fixing the gnutls code I stumbled upon a bug in 3.4.x. I reported it as https://gitlab.com/gnutls/gnutls/issues/78 Tim On Monday 14 March 2016 17:21:15 Yst Dawson wrote: > URL: > <http://savannah.gnu.org/bugs/?47408> > > Summary: Wget sends malformed SNI host names > Project: GNU Wget > Submitted by: yst > Submitted on: Mon 14 Mar 2016 05:21:14 PM GMT > Category: Program Logic > Severity: 3 - Normal > Priority: 5 - Normal > Status: None > Privacy: Public > Assigned to: None > Originator Name: > Originator Email: > Open/Closed: Open > Discussion Lock: Any > Release: 1.16 > Operating System: GNU/Linux > Reproducibility: Every Time > Fixed Release: None > Planned Release: None > Regression: None > Work Required: None > Patch Included: None > > _______________________________________________________ > > Details: > > To quote a couple specifications: > <https://tools.ietf.org/html/rfc6066#section-3> (SNI) > "HostName" contains the fully qualified DNS hostname of the server, > as understood by the client. The hostname is represented as a byte > string using ASCII encoding without a trailing dot. > > <https://tools.ietf.org/html/rfc7230#section-5.4> (HTTP) > A client MUST send a Host header field in all HTTP/1.1 request > messages. If the target URI includes an authority component, then a > client MUST send a field-value for Host that is identical to that > authority component, excluding any userinfo subcomponent and its "@" > delimiter (Section 2.7.1). > > That means that the SNI host name and HTTP Host header do not always match. > The SNI host name must never have a trailing dot, but the HTTP Host header > must reflect a host name that is identical to the host name of the URI, so > if the URI's host has a trailing dot, the HTTP Host header must include > that trailing dot. > > For example, if the URI of a page is <https://alice.sni.velox.ch./>, the > following values should be sent by the Web browser: > SNI host: alice.sni.velox.ch > HTTP host: alice.sni.velox.ch. > > However, Wget sends "alice.sni.velox.ch." as the SNI host name. In some > cases, malformed SNI host names can cause the server to throw an error, an > example of which can be seen at <https://sni.velox.ch./> or > <https://www.apache.org./>. > > Other information: > * version: 1.16 > * invoked by running "wget --no-check-certificate > https://alice.sni.velox.ch./"; > * expected result: Wget should send an SNI host name that conforms to RFC > 6066 or no SNI host name, while still sending an HTTP Host header that > includes the trailing dot, as per RFC 7230. > * actual result: Wget sent a malformed SNI host name > * The output, in case relevant, has been attached as a file upload. > > > > _______________________________________________________ > > File Attachments: > > > ------------------------------------------------------- > Date: Mon 14 Mar 2016 05:21:14 PM GMT Name: index.html Size: 5kB By: yst > > <http://savannah.gnu.org/bugs/download.php?file_id6634> > > _______________________________________________________ > > Reply to this item at: > > <http://savannah.gnu.org/bugs/?47408> > > _______________________________________________ > Message sent via/by Savannah > http://savannah.gnu.org/
From d7726f8a1366efcd09329ee20beefb7e8ece9078 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim Rühsen?= <[email protected]> Date: Wed, 16 Mar 2016 11:23:51 +0100 Subject: [PATCH] Fix SNI server names with trailing dot(s) * src/gnutls.c (ssl_connect_wget, ssl_check_certificate): Fix SNI server name * src/openssl.c (ssl_connect_wget, ssl_check_certificate): Fix SNI server name Fixes #47408 --- src/gnutls.c | 32 ++++++++++++++++++++++++++++---- src/openssl.c | 35 +++++++++++++++++++++++++++++++---- 2 files changed, 59 insertions(+), 8 deletions(-) diff --git a/src/gnutls.c b/src/gnutls.c index d39371f..3e1596a 100644 --- a/src/gnutls.c +++ b/src/gnutls.c @@ -36,6 +36,7 @@ as that of the covered work. */ #include <stdio.h> #include <dirent.h> #include <stdlib.h> +#include <xalloc.h> #include <gnutls/gnutls.h> #include <gnutls/x509.h> @@ -518,6 +519,22 @@ _do_handshake (gnutls_session_t session, int fd, double timeout) return err; } +static const char * +_sni_hostname(const char *hostname) +{ + size_t len = strlen(hostname); + + char *sni_hostname = xmemdup(hostname, len + 1); + + /* Remove trailing dot(s) to fix #47408. + * Regarding RFC 6066 (SNI): The hostname is represented as a byte + * string using ASCII encoding without a trailing dot. */ + while (len && sni_hostname[--len] == '.') + sni_hostname[len] = 0; + + return sni_hostname; +} + bool ssl_connect_wget (int fd, const char *hostname, int *continue_session) { @@ -530,8 +547,12 @@ ssl_connect_wget (int fd, const char *hostname, int *continue_session) /* We set the server name but only if it's not an IP address. */ if (! is_valid_ip_address (hostname)) { - gnutls_server_name_set (session, GNUTLS_NAME_DNS, hostname, - strlen (hostname)); + /* GnuTLS 3.4.x (x<) disrespects the length parameter, we have to construct a new string */ + /* see https://gitlab.com/gnutls/gnutls/issues/78 */ + const char *sni_hostname = _sni_hostname(hostname); + + gnutls_server_name_set (session, GNUTLS_NAME_DNS, sni_hostname, strlen(sni_hostname)); + xfree(sni_hostname); } gnutls_set_default_priority (session); @@ -719,6 +740,7 @@ ssl_check_certificate (int fd, const char *host) gnutls_x509_crt_t cert; const gnutls_datum_t *cert_list; unsigned int cert_list_size; + const char *sni_hostname; if ((err = gnutls_x509_crt_init (&cert)) < 0) { @@ -753,13 +775,15 @@ ssl_check_certificate (int fd, const char *host) logprintf (LOG_NOTQUIET, _("The certificate has expired\n")); success = false; } - if (!gnutls_x509_crt_check_hostname (cert, host)) + sni_hostname = _sni_hostname(host); + if (!gnutls_x509_crt_check_hostname (cert, sni_hostname)) { logprintf (LOG_NOTQUIET, _("The certificate's owner does not match hostname %s\n"), - quote (host)); + quote (sni_hostname)); success = false; } + xfree(sni_hostname); crt_deinit: gnutls_x509_crt_deinit (cert); } diff --git a/src/openssl.c b/src/openssl.c index 6701c0d..48eeadb 100644 --- a/src/openssl.c +++ b/src/openssl.c @@ -35,6 +35,7 @@ as that of the covered work. */ #include <errno.h> #include <unistd.h> #include <string.h> +#include <xalloc.h> #include <openssl/ssl.h> #include <openssl/x509v3.h> @@ -506,6 +507,22 @@ ssl_connect_with_timeout_callback(void *arg) ctx->result = SSL_connect(ctx->ssl); } +static const char * +_sni_hostname(const char *hostname) +{ + size_t len = strlen(hostname); + + char *sni_hostname = xmemdup(hostname, len + 1); + + /* Remove trailing dot(s) to fix #47408. + * Regarding RFC 6066 (SNI): The hostname is represented as a byte + * string using ASCII encoding without a trailing dot. */ + while (len && sni_hostname[--len] == '.') + sni_hostname[len] = 0; + + return sni_hostname; +} + /* Perform the SSL handshake on file descriptor FD, which is assumed to be connected to an SSL server. The SSL handle provided by OpenSSL is registered with the file descriptor FD using @@ -532,7 +549,12 @@ ssl_connect_wget (int fd, const char *hostname, int *continue_session) then use it whenever we have a hostname. If not, don't, ever. */ if (! is_valid_ip_address (hostname)) { - if (! SSL_set_tlsext_host_name (conn, hostname)) + const char *sni_hostname = _sni_hostname(hostname); + + long rc = SSL_set_tlsext_host_name (conn, sni_hostname); + xfree(sni_hostname); + + if (rc == 0) { DEBUGP (("Failed to set TLS server-name indication.")); goto error; @@ -762,9 +784,12 @@ ssl_check_certificate (int fd, const char *host) { /* Test subject alternative names */ + /* SNI hostname must not have a trailing dot */ + const char *sni_hostname = _sni_hostname(host); + /* Do we want to check for dNSNAmes or ipAddresses (see RFC 2818)? * Signal it by host_in_octet_string. */ - ASN1_OCTET_STRING *host_in_octet_string = a2i_IPADDRESS (host); + ASN1_OCTET_STRING *host_in_octet_string = a2i_IPADDRESS (sni_hostname); int numaltnames = sk_GENERAL_NAME_num (subjectAltNames); int i; @@ -799,7 +824,7 @@ ssl_check_certificate (int fd, const char *host) if (0 <= ASN1_STRING_to_UTF8 (&name_in_utf8, name->d.dNSName)) { /* Compare and check for NULL attack in ASN1_STRING */ - if (pattern_match ((char *)name_in_utf8, host) && + if (pattern_match ((char *)name_in_utf8, sni_hostname) && (strlen ((char *)name_in_utf8) = (size_t) ASN1_STRING_length (name->d.dNSName))) { @@ -820,9 +845,11 @@ ssl_check_certificate (int fd, const char *host) logprintf (LOG_NOTQUIET, _("%s: no certificate subject alternative name matches\n" "\trequested host name %s.\n"), - severity, quote_n (1, host)); + severity, quote_n (1, sni_hostname)); success = false; } + + xfree(sni_hostname); } if (alt_name_checked == false) -- 2.7.0
signature.asc
Description: This is a digitally signed message part.
