Re: dns fails to process response / hold valid? (since commit 2.2-dev0-13a9232)

[PiBa-NL]

Hi Baptiste,

Op 19-2-2020 om 13:06 schreef Baptiste:

Hi,

I found a couple of bugs in that part of the code.
Can you please try the attached patch? (0001 is useless but I share it 
too in case of)

Works for me, thanks!
It will allow parsing of additional records for SRV queries only and 
when done, will silently ignore any record which are not A or .


@maint team, please don't apply the patch yet, I want to test it much 
more before.



When the final patch is ready ill be happy to give it a try as well.

Baptiste

On a side note. With config below i would expect 2 servers with status 
'MAINT(resolving)'.


Using this configuration in Unbound (4 server IP's defined.):
server:
local-data: "_https._tcp.pkg.test.tld 3600 IN SRV 0 100 80 srv1.test.tld"
local-data: "_https._tcp.pkg.test.tld 3600 IN SRV 0 100 80 srv2.test.tld"
local-data: "srv1.test.tld 3600 IN A 192.168.0.51"
local-data: "srv2.test.tld 3600 IN A 192.168.0.52"
local-data: "srvX.test.tld 3600 IN A 192.168.0.53"
local-data: "srvX.test.tld 3600 IN A 192.168.0.54"

And this in a HAProxy backend:
    server-template            PB_SRVrecords 3 
ipv4@_https._tcp.pkg.test.tld:77 id 10110 check inter 18 resolvers 
globalresolvers resolve-prefer ipv4
    server-template            PB_multipleA 3 i...@srvx.test.tld:78 id 
10111 check inter 18  resolvers globalresolvers resolve-prefer 
ipv4Results in 6 servers, but 1 is


This results in 6 servers of which 1 server has 'MAINT(resolution)' 
status and 1 has an IP of 0.0.0.0 but shows as 'DOWN'. I would have 
expected 2 servers with status MAINT.?
(p.s. none of the IP's actually exist on my network so that the other 
servers are also shown as down is correct..)


PB_ipv4,PB_SRVrecords1,0,0,0,0,,0,0,0,,0,,0,0,0,0,DOWN,1,1,0,1,1,124,124,,1,10102,10110,,0,,2,0,,0,L4CON,,74995,0,0,0,0,0,0,0,0,-1,,,0,0,0,0Layer4 
connection 
problem,,2,3,0192.168.0.51:80,,http0,0,0,,,0,,0,0,0,0,0,
PB_ipv4,PB_SRVrecords2,0,0,0,0,,0,0,0,,0,,0,0,0,0,DOWN,1,1,0,1,1,94,94,,1,10102,2,,0,,2,0,,0,L4CON,,75029,0,0,0,0,0,0,0,0,-1,,,0,0,0,0Layer4 
connection 
problem,,2,3,0192.168.0.52:80,,http0,0,0,,,0,,0,0,0,0,0,
PB_ipv4,PB_SRVrecords3,0,0,0,0,,0,0,0,,0,,0,0,0,0,DOWN,1,1,0,1,1,64,64,,1,10102,3,,0,,2,0,,0,L4CON,,75039,0,0,0,0,0,0,0,0,-1,,,0,0,0,0Layer4 
connection problem,,2,3,00.0.0.0:77,,http0,0,0,,,0,,0,0,0,0,0,
PB_ipv4,PB_multipleA1,0,0,0,0,,0,0,0,,0,,0,0,0,0,DOWN,1,1,0,1,2,34,34,,1,10102,10111,,0,,2,0,,0,L4CON,,75002,0,0,0,0,0,0,0,0,-1,,,0,0,0,0Layer4 
connection 
problem,,2,3,0192.168.0.53:78,,http0,0,0,,,0,,0,0,0,0,0,
PB_ipv4,PB_multipleA2,0,0,0,0,,0,0,0,,0,,0,0,0,0,DOWN,1,1,0,1,2,4,4,,1,10102,5,,0,,2,0,,0,L4CON,,75014,0,0,0,0,0,0,0,0,-1,,,0,0,0,0Layer4 
connection 
problem,,2,3,0192.168.0.54:78,,http0,0,0,,,0,,0,0,0,0,0,
PB_ipv4,PB_multipleA3,0,0,0,0,,0,0,0,,0,,0,0,0,0,MAINT 
(resolution),1,1,0,0,1,199,199,,1,10102,6,,0,,2,0,,00,0,0,0,0,0,0,0,-1,,,0,0,0,00.0.0.0:78,,http0,0,0,,,0,,0,0,0,0,0,


If additional info is desired, please let me know :).

