From f506d09f714ce7c405d27cdf5939e1fdc3ed1a07 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 12 Jan 2016 22:14:49 +0100 Subject: [PATCH 01/16] update TODO --- TODO | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/TODO b/TODO index 369805fcee7..7dc91618e34 100644 --- a/TODO +++ b/TODO @@ -33,6 +33,15 @@ Janitorial Clean-ups: Features: +* add systemctl stop --job-mode=triggering that follows TRIGGERED_BY deps and adds them to the same transaction + +* coredump logic should use prlimit() to query RLIMIT_CORE of the dumpee and honour it + +* Add a MaxRuntimeSec= setting for service units (or units in general) to terminate units after they ran for a certain + amount of time + +* Maybe add a way how users can "pin" units into memory, so that they are not subject to automatic GC? + * PID1: find a way how we can reload unit file configuration for specific units only, without reloading the whole of systemd From 5ae5cd4052d85368ec0ca17562d404fa476badc5 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 13 Jan 2016 01:04:03 +0100 Subject: [PATCH 02/16] resolved: consider inverted RRSIG validity intervals expired --- src/resolve/resolved-dns-dnssec.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/resolve/resolved-dns-dnssec.c b/src/resolve/resolved-dns-dnssec.c index 43fcbe14605..3f487f5e0e3 100644 --- a/src/resolve/resolved-dns-dnssec.c +++ b/src/resolve/resolved-dns-dnssec.c @@ -442,8 +442,9 @@ static int dnssec_rrsig_expired(DnsResourceRecord *rrsig, usec_t realtime) { expiration = rrsig->rrsig.expiration * USEC_PER_SEC; inception = rrsig->rrsig.inception * USEC_PER_SEC; + /* Consider inverted validity intervals as expired */ if (inception > expiration) - return -EKEYREJECTED; + return true; /* Permit a certain amount of clock skew of 10% of the valid * time range. This takes inspiration from unbound's From d41084a5860999e7fd45a95130a2444dda035d7d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 13 Jan 2016 02:14:20 +0100 Subject: [PATCH 03/16] resolved: allocate bounded strings on stack instead of heap, if we can --- src/resolve/resolved-dns-dnssec.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/resolve/resolved-dns-dnssec.c b/src/resolve/resolved-dns-dnssec.c index 3f487f5e0e3..6f5a61a41d3 100644 --- a/src/resolve/resolved-dns-dnssec.c +++ b/src/resolve/resolved-dns-dnssec.c @@ -1275,8 +1275,8 @@ static int nsec3_hashed_domain_make(DnsResourceRecord *nsec3, const char *domain * name uses an NSEC3 record with the opt-out bit set. Lastly, if we are given insufficient NSEC3 records * to conclude anything we indicate this by returning NO_RR. */ static int dnssec_test_nsec3(DnsAnswer *answer, DnsResourceKey *key, DnssecNsecResult *result, bool *authenticated, uint32_t *ttl) { - _cleanup_free_ char *next_closer_domain = NULL, *wildcard = NULL, *wildcard_domain = NULL; - const char *zone, *p, *pp = NULL; + _cleanup_free_ char *next_closer_domain = NULL, *wildcard_domain = NULL; + const char *zone, *p, *pp = NULL, *wildcard; DnsResourceRecord *rr, *enclosure_rr, *zone_rr, *wildcard_rr = NULL; DnsAnswerFlags flags; int hashed_size, r; @@ -1402,10 +1402,7 @@ found_closest_encloser: /* Prove that there is no next closer and whether or not there is a wildcard domain. */ - wildcard = strappend("*.", p); - if (!wildcard) - return -ENOMEM; - + wildcard = strjoina("*.", p); r = nsec3_hashed_domain_make(enclosure_rr, wildcard, zone, &wildcard_domain); if (r < 0) return r; From 4b2e9cfcf0dffec9d990000ee1555e49a9049246 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 13 Jan 2016 02:19:57 +0100 Subject: [PATCH 04/16] systemctl: improve wording of "systemctl enable" messages Closes: #2299 --- src/systemctl/systemctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 1448d974bd4..cbf71268c60 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -1979,7 +1979,7 @@ static void dump_unit_file_changes(const UnitFileChange *changes, unsigned n_cha for (i = 0; i < n_changes; i++) { if (changes[i].type == UNIT_FILE_SYMLINK) - log_info("Created symlink from %s to %s.", changes[i].path, changes[i].source); + log_info("Created symlink %s, pointing to %s.", changes[i].path, changes[i].source); else log_info("Removed symlink %s.", changes[i].path); } From 522d85ae0a3dd80ec58033289c4f63ba7dfc63a8 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 13 Jan 2016 02:21:16 +0100 Subject: [PATCH 05/16] basic: add new ascii_strcasecmp_n() call --- src/basic/string-util.c | 15 +++++++++++++++ src/basic/string-util.h | 2 ++ src/test/test-string-util.c | 28 ++++++++++++++++++++++++++++ 3 files changed, 45 insertions(+) diff --git a/src/basic/string-util.c b/src/basic/string-util.c index 849e457439a..b7b8c97bc02 100644 --- a/src/basic/string-util.c +++ b/src/basic/string-util.c @@ -348,6 +348,21 @@ char *ascii_strlower_n(char *t, size_t n) { return t; } +int ascii_strcasecmp_n(const char *a, const char *b, size_t n) { + + for (; n > 0; a++, b++, n--) { + int x, y; + + x = (int) (uint8_t) ascii_tolower(*a); + y = (int) (uint8_t) ascii_tolower(*b); + + if (x != y) + return x - y; + } + + return 0; +} + bool chars_intersect(const char *a, const char *b) { const char *p; diff --git a/src/basic/string-util.h b/src/basic/string-util.h index 1ac6bcd6f8c..6374e11119f 100644 --- a/src/basic/string-util.h +++ b/src/basic/string-util.h @@ -134,6 +134,8 @@ char ascii_tolower(char x); char *ascii_strlower(char *s); char *ascii_strlower_n(char *s, size_t n); +int ascii_strcasecmp_n(const char *a, const char *b, size_t n); + bool chars_intersect(const char *a, const char *b) _pure_; static inline bool _pure_ in_charset(const char *s, const char* charset) { diff --git a/src/test/test-string-util.c b/src/test/test-string-util.c index 25444c794ac..47581087063 100644 --- a/src/test/test-string-util.c +++ b/src/test/test-string-util.c @@ -55,7 +55,35 @@ static void test_string_erase(void) { assert_se(streq(string_erase(x), "xxxxxxxxx")); } +static void test_ascii_strcasecmp_n(void) { + + assert_se(ascii_strcasecmp_n("", "", 0) == 0); + assert_se(ascii_strcasecmp_n("", "", 1) == 0); + assert_se(ascii_strcasecmp_n("", "a", 1) < 0); + assert_se(ascii_strcasecmp_n("", "a", 2) < 0); + assert_se(ascii_strcasecmp_n("a", "", 1) > 0); + assert_se(ascii_strcasecmp_n("a", "", 2) > 0); + assert_se(ascii_strcasecmp_n("a", "a", 1) == 0); + assert_se(ascii_strcasecmp_n("a", "a", 2) == 0); + assert_se(ascii_strcasecmp_n("a", "b", 1) < 0); + assert_se(ascii_strcasecmp_n("a", "b", 2) < 0); + assert_se(ascii_strcasecmp_n("b", "a", 1) > 0); + assert_se(ascii_strcasecmp_n("b", "a", 2) > 0); + assert_se(ascii_strcasecmp_n("xxxxyxxxx", "xxxxYxxxx", 9) == 0); + assert_se(ascii_strcasecmp_n("xxxxxxxxx", "xxxxyxxxx", 9) < 0); + assert_se(ascii_strcasecmp_n("xxxxXxxxx", "xxxxyxxxx", 9) < 0); + assert_se(ascii_strcasecmp_n("xxxxxxxxx", "xxxxYxxxx", 9) < 0); + assert_se(ascii_strcasecmp_n("xxxxXxxxx", "xxxxYxxxx", 9) < 0); + + assert_se(ascii_strcasecmp_n("xxxxYxxxx", "xxxxYxxxx", 9) == 0); + assert_se(ascii_strcasecmp_n("xxxxyxxxx", "xxxxxxxxx", 9) > 0); + assert_se(ascii_strcasecmp_n("xxxxyxxxx", "xxxxXxxxx", 9) > 0); + assert_se(ascii_strcasecmp_n("xxxxYxxxx", "xxxxxxxxx", 9) > 0); + assert_se(ascii_strcasecmp_n("xxxxYxxxx", "xxxxXxxxx", 9) > 0); +} + int main(int argc, char *argv[]) { test_string_erase(); + test_ascii_strcasecmp_n(); return 0; } From eb241cdbeea092d891137c018cacf919a895e6a6 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 13 Jan 2016 02:23:08 +0100 Subject: [PATCH 06/16] shared: add new dns_name_startswith() call dns_name_startswith() is to dns_name_endswith() as startswith() is to endswith(). --- src/shared/dns-domain.c | 49 +++++++++++++++++++++++++++++++++++++- src/shared/dns-domain.h | 1 + src/test/test-dns-domain.c | 20 ++++++++++++++++ 3 files changed, 69 insertions(+), 1 deletion(-) diff --git a/src/shared/dns-domain.c b/src/shared/dns-domain.c index 59475115ba7..e777badb6b0 100644 --- a/src/shared/dns-domain.c +++ b/src/shared/dns-domain.c @@ -263,7 +263,6 @@ int dns_label_escape(const char *p, size_t l, char *dest, size_t sz) { *(q++) = '0' + (char) ((uint8_t) *p % 10); sz -= 4; - } p++; @@ -653,6 +652,54 @@ int dns_name_endswith(const char *name, const char *suffix) { } } +static int dns_label_unescape_undo_idna(const char **name, char *dest, size_t sz) { + int r, k; + + /* Clobbers all arguments on failure... */ + + r = dns_label_unescape(name, dest, sz); + if (r <= 0) + return r; + + k = dns_label_undo_idna(dest, r, dest, sz); + if (k < 0) + return k; + if (k == 0) /* not an IDNA name */ + return r; + + return k; +} + +int dns_name_startswith(const char *name, const char *prefix) { + const char *n, *p; + int r, q; + + assert(name); + assert(prefix); + + n = name; + p = prefix; + + for (;;) { + char ln[DNS_LABEL_MAX], lp[DNS_LABEL_MAX]; + + r = dns_label_unescape_undo_idna(&p, lp, sizeof(lp)); + if (r < 0) + return r; + if (r == 0) + return true; + + q = dns_label_unescape_undo_idna(&n, ln, sizeof(ln)); + if (q < 0) + return q; + + if (r != q) + return false; + if (ascii_strcasecmp_n(ln, lp, r) != 0) + return false; + } +} + int dns_name_change_suffix(const char *name, const char *old_suffix, const char *new_suffix, char **ret) { const char *n, *s, *saved_before = NULL, *saved_after = NULL, *prefix; int r, q, k, w; diff --git a/src/shared/dns-domain.h b/src/shared/dns-domain.h index dd8ae3ac988..4fbe0a618f9 100644 --- a/src/shared/dns-domain.h +++ b/src/shared/dns-domain.h @@ -83,6 +83,7 @@ extern const struct hash_ops dns_name_hash_ops; int dns_name_between(const char *a, const char *b, const char *c); int dns_name_equal(const char *x, const char *y); int dns_name_endswith(const char *name, const char *suffix); +int dns_name_startswith(const char *name, const char *prefix); int dns_name_change_suffix(const char *name, const char *old_suffix, const char *new_suffix, char **ret); diff --git a/src/test/test-dns-domain.c b/src/test/test-dns-domain.c index 6c3c49908f1..fe3ae453490 100644 --- a/src/test/test-dns-domain.c +++ b/src/test/test-dns-domain.c @@ -276,6 +276,25 @@ static void test_dns_name_endswith(void) { test_dns_name_endswith_one("x.y\001.z", "waldo", -EINVAL); } +static void test_dns_name_startswith_one(const char *a, const char *b, int ret) { + assert_se(dns_name_startswith(a, b) == ret); +} + +static void test_dns_name_startswith(void) { + test_dns_name_startswith_one("", "", true); + test_dns_name_startswith_one("", "xxx", false); + test_dns_name_startswith_one("xxx", "", true); + test_dns_name_startswith_one("x", "x", true); + test_dns_name_startswith_one("x", "y", false); + test_dns_name_startswith_one("x.y", "x.y", true); + test_dns_name_startswith_one("x.y", "y.x", false); + test_dns_name_startswith_one("x.y", "x", true); + test_dns_name_startswith_one("x.y", "X", true); + test_dns_name_startswith_one("x.y", "y", false); + test_dns_name_startswith_one("x.y", "", true); + test_dns_name_startswith_one("x.y", "X", true); +} + static void test_dns_name_is_root(void) { assert_se(dns_name_is_root("")); assert_se(dns_name_is_root(".")); @@ -567,6 +586,7 @@ int main(int argc, char *argv[]) { test_dns_name_normalize(); test_dns_name_equal(); test_dns_name_endswith(); + test_dns_name_startswith(); test_dns_name_between(); test_dns_name_is_root(); test_dns_name_is_single_label(); From 7715f91dca0ce4622daf8beab582d57ec49d005e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 13 Jan 2016 02:25:32 +0100 Subject: [PATCH 07/16] resolved: optimize dnssec_verify_rrset() a bit Let's determine the source of synthesis once instead of for each RR in the RRset. --- src/resolve/resolved-dns-dnssec.c | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/src/resolve/resolved-dns-dnssec.c b/src/resolve/resolved-dns-dnssec.c index 6f5a61a41d3..8dfb5edbc0b 100644 --- a/src/resolve/resolved-dns-dnssec.c +++ b/src/resolve/resolved-dns-dnssec.c @@ -513,8 +513,9 @@ int dnssec_verify_rrset( DnsResourceRecord **list, *rr; gcry_md_hd_t md = NULL; int r, md_algorithm; - bool wildcard = false; size_t k, n = 0; + bool wildcard; + const char *source; assert(key); assert(rrsig); @@ -543,6 +544,12 @@ int dnssec_verify_rrset( 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); + if (r < 0) + return r; + wildcard = r > 0; + /* Collect all relevant RRs in a single array, so that we can look at the RRset */ list = newa(DnsResourceRecord *, dns_answer_size(a)); @@ -593,22 +600,19 @@ int dnssec_verify_rrset( goto finish; gcry_md_write(md, wire_format_name, r); + /* Convert the source of synthesis into wire format */ + r = dns_name_to_wire_format(source, wire_format_name, sizeof(wire_format_name), true); + if (r < 0) + goto finish; + for (k = 0; k < n; k++) { - const char *suffix; size_t l; + rr = list[k]; - r = dns_name_suffix(DNS_RESOURCE_KEY_NAME(rr->key), rrsig->rrsig.labels, &suffix); - if (r < 0) - goto finish; - if (r > 0) /* This is a wildcard! */ { + /* Hash the source of synthesis. If this is a wildcard, then prefix it with the *. label */ + if (wildcard) gcry_md_write(md, (uint8_t[]) { 1, '*'}, 2); - wildcard = true; - } - - r = dns_name_to_wire_format(suffix, wire_format_name, sizeof(wire_format_name), true); - if (r < 0) - goto finish; gcry_md_write(md, wire_format_name, r); md_add_uint16(md, rr->key->type); From 7160eb1b867a4bc64522287352fbe2a6aa687d2a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 13 Jan 2016 02:26:23 +0100 Subject: [PATCH 08/16] resolved: properly handles RRs in domains beginning in an asterisk label Properly handle RRs that begin with an asterisk label. These are the unexpanded forms of wildcard domains and appear in NSEC RRs for example. We need to make sure we handle the signatures of these RRs properly, since they mostly are considered normal RRs, except that the RRSIG labels counter is one off for them, as the asterisk label is always excluded of the signature. --- src/resolve/resolved-dns-dnssec.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/resolve/resolved-dns-dnssec.c b/src/resolve/resolved-dns-dnssec.c index 8dfb5edbc0b..a18ae56b9ea 100644 --- a/src/resolve/resolved-dns-dnssec.c +++ b/src/resolve/resolved-dns-dnssec.c @@ -548,7 +548,18 @@ int dnssec_verify_rrset( r = dns_name_suffix(DNS_RESOURCE_KEY_NAME(key), rrsig->rrsig.labels, &source); if (r < 0) return r; - wildcard = r > 0; + 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), "*"); + if (r < 0) + return r; + if (r > 0) + source = DNS_RESOURCE_KEY_NAME(key); + + wildcard = r == 0; + } else + wildcard = r > 0; /* Collect all relevant RRs in a single array, so that we can look at the RRset */ list = newa(DnsResourceRecord *, dns_answer_size(a)); From e8233bce196a14fa3ebde2969594fcdfa4404e19 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 13 Jan 2016 02:29:31 +0100 Subject: [PATCH 09/16] resolved: refuse validating wildcard RRs for SOA, NSEC3, DNAME --- src/resolve/dns-type.c | 15 +++++++++++++++ src/resolve/dns-type.h | 1 + src/resolve/resolved-dns-dnssec.c | 5 +++++ 3 files changed, 21 insertions(+) diff --git a/src/resolve/dns-type.c b/src/resolve/dns-type.c index fb8228048da..47a37fa0a75 100644 --- a/src/resolve/dns-type.c +++ b/src/resolve/dns-type.c @@ -120,6 +120,21 @@ bool dns_type_may_redirect(uint16_t type) { DNS_TYPE_KEY); } +bool dns_type_may_wildcard(uint16_t type) { + + /* The following records may not be expanded from wildcard RRsets */ + + if (dns_type_is_pseudo(type)) + return false; + + return !IN_SET(type, + DNS_TYPE_NSEC3, + DNS_TYPE_SOA, + + /* Prohibited by https://tools.ietf.org/html/rfc4592#section-4.4 */ + DNS_TYPE_DNAME); +} + 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 45080fd243a..747bc854e1a 100644 --- a/src/resolve/dns-type.h +++ b/src/resolve/dns-type.h @@ -131,6 +131,7 @@ bool dns_type_is_valid_rr(uint16_t type); 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_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 a18ae56b9ea..6f0f8f837e7 100644 --- a/src/resolve/resolved-dns-dnssec.c +++ b/src/resolve/resolved-dns-dnssec.c @@ -548,6 +548,11 @@ int dnssec_verify_rrset( r = dns_name_suffix(DNS_RESOURCE_KEY_NAME(key), rrsig->rrsig.labels, &source); if (r < 0) return r; + if (r > 0 && !dns_type_may_wildcard(rrsig->rrsig.type_covered)) { + /* We refuse to validate NSEC3 or SOA RRs that are synthesized from wildcards */ + *result = DNSSEC_INVALID; + return 0; + } 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. */ From e926785a1feff01901e6298261a9f635791d3b17 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 13 Jan 2016 02:45:28 +0100 Subject: [PATCH 10/16] resolved: implement the full NSEC and NSEC3 postive wildcard proofs --- src/resolve/resolved-dns-answer.c | 27 +++++ src/resolve/resolved-dns-answer.h | 1 + src/resolve/resolved-dns-dnssec.c | 144 ++++++++++++++++++++++++- src/resolve/resolved-dns-dnssec.h | 5 +- src/resolve/resolved-dns-transaction.c | 26 ++--- 5 files changed, 186 insertions(+), 17 deletions(-) diff --git a/src/resolve/resolved-dns-answer.c b/src/resolve/resolved-dns-answer.c index b50558e2808..c359432a7a0 100644 --- a/src/resolve/resolved-dns-answer.c +++ b/src/resolve/resolved-dns-answer.c @@ -320,6 +320,33 @@ int dns_answer_contains_nsec_or_nsec3(DnsAnswer *a) { return false; } +int dns_answer_contains_zone_nsec3(DnsAnswer *answer, const char *zone) { + DnsResourceRecord *rr; + int r; + + /* Checks whether the specified answer contains at least one NSEC3 RR in the specified zone */ + + DNS_ANSWER_FOREACH(rr, answer) { + const char *p; + + if (rr->key->type != DNS_TYPE_NSEC3) + continue; + + p = DNS_RESOURCE_KEY_NAME(rr->key); + r = dns_name_parent(&p); + if (r < 0) + return r; + if (r == 0) + continue; + + r = dns_name_equal(p, zone); + if (r != 0) + return r; + } + + return false; +} + int dns_answer_find_soa(DnsAnswer *a, const DnsResourceKey *key, DnsResourceRecord **ret, DnsAnswerFlags *flags) { DnsResourceRecord *rr, *soa = NULL; DnsAnswerFlags rr_flags, soa_flags = 0; diff --git a/src/resolve/resolved-dns-answer.h b/src/resolve/resolved-dns-answer.h index 715e487d94b..3eff21f8d09 100644 --- a/src/resolve/resolved-dns-answer.h +++ b/src/resolve/resolved-dns-answer.h @@ -64,6 +64,7 @@ int dns_answer_match_key(DnsAnswer *a, const DnsResourceKey *key, DnsAnswerFlags int dns_answer_contains_rr(DnsAnswer *a, DnsResourceRecord *rr, DnsAnswerFlags *combined_flags); int dns_answer_contains_key(DnsAnswer *a, const DnsResourceKey *key, DnsAnswerFlags *combined_flags); int dns_answer_contains_nsec_or_nsec3(DnsAnswer *a); +int dns_answer_contains_zone_nsec3(DnsAnswer *answer, const char *zone); int dns_answer_find_soa(DnsAnswer *a, const DnsResourceKey *key, DnsResourceRecord **ret, DnsAnswerFlags *flags); int dns_answer_find_cname_or_dname(DnsAnswer *a, const DnsResourceKey *key, DnsResourceRecord **ret, DnsAnswerFlags *flags); diff --git a/src/resolve/resolved-dns-dnssec.c b/src/resolve/resolved-dns-dnssec.c index 6f0f8f837e7..afff979b5af 100644 --- a/src/resolve/resolved-dns-dnssec.c +++ b/src/resolve/resolved-dns-dnssec.c @@ -1604,7 +1604,7 @@ int dnssec_nsec_test(DnsAnswer *answer, DnsResourceKey *key, DnssecNsecResult *r return 0; } -int dnssec_nsec_test_between(DnsAnswer *answer, const char *name, const char *zone, bool *authenticated) { +int dnssec_nsec_test_enclosed(DnsAnswer *answer, uint16_t type, const char *name, const char *zone, bool *authenticated) { DnsResourceRecord *rr; DnsAnswerFlags flags; int r; @@ -1618,6 +1618,9 @@ int dnssec_nsec_test_between(DnsAnswer *answer, const char *name, const char *zo DNS_ANSWER_FOREACH_FLAGS(rr, flags, answer) { bool found = false; + 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; @@ -1685,6 +1688,145 @@ int dnssec_nsec_test_between(DnsAnswer *answer, const char *name, const char *zo return 0; } +static int dnssec_test_positive_wildcard_nsec3( + DnsAnswer *answer, + const char *name, + const char *source, + const char *zone, + bool *authenticated) { + + const char *next_closer = NULL; + int r; + + /* Run a positive NSEC3 wildcard proof. Specifically: + * + * A proof that the the "next closer" of the generating wildcard does not exist. + * + * Note a key difference between the NSEC3 and NSEC versions of the proof. NSEC RRs don't have to exist for + * empty non-transients. NSEC3 RRs however have to. This means it's sufficient to check if the next closer name + * exists for the NSEC3 RR and we are done. + * + * To prove that a.b.c.d.e.f is rightfully synthesized from a wildcard *.d.e.f all we have to check is that + * c.d.e.f does not exist. */ + + for (;;) { + next_closer = name; + r = dns_name_parent(&name); + if (r < 0) + return r; + if (r == 0) + return 0; + + r = dns_name_equal(name, source); + if (r < 0) + return r; + if (r > 0) + break; + } + + return dnssec_nsec_test_enclosed(answer, DNS_TYPE_NSEC3, next_closer, zone, authenticated); +} + +static int dnssec_test_positive_wildcard_nsec( + DnsAnswer *answer, + const char *name, + const char *source, + const char *zone, + bool *_authenticated) { + + bool authenticated = true; + int r; + + /* Run a positive NSEC wildcard proof. Specifically: + * + * A proof that there's neither a wildcard name nor a non-wildcard name that is a suffix of the name "name" and + * a prefix of the synthesizing source "source" in the zone "zone". + * + * See RFC 5155, Section 8.8 and RFC 4035, Section 5.3.4 + * + * Note that if we want to prove that a.b.c.d.e.f is rightfully synthesized from a wildcard *.d.e.f, then we + * have to prove that none of the following exist: + * + * 1) a.b.c.d.e.f + * 2) *.b.c.d.e.f + * 3) b.c.d.e.f + * 4) *.c.d.e.f + * 5) c.d.e.f + * + */ + + for (;;) { + _cleanup_free_ char *wc = NULL; + bool a = false; + + /* Check if there's an NSEC or NSEC3 RR that proves that the mame we determined is really non-existing, + * i.e between the owner name and the next name of an NSEC RR. */ + r = dnssec_nsec_test_enclosed(answer, DNS_TYPE_NSEC, name, zone, &a); + if (r <= 0) + return r; + + authenticated = authenticated && a; + + /* Strip one label off */ + r = dns_name_parent(&name); + if (r <= 0) + return r; + + /* Did we reach the source of synthesis? */ + r = dns_name_equal(name, source); + if (r < 0) + return r; + if (r > 0) { + /* Successful exit */ + *_authenticated = authenticated; + return 1; + } + + /* Safety check, that the source of synthesis is still our suffix */ + r = dns_name_endswith(name, source); + if (r < 0) + return r; + if (r == 0) + return -EBADMSG; + + /* Replace the label we stripped off with an asterisk */ + wc = strappend("*.", name); + if (!wc) + return -ENOMEM; + + /* And check if the proof holds for the asterisk name, too */ + r = dnssec_nsec_test_enclosed(answer, DNS_TYPE_NSEC, wc, zone, &a); + if (r <= 0) + return r; + + authenticated = authenticated && a; + /* In the next iteration we'll check the non-asterisk-prefixed version */ + } +} + +int dnssec_test_positive_wildcard( + DnsAnswer *answer, + const char *name, + const char *source, + const char *zone, + bool *authenticated) { + + int r; + + assert(name); + assert(source); + assert(zone); + assert(authenticated); + + r = dns_answer_contains_zone_nsec3(answer, zone); + if (r < 0) + return r; + if (r > 0) + return dnssec_test_positive_wildcard_nsec3(answer, name, source, zone, authenticated); + else + return dnssec_test_positive_wildcard_nsec(answer, name, source, zone, authenticated); +} + static const char* const dnssec_result_table[_DNSSEC_RESULT_MAX] = { [DNSSEC_VALIDATED] = "validated", [DNSSEC_VALIDATED_WILDCARD] = "validated-wildcard", diff --git a/src/resolve/resolved-dns-dnssec.h b/src/resolve/resolved-dns-dnssec.h index 8a9bcf5b91a..b9d32db1200 100644 --- a/src/resolve/resolved-dns-dnssec.h +++ b/src/resolve/resolved-dns-dnssec.h @@ -83,7 +83,10 @@ typedef enum DnssecNsecResult { } DnssecNsecResult; int dnssec_nsec_test(DnsAnswer *answer, DnsResourceKey *key, DnssecNsecResult *result, bool *authenticated, uint32_t *ttl); -int dnssec_nsec_test_between(DnsAnswer *answer, const char *name, const char *zone, bool *authenticated); + +int dnssec_nsec_test_enclosed(DnsAnswer *answer, uint16_t type, const char *name, const char *zone, bool *authenticated); + +int dnssec_test_positive_wildcard(DnsAnswer *a, const char *name, const char *source, const char *zone, bool *authenticated); const char* dnssec_result_to_string(DnssecResult m) _const_; DnssecResult dnssec_result_from_string(const char *s) _pure_; diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index 9ee10f21c87..c7d2d82ecf8 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -2531,28 +2531,24 @@ int dns_transaction_validate_dnssec(DnsTransaction *t) { if (result == DNSSEC_VALIDATED_WILDCARD) { bool authenticated = false; - const char *suffix; + const char *source; - /* This RRset validated, but as a wildcard. This means we need to proof via NSEC/NSEC3 - * that no matching non-wildcard RR exists. - * - * See RFC 5155, Section 8.8 and RFC 4035, Section 5.3.4*/ + /* This RRset validated, but as a wildcard. This means we need to prove via NSEC/NSEC3 + * that no matching non-wildcard RR exists.*/ - r = dns_name_suffix(DNS_RESOURCE_KEY_NAME(rr->key), rrsig->rrsig.labels, &suffix); + /* First step, determine the source of synthesis */ + r = dns_name_suffix(DNS_RESOURCE_KEY_NAME(rr->key), rrsig->rrsig.labels, &source); if (r < 0) return r; if (r == 0) return -EBADMSG; - r = dns_name_parent(&suffix); - if (r < 0) - return r; - if (r == 0) - return -EBADMSG; - - r = dnssec_nsec_test_between(validated, DNS_RESOURCE_KEY_NAME(rr->key), suffix, &authenticated); - if (r < 0) - return r; + r = dnssec_test_positive_wildcard( + validated, + DNS_RESOURCE_KEY_NAME(rr->key), + source, + rrsig->rrsig.signer, + &authenticated); /* Unless the NSEC proof showed that the key really doesn't exist something is off. */ if (r == 0) From 3095011d15e03a5d724fa0141463434c8bf8380c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 13 Jan 2016 03:03:26 +0100 Subject: [PATCH 11/16] shared: replace a few invocations of strcasecmp() for DNS labels with ascii_strcasecmp_n() This makes our code compatible with embedded NUL bytes, as we don't care about NUL bytes anymore. --- src/shared/dns-domain.c | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/src/shared/dns-domain.c b/src/shared/dns-domain.c index e777badb6b0..a127e594ac8 100644 --- a/src/shared/dns-domain.c +++ b/src/shared/dns-domain.c @@ -559,10 +559,7 @@ int dns_name_equal(const char *x, const char *y) { assert(y); for (;;) { - char la[DNS_LABEL_MAX+1], lb[DNS_LABEL_MAX+1]; - - if (*x == 0 && *y == 0) - return true; + char la[DNS_LABEL_MAX], lb[DNS_LABEL_MAX]; r = dns_label_unescape(&x, la, sizeof(la)); if (r < 0) @@ -586,13 +583,12 @@ int dns_name_equal(const char *x, const char *y) { q = w; } - /* If one name had fewer labels than the other, this - * will show up as empty label here, which the - * strcasecmp() below will properly consider different - * from a non-empty label. */ + if (r != q) + return false; + if (r == 0) + return true; - la[r] = lb[q] = 0; - if (strcasecmp(la, lb) != 0) + if (ascii_strcasecmp_n(la, lb, r) != 0) return false; } } @@ -608,7 +604,7 @@ int dns_name_endswith(const char *name, const char *suffix) { s = suffix; for (;;) { - char ln[DNS_LABEL_MAX+1], ls[DNS_LABEL_MAX+1]; + char ln[DNS_LABEL_MAX], ls[DNS_LABEL_MAX]; r = dns_label_unescape(&n, ln, sizeof(ln)); if (r < 0) @@ -640,9 +636,7 @@ int dns_name_endswith(const char *name, const char *suffix) { if (r == 0 && saved_n == n) return false; - ln[r] = ls[q] = 0; - - if (r != q || strcasecmp(ln, ls)) { + if (r != q || ascii_strcasecmp_n(ln, ls, r) != 0) { /* Not the same, let's jump back, and try with the next label again */ s = suffix; @@ -713,7 +707,7 @@ int dns_name_change_suffix(const char *name, const char *old_suffix, const char s = old_suffix; for (;;) { - char ln[DNS_LABEL_MAX+1], ls[DNS_LABEL_MAX+1]; + char ln[DNS_LABEL_MAX], ls[DNS_LABEL_MAX]; if (!saved_before) saved_before = n; @@ -750,9 +744,7 @@ int dns_name_change_suffix(const char *name, const char *old_suffix, const char return 0; } - ln[r] = ls[q] = 0; - - if (r != q || strcasecmp(ln, ls)) { + if (r != q || ascii_strcasecmp_n(ln, ls, r) != 0) { /* Not the same, let's jump back, and try with the next label again */ s = old_suffix; From c174983474d4a010a18e3bb9a59e351a442480f5 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 13 Jan 2016 19:45:05 +0100 Subject: [PATCH 12/16] basic: add ascii_strcasecmp_nn() call In contrast to ascii_strcasecmp_nn() it takes two character buffers with their individual length. It will then compare the buffers up the smaller size of the two buffers, and finally the length themselves. --- src/basic/string-util.c | 15 +++++++++++++++ src/basic/string-util.h | 1 + src/test/test-string-util.c | 18 ++++++++++++++++++ 3 files changed, 34 insertions(+) diff --git a/src/basic/string-util.c b/src/basic/string-util.c index b7b8c97bc02..1f95a9abba2 100644 --- a/src/basic/string-util.c +++ b/src/basic/string-util.c @@ -363,6 +363,21 @@ int ascii_strcasecmp_n(const char *a, const char *b, size_t n) { return 0; } +int ascii_strcasecmp_nn(const char *a, size_t n, const char *b, size_t m) { + int r; + + r = ascii_strcasecmp_n(a, b, MIN(n, m)); + if (r != 0) + return r; + + if (n < m) + return -1; + else if (n > m) + return 1; + else + return 0; +} + bool chars_intersect(const char *a, const char *b) { const char *p; diff --git a/src/basic/string-util.h b/src/basic/string-util.h index 6374e11119f..8ea18f45aa6 100644 --- a/src/basic/string-util.h +++ b/src/basic/string-util.h @@ -135,6 +135,7 @@ char *ascii_strlower(char *s); char *ascii_strlower_n(char *s, size_t n); int ascii_strcasecmp_n(const char *a, const char *b, size_t n); +int ascii_strcasecmp_nn(const char *a, size_t n, const char *b, size_t m); bool chars_intersect(const char *a, const char *b) _pure_; diff --git a/src/test/test-string-util.c b/src/test/test-string-util.c index 47581087063..12889ce8730 100644 --- a/src/test/test-string-util.c +++ b/src/test/test-string-util.c @@ -82,8 +82,26 @@ static void test_ascii_strcasecmp_n(void) { assert_se(ascii_strcasecmp_n("xxxxYxxxx", "xxxxXxxxx", 9) > 0); } +static void test_ascii_strcasecmp_nn(void) { + assert_se(ascii_strcasecmp_nn("", 0, "", 0) == 0); + assert_se(ascii_strcasecmp_nn("", 0, "", 1) < 0); + assert_se(ascii_strcasecmp_nn("", 1, "", 0) > 0); + assert_se(ascii_strcasecmp_nn("", 1, "", 1) == 0); + + assert_se(ascii_strcasecmp_nn("aaaa", 4, "aaAa", 4) == 0); + assert_se(ascii_strcasecmp_nn("aaa", 3, "aaAa", 4) < 0); + assert_se(ascii_strcasecmp_nn("aaa", 4, "aaAa", 4) < 0); + assert_se(ascii_strcasecmp_nn("aaaa", 4, "aaA", 3) > 0); + assert_se(ascii_strcasecmp_nn("aaaa", 4, "AAA", 4) > 0); + + assert_se(ascii_strcasecmp_nn("aaaa", 4, "bbbb", 4) < 0); + assert_se(ascii_strcasecmp_nn("aaAA", 4, "BBbb", 4) < 0); + assert_se(ascii_strcasecmp_nn("BBbb", 4, "aaaa", 4) > 0); +} + int main(int argc, char *argv[]) { test_string_erase(); test_ascii_strcasecmp_n(); + test_ascii_strcasecmp_nn(); return 0; } From f6fbd9c21ff4906c0bbd9ab95281658a9d1307b3 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 13 Jan 2016 19:51:09 +0100 Subject: [PATCH 13/16] shared: simplify dns_name_is_single_label() by using dns_name_parent() to skip first label --- src/shared/dns-domain.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/shared/dns-domain.c b/src/shared/dns-domain.c index a127e594ac8..ef81341ecac 100644 --- a/src/shared/dns-domain.c +++ b/src/shared/dns-domain.c @@ -912,12 +912,11 @@ bool dns_name_is_root(const char *name) { } bool dns_name_is_single_label(const char *name) { - char label[DNS_LABEL_MAX+1]; int r; assert(name); - r = dns_label_unescape(&name, label, sizeof(label)); + r = dns_name_parent(&name); if (r <= 0) return false; From 34361485a8ff397be06a3875df0c5331ad45c845 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 13 Jan 2016 20:20:52 +0100 Subject: [PATCH 14/16] shared: port dns_name_compare_func() to make use of ascii_strcasecmp_nn() This way we become compatible with DNS names with embedded NUL bytes. --- src/shared/dns-domain.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/shared/dns-domain.c b/src/shared/dns-domain.c index ef81341ecac..d36eb5056f0 100644 --- a/src/shared/dns-domain.c +++ b/src/shared/dns-domain.c @@ -521,7 +521,7 @@ int dns_name_compare_func(const void *a, const void *b) { y = (const char *) b + strlen(b); for (;;) { - char la[DNS_LABEL_MAX+1], lb[DNS_LABEL_MAX+1]; + char la[DNS_LABEL_MAX], lb[DNS_LABEL_MAX]; if (x == NULL && y == NULL) return 0; @@ -531,8 +531,15 @@ int dns_name_compare_func(const void *a, const void *b) { if (r < 0 || q < 0) return r - q; - k = dns_label_undo_idna(la, r, la, sizeof(la)); - w = dns_label_undo_idna(lb, q, lb, sizeof(lb)); + if (r > 0) + k = dns_label_undo_idna(la, r, la, sizeof(la)); + else + k = 0; + if (q > 0) + w = dns_label_undo_idna(lb, q, lb, sizeof(lb)); + else + w = 0; + if (k < 0 || w < 0) return k - w; if (k > 0) @@ -540,8 +547,7 @@ int dns_name_compare_func(const void *a, const void *b) { if (w > 0) q = w; - la[r] = lb[q] = 0; - r = strcasecmp(la, lb); + r = ascii_strcasecmp_nn(la, r, lb, q); if (r != 0) return r; } From 45c4210ed606b1e3318bba7edb5c1b962764a1c2 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 13 Jan 2016 20:45:20 +0100 Subject: [PATCH 15/16] shared: simplify string concatenation with strjoin() --- src/shared/dns-domain.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/shared/dns-domain.c b/src/shared/dns-domain.c index d36eb5056f0..1235ff109bd 100644 --- a/src/shared/dns-domain.c +++ b/src/shared/dns-domain.c @@ -1143,17 +1143,15 @@ int dns_service_split(const char *joined, char **_name, char **_type, char **_do if (x >= 3 && srv_type_label_is_valid(c, cn)) { if (dns_service_name_label_is_valid(a, an)) { - /* OK, got . . . */ name = strndup(a, an); if (!name) return -ENOMEM; - type = new(char, bn+1+cn+1); + type = strjoin(b, ".", c, NULL); if (!type) return -ENOMEM; - strcpy(stpcpy(stpcpy(type, b), "."), c); d = p; goto finish; @@ -1165,10 +1163,9 @@ int dns_service_split(const char *joined, char **_name, char **_type, char **_do name = NULL; - type = new(char, an+1+bn+1); + type = strjoin(a, ".", b, NULL); if (!type) return -ENOMEM; - strcpy(stpcpy(stpcpy(type, a), "."), b); d = q; goto finish; From 81ec9e0887ef12bebd36d48f10372cf003df5d38 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 13 Jan 2016 20:45:56 +0100 Subject: [PATCH 16/16] shared: reuse dns_label_unescape_undo_idna() in more places We frequently unescape DNS label follwed by IDNA undoing. We now have a function that does that in one step, hence use it everywhere. --- src/shared/dns-domain.c | 180 +++++++++++++++------------------------- 1 file changed, 69 insertions(+), 111 deletions(-) diff --git a/src/shared/dns-domain.c b/src/shared/dns-domain.c index 1235ff109bd..d1fb97fe924 100644 --- a/src/shared/dns-domain.c +++ b/src/shared/dns-domain.c @@ -558,100 +558,6 @@ const struct hash_ops dns_name_hash_ops = { .compare = dns_name_compare_func }; -int dns_name_equal(const char *x, const char *y) { - int r, q, k, w; - - assert(x); - assert(y); - - for (;;) { - char la[DNS_LABEL_MAX], lb[DNS_LABEL_MAX]; - - r = dns_label_unescape(&x, la, sizeof(la)); - if (r < 0) - return r; - if (r > 0) { - k = dns_label_undo_idna(la, r, la, sizeof(la)); - if (k < 0) - return k; - if (k > 0) - r = k; - } - - q = dns_label_unescape(&y, lb, sizeof(lb)); - if (q < 0) - return q; - if (q > 0) { - w = dns_label_undo_idna(lb, q, lb, sizeof(lb)); - if (w < 0) - return w; - if (w > 0) - q = w; - } - - if (r != q) - return false; - if (r == 0) - return true; - - if (ascii_strcasecmp_n(la, lb, r) != 0) - return false; - } -} - -int dns_name_endswith(const char *name, const char *suffix) { - const char *n, *s, *saved_n = NULL; - int r, q, k, w; - - assert(name); - assert(suffix); - - n = name; - s = suffix; - - for (;;) { - char ln[DNS_LABEL_MAX], ls[DNS_LABEL_MAX]; - - r = dns_label_unescape(&n, ln, sizeof(ln)); - if (r < 0) - return r; - if (r > 0) { - k = dns_label_undo_idna(ln, r, ln, sizeof(ln)); - if (k < 0) - return k; - if (k > 0) - r = k; - } - - if (!saved_n) - saved_n = n; - - q = dns_label_unescape(&s, ls, sizeof(ls)); - if (q < 0) - return q; - if (q > 0) { - w = dns_label_undo_idna(ls, q, ls, sizeof(ls)); - if (w < 0) - return w; - if (w > 0) - q = w; - } - - if (r == 0 && q == 0) - return true; - if (r == 0 && saved_n == n) - return false; - - if (r != q || ascii_strcasecmp_n(ln, ls, r) != 0) { - - /* Not the same, let's jump back, and try with the next label again */ - s = suffix; - n = saved_n; - saved_n = NULL; - } - } -} - static int dns_label_unescape_undo_idna(const char **name, char *dest, size_t sz) { int r, k; @@ -670,6 +576,72 @@ static int dns_label_unescape_undo_idna(const char **name, char *dest, size_t sz return k; } +int dns_name_equal(const char *x, const char *y) { + int r, q; + + assert(x); + assert(y); + + for (;;) { + char la[DNS_LABEL_MAX], lb[DNS_LABEL_MAX]; + + r = dns_label_unescape_undo_idna(&x, la, sizeof(la)); + if (r < 0) + return r; + + q = dns_label_unescape_undo_idna(&y, lb, sizeof(lb)); + if (q < 0) + return q; + + if (r != q) + return false; + if (r == 0) + return true; + + if (ascii_strcasecmp_n(la, lb, r) != 0) + return false; + } +} + +int dns_name_endswith(const char *name, const char *suffix) { + const char *n, *s, *saved_n = NULL; + int r, q; + + assert(name); + assert(suffix); + + n = name; + s = suffix; + + for (;;) { + char ln[DNS_LABEL_MAX], ls[DNS_LABEL_MAX]; + + r = dns_label_unescape_undo_idna(&n, ln, sizeof(ln)); + if (r < 0) + return r; + + if (!saved_n) + saved_n = n; + + q = dns_label_unescape_undo_idna(&s, ls, sizeof(ls)); + if (q < 0) + return q; + + if (r == 0 && q == 0) + return true; + if (r == 0 && saved_n == n) + return false; + + if (r != q || ascii_strcasecmp_n(ln, ls, r) != 0) { + + /* Not the same, let's jump back, and try with the next label again */ + s = suffix; + n = saved_n; + saved_n = NULL; + } + } +} + int dns_name_startswith(const char *name, const char *prefix) { const char *n, *p; int r, q; @@ -702,7 +674,7 @@ int dns_name_startswith(const char *name, const char *prefix) { int dns_name_change_suffix(const char *name, const char *old_suffix, const char *new_suffix, char **ret) { const char *n, *s, *saved_before = NULL, *saved_after = NULL, *prefix; - int r, q, k, w; + int r, q; assert(name); assert(old_suffix); @@ -718,30 +690,16 @@ int dns_name_change_suffix(const char *name, const char *old_suffix, const char if (!saved_before) saved_before = n; - r = dns_label_unescape(&n, ln, sizeof(ln)); + r = dns_label_unescape_undo_idna(&n, ln, sizeof(ln)); if (r < 0) return r; - if (r > 0) { - k = dns_label_undo_idna(ln, r, ln, sizeof(ln)); - if (k < 0) - return k; - if (k > 0) - r = k; - } if (!saved_after) saved_after = n; - q = dns_label_unescape(&s, ls, sizeof(ls)); + q = dns_label_unescape_undo_idna(&s, ls, sizeof(ls)); if (q < 0) return q; - if (q > 0) { - w = dns_label_undo_idna(ls, q, ls, sizeof(ls)); - if (w < 0) - return w; - if (w > 0) - q = w; - } if (r == 0 && q == 0) break;