Merge pull request #34667 from rpigott/resolved-bypass

resolve: fixes for sd-resolved bypass

4 files changed, 25 insertions, 13 deletions

diff --git a/src/resolve/resolved-dns-packet.h b/src/resolve/resolved-dns-packet.h
index 13d65a02c3..437c220b6a 100644
--- a/src/resolve/resolved-dns-packet.h
+++ b/src/resolve/resolved-dns-packet.h
@@ -111,6 +111,7 @@ static inline uint8_t* DNS_PACKET_DATA(const DnsPacket *p) {
 #define DNS_PACKET_AD(p) ((be16toh(DNS_PACKET_HEADER(p)->flags) >> 5) & 1)
 #define DNS_PACKET_CD(p) ((be16toh(DNS_PACKET_HEADER(p)->flags) >> 4) & 1)
 
+#define DNS_PACKET_FLAG_CD (UINT16_C(1) << 4)
 #define DNS_PACKET_FLAG_AD (UINT16_C(1) << 5)
 #define DNS_PACKET_FLAG_TC (UINT16_C(1) << 9)
 
diff --git a/src/resolve/resolved-dns-stub.c b/src/resolve/resolved-dns-stub.c
index c604a51c4b..bd0822e6dc 100644
--- a/src/resolve/resolved-dns-stub.c
+++ b/src/resolve/resolved-dns-stub.c
@@ -462,10 +462,6 @@ static int dns_stub_finish_reply_packet(
                         rcode = DNS_RCODE_SERVFAIL;
         }
 
-        /* Don't set the CD bit unless DO is on, too */
-        if (!edns0_do)
-                cd = false;
-
         /* Note that we allow the AD bit to be set even if client didn't signal DO, as per RFC 6840, section
          * 5.7 */
 
@@ -631,7 +627,7 @@ static int dns_stub_send_reply(
                         !!q->request_packet->opt,
                         edns0_do,
                         (DNS_PACKET_AD(q->request_packet) || DNS_PACKET_DO(q->request_packet)) && dns_query_fully_authenticated(q),
-                        DNS_PACKET_CD(q->request_packet),
+                        FLAGS_SET(q->flags, SD_RESOLVED_NO_VALIDATE),
                         q->stub_listener_extra ? ADVERTISE_EXTRA_DATAGRAM_SIZE_MAX : ADVERTISE_DATAGRAM_SIZE_MAX,
                         dns_packet_has_nsid_request(q->request_packet) > 0 && !q->stub_listener_extra);
         if (r < 0)
@@ -686,6 +682,7 @@ static int dns_stub_patch_bypass_reply_packet(
                 DnsPacket **ret,       /* Where to place the patched packet */
                 DnsPacket *original,   /* The packet to patch */
                 DnsPacket *request,    /* The packet the patched packet shall look like a reply to */
+                bool validated,
                 bool authenticated) {
         _cleanup_(dns_packet_unrefp) DnsPacket *c = NULL;
         int r;
@@ -726,9 +723,14 @@ static int dns_stub_patch_bypass_reply_packet(
                 DNS_PACKET_HEADER(c)->flags = htobe16(be16toh(DNS_PACKET_HEADER(c)->flags) | DNS_PACKET_FLAG_TC);
         }
 
+        /* Patch the cd bit to reflect the state of validation: set when both we and the upstream
+         * resolver have checking disabled. */
+        DNS_PACKET_HEADER(c)->flags = htobe16(UPDATE_FLAG(be16toh(DNS_PACKET_HEADER(c)->flags),
+                                DNS_PACKET_FLAG_CD, DNS_PACKET_CD(original) && !validated));
+
         /* Ensure we don't pass along an untrusted ad flag for bypass packets */
-        if (!authenticated)
-                DNS_PACKET_HEADER(c)->flags = htobe16(be16toh(DNS_PACKET_HEADER(c)->flags) & ~DNS_PACKET_FLAG_AD);
+        DNS_PACKET_HEADER(c)->flags = htobe16(UPDATE_FLAG(be16toh(DNS_PACKET_HEADER(c)->flags),
+                                DNS_PACKET_FLAG_AD, authenticated));
 
         *ret = TAKE_PTR(c);
         return 0;
@@ -751,6 +753,7 @@ static void dns_stub_query_complete(DnsQuery *query) {
                         _cleanup_(dns_packet_unrefp) DnsPacket *reply = NULL;
 
                         r = dns_stub_patch_bypass_reply_packet(&reply, q->answer_full_packet, q->request_packet,
+                                        /* validated = */ !FLAGS_SET(q->flags, SD_RESOLVED_NO_VALIDATE),
                                         FLAGS_SET(q->answer_query_flags, SD_RESOLVED_AUTHENTICATED));
                         if (r < 0)
                                 log_debug_errno(r, "Failed to patch bypass reply packet: %m");
@@ -982,7 +985,7 @@ static void dns_stub_process_query(Manager *m, DnsStubListenerExtra *l, DnsStrea
                                   protocol_flags|
                                   SD_RESOLVED_NO_CNAME|
                                   SD_RESOLVED_NO_SEARCH|
-                                  SD_RESOLVED_NO_VALIDATE|
+                                  (DNS_PACKET_CD(p) ? SD_RESOLVED_NO_VALIDATE | SD_RESOLVED_NO_CACHE : 0)|
                                   SD_RESOLVED_REQUIRE_PRIMARY|
                                   SD_RESOLVED_CLAMP_TTL|
                                   SD_RESOLVED_RELAX_SINGLE_LABEL);
@@ -990,6 +993,7 @@ static void dns_stub_process_query(Manager *m, DnsStubListenerExtra *l, DnsStrea
                 r = dns_query_new(m, &q, p->question, p->question, NULL, 0,
                                   protocol_flags|
                                   SD_RESOLVED_NO_SEARCH|
+                                  (DNS_PACKET_CD(p) ? SD_RESOLVED_NO_VALIDATE | SD_RESOLVED_NO_CACHE : 0)|
                                   (DNS_PACKET_DO(p) ? SD_RESOLVED_REQUIRE_PRIMARY : 0)|
                                   SD_RESOLVED_CLAMP_TTL);
         if (r < 0) {
diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c
index 17a815cd44..f78bf0702b 100644
--- a/src/resolve/resolved-dns-transaction.c
+++ b/src/resolve/resolved-dns-transaction.c
@@ -840,10 +840,8 @@ static void dns_transaction_cache_answer(DnsTransaction *t) {
                       dns_transaction_key(t),
                       t->answer_rcode,
                       t->answer,
-                      DNS_PACKET_CD(t->received) ? t->received : NULL, /* only cache full packets with CD on,
-                                                                        * since our use case for caching them
-                                                                        * is "bypass" mode which is only
-                                                                        * enabled for CD packets. */
+                      /* If neither DO nor EDE is set, the full packet isn't useful to cache */
+                      DNS_PACKET_DO(t->received) || t->answer_ede_rcode > 0 || t->answer_ede_msg ? t->received : NULL,
                       t->answer_query_flags,
                       t->answer_dnssec_result,
                       t->answer_nsec_ttl,
diff --git a/test/units/TEST-75-RESOLVED.sh b/test/units/TEST-75-RESOLVED.sh
index 801104710b..4a7b759717 100755
--- a/test/units/TEST-75-RESOLVED.sh
+++ b/test/units/TEST-75-RESOLVED.sh
@@ -517,8 +517,17 @@ testcase_08_resolved() {
     grep -qF "fd00:dead:beef:cafe::11" "$RUN_OUT"
     grep -qF "authenticated: yes" "$RUN_OUT"
 
-    run dig +short signed.test
+    run dig +nostats signed.test
     grep -qF "10.0.0.10" "$RUN_OUT"
+    grep -q "flags:[^;]* ad" "$RUN_OUT"
+    run dig +nostats +cd signed.test
+    grep -qF "10.0.0.10" "$RUN_OUT"
+    grep -q "flags:[^;]* cd" "$RUN_OUT"
+    grep -qv "flags:[^;]* ad" "$RUN_OUT"
+    run dig +nostats +do signed.test
+    grep -qF "10.0.0.10" "$RUN_OUT"
+    grep -q "flags:[^;]* ad" "$RUN_OUT"
+    grep -qv "flags:[^;]* cd" "$RUN_OUT"
     run resolvectl query signed.test
     grep -qF "signed.test: 10.0.0.10" "$RUN_OUT"
     grep -qF "authenticated: yes" "$RUN_OUT"