Commit Graph
382 Commits
Author SHA1 Message Date
Ronan Pigott afdb38a39f resolved: validate noerror response for CNAMEs
CNAME doesn't exist at the zone apex. When we get an unsigned noerror
response to a direct query for a CNAME record, we don't yet know if this
name is zone apex. We already request the correct DS record in this
case, but previously skipped it at validation time, causing the answer
to appear bogus. Make sure to also consider the DS record for the query
name for negative replies.
2024-10-22 17:59:05 +02:00
Ronan Pigott fa02d04ee9 resolved: update condition for caching full packets
Previously a full packet was cached only if the CD bit was set, but this
no longer corresponds to the cases where bypass is enabled.

Update the cache to retain a full packet in the cases where it might
actually be useful.
2024-10-18 21:21:35 -07:00
Ronan Pigott ddd710a355 resolved: don't treat conn reset as packet loss
tcp reset / icmp port-unreachable are markedly different conditions than
packet loss. It doesn't make much sense to retry in this case. It's
actually not clear if there is any benefit at all retrying tcp
connections, which were presumably already retried as necessary by the
tcp stack.
2024-08-02 11:29:25 +09:00
Ronan Pigott cd2ce31adb resolved: Don't retry queries that indicate net error
This probably rarely helped anyway, but it also in some cases interferes
with auxiliary dnssec queries where the authoritative nameserver does
not support EDNS0/DNSSEC.

Fixes: ac6844460c ("resolved: support RFC 8914 EDE error codes")
2024-07-26 10:04:36 +01:00
Ronan Pigott 414a9b8e5e resolved: validate authentic insecure delegation to CNAME
If the parent zone uses a non-opt-out method that provides authenticated
negative DS replies, we still can't expect signatures from the child
zone. sd-resolved was using the authenticated status of the DS reply to
require signatures for CNAMEs, even though it had already proved that no
signature exists.

Fixes: 47690634f1 ("resolved: don't request the SOA for every dns label")
2024-05-01 20:34:08 +02:00
Ronan Pigott 5237ffdf2b resolved: probe for dnssec support in allow-downgrade mode
Previously, sd-resolved unnecessarily requested SOA records for each dns
label in the query, even though they are not needed for the chain of
trust. Since 47690634f1, only the necessary records are queried when
validating.

This is actually a problem in allow-downgrade mode, since we will no
longer attempt a query for a record that we know is signed a priori, and
will therefore never update our belief about the state of dnssec support
in the recursive resolver.

Rectify this by reintroducing a query for the root zone SOA in the
allow-downgrade case, specifically to test that the resolver attaches
the RRSIGs which we know must exist.

Fixes: 47690634f1 ("resolved: don't request the SOA for every dns label")
2024-05-01 20:32:30 +02:00
Ronan Pigott d840783db5 resolved: always progress DS queries
If we request a DS and the resolver offers an unsigned SOA, a new
auxiliary transaction for the DS will be rejected as a loop, and we
might not make any progress toward finding the DS we need. Let's ensure
that we at least always check the parent in this case.

Fixes: 47690634f1 ("resolved: don't request the SOA for every dns label")
2024-04-29 19:04:11 +02:00
Ronan Pigott 3a768db480 resolved: use NULL to represent an empty answer 2024-04-12 06:10:51 +09:00
Ronan Pigott 73f77f38cf resolve: really always initialize aux
dns_transaction_request_dnssec_rr was already adjusted in 4001710365,
to allow for the return parameter to be passed uninitialized. However
this codepath was missed, meaning this function could sometimes return
success without having actually set the parameter.

Fixes: 4001710365 ("resolved: minor dnssec fixups")
Fixes: 47690634f1 ("resolved: don't request the SOA for every dns label")
2024-04-09 09:39:49 +02:00
Ronan Pigott 0dfea62ee3 resolved: always return the validated answers when validating
We normally expect sd-resolved only to return the validated subset of a
validated response. In some cases we give up on validating, because we
have enough information already to conclude the answer is bogus.

