resolved: switch cache option to a tri-state option (systemd#5552).

Change the resolved.conf Cache option to a tri-state "no, no-negative, yes" values.

If a lookup returns SERVFAIL systemd-resolved will cache the result for 30s (See 201d995),
however, there are several use cases on which this condition is not acceptable (See systemd#5552 comments)
and the only workaround would be to disable cache entirely or flush it , which isn't optimal.

This change adds the 'no-negative' option when set it avoids putting in cache
negative answers but still works the same heuristics for positive answers.

Signed-off-by: Jorge Niedbalski <jnr@metaklass.org>

11 files changed, 49 insertions, 8 deletions

diff --git a/NEWS b/NEWS
index 18fac477fc..8aa40f7ddc 100644
--- a/NEWS
+++ b/NEWS
@@ -178,6 +178,13 @@ CHANGES WITH 243 in spe:
 
         * systemd-resolved gained support for a new 'strict' DNS-over-TLS mode.
 
+        * systemd-resolved "Cache=" configuration option in resolved.conf has been extended
+          to also accept the 'no-negative' value. Previously,
+          only a boolean option was allowed (yes/no), having yes as the default.
+          If this option is set to 'no-negative', negative answers are skipped
+          from being cached while keeping the same cache heuristics for positive answers.
+          The default remains as "yes" (i. e. caching is enabled).
+
         * The predictable naming scheme for network devices now supports
           generating predictable names for "netdevsim" devices.
 
diff --git a/man/resolved.conf.xml b/man/resolved.conf.xml
index a647a4ace7..213be1d7b2 100644
--- a/man/resolved.conf.xml
+++ b/man/resolved.conf.xml
@@ -227,10 +227,11 @@
 
       <varlistentry>
         <term><varname>Cache=</varname></term>
-        <listitem><para>Takes a boolean argument. If <literal>yes</literal> (the default), resolving a domain name
+        <listitem><para>Takes a boolean or <literal>no-negative</literal> as argument. If <literal>yes</literal> (the default), resolving a domain name
         which already got queried earlier will return the previous result as long as it is still valid, and thus does
         not result in a new network request. Be aware that turning off caching comes at a performance penalty, which
         is particularly high when DNSSEC is used.</para>
+        If <literal>no-negative</literal>, only positive answers are cached.
 
         <para>Note that caching is turned off implicitly if the configured DNS server is on a host-local IP address
         (such as 127.0.0.1 or ::1), in order to avoid duplicate local caching.</para></listitem>
diff --git a/src/resolve/resolved-dns-cache.c b/src/resolve/resolved-dns-cache.c
index 524d5cdb53..946889a834 100644
--- a/src/resolve/resolved-dns-cache.c
+++ b/src/resolve/resolved-dns-cache.c
@@ -621,6 +621,7 @@ static bool rr_eligible(DnsResourceRecord *rr) {
 
 int dns_cache_put(
                 DnsCache *c,
+                DnsCacheMode cache_mode,
                 DnsResourceKey *key,
                 int rcode,
                 DnsAnswer *answer,
@@ -728,6 +729,13 @@ int dns_cache_put(
                         return 0;
         }
 
+        if (cache_mode == DNS_CACHE_MODE_NO_NEGATIVE) {
+                char key_str[DNS_RESOURCE_KEY_STRING_MAX];
+                log_debug("Not caching negative entry for: %s, cache mode set to no-negative",
+                        dns_resource_key_to_string(key, key_str, sizeof key_str));
+                return 0;
+        }
+
         r = dns_cache_put_negative(
                         c,
                         key,
diff --git a/src/resolve/resolved-dns-cache.h b/src/resolve/resolved-dns-cache.h
index 48a3bde98b..afd7d45db6 100644
--- a/src/resolve/resolved-dns-cache.h
+++ b/src/resolve/resolved-dns-cache.h
@@ -4,6 +4,7 @@
 #include "hashmap.h"
 #include "list.h"
 #include "prioq.h"
+#include "resolve-util.h"
 #include "time-util.h"
 
 typedef struct DnsCache {
@@ -21,7 +22,7 @@ typedef struct DnsCache {
 void dns_cache_flush(DnsCache *c);
 void dns_cache_prune(DnsCache *c);
 
-int dns_cache_put(DnsCache *c, DnsResourceKey *key, int rcode, DnsAnswer *answer, bool authenticated, uint32_t nsec_ttl, usec_t timestamp, int owner_family, const union in_addr_union *owner_address);
+int dns_cache_put(DnsCache *c, DnsCacheMode cache_mode, DnsResourceKey *key, int rcode, DnsAnswer *answer, bool authenticated, uint32_t nsec_ttl, usec_t timestamp, int owner_family, const union in_addr_union *owner_address);
 int dns_cache_lookup(DnsCache *c, DnsResourceKey *key, bool clamp_ttl, int *rcode, DnsAnswer **answer, bool *authenticated);
 
 int dns_cache_check_conflicts(DnsCache *cache, DnsResourceRecord *rr, int owner_family, const union in_addr_union *owner_address);
diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c
index deb46fae7a..262f63cae3 100644
--- a/src/resolve/resolved-dns-transaction.c
+++ b/src/resolve/resolved-dns-transaction.c
@@ -672,7 +672,7 @@ static void dns_transaction_cache_answer(DnsTransaction *t) {
                 return;
 
         /* Caching disabled? */
-        if (!t->scope->manager->enable_cache)
+        if (t->scope->manager->enable_cache == DNS_CACHE_MODE_NO)
                 return;
 
         /* We never cache if this packet is from the local host, under
@@ -683,6 +683,7 @@ static void dns_transaction_cache_answer(DnsTransaction *t) {
                 return;
 
         dns_cache_put(&t->scope->cache,
+                      t->scope->manager->enable_cache,
                       t->key,
                       t->answer_rcode,
                       t->answer,
diff --git a/src/resolve/resolved-gperf.gperf b/src/resolve/resolved-gperf.gperf
index 9b9290b727..049fe9ebdd 100644
--- a/src/resolve/resolved-gperf.gperf
+++ b/src/resolve/resolved-gperf.gperf
@@ -24,6 +24,6 @@ Resolve.LLMNR,           config_parse_resolve_support,        0,
 Resolve.MulticastDNS,    config_parse_resolve_support,        0,                   offsetof(Manager, mdns_support)
 Resolve.DNSSEC,          config_parse_dnssec_mode,            0,                   offsetof(Manager, dnssec_mode)
 Resolve.DNSOverTLS,      config_parse_dns_over_tls_mode,      0,                   offsetof(Manager, dns_over_tls_mode)
-Resolve.Cache,           config_parse_bool,                   0,                   offsetof(Manager, enable_cache)
+Resolve.Cache,           config_parse_dns_cache_mode,         DNS_CACHE_MODE_YES,  offsetof(Manager, enable_cache)
 Resolve.DNSStubListener, config_parse_dns_stub_listener_mode, 0,                   offsetof(Manager, dns_stub_listener_mode)
-Resolve.ReadEtcHosts,    config_parse_bool,                   0,                   offsetof(Manager, read_etc_hosts)
+Resolve.ReadEtcHosts,    config_parse_bool,                   0,                   offsetof(Manager, read_etc_hosts)
\ No newline at end of file
diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c
index fc8607f79a..486b7a87be 100644
--- a/src/resolve/resolved-manager.c
+++ b/src/resolve/resolved-manager.c
@@ -581,7 +581,7 @@ int manager_new(Manager **ret) {
                 .mdns_support = RESOLVE_SUPPORT_YES,
                 .dnssec_mode = DEFAULT_DNSSEC_MODE,
                 .dns_over_tls_mode = DEFAULT_DNS_OVER_TLS_MODE,
-                .enable_cache = true,
+                .enable_cache = DNS_CACHE_MODE_YES,
                 .dns_stub_listener_mode = DNS_STUB_LISTENER_YES,
                 .read_resolv_conf = true,
                 .need_builtin_fallbacks = true,
diff --git a/src/resolve/resolved-manager.h b/src/resolve/resolved-manager.h
index 0f7d8ee5ee..7f7d3a6b9c 100644
--- a/src/resolve/resolved-manager.h
+++ b/src/resolve/resolved-manager.h
@@ -37,7 +37,7 @@ struct Manager {
         ResolveSupport mdns_support;
         DnssecMode dnssec_mode;
         DnsOverTlsMode dns_over_tls_mode;
-        bool enable_cache;
+        DnsCacheMode enable_cache;
         DnsStubListenerMode dns_stub_listener_mode;
 
 #if ENABLE_DNS_OVER_TLS
diff --git a/src/resolve/resolved-mdns.c b/src/resolve/resolved-mdns.c
index 67080cb01c..eba2978991 100644
--- a/src/resolve/resolved-mdns.c
+++ b/src/resolve/resolved-mdns.c
@@ -319,7 +319,7 @@ static int on_mdns_packet(sd_event_source *s, int fd, uint32_t revents, void *us
                                 dns_transaction_process_reply(t, p);
                 }
 
-                dns_cache_put(&scope->cache, NULL, DNS_PACKET_RCODE(p), p->answer, false, (uint32_t) -1, 0, p->family, &p->sender);
+                dns_cache_put(&scope->cache, scope->manager->enable_cache, NULL, DNS_PACKET_RCODE(p), p->answer, false, (uint32_t) -1, 0, p->family, &p->sender);
 
         } else if (dns_packet_validate_query(p) > 0)  {
                 log_debug("Got mDNS query packet for id %u", DNS_PACKET_ID(p));
diff --git a/src/shared/resolve-util.c b/src/shared/resolve-util.c
index 3d14410e32..615cb6d852 100644
--- a/src/shared/resolve-util.c
+++ b/src/shared/resolve-util.c
@@ -41,3 +41,12 @@ bool dns_server_address_valid(int family, const union in_addr_union *sa) {
 
         return true;
 }
+
+DEFINE_CONFIG_PARSE_ENUM(config_parse_dns_cache_mode, dns_cache_mode, DnsCacheMode, "Failed to parse DNS cache mode setting")
+
+static const char* const dns_cache_mode_table[_DNS_CACHE_MODE_MAX] = {
+        [DNS_CACHE_MODE_YES] = "yes",
+        [DNS_CACHE_MODE_NO] = "no",
+        [DNS_CACHE_MODE_NO_NEGATIVE] = "no-negative",
+};
+DEFINE_STRING_TABLE_LOOKUP_WITH_BOOLEAN(dns_cache_mode, DnsCacheMode, DNS_CACHE_MODE_YES);
diff --git a/src/shared/resolve-util.h b/src/shared/resolve-util.h
index cf097dfaa3..acf1f3dade 100644
--- a/src/shared/resolve-util.h
+++ b/src/shared/resolve-util.h
@@ -8,6 +8,16 @@
 /* 127.0.0.53 in native endian */
 #define INADDR_DNS_STUB ((in_addr_t) 0x7f000035U)
 
+typedef enum DnsCacheMode DnsCacheMode;
+
+enum DnsCacheMode {
+        DNS_CACHE_MODE_NO,
+        DNS_CACHE_MODE_YES,
+        DNS_CACHE_MODE_NO_NEGATIVE,
+        _DNS_CACHE_MODE_MAX,
+        _DNS_CACHE_MODE_INVALID = 1
+};
+
 typedef enum ResolveSupport ResolveSupport;
 typedef enum DnssecMode DnssecMode;
 typedef enum DnsOverTlsMode DnsOverTlsMode;
@@ -56,6 +66,7 @@ enum DnsOverTlsMode {
 CONFIG_PARSER_PROTOTYPE(config_parse_resolve_support);
 CONFIG_PARSER_PROTOTYPE(config_parse_dnssec_mode);
 CONFIG_PARSER_PROTOTYPE(config_parse_dns_over_tls_mode);
+CONFIG_PARSER_PROTOTYPE(config_parse_dns_cache_mode);
 
 const char* resolve_support_to_string(ResolveSupport p) _const_;
 ResolveSupport resolve_support_from_string(const char *s) _pure_;
@@ -67,3 +78,6 @@ const char* dns_over_tls_mode_to_string(DnsOverTlsMode p) _const_;
 DnsOverTlsMode dns_over_tls_mode_from_string(const char *s) _pure_;
 
 bool dns_server_address_valid(int family, const union in_addr_union *sa);
+
+const char* dns_cache_mode_to_string(DnsCacheMode p) _const_;
+DnsCacheMode dns_cache_mode_from_string(const char *s) _pure_;