From 0b491556ac79950624849d218e01204d0452cc41 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 8 Jun 2018 19:29:05 +0200 Subject: [PATCH] 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 --- src/resolve/resolved-dns-dnssec.c | 49 +++++++++++++++---------------- src/resolve/test-dnssec-complex.c | 1 + src/test/test-dns-domain.c | 1 + 3 files changed, 26 insertions(+), 25 deletions(-) diff --git a/src/resolve/resolved-dns-dnssec.c b/src/resolve/resolved-dns-dnssec.c index ef996a418d..b6944bb1e1 100644 --- 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_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_equal(name, common_suffix); - if (r < 0) - return r; - if (r > 0) - break; - } - - /* p is now the "Next Closer". */ - - 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 efacce6cc8..168ea5bc36 100644 --- 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 1b250e632f..fe03e14251 100644 --- 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) {