MINOR: ssl: diagnostic warning when both 'default-crt' and 'strict-sni' are used

It possible to use both 'strict-sni' and 'default-crt' on the same bind
line, which does not make much sense.

This patch implements a check which will look for default certificates
in the sni_w tree when strict-sni is used. (Referenced by their empty
sni ""). default-crt sets the CKCH_INST_EXPL_DEFAULT flag in
ckch_inst->is_default, so its possible to differenciate explicits
default from implicit default.

Could be backported as far as 3.0.

This was discussed in ticket #3082.

Author: wlallemand

include/haproxy/ssl_ckch-t.h

@@ -116,6 +116,11 @@ struct ckch_inst_link_ref {
        struct list list;
 };
 
+
+#define CKCH_INST_NO_DEFAULT          0   /* not the default instance */
+#define CKCH_INST_IMPL_DEFAULT        1   /* implicit default instance (declaration order) */
+#define CKCH_INST_EXPL_DEFAULT        2   /* explicit default instance (default-crt) */
+
 /*
  * This structure describe a ckch instance. An instance is generated for each
  * bind_conf.  The instance contains a linked list of the sni ctx which uses
@@ -128,7 +133,7 @@ struct ckch_inst {
 	struct crtlist_entry *crtlist_entry; /* pointer to the crtlist_entry used, or NULL */
 	struct server *server; /* pointer to the server if is_server_instance is set, NULL otherwise */
 	SSL_CTX *ctx; /* pointer to the SSL context used by this instance */
-	unsigned int is_default:1;      /* This instance is used as the default ctx for this bind_conf */
+	unsigned int is_default:2;      /* This instance is used as the default ctx for this bind_conf (1: implicit default, 2: explicit default) */
 	unsigned int is_server_instance:1; /* This instance is used by a backend server */
 	/* space for more flag there */
 	struct list sni_ctx; /* list of sni_ctx using this ckch_inst */

src/cfgparse-ssl.c

@@ -789,7 +789,7 @@ static int bind_parse_ciphersuites(char **args, int cur_arg, struct proxy *px, s
 static int bind_parse_crt(char **args, int cur_arg, struct proxy *px, struct bind_conf *conf, char **err)
 {
 	char path[MAXPATHLEN];
-	int default_crt = *args[cur_arg] == 'd' ? 1 : 0;
+	int default_crt = *args[cur_arg] == 'd' ? CKCH_INST_EXPL_DEFAULT : CKCH_INST_NO_DEFAULT;
 
 	if (!*args[cur_arg + 1]) {
 		memprintf(err, "'%s' : missing certificate location", args[cur_arg]);

src/ssl_ckch.c

@@ -2612,8 +2612,7 @@ int ckch_inst_rebuild(struct ckch_store *ckch_store, struct ckch_inst *ckchi,
 		return 1;
 
 	/* if the previous ckchi was used as the default */
-	if (ckchi->is_default)
-		(*new_inst)->is_default = 1;
+	(*new_inst)->is_default = ckchi->is_default;
 
 	(*new_inst)->is_server_instance = ckchi->is_server_instance;
 	(*new_inst)->server = ckchi->server;

src/ssl_sock.c

@@ -3211,7 +3211,7 @@ int ckch_inst_new_load_store(const char *path, struct ckch_store *ckchs, struct
 	 */
 
 	if (is_default) {
-		ckch_inst->is_default = 1;
+		ckch_inst->is_default = is_default;
 
 		/* insert an empty SNI which will be used to lookup default certificate */
 		order = ckch_inst_add_cert_sni(ctx, ckch_inst, bind_conf, ssl_conf, kinfo, "*", order);
@@ -3447,14 +3447,14 @@ int ssl_sock_load_cert_list_file(char *file, int dir, struct bind_conf *bind_con
 	list_for_each_entry(entry, &crtlist->ord_entries, by_crtlist) {
 		struct ckch_store *store;
 		struct ckch_inst *ckch_inst = NULL;
-		int is_default = 0;
+		int is_default = CKCH_INST_NO_DEFAULT;
 
 		store = entry->node.key;
 
 		/* if the SNI trees were empty the first "crt" become a default certificate,
 		 * it can be applied on multiple certificates if it's a bundle */
 		if (eb_is_empty(&bind_conf->sni_ctx) && eb_is_empty(&bind_conf->sni_w_ctx))
-			is_default = 1;
+			is_default = CKCH_INST_IMPL_DEFAULT;
 
 
 		cfgerr |= ssl_sock_load_ckchs(store->path, store, bind_conf, entry->ssl_conf, entry->filters, entry->fcount, is_default, &ckch_inst, err);
@@ -3509,9 +3509,9 @@ int ssl_sock_load_cert(char *path, struct bind_conf *bind_conf, int is_default,
 
 	/* if the SNI trees were empty the first "crt" become a default certificate,
 	 * it can be applied on multiple certificates if it's a bundle */
-	if (is_default == 0) {
+	if (is_default == CKCH_INST_NO_DEFAULT) {
 		if (eb_is_empty(&bind_conf->sni_ctx) && eb_is_empty(&bind_conf->sni_w_ctx))
-			is_default = 1;
+			is_default = CKCH_INST_IMPL_DEFAULT;
 	}
 
 	if ((ckchs = ckchs_lookup(path))) {
@@ -5027,6 +5027,34 @@ int ssl_sock_prepare_bind_conf(struct bind_conf *bind_conf)
 		}
 	}
 
+	/* check that we didn't use "strict-sni" and "default-crt" together */
+	if (bind_conf->ssl_options & BC_SSL_O_STRICT_SNI) {
+		struct ebmb_node *node, *n;
+		const char *wildp = "";
+		int is_default = CKCH_INST_NO_DEFAULT;
+
+		node = ebst_lookup(&bind_conf->sni_w_ctx, wildp);
+		for (n = node; n; n = ebmb_next_dup(n)) {
+			struct sni_ctx *sni;
+
+			sni = container_of(n, struct sni_ctx, name);
+			if (!sni->neg) {
+
+				if (sni->ckch_inst->is_default == CKCH_INST_EXPL_DEFAULT) {
+					is_default = CKCH_INST_EXPL_DEFAULT;
+					break;
+				}
+			}
+		}
+
+		if (is_default ==  CKCH_INST_EXPL_DEFAULT) {
+			ha_diag_warning("Proxy '%s': both 'default-crt' and 'strict-sni' keywords are used in bind '%s' at [%s:%d], certificates won't be used as fallback (use 'crt' instead).\n",
+				   px->id, bind_conf->arg, bind_conf->file, bind_conf->line);
+		}
+
+	}
+
+
 	if ((bind_conf->options & BC_O_GENERATE_CERTS)) {
 		struct sni_ctx *sni_ctx;