From ac0de4134f1bae246138f254c065cf70a704adb4 Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Fri, 20 Nov 2015 14:41:18 +0100 Subject: [PATCH] [4097a] Rebased and updated trac4097 code (still some comments to address) --- src/bin/dhcp4/Makefile.am | 1 + src/bin/dhcp4/dhcp4_srv.cc | 80 +++++++++++++++++++++-- src/bin/dhcp4/dhcp4_srv.h | 9 +-- src/bin/dhcp4/tests/Makefile.am | 1 + src/bin/dhcp4/tests/dhcp4_srv_unittest.cc | 77 ++++++++++++++++++++-- src/bin/dhcp4/tests/dhcp4_test_utils.h | 1 + 6 files changed, 156 insertions(+), 13 deletions(-) diff --git a/src/bin/dhcp4/Makefile.am b/src/bin/dhcp4/Makefile.am index 0512bb3501..9f225ae69c 100644 --- a/src/bin/dhcp4/Makefile.am +++ b/src/bin/dhcp4/Makefile.am @@ -75,6 +75,7 @@ kea_dhcp4_SOURCES = main.cc kea_dhcp4_LDADD = libdhcp4.la kea_dhcp4_LDADD += $(top_builddir)/src/lib/cfgrpt/libcfgrpt.la kea_dhcp4_LDADD += $(top_builddir)/src/lib/dhcpsrv/libkea-dhcpsrv.la +kea_dhcp4_LDADD += $(top_builddir)/src/lib/eval/libkea-eval.la kea_dhcp4_LDADD += $(top_builddir)/src/lib/dhcp_ddns/libkea-dhcp_ddns.la kea_dhcp4_LDADD += $(top_builddir)/src/lib/stats/libkea-stats.la kea_dhcp4_LDADD += $(top_builddir)/src/lib/config/libkea-cfgclient.la diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index e3e1e85ebb..d7c247aecd 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -38,6 +38,8 @@ #include #include #include +#include +#include #include #include #include @@ -2242,13 +2244,14 @@ Dhcpv4Srv::unpackOptions(const OptionBuffer& buf, } void Dhcpv4Srv::classifyPacket(const Pkt4Ptr& pkt) { + string classes = ""; + + // Built-in vendor class processing boost::shared_ptr vendor_class = boost::dynamic_pointer_cast(pkt->getOption(DHO_VENDOR_CLASS_IDENTIFIER)); - string classes = ""; - if (!vendor_class) { - return; + goto vendor_class_done; } // DOCSIS specific section @@ -2275,8 +2278,49 @@ void Dhcpv4Srv::classifyPacket(const Pkt4Ptr& pkt) { pkt->addClass(VENDOR_CLASS_PREFIX + DOCSIS3_CLASS_EROUTER); classes += string(VENDOR_CLASS_PREFIX + DOCSIS3_CLASS_EROUTER) + " "; } else { - classes += VENDOR_CLASS_PREFIX + vendor_class->getValue(); pkt->addClass(VENDOR_CLASS_PREFIX + vendor_class->getValue()); + classes += VENDOR_CLASS_PREFIX + vendor_class->getValue(); + } + +vendor_class_done: + + // Run match expressions + // Note getClientClassDictionary() cannot be null + const ClientClassDefMapPtr& defs_ptr = CfgMgr::instance().getCurrentCfg()-> + getClientClassDictionary()->getClasses(); + for (ClientClassDefMap::const_iterator it = defs_ptr->begin(); + it != defs_ptr->end(); ++it) { + // Note second cannot be null + const ExpressionPtr& expr_ptr = it->second->getMatchExpr(); + // Nothing to do without an expression to evaluate + if (!expr_ptr) { + continue; + } + // Evaluate the expression which can return false (no match), + // true (match) or raise an exception (error) + try { + bool status = evaluate(*expr_ptr, *pkt); + if (status) { + LOG_INFO(options4_logger, EVAL_RESULT) + .arg(it->first) + .arg(status); + // Matching: add the class + pkt->addClass(it->first); + classes += it->first + " "; + } else { + LOG_DEBUG(options4_logger, DBG_DHCP4_DETAIL, EVAL_RESULT) + .arg(it->first) + .arg(status); + } + } catch (const Exception& ex) { + LOG_ERROR(options4_logger, EVAL_RESULT) + .arg(it->first) + .arg(ex.what()); + } catch (...) { + LOG_ERROR(options4_logger, EVAL_RESULT) + .arg(it->first) + .arg("get exception?"); + } } if (!classes.empty()) { @@ -2298,6 +2342,7 @@ Dhcpv4Srv::classSpecificProcessing(const Dhcpv4Exchange& ex) { return (true); } + // DOCSIS3 class modem specific processing if (query->inClass(VENDOR_CLASS_PREFIX + DOCSIS3_CLASS_MODEM)) { // Set next-server. This is TFTP server address. Cable modems will @@ -2319,12 +2364,39 @@ Dhcpv4Srv::classSpecificProcessing(const Dhcpv4Exchange& ex) { } } + // DOCSIS3 class erouter specific processing if (query->inClass(VENDOR_CLASS_PREFIX + DOCSIS3_CLASS_EROUTER)) { // Do not set TFTP server address for eRouter devices. rsp->setSiaddr(IOAddress::IPV4_ZERO_ADDRESS()); } + // Process each class + const ClientClassDefMapPtr& defs_ptr = CfgMgr::instance().getCurrentCfg()-> + getClientClassDictionary()->getClasses(); + for (ClientClassDefMap::const_iterator it = defs_ptr->begin(); + it != defs_ptr->end(); ++it) { + // Is the query in this class? + if (!it->second || !query->inClass(it->first)) { + continue; + } + // Get the configured options of this class + const OptionContainerPtr& options = + it->second->getCfgOption()->getAll("dhcp4"); + if (!options || options->empty()) { + continue; + } + // Go through each OptionDescriptor + for (OptionContainer::const_iterator desc = options->begin(); + desc != options->end(); ++desc) { + OptionPtr opt = desc->option_; + // Add the option if it doesn't exist yet + if (!rsp->getOption(opt->getType())) { + rsp->addOption(opt); + } + } + } + return (true); } diff --git a/src/bin/dhcp4/dhcp4_srv.h b/src/bin/dhcp4/dhcp4_srv.h index b8b1077402..e86fde12aa 100644 --- a/src/bin/dhcp4/dhcp4_srv.h +++ b/src/bin/dhcp4/dhcp4_srv.h @@ -706,10 +706,11 @@ protected: /// @brief Assigns incoming packet to zero or more classes. /// - /// @note For now, the client classification is very simple. It just uses - /// content of the vendor-class-identifier option as a class. The resulting - /// class will be stored in packet (see @ref isc::dhcp::Pkt4::classes_ and - /// @ref isc::dhcp::Pkt4::inClass). + /// @note It is done in two phases: first the content of the + /// vendor-class-identifier option is used as a class. Second + /// classification match expressions are evaluated. The resulting + /// class will be stored in packet (see @ref isc::dhcp::Pkt4::classes_ + /// and @ref isc::dhcp::Pkt4::inClass). /// /// @param pkt packet to be classified void classifyPacket(const Pkt4Ptr& pkt); diff --git a/src/bin/dhcp4/tests/Makefile.am b/src/bin/dhcp4/tests/Makefile.am index 9967247f69..df7ac93a2b 100644 --- a/src/bin/dhcp4/tests/Makefile.am +++ b/src/bin/dhcp4/tests/Makefile.am @@ -109,6 +109,7 @@ dhcp4_unittests_LDADD = $(top_builddir)/src/bin/dhcp4/libdhcp4.la dhcp4_unittests_LDADD += $(top_builddir)/src/lib/cfgrpt/libcfgrpt.la dhcp4_unittests_LDADD += $(top_builddir)/src/lib/dhcpsrv/libkea-dhcpsrv.la dhcp4_unittests_LDADD += $(top_builddir)/src/lib/dhcpsrv/testutils/libdhcpsrvtest.la +dhcp4_unittests_LDADD += $(top_builddir)/src/lib/eval/libkea-eval.la dhcp4_unittests_LDADD += $(top_builddir)/src/lib/dhcp_ddns/libkea-dhcp_ddns.la dhcp4_unittests_LDADD += $(top_builddir)/src/lib/testutils/libkea-testutils.la dhcp4_unittests_LDADD += $(top_builddir)/src/lib/stats/libkea-stats.la diff --git a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc index 184075e0ef..0ab4e017ca 100644 --- a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc +++ b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc @@ -410,7 +410,7 @@ TEST_F(Dhcpv4SrvTest, initResponse) { OptionPtr resp_sbnsel = response->getOption(DHO_SUBNET_SELECTION); ASSERT_TRUE(resp_sbnsel); OptionCustomPtr resp_custom = - boost::dynamic_pointer_cast(resp_sbnsel); + boost::dynamic_pointer_cast(resp_sbnsel); ASSERT_TRUE(resp_custom); IOAddress subnet_addr("0.0.0.0"); ASSERT_NO_THROW(subnet_addr = resp_custom->readAddress()); @@ -1646,8 +1646,8 @@ TEST_F(Dhcpv4SrvTest, vendorOptionsDocsisDefinitions) { ASSERT_EQ(0, rcode_); } -// Checks if client packets are classified properly -TEST_F(Dhcpv4SrvTest, clientClassification) { +// Checks if DOCSIS client packets are classified properly +TEST_F(Dhcpv4SrvTest, docsisClientClassification) { NakedDhcpv4Srv srv(0); @@ -1674,10 +1674,77 @@ TEST_F(Dhcpv4SrvTest, clientClassification) { EXPECT_FALSE(dis2->inClass(srv.VENDOR_CLASS_PREFIX + "docsis3.0")); } +// Checks if client packets are classified properly using match expressions. +TEST_F(Dhcpv4SrvTest, matchClassification) { + NakedDhcpv4Srv srv(0); + + // The router class matches incoming packets with foo in a host-name + // option (code 12) and sets an ip-forwarding option in the response + string config = "{ \"interfaces-config\": {" + " \"interfaces\": [ \"*\" ] }, " + "\"rebind-timer\": 2000, " + "\"renew-timer\": 1000, " + "\"valid-lifetime\": 4000, " + "\"subnet4\": [ " + "{ \"pools\": [ { \"pool\": \"192.0.2.1 - 192.0.2.100\" } ]," + " \"subnet\": \"192.0.2.0/24\" } ], " + "\"client-classes\": [ " + "{ \"name\": \"router\"," + " \"option-data\": [" + " { \"name\": \"ip-forwarding\"," + " \"data\": \"true\" } ]," + " \"test\": \"option[12] == 'foo'\" } ] }"; + + ElementPtr json = Element::fromJSON(config); + ConstElementPtr status; + + // Configure the server and make sure the config is accepted + EXPECT_NO_THROW(status = configureDhcp4Server(srv, json)); + ASSERT_TRUE(status); + comment_ = config::parseAnswer(rcode_, status); + ASSERT_EQ(0, rcode_); + + CfgMgr::instance().commit(); + + // Create packets with enough to select the subnet + Pkt4Ptr query1(new Pkt4(DHCPDISCOVER, 1234)); + query1->setRemoteAddr(IOAddress("192.0.2.1")); + Pkt4Ptr query2(new Pkt4(DHCPDISCOVER, 1234)); + query2->setRemoteAddr(IOAddress("192.0.2.1")); + + // Create and add a host-name option to the first query + OptionStringPtr hostname(new OptionString(Option::V4, 12, "foo")); + ASSERT_TRUE(hostname); + query1->addOption(hostname); + + // Classify packets + srv.classifyPacket(query1); + srv.classifyPacket(query2); + + // The first packet (and only the first) should be in the router class + EXPECT_TRUE(query1->inClass("router")); + EXPECT_FALSE(query2->inClass("router")); + + Dhcpv4Exchange ex1 = createExchange(query1); + Pkt4Ptr response1 = ex1.getResponse(); + Dhcpv4Exchange ex2 = createExchange(query2); + Pkt4Ptr response2 = ex2.getResponse(); + + // Classification processing should add an ip-forwarding option + srv.classSpecificProcessing(ex1); + OptionPtr opt1 = response1->getOption(DHO_IP_FORWARDING); + EXPECT_TRUE(opt1); + + // But only for the first exchange + srv.classSpecificProcessing(ex2); + OptionPtr opt2 = response2->getOption(DHO_IP_FORWARDING); + EXPECT_FALSE(opt2); +} + // 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) { +// .*clientClassification above. +TEST_F(Dhcpv4SrvTest, clientClassify) { // This test configures 2 subnets. We actually only need the // first one, but since there's still this ugly hack that picks diff --git a/src/bin/dhcp4/tests/dhcp4_test_utils.h b/src/bin/dhcp4/tests/dhcp4_test_utils.h index 91e2ab59f1..908621a660 100644 --- a/src/bin/dhcp4/tests/dhcp4_test_utils.h +++ b/src/bin/dhcp4/tests/dhcp4_test_utils.h @@ -204,6 +204,7 @@ public: using Dhcpv4Srv::srvidToString; using Dhcpv4Srv::unpackOptions; using Dhcpv4Srv::classifyPacket; + using Dhcpv4Srv::classSpecificProcessing; using Dhcpv4Srv::accept; using Dhcpv4Srv::acceptMessageType; using Dhcpv4Srv::selectSubnet; -- 2.47.2