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 ef996a418dd67f4f9ed29f67221a43782b09744e..b6944bb1e1f09e5efa31945b739e500bede73cc2 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 efacce6cc8625bb004c5d9c16938f6fe4790d986..168ea5bc3685d3a0df11a99e7f175a7819216798 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 1b250e632fc2aa4ad45597c0ccd97b759e8f0700..fe03e1425124bc79418a4deb361abcd2f8104af4 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) {