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 0772db9b523407fb4202fa0ee7036a13cf4f184f..4defef2e750a275ff3ef0449b2717766066e9c2d 100644 (file)
--- a/include/haproxy/ech.h
+++ b/include/haproxy/ech.h
@@ -5,7 +5,7 @@
 
 #include <openssl/ech.h>
 
-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 d5de79268bff1dd848914de8482e40d08938b8b1..c62ae1b6ce3e6f9ab0ec72f94698e6a87bab8cc0 100644 (file)
--- a/src/ech.c
+++ b/src/ech.c
@@ -3,6 +3,8 @@
 
 
 #include <dirent.h>
+#include <errno.h>
+#include <string.h>
 #include <sys/stat.h>
 
 #include <haproxy/applet.h>
@@ -32,55 +34,94 @@ struct show_ech_ctx {
  * load any key files called <name>.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 18dc9650cdd81831dbc93337704ad0398ed8d985..7961ce4cd8d7e19374f46de95b3f83a00619443e 100644 (file)
--- 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 0013905a79eecc354f5a022188418a675bba04a0..2bb9d3bcf6ed21ef0ee3bc9c86e00ff40be21332 100644 (file)
--- 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))