Let's be sure to always reply with only the validated subset in these
cases too, so that we don't return bogus answers and confuse primitive
clients that won't see the SERVFAIL rcode.
2024-03-27 10:17:15 +09:00
Ronan Pigott 4001710365 resolved: minor dnssec fixups
Fixes: ce5b9d5b3c ("resolved: request DS with DNSKEY")
2024-03-27 10:16:38 +09:00
Yu Watanabe 2e097e4a91 resolve: fix typo
Follow-up for ce5b9d5b3c.
2024-03-21 03:56:36 +09:00
Yu Watanabe 2503c377fa resolve: fix typo
Follow-up for 47690634f1.
2024-03-21 03:55:37 +09:00
Ronan Pigott ce5b9d5b3c resolved: request DS with DNSKEY
When validating, when we lookup a DNSKEY for validation we will almost
certainly need the corresponding DS to complete the chain of trust.
Let's go ahead and request it right away so that we don't have to wait
in this common case.
2024-03-18 15:10:07 -07:00
Ronan Pigott 47690634f1 resolved: don't request the SOA for every dns label
When validating insecure delegations we don't actually need to request
the SOA for every single dns label. We need the DS records for the zone,
and we can seek them by querying for DS directly (in case we are at a
zone cut) and then following the SOA referrals or the parent name until
we have found a chain of trust.

Extra transactions and roundtrips, especially transactions for RRs that
aren't actually needed to validate and therefore aren't likely to be in
the recursive resolver's own cache are a big slowdown during validation.

Consequently, this change results in an enourmous speed up in validating
most names from our own cold-cache (10x or more), by eliminating a large
number of superfluous dnssec transactions.
2024-03-18 15:10:07 -07:00
Ronan Pigott ce88017255 resolved: wait to gc transactions if they might still give an answer
In some cases when a query completes there are still pending
transactions that are no longer useful to answer the query. But if this
query is repeated in the future and we don't have the answers cached,
we're going to ask and ignore the answer again.

Instead of purging these superfluous transactions, let's wait and see if
they produce an answer, since we already asked the question, and use it
to fill our cache.
2024-03-18 11:10:11 +00:00
Ronan Pigott 67d0ce8843 resolved: limit the number of signature validations in a transaction
It has been demonstrated that tolerating an unbounded number of dnssec
signature validations is a bad idea. It is easy for a maliciously
crafted DNS reply to contain as many keytag collisions as desired,
causing us to iterate every dnskey and signature combination in vain.

The solution is to impose a maximum number of validations we will
tolerate. While collisions are not hard to craft, I still expect they
are unlikely in the wild so it should be safe to pick fairly small
values.

Here two limits are imposed: one on the maximum number of invalid
signatures encountered per rrset, and another on the total number of
validations performed per transaction.
2024-02-26 01:55:57 -07:00
Yu Watanabe 3db1e6a874 resolve: on_transaction_stream_error() may free multiple transactions
Fixes #30928.
2024-01-15 10:36:14 +00:00
Yu Watanabe 37baa00fe0 resolve: also read EDE code and message from cached packet 2024-01-11 02:10:32 +09:00
Yu Watanabe a72cf22d06 resolve: introduce DNSSEC_UPSTREAM_FAILURE
and include EDE code and message in the error messages.

This replaces 9ca133e97a, and implements
originally suggested at
https://github.com/systemd/systemd/pull/30513#discussion_r1433823737
2024-01-11 02:10:32 +09:00
Yu Watanabe 41398e8793 resolve: several follow-ups for 9ca133e97a.
- add missing initialization for DnsQuery.answer_ede_rcode,

- clear EDE code and message in dns_transaction_reset_answer(),
  otherwise the previous EDE code or message may be mistakenly reused
  on restart. This fixes memory leak of DnsTransaction.answer_ede_msg.

- also clear EDE code and message in dns_query_reset_answer(),
  otherwise ede message is leaked if dns_query_accept() is called
  multiple times for the same DnsQuery.

Follow-up for 9ca133e97a.

Fixes #30752.
2024-01-11 02:10:11 +09:00
Yu Watanabe 9ae51762d7 resolve: drop DNS_TRANSACTION_UPSTREAM_DNSSEC_FAILURE
This partially reverts commit 9ca133e97a.

Not only there is no reason to introduce a new transaction failure state,
but also the commit introduces several severe issues.

