Skip to content

Commit 519ef04

Browse files
committed
resolved: rework OPT RR generation logic
This moves management of the OPT RR out of the scope management and into the server and packet management. There are now explicit calls for appending and truncating the OPT RR from a packet (dns_packet_append_opt() and dns_packet_truncate_opt()) as well as a call to do the right thing depending on a DnsServer's feature level (dns_server_adjust_opt()). This also unifies the code to pick a server between the TCP and UDP code paths, and makes sure the feature level used for the transaction is selected at the time the server is picked, and not changed until the next time we pick a server. The server selction code is now unified in dns_transaction_pick_server(). This all fixes problems when changing between UDP and TCP communication for the same server, and makes sure the UDP and TCP codepaths are more alike. It also makes sure we never keep the UDP port open when switchung to TCP, so that we don't have to handle incoming datagrams on the latter we don't expect. As the new code picks the DNS server at the time we make a connection, we don't need to invalidate the DNS server anymore when changing to the next one, thus dns_transaction_next_dns_server() has been removed.
1 parent 97cc656 commit 519ef04

File tree

7 files changed

+178
-105
lines changed

7 files changed

+178
-105
lines changed

src/resolve/resolved-dns-packet.c

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ int dns_packet_new(DnsPacket **ret, DnsProtocol protocol, size_t mtu) {
5858
p->size = p->rindex = DNS_PACKET_HEADER_SIZE;
5959
p->allocated = a;
6060
p->protocol = protocol;
61+
p->opt_start = p->opt_size = (size_t) -1;
6162
p->n_ref = 1;
6263

6364
*ret = p;
@@ -681,14 +682,19 @@ static int dns_packet_append_types(DnsPacket *p, Bitmap *types, size_t *start) {
681682
}
682683

683684
/* Append the OPT pseudo-RR described in RFC6891 */
684-
int dns_packet_append_opt_rr(DnsPacket *p, uint16_t max_udp_size, bool edns0_do, size_t *start) {
685+
int dns_packet_append_opt(DnsPacket *p, uint16_t max_udp_size, bool edns0_do, size_t *start) {
685686
size_t saved_size;
686687
int r;
687688

688689
assert(p);
689690
/* we must never advertise supported packet size smaller than the legacy max */
690691
assert(max_udp_size >= DNS_PACKET_UNICAST_SIZE_MAX);
691692

693+
if (p->opt_start != (size_t) -1)
694+
return -EBUSY;
695+
696+
assert(p->opt_size == (size_t) -1);
697+
692698
saved_size = p->size;
693699

694700
/* empty name */
@@ -721,6 +727,11 @@ int dns_packet_append_opt_rr(DnsPacket *p, uint16_t max_udp_size, bool edns0_do,
721727
if (r < 0)
722728
goto fail;
723729

730+
DNS_PACKET_HEADER(p)->arcount = htobe16(DNS_PACKET_ARCOUNT(p) + 1);
731+
732+
p->opt_start = saved_size;
733+
p->opt_size = p->size - saved_size;
734+
724735
if (start)
725736
*start = saved_size;
726737

@@ -731,6 +742,27 @@ int dns_packet_append_opt_rr(DnsPacket *p, uint16_t max_udp_size, bool edns0_do,
731742
return r;
732743
}
733744

745+
int dns_packet_truncate_opt(DnsPacket *p) {
746+
assert(p);
747+
748+
if (p->opt_start == (size_t) -1) {
749+
assert(p->opt_size == (size_t) -1);
750+
return 0;
751+
}
752+
753+
assert(p->opt_size != (size_t) -1);
754+
assert(DNS_PACKET_ARCOUNT(p) > 0);
755+
756+
if (p->opt_start + p->opt_size != p->size)
757+
return -EBUSY;
758+
759+
dns_packet_truncate(p, p->opt_start);
760+
DNS_PACKET_HEADER(p)->arcount = htobe16(DNS_PACKET_ARCOUNT(p) - 1);
761+
p->opt_start = p->opt_size = (size_t) -1;
762+
763+
return 1;
764+
}
765+
734766
int dns_packet_append_rr(DnsPacket *p, const DnsResourceRecord *rr, size_t *start, size_t *rdata_start) {
735767
size_t saved_size, rdlength_offset, end, rdlength, rds;
736768
int r;
@@ -1046,7 +1078,6 @@ int dns_packet_append_rr(DnsPacket *p, const DnsResourceRecord *rr, size_t *star
10461078
return r;
10471079
}
10481080

1049-
10501081
int dns_packet_read(DnsPacket *p, size_t sz, const void **ret, size_t *start) {
10511082
assert(p);
10521083

src/resolve/resolved-dns-packet.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ struct DnsPacket {
7676
size_t size, allocated, rindex;
7777
void *_data; /* don't access directly, use DNS_PACKET_DATA()! */
7878
Hashmap *names; /* For name compression */
79+
size_t opt_start, opt_size;
7980

8081
/* Parsed data */
8182
DnsQuestion *question;
@@ -173,9 +174,10 @@ int dns_packet_append_label(DnsPacket *p, const char *s, size_t l, bool canonica
173174
int dns_packet_append_name(DnsPacket *p, const char *name, bool allow_compression, bool canonical_candidate, size_t *start);
174175
int dns_packet_append_key(DnsPacket *p, const DnsResourceKey *key, size_t *start);
175176
int dns_packet_append_rr(DnsPacket *p, const DnsResourceRecord *rr, size_t *start, size_t *rdata_start);
176-
int dns_packet_append_opt_rr(DnsPacket *p, uint16_t max_udp_size, bool edns0_do, size_t *start);
177+
int dns_packet_append_opt(DnsPacket *p, uint16_t max_udp_size, bool edns0_do, size_t *start);
177178

178179
void dns_packet_truncate(DnsPacket *p, size_t sz);
180+
int dns_packet_truncate_opt(DnsPacket *p);
179181

180182
int dns_packet_read(DnsPacket *p, size_t sz, const void **ret, size_t *start);
181183
int dns_packet_read_blob(DnsPacket *p, void *d, size_t sz, size_t *start);

src/resolve/resolved-dns-scope.c

Lines changed: 35 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -162,17 +162,15 @@ void dns_scope_packet_lost(DnsScope *s, usec_t usec) {
162162
s->resend_timeout = MIN(s->resend_timeout * 2, MULTICAST_RESEND_TIMEOUT_MAX_USEC);
163163
}
164164

165-
static int dns_scope_emit_one(DnsScope *s, int fd, DnsServer *server, DnsPacket *p) {
165+
static int dns_scope_emit_one(DnsScope *s, int fd, DnsPacket *p) {
166166
union in_addr_union addr;
167167
int ifindex = 0, r;
168168
int family;
169169
uint32_t mtu;
170-
size_t saved_size = 0;
171170

172171
assert(s);
173172
assert(p);
174173
assert(p->protocol == s->protocol);
175-
assert((s->protocol == DNS_PROTOCOL_DNS) != (fd < 0));
176174

177175
if (s->link) {
178176
mtu = s->link->mtu;
@@ -181,30 +179,13 @@ static int dns_scope_emit_one(DnsScope *s, int fd, DnsServer *server, DnsPacket
181179
mtu = manager_find_mtu(s->manager);
182180

183181
switch (s->protocol) {
182+
184183
case DNS_PROTOCOL_DNS:
185-
assert(server);
184+
assert(fd >= 0);
186185

187186
if (DNS_PACKET_QDCOUNT(p) > 1)
188187
return -EOPNOTSUPP;
189188

190-
if (server->possible_features >= DNS_SERVER_FEATURE_LEVEL_EDNS0) {
191-
bool edns_do;
192-
size_t packet_size;
193-
194-
edns_do = server->possible_features >= DNS_SERVER_FEATURE_LEVEL_DO;
195-
196-
if (server->possible_features >= DNS_SERVER_FEATURE_LEVEL_LARGE)
197-
packet_size = DNS_PACKET_UNICAST_SIZE_LARGE_MAX;
198-
else
199-
packet_size = server->received_udp_packet_max;
200-
201-
r = dns_packet_append_opt_rr(p, packet_size, edns_do, &saved_size);
202-
if (r < 0)
203-
return r;
204-
205-
DNS_PACKET_HEADER(p)->arcount = htobe16(be16toh(DNS_PACKET_HEADER(p)->arcount) + 1);
206-
}
207-
208189
if (p->size > DNS_PACKET_UNICAST_SIZE_MAX)
209190
return -EMSGSIZE;
210191

@@ -215,15 +196,11 @@ static int dns_scope_emit_one(DnsScope *s, int fd, DnsServer *server, DnsPacket
215196
if (r < 0)
216197
return r;
217198

218-
if (saved_size > 0) {
219-
dns_packet_truncate(p, saved_size);
220-
221-
DNS_PACKET_HEADER(p)->arcount = htobe16(be16toh(DNS_PACKET_HEADER(p)->arcount) - 1);
222-
}
223-
224199
break;
225200

226201
case DNS_PROTOCOL_LLMNR:
202+
assert(fd < 0);
203+
227204
if (DNS_PACKET_QDCOUNT(p) > 1)
228205
return -EOPNOTSUPP;
229206

@@ -250,6 +227,8 @@ static int dns_scope_emit_one(DnsScope *s, int fd, DnsServer *server, DnsPacket
250227
break;
251228

252229
case DNS_PROTOCOL_MDNS:
230+
assert(fd < 0);
231+
253232
if (!ratelimit_test(&s->ratelimit))
254233
return -EBUSY;
255234

@@ -279,13 +258,13 @@ static int dns_scope_emit_one(DnsScope *s, int fd, DnsServer *server, DnsPacket
279258
return 1;
280259
}
281260

282-
int dns_scope_emit_udp(DnsScope *s, int fd, DnsServer *server, DnsPacket *p) {
261+
int dns_scope_emit_udp(DnsScope *s, int fd, DnsPacket *p) {
283262
int r;
284263

285264
assert(s);
286265
assert(p);
287266
assert(p->protocol == s->protocol);
288-
assert((s->protocol == DNS_PROTOCOL_DNS) != (fd < 0));
267+
assert((s->protocol == DNS_PROTOCOL_DNS) == (fd >= 0));
289268

290269
do {
291270
/* If there are multiple linked packets, set the TC bit in all but the last of them */
@@ -294,51 +273,52 @@ int dns_scope_emit_udp(DnsScope *s, int fd, DnsServer *server, DnsPacket *p) {
294273
dns_packet_set_flags(p, true, true);
295274
}
296275

297-
r = dns_scope_emit_one(s, fd, server, p);
276+
r = dns_scope_emit_one(s, fd, p);
298277
if (r < 0)
299278
return r;
300279

301280
p = p->more;
302-
} while(p);
281+
} while (p);
303282

304283
return 0;
305284
}
306285

307-
static int dns_scope_socket(DnsScope *s, int type, int family, const union in_addr_union *address, uint16_t port, DnsServer **server) {
308-
DnsServer *srv = NULL;
286+
static int dns_scope_socket(
287+
DnsScope *s,
288+
int type,
289+
int family,
290+
const union in_addr_union *address,
291+
DnsServer *server,
292+
uint16_t port) {
293+
309294
_cleanup_close_ int fd = -1;
310295
union sockaddr_union sa = {};
311296
socklen_t salen;
312297
static const int one = 1;
313298
int ret, r;
314299

315300
assert(s);
316-
assert((family == AF_UNSPEC) == !address);
317-
318-
if (family == AF_UNSPEC) {
319-
srv = dns_scope_get_dns_server(s);
320-
if (!srv)
321-
return -ESRCH;
322-
323-
/* Determine current feature level to use */
324-
(void) dns_server_possible_features(srv);
325301

326-
if (type == SOCK_DGRAM && srv->possible_features < DNS_SERVER_FEATURE_LEVEL_UDP)
327-
return -EAGAIN;
302+
if (server) {
303+
assert(family == AF_UNSPEC);
304+
assert(!address);
328305

329-
sa.sa.sa_family = srv->family;
330-
if (srv->family == AF_INET) {
306+
sa.sa.sa_family = server->family;
307+
if (server->family == AF_INET) {
331308
sa.in.sin_port = htobe16(port);
332-
sa.in.sin_addr = srv->address.in;
309+
sa.in.sin_addr = server->address.in;
333310
salen = sizeof(sa.in);
334-
} else if (srv->family == AF_INET6) {
311+
} else if (server->family == AF_INET6) {
335312
sa.in6.sin6_port = htobe16(port);
336-
sa.in6.sin6_addr = srv->address.in6;
313+
sa.in6.sin6_addr = server->address.in6;
337314
sa.in6.sin6_scope_id = s->link ? s->link->ifindex : 0;
338315
salen = sizeof(sa.in6);
339316
} else
340317
return -EAFNOSUPPORT;
341318
} else {
319+
assert(family != AF_UNSPEC);
320+
assert(address);
321+
342322
sa.sa.sa_family = family;
343323

344324
if (family == AF_INET) {
@@ -396,21 +376,18 @@ static int dns_scope_socket(DnsScope *s, int type, int family, const union in_ad
396376
if (r < 0 && errno != EINPROGRESS)
397377
return -errno;
398378

399-
if (server)
400-
*server = srv;
401-
402379
ret = fd;
403380
fd = -1;
404381

405382
return ret;
406383
}
407384

408-
int dns_scope_socket_udp(DnsScope *s, DnsServer **server) {
409-
return dns_scope_socket(s, SOCK_DGRAM, AF_UNSPEC, NULL, 53, server);
385+
int dns_scope_socket_udp(DnsScope *s, DnsServer *server, uint16_t port) {
386+
return dns_scope_socket(s, SOCK_DGRAM, AF_UNSPEC, NULL, server, port);
410387
}
411388

412-
int dns_scope_socket_tcp(DnsScope *s, int family, const union in_addr_union *address, uint16_t port, DnsServer **server) {
413-
return dns_scope_socket(s, SOCK_STREAM, family, address, port, server);
389+
int dns_scope_socket_tcp(DnsScope *s, int family, const union in_addr_union *address, DnsServer *server, uint16_t port) {
390+
return dns_scope_socket(s, SOCK_STREAM, family, address, server, port);
414391
}
415392

416393
DnsScopeMatch dns_scope_good_domain(DnsScope *s, int ifindex, uint64_t flags, const char *domain) {
@@ -869,7 +846,7 @@ static int on_conflict_dispatch(sd_event_source *es, usec_t usec, void *userdata
869846
return 0;
870847
}
871848

872-
r = dns_scope_emit_udp(scope, -1, NULL, p);
849+
r = dns_scope_emit_udp(scope, -1, p);
873850
if (r < 0)
874851
log_debug_errno(r, "Failed to send conflict packet: %m");
875852
}

src/resolve/resolved-dns-scope.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,9 +82,9 @@ DnsScope* dns_scope_free(DnsScope *s);
8282
void dns_scope_packet_received(DnsScope *s, usec_t rtt);
8383
void dns_scope_packet_lost(DnsScope *s, usec_t usec);
8484

85-
int dns_scope_emit_udp(DnsScope *s, int fd, DnsServer *server, DnsPacket *p);
86-
int dns_scope_socket_tcp(DnsScope *s, int family, const union in_addr_union *address, uint16_t port, DnsServer **server);
87-
int dns_scope_socket_udp(DnsScope *s, DnsServer **server);
85+
int dns_scope_emit_udp(DnsScope *s, int fd, DnsPacket *p);
86+
int dns_scope_socket_tcp(DnsScope *s, int family, const union in_addr_union *address, DnsServer *server, uint16_t port);
87+
int dns_scope_socket_udp(DnsScope *s, DnsServer *server, uint16_t port);
8888

8989
DnsScopeMatch dns_scope_good_domain(DnsScope *s, int ifindex, uint64_t flags, const char *domain);
9090
int dns_scope_good_key(DnsScope *s, DnsResourceKey *key);

src/resolve/resolved-dns-server.c

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,34 @@ DnsServerFeatureLevel dns_server_possible_features(DnsServer *s) {
342342
return s->possible_features;
343343
}
344344

345+
int dns_server_adjust_opt(DnsServer *server, DnsPacket *packet, DnsServerFeatureLevel level) {
346+
size_t packet_size;
347+
bool edns_do;
348+
int r;
349+
350+
assert(server);
351+
assert(packet);
352+
assert(packet->protocol == DNS_PROTOCOL_DNS);
353+
354+
/* Fix the OPT field in the packet to match our current feature level. */
355+
356+
r = dns_packet_truncate_opt(packet);
357+
if (r < 0)
358+
return r;
359+
360+
if (level < DNS_SERVER_FEATURE_LEVEL_EDNS0)
361+
return 0;
362+
363+
edns_do = level >= DNS_SERVER_FEATURE_LEVEL_DO;
364+
365+
if (level >= DNS_SERVER_FEATURE_LEVEL_LARGE)
366+
packet_size = DNS_PACKET_UNICAST_SIZE_LARGE_MAX;
367+
else
368+
packet_size = server->received_udp_packet_max;
369+
370+
return dns_packet_append_opt(packet, packet_size, edns_do, NULL);
371+
}
372+
345373
static void dns_server_hash_func(const void *p, struct siphash *state) {
346374
const DnsServer *s = p;
347375

src/resolve/resolved-dns-server.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,8 @@ void dns_server_packet_lost(DnsServer *s, DnsServerFeatureLevel features, usec_t
104104
void dns_server_packet_failed(DnsServer *s, DnsServerFeatureLevel features);
105105
void dns_server_packet_rrsig_missing(DnsServer *s);
106106

107+
int dns_server_adjust_opt(DnsServer *server, DnsPacket *packet, DnsServerFeatureLevel level);
108+
107109
DnsServer *dns_server_find(DnsServer *first, int family, const union in_addr_union *in_addr);
108110

109111
void dns_server_unlink_all(DnsServer *first);

0 commit comments

Comments
 (0)