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 0f425f42d404af6223f6bbf72c7c0608b1a8ae1a..4104868351d13ff67d7f7473a2443f9d25399f36 100644 (file)
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -11476,7 +11476,8 @@ sni <expression>
   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 @@ tls-tickets
 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 42d27de9fadf74b7d674d4a8f5bc7d7815a53371..c53cc063e7dc61e89684b51051942198a6e0f3a9 100644 (file)
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -3945,13 +3945,15 @@ static int ssl_sock_srv_verifycbk(int ok, X509_STORE_CTX *ctx)
        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;
        }