BUG/MEDIUM: ssl: Crash because of dangling ckch_store reference in a ckch instance

When updating CAs via the CLI, we need to create new copies of all the
impacted ckch instances (as in referenced in the ckch_inst_link list of
the updated CA) in order to use them instead of the old ones once the
updated is completed. This relies on the ckch_inst_rebuild function that
would set the ckch_store field of the ckch_inst. But we forgot to also
add the newly created instances in the ckch_inst list of the
corresponding ckch_store.

When updating a certificate afterwards, we iterate over all the
instances linked in the ckch_inst list of the ckch_store (which is
missing some instances because of the previous command) and rebuild the
instances before replacing the ckch_store. The previous ckch_store,
still referenced by the dangling ckch instance then gets deleted which
means that the instance keeps a reference to a free'd object.

Then if we were to once again update the CA file, we would iterate over
the ckch instances referenced in the cafile_entry's ckch_inst_link list,
which includes the first mentioned ckch instance with the dead
ckch_store reference. This ends up crashing during the ckch_inst_rebuild
operation.

This bug was raised in GitHub #3165.
This patch should be backported to all stable branches.

diff --git a/src/ssl_ckch.c b/src/ssl_ckch.c
index 43a76ca74d6359aec48de2f69ae21489fe88c49a..bc0f3d43d7c675ce98841c0477fe0bbd50663cf1 100644 (file)
--- a/src/ssl_ckch.c
+++ b/src/ssl_ckch.c
@@ -3584,6 +3584,7 @@ static int cli_io_handler_commit_cafile_crlfile(struct appctx *appctx)
 
                                list_for_each_entry_from(ckchi_link, &old_cafile_entry->ckch_inst_link, list) {
                                        struct ckch_inst *new_inst;
+                                       struct ckch_store *ckch_store = ckchi_link->ckch_inst->ckch_store;
 
                                        /* save the next ckchi to compute */
                                        ctx->next_ckchi_link = ckchi_link;
@@ -3601,11 +3602,14 @@ static int cli_io_handler_commit_cafile_crlfile(struct appctx *appctx)
                                        /* Rebuild a new ckch instance that uses the same ckch_store
                                         * than a reference ckchi instance but will use a new CA file. */
                                        ctx->err = NULL;
-                                       if (ckch_inst_rebuild(ckchi_link->ckch_inst->ckch_store, ckchi_link->ckch_inst, &new_inst, &ctx->err)) {
+                                       if (ckch_inst_rebuild(ckch_store, ckchi_link->ckch_inst, &new_inst, &ctx->err)) {
                                                ctx->state = CACRL_ST_ERROR;
                                                goto error;
                                        }
 
+                                       /* link the new ckch_inst to the duplicate */
+                                       LIST_APPEND(&ckch_store->ckch_inst, &new_inst->by_ckchs);
+
                                        y++;
                                }