From 588c53d0441ee33b617582429434b47492f51744 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 14 Jan 2016 17:25:06 +0100 Subject: resolved: some RR types may appear only or not at all in a zone apex Add extra checks when validating with RRSIGs. This follows recommendations from: http://www.george-barwood.pwp.blueyonder.co.uk/DnsServer/NotesOnDNSSSEC.htm --- src/resolve/dns-type.c | 11 +++++++++++ src/resolve/dns-type.h | 1 + src/resolve/resolved-dns-dnssec.c | 36 ++++++++++++++++++++++++++++++------ 3 files changed, 42 insertions(+), 6 deletions(-) (limited to 'src') diff --git a/src/resolve/dns-type.c b/src/resolve/dns-type.c index 47a37fa0a7..058d14009a 100644 --- a/src/resolve/dns-type.c +++ b/src/resolve/dns-type.c @@ -135,6 +135,17 @@ bool dns_type_may_wildcard(uint16_t type) { DNS_TYPE_DNAME); } +bool dns_type_apex_only(uint16_t type) { + + /* Returns true for all RR types that may only appear signed in a zone apex */ + + return IN_SET(type, + DNS_TYPE_SOA, + DNS_TYPE_NS, /* this one can appear elsewhere, too, but not signed */ + DNS_TYPE_DNSKEY, + DNS_TYPE_NSEC3PARAM); +} + bool dns_type_is_dnssec(uint16_t type) { return IN_SET(type, DNS_TYPE_DS, diff --git a/src/resolve/dns-type.h b/src/resolve/dns-type.h index 747bc854e1..78ff71b06e 100644 --- a/src/resolve/dns-type.h +++ b/src/resolve/dns-type.h @@ -132,6 +132,7 @@ bool dns_type_may_redirect(uint16_t type); bool dns_type_is_dnssec(uint16_t type); bool dns_type_is_obsolete(uint16_t type); bool dns_type_may_wildcard(uint16_t type); +bool dns_type_apex_only(uint16_t type); bool dns_class_is_pseudo(uint16_t class); bool dns_class_is_valid_rr(uint16_t class); diff --git a/src/resolve/resolved-dns-dnssec.c b/src/resolve/resolved-dns-dnssec.c index afff979b5a..4aade4829e 100644 --- a/src/resolve/resolved-dns-dnssec.c +++ b/src/resolve/resolved-dns-dnssec.c @@ -508,14 +508,14 @@ int dnssec_verify_rrset( DnssecResult *result) { uint8_t wire_format_name[DNS_WIRE_FOMAT_HOSTNAME_MAX]; - size_t hash_size; - void *hash; DnsResourceRecord **list, *rr; + const char *source, *name; gcry_md_hd_t md = NULL; int r, md_algorithm; size_t k, n = 0; + size_t hash_size; + void *hash; bool wildcard; - const char *source; assert(key); assert(rrsig); @@ -544,8 +544,32 @@ int dnssec_verify_rrset( return 0; } + name = DNS_RESOURCE_KEY_NAME(key); + + /* Some keys may only appear signed in the zone apex, and are invalid anywhere else. (SOA, NS...) */ + if (dns_type_apex_only(rrsig->rrsig.type_covered)) { + r = dns_name_equal(rrsig->rrsig.signer, name); + if (r < 0) + return r; + if (r == 0) { + *result = DNSSEC_INVALID; + return 0; + } + } + + /* OTOH DS RRs may not appear in the zone apex, but are valid everywhere else. */ + if (rrsig->rrsig.type_covered == DNS_TYPE_DS) { + r = dns_name_equal(rrsig->rrsig.signer, name); + if (r < 0) + return r; + if (r > 0) { + *result = DNSSEC_INVALID; + return 0; + } + } + /* Determine the "Source of Synthesis" and whether this is a wildcard RRSIG */ - r = dns_name_suffix(DNS_RESOURCE_KEY_NAME(key), rrsig->rrsig.labels, &source); + r = dns_name_suffix(name, rrsig->rrsig.labels, &source); if (r < 0) return r; if (r > 0 && !dns_type_may_wildcard(rrsig->rrsig.type_covered)) { @@ -556,11 +580,11 @@ int dnssec_verify_rrset( if (r == 1) { /* If we stripped a single label, then let's see if that maybe was "*". If so, we are not really * synthesized from a wildcard, we are the wildcard itself. Treat that like a normal name. */ - r = dns_name_startswith(DNS_RESOURCE_KEY_NAME(key), "*"); + r = dns_name_startswith(name, "*"); if (r < 0) return r; if (r > 0) - source = DNS_RESOURCE_KEY_NAME(key); + source = name; wildcard = r == 0; } else -- cgit v1.2.3-54-g00ecf From 54b778e7d63ce0af0d5e9401b563c6dd28eff9d3 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 14 Jan 2016 17:27:28 +0100 Subject: resolved: ignore DS RRs without generating an error if they use an unsupported digest algorithm --- src/resolve/resolved-dns-dnssec.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/resolve/resolved-dns-dnssec.c b/src/resolve/resolved-dns-dnssec.c index 4aade4829e..f39454b9f9 100644 --- a/src/resolve/resolved-dns-dnssec.c +++ b/src/resolve/resolved-dns-dnssec.c @@ -1117,8 +1117,8 @@ int dnssec_verify_dnskey_search(DnsResourceRecord *dnskey, DnsAnswer *validated_ continue; r = dnssec_verify_dnskey(dnskey, ds, false); - if (r == -EKEYREJECTED) - return 0; /* The DNSKEY is revoked or otherwise invalid, we won't bless it */ + if (IN_SET(r, -EKEYREJECTED, -EOPNOTSUPP)) + return 0; /* The DNSKEY is revoked or otherwise invalid, or we don't support the digest algorithm */ if (r < 0) return r; if (r > 0) -- cgit v1.2.3-54-g00ecf From 1827a1582cbd9dcd1ce337b2404ec4425cb0dfd0 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 14 Jan 2016 17:28:58 +0100 Subject: resolved: do not use NSEC RRs from the wrong zone for proofs When proving NODATA DS lookups we need to insist on looking at the parent zone's NSEC RR, not the child zone's. When proving any other NODATA lookups we need to insist on looking at the child zone's NSEC RR, not the parent's. --- src/resolve/resolved-dns-dnssec.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'src') diff --git a/src/resolve/resolved-dns-dnssec.c b/src/resolve/resolved-dns-dnssec.c index f39454b9f9..b6fb362daa 100644 --- a/src/resolve/resolved-dns-dnssec.c +++ b/src/resolve/resolved-dns-dnssec.c @@ -1583,6 +1583,19 @@ int dnssec_nsec_test(DnsAnswer *answer, DnsResourceKey *key, DnssecNsecResult *r if (r < 0) return r; if (r > 0) { + if (key->type == DNS_TYPE_DS) { + /* If we look for a DS RR and the server sent us the NSEC RR of the child zone + * we have a problem. For DS RRs we want the NSEC RR from the parent */ + if (bitmap_isset(rr->nsec.types, DNS_TYPE_SOA)) + continue; + } else { + /* For all RR types, ensure that if NS is set SOA is set too, so that we know + * we got the child's NSEC. */ + if (bitmap_isset(rr->nsec.types, DNS_TYPE_NS) && + !bitmap_isset(rr->nsec.types, DNS_TYPE_SOA)) + continue; + } + if (bitmap_isset(rr->nsec.types, key->type)) *result = DNSSEC_NSEC_FOUND; else if (bitmap_isset(rr->nsec.types, DNS_TYPE_CNAME)) -- cgit v1.2.3-54-g00ecf From 97c67192eadaffe67b803ec5b991a92bb1137d0b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 14 Jan 2016 18:03:03 +0100 Subject: resolved: when validating an RRset, store information about the synthesizing source and zone in each RR Having this information available is useful when we need to check whether various RRs are suitable for proofs. This information is stored in the RRs as number of labels to skip from the beginning of the owner name to reach the synthesizing source/signer. Simple accessor calls are then added to retrieve the signer/source from the RR using this information. This also moves validation of a a number of RRSIG parameters into a new call dnssec_rrsig_prepare() that as side-effect initializes the two numeric values. --- src/resolve/resolved-dns-dnssec.c | 158 +++++++++++++++++++++++---------- src/resolve/resolved-dns-rr.c | 58 ++++++++++++ src/resolve/resolved-dns-rr.h | 14 +++ src/resolve/resolved-dns-transaction.c | 4 +- src/shared/dns-domain.c | 31 +++++-- src/shared/dns-domain.h | 1 + 6 files changed, 206 insertions(+), 60 deletions(-) (limited to 'src') diff --git a/src/resolve/resolved-dns-dnssec.c b/src/resolve/resolved-dns-dnssec.c index b6fb362daa..37fc3150f0 100644 --- a/src/resolve/resolved-dns-dnssec.c +++ b/src/resolve/resolved-dns-dnssec.c @@ -430,6 +430,57 @@ static void md_add_uint32(gcry_md_hd_t md, uint32_t v) { gcry_md_write(md, &v, sizeof(v)); } +static int dnssec_rrsig_prepare(DnsResourceRecord *rrsig) { + int n_key_labels, n_signer_labels; + const char *name; + int r; + + /* Checks whether the specified RRSIG RR is somewhat valid, and initializes the .n_skip_labels_source and + * .n_skip_labels_signer fields so that we can use them later on. */ + + assert(rrsig); + assert(rrsig->key->type == DNS_TYPE_RRSIG); + + /* Check if this RRSIG RR is already prepared */ + if (rrsig->n_skip_labels_source != (unsigned) -1) + return 0; + + if (rrsig->rrsig.inception > rrsig->rrsig.expiration) + return -EINVAL; + + name = DNS_RESOURCE_KEY_NAME(rrsig->key); + + n_key_labels = dns_name_count_labels(name); + if (n_key_labels < 0) + return n_key_labels; + if (rrsig->rrsig.labels > n_key_labels) + return -EINVAL; + + n_signer_labels = dns_name_count_labels(rrsig->rrsig.signer); + if (n_signer_labels < 0) + return n_signer_labels; + if (n_signer_labels > rrsig->rrsig.labels) + return -EINVAL; + + r = dns_name_skip(name, n_key_labels - n_signer_labels, &name); + if (r < 0) + return r; + if (r == 0) + return -EINVAL; + + /* Check if the signer is really a suffix of us */ + r = dns_name_equal(name, rrsig->rrsig.signer); + if (r < 0) + return r; + if (r == 0) + return -EINVAL; + + rrsig->n_skip_labels_source = n_key_labels - rrsig->rrsig.labels; + rrsig->n_skip_labels_signer = n_key_labels - n_signer_labels; + + return 0; +} + static int dnssec_rrsig_expired(DnsResourceRecord *rrsig, usec_t realtime) { usec_t expiration, inception, skew; @@ -499,6 +550,35 @@ static int algorithm_to_gcrypt_md(uint8_t algorithm) { } } +static void dnssec_fix_rrset_ttl( + DnsResourceRecord *list[], + unsigned n, + DnsResourceRecord *rrsig, + usec_t realtime) { + + unsigned k; + + assert(list); + assert(n > 0); + assert(rrsig); + + for (k = 0; k < n; k++) { + DnsResourceRecord *rr = list[k]; + + /* Pick the TTL as the minimum of the RR's TTL, the + * RR's original TTL according to the RRSIG and the + * RRSIG's own TTL, see RFC 4035, Section 5.3.3 */ + rr->ttl = MIN3(rr->ttl, rrsig->rrsig.original_ttl, rrsig->ttl); + rr->expiry = rrsig->rrsig.expiration * USEC_PER_SEC; + + /* Copy over information about the signer and wildcard source of synthesis */ + rr->n_skip_labels_source = rrsig->n_skip_labels_source; + rr->n_skip_labels_signer = rrsig->n_skip_labels_signer; + } + + rrsig->expiry = rrsig->rrsig.expiration * USEC_PER_SEC; +} + int dnssec_verify_rrset( DnsAnswer *a, const DnsResourceKey *key, @@ -536,6 +616,14 @@ int dnssec_verify_rrset( if (md_algorithm < 0) return md_algorithm; + r = dnssec_rrsig_prepare(rrsig); + if (r == -EINVAL) { + *result = DNSSEC_INVALID; + return r; + } + if (r < 0) + return r; + r = dnssec_rrsig_expired(rrsig, realtime); if (r < 0) return r; @@ -699,12 +787,17 @@ int dnssec_verify_rrset( if (r < 0) goto finish; - if (!r) + /* Now, fix the ttl, expiry, and remember the synthesizing source and the signer */ + if (r > 0) + dnssec_fix_rrset_ttl(list, n, rrsig, realtime); + + if (r == 0) *result = DNSSEC_INVALID; else if (wildcard) *result = DNSSEC_VALIDATED_WILDCARD; else *result = DNSSEC_VALIDATED; + r = 0; finish: @@ -743,8 +836,6 @@ int dnssec_rrsig_match_dnskey(DnsResourceRecord *rrsig, DnsResourceRecord *dnske } int dnssec_key_match_rrsig(const DnsResourceKey *key, DnsResourceRecord *rrsig) { - int r; - assert(key); assert(rrsig); @@ -757,45 +848,9 @@ int dnssec_key_match_rrsig(const DnsResourceKey *key, DnsResourceRecord *rrsig) if (rrsig->rrsig.type_covered != key->type) return 0; - /* Make sure signer is a parent of the RRset */ - r = dns_name_endswith(DNS_RESOURCE_KEY_NAME(rrsig->key), rrsig->rrsig.signer); - if (r <= 0) - return r; - - /* Make sure the owner name has at least as many labels as the "label" fields indicates. */ - r = dns_name_count_labels(DNS_RESOURCE_KEY_NAME(rrsig->key)); - if (r < 0) - return r; - if (r < rrsig->rrsig.labels) - return 0; - return dns_name_equal(DNS_RESOURCE_KEY_NAME(rrsig->key), DNS_RESOURCE_KEY_NAME(key)); } -static int dnssec_fix_rrset_ttl(DnsAnswer *a, const DnsResourceKey *key, DnsResourceRecord *rrsig, usec_t realtime) { - DnsResourceRecord *rr; - int r; - - assert(key); - assert(rrsig); - - DNS_ANSWER_FOREACH(rr, a) { - r = dns_resource_key_equal(key, rr->key); - if (r < 0) - return r; - if (r == 0) - continue; - - /* Pick the TTL as the minimum of the RR's TTL, the - * RR's original TTL according to the RRSIG and the - * RRSIG's own TTL, see RFC 4035, Section 5.3.3 */ - rr->ttl = MIN3(rr->ttl, rrsig->rrsig.original_ttl, rrsig->ttl); - rr->expiry = rrsig->rrsig.expiration * USEC_PER_SEC; - } - - return 0; -} - int dnssec_verify_rrset_search( DnsAnswer *a, const DnsResourceKey *key, @@ -865,10 +920,6 @@ int dnssec_verify_rrset_search( case DNSSEC_VALIDATED_WILDCARD: /* Yay, the RR has been validated, * return immediately, but fix up the expiry */ - r = dnssec_fix_rrset_ttl(a, key, rrsig, realtime); - if (r < 0) - return r; - if (ret_rrsig) *ret_rrsig = rrsig; @@ -1658,15 +1709,17 @@ int dnssec_nsec_test_enclosed(DnsAnswer *answer, uint16_t type, const char *name if (rr->key->type != type && type != DNS_TYPE_ANY) continue; - r = dns_name_endswith(DNS_RESOURCE_KEY_NAME(rr->key), zone); - if (r < 0) - return r; - if (r == 0) - continue; - switch (rr->key->type) { case DNS_TYPE_NSEC: + + /* We only care for NSEC RRs from the indicated zone */ + r = dns_resource_record_is_signer(rr, zone); + if (r < 0) + return r; + if (r == 0) + continue; + r = dns_name_between(DNS_RESOURCE_KEY_NAME(rr->key), name, rr->nsec.next_domain_name); if (r < 0) return r; @@ -1677,6 +1730,13 @@ int dnssec_nsec_test_enclosed(DnsAnswer *answer, uint16_t type, const char *name case DNS_TYPE_NSEC3: { _cleanup_free_ char *hashed_domain = NULL, *next_hashed_domain = NULL; + /* We only care for NSEC3 RRs from the indicated zone */ + r = dns_resource_record_is_signer(rr, zone); + if (r < 0) + return r; + if (r == 0) + continue; + r = nsec3_is_good(rr, NULL); if (r < 0) return r; diff --git a/src/resolve/resolved-dns-rr.c b/src/resolve/resolved-dns-rr.c index dbf840157f..53fd708365 100644 --- a/src/resolve/resolved-dns-rr.c +++ b/src/resolve/resolved-dns-rr.c @@ -344,6 +344,7 @@ DnsResourceRecord* dns_resource_record_new(DnsResourceKey *key) { rr->n_ref = 1; rr->key = dns_resource_key_ref(key); rr->expiry = USEC_INFINITY; + rr->n_skip_labels_signer = rr->n_skip_labels_source = (unsigned) -1; return rr; } @@ -1085,6 +1086,63 @@ int dns_resource_record_to_wire_format(DnsResourceRecord *rr, bool canonical) { return 0; } +int dns_resource_record_signer(DnsResourceRecord *rr, const char **ret) { + const char *n; + int r; + + assert(rr); + assert(ret); + + /* Returns the RRset's signer, if it is known. */ + + if (rr->n_skip_labels_signer == (unsigned) -1) + return -ENODATA; + + n = DNS_RESOURCE_KEY_NAME(rr->key); + r = dns_name_skip(n, rr->n_skip_labels_signer, &n); + if (r < 0) + return r; + if (r == 0) + return -EINVAL; + + *ret = n; + return 0; +} + +int dns_resource_record_source(DnsResourceRecord *rr, const char **ret) { + const char *n; + int r; + + assert(rr); + assert(ret); + + /* Returns the RRset's synthesizing source, if it is known. */ + + if (rr->n_skip_labels_source == (unsigned) -1) + return -ENODATA; + + n = DNS_RESOURCE_KEY_NAME(rr->key); + r = dns_name_skip(n, rr->n_skip_labels_source, &n); + if (r < 0) + return r; + if (r == 0) + return -EINVAL; + + *ret = n; + return 0; +} + +int dns_resource_record_is_signer(DnsResourceRecord *rr, const char *zone) { + const char *signer; + int r; + + r = dns_resource_record_signer(rr, &signer); + if (r < 0) + return r; + + return dns_name_equal(zone, signer); +} + static void dns_resource_record_hash_func(const void *i, struct siphash *state) { const DnsResourceRecord *rr = i; diff --git a/src/resolve/resolved-dns-rr.h b/src/resolve/resolved-dns-rr.h index fe29a41566..8e7bfaa7c7 100644 --- a/src/resolve/resolved-dns-rr.h +++ b/src/resolve/resolved-dns-rr.h @@ -108,14 +108,24 @@ struct DnsTxtItem { struct DnsResourceRecord { unsigned n_ref; DnsResourceKey *key; + char *to_string; + uint32_t ttl; usec_t expiry; /* RRSIG signature expiry */ + + /* How many labels to strip to determine "signer" of the RRSIG (aka, the zone). -1 if not signed. */ + unsigned n_skip_labels_signer; + /* How many labels to strip to determine "synthesizing source" of this RR, i.e. the wildcard's immediate parent. -1 if not signed. */ + unsigned n_skip_labels_source; + bool unparseable:1; + bool wire_format_canonical:1; void *wire_format; size_t wire_format_size; size_t wire_format_rdata_offset; + union { struct { void *data; @@ -296,6 +306,10 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(DnsResourceRecord*, dns_resource_record_unref); int dns_resource_record_to_wire_format(DnsResourceRecord *rr, bool canonical); +int dns_resource_record_signer(DnsResourceRecord *rr, const char **ret); +int dns_resource_record_source(DnsResourceRecord *rr, const char **ret); +int dns_resource_record_is_signer(DnsResourceRecord *rr, const char *zone); + DnsTxtItem *dns_txt_item_free_all(DnsTxtItem *i); bool dns_txt_item_equal(DnsTxtItem *a, DnsTxtItem *b); diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index c7d2d82ecf..8fe581b33c 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -2537,11 +2537,9 @@ int dns_transaction_validate_dnssec(DnsTransaction *t) { * that no matching non-wildcard RR exists.*/ /* First step, determine the source of synthesis */ - r = dns_name_suffix(DNS_RESOURCE_KEY_NAME(rr->key), rrsig->rrsig.labels, &source); + r = dns_resource_record_source(rrsig, &source); if (r < 0) return r; - if (r == 0) - return -EBADMSG; r = dnssec_test_positive_wildcard( validated, diff --git a/src/shared/dns-domain.c b/src/shared/dns-domain.c index d1fb97fe92..ee0108715d 100644 --- a/src/shared/dns-domain.c +++ b/src/shared/dns-domain.c @@ -1189,6 +1189,26 @@ int dns_name_suffix(const char *name, unsigned n_labels, const char **ret) { return (int) (n - n_labels); } +int dns_name_skip(const char *a, unsigned n_labels, const char **ret) { + int r; + + assert(a); + assert(ret); + + for (; n_labels > 0; n_labels --) { + r = dns_name_parent(&a); + if (r < 0) + return r; + if (r == 0) { + *ret = ""; + return 0; + } + } + + *ret = a; + return 1; +} + int dns_name_count_labels(const char *name) { unsigned n = 0; const char *p; @@ -1219,14 +1239,9 @@ int dns_name_equal_skip(const char *a, unsigned n_labels, const char *b) { assert(a); assert(b); - while (n_labels > 0) { - - r = dns_name_parent(&a); - if (r <= 0) - return r; - - n_labels --; - } + r = dns_name_skip(a, n_labels, &a); + if (r <= 0) + return r; return dns_name_equal(a, b); } diff --git a/src/shared/dns-domain.h b/src/shared/dns-domain.h index 4fbe0a618f..a679d40958 100644 --- a/src/shared/dns-domain.h +++ b/src/shared/dns-domain.h @@ -104,4 +104,5 @@ int dns_service_split(const char *joined, char **name, char **type, char **domai int dns_name_suffix(const char *name, unsigned n_labels, const char **ret); int dns_name_count_labels(const char *name); +int dns_name_skip(const char *a, unsigned n_labels, const char **ret); int dns_name_equal_skip(const char *a, unsigned n_labels, const char *b); -- cgit v1.2.3-54-g00ecf From 93a3b9294f7fa98ee10c66163f86cd0232728453 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 14 Jan 2016 18:14:43 +0100 Subject: resolved: be stricter when using NSEC3 We can user signer and synthesizing source information to check that the NSEC3 RRs we want to use are actually reasonable and properly signed. --- src/resolve/resolved-dns-dnssec.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/resolve/resolved-dns-dnssec.c b/src/resolve/resolved-dns-dnssec.c index 37fc3150f0..2202daafc0 100644 --- a/src/resolve/resolved-dns-dnssec.c +++ b/src/resolve/resolved-dns-dnssec.c @@ -1157,7 +1157,6 @@ int dnssec_verify_dnskey_search(DnsResourceRecord *dnskey, DnsAnswer *validated_ if (ds->key->type != DNS_TYPE_DS) continue; - if (ds->key->class != dnskey->key->class) continue; @@ -1286,6 +1285,13 @@ static int nsec3_is_good(DnsResourceRecord *rr, DnsResourceRecord *nsec3) { if (rr->nsec3.iterations > NSEC3_ITERATIONS_MAX) return 0; + /* Ignore NSEC3 RRs generated from wildcards */ + if (rr->n_skip_labels_source != 0) + return 0; + /* Ignore NSEC3 RRs that are located anywhere else than one label below the zone */ + if (rr->n_skip_labels_signer != 1) + return 0; + if (!nsec3) return 1; @@ -1319,6 +1325,7 @@ static int nsec3_is_good(DnsResourceRecord *rr, DnsResourceRecord *nsec3) { if (r == 0) return 0; + /* Make sure both have the same parent */ return dns_name_equal(a, b); } -- cgit v1.2.3-54-g00ecf From 96bb76734d8e1c8520a2456901079610813eac6d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 14 Jan 2016 20:11:11 +0100 Subject: resolved: rename dnssec_verify_dnskey() → dnssec_verify_dnskey_by_ds() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This should clarify that this is not regular signature-based validation, but validation through DS RR fingerprints. --- src/resolve/resolved-dns-dnssec.c | 6 +++--- src/resolve/resolved-dns-dnssec.h | 4 ++-- src/resolve/resolved-dns-transaction.c | 2 +- src/resolve/resolved-dns-trust-anchor.c | 2 +- src/resolve/test-dnssec.c | 4 ++-- 5 files changed, 9 insertions(+), 9 deletions(-) (limited to 'src') diff --git a/src/resolve/resolved-dns-dnssec.c b/src/resolve/resolved-dns-dnssec.c index 2202daafc0..1ee4aa5b36 100644 --- a/src/resolve/resolved-dns-dnssec.c +++ b/src/resolve/resolved-dns-dnssec.c @@ -1070,7 +1070,7 @@ static int digest_to_gcrypt_md(uint8_t algorithm) { } } -int dnssec_verify_dnskey(DnsResourceRecord *dnskey, DnsResourceRecord *ds, bool mask_revoke) { +int dnssec_verify_dnskey_by_ds(DnsResourceRecord *dnskey, DnsResourceRecord *ds, bool mask_revoke) { char owner_name[DNSSEC_CANONICAL_HOSTNAME_MAX]; gcry_md_hd_t md = NULL; size_t hash_size; @@ -1140,7 +1140,7 @@ finish: return r; } -int dnssec_verify_dnskey_search(DnsResourceRecord *dnskey, DnsAnswer *validated_ds) { +int dnssec_verify_dnskey_by_ds_search(DnsResourceRecord *dnskey, DnsAnswer *validated_ds) { DnsResourceRecord *ds; DnsAnswerFlags flags; int r; @@ -1166,7 +1166,7 @@ int dnssec_verify_dnskey_search(DnsResourceRecord *dnskey, DnsAnswer *validated_ if (r == 0) continue; - r = dnssec_verify_dnskey(dnskey, ds, false); + r = dnssec_verify_dnskey_by_ds(dnskey, ds, false); if (IN_SET(r, -EKEYREJECTED, -EOPNOTSUPP)) return 0; /* The DNSKEY is revoked or otherwise invalid, or we don't support the digest algorithm */ if (r < 0) diff --git a/src/resolve/resolved-dns-dnssec.h b/src/resolve/resolved-dns-dnssec.h index b9d32db120..955017e8cb 100644 --- a/src/resolve/resolved-dns-dnssec.h +++ b/src/resolve/resolved-dns-dnssec.h @@ -61,8 +61,8 @@ int dnssec_key_match_rrsig(const DnsResourceKey *key, DnsResourceRecord *rrsig); int dnssec_verify_rrset(DnsAnswer *answer, const DnsResourceKey *key, DnsResourceRecord *rrsig, DnsResourceRecord *dnskey, usec_t realtime, DnssecResult *result); int dnssec_verify_rrset_search(DnsAnswer *answer, const DnsResourceKey *key, DnsAnswer *validated_dnskeys, usec_t realtime, DnssecResult *result, DnsResourceRecord **rrsig); -int dnssec_verify_dnskey(DnsResourceRecord *dnskey, DnsResourceRecord *ds, bool mask_revoke); -int dnssec_verify_dnskey_search(DnsResourceRecord *dnskey, DnsAnswer *validated_ds); +int dnssec_verify_dnskey_by_ds(DnsResourceRecord *dnskey, DnsResourceRecord *ds, bool mask_revoke); +int dnssec_verify_dnskey_by_ds_search(DnsResourceRecord *dnskey, DnsAnswer *validated_ds); int dnssec_has_rrsig(DnsAnswer *a, const DnsResourceKey *key); diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index 8fe581b33c..ef38812c85 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -1950,7 +1950,7 @@ static int dns_transaction_validate_dnskey_by_ds(DnsTransaction *t) { DNS_ANSWER_FOREACH_IFINDEX(rr, ifindex, t->answer) { - r = dnssec_verify_dnskey_search(rr, t->validated_keys); + r = dnssec_verify_dnskey_by_ds_search(rr, t->validated_keys); if (r < 0) return r; if (r == 0) diff --git a/src/resolve/resolved-dns-trust-anchor.c b/src/resolve/resolved-dns-trust-anchor.c index 9bee44b5c7..02d7ac91e1 100644 --- a/src/resolve/resolved-dns-trust-anchor.c +++ b/src/resolve/resolved-dns-trust-anchor.c @@ -665,7 +665,7 @@ static int dns_trust_anchor_check_revoked_one(DnsTrustAnchor *d, DnsResourceReco * DS fingerprint will be the one of the * unrevoked DNSKEY, but the one we got passed * here has the bit set. */ - r = dnssec_verify_dnskey(revoked_dnskey, anchor, true); + r = dnssec_verify_dnskey_by_ds(revoked_dnskey, anchor, true); if (r < 0) return r; if (r == 0) diff --git a/src/resolve/test-dnssec.c b/src/resolve/test-dnssec.c index 0c9efde1fe..45fe1997e2 100644 --- a/src/resolve/test-dnssec.c +++ b/src/resolve/test-dnssec.c @@ -270,8 +270,8 @@ static void test_dnssec_verify_dns_key(void) { log_info("DNSKEY: %s", strna(dns_resource_record_to_string(dnskey))); log_info("DNSKEY keytag: %u", dnssec_keytag(dnskey, false)); - assert_se(dnssec_verify_dnskey(dnskey, ds1, false) > 0); - assert_se(dnssec_verify_dnskey(dnskey, ds2, false) > 0); + assert_se(dnssec_verify_dnskey_by_ds(dnskey, ds1, false) > 0); + assert_se(dnssec_verify_dnskey_by_ds(dnskey, ds2, false) > 0); } static void test_dnssec_canonicalize_one(const char *original, const char *canonical, int r) { -- cgit v1.2.3-54-g00ecf From b9282bc12840aff500a334836226f6b8df24926d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 14 Jan 2016 20:12:29 +0100 Subject: resolved: on negative NODATA replies, properly deal with empty non-terminals empty non-terminals generally lack NSEC RRs, which means we can deduce their existance only from the fact that there are other RRs that contain them in their suffix. Specifically, the NSEC proof for NODATA on ENTs works by sending the NSEC whose next name is a suffix of the queried name to the client. Use this information properly. --- src/resolve/resolved-dns-dnssec.c | 63 +++++++++++++++++++++++++++++++--- src/shared/dns-domain.c | 72 +++++++++++++++++++++++++++++++++++---- src/shared/dns-domain.h | 2 ++ src/test/test-dns-domain.c | 21 ++++++++++++ 4 files changed, 146 insertions(+), 12 deletions(-) (limited to 'src') diff --git a/src/resolve/resolved-dns-dnssec.c b/src/resolve/resolved-dns-dnssec.c index 1ee4aa5b36..69ed7afaf7 100644 --- a/src/resolve/resolved-dns-dnssec.c +++ b/src/resolve/resolved-dns-dnssec.c @@ -1617,10 +1617,47 @@ found_closest_encloser: return 0; } +static int dnssec_nsec_in_path(DnsResourceRecord *rr, const char *name) { + const char *nn, *common_suffix; + int r; + + assert(rr); + assert(rr->key->type == DNS_TYPE_NSEC); + + /* Checks whether the specified nsec RR indicates that name is an empty non-terminal (ENT) + * + * A couple of examples: + * + * NSEC bar → waldo.foo.bar: indicates that foo.bar exists and is an ENT + * NSEC waldo.foo.bar → yyy.zzz.xoo.bar: indicates that xoo.bar and zzz.xoo.bar exist and are ENTs + * NSEC yyy.zzz.xoo.bar → bar: indicates pretty much nothing about ENTs + */ + + /* First, determine parent of next domain. */ + nn = rr->nsec.next_domain_name; + r = dns_name_parent(&nn); + if (r <= 0) + return r; + + /* If the name we just determined is not equal or child of the name we are interested in, then we can't say + * anything at all. */ + r = dns_name_endswith(nn, name); + if (r <= 0) + return r; + + /* If the name we we are interested in is not a prefix of the common suffix of the NSEC RR's owner and next domain names, then we can't say anything either. */ + r = dns_name_common_suffix(DNS_RESOURCE_KEY_NAME(rr->key), rr->nsec.next_domain_name, &common_suffix); + if (r < 0) + return r; + + return dns_name_endswith(name, common_suffix); +} + int dnssec_nsec_test(DnsAnswer *answer, DnsResourceKey *key, DnssecNsecResult *result, bool *authenticated, uint32_t *ttl) { - DnsResourceRecord *rr; bool have_nsec3 = false; + DnsResourceRecord *rr; DnsAnswerFlags flags; + const char *name; int r; assert(key); @@ -1628,6 +1665,8 @@ int dnssec_nsec_test(DnsAnswer *answer, DnsResourceKey *key, DnssecNsecResult *r /* Look for any NSEC/NSEC3 RRs that say something about the specified key. */ + name = DNS_RESOURCE_KEY_NAME(key); + DNS_ANSWER_FOREACH_FLAGS(rr, flags, answer) { if (rr->key->class != key->class) @@ -1637,7 +1676,7 @@ int dnssec_nsec_test(DnsAnswer *answer, DnsResourceKey *key, DnssecNsecResult *r case DNS_TYPE_NSEC: - r = dns_name_equal(DNS_RESOURCE_KEY_NAME(rr->key), DNS_RESOURCE_KEY_NAME(key)); + r = dns_name_equal(DNS_RESOURCE_KEY_NAME(rr->key), name); if (r < 0) return r; if (r > 0) { @@ -1669,11 +1708,13 @@ int dnssec_nsec_test(DnsAnswer *answer, DnsResourceKey *key, DnssecNsecResult *r return 0; } - r = dns_name_between(DNS_RESOURCE_KEY_NAME(rr->key), DNS_RESOURCE_KEY_NAME(key), rr->nsec.next_domain_name); + /* Check if the name we are looking for is an empty non-terminal within the owner or next name + * of the NSEC RR. */ + r = dnssec_nsec_in_path(rr, name); if (r < 0) return r; if (r > 0) { - *result = DNSSEC_NSEC_NXDOMAIN; + *result = DNSSEC_NSEC_NODATA; if (authenticated) *authenticated = flags & DNS_ANSWER_AUTHENTICATED; @@ -1682,7 +1723,19 @@ int dnssec_nsec_test(DnsAnswer *answer, DnsResourceKey *key, DnssecNsecResult *r return 0; } - break; + + r = dns_name_between(DNS_RESOURCE_KEY_NAME(rr->key), name, rr->nsec.next_domain_name); + if (r < 0) + return r; + if (r > 0) + *result = DNSSEC_NSEC_NXDOMAIN; + + if (authenticated) + *authenticated = flags & DNS_ANSWER_AUTHENTICATED; + if (ttl) + *ttl = rr->ttl; + + return 0; case DNS_TYPE_NSEC3: have_nsec3 = true; diff --git a/src/shared/dns-domain.c b/src/shared/dns-domain.c index ee0108715d..04624734dc 100644 --- a/src/shared/dns-domain.c +++ b/src/shared/dns-domain.c @@ -1157,22 +1157,20 @@ finish: return 0; } -int dns_name_suffix(const char *name, unsigned n_labels, const char **ret) { - const char* labels[DNS_N_LABELS_MAX+1]; - unsigned n = 0; +static int dns_name_build_suffix_table(const char *name, const char*table[]) { const char *p; + unsigned n = 0; int r; assert(name); - assert(ret); + assert(table); p = name; for (;;) { if (n > DNS_N_LABELS_MAX) return -EINVAL; - labels[n] = p; - + table[n] = p; r = dns_name_parent(&p); if (r < 0) return r; @@ -1182,7 +1180,21 @@ int dns_name_suffix(const char *name, unsigned n_labels, const char **ret) { n++; } - if (n < n_labels) + return (int) n; +} + +int dns_name_suffix(const char *name, unsigned n_labels, const char **ret) { + const char* labels[DNS_N_LABELS_MAX+1]; + int n; + + assert(name); + assert(ret); + + n = dns_name_build_suffix_table(name, labels); + if (n < 0) + return n; + + if ((unsigned) n < n_labels) return -EINVAL; *ret = labels[n - n_labels]; @@ -1245,3 +1257,49 @@ int dns_name_equal_skip(const char *a, unsigned n_labels, const char *b) { return dns_name_equal(a, b); } + +int dns_name_common_suffix(const char *a, const char *b, const char **ret) { + const char *a_labels[DNS_N_LABELS_MAX+1], *b_labels[DNS_N_LABELS_MAX+1]; + int n = 0, m = 0, k = 0, r, q; + + assert(a); + assert(b); + assert(ret); + + /* Determines the common suffix of domain names a and b */ + + n = dns_name_build_suffix_table(a, a_labels); + if (n < 0) + return n; + + m = dns_name_build_suffix_table(b, b_labels); + if (m < 0) + return m; + + for (;;) { + char la[DNS_LABEL_MAX], lb[DNS_LABEL_MAX]; + const char *x, *y; + + if (k >= n || k >= m) { + *ret = a_labels[n - k]; + return 0; + } + + x = a_labels[n - 1 - k]; + r = dns_label_unescape_undo_idna(&x, la, sizeof(la)); + if (r < 0) + return r; + + y = b_labels[m - 1 - k]; + q = dns_label_unescape_undo_idna(&y, lb, sizeof(lb)); + if (q < 0) + return q; + + if (r != q || ascii_strcasecmp_n(la, lb, r) != 0) { + *ret = a_labels[n - k]; + return 0; + } + + k++; + } +} diff --git a/src/shared/dns-domain.h b/src/shared/dns-domain.h index a679d40958..5f9542ef98 100644 --- a/src/shared/dns-domain.h +++ b/src/shared/dns-domain.h @@ -106,3 +106,5 @@ int dns_name_count_labels(const char *name); int dns_name_skip(const char *a, unsigned n_labels, const char **ret); int dns_name_equal_skip(const char *a, unsigned n_labels, const char *b); + +int dns_name_common_suffix(const char *a, const char *b, const char **ret); diff --git a/src/test/test-dns-domain.c b/src/test/test-dns-domain.c index fe3ae45349..987f1fc887 100644 --- a/src/test/test-dns-domain.c +++ b/src/test/test-dns-domain.c @@ -578,6 +578,26 @@ static void test_dns_name_compare_func(void) { assert_se(dns_name_compare_func("de.", "heise.de") != 0); } +static void test_dns_name_common_suffix_one(const char *a, const char *b, const char *result) { + const char *c; + + assert_se(dns_name_common_suffix(a, b, &c) >= 0); + assert_se(streq(c, result)); +} + +static void test_dns_name_common_suffix(void) { + test_dns_name_common_suffix_one("", "", ""); + test_dns_name_common_suffix_one("foo", "", ""); + test_dns_name_common_suffix_one("", "foo", ""); + test_dns_name_common_suffix_one("foo", "bar", ""); + test_dns_name_common_suffix_one("bar", "foo", ""); + test_dns_name_common_suffix_one("foo", "foo", "foo"); + test_dns_name_common_suffix_one("quux.foo", "foo", "foo"); + test_dns_name_common_suffix_one("foo", "quux.foo", "foo"); + test_dns_name_common_suffix_one("this.is.a.short.sentence", "this.is.another.short.sentence", "short.sentence"); + test_dns_name_common_suffix_one("FOO.BAR", "tEST.bAR", "BAR"); +} + int main(int argc, char *argv[]) { test_dns_label_unescape(); @@ -603,6 +623,7 @@ int main(int argc, char *argv[]) { test_dns_name_count_labels(); test_dns_name_equal_skip(); test_dns_name_compare_func(); + test_dns_name_common_suffix(); return 0; } -- cgit v1.2.3-54-g00ecf From d86c982a3476bcff39a196868c835309c7a6c7fc Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 14 Jan 2016 21:05:57 +0100 Subject: resolved: make sure the NSEC proof-of-non-existance check also looks for wildcard domains --- src/resolve/resolved-dns-dnssec.c | 75 +++++++++++++++++++++++++++++++++------ 1 file changed, 64 insertions(+), 11 deletions(-) (limited to 'src') diff --git a/src/resolve/resolved-dns-dnssec.c b/src/resolve/resolved-dns-dnssec.c index 69ed7afaf7..ac274843f0 100644 --- a/src/resolve/resolved-dns-dnssec.c +++ b/src/resolve/resolved-dns-dnssec.c @@ -1617,7 +1617,7 @@ found_closest_encloser: return 0; } -static int dnssec_nsec_in_path(DnsResourceRecord *rr, const char *name) { +static int dnssec_nsec_test_in_path(DnsResourceRecord *rr, const char *name) { const char *nn, *common_suffix; int r; @@ -1653,9 +1653,38 @@ static int dnssec_nsec_in_path(DnsResourceRecord *rr, const char *name) { return dns_name_endswith(name, common_suffix); } +static int dns_dnssec_test_wildcard_at_closest_encloser(DnsResourceRecord *rr, const char *name) { + const char *common_suffix, *wc; + int r; + + assert(rr); + assert(rr->key->type == DNS_TYPE_NSEC); + + /* Checks whether the "Wildcard at the Closest Encloser" is within the space covered by the specified + * RR. Specifically, checks whether 'name' has the common suffix of the NSEC RR's owner and next names as + * suffix, and whether the NSEC covers the name generated by that suffix prepended with an asterisk label. + * + * NSEC bar → waldo.foo.bar: indicates that *.bar and *.foo.bar do not exist + * NSEC waldo.foo.bar → yyy.zzz.xoo.bar: indicates that *.xoo.bar and *.zzz.xoo.bar do not exist (and more ...) + * 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); + 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); + if (r <= 0) + return r; + + wc = strjoina("*.", common_suffix, NULL); + return dns_name_between(DNS_RESOURCE_KEY_NAME(rr->key), wc, rr->nsec.next_domain_name); +} + int dnssec_nsec_test(DnsAnswer *answer, DnsResourceKey *key, DnssecNsecResult *result, bool *authenticated, uint32_t *ttl) { - bool have_nsec3 = false; - DnsResourceRecord *rr; + bool have_nsec3 = false, covering_rr_authenticated = false, wildcard_rr_authenticated = false; + DnsResourceRecord *rr, *covering_rr = NULL, *wildcard_rr = NULL; DnsAnswerFlags flags; const char *name; int r; @@ -1708,9 +1737,13 @@ int dnssec_nsec_test(DnsAnswer *answer, DnsResourceKey *key, DnssecNsecResult *r return 0; } + /* The following three checks only make sense for NSEC RRs that are not expanded from a wildcard */ + if (rr->n_skip_labels_source != 0) + continue; + /* Check if the name we are looking for is an empty non-terminal within the owner or next name * of the NSEC RR. */ - r = dnssec_nsec_in_path(rr, name); + r = dnssec_nsec_test_in_path(rr, name); if (r < 0) return r; if (r > 0) { @@ -1724,18 +1757,25 @@ int dnssec_nsec_test(DnsAnswer *answer, DnsResourceKey *key, DnssecNsecResult *r return 0; } + /* Check if this NSEC RR proves the absence of an explicit RR under this name */ r = dns_name_between(DNS_RESOURCE_KEY_NAME(rr->key), name, rr->nsec.next_domain_name); if (r < 0) return r; - if (r > 0) - *result = DNSSEC_NSEC_NXDOMAIN; + if (r > 0 && (!covering_rr || !covering_rr_authenticated)) { + covering_rr = rr; + covering_rr_authenticated = flags & DNS_ANSWER_AUTHENTICATED; + } - if (authenticated) - *authenticated = flags & DNS_ANSWER_AUTHENTICATED; - if (ttl) - *ttl = rr->ttl; + /* Check if this NSEC RR proves the absence of a wildcard RR under this name */ + r = dns_dnssec_test_wildcard_at_closest_encloser(rr, name); + if (r < 0) + return r; + if (r > 0 && (!wildcard_rr || !wildcard_rr_authenticated)) { + wildcard_rr = rr; + wildcard_rr_authenticated = flags & DNS_ANSWER_AUTHENTICATED; + } - return 0; + break; case DNS_TYPE_NSEC3: have_nsec3 = true; @@ -1743,6 +1783,19 @@ int dnssec_nsec_test(DnsAnswer *answer, DnsResourceKey *key, DnssecNsecResult *r } } + if (covering_rr && wildcard_rr) { + /* If we could prove that neither the name itself, nor the wildcard at the closest encloser exists, we + * proved the NXDOMAIN case. */ + *result = DNSSEC_NSEC_NXDOMAIN; + + if (authenticated) + *authenticated = covering_rr_authenticated && wildcard_rr_authenticated; + if (ttl) + *ttl = MIN(covering_rr->ttl, wildcard_rr->ttl); + + return 0; + } + /* OK, this was not sufficient. Let's see if NSEC3 can help. */ if (have_nsec3) return dnssec_test_nsec3(answer, key, result, authenticated, ttl); -- cgit v1.2.3-54-g00ecf From ab481675f98d3d3f12e7e48ba6d2159123b9c7bf Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 15 Jan 2016 02:21:22 +0100 Subject: resolved: complete NSEC non-existance proofs This fills in the last few gaps: - When checking if a domain is non-existing, also check that no wildcard for it exists - Ensure we don't base "covering" tests on NSEC RRs from a parent zone - Refuse to accept expanded wildcard NSEC RRs for absence proofs. --- src/resolve/resolved-dns-dnssec.c | 229 +++++++++++++++++++++++++++----------- src/resolve/resolved-dns-rr.c | 25 +++++ src/resolve/resolved-dns-rr.h | 1 + 3 files changed, 187 insertions(+), 68 deletions(-) (limited to 'src') diff --git a/src/resolve/resolved-dns-dnssec.c b/src/resolve/resolved-dns-dnssec.c index ac274843f0..2ac085dfd3 100644 --- a/src/resolve/resolved-dns-dnssec.c +++ b/src/resolve/resolved-dns-dnssec.c @@ -1617,7 +1617,30 @@ found_closest_encloser: return 0; } -static int dnssec_nsec_test_in_path(DnsResourceRecord *rr, const char *name) { +static int dnssec_nsec_wildcard_equal(DnsResourceRecord *rr, const char *name) { + char label[DNS_LABEL_MAX]; + const char *n; + int r; + + assert(rr); + assert(rr->key->type == DNS_TYPE_NSEC); + + /* Checks whether the specified RR has a name beginning in "*.", and if the rest is a suffix of our name */ + + if (rr->n_skip_labels_source != 1) + return 0; + + n = DNS_RESOURCE_KEY_NAME(rr->key); + r = dns_label_unescape(&n, label, sizeof(label)); + if (r <= 0) + return r; + if (r != 1 || label[0] != '*') + return 0; + + return dns_name_endswith(name, n); +} + +static int dnssec_nsec_in_path(DnsResourceRecord *rr, const char *name) { const char *nn, *common_suffix; int r; @@ -1653,7 +1676,66 @@ static int dnssec_nsec_test_in_path(DnsResourceRecord *rr, const char *name) { return dns_name_endswith(name, common_suffix); } -static int dns_dnssec_test_wildcard_at_closest_encloser(DnsResourceRecord *rr, const char *name) { +static int dnssec_nsec_from_parent_zone(DnsResourceRecord *rr, const char *name) { + int r; + + assert(rr); + assert(rr->key->type == DNS_TYPE_NSEC); + + /* Checks whether this NSEC originates to the parent zone or the child zone. */ + + r = dns_name_parent(&name); + if (r <= 0) + return r; + + r = dns_name_equal(name, DNS_RESOURCE_KEY_NAME(rr->key)); + if (r <= 0) + return r; + + /* DNAME, and NS without SOA is an indication for a delegation. */ + if (bitmap_isset(rr->nsec.types, DNS_TYPE_DNAME)) + return 1; + + if (bitmap_isset(rr->nsec.types, DNS_TYPE_NS) && !bitmap_isset(rr->nsec.types, DNS_TYPE_SOA)) + return 1; + + return 0; +} + +static int dnssec_nsec_covers(DnsResourceRecord *rr, const char *name) { + const char *common_suffix, *p; + 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. */ + + r = dns_name_common_suffix(DNS_RESOURCE_KEY_NAME(rr->key), rr->nsec.next_domain_name, &common_suffix); + 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". */ + + return dns_name_between(DNS_RESOURCE_KEY_NAME(rr->key), p, rr->nsec.next_domain_name); +} + +static int dnssec_nsec_covers_wildcard(DnsResourceRecord *rr, const char *name) { const char *common_suffix, *wc; int r; @@ -1701,85 +1783,96 @@ int dnssec_nsec_test(DnsAnswer *answer, DnsResourceKey *key, DnssecNsecResult *r if (rr->key->class != key->class) continue; - switch (rr->key->type) { + have_nsec3 = have_nsec3 || (rr->key->type == DNS_TYPE_NSEC3); - case DNS_TYPE_NSEC: + if (rr->key->type != DNS_TYPE_NSEC) + continue; - r = dns_name_equal(DNS_RESOURCE_KEY_NAME(rr->key), name); + /* The following checks only make sense for NSEC RRs that are not expanded from a wildcard */ + r = dns_resource_record_is_synthetic(rr); + if (r < 0) + return r; + if (r > 0) + continue; + + /* Check if this is a direct match. If so, we have encountered a NODATA case */ + r = dns_name_equal(DNS_RESOURCE_KEY_NAME(rr->key), name); + if (r < 0) + return r; + if (r == 0) { + /* If it's not a direct match, maybe it's a wild card match? */ + r = dnssec_nsec_wildcard_equal(rr, name); if (r < 0) return r; - if (r > 0) { - if (key->type == DNS_TYPE_DS) { - /* If we look for a DS RR and the server sent us the NSEC RR of the child zone - * we have a problem. For DS RRs we want the NSEC RR from the parent */ - if (bitmap_isset(rr->nsec.types, DNS_TYPE_SOA)) - continue; - } else { - /* For all RR types, ensure that if NS is set SOA is set too, so that we know - * we got the child's NSEC. */ - if (bitmap_isset(rr->nsec.types, DNS_TYPE_NS) && - !bitmap_isset(rr->nsec.types, DNS_TYPE_SOA)) - continue; - } - - if (bitmap_isset(rr->nsec.types, key->type)) - *result = DNSSEC_NSEC_FOUND; - else if (bitmap_isset(rr->nsec.types, DNS_TYPE_CNAME)) - *result = DNSSEC_NSEC_CNAME; - else - *result = DNSSEC_NSEC_NODATA; - - if (authenticated) - *authenticated = flags & DNS_ANSWER_AUTHENTICATED; - if (ttl) - *ttl = rr->ttl; - - return 0; + } + if (r > 0) { + if (key->type == DNS_TYPE_DS) { + /* If we look for a DS RR and the server sent us the NSEC RR of the child zone + * we have a problem. For DS RRs we want the NSEC RR from the parent */ + if (bitmap_isset(rr->nsec.types, DNS_TYPE_SOA)) + continue; + } else { + /* For all RR types, ensure that if NS is set SOA is set too, so that we know + * we got the child's NSEC. */ + if (bitmap_isset(rr->nsec.types, DNS_TYPE_NS) && + !bitmap_isset(rr->nsec.types, DNS_TYPE_SOA)) + continue; } - /* The following three checks only make sense for NSEC RRs that are not expanded from a wildcard */ - if (rr->n_skip_labels_source != 0) - continue; - - /* Check if the name we are looking for is an empty non-terminal within the owner or next name - * of the NSEC RR. */ - r = dnssec_nsec_test_in_path(rr, name); - if (r < 0) - return r; - if (r > 0) { + if (bitmap_isset(rr->nsec.types, key->type)) + *result = DNSSEC_NSEC_FOUND; + else if (bitmap_isset(rr->nsec.types, DNS_TYPE_CNAME)) + *result = DNSSEC_NSEC_CNAME; + else *result = DNSSEC_NSEC_NODATA; - if (authenticated) - *authenticated = flags & DNS_ANSWER_AUTHENTICATED; - if (ttl) - *ttl = rr->ttl; + if (authenticated) + *authenticated = flags & DNS_ANSWER_AUTHENTICATED; + if (ttl) + *ttl = rr->ttl; - return 0; - } + return 0; + } - /* Check if this NSEC RR proves the absence of an explicit RR under this name */ - r = dns_name_between(DNS_RESOURCE_KEY_NAME(rr->key), name, rr->nsec.next_domain_name); - if (r < 0) - return r; - if (r > 0 && (!covering_rr || !covering_rr_authenticated)) { - covering_rr = rr; - covering_rr_authenticated = flags & DNS_ANSWER_AUTHENTICATED; - } + /* Check if the name we are looking for is an empty non-terminal within the owner or next name + * of the NSEC RR. */ + r = dnssec_nsec_in_path(rr, name); + if (r < 0) + return r; + if (r > 0) { + *result = DNSSEC_NSEC_NODATA; - /* Check if this NSEC RR proves the absence of a wildcard RR under this name */ - r = dns_dnssec_test_wildcard_at_closest_encloser(rr, name); - if (r < 0) - return r; - if (r > 0 && (!wildcard_rr || !wildcard_rr_authenticated)) { - wildcard_rr = rr; - wildcard_rr_authenticated = flags & DNS_ANSWER_AUTHENTICATED; - } + if (authenticated) + *authenticated = flags & DNS_ANSWER_AUTHENTICATED; + if (ttl) + *ttl = rr->ttl; - break; + return 0; + } - case DNS_TYPE_NSEC3: - have_nsec3 = true; - break; + /* The following two "covering" checks, are not useful if the NSEC is from the parent */ + r = dnssec_nsec_from_parent_zone(rr, name); + if (r < 0) + return r; + if (r > 0) + continue; + + /* Check if this NSEC RR proves the absence of an explicit RR under this name */ + r = dnssec_nsec_covers(rr, name); + if (r < 0) + return r; + if (r > 0 && (!covering_rr || !covering_rr_authenticated)) { + covering_rr = rr; + covering_rr_authenticated = flags & DNS_ANSWER_AUTHENTICATED; + } + + /* Check if this NSEC RR proves the absence of a wildcard RR under this name */ + r = dnssec_nsec_covers_wildcard(rr, name); + if (r < 0) + return r; + if (r > 0 && (!wildcard_rr || !wildcard_rr_authenticated)) { + wildcard_rr = rr; + wildcard_rr_authenticated = flags & DNS_ANSWER_AUTHENTICATED; } } diff --git a/src/resolve/resolved-dns-rr.c b/src/resolve/resolved-dns-rr.c index 53fd708365..02c6b239d5 100644 --- a/src/resolve/resolved-dns-rr.c +++ b/src/resolve/resolved-dns-rr.c @@ -1136,6 +1136,8 @@ int dns_resource_record_is_signer(DnsResourceRecord *rr, const char *zone) { const char *signer; int r; + assert(rr); + r = dns_resource_record_signer(rr, &signer); if (r < 0) return r; @@ -1143,6 +1145,29 @@ int dns_resource_record_is_signer(DnsResourceRecord *rr, const char *zone) { return dns_name_equal(zone, signer); } +int dns_resource_record_is_synthetic(DnsResourceRecord *rr) { + int r; + + assert(rr); + + /* Returns > 0 if the RR is generated from a wildcard, and is not the asterisk name itself */ + + if (rr->n_skip_labels_source == (unsigned) -1) + return -ENODATA; + + if (rr->n_skip_labels_source == 0) + return 0; + + if (rr->n_skip_labels_source > 1) + return 1; + + r = dns_name_startswith(DNS_RESOURCE_KEY_NAME(rr->key), "*"); + if (r < 0) + return r; + + return !r; +} + static void dns_resource_record_hash_func(const void *i, struct siphash *state) { const DnsResourceRecord *rr = i; diff --git a/src/resolve/resolved-dns-rr.h b/src/resolve/resolved-dns-rr.h index 8e7bfaa7c7..8ab53211a9 100644 --- a/src/resolve/resolved-dns-rr.h +++ b/src/resolve/resolved-dns-rr.h @@ -309,6 +309,7 @@ int dns_resource_record_to_wire_format(DnsResourceRecord *rr, bool canonical); int dns_resource_record_signer(DnsResourceRecord *rr, const char **ret); int dns_resource_record_source(DnsResourceRecord *rr, const char **ret); int dns_resource_record_is_signer(DnsResourceRecord *rr, const char *zone); +int dns_resource_record_is_synthetic(DnsResourceRecord *rr); DnsTxtItem *dns_txt_item_free_all(DnsTxtItem *i); bool dns_txt_item_equal(DnsTxtItem *a, DnsTxtItem *b); -- cgit v1.2.3-54-g00ecf From 412577e3c8045175ef185d3344a81b603d6225f8 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 15 Jan 2016 02:24:39 +0100 Subject: resolved: add complex test case This new test case tries to resolve a couple of known domains, to verify the validation results. It talks to resolved via the bus, thus comprehensively testing the whole shebang. Of course, it requires network connectivity and a DNSSEC capable DNS server, hence this is a manual test. --- Makefile.am | 11 +++ src/resolve/test-dnssec-complex.c | 146 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 157 insertions(+) create mode 100644 src/resolve/test-dnssec-complex.c (limited to 'src') diff --git a/Makefile.am b/Makefile.am index e895bc8cec..9fe7665ab5 100644 --- a/Makefile.am +++ b/Makefile.am @@ -5297,6 +5297,9 @@ tests += \ test-dns-domain \ test-dnssec +manual_tests += \ + test-dnssec-complex + test_dnssec_SOURCES = \ src/resolve/test-dnssec.c \ src/resolve/resolved-dns-packet.c \ @@ -5315,6 +5318,14 @@ test_dnssec_SOURCES = \ test_dnssec_LDADD = \ libshared.la +test_dnssec_complex_SOURCES = \ + src/resolve/test-dnssec-complex.c \ + src/resolve/dns-type.c \ + src/resolve/dns-type.h + +test_dnssec_complex_LDADD = \ + libshared.la + endif endif diff --git a/src/resolve/test-dnssec-complex.c b/src/resolve/test-dnssec-complex.c new file mode 100644 index 0000000000..3093fb052d --- /dev/null +++ b/src/resolve/test-dnssec-complex.c @@ -0,0 +1,146 @@ +/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/ + +/*** + This file is part of systemd. + + Copyright 2016 Lennart Poettering + + systemd is free software; you can redistribute it and/or modify it + under the terms of the GNU Lesser General Public License as published by + the Free Software Foundation; either version 2.1 of the License, or + (at your option) any later version. + + systemd is distributed in the hope that it will be useful, but + WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public License + along with systemd; If not, see . +***/ + +#include "sd-bus.h" + +#include "alloc-util.h" +#include "bus-common-errors.h" +#include "dns-type.h" +#include "random-util.h" +#include "string-util.h" +#include "time-util.h" + +#define DNS_CALL_TIMEOUT_USEC (45*USEC_PER_SEC) + +static void test_lookup(sd_bus *bus, const char *name, uint16_t type, const char *result) { + _cleanup_(sd_bus_message_unrefp) sd_bus_message *req = NULL, *reply = NULL; + _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; + _cleanup_free_ char *m = NULL; + int r; + + /* If the name starts with a dot, we prefix one to three random labels */ + if (startswith(name, ".")) { + uint64_t i, u; + + u = 1 + (random_u64() & 3); + name ++; + + for (i = 0; i < u; i++) { + _cleanup_free_ char *b = NULL; + char *x; + + assert_se(asprintf(&b, "x%" PRIu64 "x", random_u64())); + x = strjoin(b, ".", name, NULL); + assert_se(x); + free(m); + name = m = x; + } + } + + assert_se(sd_bus_message_new_method_call( + bus, + &req, + "org.freedesktop.resolve1", + "/org/freedesktop/resolve1", + "org.freedesktop.resolve1.Manager", + "ResolveRecord") >= 0); + + assert_se(sd_bus_message_append(req, "isqqt", 0, name, DNS_CLASS_IN, type, UINT64_C(0)) >= 0); + + r = sd_bus_call(bus, req, DNS_CALL_TIMEOUT_USEC, &error, &reply); + + if (r < 0) { + assert_se(result); + assert_se(sd_bus_error_has_name(&error, result)); + log_info("[OK] %s/%s resulted in <%s>.", name, dns_type_to_string(type), error.name); + } else { + assert_se(!result); + log_info("[OK] %s/%s succeeded.", name, dns_type_to_string(type)); + } +} + +int main(int argc, char* argv[]) { + _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; + + /* Note that this is a manual test as it requires: + * + * Full network access + * A DNSSEC capable DNS server + * That zones contacted are still set up as they were when I wrote this. + */ + + assert_se(sd_bus_open_system(&bus) >= 0); + + /* Normally signed */ + test_lookup(bus, "www.eurid.eu", DNS_TYPE_A, NULL); + test_lookup(bus, "sigok.verteiltesysteme.net", DNS_TYPE_A, NULL); + + /* Normally signed, NODATA */ + test_lookup(bus, "www.eurid.eu", DNS_TYPE_RP, BUS_ERROR_NO_SUCH_RR); + test_lookup(bus, "sigok.verteiltesysteme.net", DNS_TYPE_RP, BUS_ERROR_NO_SUCH_RR); + + /* Invalid signature */ + test_lookup(bus, "sigfail.verteiltesysteme.net", DNS_TYPE_A, BUS_ERROR_DNSSEC_FAILED); + + /* Invalid signature, RSA, wildcard */ + test_lookup(bus, ".wilda.rhybar.0skar.cz", DNS_TYPE_A, BUS_ERROR_DNSSEC_FAILED); + + /* Invalid signature, ECDSA, wildcard */ + test_lookup(bus, ".wilda.rhybar.ecdsa.0skar.cz", DNS_TYPE_A, BUS_ERROR_DNSSEC_FAILED); + + /* NXDOMAIN in NSEC domain */ + test_lookup(bus, "hhh.nasa.gov", DNS_TYPE_A, _BUS_ERROR_DNS "NXDOMAIN"); + + /* wildcard, NSEC zone */ + test_lookup(bus, ".wilda.nsec.0skar.cz", DNS_TYPE_A, NULL); + + /* wildcard, NSEC zone, NODATA */ + test_lookup(bus, ".wilda.nsec.0skar.cz", DNS_TYPE_RP, BUS_ERROR_NO_SUCH_RR); + + /* wildcard, NSEC3 zone */ + test_lookup(bus, ".wilda.0skar.cz", DNS_TYPE_A, NULL); + + /* wildcard, NSEC3 zone, NODATA */ + test_lookup(bus, ".wilda.0skar.cz", DNS_TYPE_RP, BUS_ERROR_NO_SUCH_RR); + + /* wildcard, NSEC zone, CNAME */ + test_lookup(bus, ".wild.nsec.0skar.cz", DNS_TYPE_A, NULL); + + /* wildcard, NSEC zone, NODATA, CNAME */ + test_lookup(bus, ".wild.nsec.0skar.cz", DNS_TYPE_RP, BUS_ERROR_NO_SUCH_RR); + + /* wildcard, NSEC3 zone, CNAME */ + test_lookup(bus, ".wild.0skar.cz", DNS_TYPE_A, NULL); + + /* wildcard, NSEC3 zone, NODATA, CNAME */ + test_lookup(bus, ".wild.0skar.cz", DNS_TYPE_RP, BUS_ERROR_NO_SUCH_RR); + + /* NODATA due to empty non-terminal in NSEC domain */ + test_lookup(bus, "herndon.nasa.gov", DNS_TYPE_A, BUS_ERROR_NO_SUCH_RR); + + /* NXDOMAIN in NSEC root zone: */ + test_lookup(bus, "jasdhjas.kjkfgjhfjg", DNS_TYPE_A, _BUS_ERROR_DNS "NXDOMAIN"); + + /* NXDOMAIN in NSEC3 .com zone: */ + test_lookup(bus, "kjkfgjhfjgsdfdsfd.com", DNS_TYPE_A, _BUS_ERROR_DNS "NXDOMAIN"); + + return 0; +} -- cgit v1.2.3-54-g00ecf From afc58cc2fb5841154fe036ee7a6e1c8a06bc5d29 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 15 Jan 2016 02:48:56 +0100 Subject: resolved: update RFCs list and TODO list --- src/resolve/RFCs | 18 +++++++++--------- src/resolve/resolved-dns-dnssec.c | 13 +++++-------- 2 files changed, 14 insertions(+), 17 deletions(-) (limited to 'src') diff --git a/src/resolve/RFCs b/src/resolve/RFCs index 33f4dd9cb6..22004a00cd 100644 --- a/src/resolve/RFCs +++ b/src/resolve/RFCs @@ -13,14 +13,14 @@ Y https://tools.ietf.org/html/rfc1123 → Requirements for Internet Hosts -- App Y https://tools.ietf.org/html/rfc1536 → Common DNS Implementation Errors and Suggested Fixes Y https://tools.ietf.org/html/rfc1876 → A Means for Expressing Location Information in the Domain Name System Y https://tools.ietf.org/html/rfc2181 → Clarifications to the DNS Specification - https://tools.ietf.org/html/rfc2308 → Negative Caching of DNS Queries (DNS NCACHE) +Y https://tools.ietf.org/html/rfc2308 → Negative Caching of DNS Queries (DNS NCACHE) Y https://tools.ietf.org/html/rfc2782 → A DNS RR for specifying the location of services (DNS SRV) D https://tools.ietf.org/html/rfc3492 → Punycode: A Bootstring encoding of Unicode for Internationalized Domain Names in Applications (IDNA) Y https://tools.ietf.org/html/rfc3596 → DNS Extensions to Support IP Version 6 Y https://tools.ietf.org/html/rfc3597 → Handling of Unknown DNS Resource Record (RR) Types - https://tools.ietf.org/html/rfc4033 → DNS Security Introduction and Requirements - https://tools.ietf.org/html/rfc4034 → Resource Records for the DNS Security Extensions - https://tools.ietf.org/html/rfc4035 → Protocol Modifications for the DNS Security Extensions +Y https://tools.ietf.org/html/rfc4033 → DNS Security Introduction and Requirements +Y https://tools.ietf.org/html/rfc4034 → Resource Records for the DNS Security Extensions +Y https://tools.ietf.org/html/rfc4035 → Protocol Modifications for the DNS Security Extensions ! https://tools.ietf.org/html/rfc4183 → A Suggested Scheme for DNS Resolution of Networks and Gateways Y https://tools.ietf.org/html/rfc4255 → Using DNS to Securely Publish Secure Shell (SSH) Key Fingerprints Y https://tools.ietf.org/html/rfc4343 → Domain Name System (DNS) Case Insensitivity Clarification @@ -31,26 +31,26 @@ Y https://tools.ietf.org/html/rfc4509 → Use of SHA-256 in DNSSEC Delegation Si ~ https://tools.ietf.org/html/rfc4697 → Observed DNS Resolution Misbehavior Y https://tools.ietf.org/html/rfc4795 → Link-Local Multicast Name Resolution (LLMNR) Y https://tools.ietf.org/html/rfc5011 → Automated Updates of DNS Security (DNSSEC) Trust Anchors - https://tools.ietf.org/html/rfc5155 → DNS Security (DNSSEC) Hashed Authenticated Denial of Existence +Y https://tools.ietf.org/html/rfc5155 → DNS Security (DNSSEC) Hashed Authenticated Denial of Existence Y https://tools.ietf.org/html/rfc5452 → Measures for Making DNS More Resilient against Forged Answers Y https://tools.ietf.org/html/rfc5702 → Use of SHA-2 Algorithms with RSA in DNSKEY and RRSIG Resource Records for DNSSEC Y https://tools.ietf.org/html/rfc5890 → Internationalized Domain Names for Applications (IDNA): Definitions and Document Framework Y https://tools.ietf.org/html/rfc5891 → Internationalized Domain Names in Applications (IDNA): Protocol Y https://tools.ietf.org/html/rfc5966 → DNS Transport over TCP - Implementation Requirements Y https://tools.ietf.org/html/rfc6303 → Locally Served DNS Zones - https://tools.ietf.org/html/rfc6604 → xNAME RCODE and Status Bits Clarification +Y https://tools.ietf.org/html/rfc6604 → xNAME RCODE and Status Bits Clarification Y https://tools.ietf.org/html/rfc6605 → Elliptic Curve Digital Signature Algorithm (DSA) for DNSSEC https://tools.ietf.org/html/rfc6672 → DNAME Redirection in the DNS ! https://tools.ietf.org/html/rfc6731 → Improved Recursive DNS Server Selection for Multi-Interfaced Nodes Y https://tools.ietf.org/html/rfc6761 → Special-Use Domain Names https://tools.ietf.org/html/rfc6762 → Multicast DNS https://tools.ietf.org/html/rfc6763 → DNS-Based Service Discovery - https://tools.ietf.org/html/rfc6781 → DNSSEC Operational Practices, Version 2 - https://tools.ietf.org/html/rfc6840 → Clarifications and Implementation Notes for DNS Security (DNSSEC) +~ https://tools.ietf.org/html/rfc6781 → DNSSEC Operational Practices, Version 2 +Y https://tools.ietf.org/html/rfc6840 → Clarifications and Implementation Notes for DNS Security (DNSSEC) Y https://tools.ietf.org/html/rfc6891 → Extension Mechanisms for DNS (EDNS(0)) Y https://tools.ietf.org/html/rfc6944 → Applicability Statement: DNS Security (DNSSEC) DNSKEY Algorithm Implementation Status Y https://tools.ietf.org/html/rfc6975 → Signaling Cryptographic Algorithm Understanding in DNS Security Extensions (DNSSEC) - https://tools.ietf.org/html/rfc7129 → Authenticated Denial of Existence in the DNS +Y https://tools.ietf.org/html/rfc7129 → Authenticated Denial of Existence in the DNS Y https://tools.ietf.org/html/rfc7646 → Definition and Use of DNSSEC Negative Trust Anchors ~ https://tools.ietf.org/html/rfc7719 → DNS Terminology diff --git a/src/resolve/resolved-dns-dnssec.c b/src/resolve/resolved-dns-dnssec.c index 2ac085dfd3..43fb365d68 100644 --- a/src/resolve/resolved-dns-dnssec.c +++ b/src/resolve/resolved-dns-dnssec.c @@ -35,17 +35,14 @@ * * TODO: * - * - wildcard zones compatibility (NSEC/NSEC3 wildcard check is missing) - * - multi-label zone compatibility - * - cname/dname compatibility - * - nxdomain on qname * - bus calls to override DNSEC setting per interface * - log all DNSSEC downgrades + * - log all RRs that failed validation * - enable by default - * - * - RFC 4035, Section 5.3.4 (When receiving a positive wildcard reply, use NSEC to ensure it actually really applies) - * - RFC 6840, Section 4.1 (ensure we don't get fed a glue NSEC from the parent zone) - * - RFC 6840, Section 4.3 (check for CNAME on NSEC too) + * - Allow clients to request DNSSEC even if DNSSEC is off + * - find public DNAME test domain + * - make sure when getting an NXDOMAIN response through CNAME, we still process the first CNAMEs in the packet + * - flush cache when DNSSEC setting changes * */ #define VERIFY_RRS_MAX 256 -- cgit v1.2.3-54-g00ecf From c3f7000e611b2c08052aca6db47245e77c008ae6 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 15 Jan 2016 18:18:54 +0100 Subject: resolved: ignore invalid OPT RRs in incoming packets This validates OPT RRs more rigorously, before honouring them: if we any of the following condition holds, we'll ignore them: a) Multiple OPT RRs in the same message b) OPT RR not owned by the root domain c) OPT RR in the wrong section (Belkin routers do this) d) OPT RR contain rfc6975 algorithm data (Belkin routers do this) e) OPT version is not 0 f) OPT payload doesn't add up with the lengths Note that d) may be an indication that the server just blindly copied OPT data from the response into the reply. RFC6975 data is only supposed to be included in queries, and we do so. It's not supposed to be included in responses (and the RFC is very clear on that). Hence if we get it back in a reply, then the server probably just copied the OPT RR. --- src/resolve/resolved-dns-packet.c | 107 +++++++++++++++++++++++++++++++------- 1 file changed, 88 insertions(+), 19 deletions(-) (limited to 'src') diff --git a/src/resolve/resolved-dns-packet.c b/src/resolve/resolved-dns-packet.c index a8a8632491..57cfc1ab9b 100644 --- a/src/resolve/resolved-dns-packet.c +++ b/src/resolve/resolved-dns-packet.c @@ -2017,6 +2017,48 @@ fail: return r; } +static bool opt_is_good(DnsResourceRecord *rr, bool *rfc6975) { + const uint8_t* p; + bool found_dau_dhu_n3u = false; + size_t l; + + /* Checks whether the specified OPT RR is well-formed and whether it contains RFC6975 data (which is not OK in + * a reply). */ + + assert(rr); + assert(rr->key->type == DNS_TYPE_OPT); + + /* Check that the version is 0 */ + if (((rr->ttl >> 16) & UINT32_C(0xFF)) != 0) + return false; + + p = rr->opt.data; + l = rr->opt.size; + while (l > 0) { + uint16_t option_code, option_length; + + /* At least four bytes for OPTION-CODE and OPTION-LENGTH are required */ + if (l < 4U) + return false; + + option_code = unaligned_read_be16(p); + option_length = unaligned_read_be16(p + 2); + + if (l < option_length + 4U) + return false; + + /* RFC 6975 DAU, DHU or N3U fields found. */ + if (IN_SET(option_code, 5, 6, 7)) + found_dau_dhu_n3u = true; + + p += option_length + 4U; + l -= option_length + 4U; + } + + *rfc6975 = found_dau_dhu_n3u; + return true; +} + int dns_packet_extract(DnsPacket *p) { _cleanup_(dns_question_unrefp) DnsQuestion *question = NULL; _cleanup_(dns_answer_unrefp) DnsAnswer *answer = NULL; @@ -2064,6 +2106,8 @@ int dns_packet_extract(DnsPacket *p) { n = DNS_PACKET_RRCOUNT(p); if (n > 0) { + bool bad_opt = false; + answer = dns_answer_new(n); if (!answer) { r = -ENOMEM; @@ -2079,35 +2123,57 @@ int dns_packet_extract(DnsPacket *p) { goto finish; if (rr->key->type == DNS_TYPE_OPT) { + bool has_rfc6975; + + if (p->opt || bad_opt) { + /* Multiple OPT RRs? if so, let's ignore all, because there's something wrong + * with the server, and if one is valid we wouldn't know which one. */ + log_debug("Multiple OPT RRs detected, ignoring all."); + bad_opt = true; + continue; + } if (!dns_name_is_root(DNS_RESOURCE_KEY_NAME(rr->key))) { - r = -EBADMSG; - goto finish; + /* If the OPT RR qis not owned by the root domain, then it is bad, let's ignore + * it. */ + log_debug("OPT RR is not owned by root domain, ignoring."); + bad_opt = true; + continue; + } + + if (i < DNS_PACKET_ANCOUNT(p) + DNS_PACKET_NSCOUNT(p)) { + /* OPT RR is in the wrong section? Some Belkin routers do this. This is a hint + * the EDNS implementation is borked, like the Belkin one is, hence ignore + * it. */ + log_debug("OPT RR in wrong section, ignoring."); + bad_opt = true; + continue; } - /* Note that we accept the OPT RR in - * any section, not just in the - * additional section, as some routers - * (Belkin!) blindly copy the OPT RR - * from the query to the reply packet, - * and don't get the section right. */ + if (!opt_is_good(rr, &has_rfc6975)) { + log_debug("Malformed OPT RR, ignoring."); + bad_opt = true; + continue; + } - /* Two OPT RRs? */ - if (p->opt) { - r = -EBADMSG; - goto finish; + if (has_rfc6975) { + /* OPT RR contains RFC6975 algorithm data, then this is indication that the + * server just copied the OPT it got from us (which contained that data) back + * into the reply. If so, then it doesn't properly support EDNS, as RFC6975 + * makes it very clear that the algorithm data should only be contained in + * questions, never in replies. Crappy Belkin copy the OPT data for example, + * hence let's detect this so that we downgrade early. */ + log_debug("OPT RR contained RFC6975 data, ignoring."); + bad_opt = true; + continue; } p->opt = dns_resource_record_ref(rr); } else { - /* According to RFC 4795, section - * 2.9. only the RRs from the Answer - * section shall be cached. Hence mark - * only those RRs as cacheable by - * default, but not the ones from the - * Additional or Authority - * sections. */ + /* According to RFC 4795, section 2.9. only the RRs from the Answer section shall be + * cached. Hence mark only those RRs as cacheable by default, but not the ones from the + * Additional or Authority sections. */ r = dns_answer_add(answer, rr, p->ifindex, (i < DNS_PACKET_ANCOUNT(p) ? DNS_ANSWER_CACHEABLE : 0) | @@ -2116,6 +2182,9 @@ int dns_packet_extract(DnsPacket *p) { goto finish; } } + + if (bad_opt) + p->opt = dns_resource_record_unref(p->opt); } p->question = question; -- cgit v1.2.3-54-g00ecf From de54e62b4bd7856fb897c9a2ee93cc228adb2135 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 15 Jan 2016 19:23:51 +0100 Subject: resolved: downgrade server feature level more aggressively when we have reason to This adds logic to downgrade the feature level more aggressively when we have reason to. Specifically: - When we get a response packet that lacks an OPT RR for a query that had it. If so, downgrade immediately to UDP mode, i.e. don't generate EDNS0 packets anymore. - When we get a response which we are sure should be signed, but lacks RRSIG RRs, we downgrade to EDNS0 mode, i.e. below DO mode, since DO is apparently not really supported. This should increase compatibility with servers that generate non-sensical responses if they messages with OPT RRs and suchlike, for example the situation described here: https://open.nlnetlabs.nl/pipermail/dnssec-trigger/2014-November/000376.html This also changes the downgrade code to explain in a debug log message why a specific downgrade happened. --- src/resolve/resolved-dns-server.c | 117 ++++++++++++++++++++++----------- src/resolve/resolved-dns-server.h | 15 +++-- src/resolve/resolved-dns-transaction.c | 10 ++- 3 files changed, 94 insertions(+), 48 deletions(-) (limited to 'src') diff --git a/src/resolve/resolved-dns-server.c b/src/resolve/resolved-dns-server.c index 0969e31e8a..6732030d65 100644 --- a/src/resolve/resolved-dns-server.c +++ b/src/resolve/resolved-dns-server.c @@ -232,7 +232,9 @@ static void dns_server_verified(DnsServer *s, DnsServerFeatureLevel level) { return; if (s->verified_feature_level != level) { - log_debug("Verified feature level %s.", dns_server_feature_level_to_string(level)); + log_debug("Verified we get a response at feature level %s from DNS server %s.", + dns_server_feature_level_to_string(level), + dns_server_string(s)); s->verified_feature_level = level; } @@ -294,7 +296,6 @@ void dns_server_packet_lost(DnsServer *s, int protocol, DnsServerFeatureLevel le void dns_server_packet_failed(DnsServer *s, DnsServerFeatureLevel level) { assert(s); - assert(s->manager); /* Invoked whenever we get a FORMERR, SERVFAIL or NOTIMP rcode from a server. */ @@ -306,7 +307,6 @@ void dns_server_packet_failed(DnsServer *s, DnsServerFeatureLevel level) { void dns_server_packet_truncated(DnsServer *s, DnsServerFeatureLevel level) { assert(s); - assert(s->manager); /* Invoked whenever we get a packet with TC bit set. */ @@ -316,13 +316,22 @@ void dns_server_packet_truncated(DnsServer *s, DnsServerFeatureLevel level) { s->packet_truncated = true; } -void dns_server_packet_rrsig_missing(DnsServer *s) { +void dns_server_packet_rrsig_missing(DnsServer *s, DnsServerFeatureLevel level) { + assert(s); + + if (level < DNS_SERVER_FEATURE_LEVEL_DO) + return; + + s->packet_rrsig_missing = true; +} + +void dns_server_packet_bad_opt(DnsServer *s, DnsServerFeatureLevel level) { assert(s); - assert(s->manager); - log_warning("DNS server %s does not augment replies with RRSIG records, DNSSEC not available.", dns_server_string(s)); + if (level < DNS_SERVER_FEATURE_LEVEL_EDNS0) + return; - s->rrsig_missing = true; + s->packet_bad_opt = true; } static bool dns_server_grace_period_expired(DnsServer *s) { @@ -351,6 +360,8 @@ static void dns_server_reset_counters(DnsServer *s) { s->n_failed_tcp = 0; s->packet_failed = false; s->packet_truncated = false; + s->packet_bad_opt = false; + s->packet_rrsig_missing = false; s->verified_usec = 0; } @@ -361,11 +372,9 @@ DnsServerFeatureLevel dns_server_possible_feature_level(DnsServer *s) { dns_server_grace_period_expired(s)) { s->possible_feature_level = DNS_SERVER_FEATURE_LEVEL_BEST; - s->rrsig_missing = false; - dns_server_reset_counters(s); - log_info("Grace period over, resuming full feature set (%s) for DNS server %s", + log_info("Grace period over, resuming full feature set (%s) for DNS server %s.", dns_server_feature_level_to_string(s->possible_feature_level), dns_server_string(s)); @@ -375,46 +384,75 @@ DnsServerFeatureLevel dns_server_possible_feature_level(DnsServer *s) { DnsServerFeatureLevel p = s->possible_feature_level; if (s->n_failed_tcp >= DNS_SERVER_FEATURE_RETRY_ATTEMPTS && - s->possible_feature_level == DNS_SERVER_FEATURE_LEVEL_TCP) + s->possible_feature_level == DNS_SERVER_FEATURE_LEVEL_TCP) { /* We are at the TCP (lowest) level, and we tried a couple of TCP connections, and it didn't * work. Upgrade back to UDP again. */ + log_debug("Reached maximum number of failed TCP connection attempts, trying UDP again..."); + s->possible_feature_level = DNS_SERVER_FEATURE_LEVEL_UDP; + + } else if (s->packet_bad_opt && + s->possible_feature_level >= DNS_SERVER_FEATURE_LEVEL_EDNS0) { + + /* A reply to one of our EDNS0 queries didn't carry a valid OPT RR, then downgrade to below + * EDNS0 levels. After all, some records generate different responses with and without OPT RR + * in the request. Example: + * https://open.nlnetlabs.nl/pipermail/dnssec-trigger/2014-November/000376.html */ + + log_debug("Server doesn't support EDNS(0) properly, downgrading feature level..."); s->possible_feature_level = DNS_SERVER_FEATURE_LEVEL_UDP; - else if ((s->n_failed_udp >= DNS_SERVER_FEATURE_RETRY_ATTEMPTS && - s->possible_feature_level >= DNS_SERVER_FEATURE_LEVEL_UDP) || - (s->packet_failed && - s->possible_feature_level > DNS_SERVER_FEATURE_LEVEL_UDP) || - (s->n_failed_tcp >= DNS_SERVER_FEATURE_RETRY_ATTEMPTS && - s->packet_truncated && - s->possible_feature_level > DNS_SERVER_FEATURE_LEVEL_UDP)) - - /* Downgrade the feature one level, maybe things will work better then. We do this under any of - * three conditions: - * - * 1. We lost too many UDP packets in a row, and are on a feature level of UDP or higher. If - * the packets are lost, maybe the server cannot parse them, hence downgrading sounds like a - * good idea. We might downgrade all the way down to TCP this way. - * - * 2. We got a failure packet, and are at a feature level above UDP. Note that in this case we - * downgrade no further than UDP, under the assumption that a failure packet indicates an - * incompatible packet contents, but not a problem with the transport. - * - * 3. We got too many TCP connection failures in a row, we had at least one truncated packet, - * and are on a feature level above UDP. By downgrading things and getting rid of DNSSEC or - * EDNS0 data we hope to make the packet smaller, so that it still works via UDP given that - * TCP appears not to be a fallback. Note that if we are already at the lowest UDP level, we - * don't go further down, since that's TCP, and TCP failed too often after all. - */ + } else if (s->packet_rrsig_missing && + s->possible_feature_level >= DNS_SERVER_FEATURE_LEVEL_DO) { + /* RRSIG data was missing on a EDNS0 packet with DO bit set. This means the server doesn't + * augment responses with DNSSEC RRs. If so, let's better not ask the server for it anymore, + * after all some servers generate different replies depending if an OPT RR is in the query or + * not. */ + + log_debug("Detected server responses lack RRSIG records, downgrading feature level..."); + s->possible_feature_level = DNS_SERVER_FEATURE_LEVEL_EDNS0; + + } else if (s->n_failed_udp >= DNS_SERVER_FEATURE_RETRY_ATTEMPTS && + s->possible_feature_level >= DNS_SERVER_FEATURE_LEVEL_UDP) { + + /* We lost too many UDP packets in a row, and are on a feature level of UDP or higher. If the + * packets are lost, maybe the server cannot parse them, hence downgrading sounds like a good + * idea. We might downgrade all the way down to TCP this way. */ + + log_debug("Lost too many UDP packets, downgrading feature level..."); + s->possible_feature_level--; + + } else if (s->packet_failed && + s->possible_feature_level > DNS_SERVER_FEATURE_LEVEL_UDP) { + + /* We got a failure packet, and are at a feature level above UDP. Note that in this case we + * downgrade no further than UDP, under the assumption that a failure packet indicates an + * incompatible packet contents, but not a problem with the transport. */ + + log_debug("Got server failure, downgrading feature level..."); s->possible_feature_level--; + } else if (s->n_failed_tcp >= DNS_SERVER_FEATURE_RETRY_ATTEMPTS && + s->packet_truncated && + s->possible_feature_level > DNS_SERVER_FEATURE_LEVEL_UDP) { + + /* We got too many TCP connection failures in a row, we had at least one truncated packet, and + * are on a feature level above UDP. By downgrading things and getting rid of DNSSEC or EDNS0 + * data we hope to make the packet smaller, so that it still works via UDP given that TCP + * appears not to be a fallback. Note that if we are already at the lowest UDP level, we don't + * go further down, since that's TCP, and TCP failed too often after all. */ + + log_debug("Got too many failed TCP connection failures and truncated UDP packets, downgrading feature level..."); + s->possible_feature_level--; + } + if (p != s->possible_feature_level) { /* We changed the feature level, reset the counting */ dns_server_reset_counters(s); - log_warning("Using degraded feature set (%s) for DNS server %s", + log_warning("Using degraded feature set (%s) for DNS server %s.", dns_server_feature_level_to_string(s->possible_feature_level), dns_server_string(s)); } @@ -468,7 +506,10 @@ bool dns_server_dnssec_supported(DnsServer *server) { if (server->possible_feature_level < DNS_SERVER_FEATURE_LEVEL_DO) return false; - if (server->rrsig_missing) + if (server->packet_bad_opt) + return false; + + if (server->packet_rrsig_missing) return false; /* DNSSEC servers need to support TCP properly (see RFC5966), if they don't, we assume DNSSEC is borked too */ diff --git a/src/resolve/resolved-dns-server.h b/src/resolve/resolved-dns-server.h index 323f702903..02bd3463a7 100644 --- a/src/resolve/resolved-dns-server.h +++ b/src/resolve/resolved-dns-server.h @@ -68,20 +68,20 @@ struct DnsServer { DnsServerFeatureLevel verified_feature_level; DnsServerFeatureLevel possible_feature_level; + size_t received_udp_packet_max; + unsigned n_failed_udp; unsigned n_failed_tcp; + bool packet_failed:1; bool packet_truncated:1; + bool packet_bad_opt:1; + bool packet_rrsig_missing:1; + usec_t verified_usec; usec_t features_grace_period_usec; - /* Indicates whether responses are augmented with RRSIG by - * server or not. Note that this is orthogonal to the feature - * level stuff, as it's only information describing responses, - * and has no effect on how the questions are asked. */ - bool rrsig_missing:1; - /* Used when GC'ing old DNS servers when configuration changes. */ bool marked:1; @@ -108,7 +108,8 @@ void dns_server_packet_received(DnsServer *s, int protocol, DnsServerFeatureLeve void dns_server_packet_lost(DnsServer *s, int protocol, DnsServerFeatureLevel level, usec_t usec); void dns_server_packet_failed(DnsServer *s, DnsServerFeatureLevel level); void dns_server_packet_truncated(DnsServer *s, DnsServerFeatureLevel level); -void dns_server_packet_rrsig_missing(DnsServer *s); +void dns_server_packet_rrsig_missing(DnsServer *s, DnsServerFeatureLevel level); +void dns_server_packet_bad_opt(DnsServer *s, DnsServerFeatureLevel level); DnsServerFeatureLevel dns_server_possible_feature_level(DnsServer *s); diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index ef38812c85..968bb1467b 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -726,13 +726,17 @@ void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p) { return; } - /* Parse message, if it isn't parsed yet. */ + /* After the superficial checks, actually parse the message. */ r = dns_packet_extract(p); if (r < 0) { dns_transaction_complete(t, DNS_TRANSACTION_INVALID_REPLY); return; } + /* Report that the OPT RR was missing */ + if (t->server && !p->opt) + dns_server_packet_bad_opt(t->server, t->current_feature_level); + if (IN_SET(t->scope->protocol, DNS_PROTOCOL_DNS, DNS_PROTOCOL_LLMNR)) { /* Only consider responses with equivalent query section to the request */ @@ -2416,7 +2420,7 @@ int dns_transaction_validate_dnssec(DnsTransaction *t) { if (!dns_transaction_dnssec_supported_full(t)) { /* The server does not support DNSSEC, or doesn't augment responses with RRSIGs. */ t->answer_dnssec_result = DNSSEC_INCOMPATIBLE_SERVER; - log_debug("Cannot validate response, server lacks DNSSEC support."); + log_debug("Not validating response, server lacks DNSSEC support."); return 0; } @@ -2590,7 +2594,7 @@ int dns_transaction_validate_dnssec(DnsTransaction *t) { /* This is an RR we know has to be signed. If it isn't this means * the server is not attaching RRSIGs, hence complain. */ - dns_server_packet_rrsig_missing(t->server); + dns_server_packet_rrsig_missing(t->server, t->current_feature_level); if (t->scope->dnssec_mode == DNSSEC_ALLOW_DOWNGRADE) { -- cgit v1.2.3-54-g00ecf From b64513580ce627578351b76a502455e7bc62cae4 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 15 Jan 2016 20:29:56 +0100 Subject: resolved: when we receive an reply which is OPT-less or RRSIG-less, downgrade what we verified If we receive a reply that lacks the OPT RR, then this is reason to downgrade what was verified before, as it's apparently no longer true, and the previous OPT RR we saw was only superficially OK. Similar, if we realize that RRSIGs are not augmented, then also downgrade the feature level that was verified, as DNSSEC is after all not supported. This check is in particular necessary, as we might notice the fact that RRSIG is not augmented only very late, when verifying the root domain. Also, when verifying a successful response, actually take in consideration that it might have been reported already that RRSIG or OPT are missing in the response. --- src/resolve/resolved-dns-server.c | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/src/resolve/resolved-dns-server.c b/src/resolve/resolved-dns-server.c index 6732030d65..949c0d66e1 100644 --- a/src/resolve/resolved-dns-server.c +++ b/src/resolve/resolved-dns-server.c @@ -248,13 +248,18 @@ void dns_server_packet_received(DnsServer *s, int protocol, DnsServerFeatureLeve if (s->possible_feature_level == level) s->n_failed_udp = 0; + /* If the RRSIG data is missing, then we can only validate EDNS0 at max */ + if (s->packet_rrsig_missing && level >= DNS_SERVER_FEATURE_LEVEL_DO) + level = DNS_SERVER_FEATURE_LEVEL_DO - 1; + + /* If the OPT RR got lost, then we can only validate UDP at max */ + if (s->packet_bad_opt && level >= DNS_SERVER_FEATURE_LEVEL_EDNS0) + level = DNS_SERVER_FEATURE_LEVEL_EDNS0 - 1; + + /* Even if we successfully receive a reply to a request announcing support for large packets, + that does not mean we can necessarily receive large packets. */ if (level == DNS_SERVER_FEATURE_LEVEL_LARGE) - /* Even if we successfully receive a reply to a request announcing support for large packets, - that does not mean we can necessarily receive large packets. */ - dns_server_verified(s, DNS_SERVER_FEATURE_LEVEL_LARGE - 1); - else - /* A successful UDP reply, verifies UDP, ENDS0 and DO levels */ - dns_server_verified(s, level); + level = DNS_SERVER_FEATURE_LEVEL_LARGE - 1; } else if (protocol == IPPROTO_TCP) { @@ -262,9 +267,11 @@ void dns_server_packet_received(DnsServer *s, int protocol, DnsServerFeatureLeve s->n_failed_tcp = 0; /* Successful TCP connections are only useful to verify the TCP feature level. */ - dns_server_verified(s, DNS_SERVER_FEATURE_LEVEL_TCP); + level = DNS_SERVER_FEATURE_LEVEL_TCP; } + dns_server_verified(s, level); + /* Remember the size of the largest UDP packet we received from a server, we know that we can always announce support for packets with at least this size. */ @@ -322,6 +329,10 @@ void dns_server_packet_rrsig_missing(DnsServer *s, DnsServerFeatureLevel level) if (level < DNS_SERVER_FEATURE_LEVEL_DO) return; + /* If the RRSIG RRs are missing, we have to downgrade what we previously verified */ + if (s->verified_feature_level >= DNS_SERVER_FEATURE_LEVEL_DO) + s->verified_feature_level = DNS_SERVER_FEATURE_LEVEL_DO-1; + s->packet_rrsig_missing = true; } @@ -331,6 +342,10 @@ void dns_server_packet_bad_opt(DnsServer *s, DnsServerFeatureLevel level) { if (level < DNS_SERVER_FEATURE_LEVEL_EDNS0) return; + /* If the OPT RR got lost, we have to downgrade what we previously verified */ + if (s->verified_feature_level >= DNS_SERVER_FEATURE_LEVEL_EDNS0) + s->verified_feature_level = DNS_SERVER_FEATURE_LEVEL_EDNS0-1; + s->packet_bad_opt = true; } -- cgit v1.2.3-54-g00ecf From c5b4f86178f2ea72d16a26452440e84e37778f8d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 15 Jan 2016 20:34:09 +0100 Subject: resolved: when restarting a DNS transaction, remove all auxiliary DNSSEC transactions When we restart a DNS transaction, remove all connections to any auxiliary DNSSEC transactions, after all we might acquire completely different data this time, requiring different auxiliary DNSSEC transactions. --- src/resolve/resolved-dns-transaction.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index 968bb1467b..ab619e1d3b 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -43,6 +43,17 @@ static void dns_transaction_reset_answer(DnsTransaction *t) { t->answer_nsec_ttl = (uint32_t) -1; } +static void dns_transaction_flush_dnssec_transactions(DnsTransaction *t) { + DnsTransaction *z; + + assert(t); + + while ((z = set_steal_first(t->dnssec_transactions))) { + set_remove(z->notify_transactions, t); + dns_transaction_gc(z); + } +} + static void dns_transaction_close_connection(DnsTransaction *t) { assert(t); @@ -95,10 +106,7 @@ DnsTransaction* dns_transaction_free(DnsTransaction *t) { set_remove(z->dnssec_transactions, t); set_free(t->notify_transactions); - while ((z = set_steal_first(t->dnssec_transactions))) { - set_remove(z->notify_transactions, t); - dns_transaction_gc(z); - } + dns_transaction_flush_dnssec_transactions(t); set_free(t->dnssec_transactions); dns_answer_unref(t->validated_keys); @@ -965,6 +973,7 @@ static int dns_transaction_prepare(DnsTransaction *t, usec_t ts) { t->start_usec = ts; dns_transaction_reset_answer(t); + dns_transaction_flush_dnssec_transactions(t); /* Check the trust anchor. Do so only on classic DNS, since DNSSEC does not apply otherwise. */ if (t->scope->protocol == DNS_PROTOCOL_DNS) { -- cgit v1.2.3-54-g00ecf From ed9717fcbf52b0890a249b65418a95a9382de062 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 15 Jan 2016 20:36:40 +0100 Subject: resolved: check OPT RR before accepting a reply for verification of server feature level Let's make sure we first check if the OPT was lost in the reply, before we accept a reply as successful and use it for verifying the current feature level. --- src/resolve/resolved-dns-transaction.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index ab619e1d3b..ee055236fa 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -683,8 +683,6 @@ void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p) { return; } else if (DNS_PACKET_TC(p)) dns_server_packet_truncated(t->server, t->current_feature_level); - else - dns_server_packet_received(t->server, p->ipproto, t->current_feature_level, ts - t->start_usec, p->size); break; @@ -742,8 +740,12 @@ void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p) { } /* Report that the OPT RR was missing */ - if (t->server && !p->opt) - dns_server_packet_bad_opt(t->server, t->current_feature_level); + if (t->server) { + if (!p->opt) + dns_server_packet_bad_opt(t->server, t->current_feature_level); + + dns_server_packet_received(t->server, p->ipproto, t->current_feature_level, ts - t->start_usec, p->size); + } if (IN_SET(t->scope->protocol, DNS_PROTOCOL_DNS, DNS_PROTOCOL_LLMNR)) { -- cgit v1.2.3-54-g00ecf From c02cf2f41fc9b4c33a3c353b6181847733d65e8c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 15 Jan 2016 20:45:17 +0100 Subject: resolved: when the server feature level changes between query and response restart transaction In some cases we learn something about a server's feature level through its responses. If we notice that after doing basic checking of a response, and after collecting all auxiliary DNSSEC info the feature level of the server is lower than where we started, restart the whole transaction. This is useful to deal with servers that response rubbish when talked to with too high feature levels. --- src/resolve/resolved-dns-transaction.c | 38 ++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) (limited to 'src') diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index ee055236fa..72ef283dde 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -362,6 +362,25 @@ static void dns_transaction_retry(DnsTransaction *t) { dns_transaction_complete(t, DNS_TRANSACTION_RESOURCES); } +static int dns_transaction_maybe_restart(DnsTransaction *t) { + assert(t); + + if (!t->server) + return 0; + + if (t->current_feature_level <= dns_server_possible_feature_level(t->server)) + return 0; + + /* The server's current feature level is lower than when we sent the original query. We learnt something from + the response or possibly an auxiliary DNSSEC response that we didn't know before. We take that as reason to + restart the whole transaction. This is a good idea to deal with servers that respond rubbish if we include + OPT RR or DO bit. One of these cases is documented here, for example: + https://open.nlnetlabs.nl/pipermail/dnssec-trigger/2014-November/000376.html */ + + log_debug("Server feature level is now lower than when we began our transaction. Restarting."); + return dns_transaction_go(t); +} + static int on_stream_complete(DnsStream *s, int error) { _cleanup_(dns_packet_unrefp) DnsPacket *p = NULL; DnsTransaction *t; @@ -546,6 +565,16 @@ static void dns_transaction_process_dnssec(DnsTransaction *t) { if (dns_transaction_dnssec_is_live(t)) return; + /* See if we learnt things from the additional DNSSEC transactions, that we didn't know before, and better + * restart the lookup immediately. */ + r = dns_transaction_maybe_restart(t); + if (r < 0) { + dns_transaction_complete(t, DNS_TRANSACTION_RESOURCES); + return; + } + if (r > 0) /* Transaction got restarted... */ + return; + /* All our auxiliary DNSSEC transactions are complete now. Try * to validate our RRset now. */ r = dns_transaction_validate_dnssec(t); @@ -747,6 +776,15 @@ void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p) { dns_server_packet_received(t->server, p->ipproto, t->current_feature_level, ts - t->start_usec, p->size); } + /* See if we know things we didn't know before that indicate we better restart the lookup immediately. */ + r = dns_transaction_maybe_restart(t); + if (r < 0) { + dns_transaction_complete(t, DNS_TRANSACTION_RESOURCES); + return; + } + if (r > 0) /* Transaction got restarted... */ + return; + if (IN_SET(t->scope->protocol, DNS_PROTOCOL_DNS, DNS_PROTOCOL_LLMNR)) { /* Only consider responses with equivalent query section to the request */ -- cgit v1.2.3-54-g00ecf From 2e1bab34bdb1a5e849060afa8361b865ce39f87f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 15 Jan 2016 21:07:21 +0100 Subject: resolved: when switching between DNSSEC modes, possibly flush caches If the networkd configuration changes during runtime, make sure to flush all caches when we switch from a less trusted to a more trusted mode. --- src/resolve/resolved-link.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/resolve/resolved-link.c b/src/resolve/resolved-link.c index 928307e004..1e8f88024b 100644 --- a/src/resolve/resolved-link.c +++ b/src/resolve/resolved-link.c @@ -279,6 +279,7 @@ clear: static int link_update_dnssec_mode(Link *l) { _cleanup_free_ char *m = NULL; + DnssecMode mode; int r; assert(l); @@ -291,12 +292,23 @@ static int link_update_dnssec_mode(Link *l) { if (r < 0) goto clear; - l->dnssec_mode = dnssec_mode_from_string(m); - if (l->dnssec_mode < 0) { + mode = dnssec_mode_from_string(m); + if (mode < 0) { r = -EINVAL; goto clear; } + if ((l->dnssec_mode == DNSSEC_NO && mode != DNSSEC_NO) || + (l->dnssec_mode == DNSSEC_ALLOW_DOWNGRADE && mode == DNSSEC_YES)) { + + /* When switching from non-DNSSEC mode to DNSSEC mode, flush the cache. Also when switching from the + * allow-downgrade mode to full DNSSEC mode, flush it too. */ + if (l->unicast_scope) + dns_cache_flush(&l->unicast_scope->cache); + } + + l->dnssec_mode = mode; + return 0; clear: -- cgit v1.2.3-54-g00ecf From f57e3cd5fa709ec0f52531eccba909ac0851927c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 15 Jan 2016 21:38:27 +0100 Subject: resolved: try to reduce number or DnsResourceKeys we keep around by merging them Quite often we read the same RR key multiple times from the same message. Try to replace them by a single object when we notice this. Do so again when we add things to the cache. This should reduce memory consumption a tiny bit. --- src/resolve/resolved-dns-cache.c | 13 +++++++++++++ src/resolve/resolved-dns-packet.c | 5 +++++ src/resolve/resolved-dns-rr.c | 40 +++++++++++++++++++++++++++++++++++++++ src/resolve/resolved-dns-rr.h | 4 +++- 4 files changed, 61 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/resolve/resolved-dns-cache.c b/src/resolve/resolved-dns-cache.c index 301f383809..fdb34d11df 100644 --- a/src/resolve/resolved-dns-cache.c +++ b/src/resolve/resolved-dns-cache.c @@ -247,6 +247,19 @@ static int dns_cache_link_item(DnsCache *c, DnsCacheItem *i) { first = hashmap_get(c->by_key, i->key); if (first) { + _cleanup_(dns_resource_key_unrefp) DnsResourceKey *k = NULL; + + /* Keep a reference to the original key, while we manipulate the list. */ + k = dns_resource_key_ref(first->key); + + /* Now, try to reduce the number of keys we keep */ + dns_resource_key_reduce(&first->key, &i->key); + + if (first->rr) + dns_resource_key_reduce(&first->rr->key, &i->key); + if (i->rr) + dns_resource_key_reduce(&i->rr->key, &i->key); + LIST_PREPEND(by_key, first, i); assert_se(hashmap_replace(c->by_key, first->key, first) >= 0); } else { diff --git a/src/resolve/resolved-dns-packet.c b/src/resolve/resolved-dns-packet.c index 57cfc1ab9b..0acbcfe261 100644 --- a/src/resolve/resolved-dns-packet.c +++ b/src/resolve/resolved-dns-packet.c @@ -2106,6 +2106,7 @@ int dns_packet_extract(DnsPacket *p) { n = DNS_PACKET_RRCOUNT(p); if (n > 0) { + DnsResourceRecord *previous = NULL; bool bad_opt = false; answer = dns_answer_new(n); @@ -2122,6 +2123,10 @@ int dns_packet_extract(DnsPacket *p) { if (r < 0) goto finish; + /* Try to reduce memory usage a bit */ + if (previous) + dns_resource_key_reduce(&rr->key, &previous->key); + if (rr->key->type == DNS_TYPE_OPT) { bool has_rfc6975; diff --git a/src/resolve/resolved-dns-rr.c b/src/resolve/resolved-dns-rr.c index 02c6b239d5..7273ef3825 100644 --- a/src/resolve/resolved-dns-rr.c +++ b/src/resolve/resolved-dns-rr.c @@ -334,6 +334,46 @@ int dns_resource_key_to_string(const DnsResourceKey *key, char **ret) { return 0; } +bool dns_resource_key_reduce(DnsResourceKey **a, DnsResourceKey **b) { + assert(a); + assert(b); + + /* Try to replace one RR key by another if they are identical, thus saving a bit of memory. Note that we do + * this only for RR keys, not for RRs themselves, as they carry a lot of additional metadata (where they come + * from, validity data, and suchlike), and cannot be replaced so easily by other RRs that have the same + * superficial data. */ + + if (!*a) + return false; + if (!*b) + return false; + + /* We refuse merging const keys */ + if ((*a)->n_ref == (unsigned) -1) + return false; + if ((*b)->n_ref == (unsigned) -1) + return false; + + /* Already the same? */ + if (*a == *b) + return true; + + /* Are they really identical? */ + if (dns_resource_key_equal(*a, *b) <= 0) + return false; + + /* Keep the one which already has more references. */ + if ((*a)->n_ref > (*b)->n_ref) { + dns_resource_key_unref(*b); + *b = dns_resource_key_ref(*a); + } else { + dns_resource_key_unref(*a); + *a = dns_resource_key_ref(*b); + } + + return true; +} + DnsResourceRecord* dns_resource_record_new(DnsResourceKey *key) { DnsResourceRecord *rr; diff --git a/src/resolve/resolved-dns-rr.h b/src/resolve/resolved-dns-rr.h index 8ab53211a9..d9c31e81c5 100644 --- a/src/resolve/resolved-dns-rr.h +++ b/src/resolve/resolved-dns-rr.h @@ -81,7 +81,7 @@ enum { }; struct DnsResourceKey { - unsigned n_ref; + unsigned n_ref; /* (unsigned -1) for const keys, see below */ uint16_t class, type; char *_name; /* don't access directy, use DNS_RESOURCE_KEY_NAME()! */ }; @@ -294,6 +294,8 @@ static inline bool dns_key_is_shared(const DnsResourceKey *key) { return IN_SET(key->type, DNS_TYPE_PTR); } +bool dns_resource_key_reduce(DnsResourceKey **a, DnsResourceKey **b); + DnsResourceRecord* dns_resource_record_new(DnsResourceKey *key); DnsResourceRecord* dns_resource_record_new_full(uint16_t class, uint16_t type, const char *name); DnsResourceRecord* dns_resource_record_ref(DnsResourceRecord *rr); -- cgit v1.2.3-54-g00ecf From c15493f4822b7176f0a6449e8f335844f512eed4 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 15 Jan 2016 21:40:20 +0100 Subject: resolved: update DNSSEC TODO --- src/resolve/resolved-dns-dnssec.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/resolve/resolved-dns-dnssec.c b/src/resolve/resolved-dns-dnssec.c index 43fb365d68..f999a8cc77 100644 --- a/src/resolve/resolved-dns-dnssec.c +++ b/src/resolve/resolved-dns-dnssec.c @@ -40,9 +40,10 @@ * - log all RRs that failed validation * - enable by default * - Allow clients to request DNSSEC even if DNSSEC is off - * - find public DNAME test domain * - make sure when getting an NXDOMAIN response through CNAME, we still process the first CNAMEs in the packet - * - flush cache when DNSSEC setting changes + * - update test-complex to also do ResolveAddress lookups + * - extend complex test to check "xn--kprw13d." DNAME domain + * - rework IDNA stuff: only to IDNA for ResolveAddress and ResolveService, and prepare a pair of lookup keys then, never do IDNA comparisons after that * */ #define VERIFY_RRS_MAX 256 -- cgit v1.2.3-54-g00ecf From 43e6779ac2ee8a8a522350eda97311c4f8487ffe Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sun, 17 Jan 2016 21:50:10 +0100 Subject: resolved: when we find a DNAME RR, don't insist in a signed CNAME RR If we have a signed DNAME RR response, there's no need to insist on a signature for a CNAME RR response, after all it is unlikely to be signed, given the implicit synthethis of CNAME through DNAME RRs. --- src/resolve/resolved-dns-answer.c | 37 ++++++++++++++++++++++++++++++++++ src/resolve/resolved-dns-answer.h | 2 ++ src/resolve/resolved-dns-transaction.c | 37 +++++++++++++++++++++++++--------- 3 files changed, 67 insertions(+), 9 deletions(-) (limited to 'src') diff --git a/src/resolve/resolved-dns-answer.c b/src/resolve/resolved-dns-answer.c index c359432a7a..f74e440531 100644 --- a/src/resolve/resolved-dns-answer.c +++ b/src/resolve/resolved-dns-answer.c @@ -821,3 +821,40 @@ void dns_answer_dump(DnsAnswer *answer, FILE *f) { fputc('\n', f); } } + +bool dns_answer_has_dname_for_cname(DnsAnswer *a, DnsResourceRecord *cname) { + DnsResourceRecord *rr; + int r; + + assert(cname); + + /* Checks whether the answer contains a DNAME record that indicates that the specified CNAME record is + * synthesized from it */ + + if (cname->key->type != DNS_TYPE_CNAME) + return 0; + + DNS_ANSWER_FOREACH(rr, a) { + _cleanup_free_ char *n = NULL; + + if (rr->key->type != DNS_TYPE_DNAME) + continue; + if (rr->key->class != cname->key->class) + continue; + + r = dns_name_change_suffix(cname->cname.name, rr->dname.name, DNS_RESOURCE_KEY_NAME(rr->key), &n); + if (r < 0) + return r; + if (r == 0) + continue; + + r = dns_name_equal(n, DNS_RESOURCE_KEY_NAME(cname->key)); + if (r < 0) + return r; + if (r > 0) + return 1; + + } + + return 0; +} diff --git a/src/resolve/resolved-dns-answer.h b/src/resolve/resolved-dns-answer.h index 3eff21f8d0..1875fd6136 100644 --- a/src/resolve/resolved-dns-answer.h +++ b/src/resolve/resolved-dns-answer.h @@ -83,6 +83,8 @@ int dns_answer_remove_by_rr(DnsAnswer **a, DnsResourceRecord *rr); int dns_answer_copy_by_key(DnsAnswer **a, DnsAnswer *source, const DnsResourceKey *key, DnsAnswerFlags or_flags); int dns_answer_move_by_key(DnsAnswer **to, DnsAnswer **from, const DnsResourceKey *key, DnsAnswerFlags or_flags); +bool dns_answer_has_dname_for_cname(DnsAnswer *a, DnsResourceRecord *cname); + static inline unsigned dns_answer_size(DnsAnswer *a) { return a ? a->n_rrs : 0; } diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index 72ef283dde..393171a2ad 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -1827,6 +1827,12 @@ int dns_transaction_request_dnssec_keys(DnsTransaction *t) { if (r > 0) continue; + r = dns_answer_has_dname_for_cname(t->answer, rr); + if (r < 0) + return r; + if (r > 0) + continue; + name = DNS_RESOURCE_KEY_NAME(rr->key); r = dns_name_parent(&name); if (r < 0) @@ -2719,17 +2725,30 @@ int dns_transaction_validate_dnssec(DnsTransaction *t) { if (r < 0) return r; if (r > 0) { - /* This is a primary response - * to our question, and it - * failed validation. That's - * fatal. */ - t->answer_dnssec_result = result; - return 0; + + /* Look for a matching DNAME for this CNAME */ + r = dns_answer_has_dname_for_cname(t->answer, rr); + if (r < 0) + return r; + if (r == 0) { + /* Also look among the stuff we already validated */ + r = dns_answer_has_dname_for_cname(validated, rr); + if (r < 0) + return r; + } + + if (r == 0) { + /* This is a primary response to our question, and it failed validation. That's + * fatal. */ + t->answer_dnssec_result = result; + return 0; + } + + /* This is a primary response, but we do have a DNAME RR in the RR that can replay this + * CNAME, hence rely on that, and we can remove the CNAME in favour of it. */ } - /* This is just some auxiliary - * data. Just remove the RRset and - * continue. */ + /* This is just some auxiliary data. Just remove the RRset and continue. */ r = dns_answer_remove_by_key(&t->answer, rr->key); if (r < 0) return r; -- cgit v1.2.3-54-g00ecf From bc7669cff9b7884af28814e3e47f1711315da482 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sun, 17 Jan 2016 21:53:16 +0100 Subject: resolved: fix logging about DNAME redirection --- src/resolve/resolved-dns-query.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/resolve/resolved-dns-query.c b/src/resolve/resolved-dns-query.c index 1948d59fc4..c6da8d0a87 100644 --- a/src/resolve/resolved-dns-query.c +++ b/src/resolve/resolved-dns-query.c @@ -1120,8 +1120,6 @@ static int dns_query_cname_redirect(DnsQuery *q, const DnsResourceRecord *cname) assert(q); - log_debug("Following CNAME %s → %s", dns_question_first_name(q->question), cname->cname.name); - q->n_cname_redirects ++; if (q->n_cname_redirects > CNAME_MAX) return -ELOOP; @@ -1130,6 +1128,8 @@ static int dns_query_cname_redirect(DnsQuery *q, const DnsResourceRecord *cname) if (r < 0) return r; + log_debug("Following CNAME/DNAME %s → %s", dns_question_first_name(q->question), dns_question_first_name(nq)); + dns_question_unref(q->question); q->question = nq; nq = NULL; -- cgit v1.2.3-54-g00ecf