resolved: rework how we handle truncation in the stub resolver

When we a reply message gets longer than the client supports we need to
truncate the response and set the TC bit, and we already do that.
However, we are not supposed to send incomplete RRs in that case, but
instead truncate right at a record boundary. Do that.

This fixes the "Message parser reports malformed message packet."
warning the venerable "host" tool outputs when a very large response is
requested.

See: #6520

diff --git a/src/resolve/resolve-tool.c b/src/resolve/resolve-tool.c
index c62058917fca9231de45c32fb807a8175c6f0f9c..195555c3c0a4684696472e9051576a5279e1e6b8 100644 (file)
--- a/src/resolve/resolve-tool.c
+++ b/src/resolve/resolve-tool.c
@@ -357,7 +357,7 @@ static int output_rr_packet(const void *d, size_t l, int ifindex) {
         int r;
         char ifname[IF_NAMESIZE] = "";
 
-        r = dns_packet_new(&p, DNS_PROTOCOL_DNS, 0);
+        r = dns_packet_new(&p, DNS_PROTOCOL_DNS, 0, DNS_PACKET_SIZE_MAX);
         if (r < 0)
                 return log_oom();
 

diff --git a/src/resolve/resolved-dns-packet.c b/src/resolve/resolved-dns-packet.c
index bf879475d4033d2ab1674a130d8557b283a42ab7..e2f227bfc64c84d4f75fc3fbdc3c2ab39145894d 100644 (file)
--- a/src/resolve/resolved-dns-packet.c
+++ b/src/resolve/resolved-dns-packet.c
@@ -43,11 +43,20 @@ static void rewind_dns_packet(DnsPacketRewinder *rewinder) {
 #define INIT_REWINDER(rewinder, p) do { rewinder.packet = p; rewinder.saved_rindex = p->rindex; } while (0)
 #define CANCEL_REWINDER(rewinder) do { rewinder.packet = NULL; } while (0)
 
-int dns_packet_new(DnsPacket **ret, DnsProtocol protocol, size_t min_alloc_dsize) {
+int dns_packet_new(
+                DnsPacket **ret,
+                DnsProtocol protocol,
+                size_t min_alloc_dsize,
+                size_t max_size) {
+
         DnsPacket *p;
         size_t a;
 
         assert(ret);
+        assert(max_size >= DNS_PACKET_HEADER_SIZE);
+
+        if (max_size > DNS_PACKET_SIZE_MAX)
+                max_size = DNS_PACKET_SIZE_MAX;
 
         /* The caller may not check what is going to be truly allocated, so do not allow to
          * allocate a DNS packet bigger than DNS_PACKET_SIZE_MAX.
@@ -70,8 +79,8 @@ int dns_packet_new(DnsPacket **ret, DnsProtocol protocol, size_t min_alloc_dsize
         a = PAGE_ALIGN(ALIGN(sizeof(DnsPacket)) + a) - ALIGN(sizeof(DnsPacket));
 
         /* make sure we never allocate more than useful */
-        if (a > DNS_PACKET_SIZE_MAX)
-                a = DNS_PACKET_SIZE_MAX;
+        if (a > max_size)
+                a = max_size;
 
         p = malloc0(ALIGN(sizeof(DnsPacket)) + a);
         if (!p)
@@ -79,6 +88,7 @@ int dns_packet_new(DnsPacket **ret, DnsProtocol protocol, size_t min_alloc_dsize
 
         p->size = p->rindex = DNS_PACKET_HEADER_SIZE;
         p->allocated = a;
+        p->max_size = max_size;
         p->protocol = protocol;
         p->opt_start = p->opt_size = (size_t) -1;
         p->n_ref = 1;
@@ -144,7 +154,7 @@ int dns_packet_new_query(DnsPacket **ret, DnsProtocol protocol, size_t min_alloc
 
         assert(ret);
 
-        r = dns_packet_new(&p, protocol, min_alloc_dsize);
+        r = dns_packet_new(&p, protocol, min_alloc_dsize, DNS_PACKET_SIZE_MAX);
         if (r < 0)
                 return r;
 
@@ -313,11 +323,13 @@ static int dns_packet_extend(DnsPacket *p, size_t add, void **ret, size_t *start
         assert(p);
 
         if (p->size + add > p->allocated) {
-                size_t a;
+                size_t a, ms;
 
                 a = PAGE_ALIGN((p->size + add) * 2);
-                if (a > DNS_PACKET_SIZE_MAX)
-                        a = DNS_PACKET_SIZE_MAX;
+
+                ms = dns_packet_size_max(p);
+                if (a > ms)
+                        a = ms;
 
                 if (p->size + add > a)
                         return -EMSGSIZE;

diff --git a/src/resolve/resolved-dns-packet.h b/src/resolve/resolved-dns-packet.h
index d4c6b3c9cbe823380bf1336d8473a544fccafed4..b873c0f745de8d22665bc0ec88f6025215d88a05 100644 (file)
--- a/src/resolve/resolved-dns-packet.h
+++ b/src/resolve/resolved-dns-packet.h
@@ -73,7 +73,7 @@ struct DnsPacketHeader {
 struct DnsPacket {
         int n_ref;
         DnsProtocol protocol;
-        size_t size, allocated, rindex;
+        size_t size, allocated, rindex, max_size;
         void *_data; /* don't access directly, use DNS_PACKET_DATA()! */
         Hashmap *names; /* For name compression */
         size_t opt_start, opt_size;
@@ -187,7 +187,7 @@ static inline unsigned DNS_PACKET_RRCOUNT(DnsPacket *p) {
                 (unsigned) DNS_PACKET_ARCOUNT(p);
 }
 
-int dns_packet_new(DnsPacket **p, DnsProtocol protocol, size_t min_alloc_dsize);
+int dns_packet_new(DnsPacket **p, DnsProtocol protocol, size_t min_alloc_dsize, size_t max_size);
 int dns_packet_new_query(DnsPacket **p, DnsProtocol protocol, size_t min_alloc_dsize, bool dnssec_checking_disabled);
 
 void dns_packet_set_flags(DnsPacket *p, bool dnssec_checking_disabled, bool truncated);
@@ -303,3 +303,13 @@ static inline uint64_t SD_RESOLVED_FLAGS_MAKE(DnsProtocol protocol, int family,
                 return f;
         }
 }
+
+static inline size_t dns_packet_size_max(DnsPacket *p) {
+        assert(p);
+
+        /* Why not insist on a fully initialized max_size during DnsPacket construction? Well, this way it's easy to
+         * allocate a transient, throw-away DnsPacket on the stack by simple zero initialization, without having to
+         * deal with explicit field initialization. */
+
+        return p->max_size != 0 ? p->max_size : DNS_PACKET_SIZE_MAX;
+}

diff --git a/src/resolve/resolved-dns-scope.c b/src/resolve/resolved-dns-scope.c
index b01ee6c9ccfdd41f02213954179331c864357c45..ca54158898a3ea6e73ff568178d7256da983060e 100644 (file)
--- a/src/resolve/resolved-dns-scope.c
+++ b/src/resolve/resolved-dns-scope.c
@@ -632,7 +632,7 @@ int dns_scope_make_reply_packet(
             dns_answer_isempty(soa))
                 return -EINVAL;
 
-        r = dns_packet_new(&p, s->protocol, 0);
+        r = dns_packet_new(&p, s->protocol, 0, DNS_PACKET_SIZE_MAX);
         if (r < 0)
                 return r;
 
@@ -818,7 +818,7 @@ static int dns_scope_make_conflict_packet(
         assert(rr);
         assert(ret);
 
-        r = dns_packet_new(&p, s->protocol, 0);
+        r = dns_packet_new(&p, s->protocol, 0, DNS_PACKET_SIZE_MAX);
         if (r < 0)
                 return r;
 

diff --git a/src/resolve/resolved-dns-stream.c b/src/resolve/resolved-dns-stream.c
index 4a94fa991696a2c77cd07fc4dfe322c78c067c9d..58925346875218e58c5487acc5e11ed61fe1f0ac 100644 (file)
--- a/src/resolve/resolved-dns-stream.c
+++ b/src/resolve/resolved-dns-stream.c
@@ -260,7 +260,7 @@ static int on_stream_io(sd_event_source *es, int fd, uint32_t revents, void *use
                                 ssize_t ss;
 
                                 if (!s->read_packet) {
-                                        r = dns_packet_new(&s->read_packet, s->protocol, be16toh(s->read_size));
+                                        r = dns_packet_new(&s->read_packet, s->protocol, be16toh(s->read_size), DNS_PACKET_SIZE_MAX);
                                         if (r < 0)
                                                 return dns_stream_complete(s, -r);
 

diff --git a/src/resolve/resolved-dns-stub.c b/src/resolve/resolved-dns-stub.c
index 292e94daa3de637a0468ea26522f1a3a612241d8..5bc79a313ec2b70d7d2fa992e2242c11da8681c6 100644 (file)
--- a/src/resolve/resolved-dns-stub.c
+++ b/src/resolve/resolved-dns-stub.c
@@ -30,9 +30,12 @@ static int manager_dns_stub_tcp_fd(Manager *m);
 
 static int dns_stub_make_reply_packet(
                 DnsPacket **p,
+                size_t max_size,
                 DnsQuestion *q,
-                DnsAnswer *answer) {
+                DnsAnswer *answer,
+                bool *ret_truncated) {
 
+        bool truncated = false;
         DnsResourceRecord *rr;
         unsigned c = 0;
         int r;
@@ -43,7 +46,7 @@ static int dns_stub_make_reply_packet(
          * roundtrips aren't expensive. */
 
         if (!*p) {
-                r = dns_packet_new(p, DNS_PROTOCOL_DNS, 0);
+                r = dns_packet_new(p, DNS_PROTOCOL_DNS, 0, max_size);
                 if (r < 0)
                         return r;
 
@@ -71,12 +74,21 @@ static int dns_stub_make_reply_packet(
                 continue;
         add:
                 r = dns_packet_append_rr(*p, rr, 0, NULL, NULL);
+                if (r == -EMSGSIZE) {
+                        truncated = true;
+                        break;
+                }
                 if (r < 0)
                         return r;
 
                 c++;
         }
 
+        if (ret_truncated)
+                *ret_truncated = truncated;
+        else if (truncated)
+                return -EMSGSIZE;
+
         DNS_PACKET_HEADER(*p)->ancount = htobe16(be16toh(DNS_PACKET_HEADER(*p)->ancount) + c);
 
         return 0;
@@ -86,6 +98,7 @@ static int dns_stub_finish_reply_packet(
                 DnsPacket *p,
                 uint16_t id,
                 int rcode,
+                bool tc,        /* set the Truncated bit? */
                 bool add_opt,   /* add an OPT RR to this packet? */
                 bool edns0_do,  /* set the EDNS0 DNSSEC OK bit? */
                 bool ad) {      /* set the DNSSEC authenticated data bit? */
@@ -110,14 +123,14 @@ static int dns_stub_finish_reply_packet(
         DNS_PACKET_HEADER(p)->id = id;
 
         DNS_PACKET_HEADER(p)->flags = htobe16(DNS_PACKET_MAKE_FLAGS(
-                                                              1 /* qr */,
-                                                              0 /* opcode */,
-                                                              0 /* aa */,
-                                                              0 /* tc */,
-                                                              1 /* rd */,
-                                                              1 /* ra */,
+                                                              1  /* qr */,
+                                                              0  /* opcode */,
+                                                              0  /* aa */,
+                                                              tc /* tc */,
+                                                              1  /* rd */,
+                                                              1  /* ra */,
                                                               ad /* ad */,
-                                                              0 /* cd */,
+                                                              0  /* cd */,
                                                               rcode));
 
         if (add_opt) {
@@ -149,12 +162,6 @@ static int dns_stub_send(Manager *m, DnsStream *s, DnsPacket *p, DnsPacket *repl
         else {
                 int fd;
 
-                /* Truncate the message to the right size */
-                if (reply->size > DNS_PACKET_PAYLOAD_SIZE_MAX(p)) {
-                        dns_packet_truncate(reply, DNS_PACKET_UNICAST_SIZE_MAX);
-                        DNS_PACKET_HEADER(reply)->flags = htobe16(be16toh(DNS_PACKET_HEADER(reply)->flags) | DNS_PACKET_FLAG_TC);
-                }
-
                 fd = manager_dns_stub_udp_fd(m);
                 if (fd < 0)
                         return log_debug_errno(fd, "Failed to get reply socket: %m");
@@ -178,11 +185,11 @@ static int dns_stub_send_failure(Manager *m, DnsStream *s, DnsPacket *p, int rco
         assert(m);
         assert(p);
 
-        r = dns_stub_make_reply_packet(&reply, p->question, NULL);
+        r = dns_stub_make_reply_packet(&reply, DNS_PACKET_PAYLOAD_SIZE_MAX(p), p->question, NULL, NULL);
         if (r < 0)
                 return log_debug_errno(r, "Failed to make failure packet: %m");
 
-        r = dns_stub_finish_reply_packet(reply, DNS_PACKET_ID(p), rcode, !!p->opt, DNS_PACKET_DO(p), authenticated);
+        r = dns_stub_finish_reply_packet(reply, DNS_PACKET_ID(p), rcode, false, !!p->opt, DNS_PACKET_DO(p), authenticated);
         if (r < 0)
                 return log_debug_errno(r, "Failed to build failure packet: %m");
 
@@ -197,9 +204,10 @@ static void dns_stub_query_complete(DnsQuery *q) {
 
         switch (q->state) {
 
-        case DNS_TRANSACTION_SUCCESS:
+        case DNS_TRANSACTION_SUCCESS: {
+                bool truncated;
 
-                r = dns_stub_make_reply_packet(&q->reply_dns_packet, q->question_idna, q->answer);
+                r = dns_stub_make_reply_packet(&q->reply_dns_packet, DNS_PACKET_PAYLOAD_SIZE_MAX(q->request_dns_packet), q->question_idna, q->answer, &truncated);
                 if (r < 0) {
                         log_debug_errno(r, "Failed to build reply packet: %m");
                         break;
@@ -221,6 +229,7 @@ static void dns_stub_query_complete(DnsQuery *q) {
                                 q->reply_dns_packet,
                                 DNS_PACKET_ID(q->request_dns_packet),
                                 q->answer_rcode,
+                                truncated,
                                 !!q->request_dns_packet->opt,
                                 DNS_PACKET_DO(q->request_dns_packet),
                                 dns_query_fully_authenticated(q));
@@ -231,6 +240,7 @@ static void dns_stub_query_complete(DnsQuery *q) {
 
                 (void) dns_stub_send(q->manager, 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));

diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c
index 1270ca2b25c4984fda7397301aa468ef7f803e7b..58fe572d3b9969c7bbfb2fa68c6d05a6498b69da 100644 (file)
--- a/src/resolve/resolved-manager.c
+++ b/src/resolve/resolved-manager.c
@@ -723,7 +723,7 @@ int manager_recv(Manager *m, int fd, DnsProtocol protocol, DnsPacket **ret) {
         if (ms < 0)
                 return ms;
 
-        r = dns_packet_new(&p, protocol, ms);
+        r = dns_packet_new(&p, protocol, ms, DNS_PACKET_SIZE_MAX);
         if (r < 0)
                 return r;
 

diff --git a/src/resolve/test-dns-packet.c b/src/resolve/test-dns-packet.c
index 8cbe492526ed650c2dc2c62935da2a72529a38e8..00dde9b6bde148f12773a62c8a3590ab9c87f274 100644 (file)
--- a/src/resolve/test-dns-packet.c
+++ b/src/resolve/test-dns-packet.c
@@ -75,7 +75,7 @@ static void test_packet_from_file(const char* filename, bool canonical) {
                 assert_se(packet_size > 0);
                 assert_se(offset + 8 + packet_size <= data_size);
 
-                assert_se(dns_packet_new(&p, DNS_PROTOCOL_DNS, 0) >= 0);
+                assert_se(dns_packet_new(&p, DNS_PROTOCOL_DNS, 0, DNS_PACKET_SIZE_MAX) >= 0);
 
                 assert_se(dns_packet_append_blob(p, data + offset + 8, packet_size, NULL) >= 0);
                 assert_se(dns_packet_read_rr(p, &rr, NULL, NULL) >= 0);
@@ -90,7 +90,7 @@ static void test_packet_from_file(const char* filename, bool canonical) {
 
                 assert_se(dns_resource_record_to_wire_format(rr, canonical) >= 0);
 
-                assert_se(dns_packet_new(&p2, DNS_PROTOCOL_DNS, 0) >= 0);
+                assert_se(dns_packet_new(&p2, DNS_PROTOCOL_DNS, 0, DNS_PACKET_SIZE_MAX) >= 0);
                 assert_se(dns_packet_append_blob(p2, rr->wire_format, rr->wire_format_size, NULL) >= 0);
                 assert_se(dns_packet_read_rr(p2, &rr2, NULL, NULL) >= 0);
 

diff --git a/src/resolve/test-resolved-packet.c b/src/resolve/test-resolved-packet.c
index cf886b1bf2c8f81f6b5dcd0de15d7308d657fb48..ab11fbcd32769c3f400fab714b3e8ada8f275b69 100644 (file)
--- a/src/resolve/test-resolved-packet.c
+++ b/src/resolve/test-resolved-packet.c
@@ -27,7 +27,7 @@ static void test_dns_packet_new(void) {
         for (i = 0; i <= DNS_PACKET_SIZE_MAX; i++) {
                 _cleanup_(dns_packet_unrefp) DnsPacket *p = NULL;
 
-                assert_se(dns_packet_new(&p, DNS_PROTOCOL_DNS, i) == 0);
+                assert_se(dns_packet_new(&p, DNS_PROTOCOL_DNS, i, DNS_PACKET_SIZE_MAX) == 0);
 
                 log_debug("dns_packet_new: %zu → %zu", i, p->allocated);
                 assert_se(p->allocated >= MIN(DNS_PACKET_SIZE_MAX, i));
@@ -36,7 +36,7 @@ static void test_dns_packet_new(void) {
                         i = MIN(i * 2, DNS_PACKET_SIZE_MAX - 10);
         }
 
-        assert_se(dns_packet_new(&p2, DNS_PROTOCOL_DNS, DNS_PACKET_SIZE_MAX + 1) == -EFBIG);
+        assert_se(dns_packet_new(&p2, DNS_PROTOCOL_DNS, DNS_PACKET_SIZE_MAX + 1, DNS_PACKET_SIZE_MAX) == -EFBIG);
 }
 
 int main(int argc, char **argv) {