On Tue, Feb 18, 2020 at 2:03 PM Baptiste > wrote:


Hi guys,

Thx Tim for investigating.
I'll check the PCAP and see why such behavior happens.

Baptiste


On Tue, Feb 18, 2020 at 12:09 AM Tim Düsterhus mailto:t...@bastelstu.be>> wrote:

Pieter,

Am 09.02.20 um 15:35 schrieb PiBa-NL:
> Before commit '2.2-dev0-13a9232, released 2020/01/22 (use
additional
> records from SRV responses)' i get seemingly proper working
resolving of
> server a name.
> After this commit all responses are counted as 'invalid' in
the socket
> stats.

I can confirm the issue with the provided configuration. The
'if (len ==
0) {' check in line 1045 of the commit causes HAProxy to
consider the
responses 'invalid':


Thanks for confirming :).




https://github.com/haproxy/haproxy/commit/13a9232ebc63fdf357ffcf4fa7a1a5e77a1eac2b#diff-b2ddf457bc423779995466f7d8b9d147R1045-R1048

Best regards
Tim Düsterhus


Regards,
PiBa-NL (Pieter)

Re: dns fails to process response / hold valid? (since commit 2.2-dev0-13a9232)

[Baptiste]

Hi,

I found a couple of bugs in that part of the code.
Can you please try the attached patch? (0001 is useless but I share it too
in case of)
It will allow parsing of additional records for SRV queries only and when
done, will silently ignore any record which are not A or .

@maint team, please don't apply the patch yet, I want to test it much more
before.

Baptiste


On Tue, Feb 18, 2020 at 2:03 PM Baptiste  wrote:

