resolved: never go below DNSSEC feature level in DNSSEC strict mode

This adjusts our feature level handling: when DNSSEC strict mode is on,
let's never lower the feature level below the lowest DNSSEC mode.

Also, when asking whether DNSSEC is supproted, always say yes in strict
mode. This means that error reporting about transactions that fail
because of missing DNSSEC RRs will not report "incompatible-server" but
instead "missing-signature" or suchlike.

The main difference here is that DNSSEC failures become local to a
transaction, instead of propagating into the feature level we reuse for
future transactions. This is beneficial with routers that implement
"mostly a DNS proxy", i.e. that propagate most DNS requests 1:1 to their
upstream servers, but synthesize local answers for a select few domains.
For example, AVM Fritz!Boxes operate that way: they proxy most traffic
1:1 upstream in an DNSSEC-compatible fashion, but synthesize the
"fritz.box" locally, so that it can be used to configure the router.
This local domain cannot be DNSSEC verified, it comes without
signatures. Previously this would mean once that domain was resolved
feature level would be downgraded, and we'd thus fail all future DNSSEC
attempts. With this change, the immediate lookup for "fritz.box" will
fail validation, but for all other unrelated future ones that comes
without prejudice.

(While we are at it, also make a couple of other downgrade paths a bit
tighter.)

Fixes: #10570 #14435 #6490

