MINOR: ssl: support strict-sni in ssl-default-bind-options

Several users already reported that it would be nice to support
strict-sni in ssl-default-bind-options. However, in order to support
it, we also need an option to disable it.

This patch moves the setting of the option from the strict_sni field
to a flag in the ssl_options field so that it can be inherited from
the default bind options, and adds a new "no-strict-sni" directive to
allow to disable it on a specific "bind" line.

The test file "del_ssl_crt-list.vtc" which already tests both options
was updated to make use of the default option and the no- variant to
confirm everything continues to work.

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 734dc49824597cde8799b4e0c83bd47499fc7887..7fcbd266d6067353407f4a430eba6b633f65f710 100644 (file)
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -16794,6 +16794,13 @@ no-sslv3
   global statement "ssl-default-bind-options". Use "ssl-min-ver" and
   "ssl-max-ver" instead.
 
+no-strict-sni
+  This setting is only available when support for OpenSSL was built in. It
+  disables strict-sni enforcement from a previous "strict-sni" directive. It
+  may be needed in order to selectively disable strict-sni usage on a "bind"
+  line when it was already globally enforced via "ssl-default-bind-options".
+  See also the "strict-sni" bind option.
+
 no-tls-tickets
   This setting is only available when support for OpenSSL was built in. It
   disables the stateless session resumption (RFC 5077 TLS Ticket
@@ -17007,9 +17014,10 @@ strict-sni
   SSL/TLS negotiation is allowed only if the client provided an SNI that matches
   a certificate. The default certificate is not used. This option also allows
   starting without any certificate on a bind line, so an empty directory could
-  be used and filled later from the stats socket.
-  See the "crt" option for more information. See "add ssl crt-list" command in
-  the management guide.
+  be used and filled later from the stats socket. This option is also available
+  on global statement "ssl-default-bind-options", and may be selectively
+  disabled on a "bind" line using "no-strict-sni". See the "crt" option for
+  more information. See "add ssl crt-list" command in the management guide.
 
 tcp-ut <delay>
   Sets the TCP User Timeout for all incoming connections instantiated from this

diff --git a/include/haproxy/listener-t.h b/include/haproxy/listener-t.h
index 7518aa6ec121053a60e1f0405be5f980a1dab5f6..b054b75a6bf7e9a4043f6e4aca75c594ee6780f3 100644 (file)
--- a/include/haproxy/listener-t.h
+++ b/include/haproxy/listener-t.h
@@ -121,6 +121,7 @@ enum li_status {
 #define BC_SSL_O_NONE           0x0000
 #define BC_SSL_O_NO_TLS_TICKETS 0x0100 /* disable session resumption tickets */
 #define BC_SSL_O_PREF_CLIE_CIPH 0x0200  /* prefer client ciphers */
+#define BC_SSL_O_STRICT_SNI     0x0400  /* refuse negotiation if sni doesn't match a certificate */
 #endif
 
 struct tls_version_filter {
@@ -169,7 +170,6 @@ struct bind_conf {
        unsigned long long ca_ignerr_bitfield[IGNERR_BF_SIZE];   /* ignored verify errors in handshake if depth > 0 */
        unsigned long long crt_ignerr_bitfield[IGNERR_BF_SIZE];  /* ignored verify errors in handshake if depth == 0 */
        void *initial_ctx;             /* SSL context for initial negotiation */
-       int strict_sni;            /* refuse negotiation if sni doesn't match a certificate */
        int ssl_options;           /* ssl options */
        struct eb_root sni_ctx;    /* sni_ctx tree of all known certs full-names sorted by name */
        struct eb_root sni_w_ctx;  /* sni_ctx tree of all known certs wildcards sorted by name */

diff --git a/reg-tests/ssl/del_ssl_crt-list.vtc b/reg-tests/ssl/del_ssl_crt-list.vtc
index 3a2beccb64091db1ce82b05b5cf0dd8e713c7873..d711475eae39fc4962fbf928bbdcf4c5f0a77fa3 100644 (file)
--- a/reg-tests/ssl/del_ssl_crt-list.vtc
+++ b/reg-tests/ssl/del_ssl_crt-list.vtc
@@ -26,6 +26,7 @@ haproxy h1 -conf {
         tune.ssl.capture-buffer-size 1
         crt-base ${testdir}
         stats socket "${tmpdir}/h1/stats" level admin
+        ssl-default-bind-options strict-sni
 
     defaults
         mode http
@@ -47,13 +48,14 @@ haproxy h1 -conf {
         server s3 "${tmpdir}/first-ssl.sock" ssl verify none sni str(record2.bug940.domain.tld)
 
     listen first-ssl-fe
+        # note: strict-sni is enforced from ssl-default-bind-options above
         mode http
-        bind "${tmpdir}/first-ssl.sock" ssl strict-sni crt-list ${testdir}/simple.crt-list
+        bind "${tmpdir}/first-ssl.sock" ssl crt-list ${testdir}/simple.crt-list
         server s1 ${s1_addr}:${s1_port}
 
     listen second-ssl-fe
         mode http
-        bind "${tmpdir}/second-ssl.sock" ssl crt-list ${testdir}/localhost.crt-list
+        bind "${tmpdir}/second-ssl.sock" ssl no-strict-sni crt-list ${testdir}/localhost.crt-list
         server s1 ${s1_addr}:${s1_port}
 } -start
 

diff --git a/src/cfgparse-ssl.c b/src/cfgparse-ssl.c
index 3192121d6a83c528582592572956ad183356f3a0..d100262f58e6f62d7c63260b3c6a13a476fa917a 100644 (file)
--- a/src/cfgparse-ssl.c
+++ b/src/cfgparse-ssl.c
@@ -1307,10 +1307,13 @@ static int bind_parse_generate_certs(char **args, int cur_arg, struct proxy *px,
        return 0;
 }
 
-/* parse the "strict-sni" bind keyword */
+/* parse the "strict-sni" and "no-strict-sni" bind keywords */
 static int bind_parse_strict_sni(char **args, int cur_arg, struct proxy *px, struct bind_conf *conf, char **err)
 {
-       conf->strict_sni = 1;
+       if (strncmp(args[cur_arg], "no-", 3) != 0)
+               conf->ssl_options |= BC_SSL_O_STRICT_SNI;
+       else
+               conf->ssl_options &= ~BC_SSL_O_STRICT_SNI;
        return 0;
 }
 
@@ -2029,6 +2032,10 @@ static int ssl_parse_default_bind_options(char **args, int section_type, struct
                        global_ssl.listen_default_ssloptions |= BC_SSL_O_NO_TLS_TICKETS;
                else if (strcmp(args[i], "prefer-client-ciphers") == 0)
                        global_ssl.listen_default_ssloptions |= BC_SSL_O_PREF_CLIE_CIPH;
+               else if (strcmp(args[i], "strict-sni") == 0)
+                       global_ssl.listen_default_ssloptions |= BC_SSL_O_STRICT_SNI;
+               else if (strcmp(args[i], "no-strict-sni") == 0)
+                       global_ssl.listen_default_ssloptions &= ~BC_SSL_O_STRICT_SNI;
                else if (strcmp(args[i], "ssl-min-ver") == 0 || strcmp(args[i], "ssl-max-ver") == 0) {
                        if (!parse_tls_method_minmax(args, i, &global_ssl.listen_default_sslmethods, err))
                                i++;
@@ -2446,6 +2453,7 @@ static struct bind_kw_list bind_kws = { "SSL", { }, {
        { "no-alpn",               bind_parse_no_alpn,            0 }, /* disable sending ALPN */
        { "no-ca-names",           bind_parse_no_ca_names,        0 }, /* do not send ca names to clients (ca_file related) */
        { "no-sslv3",              bind_parse_tls_method_options, 0 }, /* disable SSLv3 */
+       { "no-strict-sni",         bind_parse_strict_sni,         0 }, /* do not refuse negotiation if sni doesn't match a certificate */
        { "no-tlsv10",             bind_parse_tls_method_options, 0 }, /* disable TLSv10 */
        { "no-tlsv11",             bind_parse_tls_method_options, 0 }, /* disable TLSv11 */
        { "no-tlsv12",             bind_parse_tls_method_options, 0 }, /* disable TLSv12 */

diff --git a/src/ssl_clienthello.c b/src/ssl_clienthello.c
index 1ee13bae1eda8273a503cf4984f820d3ab1b73c6..19f1411549fcc1af456ab7c56a92c7aa18833509 100644 (file)
--- a/src/ssl_clienthello.c
+++ b/src/ssl_clienthello.c
@@ -274,7 +274,7 @@ int ssl_sock_switchctx_cbk(SSL *ssl, int *al, void *arg)
 #endif
 
                /* no servername field is not compatible with strict-sni */
-               if (s->strict_sni) {
+               if (s->ssl_options & BC_SSL_O_STRICT_SNI) {
                        TRACE_ERROR("No server_name provided and 'strict-sni' enabled", SSL_EV_CONN_SWITCHCTX_CB|SSL_EV_CONN_ERR, conn);
                        goto abort;
                }
@@ -435,7 +435,7 @@ sni_lookup:
        }
 #endif
 
-       if (!s->strict_sni && !default_lookup) {
+       if (!(s->ssl_options & BC_SSL_O_STRICT_SNI) && !default_lookup) {
                /* we didn't find a SNI, and we didn't look for a default
                 * look again to find a matching default cert */
                servername = "";
@@ -541,7 +541,7 @@ int ssl_sock_switchctx_cbk(SSL *ssl, int *al, void *priv)
                        return SSL_TLSEXT_ERR_OK;
                }
 #endif
-               if (s->strict_sni) {
+               if (s->ssl_options & BC_SSL_O_STRICT_SNI) {
                        TRACE_ERROR("No server_name provided and 'strict-sni' enabled", SSL_EV_CONN_SWITCHCTX_CB|SSL_EV_CONN_ERR);
                        return SSL_TLSEXT_ERR_ALERT_FATAL;
                }
@@ -598,7 +598,7 @@ sni_lookup:
 #endif
                HA_RWLOCK_RDUNLOCK(SNI_LOCK, &s->sni_lock);
 
-               if (!s->strict_sni && !default_lookup) {
+               if (!(s->ssl_options & BC_SSL_O_STRICT_SNI) && !default_lookup) {
                        /* we didn't find a SNI, and we didn't look for a default
                         * look again to find a matching default cert */
                        servername = "";
@@ -644,7 +644,7 @@ int ssl_sock_switchctx_wolfSSL_cbk(WOLFSSL* ssl, void* arg)
 
        servername = SSL_get_servername(ssl, TLSEXT_NAMETYPE_host_name);
        if (!servername) {
-               if (s->strict_sni)
+               if (s->ssl_options & BC_SSL_O_STRICT_SNI)
                        goto abort;
 
                /* without servername extension, look for the defaults which is
@@ -720,7 +720,7 @@ sni_lookup:
        }
 
        HA_RWLOCK_RDUNLOCK(SNI_LOCK, &s->sni_lock);
-       if (!s->strict_sni && !default_lookup) {
+       if (!(s->ssl_options & BC_SSL_O_STRICT_SNI) && !default_lookup) {
                /* we didn't find a SNI, and we didn't look for a default
                 * look again to find a matching default cert */
                servername = "";

diff --git a/src/ssl_crtlist.c b/src/ssl_crtlist.c
index 2f158db5bd9d8f9ca7495aebebacfd54e12df5c5..f984244003ef3a646d64c0664e24e67a2cc71ce3 100644 (file)
--- a/src/ssl_crtlist.c
+++ b/src/ssl_crtlist.c
@@ -1577,7 +1577,7 @@ static int cli_parse_del_crtlist(char **args, char *payload, struct appctx *appc
        /* Iterate over all the instances in order to see if any of them is a
         * default instance. If this is the case, the entry won't be suppressed. */
        list_for_each_entry_safe(inst, inst_s, &entry->ckch_inst, by_crtlist_entry) {
-               if (inst->is_default && !inst->bind_conf->strict_sni) {
+               if (inst->is_default && !(inst->bind_conf->ssl_options & BC_SSL_O_STRICT_SNI)) {
                        if (!error_message_dumped) {
                                memprintf(&err, "certificate '%s' cannot be deleted, it is used as default certificate by the following frontends:\n", cert_path);
                                error_message_dumped = 1;

diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index e7f01bc84ffc407004626f6c2763707b9670fe06..c897bbf18efa9ea084e64a803ac9928ca3178451 100644 (file)
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -4776,7 +4776,7 @@ int ssl_sock_prepare_bind_conf(struct bind_conf *bind_conf)
 
        /* check if we have certificates */
        if (eb_is_empty(&bind_conf->sni_ctx) && eb_is_empty(&bind_conf->sni_w_ctx)) {
-               if (bind_conf->strict_sni && !(bind_conf->options & BC_O_GENERATE_CERTS)) {
+               if ((bind_conf->ssl_options & BC_SSL_O_STRICT_SNI) && !(bind_conf->options & BC_O_GENERATE_CERTS)) {
                        ha_warning("Proxy '%s': no SSL certificate specified for bind '%s' at [%s:%d], ssl connections will fail (use 'crt').\n",
                                   px->id, bind_conf->arg, bind_conf->file, bind_conf->line);
                }