> Hi guys,
>
> Thx Tim for investigating.
> I'll check the PCAP and see why such behavior happens.
>
> Baptiste
>
>
> On Tue, Feb 18, 2020 at 12:09 AM Tim Düsterhus  wrote:
>
>> Pieter,
>>
>> Am 09.02.20 um 15:35 schrieb PiBa-NL:
>> > Before commit '2.2-dev0-13a9232, released 2020/01/22 (use additional
>> > records from SRV responses)' i get seemingly proper working resolving of
>> > server a name.
>> > After this commit all responses are counted as 'invalid' in the socket
>> > stats.
>>
>> I can confirm the issue with the provided configuration. The 'if (len ==
>> 0) {' check in line 1045 of the commit causes HAProxy to consider the
>> responses 'invalid':
>>
>>
>> https://github.com/haproxy/haproxy/commit/13a9232ebc63fdf357ffcf4fa7a1a5e77a1eac2b#diff-b2ddf457bc423779995466f7d8b9d147R1045-R1048
>>
>> Best regards
>> Tim Düsterhus
>>
>
From fa0b9563c40006be83c3fa1b52eeb3dbbb1b028b Mon Sep 17 00:00:00 2001
From: Baptiste Assmann 
Date: Wed, 19 Feb 2020 00:53:26 +0100
Subject: [PATCH 1/2] CLEANUP: remove obsolete comments

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

diff --git a/src/dns.c b/src/dns.c
index 86147a417..9e49babf1 100644
--- a/src/dns.c
+++ b/src/dns.c
@@ -1030,7 +1030,6 @@ static int dns_validate_dns_response(unsigned char *resp, unsigned char *bufend,
 
 	/* now parsing additional records */
 	nb_saved_records = 0;
-	//TODO: check with Dinko for DNS poisoning
 	for (i = 0; i < dns_p->header.arcount; i++) {
 		if (reader >= bufend)
 			return DNS_RESP_INVALID;
@@ -1202,7 +1201,6 @@ static int dns_validate_dns_response(unsigned char *resp, unsigned char *bufend,
 	continue;
 tmp_record->ar_item = dns_answer_record;
 			}
-			//TODO: there is a leak for now, since we don't clean up AR records
 
 			LIST_ADDQ(_p->ar_list, _answer_record->list);
 		}
-- 
2.17.1

From 96a09ab7538af2644c7247be2313fc0cc294949b Mon Sep 17 00:00:00 2001
From: Baptiste Assmann 
Date: Wed, 19 Feb 2020 01:08:51 +0100
Subject: [PATCH 2/2] BUG/MEDIUM: dns: improper parsing of aditional records

---
 src/dns.c | 26 ++
 1 file changed, 6 insertions(+), 20 deletions(-)

diff --git a/src/dns.c b/src/dns.c
index 9e49babf1..5550ab976 100644
--- a/src/dns.c
+++ b/src/dns.c
@@ -1028,7 +1028,9 @@ static int dns_validate_dns_response(unsigned char *resp, unsigned char *bufend,
 	/* Save the number of records we really own */
 	dns_p->header.ancount = nb_saved_records;
 
-	/* now parsing additional records */
+	/* now parsing additional records for SRV queries only */
+	if (dns_query->type != DNS_RTYPE_SRV)
+		goto skip_parsing_additional_records;
 	nb_saved_records = 0;
 	for (i = 0; i < dns_p->header.arcount; i++) {
 		if (reader >= bufend)
@@ -1043,25 +1045,7 @@ static int dns_validate_dns_response(unsigned char *resp, unsigned char *bufend,
 
 		if (len == 0) {
 			pool_free(dns_answer_item_pool, dns_answer_record);
-			return DNS_RESP_INVALID;
-		}
-
-		/* Check if the current record dname is valid.  previous_dname
-		 * points either to queried dname or last CNAME target */
-		if (dns_query->type != DNS_RTYPE_SRV && memcmp(previous_dname, tmpname, len) != 0) {
-			pool_free(dns_answer_item_pool, dns_answer_record);
-			if (i == 0) {
-/* First record, means a mismatch issue between
- * queried dname and dname found in the first
- * record */
-return DNS_RESP_INVALID;
-			}
-			else {
-/* If not the first record, this means we have a
- * CNAME resolution error */
-return DNS_RESP_CNAME_ERROR;
-			}
-
+			continue;
 		}
 
 		memcpy(dns_answer_record->name, tmpname, len);
@@ -1206,6 +1190,8 @@ static int dns_validate_dns_response(unsigned char *resp, unsigned char *bufend,
 		}
 	} /* for i 0 to arcount */
 
+ skip_parsing_additional_records:
+
 	/* Save the number of records we really own */
 	dns_p->header.arcount = nb_saved_records;
 
-- 
2.17.1

Re: dns fails to process response / hold valid? (since commit 2.2-dev0-13a9232)

[Baptiste]

Hi guys,

Thx Tim for investigating.
I'll check the PCAP and see why such behavior happens.

Baptiste


On Tue, Feb 18, 2020 at 12:09 AM Tim Düsterhus  wrote:

> Pieter,
>
> Am 09.02.20 um 15:35 schrieb PiBa-NL:
> > Before commit '2.2-dev0-13a9232, released 2020/01/22 (use additional
> > records from SRV responses)' i get seemingly proper working resolving of
> > server a name.
> > After this commit all responses are counted as 'invalid' in the socket
> > stats.
>
> I can confirm the issue with the provided configuration. The 'if (len ==
> 0) {' check in line 1045 of the commit causes HAProxy to consider the
> responses 'invalid':
>
>
> https://github.com/haproxy/haproxy/commit/13a9232ebc63fdf357ffcf4fa7a1a5e77a1eac2b#diff-b2ddf457bc423779995466f7d8b9d147R1045-R1048
>
> Best regards
> Tim Düsterhus
>

Re: dns fails to process response / hold valid? (since commit 2.2-dev0-13a9232)

[Tim Düsterhus]

Pieter,

Am 09.02.20 um 15:35 schrieb PiBa-NL:
> Before commit '2.2-dev0-13a9232, released 2020/01/22 (use additional
> records from SRV responses)' i get seemingly proper working resolving of
> server a name.
> After this commit all responses are counted as 'invalid' in the socket
> stats.

I can confirm the issue with the provided configuration. The 'if (len ==
0) {' check in line 1045 of the commit causes HAProxy to consider the
responses 'invalid':

https://github.com/haproxy/haproxy/commit/13a9232ebc63fdf357ffcf4fa7a1a5e77a1eac2b#diff-b2ddf457bc423779995466f7d8b9d147R1045-R1048

Best regards
Tim Düsterhus

Re: dns fails to process response / hold valid? (since commit 2.2-dev0-13a9232)

[Tim Düsterhus]

Pieter,

Am 17.02.20 um 22:14 schrieb PiBa-NL:
> (maybe the pcap attachment didn't fly well through spam filters. (or the
> email formatting..)?)

I received your first email perfectly fine.

> If someone was already planning to, please don't feel 'pushed' by this
> mail. i'm just trying to make sure this doesn't fall through the cracks :).

