From: Marcin Siodelski Date: Fri, 20 Oct 2017 10:10:31 +0000 (+0200) Subject: [5393] Fixed conflict resolution in allocation engine. X-Git-Tag: trac5389_base~1^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e3a6262f473c534367eb01850b71e40a11a4f03a;p=thirdparty%2Fkea.git [5393] Fixed conflict resolution in allocation engine. This now works for any identifier, instead of just MAC address. --- diff --git a/src/bin/dhcp4/tests/dora_unittest.cc b/src/bin/dhcp4/tests/dora_unittest.cc index 14f0edf440..8bf9a1de29 100644 --- a/src/bin/dhcp4/tests/dora_unittest.cc +++ b/src/bin/dhcp4/tests/dora_unittest.cc @@ -103,6 +103,11 @@ namespace { /// - Configuration 10: /// - Simple configuration with a single subnet and single pool /// - Using Cassandra lease database backend to store leases +/// +/// - Configuration 11: +/// - Simple configuration with a single subnet +/// - One in-pool reservation for a circuit-id of 'charter950' +/// const char* DORA_CONFIGS[] = { // Configuration 0 "{ \"interfaces-config\": {" @@ -383,7 +388,25 @@ const char* DORA_CONFIGS[] = { " \"id\": 1," " \"pools\": [ { \"pool\": \"10.0.0.10-10.0.0.100\" } ]" " } ]" - "}" + "}", + +// Configuration 11 + "{ \"interfaces-config\": {" + " \"interfaces\": [ \"*\" ]" + "}," + "\"host-reservation-identifiers\": [ \"circuit-id\" ]," + "\"valid-lifetime\": 600," + "\"subnet4\": [ { " + " \"subnet\": \"10.0.0.0/24\", " + " \"pools\": [ { \"pool\": \"10.0.0.5-10.0.0.100\" } ]," + " \"reservations\": [ " + " {" + " \"circuit-id\": \"'charter950'\"," + " \"ip-address\": \"10.0.0.9\"" + " }" + " ]" + "} ]" + "}", }; /// @brief Test fixture class for testing 4-way (DORA) exchanges. @@ -1658,6 +1681,66 @@ TEST_F(DORATest, customServerIdentifier) { EXPECT_EQ("3.4.5.6", client3.config_.serverid_.toText()); } +// This test verifies that reserved lease is not assigned to a client which +// identifier doesn't match the identifier in the reservation. +TEST_F(DORATest, changingCircuitId) { + Dhcp4Client client(Dhcp4Client::SELECTING); + client.setHWAddress("aa:bb:cc:dd:ee:ff"); + // Use relay agent so as the circuit-id can be inserted. + client.useRelay(true, IOAddress("10.0.0.1"), IOAddress("10.0.0.2")); + + // Configure DHCP server. + configure(DORA_CONFIGS[11], *client.getServer()); + + // Send DHCPDISCOVER. + boost::shared_ptr requested_address(new IOAddress("10.0.0.9")); + ASSERT_NO_THROW(client.doDiscover(requested_address)); + // Make sure that the server responded. + ASSERT_TRUE(client.getContext().response_); + Pkt4Ptr resp = client.getContext().response_; + // Make sure that the server has responded with DHCPOFFER + ASSERT_EQ(DHCPOFFER, static_cast(resp->getType())); + // Make sure that the client has been offerred a different address + // given that circuit-id is not used. + ASSERT_NE(resp->getYiaddr().toText(), "10.0.0.9"); + + // Specify circuit-id matching the one in the configuration. + client.setCircuitId("charter950"); + + // Send DHCPDISCOVER. + ASSERT_NO_THROW(client.doDiscover()); + // Make sure that the server responded. + ASSERT_TRUE(client.getContext().response_); + resp = client.getContext().response_; + // Make sure that the server has responded with DHCPOFFER + ASSERT_EQ(DHCPOFFER, static_cast(resp->getType())); + // Make sure that the client has been offerred reserved address given that + // matching circuit-id has been specified. + ASSERT_EQ("10.0.0.9", resp->getYiaddr().toText()); + + // Let's now change the circuit-id. + client.setCircuitId("gdansk"); + + // The client requests offerred address but should be refused this address + // given that the circuit-id is not matching. + ASSERT_NO_THROW(client.doRequest()); + // Make sure that the server responded. + ASSERT_TRUE(client.getContext().response_); + resp = client.getContext().response_; + // The client should be refused this address. + ASSERT_EQ(DHCPNAK, static_cast(resp->getType())); + + // In this case, the client falls back to the 4-way exchange and should be + // allocated an address from the dynamic pool. + ASSERT_NO_THROW(client.doDORA()); + // Make sure that the server responded. + ASSERT_TRUE(client.getContext().response_); + resp = client.getContext().response_; + // The client should be allocated some address. + ASSERT_EQ(DHCPACK, static_cast(resp->getType())); + ASSERT_NE(client.config_.lease_.addr_.toText(), "10.0.0.9"); +} + // Starting tests which require MySQL backend availability. Those tests // will not be executed if Kea has been compiled without the // --with-dhcp-mysql. diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc index 55effd88a0..c7c08c371a 100644 --- a/src/lib/dhcpsrv/alloc_engine.cc +++ b/src/lib/dhcpsrv/alloc_engine.cc @@ -2336,9 +2336,10 @@ namespace { /// @brief Check if the specific address is reserved for another client. /// -/// This function uses the HW address from the context to check if the -/// requested address (specified as first parameter) is reserved for -/// another client, i.e. client using a different HW address. +/// This function finds a host reservation for a given address and then +/// it verifies if the host identifier for this reservation is matching +/// a host identifier found for the current client. If it does not, the +/// address is assumed to be reserved for another client. /// /// @param address An address for which the function should check if /// there is a reservation for the different client. @@ -2349,20 +2350,14 @@ namespace { bool addressReserved(const IOAddress& address, const AllocEngine::ClientContext4& ctx) { ConstHostPtr host = HostMgr::instance().get4(ctx.subnet_->getID(), address); - HWAddrPtr host_hwaddr; if (host) { - host_hwaddr = host->getHWAddress(); - if (ctx.hwaddr_ && host_hwaddr) { - /// @todo Use the equality operators for HWAddr class. - /// Currently, this is impossible because the HostMgr uses the - /// HTYPE_ETHER type, whereas the unit tests may use other types - /// which HostMgr doesn't support yet. - return (host_hwaddr->hwaddr_ != ctx.hwaddr_->hwaddr_); - - } else { - return (false); - + for (auto id = ctx.host_identifiers_.cbegin(); id != ctx.host_identifiers_.cend(); + ++id) { + if (id->first == host->getIdentifierType()) { + return (id->second != host->getIdentifier()); + } } + return (true); } return (false); }