From: Francis Dupont Date: Sat, 14 Oct 2017 23:08:03 +0000 (+0200) Subject: [5297] Addressed point 3 X-Git-Tag: Kea-1.3.0~2^2~1^2~2 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=cc2afb3a936bad1c8ef3129e4db3d9059cd6a454;p=thirdparty%2Fkea.git [5297] Addressed point 3 --- diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc index 8c47e36f51..4f81780540 100644 --- a/src/bin/dhcp4/tests/config_parser_unittest.cc +++ b/src/bin/dhcp4/tests/config_parser_unittest.cc @@ -1661,6 +1661,64 @@ TEST_F(Dhcp4ParserTest, badPools) { EXPECT_TRUE(errorContainsPosition(status, "")); } +// Goal of this test is to verify no pool definitions is invalid +// and returns a location in the error message. +TEST_F(Dhcp4ParserTest, noPools) { + + // Configuration string. + string config = "{ " + genIfaceConfig() + "," + + "\"rebind-timer\": 2000, " + "\"renew-timer\": 1000, " + "\"subnet4\": [ { " + " \"pools\": [ { \"user-context\": { } } ]," + " \"subnet\": \"192.0.2.0/24\" } ]," + "\"valid-lifetime\": 4000 }"; + + EXPECT_THROW(parseDHCP4(config, true), Dhcp4ParseError); +} + +// Goal of this test is to verify that invalid subnet fails to be parsed. +TEST_F(Dhcp4ParserTest, badSubnet) { + + // Configuration string. + string config = "{ " + genIfaceConfig() + "," + + "\"rebind-timer\": 2000, " + "\"renew-timer\": 1000, " + "\"subnet4\": [ { " + " \"pools\": [ ]," + " \"subnet\": \"192.0.2.0\" } ]," + "\"valid-lifetime\": 4000 }"; + + ConstElementPtr json; + ASSERT_NO_THROW(json = parseDHCP4(config, true)); + ConstElementPtr status; + EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json)); + checkResult(status, 1); + EXPECT_TRUE(errorContainsPosition(status, "")); +} + +// Goal of this test is to verify that unknown interface fails +// to be parsed. +TEST_F(Dhcp4ParserTest, unknownInterface) { + + // Configuration string. + string config = "{ " + genIfaceConfig() + "," + + "\"rebind-timer\": 2000, " + "\"renew-timer\": 1000, " + "\"subnet4\": [ { " + " \"pools\": [ ]," + " \"subnet\": \"192.0.2.0/24\"," + " \"interface\": \"ethX\" } ]," + "\"valid-lifetime\": 4000 }"; + + ConstElementPtr json; + ASSERT_NO_THROW(json = parseDHCP4(config, true)); + ConstElementPtr status; + EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json)); + checkResult(status, 1); + EXPECT_TRUE(errorContainsPosition(status, "")); +} + // The goal of this test is to check whether an option definition // that defines an option carrying an IPv4 address can be created. TEST_F(Dhcp4ParserTest, optionDefIpv4Address) { diff --git a/src/bin/dhcp4/tests/d2_unittest.cc b/src/bin/dhcp4/tests/d2_unittest.cc index 3fbd5eb5bd..8d71f6023a 100644 --- a/src/bin/dhcp4/tests/d2_unittest.cc +++ b/src/bin/dhcp4/tests/d2_unittest.cc @@ -386,6 +386,125 @@ TEST_F(Dhcp4SrvD2Test, queueMaxError) { ASSERT_FALSE(mgr.amSending()); } +// Tests invalid config with TCP protocol +TEST_F(Dhcp4SrvD2Test, badTCP) { + std::string config = + "{ \"interfaces-config\": {" + " \"interfaces\": [ \"*\" ]" + "}," + "\"rebind-timer\": 2000, " + "\"renew-timer\": 1000, " + "\"subnet4\": [ { " + " \"pools\": [ { \"pool\": \"192.0.2.1 - 192.0.2.100\" } ]," + " \"subnet\": \"192.0.2.0/24\" } ]," + " \"dhcp-ddns\" : {" + " \"enable-updates\" : true, " + " \"server-ip\" : \"127.0.0.1\", " + " \"server-port\" : 53001, " + " \"sender-ip\" : \"0.0.0.0\", " + " \"sender-port\" : 0, " + " \"max-queue-size\" : 1024, " + " \"ncr-protocol\" : \"TCP\", " + " \"ncr-format\" : \"JSON\", " + " \"always-include-fqdn\" : true, " + " \"override-no-update\" : true, " + " \"override-client-update\" : true, " + " \"replace-client-name\" : \"when-present\", " + " \"generated-prefix\" : \"test.prefix\", " + " \"qualifying-suffix\" : \"test.suffix.\" }," + "\"valid-lifetime\": 4000 }"; + + ElementPtr json = Element::fromJSON(config); + ConstElementPtr status; + + CfgMgr::instance().clear(); + + EXPECT_NO_THROW(status = configureDhcp4Server(srv_, json)); + ASSERT_TRUE(status); + int rcode; + ConstElementPtr comment = config::parseAnswer(rcode, status); + EXPECT_EQ(1, rcode); +} + +// Tests invalid config with bad sender family +TEST_F(Dhcp4SrvD2Test, badFamily) { + std::string config = + "{ \"interfaces-config\": {" + " \"interfaces\": [ \"*\" ]" + "}," + "\"rebind-timer\": 2000, " + "\"renew-timer\": 1000, " + "\"subnet4\": [ { " + " \"pools\": [ { \"pool\": \"192.0.2.1 - 192.0.2.100\" } ]," + " \"subnet\": \"192.0.2.0/24\" } ]," + " \"dhcp-ddns\" : {" + " \"enable-updates\" : true, " + " \"server-ip\" : \"127.0.0.1\", " + " \"server-port\" : 53001, " + " \"sender-ip\" : \"::\", " + " \"sender-port\" : 0, " + " \"max-queue-size\" : 1024, " + " \"ncr-protocol\" : \"UDP\", " + " \"ncr-format\" : \"JSON\", " + " \"always-include-fqdn\" : true, " + " \"override-no-update\" : true, " + " \"override-client-update\" : true, " + " \"replace-client-name\" : \"when-present\", " + " \"generated-prefix\" : \"test.prefix\", " + " \"qualifying-suffix\" : \"test.suffix.\" }," + "\"valid-lifetime\": 4000 }"; + + ElementPtr json = Element::fromJSON(config); + ConstElementPtr status; + + CfgMgr::instance().clear(); + + EXPECT_NO_THROW(status = configureDhcp4Server(srv_, json)); + ASSERT_TRUE(status); + int rcode; + ConstElementPtr comment = config::parseAnswer(rcode, status); + EXPECT_EQ(1, rcode); +} + +// Tests invalid config with server == sender +TEST_F(Dhcp4SrvD2Test, senderEqServer) { + std::string config = + "{ \"interfaces-config\": {" + " \"interfaces\": [ \"*\" ]" + "}," + "\"rebind-timer\": 2000, " + "\"renew-timer\": 1000, " + "\"subnet4\": [ { " + " \"pools\": [ { \"pool\": \"192.0.2.1 - 192.0.2.100\" } ]," + " \"subnet\": \"192.0.2.0/24\" } ]," + " \"dhcp-ddns\" : {" + " \"enable-updates\" : true, " + " \"server-ip\" : \"127.0.0.1\", " + " \"server-port\" : 53001, " + " \"sender-ip\" : \"127.0.0.1\", " + " \"sender-port\" : 53001, " + " \"max-queue-size\" : 1024, " + " \"ncr-protocol\" : \"UDP\", " + " \"ncr-format\" : \"JSON\", " + " \"always-include-fqdn\" : true, " + " \"override-no-update\" : true, " + " \"override-client-update\" : true, " + " \"replace-client-name\" : \"when-present\", " + " \"generated-prefix\" : \"test.prefix\", " + " \"qualifying-suffix\" : \"test.suffix.\" }," + "\"valid-lifetime\": 4000 }"; + + ElementPtr json = Element::fromJSON(config); + ConstElementPtr status; + + CfgMgr::instance().clear(); + + EXPECT_NO_THROW(status = configureDhcp4Server(srv_, json)); + ASSERT_TRUE(status); + int rcode; + ConstElementPtr comment = config::parseAnswer(rcode, status); + EXPECT_EQ(1, rcode); +} } // namespace test } // namespace dhcp diff --git a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc index 4559346a17..5d9233c5c5 100644 --- a/src/lib/dhcpsrv/parsers/dhcp_parsers.cc +++ b/src/lib/dhcpsrv/parsers/dhcp_parsers.cc @@ -72,10 +72,12 @@ MACSourcesListConfigParser::parse(CfgMACSource& mac_sources, ConstElementPtr val // ******************** ControlSocketParser ************************* void ControlSocketParser::parse(SrvConfig& srv_cfg, isc::data::ConstElementPtr value) { if (!value) { + // Sanity check: not supposed to fail. isc_throw(DhcpConfigError, "Logic error: specified control-socket is null"); } if (value->getType() != Element::map) { + // Sanity check: not supposed to fail. isc_throw(DhcpConfigError, "Specified control-socket is expected to be a map" ", i.e. a structure defined within { }"); } @@ -201,6 +203,7 @@ OptionDefParser::parse(ConstElementPtr option_def) { void OptionDefListParser::parse(CfgOptionDefPtr storage, ConstElementPtr option_def_list) { if (!option_def_list) { + // Sanity check: not supposed to fail. isc_throw(DhcpConfigError, "parser error: a pointer to a list of" << " option definitions is NULL (" << option_def_list->getPosition() << ")"); @@ -355,6 +358,7 @@ PoolParser::parse(PoolStoragePtr pools, // If there's user-context specified, store it. ConstElementPtr user_context = pool_structure->get("user-context"); if (user_context) { + // The grammar accepts only maps but still check it. if (user_context->getType() != Element::map) { isc_throw(isc::dhcp::DhcpConfigError, "User context has to be a map (" << user_context->getPosition() << ")"); @@ -445,6 +449,7 @@ SubnetConfigParser::hrModeFromText(const std::string& txt) { } else if (txt.compare("all") == 0) { return (Network::HR_ALL); } else { + // Should never happen... isc_throw(BadValue, "Can't convert '" << txt << "' into any valid reservation-mode values"); } @@ -502,6 +507,7 @@ SubnetConfigParser::createSubnet(ConstElementPtr params) { // If there's user-context specified, store it. ConstElementPtr user_context = params->get("user-context"); if (user_context) { + // The grammar accepts only maps but still check it. if (user_context->getType() != Element::map) { isc_throw(isc::dhcp::DhcpConfigError, "User context has to be a map (" << user_context->getPosition() << ")"); @@ -529,6 +535,7 @@ Subnet4ConfigParser::parse(ConstElementPtr subnet) { SubnetPtr generic = SubnetConfigParser::parse(subnet); if (!generic) { + // Sanity check: not supposed to fail. isc_throw(DhcpConfigError, "Failed to create an IPv4 subnet (" << subnet->getPosition() << ")"); @@ -875,6 +882,7 @@ Subnet6ConfigParser::parse(ConstElementPtr subnet) { SubnetPtr generic = SubnetConfigParser::parse(subnet); if (!generic) { + // Sanity check: not supposed to fail. isc_throw(DhcpConfigError, "Failed to create an IPv6 subnet (" << subnet->getPosition() << ")"); diff --git a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc index cf8ffba25f..149c57b92e 100644 --- a/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc +++ b/src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc @@ -677,6 +677,24 @@ TEST_F(ParseConfigTest, defaultSpaceOptionDefTest) { cfg.runCfgOptionsTest(family_, config); } +/// @brief Check parsing of option definitions using invalid space fails. +TEST_F(ParseConfigTest, badSpaceOptionDefTest) { + + // Configuration string. + std::string config = + "{ \"option-def\": [ {" + " \"name\": \"foo\"," + " \"code\": 100000," + " \"type\": \"ipv6-address\"," + " \"space\": \"-1\"" + " } ]" + "}"; + + // Verify that the configuration string does not parse. + int rcode = parseConfiguration(config, true); + ASSERT_NE(0, rcode); +} + /// @brief Check basic parsing of options. /// /// Note that this tests basic operation of the OptionDataListParser and @@ -761,6 +779,23 @@ TEST_F(ParseConfigTest, minimalOptionDataTest) { cfg.runCfgOptionsTest(family_, expected); } +/// @brief Check parsing of options with invalid space fails. +TEST_F(ParseConfigTest, badSpaceOptionDataTest) { + + // Configuration string. + std::string config = + "{ \"option-data\": [ {" + " \"code\": 100," + " \"data\": \"01\"," + " \"space\": \"-1\"" + " } ]" + "}"; + + // Verify that the configuration string does not parse. + int rcode = parseConfiguration(config, true); + ASSERT_NE(0, rcode); +} + /// @brief Check parsing of options with escape characters. /// /// Note that this tests basic operation of the OptionDataListParser and