[#4048] Check subnet before assignZero() call

/src/bin/dhcp4/dhcp4_srv.cc
    Dhcpv4Srv::assignLease() - avoid call to assignZero() when
    there is no selected subnet

/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
    TEST_F(Dhcpv4SrvTest, v6OnlyPreferredNoSelectedSubnet) - new test

/src/bin/dhcp4/tests/dhcp4_test_utils.h
    Allow log content checking

diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc
index 774cb9044eaa78cc4dc3050c13ddc74d57172e00..9224ccdfd473be3a74313ba8cb2563f155ed1036 100644 (file)
--- a/src/bin/dhcp4/dhcp4_srv.cc
+++ b/src/bin/dhcp4/dhcp4_srv.cc
@@ -774,7 +774,6 @@ Dhcpv4Srv::shutdown() {
 
 isc::dhcp::ConstSubnet4Ptr
 Dhcpv4Srv::selectSubnet(const Pkt4Ptr& query, bool& drop, bool allow_answer_park) {
-
     // DHCPv4-over-DHCPv6 is a special (and complex) case
     if (query->isDhcp4o6()) {
         return (selectSubnet4o6(query, drop, allow_answer_park));
@@ -2966,21 +2965,23 @@ Dhcpv4Srv::assignLease(Dhcpv4Exchange& ex) {
     // allocation.
     bool fake_allocation = (query->getType() == DHCPDISCOVER);
 
-    // Check if IPv6-Only Preferred was requested.
-    OptionUint8ArrayPtr option_prl = boost::dynamic_pointer_cast<
-        OptionUint8Array>(query->getOption(DHO_DHCP_PARAMETER_REQUEST_LIST));
-    if (option_prl) {
-        auto const& requested_opts = option_prl->getValues();
-        if ((std::find(requested_opts.cbegin(), requested_opts.cend(),
-                       DHO_V6_ONLY_PREFERRED) != requested_opts.cend()) &&
-            assignZero(subnet, query->getClasses())) {
-            ex.setIPv6OnlyPreferred(true);
-            ctx->subnet_ = subnet;
-            resp->setYiaddr(IOAddress::IPV4_ZERO_ADDRESS());
-            if (!fake_allocation) {
-                resp->setCiaddr(query->getCiaddr());
+    if (subnet) {
+        // Check if IPv6-Only Preferred was requested.
+        OptionUint8ArrayPtr option_prl = boost::dynamic_pointer_cast<
+            OptionUint8Array>(query->getOption(DHO_DHCP_PARAMETER_REQUEST_LIST));
+        if (option_prl) {
+            auto const& requested_opts = option_prl->getValues();
+            if ((std::find(requested_opts.cbegin(), requested_opts.cend(),
+                           DHO_V6_ONLY_PREFERRED) != requested_opts.cend()) &&
+                assignZero(subnet, query->getClasses())) {
+                ex.setIPv6OnlyPreferred(true);
+                ctx->subnet_ = subnet;
+                resp->setYiaddr(IOAddress::IPV4_ZERO_ADDRESS());
+                if (!fake_allocation) {
+                    resp->setCiaddr(query->getCiaddr());
+                }
+                return;
             }
-            return;
         }
     }
 

diff --git a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
index 5c8caf7b646e34d52f9c2fc62d0463543d682eab..ff9c900de47c906f021b365558b0be1ae287d323 100644 (file)
--- a/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
+++ b/src/bin/dhcp4/tests/dhcp4_srv_unittest.cc
@@ -6267,6 +6267,38 @@ TEST_F(Dhcpv4SrvTest, v6OnlyPreferredRequestError) {
     EXPECT_FALSE(srv_->processRequest(req));
 }
 
+// Verify that a discover requesting v6-only-preferred
+// is handled gracefully when there is no matching subnet.
+TEST_F(Dhcpv4SrvTest, v6OnlyPreferredNoSelectedSubnet) {
+    IfaceMgrTestConfig test_config(true);
+    IfaceMgr::instance().openSockets4();
+
+    Pkt4Ptr dis = Pkt4Ptr(new Pkt4(DHCPDISCOVER, 1234));
+    OptionPtr clientid = generateClientId();
+    dis->addOption(clientid);
+    // Sort source address to non-matching subnet.
+    dis->setRemoteAddr(IOAddress("192.0.99.1"));
+    dis->setIface("eth1");
+    dis->setIndex(ETH1_INDEX);
+
+    // Add a PRL with v6-only-preferred.
+    OptionUint8ArrayPtr prl(new OptionUint8Array(Option::V4,
+                                                 DHO_DHCP_PARAMETER_REQUEST_LIST));
+    ASSERT_TRUE(prl);
+    prl->addValue(DHO_V6_ONLY_PREFERRED);
+    dis->addOption(prl);
+
+    // Should not have gotten a response.
+    Pkt4Ptr resp = srv_->processDiscover(dis);
+    ASSERT_FALSE(resp);
+
+    // Should have logged a NAK_0001.
+    EXPECT_EQ(1, countFile("DHCP4_PACKET_NAK_0001 [hwtype=1 ], cid=[64:65:66:67],"
+                           " tid=0x4d2: failed to select a subnet for incoming packet,"
+                           " src 192.0.99.1, type DHCPDISCOVER"));
+
+}
+
 /// @brief Test fixture for recoverStashedAgentOption.
 class StashAgentOptionTest : public Dhcpv4SrvTest {
 public:

diff --git a/src/bin/dhcp4/tests/dhcp4_test_utils.h b/src/bin/dhcp4/tests/dhcp4_test_utils.h
index c4867adcd10f2a08141936d354a2e0b06b485093..ae3e7b516991dbc34c2f1154c4991646a0b6679a 100644 (file)
--- a/src/bin/dhcp4/tests/dhcp4_test_utils.h
+++ b/src/bin/dhcp4/tests/dhcp4_test_utils.h
@@ -30,6 +30,7 @@
 #include <config/command_mgr.h>
 #include <config/unix_command_mgr.h>
 #include <util/multi_threading_mgr.h>
+#include <testutils/log_utils.h>
 #include <list>
 
 #include <boost/shared_ptr.hpp>
@@ -333,7 +334,7 @@ class Dhcp4Client;
 /// Currently it configures the test data path directory in
 /// the @c CfgMgr. When the object is destroyed, the original
 /// path is reverted.
-class BaseServerTest : public ::testing::Test {
+class BaseServerTest : public /* ::testing::Test */ LogContentTest {
 public:
 
     /// @brief Constructor.