From 8f4f6307313ccad0b068697bb054a105e5a50e92 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 14 Jul 2022 04:56:07 +0900 Subject: [PATCH 1/7] Revert "network: configure DHCP clients after MAC address is assigned" This reverts commit f42d41cc5f9cd8cac538a1c30fda04d346b5bae2. DHCPv6 client does not require MAC address. DHCPv4 client will be handled in a different way in a later commit. Partially fixes #23546. --- src/network/networkd-dhcp-common.c | 7 ------- src/network/networkd-dhcp4.c | 5 ----- src/network/networkd-dhcp6.c | 5 ----- 3 files changed, 17 deletions(-) diff --git a/src/network/networkd-dhcp-common.c b/src/network/networkd-dhcp-common.c index 43c8ca42fe..45faf4f128 100644 --- a/src/network/networkd-dhcp-common.c +++ b/src/network/networkd-dhcp-common.c @@ -61,13 +61,6 @@ bool link_dhcp_enabled(Link *link, int family) { if (link->iftype == ARPHRD_CAN) return false; - if (!IN_SET(link->hw_addr.length, ETH_ALEN, INFINIBAND_ALEN) && - !streq_ptr(link->kind, "wwan")) - /* Currently, only interfaces whose MAC address length is ETH_ALEN or INFINIBAND_ALEN - * are supported. Note, wwan interfaces may be assigned MAC address slightly later. - * Hence, let's wait for a while.*/ - return false; - if (!link->network) return false; diff --git a/src/network/networkd-dhcp4.c b/src/network/networkd-dhcp4.c index 5770b48767..20db4f5f83 100644 --- a/src/network/networkd-dhcp4.c +++ b/src/network/networkd-dhcp4.c @@ -1545,11 +1545,6 @@ static int dhcp4_process_request(Request *req, Link *link, void *userdata) { if (!IN_SET(link->state, LINK_STATE_CONFIGURING, LINK_STATE_CONFIGURED)) return 0; - if (!IN_SET(link->hw_addr.length, ETH_ALEN, INFINIBAND_ALEN) || - hw_addr_is_null(&link->hw_addr)) - /* No MAC address is assigned to the hardware, or non-supported MAC address length. */ - return 0; - r = dhcp4_configure_duid(link); if (r <= 0) return r; diff --git a/src/network/networkd-dhcp6.c b/src/network/networkd-dhcp6.c index 3dc34f0e52..72ec6826bd 100644 --- a/src/network/networkd-dhcp6.c +++ b/src/network/networkd-dhcp6.c @@ -738,11 +738,6 @@ static int dhcp6_process_request(Request *req, Link *link, void *userdata) { if (!IN_SET(link->state, LINK_STATE_CONFIGURING, LINK_STATE_CONFIGURED)) return 0; - if (!IN_SET(link->hw_addr.length, ETH_ALEN, INFINIBAND_ALEN) || - hw_addr_is_null(&link->hw_addr)) - /* No MAC address is assigned to the hardware, or non-supported MAC address length. */ - return 0; - r = dhcp_configure_duid(link, link_get_dhcp6_duid(link)); if (r <= 0) return r; From 7e2f684e1fa391bd511b52bbbe4a3971cdc5ec33 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 14 Jul 2022 05:05:32 +0900 Subject: [PATCH 2/7] network: dhcp4: disable DHCPv4 client on interfaces with non-supported types Replaces f42d41cc5f9cd8cac538a1c30fda04d346b5bae2. --- src/network/networkd-dhcp-common.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/network/networkd-dhcp-common.c b/src/network/networkd-dhcp-common.c index 45faf4f128..ce5262fcd6 100644 --- a/src/network/networkd-dhcp-common.c +++ b/src/network/networkd-dhcp-common.c @@ -52,6 +52,10 @@ bool link_dhcp_enabled(Link *link, int family) { assert(link); assert(IN_SET(family, AF_INET, AF_INET6)); + /* Currently, sd-dhcp-client supports only ethernet and infiniband. */ + if (family == AF_INET && !IN_SET(link->iftype, ARPHRD_ETHER, ARPHRD_INFINIBAND)) + return false; + if (family == AF_INET6 && !socket_ipv6_is_supported()) return false; From 56bb67180a18562b04cb66e07f7fa53d6014ecf8 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 14 Jul 2022 06:50:15 +0900 Subject: [PATCH 3/7] network: NDisc does not require MAC address This effectively revert ba4c7184b320bb8698d470530d46a6c94641cc6e. Fixes #23546. --- src/network/networkd-ndisc.c | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/network/networkd-ndisc.c b/src/network/networkd-ndisc.c index f66745a4ff..8ace451f7f 100644 --- a/src/network/networkd-ndisc.c +++ b/src/network/networkd-ndisc.c @@ -38,12 +38,6 @@ bool link_ipv6_accept_ra_enabled(Link *link) { if (link->iftype == ARPHRD_CAN) return false; - if (link->hw_addr.length != ETH_ALEN && !streq_ptr(link->kind, "wwan")) - /* Currently, only interfaces whose MAC address length is ETH_ALEN are supported. - * Note, wwan interfaces may be assigned MAC address slightly later. - * Hence, let's wait for a while.*/ - return false; - if (!link->network) return false; @@ -1106,10 +1100,6 @@ static int ndisc_process_request(Request *req, Link *link, void *userdata) { if (!IN_SET(link->state, LINK_STATE_CONFIGURING, LINK_STATE_CONFIGURED)) return 0; - if (link->hw_addr.length != ETH_ALEN || hw_addr_is_null(&link->hw_addr)) - /* No MAC address is assigned to the hardware, or non-supported MAC address length. */ - return 0; - r = ndisc_configure(link); if (r < 0) return log_link_warning_errno(link, r, "Failed to configure IPv6 Router Discovery: %m"); From 3be64aa462642de9fbe9f95536f8d167c19994bb Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 14 Jul 2022 05:23:01 +0900 Subject: [PATCH 4/7] network: do not set invalid MAC address for non-ethernet interface --- src/network/networkd-link.c | 2 +- src/network/networkd-ndisc.c | 8 +++++--- src/network/networkd-radv.c | 11 ++++++++--- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 29a5609483..b314f5db8f 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -2192,7 +2192,7 @@ static int link_update_hardware_address(Link *link, sd_netlink_message *message) if (r < 0) return log_link_debug_errno(link, r, "Could not update MAC address for Router Advertisement: %m"); - if (link->ndisc) { + if (link->ndisc && link->hw_addr.length == ETH_ALEN) { r = sd_ndisc_set_mac(link->ndisc, &link->hw_addr.ether); if (r < 0) return log_link_debug_errno(link, r, "Could not update MAC for NDisc: %m"); diff --git a/src/network/networkd-ndisc.c b/src/network/networkd-ndisc.c index 8ace451f7f..2b02978ab4 100644 --- a/src/network/networkd-ndisc.c +++ b/src/network/networkd-ndisc.c @@ -1054,9 +1054,11 @@ static int ndisc_configure(Link *link) { if (r < 0) return r; - r = sd_ndisc_set_mac(link->ndisc, &link->hw_addr.ether); - if (r < 0) - return r; + if (link->hw_addr.length == ETH_ALEN) { + r = sd_ndisc_set_mac(link->ndisc, &link->hw_addr.ether); + if (r < 0) + return r; + } r = sd_ndisc_set_ifindex(link->ndisc, link->ifindex); if (r < 0) diff --git a/src/network/networkd-radv.c b/src/network/networkd-radv.c index 26448b518d..32ee7aa7ab 100644 --- a/src/network/networkd-radv.c +++ b/src/network/networkd-radv.c @@ -458,9 +458,11 @@ static int radv_configure(Link *link) { if (r < 0) return r; - r = sd_radv_set_mac(link->radv, &link->hw_addr.ether); - if (r < 0) - return r; + if (link->hw_addr.length == ETH_ALEN) { + r = sd_radv_set_mac(link->radv, &link->hw_addr.ether); + if (r < 0) + return r; + } r = sd_radv_set_ifindex(link->radv, link->ifindex); if (r < 0) @@ -518,6 +520,9 @@ int radv_update_mac(Link *link) { if (!link->radv) return 0; + if (link->hw_addr.length != ETH_ALEN) + return 0; + restart = sd_radv_is_running(link->radv); r = sd_radv_stop(link->radv); From 50309ff7857b0dfcec07172bdbfd183ca3f4ca88 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 14 Jul 2022 08:37:53 +0900 Subject: [PATCH 5/7] sd-dhcp6-client: add more debugging logs on parsing message --- src/libsystemd-network/sd-dhcp6-lease.c | 34 ++++++++++++++++--------- 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/src/libsystemd-network/sd-dhcp6-lease.c b/src/libsystemd-network/sd-dhcp6-lease.c index 2475cdb7ed..f588514cb6 100644 --- a/src/libsystemd-network/sd-dhcp6-lease.c +++ b/src/libsystemd-network/sd-dhcp6-lease.c @@ -467,7 +467,9 @@ static int dhcp6_lease_parse_message( r = dhcp6_option_parse(message->options, len, &offset, &optcode, &optlen, &optval); if (r < 0) - return r; + return log_dhcp6_client_errno(client, r, + "Failed to parse option header at offset %zu of total length %zu: %m", + offset, len); switch (optcode) { case SD_DHCP6_OPTION_CLIENTID: @@ -477,7 +479,7 @@ static int dhcp6_lease_parse_message( r = dhcp6_lease_set_clientid(lease, optval, optlen); if (r < 0) - return r; + return log_dhcp6_client_errno(client, r, "Failed to set client ID: %m"); break; @@ -488,17 +490,17 @@ static int dhcp6_lease_parse_message( r = dhcp6_lease_set_serverid(lease, optval, optlen); if (r < 0) - return r; + return log_dhcp6_client_errno(client, r, "Failed to set server ID: %m"); break; case SD_DHCP6_OPTION_PREFERENCE: if (optlen != 1) - return -EINVAL; + return log_dhcp6_client_errno(client, SYNTHETIC_ERRNO(EINVAL), "Received invalid length for preference."); r = dhcp6_lease_set_preference(lease, optval[0]); if (r < 0) - return r; + return log_dhcp6_client_errno(client, r, "Failed to set preference: %m"); break; @@ -507,7 +509,7 @@ static int dhcp6_lease_parse_message( r = dhcp6_option_parse_status(optval, optlen, &msg); if (r < 0) - return r; + return log_dhcp6_client_errno(client, r, "Failed to parse status code: %m"); if (r > 0) return log_dhcp6_client_errno(client, SYNTHETIC_ERRNO(EINVAL), @@ -527,9 +529,11 @@ static int dhcp6_lease_parse_message( r = dhcp6_option_parse_ia(client, client->ia_na.header.id, optcode, optlen, optval, &ia); if (r == -ENOMEM) - return r; - if (r < 0) + return log_oom_debug(); + if (r < 0) { + log_dhcp6_client_errno(client, r, "Failed to parse IA_NA option, ignoring: %m"); continue; + } if (lease->ia_na) { log_dhcp6_client(client, "Received duplicate matching IA_NA option, ignoring."); @@ -550,9 +554,11 @@ static int dhcp6_lease_parse_message( r = dhcp6_option_parse_ia(client, client->ia_pd.header.id, optcode, optlen, optval, &ia); if (r == -ENOMEM) - return r; - if (r < 0) + return log_oom_debug(); + if (r < 0) { + log_dhcp6_client_errno(client, r, "Failed to parse IA_PD option, ignoring: %m"); continue; + } if (lease->ia_pd) { log_dhcp6_client(client, "Received duplicate matching IA_PD option, ignoring."); @@ -564,9 +570,12 @@ static int dhcp6_lease_parse_message( break; } case SD_DHCP6_OPTION_RAPID_COMMIT: + if (optlen != 0) + log_dhcp6_client(client, "Received rapid commit option with an invalid length (%zu), ignoring.", optlen); + r = dhcp6_lease_set_rapid_commit(lease); if (r < 0) - return r; + return log_dhcp6_client_errno(client, r, "Failed to set rapid commit flag: %m"); break; @@ -607,7 +616,8 @@ static int dhcp6_lease_parse_message( case SD_DHCP6_OPTION_INFORMATION_REFRESH_TIME: if (optlen != 4) - return -EINVAL; + return log_dhcp6_client_errno(client, SYNTHETIC_ERRNO(EINVAL), + "Received information refresh time option with an invalid length (%zu).", optlen); irt = unaligned_read_be32((be32_t *) optval) * USEC_PER_SEC; break; From d81b5a191e6f06fe51befe7c7fb20cff4ff4f762 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 14 Jul 2022 10:13:00 +0900 Subject: [PATCH 6/7] sd-dhcp6-client: allow NULL option value when length is zero --- src/libsystemd-network/dhcp6-option.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libsystemd-network/dhcp6-option.c b/src/libsystemd-network/dhcp6-option.c index 63d3f60513..0f46249533 100644 --- a/src/libsystemd-network/dhcp6-option.c +++ b/src/libsystemd-network/dhcp6-option.c @@ -525,7 +525,7 @@ int dhcp6_option_parse( } int dhcp6_option_parse_status(const uint8_t *data, size_t data_len, char **ret_status_message) { - assert(data); + assert(data || data_len == 0); if (data_len < sizeof(uint16_t)) return -EBADMSG; @@ -803,7 +803,7 @@ int dhcp6_option_parse_addresses( struct in6_addr **addrs, size_t *count) { - assert(optval); + assert(optval || optlen == 0); assert(addrs); assert(count); @@ -826,8 +826,8 @@ static int parse_domain(const uint8_t **data, size_t *len, char **ret) { int r; assert(data); - assert(*data); assert(len); + assert(*data || *len == 0); assert(ret); optval = *data; @@ -891,7 +891,7 @@ int dhcp6_option_parse_domainname(const uint8_t *optval, size_t optlen, char **r _cleanup_free_ char *domain = NULL; int r; - assert(optval); + assert(optval || optlen == 0); assert(ret); r = parse_domain(&optval, &optlen, &domain); @@ -910,7 +910,7 @@ int dhcp6_option_parse_domainname_list(const uint8_t *optval, size_t optlen, cha _cleanup_strv_free_ char **names = NULL; int r; - assert(optval); + assert(optval || optlen == 0); assert(ret); if (optlen <= 1) From 68870a46b3b5d3d5b51d1c27b4128b8fb5188ecb Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 14 Jul 2022 09:23:09 +0900 Subject: [PATCH 7/7] sd-dhcp6-client: fix off-by-one error in parsing dhcp6 options This fixes error in parsing message when the rapid commit option is located at the end of the message. Fixes an issure reported in #24002. --- src/libsystemd-network/dhcp6-option.c | 4 +-- src/libsystemd-network/test-dhcp6-client.c | 42 ++++++++++++++++++++++ 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/src/libsystemd-network/dhcp6-option.c b/src/libsystemd-network/dhcp6-option.c index 0f46249533..5e91e86f53 100644 --- a/src/libsystemd-network/dhcp6-option.c +++ b/src/libsystemd-network/dhcp6-option.c @@ -508,7 +508,7 @@ int dhcp6_option_parse( if (buflen < offsetof(DHCP6Option, data)) return -EBADMSG; - if (*offset >= buflen - offsetof(DHCP6Option, data)) + if (*offset > buflen - offsetof(DHCP6Option, data)) return -EBADMSG; len = unaligned_read_be16(buf + *offset + offsetof(DHCP6Option, len)); @@ -518,7 +518,7 @@ int dhcp6_option_parse( *ret_option_code = unaligned_read_be16(buf + *offset + offsetof(DHCP6Option, code)); *ret_option_data_len = len; - *ret_option_data = buf + *offset + offsetof(DHCP6Option, data); + *ret_option_data = len == 0 ? NULL : buf + *offset + offsetof(DHCP6Option, data); *offset += offsetof(DHCP6Option, data) + len; return 0; diff --git a/src/libsystemd-network/test-dhcp6-client.c b/src/libsystemd-network/test-dhcp6-client.c index 95a6c514c6..e5c09f9664 100644 --- a/src/libsystemd-network/test-dhcp6-client.c +++ b/src/libsystemd-network/test-dhcp6-client.c @@ -443,6 +443,48 @@ TEST(client_parse_message_issue_22099) { assert_se(dhcp6_lease_new_from_message(client, (const DHCP6Message*) msg, sizeof(msg), NULL, NULL, &lease) >= 0); } +TEST(client_parse_message_issue_24002) { + static const uint8_t msg[] = { + /* Message Type */ + 0x07, + /* Transaction ID */ + 0x0e, 0xa5, 0x7c, + /* Client ID */ + 0x00, SD_DHCP6_OPTION_CLIENTID, 0x00, 0x0e, + 0x00, 0x02, /* DUID-EN */ + 0x00, 0x00, 0xab, 0x11, /* pen */ + 0x5c, 0x6b, 0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff, /* id */ + /* Server ID */ + 0x00, 0x02, 0x00, 0x1a, + 0x00, 0x02, 0x00, 0x00, 0x05, 0x83, 0x30, 0x63, 0x3a, 0x38, 0x00, 0x11, 0x22, 0x33, 0x44, 0x55, + 0x66, 0x77, 0x88, 0x99, 0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff, + /* IA_PD */ + 0x00, 0x19, 0x00, 0x29, + 0xaa, 0xbb, 0xcc, 0xdd, /* iaid */ + 0x00, 0x00, 0x03, 0x84, /* lifetime (T1) */ + 0x00, 0x00, 0x05, 0xa0, /* lifetime (T2) */ + /* IA_PD (iaprefix suboption) */ + 0x00, 0x1a, 0x00, 0x19, + 0x00, 0x00, 0x07, 0x08, /* preferred lifetime */ + 0x00, 0x00, 0x38, 0x40, /* valid lifetime */ + 0x38, /* prefixlen */ + 0x20, 0x03, 0x00, 0xff, 0xaa, 0xbb, 0xcc, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, /* prefix */ + /* Rapid commit */ + 0x00, 0x0e, 0x00, 0x00, + }; + static const uint8_t duid[] = { + 0x00, 0x00, 0xab, 0x11, 0x5c, 0x6b, 0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff, + }; + _cleanup_(sd_dhcp6_client_unrefp) sd_dhcp6_client *client = NULL; + _cleanup_(sd_dhcp6_lease_unrefp) sd_dhcp6_lease *lease = NULL; + + assert_se(sd_dhcp6_client_new(&client) >= 0); + assert_se(sd_dhcp6_client_set_iaid(client, 0xaabbccdd) >= 0); + assert_se(sd_dhcp6_client_set_duid(client, 2, duid, sizeof(duid)) >= 0); + + assert_se(dhcp6_lease_new_from_message(client, (const DHCP6Message*) msg, sizeof(msg), NULL, NULL, &lease) >= 0); +} + static const uint8_t msg_information_request[] = { /* Message type */ DHCP6_MESSAGE_INFORMATION_REQUEST,