Browse Source

[4498] Removed server specific function for parsing DHCPv6 options.

Marcin Siodelski 9 years ago
parent
commit
be3e5317ec

+ 6 - 121
src/bin/dhcp6/dhcp6_srv.cc

@@ -461,15 +461,6 @@ void Dhcpv6Srv::run_one() {
 
 void
 Dhcpv6Srv::processPacket(Pkt6Ptr& query, Pkt6Ptr& rsp) {
-    // In order to parse the DHCP options, the server needs to use some
-    // configuration information such as: existing option spaces, option
-    // definitions etc. This is the kind of information which is not
-    // available in the libdhcp, so we need to supply our own implementation
-    // of the option parsing function here, which would rely on the
-    // configuration data.
-    query->setCallback(boost::bind(&Dhcpv6Srv::unpackOptions, this, _1, _2,
-                                   _3, _4, _5));
-
     bool skip_unpack = false;
 
     // The packet has just been received so contains the uninterpreted wire
@@ -1596,11 +1587,9 @@ Dhcpv6Srv::extendIA_NA(const Pkt6Ptr& query, const Pkt6Ptr& answer,
         Option6IAAddrPtr iaaddr = boost::dynamic_pointer_cast<Option6IAAddr>(it->second);
         if (!iaaddr) {
             // That's weird. Option code was ok, but the object type was not.
-            // As we use Dhcpv6Srv::unpackOptions() that is guaranteed to use
-            // Option6IAAddr for D6O_IAADDR, this should never happen. The only
-            // case would be with badly mis-implemented hook libraries that
-            // insert invalid option objects. There's no way to protect against
-            // this.
+            // This should never happen. The only case would be with badly
+            // mis-implemented hook libraries that insert invalid option objects.
+            // There's no way to protect against this.
             continue;
         }
         ctx.hints_.push_back(make_pair(iaaddr->getAddress(), 128));
@@ -1750,11 +1739,9 @@ Dhcpv6Srv::extendIA_PD(const Pkt6Ptr& query,
         Option6IAPrefixPtr prf = boost::dynamic_pointer_cast<Option6IAPrefix>(it->second);
         if (!prf) {
             // That's weird. Option code was ok, but the object type was not.
-            // As we use Dhcpv6Srv::unpackOptions() that is guaranteed to use
-            // Option6IAPrefix for D6O_IAPREFIX, this should never happen. The only
-            // case would be with badly mis-implemented hook libraries that
-            // insert invalid option objects. There's no way to protect against
-            // this.
+            // This should never happen. The only case would be with badly
+            // mis-implemented hook libraries that insert invalid option objects.
+            // There's no way to protect against this.
             continue;
         }
 
@@ -2771,108 +2758,6 @@ Dhcpv6Srv::processInfRequest(const Pkt6Ptr& inf_request) {
     return (reply);
 }
 
-size_t
-Dhcpv6Srv::unpackOptions(const OptionBuffer& buf,
-                         const std::string& option_space,
-                         isc::dhcp::OptionCollection& options,
-                         size_t* relay_msg_offset,
-                         size_t* relay_msg_len) {
-    size_t offset = 0;
-    size_t length = buf.size();
-
-    OptionDefContainer option_defs;
-    if (option_space == "dhcp6") {
-        // Get the list of standard option definitions.
-        option_defs = LibDHCP::getOptionDefs(Option::V6);
-    } else if (!option_space.empty()) {
-        OptionDefContainerPtr option_defs_ptr =
-            CfgMgr::instance().getCurrentCfg()->getCfgOptionDef()->
-            getAll(option_space);
-        if (option_defs_ptr != NULL) {
-            option_defs = *option_defs_ptr;
-        }
-    }
-
-    // Get the search index #1. It allows to search for option definitions
-    // using option code.
-    const OptionDefContainerTypeIndex& idx = option_defs.get<1>();
-
-    // The buffer being read comprises a set of options, each starting with
-    // a two-byte type code and a two-byte length field.
-    while (offset + 4 <= length) {
-        // At this point, from the while condition, we know that there
-        // are at least 4 bytes available following offset in the
-        // buffer.
-        uint16_t opt_type = isc::util::readUint16(&buf[offset], 2);
-        offset += 2;
-
-        uint16_t opt_len = isc::util::readUint16(&buf[offset], 2);
-        offset += 2;
-
-        if (offset + opt_len > length) {
-            // @todo: consider throwing exception here.
-
-            // We peeked at the option header of the next option, but discovered
-            // that it would end up beyond buffer end, so the option is
-            // truncated. Hence we can't parse it. Therefore we revert
-            // by by those four bytes (as if we never parsed them).
-            return (offset - 4);
-        }
-
-        if (opt_type == D6O_RELAY_MSG && relay_msg_offset && relay_msg_len) {
-            // remember offset of the beginning of the relay-msg option
-            *relay_msg_offset = offset;
-            *relay_msg_len = opt_len;
-
-            // do not create that relay-msg option
-            offset += opt_len;
-            continue;
-        }
-
-        // Get all definitions with the particular option code. Note that option
-        // code is non-unique within this container however at this point we
-        // expect to get one option definition with the particular code. If more
-        // are returned we report an error.
-        const OptionDefContainerTypeRange& range = idx.equal_range(opt_type);
-        // Get the number of returned option definitions for the option code.
-        size_t num_defs = distance(range.first, range.second);
-
-        OptionPtr opt;
-        if (num_defs > 1) {
-            // Multiple options of the same code are not supported right now!
-            isc_throw(isc::Unexpected, "Internal error: multiple option definitions"
-                      " for option type " << opt_type << " returned. Currently it is not"
-                      " supported to initialize multiple option definitions"
-                      " for the same option code. This will be supported once"
-                      " support for option spaces is implemented");
-        } else if (num_defs == 0) {
-            // @todo Don't crash if definition does not exist because only a few
-            // option definitions are initialized right now. In the future
-            // we will initialize definitions for all options and we will
-            // remove this elseif. For now, return generic option.
-            opt = OptionPtr(new Option(Option::V6, opt_type,
-                                       buf.begin() + offset,
-                                       buf.begin() + offset + opt_len));
-            opt->setEncapsulatedSpace("dhcp6");
-        } else {
-            // The option definition has been found. Use it to create
-            // the option instance from the provided buffer chunk.
-            const OptionDefinitionPtr& def = *(range.first);
-            assert(def);
-            opt = def->optionFactory(Option::V6, opt_type,
-                                     buf.begin() + offset,
-                                     buf.begin() + offset + opt_len,
-                                     boost::bind(&Dhcpv6Srv::unpackOptions, this, _1, _2,
-                                                 _3, _4, _5));
-        }
-        // add option to options
-        options.insert(std::make_pair(opt_type, opt));
-        offset += opt_len;
-    }
-
-    return (offset);
-}
-
 void Dhcpv6Srv::classifyByVendor(const Pkt6Ptr& pkt, std::string& classes) {
     OptionVendorClassPtr vclass = boost::dynamic_pointer_cast<
         OptionVendorClass>(pkt->getOption(D6O_VENDOR_CLASS));

+ 0 - 18
src/bin/dhcp6/dhcp6_srv.h

@@ -607,24 +607,6 @@ protected:
     /// simulates transmission of a packet. For that purpose it is protected.
     virtual void sendPacket(const Pkt6Ptr& pkt);
 
-    /// @brief Implements a callback function to parse options in the message.
-    ///
-    /// @param buf a A buffer holding options in on-wire format.
-    /// @param option_space A name of the option space which holds definitions
-    /// of to be used to parse options in the packets.
-    /// @param [out] options A reference to the collection where parsed options
-    /// will be stored.
-    /// @param relay_msg_offset Reference to a size_t structure. If specified,
-    /// offset to beginning of relay_msg option will be stored in it.
-    /// @param relay_msg_len reference to a size_t structure. If specified,
-    /// length of the relay_msg option will be stored in it.
-    /// @return An offset to the first byte after last parsed option.
-    size_t unpackOptions(const OptionBuffer& buf,
-                         const std::string& option_space,
-                         isc::dhcp::OptionCollection& options,
-                         size_t* relay_msg_offset,
-                         size_t* relay_msg_len);
-
     /// @brief Assigns incoming packet to zero or more classes.
     ///
     /// @note This is done in two phases: first the content of the

+ 0 - 73
src/bin/dhcp6/tests/dhcp6_srv_unittest.cc

@@ -1721,79 +1721,6 @@ TEST_F(Dhcpv6SrvTest, vendorOptionsDocsisDefinitions) {
     ASSERT_EQ(0, rcode_);
 }
 
-// This test verifies that the following option structure can be parsed:
-// - option (option space 'foobar')
-//   - sub option (option space 'foo')
-//      - sub option (option space 'bar')
-TEST_F(Dhcpv6SrvTest, unpackOptions) {
-    // Create option definition for each level of encapsulation. Each option
-    // definition is for the option code 1. Options may have the same
-    // option code because they belong to different option spaces.
-
-    // Top level option encapsulates options which belong to 'space-foo'.
-    OptionDefinitionPtr opt_def(new OptionDefinition("option-foobar", 1, "uint32",
-                                                      "space-foo"));\
-    // Middle option encapsulates options which belong to 'space-bar'
-    OptionDefinitionPtr opt_def2(new OptionDefinition("option-foo", 1, "uint16",
-                                                      "space-bar"));
-    // Low level option doesn't encapsulate any option space.
-    OptionDefinitionPtr opt_def3(new OptionDefinition("option-bar", 1,
-                                                      "uint8"));
-
-    // Add option definitions to the Configuration Manager. Each goes under
-    // different option space.
-    CfgOptionDefPtr cfg_option_def =
-        CfgMgr::instance().getStagingCfg()->getCfgOptionDef();
-    ASSERT_NO_THROW(cfg_option_def->add(opt_def, "space-foobar"));
-    ASSERT_NO_THROW(cfg_option_def->add(opt_def2, "space-foo"));
-    ASSERT_NO_THROW(cfg_option_def->add(opt_def3, "space-bar"));
-    CfgMgr::instance().commit();
-
-    // Create the buffer holding the structure of options.
-    const char raw_data[] = {
-        // First option starts here.
-        0x00, 0x01,   // option code = 1
-        0x00, 0x0F,   // option length = 15
-        0x00, 0x01, 0x02, 0x03, // This option carries uint32 value
-        // Sub option starts here.
-        0x00, 0x01,  // option code = 1
-        0x00, 0x07,  // option length = 7
-        0x01, 0x02,  // this option carries uint16 value
-        // Last option starts here.
-        0x00, 0x01,  // option code = 1
-        0x00, 0x01,  // option length = 1
-        0x00 // This option carries a single uint8 value and has no sub options.
-    };
-    OptionBuffer buf(raw_data, raw_data + sizeof(raw_data));
-
-    // Parse options.
-    NakedDhcpv6Srv srv(0);
-    OptionCollection options;
-    ASSERT_NO_THROW(srv.unpackOptions(buf, "space-foobar", options, 0, 0));
-
-    // There should be one top level option.
-    ASSERT_EQ(1, options.size());
-    boost::shared_ptr<OptionInt<uint32_t> > option_foobar =
-        boost::dynamic_pointer_cast<OptionInt<uint32_t> >(options.begin()->
-                                                          second);
-    ASSERT_TRUE(option_foobar);
-    EXPECT_EQ(1, option_foobar->getType());
-    EXPECT_EQ(0x00010203, option_foobar->getValue());
-    // There should be a middle level option held in option_foobar.
-    boost::shared_ptr<OptionInt<uint16_t> > option_foo =
-        boost::dynamic_pointer_cast<OptionInt<uint16_t> >(option_foobar->
-                                                          getOption(1));
-    ASSERT_TRUE(option_foo);
-    EXPECT_EQ(1, option_foo->getType());
-    EXPECT_EQ(0x0102, option_foo->getValue());
-    // Finally, there should be a low level option under option_foo.
-    boost::shared_ptr<OptionInt<uint8_t> > option_bar =
-        boost::dynamic_pointer_cast<OptionInt<uint8_t> >(option_foo->getOption(1));
-    ASSERT_TRUE(option_bar);
-    EXPECT_EQ(1, option_bar->getType());
-    EXPECT_EQ(0x0, option_bar->getValue());
-}
-
 // Checks if DOCSIS client packets are classified properly
 TEST_F(Dhcpv6SrvTest, docsisClientClassification) {
 

+ 1 - 2
src/bin/dhcp6/tests/dhcp6_test_utils.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2013-2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013-2016 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
@@ -127,7 +127,6 @@ public:
     using Dhcpv6Srv::testUnicast;
     using Dhcpv6Srv::sanityCheck;
     using Dhcpv6Srv::classifyPacket;
-    using Dhcpv6Srv::unpackOptions;
     using Dhcpv6Srv::shutdown_;
     using Dhcpv6Srv::name_change_reqs_;
     using Dhcpv6Srv::VENDOR_CLASS_PREFIX;

+ 71 - 0
src/lib/dhcp/tests/libdhcp++_unittest.cc

@@ -516,6 +516,77 @@ TEST_F(LibDhcpTest, unpackOptions6) {
     EXPECT_TRUE(x == options.end()); // option 32000 not found */
 }
 
+// This test verifies that the following option structure can be parsed:
+// - option (option space 'foobar')
+//   - sub option (option space 'foo')
+//      - sub option (option space 'bar')
+TEST_F(LibDhcpTest, unpackSubOptions6) {
+    // Create option definition for each level of encapsulation. Each option
+    // definition is for the option code 1. Options may have the same
+    // option code because they belong to different option spaces.
+
+    // Top level option encapsulates options which belong to 'space-foo'.
+    OptionDefinitionPtr opt_def(new OptionDefinition("option-foobar", 1, "uint32",
+                                                      "space-foo"));\
+    // Middle option encapsulates options which belong to 'space-bar'
+    OptionDefinitionPtr opt_def2(new OptionDefinition("option-foo", 1, "uint16",
+                                                      "space-bar"));
+    // Low level option doesn't encapsulate any option space.
+    OptionDefinitionPtr opt_def3(new OptionDefinition("option-bar", 1,
+                                                      "uint8"));
+
+    // Register created option definitions as runtime option definitions.
+    OptionDefSpaceContainer defs;
+    ASSERT_NO_THROW(defs.addItem(opt_def, "space-foobar"));
+    ASSERT_NO_THROW(defs.addItem(opt_def2, "space-foo"));
+    ASSERT_NO_THROW(defs.addItem(opt_def3, "space-bar"));
+    LibDHCP::setRuntimeOptionDefs(defs);
+    LibDHCP::commitRuntimeOptionDefs();
+
+    // Create the buffer holding the structure of options.
+    const char raw_data[] = {
+        // First option starts here.
+        0x00, 0x01,   // option code = 1
+        0x00, 0x0F,   // option length = 15
+        0x00, 0x01, 0x02, 0x03, // This option carries uint32 value
+        // Sub option starts here.
+        0x00, 0x01,  // option code = 1
+        0x00, 0x07,  // option length = 7
+        0x01, 0x02,  // this option carries uint16 value
+        // Last option starts here.
+        0x00, 0x01,  // option code = 1
+        0x00, 0x01,  // option length = 1
+        0x00 // This option carries a single uint8 value and has no sub options.
+    };
+    OptionBuffer buf(raw_data, raw_data + sizeof(raw_data));
+
+    // Parse options.
+    OptionCollection options;
+    ASSERT_NO_THROW(LibDHCP::unpackOptions6(buf, "space-foobar", options, 0, 0));
+
+    // There should be one top level option.
+    ASSERT_EQ(1, options.size());
+    boost::shared_ptr<OptionInt<uint32_t> > option_foobar =
+        boost::dynamic_pointer_cast<OptionInt<uint32_t> >(options.begin()->
+                                                          second);
+    ASSERT_TRUE(option_foobar);
+    EXPECT_EQ(1, option_foobar->getType());
+    EXPECT_EQ(0x00010203, option_foobar->getValue());
+    // There should be a middle level option held in option_foobar.
+    boost::shared_ptr<OptionInt<uint16_t> > option_foo =
+        boost::dynamic_pointer_cast<OptionInt<uint16_t> >(option_foobar->
+                                                          getOption(1));
+    ASSERT_TRUE(option_foo);
+    EXPECT_EQ(1, option_foo->getType());
+    EXPECT_EQ(0x0102, option_foo->getValue());
+    // Finally, there should be a low level option under option_foo.
+    boost::shared_ptr<OptionInt<uint8_t> > option_bar =
+        boost::dynamic_pointer_cast<OptionInt<uint8_t> >(option_foo->getOption(1));
+    ASSERT_TRUE(option_bar);
+    EXPECT_EQ(1, option_bar->getType());
+    EXPECT_EQ(0x0, option_bar->getValue());
+}
+
 /// V4 Options being used to test pack/unpack operations.
 /// These are variable length options only so as there
 /// is no restriction on the data length being carried by them.