diff options
author | Tomek Mrugalski <tomasz@isc.org> | 2015-09-08 14:35:28 +0200 |
---|---|---|
committer | Tomek Mrugalski <tomasz@isc.org> | 2015-09-08 16:04:57 +0200 |
commit | 348b7382dd9ddfeaffc0bfa45bb862f01e42df36 (patch) | |
tree | c141fe27988654c654e24eddd4cd3db8514941ef /src | |
parent | [master] ChangeLog updated after 3983 merge. (diff) | |
download | kea-348b7382dd9ddfeaffc0bfa45bb862f01e42df36.tar.xz kea-348b7382dd9ddfeaffc0bfa45bb862f01e42df36.zip |
[3981] DHCPDECLINE support implemented.
Diffstat (limited to 'src')
-rw-r--r-- | src/bin/dhcp4/dhcp4_messages.mes | 12 | ||||
-rw-r--r-- | src/bin/dhcp4/dhcp4_srv.cc | 107 | ||||
-rw-r--r-- | src/bin/dhcp4/tests/Makefile.am | 1 | ||||
-rw-r--r-- | src/bin/dhcp4/tests/decline_unittest.cc | 291 | ||||
-rw-r--r-- | src/bin/dhcp4/tests/dhcp4_client.cc | 24 | ||||
-rw-r--r-- | src/bin/dhcp4/tests/dhcp4_client.h | 7 |
6 files changed, 440 insertions, 2 deletions
diff --git a/src/bin/dhcp4/dhcp4_messages.mes b/src/bin/dhcp4/dhcp4_messages.mes index b269e024b5..aa8dbdc258 100644 --- a/src/bin/dhcp4/dhcp4_messages.mes +++ b/src/bin/dhcp4/dhcp4_messages.mes @@ -177,6 +177,18 @@ This message is printed when DHCPv4 server disables an interface from being used to receive DHCPv4 traffic. Sockets on this interface will not be opened by the Interface Manager until interface is enabled. +% DHCP4_DECLINE_LEASE_NOT_FOUND Received DHCPDECLINE for addr %1, but no such lease found. Client: %2 +This warning message indicates that a client reported that his address was +detected as a duplicate (i.e. other device in the network is using this address). +However, the server hoes not have a record for this address. This may indicate +client's error or purged server's database. + +% DHCP4_DECLINE_LEASE_MISMATCH Received DHCPDECLINE for addr %1 from %2, but the data doesn't match: received hwaddr: %3, lease hwaddr: %4, received client-id: %5, lease client-id: %6 +This informational message means that a client attempts to report his address +as declined (i.e. used by unknown entity). The server has information about +a lease for that address, but the client's hardware address or client identifier +do not match the server's stored information. Client's request will be ignored. + % DHCP4_DHCID_COMPUTE_ERROR failed to compute the DHCID for lease: %1, reason: %2 This error message is logged when the attempt to compute DHCID for a specified lease has failed. The lease details and reason for failure is logged in the diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index 634b0b2309..f6b6bc1b8d 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -1841,8 +1841,111 @@ Dhcpv4Srv::processRelease(Pkt4Ptr& release) { } void -Dhcpv4Srv::processDecline(Pkt4Ptr&) { - /// @todo Implement this (also see ticket #3116) +Dhcpv4Srv::processDecline(Pkt4Ptr& decline) { + + // Server-id is mandatory in DHCPDECLINE (see table 5, RFC2131) + sanityCheck(decline, MANDATORY); + + // Client is supposed to specify the address being declined in + // Requested IP address option, but must not set its ciaddr. + // (again, see table 5 in RFC2131). + + OptionCustomPtr opt_requested_address = boost::dynamic_pointer_cast< + OptionCustom>(query->getOption(DHO_DHCP_REQUESTED_ADDRESS)); + if (!opt_requested_address) { + + isc_throw(RFCViolation, "Mandatory 'Requested IP address' option missing" + "in DHCPDECLINE sent from " << decline->getLabel()); + } + IOAddress addr(opt_requested_address->readAddress()); + + // We could also extract client's address from ciaddr, but that's clearly + // against RFC2131. + + // Now we need to check whether this address really belongs to the client + // that attempts to decline it. + Lease4Ptr lease = LeaseMgrFactory::instance().getLease4(addr); + + if (!lease) { + // Client tried to decline an address, but we don't have a lease for + // that address. Let's ignore it. + // + // We could assume that we're recovering from a mishandled migration + // to a new server and mark the address as declined, but the window of + // opportunity for that to be useful is small and the attack vector + // would be pretty severe. + LOG_WARN(dhcp4_logger, DHCP4_DECLINE_LEASE_NOT_FOUND) + .arg(addr->toText()).arg(decline->getLabel()); + return; + } + + // Get client-id, if available. + OptionPtr opt_clientid = query->getOption(DHO_DHCP_CLIENT_IDENTIFIER); + ClientIdPtr client_id; + if (opt_clientid) { + client_id.reset(new ClientId(opt_clientid->getData())); + } + + // Check if the client attempted to decline a lease it doesn't own. + if (!lease->belongsToClient(ctx.hwaddr_, ctx.clientid_)) { + + // Get printable hardware addresses + string client_hw = decline->getHWAddr() ? + decline->getHWAddr()->toText(false) : "(none)"; + string lease_hw = lease->hwaddr_ ? + lease->hwaddr_->getHWAddr()->toText(false) : "(none)"; + + // Get printable client-ids + string client_id = client_id ? client_id->toText() : "(none)"; + string lease_id = lease->client_id ? lease->client_id_->toText() : "(none)"; + + LOG_WARN(dhcp4_logger, DHCP4_DECLINE_LEASE_MISMATCH) + .arg(addr->toText()).arg(decline->getLabel()) + .arg(client_hw).arg(lease_hw).arg(client_id).arg(lease_id); + return; + } + + // Ok, all is good. The client is reporting its own address. Let's + // process it. + declineLease(lease); +} + +void +Dhcpv4Srv::declineLease(const Lease4Ptr& lease, const std::string& descr) { + + // Clean up DDNS, if needed. + if (CfgMgr::instance().ddnsEnabled()) { + // Remove existing DNS entries for the lease, if any. + // queueNameChangeRequest will do the necessary checks and will + // skip the update, if not needed. + queueNameChangeRequest(isc::dhcp_ddns::CHG_REMOVE, lease); + } + + // Bump up the statistics. + isc::stats::StatsMgr::instance().addValue("pkt4-parse-failed", + static_cast<int64_t>(1)); + + // @todo: Call hooks. + + // We need to disassociate the lease from the client. Once we move a lease + // to declined state, it is no longer associated with the client in any + // way. + lease->hwaddr_.resize(0); + lease->hwaddr_.client_id_.reset(); + lease->t1_ = 0; + lease->t2_ = 0; + lease->valid_lft_ = CfgMgr::instance().getCurrentCfg()->getDelinedPeriod(); + lease->cltt_ = now(); + lease->hostname_ = string(""); + lease->fqdn_fwd_ = false; + lease->fqdn_rev_ = false; + + lease->state = DECLINED; + LeaseMgrFactory::instance().updateLease4(); + + LOG_INFO(dhcp4_logger, DHCP4_DECLINE_ADDR) + .arg(lease->addr_->toText()).arg(descr) + .arg(lease->valid_lft_); } Pkt4Ptr diff --git a/src/bin/dhcp4/tests/Makefile.am b/src/bin/dhcp4/tests/Makefile.am index 1bff313c67..83966fe06b 100644 --- a/src/bin/dhcp4/tests/Makefile.am +++ b/src/bin/dhcp4/tests/Makefile.am @@ -86,6 +86,7 @@ dhcp4_unittests_SOURCES += dhcp4_client.cc dhcp4_client.h dhcp4_unittests_SOURCES += inform_unittest.cc dhcp4_unittests_SOURCES += dora_unittest.cc dhcp4_unittests_SOURCES += release_unittest.cc +dhcp4_unittests_SOURCES += decline_unittest.cc dhcp4_unittests_SOURCES += kea_controller_unittest.cc diff --git a/src/bin/dhcp4/tests/decline_unittest.cc b/src/bin/dhcp4/tests/decline_unittest.cc new file mode 100644 index 0000000000..78a56a09e5 --- /dev/null +++ b/src/bin/dhcp4/tests/decline_unittest.cc @@ -0,0 +1,291 @@ +// Copyright (C) 2015 Internet Systems Consortium, Inc. ("ISC") +// +// Permission to use, copy, modify, and/or distribute this software for any +// purpose with or without fee is hereby granted, provided that the above +// copyright notice and this permission notice appear in all copies. +// +// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH +// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY +// AND FITNESS. IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT, +// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM +// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE +// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR +// PERFORMANCE OF THIS SOFTWARE. + + +#include <config.h> +#include <asiolink/io_address.h> +#include <cc/data.h> +#include <dhcp/dhcp4.h> +#include <dhcp/tests/iface_mgr_test_config.h> +#include <dhcpsrv/cfgmgr.h> +#include <dhcpsrv/subnet_id.h> +#include <dhcp4/tests/dhcp4_test_utils.h> +#include <dhcp4/tests/dhcp4_client.h> +#include <stats/stats_mgr.h> +#include <boost/shared_ptr.hpp> +#include <sstream> + +using namespace isc; +using namespace isc::asiolink; +using namespace isc::data; +using namespace isc::dhcp; +using namespace isc::dhcp::test; +using namespace isc::stats; + +namespace { + +/// @brief Set of JSON configurations used throughout the Decline tests. +/// +/// - Configuration 0: +/// - Used for testing Release message processing +/// - 1 subnet: 10.0.0.0/24 +/// - 1 pool: 10.0.0.10-10.0.0.100 +/// - Router option present: 10.0.0.200 and 10.0.0.201 +const char* DECLINE_CONFIGS[] = { +// Configuration 0 + "{ \"interfaces-config\": {" + " \"interfaces\": [ \"*\" ]" + "}," + "\"valid-lifetime\": 600," + "\"subnet4\": [ { " + " \"subnet\": \"10.0.0.0/24\", " + " \"id\": 1," + " \"pools\": [ { \"pool\": \"10.0.0.10-10.0.0.100\" } ]," + " \"option-data\": [ {" + " \"name\": \"routers\"," + " \"code\": 3," + " \"data\": \"10.0.0.200,10.0.0.201\"," + " \"csv-format\": true," + " \"space\": \"dhcp4\"" + " } ]" + " } ]" + "}" +}; + +/// @brief Test fixture class for testing 4-way (DORA) exchanges. +/// +/// @todo Currently there is a limit number of test cases covered here. +/// In the future it is planned that the tests from the +/// dhcp4_srv_unittest.cc will be migrated here and will use the +/// @c Dhcp4Client class. +class DeclineTest : public Dhcpv4SrvTest { +public: + + enum ExpectedResult { + SHOULD_PASS, + SHOULD_FAIL + }; + + /// @brief Constructor. + /// + /// Sets up fake interfaces. + DeclineTest() + : Dhcpv4SrvTest(), + iface_mgr_test_config_(true) { + IfaceMgr::instance().openSockets4(); + } + + /// @brief Performs 4-way exchange to obtain new lease. + /// + /// @param client Client to be used to obtain a lease. + void acquireLease(Dhcp4Client& client); + + /// @brief Tests if the acquired lease is or is not declined. + /// + /// @param hw_address_1 HW Address to be used to acquire the lease. + /// @param client_id_1 Client id to be used to acquire the lease. + /// @param hw_address_2 HW Address to be used to decline the lease. + /// @param client_id_2 Client id to be used to decline the lease. + /// @param expected_result SHOULD_PASS if the lease is expected to + /// be successfully released, or SHOULD_FAIL if the lease is expected + /// to not be released. + void acquireAndDecline(const std::string& hw_address_1, + const std::string& client_id_1, + const std::string& hw_address_2, + const std::string& client_id_2, + ExpectedResult expected_result); + + /// @brief Interface Manager's fake configuration control. + IfaceMgrTestConfig iface_mgr_test_config_; + +}; + +void +DeclineTest::acquireLease(Dhcp4Client& client) { + // Perform 4-way exchange with the server but to not request any + // specific address in the DHCPDISCOVER message. + 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<int>(resp->getType())); + // Response must not be relayed. + EXPECT_FALSE(resp->isRelayed()); + // Make sure that the server id is present. + EXPECT_EQ("10.0.0.1", client.config_.serverid_.toText()); + // Make sure that the client has got the lease with the requested address. + ASSERT_NE(client.config_.lease_.addr_.toText(), "0.0.0.0"); + Lease4Ptr lease = LeaseMgrFactory::instance().getLease4(client.config_.lease_.addr_); + ASSERT_TRUE(lease); +} + +void +DeclineTest::acquireAndDecline(const std::string& hw_address_1, + const std::string& client_id_1, + const std::string& hw_address_2, + const std::string& client_id_2, + ExpectedResult expected_result) { + CfgMgr::instance().clear(); + Dhcp4Client client(Dhcp4Client::SELECTING); + // Configure DHCP server. + configure(RELEASE_CONFIGS[0], *client.getServer()); + // Explicitly set the client id. + client.includeClientId(client_id_1); + // Explicitly set the HW address. + client.setHWAddress(hw_address_1); + // Perform 4-way exchange to obtain a new lease. + acquireLease(client); + + std::stringstream name; + + // Let's get the subnet-id and generate statistics name out of it + const Subnet4Collection* subnets = + CfgMgr::instance().getCurrentCfg()->getCfgSubnets4()->getAll(); + ASSERT_EQ(1, subnets->size()); + name << "subnet[" << subnets->at(0)->getID() << "].declined-addresses"; + + ObservationPtr declined_cnt = StatsMgr::instance().getObservation(name.str()); + ASSERT_TRUE(declined_cnt); + uint64_t before = declined_cnt->getInteger().first; + + // Remember the acquired address. + IOAddress declined_address = client.config_.lease_.addr_; + + // Explicitly set the client id for DHCPRELEASE. + client.includeClientId(client_id_2); + // Explicitly set the HW address for DHCPRELEASE. + client.setHWAddress(hw_address_2); + + // Send the release and make sure that the lease is removed from the + // server. + ASSERT_NO_THROW(client.doDecline()); + Lease4Ptr lease = LeaseMgrFactory::instance().getLease4(declined_address); + + declined_cnt = StatsMgr::instance().getObservation(name.str()); + ASSERT_TRUE(declined_cnt); + uint64_t after = declined_cnt->getInteger().first; + + ASSERT_TRUE(lease); + // We check if the deline process was successful by checking if the + // lease is in the database and what is its state. + if (expected_result == SHOULD_PASS) { + EXPECT_EQ(DECLINED, lease->state); + + // The decline succeded, so the declined-addresses statistic should + // be increased by one + EXPECT_EQ(after, before + 1); + } else { + // the decline was supposed, to be rejected. + EXPECT_EQ(DEFAULT, lease->state); + + // The decline failed, so the declined-addresses should be the same + // as before + EXPECT_EQ(before, after); + } +} + +// This test checks that the client can acquire and release the lease. +TEST_F(DeclineTest, releaseNoIdentifierChange) { + acquireAndDecline("01:02:03:04:05:06", "12:14", + "01:02:03:04:05:06", "12:14", + SHOULD_PASS); +} + +// This test verifies the release correctness in the following case: +// - Client acquires new lease using HW address only +// - Client sends the DHCPRELEASE with valid HW address and without +// client identifier. +// - The server successfully releases the lease. +TEST_F(DeclineTest, releaseHWAddressOnly) { + acquireAndDecline("01:02:03:04:05:06", "", + "01:02:03:04:05:06", "", + SHOULD_PASS); +} + +// This test verifies the release correctness in the following case: +// - Client acquires new lease using the client identifier and HW address +// - Client sends the DHCPRELEASE with valid HW address but with no +// client identifier. +// - The server successfully releases the lease. +TEST_F(DeclineTest, releaseNoClientId) { + acquireAndDecline("01:02:03:04:05:06", "12:14", + "01:02:03:04:05:06", "", + SHOULD_PASS); +} + +// This test verifies the release correctness in the following case: +// - Client acquires new lease using HW address +// - Client sends the DHCPRELEASE with valid HW address and some +// client identifier. +// - The server identifies the lease using HW address and releases +// this lease. +TEST_F(DeclineTest, releaseNoClientId2) { + acquireAndDecline("01:02:03:04:05:06", "", + "01:02:03:04:05:06", "12:14", + SHOULD_PASS); +} + +// This test checks the server's behavior in the following case: +// - Client acquires new lease using the client identifier and HW address +// - Client sends the DHCPRELEASE with the valid HW address but with invalid +// client identifier. +// - The server should not remove the lease. +TEST_F(DeclineTest, releaseNonMatchingClientId) { + acquireAndDecline("01:02:03:04:05:06", "12:14", + "01:02:03:04:05:06", "12:15:16", + SHOULD_FAIL); +} + +// This test checks the server's behavior in the following case: +// - Client acquires new lease using client identifier and HW address +// - Client sends the DHCPRELEASE with the same client identifier but +// different HW address. +// - The server uses client identifier to find the client's lease and +// releases it. +TEST_F(DeclineTest, releaseNonMatchingHWAddress) { + acquireAndDecline("01:02:03:04:05:06", "12:14", + "06:06:06:06:06:06", "12:14", + SHOULD_PASS); +} + +// This test checks the server's behavior in the following case: +// - Client acquires new lease. +// - Client sends DHCPRELEASE with the ciaddr set to a different +// address than it has acquired from the server. +// - Server determines that the client is trying to release a +// wrong address and will refuse to release. +TEST_F(DeclineTest, releaseNonMatchingIPAddress) { + Dhcp4Client client(Dhcp4Client::SELECTING); + // Configure DHCP server. + configure(RELEASE_CONFIGS[0], *client.getServer()); + // Perform 4-way exchange to obtain a new lease. + acquireLease(client); + + // Remember the acquired address. + IOAddress leased_address = client.config_.lease_.addr_; + + // Modify the client's address to force it to release a different address + // than it has obtained from the server. + client.config_.lease_.addr_ = IOAddress(static_cast<uint32_t>(leased_address) + 1); + + // Send DHCPRELEASE and make sure it was unsuccessful, i.e. the lease + // remains in the database. + ASSERT_NO_THROW(client.doDecline()); + Lease4Ptr lease = LeaseMgrFactory::instance().getLease4(leased_address); + ASSERT_TRUE(lease); + EXPECT_EQ(DEFAULT, lease->state_); +} + +} // end of anonymous namespace diff --git a/src/bin/dhcp4/tests/dhcp4_client.cc b/src/bin/dhcp4/tests/dhcp4_client.cc index 750e6657b2..6ff104aba8 100644 --- a/src/bin/dhcp4/tests/dhcp4_client.cc +++ b/src/bin/dhcp4/tests/dhcp4_client.cc @@ -281,6 +281,30 @@ Dhcp4Client::doRelease() { } void +Dhcp4Client::doDecline() { + if (config_.lease_.addr_ == IOAddress::IPV4_ZERO_ADDRESS()) { + isc_throw(Dhcp4ClientError, "failed to send the decline" + " message because client doesn't have a lease"); + } + context_.query_ = createMsg(DHCPDECLINE); + + // Set ciaddr to 0. + context_.query_->setCiaddr(IOAddress("0.0.0.0")); + + // Include Requested IP Address Option + addRequestedAddress(config_.lease_.addr_); + + // Include client identifier. + appendClientId(); + + // Remove configuration. + config_.reset(); + + // Send the message to the server. + sendMsg(context_.query_); +} + +void Dhcp4Client::doRequest() { context_.query_ = createMsg(DHCPREQUEST); diff --git a/src/bin/dhcp4/tests/dhcp4_client.h b/src/bin/dhcp4/tests/dhcp4_client.h index 4f50129c50..533a42489a 100644 --- a/src/bin/dhcp4/tests/dhcp4_client.h +++ b/src/bin/dhcp4/tests/dhcp4_client.h @@ -172,6 +172,13 @@ public: /// The released lease is removed from the client's configuration. void doRelease(); + + /// @brief Sends DHCPDECLINE Message to the server. + /// + /// This method simulates sending the DHCPDECLINE message to the server. + /// The released lease is removed from the client's configuration. + void doDecline(); + /// @brief Sends DHCPREQUEST Message to the server and receives a response. /// /// This method simulates sending the DHCPREQUEST message to the server and |