| Commit message (Collapse) | Author | Age | Files | Lines |
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
In commit 2aaf6bb6e99b0f2bd73e0c49bef9e11a2844bf1a, an issue was fixed where
systemd-resolved could get stuck for multiple seconds waiting for incoming data,
since GnuTLS/OpenSSL can buffer a TLS record, so data could be available, but
no EPOLLIN event would be generated.
To fix this, a somewhat elaborate logic consisting on asking the TLS library
whether it had buffered data, then "faking" an EPOLLIN event was implemented.
However, there is a much simpler solution: Always read as much data as available
(i.e. until we get an event like EAGAIN when trying to read) from the stream
when we get an EPOLLIN event, instead of at most a single packet per event.
This approach does not require asking the TLS library whether it has buffered
data, and the logic is exactly the same for both the TCP and TLS case.
test-resolved-stream is fixed to avoid a latent double free bug.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
In the DoT case, dns_stream_writev decomposed an iovec into multiple
dnstls_stream_write calls, which resulted in multiple SSL writes and multiple
TLS records. This can be checked from a network capture, e.g. using socat:
socat -v -x openssl-listen:853,reuseaddr,fork,cert=my.cert,key=my.key,verify=0 openssl:8.8.8.8:853
Instead, propagate the iovec as-is into the DoT handling code. For GnuTLS, the
library provides support for buffering ('corking') a record. OpenSSL has no
such facility, so we join the iovec into a single buffer then call SSL_write.
socat capture of `resolvectl -4 query --cache=no example.com` before the commit:
> 2022/01/30 13:35:52.194200 length=2 from=0 to=1
00 28 .(
--
> 2022/01/30 13:35:52.194253 length=40 from=2 to=41
1e b2 01 00 00 01 00 00 00 00 00 01 07 65 78 61 .............exa
6d 70 6c 65 03 63 6f 6d 00 00 01 00 01 00 00 29 mple.com.......)
ff e4 00 00 00 00 00 00 ........
--
< 2022/01/30 13:35:52.232798 length=58 from=0 to=57
00 38 1e b2 81 80 00 01 00 01 00 00 00 01 07 65 .8.............e
78 61 6d 70 6c 65 03 63 6f 6d 00 00 01 00 01 c0 xample.com......
0c 00 01 00 01 00 00 53 6f 00 04 5d b8 d8 22 00 .......So..]..".
00 29 02 00 00 00 00 00 00 00 .)........
socat capture of `resolvectl -4 query --cache=no example.com` after the commit:
> 2022/01/30 13:34:47.598099 length=42 from=504 to=545
00 28 37 86 01 00 00 01 00 00 00 00 00 01 07 65 .(7............e
78 61 6d 70 6c 65 03 63 6f 6d 00 00 01 00 01 00 xample.com......
00 29 ff e4 00 00 00 00 00 00 .)........
--
< 2022/01/30 13:34:47.613203 length=58 from=756 to=813
00 38 37 86 81 80 00 01 00 01 00 00 00 01 07 65 .87............e
78 61 6d 70 6c 65 03 63 6f 6d 00 00 01 00 01 c0 xample.com......
0c 00 01 00 01 00 00 52 5e 00 04 5d b8 d8 22 00 .......R^..]..".
00 29 02 00 00 00 00 00 00 00 .)........
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
When sending multiple DNS questions to a DNS-over-TLS server (e.g. a question
for A and AAAA records, as is typical) on the same session, the server may
answer to each question in a separate TLS record, but it may also aggregate
multiple answers in a single TLS record.
(Some servers do this very often (e.g. Cloudflare 1.0.0.1), some do it sometimes
(e.g. Google 8.8.8.8) and some seem to never do it (e.g. Quad9 9.9.9.10)).
Both cases should be handled equivalently, as the byte stream is the same, but
when multiple answers came in a single TLS record, usually the first answer was
processed, but the second answer was entirely ignored, which caused a 10s delay
until the resolution timed out and the missing question was retried.
This can be reproduced by configuring one of the offending server and running
`resolvectl query google.com --cache=no` a few times.
To be notified of incoming data, systemd-resolved listens to `EPOLLIN` events
on the underlying socket. However, when DNS-over-TLS is used, the TLS library
(OpenSSL or GnuTLS) may read and buffer the entire TLS record when reading the
first answer, so usually no further `EPOLLIN` events will be generated, and the
second answer will never be processed.
To avoid this, if there's buffered TLS data, generate a "fake" EPOLLIN event.
This is hacky, but it makes this case transparent to the rest of the IO code.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
If the cleanup function returns the appropriate type, use that to reset the
variable. For other functions (usually the foreign ones which return void), add
an explicit value to reset to.
This causes a bit of code churn, but I think it might be worth it. In a
following patch static destructors will be called from a fuzzer, and this
change allows them to be called multiple times. But I think such a change might
help with detecting unitialized code reuse too. We hit various bugs like this,
and things are more obvious when a pointer has been set to NULL.
I was worried whether this change increases text size, but it doesn't seem to:
-Dbuildtype=debug:
before "tree-wide: return NULL from freeing functions":
-rwxrwxr-x 1 zbyszek zbyszek 4117672 Feb 16 14:36 build/libsystemd.so.0.30.0*
-rwxrwxr-x 1 zbyszek zbyszek 4494520 Feb 16 15:06 build/systemd*
after "tree-wide: return NULL from freeing functions":
-rwxrwxr-x 1 zbyszek zbyszek 4117672 Feb 16 14:36 build/libsystemd.so.0.30.0*
-rwxrwxr-x 1 zbyszek zbyszek 4494576 Feb 16 15:10 build/systemd*
now:
-rwxrwxr-x 1 zbyszek zbyszek 4117672 Feb 16 14:36 build/libsystemd.so.0.30.0*
-rwxrwxr-x 1 zbyszek zbyszek 4494640 Feb 16 15:15 build/systemd*
-Dbuildtype=release:
before "tree-wide: return NULL from freeing functions":
-rwxrwxr-x 1 zbyszek zbyszek 5252256 Feb 14 14:47 build-rawhide/libsystemd.so.0.30.0*
-rwxrwxr-x 1 zbyszek zbyszek 1834184 Feb 16 15:09 build-rawhide/systemd*
after "tree-wide: return NULL from freeing functions":
-rwxrwxr-x 1 zbyszek zbyszek 5252256 Feb 14 14:47 build-rawhide/libsystemd.so.0.30.0*
-rwxrwxr-x 1 zbyszek zbyszek 1834184 Feb 16 15:10 build-rawhide/systemd*
now:
-rwxrwxr-x 1 zbyszek zbyszek 5252256 Feb 14 14:47 build-rawhide/libsystemd.so.0.30.0*
-rwxrwxr-x 1 zbyszek zbyszek 1834184 Feb 16 15:16 build-rawhide/systemd*
I would expect that the compiler would be able to elide the setting of a
variable if the variable is never used again. And this seems to be the case:
in optimized builds there is no change in size whatsoever. And the change in
size in unoptimized build is negligible.
Something strange is happening with size of libsystemd: it's bigger in
optimized builds. Something to figure out, but unrelated to this patch.
|
| |
|
| |
|
| |
|
|
|
|
|
|
|
| |
Widely accepted certificates for IP addresses are expensive and only
affordable for larger organizations. Therefore if the user provides
the hostname in the DNS= option, we should use it instead of the IP
address.
|
|
|
|
|
|
|
|
|
| |
Some DNS providers need SNI to identify client.
This can be used by adding #name to a DNS.
Example:
[Resolve]
DNS=192.168.1.1#example.com
|
| |
|
|
|
| |
Validate the IP address in the certificate for DNS-over-TLS in strict mode when GnuTLS is used. As this is not yet the case in contrast to the documentation.
|
|
|
| |
Increase the required version to ensure TLS 1.3 is always supported when using GnuTLS for DNS-over-TLS and allow further changes to use recent API additions.
|
|
|
|
|
|
|
|
|
|
|
|
| |
Prefer TLS 1.3 before TLS 1.2 for DNS-over-TLS support, otherwise
servers compliant with RFC 8446 might end up agreeing TLS 1.2 plus a
downgrade signal which is not expected by GnuTLS clients. This manifests
in the following error:
Failed to invoke gnutls_handshake: An illegal parameter has been received.
Fixes: #13528
Fixes: v242-962-g9c0624dcdb ("resolved: support TLS 1.3 when using GnuTLS for DNS-over-TLS")
|
| |
|
|
|
|
| |
Add strict mode for DNS-over-TLS, which will require TLS support from the server. Closes #10755
|
| |
|
|
|
|
| |
Instead of having a context and/or trusted CA list per server this is now moved to the server. Ensures future TLS configuration options are global instead of per server.
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
Ideally, coccinelle would strip unnecessary braces too. But I do not see any
option in coccinelle for this, so instead, I edited the patch text using
search&replace to remove the braces. Unfortunately this is not fully automatic,
in particular it didn't deal well with if-else-if-else blocks and ifdefs, so
there is an increased likelikehood be some bugs in such spots.
I also removed part of the patch that coccinelle generated for udev, where we
returns -1 for failure. This should be fixed independently.
|
| |
|
|
|
| |
To decreae latency this add support for TFO and TLS Session Tickets. As OpenSSL wouldn't let you easily set a different function all written data is temporarily cached and therefore needs to be flushed after each SSL function which can write data.
|
|
|
|
|
|
| |
This provides basic OpenSSL support without optimizations like TCP Fast Open and TLS Session Tickets.
Notice only a single SSL library can be enabled at a time and therefore journald functions provided by GnuTLS will be disabled when using OpenSSL.
Fixes #9531
|
|
|
| |
During handshake and TLS session closing, messages needs to be exchanged. Therefore this patch overrides the requested IO events for the TCP stream when the TLS is waiting for sending or receiving of messages during theses periods. This fixes issues with correctly closing the TLS stream and prevents the handshake from hanging in rare cases (not seen yet).
|
|
This is a first step towards supporting alternative TLS implementations for DNS-over-TLS.
Co-authored-by: Filipe Brandenburger <filbranden@google.com>
|