MINOR: ssl: compare server certificate names to the SNI on outgoing connections
When support for passing SNI to the server was added in 1.6-dev3, there
was no way to validate that the certificate presented by the server would
really match the name requested in the SNI, which is quite a problem as
it allows other (valid) certificates to be presented instead (when hitting
the wrong server or due to a man in the middle).
This patch adds the missing check against the value passed in the SNI.
The "verifyhost" value keeps precedence if set. If no SNI is used and
no verifyhost directive is specified, then the certificate name is not
checked (this is unchanged).
In order to extract the SNI value, it was necessary to make use of
SSL_SESSION_get0_hostname(), which appeared in openssl 1.1.0. This is
a trivial function which returns the value of s->tlsext_hostname, so
it was provided in the compat layer for older versions. After some
refinements from Emmanuel, it now builds with openssl 1.0.2, openssl
1.1.0 and boringssl. A test file was provided to ease testing all cases.
After some careful observation period it may make sense to backport
this to 1.7 and 1.6 as some users rightfully consider this limitation
as a bug.
Cc: Emmanuel Hocdet <manu@gandi.net>
Signed-off-by: Willy Tarreau <w@1wt.eu>
diff --git a/doc/configuration.txt b/doc/configuration.txt
index 7cac3ca..0f425f4 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -11474,7 +11474,9 @@
string and uses the result as the host name sent in the SNI TLS extension to
the server. A typical use case is to send the SNI received from the client in
a bridged HTTPS scenario, using the "ssl_fc_sni" sample fetch for the
- expression, though alternatives such as req.hdr(host) can also make sense.
+ expression, though alternatives such as req.hdr(host) can also make sense. If
+ "verify required" is set (which is the recommended setting), the resulting
+ name will also be matched against the server certificate's names.
source <addr>[:<pl>[-<ph>]] [usesrc { <addr2>[:<port2>] | client | clientip } ]
source <addr>[:<port>] [usesrc { <addr2>[:<port2>] | hdr_ip(<hdr>[,<occ>]) } ]
@@ -11562,11 +11564,15 @@
This setting is only available when support for OpenSSL was built in. If set
to 'none', server certificate is not verified. In the other case, The
certificate provided by the server is verified using CAs from 'ca-file'
- and optional CRLs from 'crl-file'. If 'ssl_server_verify' is not specified
- in global section, this is the default. On verify failure the handshake
- is aborted. It is critically important to verify server certificates when
- using SSL to connect to servers, otherwise the communication is prone to
- trivial man-in-the-middle attacks rendering SSL totally useless.
+ and optional CRLs from 'crl-file' after having checked that the names
+ provided in the certificate match either the static host name passed using
+ the "verifyhost" directive, or if not provided, the name passed using the
+ "sni" directive. When no name is found, the certificate's names are ignored.
+ If 'ssl_server_verify' is not specified in global section, this is the
+ default. On verify failure the handshake is aborted. It is critically
+ important to verify server certificates when using SSL to connect to servers,
+ otherwise the communication is prone to trivial man-in-the-middle attacks
+ rendering SSL totally useless.
verifyhost <hostname>
This setting is only available when support for OpenSSL was built in, and
@@ -11574,8 +11580,9 @@
hostnames in the subject and subjectAlternateNames of the certificate
provided by the server are checked. If none of the hostnames in the
certificate match the specified hostname, the handshake is aborted. The
- hostnames in the server-provided certificate may include wildcards.
- See also "no-verifyhost" option.
+ hostnames in the server-provided certificate may include wildcards. Note
+ that the name provided here overrides (for the checks) any possible name
+ passed using "sni". See also "no-verifyhost" option.
weight <weight>
The "weight" parameter is used to adjust the server's weight relative to
diff --git a/include/proto/openssl-compat.h b/include/proto/openssl-compat.h
index 7261a7b..6ff4fdd 100644
--- a/include/proto/openssl-compat.h
+++ b/include/proto/openssl-compat.h
@@ -94,6 +94,11 @@
* Functions introduced in OpenSSL 1.1.0 and not yet present in LibreSSL
*/
+static inline const char *SSL_SESSION_get0_hostname(const SSL_SESSION *sess)
+{
+ return sess->tlsext_hostname;
+}
+
static inline const unsigned char *SSL_SESSION_get0_id_context(const SSL_SESSION *sess, unsigned int *sid_ctx_length)
{
*sid_ctx_length = sess->sid_ctx_length;
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 283b22b..19cc526 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -3926,7 +3926,7 @@
{
SSL *ssl;
struct connection *conn;
- char *servername;
+ const char *servername;
int depth;
X509 *cert;
@@ -3941,7 +3941,20 @@
ssl = X509_STORE_CTX_get_ex_data(ctx, SSL_get_ex_data_X509_STORE_CTX_idx());
conn = SSL_get_app_data(ssl);
+ /* we're checking against the configured "verifyhost" directive if
+ * present, or against the SNI used on this connection if present.
+ * If neither is set, the verification is OK.
+ */
servername = objt_server(conn->target)->ssl_ctx.verify_host;
+ if (!servername) {
+ SSL_SESSION *ssl_sess = SSL_get_session(conn->xprt_ctx);
+ if (!ssl_sess)
+ return ok;
+
+ servername = SSL_SESSION_get0_hostname(ssl_sess);
+ if (!servername)
+ return ok;
+ }
/* We only need to verify the CN on the actual server cert,
* not the indirect CAs */
@@ -4138,7 +4151,7 @@
}
SSL_CTX_set_verify(srv->ssl_ctx.ctx,
verify,
- srv->ssl_ctx.verify_host ? ssl_sock_srv_verifycbk : NULL);
+ (srv->ssl_ctx.verify_host || (verify & SSL_VERIFY_PEER)) ? ssl_sock_srv_verifycbk : NULL);
if (verify & SSL_VERIFY_PEER) {
if (srv->ssl_ctx.ca_file) {
/* load CAfile to verify */
diff --git a/tests/test-srv-verify.cfg b/tests/test-srv-verify.cfg
new file mode 100644
index 0000000..e3ccc83
--- /dev/null
+++ b/tests/test-srv-verify.cfg
@@ -0,0 +1,57 @@
+global
+ maxconn 490
+ stats socket /tmp/sock1 mode 666 level admin
+ stats timeout 10m
+ ssl-server-verify none
+ tune.ssl.default-dh-param 1024
+ log /dev/log local0 debug info
+
+defaults
+ mode http
+ log global
+ option httplog
+ option dontlognull
+ timeout connect 5s
+ timeout http-keep-alive 15s
+ timeout http-request 15s
+ timeout queue 30s
+ timeout tarpit 1m
+ timeout tunnel 300s
+ timeout client 30s
+ timeout server 60s
+
+listen 1
+ bind :8001
+
+ # passes checks and traffic (no hostname check)
+ # server ssl 127.0.0.1:8443 ssl verify required check inter 500 ca-file rsa2048.pem
+
+ # passes checks and traffic (localhost is what the server presents)
+ # server ssl 127.0.0.1:8443 ssl verify required check inter 500 ca-file rsa2048.pem verifyhost localhost
+
+ # fails checks and traffic (foo not matched on the server)
+ # server ssl 127.0.0.1:8443 ssl verify required check inter 500 ca-file rsa2048.pem verifyhost foo
+
+ # passes checks and traffic (verify none ignores the host)
+ # server ssl 127.0.0.1:8443 ssl verify none check inter 500 ca-file rsa2048.pem verifyhost foo
+
+ # passes checks and traffic (localhost is fine)
+ # server ssl 127.0.0.1:8443 ssl verify required check inter 500 ca-file rsa2048.pem sni str(localhost) verifyhost localhost
+
+ # passes checks and traffic (verifyhost overrides sni)
+ # server ssl 127.0.0.1:8443 ssl verify required check inter 500 ca-file rsa2048.pem sni str(foo) verifyhost localhost
+
+ # passes checks and traffic (localhost always valid)
+ # server ssl 127.0.0.1:8443 ssl verify required check inter 500 ca-file rsa2048.pem sni str(localhost)
+
+ # passes checks, and traffic without host or with "host: localhost" and fails other hosts.
+ server ssl 127.0.0.1:8443 ssl verify required check inter 500 ca-file rsa2048.pem sni req.hdr(host)
+
+ # just for tests
+ #server clear 127.0.0.1:8480
+
+listen 2
+ bind :8480
+ bind :8443 ssl crt rsa2048.pem
+ stats uri /
+