resolved: rework NSEC covering tests

This makes two changes: first of all we will now explicitly check
whether a domain to test against an NSEC record is actually below the
signer's name. This is relevant for NSEC records that chain up the end
and the beginning of a zone: we shouldn't alow that NSEC record to match
against domains outside of the zone.

This also fixes how we handle NSEC checks for domains that are prefixes
of the NSEC RR domain itself, fixing #8164 which triggers this specific
case. The non-wildcard NSEC check is simplified for that, we can
directly make our between check, there's no need to find the "Next
Closer" first, as the between check should not be affected by additional
prefixes. For the wild card NSEC check we'll prepend the asterisk in
this case to the NSEC RR itself to make a correct check.

Fixes: #8164

diff --git a/src/resolve/resolved-dns-dnssec.c b/src/resolve/resolved-dns-dnssec.c
index ef996a4..b6944bb 100644 (file)
--- a/src/resolve/resolved-dns-dnssec.c
+++ b/src/resolve/resolved-dns-dnssec.c
@@ -1793,41 +1793,29 @@ static int dnssec_nsec_from_parent_zone(DnsResourceRecord *rr, const char *name)
 }
 
 static int dnssec_nsec_covers(DnsResourceRecord *rr, const char *name) {
-        const char *common_suffix, *p;
+        const char *signer;
         int r;
 
         assert(rr);
         assert(rr->key->type == DNS_TYPE_NSEC);
 
-        /* Checks whether the "Next Closer" is witin the space covered by the specified RR. */
+        /* Checks whether the name is covered by this NSEC RR. This means, that the name is somewhere below the NSEC's
+         * signer name, and between the NSEC's two names. */
 
-        r = dns_name_common_suffix(dns_resource_key_name(rr->key), rr->nsec.next_domain_name, &common_suffix);
+        r = dns_resource_record_signer(rr, &signer);
         if (r < 0)
                 return r;
 
-        for (;;) {
-                p = name;
-                r = dns_name_parent(&name);
-                if (r < 0)
-                        return r;
-                if (r == 0)
-                        return 0;
-
-                r = dns_name_equal(name, common_suffix);
-                if (r < 0)
-                        return r;
-                if (r > 0)
-                        break;
-        }
-
-        /* p is now the "Next Closer". */
+        r = dns_name_endswith(name, signer); /* this NSEC isn't suitable the name is not in the signer's domain */
+        if (r <= 0)
+                return r;
 
-        return dns_name_between(dns_resource_key_name(rr->key), p, rr->nsec.next_domain_name);
+        return dns_name_between(dns_resource_key_name(rr->key), name, rr->nsec.next_domain_name);
 }
 
 static int dnssec_nsec_covers_wildcard(DnsResourceRecord *rr, const char *name) {
         _cleanup_free_ char *wc = NULL;
-        const char *common_suffix;
+        const char *common_suffix, *signer;
         int r;
 
         assert(rr);
@@ -1842,16 +1830,27 @@ static int dnssec_nsec_covers_wildcard(DnsResourceRecord *rr, const char *name)
          *     NSEC yyy.zzz.xoo.bar →             bar: indicates that a number of wildcards don#t exist either...
          */
 
-        r = dns_name_common_suffix(dns_resource_key_name(rr->key), rr->nsec.next_domain_name, &common_suffix);
+        r = dns_resource_record_signer(rr, &signer);
         if (r < 0)
                 return r;
 
-        /* If the common suffix is not shared by the name we are interested in, it has nothing to say for us. */
-        r = dns_name_endswith(name, common_suffix);
+        r = dns_name_endswith(name, signer); /* this NSEC isn't suitable the name is not in the signer's domain */
         if (r <= 0)
                 return r;
 
-        r = dns_name_concat("*", common_suffix, &wc);
+        r = dns_name_endswith(name, dns_resource_key_name(rr->key));
+        if (r < 0)
+                return r;
+        if (r > 0)  /* If the name we are interested in is a child of the NSEC RR, then append the asterisk to the NSEC
+                     * RR's name. */
+                r = dns_name_concat("*", dns_resource_key_name(rr->key), &wc);
+        else {
+                r = dns_name_common_suffix(dns_resource_key_name(rr->key), rr->nsec.next_domain_name, &common_suffix);
+                if (r < 0)
+                        return r;
+
+                r = dns_name_concat("*", common_suffix, &wc);
+        }
         if (r < 0)
                 return r;
 

diff --git a/src/resolve/test-dnssec-complex.c b/src/resolve/test-dnssec-complex.c
index efacce6..168ea5b 100644 (file)
--- a/src/resolve/test-dnssec-complex.c
+++ b/src/resolve/test-dnssec-complex.c
@@ -153,6 +153,7 @@ int main(int argc, char* argv[]) {
         /* NXDOMAIN in NSEC domain */
         test_rr_lookup(bus, "hhh.nasa.gov", DNS_TYPE_A, _BUS_ERROR_DNS "NXDOMAIN");
         test_hostname_lookup(bus, "hhh.nasa.gov", AF_UNSPEC, _BUS_ERROR_DNS "NXDOMAIN");
+        test_rr_lookup(bus, "_pgpkey-https._tcp.hkps.pool.sks-keyservers.net", DNS_TYPE_SRV, _BUS_ERROR_DNS "NXDOMAIN");
 
         /* wildcard, NSEC zone */
         test_rr_lookup(bus, ".wilda.nsec.0skar.cz", DNS_TYPE_A, NULL);

diff --git a/src/test/test-dns-domain.c b/src/test/test-dns-domain.c
index 1b250e6..fe03e14 100644 (file)
--- a/src/test/test-dns-domain.c
+++ b/src/test/test-dns-domain.c
@@ -240,6 +240,7 @@ static void test_dns_name_between(void) {
         test_dns_name_between_one("example", "example", "example", false);
         test_dns_name_between_one("example", "example", "yljkjljk.a.example", false);
         test_dns_name_between_one("example", "yljkjljk.a.example", "yljkjljk.a.example", false);
+        test_dns_name_between_one("hkps.pool.sks-keyservers.net", "_pgpkey-https._tcp.hkps.pool.sks-keyservers.net", "ipv4.pool.sks-keyservers.net", true);
 }
 
 static void test_dns_name_endswith_one(const char *a, const char *b, int ret) {