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.

diff --git a/include/haproxy/ssl_ckch-t.h b/include/haproxy/ssl_ckch-t.h
index f3076e9a768dfc76c88c22d89a6b1d0e562f9687..12739d501fd7e379068f25ff62d58ab237b82276 100644 (file)
--- 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 1f3e0d966c48bcd1e5e626990b296be282a59fe3..0204dca7967fa6feff5c8272485af82683a68bfd 100644 (file)
--- 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 addf77469d1776277f857fa841417b2a16bee153..b1c4c547a989ce3fd9ceda37f809e919b2933ad4 100644 (file)
--- 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 f5f66b172e8a9e7f3b8621e9ca249501c2979f6f..347256d35dc121ac8271c96b4e9f9bf28087e363 100644 (file)
--- 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;