From d817dc733eacfd7cf5bb0bbc6128f44644db078e Mon Sep 17 00:00:00 2001 From: Remi Tricot-Le Breton Date: Mon, 25 Jan 2021 17:19:43 +0100 Subject: [PATCH] MEDIUM: ssl: Load client certificates in a ckch for backend servers In order for the backend server's certificate to be hot-updatable, it needs to fit into the implementation used for the "bind" certificates. This patch follows the architecture implemented for the frontend implementation and reuses its structures and general function calls (adapted for the server side). The ckch store logic is kept and a dedicated ckch instance is used (one per server). The whole sni_ctx logic was not kept though because it is not needed. All the new functions added in this patch are basically server-side copies of functions that already exist on the frontend side with all the sni and bind_cond references removed. The ckch_inst structure has a new 'is_server_instance' flag which is used to distinguish regular instances from the server-side ones, and a new pointer to the server's structure in case of backend instance. Since the new server ckch instances are linked to a standard ckch_store, a lookup in the ckch store table will succeed so the cli code used to update bind certificates needs to be covered to manage those new server side ckch instances. --- include/haproxy/server-t.h | 3 + include/haproxy/ssl_ckch-t.h | 2 + include/haproxy/ssl_ckch.h | 2 + include/haproxy/ssl_sock.h | 2 + src/cfgparse-ssl.c | 2 +- src/ssl_ckch.c | 3 + src/ssl_sock.c | 209 ++++++++++++++++++++++++++++++----- 7 files changed, 192 insertions(+), 31 deletions(-) diff --git a/include/haproxy/server-t.h b/include/haproxy/server-t.h index 68463c57c..a1f72fd00 100644 --- a/include/haproxy/server-t.h +++ b/include/haproxy/server-t.h @@ -309,6 +309,9 @@ struct server { int size; int allocated_size; } * reused_sess; + + struct ckch_inst *inst; /* Instance of the ckch_store in which the certificate was loaded (might be null if server has no certificate) */ + char *ciphers; /* cipher suite to use if non-null */ #ifdef HAVE_SSL_CTX_SET_CIPHERSUITES char *ciphersuites; /* TLS 1.3 cipher suite to use if non-null */ diff --git a/include/haproxy/ssl_ckch-t.h b/include/haproxy/ssl_ckch-t.h index 9cb0dc486..37ef44681 100644 --- a/include/haproxy/ssl_ckch-t.h +++ b/include/haproxy/ssl_ckch-t.h @@ -85,7 +85,9 @@ struct ckch_inst { struct ssl_bind_conf *ssl_conf; /* pointer to the ssl_conf which is used by every sni_ctx of 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 server *server; /* pointer to the server if is_server_instance is set, NULL otherwise */ 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 */ 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 */ diff --git a/include/haproxy/ssl_ckch.h b/include/haproxy/ssl_ckch.h index 87c69ec19..0bcd0d764 100644 --- a/include/haproxy/ssl_ckch.h +++ b/include/haproxy/ssl_ckch.h @@ -49,6 +49,8 @@ void ckch_inst_free(struct ckch_inst *inst); struct ckch_inst *ckch_inst_new(); int ckch_inst_new_load_store(const char *path, struct ckch_store *ckchs, struct bind_conf *bind_conf, struct ssl_bind_conf *ssl_conf, char **sni_filter, int fcount, struct ckch_inst **ckchi, char **err); +int ckch_inst_new_load_srv_store(const char *path, struct ckch_store *ckchs, + struct ckch_inst **ckchi, SSL_CTX **ssl_ctx, char **err); void ckch_deinit(); diff --git a/include/haproxy/ssl_sock.h b/include/haproxy/ssl_sock.h index 4da5925df..db6d8387e 100644 --- a/include/haproxy/ssl_sock.h +++ b/include/haproxy/ssl_sock.h @@ -57,6 +57,7 @@ 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); @@ -115,6 +116,7 @@ void ssl_async_fd_free(int fd); struct issuer_chain* ssl_get0_issuer_chain(X509 *cert); int ssl_load_global_issuer_from_BIO(BIO *in, char *fp, char **err); int ssl_sock_load_cert(char *path, struct bind_conf *bind_conf, char **err); +int ssl_sock_load_srv_cert(char *path, struct server *server, char **err); void ssl_free_global_issuers(void); int ssl_sock_load_cert_list_file(char *file, int dir, struct bind_conf *bind_conf, struct proxy *curproxy, char **err); int ssl_init_single_engine(const char *engine_id, const char *def_algorithms); diff --git a/src/cfgparse-ssl.c b/src/cfgparse-ssl.c index ec7079ebd..55fe08463 100644 --- a/src/cfgparse-ssl.c +++ b/src/cfgparse-ssl.c @@ -1452,7 +1452,7 @@ static int srv_parse_crt(char **args, int *cur_arg, struct proxy *px, struct ser else memprintf(&newsrv->ssl_ctx.client_crt, "%s", args[*cur_arg + 1]); - return 0; + return ssl_sock_load_srv_cert(newsrv->ssl_ctx.client_crt, newsrv, err); } /* parse the "no-check-ssl" server keyword */ diff --git a/src/ssl_ckch.c b/src/ssl_ckch.c index dafe59eaf..1a09e8db8 100644 --- a/src/ssl_ckch.c +++ b/src/ssl_ckch.c @@ -1328,6 +1328,9 @@ static int cli_io_handler_commit_cert(struct appctx *appctx) fcount = ckchi->crtlist_entry->fcount; } + 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); if (errcode & ERR_CODE) diff --git a/src/ssl_sock.c b/src/ssl_sock.c index c1acdfe39..89a698b3b 100644 --- a/src/ssl_sock.c +++ b/src/ssl_sock.c @@ -3244,6 +3244,44 @@ static int ssl_sock_put_ckch_into_ctx(const char *path, const struct cert_key_an return errcode; } + +/* Loads the info of a ckch built out of a backend certificate into an SSL ctx + * Returns a bitfield containing the flags: + * ERR_FATAL in any fatal error case + * ERR_ALERT if the reason of the error is available in err + * ERR_WARN if a warning is available into err + * The value 0 means there is no error nor warning and + * the operation succeed. + */ +static int ssl_sock_put_srv_ckch_into_ctx(const char *path, const struct cert_key_and_chain *ckch, + SSL_CTX *ctx, char **err) +{ + int errcode = 0; + STACK_OF(X509) *find_chain = NULL; + + /* Load the private key */ + if (SSL_CTX_use_PrivateKey(ctx, ckch->key) <= 0) { + memprintf(err, "%sunable to load SSL private key into SSL Context '%s'.\n", + err && *err ? *err : "", path); + errcode |= ERR_ALERT | ERR_FATAL; + } + + /* Load certificate chain */ + errcode |= ssl_sock_load_cert_chain(path, ckch, ctx, &find_chain, err); + if (errcode & ERR_CODE) + goto end; + + if (SSL_CTX_check_private_key(ctx) <= 0) { + memprintf(err, "%sinconsistencies between private key and certificate loaded from PEM file '%s'.\n", + err && *err ? *err : "", path); + errcode |= ERR_ALERT | ERR_FATAL; + } + +end: + return errcode; +} + + /* * This function allocate a ckch_inst and create its snis * @@ -3405,6 +3443,78 @@ error: return errcode; } + +/* + * This function allocate a ckch_inst that will be used on the backend side + * (server line) + * + * Returns a bitfield containing the flags: + * ERR_FATAL in any fatal error case + * ERR_ALERT if the reason of the error is available in err + * ERR_WARN if a warning is available into err + */ +int ckch_inst_new_load_srv_store(const char *path, struct ckch_store *ckchs, + struct ckch_inst **ckchi, SSL_CTX **ssl_ctx, char **err) +{ + SSL_CTX *ctx; + struct cert_key_and_chain *ckch; + struct ckch_inst *ckch_inst = NULL; + int errcode = 0; + + *ckchi = NULL; + + if (!ckchs || !ckchs->ckch) + return ERR_FATAL; + + ckch = ckchs->ckch; + + ctx = SSL_CTX_new(SSLv23_client_method()); + if (!ctx) { + memprintf(err, "%sunable to allocate SSL context for cert '%s'.\n", + err && *err ? *err : "", path); + errcode |= ERR_ALERT | ERR_FATAL; + goto error; + } + + if (*ssl_ctx) + SSL_CTX_free(*ssl_ctx); + *ssl_ctx = ctx; + + errcode |= ssl_sock_put_srv_ckch_into_ctx(path, ckch, ctx, err); + if (errcode & ERR_CODE) + goto error; + + ckch_inst = ckch_inst_new(); + if (!ckch_inst) { + memprintf(err, "%sunable to allocate SSL context for cert '%s'.\n", + err && *err ? *err : "", path); + errcode |= ERR_ALERT | ERR_FATAL; + goto error; + } + + SSL_CTX_up_ref(ctx); + + /* everything succeed, the ckch instance can be used */ + ckch_inst->bind_conf = NULL; + ckch_inst->ssl_conf = NULL; + ckch_inst->ckch_store = ckchs; + + SSL_CTX_free(ctx); /* we need to free the ctx since we incremented the refcount where it's used */ + + *ckchi = ckch_inst; + return errcode; + +error: + /* free the allocated sni_ctxs */ + if (ckch_inst) { + ckch_inst_free(ckch_inst); + ckch_inst = NULL; + } + SSL_CTX_free(ctx); + + return errcode; +} + /* Returns a set of ERR_* flags possibly with an error in . */ static int ssl_sock_load_ckchs(const char *path, struct ckch_store *ckchs, struct bind_conf *bind_conf, struct ssl_bind_conf *ssl_conf, @@ -3425,6 +3535,23 @@ static int ssl_sock_load_ckchs(const char *path, struct ckch_store *ckchs, return errcode; } +static int ssl_sock_load_srv_ckchs(const char *path, struct ckch_store *ckchs, + struct ckch_inst **ckch_inst, + SSL_CTX **ssl_ctx, char **err) +{ + int errcode = 0; + + /* we found the ckchs in the tree, we can use it directly */ + errcode |= ckch_inst_new_load_srv_store(path, ckchs, ckch_inst, ssl_ctx, err); + + if (errcode & ERR_CODE) + return errcode; + + /* succeed, add the instance to the ckch_store's list of instance */ + LIST_ADDQ(&ckchs->ckch_inst, &((*ckch_inst)->by_ckchs)); + return errcode; +} + @@ -3610,6 +3737,45 @@ int ssl_sock_load_cert(char *path, struct bind_conf *bind_conf, char **err) return cfgerr; } + +/* Create a full ssl context and ckch instance that will be used for a specific + * backend server (server configuration line). + * Returns a set of ERR_* flags possibly with an error in . + */ +int ssl_sock_load_srv_cert(char *path, struct server *server, char **err) +{ + struct stat buf; + int cfgerr = 0; + struct ckch_store *ckchs; + int found = 0; /* did we found a file to load ? */ + + if ((ckchs = ckchs_lookup(path))) { + /* we found the ckchs in the tree, we can use it directly */ + cfgerr |= ssl_sock_load_srv_ckchs(path, ckchs, &server->ssl_ctx.inst, &server->ssl_ctx.ctx, err); + found++; + } else if (stat(path, &buf) == 0) { + /* We do not manage directories on backend side. */ + if (S_ISDIR(buf.st_mode) == 0) { + ++found; + ckchs = ckchs_load_cert_file(path, err); + if (!ckchs) + cfgerr |= ERR_ALERT | ERR_FATAL; + cfgerr |= ssl_sock_load_srv_ckchs(path, ckchs, &server->ssl_ctx.inst, &server->ssl_ctx.ctx, err); + if (server->ssl_ctx.inst) { + server->ssl_ctx.inst->is_server_instance = 1; + server->ssl_ctx.inst->server = server; + } + } + } + if (!found) { + memprintf(err, "%sunable to stat SSL certificate from file '%s' : %s.\n", + err && *err ? *err : "", path, strerror(errno)); + cfgerr |= ERR_ALERT | ERR_FATAL; + } + + return cfgerr; +} + /* Create an initial CTX used to start the SSL connection before switchctx */ static int ssl_sock_initial_ctx(struct bind_conf *bind_conf) @@ -4472,7 +4638,7 @@ int ssl_sock_prepare_srv_ctx(struct server *srv) { struct proxy *curproxy = srv->proxy; int cfgerr = 0; - SSL_CTX *ctx = NULL; + SSL_CTX *ctx = srv->ssl_ctx.ctx; /* Make sure openssl opens /dev/urandom before the chroot */ if (!ssl_initialize_random()) { @@ -4496,16 +4662,20 @@ int ssl_sock_prepare_srv_ctx(struct server *srv) if (srv->use_ssl == 1) srv->xprt = &ssl_sock; - ctx = SSL_CTX_new(SSLv23_client_method()); + /* The context will be uninitialized if there wasn't any "cert" option + * in the server line. */ if (!ctx) { - ha_alert("config : %s '%s', server '%s': unable to allocate ssl context.\n", - proxy_type_str(curproxy), curproxy->id, - srv->id); - cfgerr++; - return cfgerr; - } + ctx = SSL_CTX_new(SSLv23_client_method()); + if (!ctx) { + ha_alert("config : %s '%s', server '%s': unable to allocate ssl context.\n", + proxy_type_str(curproxy), curproxy->id, + srv->id); + cfgerr++; + return cfgerr; + } - srv->ssl_ctx.ctx = ctx; + srv->ssl_ctx.ctx = ctx; + } cfgerr += ssl_sock_prepare_srv_ssl_ctx(srv, srv->ssl_ctx.ctx); @@ -4602,27 +4772,6 @@ int ssl_sock_prepare_srv_ssl_ctx(const struct server *srv, SSL_CTX *ctx) #endif SSL_CTX_set_mode(ctx, mode); - if (srv->ssl_ctx.client_crt) { - if (SSL_CTX_use_PrivateKey_file(srv->ssl_ctx.ctx, srv->ssl_ctx.client_crt, SSL_FILETYPE_PEM) <= 0) { - ha_alert("config : %s '%s', server '%s': unable to load SSL private key from PEM file '%s'.\n", - proxy_type_str(curproxy), curproxy->id, - srv->id, srv->ssl_ctx.client_crt); - cfgerr++; - } - else if (SSL_CTX_use_certificate_chain_file(srv->ssl_ctx.ctx, srv->ssl_ctx.client_crt) <= 0) { - ha_alert("config : %s '%s', server '%s': unable to load ssl certificate from PEM file '%s'.\n", - proxy_type_str(curproxy), curproxy->id, - srv->id, srv->ssl_ctx.client_crt); - cfgerr++; - } - else if (SSL_CTX_check_private_key(srv->ssl_ctx.ctx) <= 0) { - ha_alert("config : %s '%s', server '%s': inconsistencies between private key and certificate loaded from PEM file '%s'.\n", - proxy_type_str(curproxy), curproxy->id, - srv->id, srv->ssl_ctx.client_crt); - cfgerr++; - } - } - if (global.ssl_server_verify == SSL_SERVER_VERIFY_REQUIRED) verify = SSL_VERIFY_PEER; switch (srv->ssl_ctx.verify) {