From: Francis Dupont Date: Sat, 12 Oct 2019 18:28:52 +0000 (+0200) Subject: [950-dhcp-option-43-pad-end] Fixed the option 43 END bug X-Git-Tag: Kea-1.7.4~96 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=aab25f42410221178960d223da5848bed0f4f550;p=thirdparty%2Fkea.git [950-dhcp-option-43-pad-end] Fixed the option 43 END bug --- diff --git a/src/lib/dhcp/libdhcp++.cc b/src/lib/dhcp/libdhcp++.cc index 6be24dbc54..61d0f9f42a 100644 --- a/src/lib/dhcp/libdhcp++.cc +++ b/src/lib/dhcp/libdhcp++.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2011-2019 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2011-2020 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 @@ -458,7 +458,8 @@ size_t LibDHCP::unpackOptions4(const OptionBuffer& buf, const std::string& option_space, isc::dhcp::OptionCollection& options, - std::list& deferred) { + std::list& deferred, + bool flexible_pad_end) { size_t offset = 0; size_t last_offset = 0; @@ -486,7 +487,11 @@ LibDHCP::unpackOptions4(const OptionBuffer& buf, uint8_t opt_type = buf[offset++]; // DHO_END is a special, one octet long option - if (space_is_dhcp4 && (opt_type == DHO_END)) { + // Valid in dhcp4 space or when flexible_pad_end is true and + // there is a sub-option configured for this code. + if ((opt_type == DHO_END) && + (space_is_dhcp4 || + (flexible_pad_end && (runtime_idx.count(DHO_END) == 0)))) { // just return. Don't need to add DHO_END option // Don't return offset because it makes this condition // and partial parsing impossible to recognize. @@ -495,7 +500,11 @@ LibDHCP::unpackOptions4(const OptionBuffer& buf, // DHO_PAD is just a padding after DHO_END. Let's continue parsing // in case we receive a message without DHO_END. - if (space_is_dhcp4 && (opt_type == DHO_PAD)) { + // Valid in dhcp4 space or when flexible_pad_end is true and + // there is a sub-option configured for this code. + if ((opt_type == DHO_PAD) && + (space_is_dhcp4 || + (flexible_pad_end && (runtime_idx.count(DHO_PAD) == 0)))) { continue; } diff --git a/src/lib/dhcp/libdhcp++.h b/src/lib/dhcp/libdhcp++.h index 1b22fb547e..469e95b42c 100644 --- a/src/lib/dhcp/libdhcp++.h +++ b/src/lib/dhcp/libdhcp++.h @@ -1,4 +1,4 @@ -// Copyright (C) 2011-2017 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 @@ -259,6 +259,8 @@ public: /// put here. /// @param deferred Reference to an option code list. Options which /// processing is deferred will be put here. + /// @param flexible_pad_end Parse options 0 and 255 as PAD and END + /// when they are not defined in the option space. /// @return offset to the first byte after the last successfully /// parsed option or the offset of the DHO_END option type. /// @@ -266,7 +268,8 @@ public: static size_t unpackOptions4(const OptionBuffer& buf, const std::string& option_space, isc::dhcp::OptionCollection& options, - std::list& deferred); + std::list& deferred, + bool flexible_pad_end); /// Registers factory method that produces options of specific option types. /// diff --git a/src/lib/dhcp/option.cc b/src/lib/dhcp/option.cc index c2485eb5ef..c404b2a878 100644 --- a/src/lib/dhcp/option.cc +++ b/src/lib/dhcp/option.cc @@ -5,8 +5,10 @@ // file, You can obtain one at http://mozilla.org/MPL/2.0/. #include +#include #include #include +#include #include #include #include @@ -165,7 +167,8 @@ Option::unpackOptions(const OptionBuffer& buf) { switch (universe_) { case V4: LibDHCP::unpackOptions4(buf, getEncapsulatedSpace(), - options_, deferred); + options_, deferred, + getType() == DHO_VENDOR_ENCAPSULATED_OPTIONS); return; case V6: LibDHCP::unpackOptions6(buf, getEncapsulatedSpace(), options_); diff --git a/src/lib/dhcp/pkt4.cc b/src/lib/dhcp/pkt4.cc index b01ec7b665..dbd2b15704 100644 --- a/src/lib/dhcp/pkt4.cc +++ b/src/lib/dhcp/pkt4.cc @@ -205,7 +205,7 @@ Pkt4::unpack() { // a vector as an input. buffer_in.readVector(opts_buffer, opts_len); - size_t offset = LibDHCP::unpackOptions4(opts_buffer, DHCP4_OPTION_SPACE, options_, deferred_options_); + size_t offset = LibDHCP::unpackOptions4(opts_buffer, DHCP4_OPTION_SPACE, options_, deferred_options_, false); // If offset is not equal to the size and there is no DHO_END, // then something is wrong here. We either parsed past input diff --git a/src/lib/dhcp/tests/libdhcp++_unittest.cc b/src/lib/dhcp/tests/libdhcp++_unittest.cc index e63c45bd54..3c8d4f2a38 100644 --- a/src/lib/dhcp/tests/libdhcp++_unittest.cc +++ b/src/lib/dhcp/tests/libdhcp++_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2011-2019 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2011-2020 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 @@ -779,7 +779,8 @@ TEST_F(LibDhcpTest, unpackOptions4) { list deferred; ASSERT_NO_THROW( - LibDHCP::unpackOptions4(v4packed, DHCP4_OPTION_SPACE, options, deferred); + LibDHCP::unpackOptions4(v4packed, DHCP4_OPTION_SPACE, options, + deferred false); ); isc::dhcp::OptionCollection::const_iterator x = options.find(12); @@ -924,7 +925,7 @@ TEST_F(LibDhcpTest, unpackEmptyOption4) { OptionCollection options; list deferred; ASSERT_NO_THROW(LibDHCP::unpackOptions4(buf, DHCP4_OPTION_SPACE, - options, deferred)); + options, deferred, false)); // There should be one option. ASSERT_EQ(1, options.size()); @@ -983,7 +984,7 @@ TEST_F(LibDhcpTest, unpackSubOptions4) { OptionCollection options; list deferred; ASSERT_NO_THROW(LibDHCP::unpackOptions4(buf, "space-foobar", - options, deferred)); + options, deferred, false)); // There should be one top level option. ASSERT_EQ(1, options.size()); @@ -1057,7 +1058,7 @@ TEST_F(LibDhcpTest, unpackPadEnd) { list deferred; size_t offset = 0; ASSERT_NO_THROW(offset = LibDHCP::unpackOptions4(buf, DHCP4_OPTION_SPACE, - options, deferred)); + options, deferred, false)); // Returned offset should point to the END. EXPECT_EQ(0xff, buf[offset]); @@ -1097,6 +1098,176 @@ TEST_F(LibDhcpTest, unpackPadEnd) { EXPECT_EQ("foo", sub->getValue()); } +// Verfies that option 0 (PAD) is handled as PAD in option 43 (so when +// flexible pad end flag is true) only when option 0 (PAD) is not defined. +TEST_F(LibDhcpTest, option43Pad) { + string space = "my-option43-space"; + + // Create option definition for option 1. + OptionDefinitionPtr opt_def1(new OptionDefinition("one", 1, "binary")); + + // Create option definition for option 2. + OptionDefinitionPtr opt_def2(new OptionDefinition("two", 2, "uint8")); + + // Register created option definitions as runtime option definitions. + OptionDefSpaceContainer defs; + ASSERT_NO_THROW(defs.addItem(opt_def1, space)); + ASSERT_NO_THROW(defs.addItem(opt_def2, space)); + LibDHCP::setRuntimeOptionDefs(defs); + LibDHCP::commitRuntimeOptionDefs(); + + // Create the buffer holding an option 43 content. + OptionBuffer buf = { + // Suboption 0, + 0x00, 0x01, 0x00, // code = 0, length = 1, content = 0 + // or option code = 0 (PAD) followed by + // code = 1, length = 0 + // Suboption 2. + 0x02, 0x01, 0x01, // code = 2, length = 1, content = 1 + }; + + // Parse options. + OptionCollection options; + list deferred; + size_t offset = 0; + ASSERT_NO_THROW(offset = LibDHCP::unpackOptions4(buf, space, + options, deferred, true)); + + // There should be 2 suboptions (1 and 2). + ASSERT_EQ(2, options.size()); + + // Get suboption 1. + OptionPtr sub1 = options.begin()->second; + ASSERT_TRUE(sub1); + EXPECT_EQ(1, sub1->getType()); + EXPECT_EQ(0, sub1->len() - sub1->getHeaderLen()); + + // Get suboption 2. + boost::shared_ptr > sub2 = + boost::dynamic_pointer_cast > + (options.rbegin()->second); + ASSERT_TRUE(sub2); + EXPECT_EQ(2, sub2->getType()); + EXPECT_EQ(1, sub2->getValue()); + + // Create option definition for option 0 and register it. + OptionDefinitionPtr opt_def0(new OptionDefinition("zero", 0, "uint8")); + ASSERT_NO_THROW(defs.addItem(opt_def0, space)); + LibDHCP::clearRuntimeOptionDefs(); + LibDHCP::setRuntimeOptionDefs(defs); + LibDHCP::commitRuntimeOptionDefs(); + + options.clear(); + offset = 0; + ASSERT_NO_THROW(offset = LibDHCP::unpackOptions4(buf, space, + options, deferred, true)); + + // There should be 2 suboptions (0 and 1). + EXPECT_EQ(2, options.size()); + + // Get suboption 0 + boost::shared_ptr > sub0 = + boost::dynamic_pointer_cast > + (options.begin()->second); + ASSERT_TRUE(sub0); + EXPECT_EQ(0, sub0->getType()); + EXPECT_EQ(0, sub0->getValue()); + + // Get suboption 2. + sub2 = + boost::dynamic_pointer_cast > + (options.rbegin()->second); + ASSERT_TRUE(sub2); + EXPECT_EQ(2, sub2->getType()); + EXPECT_EQ(1, sub2->getValue()); +} + +// Verfies that option 255 (END) is handled as END in option 43 (so when +//flexible pad end flag is true) only when option 255 (END) is not defined. +TEST_F(LibDhcpTest, option43End) { + string space = "my-option43-space"; + + // Create the buffer holding an option 43 content. + OptionBuffer buf = { + // Suboption 255, + 0xff, 0x01, 0x02 // code = 255, length = 1, content = 2 + }; + + // Parse options. + OptionCollection options; + list deferred; + size_t offset = 0; + ASSERT_NO_THROW(offset = LibDHCP::unpackOptions4(buf, space, + options, deferred, true)); + + // Parsing should stop at the first byte. + EXPECT_EQ(0, offset); + + // There should be 0 suboptions. + EXPECT_EQ(0, options.size()); + + + // Create option definition for option 255. + OptionDefinitionPtr opt_def255(new OptionDefinition("max", 255, "uint8")); + + // Register created option definition as runtime option definitions. + OptionDefSpaceContainer defs; + ASSERT_NO_THROW(defs.addItem(opt_def255, space)); + LibDHCP::setRuntimeOptionDefs(defs); + LibDHCP::commitRuntimeOptionDefs(); + + options.clear(); + offset = 0; + ASSERT_NO_THROW(offset = LibDHCP::unpackOptions4(buf, space, + options, deferred, true)); + + // There should be 1 suboption. + ASSERT_EQ(1, options.size()); + + // Get suboption 255. + boost::shared_ptr > sub255 = + boost::dynamic_pointer_cast > + (options.begin()->second); + ASSERT_TRUE(sub255); + EXPECT_EQ(255, sub255->getType()); + EXPECT_EQ(2, sub255->getValue()); +} + +// Verify the option 43 DEND bug is fixed. +TEST_F(LibDhcpTest, option32Factory) { + // Create the buffer holding the structure of option 43 content. + OptionBuffer buf = { + // Suboption 1. + 0x01, 0x00, // option code = 1, option length = 0 + // END + 0xff + }; + + // Get last resort definition. + OptionDefinitionPtr def = + LibDHCP::getLastResortOptionDef(DHCP4_OPTION_SPACE, 43); + ASSERT_TRUE(def); + + // Apply the definition. + OptionPtr option; + ASSERT_NO_THROW(option = def->optionFactory(Option::V4, 43, buf)); + ASSERT_TRUE(option); + EXPECT_EQ(DHO_VENDOR_ENCAPSULATED_OPTIONS, option->getType()); + EXPECT_EQ(def->getEncapsulatedSpace(), option->getEncapsulatedSpace()); + + // There should be 1 suboption. + EXPECT_EQ(1, option->getOptions().size()); + + // Get suboption 1. + OptionPtr sub1 = option->getOption(1); + ASSERT_TRUE(sub1); + EXPECT_EQ(1, sub1->getType()); + EXPECT_EQ(0, sub1->len() - sub1->getHeaderLen()); + + // Of course no suboption 255. + EXPECT_FALSE(option->getOption(255)); +} + // Verifies that an Host Name (option 12), will be dropped when empty, // while subsequent options will still be unpacked. TEST_F(LibDhcpTest, emptyHostName) { @@ -1111,7 +1282,7 @@ TEST_F(LibDhcpTest, emptyHostName) { list deferred; ASSERT_NO_THROW( - LibDHCP::unpackOptions4(packed, "dhcp4", options, deferred); + LibDHCP::unpackOptions4(packed, "dhcp4", options, deferred, false); ); // Host Name should not exist, we quietly drop it when empty.