I can't comment on the issue you are seeing, but consider filing an
issue in the tracker if you don't get a reply on this "push" either:
https://github.com/haproxy/haproxy/issues

Other users have reported issues with 2.2-dev before.

Best regards
Tim Düsterhus

Re: dns fails to process response / hold valid? (since commit 2.2-dev0-13a9232)

[PiBa-NL]

Hi List,
Hereby a little bump. Can someone take a look?
(maybe the pcap attachment didn't fly well through spam filters. (or the 
email formatting..)?)
(or because i (wrongly?) chose to include Baptiste specifically in my 
addressing (he committed the original patch that caused the change in 
behaviour)..)


Anyhow the current '2.2-dev2-a71667c, released 2020/02/17' is still 
affected.


If someone was already planning to, please don't feel 'pushed' by this 
mail. i'm just trying to make sure this doesn't fall through the cracks :).

Regards,
PiBa-NL (Pieter)

Op 9-2-2020 om 15:35 schreef PiBa-NL:

Hi List, Baptiste,

After updating haproxy i found that the DNS resolver is no longer 
working for me. Also i wonder about the exact effect that 'hold valid' 
should have.
I pointed haproxy to a 'Unbound 1.9.4' dns server that does the 
recursive resolving of the dns request made by haproxy.


Before commit '2.2-dev0-13a9232, released 2020/01/22 (use additional 
records from SRV responses)' i get seemingly proper working resolving 
of server a name.
After this commit all responses are counted as 'invalid' in the socket 
stats.


Attached also a pcap of the dns traffic. Which shows a short capture 
of a single attempt where 3 retries for both A and  records show 
up. There is a additional record of type 'OPT' is present in the 
response.. But the exact same keeps repeating every 5 seconds.
As for 'hold valid' (tested with the commit before this one) it seems 
that the stats page of haproxy shows the server in 'resolution' status 
way before the 3 minute 'hold valid' has passed when i simply 
disconnect the network of the server running the Unbound-DNS server. 
Though i guess that is less important that dns working at all in the 
first place..


If any additional information is needed please let me know :).

Can you/someone take a look? Thanks in advance.

p.s. i think i read something about a 'vtest' that can test the 
haproxy DNS functionality, if you have a example that does this i 
would be happy to provide a vtest with a reproduction of the issue 
though i guess it will be kinda 'slow' if it needs to test for hold 
valid timings..


Regards,
PiBa-NL (Pieter)

 haproxy config:

resolvers globalresolvers
    nameserver pfs_routerbox 192.168.0.18:53
    resolve_retries 3
    timeout retry 200
    hold valid 3m
    hold nx 10s
    hold other 15s
    hold refused 20s
    hold timeout 25s
    hold obsolete 30s
    timeout resolve 5s

frontend nu_nl
    bind            192.168.0.19:433 name 192.168.0.19:433   ssl 
crt-list /var/etc/haproxy/nu_nl.crt_list

    mode            http
    log            global
    option            http-keep-alive
    timeout client        3
    use_backend nu.nl_ipvANY

backend nu.nl_ipvANY
    mode            http
    id            2113
    log            global
    timeout connect        3
    timeout server        3
    retries            3
    option            httpchk GET / HTTP/1.0\r\nHost:\ 
