]> git.ipfire.org Git - thirdparty/systemd.git/blobdiff - src/resolve/resolved-dns-transaction.c
resolved: add missing error code check when initializing DNS-over-TLS
[thirdparty/systemd.git] / src / resolve / resolved-dns-transaction.c
index fbc4735855f6968af45301acd877033a17ec0b5d..2ef02348064bf8cf9488a12d9b8fa2373f330869 100644 (file)
@@ -6,6 +6,7 @@
 #include "alloc-util.h"
 #include "dns-domain.h"
 #include "errno-list.h"
+#include "errno-util.h"
 #include "fd-util.h"
 #include "random-util.h"
 #include "resolved-dns-cache.h"
@@ -267,7 +268,7 @@ static void dns_transaction_tentative(DnsTransaction *t, DnsPacket *p) {
                   t->id,
                   dns_resource_key_to_string(t->key, key_str, sizeof key_str),
                   dns_protocol_to_string(t->scope->protocol),
-                  t->scope->link ? t->scope->link->name : "*",
+                  t->scope->link ? t->scope->link->ifname : "*",
                   af_to_name_short(t->scope->family),
                   strnull(pretty));
 
@@ -332,7 +333,7 @@ void dns_transaction_complete(DnsTransaction *t, DnsTransactionState state) {
                   t->id,
                   dns_resource_key_to_string(t->key, key_str, sizeof key_str),
                   dns_protocol_to_string(t->scope->protocol),
-                  t->scope->link ? t->scope->link->name : "*",
+                  t->scope->link ? t->scope->link->ifname : "*",
                   af_to_name_short(t->scope->family),
                   st,
                   t->answer_source < 0 ? "none" : dns_transaction_source_to_string(t->answer_source),
@@ -503,65 +504,57 @@ static int dns_transaction_on_stream_packet(DnsTransaction *t, DnsPacket *p) {
 }
 
 static int on_stream_complete(DnsStream *s, int error) {
-        _cleanup_(dns_stream_unrefp) DnsStream *p = NULL;
-        DnsTransaction *t, *n;
-        int r = 0;
-
-        /* Do not let new transactions use this stream */
-        if (s->server && s->server->stream == s)
-                p = TAKE_PTR(s->server->stream);
+        assert(s);
 
         if (ERRNO_IS_DISCONNECT(error) && s->protocol != DNS_PROTOCOL_LLMNR) {
-                usec_t usec;
-
                 log_debug_errno(error, "Connection failure for DNS TCP stream: %m");
 
                 if (s->transactions) {
+                        DnsTransaction *t;
+
                         t = s->transactions;
-                        assert_se(sd_event_now(t->scope->manager->event, clock_boottime_or_monotonic(), &usec) >= 0);
                         dns_server_packet_lost(t->server, IPPROTO_TCP, t->current_feature_level);
                 }
         }
 
-        LIST_FOREACH_SAFE(transactions_by_stream, t, n, s->transactions)
-                if (error != 0)
+        if (error != 0) {
+                DnsTransaction *t, *n;
+
+                LIST_FOREACH_SAFE(transactions_by_stream, t, n, s->transactions)
                         on_transaction_stream_error(t, error);
-                else if (DNS_PACKET_ID(s->read_packet) == t->id)
-                        /* As each transaction have a unique id the return code is only set once */
-                        r = dns_transaction_on_stream_packet(t, s->read_packet);
+        }
 
-        return r;
+        return 0;
 }
 
-static int dns_stream_on_packet(DnsStream *s) {
+static int on_stream_packet(DnsStream *s) {
         _cleanup_(dns_packet_unrefp) DnsPacket *p = NULL;
-        int r = 0;
         DnsTransaction *t;
 
+        assert(s);
+
         /* Take ownership of packet to be able to receive new packets */
-        p = TAKE_PTR(s->read_packet);
-        s->n_read = 0;
+        p = dns_stream_take_read_packet(s);
+        assert(p);
 
         t = hashmap_get(s->manager->dns_transactions, UINT_TO_PTR(DNS_PACKET_ID(p)));
-
-        /* Ignore incorrect transaction id as transaction can have been canceled */
         if (t)
-                r = dns_transaction_on_stream_packet(t, p);
-        else {
-                if (dns_packet_validate_reply(p) <= 0) {
-                        log_debug("Invalid TCP reply packet.");
-                        on_stream_complete(s, 0);
-                }
-                return 0;
-        }
+                return dns_transaction_on_stream_packet(t, p);
 
-        return r;
+        /* Ignore incorrect transaction id as an old transaction can have been canceled. */
+        log_debug("Received unexpected TCP reply packet with id %" PRIu16 ", ignoring.", DNS_PACKET_ID(p));
+        return 0;
+}
+
+static uint16_t dns_port_for_feature_level(DnsServerFeatureLevel level) {
+        return DNS_SERVER_FEATURE_LEVEL_IS_TLS(level) ? 853 : 53;
 }
 
 static int dns_transaction_emit_tcp(DnsTransaction *t) {
-        _cleanup_close_ int fd = -1;
         _cleanup_(dns_stream_unrefp) DnsStream *s = NULL;
+        _cleanup_close_ int fd = -1;
         union sockaddr_union sa;
+        DnsStreamType type;
         int r;
 
         assert(t);
@@ -585,8 +578,9 @@ static int dns_transaction_emit_tcp(DnsTransaction *t) {
                 if (t->server->stream && (DNS_SERVER_FEATURE_LEVEL_IS_TLS(t->current_feature_level) == t->server->stream->encrypted))
                         s = dns_stream_ref(t->server->stream);
                 else
-                        fd = dns_scope_socket_tcp(t->scope, AF_UNSPEC, NULL, t->server, DNS_SERVER_FEATURE_LEVEL_IS_TLS(t->current_feature_level) ? 853 : 53, &sa);
+                        fd = dns_scope_socket_tcp(t->scope, AF_UNSPEC, NULL, t->server, dns_port_for_feature_level(t->current_feature_level), &sa);
 
+                type = DNS_STREAM_LOOKUP;
                 break;
 
         case DNS_PROTOCOL_LLMNR:
@@ -612,6 +606,7 @@ static int dns_transaction_emit_tcp(DnsTransaction *t) {
                         fd = dns_scope_socket_tcp(t->scope, family, &address, NULL, LLMNR_PORT, &sa);
                 }
 
+                type = DNS_STREAM_LLMNR_SEND;
                 break;
 
         default:
@@ -622,14 +617,16 @@ static int dns_transaction_emit_tcp(DnsTransaction *t) {
                 if (fd < 0)
                         return fd;
 
-                r = dns_stream_new(t->scope->manager, &s, t->scope->protocol, fd, &sa);
+                r = dns_stream_new(t->scope->manager, &s, type, t->scope->protocol, fd, &sa);
                 if (r < 0)
                         return r;
 
                 fd = -1;
 
 #if ENABLE_DNS_OVER_TLS
-                if (DNS_SERVER_FEATURE_LEVEL_IS_TLS(t->current_feature_level)) {
+                if (t->scope->protocol == DNS_PROTOCOL_DNS &&
+                    DNS_SERVER_FEATURE_LEVEL_IS_TLS(t->current_feature_level)) {
+
                         assert(t->server);
                         r = dnstls_stream_connect_tls(s, t->server);
                         if (r < 0)
@@ -638,13 +635,13 @@ static int dns_transaction_emit_tcp(DnsTransaction *t) {
 #endif
 
                 if (t->server) {
-                        dns_stream_unref(t->server->stream);
-                        t->server->stream = dns_stream_ref(s);
+                        dns_server_unref_stream(t->server);
                         s->server = dns_server_ref(t->server);
+                        t->server->stream = dns_stream_ref(s);
                 }
 
                 s->complete = on_stream_complete;
-                s->on_packet = dns_stream_on_packet;
+                s->on_packet = on_stream_packet;
 
                 /* The interface index is difficult to determine if we are
                  * connecting to the local host, hence fill this in right away
@@ -737,7 +734,7 @@ static int dns_transaction_dnssec_ready(DnsTransaction *t) {
                         }
 
                         /* Fall-through: NXDOMAIN/SERVFAIL is good enough for us. This is because some DNS servers
-                         * erronously return NXDOMAIN/SERVFAIL for empty non-terminals (Akamai...) or missing DS
+                         * erroneously return NXDOMAIN/SERVFAIL for empty non-terminals (Akamai...) or missing DS
                          * records (Facebook), and we need to handle that nicely, when asking for parent SOA or similar
                          * RRs to make unsigned proofs. */
 
@@ -1505,7 +1502,7 @@ static int dns_transaction_make_packet_mdns(DnsTransaction *t) {
         /*
          * For mDNS, we want to coalesce as many open queries in pending transactions into one single
          * query packet on the wire as possible. To achieve that, we iterate through all pending transactions
-         * in our current scope, and see whether their timing contraints allow them to be sent.
+         * in our current scope, and see whether their timing constraints allow them to be sent.
          */
 
         assert_se(sd_event_now(t->scope->manager->event, clock_boottime_or_monotonic(), &ts) >= 0);
@@ -1651,7 +1648,7 @@ int dns_transaction_go(DnsTransaction *t) {
                   t->id,
                   dns_resource_key_to_string(t->key, key_str, sizeof key_str),
                   dns_protocol_to_string(t->scope->protocol),
-                  t->scope->link ? t->scope->link->name : "*",
+                  t->scope->link ? t->scope->link->ifname : "*",
                   af_to_name_short(t->scope->family));
 
         if (!t->initial_jitter_scheduled &&
@@ -1816,13 +1813,12 @@ static int dns_transaction_add_dnssec_transaction(DnsTransaction *t, DnsResource
                 if (r > 0) {
                         char s[DNS_RESOURCE_KEY_STRING_MAX], saux[DNS_RESOURCE_KEY_STRING_MAX];
 
-                        log_debug("Potential cyclic dependency, refusing to add transaction %" PRIu16 " (%s) as dependency for %" PRIu16 " (%s).",
-                                  aux->id,
-                                  dns_resource_key_to_string(t->key, s, sizeof s),
-                                  t->id,
-                                  dns_resource_key_to_string(aux->key, saux, sizeof saux));
-
-                        return -ELOOP;
+                        return log_debug_errno(SYNTHETIC_ERRNO(ELOOP),
+                                               "Potential cyclic dependency, refusing to add transaction %" PRIu16 " (%s) as dependency for %" PRIu16 " (%s).",
+                                               aux->id,
+                                               dns_resource_key_to_string(t->key, s, sizeof s),
+                                               t->id,
+                                               dns_resource_key_to_string(aux->key, saux, sizeof saux));
                 }
         }
 
@@ -2030,7 +2026,7 @@ int dns_transaction_request_dnssec_keys(DnsTransaction *t) {
         if (t->answer_source != DNS_TRANSACTION_NETWORK)
                 return 0; /* We only need to validate stuff from the network */
         if (!dns_transaction_dnssec_supported(t))
-                return 0; /* If we can't do DNSSEC anyway there's no point in geting the auxiliary RRs */
+                return 0; /* If we can't do DNSSEC anyway there's no point in getting the auxiliary RRs */
 
         DNS_ANSWER_FOREACH(rr, t->answer) {
 
@@ -2070,7 +2066,7 @@ int dns_transaction_request_dnssec_keys(DnsTransaction *t) {
                          * RRs for stuff we didn't really ask for, and
                          * also to avoid request loops, where
                          * additional RRs from one transaction result
-                         * in another transaction whose additonal RRs
+                         * in another transaction whose additional RRs
                          * point back to the original transaction, and
                          * we deadlock. */
                         r = dns_name_endswith(dns_resource_key_name(t->key), rr->rrsig.signer);
@@ -2146,6 +2142,14 @@ int dns_transaction_request_dnssec_keys(DnsTransaction *t) {
                                 if (r > 0) /* positive reply, we won't need the SOA and hence don't need to validate
                                             * it. */
                                         continue;
+
+                                /* Only bother with this if the SOA/NS RR we are looking at is actually a parent of
+                                 * what we are looking for, otherwise there's no value in it for us. */
+                                r = dns_name_endswith(dns_resource_key_name(t->key), dns_resource_key_name(rr->key));
+                                if (r < 0)
+                                        return r;
+                                if (r == 0)
+                                        continue;
                         }
 
                         r = dnssec_has_rrsig(t->answer, rr->key);
@@ -2280,21 +2284,21 @@ int dns_transaction_request_dnssec_keys(DnsTransaction *t) {
                         r = dns_name_parent(&name);
                         if (r > 0) {
                                 type = DNS_TYPE_SOA;
-                                log_debug("Requesting parent SOA to validate transaction %" PRIu16 " (%s, unsigned empty DS response).",
-                                          t->id, dns_resource_key_name(t->key));
+                                log_debug("Requesting parent SOA (→ %s) to validate transaction %" PRIu16 " (%s, unsigned empty DS response).",
+                                          name, t->id, dns_resource_key_name(t->key));
                         } else
                                 name = NULL;
 
                 } else if (IN_SET(t->key->type, DNS_TYPE_SOA, DNS_TYPE_NS)) {
 
                         type = DNS_TYPE_DS;
-                        log_debug("Requesting DS to validate transaction %" PRIu16 " (%s, unsigned empty SOA/NS response).",
-                                  t->id, dns_resource_key_name(t->key));
+                        log_debug("Requesting DS (→ %s) to validate transaction %" PRIu16 " (%s, unsigned empty SOA/NS response).",
+                                  name, t->id, name);
 
                 } else {
                         type = DNS_TYPE_SOA;
-                        log_debug("Requesting SOA to validate transaction %" PRIu16 " (%s, unsigned empty non-SOA/NS/DS response).",
-                                  t->id, dns_resource_key_name(t->key));
+                        log_debug("Requesting SOA (→ %s) to validate transaction %" PRIu16 " (%s, unsigned empty non-SOA/NS/DS response).",
+                                  name, t->id, name);
                 }
 
                 if (name) {
@@ -2445,8 +2449,8 @@ static int dns_transaction_requires_rrsig(DnsTransaction *t, DnsResourceRecord *
                                                 return true;
 
                                         /* A CNAME/DNAME without a parent? That's sooo weird. */
-                                        log_debug("Transaction %" PRIu16 " claims CNAME/DNAME at root. Refusing.", t->id);
-                                        return -EBADMSG;
+                                        return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG),
+                                                               "Transaction %" PRIu16 " claims CNAME/DNAME at root. Refusing.", t->id);
                                 }
                         }