Skip to content

Commit

Permalink
resolved: don't store udp/tcp fd in DnsPacket object
Browse files Browse the repository at this point in the history 
DnsPacket should better be a "dead" object, i.e. list facts, not track
resources. By including an fd in its fields it started tracking
resources however, without actually taking a ref to the fd (i.e. no
dup() or so was called on it).

Let's hence rework things so that we don#t have to keep track of the fd
a packet came in from. Instead, pass around the DnsStubListenerExtra
object wherever we need to.

This should be useful as soon as we start caching whole DnsPacket
objects to allow replying to DNSSEC/CO packets, i.e. where we have to
keep a copy of the original DnsPacket around for a long time in cache,
potentially much longer than the fds the packet was received on.
  • Loading branch information
@poettering
poettering committed Sep 8, 2020
1 parent ae8f0ec commit 0354029
Show file tree
Hide file tree
Showing 7 changed files with 83 additions and 50 deletions.
2 changes: 1 addition & 1 deletion src/resolve/resolved-conf.c
View file
Expand Up @@ -406,7 +406,7 @@ int config_parse_dns_stub_listener_extra(
return 0;
}

r = dns_stub_listener_extra_new(&stub);
r = dns_stub_listener_extra_new(m, &stub);
if (r < 0)
return log_oom();

