From: Marcin Siodelski Date: Tue, 16 May 2023 15:37:21 +0000 (+0200) Subject: [#2860] Fix in delegated prefix permutation X-Git-Tag: Kea-2.3.8~90 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=990298b23cd353740645b7d87fba8ebe90d65ab0;p=thirdparty%2Fkea.git [#2860] Fix in delegated prefix permutation The random device wasn't properly initialized for the prefix permuations. As a result, the same set of prefixes have been handed out by the random and FLQ allocators. This issue has been addressed and the proper unit tests have been added. --- diff --git a/src/lib/dhcpsrv/ip_range_permutation.cc b/src/lib/dhcpsrv/ip_range_permutation.cc index 3ffbc9c61c..183ce96683 100644 --- a/src/lib/dhcpsrv/ip_range_permutation.cc +++ b/src/lib/dhcpsrv/ip_range_permutation.cc @@ -26,6 +26,8 @@ IPRangePermutation::IPRangePermutation(const PrefixRange& range) : range_start_(range.start_), step_(static_cast(1) << (128 - range.delegated_length_)), cursor_(prefixesInRange(range.prefix_length_, range.delegated_length_) - 1), initial_cursor_(cursor_), state_(), done_(false), generator_() { + std::random_device rd; + generator_.seed(rd()); } IOAddress diff --git a/src/lib/dhcpsrv/tests/ip_range_permutation_unittest.cc b/src/lib/dhcpsrv/tests/ip_range_permutation_unittest.cc index 9285f063ba..544c63bc77 100644 --- a/src/lib/dhcpsrv/tests/ip_range_permutation_unittest.cc +++ b/src/lib/dhcpsrv/tests/ip_range_permutation_unittest.cc @@ -10,6 +10,7 @@ #include #include +#include using namespace isc; using namespace isc::asiolink; @@ -31,110 +32,171 @@ TEST(IPRangePermutationTest, constructor) { } // This test verifies that a permutation of IPv4 address range can -// be generated. +// be generated and each time a different permutation is generated. TEST(IPRangePermutationTest, ipv4) { // Create address range with 91 addresses. AddressRange range(IOAddress("192.0.2.10"), IOAddress("192.0.2.100")); - IPRangePermutation perm(range); - - // This set will record unique IP addresses generated. - std::set addrs; - bool done = false; - // Call the next() function 95 times. The first 91 calls should return non-zero - // IP addresses. - for (auto i = 0; i < 95; ++i) { - auto next = perm.next(done); - if (!next.isV4Zero()) { - // Make sure the returned address is within the range. - EXPECT_LE(range.start_, next); - EXPECT_LE(next, range.end_); + std::vector> iterations; + for (auto i = 0; i < 2; ++i) { + IPRangePermutation perm(range); + // This set will record unique IP addresses generated. + std::set addrs; + // This vector will record the addresses assignment order. + std::vector ordered_addrs; + bool done = false; + // Call the next() function 95 times. The first 91 calls should return non-zero + // IP addresses. + for (auto i = 0; i < 95; ++i) { + auto next = perm.next(done); + if (!next.isV4Zero()) { + // Make sure the returned address is within the range. + EXPECT_LE(range.start_, next); + EXPECT_LE(next, range.end_); + } + // If we went over all addresses in the range, the flags indicating that + // the permutation is exhausted should be set to true. + if (i >= 90) { + EXPECT_TRUE(done); + EXPECT_TRUE(perm.exhausted()); + } else { + // We're not done yet, so these flag should still be false. + EXPECT_FALSE(done); + EXPECT_FALSE(perm.exhausted()); + } + // Insert the address returned to the set and vector. + addrs.insert(next); + ordered_addrs.push_back(next); } - // If we went over all addresses in the range, the flags indicating that - // the permutation is exhausted should be set to true. - if (i >= 90) { - EXPECT_TRUE(done); - EXPECT_TRUE(perm.exhausted()); - } else { - // We're not done yet, so these flag should still be false. - EXPECT_FALSE(done); - EXPECT_FALSE(perm.exhausted()); + + // We should have recorded 92 unique addresses, including the zero address. + EXPECT_EQ(92, addrs.size()); + EXPECT_TRUE(addrs.begin()->isV4Zero()); + + iterations.push_back(ordered_addrs); + } + + int overlaps = 0; + for (auto i = 0; i < iterations[0].size(); ++i) { + if (iterations[0][i] == iterations[1][i]) { + ++overlaps; } - // Insert the address returned to the set. - addrs.insert(next); } - // We should have recorded 92 unique addresses, including the zero address. - EXPECT_EQ(92, addrs.size()); - EXPECT_TRUE(addrs.begin()->isV4Zero()); + EXPECT_LE(overlaps, iterations[0].size()/5) + << "The number of overlapping random address between the test two iterations" + << " is greater than 20% of all allocated addresses in each iteration." + << " It means that the permutation mechanism does not sufficiently randomize" + << " addresses. Perhaps the randomization device is not properly initialized?"; } // This test verifies that a permutation of IPv6 address range can -// be generated. +// be generated and each time a different permutation is generated. TEST(IPRangePermutationTest, ipv6) { AddressRange range(IOAddress("2001:db8:1::1:fea0"), IOAddress("2001:db8:1::2:abcd")); - IPRangePermutation perm(range); - std::set addrs; - bool done = false; - for (auto i = 0; i < 44335; ++i) { - auto next = perm.next(done); - if (!next.isV6Zero()) { - // Make sure that the address is within the range. - EXPECT_LE(range.start_, next); - EXPECT_LE(next, range.end_); + std::vector> iterations; + for (auto i = 0; i < 2; ++i) { + IPRangePermutation perm(range); + std::set addrs; + std::vector ordered_addrs; + bool done = false; + for (auto i = 0; i < 44335; ++i) { + auto next = perm.next(done); + if (!next.isV6Zero()) { + // Make sure that the address is within the range. + EXPECT_LE(range.start_, next); + EXPECT_LE(next, range.end_); + } + // If we went over all addresses in the range, the flags indicating that + // the permutation is exhausted should be set to true. + if (i >= 44333) { + EXPECT_TRUE(done); + EXPECT_TRUE(perm.exhausted()); + } else { + // We're not done yet, so these flag should still be false. + EXPECT_FALSE(done); + EXPECT_FALSE(perm.exhausted()); + } + // Insert the address returned to the set and vector. + addrs.insert(next); + ordered_addrs.push_back(next); } - // If we went over all addresses in the range, the flags indicating that - // the permutation is exhausted should be set to true. - if (i >= 44333) { - EXPECT_TRUE(done); - EXPECT_TRUE(perm.exhausted()); - } else { - // We're not done yet, so these flag should still be false. - EXPECT_FALSE(done); - EXPECT_FALSE(perm.exhausted()); + // We should have recorded 44335 unique addresses, including the zero address. + EXPECT_EQ(44335, addrs.size()); + EXPECT_TRUE(addrs.begin()->isV6Zero()); + + iterations.push_back(ordered_addrs); + } + + int overlaps = 0; + for (auto i = 0; i < iterations[0].size(); ++i) { + if (iterations[0][i] == iterations[1][i]) { + ++overlaps; } - // Insert the address returned to the set. - addrs.insert(next); } - // We should have recorded 44335 unique addresses, including the zero address. - EXPECT_EQ(44335, addrs.size()); - EXPECT_TRUE(addrs.begin()->isV6Zero()); + + EXPECT_LE(overlaps, iterations[0].size()/5) + << "The number of overlapping random address between the test two iterations" + << " is greater than 20% of all allocated addresses in each iteration." + << " It means that the permutation mechanism does not sufficiently randomize" + << " addresses. Perhaps the randomization device is not properly initialized?"; } // This test verifies that a permutation of delegated prefixes can be -// generated. +// generated and each time a different permutation is generated. TEST(IPRangePermutationTest, pd) { PrefixRange range(IOAddress("3000::"), 112, 120); - IPRangePermutation perm(range); - std::set addrs; - bool done = false; - for (auto i = 0; i < 257; ++i) { - auto next = perm.next(done); - if (!next.isV6Zero()) { - // Make sure the prefix is within the range. - EXPECT_LE(range.start_, next); - EXPECT_LE(next, range.end_); + std::vector> iterations; + for (auto i = 0; i < 2; ++i) { + IPRangePermutation perm(range); + std::set addrs; + std::vector ordered_addrs; + bool done = false; + for (auto i = 0; i < 257; ++i) { + auto next = perm.next(done); + if (!next.isV6Zero()) { + // Make sure the prefix is within the range. + EXPECT_LE(range.start_, next); + EXPECT_LE(next, range.end_); + } + // If we went over all delegated prefixes in the range, the flags indicating + // that the permutation is exhausted should be set to true. + if (i >= 255) { + EXPECT_TRUE(done); + EXPECT_TRUE(perm.exhausted()); + } else { + // We're not done yet, so these flag should still be false. + EXPECT_FALSE(done); + EXPECT_FALSE(perm.exhausted()); + } + // Insert the prefix returned to the set and vector. + addrs.insert(next); + ordered_addrs.push_back(next); } - // If we went over all delegated prefixes in the range, the flags indicating - // that the permutation is exhausted should be set to true. - if (i >= 255) { - EXPECT_TRUE(done); - EXPECT_TRUE(perm.exhausted()); - } else { - // We're not done yet, so these flag should still be false. - EXPECT_FALSE(done); - EXPECT_FALSE(perm.exhausted()); + // We should have recorded 257 unique addresses, including the zero address. + EXPECT_EQ(257, addrs.size()); + EXPECT_TRUE(addrs.begin()->isV6Zero()); + + iterations.push_back(ordered_addrs); + } + + ASSERT_EQ(2, iterations.size()); + + int overlaps = 0; + for (auto i = 0; i < iterations[0].size(); ++i) { + if (iterations[0][i] == iterations[1][i]) { + ++overlaps; } - // Insert the prefix returned to the set. - addrs.insert(next); } - // We should have recorded 257 unique addresses, including the zero address. - EXPECT_EQ(257, addrs.size()); - EXPECT_TRUE(addrs.begin()->isV6Zero()); + EXPECT_LE(overlaps, iterations[0].size()/5) + << "The number of overlapping random prefixes between the test two iterations" + << " is greater than 20% of all allocated addresses in each iteration." + << " It means that the permutation mechanism does not sufficiently randomize" + << " prefixes. Perhaps the randomization device is not properly initialized?"; } // This test verifies that a permutation of delegated prefixes is