From: Thomas Markwalder Date: Tue, 4 Feb 2020 16:08:16 +0000 (-0500) Subject: [#1111] Option 43 work-around for Cisco Meraki router Clients X-Git-Tag: Kea-1.6.2~3 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3c52309e06e7dd1cc1d7fceb8f189bc720a106dd;p=thirdparty%2Fkea.git [#1111] Option 43 work-around for Cisco Meraki router Clients Backport #950,!661 changes to v1_6_0 modified: ChangeLog doc/sphinx/arm/dhcp4-srv.rst src/lib/dhcp/libdhcp++.cc src/lib/dhcp/libdhcp++.h src/lib/dhcp/option.cc src/lib/dhcp/pkt4.cc src/lib/dhcp/tests/libdhcp++_unittest.cc --- diff --git a/ChangeLog b/ChangeLog index 93434b2fcb..9283b2af5a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,10 @@ +1665. [bug] tmark + Modified option 43 parsing to interpret sub-option codes 0 and 255 + as PAD and END when no sub-option with these codes are defined. + This adds control of illegal but common use of these reserved + code points in option 43. + (Gitlab #1111,#950) + 1664. [bug] tmark Corrected an issue in the MySQL CB hook library which could cause subnet and shared-network options, properly added to diff --git a/doc/sphinx/arm/dhcp4-srv.rst b/doc/sphinx/arm/dhcp4-srv.rst index b44f74cc45..65e2ccadc1 100644 --- a/doc/sphinx/arm/dhcp4-srv.rst +++ b/doc/sphinx/arm/dhcp4-srv.rst @@ -2150,6 +2150,40 @@ The definition used to decode a VSI option is: matching these cases and an option definition for the VSI option with a binary type and no encapsulation. +.. note:: + + By default, in the Vendor-Specific Information option (code 43) + sub-option code 0 and 255 mean PAD and END respectively according to + `RFC 2132 `_. In other words, the + sub-option code values of 0 and 255 are reserved. Kea does, however, + allow you to define sub-option codes from 0 to 255. If you define + sub-options with codes 0 and/or 255, bytes with that value will + no longer be treated as a PAD or an END, but as the sub-option code + when parsing a VSI option in an incoming query. + + Option 43 input processing (aka unpacking) is deferred so that it + happens after classification. This means you cannot classify clients + using option 43 suboptions. The definition used to unpack option 43 + is determined as follows: + + - If defined at the global scope this definition is used + - If defined at client class scope and the packet belongs to this + class the client class definition is used + - If not defined at global scope nor in a client class to which the + packet belongs, the built-in last resort definition is used. This + definition only says the sub-option space is + "vendor-encapsulated-options-space" + + The output definition selection is a bit simpler: + + - If the packet belongs to a client class which defines the option + 43 use this definition + - If defined at the global scope use this definition + - Otherwise use the built-in last resot definition. + + Note as they use a specific/per vendor option space the sub-options + are defined at the global scope. + .. note:: Option definitions in client classes are allowed only for this diff --git a/src/lib/dhcp/libdhcp++.cc b/src/lib/dhcp/libdhcp++.cc index 6c37b77efd..d9d4759116 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 @@ -473,10 +473,12 @@ size_t LibDHCP::unpackOptions6(const OptionBuffer& buf, return (last_offset); } -size_t LibDHCP::unpackOptions4(const OptionBuffer& buf, - const std::string& option_space, - isc::dhcp::OptionCollection& options, - std::list& deferred) { +size_t +LibDHCP::unpackOptions4(const OptionBuffer& buf, + const std::string& option_space, + isc::dhcp::OptionCollection& options, + std::list& deferred, + bool flexible_pad_end) { size_t offset = 0; size_t last_offset = 0; @@ -494,6 +496,10 @@ size_t LibDHCP::unpackOptions4(const OptionBuffer& buf, const OptionDefContainerTypeIndex& idx = option_defs->get<1>(); const OptionDefContainerTypeIndex& runtime_idx = runtime_option_defs->get<1>(); + // Flexible PAD and END parsing. + bool flex_pad = (flexible_pad_end && (runtime_idx.count(DHO_PAD) == 0)); + bool flex_end = (flexible_pad_end && (runtime_idx.count(DHO_END) == 0)); + // The buffer being read comprises a set of options, each starting with // a one-byte type code and a one-byte length field. while (offset < buf.size()) { @@ -504,7 +510,9 @@ size_t 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 || flex_end)) { // 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. @@ -513,7 +521,9 @@ size_t 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 || flex_pad)) { continue; } diff --git a/src/lib/dhcp/libdhcp++.h b/src/lib/dhcp/libdhcp++.h index 2d29f1474b..4deea051d5 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-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 @@ -260,6 +260,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. /// @@ -267,7 +269,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 = false); /// 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 73fc04cbc2..bca372c16f 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 58f1656d16..eb4b34a60a 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 @@ -780,7 +780,8 @@ TEST_F(LibDhcpTest, unpackOptions4) { list deferred; ASSERT_NO_THROW( - LibDHCP::unpackOptions4(v4packed, "dhcp4", options, deferred); + LibDHCP::unpackOptions4(v4packed, DHCP4_OPTION_SPACE, options, + deferred, false); ); isc::dhcp::OptionCollection::const_iterator x = options.find(12); @@ -925,7 +926,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()); @@ -984,7 +985,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()); @@ -1058,7 +1059,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]); @@ -1098,6 +1099,179 @@ 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) because no sub-option 0 + // was defined so code 0 means PAD. + 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 END bug is fixed (#950: option code 255 was not +// parse at END, now it is not parse at END only when an option code 255 +// is defined in the corresponding option space). +TEST_F(LibDhcpTest, option43Factory) { + // 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) { @@ -1112,7 +1286,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.