From d3157f2a67fc0263933200beb2f23cdcc2897dc0 Mon Sep 17 00:00:00 2001 From: Siteshwar Vashisht Date: Thu, 10 Sep 2026 11:04:14 +0200 Subject: [PATCH 1/4] ndisc: fix an off by 2 write in ndisc_option_build_encrypted_dns() This function constructs the NDisc Encrypted DNS option (RFC 9463). `poff` is initialized to `off + sizeof(uint16_t)` - pointing past the 2 bytes SvcParam length field. After all SvcParams are written the length is set at line 1541. `LESS_BY(poff, off) = poff - off` includes the 2 bytes length field itself. The correct value is `poff - off - sizeof(uint16_t)`. --- src/libsystemd-network/ndisc-option.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libsystemd-network/ndisc-option.c b/src/libsystemd-network/ndisc-option.c index 57418c0064..92e04bb997 100644 --- a/src/libsystemd-network/ndisc-option.c +++ b/src/libsystemd-network/ndisc-option.c @@ -1538,7 +1538,7 @@ static int ndisc_option_build_encrypted_dns(const sd_ndisc_option *option, usec_ poff += plen; } - unaligned_write_be16(buf + off, LESS_BY(poff, off)); + unaligned_write_be16(buf + off, LESS_BY(poff, off + sizeof(uint16_t))); off = poff; padding: From 1299c9cc9eb653a4031b7ab41afbe51e540bf965 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 12 Sep 2026 04:08:49 +0900 Subject: [PATCH 2/4] ndisc-option: drop unused argument --- src/libsystemd-network/ndisc-option.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libsystemd-network/ndisc-option.h b/src/libsystemd-network/ndisc-option.h index d92ff6813c..2a10af5e47 100644 --- a/src/libsystemd-network/ndisc-option.h +++ b/src/libsystemd-network/ndisc-option.h @@ -333,7 +333,6 @@ static inline int ndisc_option_add_encrypted_dns( } static inline int ndisc_option_set_encrypted_dns( Set **options, - size_t offset, sd_dns_resolver *res, usec_t lifetime, usec_t valid_until) { From b62890943012c13cef8c7c595163efe1955e624f Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 4 Sep 2026 20:25:29 +0900 Subject: [PATCH 3/4] ndisc-option: fix parsing/building extended flags The flags part of the option is 6 bytes. Let's read/write one by one. Fixes https://github.com/systemd/systemd/pull/43620#discussion_r3913713018. --- src/libsystemd-network/ndisc-option.c | 9 +++++++-- src/libsystemd-network/test-ndisc-rs.c | 11 ++++++++--- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/libsystemd-network/ndisc-option.c b/src/libsystemd-network/ndisc-option.c index 92e04bb997..228674adec 100644 --- a/src/libsystemd-network/ndisc-option.c +++ b/src/libsystemd-network/ndisc-option.c @@ -871,7 +871,10 @@ static int ndisc_option_parse_flags_extension(Set **options, size_t offset, size if (opt[0] != SD_NDISC_OPTION_FLAGS_EXTENSION) return -EBADMSG; - uint64_t flags = (unaligned_read_be64(opt) & UINT64_C(0xffffffffffff0000)) >> 8; + uint64_t flags = 0; + for (size_t i = 2; i < 8; i++) + flags |= ((uint64_t) opt[i]) << ((i - 1) * 8); + return ndisc_option_add_flags_extension(options, offset, flags); } @@ -884,10 +887,12 @@ static int ndisc_option_build_flags_extension(const sd_ndisc_option *option, uin if (!buf) return -ENOMEM; - unaligned_write_be64(buf, (option->extended_flags & UINT64_C(0x00ffffffffffff00)) << 8); buf[0] = SD_NDISC_OPTION_FLAGS_EXTENSION; buf[1] = 1; + for (size_t i = 2; i < 8; i++) + buf[i] = (option->extended_flags >> ((i - 1) * 8)) & 0xff; + *ret = TAKE_PTR(buf); return 0; } diff --git a/src/libsystemd-network/test-ndisc-rs.c b/src/libsystemd-network/test-ndisc-rs.c index 6eda6a2116..304b09920c 100644 --- a/src/libsystemd-network/test-ndisc-rs.c +++ b/src/libsystemd-network/test-ndisc-rs.c @@ -105,6 +105,7 @@ static void router_dump(sd_ndisc_router *rt) { log_info("Hop limit: %u", hop_limit); assert_se(sd_ndisc_router_get_flags(rt, &flags) >= 0); + ASSERT_EQ(flags & UINT64_C(0x00ffffffffffff00), UINT64_C(0x0066554433221100)); log_info("Flags: <%s|%s>", flags & ND_RA_FLAG_OTHER ? "OTHER" : "", flags & ND_RA_FLAG_MANAGED ? "MANAGED" : ""); @@ -218,7 +219,7 @@ static void router_dump(sd_ndisc_router *rt) { static int send_ra(uint8_t flags) { uint8_t advertisement[] = { /* struct nd_router_advert */ - 0x86, 0x00, 0xde, 0x83, 0x40, 0xc0, 0x00, 0xb4, + 0x86, 0x00, 0x00, 0x00, 0x40, 0xc0, 0x00, 0xb4, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* type = 0x03 (SD_NDISC_OPTION_PREFIX_INFORMATION), length = 32 */ 0x03, 0x04, 0x40, 0xc0, 0x00, 0x00, 0x01, 0xf4, @@ -235,6 +236,8 @@ static int send_ra(uint8_t flags) { 0x72, 0x61, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* type = 0x01 (SD_NDISC_OPTION_SOURCE_LL_ADDRESS), length = 8 */ 0x01, 0x01, 0x78, 0x2b, 0xcb, 0xb3, 0x6d, 0x53, + /* type = 0x1a (SD_NDISC_OPTION_FLAGS_EXTENSION), length = 8 */ + 0x1a, 0x01, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, }; advertisement[5] = flags; @@ -269,7 +272,7 @@ static void test_callback_ra(sd_ndisc *nd, sd_ndisc_event_t event, void *message router_dump(rt); assert_se(sd_ndisc_router_get_flags(rt, &flags) >= 0); - assert_se(flags == flags_array[idx]); + ASSERT_EQ(flags & 0xff, flags_array[idx]); idx++; log_debug("%s: Got event 0x%02" PRIx64, __func__, flags); @@ -404,7 +407,7 @@ TEST(rs) { static int send_ra_invalid_domain(uint8_t flags) { uint8_t advertisement[] = { /* struct nd_router_advert */ - 0x86, 0x00, 0xde, 0x83, 0x40, 0xc0, 0x00, 0xb4, + 0x86, 0x00, 0x00, 0x00, 0x40, 0xc0, 0x00, 0xb4, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* type = 0x03 (SD_NDISC_OPTION_PREFIX_INFORMATION), length = 32 */ 0x03, 0x04, 0x40, 0xc0, 0x00, 0x00, 0x01, 0xf4, @@ -432,6 +435,8 @@ static int send_ra_invalid_domain(uint8_t flags) { 0xdb, 0x12, 0x9c, 0x75, 0x22, 0x5f, 0x12, 0x00, /* type = 0x01 (SD_NDISC_OPTION_SOURCE_LL_ADDRESS), length = 8 */ 0x01, 0x01, 0x78, 0x2b, 0xcb, 0xb3, 0x6d, 0x53, + /* type = 0x1a (SD_NDISC_OPTION_FLAGS_EXTENSION), length = 8 */ + 0x1a, 0x01, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, }; advertisement[5] = flags; From 3f4a43f0eb373d827f899354258c5a5ba1a14c4f Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 10 Sep 2026 10:55:50 +0200 Subject: [PATCH 4/4] test-ndisc-option: add a round trip test for ndisc options Co-authored-by: Siteshwar Vashisht --- src/libsystemd-network/meson.build | 10 +- src/libsystemd-network/test-ndisc-option.c | 366 +++++++++++++++++++++ 2 files changed, 373 insertions(+), 3 deletions(-) create mode 100644 src/libsystemd-network/test-ndisc-option.c diff --git a/src/libsystemd-network/meson.build b/src/libsystemd-network/meson.build index 2ac7d29fff..4b85e333a0 100644 --- a/src/libsystemd-network/meson.build +++ b/src/libsystemd-network/meson.build @@ -115,12 +115,16 @@ executables += [ 'sources' : files('test-lldp-rx.c'), }, network_test_template + { - 'sources' : files('test-ndisc-ra.c'), + 'sources' : files('test-ndisc-option.c'), 'export' : files('icmp6-test-util.c'), }, + network_test_template + { + 'sources' : files('test-ndisc-ra.c'), + 'import' : ['test-ndisc-option'], + }, network_test_template + { 'sources' : files('test-ndisc-rs.c'), - 'import' : ['test-ndisc-ra'], + 'import' : ['test-ndisc-option'], }, network_test_template + { 'sources' : files('test-ndisc-send.c'), @@ -146,6 +150,6 @@ executables += [ }, network_fuzz_template + { 'sources' : files('fuzz-ndisc-rs.c'), - 'import' : ['test-ndisc-ra'], + 'import' : ['test-ndisc-option'], }, ] diff --git a/src/libsystemd-network/test-ndisc-option.c b/src/libsystemd-network/test-ndisc-option.c new file mode 100644 index 0000000000..6927bb385b --- /dev/null +++ b/src/libsystemd-network/test-ndisc-option.c @@ -0,0 +1,366 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#include +#include + +#include "sd-dns-resolver.h" +#include "sd-ndisc-protocol.h" + +#include "dns-resolver-internal.h" +#include "ether-addr-util.h" +#include "fd-util.h" +#include "icmp6-packet.h" +#include "in-addr-util.h" +#include "ndisc-option.h" +#include "set.h" +#include "string-util.h" +#include "strv.h" +#include "tests.h" +#include "time-util.h" + +#define EQ(a, b) \ + (CMP(a, b) == 0) + +static bool ndisc_option_equal(const sd_ndisc_option *a, const sd_ndisc_option *b) { + ASSERT_NOT_NULL(a); + ASSERT_NOT_NULL(b); + + if (!EQ(a->type, b->type)) + return false; + + switch (a->type) { + case SD_NDISC_OPTION_SOURCE_LL_ADDRESS: + case SD_NDISC_OPTION_TARGET_LL_ADDRESS: + return ether_addr_equal(&a->mac, &b->mac); + + case SD_NDISC_OPTION_PREFIX_INFORMATION: + return + EQ(a->prefix.flags, b->prefix.flags) && + EQ(a->prefix.prefixlen, b->prefix.prefixlen) && + in6_addr_equal(&a->prefix.address, &b->prefix.address) && + EQ(a->prefix.valid_lifetime, b->prefix.valid_lifetime) && + EQ(a->prefix.preferred_lifetime, b->prefix.preferred_lifetime) && + EQ(a->prefix.valid_until, b->prefix.valid_until) && + EQ(a->prefix.preferred_until, b->prefix.preferred_until); + + case SD_NDISC_OPTION_REDIRECTED_HEADER: + return memcmp(&a->hdr, &b->hdr, sizeof(struct ip6_hdr)) == 0; + + case SD_NDISC_OPTION_MTU: + return EQ(a->mtu, b->mtu); + + case SD_NDISC_OPTION_HOME_AGENT: + return + EQ(a->home_agent.preference, b->home_agent.preference) && + EQ(a->home_agent.lifetime, b->home_agent.lifetime) && + EQ(a->home_agent.valid_until, b->home_agent.valid_until); + + case SD_NDISC_OPTION_ROUTE_INFORMATION: + return + EQ(a->route.preference, b->route.preference) && + EQ(a->route.prefixlen, b->route.prefixlen) && + in6_addr_equal(&a->route.address, &b->route.address) && + EQ(a->route.lifetime, b->route.lifetime) && + EQ(a->route.valid_until, b->route.valid_until); + + case SD_NDISC_OPTION_RDNSS: + if (!EQ(a->rdnss.n_addresses, b->rdnss.n_addresses)) + return false; + + for (size_t i = 0; i < a->rdnss.n_addresses; i++) + if (!in6_addr_equal(&a->rdnss.addresses[i], &b->rdnss.addresses[i])) + return false; + + return EQ(a->rdnss.lifetime, b->rdnss.lifetime) && + EQ(a->rdnss.valid_until, b->rdnss.valid_until); + + case SD_NDISC_OPTION_FLAGS_EXTENSION: + return EQ(a->extended_flags, b->extended_flags); + + case SD_NDISC_OPTION_DNSSL: + return + strv_equal(a->dnssl.domains, b->dnssl.domains) && + EQ(a->dnssl.lifetime, b->dnssl.lifetime) && + EQ(a->dnssl.valid_until, b->dnssl.valid_until); + + case SD_NDISC_OPTION_CAPTIVE_PORTAL: + return streq(a->captive_portal, b->captive_portal); + + case SD_NDISC_OPTION_PREF64: + return + EQ(a->prefix64.prefixlen, b->prefix64.prefixlen) && + in6_addr_equal(&a->prefix64.prefix, &b->prefix64.prefix) && + EQ(a->prefix64.lifetime, b->prefix64.lifetime) && + EQ(a->prefix64.valid_until, b->prefix64.valid_until); + + case SD_NDISC_OPTION_ENCRYPTED_DNS: + if (!EQ(a->encrypted_dns.lifetime, b->encrypted_dns.lifetime) || + !EQ(a->encrypted_dns.valid_until, b->encrypted_dns.valid_until) || + !EQ(a->encrypted_dns.resolver->priority, b->encrypted_dns.resolver->priority) || + !streq_ptr(a->encrypted_dns.resolver->auth_name, b->encrypted_dns.resolver->auth_name) || + !EQ(a->encrypted_dns.resolver->family, b->encrypted_dns.resolver->family) || + !EQ(a->encrypted_dns.resolver->n_addrs, b->encrypted_dns.resolver->n_addrs)) + return false; + + for (size_t i = 0; i < a->encrypted_dns.resolver->n_addrs; i++) + if (in_addr_equal(a->encrypted_dns.resolver->family, + &a->encrypted_dns.resolver->addrs[i], + &b->encrypted_dns.resolver->addrs[i]) <= 0) + return false; + + return + EQ(a->encrypted_dns.resolver->transports, b->encrypted_dns.resolver->transports) && + EQ(a->encrypted_dns.resolver->port, b->encrypted_dns.resolver->port) && + streq_ptr(a->encrypted_dns.resolver->dohpath, b->encrypted_dns.resolver->dohpath); + break; + default: + return memcmp_nn(a->raw.bytes, a->raw.length, b->raw.bytes, b->raw.length) == 0; + } +} + +static void ndisc_round_trip(Set *options) { + _cleanup_close_pair_ int pair[2] = EBADF_PAIR; + ASSERT_OK(socketpair(AF_UNIX, SOCK_DGRAM | SOCK_CLOEXEC | SOCK_NONBLOCK, /* protocol= */ 0, pair)); + + static const struct nd_router_advert ra = { + .nd_ra_type = ND_ROUTER_ADVERT, + }; + + ASSERT_OK(ndisc_send(pair[1], &(struct in6_addr) {}, &ra.nd_ra_hdr, options, now(CLOCK_BOOTTIME))); + + _cleanup_(icmp6_packet_unrefp) ICMP6Packet *packet = NULL; + ASSERT_OK(icmp6_packet_receive(pair[0], &packet)); + + _cleanup_set_free_ Set *reparsed = NULL; + ASSERT_OK(ndisc_parse_options(packet, &reparsed)); + + ASSERT_EQ(set_size(options), set_size(reparsed)); + + sd_ndisc_option *option; + SET_FOREACH(option, options) { + bool found = false; + sd_ndisc_option *p; + SET_FOREACH(p, reparsed) + if (ndisc_option_equal(option, p)) { + found = true; + break; + } + ASSERT_TRUE(found); + } +} + +TEST(ndisc_mac) { + _cleanup_set_free_ Set *options = NULL; + ASSERT_OK(ndisc_option_set_link_layer_address( + &options, SD_NDISC_OPTION_SOURCE_LL_ADDRESS, + &(struct ether_addr) {{ 'A', 'B', 'C', '1', '2', '3' }})); + ASSERT_OK(ndisc_option_set_link_layer_address( + &options, SD_NDISC_OPTION_TARGET_LL_ADDRESS, + &(struct ether_addr) {{ 'A', 'B', 'C', '1', '2', '4' }})); + ndisc_round_trip(options); +} + +static void append_prefix( + Set **options, + uint8_t flags, + uint8_t prefixlen, + const char *addr, + usec_t valid_lifetime, + usec_t preferred_lifetime) { + + union in_addr_union a; + ASSERT_OK(in_addr_from_string(AF_INET6, addr, &a)); + ASSERT_OK(ndisc_option_set_prefix(options, flags, prefixlen, &a.in6, + valid_lifetime, preferred_lifetime, + USEC_INFINITY, USEC_INFINITY)); +} + +TEST(ndisc_prefix) { + _cleanup_set_free_ Set *options = NULL; + append_prefix(&options, + /* flags= */ 0, + /* prefixlen= */ 72, + "dead:beef:0:1::", + 2 * USEC_PER_HOUR, + USEC_PER_HOUR); + append_prefix(&options, + ND_OPT_PI_FLAG_AUTO, + /* prefixlen= */ 64, + "dead:beef:0:2::", + 3 * USEC_PER_HOUR, + USEC_PER_HOUR); + append_prefix(&options, + ND_OPT_PI_FLAG_ONLINK, + /* prefixlen= */ 64, + "dead:beef:0:3::", + 4 * USEC_PER_HOUR, + USEC_PER_HOUR); + ndisc_round_trip(options); +} + +TEST(ndisc_mtu) { + _cleanup_set_free_ Set *options = NULL; + ASSERT_OK(ndisc_option_set_mtu(&options, 4096)); + ndisc_round_trip(options); +} + +TEST(ndisc_home_agent) { + _cleanup_set_free_ Set *options = NULL; + ASSERT_OK(ndisc_option_set_home_agent(&options, 128, USEC_PER_HOUR, USEC_INFINITY)); + ndisc_round_trip(options); +} + +static void append_route( + Set **options, + uint8_t preference, + uint8_t prefixlen, + const char *prefix, + usec_t lifetime) { + + union in_addr_union a; + ASSERT_OK(in_addr_from_string(AF_INET6, prefix, &a)); + ASSERT_OK(ndisc_option_set_route(options, preference, prefixlen, &a.in6, lifetime, USEC_INFINITY)); +} + +TEST(ndisc_route) { + _cleanup_set_free_ Set *options = NULL; + append_route(&options, + /* preference= */ SD_NDISC_PREFERENCE_LOW, + /* prefixlen= */ 64, + "dead:beef:0:1::", + 1 * USEC_PER_HOUR); + append_route(&options, + /* preference= */ SD_NDISC_PREFERENCE_MEDIUM, + /* prefixlen= */ 72, + "dead:beef:0:2::", + 2 * USEC_PER_HOUR); + append_route(&options, + /* preference= */ SD_NDISC_PREFERENCE_HIGH, + /* prefixlen= */ 80, + "dead:beef:0:3::", + 3 * USEC_PER_HOUR); + ndisc_round_trip(options); +} + +static void append_rdnss( + Set **options, + char* const* addrs, + usec_t lifetime) { + + _cleanup_free_ struct in6_addr *addresses = ASSERT_NOT_NULL(new(struct in6_addr, strv_length(addrs))); + size_t n = 0; + STRV_FOREACH(s, addrs) { + union in_addr_union a; + ASSERT_OK(in_addr_from_string(AF_INET6, *s, &a)); + addresses[n++] = a.in6; + } + + ASSERT_OK(ndisc_option_set_rdnss(options, n, addresses, lifetime, USEC_INFINITY)); +} + +TEST(ndisc_rdnss) { + _cleanup_set_free_ Set *options = NULL; + append_rdnss(&options, + STRV_MAKE("dead:beef::1"), + 1 * USEC_PER_HOUR); + append_rdnss(&options, + STRV_MAKE("dead:beef::2", "dead:beef::3", "dead:beef::4"), + 2 * USEC_PER_HOUR); + ndisc_round_trip(options); +} + +TEST(ndisc_flags_extension) { + _cleanup_set_free_ Set *options = NULL; + ASSERT_OK(ndisc_option_add_flags_extension(&options, /* offset= */ 0, UINT64_C(0x0066554433221100))); + ndisc_round_trip(options); +} + +TEST(ndisc_dnssl) { + _cleanup_set_free_ Set *options = NULL; + ASSERT_OK(ndisc_option_set_dnssl( + &options, + STRV_MAKE("dns1.example.com"), + 1 * USEC_PER_HOUR, USEC_INFINITY)); + ASSERT_OK(ndisc_option_set_dnssl( + &options, + STRV_MAKE("dns2.example.com", "dns3.example.com", "dns4.example.com"), + 2 * USEC_PER_HOUR, USEC_INFINITY)); + ndisc_round_trip(options); +} + +TEST(ndisc_captive_portal) { + _cleanup_set_free_ Set *options = NULL; + ndisc_option_set_captive_portal(&options, "https://captive-portal.example.com"); + ndisc_round_trip(options); +} + +static void append_prefix64( + Set **options, + uint8_t prefixlen, + const char *prefix, + usec_t lifetime) { + + union in_addr_union a; + ASSERT_OK(in_addr_from_string(AF_INET6, prefix, &a)); + ASSERT_OK(ndisc_option_set_prefix64(options, prefixlen, &a.in6, lifetime, USEC_INFINITY)); +} + +TEST(ndisc_prefix64) { + _cleanup_set_free_ Set *options = NULL; + append_prefix64(&options, + /* prefixlen= */ 56, + "dead:beef:0:1::", + 1 * USEC_PER_HOUR); + append_prefix64(&options, + /* prefixlen= */ 64, + "dead:beef:0:2::", + 2 * USEC_PER_HOUR); + append_prefix64(&options, + /* prefixlen= */ 96, + "dead:beef:0:3::", + 3 * USEC_PER_HOUR); + ndisc_round_trip(options); +} + +static void append_dnr( + Set **options, + uint16_t priority, + const char *auth_name, + char* const* addrs, + sd_dns_alpn_flags transports, + uint16_t port, + const char *dohpath) { + + _cleanup_(sd_dns_resolver_unrefp) sd_dns_resolver *res = ASSERT_NOT_NULL(new0(sd_dns_resolver, 1)); + res->priority = priority; + ASSERT_OK(strdup_to(&res->auth_name, auth_name)); + res->addrs = ASSERT_NOT_NULL(new(union in_addr_union, strv_length(addrs))); + STRV_FOREACH(a, addrs) + ASSERT_OK(in_addr_from_string_auto(*a, &res->family, &res->addrs[res->n_addrs++])); + res->transports = transports; + res->port = port; + ASSERT_OK(strdup_to(&res->dohpath, dohpath)); + + ASSERT_OK(ndisc_option_set_encrypted_dns(options, TAKE_PTR(res), 3600 * USEC_PER_SEC, USEC_INFINITY)); +} + +TEST(ndisc_dnr) { + _cleanup_set_free_ Set *options = NULL; + append_dnr(&options, + /* priority= */ 1, + "dns.example.com", + STRV_MAKE("2001:db8::1"), + SD_DNS_ALPN_DOT, + /* port= */ 0, + /* dohpath= */ NULL); + append_dnr(&options, + /* priority= */ 5, + "resolver.example.org", + STRV_MAKE("2001:db8::53", "2001:db8::54", "2001:db8::55"), + SD_DNS_ALPN_DOT | SD_DNS_ALPN_HTTP_2_TLS | SD_DNS_ALPN_HTTP_3 | SD_DNS_ALPN_DOQ, + /* port= */ 853, + "/dns-query{?dns}"); + ndisc_round_trip(options); +} + +DEFINE_TEST_MAIN(LOG_DEBUG);