MEDIUM: ssl: Enable backend certificate hot update

When trying to update a backend certificate, we should find a
server-side ckch instance thanks to which we can rebuild a new ssl
context and a new ckch instance that replace the previous ones in the
server structure. This way any new ssl session will be built out of the
new ssl context and the newly updated certificate.

This resolves a subpart of GitHub issue #427 (the certificate part)

diff --git a/include/haproxy/ssl_ckch-t.h b/include/haproxy/ssl_ckch-t.h
index 37ef446816c59d432f571f333b9c09330bf61fe0..2604a523a27a179bdc5987707347ebc652089b5a 100644 (file)
--- a/include/haproxy/ssl_ckch-t.h
+++ b/include/haproxy/ssl_ckch-t.h
@@ -86,6 +86,7 @@ struct ckch_inst {
        struct ckch_store *ckch_store; /* pointer to the store used to generate this 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 if it is a server one (is_server_instance set) */
        unsigned int is_default:1;      /* This instance is used as the default ctx for this bind_conf */
        unsigned int is_server_instance:1; /* This instance is used by a backend server */
        /* space for more flag there */

diff --git a/reg-tests/ssl/set_ssl_server_cert.vtc b/reg-tests/ssl/set_ssl_server_cert.vtc
new file mode 100644 (file)
index 0000000..e0ee487
--- /dev/null
+++ b/reg-tests/ssl/set_ssl_server_cert.vtc
@@ -0,0 +1,104 @@
+#REGTEST_TYPE=devel
+
+# This reg-test uses the "set ssl cert" command to update a backend certificate over the CLI.
+# It requires socat to upload the certificate
+
+varnishtest "Test the 'set ssl cert' feature of the CLI"
+#REQUIRE_VERSION=2.4
+#REQUIRE_OPTIONS=OPENSSL
+#REQUIRE_BINARIES=socat
+feature ignore_unknown_macro
+
+server s1 -repeat 4 {
+  rxreq
+  txresp
+} -start
+
+haproxy h1 -conf {
+    global
+        tune.ssl.default-dh-param 2048
+        tune.ssl.capture-cipherlist-size 1
+        stats socket "${tmpdir}/h1/stats" level admin
+
+    defaults
+        mode http
+        option httplog
+        ${no-htx} option http-use-htx
+        log stderr local0 debug err
+        option logasap
+        timeout connect 100ms
+        timeout client  1s
+        timeout server  1s
+
+    listen clear-lst
+        bind "fd@${clearlst}"
+        retries 0 # 2nd SSL connection must fail so skip the retry
+        server s1 "${tmpdir}/ssl.sock" ssl verify none crt ${testdir}/client1.pem
+
+    listen ssl-lst
+        # crt: certificate of the server
+        # ca-file: CA used for client authentication request
+        # crl-file: revocation list for client auth: the client1 certificate is revoked
+        bind "${tmpdir}/ssl.sock" ssl crt ${testdir}/common.pem ca-file ${testdir}/ca-auth.crt verify optional crt-ignore-err all crl-file ${testdir}/crl-auth.pem
+
+        acl cert_expired ssl_c_verify 10
+        acl cert_revoked ssl_c_verify 23
+        acl cert_ok ssl_c_verify 0
+
+        http-response add-header X-SSL Ok if cert_ok
+        http-response add-header X-SSL Expired if cert_expired
+        http-response add-header X-SSL Revoked if cert_revoked
+
+        server s1 ${s1_addr}:${s1_port}
+} -start
+
+client c1 -connect ${h1_clearlst_sock} {
+  txreq
+  rxresp
+  expect resp.status == 200
+  expect resp.http.x-ssl == "Ok"
+} -run
+
+# Replace certificate with an expired one
+shell {
+    printf "set ssl cert ${testdir}/client1.pem <<\n$(cat ${testdir}/client2_expired.pem)\n\n" | socat "${tmpdir}/h1/stats" -
+    echo "commit ssl cert ${testdir}/client1.pem" | socat "${tmpdir}/h1/stats" -
+}
+
+# The updated client certificate is an expired one so this request should fail
+client c1 -connect ${h1_clearlst_sock} {
+    txreq
+    rxresp
+    expect resp.status == 200
+    expect resp.http.x-ssl == "Expired"
+} -run
+
+# Replace certificate with a revoked one
+shell {
+    printf "set ssl cert ${testdir}/client1.pem <<\n$(cat ${testdir}/client3_revoked.pem)\n\n" | socat "${tmpdir}/h1/stats" -
+    echo "commit ssl cert ${testdir}/client1.pem" | socat "${tmpdir}/h1/stats" -
+}
+
+# The updated client certificate is a revoked one so this request should fail
+client c1 -connect ${h1_clearlst_sock} {
+    txreq
+    rxresp
+    expect resp.status == 200
+    expect resp.http.x-ssl == "Revoked"
+} -run
+
+# Abort a transaction
+shell {
+    printf "set ssl cert ${testdir}/client1.pem <<\n$(cat ${testdir}/client3_revoked.pem)\n\n" | socat "${tmpdir}/h1/stats" -
+    echo "abort ssl cert ${testdir}/client1.pem" | socat "${tmpdir}/h1/stats" -
+}
+
+# The certificate was not updated so it should still be revoked
+client c1 -connect ${h1_clearlst_sock} {
+    txreq
+    rxresp
+    expect resp.status == 200
+    expect resp.http.x-ssl == "Revoked"
+} -run
+
+

diff --git a/src/ssl_ckch.c b/src/ssl_ckch.c
index 1a09e8db89df7a3a567b178c99a314926348b019..8d939bcc9b8715e065cd21a9f9e08e1dc920cebb 100644 (file)
--- a/src/ssl_ckch.c
+++ b/src/ssl_ckch.c
@@ -907,6 +907,8 @@ void ckch_inst_free(struct ckch_inst *inst)
                ebmb_delete(&sni->name);
                free(sni);
        }
