]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
MEDIUM: dns: use Additional records from SRV responses
authorBaptiste Assmann <bedis9@gmail.com>
Fri, 7 Jun 2019 07:40:55 +0000 (09:40 +0200)
committerWilly Tarreau <w@1wt.eu>
Wed, 22 Jan 2020 06:19:54 +0000 (07:19 +0100)
Most DNS servers provide A/AAAA records in the Additional section of a
response, which correspond to the SRV records from the Answer section:

  ;; QUESTION SECTION:
  ;_http._tcp.be1.domain.tld.     IN      SRV

  ;; ANSWER SECTION:
  _http._tcp.be1.domain.tld. 3600 IN      SRV     5 500 80 A1.domain.tld.
  _http._tcp.be1.domain.tld. 3600 IN      SRV     5 500 80 A8.domain.tld.
  _http._tcp.be1.domain.tld. 3600 IN      SRV     5 500 80 A5.domain.tld.
  _http._tcp.be1.domain.tld. 3600 IN      SRV     5 500 80 A6.domain.tld.
  _http._tcp.be1.domain.tld. 3600 IN      SRV     5 500 80 A4.domain.tld.
  _http._tcp.be1.domain.tld. 3600 IN      SRV     5 500 80 A3.domain.tld.
  _http._tcp.be1.domain.tld. 3600 IN      SRV     5 500 80 A2.domain.tld.
  _http._tcp.be1.domain.tld. 3600 IN      SRV     5 500 80 A7.domain.tld.

  ;; ADDITIONAL SECTION:
  A1.domain.tld.          3600    IN      A       192.168.0.1
  A8.domain.tld.          3600    IN      A       192.168.0.8
  A5.domain.tld.          3600    IN      A       192.168.0.5
  A6.domain.tld.          3600    IN      A       192.168.0.6
  A4.domain.tld.          3600    IN      A       192.168.0.4
  A3.domain.tld.          3600    IN      A       192.168.0.3
  A2.domain.tld.          3600    IN      A       192.168.0.2
  A7.domain.tld.          3600    IN      A       192.168.0.7

SRV record support was introduced in HAProxy 1.8 and the first design
did not take into account the records from the Additional section.
Instead, a new resolution is associated to each server with its relevant
FQDN.
This behavior generates a lot of DNS requests (1 SRV + 1 per server
associated).

This patch aims at fixing this by:
- when a DNS response is validated, we associate A/AAAA records to
  relevant SRV ones
- set a flag on associated servers to prevent them from running a DNS
  resolution for said FADN
- update server IP address with information found in the Additional
  section

If no relevant record can be found in the Additional section, then
HAProxy will failback to running a dedicated resolution for this server,
as it used to do.
This behavior is the one described in RFC 2782.

include/types/dns.h
include/types/server.h
src/dns.c
src/server.c

index 8347e93ab3772b0addb242ff9fae3704cfa33133..7e592b2858acf836a2c65be3c3f563fdc4962f71 100644 (file)
@@ -151,6 +151,7 @@ struct dns_answer_item {
        struct sockaddr address;                     /* IPv4 or IPv6, network format */
        char            target[DNS_MAX_NAME_SIZE+1]; /* Response data: SRV or CNAME type target */
        time_t          last_seen;                   /* When was the answer was last seen */
+       struct dns_answer_item *ar_item;             /* pointer to a RRset from the additionnal section, if exists */
        struct list     list;
 };
 
@@ -158,7 +159,8 @@ struct dns_response_packet {
        struct dns_header header;
        struct list       query_list;
        struct list       answer_list;
-       /* authority and additional_information ignored for now */
+       struct list       ar_list;         /* additional records */
+       /* authority ignored for now */
 };
 
 /* Resolvers section and parameters. It is linked to the name servers
index 842e033adc16b0a59b0ce504b778cfa6d7e06a06..598dfe6d8f01d8ae64e471ed49835c74d02acb05 100644 (file)
@@ -142,6 +142,7 @@ enum srv_initaddr {
 #define SRV_F_COOKIESET    0x0100        /* this server has a cookie configured, so don't generate dynamic cookies */
 #define SRV_F_FASTOPEN     0x0200        /* Use TCP Fast Open to connect to server */
 #define SRV_F_SOCKS4_PROXY 0x0400        /* this server uses SOCKS4 proxy */
+#define SRV_F_NO_RESOLUTION 0x0800       /* disable runtime DNS resolution on this server */
 
 /* configured server options for send-proxy (server->pp_opts) */
 #define SRV_PP_V1               0x0001   /* proxy protocol version 1 */
index 5ecb469057b346dd4bafa6cd4b8e127c63758f48..eefd8d0dcbd2518b028d0056f0ef027ce49852f1 100644 (file)
--- a/src/dns.c
+++ b/src/dns.c
@@ -516,6 +516,14 @@ static void dns_check_dns_response(struct dns_resolution *res)
        struct server          *srv;
        struct dns_srvrq       *srvrq;
 
