MINOR: ssl: ckch_conf_cmp() compare multiple ckch_conf structures

The ckch_conf_cmp() function allow to compare multiple ckch_conf
structures in order to check that multiple usage of the same crt in the
configuration uses the same ckch_conf definition.

A crt-list allows to use "crt-store" keywords that defines a ckch_store,
that can lead to inconsistencies when a crt is called multiple time with
different parameters.

This function compare and dump a list of differences in the err variable
to be output as error.

The variant ckch_conf_cmp_empty() compares the ckch_conf structure to an
empty one, which is useful for bind lines, that are not able to have
crt-store keywords.

These functions are used when a crt-store is already inialized and we
need to verify if the parameters are compatible.

ckch_conf_cmp() handles multiple cases:

- When the previous ckch_conf was declared with CKCH_CONF_SET_EMPTY, we
  can't define any new keyword in the next initialisation
- When the previous ckch_conf was declared with keywords in a crtlist
  (CKCH_CONF_SET_CRTLIST), the next initialisation must have the exact
  same keywords.
- When the previous ckch_conf was declared in a "crt-store"
  (CKCH_CONF_SET_CRTSTORE), the next initialisaton could use no keyword
  at all or the exact same keywords.

diff --git a/include/haproxy/ssl_ckch-t.h b/include/haproxy/ssl_ckch-t.h
index 0f95c485733b3f049620f44381120b3706811f73..0e501e5565c902e70b10fed9a1a4eb68690027c1 100644 (file)
--- a/include/haproxy/ssl_ckch-t.h
+++ b/include/haproxy/ssl_ckch-t.h
@@ -161,6 +161,16 @@ enum {
        CERT_TYPE_MAX,
 };
 
