From 16d08897bbe1c6c9357bf1a845dd3efe2a5fd316 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 10 Mar 2026 08:57:15 +0900 Subject: [PATCH] sd-dhcp-client: open socket when necessary and close it when unnecessary To make gracefully ignore unexpected messages from outside at unexpected timing. This potentially reduces work load to handle such messages, and slightly reduces attack surface by malicious DHCP messages. This also makes the socket fd is owned by the relevant IO event source. Except for the performance optimization and security hardening, this should not change any behaviors. So, just refactoring. --- src/libsystemd-network/dhcp-client-internal.h | 12 +- src/libsystemd-network/dhcp-client-send.c | 187 ++++++++++++++++++ src/libsystemd-network/dhcp-client-send.h | 18 ++ src/libsystemd-network/fuzz-dhcp-client.c | 1 + src/libsystemd-network/meson.build | 1 + src/libsystemd-network/sd-dhcp-client.c | 151 ++------------ 6 files changed, 239 insertions(+), 131 deletions(-) create mode 100644 src/libsystemd-network/dhcp-client-send.c create mode 100644 src/libsystemd-network/dhcp-client-send.h diff --git a/src/libsystemd-network/dhcp-client-internal.h b/src/libsystemd-network/dhcp-client-internal.h index 07e61b0d76..b59f0e632d 100644 --- a/src/libsystemd-network/dhcp-client-internal.h +++ b/src/libsystemd-network/dhcp-client-internal.h @@ -38,7 +38,6 @@ struct sd_dhcp_client { sd_device *dev; - int fd; uint16_t port; uint16_t server_port; union sockaddr_union link; @@ -93,6 +92,17 @@ int dhcp_client_set_state_callback( void *userdata); int dhcp_client_get_state(sd_dhcp_client *client); +int client_receive_message_raw( + sd_event_source *s, + int fd, + uint32_t revents, + void *userdata); +int client_receive_message_udp( + sd_event_source *s, + int fd, + uint32_t revents, + void *userdata); + /* If we are invoking callbacks of a dhcp-client, ensure unreffing the * client from the callback doesn't destroy the object we are working * on */ diff --git a/src/libsystemd-network/dhcp-client-send.c b/src/libsystemd-network/dhcp-client-send.c new file mode 100644 index 0000000000..a802cb5e86 --- /dev/null +++ b/src/libsystemd-network/dhcp-client-send.c @@ -0,0 +1,187 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#include "sd-event.h" + +#include "dhcp-client-internal.h" +#include "dhcp-client-send.h" +#include "dhcp-lease-internal.h" /* IWYU pragma: keep */ +#include "dhcp-network.h" +#include "dhcp-packet.h" +#include "fd-util.h" +#include "socket-util.h" + +static int client_get_socket(sd_dhcp_client *client, int domain) { + int r, d, fd; + + assert(client); + assert(IN_SET(domain, AF_PACKET, AF_INET)); + + if (!client->receive_message) + return -EBADF; + + fd = sd_event_source_get_io_fd(client->receive_message); + if (fd < 0) + return fd; + + r = getsockopt_int(fd, SOL_SOCKET, SO_DOMAIN, &d); + if (r < 0) + return r; + + if (d != domain) + return -EBADF; + + return fd; +} + +static int client_setup_io_event( + sd_dhcp_client *client, + int fd, + sd_event_io_handler_t callback, + const char *description) { + + int r; + + assert(client); + assert(fd >= 0); + assert(callback); + assert(description); + + _cleanup_(sd_event_source_unrefp) sd_event_source *s = NULL; + r = sd_event_add_io(client->event, &s, fd, EPOLLIN, callback, client); + if (r < 0) + return r; + + r = sd_event_source_set_priority(s, client->event_priority); + if (r < 0) + return r; + + r = sd_event_source_set_description(s, description); + if (r < 0) + return r; + + r = sd_event_source_set_io_fd_own(s, true); + if (r < 0) + return r; + + sd_event_source_disable_unref(client->receive_message); + client->receive_message = TAKE_PTR(s); + return 0; +} + +int dhcp_client_send_raw( + sd_dhcp_client *client, + bool expect_reply, + DHCPPacket *packet, + size_t optoffset) { + + _cleanup_close_ int fd_close = -EBADF; + int r, fd; + + assert(client); + assert(packet); + + fd = client_get_socket(client, AF_PACKET); + if (fd < 0) { + fd = dhcp_network_bind_raw_socket( + client->ifindex, + &client->link, + client->xid, + &client->hw_addr, + &client->bcast_addr, + client->arp_type, + client->port, + client->socket_priority_set, + client->socket_priority); + if (fd < 0) + return fd; + + fd_close = fd; + } + + dhcp_packet_append_ip_headers( + packet, + INADDR_ANY, + client->port, + INADDR_BROADCAST, + client->server_port, + sizeof(DHCPPacket) + optoffset, + client->ip_service_type); + + r = dhcp_network_send_raw_socket( + fd, + &client->link, + packet, + sizeof(DHCPPacket) + optoffset); + if (r < 0) + return r; + + if (!expect_reply) { + /* We do not expect any replies, hence stop the IO event source if enabled. */ + client->receive_message = sd_event_source_disable_unref(client->receive_message); + return 0; + } + + if (fd_close < 0) + return 0; /* Already opened socket is reused. Not necessary to setup new IO event source. */ + + r = client_setup_io_event(client, fd, client_receive_message_raw, "dhcp4-receive-message-raw"); + if (r < 0) + return r; + + TAKE_FD(fd_close); + return 0; +} + +int dhcp_client_send_udp( + sd_dhcp_client *client, + bool expect_reply, + DHCPPacket *packet, + size_t optoffset) { + + _cleanup_close_ int fd_close = -EBADF; + int r, fd; + + assert(client); + assert(packet); + + if (!client->lease || client->lease->address == 0) + return -EADDRNOTAVAIL; + + fd = client_get_socket(client, AF_INET); + if (fd < 0) { + fd = dhcp_network_bind_udp_socket( + client->ifindex, + client->lease->address, + client->port, + client->ip_service_type); + if (fd < 0) + return fd; + + fd_close = fd; + } + + r = dhcp_network_send_udp_socket( + fd, + client->lease->server_address, + client->server_port, + &packet->dhcp, + sizeof(DHCPMessage) + optoffset); + if (r < 0) + return r; + + if (!expect_reply) { + /* We do not expect any replies, hence stop the IO event source if enabled. */ + client->receive_message = sd_event_source_disable_unref(client->receive_message); + return 0; + } + + if (fd_close < 0) + return 0; /* Already opened socket is reused. Not necessary to setup new IO event source. */ + + r = client_setup_io_event(client, fd, client_receive_message_udp, "dhcp4-receive-message-udp"); + if (r < 0) + return r; + + TAKE_FD(fd_close); + return 0; +} diff --git a/src/libsystemd-network/dhcp-client-send.h b/src/libsystemd-network/dhcp-client-send.h new file mode 100644 index 0000000000..2dbb6a0878 --- /dev/null +++ b/src/libsystemd-network/dhcp-client-send.h @@ -0,0 +1,18 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +#pragma once + +#include "sd-forward.h" + +#include "dhcp-protocol.h" + +int dhcp_client_send_raw( + sd_dhcp_client *client, + bool expect_reply, + DHCPPacket *packet, + size_t optoffset); + +int dhcp_client_send_udp( + sd_dhcp_client *client, + bool expect_reply, + DHCPPacket *packet, + size_t optoffset); diff --git a/src/libsystemd-network/fuzz-dhcp-client.c b/src/libsystemd-network/fuzz-dhcp-client.c index 7a59faff63..21f693c487 100644 --- a/src/libsystemd-network/fuzz-dhcp-client.c +++ b/src/libsystemd-network/fuzz-dhcp-client.c @@ -3,6 +3,7 @@ #include #include "dhcp-network.h" +#include "fd-util.h" #include "fuzz.h" #include "network-internal.h" #include "sd-dhcp-client.c" diff --git a/src/libsystemd-network/meson.build b/src/libsystemd-network/meson.build index 2eeabc075e..d1e13d99b5 100644 --- a/src/libsystemd-network/meson.build +++ b/src/libsystemd-network/meson.build @@ -2,6 +2,7 @@ libsystemd_network_sources = files( 'arp-util.c', + 'dhcp-client-send.c', 'dhcp-network.c', 'dhcp-option.c', 'dhcp-packet.c', diff --git a/src/libsystemd-network/sd-dhcp-client.c b/src/libsystemd-network/sd-dhcp-client.c index 6bc96a47e7..88a831e9ce 100644 --- a/src/libsystemd-network/sd-dhcp-client.c +++ b/src/libsystemd-network/sd-dhcp-client.c @@ -10,14 +10,13 @@ #include "alloc-util.h" #include "device-util.h" #include "dhcp-client-internal.h" +#include "dhcp-client-send.h" #include "dhcp-lease-internal.h" -#include "dhcp-network.h" #include "dhcp-option.h" #include "dhcp-packet.h" #include "dns-domain.h" #include "errno-util.h" #include "event-util.h" -#include "fd-util.h" #include "hostname-util.h" #include "iovec-util.h" #include "memory-util.h" @@ -73,16 +72,6 @@ static const uint8_t default_req_opts_anonymize[] = { SD_DHCP_OPTION_PRIVATE_PROXY_AUTODISCOVERY, /* 252 */ }; -static int client_receive_message_raw( - sd_event_source *s, - int fd, - uint32_t revents, - void *userdata); -static int client_receive_message_udp( - sd_event_source *s, - int fd, - uint32_t revents, - void *userdata); static void client_stop(sd_dhcp_client *client, int error); static int client_restart(sd_dhcp_client *client); @@ -635,18 +624,22 @@ static int client_notify(sd_dhcp_client *client, int event) { return 0; } -static void client_initialize(sd_dhcp_client *client) { +static void client_disable_event_sources(sd_dhcp_client *client) { assert(client); client->receive_message = sd_event_source_disable_unref(client->receive_message); - client->fd = safe_close(client->fd); - (void) event_source_disable(client->timeout_resend); (void) event_source_disable(client->timeout_t1); (void) event_source_disable(client->timeout_t2); (void) event_source_disable(client->timeout_expire); (void) event_source_disable(client->timeout_ipv6_only_mode); +} + +static void client_initialize(sd_dhcp_client *client) { + assert(client); + + client_disable_event_sources(client); client->discover_attempt = 0; client->request_attempt = 0; @@ -896,18 +889,6 @@ static int client_append_fqdn_option( return r; } -static int dhcp_client_send_raw( - sd_dhcp_client *client, - DHCPPacket *packet, - size_t len) { - - dhcp_packet_append_ip_headers(packet, INADDR_ANY, client->port, - INADDR_BROADCAST, client->server_port, len, client->ip_service_type); - - return dhcp_network_send_raw_socket(client->fd, &client->link, - packet, len); -} - static int client_append_common_discover_request_options(sd_dhcp_client *client, DHCPPacket *packet, size_t *optoffset, size_t optlen) { sd_dhcp_option *j; int r; @@ -1022,7 +1003,7 @@ static int client_send_dhcp_discover(sd_dhcp_client *client) { if (r < 0) return r; - r = dhcp_client_send_raw(client, discover, sizeof(DHCPPacket) + optoffset); + r = dhcp_client_send_raw(client, /* expect_reply= */ true, discover, optoffset); if (r < 0) return r; @@ -1060,7 +1041,7 @@ static int client_send_bootp_discover(sd_dhcp_client *client) { optoffset = 60; } - r = dhcp_client_send_raw(client, discover, sizeof(DHCPPacket) + optoffset); + r = dhcp_client_send_raw(client, /* expect_reply= */ true, discover, optoffset); if (r < 0) return r; @@ -1153,13 +1134,9 @@ static int client_send_request(sd_dhcp_client *client) { return r; if (client->state == DHCP_STATE_RENEWING) - r = dhcp_network_send_udp_socket(client->fd, - client->lease->server_address, - client->server_port, - &request->dhcp, - sizeof(DHCPMessage) + optoffset); + r = dhcp_client_send_udp(client, /* expect_reply= */ true, request, optoffset); else - r = dhcp_client_send_raw(client, request, sizeof(DHCPPacket) + optoffset); + r = dhcp_client_send_raw(client, /* expect_reply= */ true, request, optoffset); if (r < 0) return r; @@ -1328,34 +1305,6 @@ error: return 0; } -static int client_initialize_io_events( - sd_dhcp_client *client, - sd_event_io_handler_t io_callback) { - - int r; - - assert(client); - assert(client->event); - assert(io_callback); - - _cleanup_(sd_event_source_unrefp) sd_event_source *s = NULL; - r = sd_event_add_io(client->event, &s, client->fd, EPOLLIN, io_callback, client); - if (r < 0) - return r; - - r = sd_event_source_set_priority(s, client->event_priority); - if (r < 0) - return r; - - r = sd_event_source_set_description(s, "dhcp4-receive-message"); - if (r < 0) - return r; - - sd_event_source_disable_unref(client->receive_message); - client->receive_message = TAKE_PTR(s); - return 0; -} - static int client_initialize_time_events(sd_dhcp_client *client) { assert(client); assert(client->event); @@ -1375,45 +1324,20 @@ static int client_initialize_time_events(sd_dhcp_client *client) { /* force_reset= */ true); } -static int client_initialize_events(sd_dhcp_client *client, sd_event_io_handler_t io_callback) { - int r; - - assert(client); - assert(io_callback); - - r = client_initialize_io_events(client, io_callback); - if (r < 0) - return r; - - return client_initialize_time_events(client); -} - static int client_start_delayed(sd_dhcp_client *client) { - int r; - assert_return(client, -EINVAL); assert_return(client->event, -EINVAL); assert_return(client->ifindex > 0, -EINVAL); - assert_return(client->fd < 0, -EBUSY); assert_return(client->xid == 0, -EINVAL); assert_return(IN_SET(client->state, DHCP_STATE_STOPPED, DHCP_STATE_INIT_REBOOT), -EBUSY); client->xid = random_u32(); - - r = dhcp_network_bind_raw_socket(client->ifindex, &client->link, client->xid, - &client->hw_addr, &client->bcast_addr, - client->arp_type, client->port, - client->socket_priority_set, client->socket_priority); - if (r < 0) - return r; - client->fd = r; - client->start_time = now(CLOCK_BOOTTIME); if (client->state == DHCP_STATE_STOPPED) client->state = DHCP_STATE_INIT; - return client_initialize_events(client, client_receive_message_raw); + return client_initialize_time_events(client); } static int client_start(sd_dhcp_client *client) { @@ -1452,23 +1376,12 @@ static int client_timeout_t2(sd_event_source *s, uint64_t usec, void *userdata) int r; client->receive_message = sd_event_source_disable_unref(client->receive_message); - client->fd = safe_close(client->fd); client_set_state(client, DHCP_STATE_REBINDING); client->discover_attempt = 0; client->request_attempt = 0; - r = dhcp_network_bind_raw_socket(client->ifindex, &client->link, client->xid, - &client->hw_addr, &client->bcast_addr, - client->arp_type, client->port, - client->socket_priority_set, client->socket_priority); - if (r < 0) { - client_stop(client, r); - return 0; - } - client->fd = r; - - r = client_initialize_events(client, client_receive_message_raw); + r = client_initialize_time_events(client); if (r < 0) client_stop(client, r); @@ -1713,7 +1626,7 @@ static int client_enter_requesting(sd_dhcp_client *client) { assert(client); assert(client->lease); - (void) event_source_disable(client->timeout_resend); + client_disable_event_sources(client); if (client->lease->ipv6_only_preferred_usec > 0) { if (client->ipv6_acquired) { @@ -1905,23 +1818,7 @@ static int client_enter_bound_now(sd_dhcp_client *client, int notify_event) { if (r < 0) log_dhcp_client_errno(client, r, "could not set lease timeouts: %m"); - if (client->bootp) { - client->receive_message = sd_event_source_disable_unref(client->receive_message); - client->fd = safe_close(client->fd); - } else { - r = dhcp_network_bind_udp_socket(client->ifindex, client->lease->address, client->port, client->ip_service_type); - if (r < 0) - return log_dhcp_client_errno(client, r, "could not bind UDP socket: %m"); - - client->receive_message = sd_event_source_disable_unref(client->receive_message); - close_and_replace(client->fd, r); - r = client_initialize_io_events(client, client_receive_message_udp); - if (r < 0) - return r; - } - client_notify(client, notify_event); - return 0; } @@ -1941,8 +1838,9 @@ static int client_enter_bound(sd_dhcp_client *client, int notify_event) { assert(client); assert(client->lease); + client_disable_event_sources(client); + client->start_delay = 0; - (void) event_source_disable(client->timeout_resend); /* RFC 8925 section 3.2 * If the client is in the INIT-REBOOT state, it SHOULD stop the DHCPv4 configuration process or @@ -2098,7 +1996,7 @@ static int client_handle_message(sd_dhcp_client *client, DHCPMessage *message, s return 0; } -static int client_receive_message_udp( +int client_receive_message_udp( sd_event_source *s, int fd, uint32_t revents, @@ -2151,7 +2049,7 @@ static int client_receive_message_udp( return 0; } -static int client_receive_message_raw( +int client_receive_message_raw( sd_event_source *s, int fd, uint32_t revents, @@ -2330,15 +2228,10 @@ static int client_send_release_or_decline(sd_dhcp_client *client, uint8_t type) switch (type) { case DHCP_RELEASE: - r = dhcp_network_send_udp_socket( - client->fd, - client->lease->server_address, - client->server_port, - &packet->dhcp, - sizeof(DHCPMessage) + optoffset); + r = dhcp_client_send_udp(client, /* expect_reply= */ false, packet, optoffset); break; case DHCP_DECLINE: - r = dhcp_client_send_raw(client, packet, sizeof(DHCPPacket) + optoffset); + r = dhcp_client_send_raw(client, /* expect_reply= */ false, packet, optoffset); break; default: assert_not_reached(); @@ -2401,7 +2294,6 @@ int sd_dhcp_client_set_ipv6_connectivity(sd_dhcp_client *client, int have) { int sd_dhcp_client_interrupt_ipv6_only_mode(sd_dhcp_client *client) { assert_return(client, -EINVAL); assert_return(sd_dhcp_client_is_running(client), -ESTALE); - assert_return(client->fd >= 0, -EINVAL); if (sd_event_source_get_enabled(client->timeout_ipv6_only_mode, NULL) <= 0) return 0; @@ -2497,7 +2389,6 @@ int sd_dhcp_client_new(sd_dhcp_client **ret, int anonymize) { .n_ref = 1, .state = DHCP_STATE_STOPPED, .ifindex = -1, - .fd = -EBADF, .mtu = DHCP_MIN_PACKET_SIZE, .port = DHCP_PORT_CLIENT, .server_port = DHCP_PORT_SERVER,