summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLennart Poettering <lennart@poettering.net>2016-01-04 22:25:38 +0100
committerLennart Poettering <lennart@poettering.net>2016-01-04 22:42:10 +0100
commitf2992dc184398c6361273a39d3fde5c605e045e0 (patch)
treea3cfc3913fcdfc944e04c16ca7353a6520d80535
parent51e399bcebefb27d6b147d90de84d07f010fa170 (diff)
resolved: explicitly avoid cyclic transaction dependencies
We already try hard not to create cyclic transaction dependencies, where a transaction requires another one for DNSSEC validation purposes, which in turn (possibly indirectly) pulls in the original transaction again, thus resulting in a cyclic dependency and ultimately a deadlock since each transaction waits for another one forever. So far we wanted to avoid such cyclic dependencies by only going "up the tree" when requesting auxiliary RRs and only going from one RR type to another, but never back. However this turned out to be insufficient. Consider a domain that publishes one or more DNSKEY but which has no DS for it. A request for the domain's DNSKEY triggers a request for the domain's DS, which will then fail, but return an NSEC, signed by the DNSKEY. To validate that we'd request the DNSKEY again. Thus a DNSKEY request results in a DS request which results in the original DNSKEY request again. If the original lookup had been a DS lookup we'd end up in the same cyclic dependency, hence we cannot statically break one of them, since both requests are of course fully valid. Hence, do full cyclic dependency checking: each time we are about to add a dependency to a transaction, check if the transaction is already a dependency of the dependency (recursively down the tree).
-rw-r--r--src/resolve/resolved-dns-transaction.c44
1 files changed, 37 insertions, 7 deletions
diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c
index c0626626f2..ce320e9a97 100644
--- a/src/resolve/resolved-dns-transaction.c
+++ b/src/resolve/resolved-dns-transaction.c
@@ -1233,6 +1233,28 @@ int dns_transaction_go(DnsTransaction *t) {
return 1;
}
+static int dns_transaction_find_cyclic(DnsTransaction *t, DnsTransaction *aux) {
+ DnsTransaction *n;
+ Iterator i;
+ int r;
+
+ assert(t);
+ assert(aux);
+
+ /* Try to find cyclic dependencies between transaction objects */
+
+ if (t == aux)
+ return 1;
+
+ SET_FOREACH(n, aux->notify_transactions, i) {
+ r = dns_transaction_find_cyclic(t, n);
+ if (r != 0)
+ return r;
+ }
+
+ return r;
+}
+
static int dns_transaction_add_dnssec_transaction(DnsTransaction *t, DnsResourceKey *key, DnsTransaction **ret) {
DnsTransaction *aux;
int r;
@@ -1251,6 +1273,18 @@ static int dns_transaction_add_dnssec_transaction(DnsTransaction *t, DnsResource
*ret = aux;
return 0;
}
+
+ r = dns_transaction_find_cyclic(t, aux);
+ if (r < 0)
+ return r;
+ if (r > 0) {
+ log_debug("Detected potential cyclic dependency, refusing to add transaction %" PRIu16 " (%s) as dependency for %" PRIu16 " (%s).",
+ aux->id,
+ strna(dns_transaction_key_string(aux)),
+ t->id,
+ strna(dns_transaction_key_string(t)));
+ return -ELOOP;
+ }
}
r = set_ensure_allocated(&t->dnssec_transactions, NULL);
@@ -1287,12 +1321,6 @@ static int dns_transaction_request_dnssec_rr(DnsTransaction *t, DnsResourceKey *
assert(t);
assert(key);
- r = dns_resource_key_equal(t->key, key);
- if (r < 0)
- return r;
- if (r > 0) /* Don't go in circles */
- return 0;
-
/* Try to get the data from the trust anchor */
r = dns_trust_anchor_lookup_positive(&t->scope->manager->trust_anchor, key, &a);
if (r < 0)
@@ -1307,6 +1335,8 @@ static int dns_transaction_request_dnssec_rr(DnsTransaction *t, DnsResourceKey *
/* This didn't work, ask for it via the network/cache then. */
r = dns_transaction_add_dnssec_transaction(t, key, &aux);
+ if (r == -ELOOP) /* This would result in a cyclic dependency */
+ return 0;
if (r < 0)
return r;
@@ -1316,7 +1346,7 @@ static int dns_transaction_request_dnssec_rr(DnsTransaction *t, DnsResourceKey *
return r;
}
- return 0;
+ return 1;
}
static int dns_transaction_has_positive_answer(DnsTransaction *t, DnsAnswerFlags *flags) {