From 4458b9732d97d30aeb83fe02b6b767b1fc689a38 Mon Sep 17 00:00:00 2001 From: Remi Tricot-Le Breton Date: Fri, 19 Feb 2021 17:41:55 +0100 Subject: [PATCH] MEDIUM: ssl: Chain ckch instances in ca-file entries Each ca-file entry of the tree will now hold a list of the ckch instances that use it so that we can iterate over them when updating the ca-file via a cli command. Since the link between the SSL contexts and the CA file tree entries is only built during the ssl_sock_prepare_ctx function, which are called after all the ckch instances are created, we need to add a little post processing after each ssl_sock_prepare_ctx that builds the link between the corresponding ckch instance and CA file tree entries. In order to manage the ca-file and ca-verify-file options, any ckch instance can be linked to multiple CA file tree entries and any CA file entry can link multiple ckch instances. This is done thanks to a dedicated list of ckch_inst references stored in the CA file tree entries over which we can iterate (during an update for instance). We avoid having one of those instances go stale by keeping a list of references to those references in the instances. When deleting a ckch_inst, we can then remove all the ckch_inst_link instances that reference it, and when deleting a cafile_entry, we iterate over the list of ckch_inst reference and clear the corresponding entry in their own list of ckch_inst_link references. --- include/haproxy/ssl_ckch-t.h | 19 +++++ include/haproxy/ssl_ckch.h | 3 + include/haproxy/ssl_sock.h | 7 +- src/ssl_ckch.c | 130 ++++++++++++++++++++++++++++++++++- src/ssl_crtlist.c | 8 ++- src/ssl_sock.c | 70 ++++++++++++++++--- 6 files changed, 219 insertions(+), 18 deletions(-) diff --git a/include/haproxy/ssl_ckch-t.h b/include/haproxy/ssl_ckch-t.h index 2ea1ba2bf9..584013c1cf 100644 --- a/include/haproxy/ssl_ckch-t.h +++ b/include/haproxy/ssl_ckch-t.h @@ -75,6 +75,23 @@ struct ckch_store { struct ssl_bind_conf; struct crtlist_entry; + +/* Used to keep a list of all the instances using a specific cafile_entry. + * It enables to link instances regardless of how they are using the CA file + * (either via the ca-file, ca-verify-file or crl-file option). */ +struct ckch_inst_link { + struct ckch_inst *ckch_inst; + struct list list; +}; + +/* Used to keep in a ckch instance a list of all the ckch_inst_link which + * reference it. This way, when deleting a ckch_inst, we can ensure that no + * dangling reference on it will remain. */ +struct ckch_inst_link_ref { + struct ckch_inst_link *link; + struct list list; +}; + /* * This structure describe a ckch instance. An instance is generated for each * bind_conf. The instance contains a linked list of the sni ctx which uses @@ -93,6 +110,7 @@ struct ckch_inst { struct list sni_ctx; /* list of sni_ctx using this ckch_inst */ struct list by_ckchs; /* chained in ckch_store's list of ckch_inst */ struct list by_crtlist_entry; /* chained in crtlist_entry list of inst */ + struct list cafile_link_refs; /* list of ckch_inst_link pointing to this instance */ }; @@ -102,6 +120,7 @@ struct ckch_inst { struct cafile_entry { X509_STORE *ca_store; STACK_OF(X509_NAME) *ca_list; + struct list ckch_inst_link; /* list of ckch_inst which use this CA file entry */ struct ebmb_node node; char path[0]; }; diff --git a/include/haproxy/ssl_ckch.h b/include/haproxy/ssl_ckch.h index 31cf3b5cd7..6b3830d7ad 100644 --- a/include/haproxy/ssl_ckch.h +++ b/include/haproxy/ssl_ckch.h @@ -53,8 +53,11 @@ int ckch_inst_new_load_srv_store(const char *path, struct ckch_store *ckchs, struct ckch_inst **ckchi, char **err); void ckch_deinit(); +void ckch_inst_add_cafile_link(struct ckch_inst *ckch_inst, struct bind_conf *bind_conf, + struct ssl_bind_conf *ssl_conf, const struct server *srv); /* ssl_store functions */ +struct cafile_entry *ssl_store_get_cafile_entry(char *path, int oldest_entry); X509_STORE* ssl_store_get0_locations_file(char *path); int ssl_store_load_locations_file(char *path, int create_if_none); diff --git a/include/haproxy/ssl_sock.h b/include/haproxy/ssl_sock.h index a96a67b546..bef137a732 100644 --- a/include/haproxy/ssl_sock.h +++ b/include/haproxy/ssl_sock.h @@ -52,13 +52,14 @@ extern int ssl_keylog_index; extern struct pool_head *pool_head_ssl_keylog; extern struct pool_head *pool_head_ssl_keylog_str; -int ssl_sock_prepare_ctx(struct bind_conf *bind_conf, struct ssl_bind_conf *, SSL_CTX *ctx, char **err); -int ssl_sock_prepare_srv_ssl_ctx(const struct server *srv, SSL_CTX *ctx); +int ssl_sock_prep_ctx_and_inst(struct bind_conf *bind_conf, struct ssl_bind_conf *ssl_conf, + SSL_CTX *ctx, struct ckch_inst *ckch_inst, char **err); +int ssl_sock_prep_srv_ctx_and_inst(const struct server *srv, SSL_CTX *ctx, + struct ckch_inst *ckch_inst); int ssl_sock_prepare_all_ctx(struct bind_conf *bind_conf); int ssl_sock_prepare_bind_conf(struct bind_conf *bind_conf); void ssl_sock_destroy_bind_conf(struct bind_conf *bind_conf); int ssl_sock_prepare_srv_ctx(struct server *srv); -int ssl_sock_prepare_srv_ssl_ctx(const struct server *srv, SSL_CTX *ctx); void ssl_sock_free_srv_ctx(struct server *srv); void ssl_sock_free_all_ctx(struct bind_conf *bind_conf); int ssl_sock_load_ca(struct bind_conf *bind_conf); diff --git a/src/ssl_ckch.c b/src/ssl_ckch.c index c8f0f2fed0..f2344ec22e 100644 --- a/src/ssl_ckch.c +++ b/src/ssl_ckch.c @@ -888,6 +888,7 @@ end: void ckch_inst_free(struct ckch_inst *inst) { struct sni_ctx *sni, *sni_s; + struct ckch_inst_link_ref *link_ref, *link_ref_s; if (inst == NULL) return; @@ -902,6 +903,13 @@ void ckch_inst_free(struct ckch_inst *inst) inst->ctx = NULL; LIST_DELETE(&inst->by_ckchs); LIST_DELETE(&inst->by_crtlist_entry); + + list_for_each_entry_safe(link_ref, link_ref_s, &inst->cafile_link_refs, list) { + LIST_DELETE(&link_ref->link->list); + LIST_DELETE(&link_ref->list); + free(link_ref); + } + free(inst); } @@ -917,6 +925,7 @@ struct ckch_inst *ckch_inst_new() LIST_INIT(&ckch_inst->sni_ctx); LIST_INIT(&ckch_inst->by_ckchs); LIST_INIT(&ckch_inst->by_crtlist_entry); + LIST_INIT(&ckch_inst->cafile_link_refs); return ckch_inst; } @@ -932,7 +941,7 @@ struct eb_root cafile_tree = EB_ROOT; * given path, the original one and the new one set via the CLI but not * committed yet). */ -static struct cafile_entry *ssl_store_get_cafile_entry(char *path, int oldest_entry) +struct cafile_entry *ssl_store_get_cafile_entry(char *path, int oldest_entry) { struct cafile_entry *ca_e = NULL; struct ebmb_node *eb; @@ -979,6 +988,7 @@ int ssl_store_load_locations_file(char *path, int create_if_none) if (ca_e) { memcpy(ca_e->path, path, pathlen + 1); ca_e->ca_store = store; + LIST_INIT(&ca_e->ckch_inst_link); ebst_insert(&cafile_tree, &ca_e->node); } } else { @@ -1108,6 +1118,120 @@ static int ssl_sock_get_san_oneline(X509 *cert, struct buffer *out) } #endif +/* + * Build the ckch_inst_link that will be chained in the CA file entry and the + * corresponding ckch_inst_link_ref that will be chained in the ckch instance. + * Return 0 in case of success. + */ +static int do_chain_inst_and_cafile(struct cafile_entry *cafile_entry, struct ckch_inst *ckch_inst) +{ + struct ckch_inst_link *new_link; + if (!LIST_ISEMPTY(&cafile_entry->ckch_inst_link)) { + struct ckch_inst_link *link = LIST_ELEM(cafile_entry->ckch_inst_link.n, + typeof(link), list); + /* Do not add multiple references to the same + * instance in a cafile_entry */ + if (link->ckch_inst == ckch_inst) { + return 1; + } + } + + new_link = calloc(1, sizeof(*new_link)); + if (new_link) { + struct ckch_inst_link_ref *new_link_ref = calloc(1, sizeof(*new_link_ref)); + if (!new_link_ref) { + free(new_link); + return 1; + } + + new_link->ckch_inst = ckch_inst; + new_link_ref->link = new_link; + LIST_INIT(&new_link->list); + LIST_INIT(&new_link_ref->list); + + LIST_APPEND(&cafile_entry->ckch_inst_link, &new_link->list); + LIST_APPEND(&ckch_inst->cafile_link_refs, &new_link_ref->list); + } + + return 0; +} + + +/* + * Link a CA file tree entry to the ckch instance that uses it. + * To determine if and which CA file tree entries need to be linked to the + * instance, we follow the same logic performed in ssl_sock_prepare_ctx when + * processing the verify option. + * This function works for a frontend as well as for a backend, depending on the + * configuration parameters given (bind_conf or server). + */ +void ckch_inst_add_cafile_link(struct ckch_inst *ckch_inst, struct bind_conf *bind_conf, + struct ssl_bind_conf *ssl_conf, const struct server *srv) +{ + int verify = SSL_VERIFY_NONE; + + if (srv) { + + if (global.ssl_server_verify == SSL_SERVER_VERIFY_REQUIRED) + verify = SSL_VERIFY_PEER; + switch (srv->ssl_ctx.verify) { + case SSL_SOCK_VERIFY_NONE: + verify = SSL_VERIFY_NONE; + break; + case SSL_SOCK_VERIFY_REQUIRED: + verify = SSL_VERIFY_PEER; + break; + } + } + else { + switch ((ssl_conf && ssl_conf->verify) ? ssl_conf->verify : bind_conf->ssl_conf.verify) { + case SSL_SOCK_VERIFY_NONE: + verify = SSL_VERIFY_NONE; + break; + case SSL_SOCK_VERIFY_OPTIONAL: + verify = SSL_VERIFY_PEER; + break; + case SSL_SOCK_VERIFY_REQUIRED: + verify = SSL_VERIFY_PEER|SSL_VERIFY_FAIL_IF_NO_PEER_CERT; + break; + } + } + + if (verify & SSL_VERIFY_PEER) { + struct cafile_entry *ca_file_entry = NULL; + struct cafile_entry *ca_verify_file_entry = NULL; + if (srv) { + if (srv->ssl_ctx.ca_file) { + ca_file_entry = ssl_store_get_cafile_entry(srv->ssl_ctx.ca_file, 0); + + } + } + else { + char *ca_file = (ssl_conf && ssl_conf->ca_file) ? ssl_conf->ca_file : bind_conf->ssl_conf.ca_file; + char *ca_verify_file = (ssl_conf && ssl_conf->ca_verify_file) ? ssl_conf->ca_verify_file : bind_conf->ssl_conf.ca_verify_file; + + if (ca_file) + ca_file_entry = ssl_store_get_cafile_entry(ca_file, 0); + if (ca_verify_file) + ca_verify_file_entry = ssl_store_get_cafile_entry(ca_verify_file, 0); + } + + if (ca_file_entry) { + /* If we have a ckch instance that is not already in the + * cafile_entry's list, add it to it. */ + if (do_chain_inst_and_cafile(ca_file_entry, ckch_inst)) + return; + + } + if (ca_verify_file_entry && (ca_file_entry != ca_verify_file_entry)) { + /* If we have a ckch instance that is not already in the + * cafile_entry's list, add it to it. */ + if (do_chain_inst_and_cafile(ca_verify_file_entry, ckch_inst)) + return; + } + } +} + @@ -1407,7 +1531,7 @@ static int cli_io_handler_commit_cert(struct appctx *appctx) new_inst->server = ckchi->server; /* Create a new SSL_CTX and link it to the new instance. */ if (new_inst->is_server_instance) { - retval = ssl_sock_prepare_srv_ssl_ctx(ckchi->server, new_inst->ctx); + retval = ssl_sock_prep_srv_ctx_and_inst(ckchi->server, new_inst->ctx, new_inst); if (retval) goto error; } @@ -1419,7 +1543,7 @@ static int cli_io_handler_commit_cert(struct appctx *appctx) /* this iterate on the newly generated SNIs in the new instance to prepare their SSL_CTX */ list_for_each_entry_safe(sc0, sc0s, &new_inst->sni_ctx, by_ckch_inst) { if (!sc0->order) { /* we initialized only the first SSL_CTX because it's the same in the other sni_ctx's */ - errcode |= ssl_sock_prepare_ctx(ckchi->bind_conf, ckchi->ssl_conf, sc0->ctx, &err); + errcode |= ssl_sock_prep_ctx_and_inst(ckchi->bind_conf, ckchi->ssl_conf, sc0->ctx, sc0->ckch_inst, &err); if (errcode & ERR_CODE) goto error; } diff --git a/src/ssl_crtlist.c b/src/ssl_crtlist.c index f4f26ed8fe..93168e3c7f 100644 --- a/src/ssl_crtlist.c +++ b/src/ssl_crtlist.c @@ -1094,7 +1094,7 @@ static int cli_io_handler_add_crtlist(struct appctx *appctx) /* this iterate on the newly generated SNIs in the new instance to prepare their SSL_CTX */ list_for_each_entry(sni, &new_inst->sni_ctx, by_ckch_inst) { if (!sni->order) { /* we initialized only the first SSL_CTX because it's the same in the other sni_ctx's */ - errcode |= ssl_sock_prepare_ctx(bind_conf, new_inst->ssl_conf, sni->ctx, &err); + errcode |= ssl_sock_prep_ctx_and_inst(bind_conf, new_inst->ssl_conf, sni->ctx, sni->ckch_inst, &err); if (errcode & ERR_CODE) goto error; } @@ -1407,6 +1407,7 @@ static int cli_parse_del_crtlist(char **args, char *payload, struct appctx *appc list_for_each_entry_safe(inst, inst_s, &entry->ckch_inst, by_crtlist_entry) { struct sni_ctx *sni, *sni_s; + struct ckch_inst_link_ref *link_ref, *link_ref_s; HA_RWLOCK_WRLOCK(SNI_LOCK, &inst->bind_conf->sni_lock); list_for_each_entry_safe(sni, sni_s, &inst->sni_ctx, by_ckch_inst) { @@ -1417,6 +1418,11 @@ static int cli_parse_del_crtlist(char **args, char *payload, struct appctx *appc } HA_RWLOCK_WRUNLOCK(SNI_LOCK, &inst->bind_conf->sni_lock); LIST_DELETE(&inst->by_ckchs); + list_for_each_entry_safe(link_ref, link_ref_s, &inst->cafile_link_refs, list) { + LIST_DELETE(&link_ref->link->list); + LIST_DELETE(&link_ref->list); + free(link_ref); + } free(inst); } diff --git a/src/ssl_sock.c b/src/ssl_sock.c index 0fc3388df3..8f9f535401 100644 --- a/src/ssl_sock.c +++ b/src/ssl_sock.c @@ -320,7 +320,9 @@ __decl_thread(HA_SPINLOCK_T ckch_lock); static int ssl_set_cert_crl_file(X509_STORE *store_ctx, char *path) { X509_STORE *store; - store = ssl_store_get0_locations_file(path); + struct cafile_entry *ca_e = ssl_store_get_cafile_entry(path, 0); + if (ca_e) + store = ca_e->ca_store; if (store_ctx && store) { int i; X509_OBJECT *obj; @@ -4218,7 +4220,7 @@ error: * This function applies the SSL configuration on a SSL_CTX * It returns an error code and fills the buffer */ -int ssl_sock_prepare_ctx(struct bind_conf *bind_conf, struct ssl_bind_conf *ssl_conf, SSL_CTX *ctx, char **err) +static int ssl_sock_prepare_ctx(struct bind_conf *bind_conf, struct ssl_bind_conf *ssl_conf, SSL_CTX *ctx, char **err) { struct proxy *curproxy = bind_conf->frontend; int cfgerr = 0; @@ -4444,6 +4446,28 @@ int ssl_sock_prepare_ctx(struct bind_conf *bind_conf, struct ssl_bind_conf *ssl_ return cfgerr; } + +/* + * Prepare the SSL_CTX based on the bind line configuration. + * Since the CA file loading is made depending on the verify option of the bind + * line, the link between the SSL_CTX and the CA file tree entry is made here. + * If we want to create a link between the CA file entry and the corresponding + * ckch instance (for CA file hot update), it needs to be done after + * ssl_sock_prepare_ctx. + * Returns 0 in case of success. + */ +int ssl_sock_prep_ctx_and_inst(struct bind_conf *bind_conf, struct ssl_bind_conf *ssl_conf, + SSL_CTX *ctx, struct ckch_inst *ckch_inst, char **err) +{ + int errcode = 0; + + errcode |= ssl_sock_prepare_ctx(bind_conf, ssl_conf, ctx, err); + if (!errcode && ckch_inst) + ckch_inst_add_cafile_link(ckch_inst, bind_conf, ssl_conf, NULL); + + return errcode; +} + static int ssl_sock_srv_hostcheck(const char *pattern, const char *hostname) { const char *pattern_wildcard, *pattern_left_label_end, *hostname_left_label_end; @@ -4631,16 +4655,15 @@ int ssl_sock_prepare_srv_ctx(struct server *srv) srv->ssl_ctx.ctx = ctx; } - cfgerr += ssl_sock_prepare_srv_ssl_ctx(srv, srv->ssl_ctx.ctx); + cfgerr += ssl_sock_prep_srv_ctx_and_inst(srv, srv->ssl_ctx.ctx, srv->ssl_ctx.inst); return cfgerr; } - /* Initialize an SSL context that will be used on the backend side. * Returns an error count. */ -int ssl_sock_prepare_srv_ssl_ctx(const struct server *srv, SSL_CTX *ctx) +static int ssl_sock_prepare_srv_ssl_ctx(const struct server *srv, SSL_CTX *ctx) { struct proxy *curproxy = srv->proxy; int cfgerr = 0; @@ -4808,6 +4831,29 @@ int ssl_sock_prepare_srv_ssl_ctx(const struct server *srv, SSL_CTX *ctx) return cfgerr; } +/* + * Prepare the frontend's SSL_CTX based on the server line configuration. + * Since the CA file loading is made depending on the verify option of the + * server line, the link between the SSL_CTX and the CA file tree entry is + * made here. + * If we want to create a link between the CA file entry and the corresponding + * ckch instance (for CA file hot update), it needs to be done after + * ssl_sock_prepare_srv_ssl_ctx. + * Returns an error count. + */ +int ssl_sock_prep_srv_ctx_and_inst(const struct server *srv, SSL_CTX *ctx, + struct ckch_inst *ckch_inst) +{ + int cfgerr = 0; + + cfgerr += ssl_sock_prepare_srv_ssl_ctx(srv, ctx); + if (!cfgerr && ckch_inst) + ckch_inst_add_cafile_link(ckch_inst, NULL, NULL, srv); + + return cfgerr; +} + + /* * Create an initial CTX used to start the SSL connections. * May be used by QUIC xprt which makes usage of SSL sessions initialized from SSL_CTXs. @@ -4854,18 +4900,20 @@ int ssl_sock_prepare_all_ctx(struct bind_conf *bind_conf) /* It should not be necessary to call this function, but it's necessary first to check and move all initialisation related to initial_ctx in ssl_initial_ctx. */ - errcode |= ssl_sock_prepare_ctx(bind_conf, NULL, bind_conf->initial_ctx, &errmsg); + errcode |= ssl_sock_prep_ctx_and_inst(bind_conf, NULL, bind_conf->initial_ctx, NULL, &errmsg); + } + if (bind_conf->default_ctx) { + errcode |= ssl_sock_prep_ctx_and_inst(bind_conf, bind_conf->default_ssl_conf, bind_conf->default_ctx, NULL, &errmsg); } - if (bind_conf->default_ctx) - errcode |= ssl_sock_prepare_ctx(bind_conf, bind_conf->default_ssl_conf, bind_conf->default_ctx, &errmsg); node = ebmb_first(&bind_conf->sni_ctx); while (node) { sni = ebmb_entry(node, struct sni_ctx, name); - if (!sni->order && sni->ctx != bind_conf->default_ctx) + if (!sni->order && sni->ctx != bind_conf->default_ctx) { /* only initialize the CTX on its first occurrence and if it is not the default_ctx */ - errcode |= ssl_sock_prepare_ctx(bind_conf, sni->conf, sni->ctx, &errmsg); + errcode |= ssl_sock_prep_ctx_and_inst(bind_conf, sni->conf, sni->ctx, sni->ckch_inst, &errmsg); + } node = ebmb_next(node); } @@ -4875,7 +4923,7 @@ int ssl_sock_prepare_all_ctx(struct bind_conf *bind_conf) if (!sni->order && sni->ctx != bind_conf->default_ctx) { /* only initialize the CTX on its first occurrence and if it is not the default_ctx */ - errcode |= ssl_sock_prepare_ctx(bind_conf, sni->conf, sni->ctx, &errmsg); + errcode |= ssl_sock_prep_ctx_and_inst(bind_conf, sni->conf, sni->ctx, sni->ckch_inst, &errmsg); } node = ebmb_next(node); } -- 2.39.5