[3246] Set non-significant bits of parsed prefix option to 0.

src/bin/dhcp6/dhcp6_srv.cc

@@ -1365,16 +1365,16 @@ Dhcpv6Srv::assignIA_PD(const Subnet6Ptr& subnet, const DuidPtr& duid,
     // Check if the client sent us a hint in his IA_PD. Clients may send an
     // address in their IA_NA options as a suggestion (e.g. the last address
     // they used before).
-    boost::shared_ptr<Option6IAPrefix> hintOpt =
+    boost::shared_ptr<Option6IAPrefix> hint_opt =
       boost::dynamic_pointer_cast<Option6IAPrefix>(ia->getOption(D6O_IAPREFIX));
     IOAddress hint("::");
-    if (hintOpt) {
-        hint = hintOpt->getAddress();
+    if (hint_opt) {
+        hint = hint_opt->getAddress();
     }
 
     LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL, DHCP6_PROCESS_IA_PD_REQUEST)
         .arg(duid ? duid->toText() : "(no-duid)").arg(ia->getIAID())
-        .arg(hintOpt ? hint.toText() : "(no hint)");
+        .arg(hint_opt ? hint.toText() : "(no hint)");
 
     // "Fake" allocation is processing of SOLICIT message. We pretend to do an
     // allocation, but we do not put the lease in the database. That is ok,

src/bin/dhcp6/tests/sarr_unittest.cc

@@ -79,7 +79,7 @@ TEST_F(SARRTest, directClientPrefixHint) {
     const Subnet6Collection* subnets = CfgMgr::instance().getSubnets6();
     ASSERT_EQ(1, subnets->size());
     // Append IAPREFIX option to the client's message.
-    ASSERT_NO_THROW(client.useHint(100, 200, 56, "2001:db8:3:33::33"));
+    ASSERT_NO_THROW(client.useHint(100, 200, 64, "2001:db8:3:33::33"));
     // Perform 4-way exchange.
     ASSERT_NO_THROW(client.doSARR());
     // Server should have assigned a prefix.
@@ -103,7 +103,7 @@ TEST_F(SARRTest, directClientPrefixHint) {
     client.modifyDUID();
 
     // Use the hint with some least significant bytes set.
-    ASSERT_NO_THROW(client.useHint(100, 200, 56, "2001:db8:3:33::34"));
+    ASSERT_NO_THROW(client.useHint(100, 200, 64, "2001:db8:3:33::34"));
     ASSERT_NO_THROW(client.doSARR());
     // Server should assign a lease.
     ASSERT_EQ(1, client.getLeaseNum());

src/lib/dhcp/option6_iaprefix.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2013 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013-2014 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -86,7 +86,9 @@ void Option6IAPrefix::unpack(OptionBuffer::const_iterator begin,
     begin += sizeof(uint8_t);
 
     // 16 bytes: IPv6 address
-    addr_ = IOAddress::fromBytes(AF_INET6, &(*begin));
+    OptionBuffer address_with_mask;
+    mask(begin, begin + V6ADDRESS_LEN, prefix_len_, address_with_mask);
+    addr_ = IOAddress::fromBytes(AF_INET6, &(*address_with_mask.begin()));
     begin += V6ADDRESS_LEN;
 
     // unpack encapsulated options (the only defined so far is PD_EXCLUDE)
@@ -122,5 +124,20 @@ uint16_t Option6IAPrefix::len() {
     return (length);
 }
 
+void
+Option6IAPrefix::mask(OptionBuffer::const_iterator begin,
+                      OptionBuffer::const_iterator end,
+                      const uint8_t len,
+                      OptionBuffer& output_address) {
+    output_address.resize(16, 0);
+    if (len >= 128) {
+        std::copy(begin, end, output_address.begin());
+    } else if (len > 0) {
+        std::copy(begin, begin + static_cast<uint8_t>(len/8), output_address.begin());
+        output_address[len/8] = (0xff << (8 - (len % 8)));
+    }
+}
+
+
 } // end of namespace isc::dhcp
 } // end of namespace isc

src/lib/dhcp/option6_iaprefix.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2013 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013-2014 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -33,9 +33,29 @@ namespace dhcp {
 /// a prefix length) 2 methods are used: getAddress() and getLength(). Although
 /// using getAddress() to obtain base prefix is somewhat counter-intuitive at
 /// first, it becomes obvious when one realizes that an address is a special
-/// case of a prefix with /128. It makes everyone's like much easier, because
+/// case of a prefix with /128. It makes everyone's life much easier, because
 /// the base prefix doubles as a regular address in many cases, e.g. when
 /// searching for a lease.
+///
+/// When searching for a prefix in the database or simply comparing two prefixes
+/// for equality, it is important that only the significant parts of the
+/// prefixes are compared. It is possible that the client or a server sends a
+/// prefix which has non-significant bits (beyond prefix length) set. The
+/// server or client receiving such a prefix should be liberal and not discard
+/// this prefix. It should rather ignore the non-significant bits. Therefore
+/// the unpack() function, which parses the prefix from the wire, always sets
+/// the non-significant bits to 0 so as two prefixes received on the wire can
+/// be compared without additional processing.
+///
+/// @todo Currently, the constructor which creates the option from the textual
+/// format doesn't set non-significant bits to 0. This is because it is assumed
+/// that the prefixes from the string are created locally (not received over the
+/// wire) and should be validated before the option is created. If we wanted
+/// to set non-significant bits to 0 when the prefix is created from the textual
+/// format it would have some peformance implications, because the option would
+/// need to be turned into wire format, appropriate bits set to 0 and then
+/// option would need to be created again from the wire format. We may consider
+/// doing it if we find a use case where it is required.
 class Option6IAPrefix : public Option6IAAddr {
 
 public:
@@ -100,7 +120,21 @@ public:
     /// returns data length (data length + DHCPv4/DHCPv6 option header)
     virtual uint16_t len();
 
-protected:
+private:
+
+    /// @brief Apply mask of the specific length to the IPv6 address.
+    ///
+    /// @param begin Iterator pointing to the buffer holding IPv6 address.
+    /// @param end Iterator pointing to the end of the buffer holding IPv6
+    /// address.
+    /// @param len Length of the mask to be applied.
+    /// @param [out] output_address Reference to the buffer where the address
+    /// with a mask applied is output.
+    void mask(OptionBuffer::const_iterator begin,
+              OptionBuffer::const_iterator end,
+              const uint8_t len,
+              OptionBuffer& output_address);
+
     uint8_t prefix_len_;
 };
 

src/lib/dhcp/tests/option6_iaprefix_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2011-2013 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-2014 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -36,7 +36,7 @@ using namespace isc::asiolink;
 namespace {
 class Option6IAPrefixTest : public ::testing::Test {
 public:
-    Option6IAPrefixTest() : buf_(255), outBuf_(255) {
+    Option6IAPrefixTest() : buf_(255), out_buf_(255) {
         for (int i = 0; i < 255; i++) {
             buf_[i] = 255 - i;
         }
@@ -45,7 +45,7 @@ public:
     /// @brief creates on-wire representation of IAPREFIX option
     ///
     /// buf_ field is set up to have IAPREFIX with preferred=1000,
-    /// valid=3000000000 and prefix beign 2001:db8:1::dead:beef/77
+    /// valid=3000000000 and prefix being 2001:db8:1:0:ffff:0:dead:beef/77
     void setExampleBuffer() {
         for (int i = 0; i < 255; i++) {
             buf_[i] = 0;
@@ -69,10 +69,12 @@ public:
         buf_[12] = 0xb8;
         buf_[13] = 0x00;
         buf_[14] = 0x01;
+        buf_[17] = 0xff;
+        buf_[18] = 0xff;
         buf_[21] = 0xde;
         buf_[22] = 0xad;
         buf_[23] = 0xbe;
-        buf_[24] = 0xef; // 2001:db8:1::dead:beef
+        buf_[24] = 0xef; // 2001:db8:1:0:ffff:0:dead:beef
     }
 
 
@@ -82,15 +84,22 @@ public:
     ///
     /// @param opt IAPREFIX option being tested
     /// @param expected_type expected option type
-    void checkOption(Option6IAPrefix& opt, uint16_t expected_type) {
+    /// @param expected_length Expected length of the prefix.
+    /// @param expected_address Expected prefix value in the textual format.
+    void checkOption(Option6IAPrefix& opt, const uint16_t expected_type,
+                     const uint8_t expected_length,
+                     const std::string& expected_address) {
 
         // Check if all fields have expected values
         EXPECT_EQ(Option::V6, opt.getUniverse());
         EXPECT_EQ(expected_type, opt.getType());
-        EXPECT_EQ("2001:db8:1::dead:beef", opt.getAddress().toText());
+        EXPECT_EQ(expected_address, opt.getAddress().toText());
         EXPECT_EQ(1000, opt.getPreferred());
         EXPECT_EQ(3000000000U, opt.getValid());
-        EXPECT_EQ(77, opt.getLength());
+        // uint8_t is often represented as a character type (char). Convert it
+        // to integer so as it is logged as a numeric value instead.
+        EXPECT_EQ(static_cast<int>(expected_length),
+                  static_cast<int>(opt.getLength()));
 
         // 4 bytes header + 25 bytes content
         EXPECT_EQ(Option::OPTION6_HDR_LEN + Option6IAPrefix::OPTION6_IAPREFIX_LEN,
@@ -105,7 +114,7 @@ public:
     /// @param expected_type expected option type
     void checkOutputBuffer(uint16_t expected_type) {
         // Check if pack worked properly:
-        const uint8_t* out = (const uint8_t*)outBuf_.getData();
+        const uint8_t* out = static_cast<const uint8_t*>(out_buf_.getData());
 
         // - if option type is correct
         EXPECT_EQ(expected_type, out[0]*256 + out[1]);
@@ -118,11 +127,12 @@ public:
     }
 
     OptionBuffer buf_;
-    OutputBuffer outBuf_;
+    OutputBuffer out_buf_;
 };
 
-// Tests if receiving option can be parsed correctly
-TEST_F(Option6IAPrefixTest, basic) {
+// Tests if a received option is parsed correctly. For the prefix length between
+// 0 and 128 the non-significant bits should be set to 0.
+TEST_F(Option6IAPrefixTest, parseShort) {
 
     setExampleBuffer();
 
@@ -133,17 +143,75 @@ TEST_F(Option6IAPrefixTest, basic) {
     ASSERT_TRUE(opt);
 
     // Pack this option
-    opt->pack(outBuf_);
-    EXPECT_EQ(29, outBuf_.getLength());
+    opt->pack(out_buf_);
+    EXPECT_EQ(29, out_buf_.getLength());
 
-    checkOption(*opt, D6O_IAPREFIX);
+    // The non-significant bits (above 77) of the received prefix should be
+    // set to zero.
+    checkOption(*opt, D6O_IAPREFIX, 77, "2001:db8:1:0:fff8::");
 
+    // Set non-significant bits in the reference buffer to 0, so as the buffer
+    // can be directly compared with the option buffer.
+    buf_[18] = 0xf8;
+    buf_.insert(buf_.begin() + 19, 5, 0);
     checkOutputBuffer(D6O_IAPREFIX);
 
     // Check that option can be disposed safely
     EXPECT_NO_THROW(opt.reset());
 }
 
+// Tests if a received option holding prefix of 128 bits is parsed correctly. 
+TEST_F(Option6IAPrefixTest, parseLong) {
+
+    setExampleBuffer();
+    // Set prefix length to the maximal value.
+    buf_[8] = 128;
+
+    // Create an option (unpack content)
+    boost::scoped_ptr<Option6IAPrefix> opt;
+    ASSERT_NO_THROW(opt.reset(new Option6IAPrefix(D6O_IAPREFIX, buf_.begin(),
+                                                  buf_.begin() + 25)));
+    ASSERT_TRUE(opt);
+
+    // Pack this option
+    opt->pack(out_buf_);
+    EXPECT_EQ(29, out_buf_.getLength());
+
+    checkOption(*opt, D6O_IAPREFIX, 128, "2001:db8:1:0:ffff:0:dead:beef");
+
+    checkOutputBuffer(D6O_IAPREFIX);
+
+    // Check that option can be disposed safely
+    EXPECT_NO_THROW(opt.reset());
+}
+
+// Check that the prefix having length of zero is represented as a "::".
+TEST_F(Option6IAPrefixTest, parseZero) {
+    setExampleBuffer();
+    // Set prefix length to 0.
+    buf_[8] = 0;
+
+    // Create an option (unpack content)
+    boost::scoped_ptr<Option6IAPrefix> opt;
+    ASSERT_NO_THROW(opt.reset(new Option6IAPrefix(D6O_IAPREFIX, buf_.begin(),
+                                                  buf_.begin() + 25)));
+    ASSERT_TRUE(opt);
+
+    // Pack this option
+    opt->pack(out_buf_);
+    EXPECT_EQ(29, out_buf_.getLength());
+
+    checkOption(*opt, D6O_IAPREFIX, 0, "::");
+
+    // Fill the address in the reference buffer with zeros.
+    buf_.insert(buf_.begin() + 9, 16, 0);
+    checkOutputBuffer(D6O_IAPREFIX);
+
+    // Check that option can be disposed safely
+    EXPECT_NO_THROW(opt.reset());
+}
+
+
 // Checks whether a new option can be built correctly
 TEST_F(Option6IAPrefixTest, build) {
 
@@ -151,14 +219,15 @@ TEST_F(Option6IAPrefixTest, build) {
     setExampleBuffer();
 
     ASSERT_NO_THROW(opt.reset(new Option6IAPrefix(12345,
-                    IOAddress("2001:db8:1::dead:beef"), 77, 1000, 3000000000u)));
+                    IOAddress("2001:db8:1:0:ffff:0:dead:beef"), 77,
+                                                  1000, 3000000000u)));
     ASSERT_TRUE(opt);
 
-    checkOption(*opt, 12345);
+    checkOption(*opt, 12345, 77, "2001:db8:1:0:ffff:0:dead:beef");
 
     // Check if we can build it properly
-    EXPECT_NO_THROW(opt->pack(outBuf_));
-    EXPECT_EQ(29, outBuf_.getLength());
+    EXPECT_NO_THROW(opt->pack(out_buf_));
+    EXPECT_EQ(29, out_buf_.getLength());
     checkOutputBuffer(12345);
 
     // Check that option can be disposed safely
@@ -181,7 +250,8 @@ TEST_F(Option6IAPrefixTest, negative) {
                  BadValue);
 
     // Prefix length can't be larger than 128
-    EXPECT_THROW(Option6IAPrefix(12345, IOAddress("192.0.2.1"), 255, 1000, 2000),
+    EXPECT_THROW(Option6IAPrefix(12345, IOAddress("192.0.2.1"),
+                                 255, 1000, 2000),
                  BadValue);
 }