Re: [PATCH 2/2] MINOR: ssl: Add error if a crt-list might be truncated

[Willy Tarreau]

Hi Tim,

On Mon, Sep 28, 2020 at 07:02:15PM +0200, Tim Duesterhus wrote:
> see https://github.com/haproxy/haproxy/issues/860#issuecomment-693422936
> see 0354b658f061d00d5ab4b728d7deeff2c8f1503a
> 
> This should be backported as a warning to 2.2.

As a rule of thumb, it would be good to keep in mind to always provide a
bit of context in commit messages so that people working on backports can
judge of the relevance of a patch when reading the "git log" output without
having to start a browser, switch context, or even depend on github's
accessibility.

Links are perfect for reference and to provide the full details but should
not constitute the essence of the description, which should still be there,
even if it can be synthetic. Plus imagine if two years ahead github decides
to start purge old issues, we'd lose all history!

Thanks!
Willy

[PATCH 2/2] MINOR: ssl: Add error if a crt-list might be truncated

[Tim Duesterhus]

see https://github.com/haproxy/haproxy/issues/860#issuecomment-693422936
see 0354b658f061d00d5ab4b728d7deeff2c8f1503a

This should be backported as a warning to 2.2.
---
 src/ssl_crtlist.c | 28 ++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/src/ssl_crtlist.c b/src/ssl_crtlist.c
index f1c15e051..c0987bc17 100644
--- a/src/ssl_crtlist.c
+++ b/src/ssl_crtlist.c
@@ -452,6 +452,7 @@ int crtlist_parse_file(char *file, struct bind_conf 
*bind_conf, struct proxy *cu
struct stat buf;
int linenum = 0;
int cfgerr = 0;
+   int missing_lf = -1;
 
if ((f = fopen(file, "r")) == NULL) {
memprintf(err, "cannot open file '%s' : %s", file, 
strerror(errno));
@@ -471,6 +472,14 @@ int crtlist_parse_file(char *file, struct bind_conf 
*bind_conf, struct proxy *cu
char *crt_path;
struct ckch_store *ckchs;
 
+   if (missing_lf != -1) {
+   memprintf(err, "parsing [%s:%d]: Stray NUL character at 
position %d.\n",
+ file, linenum, (missing_lf + 1));
+   cfgerr |= ERR_ALERT | ERR_FATAL;
+   missing_lf = -1;
+   break;
+   }
+
linenum++;
end = line + strlen(line);
if (end-line == sizeof(thisline)-1 && *(end-1) != '\n') {
@@ -486,14 +495,22 @@ int crtlist_parse_file(char *file, struct bind_conf 
*bind_conf, struct proxy *cu
if (*line == '#' || *line == '\n' || *line == '\r')
continue;
 
+   if (end > line && *(end-1) == '\n') {
+   /* kill trailing LF */
+   *(end - 1) = 0;
+   }
+   else {
+   /* mark this line as truncated */
+   missing_lf = end - line;
+   }
+
entry = crtlist_entry_new();
if (entry == NULL) {
memprintf(err, "Not enough memory!");
cfgerr |= ERR_ALERT | ERR_FATAL;
goto error;
}
-   if (*(end - 1) == '\n')
-   *(end - 1) = '\0'; /* line parser mustn't receive any 
\n */
+
cfgerr |= crtlist_parse_line(thisline, _path, entry, file, 
linenum, err);
if (cfgerr & ERR_CODE)
goto error;
@@ -587,6 +604,13 @@ int crtlist_parse_file(char *file, struct bind_conf 
*bind_conf, struct proxy *cu
 
entry = NULL;
}
+
+   if (missing_lf != -1) {
+   memprintf(err, "parsing [%s:%d]: Missing LF on last line, file 
might have been truncated at position %d.\n",
+ file, linenum, (missing_lf + 1));
+   cfgerr |= ERR_ALERT | ERR_FATAL;
+   }
+
if (cfgerr & ERR_CODE)
goto error;
 
-- 
2.28.0