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.

diff --git a/src/libsystemd-network/dhcp-client-internal.h b/src/libsystemd-network/dhcp-client-internal.h
index 07e61b0d760e111d5d437c18b9d9a908a2d0ee80..b59f0e632ddb6f23ce17928fb5b514d67337e6f7 100644 (file)
--- 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 (file)
index 0000000..a802cb5
--- /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 (file)
index 0000000..2dbb6a0
--- /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 7a59faff6312bdb467bd9f8e1a7bd3a6a7bb2285..21f693c4873a3454656f442f82863b484cb46325 100644 (file)
--- a/src/libsystemd-network/fuzz-dhcp-client.c
+++ b/src/libsystemd-network/fuzz-dhcp-client.c
@@ -3,6 +3,7 @@
 #include <sys/socket.h>
 
 #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 2eeabc075e04f49f4b116ae968d9bdbd8352196c..d1e13d99b536fc87902cbdb8e371df6ec0abafaa 100644 (file)
--- 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 6bc96a47e7113b65feb2911b19b084ec8ff7b4a7..88a831e9cefc4f89a81ae908eb656cdcde1aa9d1 100644 (file)
--- 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,