Fixes #30776, #30779.
2024-01-10 13:14:14 +09:00
Yu Watanabe c08fd7beca resolve: several follow-ups for ac6844460c
- fix indentation,
- drop incorrect comment, dns_packet_ede_rcode() can handle unknown EDE rcode.
- 0 is a valid EDE rcode,
- DnsTransaction.answer_ede_msg may be NULL, so needs to use strempty()
  on logging,
- drop redundant ede_rcode temporary variable.

Follow-up for ac6844460c.
2024-01-10 13:13:11 +09:00
Yu Watanabe 71682ac6bb resolve: cleanup dns_packet_ede_rcode()
This makes the following:
- make dns_packet_ede_rcode() return -EINVAL when EDE code not found.
  Otherwise, the caller may be confused that the packet has an unknown
  error code.
- make the function escape EDE message only when non-utf8 message is received.
- the message handling logic is applied even if the error code is unknown, as
  there is no reason that we escape EDE message only when an known error code
  is received.
- reduce scope of variables,
- drop redundant 'else',
- append full stop to the log messages,
- drop redundant log message in the caller,
- split out error in the function and returned EDE error code.

Follow-up for ac6844460c.
2024-01-10 13:06:02 +09:00
Lennart Poettering 6d5743c411 Merge pull request #30513 from rpigott/resolved-ede
resolved: support RFC 8914 EDE error codes
2024-01-04 09:59:19 +01:00
Ronan Pigott 9ca133e97a resolved: add transaction result for upstream failures
This new transaction result is emitted when the upstream server
indicates a fatal error that we will not try to recover from.

Currently, it is emitted when a validating recursive resolver reports an
error validating dnssec records for a domain. The extended error message
should help give context to the admin.
2024-01-03 17:35:02 -07:00
Ronan Pigott ac6844460c resolved: support RFC 8914 EDE error codes
If the server is able to indicate an extended error to us, using a
degraded feature set is unlikely to help.
2024-01-03 17:25:07 -07:00
Ronan Pigott 3fcd83645a resolved: delay server feature detection
Some fields of the DnsPacket are not populated until we extract an
answer, like p->opt, despite being referenced by macros like
DNS_PACKET_RCODE. We can reorder some of the basic checks to follow
dns_packet_extract.
2024-01-03 17:25:07 -07:00
Yu Watanabe 73780aa8a3 resolve: DnsTransaction.scope may be NULL when dns_transaction_close_connection() is called
Follow-up for ac1b7b9e19.

Fixes CID#1533020.
2023-12-31 04:51:59 +09:00
Mike Yuan 0a0f49556f Merge pull request #30622 from yuwata/space
tree-wide: drop unnecessary space or insert missing space
2023-12-25 17:52:48 +08:00
Frantisek Sumsal ac1b7b9e19 resolve: don't add sockets to the graveyard on shutdown
Since in that case the event loop is already finished and we'd hit an
assertion:

[ 1295.993300] testsuite-75.sh[50]: + systemctl stop systemd-resolved.service
[ 1296.005152] systemd-resolved[298]: Assertion 'e->state != SD_EVENT_FINISHED' failed at src/libsystemd/sd-event/sd-event.c:1252, function sd_event_add_io(). Aborting.

