diff options
24 files changed, 259 insertions, 196 deletions
@@ -170,7 +170,7 @@ CHANGES WITH 250: ProtectKernelLogs=yes can now be used. * The default maximum numbers of inodes have been raised from 64k to 1M - for /dev, and from 400k to 1M for /tmp. + for /dev/, and from 400k to 1M for /tmp/. * The per-user service manager learnt support for communicating with systemd-oomd to acquire OOM kill information. @@ -326,7 +326,7 @@ CHANGES WITH 250: non-essential output. It's honored by the "dot", "syscall-filter", "filesystems" commands. - * systemd-analyze security gained a --profile option that can be used + * systemd-analyze security gained a --profile= option that can be used to take into account a portable profile when analyzing portable services, since a lot of the security-related settings are enabled through them. diff --git a/man/systemd.netdev.xml b/man/systemd.netdev.xml index 449b23d5ac..ff0bdee51f 100644 --- a/man/systemd.netdev.xml +++ b/man/systemd.netdev.xml @@ -1994,7 +1994,7 @@ <term><varname>InterfaceId=</varname></term> <listitem> <para>Sets the ID/key of the xfrm interface which needs to be associated with a SA/policy. - Can be decimal or hexadecimal, valid range is 0-0xffffffff, defaults to 0.</para> + Can be decimal or hexadecimal, valid range is 1-0xffffffff. This is mandatory.</para> </listitem> </varlistentry> <varlistentry> diff --git a/src/analyze/analyze.c b/src/analyze/analyze.c index a1908ff442..456a43c8fc 100644 --- a/src/analyze/analyze.c +++ b/src/analyze/analyze.c @@ -2771,7 +2771,7 @@ static int parse_argv(int argc, char *argv[]) { if (arg_json_format_flags != JSON_FORMAT_OFF && !STRPTR_IN_SET(argv[optind], "security", "inspect-elf")) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), - "Option --json= is only supported for security right now."); + "Option --json= is only supported for security and inspect-elf right now."); if (arg_threshold != 100 && !streq_ptr(argv[optind], "security")) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), diff --git a/src/basic/util.h b/src/basic/util.h index 94804f28e3..68ae3b51e0 100644 --- a/src/basic/util.h +++ b/src/basic/util.h @@ -9,6 +9,12 @@ extern int saved_argc; extern char **saved_argv; static inline void save_argc_argv(int argc, char **argv) { + + /* Protect against CVE-2021-4034 style attacks */ + assert_se(argc > 0); + assert_se(argv); + assert_se(argv[0]); + saved_argc = argc; saved_argv = argv; } diff --git a/src/boot/efi/assert.c b/src/boot/efi/assert.c index 7a25526de2..bb16d2bf93 100644 --- a/src/boot/efi/assert.c +++ b/src/boot/efi/assert.c @@ -1,9 +1,8 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ -#ifndef NDEBUG - #include <efi.h> #include <efilib.h> + #include "util.h" void efi_assert(const char *expr, const char *file, unsigned line, const char *function) { @@ -11,5 +10,3 @@ void efi_assert(const char *expr, const char *file, unsigned line, const char *f for (;;) BS->Stall(60 * 1000 * 1000); } - -#endif diff --git a/src/boot/efi/util.h b/src/boot/efi/util.h index 40ce98c611..3d659f7043 100644 --- a/src/boot/efi/util.h +++ b/src/boot/efi/util.h @@ -27,16 +27,15 @@ #define xnew_alloc(type, n, alloc) \ ({ \ UINTN _alloc_size; \ - if (__builtin_mul_overflow(sizeof(type), (n), &_alloc_size)) \ - assert_not_reached(); \ + assert_se(!__builtin_mul_overflow(sizeof(type), (n), &_alloc_size)); \ (type *) alloc(_alloc_size); \ }) -#define xallocate_pool(size) ASSERT_PTR(AllocatePool(size)) -#define xallocate_zero_pool(size) ASSERT_PTR(AllocateZeroPool(size)) -#define xreallocate_pool(p, old_size, new_size) ASSERT_PTR(ReallocatePool((p), (old_size), (new_size))) -#define xpool_print(fmt, ...) ((CHAR16 *) ASSERT_PTR(PoolPrint((fmt), ##__VA_ARGS__))) -#define xstrdup(str) ((CHAR16 *) ASSERT_PTR(StrDuplicate(str))) +#define xallocate_pool(size) ASSERT_SE_PTR(AllocatePool(size)) +#define xallocate_zero_pool(size) ASSERT_SE_PTR(AllocateZeroPool(size)) +#define xreallocate_pool(p, old_size, new_size) ASSERT_SE_PTR(ReallocatePool((p), (old_size), (new_size))) +#define xpool_print(fmt, ...) ((CHAR16 *) ASSERT_SE_PTR(PoolPrint((fmt), ##__VA_ARGS__))) +#define xstrdup(str) ((CHAR16 *) ASSERT_SE_PTR(StrDuplicate(str))) #define xnew(type, n) xnew_alloc(type, (n), xallocate_pool) #define xnew0(type, n) xnew_alloc(type, (n), xallocate_zero_pool) diff --git a/src/boot/efi/xbootldr.c b/src/boot/efi/xbootldr.c index 49d5707e02..6fa7e49e5d 100644 --- a/src/boot/efi/xbootldr.c +++ b/src/boot/efi/xbootldr.c @@ -226,7 +226,7 @@ static EFI_STATUS find_device(EFI_HANDLE *device, EFI_DEVICE_PATH **ret_device_p } /* Patch in the data we found */ - EFI_DEVICE_PATH *xboot_path = ASSERT_PTR(DuplicateDevicePath(partition_path)); + EFI_DEVICE_PATH *xboot_path = ASSERT_SE_PTR(DuplicateDevicePath(partition_path)); CopyMem((UINT8 *) xboot_path + ((UINT8 *) part_node - (UINT8 *) partition_path), &hd, sizeof(hd)); *ret_device_path = xboot_path; return EFI_SUCCESS; diff --git a/src/core/execute.c b/src/core/execute.c index f2b58303df..380755eb09 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -1172,7 +1172,7 @@ static int setup_pam( uid_t uid, gid_t gid, const char *tty, - char ***env, + char ***env, /* updated on success */ const int fds[], size_t n_fds) { #if HAVE_PAM @@ -1183,10 +1183,11 @@ static int setup_pam( }; _cleanup_(barrier_destroy) Barrier barrier = BARRIER_NULL; + _cleanup_strv_free_ char **e = NULL; pam_handle_t *handle = NULL; sigset_t old_ss; int pam_code = PAM_SUCCESS, r; - char **nv, **e = NULL; + char **nv; bool close_session = false; pid_t pam_pid = 0, parent_pid; int flags = 0; @@ -1257,8 +1258,7 @@ static int setup_pam( goto fail; } - /* Block SIGTERM, so that we know that it won't get lost in - * the child */ + /* Block SIGTERM, so that we know that it won't get lost in the child */ assert_se(sigprocmask_many(SIG_BLOCK, &old_ss, SIGTERM, -1) >= 0); @@ -1270,18 +1270,16 @@ static int setup_pam( if (r == 0) { int sig, ret = EXIT_PAM; - /* The child's job is to reset the PAM session on - * termination */ + /* The child's job is to reset the PAM session on termination */ barrier_set_role(&barrier, BARRIER_CHILD); /* Make sure we don't keep open the passed fds in this child. We assume that otherwise only * those fds are open here that have been opened by PAM. */ (void) close_many(fds, n_fds); - /* Drop privileges - we don't need any to pam_close_session - * and this will make PR_SET_PDEATHSIG work in most cases. - * If this fails, ignore the error - but expect sd-pam threads - * to fail to exit normally */ + /* Drop privileges - we don't need any to pam_close_session and this will make + * PR_SET_PDEATHSIG work in most cases. If this fails, ignore the error - but expect sd-pam + * threads to fail to exit normally */ r = maybe_setgroups(0, NULL); if (r < 0) @@ -1293,20 +1291,16 @@ static int setup_pam( (void) ignore_signals(SIGPIPE); - /* Wait until our parent died. This will only work if - * the above setresuid() succeeds, otherwise the kernel - * will not allow unprivileged parents kill their privileged - * children this way. We rely on the control groups kill logic - * to do the rest for us. */ + /* Wait until our parent died. This will only work if the above setresuid() succeeds, + * otherwise the kernel will not allow unprivileged parents kill their privileged children + * this way. We rely on the control groups kill logic to do the rest for us. */ if (prctl(PR_SET_PDEATHSIG, SIGTERM) < 0) goto child_finish; - /* Tell the parent that our setup is done. This is especially - * important regarding dropping privileges. Otherwise, unit - * setup might race against our setresuid(2) call. + /* Tell the parent that our setup is done. This is especially important regarding dropping + * privileges. Otherwise, unit setup might race against our setresuid(2) call. * - * If the parent aborted, we'll detect this below, hence ignore - * return failure here. */ + * If the parent aborted, we'll detect this below, hence ignore return failure here. */ (void) barrier_place(&barrier); /* Check if our parent process might already have died? */ @@ -1343,25 +1337,27 @@ static int setup_pam( ret = 0; child_finish: - pam_end(handle, pam_code | flags); + /* NB: pam_end() when called in child processes should set PAM_DATA_SILENT to let the module + * know about this. See pam_end(3) */ + (void) pam_end(handle, pam_code | flags | PAM_DATA_SILENT); _exit(ret); } barrier_set_role(&barrier, BARRIER_PARENT); - /* If the child was forked off successfully it will do all the - * cleanups, so forget about the handle here. */ + /* If the child was forked off successfully it will do all the cleanups, so forget about the handle + * here. */ handle = NULL; /* Unblock SIGTERM again in the parent */ assert_se(sigprocmask(SIG_SETMASK, &old_ss, NULL) >= 0); - /* We close the log explicitly here, since the PAM modules - * might have opened it, but we don't want this fd around. */ + /* We close the log explicitly here, since the PAM modules might have opened it, but we don't want + * this fd around. */ closelog(); - /* Synchronously wait for the child to initialize. We don't care for - * errors as we cannot recover. However, warn loudly if it happens. */ + /* Synchronously wait for the child to initialize. We don't care for errors as we cannot + * recover. However, warn loudly if it happens. */ if (!barrier_place_and_sync(&barrier)) log_error("PAM initialization failed"); @@ -1378,12 +1374,10 @@ fail: if (close_session) pam_code = pam_close_session(handle, flags); - pam_end(handle, pam_code | flags); + (void) pam_end(handle, pam_code | flags); } - strv_free(e); closelog(); - return r; #else return 0; diff --git a/src/fundamental/macro-fundamental.h b/src/fundamental/macro-fundamental.h index d597c743bb..15b93165dc 100644 --- a/src/fundamental/macro-fundamental.h +++ b/src/fundamental/macro-fundamental.h @@ -53,14 +53,16 @@ #define CONCATENATE(x, y) XCONCATENATE(x, y) #ifdef SD_BOOT + void efi_assert(const char *expr, const char *file, unsigned line, const char *function) _noreturn_; + #ifdef NDEBUG #define assert(expr) #define assert_not_reached() __builtin_unreachable() #else - void efi_assert(const char *expr, const char *file, unsigned line, const char *function) _noreturn_; #define assert(expr) ({ _likely_(expr) ? VOID_0 : efi_assert(#expr, __FILE__, __LINE__, __PRETTY_FUNCTION__); }) #define assert_not_reached() efi_assert("Code should not be reached", __FILE__, __LINE__, __PRETTY_FUNCTION__) #endif + #define assert_se(expr) ({ _likely_(expr) ? VOID_0 : efi_assert(#expr, __FILE__, __LINE__, __PRETTY_FUNCTION__); }) #define memcpy(a, b, c) CopyMem((a), (b), (c)) #define free(a) FreePool(a) @@ -74,6 +76,13 @@ _expr_; \ }) +#define ASSERT_SE_PTR(expr) \ + ({ \ + typeof(expr) _expr_ = (expr); \ + assert_se(_expr_); \ + _expr_; \ + }) + #if defined(static_assert) #define assert_cc(expr) \ static_assert(expr, #expr) diff --git a/src/libsystemd-network/sd-dhcp-lease.c b/src/libsystemd-network/sd-dhcp-lease.c index 5a40eb94d3..b87af04736 100644 --- a/src/libsystemd-network/sd-dhcp-lease.c +++ b/src/libsystemd-network/sd-dhcp-lease.c @@ -468,41 +468,48 @@ static int lease_parse_sip_server(const uint8_t *option, size_t len, struct in_a } static int lease_parse_routes( - const uint8_t *option, size_t len, - struct sd_dhcp_route **routes, size_t *routes_size) { + const uint8_t *option, + size_t len, + struct sd_dhcp_route **routes, + size_t *routes_size) { - struct in_addr addr; + int r; assert(option || len <= 0); assert(routes); assert(routes_size); - if (len <= 0) - return 0; - if (len % 8 != 0) return -EINVAL; - if (!GREEDY_REALLOC(*routes, *routes_size + (len / 8))) - return -ENOMEM; - while (len >= 8) { - struct sd_dhcp_route *route = *routes + *routes_size; - int r; - - route->option = SD_DHCP_OPTION_STATIC_ROUTE; - r = in4_addr_default_prefixlen((struct in_addr*) option, &route->dst_prefixlen); - if (r < 0) - return -EINVAL; + struct in_addr dst, gw; + uint8_t prefixlen; - assert_se(lease_parse_be32(option, 4, &addr.s_addr) >= 0); - route->dst_addr = inet_makeaddr(inet_netof(addr), 0); + assert_se(lease_parse_be32(option, 4, &dst.s_addr) >= 0); option += 4; - assert_se(lease_parse_be32(option, 4, &route->gw_addr.s_addr) >= 0); + assert_se(lease_parse_be32(option, 4, &gw.s_addr) >= 0); option += 4; len -= 8; + + r = in4_addr_default_prefixlen(&dst, &prefixlen); + if (r < 0) + return -EINVAL; + + (void) in4_addr_mask(&dst, prefixlen); + + if (!GREEDY_REALLOC(*routes, *routes_size + 1)) + return -ENOMEM; + + (*routes)[*routes_size] = (struct sd_dhcp_route) { + .dst_addr = dst, + .gw_addr = gw, + .dst_prefixlen = prefixlen, + .option = SD_DHCP_OPTION_STATIC_ROUTE, + }; + (*routes_size)++; } @@ -1114,6 +1121,18 @@ int dhcp_lease_save(sd_dhcp_lease *lease, const char *lease_file) { return 0; } +static char **private_options_free(char **options) { + if (!options) + return NULL; + + for (unsigned i = 0; i < SD_DHCP_OPTION_PRIVATE_LAST - SD_DHCP_OPTION_PRIVATE_BASE + 1; i++) + free(options[i]); + + return mfree(options); +} + +DEFINE_TRIVIAL_CLEANUP_FUNC(char**, private_options_free); + int dhcp_lease_load(sd_dhcp_lease **ret, const char *lease_file) { _cleanup_(sd_dhcp_lease_unrefp) sd_dhcp_lease *lease = NULL; _cleanup_free_ char @@ -1136,8 +1155,8 @@ int dhcp_lease_load(sd_dhcp_lease **ret, const char *lease_file) { *vendor_specific_hex = NULL, *lifetime = NULL, *t1 = NULL, - *t2 = NULL, - *options[SD_DHCP_OPTION_PRIVATE_LAST - SD_DHCP_OPTION_PRIVATE_BASE + 1] = {}; + *t2 = NULL; + _cleanup_(private_options_freep) char **options = NULL; int r, i; @@ -1148,6 +1167,10 @@ int dhcp_lease_load(sd_dhcp_lease **ret, const char *lease_file) { if (r < 0) return r; + options = new0(char*, SD_DHCP_OPTION_PRIVATE_LAST - SD_DHCP_OPTION_PRIVATE_BASE + 1); + if (!options) + return -ENOMEM; + r = parse_env_file(NULL, lease_file, "ADDRESS", &address, "ROUTER", &router, diff --git a/src/network/netdev/xfrm.c b/src/network/netdev/xfrm.c index 05844b8321..a961d8fef2 100644 --- a/src/network/netdev/xfrm.c +++ b/src/network/netdev/xfrm.c @@ -14,6 +14,7 @@ static int xfrm_fill_message_create(NetDev *netdev, Link *link, sd_netlink_messa x = XFRM(netdev); + assert(x); assert(link || x->independent); r = sd_netlink_message_append_u32(message, IFLA_XFRM_LINK, link ? link->ifindex : LOOPBACK_IFINDEX); @@ -27,10 +28,28 @@ static int xfrm_fill_message_create(NetDev *netdev, Link *link, sd_netlink_messa return 0; } +static int xfrm_verify(NetDev *netdev, const char *filename) { + Xfrm *x; + + assert(netdev); + assert(filename); + + x = XFRM(netdev); + + assert(x); + + if (x->if_id == 0) + return log_netdev_warning_errno(netdev, SYNTHETIC_ERRNO(EINVAL), + "%s: Xfrm interface ID cannot be zero.", filename); + + return 0; +} + const NetDevVTable xfrm_vtable = { .object_size = sizeof(Xfrm), .sections = NETDEV_COMMON_SECTIONS "Xfrm\0", .fill_message_create = xfrm_fill_message_create, + .config_verify = xfrm_verify, .create_type = NETDEV_CREATE_STACKED, .iftype = ARPHRD_NONE, }; diff --git a/src/network/networkd-dhcp-server.c b/src/network/networkd-dhcp-server.c index ced077944c..32a129a6f6 100644 --- a/src/network/networkd-dhcp-server.c +++ b/src/network/networkd-dhcp-server.c @@ -58,12 +58,21 @@ void network_adjust_dhcp_server(Network *network) { ORDERED_HASHMAP_FOREACH(address, network->addresses_by_section) { if (section_is_invalid(address->section)) continue; - if (address->family == AF_INET && - !in4_addr_is_localhost(&address->in_addr.in) && - in4_addr_is_null(&address->in_addr_peer.in)) { - have = true; - break; - } + + if (address->family != AF_INET) + continue; + + if (in4_addr_is_localhost(&address->in_addr.in)) + continue; + + if (in4_addr_is_link_local(&address->in_addr.in)) + continue; + + if (in4_addr_is_set(&address->in_addr_peer.in)) + continue; + + have = true; + break; } if (!have) { log_warning("%s: DHCPServer= is enabled, but no static address configured. " @@ -130,6 +139,8 @@ static int link_find_dhcp_server_address(Link *link, Address **ret) { continue; if (in4_addr_is_localhost(&address->in_addr.in)) continue; + if (in4_addr_is_link_local(&address->in_addr.in)) + continue; if (in4_addr_is_set(&address->in_addr_peer.in)) continue; diff --git a/src/resolve/resolved-dns-stream.c b/src/resolve/resolved-dns-stream.c index cf9d1a9d5e..61e92bea83 100644 --- a/src/resolve/resolved-dns-stream.c +++ b/src/resolve/resolved-dns-stream.c @@ -27,7 +27,7 @@ static void dns_stream_stop(DnsStream *s) { } static int dns_stream_update_io(DnsStream *s) { - int f = 0; + uint32_t f = 0; assert(s); @@ -47,6 +47,8 @@ static int dns_stream_update_io(DnsStream *s) { set_size(s->queries) < DNS_QUERIES_PER_STREAM) f |= EPOLLIN; + s->requested_events = f; + #if ENABLE_DNS_OVER_TLS /* For handshake and clean closing purposes, TLS can override requested events */ if (s->dnstls_events != 0) @@ -208,22 +210,10 @@ ssize_t dns_stream_writev(DnsStream *s, const struct iovec *iov, size_t iovcnt, assert(iov); #if ENABLE_DNS_OVER_TLS - if (s->encrypted && !(flags & DNS_STREAM_WRITE_TLS_DATA)) { - ssize_t ss; - size_t i; - - m = 0; - for (i = 0; i < iovcnt; i++) { - ss = dnstls_stream_write(s, iov[i].iov_base, iov[i].iov_len); - if (ss < 0) - return ss; - - m += ss; - if (ss != (ssize_t) iov[i].iov_len) - continue; - } - } else + if (s->encrypted && !(flags & DNS_STREAM_WRITE_TLS_DATA)) + return dnstls_stream_writev(s, iov, iovcnt); #endif + if (s->tfo_salen > 0) { struct msghdr hdr = { .msg_iov = (struct iovec*) iov, @@ -289,7 +279,7 @@ static DnsPacket *dns_stream_take_read_packet(DnsStream *s) { * Even this makes a room to read in the stream, this does not call dns_stream_update(), hence * EPOLLIN flag is not set automatically. So, to read further packets from the stream, * dns_stream_update() must be called explicitly. Currently, this is only called from - * on_stream_io_impl(), and there dns_stream_update() is called. */ + * on_stream_io(), and there dns_stream_update() is called. */ if (!s->read_packet) return NULL; @@ -304,15 +294,13 @@ static DnsPacket *dns_stream_take_read_packet(DnsStream *s) { return TAKE_PTR(s->read_packet); } -static int on_stream_io_impl(DnsStream *s, uint32_t revents) { +static int on_stream_io(sd_event_source *es, int fd, uint32_t revents, void *userdata) { + _cleanup_(dns_stream_unrefp) DnsStream *s = dns_stream_ref(userdata); /* Protect stream while we process it */ bool progressed = false; int r; assert(s); - /* This returns 1 when possible remaining stream exists, 0 on completed - stream or recoverable error, and negative errno on failure. */ - #if ENABLE_DNS_OVER_TLS if (s->encrypted) { r = dnstls_stream_on_io(s, revents); @@ -364,9 +352,9 @@ static int on_stream_io_impl(DnsStream *s, uint32_t revents) { } } - if ((revents & (EPOLLIN|EPOLLHUP|EPOLLRDHUP)) && - (!s->read_packet || - s->n_read < sizeof(s->read_size) + s->read_packet->size)) { + while ((revents & (EPOLLIN|EPOLLHUP|EPOLLRDHUP)) && + (!s->read_packet || + s->n_read < sizeof(s->read_size) + s->read_packet->size)) { if (s->n_read < sizeof(s->read_size)) { ssize_t ss; @@ -375,6 +363,7 @@ static int on_stream_io_impl(DnsStream *s, uint32_t revents) { if (ss < 0) { if (!ERRNO_IS_TRANSIENT(ss)) return dns_stream_complete(s, -ss); + break; } else if (ss == 0) return dns_stream_complete(s, ECONNRESET); else { @@ -428,6 +417,7 @@ static int on_stream_io_impl(DnsStream *s, uint32_t revents) { if (ss < 0) { if (!ERRNO_IS_TRANSIENT(ss)) return dns_stream_complete(s, -ss); + break; } else if (ss == 0) return dns_stream_complete(s, ECONNRESET); else @@ -448,23 +438,19 @@ static int on_stream_io_impl(DnsStream *s, uint32_t revents) { return dns_stream_complete(s, -r); s->packet_received = true; + + /* If we just disabled the read event, stop reading */ + if (!FLAGS_SET(s->requested_events, EPOLLIN)) + break; } } } - if (s->type == DNS_STREAM_LLMNR_SEND && s->packet_received) { - uint32_t events; - - /* Complete the stream if finished reading and writing one packet, and there's nothing - * else left to write. */ - - r = sd_event_source_get_io_events(s->io_event_source, &events); - if (r < 0) - return r; - - if (!FLAGS_SET(events, EPOLLOUT)) - return dns_stream_complete(s, 0); - } + /* Complete the stream if finished reading and writing one packet, and there's nothing + * else left to write. */ + if (s->type == DNS_STREAM_LLMNR_SEND && s->packet_received && + !FLAGS_SET(s->requested_events, EPOLLOUT)) + return dns_stream_complete(s, 0); /* If we did something, let's restart the timeout event source */ if (progressed && s->timeout_event_source) { @@ -473,44 +459,6 @@ static int on_stream_io_impl(DnsStream *s, uint32_t revents) { log_warning_errno(errno, "Couldn't restart TCP connection timeout, ignoring: %m"); } - return 1; -} - -static int on_stream_io(sd_event_source *es, int fd, uint32_t revents, void *userdata) { - _cleanup_(dns_stream_unrefp) DnsStream *s = dns_stream_ref(userdata); /* Protect stream while we process it */ - int r; - - assert(s); - - r = on_stream_io_impl(s, revents); - if (r <= 0) - return r; - -#if ENABLE_DNS_OVER_TLS - if (!s->encrypted) - return 0; - - /* When using DNS-over-TLS, the underlying TLS library may read the entire TLS record - and buffer it internally. If this happens, we will not receive further EPOLLIN events, - and unless there's some unrelated activity on the socket, we will hang until time out. - 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. */ - while (dnstls_stream_has_buffered_data(s)) { - uint32_t events; - - /* Make sure the stream still wants to process more data... */ - r = sd_event_source_get_io_events(s->io_event_source, &events); - if (r < 0) - return r; - if (!FLAGS_SET(events, EPOLLIN)) - break; - - r = on_stream_io_impl(s, EPOLLIN); - if (r <= 0) - return r; - } -#endif - return 0; } diff --git a/src/resolve/resolved-dns-stream.h b/src/resolve/resolved-dns-stream.h index 1c606365cd..ba4a59e41c 100644 --- a/src/resolve/resolved-dns-stream.h +++ b/src/resolve/resolved-dns-stream.h @@ -61,6 +61,7 @@ struct DnsStream { uint32_t ttl; bool identified; bool packet_received; /* At least one packet is received. Used by LLMNR. */ + uint32_t requested_events; /* only when using TCP fast open */ union sockaddr_union tfo_address; @@ -68,7 +69,7 @@ struct DnsStream { #if ENABLE_DNS_OVER_TLS DnsTlsStreamData dnstls_data; - int dnstls_events; + uint32_t dnstls_events; #endif sd_event_source *io_event_source; diff --git a/src/resolve/resolved-dnstls-gnutls.c b/src/resolve/resolved-dnstls-gnutls.c index 8610cacab6..8c8628ebbb 100644 --- a/src/resolve/resolved-dnstls-gnutls.c +++ b/src/resolve/resolved-dnstls-gnutls.c @@ -6,6 +6,7 @@ #include <gnutls/socket.h> +#include "io-util.h" #include "resolved-dns-stream.h" #include "resolved-dnstls.h" #include "resolved-manager.h" @@ -13,7 +14,7 @@ #define TLS_PROTOCOL_PRIORITY "NORMAL:-VERS-ALL:+VERS-TLS1.3:+VERS-TLS1.2" DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(gnutls_session_t, gnutls_deinit, NULL); -static ssize_t dnstls_stream_writev(gnutls_transport_ptr_t p, const giovec_t *iov, int iovcnt) { +static ssize_t dnstls_stream_vec_push(gnutls_transport_ptr_t p, const giovec_t *iov, int iovcnt) { int r; assert(p); @@ -81,7 +82,7 @@ int dnstls_stream_connect_tls(DnsStream *stream, DnsServer *server) { gnutls_handshake_set_timeout(gs, GNUTLS_DEFAULT_HANDSHAKE_TIMEOUT); gnutls_transport_set_ptr2(gs, (gnutls_transport_ptr_t) (long) stream->fd, stream); - gnutls_transport_set_vec_push_function(gs, &dnstls_stream_writev); + gnutls_transport_set_vec_push_function(gs, &dnstls_stream_vec_push); stream->encrypted = true; stream->dnstls_data.handshake = gnutls_handshake(gs); @@ -163,15 +164,26 @@ int dnstls_stream_shutdown(DnsStream *stream, int error) { return 0; } -ssize_t dnstls_stream_write(DnsStream *stream, const char *buf, size_t count) { +ssize_t dnstls_stream_writev(DnsStream *stream, const struct iovec *iov, size_t iovcnt) { ssize_t ss; assert(stream); assert(stream->encrypted); assert(stream->dnstls_data.session); - assert(buf); + assert(iov); + assert(IOVEC_TOTAL_SIZE(iov, iovcnt) > 0); + + gnutls_record_cork(stream->dnstls_data.session); - ss = gnutls_record_send(stream->dnstls_data.session, buf, count); + for (size_t i = 0; i < iovcnt; i++) { + ss = gnutls_record_send( + stream->dnstls_data.session, + iov[i].iov_base, iov[i].iov_len); + if (ss < 0) + break; + } + + ss = gnutls_record_uncork(stream->dnstls_data.session, 0); if (ss < 0) switch(ss) { case GNUTLS_E_INTERRUPTED: @@ -211,14 +223,6 @@ ssize_t dnstls_stream_read(DnsStream *stream, void *buf, size_t count) { return ss; } -bool dnstls_stream_has_buffered_data(DnsStream *stream) { - assert(stream); - assert(stream->encrypted); - assert(stream->dnstls_data.session); - - return gnutls_record_check_pending(stream->dnstls_data.session) > 0; -} - void dnstls_server_free(DnsServer *server) { assert(server); diff --git a/src/resolve/resolved-dnstls-openssl.c b/src/resolve/resolved-dnstls-openssl.c index 7d264dd367..4d3a88c8da 100644 --- a/src/resolve/resolved-dnstls-openssl.c +++ b/src/resolve/resolved-dnstls-openssl.c @@ -292,15 +292,10 @@ int dnstls_stream_shutdown(DnsStream *stream, int error) { return 0; } -ssize_t dnstls_stream_write(DnsStream *stream, const char *buf, size_t count) { +static ssize_t dnstls_stream_write(DnsStream *stream, const char *buf, size_t count) { int error, r; ssize_t ss; - assert(stream); - assert(stream->encrypted); - assert(stream->dnstls_data.ssl); - assert(buf); - ERR_clear_error(); ss = r = SSL_write(stream->dnstls_data.ssl, buf, count); if (r <= 0) { @@ -329,6 +324,29 @@ ssize_t dnstls_stream_write(DnsStream *stream, const char *buf, size_t count) { return ss; } +ssize_t dnstls_stream_writev(DnsStream *stream, const struct iovec *iov, size_t iovcnt) { + _cleanup_free_ char *buf = NULL; + size_t count; + + assert(stream); + assert(stream->encrypted); + assert(stream->dnstls_data.ssl); + assert(iov); + assert(IOVEC_TOTAL_SIZE(iov, iovcnt) > 0); + + if (iovcnt == 1) + return dnstls_stream_write(stream, iov[0].iov_base, iov[0].iov_len); + + /* As of now, OpenSSL can not accumulate multiple writes, so join into a + single buffer. Suboptimal, but better than multiple SSL_write calls. */ + count = IOVEC_TOTAL_SIZE(iov, iovcnt); + buf = new(char, count); + for (size_t i = 0, pos = 0; i < iovcnt; pos += iov[i].iov_len, i++) + memcpy(buf + pos, iov[i].iov_base, iov[i].iov_len); + + return dnstls_stream_write(stream, buf, count); +} + ssize_t dnstls_stream_read(DnsStream *stream, void *buf, size_t count) { int error, r; ssize_t ss; @@ -343,7 +361,15 @@ ssize_t dnstls_stream_read(DnsStream *stream, void *buf, size_t count) { if (r <= 0) { error = SSL_get_error(stream->dnstls_data.ssl, r); if (IN_SET(error, SSL_ERROR_WANT_READ, SSL_ERROR_WANT_WRITE)) { - stream->dnstls_events = error == SSL_ERROR_WANT_READ ? EPOLLIN : EPOLLOUT; + /* If we receive SSL_ERROR_WANT_READ here, there are two possible scenarios: + * OpenSSL needs to renegotiate (so we want to get an EPOLLIN event), or + * There is no more application data is available, so we can just return + And apparently there's no nice way to distinguish between the two. + To handle this, never set EPOLLIN and just continue as usual. + If OpenSSL really wants to read due to renegotiation, it will tell us + again on SSL_write (at which point we will request EPOLLIN force a read); + or we will just eventually read data anyway while we wait for a packet */ + stream->dnstls_events = error == SSL_ERROR_WANT_READ ? 0 : EPOLLOUT; ss = -EAGAIN; } else if (error == SSL_ERROR_ZERO_RETURN) { stream->dnstls_events = 0; @@ -367,14 +393,6 @@ ssize_t dnstls_stream_read(DnsStream *stream, void *buf, size_t count) { return ss; } -bool dnstls_stream_has_buffered_data(DnsStream *stream) { - assert(stream); - assert(stream->encrypted); - assert(stream->dnstls_data.ssl); - - return SSL_has_pending(stream->dnstls_data.ssl) > 0; -} - void dnstls_server_free(DnsServer *server) { assert(server); diff --git a/src/resolve/resolved-dnstls.h b/src/resolve/resolved-dnstls.h index ed214dc6c4..cda97e0b12 100644 --- a/src/resolve/resolved-dnstls.h +++ b/src/resolve/resolved-dnstls.h @@ -3,8 +3,8 @@ #if ENABLE_DNS_OVER_TLS -#include <stdbool.h> #include <stdint.h> +#include <sys/uio.h> typedef struct DnsServer DnsServer; typedef struct DnsStream DnsStream; @@ -27,9 +27,8 @@ int dnstls_stream_connect_tls(DnsStream *stream, DnsServer *server); void dnstls_stream_free(DnsStream *stream); int dnstls_stream_on_io(DnsStream *stream, uint32_t revents); int dnstls_stream_shutdown(DnsStream *stream, int error); -ssize_t dnstls_stream_write(DnsStream *stream, const char *buf, size_t count); +ssize_t dnstls_stream_writev(DnsStream *stream, const struct iovec *iov, size_t iovcnt); ssize_t dnstls_stream_read(DnsStream *stream, void *buf, size_t count); -bool dnstls_stream_has_buffered_data(DnsStream *stream); void dnstls_server_free(DnsServer *server); diff --git a/src/resolve/test-resolved-stream.c b/src/resolve/test-resolved-stream.c index f9428989f0..beaa855384 100644 --- a/src/resolve/test-resolved-stream.c +++ b/src/resolve/test-resolved-stream.c @@ -2,10 +2,12 @@ #include <arpa/inet.h> #include <fcntl.h> +#include <net/if.h> #include <pthread.h> #include <signal.h> #include <stdlib.h> #include <string.h> +#include <sys/ioctl.h> #include <sys/prctl.h> #include <sys/socket.h> #include <sys/wait.h> @@ -13,6 +15,7 @@ #include "fd-util.h" #include "log.h" +#include "macro.h" #include "process-util.h" #include "resolved-dns-packet.h" #include "resolved-dns-question.h" @@ -144,7 +147,7 @@ static void *tls_dns_server(void *p) { r = safe_fork_full("(test-resolved-stream-tls-openssl)", (int[]) { fd_server, fd_tls }, 2, FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS|FORK_DEATHSIG|FORK_LOG|FORK_REOPEN_LOG, &openssl_pid); - assert(r >= 0); + assert_se(r >= 0); if (r == 0) { /* Child */ assert_se(dup2(fd_tls, STDIN_FILENO) >= 0); @@ -200,6 +203,10 @@ static int on_stream_packet(DnsStream *stream, DnsPacket *p) { return 0; } +static int on_stream_complete_do_nothing(DnsStream *s, int error) { + return 0; +} + static void test_dns_stream(bool tls) { Manager manager = {}; _cleanup_(dns_stream_unrefp) DnsStream *stream = NULL; @@ -251,9 +258,10 @@ static void test_dns_stream(bool tls) { /* systemd-resolved uses (and requires) the socket to be in nonblocking mode */ assert_se(fcntl(clientfd, F_SETFL, O_NONBLOCK) >= 0); - /* Initialize DNS stream */ + /* Initialize DNS stream (disabling the default self-destruction + behaviour when no complete callback is set) */ assert_se(dns_stream_new(&manager, &stream, DNS_STREAM_LOOKUP, DNS_PROTOCOL_DNS, - TAKE_FD(clientfd), NULL, on_stream_packet, NULL, + TAKE_FD(clientfd), NULL, on_stream_packet, on_stream_complete_do_nothing, DNS_STREAM_DEFAULT_TIMEOUT_USEC) >= 0); #if ENABLE_DNS_OVER_TLS if (tls) { @@ -322,6 +330,24 @@ static void test_dns_stream(bool tls) { log_info("test-resolved-stream: Finished %s test", tls ? "TLS" : "TCP"); } +static void try_isolate_network(void) { + _cleanup_close_ int socket_fd = -1; + + if (unshare(CLONE_NEWUSER | CLONE_NEWNET) < 0) { + log_warning("test-resolved-stream: Can't create user and network ns, running on host"); + return; + } + + /* Bring up the loopback interfaceon the newly created network namespace */ + struct ifreq req = { .ifr_ifindex = 1 }; + assert_se((socket_fd = socket(AF_INET, SOCK_STREAM | SOCK_CLOEXEC, 0)) >= 0); + assert_se(ioctl(socket_fd,SIOCGIFNAME,&req) >= 0); + assert_se(ioctl(socket_fd, SIOCGIFFLAGS, &req) >= 0); + assert_se(FLAGS_SET(req.ifr_flags, IFF_LOOPBACK)); + req.ifr_flags |= IFF_UP; + assert_se(ioctl(socket_fd, SIOCSIFFLAGS, &req) >= 0); +} + int main(int argc, char **argv) { SERVER_ADDRESS = (struct sockaddr_in) { .sin_family = AF_INET, @@ -331,6 +357,8 @@ int main(int argc, char **argv) { test_setup_logging(LOG_DEBUG); + try_isolate_network(); + test_dns_stream(false); #if ENABLE_DNS_OVER_TLS if (system("openssl version >/dev/null 2>&1") != 0) diff --git a/test/test-network/conf/25-xfrm-independent.netdev b/test/test-network/conf/25-xfrm-independent.netdev index b2378849d1..b54c659d83 100644 --- a/test/test-network/conf/25-xfrm-independent.netdev +++ b/test/test-network/conf/25-xfrm-independent.netdev @@ -4,4 +4,5 @@ Kind=xfrm Name=xfrm99 [Xfrm] +InterfaceId=0x99 Independent=yes diff --git a/test/test-network/conf/25-xfrm.netdev b/test/test-network/conf/25-xfrm.netdev index 353bfb7003..8e1d5c8122 100644 --- a/test/test-network/conf/25-xfrm.netdev +++ b/test/test-network/conf/25-xfrm.netdev @@ -1,4 +1,7 @@ # SPDX-License-Identifier: LGPL-2.1-or-later [NetDev] Kind=xfrm -Name=xfrm99 +Name=xfrm98 + +[Xfrm] +InterfaceId=0x98 diff --git a/test/test-network/conf/netdev-link-local-addressing-yes.network b/test/test-network/conf/netdev-link-local-addressing-yes.network index 0cc9cfa96b..ea1811bbfd 100644 --- a/test/test-network/conf/netdev-link-local-addressing-yes.network +++ b/test/test-network/conf/netdev-link-local-addressing-yes.network @@ -18,7 +18,7 @@ Name=geneve99 Name=ifb99 Name=ipiptun99 Name=nlmon99 -Name=xfrm99 +Name=xfrm98 xfrm99 Name=vxlan98 Name=hogehogehogehogehogehoge diff --git a/test/test-network/conf/xfrm.network b/test/test-network/conf/xfrm.network index c852601733..19f22146f8 100644 --- a/test/test-network/conf/xfrm.network +++ b/test/test-network/conf/xfrm.network @@ -4,4 +4,4 @@ Name=dummy98 [Network] IPv6AcceptRA=no -Xfrm=xfrm99 +Xfrm=xfrm98 diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index 05f3479e12..7013f73851 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -886,6 +886,7 @@ class NetworkctlTests(unittest.TestCase, Utilities): class NetworkdNetDevTests(unittest.TestCase, Utilities): links_remove_earlier = [ + 'xfrm98', 'xfrm99', ] @@ -1797,20 +1798,21 @@ class NetworkdNetDevTests(unittest.TestCase, Utilities): @expectedFailureIfModuleIsNotAvailable('xfrm_interface') def test_xfrm(self): copy_unit_to_networkd_unit_path('12-dummy.netdev', 'xfrm.network', - '25-xfrm.netdev', 'netdev-link-local-addressing-yes.network') + '25-xfrm.netdev', '25-xfrm-independent.netdev', + 'netdev-link-local-addressing-yes.network') start_networkd() - self.wait_online(['xfrm99:degraded', 'dummy98:degraded']) + self.wait_online(['dummy98:degraded', 'xfrm98:degraded', 'xfrm99:degraded']) - output = check_output('ip link show dev xfrm99') + output = check_output('ip -d link show dev xfrm98') print(output) + self.assertIn('xfrm98@dummy98:', output) + self.assertIn('xfrm if_id 0x98 ', output) - @expectedFailureIfModuleIsNotAvailable('xfrm_interface') - def test_xfrm_independent(self): - copy_unit_to_networkd_unit_path('25-xfrm-independent.netdev', 'netdev-link-local-addressing-yes.network') - start_networkd() - - self.wait_online(['xfrm99:degraded']) + output = check_output('ip -d link show dev xfrm99') + print(output) + self.assertIn('xfrm99@lo:', output) + self.assertIn('xfrm if_id 0x99 ', output) @expectedFailureIfModuleIsNotAvailable('fou') def test_fou(self): diff --git a/units/systemd-journal-flush.service b/units/systemd-journal-flush.service index 6efb8734a7..5d0b811ae3 100644 --- a/units/systemd-journal-flush.service +++ b/units/systemd-journal-flush.service @@ -11,6 +11,7 @@ Description=Flush Journal to Persistent Storage Documentation=man:systemd-journald.service(8) man:journald.conf(5) DefaultDependencies=no +Wants=systemd-journald.service After=systemd-journald.service systemd-remount-fs.service Before=systemd-tmpfiles-setup.service RequiresMountsFor=/var/log/journal |