From 92f540859520f8492162799180549a4b547c8356 Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Wed, 19 Jun 2019 16:32:50 +0200 Subject: [PATCH] [295-min-max-lease-time-configuration-options] Improved lifetime bound checks --- src/bin/dhcp4/tests/config_parser_unittest.cc | 20 +++++++++- src/bin/dhcp6/tests/config_parser_unittest.cc | 38 ++++++++++++++++++- .../dhcpsrv/parsers/base_network_parser.cc | 18 +++++++-- 3 files changed, 70 insertions(+), 6 deletions(-) diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc index 240d1e6b92..d03355c450 100644 --- a/src/bin/dhcp4/tests/config_parser_unittest.cc +++ b/src/bin/dhcp4/tests/config_parser_unittest.cc @@ -873,7 +873,7 @@ TEST_F(Dhcp4ParserTest, outBoundValidLifetime) { EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json)); string expected = "subnet configuration failed: " "the value of min-valid-lifetime (2000) is not " - "less than max-valid-lifetime (1000)"; + "less than (default) valid-lifetime (1000)"; checkResult(status, 1, expected); resetConfiguration(); @@ -885,6 +885,9 @@ TEST_F(Dhcp4ParserTest, outBoundValidLifetime) { ASSERT_NO_THROW(json = parseDHCP4(too_large)); EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json)); + expected = "subnet configuration failed: " + "the value of (default) valid-lifetime (2000) is not " + "less than max-valid-lifetime (1000)"; checkResult(status, 1, expected); resetConfiguration(); @@ -916,6 +919,21 @@ TEST_F(Dhcp4ParserTest, outBoundValidLifetime) { "the value of (default) valid-lifetime (5000) is not " "between min-valid-lifetime (1000) and max-valid-lifetime (4000)"; checkResult(status, 1, expected); + resetConfiguration(); + + string crossed = "{ " + genIfaceConfig() + "," + + "\"subnet4\": [ { " + " \"pools\": [ { \"pool\": \"192.0.2.1 - 192.0.2.100\" } ]," + " \"subnet\": \"192.0.2.0/24\" } ]," + "\"valid-lifetime\": 1500, \"min-valid-lifetime\": 2000, " + "\"max-valid-lifetime\": 1000 }"; + + ASSERT_NO_THROW(json = parseDHCP4(crossed)); + EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json)); + expected = "subnet configuration failed: " + "the value of min-valid-lifetime (2000) is not " + "less than max-valid-lifetime (1000)"; + checkResult(status, 1, expected); } /// Check that the renew-timer doesn't have to be specified, in which case diff --git a/src/bin/dhcp6/tests/config_parser_unittest.cc b/src/bin/dhcp6/tests/config_parser_unittest.cc index e458c23c63..a35bdb1939 100644 --- a/src/bin/dhcp6/tests/config_parser_unittest.cc +++ b/src/bin/dhcp6/tests/config_parser_unittest.cc @@ -1025,7 +1025,7 @@ TEST_F(Dhcp6ParserTest, outBoundValidLifetime) { EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json)); string expected = "subnet configuration failed: " "the value of min-valid-lifetime (2000) is not " - "less than max-valid-lifetime (1000)"; + "less than (default) valid-lifetime (1000)"; checkResult(status, 1, expected); resetConfiguration(); @@ -1037,6 +1037,9 @@ TEST_F(Dhcp6ParserTest, outBoundValidLifetime) { ASSERT_NO_THROW(json = parseDHCP6(too_large)); EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json)); + expected = "subnet configuration failed: " + "the value of (default) valid-lifetime (2000) is not " + "less than max-valid-lifetime (1000)"; checkResult(status, 1, expected); resetConfiguration(); @@ -1068,6 +1071,20 @@ TEST_F(Dhcp6ParserTest, outBoundValidLifetime) { "the value of (default) valid-lifetime (5000) is not " "between min-valid-lifetime (1000) and max-valid-lifetime (4000)"; checkResult(status, 1, expected); + resetConfiguration(); + + string crossed = "{ " + genIfaceConfig() + "," + + "\"subnet6\": [ { " + " \"pools\": [ { \"pool\": \"2001:db8::/64\" } ]," + " \"subnet\": \"2001:db8::/32\" } ]," + "\"valid-lifetime\": 1500, \"min-valid-lifetime\": 2000, " + "\"max-valid-lifetime\": 1000 }"; + ASSERT_NO_THROW(json = parseDHCP6(crossed)); + EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json)); + expected = "subnet configuration failed: " + "the value of min-valid-lifetime (2000) is not " + "less than max-valid-lifetime (1000)"; + checkResult(status, 1, expected); } /// Check that preferred-lifetime must be between min-preferred-lifetime and @@ -1088,7 +1105,7 @@ TEST_F(Dhcp6ParserTest, outBoundPreferredLifetime) { EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json)); string expected = "subnet configuration failed: " "the value of min-preferred-lifetime (2000) is not " - "less than max-preferred-lifetime (1000)"; + "less than (default) preferred-lifetime (1000)"; checkResult(status, 1, expected); resetConfiguration(); @@ -1100,6 +1117,9 @@ TEST_F(Dhcp6ParserTest, outBoundPreferredLifetime) { ASSERT_NO_THROW(json = parseDHCP6(too_large)); EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json)); + expected = "subnet configuration failed: " + "the value of (default) preferred-lifetime (2000) is not " + "less than max-preferred-lifetime (1000)"; checkResult(status, 1, expected); resetConfiguration(); @@ -1131,6 +1151,20 @@ TEST_F(Dhcp6ParserTest, outBoundPreferredLifetime) { "the value of (default) preferred-lifetime (5000) is not between " "min-preferred-lifetime (1000) and max-preferred-lifetime (4000)"; checkResult(status, 1, expected); + resetConfiguration(); + + string crossed = "{ " + genIfaceConfig() + "," + + "\"subnet6\": [ { " + " \"pools\": [ { \"pool\": \"2001:db8::/64\" } ]," + " \"subnet\": \"2001:db8::/32\" } ]," + "\"preferred-lifetime\": 1500, \"min-preferred-lifetime\": 2000, " + "\"max-preferred-lifetime\": 1000 }"; + ASSERT_NO_THROW(json = parseDHCP6(crossed)); + EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json)); + expected = "subnet configuration failed: " + "the value of min-preferred-lifetime (2000) is not " + "less than max-preferred-lifetime (1000)"; + checkResult(status, 1, expected); } /// The goal of this test is to verify if configuration without any diff --git a/src/lib/dhcpsrv/parsers/base_network_parser.cc b/src/lib/dhcpsrv/parsers/base_network_parser.cc index 6614a60486..a83614defe 100644 --- a/src/lib/dhcpsrv/parsers/base_network_parser.cc +++ b/src/lib/dhcpsrv/parsers/base_network_parser.cc @@ -69,9 +69,21 @@ BaseNetworkParser::parseLifetime(const ConstElementPtr& scope, } // Check that min <= max. if (min_value > max_value) { - isc_throw(DhcpConfigError, "the value of min-" << name << " (" - << min_value << ") is not less than max-" << name << " (" - << max_value << ")"); + if (has_min && has_max) { + isc_throw(DhcpConfigError, "the value of min-" << name << " (" + << min_value << ") is not less than max-" << name << " (" + << max_value << ")"); + } else if (has_min) { + // Only min and default so min > default. + isc_throw(DhcpConfigError, "the value of min-" << name << " (" + << min_value << ") is not less than (default) " << name + << " (" << value << ")"); + } else { + // Only default and max so default > max. + isc_throw(DhcpConfigError, "the value of (default) " << name + << " (" << value << ") is not less than max-" << name + << " (" << max_value << ")"); + } } // Check that value is between min and max. if ((value < min_value) || (value > max_value)) { -- 2.47.2