From cb0ba4b41b9581460efea77789f255471d4a069d Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 3 Jan 2024 06:06:49 +0900 Subject: [PATCH 1/3] sd-dhcp-server: verify address of static lease before use --- src/libsystemd-network/sd-dhcp-server-lease.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/libsystemd-network/sd-dhcp-server-lease.c b/src/libsystemd-network/sd-dhcp-server-lease.c index 16880a80b6..db1d0464ae 100644 --- a/src/libsystemd-network/sd-dhcp-server-lease.c +++ b/src/libsystemd-network/sd-dhcp-server-lease.c @@ -136,7 +136,7 @@ sd_dhcp_server_lease* dhcp_server_get_static_lease(sd_dhcp_server *server, const static_lease = hashmap_get(server->static_leases_by_client_id, &req->client_id); if (static_lease) - return static_lease; + goto verify; /* when no lease is found based on the client id fall back to chaddr */ if (!client_id_data_size_is_valid(req->message->hlen)) @@ -145,7 +145,20 @@ sd_dhcp_server_lease* dhcp_server_get_static_lease(sd_dhcp_server *server, const if (sd_dhcp_client_id_set(&client_id, /* type = */ 1, req->message->chaddr, req->message->hlen) < 0) return NULL; - return hashmap_get(server->static_leases_by_client_id, &client_id); + static_lease = hashmap_get(server->static_leases_by_client_id, &client_id); + if (!static_lease) + return NULL; + +verify: + /* Check if the address is in the same subnet. */ + if ((static_lease->address & server->netmask) != server->subnet) + return NULL; + + /* Check if the address is different from the server address. */ + if (static_lease->address == server->address) + return NULL; + + return static_lease; } int sd_dhcp_server_set_static_lease( From 0596f5939fa4d583756054cdfe8028146f52d4b4 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 3 Jan 2024 06:06:55 +0900 Subject: [PATCH 2/3] sd-dhcp-server: check address conflict more carefully - Even if we found a matching static lease, check if there is no conflicting lease. - Accept an address request that is different from the previous one. - Also return NAK if the server address is requested. --- src/libsystemd-network/sd-dhcp-server.c | 35 ++++++++++++----------- src/libsystemd-network/test-dhcp-server.c | 4 +-- 2 files changed, 21 insertions(+), 18 deletions(-) diff --git a/src/libsystemd-network/sd-dhcp-server.c b/src/libsystemd-network/sd-dhcp-server.c index b1c35064ce..0e7831f70d 100644 --- a/src/libsystemd-network/sd-dhcp-server.c +++ b/src/libsystemd-network/sd-dhcp-server.c @@ -1050,10 +1050,19 @@ int dhcp_server_handle_message(sd_dhcp_server *server, DHCPMessage *message, siz return 0; /* for now pick a random free address from the pool */ - if (static_lease) + if (static_lease) { + if (existing_lease != hashmap_get(server->bound_leases_by_address, UINT32_TO_PTR(static_lease->address))) + /* The address is already assigned to another host. Refusing. */ + return 0; + + /* Found a matching static lease. */ address = static_lease->address; - else if (existing_lease) + + } else if (existing_lease && address_is_in_pool(server, existing_lease->address)) + + /* If we previously assigned an address to the host, then reuse it. */ address = existing_lease->address; + else { struct siphash state; uint64_t hash; @@ -1151,30 +1160,24 @@ int dhcp_server_handle_message(sd_dhcp_server *server, DHCPMessage *message, siz /* Silently ignore Rapid Commit option in REQUEST message. */ req->rapid_commit = false; - /* disallow our own address */ - if (address == server->address) - return 0; - if (static_lease) { - /* Found a static lease for the client ID. */ - if (static_lease->address != address) - /* The client requested an address which is different from the static lease. Refuse. */ + /* The client requested an address which is different from the static lease. Refusing. */ return server_send_nak_or_ignore(server, init_reboot, req); + if (existing_lease != hashmap_get(server->bound_leases_by_address, UINT32_TO_PTR(address))) + /* The requested address is already assigned to another host. Refusing. */ + return server_send_nak_or_ignore(server, init_reboot, req); + + /* Found a static lease for the client ID. */ return server_ack_request(server, req, address); } - if (address_is_in_pool(server, address)) { + if (address_is_in_pool(server, address)) /* The requested address is in the pool. */ - - if (existing_lease && existing_lease->address != address) - /* We previously assigned an address, but the client requested another one. Refuse. */ - return server_send_nak_or_ignore(server, init_reboot, req); - return server_ack_request(server, req, address); - } + /* Refuse otherwise. */ return server_send_nak_or_ignore(server, init_reboot, req); } diff --git a/src/libsystemd-network/test-dhcp-server.c b/src/libsystemd-network/test-dhcp-server.c index 9c343f8368..a52f788968 100644 --- a/src/libsystemd-network/test-dhcp-server.c +++ b/src/libsystemd-network/test-dhcp-server.c @@ -184,7 +184,7 @@ static void test_message_handler(void) { assert_se(dhcp_server_handle_message(server, (DHCPMessage*)&test, sizeof(test), NULL) == 0); test.option_server_id.address = htobe32(INADDR_LOOPBACK); test.option_requested_ip.address = htobe32(INADDR_LOOPBACK + 4); - assert_se(dhcp_server_handle_message(server, (DHCPMessage*)&test, sizeof(test), NULL) == 0); + assert_se(dhcp_server_handle_message(server, (DHCPMessage*)&test, sizeof(test), NULL) == DHCP_ACK); test.option_requested_ip.address = htobe32(INADDR_LOOPBACK + 3); assert_se(dhcp_server_handle_message(server, (DHCPMessage*)&test, sizeof(test), NULL) == DHCP_ACK); @@ -200,7 +200,7 @@ static void test_message_handler(void) { assert_se(dhcp_server_handle_message(server, (DHCPMessage*)&test, sizeof(test), NULL) == DHCP_ACK); test.option_requested_ip.address = htobe32(INADDR_LOOPBACK + 30); - assert_se(dhcp_server_handle_message(server, (DHCPMessage*)&test, sizeof(test), NULL) == 0); + assert_se(dhcp_server_handle_message(server, (DHCPMessage*)&test, sizeof(test), NULL) == DHCP_ACK); /* request address reserved for static lease (unmatching client ID) */ test.option_client_id.id[6] = 'H'; From ea7a35ed6862bd73bac469f650ea7934c3b952f9 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 3 Jan 2024 06:06:59 +0900 Subject: [PATCH 3/3] sd-dhcp-server: do not forget previously assigned addresses on pool change Currently, networkd does not change pool size dynamically, so this should not change behavior. But if networkd does that, then forgetting leases causes that an address still used by a client may be assigned to another host. --- src/libsystemd-network/sd-dhcp-server.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/libsystemd-network/sd-dhcp-server.c b/src/libsystemd-network/sd-dhcp-server.c index 0e7831f70d..d59bf430d7 100644 --- a/src/libsystemd-network/sd-dhcp-server.c +++ b/src/libsystemd-network/sd-dhcp-server.c @@ -87,13 +87,6 @@ int sd_dhcp_server_configure_pool( server->address = address->s_addr; server->netmask = netmask; server->subnet = address->s_addr & netmask; - - /* Drop any leases associated with the old address range */ - hashmap_clear(server->bound_leases_by_address); - hashmap_clear(server->bound_leases_by_client_id); - - if (server->callback) - server->callback(server, SD_DHCP_SERVER_EVENT_LEASE_CHANGED, server->callback_userdata); } return 0;