Hi there,
For those using DNS service discovery through SRV record, you might be
aware that HAProxy is quite verbose with your DNS server: it does one SRV
query + 1 A/AAAA per server found in the SRV response.
This patch aims at improving this behavior par using first Additional
records if available and relevant. If none found, previous behavior will
apply (on a per server basis).
This is behavior defined in RFC 2782 for DNS SRV records.
Baptiste
From a18ab5880ee04b75234eb65ca8a8be4a425d5ba6 Mon Sep 17 00:00:00 2001
From: Baptiste Assmann <bed...@gmail.com>
Date: Fri, 7 Jun 2019 09:40:55 +0200
Subject: [PATCH] MEDIUM: dns: use Additional records from SRV responses
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 | 4 +-
include/types/server.h | 1 +
src/dns.c | 216 +++++++++++++++++++++++++++++++++++++++++
src/server.c | 9 ++
4 files changed, 229 insertions(+), 1 deletion(-)
diff --git a/include/types/dns.h b/include/types/dns.h
index 8347e93ab..7e592b285 100644
--- a/include/types/dns.h
+++ b/include/types/dns.h
@@ -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
diff --git a/include/types/server.h b/include/types/server.h
index 842e033ad..598dfe6d8 100644
--- a/include/types/server.h
+++ b/include/types/server.h
@@ -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 */
diff --git a/src/dns.c b/src/dns.c
index 5ecb46905..eefd8d0dc 100644
--- 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;
diff --git a/src/server.c b/src/server.c
index 14ff716a6..d7cb97d9a 100644
--- a/src/server.c
+++ b/src/server.c
@@ -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);
--
2.17.1
