MINOR: ssl/cli: assignate a new ckch_store

When updating a certificate from the CLI, it is not possible to revert
some of the changes if part of the certicate update failed. We now
creates a copy of the ckch_store for the changes so we can revert back
if something goes wrong.

Even if the ckch_store was affected before this change, it wasn't
affecting the SSL_CTXs used for the traffic. It was only a problem if we
try to update a certificate after we failed to do it the first time.

The new ckch_store is also linked to the new sni_ctxs so it's easy to
insert the sni_ctxs before removing the old ones.

diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index f8d8259ea04bb7078fc11563ac88e6ce10403634..fc82ba8d412864cab7f2eea0fe260ed81ec98a89 100644 (file)
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -9953,8 +9953,8 @@ struct {
 static int cli_parse_set_cert(char **args, char *payload, struct appctx *appctx, void *private)
 {
        struct ckch_store *ckchs = NULL;
+       struct ckch_store *new_ckchs = NULL;
        struct cert_key_and_chain *ckch;
-       struct list tmp_ckchi_list;
        char *tmpfp = NULL;
        char *err = NULL;
        int i;
@@ -9976,8 +9976,6 @@ static int cli_parse_set_cert(char **args, char *payload, struct appctx *appctx,
        if (HA_SPIN_TRYLOCK(CKCH_LOCK, &ckch_lock))
                return cli_err(appctx, "Can't update the certificate!\nOperations on certificates are currently locked!\n");
 
-       LIST_INIT(&tmp_ckchi_list);
-
        /* check which type of file we want to update */
        for (i = 0; i < CERT_TYPE_MAX; i++) {
                end = strrchr(tmpfp, '.');
@@ -10006,11 +10004,6 @@ static int cli_parse_set_cert(char **args, char *payload, struct appctx *appctx,
                        if (bundle >= 0 && ckchs->multi == 0)
                                continue;
 
-                       if (bundle < 0)
-                               ckch = ckchs->ckch;
-                       else
-                               ckch = &ckchs->ckch[bundle];
-
                        if (ckchs->filters) {
                                memprintf(&err, "%sCertificates used in crt-list with filters are not supported!\n",
                                          err ? err : "");
@@ -10020,12 +10013,28 @@ static int cli_parse_set_cert(char **args, char *payload, struct appctx *appctx,
 
                        found = 1;
 
+                       /* duplicate the ckch store */
+                       new_ckchs = ckchs_dup(ckchs);
+                       if (!new_ckchs) {
+                               memprintf(&err, "%sCannot allocate memory!\n",
+                                         err ? err : "");
+                               errcode |= ERR_ALERT | ERR_FATAL;
+                               goto end;
+                       }
+
+                       if (bundle < 0)
+                               ckch = new_ckchs->ckch;
+                       else
+                               ckch = &new_ckchs->ckch[bundle];
+
+
+                       /* appply the change on the duplicate */
                        if (cert_exts[type].load(tmpfp, payload, ckch, &err) != 0) {
                                errcode |= ERR_ALERT | ERR_FATAL;
                                goto end;
                        }
 
-                       /* walk through ckch_inst and creates new ckch_inst using the updated ckch */
+                       /* walk through the old ckch_inst and creates new ckch_inst using the updated ckchs */
                        list_for_each_entry(ckchi, &ckchs->ckch_inst, by_ckchs) {
                                struct ckch_inst *new_inst;
 
@@ -10037,11 +10046,18 @@ static int cli_parse_set_cert(char **args, char *payload, struct appctx *appctx,
                                if (errcode & ERR_CODE)
                                        goto end;
 
-                               /* link temporary the new ckch_inst */
-                               LIST_ADDQ(&tmp_ckchi_list, &new_inst->by_ckchs);
+                               /* link the new ckch_inst to the duplicate */
+                               LIST_ADDQ(&new_ckchs->ckch_inst, &new_inst->by_ckchs);
                        }
 
-                       /* once every allocation is done, delete the old sni_ctx & the old ckch_insts */
+                       /* insert every new SNIs in the trees */
+                       list_for_each_entry_safe(ckchi, ckchis, &new_ckchs->ckch_inst, by_ckchs) {
+                               HA_RWLOCK_WRLOCK(SNI_LOCK, &ckchi->bind_conf->sni_lock);
+                               ssl_sock_load_cert_sni(ckchi, ckchi->bind_conf);
+                               HA_RWLOCK_WRUNLOCK(SNI_LOCK, &ckchi->bind_conf->sni_lock);
+                       }
+
+                       /* delete the old sni_ctx, the old ckch_insts and the ckch_store */
                        list_for_each_entry_safe(ckchi, ckchis, &ckchs->ckch_inst, by_ckchs) {
                                struct sni_ctx *sc0, *sc0s;
 
@@ -10056,14 +10072,11 @@ static int cli_parse_set_cert(char **args, char *payload, struct appctx *appctx,
                                free(ckchi);
                                ckchi = NULL;
                        }
-                       /* insert every new ckch instance in the actual list and insert the sni_ctx in the trees  */
-                       list_for_each_entry_safe(ckchi, ckchis, &tmp_ckchi_list, by_ckchs) {
-                               LIST_DEL(&ckchi->by_ckchs);
-                               LIST_ADD(&ckchs->ckch_inst, &ckchi->by_ckchs);
-                               HA_RWLOCK_WRLOCK(SNI_LOCK, &ckchi->bind_conf->sni_lock);
-                               ssl_sock_load_cert_sni(ckchi, ckchi->bind_conf);
-                               HA_RWLOCK_WRUNLOCK(SNI_LOCK, &ckchi->bind_conf->sni_lock);
-                       }
+                       /* Replace the old ckchs by the new one */
+                       ebmb_delete(&ckchs->node);
+                       ckchs_free(ckchs);
+                       ckchs = NULL;
+                       ebst_insert(&ckchs_tree, &new_ckchs->node);
                }
 #if HA_OPENSSL_VERSION_NUMBER >= 0x1000200fL
                {
@@ -10102,18 +10115,23 @@ end:
 
        if (errcode & ERR_CODE) {
                struct ckch_inst *ckchi, *ckchis;
-               /* if the allocation failed, we need to free everything from the temporary list */
-               list_for_each_entry_safe(ckchi, ckchis, &tmp_ckchi_list, by_ckchs) {
-                       struct sni_ctx *sc0, *sc0s;
 
-                       list_for_each_entry_safe(sc0, sc0s, &ckchi->sni_ctx, by_ckch_inst) {
-                               if (sc0->order == 0) /* we only free if it's the first inserted */
-                                       SSL_CTX_free(sc0->ctx);
-                               LIST_DEL(&sc0->by_ckch_inst);
-                               free(sc0);
+               if (new_ckchs) {
+                       /* if the allocation failed, we need to free everything from the temporary list */
+                       list_for_each_entry_safe(ckchi, ckchis, &new_ckchs->ckch_inst, by_ckchs) {
+                               struct sni_ctx *sc0, *sc0s;
+
+                               list_for_each_entry_safe(sc0, sc0s, &ckchi->sni_ctx, by_ckch_inst) {
+                                       if (sc0->order == 0) /* we only free if it's the first inserted */
+                                               SSL_CTX_free(sc0->ctx);
+                                       LIST_DEL(&sc0->by_ckch_inst);
+                                       free(sc0);
+                               }
+                               LIST_DEL(&ckchi->by_ckchs);
+                               free(ckchi);
                        }
-                       LIST_DEL(&ckchi->by_ckchs);
-                       free(ckchi);
+                       ckchs_free(new_ckchs);
+                       new_ckchs = NULL;
                }
                return cli_dynerr(appctx, memprintf(&err, "%sCan't update %s!\n", err ? err : "", args[3]));
        }