BUG/MEDIUM: ssl: improve error processing and reporting in ssl_sock_load_cert_list_file()

fe61656b added the ability to load a list of certificates from a file,
but error control was incomplete and misleading, as some errors such
as missing files were not reported, and errors reported with Alert()
instead of memprintf() were inappropriate and mixed with upper errors.
Also, the code really supports a single SNI filter right now, so let's
correct it and the doc for that, leaving room for later change if needed.

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 69ee1f01362db74830f4d7699aabd1d7041edf38..449d158a9bb38e9cd4e7a4c91e2ca34daf4f9016 100644 (file)
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -7216,10 +7216,10 @@ crt-ignore-err <errors>
 
 crt-list <file>
   This setting is only available when support for OpenSSL was built in. It
-  designates a list of PEM file with an optional list of SNI filter per
-  certificate, with the following format for each line :
+  designates a list of PEM file with an optional SNI filter per certificate,
+  with the following format for each line :
 
-        <crtfile>[ <snifilter>]*
+        <crtfile> [<snifilter>]
 
   Wildcards are supported in the SNI filter. The certificates will be presented
   to clients who provide a valid TLS Server Name Indication field matching one

diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 90c72322c3b964b914da596aa29a56b1ee9e0370..6faab78f0cd2e176929c848877ac76a624eea5f7 100644 (file)
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -498,11 +498,11 @@ int ssl_sock_load_cert_list_file(char *file, struct bind_conf *bind_conf, struct
        FILE *f;
        int linenum = 0;
        int cfgerr = 0;
-       char *sni_filter = NULL;
-       char *crt_file = NULL;
 
-       if ((f = fopen(file, "r")) == NULL)
+       if ((f = fopen(file, "r")) == NULL) {
+               memprintf(err, "cannot open file '%s' : %s", file, strerror(errno));
                return 1;
+       }
 
        while (fgets(thisline, sizeof(thisline), f) != NULL) {
                int arg;
@@ -516,9 +516,10 @@ int ssl_sock_load_cert_list_file(char *file, struct bind_conf *bind_conf, struct
                        /* Check if we reached the limit and the last char is not \n.
                         * Watch out for the last line without the terminating '\n'!
                         */
-                       Alert("parsing [%s:%d]: line too long, limit: %d.\n",
-                             file, linenum, (int)sizeof(thisline)-1);
+                       memprintf(err, "line %d too long in file '%s', limit is %d characters",
+                                 linenum, file, (int)sizeof(thisline)-1);
                        cfgerr = 1;
+                       break;
                }
 
                /* skip leading spaces */
@@ -540,68 +541,29 @@ int ssl_sock_load_cert_list_file(char *file, struct bind_conf *bind_conf, struct
                                while (isspace(*line))
                                        line++;
                                args[++arg] = line;
-                               break;
-                       }
-                       else {
-                               line++;
-                       }
-               }
-               while (*line) {
-                       if (*line == '#' || *line == '\n' || *line == '\r') {
-                               /* end of string, end of loop */
-                               *line = 0;
-                               break;
                        }
                        else {
                                line++;
                        }
                }
+
                /* empty line */
                if (!**args)
                        continue;
-               if (*line) {
-                       /* we had to stop due to too many args.
-                        * Let's terminate the string, print the offending part then cut the
-                        * last arg.
-                        */
-                       while (*line && *line != '#' && *line != '\n' && *line != '\r')
-                               line++;
-                       *line = '\0';
 
-                       Alert("parsing [%s:%d]: line too long, truncating at word %d, position %ld: <%s>.\n",
-                             file, linenum, arg + 1, (long)(args[arg] - thisline + 1), args[arg]);
+               if (arg > 2) {
+                       memprintf(err, "too many args on line %d in file '%s', only one SNI filter is supported (was '%s')",
+                                 linenum, file, args[2]);
                        cfgerr = 1;
-                       args[arg] = line;
-               }
-               /* zero out remaining args and ensure that at least one entry
-                * is zeroed out.
-                */
-               while (++arg <= MAX_LINE_ARGS) {
-                       args[arg] = line;
-               }
-
-               crt_file = strdup(args[0]);
-               if (*args[1])
-                       sni_filter = strdup(args[1]);
-               cfgerr = ssl_sock_load_cert_file(crt_file, bind_conf, curproxy, sni_filter, err);
-
-               if (sni_filter) {
-                       free(sni_filter);
-                       sni_filter = NULL;
-               }
-               if (crt_file) {
-                       free(crt_file);
-                       crt_file = NULL;
+                       break;
                }
 
-               if (cfgerr)
+               cfgerr = ssl_sock_load_cert_file(args[0], bind_conf, curproxy, arg > 1 ? args[1] : NULL, err);
+               if (cfgerr) {
+                       memprintf(err, "error processing line %d in file '%s' : %s", linenum, file, *err);
                        break;
+               }
        }
-       if (sni_filter)
-               free(sni_filter);
-       if (crt_file)
-               free(crt_file);
-
        fclose(f);
        return cfgerr;
 }
@@ -2507,8 +2469,10 @@ static int bind_parse_crt_list(char **args, int cur_arg, struct proxy *px, struc
                return ERR_ALERT | ERR_FATAL;
        }
 
-       if (ssl_sock_load_cert_list_file(args[cur_arg + 1], conf, px, err) > 0)
+       if (ssl_sock_load_cert_list_file(args[cur_arg + 1], conf, px, err) > 0) {
+               memprintf(err, "'%s' : %s", args[cur_arg], *err);
                return ERR_ALERT | ERR_FATAL;
+       }
 
        return 0;
 }