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 1ed4e584f3a03f47d2313314b6b5a78c9dc6f135.
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
8a33aa199dc1cea14494469ac9d7d08dc6721df1 - seems to be working around Stubby
resolver behaviour.

Fixes: #26967

diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c
index 3f994690e622a610e05d471f4190a03365a6924b..400ca0d2e8e1920edd570c93df4948c303608b37 100644 (file)
--- a/src/resolve/resolved-dns-transaction.c
+++ b/src/resolve/resolved-dns-transaction.c
@@ -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 */

diff --git a/src/resolve/resolved-dns-transaction.h b/src/resolve/resolved-dns-transaction.h
index 5a0e35704cd2db4dbda1f86f1bea489a9352fb9d..dd3b897a67abfef7e31622ee3a871dec479b7bd9 100644 (file)
--- a/src/resolve/resolved-dns-transaction.h
+++ b/src/resolve/resolved-dns-transaction.h
@@ -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;