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.

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.
+  be used and filled later from the stats socket. This option is also available
-  See the "crt" option for more information. See "add ssl crt-list" command in
+  on global statement "ssl-default-bind-options", and may be selectively
-  the management guide.
+  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

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

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
 

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

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 = "";

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;

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);
 		}