From: Tomek Mrugalski Date: Mon, 10 Feb 2014 18:54:51 +0000 (+0100) Subject: [master] Merge branch 'trac3274' DHCP client classification X-Git-Tag: bind10-1.2.0beta1-release~57^2~1 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=e3f6de57b908d44370bad1a9cdec70ed2f21bfa8;p=thirdparty%2Fkea.git [master] Merge branch 'trac3274' DHCP client classification Conflicts: ChangeLog src/bin/dhcp4/dhcp4_srv.cc src/bin/dhcp4/tests/dhcp4_srv_unittest.cc src/bin/dhcp4/tests/dhcp4_test_utils.h src/lib/dhcpsrv/cfgmgr.cc src/lib/dhcpsrv/cfgmgr.h --- e3f6de57b908d44370bad1a9cdec70ed2f21bfa8 diff --cc ChangeLog index 9a302d1db6,85dbe072e6..be84ebb2e2 --- a/ChangeLog +++ b/ChangeLog @@@ -1,66 -1,9 +1,73 @@@ -7XX. [func] tomek ++749. [func] tomek + b10-dhcp4, b10-dhcp6: Simple client classification has been + implemented. Incoming packets can be assigned to zero or more + client classes. It is possible to restrict subnet usage to a given + client class. User's Guide and Developer's Guide has been updated. - (Trac #3274, git abcd) ++ (Trac #3274, git 1791d19899b92a6ee411199f664bdfc690ec08b2) ++ +748. [bug] marcin + b10-dhcp4 server picks a subnet, to assign address for a directly + connected client, using IP address of the interface on which the + client's message has been received. If the message is received on + the interface for which there is no suitable subnet, the message + is discarded. Also, the subnet for renewing client which unicasts + its request, is selected using ciaddr. + (Trac #3242, git 9e571cc217d6b1a2fd6fdae1565fcc6fde6d08b1) + +747. [bug] marcin + libdhcpsrv: server configuration mechanism allows creating definitions + for standard options for which Kea doesn't provide a definition yet. + Without this, the server administrator couldn't configure options for + which a definition didn't exist. + (Trac# 3309, git 16a6ed6e48a6a950670c4874a2e81b1faf287d99) + +746. [func] tomek + IOAddress no longer exposes underlying asio objects. The getAddress() + method has been removed and replaced with several convenience methods. + (Trac #1485, git ecdb62db16b3f3d447db4a9d2a4079d5260431f0) + +745. [bug] muks + b10-auth now returns rcode=REFUSED for all questions with + qtype=RRSIG (i.e., where RRSIGs are queried directly). This is + because RRSIGs are meaningless without being bundled alongside the + RRs they cover. + (Trac #2226, git 68d24e65c9c3dfee38adfbe1c93367b0083f9a58) + +744. [func] marcin + b10-dhcp6: Refactored the code which is processing Client FQDN option. + The major user-visible change is that server generates DDNS + NameChangeRequest for the first IPv6 address (instead of all) + acquired by a client. Also, the server generates fully qualified domain + name from acquired IPv6 address, if the client sends an empty name in + Client FQDN option. + (Trac# 3295, git aa1c94a54114e848c64771fde308fc9ac0c00fd0) + +743. [func] tmark + b10-dhcp4 now responds with changes in DDNS behavior based upon + configuration parameters specified through its dhcp-ddns configuration + element. The parameters now supported are override-no-update, + override-client-update, replace-client-name, generated-prefix, and + qualifying-suffix. + (Trac# 3282, git 42b1f1e4c4f5aa48b7588233402876f5012c043c) + +742. [func] muks + The authoritative server now includes the datasource configuration + when logging some errors with the + AUTH_DATASRC_CLIENTS_BUILDER_RECONFIGURE_ERROR message ID. + (Trac #2756, git 31872754f36c840b4ec0b412a86afe9f38be86e0) + +741. [bug] shane + Remove hard-coded (and unnecessary) TSIG key from error message. + This also prevents a crash if the TSIG name is missing. + (Trac #3099, git 0ba8bbabe09756a4627e80aacdbb5050407faaac) + +740. [func] muks + When displaying messages about mismatched configuration data types + in entered values (between the supplied value type and expected + schema type), bindctl now includes both the supplied and expected + configuration data types in the returned error. The user has more + information on what caused the error now. + (Trac #3239, git 84d5eda2a6ae0d737aef68d56023fc33fef623e6) 739. [bug] muks Various minor updates were made to the SSHFP RDATA parser. Mainly, diff --cc src/bin/dhcp4/dhcp4_srv.cc index d3bfe01629,ddc65ee9d6..bcafaefd2d --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@@ -1402,37 -1499,24 +1402,40 @@@ Dhcpv4Srv::serverReceivedPacketName(uin } Subnet4Ptr -Dhcpv4Srv::selectSubnet(const Pkt4Ptr& question) { +Dhcpv4Srv::selectSubnet(const Pkt4Ptr& question) const { Subnet4Ptr subnet; - // Is this relayed message? - IOAddress relay = question->getGiaddr(); static const IOAddress notset("0.0.0.0"); + static const IOAddress bcast("255.255.255.255"); - if (relay != notset) { - // Yes: Use relay address to select subnet - subnet = CfgMgr::instance().getSubnet4(relay, question->classes_); - } else { + // If a message is relayed, use the relay (giaddr) address to select subnet + // for the client. Note that this may result in exception if the value + // of hops does not correspond with the Giaddr. Such message is considered + // to be malformed anyway and the message will be dropped by the higher + // level functions. + if (question->isRelayed()) { - subnet = CfgMgr::instance().getSubnet4(question->getGiaddr()); ++ subnet = CfgMgr::instance().getSubnet4(question->getGiaddr(), ++ question->classes_); - // No: Use client's address to select subnet - subnet = CfgMgr::instance().getSubnet4(question->getRemoteAddr(), + // The message is not relayed so it is sent directly by a client. But + // the client may be renewing its lease and in such case it unicasts + // its message to the server. Note that the unicast Request bypasses + // relays and the client may be in a different network, so we can't + // use IP address on the local interface to get the subnet. Instead, + // we rely on the client's address to get the subnet. + } else if ((question->getLocalAddr() != bcast) && + (question->getCiaddr() != notset)) { - subnet = CfgMgr::instance().getSubnet4(question->getCiaddr()); ++ subnet = CfgMgr::instance().getSubnet4(question->getCiaddr(), + question->classes_); - } - /// @todo Implement getSubnet4(interface-name) + // The message has been received from a directly connected client + // and this client appears to have no address. The IPv4 address + // assigned to the interface on which this message has been received, + // will be used to determine the subnet suitable for the client. + } else { - subnet = CfgMgr::instance().getSubnet4(question->getIface()); ++ subnet = CfgMgr::instance().getSubnet4(question->getIface(), ++ question->classes_); + } // Let's execute all callouts registered for subnet4_select if (HooksManager::calloutsPresent(hook_index_subnet4_select_)) { diff --cc src/bin/dhcp4/tests/dhcp4_srv_unittest.cc index b8f6a24c6b,310b939f54..ba787aa4d6 --- a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc +++ b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc @@@ -3309,98 -3201,68 +3309,162 @@@ TEST_F(Dhcpv4SrvTest, clientClassificat EXPECT_FALSE(dis2->inClass("docsis3.0")); } + // Checks if the client-class field is indeed used for subnet selection. + // Note that packet classification is already checked in Dhcpv4SrvTest + // .clientClassification above. + TEST_F(Dhcpv4SrvTest, clientClassify2) { + + NakedDhcpv4Srv srv(0); + + ConstElementPtr status; + + // This test configures 2 subnets. We actually only need the + // first one, but since there's still this ugly hack that picks + // the pool if there is only one, we must use more than one + // subnet. That ugly hack will be removed in #3242, currently + // under review. + + // The second subnet does not play any role here. The client's + // IP address belongs to the first subnet, so only that first + // subnet it being tested. + string config = "{ \"interfaces\": [ \"*\" ]," + "\"rebind-timer\": 2000, " + "\"renew-timer\": 1000, " + "\"subnet4\": [ " + "{ \"pool\": [ \"192.0.2.1 - 192.0.2.100\" ]," + " \"client-class\": \"foo\", " + " \"subnet\": \"192.0.2.0/24\" }, " + "{ \"pool\": [ \"192.0.3.1 - 192.0.3.100\" ]," + " \"client-class\": \"xyzzy\", " + " \"subnet\": \"192.0.3.0/24\" } " + "]," + "\"valid-lifetime\": 4000 }"; + + ElementPtr json = Element::fromJSON(config); + + EXPECT_NO_THROW(status = configureDhcp4Server(srv, json)); + + // check if returned status is OK + ASSERT_TRUE(status); + comment_ = config::parseAnswer(rcode_, status); + ASSERT_EQ(0, rcode_); + + Pkt4Ptr dis = Pkt4Ptr(new Pkt4(DHCPDISCOVER, 1234)); + dis->setRemoteAddr(IOAddress("192.0.2.1")); ++ dis->setCiaddr(IOAddress("192.0.2.1")); + dis->setIface("eth0"); + OptionPtr clientid = generateClientId(); + dis->addOption(clientid); + + // This discover does not belong to foo class, so it will not + // be serviced + EXPECT_FALSE(srv.selectSubnet(dis)); + + // Let's add the packet to bar class and try again. + dis->addClass("bar"); + + // Still not supported, because it belongs to wrong class. + EXPECT_FALSE(srv.selectSubnet(dis)); + + // Let's add it to maching class. + dis->addClass("foo"); + + // This time it should work + EXPECT_TRUE(srv.selectSubnet(dis)); + } + +// This test verifies that the direct message is dropped when it has been +// received by the server via an interface for which there is no subnet +// configured. It also checks that the message is not dropped (is processed) +// when it is relayed or unicast. +TEST_F(Dhcpv4SrvTest, acceptDirectRequest) { + IfaceMgrTestConfig test_config(true); + IfaceMgr::instance().openSockets4(); + + NakedDhcpv4Srv srv(0); + + Pkt4Ptr pkt(new Pkt4(DHCPDISCOVER, 1234)); + // Set Giaddr and local server's unicast address, but don't set hops. + // Hops value must be greater than 0, when giaddr is set. Otherwise, + // message is considered malformed and the accept() function should + // return false. + pkt->setGiaddr(IOAddress("192.0.10.1")); + pkt->setLocalAddr(IOAddress("192.0.2.3")); + pkt->setIface("eth1"); + EXPECT_FALSE(srv.accept(pkt)); + + // Let's set hops and check that the message is now accepted as + // a relayed message. + pkt->setHops(1); + EXPECT_TRUE(srv.accept(pkt)); + + // Make it a direct message but keep unicast server's address. The + // messages sent to unicast address should be accepted as they are + // most likely to renew existing leases. The server should respond + // to renews so they have to be accepted and processed. + pkt->setHops(0); + pkt->setGiaddr(IOAddress("0.0.0.0")); + EXPECT_TRUE(srv.accept(pkt)); + + // Direct message is now sent to a broadcast address. The server + // should accept this message because it has been received via + // eth1 for which there is a subnet configured (see test fixture + // class constructor). + pkt->setLocalAddr(IOAddress("255.255.255.255")); + EXPECT_TRUE(srv.accept(pkt)); + + // For eth0, there is no subnet configured. Such message is expected + // to be silently dropped. + pkt->setIface("eth0"); + EXPECT_FALSE(srv.accept(pkt)); + + // But, if the message is unicast it should be accepted, even though + // it has been received via eth0. + pkt->setLocalAddr(IOAddress("10.0.0.1")); + EXPECT_TRUE(srv.accept(pkt)); + +} + +// This test checks that the server rejects a message with invalid type. +TEST_F(Dhcpv4SrvTest, acceptMessageType) { + IfaceMgrTestConfig test_config(true); + IfaceMgr::instance().openSockets4(); + + NakedDhcpv4Srv srv(0); + + // Specify messages to be accepted by the server. + int allowed[] = { + DHCPDISCOVER, + DHCPREQUEST, + DHCPRELEASE, + DHCPDECLINE, + DHCPINFORM + }; + size_t allowed_size = sizeof(allowed) / sizeof(allowed[0]); + // Check that the server actually accepts these message types. + for (int i = 0; i < allowed_size; ++i) { + EXPECT_TRUE(srv.acceptMessageType(Pkt4Ptr(new Pkt4(allowed[i], 1234)))) + << "Test failed for message type " << i; + } + // Specify messages which server is supposed to drop. + int not_allowed[] = { + DHCPOFFER, + DHCPACK, + DHCPNAK, + DHCPLEASEQUERY, + DHCPLEASEUNASSIGNED, + DHCPLEASEUNKNOWN, + DHCPLEASEACTIVE, + DHCPBULKLEASEQUERY, + DHCPLEASEQUERYDONE + }; + size_t not_allowed_size = sizeof(not_allowed) / sizeof(not_allowed[0]); + // Actually check that the server will drop these messages. + for (int i = 0; i < not_allowed_size; ++i) { + EXPECT_FALSE(srv.acceptMessageType(Pkt4Ptr(new Pkt4(not_allowed[i], + 1234)))) + << "Test failed for message type " << i; + } +} }; // end of anonymous namespace diff --cc src/bin/dhcp4/tests/dhcp4_test_utils.h index 3f7c326a75,355d8eaee5..5090c7ddd2 --- a/src/bin/dhcp4/tests/dhcp4_test_utils.h +++ b/src/bin/dhcp4/tests/dhcp4_test_utils.h @@@ -87,120 -87,6 +87,121 @@@ public typedef boost::shared_ptr PktFilterTestPtr; +/// @brief "Naked" DHCPv4 server, exposes internal fields +class NakedDhcpv4Srv: public Dhcpv4Srv { +public: + + /// @brief Constructor. + /// + /// This constructor disables default modes of operation used by the + /// Dhcpv4Srv class: + /// - Send/receive broadcast messages through sockets on interfaces + /// which support broadcast traffic. + /// - Direct DHCPv4 traffic - communication with clients which do not + /// have IP address assigned yet. + /// + /// Enabling these modes requires root privilges so they must be + /// disabled for unit testing. + /// + /// Note, that disabling broadcast options on sockets does not impact + /// the operation of these tests because they use local loopback + /// interface which doesn't have broadcast capability anyway. It rather + /// prevents setting broadcast options on other (broadcast capable) + /// sockets which are opened on other interfaces in Dhcpv4Srv constructor. + /// + /// The Direct DHCPv4 Traffic capability can be disabled here because + /// it is tested with PktFilterLPFTest unittest. The tests which belong + /// to PktFilterLPFTest can be enabled on demand when root privileges can + /// be guaranteed. + /// + /// @param port port number to listen on; the default value 0 indicates + /// that sockets should not be opened. + NakedDhcpv4Srv(uint16_t port = 0) + : Dhcpv4Srv(port, "type=memfile", false, false) { + // Create fixed server id. + server_id_.reset(new Option4AddrLst(DHO_DHCP_SERVER_IDENTIFIER, + asiolink::IOAddress("192.0.3.1"))); + } + + /// @brief Returns fixed server identifier assigned to the naked server + /// instance. + OptionPtr getServerID() const { + return (server_id_); + } + + /// @brief fakes packet reception + /// @param timeout ignored + /// + /// The method receives all packets queued in receive queue, one after + /// another. Once the queue is empty, it initiates the shutdown procedure. + /// + /// See fake_received_ field for description + virtual Pkt4Ptr receivePacket(int /*timeout*/) { + + // If there is anything prepared as fake incoming traffic, use it + if (!fake_received_.empty()) { + Pkt4Ptr pkt = fake_received_.front(); + fake_received_.pop_front(); + return (pkt); + } + + // If not, just trigger shutdown and return immediately + shutdown(); + return (Pkt4Ptr()); + } + + /// @brief fake packet sending + /// + /// Pretend to send a packet, but instead just store it in fake_send_ list + /// where test can later inspect server's response. + virtual void sendPacket(const Pkt4Ptr& pkt) { + fake_sent_.push_back(pkt); + } + + /// @brief adds a packet to fake receive queue + /// + /// See fake_received_ field for description + void fakeReceive(const Pkt4Ptr& pkt) { + fake_received_.push_back(pkt); + } + + virtual ~NakedDhcpv4Srv() { + } + + /// @brief Dummy server identifier option used by various tests. + OptionPtr server_id_; + + /// @brief packets we pretend to receive + /// + /// Instead of setting up sockets on interfaces that change between OSes, it + /// is much easier to fake packet reception. This is a list of packets that + /// we pretend to have received. You can schedule new packets to be received + /// using fakeReceive() and NakedDhcpv4Srv::receivePacket() methods. + std::list fake_received_; + + std::list fake_sent_; + + using Dhcpv4Srv::adjustIfaceData; + using Dhcpv4Srv::appendServerID; + using Dhcpv4Srv::processDiscover; + using Dhcpv4Srv::processRequest; + using Dhcpv4Srv::processRelease; + using Dhcpv4Srv::processDecline; + using Dhcpv4Srv::processInform; + using Dhcpv4Srv::processClientName; + using Dhcpv4Srv::computeDhcid; + using Dhcpv4Srv::createNameChangeRequests; + using Dhcpv4Srv::acceptServerId; + using Dhcpv4Srv::sanityCheck; + using Dhcpv4Srv::srvidToString; + using Dhcpv4Srv::unpackOptions; + using Dhcpv4Srv::name_change_reqs_; + using Dhcpv4Srv::classifyPacket; + using Dhcpv4Srv::accept; + using Dhcpv4Srv::acceptMessageType; ++ using Dhcpv4Srv::selectSubnet; +}; + class Dhcpv4SrvTest : public ::testing::Test { public: @@@ -394,30 -335,125 +395,29 @@@ /// @param msg_type DHCPDISCOVER or DHCPREQUEST void testDiscoverRequest(const uint8_t msg_type); - /// @brief Holds a pointer to the packet filter object currently used - /// by the IfaceMgr. - PktFilterTestPtr current_pkt_filter_; - -}; - -/// @brief "Naked" DHCPv4 server, exposes internal fields -class NakedDhcpv4Srv: public Dhcpv4Srv { -public: - - /// @brief Constructor. - /// - /// This constructor disables default modes of operation used by the - /// Dhcpv4Srv class: - /// - Send/receive broadcast messages through sockets on interfaces - /// which support broadcast traffic. - /// - Direct DHCPv4 traffic - communication with clients which do not - /// have IP address assigned yet. - /// - /// Enabling these modes requires root privilges so they must be - /// disabled for unit testing. + /// @brief Runs DHCPv4 configuration from the JSON string. /// - /// Note, that disabling broadcast options on sockets does not impact - /// the operation of these tests because they use local loopback - /// interface which doesn't have broadcast capability anyway. It rather - /// prevents setting broadcast options on other (broadcast capable) - /// sockets which are opened on other interfaces in Dhcpv4Srv constructor. - /// - /// The Direct DHCPv4 Traffic capability can be disabled here because - /// it is tested with PktFilterLPFTest unittest. The tests which belong - /// to PktFilterLPFTest can be enabled on demand when root privileges can - /// be guaranteed. - /// - /// @param port port number to listen on; the default value 0 indicates - /// that sockets should not be opened. - NakedDhcpv4Srv(uint16_t port = 0) - : Dhcpv4Srv(port, "type=memfile", false, false) { - // Create fixed server id. - server_id_.reset(new Option4AddrLst(DHO_DHCP_SERVER_IDENTIFIER, - asiolink::IOAddress("192.0.3.1"))); - } - - /// @brief Returns fixed server identifier assigned to the naked server - /// instance. - OptionPtr getServerID() const { - return (server_id_); - } - - /// @brief fakes packet reception - /// @param timeout ignored - /// - /// The method receives all packets queued in receive queue, one after - /// another. Once the queue is empty, it initiates the shutdown procedure. - /// - /// See fake_received_ field for description - virtual Pkt4Ptr receivePacket(int /*timeout*/) { + /// @param config String holding server configuration in JSON format. + void configure(const std::string& config); - // If there is anything prepared as fake incoming traffic, use it - if (!fake_received_.empty()) { - Pkt4Ptr pkt = fake_received_.front(); - fake_received_.pop_front(); - return (pkt); - } - - // If not, just trigger shutdown and return immediately - shutdown(); - return (Pkt4Ptr()); - } - - /// @brief fake packet sending - /// - /// Pretend to send a packet, but instead just store it in fake_send_ list - /// where test can later inspect server's response. - virtual void sendPacket(const Pkt4Ptr& pkt) { - fake_sent_.push_back(pkt); - } + /// @brief This function cleans up after the test. + virtual void TearDown(); - /// @brief adds a packet to fake receive queue - /// - /// See fake_received_ field for description - void fakeReceive(const Pkt4Ptr& pkt) { - pkt->setIface("eth0"); - fake_received_.push_back(pkt); - } + /// @brief A subnet used in most tests + Subnet4Ptr subnet_; - virtual ~NakedDhcpv4Srv() { - } + /// @brief A pool used in most tests + Pool4Ptr pool_; - /// @brief Dummy server identifier option used by various tests. - OptionPtr server_id_; + /// @brief A client-id used in most tests + ClientIdPtr client_id_; - /// @brief packets we pretend to receive - /// - /// Instead of setting up sockets on interfaces that change between OSes, it - /// is much easier to fake packet reception. This is a list of packets that - /// we pretend to have received. You can schedule new packets to be received - /// using fakeReceive() and NakedDhcpv4Srv::receivePacket() methods. - std::list fake_received_; + int rcode_; - std::list fake_sent_; + isc::data::ConstElementPtr comment_; - /// Let's make those methods public, so they can be accessed easily in tests. - using Dhcpv4Srv::adjustIfaceData; - using Dhcpv4Srv::appendServerID; - using Dhcpv4Srv::processDiscover; - using Dhcpv4Srv::processRequest; - using Dhcpv4Srv::processRelease; - using Dhcpv4Srv::processDecline; - using Dhcpv4Srv::processInform; - using Dhcpv4Srv::processClientName; - using Dhcpv4Srv::computeDhcid; - using Dhcpv4Srv::createNameChangeRequests; - using Dhcpv4Srv::acceptServerId; - using Dhcpv4Srv::sanityCheck; - using Dhcpv4Srv::srvidToString; - using Dhcpv4Srv::unpackOptions; - using Dhcpv4Srv::name_change_reqs_; - using Dhcpv4Srv::classifyPacket; - using Dhcpv4Srv::selectSubnet; + /// @brief Server object under test. + NakedDhcpv4Srv srv_; - }; }; // end of isc::dhcp::test namespace diff --cc src/bin/dhcp4/tests/direct_client_unittest.cc index 9a9dbaee17,0000000000..aacfdc79a1 mode 100644,000000..100644 --- a/src/bin/dhcp4/tests/direct_client_unittest.cc +++ b/src/bin/dhcp4/tests/direct_client_unittest.cc @@@ -1,403 -1,0 +1,413 @@@ +// Copyright (C) 2014 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 +#include ++#include +#include +#include +#include +#include +#include +#include +#include +#include + +using namespace isc; +using namespace isc::asiolink; +using namespace isc::data; +using namespace isc::dhcp; +using namespace isc::dhcp::test; + +namespace { + +/// @brief Test fixture class for testing message processing from directly +/// connected clients. +/// +/// This class provides mechanisms for testing processing of DHCPv4 messages +/// from directly connected clients. +class DirectClientTest : public Dhcpv4SrvTest { +public: + /// @brief Constructor. + /// + /// Initializes DHCPv4 server object used by various tests. + DirectClientTest(); + + /// @brief Configures the server with one subnet. + /// + /// This creates new configuration for the DHCPv4 with one subnet having + /// a specified prefix. + /// + /// The subnet parameters (such as options, timers etc.) are aribitrarily + /// selected. The subnet and pool mask is always /24. The real configuration + /// would exclude .0 (network address) and .255 (broadcast address), but we + /// ignore that fact for the sake of test simplicity. + /// + /// @param prefix Prefix for a subnet. + void configureSubnet(const std::string& prefix); + + /// @brief Configures the server with two subnets. + /// + /// This function configures DHCPv4 server with two different subnets. + /// The subnet parameters (such as options, timers etc.) are aribitrarily + /// selected. The subnet and pool mask is /24. The real configuration + /// would exclude .0 (network address) and .255 (broadcast address), but we + /// ignore that fact for the sake of test simplicity. + /// + /// @param prefix1 Prefix of the first subnet to be configured. + /// @param prefix2 Prefix of the second subnet to be configured. + void configureTwoSubnets(const std::string& prefix1, + const std::string& prefix2); + + /// @brief Creates simple message from a client. + /// + /// This function creates a DHCPv4 message having a specified type + /// (e.g. Discover, Request) and sets some properties of this + /// message: client identifier, address and interface. The copy of + /// this message is then created by parsing wire data of the original + /// message. This simulates the case when the message is received and + /// parsed by the server. + /// + /// @param msg_type Type of the message to be created. + /// @param iface Name of the interface on which the message has been + /// "received" by the server. + /// + /// @return Generated message. + Pkt4Ptr createClientMessage(const uint16_t msg_type, + const std::string& iface); + + /// @brief Creates simple message from a client. + /// + /// This function configures a client's message by adding client identifier, + /// setting interface and addresses. The copy of this message is then + /// created by parsing wire data of the original message. This simulates the + /// case when the message is received and parsed by the server. + /// + /// @param msg Caller supplied message to be configured. This object must + /// not be NULL. + /// @param iface Name of the interface on which the message has been + /// "received" by the server. + /// + /// @return Configured and parsed message. + Pkt4Ptr createClientMessage(const Pkt4Ptr &msg, const std::string& iface); + ++ /// @brief classes the client belongs to ++ /// ++ /// This is empty in most cases, but it is needed as a parameter for all ++ /// getSubnet4() calls. ++ ClientClasses classify_; +}; + +DirectClientTest::DirectClientTest() : Dhcpv4SrvTest() { +} + +void +DirectClientTest::configureSubnet(const std::string& prefix) { + std::ostringstream config; + config << "{ \"interfaces\": [ \"*\" ]," + "\"rebind-timer\": 2000, " + "\"renew-timer\": 1000, " + "\"option-data\": [ ]," + "\"subnet4\": [ { " + " \"pool\": [ \"" << prefix << "/24\" ]," + " \"subnet\": \"" << prefix << "/24\", " + " \"rebind-timer\": 2000, " + " \"renew-timer\": 1000, " + " \"valid-lifetime\": 4000" + "} ]," + "\"valid-lifetime\": 4000 }"; + + configure(config.str()); + +} + +void +DirectClientTest::configureTwoSubnets(const std::string& prefix1, + const std::string& prefix2) { + std::ostringstream config; + config << "{ \"interfaces\": [ \"*\" ]," + "\"rebind-timer\": 2000, " + "\"renew-timer\": 1000, " + "\"option-data\": [ ]," + "\"subnet4\": [ { " + " \"pool\": [ \"" << prefix1 << "/24\" ]," + " \"subnet\": \"" << prefix1 << "/24\", " + " \"rebind-timer\": 2000, " + " \"renew-timer\": 1000, " + " \"valid-lifetime\": 4000" + " }," + "{ " + " \"pool\": [ \"" << prefix2 << "/24\" ]," + " \"subnet\": \"" << prefix2 << "/24\", " + " \"rebind-timer\": 2000, " + " \"renew-timer\": 1000, " + " \"valid-lifetime\": 4000" + "} ]," + "\"valid-lifetime\": 4000 }"; + + configure(config.str()); +} + +Pkt4Ptr +DirectClientTest:: createClientMessage(const uint16_t msg_type, + const std::string& iface) { + // Create a source packet. + Pkt4Ptr msg = Pkt4Ptr(new Pkt4(msg_type, 1234)); + return (createClientMessage(msg, iface)); + +} + +Pkt4Ptr +DirectClientTest::createClientMessage(const Pkt4Ptr& msg, + const std::string& iface) { + msg->setRemoteAddr(IOAddress("255.255.255.255")); + msg->addOption(generateClientId()); + msg->setIface(iface); + + // Create copy of this packet by parsing its wire data. Make sure that the + // local and remote address are set like it was a message sent from the + // directly connected client. + Pkt4Ptr received; + createPacketFromBuffer(msg, received); + received->setIface(iface); + received->setLocalAddr(IOAddress("255.255.255.255")); + received->setRemoteAddr(IOAddress("0.0.0.0")); + + return (received); +} + +// This test checks that the message from directly connected client +// is processed and that client is offered IPv4 address from the subnet which +// is suitable for the local interface on which the client's message is +// received. This test uses two subnets, with two active interfaces which IP +// addresses belong to these subnets. The address offered to the client +// which message has been sent over eth0 should belong to a different +// subnet than the address offered for the client sending its message +// via eth1. +TEST_F(DirectClientTest, twoSubnets) { + // Configure IfaceMgr with fake interfaces lo, eth0 and eth1. + IfaceMgrTestConfig iface_config(true); + // After creating interfaces we have to open sockets as it is required + // by the message processing code. + ASSERT_NO_THROW(IfaceMgr::instance().openSockets4()); + // Add two subnets: address on eth0 belongs to the second subnet, + // address on eth1 belongs to the first subnet. + ASSERT_NO_FATAL_FAILURE(configureTwoSubnets("192.0.2.0", "10.0.0.0")); + // Create Discover and simulate reception of this message through eth0. + Pkt4Ptr dis = createClientMessage(DHCPDISCOVER, "eth0"); + srv_.fakeReceive(dis); + // Create Request and simulate reception of this message through eth1. + Pkt4Ptr req = createClientMessage(DHCPREQUEST, "eth1"); + srv_.fakeReceive(req); + + // Process clients' messages. + srv_.run(); + + // Check that the server did send reposonses. + ASSERT_EQ(2, srv_.fake_sent_.size()); + + // Make sure that we received a response. + Pkt4Ptr response = srv_.fake_sent_.front(); + ASSERT_TRUE(response); + srv_.fake_sent_.pop_front(); + + // Client should get an Offer (not a NAK). + ASSERT_EQ(DHCPOFFER, response->getType()); + // Check that the offered address belongs to the suitable subnet. - Subnet4Ptr subnet = CfgMgr::instance().getSubnet4(response->getYiaddr()); ++ Subnet4Ptr subnet = CfgMgr::instance().getSubnet4(response->getYiaddr(), ++ classify_); + ASSERT_TRUE(subnet); + EXPECT_EQ("10.0.0.0", subnet->get().first.toText()); + + // A client that sent Request over the other interface should get Ack. + response = srv_.fake_sent_.front(); + ASSERT_TRUE(response); + + // Client should get an Ack (not a NAK). + ASSERT_EQ(DHCPACK, response->getType()); + // Check that the offered address belongs to the suitable subnet. - subnet = CfgMgr::instance().getSubnet4(response->getYiaddr()); ++ subnet = CfgMgr::instance().getSubnet4(response->getYiaddr(), classify_); + ASSERT_TRUE(subnet); + EXPECT_EQ("192.0.2.0", subnet->get().first.toText()); + +} + +// This test checks that server selects a subnet when receives a message +// through an interface for which the subnet has been configured. This +// interface has IPv4 address assigned which belongs to this subnet. +// This test also verifies that when the message is received through +// the interface for which there is no suitable subnet, the message +// is discarded. +TEST_F(DirectClientTest, oneSubnet) { + // Configure IfaceMgr with fake interfaces lo, eth0 and eth1. + IfaceMgrTestConfig iface_config(true); + // After creating interfaces we have to open sockets as it is required + // by the message processing code. + ASSERT_NO_THROW(IfaceMgr::instance().openSockets4()); + // Add a subnet which will be selected when a message from directly + // connected client is received through interface eth0. + ASSERT_NO_FATAL_FAILURE(configureSubnet("10.0.0.0")); + // Create Discover and simulate reception of this message through eth0. + Pkt4Ptr dis = createClientMessage(DHCPDISCOVER, "eth0"); + srv_.fakeReceive(dis); + // Create Request and simulate reception of this message through eth1. + Pkt4Ptr req = createClientMessage(DHCPDISCOVER, "eth1"); + srv_.fakeReceive(req); + + // Process clients' messages. + srv_.run(); + + // Check that the server sent one response for the message received + // through eth0. The other client's message should be dicarded. + ASSERT_EQ(1, srv_.fake_sent_.size()); + + // Check the response. The first Discover was sent via eth0 for which + // the subnet has been configured. + Pkt4Ptr response = srv_.fake_sent_.front(); + ASSERT_TRUE(response); + srv_.fake_sent_.pop_front(); + + // Since Discover has been received through the interface for which + // the subnet has been configured, the server should respond with + // an Offer message. + ASSERT_EQ(DHCPOFFER, response->getType()); + // Check that the offered address belongs to the suitable subnet. - Subnet4Ptr subnet = CfgMgr::instance().getSubnet4(response->getYiaddr()); ++ Subnet4Ptr subnet = CfgMgr::instance().getSubnet4(response->getYiaddr(), ++ classify_); + ASSERT_TRUE(subnet); + EXPECT_EQ("10.0.0.0", subnet->get().first.toText()); + +} + +// This test verifies that the server uses ciaddr to select a subnet for a +// client which renews its lease. +TEST_F(DirectClientTest, renew) { + // Configure IfaceMgr with fake interfaces lo, eth0 and eth1. + IfaceMgrTestConfig iface_config(true); + // After creating interfaces we have to open sockets as it is required + // by the message processing code. + ASSERT_NO_THROW(IfaceMgr::instance().openSockets4()); + // Add a subnet. + ASSERT_NO_FATAL_FAILURE(configureSubnet("10.0.0.0")); + // Make sure that the subnet has been really added. Also, the subnet + // will be needed to create a lease for a client. - Subnet4Ptr subnet = CfgMgr::instance().getSubnet4(IOAddress("10.0.0.10")); ++ Subnet4Ptr subnet = CfgMgr::instance().getSubnet4(IOAddress("10.0.0.10"), ++ classify_); + // Create a lease for a client that we will later renewed. By explicitly + // creating a lease we will get to know the lease parameters, such as + // leased address etc. + const uint8_t hwaddr[] = { 1, 2, 3, 4, 5, 6 }; + Lease4Ptr lease(new Lease4(IOAddress("10.0.0.10"), hwaddr, sizeof(hwaddr), + &generateClientId()->getData()[0], + generateClientId()->getData().size(), + 100, 50, 75, time(NULL), + subnet->getID())); + LeaseMgrFactory::instance().addLease(lease); + + // Create a Request to renew client's lease. The renew request is unicast + // through eth1. Note, that in case of renewal the client unicasts its + // Request and sets the ciaddr. The server is supposed to use ciaddr to + // pick the subnet for the client. In order to make sure that the server + // uses ciaddr, we simulate reception of the packet through eth1, for which + // there is no subnet for directly connected clients. + Pkt4Ptr req = Pkt4Ptr(new Pkt4(DHCPREQUEST, 1234)); + req->setCiaddr(IOAddress("10.0.0.10")); + req = createClientMessage(req, "eth1"); + req->setLocalAddr(IOAddress("10.0.0.1")); + req->setRemoteAddr(req->getCiaddr()); + + srv_.fakeReceive(req); + + // Process clients' messages. + srv_.run(); + + // Check that the server did send reposonse. + ASSERT_EQ(1, srv_.fake_sent_.size()); + Pkt4Ptr response = srv_.fake_sent_.front(); + ASSERT_TRUE(response); + + ASSERT_EQ(DHCPACK, response->getType()); + // Check that the offered address belongs to the suitable subnet. - subnet = CfgMgr::instance().getSubnet4(response->getYiaddr()); ++ subnet = CfgMgr::instance().getSubnet4(response->getYiaddr(), classify_); + ASSERT_TRUE(subnet); + EXPECT_EQ("10.0.0.0", subnet->get().first.toText()); + +} + +// This test verifies that when a client in the Rebinding state broadcasts +// a Request message through an interface for which a subnet is configured, +// the server responds to this Request. It also verifies that when such a +// Request is sent through the interface for which there is no subnet configured +// the client's message is discarded. +TEST_F(DirectClientTest, rebind) { + // Configure IfaceMgr with fake interfaces lo, eth0 and eth1. + IfaceMgrTestConfig iface_config(true); + // After creating interfaces we have to open sockets as it is required + // by the message processing code. + ASSERT_NO_THROW(IfaceMgr::instance().openSockets4()); + // Add a subnet. + ASSERT_NO_FATAL_FAILURE(configureSubnet("10.0.0.0")); + // Make sure that the subnet has been really added. Also, the subnet + // will be needed to create a lease for a client. - Subnet4Ptr subnet = CfgMgr::instance().getSubnet4(IOAddress("10.0.0.10")); ++ Subnet4Ptr subnet = CfgMgr::instance().getSubnet4(IOAddress("10.0.0.10"), ++ classify_); + // Create a lease, which will be later renewed. By explicitly creating a + // lease we will know the lease parameters, such as leased address etc. + const uint8_t hwaddr[] = { 1, 2, 3, 4, 5, 6 }; + Lease4Ptr lease(new Lease4(IOAddress("10.0.0.10"), hwaddr, sizeof(hwaddr), + &generateClientId()->getData()[0], + generateClientId()->getData().size(), + 100, 50, 75, time(NULL), + subnet->getID())); + LeaseMgrFactory::instance().addLease(lease); + + // Broadcast Request through an interface for which there is no subnet + // configured. This messag should be discarded by the server. + Pkt4Ptr req = Pkt4Ptr(new Pkt4(DHCPREQUEST, 1234)); + req->setCiaddr(IOAddress("10.0.0.10")); + req = createClientMessage(req, "eth1"); + req->setRemoteAddr(req->getCiaddr()); + + srv_.fakeReceive(req); + + // Broadcast another Request through an interface for which there is + // a subnet configured. The server should generate a response. + req = Pkt4Ptr(new Pkt4(DHCPREQUEST, 5678)); + req->setCiaddr(IOAddress("10.0.0.10")); + req = createClientMessage(req, "eth0"); + req->setRemoteAddr(req->getCiaddr()); + + srv_.fakeReceive(req); + + // Process clients' messages. + srv_.run(); + + // Check that the server did send exactly one reposonse. + ASSERT_EQ(1, srv_.fake_sent_.size()); + Pkt4Ptr response = srv_.fake_sent_.front(); + ASSERT_TRUE(response); + + // Make sure that the server responsed with ACK, not NAK. + ASSERT_EQ(DHCPACK, response->getType()); + // Make sure that the response is generated for the second Request + // (transmitted over eth0). + EXPECT_EQ(5678, response->getTransid()); + // Check that the offered address belongs to the suitable subnet. - subnet = CfgMgr::instance().getSubnet4(response->getYiaddr()); ++ subnet = CfgMgr::instance().getSubnet4(response->getYiaddr(), classify_); + ASSERT_TRUE(subnet); + EXPECT_EQ("10.0.0.0", subnet->get().first.toText()); + +} + +} diff --cc src/lib/dhcp/tests/Makefile.am index d41246a51f,69f1d50bec..7e08bea9e8 --- a/src/lib/dhcp/tests/Makefile.am +++ b/src/lib/dhcp/tests/Makefile.am @@@ -44,9 -27,9 +44,10 @@@ libdhcptest_la_LIBADD = $(top_buildd TESTS += libdhcp++_unittests libdhcp___unittests_SOURCES = run_unittests.cc + libdhcp___unittests_SOURCES += classify_unittest.cc libdhcp___unittests_SOURCES += hwaddr_unittest.cc libdhcp___unittests_SOURCES += iface_mgr_unittest.cc +libdhcp___unittests_SOURCES += iface_mgr_test_config.cc iface_mgr_test_config.h libdhcp___unittests_SOURCES += libdhcp++_unittest.cc libdhcp___unittests_SOURCES += option4_addrlst_unittest.cc libdhcp___unittests_SOURCES += option4_client_fqdn_unittest.cc diff --cc src/lib/dhcpsrv/cfgmgr.cc index 207870e21b,2494ff946c..960cfd2261 --- a/src/lib/dhcpsrv/cfgmgr.cc +++ b/src/lib/dhcpsrv/cfgmgr.cc @@@ -211,11 -227,34 +231,19 @@@ void CfgMgr::addSubnet6(const Subnet6Pt } Subnet4Ptr - CfgMgr::getSubnet4(const isc::asiolink::IOAddress& hint) const { + CfgMgr::getSubnet4(const isc::asiolink::IOAddress& hint, - const isc::dhcp::ClientClasses& classes) { - - // If there's only one subnet configured, let's just use it - // The idea is to keep small deployments easy. In a small network - one - // router that also runs DHCPv6 server. Users specifies a single pool and - // expects it to just work. Without this, the server would complain that it - // doesn't have IP address on its interfaces that matches that - // configuration. Such requirement makes sense in IPv4, but not in IPv6. - // The server does not need to have a global address (using just link-local - // is ok for DHCPv6 server) from the pool it serves. - if (subnets4_.size() == 1) { - LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE, - DHCPSRV_CFGMGR_ONLY_SUBNET4) - .arg(subnets4_[0]->toText()).arg(hint.toText()); - return (subnets4_[0]); - } - - // If there is more than one, we need to choose the proper one - for (Subnet4Collection::iterator subnet = subnets4_.begin(); ++ const isc::dhcp::ClientClasses& classes) const { + // Iterate over existing subnets to find a suitable one for the + // given address. + for (Subnet4Collection::const_iterator subnet = subnets4_.begin(); subnet != subnets4_.end(); ++subnet) { + + // If client is rejected because of not meeting client class criteria... + if (!(*subnet)->clientSupported(classes)) { + continue; + } + + // Let's check if the client belongs to the given subnet if ((*subnet)->inRange(hint)) { LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE, DHCPSRV_CFGMGR_SUBNET4) @@@ -230,22 -269,6 +258,23 @@@ return (Subnet4Ptr()); } +Subnet4Ptr - CfgMgr::getSubnet4(const std::string& iface_name) const { ++CfgMgr::getSubnet4(const std::string& iface_name, ++ const isc::dhcp::ClientClasses& classes) const { + Iface* iface = IfaceMgr::instance().getIface(iface_name); + // This should never happen in the real life. Hence we throw an exception. + if (iface == NULL) { + isc_throw(isc::BadValue, "interface " << iface_name << + " doesn't exist and therefore it is impossible" + " to find a suitable subnet for its IPv4 address"); + } + IOAddress addr("0.0.0.0"); + // If IPv4 address assigned to the interface exists, find a suitable + // subnet for it, else return NULL pointer to indicate that no subnet + // could be found. - return (iface->getAddress4(addr) ? getSubnet4(addr) : Subnet4Ptr()); ++ return (iface->getAddress4(addr) ? getSubnet4(addr, classes) : Subnet4Ptr()); +} + void CfgMgr::addSubnet4(const Subnet4Ptr& subnet) { /// @todo: Check that this new subnet does not cross boundaries of any /// other already defined subnet. diff --cc src/lib/dhcpsrv/cfgmgr.h index 63675de6b4,6b65f929ed..20c162dbb9 --- a/src/lib/dhcpsrv/cfgmgr.h +++ b/src/lib/dhcpsrv/cfgmgr.h @@@ -241,22 -259,15 +259,31 @@@ public /// b) our global address on the interface the message was received on /// (for directly connected clients) /// + /// If there are any classes specified in a subnet, that subnet + /// will be selected only if the client belongs to appropriate class. + /// /// @param hint an address that belongs to a searched subnet + /// @param classes classes the client belongs to /// /// @return a subnet object - Subnet4Ptr getSubnet4(const isc::asiolink::IOAddress& hint) const; + Subnet4Ptr getSubnet4(const isc::asiolink::IOAddress& hint, - const isc::dhcp::ClientClasses& classes); ++ const isc::dhcp::ClientClasses& classes) const; + + /// @brief Returns a subnet for the specified local interface. + /// + /// This function checks that the IP address assigned to the specified + /// interface belongs to any IPv4 subnet configured, and returns this + /// subnet. + /// ++ /// @todo Implement classes support here. ++ /// + /// @param iface Short name of the interface which is being checked. ++ /// @param classes classes the client belongs to + /// + /// @return Pointer to the subnet matching interface specified or NULL + /// pointer if IPv4 address on the interface doesn't match any subnet. - Subnet4Ptr getSubnet4(const std::string& iface) const; ++ Subnet4Ptr getSubnet4(const std::string& iface, ++ const isc::dhcp::ClientClasses& classes) const; /// @brief adds a subnet4 void addSubnet4(const Subnet4Ptr& subnet); diff --cc src/lib/dhcpsrv/tests/cfgmgr_unittest.cc index 5962b31c5a,31db594bdc..e0639dc2b9 --- a/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc @@@ -741,47 -907,6 +939,47 @@@ TEST_F(CfgMgrTest, d2ClientConfig) EXPECT_NE(*original_config, *updated_config); } +// This test verfies that CfgMgr correctly determines that the address of the +// interface belongs to existing IPv4 subnet. +TEST_F(CfgMgrTest, getSubnet4ForInterface) { + IfaceMgrTestConfig config(true); + + // Initially, there are no subnets configured, so none of the IPv4 + // addresses assigned to eth0 and eth1 can match with any subnet. - EXPECT_FALSE(CfgMgr::instance().getSubnet4("eth0")); - EXPECT_FALSE(CfgMgr::instance().getSubnet4("eth1")); ++ EXPECT_FALSE(CfgMgr::instance().getSubnet4("eth0", classify_)); ++ EXPECT_FALSE(CfgMgr::instance().getSubnet4("eth1", classify_)); + + // Configure first subnet which address on eth0 corresponds to. + Subnet4Ptr subnet1(new Subnet4(IOAddress("10.0.0.1"), 24, 1, 2, 3)); + CfgMgr::instance().addSubnet4(subnet1); + + // The address on eth0 should match the existing subnet. + Subnet4Ptr subnet1_ret; - subnet1_ret = CfgMgr::instance().getSubnet4("eth0"); ++ subnet1_ret = CfgMgr::instance().getSubnet4("eth0", classify_); + ASSERT_TRUE(subnet1_ret); + EXPECT_EQ(subnet1->get().first, subnet1_ret->get().first); + // There should still be no match for eth1. - EXPECT_FALSE(CfgMgr::instance().getSubnet4("eth1")); ++ EXPECT_FALSE(CfgMgr::instance().getSubnet4("eth1", classify_)); + + // Configure a second subnet. + Subnet4Ptr subnet2(new Subnet4(IOAddress("192.0.2.1"), 24, 1, 2, 3)); + CfgMgr::instance().addSubnet4(subnet2); + + // There should be match between eth0 and subnet1 and between eth1 and + // subnet 2. - subnet1_ret = CfgMgr::instance().getSubnet4("eth0"); ++ subnet1_ret = CfgMgr::instance().getSubnet4("eth0", classify_); + ASSERT_TRUE(subnet1_ret); + EXPECT_EQ(subnet1->get().first, subnet1_ret->get().first); - Subnet4Ptr subnet2_ret = CfgMgr::instance().getSubnet4("eth1"); ++ Subnet4Ptr subnet2_ret = CfgMgr::instance().getSubnet4("eth1", classify_); + ASSERT_TRUE(subnet2_ret); + EXPECT_EQ(subnet2->get().first, subnet2_ret->get().first); + + // This function throws an exception if the name of the interface is wrong. - EXPECT_THROW(CfgMgr::instance().getSubnet4("bogus-interface"), ++ EXPECT_THROW(CfgMgr::instance().getSubnet4("bogus-interface", classify_), + isc::BadValue); + +} + /// @todo Add unit-tests for testing: /// - addActiveIface() with invalid interface name