From 18ebd81962e1f53b8f59bfee5b9795bff69ac16b Mon Sep 17 00:00:00 2001 From: William Lallemand Date: Wed, 27 Aug 2025 15:44:24 +0200 Subject: [PATCH] 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. --- include/haproxy/ssl_ckch-t.h | 7 ++++++- src/cfgparse-ssl.c | 2 +- src/ssl_ckch.c | 3 +-- src/ssl_sock.c | 38 +++++++++++++++++++++++++++++++----- 4 files changed, 41 insertions(+), 9 deletions(-) diff --git a/include/haproxy/ssl_ckch-t.h b/include/haproxy/ssl_ckch-t.h index f3076e9a7..12739d501 100644 --- a/include/haproxy/ssl_ckch-t.h +++ b/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 */ diff --git a/src/cfgparse-ssl.c b/src/cfgparse-ssl.c index 1f3e0d966..0204dca79 100644 --- a/src/cfgparse-ssl.c +++ b/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]); diff --git a/src/ssl_ckch.c b/src/ssl_ckch.c index addf77469..b1c4c547a 100644 --- a/src/ssl_ckch.c +++ b/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; diff --git a/src/ssl_sock.c b/src/ssl_sock.c index f5f66b172..347256d35 100644 --- a/src/ssl_sock.c +++ b/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;