nu.nl\r\nAccept:\ */*
    server            nu_nl nu.nl:443 id 2114 ssl check inter 1  
verify none resolvers globalresolvers check-sni nu.nl resolve-prefer ipv4



 haproxy_socket.sh show resolvers
Resolvers section globalresolvers
 nameserver pfs_routerbox:
  sent:    216
  snd_error:   0
  valid:   0
  update:  0
  cname:   0
  cname_error: 0
  any_err: 108
  nx:  0
  timeout: 0
  refused: 0
  other:   0
  invalid: 108
  too_big: 0
  truncated:   0
  outdated:    0

 haproxy -vv
HA-Proxy version 2.2-dev0-13a9232 2020/01/22 - https://haproxy.org/
Status: development branch - not safe for use in production.
Known bugs: https://github.com/haproxy/haproxy/issues?q=is:issue+is:open
Build options :
  TARGET  = freebsd
  CPU = generic
  CC  = cc
  CFLAGS  = -pipe -g -fstack-protector -fno-strict-aliasing 
-fno-strict-aliasing -Wdeclaration-after-statement -fwrapv 
-fno-strict-overflow -Wno-null-dereference -Wno-unused-label 
-Wno-unused-parameter -Wno-sign-compare -Wno-ignored-qualifiers 
-Wno-unused-command-line-argument -Wno-missing-field-initializers 
-Wno-address-of-packed-member -DFREEBSD_PORTS -DFREEBSD_PORTS
  OPTIONS = USE_PCRE=1 USE_PCRE_JIT=1 USE_REGPARM=1 USE_STATIC_PCRE=1 
USE_GETADDRINFO=1 USE_OPENSSL=1 USE_LUA=1 USE_ACCEPT4=1 USE_ZLIB=1


Feature list : -EPOLL +KQUEUE -MY_EPOLL -MY_SPLICE -NETFILTER +PCRE 
+PCRE_JIT -PCRE2 -PCRE2_JIT +POLL -PRIVATE_CACHE +THREAD 
-PTHREAD_PSHARED +REGPARM +STATIC_PCRE -STATIC_PCRE2 +TPROXY 
-LINUX_TPROXY -LINUX_SPLICE +LIBCRYPT -CRYPT_H -VSYSCALL +GETADDRINFO 
+OPENSSL +LUA -FUTEX +ACCEPT4 -MY_ACCEPT4 +ZLIB -SLZ +CPU_AFFINITY 
-TFO -NS -DL -RT -DEVICEATLAS -51DEGREES -WURFL -SYSTEMD 
-OBSOLETE_LINKER -PRCTL -THREAD_DUMP -EVPORTS


Default settings :
  bufsize = 16384, maxrewrite = 1024, maxpollevents = 200

Built with multi-threading support (MAX_THREADS=64, default=2).
Built with OpenSSL version : OpenSSL 1.1.1a-freebsd  20 Nov 2018
Running on OpenSSL version 

dns fails to process response / hold valid? (since commit 2.2-dev0-13a9232)

[PiBa-NL]

Hi List, Baptiste,

After updating haproxy i found that the DNS resolver is no longer 
working for me. Also i wonder about the exact effect that 'hold valid' 
should have.
I pointed haproxy to a 'Unbound 1.9.4' dns server that does the 
recursive resolving of the dns request made by haproxy.


Before commit '2.2-dev0-13a9232, released 2020/01/22 (use additional 
records from SRV responses)' i get seemingly proper working resolving of 
server a name.
After this commit all responses are counted as 'invalid' in the socket 
stats.


Attached also a pcap of the dns traffic. Which shows a short capture of 
a single attempt where 3 retries for both A and  records show up. 
There is a additional record of type 'OPT' is present in the response.. 
But the exact same keeps repeating every 5 seconds.
As for 'hold valid' (tested with the commit before this one) it seems 
that the stats page of haproxy shows the server in 'resolution' status 
way before the 3 minute 'hold valid' has passed when i simply disconnect 
the network of the server running the Unbound-DNS server. Though i guess 
that is less important that dns working at all in the first place..


If any additional information is needed please let me know :).

Can you/someone take a look? Thanks in advance.

p.s. i think i read something about a 'vtest' that can test the haproxy 
DNS functionality, if you have a example that does this i would be happy 
to provide a vtest with a reproduction of the issue though i guess it 
will be kinda 'slow' if it needs to test for hold valid timings..


Regards,
PiBa-NL (Pieter)

 haproxy config:

resolvers globalresolvers
    nameserver pfs_routerbox 192.168.0.18:53
    resolve_retries 3
    timeout retry 200
    hold valid 3m
    hold nx 10s
    hold other 15s
    hold refused 20s
    hold timeout 25s
    hold obsolete 30s
    timeout resolve 5s

frontend nu_nl
    bind            192.168.0.19:433 name 192.168.0.19:433   ssl 
crt-list /var/etc/haproxy/nu_nl.crt_list

    mode            http
    log            global
    option            http-keep-alive
    timeout client        3
    use_backend nu.nl_ipvANY

backend nu.nl_ipvANY
    mode            http
    id            2113
    log            global
    timeout connect        3
    timeout server        3
    retries            3
    option            httpchk GET / HTTP/1.0\r\nHost:\ 
nu.nl\r\nAccept:\ */*
    server            nu_nl nu.nl:443 id 2114 ssl check inter 1  
