konevsa/haproxy
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

BUG/MINOR: ssl: make use of the name in SNI before verifyhost

Browse Source 
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.
This commit is contained in:
Willy Tarreau 2017-07-28 11:38:41 +02:00
parent 96c7b8dbd2
commit ad92a9a7be
2 changed files with 28 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

39
doc/configuration.txt
View File

@ -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

12
src/ssl_sock.c
View File

@ -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;
if (!servername) {
servername = SSL_get_servername(conn->xprt_ctx, TLSEXT_NAMETYPE_host_name);
if (!servername) {
servername = objt_server(conn->target)->ssl_ctx.verify_host;
if (!servername)
return ok;
}