Expand Down
1 change: 0 additions & 1 deletion src/resolve/resolved-dns-packet.h
View file
Expand Up @@ -66,7 +66,6 @@ struct DnsPacket {
DnsResourceRecord *opt;

/* Packet reception metadata */
int fd; /* Used by UDP extra DNS stub listners */
int ifindex;
int family, ipproto;
union in_addr_union sender, destination;
Expand Down
2 changes: 2 additions & 0 deletions src/resolve/resolved-dns-query.h
View file
Expand Up @@ -8,6 +8,7 @@

typedef struct DnsQueryCandidate DnsQueryCandidate;
typedef struct DnsQuery DnsQuery;
typedef struct DnsStubListenerExtra DnsStubListenerExtra;

#include "resolved-dns-answer.h"
#include "resolved-dns-question.h"
Expand Down Expand Up @@ -82,6 +83,7 @@ struct DnsQuery {
DnsPacket *request_dns_packet;
DnsStream *request_dns_stream;
DnsPacket *reply_dns_packet;
DnsStubListenerExtra *stub_listener_extra;

/* Completion callback */
void (*complete)(DnsQuery* q);
Expand Down
3 changes: 3 additions & 0 deletions src/resolve/resolved-dns-stream.h
View file
Expand Up @@ -10,6 +10,7 @@ typedef struct DnsServer DnsServer;
typedef struct DnsStream DnsStream;
typedef struct DnsTransaction DnsTransaction;
typedef struct Manager Manager;
typedef struct DnsStubListenerExtra DnsStubListenerExtra;

#include "resolved-dns-packet.h"
#include "resolved-dnstls.h"
Expand Down Expand Up @@ -75,6 +76,8 @@ struct DnsStream {
/* used when DNS-over-TLS is enabled */
bool encrypted:1;

DnsStubListenerExtra *stub_listener_extra;

LIST_FIELDS(DnsStream, streams);
};

Expand Down
120 changes: 74 additions & 46 deletions src/resolve/resolved-dns-stub.c
View file
Expand Up @@ -15,6 +15,8 @@
* IP and UDP header sizes */
#define ADVERTISE_DATAGRAM_SIZE_MAX (65536U-14U-20U-8U)

static int manager_dns_stub_udp_fd_extra(Manager *m, DnsStubListenerExtra *l);

static void dns_stub_listener_extra_hash_func(const DnsStubListenerExtra *a, struct siphash *state) {
assert(a);

Expand Down Expand Up @@ -52,16 +54,21 @@ DEFINE_HASH_OPS_WITH_KEY_DESTRUCTOR(
dns_stub_listener_extra_compare_func,
dns_stub_listener_extra_free);

int dns_stub_listener_extra_new(DnsStubListenerExtra **ret) {
int dns_stub_listener_extra_new(
Manager *m,
DnsStubListenerExtra **ret) {

DnsStubListenerExtra *l;

l = new0(DnsStubListenerExtra, 1);
l = new(DnsStubListenerExtra, 1);
if (!l)
return -ENOMEM;

*ret = TAKE_PTR(l);
*l = (DnsStubListenerExtra) {
.manager = m,
};

*ret = TAKE_PTR(l);
return 0;
}

Expand Down Expand Up @@ -190,7 +197,13 @@ static int dns_stub_finish_reply_packet(
return 0;
}

static int dns_stub_send(Manager *m, DnsStream *s, DnsPacket *p, DnsPacket *reply) {
static int dns_stub_send(
Manager *m,
DnsStubListenerExtra *l,
DnsStream *s,
DnsPacket *p,
DnsPacket *reply) {

int r;

assert(m);
Expand All @@ -199,20 +212,29 @@ static int dns_stub_send(Manager *m, DnsStream *s, DnsPacket *p, DnsPacket *repl

if (s)
r = dns_stream_write_packet(s, reply);
else {
else
/* Note that it is essential here that we explicitly choose the source IP address for this packet. This
* is because otherwise the kernel will choose it automatically based on the routing table and will
* thus pick 127.0.0.1 rather than 127.0.0.53. */

r = manager_send(m, p->fd, p->ifindex, p->family, &p->sender, p->sender_port, &p->destination, reply);
}
r = manager_send(m,
manager_dns_stub_udp_fd_extra(m, l),
l ? p->ifindex : LOOPBACK_IFINDEX, /* force loopback iface if this is the main listener stub */
p->family, &p->sender, p->sender_port, &p->destination,
reply);
if (r < 0)
return log_debug_errno(r, "Failed to send reply packet: %m");

return 0;
}

static int dns_stub_send_failure(Manager *m, DnsStream *s, DnsPacket *p, int rcode, bool authenticated) {
static int dns_stub_send_failure(
Manager *m,
DnsStubListenerExtra *l,
DnsStream *s,
DnsPacket *p,
int rcode,
bool authenticated) {

_cleanup_(dns_packet_unrefp) DnsPacket *reply = NULL;
int r;

Expand All @@ -227,7 +249,7 @@ static int dns_stub_send_failure(Manager *m, DnsStream *s, DnsPacket *p, int rco
if (r < 0)
return log_debug_errno(r, "Failed to build failure packet: %m");

return dns_stub_send(m, s, p, reply);
return dns_stub_send(m, l, s, p, reply);
}

static void dns_stub_query_complete(DnsQuery *q) {
Expand All @@ -250,7 +272,7 @@ static void dns_stub_query_complete(DnsQuery *q) {
if (!truncated) {
r = dns_query_process_cname(q);
if (r == -ELOOP) {
(void) dns_stub_send_failure(q->manager, q->request_dns_stream, q->request_dns_packet, DNS_RCODE_SERVFAIL, false);
(void) dns_stub_send_failure(q->manager, q->stub_listener_extra, q->request_dns_stream, q->request_dns_packet, DNS_RCODE_SERVFAIL, false);
break;
}
if (r < 0) {
Expand All @@ -274,16 +296,16 @@ static void dns_stub_query_complete(DnsQuery *q) {
break;
}

(void) dns_stub_send(q->manager, q->request_dns_stream, q->request_dns_packet, q->reply_dns_packet);
(void) dns_stub_send(q->manager, q->stub_listener_extra, q->request_dns_stream, q->request_dns_packet, q->reply_dns_packet);
break;
}

case DNS_TRANSACTION_RCODE_FAILURE:
(void) dns_stub_send_failure(q->manager, q->request_dns_stream, q->request_dns_packet, q->answer_rcode, dns_query_fully_authenticated(q));
(void) dns_stub_send_failure(q->manager, q->stub_listener_extra, q->request_dns_stream, q->request_dns_packet, q->answer_rcode, dns_query_fully_authenticated(q));
break;

case DNS_TRANSACTION_NOT_FOUND:
(void) dns_stub_send_failure(q->manager, q->request_dns_stream, q->request_dns_packet, DNS_RCODE_NXDOMAIN, dns_query_fully_authenticated(q));
(void) dns_stub_send_failure(q->manager, q->stub_listener_extra, q->request_dns_stream, q->request_dns_packet, DNS_RCODE_NXDOMAIN, dns_query_fully_authenticated(q));
break;

case DNS_TRANSACTION_TIMEOUT:
Expand All @@ -299,7 +321,7 @@ static void dns_stub_query_complete(DnsQuery *q) {
case DNS_TRANSACTION_NO_TRUST_ANCHOR:
case DNS_TRANSACTION_RR_TYPE_UNSUPPORTED:
case DNS_TRANSACTION_NETWORK_DOWN:
(void) dns_stub_send_failure(q->manager, q->request_dns_stream, q->request_dns_packet, DNS_RCODE_SERVFAIL, false);
(void) dns_stub_send_failure(q->manager, q->stub_listener_extra, q->request_dns_stream, q->request_dns_packet, DNS_RCODE_SERVFAIL, false);
break;

case DNS_TRANSACTION_NULL:
Expand Down Expand Up @@ -333,64 +355,64 @@ static int dns_stub_stream_complete(DnsStream *s, int error) {
return 0;
}

static void dns_stub_process_query(Manager *m, DnsStream *s, DnsPacket *p, bool is_extra) {
static void dns_stub_process_query(Manager *m, DnsStubListenerExtra *l, DnsStream *s, DnsPacket *p) {
_cleanup_(dns_query_freep) DnsQuery *q = NULL;
int r;

assert(m);
assert(p);
assert(p->protocol == DNS_PROTOCOL_DNS);

if (!is_extra &&
if (!l && /* l == NULL if this is the main stub */
(in_addr_is_localhost(p->family, &p->sender) <= 0 ||
in_addr_is_localhost(p->family, &p->destination) <= 0)) {
log_error("Got packet on unexpected IP range, refusing.");
dns_stub_send_failure(m, s, p, DNS_RCODE_SERVFAIL, false);
dns_stub_send_failure(m, l, s, p, DNS_RCODE_SERVFAIL, false);
return;
}

r = dns_packet_extract(p);
if (r < 0) {
log_debug_errno(r, "Failed to extract resources from incoming packet, ignoring packet: %m");
dns_stub_send_failure(m, s, p, DNS_RCODE_FORMERR, false);
dns_stub_send_failure(m, l, s, p, DNS_RCODE_FORMERR, false);
return;
}

if (!DNS_PACKET_VERSION_SUPPORTED(p)) {
log_debug("Got EDNS OPT field with unsupported version number.");
dns_stub_send_failure(m, s, p, DNS_RCODE_BADVERS, false);
dns_stub_send_failure(m, l, s, p, DNS_RCODE_BADVERS, false);
return;
}

if (dns_type_is_obsolete(p->question->keys[0]->type)) {
log_debug("Got message with obsolete key type, refusing.");
dns_stub_send_failure(m, s, p, DNS_RCODE_NOTIMP, false);
dns_stub_send_failure(m, l, s, p, DNS_RCODE_NOTIMP, false);
return;
}

if (dns_type_is_zone_transer(p->question->keys[0]->type)) {
log_debug("Got request for zone transfer, refusing.");
dns_stub_send_failure(m, s, p, DNS_RCODE_NOTIMP, false);
dns_stub_send_failure(m, l, s, p, DNS_RCODE_NOTIMP, false);
return;
}

if (!DNS_PACKET_RD(p)) {
/* If the "rd" bit is off (i.e. recursion was not requested), then refuse operation */
log_debug("Got request with recursion disabled, refusing.");
dns_stub_send_failure(m, s, p, DNS_RCODE_REFUSED, false);
dns_stub_send_failure(m, l, s, p, DNS_RCODE_REFUSED, false);
return;
}

if (DNS_PACKET_DO(p) && DNS_PACKET_CD(p)) {
log_debug("Got request with DNSSEC CD bit set, refusing.");
dns_stub_send_failure(m, s, p, DNS_RCODE_NOTIMP, false);
dns_stub_send_failure(m, l, s, p, DNS_RCODE_NOTIMP, false);
return;
}

r = dns_query_new(m, &q, p->question, p->question, 0, SD_RESOLVED_PROTOCOLS_ALL|SD_RESOLVED_NO_SEARCH);
if (r < 0) {
log_error_errno(r, "Failed to generate query object: %m");
dns_stub_send_failure(m, s, p, DNS_RCODE_SERVFAIL, false);
dns_stub_send_failure(m, l, s, p, DNS_RCODE_SERVFAIL, false);
return;
}

Expand All @@ -399,6 +421,7 @@ static void dns_stub_process_query(Manager *m, DnsStream *s, DnsPacket *p, bool

q->request_dns_packet = dns_packet_ref(p);
q->request_dns_stream = dns_stream_ref(s); /* make sure the stream stays around until we can send a reply through it */
q->stub_listener_extra = l;
q->complete = dns_stub_query_complete;

if (s) {
Expand All @@ -416,15 +439,15 @@ static void dns_stub_process_query(Manager *m, DnsStream *s, DnsPacket *p, bool
r = dns_query_go(q);
if (r < 0) {
log_error_errno(r, "Failed to start query: %m");
dns_stub_send_failure(m, s, p, DNS_RCODE_SERVFAIL, false);
dns_stub_send_failure(m, l, s, p, DNS_RCODE_SERVFAIL, false);
return;
}

log_debug("Processing query...");
TAKE_PTR(q);
}

static int on_dns_stub_packet_internal(sd_event_source *s, int fd, uint32_t revents, Manager *m, bool is_extra) {
static int on_dns_stub_packet_internal(sd_event_source *s, int fd, uint32_t revents, Manager *m, DnsStubListenerExtra *l) {
_cleanup_(dns_packet_unrefp) DnsPacket *p = NULL;
int r;

Expand All @@ -435,19 +458,23 @@ static int on_dns_stub_packet_internal(sd_event_source *s, int fd, uint32_t reve
if (dns_packet_validate_query(p) > 0) {
log_debug("Got DNS stub UDP query packet for id %u", DNS_PACKET_ID(p));

dns_stub_process_query(m, NULL, p, is_extra);
dns_stub_process_query(m, l, NULL, p);
} else
log_debug("Invalid DNS stub UDP packet, ignoring.");

return 0;
}

static int on_dns_stub_packet(sd_event_source *s, int fd, uint32_t revents, void *userdata) {
return on_dns_stub_packet_internal(s, fd, revents, userdata, false);
return on_dns_stub_packet_internal(s, fd, revents, userdata, NULL);
}

static int on_dns_stub_packet_extra(sd_event_source *s, int fd, uint32_t revents, void *userdata) {
return on_dns_stub_packet_internal(s, fd, revents, userdata, true);
DnsStubListenerExtra *l = userdata;

assert(l);

return on_dns_stub_packet_internal(s, fd, revents, l->manager, l);
}

static int set_dns_stub_common_socket_options(int fd, int family) {
Expand Down Expand Up @@ -528,6 +555,11 @@ static int manager_dns_stub_udp_fd_extra(Manager *m, DnsStubListenerExtra *l) {
union sockaddr_union sa;
int r;

assert(m);

if (!l)
return manager_dns_stub_udp_fd(m);

if (l->udp_event_source)
return 0;

Expand Down Expand Up @@ -565,7 +597,7 @@ static int manager_dns_stub_udp_fd_extra(Manager *m, DnsStubListenerExtra *l) {
goto fail;
}

r = sd_event_add_io(m->event, &l->udp_event_source, fd, EPOLLIN, on_dns_stub_packet_extra, m);
r = sd_event_add_io(m->event, &l->udp_event_source, fd, EPOLLIN, on_dns_stub_packet_extra, l);
if (r < 0)
goto fail;

Expand All @@ -590,7 +622,7 @@ static int manager_dns_stub_udp_fd_extra(Manager *m, DnsStubListenerExtra *l) {
return log_warning_errno(r, "Failed to listen on UDP socket %s: %m", strnull(pretty));
}

static int on_dns_stub_stream_packet_internal(DnsStream *s, bool is_extra) {
static int on_dns_stub_stream_packet(DnsStream *s) {
_cleanup_(dns_packet_unrefp) DnsPacket *p = NULL;

assert(s);
Expand All @@ -601,22 +633,14 @@ static int on_dns_stub_stream_packet_internal(DnsStream *s, bool is_extra) {
if (dns_packet_validate_query(p) > 0) {
log_debug("Got DNS stub TCP query packet for id %u", DNS_PACKET_ID(p));

dns_stub_process_query(s->manager, s, p, is_extra);
dns_stub_process_query(s->manager, s->stub_listener_extra, s, p);
} else
log_debug("Invalid DNS stub TCP packet, ignoring.");

return 0;
}

static int on_dns_stub_stream_packet(DnsStream *s) {
return on_dns_stub_stream_packet_internal(s, false);
}

static int on_dns_stub_stream_packet_extra(DnsStream *s) {
return on_dns_stub_stream_packet_internal(s, true);
}

static int on_dns_stub_stream_internal(sd_event_source *s, int fd, uint32_t revents, Manager *m, bool is_extra) {
static int on_dns_stub_stream_internal(sd_event_source *s, int fd, uint32_t revents, Manager *m, DnsStubListenerExtra *l) {
DnsStream *stream;
int cfd, r;

Expand All @@ -634,7 +658,8 @@ static int on_dns_stub_stream_internal(sd_event_source *s, int fd, uint32_t reve
return r;
}

stream->on_packet = is_extra ? on_dns_stub_stream_packet_extra : on_dns_stub_stream_packet;
stream->stub_listener_extra = l;
stream->on_packet = on_dns_stub_stream_packet;
stream->complete = dns_stub_stream_complete;

/* We let the reference to the stream dangle here, it will be dropped later by the complete callback. */
Expand All @@ -643,11 +668,14 @@ static int on_dns_stub_stream_internal(sd_event_source *s, int fd, uint32_t reve
}

static int on_dns_stub_stream(sd_event_source *s, int fd, uint32_t revents, void *userdata) {
return on_dns_stub_stream_internal(s, fd, revents, userdata, false);
return on_dns_stub_stream_internal(s, fd, revents, userdata, NULL);
}

static int on_dns_stub_stream_extra(sd_event_source *s, int fd, uint32_t revents, void *userdata) {
return on_dns_stub_stream_internal(s, fd, revents, userdata, true);
DnsStubListenerExtra *l = userdata;

assert(l);
return on_dns_stub_stream_internal(s, fd, revents, l->manager, l);
}

static int manager_dns_stub_tcp_fd(Manager *m) {
Expand Down Expand Up @@ -750,7 +778,7 @@ static int manager_dns_stub_tcp_fd_extra(Manager *m, DnsStubListenerExtra *l) {
goto fail;
}

r = sd_event_add_io(m->event, &l->tcp_event_source, fd, EPOLLIN, on_dns_stub_stream_extra, m);
r = sd_event_add_io(m->event, &l->tcp_event_source, fd, EPOLLIN, on_dns_stub_stream_extra, l);
if (r < 0)
goto fail;

Expand Down

0 comments on commit 0354029

Please sign in to comment.