From 8e8581e242f94625340028b9006bb62a3cbfd898 Mon Sep 17 00:00:00 2001 From: William Lallemand Date: Tue, 20 Oct 2020 17:36:46 +0200 Subject: [PATCH] MINOR: ssl: 'ssl-load-extra-del-ext' removes the certificate extension In issue #785, users are reporting that it's not convenient to load a ".crt.key" when the configuration contains a ".crt". This option allows to remove the extension of the certificate before trying to load any extra SSL file (.key, .ocsp, .sctl, .issuer etc.) The patch changes a little bit the way ssl_sock_load_files_into_ckch() looks for the file. --- doc/configuration.txt | 12 ++++ include/haproxy/ssl_sock-t.h | 1 + src/cfgparse-ssl.c | 10 +++ src/ssl_ckch.c | 121 ++++++++++++++++++++++++++++------- src/ssl_sock.c | 1 + 5 files changed, 122 insertions(+), 23 deletions(-) diff --git a/doc/configuration.txt b/doc/configuration.txt index bd9781cd1f..ea6c589c4e 100644 --- a/doc/configuration.txt +++ b/doc/configuration.txt @@ -1374,6 +1374,18 @@ ssl-dh-param-file "openssl dhparam ", where size should be at least 2048, as 1024-bit DH parameters should not be considered secure anymore. +ssl-load-extra-del-ext + This setting allows to configure the way HAProxy does the lookup for the + extra SSL files. By default HAProxy adds a new extension to the filename. + (ex: with "foobar.pem" load "foobar.pem.key"). With this option enabled, + HAProxy removes the extension before adding the new one (ex: with + "foobar.pem" load "foobar.key"). + + This option is not compatible with bundle extensions (.ecdsa, .rsa. .dsa) + and won't try to remove them. + + This option is disabled by default. See also "ssl-load-extra-files". + ssl-load-extra-files * This setting alters the way HAProxy will look for unspecified files during the loading of the SSL certificates associated to "bind" lines. It does not diff --git a/include/haproxy/ssl_sock-t.h b/include/haproxy/ssl_sock-t.h index a436a4a880..5b537fabac 100644 --- a/include/haproxy/ssl_sock-t.h +++ b/include/haproxy/ssl_sock-t.h @@ -290,6 +290,7 @@ struct global_ssl { int capture_cipherlist; /* Size of the cipherlist buffer. */ int keylog; /* activate keylog */ int extra_files; /* which files not defined in the configuration file are we looking for */ + int extra_files_noext; /* whether we remove the extension when looking up a extra file */ }; /* The order here matters for picking a default context, diff --git a/src/cfgparse-ssl.c b/src/cfgparse-ssl.c index d22ae96fb3..3bac5f9dc5 100644 --- a/src/cfgparse-ssl.c +++ b/src/cfgparse-ssl.c @@ -518,6 +518,15 @@ err_arg: } +/* parse 'ssl-load-extra-del-ext */ +static int ssl_parse_global_extra_noext(char **args, int section_type, struct proxy *curpx, + struct proxy *defpx, const char *file, int line, + char **err) +{ + global_ssl.extra_files_noext = 1; + return 0; +} + /***************************** Bind keyword Parsing ********************************************/ /* for ca-file and ca-verify-file */ @@ -1876,6 +1885,7 @@ static struct cfg_kw_list cfg_kws = {ILH, { { CFG_GLOBAL, "ssl-default-server-ciphersuites", ssl_parse_global_ciphersuites }, #endif { CFG_GLOBAL, "ssl-load-extra-files", ssl_parse_global_extra_files }, + { CFG_GLOBAL, "ssl-load-extra-del-ext", ssl_parse_global_extra_noext }, { 0, NULL, NULL }, }}; diff --git a/src/ssl_ckch.c b/src/ssl_ckch.c index 137a3a7dda..be7bd2971b 100644 --- a/src/ssl_ckch.c +++ b/src/ssl_ckch.c @@ -251,6 +251,7 @@ end: */ int ssl_sock_load_files_into_ckch(const char *path, struct cert_key_and_chain *ckch, char **err) { + struct buffer *fp = NULL; int ret = 1; /* try to load the PEM */ @@ -258,24 +259,72 @@ int ssl_sock_load_files_into_ckch(const char *path, struct cert_key_and_chain *c goto end; } + fp = alloc_trash_chunk(); + if (!fp) { + memprintf(err, "%sCan't allocate memory\n", err && *err ? *err : ""); + goto end; + } + + if (!chunk_strcpy(fp, path) || (b_data(fp) > MAXPATHLEN)) { + memprintf(err, "%s '%s' filename too long'.\n", + err && *err ? *err : "", fp->area); + ret = 1; + goto end; + } + + /* remove the extension */ + if (global_ssl.extra_files_noext) { + char *ext; + + /* look for the extension */ + if ((ext = strrchr(fp->area, '.'))) { + int n; + int found_ext = 0; /* bundle extension found ? */ + + ext++; /* we need to compare the ext after the dot */ + + for (n = 0; n < SSL_SOCK_NUM_KEYTYPES; n++) { + if (!strcmp(ext, SSL_SOCK_KEYTYPE_NAMES[n])) { + found_ext = 1; + } + } + + ext--; + if (!found_ext) /* if it wasn't a bundle extension we remove it */ + *ext = '\0'; + + fp->data = strlen(fp->area); + } + + } + /* try to load an external private key if it wasn't in the PEM */ if ((ckch->key == NULL) && (global_ssl.extra_files & SSL_GF_KEY)) { - char fp[MAXPATHLEN+1]; struct stat st; - snprintf(fp, MAXPATHLEN+1, "%s.key", path); - if (stat(fp, &st) == 0) { - if (ssl_sock_load_key_into_ckch(fp, NULL, ckch, err)) { + + if (!chunk_strcat(fp, ".key") || (b_data(fp) > MAXPATHLEN)) { + memprintf(err, "%s '%s' filename too long'.\n", + err && *err ? *err : "", fp->area); + ret = 1; + goto end; + } + + if (stat(fp->area, &st) == 0) { + if (ssl_sock_load_key_into_ckch(fp->area, NULL, ckch, err)) { memprintf(err, "%s '%s' is present but cannot be read or parsed'.\n", - err && *err ? *err : "", fp); + err && *err ? *err : "", fp->area); goto end; } } - } - if (ckch->key == NULL) { - memprintf(err, "%sNo Private Key found in '%s' or '%s.key'.\n", err && *err ? *err : "", path, path); - goto end; + if (ckch->key == NULL) { + memprintf(err, "%sNo Private Key found in '%s'.\n", err && *err ? *err : "", fp->area); + goto end; + } + /* remove the added extension */ + *(fp->area + fp->data - strlen(".key")) = '\0'; + b_sub(fp, strlen(".key")); } if (!X509_check_private_key(ckch->cert, ckch->key)) { @@ -287,33 +336,49 @@ int ssl_sock_load_files_into_ckch(const char *path, struct cert_key_and_chain *c #if (HA_OPENSSL_VERSION_NUMBER >= 0x1000200fL && !defined OPENSSL_NO_TLSEXT && !defined OPENSSL_IS_BORINGSSL) /* try to load the sctl file */ if (global_ssl.extra_files & SSL_GF_SCTL) { - char fp[MAXPATHLEN+1]; struct stat st; - snprintf(fp, MAXPATHLEN+1, "%s.sctl", path); - if (stat(fp, &st) == 0) { - if (ssl_sock_load_sctl_from_file(fp, NULL, ckch, err)) { + if (!chunk_strcat(fp, ".sctl") || b_data(fp) > MAXPATHLEN) { + memprintf(err, "%s '%s' filename too long'.\n", + err && *err ? *err : "", fp->area); + ret = 1; + goto end; + } + + if (stat(fp->area, &st) == 0) { + if (ssl_sock_load_sctl_from_file(fp->area, NULL, ckch, err)) { memprintf(err, "%s '%s.sctl' is present but cannot be read or parsed'.\n", - err && *err ? *err : "", fp); + err && *err ? *err : "", fp->area); ret = 1; goto end; } } + /* remove the added extension */ + *(fp->area + fp->data - strlen(".sctl")) = '\0'; + b_sub(fp, strlen(".sctl")); } #endif /* try to load an ocsp response file */ if (global_ssl.extra_files & SSL_GF_OCSP) { - char fp[MAXPATHLEN+1]; struct stat st; - snprintf(fp, MAXPATHLEN+1, "%s.ocsp", path); - if (stat(fp, &st) == 0) { - if (ssl_sock_load_ocsp_response_from_file(fp, NULL, ckch, err)) { + if (!chunk_strcat(fp, ".ocsp") || b_data(fp) > MAXPATHLEN) { + memprintf(err, "%s '%s' filename too long'.\n", + err && *err ? *err : "", fp->area); + ret = 1; + goto end; + } + + if (stat(fp->area, &st) == 0) { + if (ssl_sock_load_ocsp_response_from_file(fp->area, NULL, ckch, err)) { ret = 1; goto end; } } + /* remove the added extension */ + *(fp->area + fp->data - strlen(".ocsp")) = '\0'; + b_sub(fp, strlen(".ocsp")); } #ifndef OPENSSL_IS_BORINGSSL /* Useless for BoringSSL */ @@ -321,22 +386,30 @@ int ssl_sock_load_files_into_ckch(const char *path, struct cert_key_and_chain *c /* if no issuer was found, try to load an issuer from the .issuer */ if (!ckch->ocsp_issuer) { struct stat st; - char fp[MAXPATHLEN+1]; - snprintf(fp, MAXPATHLEN+1, "%s.issuer", path); - if (stat(fp, &st) == 0) { - if (ssl_sock_load_issuer_file_into_ckch(fp, NULL, ckch, err)) { + if (!chunk_strcat(fp, ".issuer") || b_data(fp) > MAXPATHLEN) { + memprintf(err, "%s '%s' filename too long'.\n", + err && *err ? *err : "", fp->area); + ret = 1; + goto end; + } + + if (stat(fp->area, &st) == 0) { + if (ssl_sock_load_issuer_file_into_ckch(fp->area, NULL, ckch, err)) { ret = 1; goto end; } if (X509_check_issued(ckch->ocsp_issuer, ckch->cert) != X509_V_OK) { memprintf(err, "%s '%s' is not an issuer'.\n", - err && *err ? *err : "", fp); + err && *err ? *err : "", fp->area); ret = 1; goto end; } } + /* remove the added extension */ + *(fp->area + fp->data - strlen(".issuer")) = '\0'; + b_sub(fp, strlen(".issuer")); } } #endif @@ -351,6 +424,8 @@ end: if (ret != 0) ssl_sock_free_cert_key_and_chain_contents(ckch); + free_trash_chunk(fp); + return ret; } diff --git a/src/ssl_sock.c b/src/ssl_sock.c index 69d76b05d1..2c146488bd 100644 --- a/src/ssl_sock.c +++ b/src/ssl_sock.c @@ -129,6 +129,7 @@ struct global_ssl global_ssl = { .ctx_cache = DEFAULT_SSL_CTX_CACHE, .capture_cipherlist = 0, .extra_files = SSL_GF_ALL, + .extra_files_noext = 0, #if (HA_OPENSSL_VERSION_NUMBER >= 0x10101000L) .keylog = 0 #endif -- 2.39.5