[#4545] Made iaprefix unpack stricter

diff --git a/changelog_unreleased/4545-iaprefix-stricter-unpack b/changelog_unreleased/4545-iaprefix-stricter-unpack
new file mode 100644 (file)
index 0000000..b8ac61a
--- /dev/null
+++ b/changelog_unreleased/4545-iaprefix-stricter-unpack
@@ -0,0 +1,6 @@
+[func]         fdupont
+       Made the parsing of the the iaprefix option stricter:
+       now it refuses too bug prefix length values. Extended
+       the "lenient-option-parsing" compatibility flag to
+       replace such too big values by 128, the maximal one.
+       (Gitlab #4545)

diff --git a/doc/sphinx/arm/dhcp6-srv.rst b/doc/sphinx/arm/dhcp6-srv.rst
index d4ad8b8aa62ff17be14ce19615f135fa6f5179aa..9e283a63ecf5bcf3349cbf8c46d00c0d0c5b9cda 100644 (file)
--- a/doc/sphinx/arm/dhcp6-srv.rst
+++ b/doc/sphinx/arm/dhcp6-srv.rst
@@ -8768,6 +8768,9 @@ client-fqdn (39) options with some invalid domain names, and starting with Kea
 version 3.1.9 to fix invalid flags, i.e. when 'S' and 'N' flags set to 1
 the 'N' flag is reset to 0 for compatibility with ISC DHCP behavior.
 
+Starting with Kea verion 3.3.0, the parsing of the iaprefix option replaces
+too big prefix length by its maximal value 128.
+
 .. _dhcp6_allocation_strategies:
 
 Allocation Strategies in DHCPv6

diff --git a/src/lib/dhcp/option6_iaprefix.cc b/src/lib/dhcp/option6_iaprefix.cc
index df38cb7c60c48aafd03085d374bcd222b7554701..aee8253ff0c9856f509a423878fe473f9041cb55 100644 (file)
--- a/src/lib/dhcp/option6_iaprefix.cc
+++ b/src/lib/dhcp/option6_iaprefix.cc
@@ -103,6 +103,16 @@ void Option6IAPrefix::unpack(OptionBuffer::const_iterator begin,
     prefix_len_ = *begin;
     begin += sizeof(uint8_t);
 
+    if (prefix_len_ > 128) {
+        if (Option::lenient_parsing_) {
+            prefix_len_ = 128;
+        } else {
+            isc_throw(BadValue, static_cast<unsigned>(prefix_len_)
+                      << " is not a valid prefix length. "
+                      << "Allowed range is 0..128");
+        }
+    }
+
     // 16 bytes: IPv6 address
     OptionBuffer address_with_mask;
     mask(begin, begin + V6ADDRESS_LEN, prefix_len_, address_with_mask);
@@ -159,6 +169,5 @@ Option6IAPrefix::mask(OptionBuffer::const_iterator begin,
     }
 }
 
-
 } // end of namespace isc::dhcp
 } // end of namespace isc

diff --git a/src/lib/dhcp/tests/option6_iaprefix_unittest.cc b/src/lib/dhcp/tests/option6_iaprefix_unittest.cc
index 62684e1ac8db2d9f0e6ebe2e5b8f29853e0ccb07..08ed105ab4fe7d642f7472f2a2f531986b1d883e 100644 (file)
--- a/src/lib/dhcp/tests/option6_iaprefix_unittest.cc
+++ b/src/lib/dhcp/tests/option6_iaprefix_unittest.cc
@@ -27,6 +27,18 @@ using namespace isc::util;
 using namespace isc::asiolink;
 
 namespace {
+// RAII device to make sure that lenient parsing flag is reset to false on exit.
+class LenientOptionParsing {
+public:
+    LenientOptionParsing(bool value) {
+        Option::lenient_parsing_ = value;
+    }
+
+    ~LenientOptionParsing() {
+        Option::lenient_parsing_ = false;
+    }
+};
+
 class Option6IAPrefixTest : public ::testing::Test {
 public:
     Option6IAPrefixTest() : buf_(255), out_buf_(255) {
@@ -263,12 +275,29 @@ TEST_F(Option6IAPrefixTest, build) {
 // This test verifies that invalid prefix length is not accepted.
 TEST_F(Option6IAPrefixTest, constructorInvalidPrefixLength) {
     boost::scoped_ptr<Option6IAPrefix> opt;
-    setExampleBuffer();
 
     ASSERT_THROW(opt.reset(new Option6IAPrefix(12345,
-                 IOAddress("2001:db8:1:0:afaf:0:dead:beef"), 77,
-                                               1000, 3000000000u)), BadValue);
+                     IOAddress("2001:db8:1:0:afaf:0:dead:beef"), 77,
+                     1000, 3000000000u)), BadValue);
     ASSERT_FALSE(opt);
+
+    ASSERT_THROW(opt.reset(new Option6IAPrefix(12345,
+                     IOAddress("2001:db8:1:0:afaf:0:dead:beef"), 200,
+                     1000, 3000000000u)), BadValue);
+    ASSERT_FALSE(opt);
+
+    setExampleBuffer();
+    buf_[8] = 200;
+
+    ASSERT_THROW(opt.reset(new Option6IAPrefix(D6O_IAPREFIX,
+                     buf_.begin(), buf_.end())), BadValue);
+
+    // Lenient parsing accepts and fixes it.
+    LenientOptionParsing lop(true);
+    ASSERT_NO_THROW(opt.reset(new Option6IAPrefix(D6O_IAPREFIX,
+                        buf_.begin(), buf_.end())));
+    ASSERT_TRUE(opt);
+    EXPECT_EQ(128, opt->getLength());
 }
 
 // Checks negative cases