openssl-3.0 ?

2020-11-26 Thread Илья Шипицин
hi,

stable openssl 3.0 is going to be released soon.

lots of things deprecated, have a look:

https://github.com/chipitsine/haproxy/runs/1461112182


Ilya


[PATCH] BUG/MINOR: dns: SRV records ignores duplicated records

2020-11-26 Thread Baptiste
Hi,

This patch should fix github issue 971. I was not able to reproduce the bug
myself, but the behavior of HAProxy in Hynek's environment makes me think
this is a good candidate.

In short, when a server returns an SRV response with multiple SRV records
sharing the same target field value (the destination hostname) with a
different service port, and also provide the A/ resolution in the
Additional section, then the first occurrence of the hostname is managed by
the Addtional section, while the second (and next) occurrence(s) will be
managed by a designated A/ resolution.
This can lead to a divergence of resolution at some point, and in Hyneck's
case, the server managed via the A/ resolution was not updated.

This patch ensures that every SRV record is linked to its Additional
records, so no divergence could happen anymore.

This should be backported to 2.2 and 2.3.
From b3c9ba9a7bf207c7f648a9885decc2631308850c Mon Sep 17 00:00:00 2001
From: Baptiste Assmann 
Date: Wed, 25 Nov 2020 08:17:59 +0100
Subject: [PATCH] BUG/MINOR: dns: SRV records ignores duplicated AR records

This bug happens when a service has multiple records on the same host
and the server provides the A/ resolution in the response as AR
(Additional Records).

In such condition, the first occurence of the host will be taken from
the Additional section, while the second (and next ones) will be process
by an independent resolution task (like we used to do before 2.2).
This can lead to a situation where the "synchronisation" of the
resolution may diverge, like described in github issue #971.

Because of this behavior, HAProxy mixes various type of requests to
resolve the full list of servers: SRV+AR for all "first" occurences and
A/ for all other occurences of an existing hostname.
IE: with the following type of response:

   ;; ANSWER SECTION:
   _http._tcp.be2.tld. 3600IN  SRV 5 500 80 A2.tld.
   _http._tcp.be2.tld. 3600IN  SRV 5 500 86 A3.tld.
   _http._tcp.be2.tld. 3600IN  SRV 5 500 80 A1.tld.
   _http._tcp.be2.tld. 3600IN  SRV 5 500 85 A3.tld.

   ;; ADDITIONAL SECTION:
   A2.tld. 3600IN  A   192.168.0.2
   A3.tld. 3600IN  A   192.168.0.3
   A1.tld. 3600IN  A   192.168.0.1
   A3.tld. 3600IN  A   192.168.0.3

the first A3 host is resolved using the Additional Section and the
second one through a dedicated A request.

When linking the SRV records to their respective Additional one, a
condition was missing (chek if said SRV record is already attached to an
Additional one), leading to stop processing SRV only when the target
SRV field matches the Additional record name. Hence only the first
occurence of a target was managed by an additional record.
This patch adds a condition in this loop to ensure the record being
parsed is not already linked to an Additional Record. If so, we can
carry on the parsing to find a possible next one with the same target
field value.

backport status: down to 2.2

---
 src/dns.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/dns.c b/src/dns.c
