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

MINOR: ssl: compare server certificate names to the SNI on outgoing connections

Browse Source 
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>
This commit is contained in:
Willy Tarreau 2017-07-05 18:23:03 +02:00
parent 96fd926ccc
commit 2ab88675ec
4 changed files with 92 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
doc/configuration.txt
View File

@ -11474,7 +11474,9 @@ sni <expression>
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 @@ 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'. 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 @@ verifyhost <hostname>
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

5
include/proto/openssl-compat.h
View File

@ -94,6 +94,11 @@ static inline int SSL_SESSION_set1_id_context(SSL_SESSION *s, const unsigned cha
* 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;

17
src/ssl_sock.c
View File

@ -3926,7 +3926,7 @@ static int ssl_sock_srv_verifycbk(int ok, X509_STORE_CTX *ctx)
{
SSL *ssl;
struct connection *conn;
char *servername;
const char *servername;
int depth;
X509 *cert;
@ -3941,7 +3941,20 @@ 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.
*/
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 @@ int ssl_sock_prepare_srv_ctx(struct server *srv)
}
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 */

57
tests/test-srv-verify.cfg Normal file
View File

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