From 13a9232ebc63fdf357ffcf4fa7a1a5e77a1eac2b Mon Sep 17 00:00:00 2001 From: Baptiste Assmann 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 / 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);