BUG/MINOR: resolvers: Restore round-robin selection on records in DNS answers

Since the commit dcb696cd3 ("MEDIUM: resolvers: hash the records before
inserting them into the tree"), When several records are found in a DNS
answer, the round robin selection over these records is no longer performed.

Indeed, before a list of records was used. To ensure each records was
selected one after the other, at each selection, the first record of the
list was moved at the end. When this list was replaced bu a tree, the same
mechanism was preserved. However, the record is indexed using its key, a
hash of the record. So its position never changes. When it is removed and
reinserted in the tree, its position remains the same. When we walk though
the tree, starting from the root, the records are always evaluated in the
same order. So, even if there are several records in a DNS answer, the same
IP address is always selected.

It is quite easy to trigger the issue with a do-resolv action.

To fix the issue, the node to perform the next selection is now saved. So
instead of restarting from the root each time, we can restart from the next
node of the previous call.

Thanks to Damien Claisse for the issue analysis and for the reproducer.

This patch should fix the issue #3116. It must be backported as far as 2.6.
This commit is contained in:
Christopher Faulet 2025-09-11 15:20:35 +02:00
parent 37abe56b18
commit 3023e98199
2 changed files with 17 additions and 17 deletions

View File

@ -133,6 +133,7 @@ struct resolv_answer_item {
struct resolv_response {
struct dns_header header;
struct eb_root answer_tree;
struct eb32_node *next; /* node to start eval on the next lookup to perform a round robin selection on entries (may be NULL) */
/* authority ignored for now */
};

View File

@ -1614,7 +1614,7 @@ int resolv_get_ip_from_response(struct resolv_response *r_res,
struct server *owner)
{
struct resolv_answer_item *record, *found_record = NULL;
struct eb32_node *eb32;
struct eb32_node *eb32, *end;
int family_priority;
int currentip_found;
unsigned char *newip4, *newip6;
@ -1647,11 +1647,16 @@ int resolv_get_ip_from_response(struct resolv_response *r_res,
* 1 - current ip.
* The result with the biggest score is returned.
*/
for (eb32 = eb32_first(&r_res->answer_tree); eb32 != NULL; eb32 = eb32_next(eb32)) {
eb32 = (!r_res->next) ? eb32_first(&r_res->answer_tree) : r_res->next;
end = r_res->next;
r_res->next = eb32_next(eb32); /* get node for the next lookup */
do {
void *ip;
unsigned char ip_type;
if (eb32 == NULL)
eb32 = eb32_first(&r_res->answer_tree);
record = eb32_entry(eb32, typeof(*record), link);
if (record->type == DNS_RTYPE_A && (resolv_active_families() & RSLV_ACCEPT_IPV4)) {
ip_type = AF_INET;
@ -1662,7 +1667,7 @@ int resolv_get_ip_from_response(struct resolv_response *r_res,
ip = &record->data.in6.sin6_addr;
}
else
continue;
goto next;
score = 0;
/* Check for preferred ip protocol. */
@ -1674,7 +1679,7 @@ int resolv_get_ip_from_response(struct resolv_response *r_res,
/* Compare only the same addresses class. */
if (resolv_opts->pref_net[j].family != ip_type)
continue;
goto next;
if ((ip_type == AF_INET &&
in_net_ipv4(ip,
@ -1698,7 +1703,7 @@ int resolv_get_ip_from_response(struct resolv_response *r_res,
list_for_each_entry(srv, &record->attached_servers, ip_rec_item) {
if (srv == owner)
continue;
goto next;
if (srv->proxy == owner->proxy) {
already_used = 1;
break;
@ -1706,7 +1711,7 @@ int resolv_get_ip_from_response(struct resolv_response *r_res,
}
if (already_used) {
if (!allowed_duplicated_ip) {
continue;
goto next;
}
}
else {
@ -1748,7 +1753,9 @@ int resolv_get_ip_from_response(struct resolv_response *r_res,
}
max_score = score;
}
} /* list for each record entries */
next:
eb32 = eb32_next(eb32);
} while (eb32 != end); /* list for each record entries */
/* No IP found in the response */
if (!newip4 && !newip6)
@ -1794,15 +1801,6 @@ int resolv_get_ip_from_response(struct resolv_response *r_res,
LIST_APPEND(&found_record->attached_servers, &owner->ip_rec_item);
}
eb32 = eb32_first(&r_res->answer_tree);
if (eb32) {
/* Move the first record to the end of the list, for internal
* round robin.
*/
eb32_delete(eb32);
eb32_insert(&r_res->answer_tree, eb32);
}
return (currentip_found ? RSLV_UPD_NO : RSLV_UPD_SRVIP_NOT_FOUND);
}
@ -1973,6 +1971,7 @@ static struct resolv_resolution *resolv_pick_resolution(struct resolvers *resolv
LIST_INIT(&res->requesters);
res->response.answer_tree = EB_ROOT;
res->response.next = NULL;
res->prefered_query_type = query_type;
res->query_type = query_type;