Merge pull request #23875 from yuwata/resolve-mdns-fix-use-after-free

resolve: mdns: fix use-after-free
This commit is contained in:
Luca Boccassi 2022-07-27 22:57:31 +01:00 committed by GitHub
commit bffb318491
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 221 additions and 165 deletions

View file

@ -1242,16 +1242,14 @@ int dns_cache_check_conflicts(DnsCache *cache, DnsResourceRecord *rr, int owner_
return 1;
}
int dns_cache_export_shared_to_packet(DnsCache *cache, DnsPacket *p) {
int dns_cache_export_shared_to_packet(DnsCache *cache, DnsPacket *p, usec_t ts, unsigned max_rr) {
unsigned ancount = 0;
DnsCacheItem *i;
usec_t t;
int r;
assert(cache);
assert(p);
t = now(CLOCK_BOOTTIME);
assert(p->protocol == DNS_PROTOCOL_MDNS);
HASHMAP_FOREACH(i, cache->by_key)
LIST_FOREACH(by_key, j, i) {
@ -1263,14 +1261,17 @@ int dns_cache_export_shared_to_packet(DnsCache *cache, DnsPacket *p) {
/* RFC6762 7.1: Don't append records with less than half the TTL remaining
* as known answers. */
if (usec_sub_unsigned(j->until, t) < j->rr->ttl * USEC_PER_SEC / 2)
if (usec_sub_unsigned(j->until, ts) < j->rr->ttl * USEC_PER_SEC / 2)
continue;
r = dns_packet_append_rr(p, j->rr, 0, NULL, NULL);
if (r == -EMSGSIZE && p->protocol == DNS_PROTOCOL_MDNS) {
/* For mDNS, if we're unable to stuff all known answers into the given packet,
* allocate a new one, push the RR into that one and link it to the current one.
*/
if (r == -EMSGSIZE) {
if (max_rr == 0)
/* If max_rr == 0, do not allocate more packets. */
goto finalize;
/* If we're unable to stuff all known answers into the given packet, allocate
* a new one, push the RR into that one and link it to the current one. */
DNS_PACKET_HEADER(p)->ancount = htobe16(ancount);
ancount = 0;
@ -1288,8 +1289,21 @@ int dns_cache_export_shared_to_packet(DnsCache *cache, DnsPacket *p) {
return r;
ancount++;
if (max_rr > 0 && ancount >= max_rr) {
DNS_PACKET_HEADER(p)->ancount = htobe16(ancount);
ancount = 0;
r = dns_packet_new_query(&p->more, p->protocol, 0, true);
if (r < 0)
return r;
p = p->more;
max_rr = UINT_MAX;
}
}
finalize:
DNS_PACKET_HEADER(p)->ancount = htobe16(ancount);
return 0;

View file

@ -53,4 +53,4 @@ bool dns_cache_is_empty(DnsCache *cache);
unsigned dns_cache_size(DnsCache *cache);
int dns_cache_export_shared_to_packet(DnsCache *cache, DnsPacket *p);
int dns_cache_export_shared_to_packet(DnsCache *cache, DnsPacket *p, usec_t ts, unsigned max_rr);

View file

@ -1011,7 +1011,9 @@ void dns_scope_process_query(DnsScope *s, DnsStream *stream, DnsPacket *p) {
return;
}
assert(dns_question_size(p->question) == 1);
if (dns_question_size(p->question) != 1)
return (void) log_debug("Received LLMNR query without question or multiple questions, ignoring.");
key = dns_question_first_key(p->question);
r = dns_zone_lookup(&s->zone, key, 0, &answer, &soa, &tentative);
@ -1204,7 +1206,6 @@ static int on_conflict_dispatch(sd_event_source *es, usec_t usec, void *userdata
}
int dns_scope_notify_conflict(DnsScope *scope, DnsResourceRecord *rr) {
usec_t jitter;
int r;
assert(scope);
@ -1233,15 +1234,12 @@ int dns_scope_notify_conflict(DnsScope *scope, DnsResourceRecord *rr) {
if (scope->conflict_event_source)
return 0;
random_bytes(&jitter, sizeof(jitter));
jitter %= LLMNR_JITTER_INTERVAL_USEC;
r = sd_event_add_time_relative(
scope->manager->event,
&scope->conflict_event_source,
CLOCK_BOOTTIME,
jitter,
LLMNR_JITTER_INTERVAL_USEC,
random_u64_range(LLMNR_JITTER_INTERVAL_USEC),
0,
on_conflict_dispatch, scope);
if (r < 0)
return log_debug_errno(r, "Failed to add conflict dispatch event: %m");
@ -1511,7 +1509,7 @@ int dns_scope_announce(DnsScope *scope, bool goodbye) {
&scope->announce_event_source,
CLOCK_BOOTTIME,
MDNS_ANNOUNCE_DELAY,
MDNS_JITTER_RANGE_USEC,
0,
on_announcement_timeout, scope);
if (r < 0)
return log_debug_errno(r, "Failed to schedule second announcement: %m");

View file

@ -1551,6 +1551,33 @@ static int on_transaction_timeout(sd_event_source *s, usec_t usec, void *userdat
return 0;
}
static int dns_transaction_setup_timeout(
DnsTransaction *t,
usec_t timeout_usec /* relative */,
usec_t next_usec /* CLOCK_BOOTTIME */) {
int r;
assert(t);
dns_transaction_stop_timeout(t);
r = sd_event_add_time_relative(
t->scope->manager->event,
&t->timeout_event_source,
CLOCK_BOOTTIME,
timeout_usec, 0,
on_transaction_timeout, t);
if (r < 0)
return r;
(void) sd_event_source_set_description(t->timeout_event_source, "dns-transaction-timeout");
t->next_attempt_after = next_usec;
t->state = DNS_TRANSACTION_PENDING;
return 0;
}
static usec_t transaction_get_resend_timeout(DnsTransaction *t) {
assert(t);
assert(t->scope);
@ -1568,11 +1595,12 @@ static usec_t transaction_get_resend_timeout(DnsTransaction *t) {
return DNS_TIMEOUT_USEC;
case DNS_PROTOCOL_MDNS:
assert(t->n_attempts > 0);
if (t->probing)
return MDNS_PROBING_INTERVAL_USEC;
else
return (1 << (t->n_attempts - 1)) * USEC_PER_SEC;
/* See RFC 6762 Section 5.1 suggests that timeout should be a few seconds. */
assert(t->n_attempts > 0);
return (1 << (t->n_attempts - 1)) * USEC_PER_SEC;
case DNS_PROTOCOL_LLMNR:
return t->scope->resend_timeout;
@ -1622,7 +1650,7 @@ static int dns_transaction_prepare(DnsTransaction *t, usec_t ts) {
return 0;
}
if (t->n_attempts >= dns_transaction_attempts_max(t->scope->protocol, t->probing)) {
if (t->n_attempts >= TRANSACTION_ATTEMPTS_MAX(t->scope->protocol)) {
DnsTransactionState result;
if (t->scope->protocol == DNS_PROTOCOL_LLMNR)
@ -1752,13 +1780,30 @@ static int dns_transaction_prepare(DnsTransaction *t, usec_t ts) {
return 1;
}
static int dns_packet_append_zone(DnsPacket *p, DnsTransaction *t, DnsResourceKey *k, unsigned *nscount) {
_cleanup_(dns_answer_unrefp) DnsAnswer *answer = NULL;
bool tentative;
int r;
assert(p);
assert(t);
assert(k);
if (k->type != DNS_TYPE_ANY)
return 0;
r = dns_zone_lookup(&t->scope->zone, k, t->scope->link->ifindex, &answer, NULL, &tentative);
if (r < 0)
return r;
return dns_packet_append_answer(p, answer, nscount);
}
static int dns_transaction_make_packet_mdns(DnsTransaction *t) {
_cleanup_(dns_packet_unrefp) DnsPacket *p = NULL;
bool add_known_answers = false;
DnsResourceKey *tkey;
_cleanup_set_free_ Set *keys = NULL;
unsigned qdcount;
unsigned nscount = 0;
unsigned qdcount, ancount = 0 /* avoid false maybe-uninitialized warning */, nscount;
bool add_known_answers = false;
usec_t ts;
int r;
@ -1768,6 +1813,7 @@ static int dns_transaction_make_packet_mdns(DnsTransaction *t) {
/* Discard any previously prepared packet, so we can start over and coalesce again */
t->sent = dns_packet_unref(t->sent);
/* First, create a dummy packet to calculate packet size. */
r = dns_packet_new_query(&p, t->scope->protocol, 0, false);
if (r < 0)
return r;
@ -1781,11 +1827,14 @@ static int dns_transaction_make_packet_mdns(DnsTransaction *t) {
if (dns_key_is_shared(dns_transaction_key(t)))
add_known_answers = true;
if (dns_transaction_key(t)->type == DNS_TYPE_ANY) {
r = set_ensure_put(&keys, &dns_resource_key_hash_ops, dns_transaction_key(t));
if (r < 0)
return r;
}
r = dns_packet_append_zone(p, t, dns_transaction_key(t), NULL);
if (r < 0)
return r;
/* Save appended keys */
r = set_ensure_put(&keys, &dns_resource_key_hash_ops, dns_transaction_key(t));
if (r < 0)
return r;
/*
* For mDNS, we want to coalesce as many open queries in pending transactions into one single
@ -1795,90 +1844,114 @@ static int dns_transaction_make_packet_mdns(DnsTransaction *t) {
assert_se(sd_event_now(t->scope->manager->event, CLOCK_BOOTTIME, &ts) >= 0);
LIST_FOREACH(transactions_by_scope, other, t->scope->transactions) {
for (bool restart = true; restart;) {
restart = false;
LIST_FOREACH(transactions_by_scope, other, t->scope->transactions) {
size_t saved_packet_size;
bool append = false;
/* Skip ourselves */
if (other == t)
continue;
/* Skip ourselves */
if (other == t)
continue;
if (other->state != DNS_TRANSACTION_PENDING)
continue;
if (other->state != DNS_TRANSACTION_PENDING)
continue;
if (other->next_attempt_after > ts)
continue;
if (other->next_attempt_after > ts)
continue;
if (qdcount >= UINT16_MAX)
break;
if (!set_contains(keys, dns_transaction_key(other))) {
r = dns_packet_append_key(p, dns_transaction_key(other), 0, &saved_packet_size);
/* If we can't stuff more questions into the packet, just give up.
* One of the 'other' transactions will fire later and take care of the rest. */
if (r == -EMSGSIZE)
break;
if (r < 0)
return r;
r = dns_packet_append_key(p, dns_transaction_key(other), 0, NULL);
r = dns_packet_append_zone(p, t, dns_transaction_key(other), NULL);
if (r == -EMSGSIZE)
break;
if (r < 0)
return r;
/*
* If we can't stuff more questions into the packet, just give up.
* One of the 'other' transactions will fire later and take care of the rest.
*/
if (r == -EMSGSIZE)
break;
append = true;
}
if (r < 0)
return r;
r = dns_transaction_prepare(other, ts);
if (r <= 0)
continue;
ts += transaction_get_resend_timeout(other);
r = sd_event_add_time(
other->scope->manager->event,
&other->timeout_event_source,
CLOCK_BOOTTIME,
ts, 0,
on_transaction_timeout, other);
if (r < 0)
return r;
(void) sd_event_source_set_description(other->timeout_event_source, "dns-transaction-timeout");
other->state = DNS_TRANSACTION_PENDING;
other->next_attempt_after = ts;
qdcount++;
if (dns_key_is_shared(dns_transaction_key(other)))
add_known_answers = true;
if (dns_transaction_key(other)->type == DNS_TYPE_ANY) {
r = set_ensure_put(&keys, &dns_resource_key_hash_ops, dns_transaction_key(other));
r = dns_transaction_prepare(other, ts);
if (r < 0)
return r;
if (r == 0) {
if (append)
dns_packet_truncate(p, saved_packet_size);
/* In this case, not only this transaction, but multiple transactions may be
* freed. Hence, we need to restart the loop. */
restart = true;
break;
}
usec_t timeout = transaction_get_resend_timeout(other);
r = dns_transaction_setup_timeout(other, timeout, usec_add(ts, timeout));
if (r < 0)
return r;
if (dns_key_is_shared(dns_transaction_key(other)))
add_known_answers = true;
if (append) {
r = set_ensure_put(&keys, &dns_resource_key_hash_ops, dns_transaction_key(other));
if (r < 0)
return r;
}
qdcount++;
if (qdcount >= UINT16_MAX)
break;
}
}
DNS_PACKET_HEADER(p)->qdcount = htobe16(qdcount);
/* Append known answer section if we're asking for any shared record */
if (add_known_answers) {
r = dns_cache_export_shared_to_packet(&t->scope->cache, p);
r = dns_cache_export_shared_to_packet(&t->scope->cache, p, ts, 0);
if (r < 0)
return r;
ancount = be16toh(DNS_PACKET_HEADER(p)->ancount);
}
/* Then, create acctual packet. */
p = dns_packet_unref(p);
r = dns_packet_new_query(&p, t->scope->protocol, 0, false);
if (r < 0)
return r;
/* Questions */
DnsResourceKey *k;
SET_FOREACH(k, keys) {
r = dns_packet_append_key(p, k, 0, NULL);
if (r < 0)
return r;
}
DNS_PACKET_HEADER(p)->qdcount = htobe16(qdcount);
/* Known answers */
if (add_known_answers) {
r = dns_cache_export_shared_to_packet(&t->scope->cache, p, ts, ancount);
if (r < 0)
return r;
}
SET_FOREACH(tkey, keys) {
_cleanup_(dns_answer_unrefp) DnsAnswer *answer = NULL;
bool tentative;
r = dns_zone_lookup(&t->scope->zone, tkey, t->scope->link->ifindex, &answer, NULL, &tentative);
if (r < 0)
return r;
r = dns_packet_append_answer(p, answer, &nscount);
/* Authorities */
nscount = 0;
SET_FOREACH(k, keys) {
r = dns_packet_append_zone(p, t, k, &nscount);
if (r < 0)
return r;
}
DNS_PACKET_HEADER(p)->nscount = htobe16(nscount);
t->sent = TAKE_PTR(p);
return 0;
}
@ -1950,45 +2023,36 @@ int dns_transaction_go(DnsTransaction *t) {
if (!t->initial_jitter_scheduled &&
IN_SET(t->scope->protocol, DNS_PROTOCOL_LLMNR, DNS_PROTOCOL_MDNS)) {
usec_t jitter, accuracy;
usec_t jitter;
/* RFC 4795 Section 2.7 suggests all queries should be delayed by a random time from 0 to
* JITTER_INTERVAL. */
/* RFC 4795 Section 2.7 suggests all LLMNR queries should be delayed by a random time from 0 to
* JITTER_INTERVAL.
* RFC 6762 Section 8.1 suggests initial probe queries should be delayed by a random time from
* 0 to 250ms. */
t->initial_jitter_scheduled = true;
t->n_attempts = 0;
switch (t->scope->protocol) {
case DNS_PROTOCOL_LLMNR:
jitter = random_u64_range(LLMNR_JITTER_INTERVAL_USEC);
accuracy = LLMNR_JITTER_INTERVAL_USEC;
break;
case DNS_PROTOCOL_MDNS:
jitter = usec_add(random_u64_range(MDNS_JITTER_RANGE_USEC), MDNS_JITTER_MIN_USEC);
accuracy = MDNS_JITTER_RANGE_USEC;
if (t->probing)
jitter = random_u64_range(MDNS_PROBING_INTERVAL_USEC);
else
jitter = 0;
break;
default:
assert_not_reached();
}
assert(!t->timeout_event_source);
r = sd_event_add_time_relative(
t->scope->manager->event,
&t->timeout_event_source,
CLOCK_BOOTTIME,
jitter, accuracy,
on_transaction_timeout, t);
r = dns_transaction_setup_timeout(t, jitter, ts);
if (r < 0)
return r;
(void) sd_event_source_set_description(t->timeout_event_source, "dns-transaction-timeout");
t->n_attempts = 0;
t->next_attempt_after = ts;
t->state = DNS_TRANSACTION_PENDING;
log_debug("Delaying %s transaction %" PRIu16 " for " USEC_FMT "us.",
dns_protocol_to_string(t->scope->protocol),
t->id,
@ -2062,22 +2126,11 @@ int dns_transaction_go(DnsTransaction *t) {
return dns_transaction_go(t);
}
ts += transaction_get_resend_timeout(t);
r = sd_event_add_time(
t->scope->manager->event,
&t->timeout_event_source,
CLOCK_BOOTTIME,
ts, 0,
on_transaction_timeout, t);
usec_t timeout = transaction_get_resend_timeout(t);
r = dns_transaction_setup_timeout(t, timeout, usec_add(ts, timeout));
if (r < 0)
return r;
(void) sd_event_source_set_description(t->timeout_event_source, "dns-transaction-timeout");
t->state = DNS_TRANSACTION_PENDING;
t->next_attempt_after = ts;
return 1;
}

View file

@ -201,10 +201,6 @@ DnsTransactionSource dns_transaction_source_from_string(const char *s) _pure_;
/* LLMNR Jitter interval, see RFC 4795 Section 7 */
#define LLMNR_JITTER_INTERVAL_USEC (100 * USEC_PER_MSEC)
/* mDNS Jitter interval, see RFC 6762 Section 5.2 */
#define MDNS_JITTER_MIN_USEC (20 * USEC_PER_MSEC)
#define MDNS_JITTER_RANGE_USEC (100 * USEC_PER_MSEC)
/* mDNS probing interval, see RFC 6762 Section 8.1 */
#define MDNS_PROBING_INTERVAL_USEC (250 * USEC_PER_MSEC)
@ -214,31 +210,11 @@ DnsTransactionSource dns_transaction_source_from_string(const char *s) _pure_;
/* Maximum attempts to send LLMNR requests, see RFC 4795 Section 2.7 */
#define LLMNR_TRANSACTION_ATTEMPTS_MAX 3
/* Maximum attempts to send MDNS requests is one except for probe requests, see RFC 6762 Section 8.1
* RFC 6762 differentiates between normal (single-shot/continuous) and probe requests.
* It therefore makes sense to attempt each normal query only once with no retries.
* Otherwise we'd be sending out three attempts for even a normal query. */
#define MDNS_TRANSACTION_ATTEMPTS_MAX 1
/* Maximum attempts to send MDNS requests, see RFC 6762 Section 8.1 */
#define MDNS_TRANSACTION_ATTEMPTS_MAX 3
#define MDNS_PROBE_TRANSACTION_ATTEMPTS_MAX 3
static inline unsigned dns_transaction_attempts_max(DnsProtocol p, bool probing) {
switch (p) {
case DNS_PROTOCOL_LLMNR:
return LLMNR_TRANSACTION_ATTEMPTS_MAX;
case DNS_PROTOCOL_MDNS:
if (probing)
return MDNS_PROBE_TRANSACTION_ATTEMPTS_MAX;
else
return MDNS_TRANSACTION_ATTEMPTS_MAX;
case DNS_PROTOCOL_DNS:
return DNS_TRANSACTION_ATTEMPTS_MAX;
default:
return 0;
}
}
#define TRANSACTION_ATTEMPTS_MAX(p) (((p) == DNS_PROTOCOL_LLMNR) ? \
LLMNR_TRANSACTION_ATTEMPTS_MAX : \
(((p) == DNS_PROTOCOL_MDNS) ? \
MDNS_TRANSACTION_ATTEMPTS_MAX : \
DNS_TRANSACTION_ATTEMPTS_MAX))

View file

@ -177,8 +177,6 @@ static int mdns_do_tiebreak(DnsResourceKey *key, DnsAnswer *answer, DnsPacket *p
if (r < 0)
return r;
assert(r > 0);
if (proposed_rrs_cmp(remote, r, our, size) > 0)
return 1;
@ -207,10 +205,10 @@ static bool mdns_should_reply_using_unicast(DnsPacket *p) {
}
/* All the questions in the query had a QU bit set, RFC 6762, section 5.4 */
DNS_QUESTION_FOREACH_ITEM(item, p->question) {
DNS_QUESTION_FOREACH_ITEM(item, p->question)
if (!FLAGS_SET(item->flags, DNS_QUESTION_WANTS_UNICAST_REPLY))
return false;
}
return true;
}
@ -256,7 +254,8 @@ static int mdns_scope_process_query(DnsScope *s, DnsPacket *p) {
if (r < 0)
return log_debug_errno(r, "Failed to extract resource records from incoming packet: %m");
assert_return((dns_question_size(p->question) > 0), -EINVAL);
if (dns_question_size(p->question) <= 0)
return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "Received mDNS query without question, ignoring.");
unicast_reply = mdns_should_reply_using_unicast(p);
if (unicast_reply && !sender_on_local_subnet(s, p)) {
@ -410,12 +409,28 @@ static int on_mdns_packet(sd_event_source *s, int fd, uint32_t revents, void *us
}
}
LIST_FOREACH(transactions_by_scope, t, scope->transactions) {
r = dns_answer_match_key(p->answer, t->key, NULL);
if (r < 0)
log_debug_errno(r, "Failed to match resource key, ignoring: %m");
else if (r > 0) /* This packet matches the transaction, let's pass it on as reply */
for (bool match = true; match;) {
match = false;
LIST_FOREACH(transactions_by_scope, t, scope->transactions) {
if (t->state != DNS_TRANSACTION_PENDING)
continue;
r = dns_answer_match_key(p->answer, dns_transaction_key(t), NULL);
if (r <= 0) {
if (r < 0)
log_debug_errno(r, "Failed to match resource key, ignoring: %m");
continue;
}
/* This packet matches the transaction, let's pass it on as reply */
dns_transaction_process_reply(t, p, false);
/* The dns_transaction_process_reply() -> dns_transaction_complete() ->
* dns_query_candidate_stop() may free multiple transactions. Hence, restart
* the loop. */
match = true;
break;
}
}
dns_cache_put(&scope->cache, scope->manager->enable_cache, NULL, DNS_PACKET_RCODE(p), p->answer, NULL, false, _DNSSEC_RESULT_INVALID, UINT32_MAX, p->family, &p->sender);