+       /* clean up obsolete Additional records */
+       list_for_each_entry_safe(item, itemback, &res->response.ar_list, list) {
+               if ((item->last_seen + resolvers->hold.obsolete / 1000) < now.tv_sec) {
+                       LIST_DEL(&item->list);
+                       pool_free(dns_answer_item_pool, item);
+               }
+       }
+
        list_for_each_entry_safe(item, itemback, &res->response.answer_list, list) {
 
                /* Remove obsolete items */
@@ -607,6 +615,28 @@ static void dns_check_dns_response(struct dns_resolution *res)
                                        HA_SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
                                        continue;
                                }
+
+                               /* Check if an Additional Record is associated to this SRV record.
+                                * Perform some sanity checks too to ensure the record can be used.
+                                * If all fine, we simply pick up the IP address found and associate
+                                * it to the server.
+                                */
+                               if ((item->ar_item != NULL) &&
+                                   (item->ar_item->type == DNS_RTYPE_A || item->ar_item->type == DNS_RTYPE_AAAA))
+                                   {
+
+                                       switch (item->ar_item->type) {
+                                               case DNS_RTYPE_A:
+                                                       update_server_addr(srv, &(((struct sockaddr_in*)&item->ar_item->address)->sin_addr), AF_INET, "DNS additional recrd");
+                                               break;
+                                               case DNS_RTYPE_AAAA:
+                                                       update_server_addr(srv, &(((struct sockaddr_in6*)&item->ar_item->address)->sin6_addr), AF_INET6, "DNS additional recrd");
+                                               break;
+                                       }
+
+                                       srv->flags |= SRV_F_NO_RESOLUTION;
+                               }
+
                                msg = update_server_fqdn(srv, hostname, "SRV record", 1);
                                if (msg)
                                        send_log(srv->proxy, LOG_NOTICE, "%s", msg);
@@ -990,12 +1020,197 @@ static int dns_validate_dns_response(unsigned char *resp, unsigned char *bufend,
                }
                else {
                        dns_answer_record->last_seen = now.tv_sec;
+                       dns_answer_record->ar_item = NULL;
                        LIST_ADDQ(&dns_p->answer_list, &dns_answer_record->list);
                }
        } /* for i 0 to ancount */
 
        /* Save the number of records we really own */
        dns_p->header.ancount = nb_saved_records;
