From: William Lallemand Date: Thu, 30 Jul 2026 13:41:12 +0000 (+0000) Subject: BUG/MINOR: ech: propagate error from load_echkeys() X-Git-Url: http://git.ipfire.org/gitweb/?a=commitdiff_plain;h=498dc1eceb44dfef22f6d5c0d1d2ca84b5745aca;p=thirdparty%2Fhaproxy.git BUG/MINOR: ech: propagate error from load_echkeys() load_echkeys() was not emitting any error messagw, so a failed load only ever produced "failed to load ECH keys". Add a char **err parameter to the function so it can emit TLS library errors or system errors. This should be backported to 3.3 and 3.4. --- diff --git a/include/haproxy/ech.h b/include/haproxy/ech.h index 0772db9b5..4defef2e7 100644 --- a/include/haproxy/ech.h +++ b/include/haproxy/ech.h @@ -5,7 +5,7 @@ #include -int load_echkeys(SSL_CTX *ctx, char *dirname, int *loaded); +int load_echkeys(SSL_CTX *ctx, char *dirname, int *loaded, char **err); int conn_get_ech_status(struct connection *conn, struct buffer *buf); int conn_get_ech_outer_sni(struct connection *conn, struct buffer *buf); diff --git a/src/ech.c b/src/ech.c index d5de79268..c62ae1b6c 100644 --- a/src/ech.c +++ b/src/ech.c @@ -3,6 +3,8 @@ #include +#include +#include #include #include @@ -32,55 +34,94 @@ struct show_ech_ctx { * load any key files called .ech we find in the named * directory */ -int load_echkeys(SSL_CTX *ctx, char *dirname, int *loaded) +int load_echkeys(SSL_CTX *ctx, char *dirname, int *loaded, char **err) { struct dirent **de_list = NULL; struct stat thestat; - int rv = 0, i, nrv, somekeyworked = 0; + int rv = 0, i, nrv = 0, somekeyworked = 0; char *den = NULL, *last4 = NULL, privname[PATH_MAX]; size_t elen = 0, nlen = 0; - OSSL_ECHSTORE * const es = OSSL_ECHSTORE_new(NULL, NULL); + OSSL_ECHSTORE *es; + ERR_clear_error(); + + es = OSSL_ECHSTORE_new(NULL, NULL); if (es == NULL) goto end; nrv = scandir(dirname, &de_list, 0, alphasort); - if (nrv < 0) + if (nrv < 0) { + memprintf(err, "%sunable to scan ECH directory '%s': %s", + err && *err ? *err : "", dirname, strerror(errno)); goto end; + } for (i = 0; i != nrv; i++) { struct dirent *de = de_list[i]; den = de->d_name; nlen = strlen(den); if (nlen > 4) { + BIO *in = NULL; + int load_failed = 1; + const int is_retry_config = OSSL_ECH_FOR_RETRY; + last4 = den + nlen - 4; if (strncmp(last4, ".ech", 4)) goto ignore_entry; if ((elen + 1 + nlen + 1) >= PATH_MAX) goto ignore_entry; snprintf(privname, PATH_MAX,"%s/%s", dirname, den); - if (stat(privname, &thestat) == 0) { - BIO *in = BIO_new_file(privname, "r"); - const int is_retry_config = OSSL_ECH_FOR_RETRY; - - if (in != NULL && 1 == OSSL_ECHSTORE_read_pem(es, in, is_retry_config)) - somekeyworked = 1; - BIO_free_all(in); + if (stat(privname, &thestat) != 0) { + memprintf(err, "%sunable to stat ECH key file '%s': %s", + err && *err ? *err : "", privname, strerror(errno)); + goto failed; + } + if ((in = BIO_new_file(privname, "r")) == NULL) { + memprintf(err, "%sunable to open ECH key file '%s': %s", + err && *err ? *err : "", privname, strerror(errno)); + goto failed; } + if (OSSL_ECHSTORE_read_pem(es, in, is_retry_config) != 1) { + memprintf(err, "%sunable to load ECH key file '%s'", + err && *err ? *err : "", privname); + goto failed; + } + load_failed = 0; + somekeyworked++; +failed: + BIO_free_all(in); + /* a ".ech" file is expected to be valid; fail immediately */ + if (load_failed) + goto end; } ignore_entry: - free(de); } - if (somekeyworked == 0) + if (somekeyworked == 0) { + memprintf(err, "%sno usable ECH key file found in '%s'", + err && *err ? *err : "", dirname); goto end; + } if (OSSL_ECHSTORE_num_keys(es, loaded) != 1) goto end; if (1 != SSL_CTX_set1_echstore(ctx, es)) goto end; rv = 1; end: + for (i = 0; i < nrv; i++) + free(de_list[i]); free(de_list); OSSL_ECHSTORE_free(es); + if (!rv) { + unsigned long ret; + + /* drain TLS library error queue */ + while ((ret = ERR_get_error()) != 0) + memprintf(err, "%s%s%s", err && *err ? *err : "", + err && *err ? ": " : "", ERR_reason_error_string(ret)); + + if (!err || !*err) + memprintf(err, "unknown error"); + } return rv; } diff --git a/src/quic_ssl.c b/src/quic_ssl.c index 18dc9650c..7961ce4cd 100644 --- a/src/quic_ssl.c +++ b/src/quic_ssl.c @@ -817,13 +817,15 @@ int ssl_quic_initial_ctx(struct bind_conf *bind_conf) #ifdef USE_ECH if (bind_conf->ssl_conf.ech_filedir) { int loaded = 0; + char *ech_err = NULL; - if (load_echkeys(ctx, bind_conf->ssl_conf.ech_filedir, &loaded) != 1) { + if (load_echkeys(ctx, bind_conf->ssl_conf.ech_filedir, &loaded, &ech_err) != 1) { cfgerr += 1; - ha_alert("Proxy '%s': failed to load ECH key s from %s for '%s' at [%s:%d].\n", + ha_alert("Proxy '%s': failed to load ECH keys from %s for '%s' at [%s:%d]: %s.\n", bind_conf->frontend->id, bind_conf->ssl_conf.ech_filedir, - bind_conf->arg, bind_conf->file, bind_conf->line); + bind_conf->arg, bind_conf->file, bind_conf->line, ech_err); } + ha_free(&ech_err); } #endif diff --git a/src/ssl_sock.c b/src/ssl_sock.c index 0013905a7..2bb9d3bcf 100644 --- a/src/ssl_sock.c +++ b/src/ssl_sock.c @@ -4108,14 +4108,16 @@ ssl_sock_initial_ctx(struct bind_conf *bind_conf) #ifdef USE_ECH if (bind_conf->ssl_conf.ech_filedir) { int loaded = 0; + char *ech_err = NULL; - if (load_echkeys(ctx, bind_conf->ssl_conf.ech_filedir, &loaded) != 1) { - cfgerr += 1; - ha_alert("Proxy '%s': failed to load ECH key s from %s for '%s' at [%s:%d].\n", - bind_conf->frontend->id, bind_conf->ssl_conf.ech_filedir, - bind_conf->arg, bind_conf->file, bind_conf->line); - } - } + if (load_echkeys(ctx, bind_conf->ssl_conf.ech_filedir, &loaded, &ech_err) != 1) { + cfgerr += 1; + ha_alert("Proxy '%s': failed to load ECH keys from %s for '%s' at [%s:%d]: %s.\n", + bind_conf->frontend->id, bind_conf->ssl_conf.ech_filedir, + bind_conf->arg, bind_conf->file, bind_conf->line, ech_err); + } + ha_free(&ech_err); + } #endif if (conf_ssl_methods->flags && (conf_ssl_methods->min || conf_ssl_methods->max))