resolve: tolerate merging a zero-ttl RR and a nonzero-ttl RR if not mDNS

resolved rejected RRsets containing a RR with a zero TTL and a RR with a nonzero TTL. In practice—see the linked issues—, this case triggered when an AF_UNSPEC query to a CNAMEd domain returned a zero TTL for the CNAME on one address family and a nonzero TTL for the CNAME on the other address family.

The zero-nonzero TTL check cites RFC 2181 § 5.2 in a comment. That section says DNS clients should reject any RRset containing differing TTLs, which the check only implements a very special case of. That the old behavior caused real-world false NXDOMAIN results is reason enough to completely ignore the RFC's recommendation. However, mDNS treats zero TTLs specially, so the error case needs to be kept for mDNS.

Fixes https://github.com/systemd/systemd/issues/22177
Fixes https://github.com/systemd/systemd/issues/20617
Fixes https://github.com/systemd/systemd/issues/19118

diff --git a/src/resolve/resolved-dns-answer.c b/src/resolve/resolved-dns-answer.c
index 3d42b0d000be2971a3525ef8bd21b94cd48402a0..bf023a77e448c7509e46926ec539939929980caa 100644 (file)
--- a/src/resolve/resolved-dns-answer.c
+++ b/src/resolve/resolved-dns-answer.c
@@ -181,11 +181,23 @@ int dns_answer_add(
 
         exist = ordered_set_get(a->items, &tmp);
         if (exist) {
-                /* There's already an RR of the same RRset in place! Let's see if the TTLs more or less
-                 * match. We don't really care if they match precisely, but we do care whether one is 0 and
-                 * the other is not. See RFC 2181, Section 5.2. */
-                if ((rr->ttl == 0) != (exist->rr->ttl == 0))
-                        return -EINVAL;
+                /* There's already an RR of the same RRset in place! Let's see if the TTLs more or
+                 * less match. RFC 2181, Section 5.2 suggests clients should reject RRsets
+                 * containing RRs with differing TTLs. We are more tolerant of this situation except
+                 * if one RR has a zero TTL and the other a nonzero TTL. In mDNS, zero TTLs are
+                 * special, so we must error in that case. */
+                if ((rr->ttl == 0) != (exist->rr->ttl == 0)) {
+                        if ((exist->flags | flags) & DNS_ANSWER_REFUSE_TTL_NO_MATCH)
+                                return log_debug_errno(
+                                                SYNTHETIC_ERRNO(EINVAL),
+                                                "Refusing to merge RRs with zero TTL and non-zero TTL: %s vs. %s",
+                                                dns_resource_record_to_string(rr),
+                                                dns_resource_record_to_string(exist->rr));
+
+                        log_debug("Merging RRs with zero TTL and non-zero TTL (not RFC 2181/5.2 compliant): %s vs. %s",
+                                  dns_resource_record_to_string(rr),
+                                  dns_resource_record_to_string(exist->rr));
+                }
 
                 /* Entry already exists, keep the entry with the higher TTL. */
                 if (rr->ttl > exist->rr->ttl) {

diff --git a/src/resolve/resolved-dns-answer.h b/src/resolve/resolved-dns-answer.h
index 93afea32d558606f06e60dbe9c15f1d922ad0760..068803c6cb13c2eb38a457574c1563a4d578e549 100644 (file)
--- a/src/resolve/resolved-dns-answer.h
+++ b/src/resolve/resolved-dns-answer.h
@@ -14,14 +14,15 @@ typedef struct DnsAnswerItem DnsAnswerItem;
  * Note that we usually encode the empty DnsAnswer object as a simple NULL. */
 
 typedef enum DnsAnswerFlags {
-        DNS_ANSWER_AUTHENTICATED      = 1 << 0, /* Item has been authenticated */
-        DNS_ANSWER_CACHEABLE          = 1 << 1, /* Item is subject to caching */
-        DNS_ANSWER_SHARED_OWNER       = 1 << 2, /* For mDNS: RRset may be owner by multiple peers */
-        DNS_ANSWER_CACHE_FLUSH        = 1 << 3, /* For mDNS: sets cache-flush bit in the rrclass of response records */
-        DNS_ANSWER_GOODBYE            = 1 << 4, /* For mDNS: item is subject to disappear */
-        DNS_ANSWER_SECTION_ANSWER     = 1 << 5, /* When parsing: RR originates from answer section */
-        DNS_ANSWER_SECTION_AUTHORITY  = 1 << 6, /* When parsing: RR originates from authority section */
-        DNS_ANSWER_SECTION_ADDITIONAL = 1 << 7, /* When parsing: RR originates from additional section */
+        DNS_ANSWER_AUTHENTICATED       = 1 << 0, /* Item has been authenticated */
+        DNS_ANSWER_CACHEABLE           = 1 << 1, /* Item is subject to caching */
+        DNS_ANSWER_SHARED_OWNER        = 1 << 2, /* For mDNS: RRset may be owner by multiple peers */
+        DNS_ANSWER_CACHE_FLUSH         = 1 << 3, /* For mDNS: sets cache-flush bit in the rrclass of response records */
+        DNS_ANSWER_GOODBYE             = 1 << 4, /* For mDNS: item is subject to disappear */
+        DNS_ANSWER_SECTION_ANSWER      = 1 << 5, /* When parsing: RR originates from answer section */
+        DNS_ANSWER_SECTION_AUTHORITY   = 1 << 6, /* When parsing: RR originates from authority section */
+        DNS_ANSWER_SECTION_ADDITIONAL  = 1 << 7, /* When parsing: RR originates from additional section */
+        DNS_ANSWER_REFUSE_TTL_NO_MATCH = 1 << 8, /* For mDNS; refuse to merge a zero TTL RR with a nonzero TTL RR */
 
         DNS_ANSWER_MASK_SECTIONS      = DNS_ANSWER_SECTION_ANSWER|
                                         DNS_ANSWER_SECTION_AUTHORITY|

diff --git a/src/resolve/resolved-dns-packet.c b/src/resolve/resolved-dns-packet.c
index d63760b7d1e2353db3e0063ba1beade35b1ba500..ecc75c6590b1e8343427d8c6f32a26629ee0b117 100644 (file)
--- a/src/resolve/resolved-dns-packet.c
+++ b/src/resolve/resolved-dns-packet.c
@@ -2348,8 +2348,11 @@ static int dns_packet_extract_answer(DnsPacket *p, DnsAnswer **ret_answer) {
                 } else {
                         DnsAnswerFlags flags = 0;
 
-                        if (p->protocol == DNS_PROTOCOL_MDNS && !cache_flush)
-                                flags |= DNS_ANSWER_SHARED_OWNER;
+                        if (p->protocol == DNS_PROTOCOL_MDNS) {
+                                flags |= DNS_ANSWER_REFUSE_TTL_NO_MATCH;
+                                if (!cache_flush)
+                                        flags |= DNS_ANSWER_SHARED_OWNER;
+                        }
 
                         /* According to RFC 4795, section 2.9. only the RRs from the Answer section shall be
                          * cached. Hence mark only those RRs as cacheable by default, but not the ones from

diff --git a/src/resolve/resolved-mdns.c b/src/resolve/resolved-mdns.c
index d4a919fc60d53f4fcad5e9618b2a70bcf6f36f50..11e3b14125313a3076cba951b07562ac15ad067c 100644 (file)
--- a/src/resolve/resolved-mdns.c
+++ b/src/resolve/resolved-mdns.c
@@ -314,7 +314,7 @@ static int mdns_scope_process_query(DnsScope *s, DnsPacket *p) {
                 }
 
                 DNS_ANSWER_FOREACH_ITEM(item, answer) {
-                        DnsAnswerFlags flags = item->flags;
+                        DnsAnswerFlags flags = item->flags | DNS_ANSWER_REFUSE_TTL_NO_MATCH;
                         /* The cache-flush bit must not be set in legacy unicast responses.
                          * See section 6.7 of RFC 6762. */
                         if (legacy_query)