diff --git a/src/resolve/resolved-dns-server.c b/src/resolve/resolved-dns-server.c
index 5f0d54acc5ae9289f342551c11eb78658c4d5b1d..a561035faae1060cbdc0f24bd6c948d8e87771bb 100644 (file)
--- a/src/resolve/resolved-dns-server.c
+++ b/src/resolve/resolved-dns-server.c
@@ -473,12 +473,19 @@ DnsServerFeatureLevel dns_server_possible_feature_level(DnsServer *s) {
                                                                                                 DNS_SERVER_FEATURE_LEVEL_UDP;
 
                 } else if (s->packet_bad_opt &&
-                           DNS_SERVER_FEATURE_LEVEL_IS_EDNS0(s->possible_feature_level)) {
+                           DNS_SERVER_FEATURE_LEVEL_IS_EDNS0(s->possible_feature_level) &&
+                           dns_server_get_dnssec_mode(s) != DNSSEC_YES &&
+                           dns_server_get_dns_over_tls_mode(s) != DNS_OVER_TLS_YES) {
 
-                        /* A reply to one of our EDNS0 queries didn't carry a valid OPT RR, then downgrade to below
-                         * EDNS0 levels. After all, some records generate different responses with and without OPT RR
-                         * in the request. Example:
-                         * https://open.nlnetlabs.nl/pipermail/dnssec-trigger/2014-November/000376.html */
+                        /* A reply to one of our EDNS0 queries didn't carry a valid OPT RR, then downgrade to
+                         * below EDNS0 levels. After all, some servers generate different responses with and
+                         * without OPT RR in the request. Example:
+                         *
+                         * https://open.nlnetlabs.nl/pipermail/dnssec-trigger/2014-November/000376.html
+                         *
+                         * If we are in strict DNSSEC or DoT mode, we don't do this kind of downgrade
+                         * however, as both modes imply EDNS0 to work (DNSSEC strictly requires it, and DoT
+                         * only in our implementation). */
 
                         log_debug("Server doesn't support EDNS(0) properly, downgrading feature level...");
                         s->possible_feature_level = DNS_SERVER_FEATURE_LEVEL_UDP;
@@ -488,42 +495,57 @@ DnsServerFeatureLevel dns_server_possible_feature_level(DnsServer *s) {
                         log_level = LOG_NOTICE;
 
                 } else if (s->packet_rrsig_missing &&
-                           DNS_SERVER_FEATURE_LEVEL_IS_DNSSEC(s->possible_feature_level)) {
+                           DNS_SERVER_FEATURE_LEVEL_IS_DNSSEC(s->possible_feature_level) &&
+                           dns_server_get_dnssec_mode(s) != DNSSEC_YES) {
 
-                        /* RRSIG data was missing on a EDNS0 packet with DO bit set. This means the server doesn't
-                         * augment responses with DNSSEC RRs. If so, let's better not ask the server for it anymore,
-                         * after all some servers generate different replies depending if an OPT RR is in the query or
-                         * not. */
+                        /* RRSIG data was missing on a EDNS0 packet with DO bit set. This means the server
+                         * doesn't augment responses with DNSSEC RRs. If so, let's better not ask the server
+                         * for it anymore, after all some servers generate different replies depending if an
+                         * OPT RR is in the query or not. If we are in strict DNSSEC mode, don't allow such
+                         * downgrades however, since a DNSSEC feature level is a requirement for strict
+                         * DNSSEC mode. */
 
                         log_debug("Detected server responses lack RRSIG records, downgrading feature level...");
-                        s->possible_feature_level = DNS_SERVER_FEATURE_LEVEL_IS_TLS(s->possible_feature_level) ? DNS_SERVER_FEATURE_LEVEL_TLS_PLAIN : DNS_SERVER_FEATURE_LEVEL_EDNS0;
+                        s->possible_feature_level = DNS_SERVER_FEATURE_LEVEL_IS_TLS(s->possible_feature_level) ? DNS_SERVER_FEATURE_LEVEL_TLS_PLAIN :
+                                                                                                                 DNS_SERVER_FEATURE_LEVEL_EDNS0;
 
                 } else if (s->n_failed_udp >= DNS_SERVER_FEATURE_RETRY_ATTEMPTS &&
-                           s->possible_feature_level >= (dns_server_get_dnssec_mode(s) == DNSSEC_YES ? DNS_SERVER_FEATURE_LEVEL_LARGE : DNS_SERVER_FEATURE_LEVEL_UDP)) {
+                           DNS_SERVER_FEATURE_LEVEL_IS_UDP(s->possible_feature_level) &&
+                           ((s->possible_feature_level != DNS_SERVER_FEATURE_LEVEL_DO) || dns_server_get_dnssec_mode(s) != DNSSEC_YES)) {
 
-                        /* We lost too many UDP packets in a row, and are on a feature level of UDP or higher. If the
-                         * packets are lost, maybe the server cannot parse them, hence downgrading sounds like a good
-                         * idea. We might downgrade all the way down to TCP this way.
+                        /* We lost too many UDP packets in a row, and are on an UDP feature level. If the
+                         * packets are lost, maybe the server cannot parse them, hence downgrading sounds
+                         * like a good idea. We might downgrade all the way down to TCP this way.
                          *
                          * If strict DNSSEC mode is used we won't downgrade below DO level however, as packet loss
                          * might have many reasons, a broken DNSSEC implementation being only one reason. And if the
                          * user is strict on DNSSEC, then let's assume that DNSSEC is not the fault here. */
 
                         log_debug("Lost too many UDP packets, downgrading feature level...");
-                        s->possible_feature_level--;
+                        if (s->possible_feature_level == DNS_SERVER_FEATURE_LEVEL_DO) /* skip over TLS_PLAIN */
+                                s->possible_feature_level = DNS_SERVER_FEATURE_LEVEL_EDNS0;
+                        else
+                                s->possible_feature_level--;
 
                 } else if (s->n_failed_tcp >= DNS_SERVER_FEATURE_RETRY_ATTEMPTS &&
                            s->packet_truncated &&
-                           s->possible_feature_level > (dns_server_get_dnssec_mode(s) == DNSSEC_YES ? DNS_SERVER_FEATURE_LEVEL_LARGE : DNS_SERVER_FEATURE_LEVEL_UDP)) {
+                           s->possible_feature_level > DNS_SERVER_FEATURE_LEVEL_UDP &&
+                           DNS_SERVER_FEATURE_LEVEL_IS_UDP(s->possible_feature_level) &&
+                           (!DNS_SERVER_FEATURE_LEVEL_IS_DNSSEC(s->possible_feature_level) || dns_server_get_dnssec_mode(s) != DNSSEC_YES)) {
 
-                         /* We got too many TCP connection failures in a row, we had at least one truncated packet, and
-                          * are on a feature level above UDP. By downgrading things and getting rid of DNSSEC or EDNS0
-                          * data we hope to make the packet smaller, so that it still works via UDP given that TCP
-                          * appears not to be a fallback. Note that if we are already at the lowest UDP level, we don't
-                          * go further down, since that's TCP, and TCP failed too often after all. */
+                         /* We got too many TCP connection failures in a row, we had at least one truncated
+                          * packet, and are on feature level above UDP. By downgrading things and getting rid
+                          * of DNSSEC or EDNS0 data we hope to make the packet smaller, so that it still
+                          * works via UDP given that TCP appears not to be a fallback. Note that if we are
+                          * already at the lowest UDP level, we don't go further down, since that's TCP, and
+                          * TCP failed too often after all. */
 
                         log_debug("Got too many failed TCP connection failures and truncated UDP packets, downgrading feature level...");
-                        s->possible_feature_level--;
+
+                        if (DNS_SERVER_FEATURE_LEVEL_IS_DNSSEC(s->possible_feature_level))
+                                s->possible_feature_level = DNS_SERVER_FEATURE_LEVEL_EDNS0; /* Go DNSSEC → EDNS0 */
+                        else
+                                s->possible_feature_level = DNS_SERVER_FEATURE_LEVEL_UDP; /* Go EDNS0 → UDP */
                 }
 
                 if (p != s->possible_feature_level) {
@@ -619,7 +641,10 @@ bool dns_server_dnssec_supported(DnsServer *server) {
 
         /* Returns whether the server supports DNSSEC according to what we know about it */
 
-        if (server->possible_feature_level < DNS_SERVER_FEATURE_LEVEL_DO)
+        if (dns_server_get_dnssec_mode(server) == DNSSEC_YES) /* If strict DNSSEC mode is enabled, always assume DNSSEC mode is supported. */
+                return true;
+
+        if (!DNS_SERVER_FEATURE_LEVEL_IS_DNSSEC(server->possible_feature_level))
                 return false;
 
         if (server->packet_bad_opt)

diff --git a/src/resolve/resolved-dns-server.h b/src/resolve/resolved-dns-server.h
index 16e492743dfa67594efb284494deb58056ae6667..7860e32183aa85abcfe61100b4a0884cd5188287 100644 (file)
--- a/src/resolve/resolved-dns-server.h
+++ b/src/resolve/resolved-dns-server.h
@@ -43,6 +43,7 @@ typedef enum DnsServerFeatureLevel {
 #define DNS_SERVER_FEATURE_LEVEL_IS_EDNS0(x) ((x) >= DNS_SERVER_FEATURE_LEVEL_EDNS0)
 #define DNS_SERVER_FEATURE_LEVEL_IS_TLS(x) IN_SET(x, DNS_SERVER_FEATURE_LEVEL_TLS_PLAIN, DNS_SERVER_FEATURE_LEVEL_TLS_DO)
 #define DNS_SERVER_FEATURE_LEVEL_IS_DNSSEC(x) ((x) >= DNS_SERVER_FEATURE_LEVEL_DO)
+#define DNS_SERVER_FEATURE_LEVEL_IS_UDP(x) IN_SET(x, DNS_SERVER_FEATURE_LEVEL_UDP, DNS_SERVER_FEATURE_LEVEL_EDNS0, DNS_SERVER_FEATURE_LEVEL_DO, DNS_SERVER_FEATURE_LEVEL_LARGE)
 
 const char* dns_server_feature_level_to_string(int i) _const_;
 int dns_server_feature_level_from_string(const char *s) _pure_;