From b713a99b1a04507412b387214a26e43a2916bd01 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 11 Aug 2021 16:18:45 +0900 Subject: [PATCH] sd-dhcp-server: support static lease outside of address pool Closes #20341. --- src/libsystemd-network/dhcp-server-internal.h | 8 +- src/libsystemd-network/fuzz-dhcp-server.c | 7 +- src/libsystemd-network/sd-dhcp-server.c | 148 +++++++++--------- src/network/networkd-dhcp-server-bus.c | 2 +- 4 files changed, 79 insertions(+), 86 deletions(-) diff --git a/src/libsystemd-network/dhcp-server-internal.h b/src/libsystemd-network/dhcp-server-internal.h index 54586e662d..2bb7f3cce3 100644 --- a/src/libsystemd-network/dhcp-server-internal.h +++ b/src/libsystemd-network/dhcp-server-internal.h @@ -30,6 +30,8 @@ typedef struct DHCPClientId { } DHCPClientId; typedef struct DHCPLease { + sd_dhcp_server *server; + DHCPClientId client_id; be32_t address; @@ -67,10 +69,10 @@ struct sd_dhcp_server { bool emit_router; - Hashmap *leases_by_client_id; + Hashmap *bound_leases_by_client_id; + Hashmap *bound_leases_by_address; Hashmap *static_leases_by_client_id; - DHCPLease **bound_leases; - DHCPLease invalid_lease; + Hashmap *static_leases_by_address; uint32_t max_lease_time, default_lease_time; diff --git a/src/libsystemd-network/fuzz-dhcp-server.c b/src/libsystemd-network/fuzz-dhcp-server.c index 94a8faaa39..e90284f6f2 100644 --- a/src/libsystemd-network/fuzz-dhcp-server.c +++ b/src/libsystemd-network/fuzz-dhcp-server.c @@ -23,7 +23,6 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { static const uint8_t chaddr[] = {3,3,3,3,3,3,3,3,3,3,3,3,3,3,3,3}; uint8_t *client_id; DHCPLease *lease; - int pool_offset; if (size < sizeof(DHCPMessage)) return 0; @@ -46,9 +45,9 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { lease->gateway = htobe32(UINT32_C(10) << 24 | UINT32_C(1)); lease->expiration = UINT64_MAX; memcpy(lease->chaddr, chaddr, 16); - pool_offset = get_pool_offset(server, lease->address); - server->bound_leases[pool_offset] = lease; - assert_se(hashmap_ensure_put(&server->leases_by_client_id, &dhcp_lease_hash_ops, &lease->client_id, lease) >= 0); + assert_se(hashmap_ensure_put(&server->bound_leases_by_client_id, &dhcp_lease_hash_ops, &lease->client_id, lease) >= 0); + assert_se(hashmap_ensure_put(&server->bound_leases_by_address, NULL, UINT32_TO_PTR(lease->address), lease) >= 0); + lease->server = server; (void) dhcp_server_handle_message(server, (DHCPMessage*)data, size); diff --git a/src/libsystemd-network/sd-dhcp-server.c b/src/libsystemd-network/sd-dhcp-server.c index 8b9cf842a0..23a1a52544 100644 --- a/src/libsystemd-network/sd-dhcp-server.c +++ b/src/libsystemd-network/sd-dhcp-server.c @@ -28,6 +28,22 @@ static DHCPLease *dhcp_lease_free(DHCPLease *lease) { if (!lease) return NULL; + if (lease->server) { + DHCPLease *e; + + e = hashmap_get(lease->server->bound_leases_by_client_id, &lease->client_id); + if (e == lease) { + hashmap_remove(lease->server->bound_leases_by_address, UINT32_TO_PTR(lease->address)); + hashmap_remove(lease->server->bound_leases_by_client_id, &lease->client_id); + } + + e = hashmap_get(lease->server->static_leases_by_client_id, &lease->client_id); + if (e == lease) { + hashmap_remove(lease->server->static_leases_by_address, UINT32_TO_PTR(lease->address)); + hashmap_remove(lease->server->static_leases_by_client_id, &lease->client_id); + } + } + free(lease->client_id.data); return mfree(lease); } @@ -85,11 +101,6 @@ int sd_dhcp_server_configure_pool( if (server->address != address->s_addr || server->netmask != netmask || server->pool_size != size || server->pool_offset != offset) { - free(server->bound_leases); - server->bound_leases = new0(DHCPLease*, size); - if (!server->bound_leases) - return -ENOMEM; - server->pool_offset = offset; server->pool_size = size; @@ -97,11 +108,9 @@ int sd_dhcp_server_configure_pool( server->netmask = netmask; server->subnet = address->s_addr & netmask; - if (server_off >= offset && server_off - offset < size) - server->bound_leases[server_off - offset] = &server->invalid_lease; - /* Drop any leases associated with the old address range */ - hashmap_clear(server->leases_by_client_id); + 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); @@ -166,8 +175,10 @@ static sd_dhcp_server *dhcp_server_free(sd_dhcp_server *server) { for (sd_dhcp_lease_server_type_t i = 0; i < _SD_DHCP_LEASE_SERVER_TYPE_MAX; i++) free(server->servers[i].addr); - hashmap_free(server->leases_by_client_id); - hashmap_free(server->static_leases_by_client_id); + server->bound_leases_by_address = hashmap_free(server->bound_leases_by_address); + server->bound_leases_by_client_id = hashmap_free(server->bound_leases_by_client_id); + server->static_leases_by_address = hashmap_free(server->static_leases_by_address); + server->static_leases_by_client_id = hashmap_free(server->static_leases_by_client_id); ordered_set_free(server->extra_options); ordered_set_free(server->vendor_options); @@ -175,8 +186,6 @@ static sd_dhcp_server *dhcp_server_free(sd_dhcp_server *server) { free(server->agent_circuit_id); free(server->agent_remote_id); - free(server->bound_leases); - free(server->ifname); return mfree(server); } @@ -837,18 +846,6 @@ static int prepare_new_lease( return 0; } -static bool static_leases_have_address(sd_dhcp_server *server, be32_t address) { - DHCPLease *s; - - assert(server); - - HASHMAP_FOREACH(s, server->static_leases_by_client_id) - if (s->address == address) - return true; - - return false; -} - #define HASH_KEY SD_ID128_MAKE(0d,1d,fe,bd,f1,24,bd,b3,47,f1,dd,6e,73,21,93,30) int dhcp_server_handle_message(sd_dhcp_server *server, DHCPMessage *message, size_t length) { @@ -878,14 +875,13 @@ int dhcp_server_handle_message(sd_dhcp_server *server, DHCPMessage *message, siz /* this only fails on critical errors */ return r; - existing_lease = hashmap_get(server->leases_by_client_id, &req->client_id); + existing_lease = hashmap_get(server->bound_leases_by_client_id, &req->client_id); static_lease = hashmap_get(server->static_leases_by_client_id, &req->client_id); switch(type) { case DHCP_DISCOVER: { be32_t address = INADDR_ANY; - unsigned i; log_dhcp_server(server, "DISCOVER (0x%x)", be32toh(req->message->xid)); @@ -901,7 +897,6 @@ int dhcp_server_handle_message(sd_dhcp_server *server, DHCPMessage *message, siz else { struct siphash state; uint64_t hash; - uint32_t next_offer; /* even with no persistence of leases, we try to offer the same client the same IP address. we do this by using the hash of the client id @@ -910,18 +905,16 @@ int dhcp_server_handle_message(sd_dhcp_server *server, DHCPMessage *message, siz siphash24_init(&state, HASH_KEY.bytes); client_id_hash_func(&req->client_id, &state); hash = htole64(siphash24_finalize(&state)); - next_offer = hash % server->pool_size; - for (i = 0; i < server->pool_size; i++) { - if (!server->bound_leases[next_offer]) { - be32_t tmp = server->subnet | htobe32(server->pool_offset + next_offer); - if (!static_leases_have_address(server, tmp)) { - address = tmp; - break; - } + for (unsigned i = 0; i < server->pool_size; i++) { + be32_t tmp_address; + + tmp_address = server->subnet | htobe32(server->pool_offset + (hash + i) % server->pool_size); + if (!hashmap_contains(server->bound_leases_by_address, &tmp_address) && + !hashmap_contains(server->static_leases_by_address, &tmp_address)) { + address = tmp_address; + break; } - - next_offer = (next_offer + 1) % server->pool_size; } } @@ -945,6 +938,7 @@ int dhcp_server_handle_message(sd_dhcp_server *server, DHCPMessage *message, siz return 1; case DHCP_REQUEST: { + DHCPLease *existing_lease_by_address; be32_t address; bool init_reboot = false; int pool_offset; @@ -995,11 +989,12 @@ int dhcp_server_handle_message(sd_dhcp_server *server, DHCPMessage *message, siz } pool_offset = get_pool_offset(server, address); + existing_lease_by_address = hashmap_get(server->bound_leases_by_address, UINT32_TO_PTR(address)); /* verify that the requested address is from the pool, and either owned by the current client or free */ - if (pool_offset >= 0 && static_lease) { - _cleanup_(dhcp_lease_freep) DHCPLease *lease = NULL, *old_lease = NULL; + if (static_lease && static_lease->address == address) { + _cleanup_(dhcp_lease_freep) DHCPLease *lease = NULL; usec_t time_now, expiration; r = sd_event_now(server->event, clock_boottime_or_monotonic(), &time_now); @@ -1020,12 +1015,16 @@ int dhcp_server_handle_message(sd_dhcp_server *server, DHCPMessage *message, siz log_dhcp_server(server, "ACK (0x%x)", be32toh(req->message->xid)); - server->bound_leases[pool_offset] = lease; - - old_lease = hashmap_remove(server->leases_by_client_id, &lease->client_id); - r = hashmap_ensure_put(&server->leases_by_client_id, &dhcp_lease_hash_ops, &lease->client_id, lease); + dhcp_lease_free(hashmap_remove(server->bound_leases_by_client_id, &lease->client_id)); + r = hashmap_ensure_put(&server->bound_leases_by_client_id, &dhcp_lease_hash_ops, &lease->client_id, lease); if (r < 0) return log_dhcp_server_errno(server, r, "Could not save lease: %m"); + + r = hashmap_ensure_put(&server->bound_leases_by_address, NULL, UINT32_TO_PTR(lease->address), lease); + if (r < 0) + return log_dhcp_server_errno(server, r, "Could not save lease: %m"); + + lease->server = server; TAKE_PTR(lease); if (server->callback) @@ -1033,11 +1032,13 @@ int dhcp_server_handle_message(sd_dhcp_server *server, DHCPMessage *message, siz return DHCP_ACK; - } else if (pool_offset >= 0 && server->bound_leases[pool_offset] == existing_lease) { + } else if (pool_offset >= 0 && existing_lease_by_address == existing_lease) { _cleanup_(dhcp_lease_freep) DHCPLease *new_lease = NULL; usec_t time_now, expiration; DHCPLease *lease; + /* Note that in the above condition we accept the case that both leases are NULL. */ + r = sd_event_now(server->event, clock_boottime_or_monotonic(), &time_now); if (r < 0) return r; @@ -1063,10 +1064,14 @@ int dhcp_server_handle_message(sd_dhcp_server *server, DHCPMessage *message, siz log_dhcp_server(server, "ACK (0x%x)", be32toh(req->message->xid)); - server->bound_leases[pool_offset] = lease; - r = hashmap_ensure_put(&server->leases_by_client_id, &dhcp_lease_hash_ops, &lease->client_id, lease); + r = hashmap_ensure_put(&server->bound_leases_by_client_id, &dhcp_lease_hash_ops, &lease->client_id, lease); if (r < 0) return log_dhcp_server_errno(server, r, "Could not save lease: %m"); + r = hashmap_ensure_put(&server->bound_leases_by_address, NULL, UINT32_TO_PTR(lease->address), lease); + if (r < 0) + return log_dhcp_server_errno(server, r, "Could not save lease: %m"); + + lease->server = server; TAKE_PTR(new_lease); if (server->callback) @@ -1088,8 +1093,6 @@ int dhcp_server_handle_message(sd_dhcp_server *server, DHCPMessage *message, siz } case DHCP_RELEASE: { - int pool_offset; - log_dhcp_server(server, "RELEASE (0x%x)", be32toh(req->message->xid)); @@ -1099,18 +1102,10 @@ int dhcp_server_handle_message(sd_dhcp_server *server, DHCPMessage *message, siz if (existing_lease->address != req->message->ciaddr) return 0; - pool_offset = get_pool_offset(server, req->message->ciaddr); - if (pool_offset < 0) - return 0; + dhcp_lease_free(existing_lease); - if (server->bound_leases[pool_offset] == existing_lease) { - server->bound_leases[pool_offset] = NULL; - hashmap_remove(server->leases_by_client_id, &existing_lease->client_id); - dhcp_lease_free(existing_lease); - - if (server->callback) - server->callback(server, SD_DHCP_SERVER_EVENT_LEASE_CHANGED, server->callback_userdata); - } + if (server->callback) + server->callback(server, SD_DHCP_SERVER_EVENT_LEASE_CHANGED, server->callback_userdata); return 0; }} @@ -1264,24 +1259,17 @@ on_error: } int sd_dhcp_server_forcerenew(sd_dhcp_server *server) { - int r = 0; + DHCPLease *lease; + int k, r = 0; assert_return(server, -EINVAL); - assert(server->bound_leases); - for (uint32_t i = 0; i < server->pool_size; i++) { - DHCPLease *lease = server->bound_leases[i]; + log_dhcp_server(server, "FORCERENEW"); - if (!lease || lease == &server->invalid_lease) - continue; - - r = server_send_forcerenew(server, lease->address, - lease->gateway, - lease->chaddr); - if (r < 0) - return r; - - log_dhcp_server(server, "FORCERENEW"); + HASHMAP_FOREACH(lease, server->bound_leases_by_client_id) { + k = server_send_forcerenew(server, lease->address, lease->gateway, lease->chaddr); + if (k < 0) + r = k; } return r; @@ -1477,8 +1465,7 @@ int sd_dhcp_server_set_static_lease( uint8_t *client_id, size_t client_id_size) { - _cleanup_(dhcp_lease_freep) DHCPLease *lease = NULL, *old = NULL; - DHCPClientId c; + _cleanup_(dhcp_lease_freep) DHCPLease *lease = NULL; int r; assert_return(server, -EINVAL); @@ -1490,6 +1477,7 @@ int sd_dhcp_server_set_static_lease( * the server removes any static lease with the specified mac address. */ if (!address || address->s_addr == 0) { _cleanup_free_ void *data = NULL; + DHCPClientId c; data = memdup(client_id, client_id_size); if (!data) @@ -1500,11 +1488,11 @@ int sd_dhcp_server_set_static_lease( .data = data, }; - old = hashmap_remove(server->static_leases_by_client_id, &c); + dhcp_lease_free(hashmap_remove(server->static_leases_by_client_id, &c)); return 0; } - if (static_leases_have_address(server, address->s_addr)) + if (hashmap_contains(server->static_leases_by_address, UINT32_TO_PTR(address->s_addr))) return -EEXIST; lease = new(DHCPLease, 1); @@ -1522,9 +1510,13 @@ int sd_dhcp_server_set_static_lease( return -ENOMEM; r = hashmap_ensure_put(&server->static_leases_by_client_id, &dhcp_lease_hash_ops, &lease->client_id, lease); + if (r < 0) + return r; + r = hashmap_ensure_put(&server->static_leases_by_address, NULL, UINT32_TO_PTR(lease->address), lease); if (r < 0) return r; + lease->server = server; TAKE_PTR(lease); return 0; } diff --git a/src/network/networkd-dhcp-server-bus.c b/src/network/networkd-dhcp-server-bus.c index a38cb99c2c..81f4a6706b 100644 --- a/src/network/networkd-dhcp-server-bus.c +++ b/src/network/networkd-dhcp-server-bus.c @@ -38,7 +38,7 @@ static int property_get_leases( if (r < 0) return r; - HASHMAP_FOREACH(lease, s->leases_by_client_id) { + HASHMAP_FOREACH(lease, s->bound_leases_by_client_id) { r = sd_bus_message_open_container(reply, 'r', "uayayayayt"); if (r < 0) return r;