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)
This commit is contained in:
Remi Tricot-Le Breton 2021-01-25 17:19:44 +01:00 committed by William Lallemand
parent d817dc733e
commit f3eedfe195
4 changed files with 157 additions and 6 deletions

View File

@ -86,6 +86,7 @@ struct ckch_inst {
struct ckch_store *ckch_store; /* pointer to the store used to generate this 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 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 */ 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_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 */ unsigned int is_server_instance:1; /* This instance is used by a backend server */
/* space for more flag there */ /* space for more flag there */

View File

@ -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

View File

@ -907,6 +907,8 @@ void ckch_inst_free(struct ckch_inst *inst)
ebmb_delete(&sni->name); ebmb_delete(&sni->name);
free(sni); free(sni);
} }
SSL_CTX_free(inst->ctx);
inst->ctx = NULL;
LIST_DEL(&inst->by_ckchs); LIST_DEL(&inst->by_ckchs);
LIST_DEL(&inst->by_crtlist_entry); LIST_DEL(&inst->by_crtlist_entry);
free(inst); free(inst);
@ -1315,6 +1317,7 @@ static int cli_io_handler_commit_cert(struct appctx *appctx)
struct ckch_inst *new_inst; struct ckch_inst *new_inst;
char **sni_filter = NULL; char **sni_filter = NULL;
int fcount = 0; int fcount = 0;
SSL_CTX *ctx = NULL;
/* it takes a lot of CPU to creates SSL_CTXs, so we yield every 10 CKCH instances */ /* it takes a lot of CPU to creates SSL_CTXs, so we yield every 10 CKCH instances */
if (y >= 10) { if (y >= 10) {
@ -1329,8 +1332,8 @@ static int cli_io_handler_commit_cert(struct appctx *appctx)
} }
if (ckchi->is_server_instance) if (ckchi->is_server_instance)
goto error; /* Not managed yet */ 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); 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) if (errcode & ERR_CODE)
@ -1340,6 +1343,17 @@ static int cli_io_handler_commit_cert(struct appctx *appctx)
if (ckchi->is_default) if (ckchi->is_default)
new_inst->is_default = 1; 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 */ /* create the link to the crtlist_entry */
new_inst->crtlist_entry = ckchi->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 */ /* 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) { list_for_each_entry_safe(ckchi, ckchis, &new_ckchs->ckch_inst, by_ckchs) {
/* 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); HA_RWLOCK_WRLOCK(SNI_LOCK, &ckchi->bind_conf->sni_lock);
ssl_sock_load_cert_sni(ckchi, ckchi->bind_conf); ssl_sock_load_cert_sni(ckchi, ckchi->bind_conf);
HA_RWLOCK_WRUNLOCK(SNI_LOCK, &ckchi->bind_conf->sni_lock); HA_RWLOCK_WRUNLOCK(SNI_LOCK, &ckchi->bind_conf->sni_lock);
} }
}
/* delete the old sni_ctx, the old ckch_insts and the ckch_store */ /* 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) { list_for_each_entry_safe(ckchi, ckchis, &old_ckchs->ckch_inst, by_ckchs) {
struct bind_conf __maybe_unused *bind_conf = ckchi->bind_conf; 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); HA_RWLOCK_WRLOCK(SNI_LOCK, &bind_conf->sni_lock);
ckch_inst_free(ckchi); ckch_inst_free(ckchi);

View File

@ -3764,6 +3764,11 @@ int ssl_sock_load_srv_cert(char *path, struct server *server, char **err)
if (server->ssl_ctx.inst) { if (server->ssl_ctx.inst) {
server->ssl_ctx.inst->is_server_instance = 1; server->ssl_ctx.inst->is_server_instance = 1;
server->ssl_ctx.inst->server = server; 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;
} }
} }
} }