+       SSL_CTX_free(inst->ctx);
+       inst->ctx = NULL;
        LIST_DEL(&inst->by_ckchs);
        LIST_DEL(&inst->by_crtlist_entry);
        free(inst);
@@ -1315,6 +1317,7 @@ static int cli_io_handler_commit_cert(struct appctx *appctx)
                                        struct ckch_inst *new_inst;
                                        char **sni_filter = NULL;
                                        int fcount = 0;
+                                       SSL_CTX *ctx = NULL;
 
                                        /* it takes a lot of CPU to creates SSL_CTXs, so we yield every 10 CKCH instances */
                                        if (y >= 10) {
@@ -1329,9 +1332,9 @@ static int cli_io_handler_commit_cert(struct appctx *appctx)
                                        }
 
                                        if (ckchi->is_server_instance)
-                                               goto error; /* Not managed yet */
-
-                                       errcode |= ckch_inst_new_load_store(new_ckchs->path, new_ckchs, ckchi->bind_conf, ckchi->ssl_conf, sni_filter, fcount, &new_inst, &err);
+                                               errcode |= ckch_inst_new_load_srv_store(new_ckchs->path, new_ckchs, &new_inst, &ctx, &err);
+                                       else
+                                               errcode |= ckch_inst_new_load_store(new_ckchs->path, new_ckchs, ckchi->bind_conf, ckchi->ssl_conf, sni_filter, fcount, &new_inst, &err);
 
                                        if (errcode & ERR_CODE)
                                                goto error;
@@ -1340,6 +1343,17 @@ static int cli_io_handler_commit_cert(struct appctx *appctx)
                                        if (ckchi->is_default)
                                                new_inst->is_default = 1;
 
+                                       new_inst->is_server_instance = ckchi->is_server_instance;
+                                       new_inst->server = ckchi->server;
+                                       /* Create a new SSL_CTX and link it to the new instance. */
+                                       if (new_inst->is_server_instance) {
+                                               errcode |= ssl_sock_prepare_srv_ssl_ctx(ckchi->server, ctx);
+                                               if (errcode & ERR_CODE)
+                                                       goto error;
+
+                                               new_inst->ctx = ctx;
+                                       }
+
                                        /* create the link to the crtlist_entry */
                                        new_inst->crtlist_entry = ckchi->crtlist_entry;
 
@@ -1388,14 +1402,41 @@ static int cli_io_handler_commit_cert(struct appctx *appctx)
 
                                /* First, we insert every new SNIs in the trees, also replace the default_ctx */
                                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);
+                                       /* The bind_conf will be null on server ckch_instances. */
+                                       if (ckchi->is_server_instance) {
+                                               struct ckch_inst *old_inst = ckchi->server->ssl_ctx.inst;
+                                               SSL_CTX *old_ctx = ckchi->server->ssl_ctx.ctx;
+
+                                               /* The certificate update on the server side (backend)
+                                                * can be done by rewritting a single pointer so no
+                                                * locks are needed here. */
+                                               SSL_CTX_up_ref(ckchi->ctx);
+                                               /* Actual ssl context update */
+                                               ckchi->server->ssl_ctx.ctx = ckchi->ctx;
+                                               ckchi->server->ssl_ctx.inst = ckchi;
+
+                                               __ha_barrier_store();
+
+                                               /* Clear any previous ssl context. */
+                                               if (old_ctx)
+                                                       SSL_CTX_free(old_ctx);
+                                               if (old_inst)
+                                                       ckch_inst_free(old_inst);
+
+                                       }
+                                       else {
+                                               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, &old_ckchs->ckch_inst, by_ckchs) {
                                        struct bind_conf __maybe_unused *bind_conf = ckchi->bind_conf;
+                                       /* The bind_conf will be null on server ckch_instances. */
+                                       if (ckchi->is_server_instance)
+                                               continue;
 
                                        HA_RWLOCK_WRLOCK(SNI_LOCK, &bind_conf->sni_lock);
                                        ckch_inst_free(ckchi);

diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 89a698b3bf7b6bd43a65269f44f6834348811ccd..c099bc6e3fe7fa0cb4a753e144cb9fefa81942ff 100644 (file)
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -3764,6 +3764,11 @@ int ssl_sock_load_srv_cert(char *path, struct server *server, char **err)
                        if (server->ssl_ctx.inst) {
                                server->ssl_ctx.inst->is_server_instance = 1;
                                server->ssl_ctx.inst->server = server;
+                               /* Keep a reference to the SSL_CTX in the
+                                * ckch_inst in order to ease certificate update
+                                * (via CLI). */
+                               SSL_CTX_up_ref(server->ssl_ctx.ctx);
+                               server->ssl_ctx.inst->ctx = server->ssl_ctx.ctx;
                        }
                }
        }