resolved: when caching negative responses, honour NSEC/NSEC3 TTLs

When storing negative responses, clamp the SOA minimum TTL (as suggested
by RFC2308) to the TTL of the NSEC/NSEC3 RRs we used to prove
non-existance, if it there is any.

This is necessary since otherwise an attacker might put together a faked
negative response for one of our question including a high-ttl SOA RR
for any parent zone, and we'd use trust the TTL.

7 files changed, 42 insertions, 22 deletions

diff --git a/src/resolve/resolved-dns-cache.c b/src/resolve/resolved-dns-cache.c
index 1c7dd56b3b..301f383809 100644
--- a/src/resolve/resolved-dns-cache.c
+++ b/src/resolve/resolved-dns-cache.c
@@ -273,13 +273,13 @@ static DnsCacheItem* dns_cache_get(DnsCache *c, DnsResourceRecord *rr) {
         return NULL;
 }
 
-static usec_t calculate_until(DnsResourceRecord *rr, usec_t timestamp, bool use_soa_minimum) {
+static usec_t calculate_until(DnsResourceRecord *rr, uint32_t nsec_ttl, usec_t timestamp, bool use_soa_minimum) {
         uint32_t ttl;
         usec_t u;
 
         assert(rr);
 
-        ttl = rr->ttl;
+        ttl = MIN(rr->ttl, nsec_ttl);
         if (rr->key->type == DNS_TYPE_SOA && use_soa_minimum) {
                 /* If this is a SOA RR, and it is requested, clamp to
                  * the SOA's minimum field. This is used when we do
@@ -339,7 +339,7 @@ static void dns_cache_item_update_positive(
         dns_resource_key_unref(i->key);
         i->key = dns_resource_key_ref(rr->key);
 
-        i->until = calculate_until(rr, timestamp, false);
+        i->until = calculate_until(rr, (uint32_t) -1, timestamp, false);
         i->authenticated = authenticated;
         i->shared_owner = shared_owner;
 
@@ -420,7 +420,7 @@ static int dns_cache_put_positive(
         i->type = DNS_CACHE_POSITIVE;
         i->key = dns_resource_key_ref(rr->key);
         i->rr = dns_resource_record_ref(rr);
-        i->until = calculate_until(rr, timestamp, false);
+        i->until = calculate_until(rr, (uint32_t) -1, timestamp, false);
         i->authenticated = authenticated;
         i->shared_owner = shared_owner;
         i->owner_family = owner_family;
@@ -448,6 +448,7 @@ static int dns_cache_put_negative(
                 DnsResourceKey *key,
                 int rcode,
                 bool authenticated,
+                uint32_t nsec_ttl,
                 usec_t timestamp,
                 DnsResourceRecord *soa,
                 int owner_family,
@@ -470,13 +471,13 @@ static int dns_cache_put_negative(
         if (dns_type_is_pseudo(key->type))
                 return 0;
 
-        if (soa->soa.minimum <= 0 || soa->ttl <= 0) {
+        if (nsec_ttl <= 0 || soa->soa.minimum <= 0 || soa->ttl <= 0) {
                 if (log_get_max_level() >= LOG_DEBUG) {
                         r = dns_resource_key_to_string(key, &key_str);
                         if (r < 0)
                                 return r;
 
-                        log_debug("Not caching negative entry with zero SOA TTL: %s", key_str);
+                        log_debug("Not caching negative entry with zero SOA/NSEC/NSEC3 TTL: %s", key_str);
                 }
 
                 return 0;
@@ -496,7 +497,7 @@ static int dns_cache_put_negative(
                 return -ENOMEM;
 
         i->type = rcode == DNS_RCODE_SUCCESS ? DNS_CACHE_NODATA : DNS_CACHE_NXDOMAIN;
-        i->until = calculate_until(soa, timestamp, true);
+        i->until = calculate_until(soa, nsec_ttl, timestamp, true);
         i->authenticated = authenticated;
         i->owner_family = owner_family;
         i->owner_address = *owner_address;
@@ -571,6 +572,7 @@ int dns_cache_put(
                 int rcode,
                 DnsAnswer *answer,
                 bool authenticated,
+                uint32_t nsec_ttl,
                 usec_t timestamp,
                 int owner_family,
                 const union in_addr_union *owner_address) {
@@ -669,6 +671,7 @@ int dns_cache_put(
                         key,
                         rcode,
                         authenticated,
+                        nsec_ttl,
                         timestamp,
                         soa,
                         owner_family, owner_address);
diff --git a/src/resolve/resolved-dns-cache.h b/src/resolve/resolved-dns-cache.h
index 9c85ca4c58..e61b285df4 100644
--- a/src/resolve/resolved-dns-cache.h
+++ b/src/resolve/resolved-dns-cache.h
@@ -41,7 +41,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, usec_t timestamp, int owner_family, const union in_addr_union *owner_address);
+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_lookup(DnsCache *c, DnsResourceKey *key, 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-dnssec.c b/src/resolve/resolved-dns-dnssec.c
index ac2362275c..32d4834aa1 100644
--- a/src/resolve/resolved-dns-dnssec.c
+++ b/src/resolve/resolved-dns-dnssec.c
@@ -40,7 +40,6 @@
  *   - cname/dname compatibility
  *   - nxdomain on qname
  *   - per-interface DNSSEC setting
- *   - when doing negative caching, use NSEC/NSEC3 RR instead of SOA for TTL
  *
  * */
 
@@ -1250,7 +1249,7 @@ static int nsec3_hashed_domain(DnsResourceRecord *nsec3, const char *domain, con
  * that there is no proof either way. The latter is the case if a the proof of non-existence of a given
  * name uses an NSEC3 record with the opt-out bit set. Lastly, if we are given insufficient NSEC3 records
  * to conclude anything we indicate this by returning NO_RR. */
-static int dnssec_test_nsec3(DnsAnswer *answer, DnsResourceKey *key, DnssecNsecResult *result, bool *authenticated) {
+static int dnssec_test_nsec3(DnsAnswer *answer, DnsResourceKey *key, DnssecNsecResult *result, bool *authenticated, uint32_t *ttl) {
         _cleanup_free_ char *next_closer_domain = NULL, *wildcard = NULL, *wildcard_domain = NULL;
         const char *zone, *p, *pp = NULL;
         DnsResourceRecord *rr, *enclosure_rr, *suffix_rr, *wildcard_rr = NULL;
@@ -1260,7 +1259,6 @@ static int dnssec_test_nsec3(DnsAnswer *answer, DnsResourceKey *key, DnssecNsecR
 
         assert(key);
         assert(result);
-        assert(authenticated);
 
         /* First step, find the zone name and the NSEC3 parameters of the zone.
          * it is sufficient to look for the longest common suffix we find with
@@ -1369,7 +1367,10 @@ found_closest_encloser:
                 else
                         *result = DNSSEC_NSEC_NODATA;
 
-                *authenticated = a;
+                if (authenticated)
+                        *authenticated = a;
+                if (ttl)
+                        *ttl = enclosure_rr->ttl;
 
                 return 0;
         }
@@ -1452,7 +1453,6 @@ found_closest_encloser:
 
         if (!no_closer) {
                 *result = DNSSEC_NSEC_NO_RR;
-
                 return 0;
         }
 
@@ -1488,12 +1488,16 @@ found_closest_encloser:
                 }
         }
 
-        *authenticated = a;
+        if (authenticated)
+                *authenticated = a;
+
+        if (ttl)
+                *ttl = enclosure_rr->ttl;
 
         return 0;
 }
 
-int dnssec_test_nsec(DnsAnswer *answer, DnsResourceKey *key, DnssecNsecResult *result, bool *authenticated) {
+int dnssec_test_nsec(DnsAnswer *answer, DnsResourceKey *key, DnssecNsecResult *result, bool *authenticated, uint32_t *ttl) {
         DnsResourceRecord *rr;
         bool have_nsec3 = false;
         DnsAnswerFlags flags;
@@ -1501,7 +1505,6 @@ int dnssec_test_nsec(DnsAnswer *answer, DnsResourceKey *key, DnssecNsecResult *r
 
         assert(key);
         assert(result);
-        assert(authenticated);
 
         /* Look for any NSEC/NSEC3 RRs that say something about the specified key. */
 
@@ -1524,7 +1527,12 @@ int dnssec_test_nsec(DnsAnswer *answer, DnsResourceKey *key, DnssecNsecResult *r
                                         *result = DNSSEC_NSEC_CNAME;
                                 else
                                         *result = DNSSEC_NSEC_NODATA;
-                                *authenticated = flags & DNS_ANSWER_AUTHENTICATED;
+
+                                if (authenticated)
+                                        *authenticated = flags & DNS_ANSWER_AUTHENTICATED;
+                                if (ttl)
+                                        *ttl = rr->ttl;
+
                                 return 0;
                         }
 
@@ -1533,7 +1541,12 @@ int dnssec_test_nsec(DnsAnswer *answer, DnsResourceKey *key, DnssecNsecResult *r
                                 return r;
                         if (r > 0) {
                                 *result = DNSSEC_NSEC_NXDOMAIN;
-                                *authenticated = flags & DNS_ANSWER_AUTHENTICATED;
+
+                                if (authenticated)
+                                        *authenticated = flags & DNS_ANSWER_AUTHENTICATED;
+                                if (ttl)
+                                        *ttl = rr->ttl;
+
                                 return 0;
                         }
                         break;
@@ -1546,7 +1559,7 @@ int dnssec_test_nsec(DnsAnswer *answer, DnsResourceKey *key, DnssecNsecResult *r
 
         /* OK, this was not sufficient. Let's see if NSEC3 can help. */
         if (have_nsec3)
-                return dnssec_test_nsec3(answer, key, result, authenticated);
+                return dnssec_test_nsec3(answer, key, result, authenticated, ttl);
 
         /* No approproate NSEC RR found, report this. */
         *result = DNSSEC_NSEC_NO_RR;
diff --git a/src/resolve/resolved-dns-dnssec.h b/src/resolve/resolved-dns-dnssec.h
index df377c8016..94d0b23f80 100644
--- a/src/resolve/resolved-dns-dnssec.h
+++ b/src/resolve/resolved-dns-dnssec.h
@@ -99,7 +99,7 @@ typedef enum DnssecNsecResult {
         DNSSEC_NSEC_OPTOUT,
 } DnssecNsecResult;
 
-int dnssec_test_nsec(DnsAnswer *answer, DnsResourceKey *key, DnssecNsecResult *result, bool *authenticated);
+int dnssec_test_nsec(DnsAnswer *answer, DnsResourceKey *key, DnssecNsecResult *result, bool *authenticated, uint32_t *ttl);
 
 const char* dnssec_mode_to_string(DnssecMode m) _const_;
 DnssecMode dnssec_mode_from_string(const char *s) _pure_;
diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c
index 677d643463..870b7586fd 100644
--- a/src/resolve/resolved-dns-transaction.c
+++ b/src/resolve/resolved-dns-transaction.c
@@ -40,6 +40,7 @@ static void dns_transaction_reset_answer(DnsTransaction *t) {
         t->answer_dnssec_result = _DNSSEC_RESULT_INVALID;
         t->answer_source = _DNS_TRANSACTION_SOURCE_INVALID;
         t->answer_authenticated = false;
+        t->answer_nsec_ttl = (uint32_t) -1;
 }
 
 static void dns_transaction_close_connection(DnsTransaction *t) {
@@ -157,6 +158,7 @@ int dns_transaction_new(DnsTransaction **ret, DnsScope *s, DnsResourceKey *key)
         t->dns_udp_fd = -1;
         t->answer_source = _DNS_TRANSACTION_SOURCE_INVALID;
         t->answer_dnssec_result = _DNSSEC_RESULT_INVALID;
+        t->answer_nsec_ttl = (uint32_t) -1;
         t->key = dns_resource_key_ref(key);
 
         /* Find a fresh, unused transaction id */
@@ -482,6 +484,7 @@ static void dns_transaction_cache_answer(DnsTransaction *t) {
                       t->answer_rcode,
                       t->answer,
                       t->answer_authenticated,
+                      t->answer_nsec_ttl,
                       0,
                       t->received->family,
                       &t->received->sender);
@@ -2385,7 +2388,7 @@ int dns_transaction_validate_dnssec(DnsTransaction *t) {
                 bool authenticated = false;
 
                 /* Bummer! Let's check NSEC/NSEC3 */
-                r = dnssec_test_nsec(t->answer, t->key, &nr, &authenticated);
+                r = dnssec_test_nsec(t->answer, t->key, &nr, &authenticated, &t->answer_nsec_ttl);
                 if (r < 0)
                         return r;
 
diff --git a/src/resolve/resolved-dns-transaction.h b/src/resolve/resolved-dns-transaction.h
index e0f29d95e7..ede33f9547 100644
--- a/src/resolve/resolved-dns-transaction.h
+++ b/src/resolve/resolved-dns-transaction.h
@@ -81,6 +81,7 @@ struct DnsTransaction {
         int answer_rcode;
         DnssecResult answer_dnssec_result;
         DnsTransactionSource answer_source;
+        uint32_t answer_nsec_ttl;
 
         /* Indicates whether the primary answer is authenticated,
          * i.e. whether the RRs from answer which directly match the
diff --git a/src/resolve/resolved-mdns.c b/src/resolve/resolved-mdns.c
index db23bc9d42..7c1012f4ea 100644
--- a/src/resolve/resolved-mdns.c
+++ b/src/resolve/resolved-mdns.c
@@ -122,7 +122,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, 0, p->family, &p->sender);
+                dns_cache_put(&scope->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));