From: Marcin Siodelski Date: Wed, 11 Mar 2015 09:13:28 +0000 (+0100) Subject: [3688] Prevent allocation engine from allocating 0 IPv4 address. X-Git-Tag: trac3764_base~12^2~13 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=df5f884a62aad5c3f0500be9590056bc5d5bf50e;p=thirdparty%2Fkea.git [3688] Prevent allocation engine from allocating 0 IPv4 address. --- diff --git a/src/bin/dhcp4/tests/fqdn_unittest.cc b/src/bin/dhcp4/tests/fqdn_unittest.cc index 1257e46820..280ed3280a 100644 --- a/src/bin/dhcp4/tests/fqdn_unittest.cc +++ b/src/bin/dhcp4/tests/fqdn_unittest.cc @@ -55,7 +55,6 @@ const char* CONFIGS[] = { " \"reservations\": [" " {" " \"hw-address\": \"aa:bb:cc:dd:ee:ff\"," - " \"ip-address\": \"10.0.0.5\"," " \"hostname\": \"unique-host.example.org\"" " }" " ]" @@ -83,7 +82,6 @@ const char* CONFIGS[] = { " \"reservations\": [" " {" " \"hw-address\": \"aa:bb:cc:dd:ee:ff\"," - " \"ip-address\": \"10.0.0.5\"," " \"hostname\": \"foobar.org\"" " }" " ]" diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc index 0e75f19980..4e38631452 100644 --- a/src/lib/dhcpsrv/alloc_engine.cc +++ b/src/lib/dhcpsrv/alloc_engine.cc @@ -1215,6 +1215,26 @@ addressReserved(const IOAddress& address, const AllocEngine::ClientContext4& ctx return (false); } +/// @brief Check if the context contains the reservation for the +/// IPv4 address. +/// +/// This convenience function checks if the context contains the reservation +/// for the IPv4 address. Note that some reservations may not assign a +/// static IPv4 address to the clients, but may rather reserve a hostname. +/// Allocation engine should check if the existing reservation is made +/// for the IPv4 address and if it is not, allocate the address from the +/// dynamic pool. The allocation engine uses this function to check if +/// the reservation is made for the IPv4 address. +/// +/// @param ctx Client context holding the data extracted from the +/// client's message. +/// +/// @return true if the context contains the reservation for the IPv4 address. +bool +hasAddressReservation(const AllocEngine::ClientContext4& ctx) { + return (ctx.host_ && !ctx.host_->getIPv4Reservation().isV4Zero()); +} + } // end of anonymous namespace namespace isc { @@ -1339,7 +1359,7 @@ AllocEngine::discoverLease4(AllocEngine::ClientContext4& ctx) { // Check if there is a reservation for the client. If there is, we want to // assign the reserved address, rather than any other one. - if (ctx.host_) { + if (hasAddressReservation(ctx)) { // If the client doesn't have a lease or the leased address is different // than the reserved one then let's try to allocate the reserved address. // Otherwise the address that the client has is the one for which it @@ -1438,7 +1458,7 @@ AllocEngine::requestLease4(AllocEngine::ClientContext4& ctx) { return (Lease4Ptr()); } - } else if (ctx.host_) { + } else if (hasAddressReservation(ctx)) { // The client hasn't specified an address to allocate, so the // allocation engine needs to find an appropriate address. // If there is a reservation for the client, let's try to @@ -1462,7 +1482,8 @@ AllocEngine::requestLease4(AllocEngine::ClientContext4& ctx) { // address it is possible that the client was offered this different // address because the reserved address is in use. We will have to // check if the address is in use. - if (ctx.host_ && (ctx.host_->getIPv4Reservation() != ctx.requested_address_)) { + if (hasAddressReservation(ctx) && + (ctx.host_->getIPv4Reservation() != ctx.requested_address_)) { existing = LeaseMgrFactory::instance().getLease4(ctx.host_->getIPv4Reservation()); // If the reserved address is not in use, i.e. the lease doesn't // exist or is expired, and the client is requesting a different @@ -1476,7 +1497,8 @@ AllocEngine::requestLease4(AllocEngine::ClientContext4& ctx) { // The use of the out-of-pool addresses is only allowed when the requested // address is reserved for the client. If the address is not reserved one // and it doesn't belong to the dynamic pool, do not allocate it. - if ((!ctx.host_ || (ctx.host_->getIPv4Reservation() != ctx.requested_address_)) && + if ((!hasAddressReservation(ctx) || + (ctx.host_->getIPv4Reservation() != ctx.requested_address_)) && !ctx.subnet_->inPool(Lease4::TYPE_V4, ctx.requested_address_)) { return (Lease4Ptr()); } diff --git a/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc b/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc index fbe49699a2..8685b82654 100644 --- a/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc +++ b/src/lib/dhcpsrv/tests/alloc_engine4_unittest.cc @@ -1263,7 +1263,7 @@ TEST_F(AllocEngine4Test, reservedAddressShortPool) { // Create short pool with only one address. initSubnet(IOAddress("192.0.2.100"), IOAddress("192.0.2.100")); // Reserve the address for a different client. - HostPtr host(new Host(&hwaddr2_->hwaddr_[0], hwaddr_->hwaddr_.size(), + HostPtr host(new Host(&hwaddr2_->hwaddr_[0], hwaddr2_->hwaddr_.size(), Host::IDENT_HWADDR, subnet_->getID(), SubnetID(0), IOAddress("192.0.2.100"))); CfgMgr::instance().getStagingCfg()->getCfgHosts()->add(host); @@ -1292,6 +1292,35 @@ TEST_F(AllocEngine4Test, reservedAddressShortPool) { EXPECT_EQ("192.0.2.100", allocated_lease->addr_.toText()); } +// This test checks that the AllocEngine allocates an address from the +// dynamic pool if the client's reservation is made for a hostname but +// not for an address. +TEST_F(AllocEngine4Test, reservedHostname) { + AllocEngine engine(AllocEngine::ALLOC_ITERATIVE, 100, false); + + // Create a reservation for a hostname. Address is set to 0 which + // indicates that there is no reservation. + HostPtr host(new Host(&hwaddr_->hwaddr_[0], hwaddr_->hwaddr_.size(), + Host::IDENT_HWADDR, subnet_->getID(), + SubnetID(0), IOAddress::IPV4_ZERO_ADDRESS(), + "foo.example.org")); + CfgMgr::instance().getStagingCfg()->getCfgHosts()->add(host); + CfgMgr::instance().commit(); + + // Try to allocate a lease. + AllocEngine::ClientContext4 ctx(subnet_, ClientIdPtr(), hwaddr_, + IOAddress::IPV4_ZERO_ADDRESS(), false, false, + "foo.example.org", true); + Lease4Ptr allocated_lease = engine.allocateLease4(ctx); + ASSERT_TRUE(allocated_lease); + ASSERT_FALSE(allocated_lease->addr_.isV4Zero()); + + ctx.requested_address_ = allocated_lease->addr_; + ctx.fake_allocation_ = false; + allocated_lease = engine.allocateLease4(ctx); + ASSERT_TRUE(allocated_lease); +} + // This test checks that the AllocEngine::findReservation method finds // and returns host reservation for the DHCPv4 client using the data from // the client context. If the host reservation can't be found, it sets