Revert "resolved: address DVE-2018-0001"

DVE-2018-0001 has been fixed by the vendor, and this workaround is no longer
needed. Removal of this workaround improves performance as queries used to be
retried more than necessory.

This reverts 1ed4e584f3.
This reverts https://github.com/systemd/systemd/pull/18638

Keep .clamp_feature_level_servfail name, as imho it is more descriptive than
just .clamp_feature_level, especially if we ever need to add similar
workarounds as the one we had for DVE-2018-0001.

However note that there is another retry which was added in
8a33aa199d - seems to be working around Stubby
resolver behaviour.

Fixes: #26967
This commit is contained in:
Dimitri John Ledkov 2023-03-30 21:58:40 +01:00 committed by Luca Boccassi
parent cb4e5d5155
commit 4aa37ad301
2 changed files with 2 additions and 50 deletions

View file

@ -282,7 +282,6 @@ int dns_transaction_new(
.bypass = dns_packet_ref(bypass),
.current_feature_level = _DNS_SERVER_FEATURE_LEVEL_INVALID,
.clamp_feature_level_servfail = _DNS_SERVER_FEATURE_LEVEL_INVALID,
.clamp_feature_level_nxdomain = _DNS_SERVER_FEATURE_LEVEL_INVALID,
.id = pick_new_id(s->manager),
};
@ -472,10 +471,8 @@ static int dns_transaction_pick_server(DnsTransaction *t) {
/* If we changed the server invalidate the feature level clamping, as the new server might have completely
* different properties. */
if (server != t->server) {
if (server != t->server)
t->clamp_feature_level_servfail = _DNS_SERVER_FEATURE_LEVEL_INVALID;
t->clamp_feature_level_nxdomain = _DNS_SERVER_FEATURE_LEVEL_INVALID;
}
t->current_feature_level = dns_server_possible_feature_level(server);
@ -483,9 +480,6 @@ static int dns_transaction_pick_server(DnsTransaction *t) {
if (t->clamp_feature_level_servfail != _DNS_SERVER_FEATURE_LEVEL_INVALID &&
t->current_feature_level > t->clamp_feature_level_servfail)
t->current_feature_level = t->clamp_feature_level_servfail;
if (t->clamp_feature_level_nxdomain != _DNS_SERVER_FEATURE_LEVEL_INVALID &&
t->current_feature_level > t->clamp_feature_level_nxdomain)
t->current_feature_level = t->clamp_feature_level_nxdomain;
log_debug("Using feature level %s for transaction %u.", dns_server_feature_level_to_string(t->current_feature_level), t->id);
@ -1277,45 +1271,6 @@ void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p, bool encrypt
return;
}
if (t->scope->protocol == DNS_PROTOCOL_DNS &&
!t->bypass &&
DNS_PACKET_RCODE(p) == DNS_RCODE_NXDOMAIN &&
p->opt && !DNS_PACKET_DO(p) &&
DNS_SERVER_FEATURE_LEVEL_IS_EDNS0(t->current_feature_level) &&
DNS_SERVER_FEATURE_LEVEL_IS_UDP(t->current_feature_level) &&
t->scope->dnssec_mode != DNSSEC_YES) {
/* Some captive portals are special in that the Aruba/Datavalet hardware will miss
* replacing the packets with the local server IP to point to the authenticated side
* of the network if EDNS0 is enabled. Instead they return NXDOMAIN, with DO bit set
* to zero... nothing to see here, yet respond with the captive portal IP, when using
* the more simple UDP level.
*
* Common portal names that fail like so are:
* secure.datavalet.io
* securelogin.arubanetworks.com
* securelogin.networks.mycompany.com
*
* Thus retry NXDOMAIN RCODES with a lower feature level.
*
* Do not lower the server's tracked feature level, as the captive portal should not
* be lying for the wider internet (e.g. _other_ queries were observed fine with
* EDNS0 on these networks, post auth), i.e. let's just lower the level transaction's
* feature level.
*
* This is reported as https://github.com/dns-violations/dns-violations/blob/master/2018/DVE-2018-0001.md
*/
t->clamp_feature_level_nxdomain = DNS_SERVER_FEATURE_LEVEL_UDP;
log_debug("Server returned error %s in EDNS0 mode, retrying transaction with reduced feature level %s (DVE-2018-0001 mitigation)",
FORMAT_DNS_RCODE(DNS_PACKET_RCODE(p)),
dns_server_feature_level_to_string(t->clamp_feature_level_nxdomain));
dns_transaction_retry(t, false /* use the same server */);
return;
}
if (t->server) {
/* Report that we successfully received a valid packet with a good rcode after we initially got a bad
* rcode and subsequently downgraded the protocol */

View file

@ -97,11 +97,8 @@ struct DnsTransaction {
/* The features of the DNS server at time of transaction start */
DnsServerFeatureLevel current_feature_level;
/* If we got SERVFAIL back, we retry the lookup, using a lower feature level than we used
* before. Similar, if we get NXDOMAIN in pure EDNS0 mode, we check in EDNS0-less mode before giving
* up (as mitigation for DVE-2018-0001). */
/* If we got SERVFAIL back, we retry the lookup, using a lower feature level than we used before. */
DnsServerFeatureLevel clamp_feature_level_servfail;
DnsServerFeatureLevel clamp_feature_level_nxdomain;
uint16_t id;