dhcp6: fix wrong length for IA_PD dhcp6 option

Fixes an issue introduced by 73b49d433c2c8e6304c8b82538bd4231d070fce4.

When PrefixDelegationHint= is not set, dhcp6_option_append_pd() sets
wrong length for IA_PD option, as `r` is `-EINVAL`.

Fixes #19021.

src/journal/journald-stream.c

@@ -334,6 +334,22 @@ static int stdout_stream_log(
         return 0;
 }
 
+static int syslog_parse_priority_and_facility(const char *s) {
+        int prio, r;
+
+        /* Parses both facility and priority in one value, i.e. is different from log_level_from_string()
+         * which only parses the priority and refuses any facility value */
+
+        r = safe_atoi(s, &prio);
+        if (r < 0)
+                return r;
+
+        if (prio < 0 || prio > 999)
+                return -ERANGE;
+
+        return prio;
+}
+
 static int stdout_stream_line(StdoutStream *s, char *p, LineBreak line_break) {
         char *orig;
         int r;
@@ -373,22 +389,22 @@ static int stdout_stream_line(StdoutStream *s, char *p, LineBreak line_break) {
                 s->state = STDOUT_STREAM_PRIORITY;
                 return 0;
 
-        case STDOUT_STREAM_PRIORITY:
-                r = safe_atoi(p, &s->priority);
-                if (r < 0 || s->priority < 0 || s->priority > 999) {
-                        log_warning("Failed to parse log priority line.");
-                        return -EINVAL;
-                }
+        case STDOUT_STREAM_PRIORITY: {
+                int priority;
 
+                priority = syslog_parse_priority_and_facility(p);
+                if (priority < 0)
+                        return log_warning_errno(priority, "Failed to parse log priority line: %m");
+
+                s->priority = priority;
                 s->state = STDOUT_STREAM_LEVEL_PREFIX;
                 return 0;
+        }
 
         case STDOUT_STREAM_LEVEL_PREFIX:
                 r = parse_boolean(p);
-                if (r < 0) {
-                        log_warning("Failed to parse level prefix line.");
-                        return -EINVAL;
-                }
+                if (r < 0)
+                        return log_warning_errno(r, "Failed to parse level prefix line: %m");
 
                 s->level_prefix = r;
                 s->state = STDOUT_STREAM_FORWARD_TO_SYSLOG;
@@ -396,10 +412,8 @@ static int stdout_stream_line(StdoutStream *s, char *p, LineBreak line_break) {
 
         case STDOUT_STREAM_FORWARD_TO_SYSLOG:
                 r = parse_boolean(p);
-                if (r < 0) {
-                        log_warning("Failed to parse forward to syslog line.");
-                        return -EINVAL;
-                }
+                if (r < 0)
+                        return log_warning_errno(r, "Failed to parse forward to syslog line: %m");
 
                 s->forward_to_syslog = r;
                 s->state = STDOUT_STREAM_FORWARD_TO_KMSG;
@@ -407,10 +421,8 @@ static int stdout_stream_line(StdoutStream *s, char *p, LineBreak line_break) {
 
         case STDOUT_STREAM_FORWARD_TO_KMSG:
                 r = parse_boolean(p);
-                if (r < 0) {
-                        log_warning("Failed to parse copy to kmsg line.");
-                        return -EINVAL;
-                }
+                if (r < 0)
+                        return log_warning_errno(r, "Failed to parse copy to kmsg line: %m");
 
                 s->forward_to_kmsg = r;
                 s->state = STDOUT_STREAM_FORWARD_TO_CONSOLE;
@@ -418,10 +430,8 @@ static int stdout_stream_line(StdoutStream *s, char *p, LineBreak line_break) {
 
         case STDOUT_STREAM_FORWARD_TO_CONSOLE:
                 r = parse_boolean(p);
-                if (r < 0) {
-                        log_warning("Failed to parse copy to console line.");
-                        return -EINVAL;
-                }
+                if (r < 0)
+                        return log_warning_errno(r, "Failed to parse copy to console line.");
 
                 s->forward_to_console = r;
                 s->state = STDOUT_STREAM_RUNNING;
@@ -750,7 +760,7 @@ static int stdout_stream_load(StdoutStream *stream, const char *fname) {
         if (priority) {
                 int p;
 
-                p = log_level_from_string(priority);
+                p = syslog_parse_priority_and_facility(priority);
                 if (p >= 0)
                         stream->priority = p;
         }

src/libsystemd-network/dhcp6-option.c

@@ -257,9 +257,9 @@ int dhcp6_option_append_pd(uint8_t **buf, size_t *buflen, const DHCP6IA *pd, con
                 len += r;
         }
 
-        if (hint_pd_prefix) {
+        if (hint_pd_prefix && hint_pd_prefix->iapdprefix.prefixlen > 0) {
                 r = option_append_pd_prefix(buf, buflen, hint_pd_prefix);
-                if (r < 0 && r != -EINVAL)
+                if (r < 0)
                         return r;
 
                 len += r;

src/resolve/resolved-dns-answer.c

@@ -879,9 +879,8 @@ void dns_answer_dump(DnsAnswer *answer, FILE *f) {
                 }
 
                 fputs(t, f);
-
-                if (item->ifindex != 0 || item->rrsig || item->flags != 0)
                 fputs("\t;", f);
+                fprintf(f, " ttl=%" PRIu32, item->rr->ttl);
 
                 if (item->ifindex != 0)
                         fprintf(f, " ifindex=%i", item->ifindex);
@@ -963,3 +962,22 @@ void dns_answer_randomize(DnsAnswer *a) {
                 SWAP_TWO(a->items[i], a->items[k]);
         }
 }
+
+uint32_t dns_answer_min_ttl(DnsAnswer *a) {
+        uint32_t ttl = UINT32_MAX;
+        DnsResourceRecord *rr;
+
+        /* Return the smallest TTL of all RRs in this answer */
+
+        DNS_ANSWER_FOREACH(rr, a) {
+                /* Don't consider OPT (where the TTL field is used for other purposes than an actual TTL) */
+
+                if (dns_type_is_pseudo(rr->key->type) ||
+                    dns_class_is_pseudo(rr->key->class))
+                        continue;
+
+                ttl = MIN(ttl, rr->ttl);
+        }
+
+        return ttl;
+}

src/resolve/resolved-dns-answer.h

@@ -87,6 +87,8 @@ void dns_answer_dump(DnsAnswer *answer, FILE *f);
 
 void dns_answer_randomize(DnsAnswer *a);
 
+uint32_t dns_answer_min_ttl(DnsAnswer *a);
+
 DEFINE_TRIVIAL_CLEANUP_FUNC(DnsAnswer*, dns_answer_unref);
 
 #define _DNS_ANSWER_FOREACH(q, kk, a)                                   \

src/resolve/resolved-dns-cache.c

@@ -312,19 +312,23 @@ static DnsCacheItem* dns_cache_get(DnsCache *c, DnsResourceRecord *rr) {
         return NULL;
 }
 
-static usec_t calculate_until(DnsResourceRecord *rr, uint32_t nsec_ttl, usec_t timestamp, bool use_soa_minimum) {
+static usec_t calculate_until(
+                DnsResourceRecord *rr,
+                uint32_t min_ttl,
+                uint32_t nsec_ttl,
+                usec_t timestamp,
+                bool use_soa_minimum) {
+
         uint32_t ttl;
         usec_t u;
 
         assert(rr);
 
-        ttl = MIN(rr->ttl, nsec_ttl);
+        ttl = MIN(min_ttl, nsec_ttl);
         if (rr->key->type == DNS_TYPE_SOA && use_soa_minimum) {
-                /* If this is a SOA RR, and it is requested, clamp to
-                 * the SOA's minimum field. This is used when we do
-                 * negative caching, to determine the TTL for the
-                 * negative caching entry.  See RFC 2308, Section
-                 * 5. */
+                /* If this is a SOA RR, and it is requested, clamp to the SOA's minimum field. This is used
+                 * when we do negative caching, to determine the TTL for the negative caching entry. See RFC
+                 * 2308, Section 5. */
 
                 if (ttl > rr->soa.minimum)
                         ttl = rr->soa.minimum;
@@ -337,8 +341,7 @@ static usec_t calculate_until(DnsResourceRecord *rr, uint32_t nsec_ttl, usec_t t
         if (rr->expiry != USEC_INFINITY) {
                 usec_t left;
 
-                /* Make use of the DNSSEC RRSIG expiry info, if we
-                 * have it */
+                /* Make use of the DNSSEC RRSIG expiry info, if we have it */
 
                 left = LESS_BY(rr->expiry, now(CLOCK_REALTIME));
                 if (u > left)
@@ -354,6 +357,7 @@ static void dns_cache_item_update_positive(
                 DnsResourceRecord *rr,
                 DnsAnswer *answer,
                 DnsPacket *full_packet,
+                uint32_t min_ttl,
                 uint64_t query_flags,
                 bool shared_owner,
                 DnssecResult dnssec_result,
@@ -390,7 +394,7 @@ static void dns_cache_item_update_positive(
         dns_packet_unref(i->full_packet);
         i->full_packet = full_packet;
 
-        i->until = calculate_until(rr, UINT32_MAX, timestamp, false);
+        i->until = calculate_until(rr, min_ttl, UINT32_MAX, timestamp, false);
         i->query_flags = query_flags & CACHEABLE_QUERY_FLAGS;
         i->shared_owner = shared_owner;
         i->dnssec_result = dnssec_result;
@@ -417,9 +421,10 @@ static int dns_cache_put_positive(
                 const union in_addr_union *owner_address) {
 
         _cleanup_(dns_cache_item_freep) DnsCacheItem *i = NULL;
-        DnsCacheItem *existing;
         char key_str[DNS_RESOURCE_KEY_STRING_MAX];
-        int r, k;
+        DnsCacheItem *existing;
+        uint32_t min_ttl;
+        int r;
 
         assert(c);
         assert(rr);
@@ -431,11 +436,18 @@ static int dns_cache_put_positive(
         if (dns_type_is_pseudo(rr->key->type))
                 return 0;
 
+        /* Determine the minimal TTL of all RRs in the answer plus the one by the main RR we are supposed to
+         * cache. Since we cache whole answers to questions we should never return answers where only some
+         * RRs are still valid, hence find the lowest here */
+        min_ttl = dns_answer_min_ttl(answer);
+        if (rr)
+                min_ttl = MIN(min_ttl, rr->ttl);
+
         /* New TTL is 0? Delete this specific entry... */
-        if (rr->ttl <= 0) {
-                k = dns_cache_remove_by_rr(c, rr);
+        if (min_ttl <= 0) {
+                r = dns_cache_remove_by_rr(c, rr);
                 log_debug("%s: %s",
-                          k > 0 ? "Removed zero TTL entry from cache" : "Not caching zero TTL cache entry",
+                          r > 0 ? "Removed zero TTL entry from cache" : "Not caching zero TTL cache entry",
                           dns_resource_key_to_string(rr->key, key_str, sizeof key_str));
                 return 0;
         }
@@ -449,6 +461,7 @@ static int dns_cache_put_positive(
                                 rr,
                                 answer,
                                 full_packet,
+                                min_ttl,
                                 query_flags,
                                 shared_owner,
                                 dnssec_result,
@@ -476,7 +489,7 @@ static int dns_cache_put_positive(
                 .rr = dns_resource_record_ref(rr),
                 .answer = dns_answer_ref(answer),
                 .full_packet = dns_packet_ref(full_packet),
-                .until = calculate_until(rr, UINT32_MAX, timestamp, false),
+                .until = calculate_until(rr, min_ttl, UINT32_MAX, timestamp, false),
                 .query_flags = query_flags & CACHEABLE_QUERY_FLAGS,
                 .shared_owner = shared_owner,
                 .dnssec_result = dnssec_result,
@@ -578,9 +591,12 @@ static int dns_cache_put_negative(
                 .full_packet = dns_packet_ref(full_packet),
         };
 
+        /* Determine how long to cache this entry. In case we have some RRs in the answer use the lowest TTL
+         * of any of them. Typically that's the SOA's TTL, which is OK, but could possibly be lower because
+         * of some other RR. Let's better take the lowest option here than a needlessly high one */
         i->until =
                 i->type == DNS_CACHE_RCODE ? timestamp + CACHE_TTL_STRANGE_RCODE_USEC :
-                calculate_until(soa, nsec_ttl, timestamp, true);
+                calculate_until(soa, dns_answer_min_ttl(answer), nsec_ttl, timestamp, true);
 
         if (i->type == DNS_CACHE_NXDOMAIN) {
                 /* NXDOMAIN entries should apply equally to all types, so we use ANY as
@@ -696,7 +712,7 @@ int dns_cache_put(
          * short time.) */
 
         if (IN_SET(rcode, DNS_RCODE_SUCCESS, DNS_RCODE_NXDOMAIN)) {
-                if (dns_answer_size(answer) <= 0) {
+                if (dns_answer_isempty(answer)) {
                         if (key) {
                                 char key_str[DNS_RESOURCE_KEY_STRING_MAX];
 
@@ -785,9 +801,8 @@ int dns_cache_put(
         if (r > 0)
                 return 0;
 
-        /* But not if it has a matching CNAME/DNAME (the negative
-         * caching will be done on the canonical name, not on the
-         * alias) */
+        /* But not if it has a matching CNAME/DNAME (the negative caching will be done on the canonical name,
+         * not on the alias) */
         r = dns_answer_find_cname_or_dname(answer, key, NULL, NULL);
         if (r < 0)
                 goto fail;
@@ -803,8 +818,7 @@ int dns_cache_put(
         if (r == 0 && !weird_rcode)
                 return 0;
         if (r > 0) {
-                /* Refuse using the SOA data if it is unsigned, but the key is
-                 * signed */
+                /* Refuse using the SOA data if it is unsigned, but the key is signed */
                 if (FLAGS_SET(query_flags, SD_RESOLVED_AUTHENTICATED) &&
                     (flags & DNS_ANSWER_AUTHENTICATED) == 0)
                         return 0;
@@ -923,9 +937,18 @@ static int answer_add_clamp_ttl(
         assert(rr);
 
         if (FLAGS_SET(query_flags, SD_RESOLVED_CLAMP_TTL)) {
+                uint32_t left_ttl;
+
+                /* Let's determine how much time is left for this cache entry. Note that we round down, but
+                 * clamp this to be 1s at minimum, since we usually want records to remain cached better too
+                 * short a time than too long a time, but otoh don't want to return 0 ever, since that has
+                 * special semantics in various contexts — in particular in mDNS */
+
+                left_ttl = MAX(1U, LESS_BY(until, current) / USEC_PER_SEC);
+
                 patched = dns_resource_record_ref(rr);
 
-                r = dns_resource_record_clamp_ttl(&patched, LESS_BY(until, current) / USEC_PER_SEC);
+                r = dns_resource_record_clamp_ttl(&patched, left_ttl);
                 if (r < 0)
                         return r;
 
@@ -933,7 +956,7 @@ static int answer_add_clamp_ttl(
 
                 if (rrsig) {
                         patched_rrsig = dns_resource_record_ref(rrsig);
-                        r = dns_resource_record_clamp_ttl(&patched_rrsig, LESS_BY(until, current) / USEC_PER_SEC);
+                        r = dns_resource_record_clamp_ttl(&patched_rrsig, left_ttl);
                         if (r < 0)
                                 return r;
 
@@ -1051,14 +1074,23 @@ int dns_cache_lookup(
                                 DnsAnswerItem *item;
 
                                 DNS_ANSWER_FOREACH_ITEM(item, j->answer) {
-                                        r = answer_add_clamp_ttl(&answer, item->rr, item->ifindex, item->flags, item->rrsig, query_flags, j->until, current);
+                                        r = answer_add_clamp_ttl(
+                                                        &answer,
+                                                        item->rr,
+                                                        item->ifindex,
+                                                        item->flags,
+                                                        item->rrsig,
+                                                        query_flags,
+                                                        j->until,
+                                                        current);
                                         if (r < 0)
                                                 return r;
                                 }
                         }
 
                 } else if (j->rr) {
-                        r = answer_add_clamp_ttl(&answer,
+                        r = answer_add_clamp_ttl(
+                                        &answer,
                                         j->rr,
                                         j->ifindex,
                                         FLAGS_SET(j->query_flags, SD_RESOLVED_AUTHENTICATED) ? DNS_ANSWER_AUTHENTICATED : 0,

src/resolve/resolved-dns-query.c

@@ -1019,7 +1019,9 @@ static int dns_query_cname_redirect(DnsQuery *q, const DnsResourceRecord *cname)
         q->question_utf8 = TAKE_PTR(nq_utf8);
 
         dns_query_unref_candidates(q);
-        dns_query_reset_answer(q);
+
+        /* Note that we do *not* reset the answer here, because the answer we previously got might already
+         * include everything we need, let's check that first */
 
         q->state = DNS_TRANSACTION_NULL;
 
@@ -1069,8 +1071,7 @@ int dns_query_process_cname(DnsQuery *q) {
         if (r < 0)
                 return r;
 
-        /* Let's see if the answer can already answer the new
-         * redirected question */
+        /* Let's see if the answer can already answer the new redirected question */
         r = dns_query_process_cname(q);
         if (r != DNS_QUERY_NOMATCH)
                 return r;

src/resolve/resolved-dns-question.c

@@ -445,3 +445,21 @@ int dns_question_new_service(
 
         return 0;
 }
+
+/*
+ * This function is not used in the code base, but is useful when debugging. Do not delete.
+ */
+void dns_question_dump(DnsQuestion *question, FILE *f) {
+        DnsResourceKey *k;
+
+        if (!f)
+                f = stdout;
+
+        DNS_QUESTION_FOREACH(k, question) {
+                char buf[DNS_RESOURCE_KEY_STRING_MAX];
+
+                fputc('\t', f);
+                fputs(dns_resource_key_to_string(k, buf, sizeof(buf)), f);
+                fputc('\n', f);
+        }
+}

src/resolve/resolved-dns-question.h

@@ -33,6 +33,8 @@ int dns_question_is_equal(DnsQuestion *a, DnsQuestion *b);
 
 int dns_question_cname_redirect(DnsQuestion *q, const DnsResourceRecord *cname, DnsQuestion **ret);
 
+void dns_question_dump(DnsQuestion *q, FILE *f);
+
 const char *dns_question_first_name(DnsQuestion *q);
 
 static inline size_t dns_question_size(DnsQuestion *q) {

src/resolve/resolved-dns-stub.c

@@ -275,7 +275,7 @@ static int dns_stub_collect_answer_by_section(
                     dns_type_is_dnssec(item->rr->key->type))
                         continue;
 
-                if (((item->flags ^ section) & (DNS_ANSWER_SECTION_ANSWER|DNS_ANSWER_SECTION_AUTHORITY|DNS_ANSWER_SECTION_ADDITIONAL)) != 0)
+                if (((item->flags ^ section) & DNS_ANSWER_MASK_SECTIONS) != 0)
                         continue;
 
                 r = reply_add_with_rrsig(
@@ -761,7 +761,7 @@ static void dns_stub_query_complete(DnsQuery *q) {
          * and keep adding all RRs in the CNAME chain. */
         r = dns_stub_assign_sections(
                         q,
-                        q->request_packet->question,
+                        dns_query_question_for_protocol(q, DNS_PROTOCOL_DNS),
                         dns_stub_reply_with_edns0_do(q));
         if (r < 0) {
                 log_debug_errno(r, "Failed to assign sections: %m");