Thread 1 (Thread 0x7f17d25e2940 (LWP 298)):
 #0  __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at pthread_kill.c:44
 #1  0x00007f17d16ac8a3 in __pthread_kill_internal (signo=6, threadid=<optimized out>) at pthread_kill.c:78
 #2  0x00007f17d165c668 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
 #3  0x00007f17d16444b8 in __GI_abort () at abort.c:79
 #4  0x00007f17d2402d2d in log_assert_failed (text=<optimized out>, file=<optimized out>, line=<optimized out>, func=<optimized out>) at ../build/src/basic/log.c:968
 #5  0x00007f17d240401c in log_assert_failed_return (text=text@entry=0x7f17d2533f13 "e->state != SD_EVENT_FINISHED", file=file@entry=0x7f17d25195d9 "src/libsystemd/sd-event/sd-event.c", line=line@entry=1252, func=func@entry=0x7f17d2567260 <__func__.140> "sd_event_add_io") at ../build/src/basic/log.c:987
 #6  0x00007f17d24d011a in sd_event_add_io (e=0x55e5cb497270, ret=0x55e5cb4a5120, fd=fd@entry=26, events=events@entry=1, callback=callback@entry=0x55e5caff5466 <on_io_event>, userdata=0x55e5cb4a5110) at ../build/src/libsystemd/sd-event/sd-event.c:1252
 #7  0x000055e5caff571c in manager_add_socket_to_graveyard (m=0x55e5cb43cf00, fd=26) at ../build/src/resolve/resolved-socket-graveyard.c:117
 #8  0x000055e5cafd4253 in dns_transaction_close_connection (t=t@entry=0x55e5cb57c7d0, use_graveyard=use_graveyard@entry=true) at ../build/src/resolve/resolved-dns-transaction.c:78
 #9  0x000055e5cafd8444 in dns_transaction_complete (t=t@entry=0x55e5cb57c7d0, state=state@entry=DNS_TRANSACTION_ABORTED) at ../build/src/resolve/resolved-dns-transaction.c:427
 #10 0x000055e5cafc4969 in dns_scope_abort_transactions (s=s@entry=0x55e5cb4b1a70) at ../build/src/resolve/resolved-dns-scope.c:91
 #11 0x000055e5cafc6aee in dns_scope_free (s=0x55e5cb4b1a70) at ../build/src/resolve/resolved-dns-scope.c:106
 #12 0x000055e5cafe72d1 in link_free (l=0x55e5cb4a5160) at ../build/src/resolve/resolved-link.c:94
 #13 0x000055e5cafedefc in manager_free (m=0x55e5cb43cf00) at ../build/src/resolve/resolved-manager.c:697
 #14 0x000055e5caff99b6 in manager_freep (p=p@entry=0x7ffd71fab8f8) at ../build/src/resolve/resolved-manager.h:198
 #15 0x000055e5caff9d66 in run (argc=argc@entry=1, argv=argv@entry=0x7ffd71faba78) at ../build/src/resolve/resolved.c:25
 #16 0x000055e5caff9fe3 in main (argc=1, argv=0x7ffd71faba78) at ../build/src/resolve/resolved.c:99

Resolves: #30618
2023-12-25 15:38:26 +09:00
Yu Watanabe b3a9d980f3 tree-wide: drop space between variable and an increment/decrement 2023-12-25 01:56:40 +09:00
Michal Sekletar 3b4cc1437b resolved: actually check authenticated flag of SOA transaction
Fixes #25676
2023-12-21 18:54:21 +09:00
Yu Watanabe 7645e5a813 resolve/mdns: split out mdns_make_dummy_packet()
Then, this fixes the following issues:
- if dns_packet_append_zone() for other transaction is failed with
  EMSGSIZE, the previously added key was not removed,
- if dns_transaction_prepare() for other transaction returns 0, then
  we restated the loop without dropping previously appended keys, which
  might not be necessary any more.
2023-11-08 21:38:54 +09:00
Joerg Behrmann 7227dd816f treewide: fix typos
- mostly: usecase -> use case
- continously -> continuously
- single typos in docs/FILE_DESCRIPTOR_STORE.md
2023-09-19 10:05:38 +02:00
Zbigniew Jędrzejewski-Szmek 13d84288bc tree-wide: use cocinnelle to apply _NEG_ macros 2023-08-16 12:52:56 +02:00
Kiran Vemula bc837621a3 resolved: added show-server-state verb and DumpStatistics varlink method
Added show-server-state verb to resolvectl
Added DumpStatistics and ResetStatistics  methods to varlink
2023-07-31 02:02:03 +09:00
Dmitry V. Levin 515669c460 resolved: cleanup use of ERRNO_IS_DISCONNECT()
Given that ERRNO_IS_DISCONNECT() also matches positive values,
make sure this macro is not called with arguments that do not have
errno semantics.

