From 292e29351881f61ade851a57e61499b8512bb7ae Mon Sep 17 00:00:00 2001 From: William Lallemand Date: Wed, 19 Mar 2025 17:43:15 +0100 Subject: [PATCH] MEDIUM: ssl/ckch: make the ckch_conf more generic The ckch_store_load_files() functions make specific processing for PARSE_TYPE_STR as if it was a type only used for paths. This patch changes a little bit the way it's done, PARSE_TYPE_STR is only meant to strdup() a string and stores the resulting pointer in the ckch_conf structure. Any processing regarding the path is now done in the callback. Since the callbacks were basically doing the same thing, they were transformed into the DECLARE_CKCH_CONF_LOAD() macros which allows to do some templating of these functions. The resulting ckch_conf_load_* functions will do the same as before, except they will also do the path processing instead of letting ckch_store_load_files() do it, which mean we don't need the 'base' member anymore in the struct ckch_conf_kws. --- include/haproxy/ssl_ckch-t.h | 1 - include/haproxy/ssl_ckch.h | 28 ++++++++++++++++++------ src/ssl_ckch.c | 41 ++++++++++++++++-------------------- 3 files changed, 40 insertions(+), 30 deletions(-) diff --git a/include/haproxy/ssl_ckch-t.h b/include/haproxy/ssl_ckch-t.h index 273383318..83ac3a30c 100644 --- a/include/haproxy/ssl_ckch-t.h +++ b/include/haproxy/ssl_ckch-t.h @@ -192,7 +192,6 @@ struct ckch_conf_kws { ssize_t offset; enum parse_type_t type; int (*func)(void *value, char *buf, struct ckch_data *d, int cli, char **err); - char **base; /* ptr to the base path */ }; extern struct ckch_conf_kws ckch_conf_kws[]; diff --git a/include/haproxy/ssl_ckch.h b/include/haproxy/ssl_ckch.h index 8071f132e..cdae62435 100644 --- a/include/haproxy/ssl_ckch.h +++ b/include/haproxy/ssl_ckch.h @@ -25,6 +25,9 @@ #include +#include +#include + /* cert_key_and_chain functions */ int ssl_sock_load_files_into_ckch(const char *path, struct ckch_data *data, struct ckch_conf *conf, char **err); @@ -81,12 +84,25 @@ int __ssl_store_load_locations_file(char *path, int create_if_none, enum cafile_ extern struct cert_exts cert_exts[]; extern int (*ssl_commit_crlfile_cb)(const char *path, X509_STORE *ctx, char **err); -/* ckch_conf keyword loading */ -static inline int ckch_conf_load_pem(void *value, char *buf, struct ckch_data *d, int cli, char **err) { if (cli) return 0; return ssl_sock_load_pem_into_ckch(value, buf, d, err); } -static inline int ckch_conf_load_key(void *value, char *buf, struct ckch_data *d, int cli, char **err) { if (cli) return 0; return ssl_sock_load_key_into_ckch(value, buf, d, err); } -static inline int ckch_conf_load_ocsp_response(void *value, char *buf, struct ckch_data *d, int cli, char **err) { if (cli) return 0; return ssl_sock_load_ocsp_response_from_file(value, buf, d, err); } -static inline int ckch_conf_load_ocsp_issuer(void *value, char *buf, struct ckch_data *d, int cli, char **err) { if (cli) return 0; return ssl_sock_load_issuer_file_into_ckch(value, buf, d, err); } -static inline int ckch_conf_load_sctl(void *value, char *buf, struct ckch_data *d, int cli, char **err) { if (cli) return 0; return ssl_sock_load_sctl_from_file(value, buf, d, err); } +/* + * ckch_conf keywords loading + * The following macro allow to declare a wrapper on function that actually load files + * + */ +#define DECLARE_CKCH_CONF_LOAD(name, base, callback) \ +static inline int ckch_conf_load_##name(void *value, char *buf, struct ckch_data *d, int cli, char **err) \ +{ \ + char path[PATH_MAX]; \ + int err_code = 0; \ + if (cli) \ + return 0; \ + err_code |= path_base(value, (base), path, err); \ + if (err_code & ERR_CODE) \ + goto out; \ + err_code |= (callback)(path, buf, d, err); \ +out: \ + return err_code; \ +}; #endif /* USE_OPENSSL */ #endif /* _HAPROXY_SSL_CRTLIST_H */ diff --git a/src/ssl_ckch.c b/src/ssl_ckch.c index 90887c55b..8e49fcb93 100644 --- a/src/ssl_ckch.c +++ b/src/ssl_ckch.c @@ -4536,21 +4536,29 @@ static char *current_crtbase = NULL; static char *current_keybase = NULL; static int crtstore_load = 0; /* did we already load in this crt-store */ +/* declare the ckch_conf_load_* wrapper functions */ +DECLARE_CKCH_CONF_LOAD(pem, current_crtbase, ssl_sock_load_pem_into_ckch); +DECLARE_CKCH_CONF_LOAD(key, current_keybase, ssl_sock_load_key_into_ckch); +DECLARE_CKCH_CONF_LOAD(ocsp_response, current_crtbase, ssl_sock_load_ocsp_response_from_file); +DECLARE_CKCH_CONF_LOAD(ocsp_issuer, current_crtbase, ssl_sock_load_issuer_file_into_ckch); +DECLARE_CKCH_CONF_LOAD(sctl, current_crtbase, ssl_sock_load_sctl_from_file); + struct ckch_conf_kws ckch_conf_kws[] = { - { "alias", -1, PARSE_TYPE_NONE, NULL, NULL }, - { "crt", offsetof(struct ckch_conf, crt), PARSE_TYPE_STR, ckch_conf_load_pem, ¤t_crtbase }, - { "key", offsetof(struct ckch_conf, key), PARSE_TYPE_STR, ckch_conf_load_key, ¤t_keybase }, + { "alias", -1, PARSE_TYPE_NONE, NULL, }, + { "crt", offsetof(struct ckch_conf, crt), PARSE_TYPE_STR, ckch_conf_load_pem, }, + { "key", offsetof(struct ckch_conf, key), PARSE_TYPE_STR, ckch_conf_load_key, }, #ifdef HAVE_SSL_OCSP - { "ocsp", offsetof(struct ckch_conf, ocsp), PARSE_TYPE_STR, ckch_conf_load_ocsp_response, ¤t_crtbase }, + { "ocsp", offsetof(struct ckch_conf, ocsp), PARSE_TYPE_STR, ckch_conf_load_ocsp_response, }, #endif - { "issuer", offsetof(struct ckch_conf, issuer), PARSE_TYPE_STR, ckch_conf_load_ocsp_issuer, ¤t_crtbase }, - { "sctl", offsetof(struct ckch_conf, sctl), PARSE_TYPE_STR, ckch_conf_load_sctl, ¤t_crtbase }, + { "issuer", offsetof(struct ckch_conf, issuer), PARSE_TYPE_STR, ckch_conf_load_ocsp_issuer, }, + { "sctl", offsetof(struct ckch_conf, sctl), PARSE_TYPE_STR, ckch_conf_load_sctl, }, #if defined(HAVE_SSL_OCSP) - { "ocsp-update", offsetof(struct ckch_conf, ocsp_update_mode), PARSE_TYPE_ONOFF, ocsp_update_init, NULL }, + { "ocsp-update", offsetof(struct ckch_conf, ocsp_update_mode), PARSE_TYPE_ONOFF, ocsp_update_init, }, #endif - { NULL, -1, PARSE_TYPE_STR, NULL, NULL } + { NULL, -1, PARSE_TYPE_STR, NULL, } }; + /* crt-store does not try to find files, but use the stored filename */ int ckch_store_load_files(struct ckch_conf *f, struct ckch_store *c, int cli, char **err) { @@ -4574,28 +4582,15 @@ int ckch_store_load_files(struct ckch_conf *f, struct ckch_store *c, int cli, ch case PARSE_TYPE_STR: { char *v; - char *path; - char **base = ckch_conf_kws[i].base; - char path_base[PATH_MAX]; v = *(char **)src; if (!v) goto next; - path = v; - if (base && *base && *path != '/') { - int rv = snprintf(path_base, sizeof(path_base), "%s/%s", *base, path); - if (rv >= sizeof(path_base)) { - memprintf(err, "'%s/%s' : path too long", *base, path); - err_code |= ERR_ALERT | ERR_FATAL; - goto out; - } - path = path_base; - } - rc = ckch_conf_kws[i].func(path, NULL, d, cli, err); + rc = ckch_conf_kws[i].func(v, NULL, d, cli, err); if (rc) { err_code |= ERR_ALERT | ERR_FATAL; - memprintf(err, "%s '%s' cannot be read or parsed.", err && *err ? *err : "", path); + memprintf(err, "%s '%s' cannot be read or parsed.", err && *err ? *err : "", v); goto out; } break;