BUG/MINOR: ssl/ckch: free the new instance when ckch_inst_rebuild() fails

ckch_inst_rebuild() creates the new ckch_inst of a "commit ssl cert" with
its sni_ctx and SSL_CTX, then prepares the SSL_CTX of the server or of
the bind line. When this preparation fails it returns an error to
ckch_store_rebuild_instances() which only returns -1, so the new instance,
which is not linked anywhere yet, is leaked with its SSL_CTXs, and with
everything they reference (the certificate, the chain, the key, the CA
file links).

The impact is a memory leak for each failed commit, which requires an
admin to commit a certificate that doesn't fit its bind or server lines.

Free the instance with ckch_inst_free() and reset the pointer on these
two error paths. The instance is not in any tree nor list at this point,
which ckch_inst_free() handles. The first error path, after
ckch_inst_new_load_store() or ckch_inst_new_load_srv_store(), is already
fine since these functions never leave an instance behind on failure.

This should be backported to all stable branches, the function exists
since 2.5.

diff --git a/src/ssl_ckch.c b/src/ssl_ckch.c
index 9353e772ab1e8a9596de392ea6e044421446deee..adfb79027e4a841d61b11e9daf0fec220411e109 100644 (file)
--- a/src/ssl_ckch.c
+++ b/src/ssl_ckch.c
@@ -2945,7 +2945,7 @@ int ckch_inst_rebuild(struct ckch_store *ckch_store, struct ckch_inst *ckchi,
        if ((*new_inst)->is_server_instance) {
                retval = ssl_sock_prep_srv_ctx_and_inst(ckchi->server, (*new_inst)->ctx, (*new_inst));
                if (retval)
-                       return 1;
+                       goto error;
        }
 
        /* create the link to the crtlist_entry */
@@ -2957,11 +2957,19 @@ int ckch_inst_rebuild(struct ckch_store *ckch_store, struct ckch_inst *ckchi,
                if (!sc0->order) { /* we initialized only the first SSL_CTX because it's the same in the other sni_ctx's */
                        errcode |= ssl_sock_prep_ctx_and_inst(ckchi->bind_conf, ckchi->ssl_conf, sc0->ctx, *new_inst, err);
                        if (errcode & ERR_CODE)
-                               return 1;
+                               goto error;
                }
        }
 
        return 0;
+
+ error:
+       /* the instance is not linked anywhere yet, and its contexts would
+        * keep what they reference, e.g. the certificate and its key, for ever
+        */
+       ckch_inst_free(*new_inst);
+       *new_inst = NULL;
+       return 1;
 }
 
 /*