index 3d484263c..63faf1561 100644
--- a/src/dns.c
+++ b/src/dns.c
@@ -1027,6 +1027,7 @@ static int dns_validate_dns_response(unsigned char *resp, unsigned char *bufend,
 dns_answer_record->data_len = len;
 memcpy(dns_answer_record->target, tmpname, len);
 dns_answer_record->target[len] = 0;
+dns_answer_record->ar_item = NULL;
 break;
 
 			case DNS_RTYPE_:
@@ -1276,6 +1277,7 @@ static int dns_validate_dns_response(unsigned char *resp, unsigned char *bufend,
 			// looking for the SRV record in the response list linked to this additional record
 			list_for_each_entry(tmp_record, _p->answer_list, list) {
 if (tmp_record->type == DNS_RTYPE_SRV &&
+tmp_record->ar_item == NULL &&
 !dns_hostname_cmp(tmp_record->target, dns_answer_record->name, tmp_record->data_len)) {
 	/* Always use the received additional record to refresh info */
 	if (tmp_record->ar_item)
-- 
2.25.1



[PATCH] more granular guard for SSL_CTX_add_server_custom_ext

2020-11-26 Thread Илья Шипицин
Hello,

let us continue to improve ssl guarding.

Ilya
From 133c8cc62dea471b306f91cf4e5363b272a80608 Mon Sep 17 00:00:00 2001
From: Ilya Shipitsin 
Date: Fri, 27 Nov 2020 02:39:48 +0500
Subject: [PATCH] BUILD: SSL: fine guard for SSL_CTX_add_server_custom_ext call

SSL_CTX_add_server_custom_ext is openssl specific function present
since openssl-1.0.2, let us define readable guard for it, not depending
on HA_OPENSSL_VERSION
---
 include/haproxy/openssl-compat.h | 4 
 src/ssl_sock.c   | 4 ++--
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/include/haproxy/openssl-compat.h b/include/haproxy/openssl-compat.h
index 42349d7c1..564d1ea78 100644
--- a/include/haproxy/openssl-compat.h
+++ b/include/haproxy/openssl-compat.h
@@ -45,6 +45,10 @@
 #define HAVE_SSL_CTX_SET_CIPHERSUITES
 #endif
 
+#if ((OPENSSL_VERSION_NUMBER >= 0x1000200fL) && !defined(OPENSSL_NO_TLSEXT) && !defined(LIBRESSL_VERSION_NUMBER) && !defined(OPENSSL_IS_BORINGSSL))
+#define HAVE_SL_CTX_ADD_SERVER_CUSTOM_EXT
+#endif
+
 #if (HA_OPENSSL_VERSION_NUMBER < 0x0090800fL)
 /* Functions present in OpenSSL 0.9.8, older not tested */
 static inline const unsigned char *SSL_SESSION_get_id(const SSL_SESSION *sess, unsigned int *sid_length)
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index b7d3b92b4..2b0fa5014 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -1495,7 +1495,7 @@ static int ssl_sock_load_ocsp(SSL_CTX *ctx, const struct cert_key_and_chain *ckc
 #endif
 
 
-#if (HA_OPENSSL_VERSION_NUMBER >= 0x1000200fL && !defined OPENSSL_NO_TLSEXT && !defined OPENSSL_IS_BORINGSSL)
+#ifdef HAVE_SL_CTX_ADD_SERVER_CUSTOM_EXT
 
 #define CT_EXTENSION_TYPE 18
 
@@ -3168,7 +3168,7 @@ static int ssl_sock_put_ckch_into_ctx(const char *path, const struct cert_key_an
 	}
 #endif
 
-#if (HA_OPENSSL_VERSION_NUMBER >= 0x1000200fL && !defined OPENSSL_NO_TLSEXT && !defined OPENSSL_IS_BORINGSSL)
+#ifdef HAVE_SL_CTX_ADD_SERVER_CUSTOM_EXT
 	if (sctl_ex_index >= 0 && ckch->sctl) {
 		if (ssl_sock_load_sctl(ctx, ckch->sctl) < 0) {
 			memprintf(err, "%s '%s.sctl' is present but cannot be read or parsed'.\n",
-- 
2.28.0



Re: [PATCH] CI: partially enable 51 degrees, update libressl to 3.3.0

2020-11-26 Thread Willy Tarreau
On Thu, Nov 26, 2020 at 06:14:34PM +0100, Tim Düsterhus wrote:
> Ilya,
> 
> Am 26.11.20 um 17:08 schrieb  ???:
> > this partially enables 51 degrees (only "pattern" flavour).
> > libressl updated to 3.3.0
> > 
> 
> Oh, *that* explains why I couldn't get 51d to work. The Travis
> configuration contains:
> 
> 51DEGREES_SRC=$FIFTYONEDEGREES_SRC
> 
> and I did not notice that it was different names there :-/
> 
> Willy: These two patches look good to me.

OK now applied, thank you!

Willy



Re: [PATCH] DOC: Clarify %HP description in log-format

2020-11-26 Thread Willy Tarreau
Hi Maciej,

On Thu, Nov 26, 2020 at 12:39:19PM +0100, Maciej Zdeb wrote:
> Hi,
> 
> Since 30ee1efe676e8264af16bab833c621d60a72a4d7 "MEDIUM: h2: use the
> normalized URI encoding for absolute form requests" we're using absolute
> URI when possible. As stated in the commit message this also changed the
> way it is reported in logs, especially for H2.
> 
> Docs describes %HP as "HTTP request URI without query string (path)" and
> the word "path" suggests that %HP behaves exactly the same as "path" sample
> fetch (it surprised me after HAProxy update that it's not true). Shouldn't
> we delete the "(path)" from that description? Even better would be to
> change it to "rel_path/abs_path" like it is in rfc or "relative/absolute
> URI" but it is too long. :) If I'm the only one that got confused then it
> may stay. But...
> 
> Do you think that it might be useful to implement another var, let's say
> %HRU described as "HTTP request relative URI"? :) Usually in my logs I'm
> not interested in absolute uri and relative is sufficient.

Few people play with absolute URIs in practice, that's why such issues
started to pop up very late. Your example above is just yet another proof
of this. So you're probalbly not the only one, but the few others might
have silently fixed the meaning in their mind while you've fixed it in the
doc, which is better!

Applied now, thank you!
Willy



Re: [PATCH] CI: partially enable 51 degrees, update libressl to 3.3.0

2020-11-26 Thread Илья Шипицин
in low priority improvements queue I would add "sometime add 'trie' builds
as well"

чт, 26 нояб. 2020 г. в 22:14, Tim Düsterhus :

> Ilya,
>
> Am 26.11.20 um 17:08 schrieb Илья Шипицин:
> > this partially enables 51 degrees (only "pattern" flavour).
> > libressl updated to 3.3.0
> >
>
> Oh, *that* explains why I couldn't get 51d to work. The Travis
> configuration contains:
>
> 51DEGREES_SRC=$FIFTYONEDEGREES_SRC
>
> and I did not notice that it was different names there :-/
>
> Willy: These two patches look good to me.
>
> Thanks.
> Tim Düsterhus
>


Re: [PATCH] CI: partially enable 51 degrees, update libressl to 3.3.0

2020-11-26 Thread Tim Düsterhus
Ilya,

Am 26.11.20 um 17:08 schrieb Илья Шипицин:
> this partially enables 51 degrees (only "pattern" flavour).
> libressl updated to 3.3.0
> 

Oh, *that* explains why I couldn't get 51d to work. The Travis
configuration contains:

51DEGREES_SRC=$FIFTYONEDEGREES_SRC

and I did not notice that it was different names there :-/

Willy: These two patches look good to me.

Thanks.
Tim Düsterhus



[PATCH] CI: partially enable 51 degrees, update libressl to 3.3.0

2020-11-26 Thread Илья Шипицин
Hi,

this partially enables 51 degrees (only "pattern" flavour).
libressl updated to 3.3.0

Ilya
From 88c5672da982ee37505631bd5224ca29ee5a6339 Mon Sep 17 00:00:00 2001
From: Ilya Shipitsin 
Date: Thu, 26 Nov 2020 20:56:51 +0500
Subject: [PATCH 1/2] CI: github actions: update LibreSSL to 3.3.0

---
 .github/matrix.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.github/matrix.py b/.github/matrix.py
index 406218d0a..148dcde2d 100644
--- a/.github/matrix.py
+++ b/.github/matrix.py
@@ -105,7 +105,7 @@ for CC in ["gcc", "clang"]:
 "stock",
 "OPENSSL_VERSION=1.0.2u",
 "LIBRESSL_VERSION=2.9.2",
-"LIBRESSL_VERSION=3.1.1",
+"LIBRESSL_VERSION=3.3.0",
 "BORINGSSL=yes",
 ]:
 flags = ["USE_OPENSSL=1"]
-- 
2.28.0

From acef7af02a03d9503c52d51e03116960807c14e9 Mon Sep 17 00:00:00 2001
From: Ilya Shipitsin 
Date: Thu, 26 Nov 2020 20:59:42 +0500
Subject: [PATCH 2/2] CI: github actions: enable 51degrees feature

---
 .github/matrix.py | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/.github/matrix.py b/.github/matrix.py
index 148dcde2d..a5ce1919a 100644
--- a/.github/matrix.py
+++ b/.github/matrix.py
@@ -82,8 +82,8 @@ for CC in ["gcc", "clang"]:
 "USE_DEVICEATLAS=1",
 "DEVICEATLAS_SRC=contrib/deviceatlas",
 "EXTRA_OBJS=contrib/prometheus-exporter/service-prometheus.o",
-# "USE_51DEGREES=1",
-# "FIFTYONEDEGREES_SRC=contrib/51d/src/pattern",
+"USE_51DEGREES=1",
+"51DEGREES_SRC=contrib/51d/src/pattern",
 ],
 }
 )
@@ -148,8 +148,8 @@ matrix.append(
 "USE_DEVICEATLAS=1",
 "DEVICEATLAS_SRC=contrib/deviceatlas",
 "EXTRA_OBJS=contrib/prometheus-exporter/service-prometheus.o",
-# "USE_51DEGREES=1",
-# "FIFTYONEDEGREES_SRC=contrib/51d/src/pattern",
+"USE_51DEGREES=1",
+"51DEGREES_SRC=contrib/51d/src/pattern",
 ],
 }
 )
-- 
2.28.0



Re: [ANNOUNCE] haproxy-2.4-dev1

2020-11-26 Thread Baptiste
Hi,

Cool release and another +1 for the backport of the "del-header -m".

Baptiste


[PATCH] DOC: Clarify %HP description in log-format

2020-11-26 Thread Maciej Zdeb
Hi,

Since 30ee1efe676e8264af16bab833c621d60a72a4d7 "MEDIUM: h2: use the
normalized URI encoding for absolute form requests" we're using absolute
URI when possible. As stated in the commit message this also changed the
way it is reported in logs, especially for H2.

Docs describes %HP as "HTTP request URI without query string (path)" and
the word "path" suggests that %HP behaves exactly the same as "path" sample
fetch (it surprised me after HAProxy update that it's not true). Shouldn't
we delete the "(path)" from that description? Even better would be to
change it to "rel_path/abs_path" like it is in rfc or "relative/absolute
URI" but it is too long. :) If I'm the only one that got confused then it
may stay. But...

Do you think that it might be useful to implement another var, let's say
%HRU described as "HTTP request relative URI"? :) Usually in my logs I'm
not interested in absolute uri and relative is sufficient.

Kind regards,
Maciej


0001-DOC-Clarify-HP-description-in-log-format.patch
Description: Binary data