]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
Merge pull request #30720 from yuwata/dhcp-server-address-verification
authorLuca Boccassi <bluca@debian.org>
Wed, 10 Jan 2024 19:29:25 +0000 (19:29 +0000)
committerGitHub <noreply@github.com>
Wed, 10 Jan 2024 19:29:25 +0000 (19:29 +0000)
dhcp-server: several fixlets for address verification

src/libsystemd-network/sd-dhcp-server-lease.c
src/libsystemd-network/sd-dhcp-server.c
src/libsystemd-network/test-dhcp-server.c

index 16880a80b61ea325c5816b61d1849f205379fc60..db1d0464ae8c07ec85a1bab035279e90237f4c82 100644 (file)
@@ -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(
index b1c35064ce8c24fa50c4df209a762c5bc8b4686f..d59bf430d7ace6f9e174bbb0ec8438200b60997e 100644 (file)
@@ -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;
@@ -1050,10 +1043,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 +1153,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);
         }
 
index 9c343f8368d77217052c7071ecbd6ac632d4bec3..a52f788968d9e9b1f5b1ace86e72f968825d41ae 100644 (file)
@@ -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';