+/*
+ * When crt-store options are set from a crt-list, the crt-store options must be explicit everywhere.
+ * When crt-store options are set from a crt-store, the crt-store options can be empty, or the exact same
+ */
+enum {
+       CKCH_CONF_SET_EMPTY    = 0,     /* config is empty */
+       CKCH_CONF_SET_CRTLIST  = 1,     /* config is set from a crt-list */
+       CKCH_CONF_SET_CRTSTORE = 2,     /* config is defined in a crt-store */
+};
+
 struct cert_exts {
        const char *ext;
        int type;

diff --git a/include/haproxy/ssl_ckch.h b/include/haproxy/ssl_ckch.h
index 4ee3541cffca3feb46354e4e963185ac101fb1fa..e6356637f86e0de49ede77695260f6d97d2add38 100644 (file)
--- a/include/haproxy/ssl_ckch.h
+++ b/include/haproxy/ssl_ckch.h
@@ -50,6 +50,8 @@ int ckch_store_load_files(struct ckch_conf *f, struct ckch_store *c, int cli, ch
 
 int ckch_conf_parse(char **args, int cur_arg, struct ckch_conf *f, int *found, const char *file, int linenum, char **err);
 void ckch_conf_clean(struct ckch_conf *conf);
+int ckch_conf_cmp(struct ckch_conf *conf1, struct ckch_conf *conf2, char **err);
+int ckch_conf_cmp_empty(struct ckch_conf *prev, char **err);
 
 /* ckch_inst functions */
 void ckch_inst_free(struct ckch_inst *inst);

diff --git a/src/ssl_ckch.c b/src/ssl_ckch.c
index a72da241571ef6d200d3acb60ba88a80e8f73796..a354b9e3af2a381ecf18a0e410030773b71f6b08 100644 (file)
--- a/src/ssl_ckch.c
+++ b/src/ssl_ckch.c
@@ -1003,6 +1003,8 @@ struct ckch_store *ckch_store_new_load_files_path(char *path, char **err)
        if (ssl_sock_load_files_into_ckch(path, ckchs->data, err) == 1)
                goto end;
 
+       ckchs->conf.used = CKCH_CONF_SET_EMPTY;
+
        /* insert into the ckchs tree */
        memcpy(ckchs->path, path, strlen(path) + 1);
        ebst_insert(&ckchs_tree, &ckchs->node);
@@ -4147,6 +4149,122 @@ out:
        return err_code;
 }
 
+/*
+ * Check if ckch_conf <prev> and <new> are compatible:
+ *
+ * new \ prev | EMPTY | CRTLIST  | CRTSTORE
+ * ----------------------------------------
+ *  EMPTY     |  OK   |  X       |   OK
+ * ----------------------------------------
+ *  CRTLIST   |  X    | CMP      |  CMP
+ * ----------------------------------------
+ *
+ * Return:
+ *  1 when the 2 structures have different variables or are incompatible
+ *  0 when the 2 structures have equal variables or are compatibles
+ */
+int ckch_conf_cmp(struct ckch_conf *prev, struct ckch_conf *new, char **err)
+{
+       int ret = 0;
+       int i;
+
+       /* compatibility check */
+
+       if (prev->used == CKCH_CONF_SET_EMPTY) {
+               if (new->used == CKCH_CONF_SET_CRTLIST) {
+                       memprintf(err, "%sCan't use the certificate previously defined without any keyword with these keywords:\n", *err ? *err : "");
+                       ret = 1;
+               }
+               if (new->used == CKCH_CONF_SET_EMPTY)
+                       return 0;
+
+       } else if (prev->used == CKCH_CONF_SET_CRTLIST) {
+               if (new->used == CKCH_CONF_SET_EMPTY) {
+                       memprintf(err, "%sCan't use the certificate previously defined with keywords without these keywords:\n", *err ? *err : "");
+                       ret = 1;
+               }
+       } else if (prev->used == CKCH_CONF_SET_CRTSTORE) {
+               if (new->used == CKCH_CONF_SET_EMPTY)
+                       return 0;
+       }
+
+
+       for (i = 0; ckch_conf_kws[i].name != NULL; i++) {
+
+               if (strcmp(ckch_conf_kws[i].name, "crt") == 0)
+                       continue;
+
+               switch (ckch_conf_kws[i].type) {
+                       case PARSE_TYPE_STR: {
+                               char *avail1, *avail2;
+                               avail1 = prev ? *(char **)((intptr_t)prev + (ptrdiff_t)ckch_conf_kws[i].offset) : NULL;
+                               avail2 = new ? *(char **)((intptr_t)new + (ptrdiff_t)ckch_conf_kws[i].offset) : NULL;
+
+                               /* must alert when strcmp is wrong, or when one of the field is NULL */
+                               if (((avail1 && avail2) && strcmp(avail1, avail2) != 0) || (!!avail1 ^ !!avail2)) {
+                                       memprintf(err, "%s- different parameter '%s' : previously '%s' vs '%s'\n", *err ? *err : "", ckch_conf_kws[i].name, avail1, avail2);
+                                       ret = 1;
+                               }
+                       }
+                       break;
+
+                       default:
+                               break;
+               }
+               /* special case for ocsp-update and default */
+               if (strcmp(ckch_conf_kws[i].name, "ocsp-update") == 0) {
+                       int o1, o2; /* ocsp-update from the configuration */
+                       int q1, q2; /* final ocsp-update value (from default) */
+
+
+                       o1 = prev ? *(int *)((intptr_t)prev + (ptrdiff_t)ckch_conf_kws[i].offset) : 0;
+                       o2 = new ? *(int *)((intptr_t)new + (ptrdiff_t)ckch_conf_kws[i].offset) : 0;
+
+                       q1 = (o1 == SSL_SOCK_OCSP_UPDATE_DFLT) ? global_ssl.ocsp_update.mode : o1;
+                       q2 = (o2 == SSL_SOCK_OCSP_UPDATE_DFLT) ? global_ssl.ocsp_update.mode : o2;
+
+                       if (q1 != q2) {
+                               int j = 1;
+                               int o = o1;
+                               int q = q1;
+                               memprintf(err, "%s- different parameter '%s' : previously ", *err ? *err : "", ckch_conf_kws[i].name);
+
+                               do {
+                                       switch (o) {
+                                               case SSL_SOCK_OCSP_UPDATE_DFLT:
+                                                       memprintf(err, "%s'default' (ocsp-update.mode %s)", *err ? *err : "", (q > 0) ? "on" : "off");
+                                               break;
+                                               case SSL_SOCK_OCSP_UPDATE_ON:
+                                                       memprintf(err, "%s'%s'", *err ? *err : "", "on");
+                                               break;
+                                               case SSL_SOCK_OCSP_UPDATE_OFF:
+                                                       memprintf(err, "%s'%s'", *err ? *err : "", "off");
+                                               break;
+                                       }
+                                       o = o2;
+                                       q = q2;
+                                       if (j)
+                                               memprintf(err, "%s vs ", *err ? *err : "");
+                               } while (j--);
+                               memprintf(err, "%s\n", *err ? *err : "");
+                               ret = 1;
+                       }
+               }
+       }
+
+out:
+       return ret;
+}
+
+/*
+ *  Compare a previously generated ckch_conf with an empty one, using ckch_conf_cmp().
+ */
+int ckch_conf_cmp_empty(struct ckch_conf *prev, char **err)
+{
+       struct ckch_conf new = {};
+
+       return ckch_conf_cmp(prev, &new, err);
+}
 
 /* parse ckch_conf keywords for crt-list */
 int ckch_conf_parse(char **args, int cur_arg, struct ckch_conf *f, int *found, const char *file, int linenum, char **err)
@@ -4315,6 +4433,7 @@ static int crtstore_parse_load(char **args, int section_type, struct proxy *curp
                goto out;
 
        c->conf = f;
+       c->conf.used = CKCH_CONF_SET_CRTSTORE;
 
        if (ebst_insert(&ckchs_tree, &c->node) != &c->node) {
                memprintf(err,"parsing [%s:%d] : '%s' in section 'crt-store': store '%s' was already defined.",

diff --git a/src/ssl_crtlist.c b/src/ssl_crtlist.c
index a0dfbcf66f035ccd8de00c66b8728e6a07cdd059..d8e100318a640d42028e789a52d18045285979c8 100644 (file)
--- a/src/ssl_crtlist.c
+++ b/src/ssl_crtlist.c
@@ -472,9 +472,11 @@ int crtlist_parse_line(char *line, char **crt_path, struct crtlist_entry *entry,
                        if (cfgerr & ERR_FATAL)
                                goto error;
 
-                       cc->used = 1;
-                       if (newarg) /* skip 2 words if the keyword was found */
-                               cur_arg += 2;
+                       if (newarg) {
+                               cur_arg += 2;  /* skip 2 words if the keyword was found */
+                               cc->used = CKCH_CONF_SET_CRTLIST; /* if they are options they must be used everywhere */
+                       }
+
                }
 out:
                if (!cfgerr && !newarg) {
@@ -697,6 +699,12 @@ int crtlist_parse_file(char *file, struct bind_conf *bind_conf, struct proxy *cu
                        }
 
                } else {
+                       if (ckch_conf_cmp(&ckchs->conf, &cc, err) != 0) {
+                               memprintf(err, "'%s' in crt-list '%s' line %d, is already defined with incompatible parameters:\n %s", crt_path, file, linenum, err ? *err : "");
+                               cfgerr |= ERR_ALERT | ERR_FATAL;
+                               goto error;
+                       }
+
                        entry->node.key = ckchs;
                        entry->crtlist = newlist;
 

diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 6a94e8b279d439b04d464bf9b72866323afd25d8..b2d643c51c49eff2b604236cda6bbf4a0614f863 100644 (file)
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -3843,6 +3843,13 @@ int ssl_sock_load_cert(char *path, struct bind_conf *bind_conf, int is_default,
        }
 
        if ((ckchs = ckchs_lookup(path))) {
+
+               cfgerr |= ckch_conf_cmp_empty(&ckchs->conf, err);
+               if (cfgerr & ERR_CODE) {
+                       memprintf(err, "Can't load '%s', is already defined with incompatible parameters:\n %s", path, err ? *err : "");
+                       return cfgerr;
+               }
+
                /* we found the ckchs in the tree, we can use it directly */
                 cfgerr |= ssl_sock_load_ckchs(path, ckchs, bind_conf, NULL, NULL, 0, is_default, &ckch_inst, err);