From 10bd31217d8a0a77345c4cba7a59314f70c1b509 Mon Sep 17 00:00:00 2001 From: Francis Dupont Date: Thu, 13 Jun 2019 11:49:37 +0200 Subject: [PATCH] [583-cb-cmds-config-get-returns-wrong-subnet-range] Fixed prefixLengthFromRange --- src/lib/asiolink/addr_utilities.cc | 12 +++- .../asiolink/tests/addr_utilities_unittest.cc | 12 +++- src/lib/asiolink/tests/udp_socket_unittest.cc | 4 +- src/lib/dhcpsrv/pool.h | 2 +- src/lib/dhcpsrv/tests/pool_unittest.cc | 72 +++++++++++++++++++ 5 files changed, 97 insertions(+), 5 deletions(-) diff --git a/src/lib/asiolink/addr_utilities.cc b/src/lib/asiolink/addr_utilities.cc index 62a59a565b..f580782313 100644 --- a/src/lib/asiolink/addr_utilities.cc +++ b/src/lib/asiolink/addr_utilities.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2012-2018 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2012-2019 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 @@ -286,7 +286,12 @@ prefixLengthFromRange(const IOAddress& min, const IOAddress& max) { uint32_t min_numeric = min.toUint32(); // Get the exclusive or which must be one of the bit masks + // and the min must be at the beginning of the prefix + // so it does not contribute to trailing ones. uint32_t xor_numeric = max_numeric ^ min_numeric; + if ((min_numeric & ~xor_numeric) != min_numeric) { + return (-1); + } for (uint8_t prefix_len = 0; prefix_len <= 32; ++prefix_len) { if (xor_numeric == bitMask4[prefix_len]) { // Got it: the wanted value is also the index @@ -308,6 +313,11 @@ prefixLengthFromRange(const IOAddress& min, const IOAddress& max) { bool zeroes = true; for (uint8_t i = 0; i < 16; ++i) { uint8_t xor_byte = min_packed[i] ^ max_packed[i]; + // The min must be at the beginning of the prefix + // so it does not contribute to trailing ones. + if ((min_packed[i] & ~xor_byte) != min_packed[i]) { + return (-1); + } if (zeroes) { // Skipping zero bits searching for one bits if (xor_byte == 0) { diff --git a/src/lib/asiolink/tests/addr_utilities_unittest.cc b/src/lib/asiolink/tests/addr_utilities_unittest.cc index 53279a4c7f..4c6461fc83 100644 --- a/src/lib/asiolink/tests/addr_utilities_unittest.cc +++ b/src/lib/asiolink/tests/addr_utilities_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2012-2018 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2012-2019 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 @@ -266,6 +266,9 @@ TEST(AddrUtilitiesTest, prefixLengthFromRange4) { // Fail if a network boundary is crossed EXPECT_EQ(-1, plfr(IOAddress("10.0.0.255"), IOAddress("10.0.1.1"))); + // Fail if first is not at the begin + EXPECT_EQ(-1, plfr(IOAddress("10.0.0.2"), IOAddress("10.0.0.5"))); + // The upper bound cannot be smaller than the lower bound EXPECT_THROW(plfr(IOAddress("192.0.2.5"), IOAddress("192.0.2.4")), isc::BadValue); @@ -323,6 +326,13 @@ TEST(AddrUtilitiesTest, prefixLengthFromRange6) { EXPECT_EQ(-1, plfr(IOAddress("2001:db8::ffff"), IOAddress("2001:db8::1:1"))); + // Fail if first is not at the begin + EXPECT_EQ(-1, plfr(IOAddress("2001:db8::2"), IOAddress("2001:db8::5"))); + EXPECT_EQ(-1, plfr(IOAddress("2001:db8::2:0"), + IOAddress("2001:db8::5:ffff"))); + EXPECT_EQ(-1, plfr(IOAddress("2001:db8::2:ff00:0"), + IOAddress("2001:db8::3:00ff:ffff"))); + // The upper bound cannot be smaller than the lower bound EXPECT_THROW(plfr(IOAddress("fe80::5"), IOAddress("fe80::4")), isc::BadValue); diff --git a/src/lib/asiolink/tests/udp_socket_unittest.cc b/src/lib/asiolink/tests/udp_socket_unittest.cc index 1a5b2a6568..4f61200aa3 100644 --- a/src/lib/asiolink/tests/udp_socket_unittest.cc +++ b/src/lib/asiolink/tests/udp_socket_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2011-2016 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2011-2019 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 @@ -166,7 +166,7 @@ TEST(UDPSocket, processReceivedData) { // Where data is put // cppcheck-suppress variableScope size_t expected; // Expected amount of data - // cppcheck-suppress variableScope + // cppcheck-suppress variableScope size_t offset; // Where to put next data // cppcheck-suppress variableScope size_t cumulative; // Cumulative data received diff --git a/src/lib/dhcpsrv/pool.h b/src/lib/dhcpsrv/pool.h index f7cd013654..d81d2ab2c4 100644 --- a/src/lib/dhcpsrv/pool.h +++ b/src/lib/dhcpsrv/pool.h @@ -26,7 +26,7 @@ namespace dhcp { /// /// Stores information about pool of IPv4 or IPv6 addresses. /// That is a basic component of a configuration. -class Pool : public isc::data::UserContext { +class Pool : public isc::data::UserContext, public isc::data::CfgToElement { public: /// @note: diff --git a/src/lib/dhcpsrv/tests/pool_unittest.cc b/src/lib/dhcpsrv/tests/pool_unittest.cc index 494e40663b..fd3cef07e8 100644 --- a/src/lib/dhcpsrv/tests/pool_unittest.cc +++ b/src/lib/dhcpsrv/tests/pool_unittest.cc @@ -10,6 +10,7 @@ #include #include #include +#include #include @@ -120,6 +121,33 @@ TEST(Pool4Test, toText) { Pool4 pool2(IOAddress("192.0.2.128"), 28); EXPECT_EQ("type=V4, 192.0.2.128-192.0.2.143", pool2.toText()); + + Pool4 pool3(IOAddress("192.0.2.0"), IOAddress("192.0.2.127")); + EXPECT_EQ("type=V4, 192.0.2.0-192.0.2.127", pool3.toText()); +} + +// Simple check if toElement returns reasonable values +TEST(Pool4Test, toElement) { + Pool4 pool1(IOAddress("192.0.2.7"), IOAddress("192.0.2.17")); + std::string expected1 = "{" + " \"pool\": \"192.0.2.7-192.0.2.17\", " + " \"option-data\": [ ] " + "}"; + isc::test::runToElementTest(expected1, pool1); + + Pool4 pool2(IOAddress("192.0.2.128"), 28); + std::string expected2 = "{" + " \"pool\": \"192.0.2.128/28\", " + " \"option-data\": [ ] " + "}"; + isc::test::runToElementTest(expected2, pool2); + + Pool4 pool3(IOAddress("192.0.2.0"), IOAddress("192.0.2.127")); + std::string expected3 = "{" + " \"pool\": \"192.0.2.0/25\", " + " \"option-data\": [ ] " + "}"; + isc::test::runToElementTest(expected3, pool3); } // This test checks that it is possible to specify pool specific options. @@ -506,6 +534,50 @@ TEST(Pool6Test, toText) { " excluded_prefix_len=120", pool3.toText()); + Pool6 pool4(Lease::TYPE_NA, IOAddress("2001:db8::"), + IOAddress("2001:db8::ffff")); + EXPECT_EQ("type=IA_NA, 2001:db8::-2001:db8::ffff, delegated_len=128", + pool4.toText()); +} + +// Simple check if toElement returns reasonable values +TEST(Pool6Test, toElement) { + Pool6 pool1(Lease::TYPE_NA, IOAddress("2001:db8::1"), + IOAddress("2001:db8::2")); + std::string expected1 = "{" + " \"pool\": \"2001:db8::1-2001:db8::2\", " + " \"option-data\": [ ] " + "}"; + isc::test::runToElementTest(expected1, pool1); + + Pool6 pool2(Lease::TYPE_PD, IOAddress("2001:db8:1::"), 96, 112); + std::string expected2 = "{" + " \"prefix\": \"2001:db8:1::\", " + " \"prefix-len\": 96, " + " \"delegated-len\": 112, " + " \"option-data\": [ ] " + "}"; + isc::test::runToElementTest(expected2, pool2); + + Pool6 pool3(IOAddress("2001:db8:1::"), 96, 112, + IOAddress("2001:db8:1::1000"), 120); + std::string expected3 = "{" + " \"prefix\": \"2001:db8:1::\", " + " \"prefix-len\": 96, " + " \"delegated-len\": 112, " + " \"excluded-prefix\": \"2001:db8:1::1000\", " + " \"excluded-prefix-len\": 120, " + " \"option-data\": [ ] " + "}"; + isc::test::runToElementTest(expected3, pool3); + + Pool6 pool4(Lease::TYPE_NA, IOAddress("2001:db8::"), + IOAddress("2001:db8::ffff")); + std::string expected4 = "{" + " \"pool\": \"2001:db8::/112\", " + " \"option-data\": [ ] " + "}"; + isc::test::runToElementTest(expected4, pool4); } // Checks if the number of possible leases in range is reported correctly. -- 2.47.3