From: Razvan Becheriu Date: Tue, 18 Aug 2020 12:52:46 +0000 (+0300) Subject: [#505] add check for T1 and T2 at subnet and network level X-Git-Tag: Kea-1.8.0~17 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7c8c11dc36c93ee4c459c7de725dd156467730f1;p=thirdparty%2Fkea.git [#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 d1bfbb6f10..89ebfe9df8 100644 --- 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 4987b35030..11265f5999 100644 --- 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(subnet4); parseCommon(params, network); - stringstream s; - s << addr << "/" << static_cast(len) << " with params: "; + std::ostringstream output; + output << addr << "/" << static_cast(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(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 629132a5fa..6b3db1c9c3 100644 --- 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(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(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) {