BUG/MINOR: dns: SRV records ignores duplicated AR records (v2)

V2 of this fix which includes a missing pointer initialization which was
causing a segfault in v1 (949a7f64591458eb06c998acf409093ea991dc3a)

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

diff --git a/src/dns.c b/src/dns.c
index d71a5d3024cc2e0ccc22283909e5b332d07515bf..8c97df46b7c5cd2665af592b35d22ddca2ac57c4 100644 (file)
--- a/src/dns.c
+++ b/src/dns.c
@@ -900,6 +900,9 @@ static int dns_validate_dns_response(unsigned char *resp, unsigned char *bufend,
                if (dns_answer_record == NULL)
                        goto invalid_resp;
 
+               /* initialization */
+               dns_answer_record->ar_item = NULL;
+
                offset = 0;
                len = dns_read_name(resp, bufend, reader, tmpname, DNS_MAX_NAME_SIZE, &offset, 0);
 
@@ -1027,6 +1030,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,10 +1283,9 @@ 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)
-                                               pool_free(dns_answer_item_pool, tmp_record->ar_item);
                                        tmp_record->ar_item = dns_answer_record;
                                        break;
                                }