[5393] Fixed conflict resolution in allocation engine.

This now works for any identifier, instead of just MAC address.

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<IOAddress> 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<int>(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<int>(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<int>(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<int>(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.

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
+/// This function finds a host reservation for a given address and then
-/// requested address (specified as first parameter) is reserved for
+/// it verifies if the host identifier for this reservation is matching
-/// another client, i.e. client using a different HW address.
+/// 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();
+        for (auto id = ctx.host_identifiers_.cbegin(); id != ctx.host_identifiers_.cend();
-        if (ctx.hwaddr_ && host_hwaddr) {
+             ++id) {
-            /// @todo Use the equality operators for HWAddr class.
+            if (id->first == host->getIdentifierType()) {
-            /// Currently, this is impossible because the HostMgr uses the
+                return (id->second != host->getIdentifier());
-            /// 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);
-
         }
+        return (true);
     }
     return (false);
 }