In this case the argument passed to ERRNO_IS_DISCONNECT() is the value
returned by dns_transaction_emit_tcp() and dns_transaction_emit_udp()
which are not expected to return any positive values, but let's be
consistent anyway and move the ERRNO_IS_DISCONNECT() invocation
to the branch where the return value is known to be negative.
2023-07-28 12:28:35 +00:00
Dmitry V. Levin 0bdea17c0a resolved: fix use of ERRNO_IS_DISCONNECT()
Given that ERRNO_IS_DISCONNECT() also matches positive values,
make sure this macro is not called with arguments that do not have
errno semantics.

In this case the argument passed to ERRNO_IS_DISCONNECT() is the value
returned by manager_recv() which can legitimately return 1 without errno
semantics, so fix this by moving ERRNO_IS_DISCONNECT() invocation to the
branch where the return value is known to be negative.
2023-07-16 10:53:30 +00:00
Joerg Behrmann 5bc9ea070f treewide: fix "an" before consonant U sounds
The article "a" goes before consonant sounds and "an" goes before vowel
sounds. This commit changes an to a for UKI, UDP, UTF-8, URL, UUID, U-Label, UI
and USB, since they start with the sound /ˌjuː/.
2023-07-06 11:59:41 +01:00
Kiran Vemula 5ed91481ab resolved: added serve stale feature implementation of RFC 8767
serve stale feature to keep the DNS resource records beyond TTL to return them as stale records in case of upstream server is not reachable or returns negative response.
SD_RESOLVED_NO_STALE flag has been added to disable serving stale records via dbus.
added serve stale test cases to TEST-75-RESOLVED
Fixes: #21815
2023-06-16 10:20:15 +01:00
Dimitri John Ledkov 4aa37ad301 Revert "resolved: address DVE-2018-0001"
DVE-2018-0001 has been fixed by the vendor, and this workaround is no longer
needed. Removal of this workaround improves performance as queries used to be
retried more than necessory.

This reverts 1ed4e584f3.
This reverts https://github.com/systemd/systemd/pull/18638

Keep .clamp_feature_level_servfail name, as imho it is more descriptive than
just .clamp_feature_level, especially if we ever need to add similar
workarounds as the one we had for DVE-2018-0001.

However note that there is another retry which was added in
8a33aa199d - seems to be working around Stubby
resolver behaviour.

Fixes: #26967
2023-05-18 12:20:12 +01:00
Joan Bruguera 3dd6336ad0 resolved: Fall back to TCP if UDP is blocked
If UDP is blocked on the system (e.g. by iptables or BPF), the kernel will
return EPERM on some or all of the system calls (connect, sendmsg, etc.).
In this case, try to fall back to TCP, which hopefully will not be blocked.
2023-02-15 20:01:39 +00:00
Zbigniew Jędrzejewski-Szmek 254d1313ae tree-wide: use -EBADF for fd initialization
-1 was used everywhere, but -EBADF or -EBADFD started being used in various
places. Let's make things consistent in the new style.

Note that there are two candidates:
EBADF 9 Bad file descriptor
EBADFD 77 File descriptor in bad state

Since we're initializating the fd, we're just assigning a value that means
"no fd yet", so it's just a bad file descriptor, and the first errno fits
better. If instead we had a valid file descriptor that became invalid because
of some operation or state change, the other errno would fit better.

In some places, initialization is dropped if unnecessary.
2022-12-19 15:00:57 +01:00
Jacek Migacz 1ca3600120 resolve: unsupported DNSSEC algorithms are considered INSECURE; not BOGUS
Resolves: #19824
2022-09-30 18:46:08 +01:00
Yu Watanabe a78049fc0e resolve: do not cache mDNS goodbye packet
Fixes #24842.
2022-09-30 18:50:56 +09:00
David Tardon 995340074e tree-wide: use ASSERT_PTR more 2022-09-13 08:13:27 +02:00
Yu Watanabe 5162b2a1c4 tree-wide: fix typo 2022-08-02 02:43:38 +09:00
Luca Boccassi bffb318491 Merge pull request #23875 from yuwata/resolve-mdns-fix-use-after-free
resolve: mdns: fix use-after-free
2022-07-27 22:57:31 +01:00
Yu Watanabe 0d609349ba resolve: introduce FORMAT_DNS_RCODE() macro
Fixes #23958.
2022-07-09 19:51:23 +09:00