Re: Broken SNI with crt-list for HAProxy 2.1.x after upgrade from Stretch to Buster
On Mon, Jun 15, 2020 at 03:48:40PM +0200, Tim Düsterhus wrote: > William, > > Am 15.06.20 um 14:56 schrieb William Lallemand: > > I think I found the problem, could you try the attached patch for 2.1? > > > > I'd prefer not, because I don't have a staging system where I could > easily reproduce the issue (and generating SSL certs to test this > properly is annoying). I was encountering the issue on a production box > and I don't like to mess around with manually compiled HAProxy versions > more than necessary. > No problem, I was able to reproduce the problem and confirmed that the crt-list is entirely parsed after a warning now. Thanks for the report, -- William Lallemand
Re: Broken SNI with crt-list for HAProxy 2.1.x after upgrade from Stretch to Buster
William, Am 15.06.20 um 14:56 schrieb William Lallemand: > I think I found the problem, could you try the attached patch for 2.1? > I'd prefer not, because I don't have a staging system where I could easily reproduce the issue (and generating SSL certs to test this properly is annoying). I was encountering the issue on a production box and I don't like to mess around with manually compiled HAProxy versions more than necessary. For me the issue is sufficiently resolved by specifying the DH bit size (and the fact that I plan to upgrade to 2.2 as soon as packages are available). I trust you that the patch fixes *a* bug, even if it might not be *my* bug, thus feel free to apply. Best regards Tim Düsterhus
Re: Broken SNI with crt-list for HAProxy 2.1.x after upgrade from Stretch to Buster
On Sat, Jun 13, 2020 at 04:55:53PM +0200, Tim Düsterhus wrote:
> William,
>
> Am 13.06.20 um 16:46 schrieb Tim Düsterhus:
> > tune.ssl.default-dh-param 2048 solved the issue for me.
> >
> > I'd argue that this is a bug in HAProxy nonetheless, because apparently
> > the crt-list file is not fully parsed in case of DH parameter warnings
> > (not errors). In fact I can remember that some similar issue was
> > previously fixed.
> >
>
> Could this be another version of this issue:
> https://github.com/haproxy/haproxy/issues/483? Should I file a bug report?
>
> Best regards
> Tim Düsterhus
Hello Tim,
I think I found the problem, could you try the attached patch for 2.1?
Thanks,
--
William Lallemand
>From 671197ebf116b053169d6a2ec27ded0b2d090f93 Mon Sep 17 00:00:00 2001
From: William Lallemand
Date: Mon, 15 Jun 2020 14:37:19 +0200
Subject: [PATCH] BUG/MINOR: ssl: crt-list should continue parsing on ERR_WARN
The original crt-list parsing was using any value in the cfgerr variable
as an error. This is wrong since the certificate loading could return
an ERR_WARN and should be able to be parsed. The parsing must be only
stopped on an ERR_CODE.
This commit is 2.1 only since it was fixed
in 2.2 by commit 2954c47 ("MEDIUM: ssl: allow crt-list caching")
and accidently in 2.0 by commit b131c87 ("CLEANUP: ssl: make
ssl_sock_load_cert*() return real error codes").
---
src/ssl_sock.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index adf06dd7a..574cd15dd 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -4364,7 +4364,7 @@ int ssl_sock_load_cert_list_file(char *file, struct bind_conf *bind_conf, struct
}
line++;
}
- if (cfgerr)
+ if (cfgerr & ERR_CODE)
break;
args[arg++] = line;
@@ -4409,7 +4409,7 @@ int ssl_sock_load_cert_list_file(char *file, struct bind_conf *bind_conf, struct
}
}
- if (cfgerr) {
+ if (cfgerr & ERR_CODE) {
ssl_sock_free_ssl_conf(ssl_conf);
free(ssl_conf);
ssl_conf = NULL;
@@ -4428,7 +4428,7 @@ int ssl_sock_load_cert_list_file(char *file, struct bind_conf *bind_conf, struct
else
cfgerr |= ssl_sock_load_ckchs(crt_path, ckchs, bind_conf, ssl_conf, [cur_arg], arg - cur_arg - 1, err);
- if (cfgerr) {
+ if (cfgerr & ERR_CODE) {
memprintf(err, "error processing line %d in file '%s' : %s", linenum, file, *err);
break;
}
--
2.25.3
Re: Broken SNI with crt-list for HAProxy 2.1.x after upgrade from Stretch to Buster
William, Am 13.06.20 um 16:46 schrieb Tim Düsterhus: > tune.ssl.default-dh-param 2048 solved the issue for me. > > I'd argue that this is a bug in HAProxy nonetheless, because apparently > the crt-list file is not fully parsed in case of DH parameter warnings > (not errors). In fact I can remember that some similar issue was > previously fixed. > Could this be another version of this issue: https://github.com/haproxy/haproxy/issues/483? Should I file a bug report? Best regards Tim Düsterhus
Re: Broken SNI with crt-list for HAProxy 2.1.x after upgrade from Stretch to Buster
Dear List, Am 13.06.20 um 16:11 schrieb Tim Düsterhus: > Any ideas? > Looking at the startup warnings is always a good idea: > Jun 13 14:40:52 *snip* haproxy[15815]: [WARNING] 164/144052 (15815) : > Reexecuting Master process > Jun 13 14:40:52 *snip* haproxy[15815]: [WARNING] 164/144052 (15815) : parsing > [/usr/share/haproxy//004-http.cfg:96] : 'bind :::443' : > Jun 13 14:40:52 *snip* haproxy[15815]: 'crt-list' : error processing line 1 > in file '/usr/share/haproxy/crt-list' : unable to load default 1024 bits DH > parameter for certificate '/usr/share/haproxy/ssl/*snip* > Jun 13 14:40:52 *snip* haproxy[15815]: , SSL library will use an > automatically generated DH parameter. > Jun 13 14:40:52 *snip* haproxy[15815]: [WARNING] 164/144052 (15815) : parsing > [/usr/share/haproxy//010-*snip*.cfg:7] : 'bind :::993' : > Jun 13 14:40:52 *snip* haproxy[15815]: unable to load default 1024 bits DH > parameter for certificate '/usr/share/haproxy/ssl/*snip*'. > Jun 13 14:40:52 *snip* haproxy[15815]: , SSL library will use an > automatically generated DH parameter. > Jun 13 14:40:52 *snip* haproxy[15815]: [WARNING] 164/144052 (15815) : parsing > [/usr/share/haproxy//030-*snip*.cfg:14] : 'bind :::6' : > Jun 13 14:40:52 *snip* haproxy[15815]: unable to load default 1024 bits DH > parameter for certificate '/usr/share/haproxy/ssl/*snip*'. > Jun 13 14:40:52 *snip* haproxy[15815]: , SSL library will use an > automatically generated DH parameter. > Jun 13 14:40:52 *snip* haproxy[15815]: [WARNING] 164/144052 (15815) : parsing > [/usr/share/haproxy//999-stats.cfg:2] : 'bind *:1936' : > Jun 13 14:40:52 *snip* haproxy[15815]: 'crt-list' : error processing line 1 > in file '/usr/share/haproxy/crt-list' : unable to load default 1024 bits DH > parameter for certificate '/usr/share/haproxy/ssl/*snip* > Jun 13 14:40:52 *snip* haproxy[15815]: , SSL library will use an > automatically generated DH parameter. tune.ssl.default-dh-param 2048 solved the issue for me. I'd argue that this is a bug in HAProxy nonetheless, because apparently the crt-list file is not fully parsed in case of DH parameter warnings (not errors). In fact I can remember that some similar issue was previously fixed. Best regards Tim Düsterhus