+
+       /* 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;
+
+               dns_answer_record = pool_alloc(dns_answer_item_pool);
+               if (dns_answer_record == NULL)
+                       return (DNS_RESP_INVALID);
+
+               offset = 0;
+               len = dns_read_name(resp, bufend, reader, tmpname, DNS_MAX_NAME_SIZE, &offset, 0);
+
+               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;
+                       }
+
+               }
+
+               memcpy(dns_answer_record->name, tmpname, len);
+               dns_answer_record->name[len] = 0;
+
+               reader += offset;
+               if (reader >= bufend) {
+                       pool_free(dns_answer_item_pool, dns_answer_record);
+                       return DNS_RESP_INVALID;
+               }
+
+               /* 2 bytes for record type (A, AAAA, CNAME, etc...) */
+               if (reader + 2 > bufend) {
+                       pool_free(dns_answer_item_pool, dns_answer_record);
+                       return DNS_RESP_INVALID;
+               }
+               dns_answer_record->type = reader[0] * 256 + reader[1];
+               reader += 2;
+
+               /* 2 bytes for class (2) */
+               if (reader + 2 > bufend) {
+                       pool_free(dns_answer_item_pool, dns_answer_record);
+                       return DNS_RESP_INVALID;
+               }
+               dns_answer_record->class = reader[0] * 256 + reader[1];
+               reader += 2;
+
+               /* 4 bytes for ttl (4) */
+               if (reader + 4 > bufend) {
+                       pool_free(dns_answer_item_pool, dns_answer_record);
+                       return DNS_RESP_INVALID;
+               }
+               dns_answer_record->ttl =   reader[0] * 16777216 + reader[1] * 65536
+                                        + reader[2] * 256 + reader[3];
+               reader += 4;
+
+               /* Now reading data len */
+               if (reader + 2 > bufend) {
+                       pool_free(dns_answer_item_pool, dns_answer_record);
+                       return DNS_RESP_INVALID;
+               }
+               dns_answer_record->data_len = reader[0] * 256 + reader[1];
+
+               /* Move forward 2 bytes for data len */
+               reader += 2;
+
+               if (reader + dns_answer_record->data_len > bufend) {
+                       pool_free(dns_answer_item_pool, dns_answer_record);
+                       return DNS_RESP_INVALID;
+               }
+
+               /* Analyzing record content */
+               switch (dns_answer_record->type) {
+                       case DNS_RTYPE_A:
+                               /* ipv4 is stored on 4 bytes */
+                               if (dns_answer_record->data_len != 4) {
+                                       pool_free(dns_answer_item_pool, dns_answer_record);
+                                       return DNS_RESP_INVALID;
+                               }
+                               dns_answer_record->address.sa_family = AF_INET;
+                               memcpy(&(((struct sockaddr_in *)&dns_answer_record->address)->sin_addr),
+                                               reader, dns_answer_record->data_len);
+                               break;
+
+                       case DNS_RTYPE_AAAA:
+                               /* ipv6 is stored on 16 bytes */
+                               if (dns_answer_record->data_len != 16) {
+                                       pool_free(dns_answer_item_pool, dns_answer_record);
+                                       return DNS_RESP_INVALID;
+                               }
+                               dns_answer_record->address.sa_family = AF_INET6;
+                               memcpy(&(((struct sockaddr_in6 *)&dns_answer_record->address)->sin6_addr),
+                                               reader, dns_answer_record->data_len);
+                               break;
+
+                       default:
+                               pool_free(dns_answer_item_pool, dns_answer_record);
+                               continue;
+
+               } /* switch (record type) */
+
+               /* Increment the counter for number of records saved into our
+                * local response */
+               nb_saved_records++;
+
+               /* Move forward dns_answer_record->data_len for analyzing next
+                * record in the response */
+               reader += ((dns_answer_record->type == DNS_RTYPE_SRV)
+                          ? offset
+                          : dns_answer_record->data_len);
+
+               /* Lookup to see if we already had this entry */
+               found = 0;
+               list_for_each_entry(tmp_record, &dns_p->answer_list, list) {
+                       if (tmp_record->type != dns_answer_record->type)
+                               continue;
+
+                       switch(tmp_record->type) {
+                               case DNS_RTYPE_A:
+                                       if (!memcmp(&((struct sockaddr_in *)&dns_answer_record->address)->sin_addr,
+                                                   &((struct sockaddr_in *)&tmp_record->address)->sin_addr,
+                                                   sizeof(in_addr_t)))
+                                               found = 1;
+                                       break;
+
+                               case DNS_RTYPE_AAAA:
+                                       if (!memcmp(&((struct sockaddr_in6 *)&dns_answer_record->address)->sin6_addr,
+                                                   &((struct sockaddr_in6 *)&tmp_record->address)->sin6_addr,
+                                                   sizeof(struct in6_addr)))
+                                               found = 1;
+                                       break;
+
+                               default:
+                                       break;
+                       }
+
+                       if (found == 1)
+                               break;
+               }
+
+               if (found == 1) {
+                       tmp_record->last_seen = now.tv_sec;
+                       pool_free(dns_answer_item_pool, dns_answer_record);
+               }
+               else {
+                       dns_answer_record->last_seen = now.tv_sec;
+                       dns_answer_record->ar_item = NULL;
+
+                       // 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) &&
+                                       (memcmp(tmp_record->target, dns_answer_record->name, tmp_record->data_len) == 0)
+                                     )
+                                  )
+                                       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(&dns_p->ar_list, &dns_answer_record->list);
+               }
+       } /* for i 0 to arcount */
+
+       /* Save the number of records we really own */
+       dns_p->header.arcount = nb_saved_records;
+
        dns_check_dns_response(resolution);
        return DNS_RESP_VALID;
 }
@@ -1347,6 +1562,7 @@ static struct dns_resolution *dns_pick_resolution(struct dns_resolvers *resolver
 
                LIST_INIT(&res->requesters);
                LIST_INIT(&res->response.answer_list);
+               LIST_INIT(&res->response.ar_list);
 
                res->prefered_query_type = query_type;
                res->query_type          = query_type;
index 14ff716a65c5856c4bd8e4a24fd912875abb6f99..d7cb97d9abb7838de457656667109d4677cb7339 100644 (file)
@@ -3320,9 +3320,11 @@ static void srv_update_state(struct server *srv, int version, char **params)
                                        /* If the FDQN has been changed from stats socket,
                                         * apply fqdn state file value (which is the value set
                                         * from stats socket).
+                                        * Also ensure the runtime resolver will process this resolution.
                                         */
                                        if (fqdn_set_by_cli) {
                                                srv_set_fqdn(srv, fqdn, 0);
+                                               srv->flags &= ~SRV_F_NO_RESOLUTION;
                                                srv->next_admin |= SRV_ADMF_HMAINT;
                                        }
                                }
@@ -4429,6 +4431,9 @@ int srv_set_fqdn(struct server *srv, const char *hostname, int dns_locked)
        if (!srv->hostname || !srv->hostname_dn)
                goto err;
 
+       if (srv->flags & SRV_F_NO_RESOLUTION)
+               goto end;
+
        if (dns_link_resolution(srv, OBJ_TYPE_SERVER, 1) == -1)
                goto err;
 
@@ -4770,6 +4775,10 @@ static int cli_parse_set_server(char **args, char *payload, struct appctx *appct
                        cli_err(appctx, "set server <b>/<s> fqdn requires a FQDN.\n");
                        goto out_unlock;
                }
+               /* ensure runtime resolver will process this new fqdn */
+               if (sv->flags & SRV_F_NO_RESOLUTION) {
+                       sv->flags &= ~SRV_F_NO_RESOLUTION;
+               }
                warning = update_server_fqdn(sv, args[4], "stats socket command", 0);
                if (warning)
                        cli_msg(appctx, LOG_WARNING, warning);