[#505] add check for T1 and T2 at subnet and network level

diff --git a/src/lib/dhcpsrv/parsers/base_network_parser.cc b/src/lib/dhcpsrv/parsers/base_network_parser.cc
index d1bfbb6f10400ca7a48a0afdb4c4d65e6d83baf1..89ebfe9df885898fae191dddc35f73a6e8064b2a 100644 (file)
--- a/src/lib/dhcpsrv/parsers/base_network_parser.cc
+++ b/src/lib/dhcpsrv/parsers/base_network_parser.cc
@@ -100,12 +100,32 @@ BaseNetworkParser::parseLifetime(const ConstElementPtr& scope,
 void
 BaseNetworkParser::parseCommon(const ConstElementPtr& network_data,
                                NetworkPtr& network) {
-    if (network_data->contains("renew-timer")) {
-        network->setT1(getInteger(network_data, "renew-timer"));
+    bool has_renew = network_data->contains("renew-timer");
+    bool has_rebind = network_data->contains("rebind-timer");
+    int64_t renew = -1;
+    int64_t rebind = -1;
+
+    if (has_renew) {
+        renew = getInteger(network_data, "renew-timer");
+        if (renew < 0) {
+            isc_throw(DhcpConfigError, "the value of renew-timer" << " ("
+                      << renew << ") must be a positive number");
+        }
+        network->setT1(renew);
+    }
+
+    if (has_rebind) {
+        rebind = getInteger(network_data, "rebind-timer");
+        if (rebind < 0) {
+            isc_throw(DhcpConfigError, "the value of rebind-timer" << " ("
+                      << rebind << ") must be a positive number");
+        }
+        network->setT2(rebind);
     }
 
-    if (network_data->contains("rebind-timer")) {
-        network->setT2(getInteger(network_data, "rebind-timer"));
+    if (has_renew && has_rebind && (renew > rebind)) {
+        isc_throw(DhcpConfigError, "the value of renew-timer" << " (" << renew
+                  << ") is not less than rebind-timer" << " (" << rebind << ")");
     }
 
     network->setValid(parseLifetime(network_data, "valid-lifetime"));

diff --git a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc
index 4987b35030621dc05fde333ffa842aebe4d30733..11265f5999c3207f27dafffbd9e7c63cea89715d 100644 (file)
--- a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc
+++ b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc
@@ -747,20 +747,34 @@ Subnet4ConfigParser::initSubnet(data::ConstElementPtr params,
     NetworkPtr network = boost::dynamic_pointer_cast<Network>(subnet4);
     parseCommon(params, network);
 
-    stringstream s;
-    s << addr << "/" << static_cast<int>(len) << " with params: ";
+    std::ostringstream output;
+    output << addr << "/" << static_cast<int>(len) << " with params: ";
+
+    bool has_renew = !subnet4->getT1().unspecified();
+    bool has_rebind = !subnet4->getT2().unspecified();
+    int64_t renew = -1;
+    int64_t rebind = -1;
+
     // t1 and t2 are optional may be not specified.
-    if (!subnet4->getT1().unspecified()) {
-        s << "t1=" << subnet4->getT1().get() << ", ";
+    if (has_renew) {
+        renew = subnet4->getT1().get();
+        output << "t1=" << renew << ", ";
+    }
+    if (rebind) {
+        rebind = subnet4->getT2().get();
+        output << "t2=" << rebind << ", ";
     }
-    if (!subnet4->getT2().unspecified()) {
-        s << "t2=" << subnet4->getT2().get() << ", ";
+
+    if (has_renew && has_rebind && (renew > rebind)) {
+        isc_throw(DhcpConfigError, "the value of renew-timer" << " (" << renew
+                  << ") is not less than rebind-timer" << " (" << rebind << ")");
     }
+
     if (!subnet4->getValid().unspecified()) {
-        s << "valid-lifetime=" << subnet4->getValid().get();
+        output << "valid-lifetime=" << subnet4->getValid().get();
     }
 
-    LOG_INFO(dhcpsrv_logger, DHCPSRV_CFGMGR_NEW_SUBNET4).arg(s.str());
+    LOG_INFO(dhcpsrv_logger, DHCPSRV_CFGMGR_NEW_SUBNET4).arg(output.str());
 
     // Set the match-client-id value for the subnet.
     if (params->contains("match-client-id")) {
@@ -1231,12 +1245,26 @@ Subnet6ConfigParser::initSubnet(data::ConstElementPtr params,
     std::ostringstream output;
     output << addr << "/" << static_cast<int>(len) << " with params: ";
     // t1 and t2 are optional may be not specified.
-    if (!subnet6->getT1().unspecified()) {
-        output << "t1=" << subnet6->getT1().get() << ", ";
+
+    bool has_renew = !subnet6->getT1().unspecified();
+    bool has_rebind = !subnet6->getT2().unspecified();
+    int64_t renew = -1;
+    int64_t rebind = -1;
+
+    if (has_renew) {
+        renew = subnet6->getT1().get();
+        output << "t1=" << renew << ", ";
     }
-    if (!subnet6->getT2().unspecified()) {
-        output << "t2=" << subnet6->getT2().get() << ", ";
+    if (has_rebind) {
+        rebind = subnet6->getT2().get();
+        output << "t2=" << rebind << ", ";
     }
+
+    if (has_renew && has_rebind && (renew > rebind)) {
+        isc_throw(DhcpConfigError, "the value of renew-timer" << " (" << renew
+                  << ") is not less than rebind-timer" << " (" << rebind << ")");
+    }
+
     if (!subnet6->getPreferred().unspecified()) {
         output << "preferred-lifetime=" << subnet6->getPreferred().get() << ", ";
     }

diff --git a/src/lib/dhcpsrv/tests/shared_network_parser_unittest.cc b/src/lib/dhcpsrv/tests/shared_network_parser_unittest.cc
index 629132a5fa59c61ff998351f6d7931be12f5b39e..6b3db1c9c3ccae589433ad3e81b0bb9c3d0b09e1 100644 (file)
--- a/src/lib/dhcpsrv/tests/shared_network_parser_unittest.cc
+++ b/src/lib/dhcpsrv/tests/shared_network_parser_unittest.cc
@@ -469,6 +469,27 @@ TEST_F(SharedNetwork4ParserTest, iface) {
     EXPECT_EQ("eth1", network->getIface().get());
 }
 
+// This test verifies that shared network parser for IPv4 works properly
+// when using invalid renew and rebind timers.
+TEST_F(SharedNetwork4ParserTest, parseWithInvalidRenewRebind) {
+    IfaceMgrTestConfig ifmgr(true);
+
+    // Basic configuration for shared network. A bunch of parameters
+    // have to be specified for subnets because subnet parsers expect
+    // that default and global values are set.
+    std::string config = getWorkingConfig();
+    ElementPtr config_element = Element::fromJSON(config);
+    ConstElementPtr valid_element = config_element->get("renew-timer");
+    ElementPtr invalid_element = boost::const_pointer_cast<Element>(valid_element);
+    invalid_element->setValue(200);
+
+    // Parse configuration specified above.
+    SharedNetwork4Parser parser;
+    SharedNetwork4Ptr network;
+
+    ASSERT_THROW(network = parser.parse(config_element), DhcpConfigError);
+    ASSERT_FALSE(network);
+}
 
 /// @brief Test fixture class for SharedNetwork6Parser class.
 class SharedNetwork6ParserTest : public SharedNetworkParserTest {
@@ -575,7 +596,7 @@ public:
     bool use_iface_id_;
 };
 
-// This test verifies that shared network parser for IPv4 works properly
+// This test verifies that shared network parser for IPv6 works properly
 // in a positive test scenario.
 TEST_F(SharedNetwork6ParserTest, parse) {
     IfaceMgrTestConfig ifmgr(true);
@@ -673,7 +694,7 @@ TEST_F(SharedNetwork6ParserTest, parse) {
     EXPECT_EQ("2001:db8:1::cafe", addresses[0].toText());
 }
 
-// This test verifies that shared network parser for IPv4 works properly
+// This test verifies that shared network parser for IPv6 works properly
 // in a positive test scenario.
 TEST_F(SharedNetwork6ParserTest, parseWithInterfaceId) {
     IfaceMgrTestConfig ifmgr(true);
@@ -694,6 +715,27 @@ TEST_F(SharedNetwork6ParserTest, parseWithInterfaceId) {
     ASSERT_TRUE(opt_iface_id);
 }
 
+// This test verifies that shared network parser for IPv6 works properly
+// when using invalid renew and rebind timers.
+TEST_F(SharedNetwork6ParserTest, parseWithInvalidRenewRebind) {
+    IfaceMgrTestConfig ifmgr(true);
+
+    // Use the configuration with interface-id instead of interface parameter.
+    use_iface_id_ = true;
+    std::string config = getWorkingConfig();
+    ElementPtr config_element = Element::fromJSON(config);
+    ConstElementPtr valid_element = config_element->get("renew-timer");
+    ElementPtr invalid_element = boost::const_pointer_cast<Element>(valid_element);
+    invalid_element->setValue(200);
+
+    // Parse configuration specified above.
+    SharedNetwork6Parser parser;
+    SharedNetwork6Ptr network;
+
+    ASSERT_THROW(network = parser.parse(config_element), DhcpConfigError);
+    ASSERT_FALSE(network);
+}
+
 // This test verifies that error is returned when trying to configure a
 // shared network with both interface and interface id.
 TEST_F(SharedNetwork6ParserTest, mutuallyExclusiveInterfaceId) {