From: Marcin Siodelski Date: Mon, 11 May 2015 12:24:58 +0000 (+0200) Subject: [3747] Use client identifier over MAC to get existing client's lease. X-Git-Tag: trac3867_base~2^2~14 X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=e0bbec0e10b1539df28fa987a2b21326c0fff0e5;p=thirdparty%2Fkea.git [3747] Use client identifier over MAC to get existing client's lease. --- diff --git a/src/bin/dhcp4/tests/dora_unittest.cc b/src/bin/dhcp4/tests/dora_unittest.cc index 7ab47b3337..1e9d7cb4dd 100644 --- a/src/bin/dhcp4/tests/dora_unittest.cc +++ b/src/bin/dhcp4/tests/dora_unittest.cc @@ -173,71 +173,30 @@ public: IfaceMgr::instance().openSockets4(); } - /// @brief Test that server doesn't allocate the lease for a client - /// which has the same address or client identifier as another client. + /// @brief Test that server returns the same lease for the client which is + /// sometimes using client identifier, sometimes not. /// /// This test checks the server's behavior in the following scenario: - /// - Client A identifies itself to the server using the hardware address - /// and client identifier or only one of those. - /// - Client A performs the 4-way exchange and obtains a lease from the server. - /// - Client B uses the same HW address or client identifier as the client A. - /// - Client B uses both HW address and client identifier if the client A is using - /// only one of them. Client B uses one of the HW address or client - /// identifier if the client A is using both. - /// - Client B sends the DHCPDISCOVER to the server. - /// The server determines that there is a lease for the client A using the - /// same HW address as the client B. Server discards the client's message and - /// doesn't offer the lease for the client B to prevent allocation of the - /// lease without a unique identifier. - /// - The client sends the DHCPREQUEST and the server sends the DHCPNAK for the - /// same reason. - /// - The client A renews its address successfully. + /// - Client identifies itself to the server using HW address, and may use + /// client identifier. + /// - Client performs the 4-way exchange and obtains a lease from the server. + /// - If the client identifier was in use when the client has acquired the lease, + /// the client uses null client identifier in the next exchange with the server. + /// - If the client identifier was not in use when the client has acquired the + /// lease, the client uses client identifier in the next exchange with the + /// server. + /// - When the client contacts the server for the second time using the + /// DHCPDISCOVER the server determines (using HW address) that the client + /// already has a lease and returns this lease to the client. + /// - The client renews the existing lease. /// - /// The specific test cases using this test must make sure that one of the - /// provided parameters is an empty string. This simulates the situation where - /// one of the clients has only one of the identifiers and the other one has - /// two. - /// - /// @param hwaddr_a HW address of client A. - /// @param clientid_a Client id of client A. - /// @param hwaddr_b HW address of client B. - /// @param clientid_b Client id of client B. - void oneAllocationOverlapTest(const std::string& hwaddr_a, - const std::string& clientid_a, - const std::string& hwaddr_b, + /// @param clientid_a Client identifier when the client initially allocates + /// the lease. An empty value means "no client identifier". + /// @param clientid_b Client identifier when the client sends the DHCPDISCOVER + /// and then DHCPREQUEST to renew lease. + void oneAllocationOverlapTest(const std::string& clientid_a, const std::string& clientid_b); - /// @brief Test that server can allocate the lease for a client having - /// the same HW Address or client id as another client. - /// - /// This test checks the server behavior in the following situation: - /// - Client A identifies itself to the server using client identifier - /// and the hardware address and requests allocation of the new lease. - /// - Server allocates the lease to the client. - /// - Client B has a different hardware address or client identifier than - /// the client A, but the other identifier is equal to the corresponding - /// identifier of the client A. - /// - Client B sends DHCPDISCOVER. - /// - Server should determine that the client B is not client A, because - /// it is using a different hadrware address or client identifier. - /// As a consequence, the server should offer a different address to the - /// client B. - /// - The client B performs the 4-way exchange again, and the server - /// allocates a new address to the client, which should be different - /// than the address used by the client A. - /// - Client B is in the renewing state and it successfully renews its - /// address. - /// - The client A also renews its address successfully. - /// - /// @param hwaddr_a HW address of client A. - /// @param clientid_a Client id of client A. - /// @param hwaddr_b HW address of client B. - /// @param clientid_b Client id of client B. - void twoAllocationsOverlapTest(const std::string& hwaddr_a, - const std::string& clientid_a, - const std::string& hwaddr_b, - const std::string& clientid_b); - /// @brief Interface Manager's fake configuration control. IfaceMgrTestConfig iface_mgr_test_config_; @@ -507,14 +466,67 @@ TEST_F(DORATest, ciaddr) { } void -DORATest::twoAllocationsOverlapTest(const std::string& hwaddr_a, - const std::string& clientid_a, - const std::string& hwaddr_b, - const std::string& clientid_b) { +DORATest::oneAllocationOverlapTest(const std::string& clientid_a, + const std::string& clientid_b) { + // Allocate a lease by client using the 4-way exchange. + Dhcp4Client client(Dhcp4Client::SELECTING); + client.includeClientId(clientid_a); + client.setHWAddress("01:02:03:04:05:06"); + configure(DORA_CONFIGS[0], *client.getServer()); + 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())); + Lease4Ptr lease_a = LeaseMgrFactory::instance().getLease4(client.config_.lease_.addr_); + ASSERT_TRUE(lease_a); + // Remember the allocated address. + IOAddress yiaddr = lease_a->addr_; + + // Change client identifier. If parameters clientid_a and clientid_b + // are specified correctly, this removes the client identifier from + // client's requests if the lease has been acquired with the client + // identifier, or adds the client identifier otherwise. + client.includeClientId(clientid_b); + + // Check if the server will offer the same address. + ASSERT_NO_THROW(client.doDiscover()); + resp = client.getContext().response_; + ASSERT_TRUE(resp); + EXPECT_EQ(yiaddr, resp->getYiaddr()); + + // Client should also be able to renew its address. + client.setState(Dhcp4Client::RENEWING); + ASSERT_NO_THROW(client.doRequest()); + ASSERT_TRUE(client.getContext().response_); + resp = client.getContext().response_; + ASSERT_EQ(DHCPACK, static_cast(resp->getType())); + ASSERT_EQ(yiaddr, client.config_.lease_.addr_); +} + +// This test checks the server behavior in the following situation: +// - Client A identifies itself to the server using client identifier +// and the hardware address and requests allocation of the new lease. +// - Server allocates the lease to the client. +// - Client B has the same hardware address but is using a different +// client identifier then Client A. +// - Client B sends DHCPDISCOVER. +// - Server should determine that the client B is not client A, because +// it is using a different client identifier, even though they use the +// same HW address. As a consequence, the server should offer a +// different address to the client B. +// - The client B performs the 4-way exchange again and the server +// allocates a new address to the client, which should be different +// than the address used by the client A. +// - Client B is in the renewing state and it successfully renews its +// address. +// - Client A also renews its address successfully. +TEST_F(DORATest, twoAllocationsOverlap) { // Allocate a lease by client A using the 4-way exchange. Dhcp4Client client_a(Dhcp4Client::SELECTING); - client_a.includeClientId(clientid_a); - client_a.setHWAddress(hwaddr_a); + client_a.includeClientId("12:34"); + client_a.setHWAddress("01:02:03:04:05:06"); configure(DORA_CONFIGS[0], *client_a.getServer()); ASSERT_NO_THROW(client_a.doDORA()); // Make sure that the server responded. @@ -529,8 +541,8 @@ DORATest::twoAllocationsOverlapTest(const std::string& hwaddr_a, // Create client B. Dhcp4Client client_b(client_a.getServer(), Dhcp4Client::SELECTING); - client_b.setHWAddress(hwaddr_b); - client_b.includeClientId(clientid_b); + client_b.setHWAddress("01:02:03:04:05:06"); + client_b.includeClientId("45:67"); // Send DHCPDISCOVER and expect the response. ASSERT_NO_THROW(client_b.doDiscover()); Pkt4Ptr resp_b = client_b.getContext().response_; @@ -581,86 +593,6 @@ DORATest::twoAllocationsOverlapTest(const std::string& hwaddr_a, ASSERT_NE(client_a.config_.lease_.addr_, client_b.config_.lease_.addr_); } -void -DORATest::oneAllocationOverlapTest(const std::string& hwaddr_a, - const std::string& clientid_a, - const std::string& hwaddr_b, - const std::string& clientid_b) { - // Allocate a lease by client A using the 4-way exchange. - Dhcp4Client client_a(Dhcp4Client::SELECTING); - client_a.includeClientId(clientid_a); - client_a.setHWAddress(hwaddr_a); - configure(DORA_CONFIGS[0], *client_a.getServer()); - ASSERT_NO_THROW(client_a.doDORA()); - // Make sure that the server responded. - ASSERT_TRUE(client_a.getContext().response_); - Pkt4Ptr resp_a = client_a.getContext().response_; - // Make sure that the server has responded with DHCPACK. - ASSERT_EQ(DHCPACK, static_cast(resp_a->getType())); - Lease4Ptr lease_a = LeaseMgrFactory::instance().getLease4(client_a.config_.lease_.addr_); - ASSERT_TRUE(lease_a); - - // Client B sends a DHCPDISCOVER. - Dhcp4Client client_b(client_a.getServer(), Dhcp4Client::SELECTING); - client_b.setHWAddress(hwaddr_b); - client_b.includeClientId(clientid_b); - // Client A and Client B have one common identifier (HW address - // or client identifier) and one of them is missing the other - // identifier. The allocation engine can't offer an address for - // the client which has the same identifier as the other client and - // which doesn't have any other (unique) identifier. It should - // discard the DHCPDISCOVER. - ASSERT_NO_THROW(client_b.doDiscover()); - Pkt4Ptr resp_b = client_b.getContext().response_; - ASSERT_FALSE(resp_b); - - // Now repeat the same test but send the DHCPREQUEST. This time the - // server should send the DHCPNAK. - client_b.config_.lease_.addr_ = IOAddress::IPV4_ZERO_ADDRESS(); - client_b.setState(Dhcp4Client::INIT_REBOOT); - ASSERT_NO_THROW(client_b.doRequest()); - resp_b = client_b.getContext().response_; - ASSERT_TRUE(resp_b); - ASSERT_EQ(DHCPNAK, static_cast(resp_b->getType())); - - // Client A should also be able to renew its address. - client_a.setState(Dhcp4Client::RENEWING); - ASSERT_NO_THROW(client_a.doRequest()); - ASSERT_TRUE(client_a.getContext().response_); - resp_b = client_a.getContext().response_; - ASSERT_EQ(DHCPACK, static_cast(resp_b->getType())); - ASSERT_NE(client_a.config_.lease_.addr_, client_b.config_.lease_.addr_); -} - -// This test checks the server behavior in the following situation: -// - Client A identifies itself to the server using client identifier -// and the hardware address and requests allocation of the new lease. -// - Server allocates the lease to the client. -// - Client B has a different hardware address but is using the same -// client identifier as Client A. -// - Client B sends DHCPDISCOVER. -// - Server should determine that the client B is not client A, because -// it is using a different hadrware address, even though they use the -// same client identifier. As a consequence, the server should offer -// a different address to the client B. -// - The client B performs the 4-way exchange again and the server -// allocates a new address to the client, which should be different -// than the address used by the client A. -// - Client B is in the renewing state and it successfully renews its -// address. -// - Client A also renews its address successfully. -TEST_F(DORATest, twoAllocationsOverlap1) { - twoAllocationsOverlapTest("01:02:03:04:05:06", "12:34", - "02:02:03:03:04:04", "12:34"); -} - -// This test is similar to twoAllocationsOverlap1, but the -// clients differ by client identifier. -TEST_F(DORATest, twoAllocationsOverlap2) { - twoAllocationsOverlapTest("01:02:03:04:05:06", "12:34", - "01:02:03:04:05:06", "22:34"); -} - // This test checks the server behavior in the following situation: // - Client A identifies itself to the server using the hardware address // and client identifier. @@ -676,16 +608,14 @@ TEST_F(DORATest, twoAllocationsOverlap2) { // same reason. // - Client A renews its address successfully. TEST_F(DORATest, oneAllocationOverlap1) { - oneAllocationOverlapTest("01:02:03:04:05:06", "12:34", - "01:02:03:04:05:06", ""); + oneAllocationOverlapTest("12:34", ""); } // This test is similar to oneAllocationOverlap2 but this time the client A // uses no client identifier, and the client B uses the HW address and the // client identifier. The server behaves as previously. TEST_F(DORATest, oneAllocationOverlap2) { - oneAllocationOverlapTest("01:02:03:04:05:06", "", - "01:02:03:04:05:06", "12:34"); + oneAllocationOverlapTest("", "12:34"); } // This is a simple test for the host reservation. It creates a reservation diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc index eabfc8e6fe..53d6cb0fe6 100644 --- a/src/lib/dhcpsrv/alloc_engine.cc +++ b/src/lib/dhcpsrv/alloc_engine.cc @@ -1228,69 +1228,43 @@ hasAddressReservation(const AllocEngine::ClientContext4& ctx) { return (ctx.host_ && !ctx.host_->getIPv4Reservation().isV4Zero()); } -/// @brief Check if there is a lease for the client which message is processed. +/// @brief Finds existing lease in the database. /// -/// This function searches the lease database to find existing lease for the client. -/// It finds the lease using the client's HW address first. If the lease exists and -/// appears to belong to the client the lease is returned. Otherwise, the function -/// will search for the lease using the client identifier (if supplied). If the -/// lease exists and appears to belong to the client, it is returned. +/// This function searches for the lease in the database which belongs to the +/// client requesting allocation. If the client has supplied the client +/// identifier this identifier is used to look up the lease. If the lease is +/// not found using the client identifier, an additional lookup is performed +/// using the HW address, if supplied. If the lease is found using the HW +/// address, the function also checks if the lease belongs to the client, i.e. +/// there is no conflict between the client identifiers. /// -/// This function also identifies the conflicts between existing leases and the -/// lease to be allocated for the client, when the client is using a HW address -/// or client identifier which is already in use by the client having a lease in -/// the database. If the client uses an identifier which is already used by another -/// client and no other unique identifier which could be used to identify the client's -/// lease this function signals the conflict by returning 'true'. -/// -/// @param ctx Client context. -/// @param [out] client_lease Client's lease found in the database. -/// -/// @return true if there is a conflict of identifiers (HW address or client id) -/// between the client which message is being processed and the client which has -/// a lease in the database. When the value 'true' is returned, the caller should -/// cease the lease allocation for the client. -bool matchClientLease(const AllocEngine::ClientContext4& ctx, Lease4Ptr& client_lease) { - // Obtain the sole instance of the LeaseMgr. +/// @param ctx Context holding data extracted from the client's message, +/// including the HW address and client identifier. +/// @param [out] client_lease A pointer to the lease returned by this function +/// or null value if no has been lease found. +void findClientLease(const AllocEngine::ClientContext4& ctx, Lease4Ptr& client_lease) { LeaseMgr& lease_mgr = LeaseMgrFactory::instance(); - - // The server should hand out existing lease to the client, so we have to check - // if there is one. First, try to use the client's HW address. - client_lease = lease_mgr.getLease4(*ctx.hwaddr_, ctx.subnet_->getID()); - // If there is no lease for this HW address or the lease doesn't seem to be ours, - // we will have to use the client identifier. Note that in some situations two - // clients may use the same HW address so even if we find the lease for the HW - // address it doesn't mean it is ours, because client identifier may not match. - if (ctx.clientid_ && ((!client_lease) || (client_lease && !ctx.myLease(*client_lease)))) { - // Check if the lease is in conflict with the lease that we want to allocate. - // If the lease is in conflict because of using overlapping HW address or - // client identifier, we can't allocate the lease for this client. - if (client_lease && ctx.isInConflict(*client_lease)) { - return (true); - } - // There is no lease or the lease we found is not conflicting with the lease - // which we have found for the HW address, so there is still a chance that - // we will allocate the lease. Check if there is a lease using the client - // identifier. + // If client identifier has been supplied, use it to lookup the lease. This + // search will return no lease if the client doesn't have any lease in the + // database or if the client didn't use client identifier to allocate the + // existing lease (this include cases when the server was explicitly + // configured to ignore client identifier). + if (ctx.clientid_) { client_lease = lease_mgr.getLease4(*ctx.clientid_, ctx.subnet_->getID()); } - // Check if the lease we have found belongs to us. - if (client_lease && !ctx.myLease(*client_lease)) { - // If the lease doesn't belong to us, check if we can add new lease for - // the client which message we're processing, or its identifiers are - // in conflict with this lease. - if (ctx.isInConflict(*client_lease)) { - return (true); + // If no lease found using the client identifier, try the lookup using + // the HW address. + if (!client_lease && ctx.hwaddr_) { + client_lease = lease_mgr.getLease4(*ctx.hwaddr_, ctx.subnet_->getID()); + // This lookup may return the lease which has conflicting client + // identifier and thus is considered to belong to someone else. + // If this is the case, we need to toss the result and force the + // Allocation Engine to allocate another lease. + if (client_lease && !client_lease->belongsToClient(ctx.hwaddr_, ctx.clientid_)) { + client_lease.reset(); } - // If there is no conflict we can proceed and try to find the appropriate - // lease but we don't use the one we found, because it is assigned to - // someone else. Reset the pointer to indicate that we're not - // renewing this lease. - client_lease.reset(); } - - return (false); } } // end of anonymous namespace @@ -1321,37 +1295,6 @@ AllocEngine::ClientContext4::ClientContext4(const Subnet4Ptr& subnet, fake_allocation_(fake_allocation), old_lease_(), host_() { } - - -bool -AllocEngine::ClientContext4::myLease(const Lease4& lease) const { - if ((!hwaddr_ && lease.hwaddr_) || (hwaddr_ && !lease.hwaddr_)) { - return (false); - } - - if ((hwaddr_ && lease.hwaddr_) && (hwaddr_->hwaddr_ != lease.hwaddr_->hwaddr_)) { - return (false); - } - - if ((!clientid_ && lease.client_id_) || (clientid_ && !lease.client_id_)) { - return (false); - } - - if ((clientid_ && lease.client_id_) && (*clientid_ != *lease.client_id_)) { - return (false); - } - - return (true); -} - -bool -AllocEngine::ClientContext4::isInConflict(const Lease4& lease) const { - return ((!(hwaddr_ && lease.hwaddr_) && (clientid_ && lease.client_id_) && - (*clientid_ == *lease.client_id_)) || - (!(clientid_ && lease.client_id_) && (hwaddr_ && lease.hwaddr_) && - (hwaddr_->hwaddr_ == lease.hwaddr_->hwaddr_))); -} - Lease4Ptr AllocEngine::allocateLease4(ClientContext4& ctx) { // The NULL pointer indicates that the old lease didn't exist. It may @@ -1410,10 +1353,7 @@ AllocEngine::discoverLease4(AllocEngine::ClientContext4& ctx) { // if there is a conflict with existing lease and the allocation should // not be continued. Lease4Ptr client_lease; - bool conflict = matchClientLease(ctx, client_lease); - if (conflict) { - return (Lease4Ptr()); - } + findClientLease(ctx, client_lease); // new_lease will hold the pointer to the lease that we will offer to the // caller. @@ -1499,10 +1439,7 @@ AllocEngine::requestLease4(AllocEngine::ClientContext4& ctx) { // if there is a conflict with existing lease and the allocation should // not be continued. Lease4Ptr client_lease; - bool conflict = matchClientLease(ctx, client_lease); - if (conflict) { - return (Lease4Ptr()); - } + findClientLease(ctx, client_lease); // Obtain the sole instance of the LeaseMgr. LeaseMgr& lease_mgr = LeaseMgrFactory::instance(); @@ -1537,7 +1474,8 @@ AllocEngine::requestLease4(AllocEngine::ClientContext4& ctx) { // if the address is in use by our client or another client. // If it is in use by another client, the address can't be // allocated. - if (existing && !existing->expired() && !ctx.myLease(*existing)) { + if (existing && !existing->expired() && + !existing->belongsToClient(ctx.hwaddr_, ctx.clientid_)) { return (Lease4Ptr()); } diff --git a/src/lib/dhcpsrv/alloc_engine.h b/src/lib/dhcpsrv/alloc_engine.h index cb012b9af0..a93ef6205b 100644 --- a/src/lib/dhcpsrv/alloc_engine.h +++ b/src/lib/dhcpsrv/alloc_engine.h @@ -760,45 +760,6 @@ public: const bool fwd_dns_update, const bool rev_dns_update, const std::string& hostname, const bool fake_allocation); - /// @brief Check if the specified lease belongs to the client. - /// - /// This method compares the hardware address and the client id - /// in the lease with the relevant values in the context. That - /// way the method determines whether the lease belongs to the - /// client which message the server is processing. - /// - /// @return true if the lease belongs to the client for which - /// the context has been created, false otherwise. - bool myLease(const Lease4& lease) const; - - /// @brief Check if the lease conflicts with the context. - /// - /// This method is used in cases when there is a lease in the lease database - /// for the client and the server receives the message from another client - /// which may be potentially using the same client identifier or HW address. - /// - /// Currently the allocation engine allows for allocating a lease for the - /// client which is using the same HW address or client identifier as the - /// client which already has a lease in the database. However, the value of - /// one of the identifiers must be unique. In other words, two clients may - /// have the same client identifier but different HW addresses and the - /// leases can be allocated for both (there is no conflict). - /// - /// The other possible case is that one of the clients uses both HW address - /// and client identifier, and another client is using only one of those - /// and its value is the same as for the former client. The allocation - /// engine treats it as a conflict because there is no unique value in the - /// lease database by which the server could distinguish the clients. - /// Hence, it doesn't allocate the lease from the context in conflict. - /// - /// This method detects the conflict described above. - /// - /// @param lease Existing lease to be checked. - /// - /// @return true if the lease is in conflict with the context, false - /// otherwise. - bool isInConflict(const Lease4& lease) const; - }; /// @brief Pointer to the @c ClientContext4. diff --git a/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc index 7935aa65dc..4c36408624 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc +++ b/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc @@ -536,6 +536,65 @@ TEST_F(AllocEngine4Test, requestReuseExpiredLease4) { EXPECT_TRUE(*ctx.old_lease_ == original_lease); } +// This test checks that the Allocation Engine correcly identifies the +// existing client's lease in the lease database, using the client +// identifier and HW address. +TEST_F(AllocEngine4Test, identifyClientLease) { + Lease4Ptr lease(new Lease4(IOAddress("192.0.2.101"), hwaddr_, clientid_, + 100, 30, 60, time(NULL), subnet_->getID())); + LeaseMgrFactory::instance().addLease(lease); + + AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false); + AllocEngine::ClientContext4 ctx(subnet_, clientid_, hwaddr_, + IOAddress::IPV4_ZERO_ADDRESS(), + false, false, "", true); + + Lease4Ptr identified_lease = engine.allocateLease4(ctx); + ASSERT_TRUE(identified_lease); + EXPECT_EQ("192.0.2.101", identified_lease->addr_.toText()); + + ctx.hwaddr_ = hwaddr2_; + identified_lease = engine.allocateLease4(ctx); + ASSERT_TRUE(identified_lease); + EXPECT_EQ("192.0.2.101", identified_lease->addr_.toText()); + + ctx.hwaddr_ = hwaddr_; + ctx.clientid_ = clientid2_; + identified_lease = engine.allocateLease4(ctx); + ASSERT_TRUE(identified_lease); + EXPECT_NE(identified_lease->addr_.toText(), "192.0.2.101"); + + ctx.clientid_.reset(); + identified_lease = engine.allocateLease4(ctx); + ASSERT_TRUE(identified_lease); + EXPECT_EQ("192.0.2.101", identified_lease->addr_.toText()); + + ctx.hwaddr_ = hwaddr2_; + identified_lease = engine.allocateLease4(ctx); + ASSERT_TRUE(identified_lease); + EXPECT_NE(identified_lease->addr_.toText(), "192.0.2.101"); + + lease->client_id_.reset(); + ASSERT_NO_THROW(LeaseMgrFactory::instance().updateLease4(lease)); + + ctx.hwaddr_ = hwaddr_; + ctx.clientid_ = clientid_; + identified_lease = engine.allocateLease4(ctx); + ASSERT_TRUE(identified_lease); + EXPECT_EQ("192.0.2.101", identified_lease->addr_.toText()); + + ctx.clientid_.reset(); + identified_lease = engine.allocateLease4(ctx); + ASSERT_TRUE(identified_lease); + EXPECT_EQ("192.0.2.101", identified_lease->addr_.toText()); + + ctx.clientid_ = clientid_; + ctx.hwaddr_ = hwaddr2_; + identified_lease = engine.allocateLease4(ctx); + ASSERT_TRUE(identified_lease); + EXPECT_NE(identified_lease->addr_.toText(), "192.0.2.101"); +} + // This test checks that when the client requests the address which belongs // to another client, the allocation engine returns NULL (for the // DHCPREQUEST case) or a lease for the address which belongs to this @@ -924,7 +983,7 @@ TEST_F(AllocEngine4Test, reservedAddressExistingLeaseInvalidHint) { CfgMgr::instance().commit(); // Create a lease for the client for a different address than reserved. - Lease4Ptr lease(new Lease4(IOAddress("192.0.2.101"), hwaddr_, 0, 0, + Lease4Ptr lease(new Lease4(IOAddress("192.0.2.101"), hwaddr_, ClientIdPtr(), 100, 30, 60, time(NULL), subnet_->getID(), false, false, "")); LeaseMgrFactory::instance().addLease(lease); @@ -946,6 +1005,7 @@ TEST_F(AllocEngine4Test, reservedAddressExistingLeaseInvalidHint) { AllocEngine::ClientContext4 ctx2(subnet_, clientid_, hwaddr_, IOAddress("192.0.2.101"), false, false, "", false); + AllocEngine::findReservation(ctx2); allocated_lease = engine.allocateLease4(ctx2); // The client has reservation so the server wants to allocate a // reserved address and doesn't want to renew the address that the diff --git a/src/lib/dhcpsrv/tests/alloc_engine_utils.cc b/src/lib/dhcpsrv/tests/alloc_engine_utils.cc index f0ff369ab6..1f20085dca 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine_utils.cc +++ b/src/lib/dhcpsrv/tests/alloc_engine_utils.cc @@ -351,6 +351,8 @@ AllocEngine4Test::AllocEngine4Test() { HostMgr::instance().create(); clientid_ = ClientIdPtr(new ClientId(vector(8, 0x44))); + clientid2_ = ClientIdPtr(new ClientId(vector(8, 0x56))); + uint8_t mac[] = { 0, 1, 22, 33, 44, 55}; // Let's use odd hardware type to check if there is no Ethernet diff --git a/src/lib/dhcpsrv/tests/alloc_engine_utils.h b/src/lib/dhcpsrv/tests/alloc_engine_utils.h index 80f159dad5..bc0728aa57 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine_utils.h +++ b/src/lib/dhcpsrv/tests/alloc_engine_utils.h @@ -383,6 +383,7 @@ public: } ClientIdPtr clientid_; ///< Client-identifier (value used in tests) + ClientIdPtr clientid2_; ///< Alternative client-identifier. HWAddrPtr hwaddr_; ///< Hardware address (value used in tests) HWAddrPtr hwaddr2_; ///< Alternative hardware address. Subnet4Ptr subnet_; ///< Subnet4 (used in tests)