[PATCH 2/2] MINOR: ssl: Add error if a crt-list might be truncated
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
[PATCH 1/2] CLEANUP: ssl: Use structured format for error line report during crt-list parsing
This reuses the known `parsing [%s:%d]:` from regular config file error reporting. --- src/ssl_crtlist.c | 30 +++--- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/ssl_crtlist.c b/src/ssl_crtlist.c index fd141fc50..f1c15e051 100644 --- a/src/ssl_crtlist.c +++ b/src/ssl_crtlist.c @@ -327,8 +327,8 @@ int crtlist_parse_line(char *line, char **crt_path, struct crtlist_entry *entry, /* Check if we reached the limit and the last char is not \n. * Watch out for the last line without the terminating '\n'! */ - memprintf(err, "line %d too long in file '%s', limit is %d characters", - linenum, file, CRT_LINESIZE-1); + memprintf(err, "parsing [%s:%d]: line too longlimit is %d characters", + file, linenum, CRT_LINESIZE-1); cfgerr |= ERR_ALERT | ERR_FATAL; goto error; } @@ -340,12 +340,12 @@ int crtlist_parse_line(char *line, char **crt_path, struct crtlist_entry *entry, *line = 0; } else if (*line == '[') { if (ssl_b) { - memprintf(err, "too many '[' on line %d in file '%s'.", linenum, file); + memprintf(err, "parsing [%s:%d]: too many '['", file, linenum); cfgerr |= ERR_ALERT | ERR_FATAL; goto error; } if (!arg) { - memprintf(err, "file must start with a cert on line %d in file '%s'", linenum, file); + memprintf(err, "parsing [%s:%d]: file must start with a cert", file, linenum); cfgerr |= ERR_ALERT | ERR_FATAL; goto error; } @@ -354,12 +354,12 @@ int crtlist_parse_line(char *line, char **crt_path, struct crtlist_entry *entry, *line = 0; } else if (*line == ']') { if (ssl_e) { - memprintf(err, "too many ']' on line %d in file '%s'.", linenum, file); + memprintf(err, "parsing [%s:%d]: too many ']'", file, linenum); cfgerr |= ERR_ALERT | ERR_FATAL; goto error; } if (!ssl_b) { - memprintf(err, "missing '[' in line %d in file '%s'.", linenum, file); + memprintf(err, "parsing [%s:%d]: missing '['", file, linenum); cfgerr |= ERR_ALERT | ERR_FATAL; goto error; } @@ -368,7 +368,7 @@ int crtlist_parse_line(char *line, char **crt_path, struct crtlist_entry *entry, *line = 0; } else if (newarg) { if (arg == MAX_CRT_ARGS) { - memprintf(err, "too many args on line %d in file '%s'.", linenum, file); + memprintf(err, "parsing [%s:%d]: too many args ", file, linenum); cfgerr |= ERR_ALERT | ERR_FATAL; goto error; } @@ -403,8 +403,8 @@ int crtlist_parse_line(char *line, char **crt_path, struct crtlist_entry *entry, newarg = 1; cfgerr |= ssl_bind_kws[i].parse(args, cur_arg, NULL, ssl_conf, err); if (cur_arg + 1 + ssl_bind_kws[i].skip > ssl_e) { - memprintf(err, "ssl args out of '[]' for %s on line %d in file '%s'", - args[cur_arg], linenum, file); + memprintf(err, "parsing [%s:%d]: ssl args out of '[]' for %s", + file, linenum, args[cur_arg]); cfgerr |= ERR_ALERT | ERR_FATAL; goto error; } @@ -413,8 +413,8 @@ int crtlist_parse_line(char *line, char **crt_path, struct crtlist_entry *entry, } } if (!cfgerr && !newarg) { - memprintf(err, "unknown ssl keyword %s on line %d in file '%s'.", - args[cur_arg], linenum, file); + memprintf(err, "parsing [%s:%d]: unknown ssl keyword %s", + file, linenum, args[cur_arg]); cfgerr |= ERR_ALERT | ERR_FATAL; goto error; } @@ -477,8 +477,8 @@ int crtlist_parse_file(char *file, struct bind_conf *bind_conf, struct
Re: Question regarding the use of the Community Edition for commercial purposes
On Mon, 28 Sep 2020 at 12:15, Tobias Wengenroth wrote: > Dear HAProxy Support Team, > Hi Tobias. Just FYI, this is the public mailing list for the open source project, not a support team :-) > Our customer needs a loadbalancer for their media webservers and we think > about it to use HAProxy. Can we use your community edition of HAProxy for > commercial purposes? > For the avoidance of doubt: I’m not a lawyer, and this is *not* legal advice :-) The open source project is licensed under the GPL, with some being covered by the LGPL, as described here: http://git.haproxy.org/?p=haproxy-2.2.git;a=blob_plain;f=LICENSE;hb=refs/heads/master You should evaluate your use of the software as you would any other licensed project: by reading the license and talking to your lawyers! My personal experience is that I have used it in many commercial projects without issue, and without individualised permissions being sought from anyone. Does your company have a policy on the use of Free / Open Source software; perhaps one which explicitly deals with the (L)GPL? All the best, Jonathan -- Jonathan Matthews https://jpluscplusm.com
Re: [PATCH 2/2] DOC: crt: advise to move away from cert bundle
On Sat, Sep 26, 2020 at 01:35:52PM +0200, William Dauchy wrote: > especially when starting to use `new ssl cert` runtime API, it might > become a bit confusing for users to mix bundle and single cert, > especially when it comes to use the commit command: > e.g.: > - start the process with `crt` loading a bundle > - use `set ssl cert my_cert.pem.ecdsa`: API detects it as a replacement > of a bundle. > - `commit` has to be done on the bundle: `commit ssl cert my_cert.pem` > > however: > - add a new cert: `new ssl cert my_cert.pem.rsa`: added as a single > certificate > - `commit` has to be done on the certificate: `commit ssl cert > my_cert.pem.rsa` > > this should resolve github issue #872 > > this should probably be backported in >= v2.2 in order to encourage > people to move away from bundle certificates loading. > > Signed-off-by: William Dauchy > --- > doc/configuration.txt | 7 ++- > doc/management.txt| 4 > 2 files changed, 10 insertions(+), 1 deletion(-) > > diff --git a/doc/configuration.txt b/doc/configuration.txt > index 97ff2e499..87f35e984 100644 > --- a/doc/configuration.txt > +++ b/doc/configuration.txt > @@ -12560,10 +12560,15 @@ crt >connecting with "ecdsa.example.com" will only be able to use ECDSA cipher >suites. With BoringSSL and Openssl >= 1.1.1 multi-cert is natively > supported, >no need to bundle certificates. ECDSA certificate will be preferred if > client > - support it. > + supports it. > >If a directory name is given as the argument, haproxy will >automatically search and load bundled files in that directory. > + It is however recommended to move away from bundle loading, especially if > you > + want to use the runtime API to load new certificate which does not support > + bundle. A recommended way to migrate is to set `ssl-load-extra-file` > + parameter to `none` in global config so that each certificate is loaded as > a > + single one. > >OSCP files (.ocsp) and issuer files (.issuer) are supported with multi-cert >bundling. Each certificate can have its own .ocsp and .issuer file. At this > diff --git a/doc/management.txt b/doc/management.txt > index adbad95d3..42e8ddbca 100644 > --- a/doc/management.txt > +++ b/doc/management.txt > @@ -1725,6 +1725,10 @@ new ssl cert >Create a new empty SSL certificate store to be filled with a certificate > and >added to a directory or a crt-list. This command should be used in >combination with "set ssl cert" and "add ssl crt-list". > + Note that bundle certificates are not supported; it is recommended to use > + `ssl-load-extra-file none` in global config to avoid loading certificates > as > + bundle and then mixing with single certificates in the runtime API. This > will > + avoid confusion, especailly when it comes to the `commit` command. > > prompt >Toggle the prompt at the beginning of the line and enter or leave > interactive I don't think that's the good approach for 2.3, I replied on the github issue: https://github.com/haproxy/haproxy/issues/872 -- William Lallemand
show errors from stats socket
Hi I'm trying to get details about some errors in one of my backends. Looking at the stats, there are a bunch of errors per server. However, when I try to get details about the errors via the stats socket. # echo "show errors -1" | socat stdio /var/lib/haproxy/stats.sock Total events captured on [28/Sep/2020:12:44:29.832] : 0 I get nothing. Anyone with a clue on what I might be doing wrong? The stats socket has operator level. Thanks Elias