[#2677] Ignore invalid renew-timer values

src/lib/dhcpsrv/dhcpsrv_messages.*
    DHCPSRV_CFGMGR_RENEW_GTR_REBIND - new message

src/lib/dhcpsrv/network.h
    Added Network::getLabel()

src/lib/dhcpsrv/parsers/base_network_parser.cc
    BaseNetworkParser::parseCommon() - log renew > rebind
    rather than throw

src/lib/dhcpsrv/parsers/dhcp_parsers.cc
    Subnet4ConfigParser::initSubnet() - removed duplicate timer check
    Subnet6ConfigParser::initSubnet() - removed duplicate timer check

src/lib/dhcpsrv/shared_network.h
    Added SharedNetwork4::getLabel()
    Added SharedNetwork6::getLabel()

src/lib/dhcpsrv/subnet.h
    Added Subnet::getLabel()

src/lib/dhcpsrv/tests/cfg_subnets4_unittest.cc
    TEST_F(Subnet4ParserTest, parseWithInvalidRenewRebind) - new test

src/lib/dhcpsrv/tests/cfg_subnets6_unittest.cc
    TEST_F(Subnet4ParserTest, parseWithInvalidRenewRebind) - new test

src/lib/dhcpsrv/tests/shared_network_parser_unittest.cc
    Updated tests

12 files changed, 167 insertions, 25 deletions

diff --git a/ChangeLog b/ChangeLog
index f11deb8217..32fa941778 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2097.	[func]		tmark
+	kea-dhcp4 and kea-dhcp6 will now accept but warn about
+	and then ignore renew-timer	values that exceed rebind-timer
+	values.  Prior to this the servers treated this as a fatal
+	configuration error.
+	(Gitlab #2677)
+
 2096.	[bug]		[tmark]
 	Corrected a bug which allowed options to be added to themselves
 	as suboptions.
diff --git a/src/lib/dhcpsrv/dhcpsrv_messages.cc b/src/lib/dhcpsrv/dhcpsrv_messages.cc
index 08a84874fd..a96c03aea9 100644
--- a/src/lib/dhcpsrv/dhcpsrv_messages.cc
+++ b/src/lib/dhcpsrv/dhcpsrv_messages.cc
@@ -31,6 +31,7 @@ extern const isc::log::MessageID DHCPSRV_CFGMGR_ONLY_SUBNET4 = "DHCPSRV_CFGMGR_O
 extern const isc::log::MessageID DHCPSRV_CFGMGR_ONLY_SUBNET6 = "DHCPSRV_CFGMGR_ONLY_SUBNET6";
 extern const isc::log::MessageID DHCPSRV_CFGMGR_OPTION_DUPLICATE = "DHCPSRV_CFGMGR_OPTION_DUPLICATE";
 extern const isc::log::MessageID DHCPSRV_CFGMGR_RELAY_IP_ADDRESS_DEPRECATED = "DHCPSRV_CFGMGR_RELAY_IP_ADDRESS_DEPRECATED";
+extern const isc::log::MessageID DHCPSRV_CFGMGR_RENEW_GTR_REBIND = "DHCPSRV_CFGMGR_RENEW_GTR_REBIND";
 extern const isc::log::MessageID DHCPSRV_CFGMGR_SOCKET_RAW_UNSUPPORTED = "DHCPSRV_CFGMGR_SOCKET_RAW_UNSUPPORTED";
 extern const isc::log::MessageID DHCPSRV_CFGMGR_SOCKET_TYPE_DEFAULT = "DHCPSRV_CFGMGR_SOCKET_TYPE_DEFAULT";
 extern const isc::log::MessageID DHCPSRV_CFGMGR_SOCKET_TYPE_SELECT = "DHCPSRV_CFGMGR_SOCKET_TYPE_SELECT";
@@ -282,6 +283,7 @@ const char* values[] = {
     "DHCPSRV_CFGMGR_ONLY_SUBNET6", "retrieved subnet %1 for address hint %2",
     "DHCPSRV_CFGMGR_OPTION_DUPLICATE", "multiple options with the code: %1 added to the subnet: %2",
     "DHCPSRV_CFGMGR_RELAY_IP_ADDRESS_DEPRECATED", "\"relay\" uses \"ip-address\", which has been deprecated, please use \"ip-addresses\": %1",
+    "DHCPSRV_CFGMGR_RENEW_GTR_REBIND", "in %1, the value of renew-timer %2 is greater than the value of rebind-timer %3, ignoring renew-timer",
     "DHCPSRV_CFGMGR_SOCKET_RAW_UNSUPPORTED", "use of raw sockets is unsupported on this OS, UDP sockets will be used",
     "DHCPSRV_CFGMGR_SOCKET_TYPE_DEFAULT", "\"dhcp-socket-type\" not specified , using default socket type %1",
     "DHCPSRV_CFGMGR_SOCKET_TYPE_SELECT", "using socket type %1",
diff --git a/src/lib/dhcpsrv/dhcpsrv_messages.h b/src/lib/dhcpsrv/dhcpsrv_messages.h
index a3d9c5362f..b5905a3743 100644
--- a/src/lib/dhcpsrv/dhcpsrv_messages.h
+++ b/src/lib/dhcpsrv/dhcpsrv_messages.h
@@ -32,6 +32,7 @@ extern const isc::log::MessageID DHCPSRV_CFGMGR_ONLY_SUBNET4;
 extern const isc::log::MessageID DHCPSRV_CFGMGR_ONLY_SUBNET6;
 extern const isc::log::MessageID DHCPSRV_CFGMGR_OPTION_DUPLICATE;
 extern const isc::log::MessageID DHCPSRV_CFGMGR_RELAY_IP_ADDRESS_DEPRECATED;
+extern const isc::log::MessageID DHCPSRV_CFGMGR_RENEW_GTR_REBIND;
 extern const isc::log::MessageID DHCPSRV_CFGMGR_SOCKET_RAW_UNSUPPORTED;
 extern const isc::log::MessageID DHCPSRV_CFGMGR_SOCKET_TYPE_DEFAULT;
 extern const isc::log::MessageID DHCPSRV_CFGMGR_SOCKET_TYPE_SELECT;
diff --git a/src/lib/dhcpsrv/dhcpsrv_messages.mes b/src/lib/dhcpsrv/dhcpsrv_messages.mes
index d6578bb743..76688654ae 100644
--- a/src/lib/dhcpsrv/dhcpsrv_messages.mes
+++ b/src/lib/dhcpsrv/dhcpsrv_messages.mes
@@ -1249,3 +1249,9 @@ indicate an error in the source code, please submit a bug report.
 % DHCPSRV_UNKNOWN_DB unknown database type: %1
 The database access string specified a database type (given in the
 message) that is unknown to the software.  This is a configuration error.
+
+% DHCPSRV_CFGMGR_RENEW_GTR_REBIND in %1, the value of renew-timer %2 is greater than the value of rebind-timer %3, ignoring renew-timer
+A warning message that indicates the configured renew-timer is greater
+than the configured rebind-timer.  The server will ignore the the renew
+timer value and send the rebind timer value only.  This is considered
+a non-fatal configuration error.
diff --git a/src/lib/dhcpsrv/network.h b/src/lib/dhcpsrv/network.h
index c61720fa95..e5e0b90ca8 100644
--- a/src/lib/dhcpsrv/network.h
+++ b/src/lib/dhcpsrv/network.h
@@ -806,6 +806,13 @@ public:
     /// @return A pointer to unparsed network configuration.
     virtual data::ElementPtr toElement() const;
 
+    /// @brief Generates an identifying label for logging.
+    ///
+    /// @return string containing the label
+    virtual std::string getLabel() const {
+        return ("base-network");
+    }
+
 protected:
 
     /// @brief Gets the optional callback function used to fetch globally
diff --git a/src/lib/dhcpsrv/parsers/base_network_parser.cc b/src/lib/dhcpsrv/parsers/base_network_parser.cc
index 14fd13921b..b1f00a6918 100644
--- a/src/lib/dhcpsrv/parsers/base_network_parser.cc
+++ b/src/lib/dhcpsrv/parsers/base_network_parser.cc
@@ -6,6 +6,7 @@
 
 #include <config.h>
 #include <util/triplet.h>
+#include <dhcpsrv/dhcpsrv_log.h>
 #include <dhcpsrv/parsers/base_network_parser.h>
 #include <util/optional.h>
 #include <util/strutil.h>
@@ -114,9 +115,13 @@ BaseNetworkParser::parseCommon(const ConstElementPtr& network_data,
     }
 
     if (has_renew && has_rebind && (renew > rebind)) {
-        isc_throw(DhcpConfigError, "the value of renew-timer (" << renew
-                  << ") is greater than the value of rebind-timer ("
-                  << rebind << ")");
+        // The renew-timer value is too large and server logic
+        // later on will end up not sending it. Warn the user but
+        // allow the configuration to pass.
+        LOG_WARN(dhcpsrv_logger, DHCPSRV_CFGMGR_RENEW_GTR_REBIND)
+                 .arg(network->getLabel())
+                 .arg(renew)
+                 .arg(rebind);
     }
 
     network->setValid(parseIntTriplet(network_data, "valid-lifetime"));
diff --git a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc
index a6adacb9a5..6ee1f51f0d 100644
--- a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc
+++ b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc
@@ -786,12 +786,6 @@ Subnet4ConfigParser::initSubnet(data::ConstElementPtr params,
         output << "t2=" << rebind << ", ";
     }
 
-    if (has_renew && has_rebind && (renew > rebind)) {
-        isc_throw(DhcpConfigError, "the value of renew-timer (" << renew
-                  << ") is greater than the value of rebind-timer ("
-                  << rebind << ")");
-    }
-
     if (!subnet4->getValid().unspecified()) {
         output << "valid-lifetime=" << subnet4->getValid().get();
     }
@@ -1373,12 +1367,6 @@ Subnet6ConfigParser::initSubnet(data::ConstElementPtr params,
         output << "t2=" << rebind << ", ";
     }
 
-    if (has_renew && has_rebind && (renew > rebind)) {
-        isc_throw(DhcpConfigError, "the value of renew-timer (" << renew
-                  << ") is greater than the value of rebind-timer ("
-                  << rebind << ")");
-    }
-
     if (!subnet6->getPreferred().unspecified()) {
         output << "preferred-lifetime=" << subnet6->getPreferred().get() << ", ";
     }
diff --git a/src/lib/dhcpsrv/shared_network.h b/src/lib/dhcpsrv/shared_network.h
index 41062369a1..da95baa2d7 100644
--- a/src/lib/dhcpsrv/shared_network.h
+++ b/src/lib/dhcpsrv/shared_network.h
@@ -1,4 +1,4 @@
-// Copyright (C) 2017-2022 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2017-2023 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -200,6 +200,15 @@ public:
     /// @return A pointer to unparsed shared network configuration.
     virtual data::ElementPtr toElement() const;
 
+    /// @brief Generates an identifying label for logging.
+    ///
+    /// @return string containing the label
+    virtual std::string getLabel() const {
+        std::stringstream ss;
+        ss << "shared-network " << name_;
+        return (ss.str());
+    }
+
 private:
 
     /// @brief Holds a name of a shared network.
@@ -398,6 +407,15 @@ public:
     /// @return A pointer to unparsed shared network configuration.
     virtual data::ElementPtr toElement() const;
 
+    /// @brief Generates an identifying label for logging.
+    ///
+    /// @return string containing the label
+    virtual std::string getLabel() const {
+        std::stringstream ss;
+        ss << "shared-network " << name_;
+        return (ss.str());
+    }
+
 private:
 
     /// @brief Holds a name of a shared network.
diff --git a/src/lib/dhcpsrv/subnet.h b/src/lib/dhcpsrv/subnet.h
index 0521a5fbee..3e0291a4d5 100644
--- a/src/lib/dhcpsrv/subnet.h
+++ b/src/lib/dhcpsrv/subnet.h
@@ -422,6 +422,12 @@ protected:
     /// @return A pointer to unparsed subnet configuration.
     virtual data::ElementPtr toElement() const;
 
+    virtual std::string getLabel() const {
+        std::stringstream ss;
+        ss << "subnet-id " << id_;
+        return (ss.str());
+    }
+
     /// @brief Converts subnet prefix to a pair of prefix/length pair.
     ///
     /// IPv4 and IPv6 specific conversion functions should apply extra checks
diff --git a/src/lib/dhcpsrv/tests/cfg_subnets4_unittest.cc b/src/lib/dhcpsrv/tests/cfg_subnets4_unittest.cc
index a1008f33f7..899abb75a4 100644
--- a/src/lib/dhcpsrv/tests/cfg_subnets4_unittest.cc
+++ b/src/lib/dhcpsrv/tests/cfg_subnets4_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2014-2022 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2014-2023 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -6,6 +6,7 @@
 
 #include <config.h>
 
+#include <cc/data.h>
 #include <dhcp/classify.h>
 #include <dhcp/libdhcp++.h>
 #include <dhcp/option_custom.h>
@@ -25,6 +26,7 @@
 #include <dhcpsrv/cfg_hosts.h>
 #include <stats/stats_mgr.h>
 #include <testutils/gtest_utils.h>
+#include <testutils/log_utils.h>
 #include <testutils/test_to_element.h>
 #include <util/doubles.h>
 
@@ -33,6 +35,7 @@
 
 using namespace isc;
 using namespace isc::asiolink;
+using namespace isc::data;
 using namespace isc::dhcp;
 using namespace isc::dhcp::test;
 using namespace isc::stats;
@@ -2176,4 +2179,44 @@ TEST(CfgSubnets4Test, getLinks) {
     EXPECT_EQ(24, link_len);
 }
 
+/// @brief Test fixture for parsing v6 Subnets that can verify log output.
+class Subnet4ParserTest :  public LogContentTest {
+public:
+
+    /// @brief virtual destructor
+    virtual ~Subnet4ParserTest(){};
+};
+
+// This test verifies that subnet parser for IPv4 works properly
+// when using invalid renew and rebind timers.
+TEST_F(Subnet4ParserTest, parseWithInvalidRenewRebind) {
+    //IfaceMgrTestConfig ifmgr(true);
+    std::string config =
+        "{\n"
+        "    \"id\": 1,\n"
+        "    \"subnet\": \"10.1.2.0/24\",\n"
+        "    \"valid-lifetime\": 399,\n"
+        "    \"rebind-timer\": 199,\n"
+        "    \"renew-timer\": 200\n"
+        "}";
+
+    // Basic configuration for subnet4 but with a renew-timer value
+    // larger than rebind-timer.
+    ElementPtr config_element = Element::fromJSON(config);
+
+    // Parse configuration specified above.
+    Subnet4ConfigParser parser(false);
+    Subnet4Ptr subnet;
+
+    // Parser should not throw.
+    ASSERT_NO_THROW(subnet = parser.parse(config_element));
+    ASSERT_TRUE(subnet);
+
+    // Veriy we emitted the proper log message.
+    addString("DHCPSRV_CFGMGR_RENEW_GTR_REBIND in subnet-id 1,"
+              " the value of renew-timer 200 is greater than the value"
+              " of rebind-timer 199, ignoring renew-timer");
+    EXPECT_TRUE(checkFile());
+}
+
 } // end of anonymous namespace
diff --git a/src/lib/dhcpsrv/tests/cfg_subnets6_unittest.cc b/src/lib/dhcpsrv/tests/cfg_subnets6_unittest.cc
index 53e5351753..3471daea3f 100644
--- a/src/lib/dhcpsrv/tests/cfg_subnets6_unittest.cc
+++ b/src/lib/dhcpsrv/tests/cfg_subnets6_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2014-2022 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2014-2023 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -6,6 +6,7 @@
 
 #include <config.h>
 
+#include <cc/data.h>
 #include <dhcp/classify.h>
 #include <dhcp/dhcp6.h>
 #include <dhcp/option.h>
@@ -22,6 +23,7 @@
 #include <dhcpsrv/cfg_hosts.h>
 #include <stats/stats_mgr.h>
 #include <testutils/gtest_utils.h>
+#include <testutils/log_utils.h>
 #include <testutils/test_to_element.h>
 #include <util/doubles.h>
 
@@ -30,6 +32,7 @@
 
 using namespace isc;
 using namespace isc::asiolink;
+using namespace isc::data;
 using namespace isc::dhcp;
 using namespace isc::dhcp::test;
 using namespace isc::stats;
@@ -2098,4 +2101,43 @@ TEST(CfgSubnets6Test, getLinks) {
     EXPECT_EQ(48, link_len);
 }
 
+/// @brief Test fixture for parsing v6 Subnets that can verify log output.
+class Subnet6ParserTest :  public LogContentTest {
+public:
+
+    /// @brief virtual destructor
+    virtual ~Subnet6ParserTest(){};
+};
+
+// This test verifies that subnet parser for IPv6 works properly
+// when using invalid renew and rebind timers.
+TEST_F(Subnet6ParserTest, parseWithInvalidRenewRebind) {
+    std::string config =
+        "{\n"
+        "    \"id\": 1,\n"
+        "    \"subnet\": \"2001:db8:1::/64\",\n"
+        "    \"valid-lifetime\": 399,\n"
+        "    \"rebind-timer\": 199,\n"
+        "    \"renew-timer\": 200\n"
+        "}";
+
+    // Basic configuration for subnet4 but with a renew-timer value
+    // larger than rebind-timer.
+    ElementPtr config_element = Element::fromJSON(config);
+
+    // Parse configuration specified above.
+    Subnet6ConfigParser parser(false);
+    Subnet6Ptr subnet;
+
+    // Parser should not throw.
+    ASSERT_NO_THROW(subnet = parser.parse(config_element));
+    ASSERT_TRUE(subnet);
+
+    // Veriy we emitted the proper log message.
+    addString("DHCPSRV_CFGMGR_RENEW_GTR_REBIND in subnet-id 1,"
+              " the value of renew-timer 200 is greater than the value"
+              " of rebind-timer 199, ignoring renew-timer");
+    EXPECT_TRUE(checkFile());
+}
+
 } // end of anonymous namespace
diff --git a/src/lib/dhcpsrv/tests/shared_network_parser_unittest.cc b/src/lib/dhcpsrv/tests/shared_network_parser_unittest.cc
index 6090640613..a95ddfd940 100644
--- a/src/lib/dhcpsrv/tests/shared_network_parser_unittest.cc
+++ b/src/lib/dhcpsrv/tests/shared_network_parser_unittest.cc
@@ -16,6 +16,7 @@
 #include <dhcpsrv/cfg_option.h>
 #include <dhcpsrv/parsers/shared_network_parser.h>
 #include <testutils/gtest_utils.h>
+#include <testutils/log_utils.h>
 #include <gtest/gtest.h>
 #include <string>
 
@@ -26,7 +27,7 @@ using namespace isc::dhcp;
 using namespace isc::dhcp::test;
 
 namespace {
-class SharedNetworkParserTest : public ::testing::Test {
+class SharedNetworkParserTest :  public LogContentTest {
 public:
 
     /// @brief Structure for describing a single relay test scenario
@@ -495,7 +496,8 @@ TEST_F(SharedNetwork4ParserTest, iface) {
 TEST_F(SharedNetwork4ParserTest, parseWithInvalidRenewRebind) {
     IfaceMgrTestConfig ifmgr(true);
 
-    // Basic configuration for shared network.
+    // Basic configuration for shared network but with a renew-timer value
+    // larger than rebind-timer.
     std::string config = getWorkingConfig();
     ElementPtr config_element = Element::fromJSON(config);
     ConstElementPtr valid_element = config_element->get("rebind-timer");
@@ -508,8 +510,15 @@ TEST_F(SharedNetwork4ParserTest, parseWithInvalidRenewRebind) {
     SharedNetwork4Parser parser;
     SharedNetwork4Ptr network;
 
-    ASSERT_THROW(network = parser.parse(config_element), DhcpConfigError);
-    ASSERT_FALSE(network);
+    // Parser should not throw.
+    ASSERT_NO_THROW(network = parser.parse(config_element));
+    ASSERT_TRUE(network);
+
+    // Veriy we emitted the proper log message.
+    addString("DHCPSRV_CFGMGR_RENEW_GTR_REBIND in shared-network bird,"
+              " the value of renew-timer 200 is greater than the value"
+              " of rebind-timer 199, ignoring renew-timer");
+    EXPECT_TRUE(checkFile());
 }
 
 // This test verifies that shared network parser for IPv4 works properly
@@ -785,7 +794,8 @@ TEST_F(SharedNetwork6ParserTest, parseWithInterfaceId) {
 TEST_F(SharedNetwork6ParserTest, parseWithInvalidRenewRebind) {
     IfaceMgrTestConfig ifmgr(true);
 
-    // Use the configuration with interface-id instead of interface parameter.
+    // Basic configuration for shared network but with a renew-timer value
+    // larger than rebind-timer.
     use_iface_id_ = true;
     std::string config = getWorkingConfig();
     ElementPtr config_element = Element::fromJSON(config);
@@ -799,8 +809,15 @@ TEST_F(SharedNetwork6ParserTest, parseWithInvalidRenewRebind) {
     SharedNetwork6Parser parser;
     SharedNetwork6Ptr network;
 
-    ASSERT_THROW(network = parser.parse(config_element), DhcpConfigError);
-    ASSERT_FALSE(network);
+    // Parser should not throw.
+    ASSERT_NO_THROW(network = parser.parse(config_element));
+    ASSERT_TRUE(network);
+
+    // Veriy we emitted the proper log message.
+    addString("DHCPSRV_CFGMGR_RENEW_GTR_REBIND in shared-network bird,"
+              " the value of renew-timer 200 is greater than the value"
+              " of rebind-timer 199, ignoring renew-timer");
+    EXPECT_TRUE(checkFile());
 }
 
 // This test verifies that shared network parser for IPv6 works properly