From: Marcin Siodelski Date: Tue, 12 May 2015 17:01:27 +0000 (+0200) Subject: [3747] Use record-client-id parameter when (re)allocating leases. X-Git-Tag: trac3867_base~2^2~5 X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=b1d5d0ba95fe0280caa634f9426491b44a44ebf1;p=thirdparty%2Fkea.git [3747] Use record-client-id parameter when (re)allocating leases. --- diff --git a/src/bin/dhcp4/dhcp4_messages.mes b/src/bin/dhcp4/dhcp4_messages.mes index ae1a911fff..9c2f175f39 100644 --- a/src/bin/dhcp4/dhcp4_messages.mes +++ b/src/bin/dhcp4/dhcp4_messages.mes @@ -31,6 +31,20 @@ to establish a session with the Kea control channel. This debug message informs that incoming packet has been assigned to specified class or classes. This is a normal behavior and indicates successful operation. +% DHCP4_CLIENTID_IGNORED_FOR_LEASES %1: not using client identifier for lease allocation for subnet %2 +This debug message is issued when the server is processing the DHCPv4 message +for which client identifier will not be used when allocating new lease or +renewing existing lease. The server is explicitly configured to not use +client identifier to lookup existing leases for the client and will not +record client identifier in the lease database. This mode of operation +is useful when clients don't use stable client identifiers, e.g. multi +stage booting. Note that the client identifier may be used for other +operations than lease allocation, e.g. identifying host reservations +for the client using client identifier. The first argument includes the +client and transaction identification information. The second argument +specifies the identifier of the subnet where the client is connected +and for which this mode of operation is configured on the server. + % DHCP4_CLIENT_NAME_PROC_FAIL failed to process the fqdn or hostname sent by a client: %1 This debug message is issued when the DHCP server was unable to process the FQDN or Hostname option sent by a client. This is likely because the client's diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index e5a5183e34..25054d13b0 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -107,10 +107,22 @@ Dhcpv4Exchange::Dhcpv4Exchange(const AllocEnginePtr& alloc_engine, context_->subnet_ = subnet; // Hardware address. context_->hwaddr_ = query->getHWAddr(); - // Client Identifier - OptionPtr opt_clientid = query->getOption(DHO_DHCP_CLIENT_IDENTIFIER); - if (opt_clientid) { - context_->clientid_.reset(new ClientId(opt_clientid->getData())); + + // Set client identifier if the record-client-id flag is enabled (default). + // If the subnet wasn't found it doesn't matter because we will not be + // able to allocate a lease anyway so this context will not be used. + if (subnet) { + if (subnet->getRecordClientId()) { + OptionPtr opt_clientid = query->getOption(DHO_DHCP_CLIENT_IDENTIFIER); + if (opt_clientid) { + context_->clientid_.reset(new ClientId(opt_clientid->getData())); + } + } else { + /// @todo When merging with #3806 use different logger. + LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_CLIENTID_IGNORED_FOR_LEASES) + .arg(query->getLabel()) + .arg(subnet->getID()); + } } // Check for static reservations. alloc_engine->findReservation(*context_); @@ -1073,9 +1085,6 @@ Dhcpv4Srv::assignLease(Dhcpv4Exchange& ex) { LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL_DATA, DHCP4_SUBNET_SELECTED) .arg(subnet->toText()); - // Get client-id. It is not mandatory in DHCPv4. - ClientIdPtr client_id = ex.getContext()->clientid_; - // Get the server identifier. It will be used to determine the state // of the client. OptionCustomPtr opt_serverid = boost::dynamic_pointer_cast< @@ -1104,6 +1113,9 @@ Dhcpv4Srv::assignLease(Dhcpv4Exchange& ex) { // allocation. bool fake_allocation = (query->getType() == DHCPDISCOVER); + // Get client-id. It is not mandatory in DHCPv4. + ClientIdPtr client_id = ex.getContext()->clientid_; + // If there is no server id and there is a Requested IP Address option // the client is in the INIT-REBOOT state in which the server has to // determine whether the client's notion of the address is correct @@ -1121,6 +1133,10 @@ Dhcpv4Srv::assignLease(Dhcpv4Exchange& ex) { // Check the first error case: unknown client. We check this before // validating the address sent because we don't want to respond if // we don't know this client. + /// @todo The client_id logged here should be the client_id from the message + /// rather than effective client id which is null when the + /// record-client-id is set to false. This is addressed by the #3806 + /// which is under review. if (!lease || !lease->belongsToClient(hwaddr, client_id)) { LOG_DEBUG(dhcp4_logger, DBG_DHCP4_DETAIL, DHCP4_NO_LEASE_INIT_REBOOT) @@ -1561,7 +1577,16 @@ Dhcpv4Srv::processRelease(Pkt4Ptr& release) { /// @todo Uncomment this (see ticket #3116) /// sanityCheck(release, MANDATORY); - // Try to find client-id + // Try to find client-id. Note that for the DHCPRELEASE we don't check if the + // record-client-id configuration parameter is disabled because this parameter + // is configured for subnets and we don't select subnet for the DHCPRELEASE. + // Bogus clients usually generate new client identifiers when they first + // connect to the network, so whatever client identifier has been used to + // acquire the lease, the client identifier carried in the DHCPRELEASE is + // likely to be the same and the lease will be correctly identified in the + // lease database. If supplied client identifier differs from the one used + // to acquire the lease then the lease will remain in the database and + // simply expire. ClientIdPtr client_id; OptionPtr opt = release->getOption(DHO_DHCP_CLIENT_IDENTIFIER); if (opt) { diff --git a/src/bin/dhcp4/tests/dora_unittest.cc b/src/bin/dhcp4/tests/dora_unittest.cc index d2aa88dd3c..2835bbcbd0 100644 --- a/src/bin/dhcp4/tests/dora_unittest.cc +++ b/src/bin/dhcp4/tests/dora_unittest.cc @@ -57,6 +57,13 @@ namespace { /// - 1 subnet: 10.0.0.0/24 /// - One reservation for the client using MAC address: /// aa:bb:cc:dd:ee:ff, reserved address 10.0.0.7 +/// +/// - Configuration 3: +/// - Use for testing record-client-id flag +/// - 1 subnet: 10.0.0.0/24 +/// - 1 pool: 10.0.0.10-10.0.0.100 +/// - record-client-id flag is set to false, thus the server +/// uses HW address for lease lookup, rather than client id const char* DORA_CONFIGS[] = { // Configuration 0 "{ \"interfaces-config\": {" @@ -152,7 +159,27 @@ const char* DORA_CONFIGS[] = { " }" " ]" "} ]" - "}" + "}", + +// Configuration 3 + "{ \"interfaces-config\": {" + " \"interfaces\": [ \"*\" ]" + "}," + "\"valid-lifetime\": 600," + "\"record-client-id\": false," + "\"subnet4\": [ { " + " \"subnet\": \"10.0.0.0/24\", " + " \"id\": 1," + " \"pools\": [ { \"pool\": \"10.0.0.10-10.0.0.100\" } ]," + " \"option-data\": [ {" + " \"name\": \"routers\"," + " \"code\": 3," + " \"data\": \"10.0.0.200,10.0.0.201\"," + " \"csv-format\": true," + " \"space\": \"dhcp4\"" + " } ]" + " } ]" + "}", }; /// @brief Test fixture class for testing 4-way (DORA) exchanges. @@ -684,6 +711,78 @@ TEST_F(DORATest, reservation) { ASSERT_TRUE(subnet->inPool(Lease::TYPE_V4, clientB.config_.lease_.addr_)); } +// This test checks that setting the record-client-id value to false causes +// the server to ignore changing client identifier when the client is +// using consistent HW address. +TEST_F(DORATest, ignoreChangingClientId) { + Dhcp4Client client(Dhcp4Client::SELECTING); + // Configure DHCP server. + configure(DORA_CONFIGS[3], *client.getServer()); + client.includeClientId("12:12"); + // Obtain the lease using 4-way exchange. + ASSERT_NO_THROW(client.doDORA()); + // Make sure that the server responded. + ASSERT_TRUE(client.getContext().response_); + Pkt4Ptr resp = client.getContext().response_; + // Make sure that the server has responded with DHCPACK. + ASSERT_EQ(DHCPACK, static_cast(resp->getType())); + + // Remember address which the client has obtained. + IOAddress leased_address = client.config_.lease_.addr_; + + // Modify client id. Because we have set the configuration flag which + // forces the server to lookup leases using the HW address, the + // client id modification should not matter and the client should + // obtain the same lease. + client.includeClientId("14:14"); + // Obtain the lease using 4-way exchange. + ASSERT_NO_THROW(client.doDORA()); + // Make sure that the server responded. + ASSERT_TRUE(client.getContext().response_); + resp = client.getContext().response_; + // Make sure that the server has responded with DHCPACK. + ASSERT_EQ(DHCPACK, static_cast(resp->getType())); + // Make sure that the server assigned the same address, even though the + // client id has changed. + EXPECT_EQ(leased_address, client.config_.lease_.addr_); +} + +// This test checks that the record-client-id parameter doesn't have +// effect on the lease lookup using the HW address. +TEST_F(DORATest, changingHWAddress) { + Dhcp4Client client(Dhcp4Client::SELECTING); + // Configure DHCP server. + configure(DORA_CONFIGS[3], *client.getServer()); + client.includeClientId("12:12"); + client.setHWAddress("00:01:02:03:04:05"); + // Obtain the lease using 4-way exchange. + ASSERT_NO_THROW(client.doDORA()); + // Make sure that the server responded. + ASSERT_TRUE(client.getContext().response_); + Pkt4Ptr resp = client.getContext().response_; + // Make sure that the server has responded with DHCPACK. + ASSERT_EQ(DHCPACK, static_cast(resp->getType())); + + // Remember address which the client has obtained. + IOAddress leased_address = client.config_.lease_.addr_; + + // Modify HW address but leave client id in place. The value of the + // record-client-id set to false must not have any effect on the + // case when the HW address is changing. In such case the server will + // allocate the new address for the client. + client.setHWAddress("01:01:01:01:01:01"); + // Obtain a lease. + ASSERT_NO_THROW(client.doDORA()); + // Make sure that the server responded. + ASSERT_TRUE(client.getContext().response_); + resp = client.getContext().response_; + // Make sure that the server has responded with DHCPACK. + ASSERT_EQ(DHCPACK, static_cast(resp->getType())); + // Client must assign different address because the client id is + // ignored and the HW address was changed. + EXPECT_NE(client.config_.lease_.addr_, leased_address); +} + // This test checks the following scenario: // 1. Client A performs 4-way exchange and obtains a lease from the dynamic pool. // 2. Reservation is created for the client A using an address out of the dynamic