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

2020-09-28 Thread 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, &crt_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

2020-09-28 Thread Tim Duesterhus
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

2020-09-28 Thread Jonathan Matthews
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

2020-09-28 Thread William Lallemand
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

2020-09-28 Thread Elias Abacioglu
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