BUG/MINOR: ssl: make use of the name in SNI before verifyhost
Commit 2ab8867 ("MINOR: ssl: compare server certificate names to the SNI
on outgoing connections") introduced the ability to check server cert
names against the name provided with in the SNI, but verifyhost was kept
as a way to force the name to check against. This was a mistake, because :
- if an SNI is used, any static hostname in verifyhost will be wrong ;
worse, if it matches and doesn't match the SNI, the server presented
the wrong certificate ;
- there's no way to have a default name to check against for health
checks anymore because the point above mandates the removal of the
verifyhost directive
This patch reverses the ordering of the check : whenever SNI is used, the
name provided always has precedence (ie the server must always present a
certificate that matches the requested name). And if no SNI is provided,
then verifyhost is used, and will be configured to match the server's
default certificate name. This will work both when SNI is not used and
for health checks.
If the commit 2ab8867 is backported in 1.7 and/or 1.6, this one must be
backported too.
diff --git a/doc/configuration.txt b/doc/configuration.txt
index 0f425f4..4104868 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -11476,7 +11476,8 @@
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. If
"verify required" is set (which is the recommended setting), the resulting
- name will also be matched against the server certificate's names.
+ name will also be matched against the server certificate's names. See the
+ "verify" directive for more details.
source <addr>[:<pl>[-<ph>]] [usesrc { <addr2>[:<port2>] | client | clientip } ]
source <addr>[:<port>] [usesrc { <addr2>[:<port2>] | hdr_ip(<hdr>[,<occ>]) } ]
@@ -11563,26 +11564,28 @@
verify [none|required]
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' 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.
+ certificate provided by the server is verified using CAs from 'ca-file' and
+ optional CRLs from 'crl-file' after having checked that the names provided in
+ the certificate's subject and subjectAlternateNames attributs match either
+ the name passed using the "sni" directive, or if not provided, the static
+ host name passed using the "verifyhost" directive. When no name is found, the
+ certificate's names are ignored. For this reason, without SNI it's important
+ to use "verifyhost". On verification 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. Unless "ssl_server_verify" appears in
+ the global section, "verify" is set to "required" by default.
verifyhost <hostname>
This setting is only available when support for OpenSSL was built in, and
- only takes effect if 'verify required' is also specified. When set, the
- 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. Note
- that the name provided here overrides (for the checks) any possible name
- passed using "sni". See also "no-verifyhost" option.
+ only takes effect if 'verify required' is also specified. This directive sets
+ a default static hostname to check the server's certificate against when no
+ SNI was used to connect to the server. If SNI is not used, this is the only
+ way to enable hostname verification. This static hostname, when set, will
+ also be used for health checks (which cannot provide an SNI value). 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 "verify", "sni" and "no-verifyhost" options.
weight <weight>
The "weight" parameter is used to adjust the server's weight relative to
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 42d27de..c53cc06 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -3945,13 +3945,15 @@
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.
+ /* We're checking if the provided hostnames match the desired one. The
+ * desired hostname comes from the SNI we presented if any, or if not
+ * provided then it may have been explicitly stated using a "verifyhost"
+ * directive. If neither is set, we don't care about the name so the
+ * verification is OK.
*/
- servername = objt_server(conn->target)->ssl_ctx.verify_host;
+ servername = SSL_get_servername(conn->xprt_ctx, TLSEXT_NAMETYPE_host_name);
if (!servername) {
- servername = SSL_get_servername(conn->xprt_ctx, TLSEXT_NAMETYPE_host_name);
+ servername = objt_server(conn->target)->ssl_ctx.verify_host;
if (!servername)
return ok;
}