summaryrefslogtreecommitdiffstats
path: root/src/lib
diff options
context:
space:
mode:
authorMarcin Siodelski <marcin@isc.org>2015-05-11 14:24:58 +0200
committerMarcin Siodelski <marcin@isc.org>2015-05-11 17:14:29 +0200
commite0bbec0e10b1539df28fa987a2b21326c0fff0e5 (patch)
tree2af01416be647a76f9cf571b151cd682d9928717 /src/lib
parent[3747] Added method which checks if lease belongs to the client. (diff)
downloadkea-e0bbec0e10b1539df28fa987a2b21326c0fff0e5.tar.xz
kea-e0bbec0e10b1539df28fa987a2b21326c0fff0e5.zip
[3747] Use client identifier over MAC to get existing client's lease.
Diffstat (limited to 'src/lib')
-rw-r--r--src/lib/dhcpsrv/alloc_engine.cc128
-rw-r--r--src/lib/dhcpsrv/alloc_engine.h39
-rw-r--r--src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc62
-rw-r--r--src/lib/dhcpsrv/tests/alloc_engine_utils.cc2
-rw-r--r--src/lib/dhcpsrv/tests/alloc_engine_utils.h1
5 files changed, 97 insertions, 135 deletions
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<uint8_t>(8, 0x44)));
+ clientid2_ = ClientIdPtr(new ClientId(vector<uint8_t>(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)