Hi,

Since 2.2, HAProxy runtime resolver uses the Additional records of an SRV
response when available. That said, there was a small bug when multiple
records points to the same hostname and IP, then the subsequent ones may
become "unsynchronized".

This patch fixes this issue. This is also related to github issue 971.
Backport status is 2.2 and above.

Baptiste
From 78ddb9c32a1bb09e05ac592f8f8862491465aa69 Mon Sep 17 00:00:00 2001
From: Baptiste Assmann <bed...@gmail.com>
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/AAAA 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/AAAA for all other occurences of an existing hostname.
IE: with the following type of response:

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

   ;; ADDITIONAL SECTION:
   A2.tld.                 3600    IN      A       192.168.0.2
   A3.tld.                 3600    IN      A       192.168.0.3
   A1.tld.                 3600    IN      A       192.168.0.1
   A3.tld.                 3600    IN      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: 2.2 and above

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

diff --git a/src/dns.c b/src/dns.c
index 3d484263c..90efdad34 100644
--- a/src/dns.c
+++ b/src/dns.c
@@ -1027,6 +1027,10 @@ 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;
+				if (dns_answer_record->ar_item != NULL) {
+					pool_free(dns_answer_item_pool, dns_answer_record->ar_item);
+					dns_answer_record->ar_item = NULL;
+				}
 				break;
 
 			case DNS_RTYPE_AAAA:
@@ -1276,6 +1280,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, &dns_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

Reply via email to