verify none resolvers globalresolvers check-sni nu.nl resolve-prefer ipv4



 haproxy_socket.sh show resolvers
Resolvers section globalresolvers
 nameserver pfs_routerbox:
  sent:    216
  snd_error:   0
  valid:   0
  update:  0
  cname:   0
  cname_error: 0
  any_err: 108
  nx:  0
  timeout: 0
  refused: 0
  other:   0
  invalid: 108
  too_big: 0
  truncated:   0
  outdated:    0

 haproxy -vv
HA-Proxy version 2.2-dev0-13a9232 2020/01/22 - https://haproxy.org/
Status: development branch - not safe for use in production.
Known bugs: https://github.com/haproxy/haproxy/issues?q=is:issue+is:open
Build options :
  TARGET  = freebsd
  CPU = generic
  CC  = cc
  CFLAGS  = -pipe -g -fstack-protector -fno-strict-aliasing 
-fno-strict-aliasing -Wdeclaration-after-statement -fwrapv 
-fno-strict-overflow -Wno-null-dereference -Wno-unused-label 
-Wno-unused-parameter -Wno-sign-compare -Wno-ignored-qualifiers 
-Wno-unused-command-line-argument -Wno-missing-field-initializers 
-Wno-address-of-packed-member -DFREEBSD_PORTS -DFREEBSD_PORTS
  OPTIONS = USE_PCRE=1 USE_PCRE_JIT=1 USE_REGPARM=1 USE_STATIC_PCRE=1 
USE_GETADDRINFO=1 USE_OPENSSL=1 USE_LUA=1 USE_ACCEPT4=1 USE_ZLIB=1


Feature list : -EPOLL +KQUEUE -MY_EPOLL -MY_SPLICE -NETFILTER +PCRE 
+PCRE_JIT -PCRE2 -PCRE2_JIT +POLL -PRIVATE_CACHE +THREAD 
-PTHREAD_PSHARED +REGPARM +STATIC_PCRE -STATIC_PCRE2 +TPROXY 
-LINUX_TPROXY -LINUX_SPLICE +LIBCRYPT -CRYPT_H -VSYSCALL +GETADDRINFO 
+OPENSSL +LUA -FUTEX +ACCEPT4 -MY_ACCEPT4 +ZLIB -SLZ +CPU_AFFINITY -TFO 
-NS -DL -RT -DEVICEATLAS -51DEGREES -WURFL -SYSTEMD -OBSOLETE_LINKER 
-PRCTL -THREAD_DUMP -EVPORTS


Default settings :
  bufsize = 16384, maxrewrite = 1024, maxpollevents = 200

Built with multi-threading support (MAX_THREADS=64, default=2).
Built with OpenSSL version : OpenSSL 1.1.1a-freebsd  20 Nov 2018
Running on OpenSSL version : OpenSSL 1.1.1a-freebsd  20 Nov 2018
OpenSSL library supports TLS extensions : yes
OpenSSL library supports SNI : yes
OpenSSL library supports : SSLv3 TLSv1.0 TLSv1.1 TLSv1.2 TLSv1.3
Built with Lua version : Lua 5.3.5
Built with transparent proxy support using: IP_BINDANY IPV6_BINDANY
Built with PCRE version : 8.43 2019-02-23
Running on PCRE version : 8.43 2019-02-23
PCRE library supports JIT : yes
Encrypted password support via crypt(3): yes
Built with zlib version : 1.2.11
Running on zlib version : 1.2.11
Compression algorithms supported : identity("identity"), 
deflate("deflate"),