Browse Source

Merge branch 'trac3546'

Tomek Mrugalski 10 years ago
parent
commit
6e68af7dfe

+ 10 - 3
src/bin/dhcp4/dhcp4_srv.cc

@@ -1839,9 +1839,16 @@ Dhcpv4Srv::unpackOptions(const OptionBuffer& buf,
 
         uint8_t opt_len =  buf[offset++];
         if (offset + opt_len > buf.size()) {
-            isc_throw(OutOfRange, "Option parse failed. Tried to parse "
-                      << offset + opt_len << " bytes from " << buf.size()
-                      << "-byte long buffer.");
+
+            // 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
+            // back by two bytes (as if we never parsed them).
+            return (offset - 2);
+
+            // isc_throw(OutOfRange, "Option parse failed. Tried to parse "
+            //          << offset + opt_len << " bytes from " << buf.size()
+            //          << "-byte long buffer.");
         }
 
         // Get all definitions with the particular option code. Note that option code

+ 5 - 2
src/bin/dhcp6/dhcp6_messages.mes

@@ -319,8 +319,11 @@ for the failure is appended as an argument of the log message.
 A debug message noting that server has received message with server identifier
 option that not matching server identifier that server is using.
 
-% DHCP6_PACKET_PARSE_FAIL failed to parse incoming packet
-The IPv6 DHCP server has received a packet that it is unable to interpret.
+% DHCP6_PACKET_PARSE_FAIL failed to parse incoming packet: %1
+The DHCPv6 server has received a packet that it is unable to interpret.
+There may be many causes: truncated header, truncated or malformed option,
+trailing padding that contains garbage etc. More information is specified
+as a parameter.
 
 % DHCP6_PACKET_PROCESS_FAIL processing of %1 message received from %2 failed: %3
 This is a general catch-all message indicating that the processing of the

+ 10 - 3
src/bin/dhcp6/dhcp6_srv.cc

@@ -311,9 +311,11 @@ bool Dhcpv6Srv::run() {
         // Unpack the packet information unless the buffer6_receive callouts
         // indicated they did it
         if (!skip_unpack) {
-            if (!query->unpack()) {
+            try {
+                query->unpack();
+            } catch (const std::exception &e) {
                 LOG_DEBUG(dhcp6_logger, DBG_DHCP6_DETAIL,
-                          DHCP6_PACKET_PARSE_FAIL);
+                          DHCP6_PACKET_PARSE_FAIL).arg(e.what());
                 continue;
             }
         }
@@ -2469,7 +2471,12 @@ Dhcpv6Srv::unpackOptions(const OptionBuffer& buf,
 
         if (offset + opt_len > length) {
             // @todo: consider throwing exception here.
-            return (offset);
+
+            // 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) {

+ 8 - 3
src/bin/perfdhcp/test_control.cc

@@ -1167,6 +1167,9 @@ TestControl::receivePackets(const TestControlSocket& socket) {
                 if ((received > 1) && testDiags('i')) {
                     stats_mgr4_->incrementCounter("multircvd");
                 }
+
+                /// @todo: Add packet exception handling here. Right now any
+                /// malformed packet will cause perfdhcp to abort.
                 pkt4->unpack();
                 processReceivedPacket4(socket, pkt4);
             }
@@ -1185,9 +1188,11 @@ TestControl::receivePackets(const TestControlSocket& socket) {
                 if ((received > 1) && testDiags('i')) {
                     stats_mgr6_->incrementCounter("multircvd");
                 }
-                if (pkt6->unpack()) {
-                    processReceivedPacket6(socket, pkt6);
-                }
+
+                /// @todo: Add packet exception handling here. Right now any
+                /// malformed packet will cause perfdhcp to abort.
+                pkt6->unpack();
+                processReceivedPacket6(socket, pkt6);
             }
         }
     }

+ 1 - 0
src/lib/dhcp/Makefile.am

@@ -43,6 +43,7 @@ libkea_dhcp___la_SOURCES += option_definition.cc option_definition.h
 libkea_dhcp___la_SOURCES += option_space.cc option_space.h
 libkea_dhcp___la_SOURCES += option_string.cc option_string.h
 libkea_dhcp___la_SOURCES += protocol_util.cc protocol_util.h
+libkea_dhcp___la_SOURCES += pkt.cc pkt.h
 libkea_dhcp___la_SOURCES += pkt6.cc pkt6.h
 libkea_dhcp___la_SOURCES += pkt4.cc pkt4.h
 libkea_dhcp___la_SOURCES += pkt_filter.h pkt_filter.cc

+ 4 - 4
src/lib/dhcp/hwaddr.cc

@@ -31,14 +31,14 @@ HWAddr::HWAddr()
     :htype_(HTYPE_ETHER) {
 }
 
-HWAddr::HWAddr(const uint8_t* hwaddr, size_t len, uint8_t htype)
+HWAddr::HWAddr(const uint8_t* hwaddr, size_t len, uint16_t htype)
     :hwaddr_(hwaddr, hwaddr + len), htype_(htype) {
     if (len > MAX_HWADDR_LEN) {
         isc_throw(InvalidParameter, "hwaddr length exceeds MAX_HWADDR_LEN");
     }
 }
 
-HWAddr::HWAddr(const std::vector<uint8_t>& hwaddr, uint8_t htype)
+HWAddr::HWAddr(const std::vector<uint8_t>& hwaddr, uint16_t htype)
     :hwaddr_(hwaddr), htype_(htype) {
     if (hwaddr.size() > MAX_HWADDR_LEN) {
         isc_throw(InvalidParameter,
@@ -49,7 +49,7 @@ HWAddr::HWAddr(const std::vector<uint8_t>& hwaddr, uint8_t htype)
 std::string HWAddr::toText(bool include_htype) const {
     std::stringstream tmp;
     if (include_htype) {
-        tmp << "hwtype=" << static_cast<int>(htype_) << " ";
+        tmp << "hwtype=" << static_cast<unsigned int>(htype_) << " ";
     }
     tmp << std::hex;
     bool delim = false;
@@ -65,7 +65,7 @@ std::string HWAddr::toText(bool include_htype) const {
 }
 
 HWAddr
-HWAddr::fromText(const std::string& text, const uint8_t htype) {
+HWAddr::fromText(const std::string& text, const uint16_t htype) {
     /// @todo optimize stream operations here.
     std::vector<std::string> split_text;
     boost::split(split_text, text, boost::is_any_of(":"),

+ 9 - 5
src/lib/dhcp/hwaddr.h

@@ -41,18 +41,22 @@ public:
     /// @param hwaddr pointer to hardware address
     /// @param len length of the address pointed by hwaddr
     /// @param htype hardware type
-    HWAddr(const uint8_t* hwaddr, size_t len, uint8_t htype);
+    HWAddr(const uint8_t* hwaddr, size_t len, uint16_t htype);
 
     /// @brief constructor, based on C++ vector<uint8_t>
     /// @param hwaddr const reference to hardware address
     /// @param htype hardware type
-    HWAddr(const std::vector<uint8_t>& hwaddr, uint8_t htype);
+    HWAddr(const std::vector<uint8_t>& hwaddr, uint16_t htype);
 
     // Vector that keeps the actual hardware address
     std::vector<uint8_t> hwaddr_;
 
-    // Hardware type
-    uint8_t htype_;
+    /// Hardware type
+    ///
+    /// @note It used to be uint8_t as used in DHCPv4. However, since we're
+    /// expanding MAC addresses support to DHCPv6 that uses hw_type as
+    /// 16 bits, we need to be able to store that wider format.
+    uint16_t htype_;
 
     /// @brief Returns textual representation of a hardware address
     /// (e.g. 00:01:02:03:04:05)
@@ -83,7 +87,7 @@ public:
     ///
     /// @return Instance of the HW address created from text.
     static HWAddr fromText(const std::string& text,
-                           const uint8_t htype = HTYPE_ETHER);
+                           const uint16_t htype = HTYPE_ETHER);
 
     /// @brief Compares two hardware addresses for equality
     bool operator==(const HWAddr& other) const;

+ 35 - 12
src/lib/dhcp/libdhcp++.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
@@ -239,7 +239,12 @@ size_t LibDHCP::unpackOptions6(const OptionBuffer& buf,
 
         if (offset + opt_len > length) {
             // @todo: consider throwing exception here.
-            return (offset);
+
+            // 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
+            // back 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) {
@@ -255,8 +260,9 @@ size_t LibDHCP::unpackOptions6(const OptionBuffer& buf,
         if (opt_type == D6O_VENDOR_OPTS) {
             if (offset + 4 > length) {
                 // Truncated vendor-option. There is expected at least 4 bytes
-                // long enterprise-id field
-                return (offset);
+                // long enterprise-id field. Let's roll back option code + option
+                // length (4 bytes) and return.
+                return (offset - 4);
             }
 
             // Parse this as vendor option
@@ -351,9 +357,16 @@ size_t LibDHCP::unpackOptions4(const OptionBuffer& buf,
 
         uint8_t opt_len =  buf[offset++];
         if (offset + opt_len > buf.size()) {
-            isc_throw(OutOfRange, "Option parse failed. Tried to parse "
-                      << offset + opt_len << " bytes from " << buf.size()
-                      << "-byte long buffer.");
+
+            // 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
+            // back by two bytes (as if we never parsed them).
+            return (offset - 2);
+
+            // isc_throw(OutOfRange, "Option parse failed. Tried to parse "
+            //          << offset + opt_len << " bytes from " << buf.size()
+            //          << "-byte long buffer.");
         }
 
         // Get all definitions with the particular option code. Note that option code
@@ -421,7 +434,12 @@ size_t LibDHCP::unpackVendorOptions6(const uint32_t vendor_id,
 
         if (offset + opt_len > length) {
             // @todo: consider throwing exception here.
-            return (offset);
+
+            // 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
+            // back by those four bytes (as if we never parsed them).
+            return (offset - 4);
         }
 
         OptionPtr opt;
@@ -497,8 +515,13 @@ size_t LibDHCP::unpackVendorOptions4(const uint32_t vendor_id, const OptionBuffe
         uint8_t data_len = buf[offset++];
 
         if (offset + data_len > buf.size()) {
-            // Truncated data-option
-            return (offset);
+            // The option is truncated.
+
+            // We peeked at the data_len, but discovered that it would end up
+            // beyond buffer end, so the data block is truncated. Hence we can't
+            // parse it. Therefore we revert back by one byte (as if we never
+            // parsed it).
+            return (offset - 1);
         }
 
         uint8_t offset_end = offset + data_len;
@@ -519,7 +542,7 @@ size_t LibDHCP::unpackVendorOptions4(const uint32_t vendor_id, const OptionBuffe
             if (offset + 1 >= buf.size()) {
                 // opt_type must be cast to integer so as it is not treated as
                 // unsigned char value (a number is presented in error message).
-                isc_throw(OutOfRange, "Attempt to parse truncated option "
+                isc_throw(OutOfRange, "Attempt to parse truncated vendor option "
                           << static_cast<int>(opt_type));
             }
 
@@ -539,7 +562,7 @@ size_t LibDHCP::unpackVendorOptions4(const uint32_t vendor_id, const OptionBuffe
                 // 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.
+                // Get the number of returned option definitions for the option code.
                 size_t num_defs = distance(range.first, range.second);
 
                 if (num_defs > 1) {

+ 4 - 1
src/lib/dhcp/libdhcp++.h

@@ -123,6 +123,7 @@ public:
     /// of to be used to parse options in the packets.
     /// @param options Reference to option container. Options will be
     ///        put here.
+    /// @return offset to the first byte after the last successfully parsed option
     static size_t unpackOptions4(const OptionBuffer& buf,
                                  const std::string& option_space,
                                  isc::dhcp::OptionCollection& options);
@@ -147,7 +148,7 @@ public:
     ///        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 offset to the first byte after last parsed option
+    /// @return offset to the first byte after the last successfully parsed option
     static size_t unpackOptions6(const OptionBuffer& buf,
                                  const std::string& option_space,
                                  isc::dhcp::OptionCollection& options,
@@ -192,6 +193,7 @@ public:
     /// @param buf Buffer to be parsed.
     /// @param options Reference to option container. Options will be
     ///        put here.
+    /// @return offset to the first byte after the last successfully parsed option
     static size_t unpackVendorOptions6(const uint32_t vendor_id,
                                        const OptionBuffer& buf,
                                        isc::dhcp::OptionCollection& options);
@@ -206,6 +208,7 @@ public:
     /// @param buf Buffer to be parsed.
     /// @param options Reference to option container. Options will be
     ///        put here.
+    /// @return offset to the first byte after the last successfully parsed option
     static size_t unpackVendorOptions4(const uint32_t vendor_id, const OptionBuffer& buf,
                                        isc::dhcp::OptionCollection& options);
 

+ 146 - 0
src/lib/dhcp/pkt.cc

@@ -0,0 +1,146 @@
+// Copyright (C) 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
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <utility>
+#include <dhcp/pkt.h>
+
+namespace isc {
+namespace dhcp {
+
+Pkt::Pkt(uint32_t transid, const isc::asiolink::IOAddress& local_addr,
+         const isc::asiolink::IOAddress& remote_addr, uint16_t local_port,
+         uint16_t remote_port)
+    :transid_(transid),
+     iface_(""),
+     ifindex_(-1),
+     local_addr_(local_addr),
+     remote_addr_(remote_addr),
+     local_port_(local_port),
+     remote_port_(remote_port),
+     buffer_out_(0)
+{
+}
+
+Pkt::Pkt(const uint8_t* buf, uint32_t len, const isc::asiolink::IOAddress& local_addr,
+         const isc::asiolink::IOAddress& remote_addr, uint16_t local_port,
+         uint16_t remote_port)
+    :transid_(0),
+     iface_(""),
+     ifindex_(-1),
+     local_addr_(local_addr),
+     remote_addr_(remote_addr),
+     local_port_(local_port),
+     remote_port_(remote_port),
+     buffer_out_(0)
+{
+    data_.resize(len);
+    if (len) {
+        memcpy(&data_[0], buf, len);
+    }
+}
+
+void
+Pkt::addOption(const OptionPtr& opt) {
+    options_.insert(std::pair<int, OptionPtr>(opt->getType(), opt));
+}
+
+OptionPtr
+Pkt::getOption(uint16_t type) const {
+    OptionCollection::const_iterator x = options_.find(type);
+    if (x != options_.end()) {
+        return (*x).second;
+    }
+    return (OptionPtr()); // NULL
+}
+
+bool
+Pkt::delOption(uint16_t type) {
+
+    isc::dhcp::OptionCollection::iterator x = options_.find(type);
+    if (x!=options_.end()) {
+        options_.erase(x);
+        return (true); // delete successful
+    } else {
+        return (false); // can't find option to be deleted
+    }
+}
+
+bool
+Pkt::inClass(const std::string& client_class) {
+    return (classes_.find(client_class) != classes_.end());
+}
+
+void
+Pkt::addClass(const std::string& client_class) {
+    if (classes_.find(client_class) == classes_.end()) {
+        classes_.insert(client_class);
+    }
+}
+
+void
+Pkt::updateTimestamp() {
+    timestamp_ = boost::posix_time::microsec_clock::universal_time();
+}
+
+void Pkt::repack() {
+    if (!data_.empty()) {
+        buffer_out_.writeData(&data_[0], data_.size());
+    }
+}
+
+void
+Pkt::setRemoteHWAddr(const uint8_t htype, const uint8_t hlen,
+                      const std::vector<uint8_t>& hw_addr) {
+    setHWAddrMember(htype, hlen, hw_addr, remote_hwaddr_);
+}
+
+void
+Pkt::setRemoteHWAddr(const HWAddrPtr& hw_addr) {
+    if (!hw_addr) {
+        isc_throw(BadValue, "Setting remote HW address to NULL is"
+                  << " forbidden.");
+    }
+    remote_hwaddr_ = hw_addr;
+}
+
+void
+Pkt::setHWAddrMember(const uint8_t htype, const uint8_t,
+                      const std::vector<uint8_t>& hw_addr,
+                      HWAddrPtr& storage) {
+
+    storage.reset(new HWAddr(hw_addr, htype));
+}
+
+HWAddrPtr
+Pkt::getMAC(uint32_t hw_addr_src) {
+    HWAddrPtr mac;
+    if (hw_addr_src & HWADDR_SOURCE_RAW) {
+        mac = getRemoteHWAddr();
+        if (mac) {
+            return (mac);
+        } else if (hw_addr_src == HWADDR_SOURCE_RAW) {
+            // If we're interested only in RAW sockets as source of that info,
+            // there's no point in trying other options.
+            return (HWAddrPtr());
+        }
+    }
+
+    /// @todo: add other MAC acquisition methods here
+
+    // Ok, none of the methods were suitable. Return NULL.
+    return (HWAddrPtr());
+}
+
+};
+};

+ 577 - 0
src/lib/dhcp/pkt.h

@@ -0,0 +1,577 @@
+// Copyright (C) 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
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef PKT_H
+#define PKT_H
+
+#include <asiolink/io_address.h>
+#include <util/buffer.h>
+#include <dhcp/option.h>
+#include <dhcp/hwaddr.h>
+#include <dhcp/classify.h>
+
+#include <boost/date_time/posix_time/posix_time.hpp>
+
+namespace isc {
+
+namespace dhcp {
+
+/// @brief Base class for classes representing DHCP messages.
+///
+/// This is a base class that holds common information (e.g. source
+/// and destination ports) and operations (e.g. add, get, delete options)
+/// for derived classes representing both DHCPv4 and DHCPv6 messages.
+/// The @c Pkt4 and @c Pkt6 classes derive from it.
+///
+/// @note This is abstract class. Please instantiate derived classes
+/// such as @c Pkt4 or @c Pkt6.
+class Pkt {
+public:
+
+    /// @defgroup hw_sources Specifies where a given MAC/hardware address was
+    ///                      obtained.
+    ///
+    /// @brief The list covers all possible MAC/hw address sources.
+    ///
+    /// @note The uncommented ones are currently supported. When you implement
+    /// a new method, please uncomment appropriate line here.
+    ///
+    /// @{
+
+    /// Not really a type, only used in getMAC() calls.
+    static const uint32_t HWADDR_SOURCE_ANY = 0xffff;
+
+    /// Obtained first hand from raw socket (100% reliable).
+    static const uint32_t HWADDR_SOURCE_RAW = 0x0001;
+
+    /// Extracted from DUID-LL or DUID-LLT (not 100% reliable as the client
+    /// can send fake DUID).
+    //static const uint32_t HWADDR_SOURCE_DUID = 0x0002;
+
+    /// Extracted from IPv6 link-local address. Not 100% reliable, as the
+    /// client can use different IID other than EUI-64, e.g. Windows supports
+    /// RFC4941 and uses random values instead of EUI-64.
+    //static const uint32_t HWADDR_SOURCE_IPV6_LINK_LOCAL = 0x0004;
+
+    /// Get it from RFC6939 option. (A relay agent can insert client link layer
+    /// address option). Note that a skilled attacker can fake that by sending
+    /// his request relayed, so the legitimate relay will think it's a second
+    /// relay.
+    //static const uint32_t HWADDR_SOURCE_CLIENT_ADDR_RELAY_OPTION = 0x0008;
+
+    /// A relay can insert remote-id. In some deployments it contains a MAC
+    /// address (RFC4649).
+    //static const uint32_t HWADDR_SOURCE_REMOTE_ID = 0x0010;
+
+    /// A relay can insert a subscriber-id option. In some deployments it
+    /// contains a MAC address (RFC4580).
+    //static const uint32_t HWADDR_SOURCE_SUBSCRIBER_ID = 0x0020;
+
+    /// A CMTS (acting as DHCP relay agent) that supports DOCSIS standard
+    /// can insert DOCSIS options that contain client's MAC address.
+    /// Client in this context would be a cable modem.
+    //static const uint32_t HWADDR_SOURCE_DOCSIS_OPTIONS = 0x0040;
+
+    /// @}
+
+protected:
+
+    /// @brief Constructor.
+    ///
+    /// This constructor is typically used for transmitted messages as it
+    /// creates an empty (no options) packet. The constructor is protected,
+    /// so only derived classes can call it. Pkt class cannot be instantiated
+    /// anyway, because it is an abstract class.
+    ///
+    /// @param transid transaction-id
+    /// @param local_addr local IPv4 or IPv6 address
+    /// @param remote_addr remote IPv4 or IPv6 address
+    /// @param local_port local UDP (one day also TCP) port
+    /// @param remote_port remote UDP (one day also TCP) port
+    Pkt(uint32_t transid, const isc::asiolink::IOAddress& local_addr,
+        const isc::asiolink::IOAddress& remote_addr, uint16_t local_port,
+        uint16_t remote_port);
+
+    /// @brief Constructor.
+    ///
+    /// This constructor is typically used for received messages as it takes
+    /// a buffer that's going to be parsed as one of arguments. The constructor
+    /// is protected, so only derived classes can call it. Pkt class cannot be
+    /// instantiated anyway, because it is an abstract class.
+    ///
+    /// @param buf pointer to a buffer that contains on-wire data
+    /// @param len length of the pointer specified in buf
+    /// @param local_addr local IPv4 or IPv6 address
+    /// @param remote_addr remote IPv4 or IPv6 address
+    /// @param local_port local UDP (one day also TCP) port
+    /// @param remote_port remote UDP (one day also TCP) port
+    Pkt(const uint8_t* buf, uint32_t len,
+        const isc::asiolink::IOAddress& local_addr,
+        const isc::asiolink::IOAddress& remote_addr, uint16_t local_port,
+        uint16_t remote_port);
+
+public:
+
+    /// @brief Prepares on-wire format of DHCP (either v4 or v6) packet.
+    ///
+    /// Prepares on-wire format of message and all its options.
+    /// A caller must ensure that options are stored in options_ field
+    /// prior to calling this method.
+    ///
+    /// Output buffer will be stored in buffer_out_.
+    /// The buffer_out_ should be cleared before writting to the buffer
+    /// in the derived classes.
+    ///
+    /// @note This is a pure virtual method and must be implemented in
+    /// the derived classes. The @c Pkt4 and @c Pkt6 class have respective
+    /// implementations of this method.
+    ///
+    /// @throw InvalidOperation if packing fails
+    virtual void pack() = 0;
+
+    /// @brief Parses on-wire form of DHCP (either v4 or v6) packet.
+    ///
+    /// Parses received packet, stored in on-wire format in data_.
+    ///
+    /// Will create a collection of option objects that will
+    /// be stored in options_ container.
+    ///
+    /// @note This is a pure virtual method and must be implemented in
+    /// the derived classes. The @c Pkt4 and @c Pkt6 class have respective
+    /// implementations of this method.
+    ///
+    /// Method will throw exception if packet parsing fails.
+    ///
+    /// @throw tbd
+    virtual void unpack() = 0;
+
+    /// @brief Returns reference to output buffer.
+    ///
+    /// Returned buffer will contain reasonable data only for
+    /// output (TX) packet and after pack() was called.
+    ///
+    /// RX packet or TX packet before pack() will return buffer with
+    /// zero length. This buffer is returned as non-const, so hooks
+    /// framework (and user's callouts) can modify them if needed
+    ///
+    /// @note This buffer is only valid till object that returned it exists.
+    ///
+    /// @return reference to output buffer
+    isc::util::OutputBuffer& getBuffer() { return (buffer_out_); };
+
+    /// @brief Adds an option to this packet.
+    ///
+    /// Derived classes may provide more specialized implementations.
+    /// In particular @c Pkt4 provides one that checks if option is
+    /// unique.
+    ///
+    /// @param opt option to be added.
+    virtual void addOption(const OptionPtr& opt);
+
+    /// @brief Attempts to delete first suboption of requested type.
+    ///
+    /// If there are several options of the same type present, only
+    /// the first option will be deleted.
+    ///
+    /// @param type Type of option to be deleted.
+    ///
+    /// @return true if option was deleted, false if no such option existed
+    bool delOption(uint16_t type);
+
+    /// @brief Returns text representation of the packet.
+    ///
+    /// This function is useful mainly for debugging.
+    ///
+    /// @note This is a pure virtual method and must be implemented in
+    /// the derived classes. The @c Pkt4 and @c Pkt6 class have respective
+    /// implementations of this method.
+    ///
+    /// @return string with text representation
+    virtual std::string toText() = 0;
+
+    /// @brief Returns packet size in binary format.
+    ///
+    /// Returns size of the packet in on-wire format or size needed to store
+    /// it in on-wire format.
+    ///
+    /// @note This is a pure virtual method and must be implemented in
+    /// the derived classes. The @c Pkt4 and @c Pkt6 class have respective
+    /// implementations of this method.
+    ///
+    /// @return packet size in bytes
+    virtual size_t len() = 0;
+
+    /// @brief Returns message type (e.g. 1 = SOLICIT).
+    ///
+    /// @note This is a pure virtual method and must be implemented in
+    /// the derived classes. The @c Pkt4 and @c Pkt6 class have respective
+    /// implementations of this method.
+    ///
+    /// @return message type
+    virtual uint8_t getType() const = 0;
+
+    /// @brief Sets message type (e.g. 1 = SOLICIT).
+    ///
+    /// @note This is a pure virtual method and must be implemented in
+    /// the derived classes. The @c Pkt4 and @c Pkt6 class have respective
+    /// implementations of this method.
+    ///
+    /// @param type message type to be set
+    virtual void setType(uint8_t type) = 0;
+
+    /// @brief Sets transaction-id value.
+    ///
+    /// @param transid transaction-id to be set.
+    void setTransid(uint32_t transid) { transid_ = transid; }
+
+    /// @brief Returns value of transaction-id field.
+    ///
+    /// @return transaction-id
+    uint32_t getTransid() const { return (transid_); };
+
+    /// @brief Checks whether a client belongs to a given class.
+    ///
+    /// @param client_class name of the class
+    /// @return true if belongs
+    bool inClass(const isc::dhcp::ClientClass& client_class);
+
+    /// @brief Adds packet to a specified class.
+    ///
+    /// A packet can be added to the same class repeatedly. Any additional
+    /// attempts to add to a class the packet already belongs to, will be
+    /// ignored silently.
+    ///
+    /// @note It is a matter of naming convention. Conceptually, the server
+    /// processes a stream of packets, with some packets belonging to given
+    /// classes. From that perspective, this method adds a packet to specifed
+    /// class. Implementation wise, it looks the opposite - the class name
+    /// is added to the packet. Perhaps the most appropriate name for this
+    /// method would be associateWithClass()? But that seems overly long,
+    /// so I decided to stick with addClass().
+    ///
+    /// @param client_class name of the class to be added
+    void addClass(const isc::dhcp::ClientClass& client_class);
+
+    /// @brief Unparsed data (in received packets).
+    ///
+    /// @warning This public member is accessed by derived
+    /// classes directly. One of such derived classes is
+    /// @ref perfdhcp::PerfPkt6. The impact on derived clasess'
+    /// behavior must be taken into consideration before making
+    /// changes to this member such as access scope restriction or
+    /// data format change etc.
+    OptionBuffer data_;
+
+    /// @brief Returns the first option of specified type.
+    ///
+    /// Returns the first option of specified type. Note that in DHCPv6 several
+    /// instances of the same option are allowed (and frequently used).
+    /// Also see \ref Pkt6::getOptions().
+    ///
+    /// The options will be only returned after unpack() is called.
+    ///
+    /// @param type option type we are looking for
+    ///
+    /// @return pointer to found option (or NULL)
+    OptionPtr getOption(uint16_t type) const;
+
+    /// @brief Update packet timestamp.
+    ///
+    /// Updates packet timestamp. This method is invoked
+    /// by interface manager just before sending or
+    /// just after receiving it.
+    /// @throw isc::Unexpected if timestamp update failed
+    void updateTimestamp();
+
+    /// @brief Returns packet timestamp.
+    ///
+    /// Returns packet timestamp value updated when
+    /// packet is received or send.
+    ///
+    /// @return packet timestamp.
+    const boost::posix_time::ptime& getTimestamp() const {
+        return timestamp_;
+    }
+
+    /// @brief Copies content of input buffer to output buffer.
+    ///
+    /// This is mostly a diagnostic function. It is being used for sending
+    /// received packet. Received packet is stored in data_, but
+    /// transmitted data is stored in buffer_out_. If we want to send packet
+    /// that we just received, a copy between those two buffers is necessary.
+    void repack();
+
+    /// @brief Set callback function to be used to parse options.
+    ///
+    /// @param callback An instance of the callback function or NULL to
+    /// uninstall callback.
+    void setCallback(UnpackOptionsCallback callback) {
+        callback_ = callback;
+    }
+
+    /// @brief Sets remote IP address.
+    ///
+    /// @param remote specifies remote address
+    void setRemoteAddr(const isc::asiolink::IOAddress& remote) {
+        remote_addr_ = remote;
+    }
+
+    /// @brief Returns remote IP address.
+    ///
+    /// @return remote address
+    const isc::asiolink::IOAddress& getRemoteAddr() const {
+        return (remote_addr_);
+    }
+
+    /// @brief Sets local IP address.
+    ///
+    /// @param local specifies local address
+    void setLocalAddr(const isc::asiolink::IOAddress& local) {
+        local_addr_ = local;
+    }
+
+    /// @brief Returns local IP address.
+    ///
+    /// @return local address
+    const isc::asiolink::IOAddress& getLocalAddr() const {
+        return (local_addr_);
+    }
+
+    /// @brief Sets local UDP (and soon TCP) port.
+    ///
+    /// This sets a local port, i.e. destination port for recently received
+    /// packet or a source port for to be transmitted packet.
+    ///
+    /// @param local specifies local port
+    void setLocalPort(uint16_t local) {
+        local_port_ = local;
+    }
+
+    /// @brief Returns local UDP (and soon TCP) port.
+    ///
+    /// This sets a local port, i.e. destination port for recently received
+    /// packet or a source port for to be transmitted packet.
+    ///
+    /// @return local port
+    uint16_t getLocalPort() const {
+        return (local_port_);
+    }
+
+    /// @brief Sets remote UDP (and soon TCP) port.
+    ///
+    /// This sets a remote port, i.e. source port for recently received
+    /// packet or a destination port for to be transmitted packet.
+    ///
+    /// @param remote specifies remote port
+    void setRemotePort(uint16_t remote) {
+        remote_port_ = remote;
+    }
+
+    /// @brief Returns remote port.
+    ///
+    /// @return remote port
+    uint16_t getRemotePort() const {
+        return (remote_port_);
+    }
+
+    /// @brief Sets interface index.
+    ///
+    /// @param ifindex specifies interface index.
+    void setIndex(uint32_t ifindex) {
+        ifindex_ = ifindex;
+    };
+
+    /// @brief Returns interface index.
+    ///
+    /// @return interface index
+    uint32_t getIndex() const {
+        return (ifindex_);
+    };
+
+    /// @brief Returns interface name.
+    ///
+    /// Returns interface name over which packet was received or is
+    /// going to be transmitted.
+    ///
+    /// @return interface name
+    std::string getIface() const { return (iface_); };
+
+    /// @brief Sets interface name.
+    ///
+    /// Sets interface name over which packet was received or is
+    /// going to be transmitted.
+    ///
+    /// @return interface name
+    void setIface(const std::string& iface ) { iface_ = iface; };
+
+    /// @brief Sets remote hardware address.
+    ///
+    /// Sets hardware address (MAC) from an existing HWAddr structure.
+    /// The remote address is a destination address for outgoing
+    /// packet and source address for incoming packet. When this
+    /// is an outgoing packet, this address will be used to
+    /// construct the link layer header.
+    ///
+    /// @param hw_addr structure representing HW address.
+    ///
+    /// @throw BadValue if addr is null
+    void setRemoteHWAddr(const HWAddrPtr& hw_addr);
+
+    /// @brief Sets remote hardware address.
+    ///
+    /// Sets the destination hardware (MAC) address for the outgoing packet
+    /// or source HW address for the incoming packet. When this
+    /// is an outgoing packet this address will be used to construct
+    /// the link layer header.
+    ///
+    /// @note mac_addr must be a buffer of at least hlen bytes.
+    ///
+    /// In a typical case, hlen field would be redundant, as it could
+    /// be extracted from mac_addr.size(). However, the difference is
+    /// when running on exotic hardware, like Infiniband, that had
+    /// MAC addresses 20 bytes long. In that case, hlen is set to zero
+    /// in DHCPv4.
+    ///
+    /// @param htype hardware type (will be sent in htype field)
+    /// @param hlen hardware length (will be sent in hlen field)
+    /// @param hw_addr pointer to hardware address
+    void setRemoteHWAddr(const uint8_t htype, const uint8_t hlen,
+                         const std::vector<uint8_t>& hw_addr);
+
+    /// @brief Returns the remote HW address obtained from raw sockets.
+    ///
+    /// @return remote HW address.
+    HWAddrPtr getRemoteHWAddr() const {
+        return (remote_hwaddr_);
+    }
+
+    /// @brief Returns MAC address.
+    ///
+    /// The difference between this method and getRemoteHWAddr() is that
+    /// getRemoteHWAddr() returns only what was obtained from raw sockets.
+    /// This method is more generic and can attempt to obtain MAC from
+    /// varied sources: raw sockets, client-id, link-local IPv6 address,
+    /// and various relay options.
+    ///
+    /// @note Technically the proper term for this information is a link layer
+    /// address, but it is frequently referred to MAC or hardware address.
+    /// Since we're calling the feature "MAC addresses in DHCPv6", we decided
+    /// to keep the name of getMAC().
+    ///
+    /// hw_addr_src takes a combination of bit values specified in
+    /// HWADDR_SOURCE_* constants.
+    ///
+    /// @param hw_addr_src a bitmask that specifies hardware address source
+    HWAddrPtr getMAC(uint32_t hw_addr_src);
+
+    /// @brief Virtual desctructor.
+    ///
+    /// There is nothing to clean up here, but since there are virtual methods,
+    /// we define virtual destructor to ensure that derived classes will have
+    /// a virtual one, too.
+    virtual ~Pkt() {
+    }
+
+    /// @brief Classes this packet belongs to.
+    ///
+    /// This field is public, so the code outside of Pkt4 or Pkt6 class can
+    /// iterate over existing classes. Having it public also solves the problem
+    /// of returned reference lifetime. It is preferred to use @ref inClass and
+    /// @ref addClass should be used to operate on this field.
+    ClientClasses classes_;
+
+    /// @brief Collection of options present in this message.
+    ///
+    /// @warning This public member is accessed by derived
+    /// classes directly. One of such derived classes is
+    /// @ref perfdhcp::PerfPkt6. The impact on derived clasess'
+    /// behavior must be taken into consideration before making
+    /// changes to this member such as access scope restriction or
+    /// data format change etc.
+    isc::dhcp::OptionCollection options_;
+
+protected:
+
+    /// Transaction-id (32 bits for v4, 24 bits for v6)
+    uint32_t transid_;
+
+    /// Name of the network interface the packet was received/to be sent over.
+    std::string iface_;
+
+    /// @brief Interface index.
+    ///
+    /// Each network interface has assigned an unique ifindex.
+    /// It is a functional equivalent of a name, but sometimes more useful, e.g.
+    /// when using odd systems that allow spaces in interface names.
+    int ifindex_;
+
+    /// @brief Local IP (v4 or v6) address.
+    ///
+    /// Specifies local IPv4 or IPv6 address. It is a destination address for
+    /// received packet, and a source address if it packet is being transmitted.
+    isc::asiolink::IOAddress local_addr_;
+
+    /// @brief Remote IP address.
+    ///
+    /// Specifies local IPv4 or IPv6 address. It is source address for received
+    /// packet and a destination address for packet being transmitted.
+    isc::asiolink::IOAddress remote_addr_;
+
+    /// local TDP or UDP port
+    uint16_t local_port_;
+
+    /// remote TCP or UDP port
+    uint16_t remote_port_;
+
+    /// Output buffer (used during message transmission)
+    ///
+    /// @warning This protected member is accessed by derived
+    /// classes directly. One of such derived classes is
+    /// @ref perfdhcp::PerfPkt6. The impact on derived clasess'
+    /// behavior must be taken into consideration before making
+    /// changes to this member such as access scope restriction or
+    /// data format change etc.
+    isc::util::OutputBuffer buffer_out_;
+
+    /// packet timestamp
+    boost::posix_time::ptime timestamp_;
+
+    // remote HW address (src if receiving packet, dst if sending packet)
+    HWAddrPtr remote_hwaddr_;
+
+    /// A callback to be called to unpack options from the packet.
+    UnpackOptionsCallback callback_;
+
+private:
+
+    /// @brief Generic method that validates and sets HW address.
+    ///
+    /// This is a generic method used by all modifiers of this class
+    /// which set class members representing HW address.
+    ///
+    /// @param htype hardware type.
+    /// @param hlen hardware length.
+    /// @param hw_addr pointer to actual hardware address.
+    /// @param [out] storage pointer to a class member to be modified.
+    ///
+    /// @trow isc::OutOfRange if invalid HW address specified.
+    virtual void setHWAddrMember(const uint8_t htype, const uint8_t hlen,
+                                 const std::vector<uint8_t>& hw_addr,
+                                 HWAddrPtr& storage);
+};
+
+}; // namespace isc::dhcp
+}; // namespace isc
+
+#endif

+ 32 - 76
src/lib/dhcp/pkt4.cc

@@ -27,23 +27,21 @@ using namespace std;
 using namespace isc::dhcp;
 using namespace isc::asiolink;
 
-namespace isc {
-namespace dhcp {
+namespace {
 
+/// @brief Default address used in Pkt4 constructor
 const IOAddress DEFAULT_ADDRESS("0.0.0.0");
+}
+
+namespace isc {
+namespace dhcp {
 
 Pkt4::Pkt4(uint8_t msg_type, uint32_t transid)
-     :buffer_out_(DHCPV4_PKT_HDR_LEN),
-      local_addr_(DEFAULT_ADDRESS),
-      remote_addr_(DEFAULT_ADDRESS),
-      iface_(""),
-      ifindex_(0),
-      local_port_(DHCP4_SERVER_PORT),
-      remote_port_(DHCP4_CLIENT_PORT),
+     :Pkt(transid, DEFAULT_ADDRESS, DEFAULT_ADDRESS, DHCP4_SERVER_PORT,
+          DHCP4_CLIENT_PORT),
       op_(DHCPTypeToBootpType(msg_type)),
       hwaddr_(new HWAddr()),
       hops_(0),
-      transid_(transid),
       secs_(0),
       flags_(0),
       ciaddr_(DEFAULT_ADDRESS),
@@ -58,17 +56,11 @@ Pkt4::Pkt4(uint8_t msg_type, uint32_t transid)
 }
 
 Pkt4::Pkt4(const uint8_t* data, size_t len)
-     :buffer_out_(0), // not used, this is RX packet
-      local_addr_(DEFAULT_ADDRESS),
-      remote_addr_(DEFAULT_ADDRESS),
-      iface_(""),
-      ifindex_(0),
-      local_port_(DHCP4_SERVER_PORT),
-      remote_port_(DHCP4_CLIENT_PORT),
+     :Pkt(data, len, DEFAULT_ADDRESS, DEFAULT_ADDRESS, DHCP4_SERVER_PORT,
+          DHCP4_CLIENT_PORT),
       op_(BOOTREQUEST),
       hwaddr_(new HWAddr()),
       hops_(0),
-      transid_(0),
       secs_(0),
       flags_(0),
       ciaddr_(DEFAULT_ADDRESS),
@@ -76,6 +68,7 @@ Pkt4::Pkt4(const uint8_t* data, size_t len)
       siaddr_(DEFAULT_ADDRESS),
       giaddr_(DEFAULT_ADDRESS)
 {
+
     if (len < DHCPV4_PKT_HDR_LEN) {
         isc_throw(OutOfRange, "Truncated DHCPv4 packet (len=" << len
                   << ") received, at least " << DHCPV4_PKT_HDR_LEN
@@ -144,7 +137,6 @@ Pkt4::pack() {
         // write (len) bytes of padding
         vector<uint8_t> zeros(hw_len, 0);
         buffer_out_.writeData(&zeros[0], hw_len);
-        // buffer_out_.writeData(chaddr_, MAX_CHADDR_LEN);
 
         buffer_out_.writeData(sname_, MAX_SNAME_LEN);
         buffer_out_.writeData(file_, MAX_FILE_LEN);
@@ -218,17 +210,34 @@ Pkt4::unpack() {
 
     // Use readVector because a function which parses option requires
     // a vector as an input.
+    size_t offset;
     buffer_in.readVector(opts_buffer, opts_len);
     if (callback_.empty()) {
-        LibDHCP::unpackOptions4(opts_buffer, "dhcp4", options_);
+        offset = LibDHCP::unpackOptions4(opts_buffer, "dhcp4", options_);
     } else {
         // The last two arguments are set to NULL because they are
         // specific to DHCPv6 options parsing. They are unused for
         // DHCPv4 case. In DHCPv6 case they hold are the relay message
         // offset and length.
-        callback_(opts_buffer, "dhcp4", options_, NULL, NULL);
+        offset = callback_(opts_buffer, "dhcp4", options_, NULL, NULL);
     }
 
+    // If offset is not equal to the size, then something is wrong here. We
+    // either parsed past input buffer (bug in our code) or we haven't parsed
+    // everything (received trailing garbage or truncated option).
+    //
+    // Invoking Jon Postel's law here: be conservative in what you send, and be
+    // liberal in what you accept. There's no easy way to log something from
+    // libdhcp++ library, so we just choose to be silent about remaining
+    // bytes. We also need to quell compiler warning about unused offset
+    // variable.
+    //
+    // if (offset != size) {
+    //        isc_throw(BadValue, "Received DHCPv6 buffer of size " << size
+    //                  << ", were able to parse " << offset << " bytes.");
+    // }
+    (void)offset;
+
     // @todo check will need to be called separately, so hooks can be called
     // after the packet is parsed, but before its content is verified
     check();
@@ -272,10 +281,6 @@ void Pkt4::setType(uint8_t dhcp_type) {
     }
 }
 
-void Pkt4::repack() {
-    buffer_out_.writeData(&data_[0], data_.size());
-}
-
 std::string
 Pkt4::toText() {
     stringstream tmp;
@@ -342,21 +347,6 @@ Pkt4::setLocalHWAddr(const HWAddrPtr& addr) {
 }
 
 void
-Pkt4::setRemoteHWAddr(const uint8_t htype, const uint8_t hlen,
-                      const std::vector<uint8_t>& mac_addr) {
-    setHWAddrMember(htype, hlen, mac_addr, remote_hwaddr_);
-}
-
-void
-Pkt4::setRemoteHWAddr(const HWAddrPtr& addr) {
-    if (!addr) {
-        isc_throw(BadValue, "Setting remote HW address to NULL is"
-                  << " forbidden.");
-    }
-    remote_hwaddr_ = addr;
-}
-
-void
 Pkt4::setSname(const uint8_t* sname, size_t snameLen /*= MAX_SNAME_LEN*/) {
     if (snameLen > MAX_SNAME_LEN) {
         isc_throw(OutOfRange, "sname field (len=" << snameLen
@@ -433,37 +423,14 @@ Pkt4::getHlen() const {
 }
 
 void
-Pkt4::addOption(boost::shared_ptr<Option> opt) {
+Pkt4::addOption(const OptionPtr& opt) {
     // Check for uniqueness (DHCPv4 options must be unique)
     if (getOption(opt->getType())) {
         isc_throw(BadValue, "Option " << opt->getType()
                   << " already present in this message.");
     }
-    options_.insert(pair<int, boost::shared_ptr<Option> >(opt->getType(), opt));
-}
 
-boost::shared_ptr<isc::dhcp::Option>
-Pkt4::getOption(uint8_t type) const {
-    OptionCollection::const_iterator x = options_.find(type);
-    if (x != options_.end()) {
-        return (*x).second;
-    }
-    return boost::shared_ptr<isc::dhcp::Option>(); // NULL
-}
-
-bool
-Pkt4::delOption(uint8_t type) {
-    isc::dhcp::OptionCollection::iterator x = options_.find(type);
-    if (x != options_.end()) {
-        options_.erase(x);
-        return (true); // delete successful
-    }
-    return (false); // can't find option to be deleted
-}
-
-void
-Pkt4::updateTimestamp() {
-    timestamp_ = boost::posix_time::microsec_clock::universal_time();
+    Pkt::addOption(opt);
 }
 
 bool
@@ -485,17 +452,6 @@ Pkt4::isRelayed() const {
               "hops != 0)");
 }
 
-bool Pkt4::inClass(const isc::dhcp::ClientClass& client_class) {
-    return (classes_.find(client_class) != classes_.end());
-}
-
-void
-Pkt4::addClass(const isc::dhcp::ClientClass& client_class) {
-    if (classes_.find(client_class) == classes_.end()) {
-        classes_.insert(client_class);
-    }
-}
-
 } // end of namespace isc::dhcp
 
 } // end of namespace isc

+ 18 - 282
src/lib/dhcp/pkt4.h

@@ -19,10 +19,9 @@
 #include <dhcp/option.h>
 #include <util/buffer.h>
 #include <dhcp/option.h>
-#include <dhcp/hwaddr.h>
 #include <dhcp/classify.h>
+#include <dhcp/pkt.h>
 
-#include <boost/date_time/posix_time/posix_time.hpp>
 #include <boost/shared_ptr.hpp>
 
 #include <iostream>
@@ -35,7 +34,14 @@ namespace isc {
 
 namespace dhcp {
 
-class Pkt4 {
+/// @brief Represents DHCPv4 packet
+///
+/// This class represents a single DHCPv4 packet. It handles both incoming
+/// and transmitted packets, parsing incoming options, options handling
+/// (add, get, remove), on-wire assembly, sanity checks and other operations.
+/// This specific class has several DHCPv4-specific methods, but it uses a lot
+/// of common operations from its base @c Pkt class that is shared with Pkt6.
+class Pkt4 : public Pkt {
 public:
 
     /// length of the CHADDR field in DHCPv4 message
@@ -77,8 +83,7 @@ public:
     /// The buffer_out_ is cleared before writting to the buffer.
     ///
     /// @throw InvalidOperation if packing fails
-    void
-    pack();
+    virtual void pack();
 
     /// @brief Parses on-wire form of DHCPv4 packet.
     ///
@@ -88,7 +93,7 @@ public:
     /// be stored in options_ container.
     ///
     /// Method with throw exception if packet parsing fails.
-    void unpack();
+    virtual void unpack();
 
     /// @brief performs sanity check on a packet.
     ///
@@ -103,14 +108,6 @@ public:
     /// Method will throw exception if anomaly is found.
     void check();
 
-    /// @brief Copies content of input buffer to output buffer.
-    ///
-    /// This is mostly a diagnostic function. It is being used for sending
-    /// received packet. Received packet is stored in bufferIn_, but
-    /// transmitted data is stored in buffer_out_. If we want to send packet
-    /// that we just received, a copy between those two buffers is necessary.
-    void repack();
-
     /// @brief Returns text representation of the packet.
     ///
     /// This function is useful mainly for debugging.
@@ -164,7 +161,6 @@ public:
     /// @return flags field
     uint16_t getFlags() const { return (flags_); };
 
-
     /// @brief Returns ciaddr field.
     ///
     /// @return ciaddr field
@@ -190,7 +186,6 @@ public:
     void
     setSiaddr(const isc::asiolink::IOAddress& siaddr) { siaddr_ = siaddr; };
 
-
     /// @brief Returns yiaddr field.
     ///
     /// @return yiaddr field
@@ -203,7 +198,6 @@ public:
     void
     setYiaddr(const isc::asiolink::IOAddress& yiaddr) { yiaddr_ = yiaddr; };
 
-
     /// @brief Returns giaddr field.
     ///
     /// @return giaddr field
@@ -216,16 +210,6 @@ public:
     void
     setGiaddr(const isc::asiolink::IOAddress& giaddr) { giaddr_ = giaddr; };
 
-    /// @brief Sets transaction-id value
-    ///
-    /// @param transid transaction-id to be set.
-    void setTransid(uint32_t transid) { transid_ = transid; }
-
-    /// @brief Returns value of transaction-id field.
-    ///
-    /// @return transaction-id
-    uint32_t getTransid() const { return (transid_); };
-
     /// @brief Returns DHCP message type (e.g. 1 = DHCPDISCOVER).
     ///
     /// @return message type
@@ -304,108 +288,13 @@ public:
     /// @return hardware address structure
     HWAddrPtr getHWAddr() const { return (hwaddr_); }
 
-    /// @brief Returns reference to output buffer.
-    ///
-    /// Returned buffer will contain reasonable data only for
-    /// output (TX) packet and after pack() was called. This buffer
-    /// is only valid till Pkt4 object is valid.
-    ///
-    /// RX packet or TX packet before pack() will return buffer with
-    /// zero length. This buffer is returned as non-const, so hooks
-    /// framework (and user's callouts) can modify them if needed
-    ///
-    /// @return reference to output buffer
-    isc::util::OutputBuffer&
-    getBuffer() { return (buffer_out_); };
-
     /// @brief Add an option.
     ///
-    /// Throws BadValue if option with that type is already present.
+    /// @throw BadValue if option with that type is already present.
     ///
     /// @param opt option to be added
-    void
-    addOption(boost::shared_ptr<Option> opt);
-
-    /// @brief Returns an option of specified type.
-    ///
-    /// @return returns option of requested type (or NULL)
-    ///         if no such option is present
-    boost::shared_ptr<Option>
-    getOption(uint8_t opt_type) const;
-
-    /// @brief Deletes specified option
-    /// @param type option type to be deleted
-    /// @return true if anything was deleted, false otherwise
-    bool delOption(uint8_t type);
-
-    /// @brief Returns interface name.
-    ///
-    /// Returns interface name over which packet was received or is
-    /// going to be transmitted.
-    ///
-    /// @return interface name
-    std::string getIface() const { return iface_; };
-
-    /// @brief Returns packet timestamp.
-    ///
-    /// Returns packet timestamp value updated when
-    /// packet is received or send.
-    ///
-    /// @return packet timestamp.
-    const boost::posix_time::ptime& getTimestamp() const { return timestamp_; }
-
-    /// @brief Sets interface name.
-    ///
-    /// Sets interface name over which packet was received or is
-    /// going to be transmitted.
-    ///
-    /// @return interface name
-    void setIface(const std::string& iface ) { iface_ = iface; };
-
-    /// @brief Sets interface index.
-    ///
-    /// @param ifindex specifies interface index.
-    void setIndex(uint32_t ifindex) { ifindex_ = ifindex; };
-
-    /// @brief Returns interface index.
-    ///
-    /// @return interface index
-    uint32_t getIndex() const { return (ifindex_); };
-
-    /// @brief Sets remote HW address.
-    ///
-    /// Sets the destination HW address for the outgoing packet
-    /// or source HW address for the incoming packet. When this
-    /// is an outgoing packet this address will be used to construct
-    /// the link layer header.
-    ///
-    /// @note mac_addr must be a buffer of at least hlen bytes.
-    ///
-    /// @param htype hardware type (will be sent in htype field)
-    /// @param hlen hardware length (will be sent in hlen field)
-    /// @param mac_addr pointer to hardware address
-    void setRemoteHWAddr(const uint8_t htype, const uint8_t hlen,
-                         const std::vector<uint8_t>& mac_addr);
-
-    /// @brief Sets remote HW address.
-    ///
-    /// Sets hardware address from an existing HWAddr structure.
-    /// The remote address is a destination address for outgoing
-    /// packet and source address for incoming packet. When this
-    /// is an outgoing packet, this address will be used to
-    /// construct the link layer header.
-    ///
-    /// @param addr structure representing HW address.
-    ///
-    /// @throw BadValue if addr is null
-    void setRemoteHWAddr(const HWAddrPtr& addr);
-
-    /// @brief Returns the remote HW address.
-    ///
-    /// @return remote HW address.
-    HWAddrPtr getRemoteHWAddr() const {
-        return (remote_hwaddr_);
-    }
+    virtual void
+    addOption(const OptionPtr& opt);
 
     /// @brief Sets local HW address.
     ///
@@ -438,54 +327,6 @@ public:
         return (local_hwaddr_);
     }
 
-    /// @brief Sets remote address.
-    ///
-    /// @param remote specifies remote address
-    void setRemoteAddr(const isc::asiolink::IOAddress& remote) {
-        remote_addr_ = remote;
-    }
-
-    /// @brief Returns remote address
-    ///
-    /// @return remote address
-    const isc::asiolink::IOAddress& getRemoteAddr() const {
-        return (remote_addr_);
-    }
-
-    /// @brief Sets local address.
-    ///
-    /// @param local specifies local address
-    void setLocalAddr(const isc::asiolink::IOAddress& local) {
-        local_addr_ = local;
-    }
-
-    /// @brief Returns local address.
-    ///
-    /// @return local address
-    const isc::asiolink::IOAddress& getLocalAddr() const {
-        return (local_addr_);
-    }
-
-    /// @brief Sets local port.
-    ///
-    /// @param local specifies local port
-    void setLocalPort(uint16_t local) { local_port_ = local; }
-
-    /// @brief Returns local port.
-    ///
-    /// @return local port
-    uint16_t getLocalPort() const { return (local_port_); }
-
-    /// @brief Sets remote port.
-    ///
-    /// @param remote specifies remote port
-    void setRemotePort(uint16_t remote) { remote_port_ = remote; }
-
-    /// @brief Returns remote port.
-    ///
-    /// @return remote port
-    uint16_t getRemotePort() const { return (remote_port_); }
-
     /// @brief Checks if a DHCPv4 message has been relayed.
     ///
     /// This function returns a boolean value which indicates whether a DHCPv4
@@ -504,36 +345,6 @@ public:
     /// found.
     bool isRelayed() const;
 
-    /// @brief Set callback function to be used to parse options.
-    ///
-    /// @param callback An instance of the callback function or NULL to
-    /// uninstall callback.
-    void setCallback(UnpackOptionsCallback callback) {
-        callback_ = callback;
-    }
-
-    /// @brief Update packet timestamp.
-    ///
-    /// Updates packet timestamp. This method is invoked
-    /// by interface manager just before sending or
-    /// just after receiving it.
-    /// @throw isc::Unexpected if timestamp update failed
-    void updateTimestamp();
-
-    /// Output buffer (used during message transmission)
-    ///
-    /// @warning This public member is accessed by derived
-    /// classes directly. One of such derived classes is
-    /// @ref perfdhcp::PerfPkt4. The impact on derived clasess'
-    /// behavior must be taken into consideration before making
-    /// changes to this member such as access scope restriction or
-    /// data format change etc. This field is also public, because
-    /// it may be modified by callouts (which are written in C++ now,
-    /// but we expect to also have them in Python, so any accesibility
-    /// methods would overly complicate things here and degrade
-    /// performance).
-    isc::util::OutputBuffer buffer_out_;
-
     /// @brief That's the data of input buffer used in RX packet.
     ///
     /// @note Note that InputBuffer does not store the data itself, but just
@@ -552,37 +363,6 @@ public:
     /// performance).
     std::vector<uint8_t> data_;
 
-    /// @brief Checks whether a client belongs to a given class
-    ///
-    /// @param client_class name of the class
-    /// @return true if belongs
-    bool inClass(const isc::dhcp::ClientClass& client_class);
-
-    /// @brief Adds packet to a specified class
-    ///
-    /// A packet can be added to the same class repeatedly. Any additional
-    /// attempts to add to a class the packet already belongs to, will be
-    /// ignored silently.
-    ///
-    /// @note It is a matter of naming convention. Conceptually, the server
-    /// processes a stream of packets, with some packets belonging to given
-    /// classes. From that perspective, this method adds a packet to specifed
-    /// class. Implementation wise, it looks the opposite - the class name
-    /// is added to the packet. Perhaps the most appropriate name for this
-    /// method would be associateWithClass()? But that seems overly long,
-    /// so I decided to stick with addClass().
-    ///
-    /// @param client_class name of the class to be added
-    void addClass(const isc::dhcp::ClientClass& client_class);
-
-    /// @brief Classes this packet belongs to.
-    ///
-    /// This field is public, so the code outside of Pkt4 class can iterate over
-    /// existing classes. Having it public also solves the problem of returned
-    /// reference lifetime. It is preferred to use @ref inClass and @ref addClass
-    /// should be used to operate on this field.
-    ClientClasses classes_;
-
 private:
 
     /// @brief Generic method that validates and sets HW address.
@@ -596,9 +376,9 @@ private:
     /// @param [out] hw_addr pointer to a class member to be modified.
     ///
     /// @trow isc::OutOfRange if invalid HW address specified.
-    void setHWAddrMember(const uint8_t htype, const uint8_t hlen,
-                         const std::vector<uint8_t>& mac_addr,
-                         HWAddrPtr& hw_addr);
+    virtual void setHWAddrMember(const uint8_t htype, const uint8_t hlen,
+                                 const std::vector<uint8_t>& mac_addr,
+                                 HWAddrPtr& hw_addr);
 
 protected:
 
@@ -613,31 +393,6 @@ protected:
     /// local HW address (dst if receiving packet, src if sending packet)
     HWAddrPtr local_hwaddr_;
 
-    // remote HW address (src if receiving packet, dst if sending packet)
-    HWAddrPtr remote_hwaddr_;
-
-    /// local address (dst if receiving packet, src if sending packet)
-    isc::asiolink::IOAddress local_addr_;
-
-    /// remote address (src if receiving packet, dst if sending packet)
-    isc::asiolink::IOAddress remote_addr_;
-
-    /// name of the network interface the packet was received/to be sent over
-    std::string iface_;
-
-    /// @brief interface index
-    ///
-    /// Each network interface has assigned unique ifindex. It is functional
-    /// equivalent of name, but sometimes more useful, e.g. when using crazy
-    /// systems that allow spaces in interface names e.g. MS Windows)
-    uint32_t ifindex_;
-
-    /// local UDP port
-    uint16_t local_port_;
-
-    /// remote UDP port
-    uint16_t remote_port_;
-
     /// @brief message operation code
     ///
     /// Note: This is legacy BOOTP field. There's no need to manipulate it
@@ -656,9 +411,6 @@ protected:
     /// Number of relay agents traversed
     uint8_t hops_;
 
-    /// DHCPv4 transaction-id (32 bits, not 24 bits as in DHCPv6)
-    uint32_t transid_;
-
     /// elapsed (number of seconds since beginning of transmission)
     uint16_t secs_;
 
@@ -684,25 +436,9 @@ protected:
     uint8_t file_[MAX_FILE_LEN];
 
     // end of real DHCPv4 fields
-
-    /// collection of options present in this message
-    ///
-    /// @warning This protected member is accessed by derived
-    /// classes directly. One of such derived classes is
-    /// @ref perfdhcp::PerfPkt4. The impact on derived classes'
-    /// behavior must be taken into consideration before making
-    /// changes to this member such as access scope restriction or
-    /// data format change etc.
-    isc::dhcp::OptionCollection options_;
-
-    /// packet timestamp
-    boost::posix_time::ptime timestamp_;
-
-    /// A callback to be called to unpack options from the packet.
-    UnpackOptionsCallback callback_;
-
 }; // Pkt4 class
 
+/// @brief A pointer to Pkt4 object.
 typedef boost::shared_ptr<Pkt4> Pkt4Ptr;
 
 } // isc::dhcp namespace

+ 93 - 149
src/lib/dhcp/pkt6.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
@@ -24,44 +24,28 @@
 using namespace std;
 using namespace isc::asiolink;
 
+/// @brief Default address used in Pkt6 constructor
+const IOAddress DEFAULT_ADDRESS6("::");
+
 namespace isc {
 namespace dhcp {
 
 Pkt6::RelayInfo::RelayInfo()
-    :msg_type_(0), hop_count_(0), linkaddr_("::"), peeraddr_("::"), relay_msg_len_(0) {
-    // interface_id_, subscriber_id_, remote_id_ initialized to NULL
-    // echo_options_ initialized to empty collection
+    :msg_type_(0), hop_count_(0), linkaddr_(DEFAULT_ADDRESS6),
+    peeraddr_(DEFAULT_ADDRESS6), relay_msg_len_(0) {
 }
 
-Pkt6::Pkt6(const uint8_t* buf, uint32_t buf_len, DHCPv6Proto proto /* = UDP */) :
-    proto_(proto),
-    msg_type_(0),
-    transid_(rand()%0xffffff),
-    iface_(""),
-    ifindex_(-1),
-    local_addr_("::"),
-    remote_addr_("::"),
-    local_port_(0),
-    remote_port_(0),
-    buffer_out_(0) {
-    data_.resize(buf_len);
-    memcpy(&data_[0], buf, buf_len);
+Pkt6::Pkt6(const uint8_t* buf, uint32_t buf_len, DHCPv6Proto proto /* = UDP */)
+   :Pkt(buf, buf_len, DEFAULT_ADDRESS6, DEFAULT_ADDRESS6, 0, 0),
+    proto_(proto), msg_type_(0) {
 }
 
-Pkt6::Pkt6(uint8_t msg_type, uint32_t transid, DHCPv6Proto proto /*= UDP*/) :
-    proto_(proto),
-    msg_type_(msg_type),
-    transid_(transid),
-    iface_(""),
-    ifindex_(-1),
-    local_addr_("::"),
-    remote_addr_("::"),
-    local_port_(0),
-    remote_port_(0),
-    buffer_out_(0) {
+Pkt6::Pkt6(uint8_t msg_type, uint32_t transid, DHCPv6Proto proto /*= UDP*/)
+:Pkt(transid, DEFAULT_ADDRESS6, DEFAULT_ADDRESS6, 0, 0), proto_(proto),
+    msg_type_(msg_type) {
 }
 
-uint16_t Pkt6::len() {
+size_t Pkt6::len() {
     if (relay_info_.empty()) {
         return (directLen());
     } else {
@@ -128,7 +112,6 @@ OptionPtr Pkt6::getAnyRelayOption(uint16_t opt_type, RelaySearchOrder order) {
     return (OptionPtr());
 }
 
-
 OptionPtr Pkt6::getRelayOption(uint16_t opt_type, uint8_t relay_level) {
     if (relay_level >= relay_info_.size()) {
         isc_throw(OutOfRange, "This message was relayed " << relay_info_.size() << " time(s)."
@@ -268,7 +251,7 @@ Pkt6::packTCP() {
               "not implemented yet.");
 }
 
-bool
+void
 Pkt6::unpack() {
     switch (proto_) {
     case UDP:
@@ -278,15 +261,13 @@ Pkt6::unpack() {
     default:
         isc_throw(BadValue, "Invalid protocol specified (non-TCP, non-UDP)");
     }
-    return (false); // never happens
 }
 
-bool
+void
 Pkt6::unpackUDP() {
     if (data_.size() < 4) {
-        // @todo: throw exception here informing that packet is truncated
-        // once we turn this function to void.
-        return (false);
+        isc_throw(BadValue, "Received truncated UDP DHCPv6 packet of size "
+                  << data_.size() << ", DHCPv6 header alone has 4 bytes.");
     }
     msg_type_ = data_[0];
     switch (msg_type_) {
@@ -310,12 +291,14 @@ Pkt6::unpackUDP() {
     }
 }
 
-bool
+void
 Pkt6::unpackMsg(OptionBuffer::const_iterator begin,
                 OptionBuffer::const_iterator end) {
-    if (std::distance(begin, end) < 4) {
+    size_t size = std::distance(begin, end);
+    if (size < 4) {
         // truncated message (less than 4 bytes)
-        return (false);
+        isc_throw(BadValue, "Received truncated UDP DHCPv6 packet of size "
+                  << data_.size() << ", DHCPv6 header alone has 4 bytes.");
     }
 
     msg_type_ = *begin++;
@@ -324,27 +307,40 @@ Pkt6::unpackMsg(OptionBuffer::const_iterator begin,
         ((*begin++) << 8) + (*begin++);
     transid_ = transid_ & 0xffffff;
 
-    try {
-        OptionBuffer opt_buffer(begin, end);
+    size -= sizeof(uint32_t); // We just parsed 4 bytes header
 
-        // If custom option parsing function has been set, use this function
-        // to parse options. Otherwise, use standard function from libdhcp.
-        if (callback_.empty()) {
-            LibDHCP::unpackOptions6(opt_buffer, "dhcp6", options_);
-        } else {
-            // The last two arguments hold the DHCPv6 Relay message offset and
-            // length. Setting them to NULL because we are dealing with the
-            // not-relayed message.
-            callback_(opt_buffer, "dhcp6", options_, NULL, NULL);
-        }
-    } catch (const Exception& e) {
-        // @todo: throw exception here once we turn this function to void.
-        return (false);
+    OptionBuffer opt_buffer(begin, end);
+
+    // If custom option parsing function has been set, use this function
+    // to parse options. Otherwise, use standard function from libdhcp.
+    size_t offset;
+    if (callback_.empty()) {
+        offset = LibDHCP::unpackOptions6(opt_buffer, "dhcp6", options_);
+    } else {
+        // The last two arguments hold the DHCPv6 Relay message offset and
+        // length. Setting them to NULL because we are dealing with the
+        // not-relayed message.
+        offset = callback_(opt_buffer, "dhcp6", options_, NULL, NULL);
     }
-    return (true);
+
+    // If offset is not equal to the size, then something is wrong here. We
+    // either parsed past input buffer (bug in our code) or we haven't parsed
+    // everything (received trailing garbage or truncated option).
+    //
+    // Invoking Jon Postel's law here: be conservative in what you send, and be
+    // liberal in what you accept. There's no easy way to log something from
+    // libdhcp++ library, so we just choose to be silent about remaining
+    // bytes. We also need to quell compiler warning about unused offset
+    // variable.
+    //
+    // if (offset != size) {
+    //        isc_throw(BadValue, "Received DHCPv6 buffer of size " << size
+    //                  << ", were able to parse " << offset << " bytes.");
+    // }
+    (void)offset;
 }
 
-bool
+void
 Pkt6::unpackRelayMsg() {
 
     // we use offset + bufsize, because we want to avoid creating unnecessary
@@ -370,67 +366,60 @@ Pkt6::unpackRelayMsg() {
         offset += isc::asiolink::V6ADDRESS_LEN;
         bufsize -= DHCPV6_RELAY_HDR_LEN; // 34 bytes (1+1+16+16)
 
-        try {
-            // parse the rest as options
-            OptionBuffer opt_buffer(&data_[offset], &data_[offset+bufsize]);
-
-            // If custom option parsing function has been set, use this function
-            // to parse options. Otherwise, use standard function from libdhcp.
-            if (callback_.empty()) {
-                LibDHCP::unpackOptions6(opt_buffer, "dhcp6", relay.options_,
-                                        &relay_msg_offset, &relay_msg_len);
-            } else {
-                callback_(opt_buffer, "dhcp6", relay.options_,
-                          &relay_msg_offset, &relay_msg_len);
-            }
+        // parse the rest as options
+        OptionBuffer opt_buffer(&data_[offset], &data_[offset+bufsize]);
 
-            /// @todo: check that each option appears at most once
-            //relay.interface_id_ = options->getOption(D6O_INTERFACE_ID);
-            //relay.subscriber_id_ = options->getOption(D6O_SUBSCRIBER_ID);
-            //relay.remote_id_ = options->getOption(D6O_REMOTE_ID);
+        // If custom option parsing function has been set, use this function
+        // to parse options. Otherwise, use standard function from libdhcp.
+        if (callback_.empty()) {
+            LibDHCP::unpackOptions6(opt_buffer, "dhcp6", relay.options_,
+                                    &relay_msg_offset, &relay_msg_len);
+        } else {
+            callback_(opt_buffer, "dhcp6", relay.options_,
+                      &relay_msg_offset, &relay_msg_len);
+        }
 
-            if (relay_msg_offset == 0 || relay_msg_len == 0) {
-                isc_throw(BadValue, "Mandatory relay-msg option missing");
-            }
+        /// @todo: check that each option appears at most once
+        //relay.interface_id_ = options->getOption(D6O_INTERFACE_ID);
+        //relay.subscriber_id_ = options->getOption(D6O_SUBSCRIBER_ID);
+        //relay.remote_id_ = options->getOption(D6O_REMOTE_ID);
 
-            // store relay information parsed so far
-            addRelayInfo(relay);
+        if (relay_msg_offset == 0 || relay_msg_len == 0) {
+            isc_throw(BadValue, "Mandatory relay-msg option missing");
+        }
 
-            /// @todo: implement ERO here
+        // store relay information parsed so far
+        addRelayInfo(relay);
 
-            if (relay_msg_len >= bufsize) {
-                // length of the relay_msg option extends beyond end of the message
-                isc_throw(Unexpected, "Relay-msg option is truncated.");
-                return false;
-            }
-            uint8_t inner_type = data_[offset + relay_msg_offset];
-            offset += relay_msg_offset; // offset is relative
-            bufsize = relay_msg_len;    // length is absolute
-
-            if ( (inner_type != DHCPV6_RELAY_FORW) &&
-                 (inner_type != DHCPV6_RELAY_REPL)) {
-                // Ok, the inner message is not encapsulated, let's decode it
-                // directly
-                return (unpackMsg(data_.begin() + offset, data_.begin() + offset
-                                  + relay_msg_len));
-            }
+        /// @todo: implement ERO (Echo Request Option, RFC 4994) here
 
-            // Oh well, there's inner relay-forw or relay-repl inside. Let's
-            // unpack it as well. The next loop iteration will take care
-            // of that.
-        } catch (const Exception& e) {
-            /// @todo: throw exception here once we turn this function to void.
-            return (false);
+        if (relay_msg_len >= bufsize) {
+            // length of the relay_msg option extends beyond end of the message
+            isc_throw(Unexpected, "Relay-msg option is truncated.");
+        }
+        uint8_t inner_type = data_[offset + relay_msg_offset];
+        offset += relay_msg_offset; // offset is relative
+        bufsize = relay_msg_len;    // length is absolute
+
+        if ( (inner_type != DHCPV6_RELAY_FORW) &&
+             (inner_type != DHCPV6_RELAY_REPL)) {
+            // Ok, the inner message is not encapsulated, let's decode it
+            // directly
+            return (unpackMsg(data_.begin() + offset, data_.begin() + offset
+                              + relay_msg_len));
         }
+
+        // Oh well, there's inner relay-forw or relay-repl inside. Let's
+        // unpack it as well. The next loop iteration will take care
+        // of that.
     }
 
     if ( (offset == data_.size()) && (bufsize == 0) ) {
         // message has been parsed completely
-        return (true);
+        return;
     }
 
     /// @todo: log here that there are additional unparsed bytes
-    return (true);
 }
 
 void
@@ -443,7 +432,7 @@ Pkt6::addRelayInfo(const RelayInfo& relay) {
     relay_info_.push_back(relay);
 }
 
-bool
+void
 Pkt6::unpackTCP() {
     isc_throw(Unexpected, "DHCPv6 over TCP (bulk leasequery and failover) "
               "not implemented yet.");
@@ -466,15 +455,6 @@ Pkt6::toText() {
     return tmp.str();
 }
 
-OptionPtr
-Pkt6::getOption(uint16_t opt_type) {
-    isc::dhcp::OptionCollection::const_iterator x = options_.find(opt_type);
-    if (x!=options_.end()) {
-        return (*x).second;
-    }
-    return OptionPtr(); // NULL
-}
-
 isc::dhcp::OptionCollection
 Pkt6::getOptions(uint16_t opt_type) {
     isc::dhcp::OptionCollection found;
@@ -488,30 +468,6 @@ Pkt6::getOptions(uint16_t opt_type) {
     return (found);
 }
 
-void
-Pkt6::addOption(const OptionPtr& opt) {
-    options_.insert(pair<int, boost::shared_ptr<Option> >(opt->getType(), opt));
-}
-
-bool
-Pkt6::delOption(uint16_t type) {
-    isc::dhcp::OptionCollection::iterator x = options_.find(type);
-    if (x!=options_.end()) {
-        options_.erase(x);
-        return (true); // delete successful
-    }
-    return (false); // can't find option to be deleted
-}
-
-void Pkt6::repack() {
-    buffer_out_.writeData(&data_[0], data_.size());
-}
-
-void
-Pkt6::updateTimestamp() {
-    timestamp_ = boost::posix_time::microsec_clock::universal_time();
-}
-
 const char*
 Pkt6::getName(uint8_t type) {
     static const char* CONFIRM = "CONFIRM";
@@ -586,17 +542,5 @@ void Pkt6::copyRelayInfo(const Pkt6Ptr& question) {
     }
 }
 
-bool
-Pkt6::inClass(const std::string& client_class) {
-    return (classes_.find(client_class) != classes_.end());
-}
-
-void
-Pkt6::addClass(const std::string& client_class) {
-    if (classes_.find(client_class) == classes_.end()) {
-        classes_.insert(client_class);
-    }
-}
-
 } // end of isc::dhcp namespace
 } // end of isc namespace

+ 45 - 266
src/lib/dhcp/pkt6.h

@@ -17,7 +17,7 @@
 
 #include <asiolink/io_address.h>
 #include <dhcp/option.h>
-#include <dhcp/classify.h>
+#include <dhcp/pkt.h>
 
 #include <boost/date_time/posix_time/posix_time.hpp>
 #include <boost/shared_array.hpp>
@@ -33,9 +33,22 @@ namespace isc {
 namespace dhcp {
 
 class Pkt6;
+
+/// @brief A pointer to Pkt6 packet
 typedef boost::shared_ptr<Pkt6> Pkt6Ptr;
 
-class Pkt6 {
+/// @brief Represents a DHCPv6 packet
+///
+/// This class represents a single DHCPv6 packet. It handles both incoming
+/// and transmitted packets, parsing incoming options, options handling
+/// (add, get, remove), on-wire assembly, sanity checks and other operations.
+/// This specific class has several DHCPv6-specific methods, but it uses a lot
+/// of common operations from its base @c Pkt class that is shared with Pkt4.
+///
+/// This class also handles relayed packets. For example, a RELAY-FORW message
+/// with a SOLICIT inside will be represented as SOLICIT and the RELAY-FORW
+/// layers will be stored in relay_info_ vector.
+class Pkt6 : public Pkt {
 public:
     /// specifies non-relayed DHCPv6 packet header length (over UDP)
     const static size_t DHCPV6_PKT_HDR_LEN = 4;
@@ -122,44 +135,34 @@ public:
     /// @throw BadValue if packet protocol is invalid, InvalidOperation
     /// if packing fails, or NotImplemented if protocol is TCP (IPv6 over TCP is
     /// not yet supported).
-    void pack();
+    virtual void pack();
 
     /// @brief Dispatch method that handles binary packet parsing.
     ///
     /// This method calls appropriate dispatch function (unpackUDP or
     /// unpackTCP).
     ///
-    /// @return true if parsing was successful
-    bool unpack();
-
-    /// @brief Returns reference to output buffer.
-    ///
-    /// Returned buffer will contain reasonable data only for
-    /// output (TX) packet and after pack() was called. This buffer
-    /// is only valid till Pkt6 object is valid.
-    ///
-    /// RX packet or TX packet before pack() will return buffer with
-    /// zero length
-    ///
-    /// @return reference to output buffer
-    const isc::util::OutputBuffer& getBuffer() const { return (buffer_out_); };
+    /// @throw tbd
+    virtual void unpack();
 
     /// @brief Returns protocol of this packet (UDP or TCP).
     ///
     /// @return protocol type
     DHCPv6Proto getProto();
 
-    /// Sets protocol of this packet.
+    /// @brief Sets protocol of this packet.
     ///
     /// @param proto protocol (UDP or TCP)
-    void setProto(DHCPv6Proto proto = UDP) { proto_ = proto; }
+    void setProto(DHCPv6Proto proto = UDP) {
+        proto_ = proto;
+    }
 
     /// @brief Returns text representation of the packet.
     ///
     /// This function is useful mainly for debugging.
     ///
     /// @return string with text representation
-    std::string toText();
+    virtual std::string toText();
 
     /// @brief Returns length of the packet.
     ///
@@ -171,43 +174,17 @@ public:
     /// before they are unpacked.
     ///
     /// @return number of bytes required to assemble this packet
-    uint16_t len();
+    virtual size_t len();
 
-    /// Returns message type (e.g. 1 = SOLICIT)
+    /// @brief Returns message type (e.g. 1 = SOLICIT).
     ///
     /// @return message type
-    uint8_t getType() const { return (msg_type_); }
+    virtual uint8_t getType() const { return (msg_type_); }
 
-    /// Sets message type (e.g. 1 = SOLICIT)
+    /// @brief Sets message type (e.g. 1 = SOLICIT).
     ///
     /// @param type message type to be set
-    void setType(uint8_t type) { msg_type_=type; };
-
-    /// @brief Sets transaction-id value
-    ///
-    /// @param transid transaction-id to be set.
-    void setTransid(uint32_t transid) { transid_ = transid; }
-
-    /// Returns value of transaction-id field
-    ///
-    /// @return transaction-id
-    uint32_t getTransid() const { return (transid_); };
-
-    /// Adds an option to this packet.
-    ///
-    /// @param opt option to be added.
-    void addOption(const OptionPtr& opt);
-
-    /// @brief Returns the first option of specified type.
-    ///
-    /// Returns the first option of specified type. Note that in DHCPv6 several
-    /// instances of the same option are allowed (and frequently used).
-    /// Also see \ref getOptions().
-    ///
-    /// @param type option type we are looking for
-    ///
-    /// @return pointer to found option (or NULL)
-    OptionPtr getOption(uint16_t type);
+    virtual void setType(uint8_t type) { msg_type_=type; };
 
     /// @brief returns option inserted by relay
     ///
@@ -247,96 +224,6 @@ public:
     /// @return instance of option collection with requested options
     isc::dhcp::OptionCollection getOptions(uint16_t type);
 
-    /// Attempts to delete first suboption of requested type
-    ///
-    /// @param type Type of option to be deleted.
-    ///
-    /// @return true if option was deleted, false if no such option existed
-    bool delOption(uint16_t type);
-
-    /// @brief This method copies data from output buffer to input buffer
-    ///
-    /// This is useful only in testing
-    void repack();
-
-    /// @brief Sets remote address.
-    ///
-    /// @param remote specifies remote address
-    void setRemoteAddr(const isc::asiolink::IOAddress& remote) { remote_addr_ = remote; }
-
-    /// @brief Returns remote address
-    ///
-    /// @return remote address
-    const isc::asiolink::IOAddress& getRemoteAddr() const {
-        return (remote_addr_);
-    }
-
-    /// @brief Sets local address.
-    ///
-    /// @param local specifies local address
-    void setLocalAddr(const isc::asiolink::IOAddress& local) { local_addr_ = local; }
-
-    /// @brief Returns local address.
-    ///
-    /// @return local address
-    const isc::asiolink::IOAddress& getLocalAddr() const {
-        return (local_addr_);
-    }
-
-    /// @brief Sets local port.
-    ///
-    /// @param local specifies local port
-    void setLocalPort(uint16_t local) { local_port_ = local; }
-
-    /// @brief Returns local port.
-    ///
-    /// @return local port
-    uint16_t getLocalPort() const { return (local_port_); }
-
-    /// @brief Sets remote port.
-    ///
-    /// @param remote specifies remote port
-    void setRemotePort(uint16_t remote) { remote_port_ = remote; }
-
-    /// @brief Returns remote port.
-    ///
-    /// @return remote port
-    uint16_t getRemotePort() const { return (remote_port_); }
-
-    /// @brief Sets interface index.
-    ///
-    /// @param ifindex specifies interface index.
-    void setIndex(uint32_t ifindex) { ifindex_ = ifindex; };
-
-    /// @brief Returns interface index.
-    ///
-    /// @return interface index
-    uint32_t getIndex() const { return (ifindex_); };
-
-    /// @brief Returns interface name.
-    ///
-    /// Returns interface name over which packet was received or is
-    /// going to be transmitted.
-    ///
-    /// @return interface name
-    std::string getIface() const { return iface_; };
-
-    /// @brief Returns packet timestamp.
-    ///
-    /// Returns packet timestamp value updated when
-    /// packet is received or sent.
-    ///
-    /// @return packet timestamp.
-    const boost::posix_time::ptime& getTimestamp() const { return timestamp_; }
-
-    /// @brief Sets interface name.
-    ///
-    /// Sets interface name over which packet was received or is
-    /// going to be transmitted.
-    ///
-    /// @return interface name
-    void setIface(const std::string& iface ) { iface_ = iface; };
-
     /// @brief add information about one traversed relay
     ///
     /// This adds information about one traversed relay, i.e.
@@ -345,24 +232,6 @@ public:
     /// @param relay structure with necessary relay information
     void addRelayInfo(const RelayInfo& relay);
 
-    /// collection of options present in this message
-    ///
-    /// @warning This public member is accessed by derived
-    /// classes directly. One of such derived classes is
-    /// @ref perfdhcp::PerfPkt6. The impact on derived clasess'
-    /// behavior must be taken into consideration before making
-    /// changes to this member such as access scope restriction or
-    /// data format change etc.
-    isc::dhcp::OptionCollection options_;
-
-    /// @brief Update packet timestamp.
-    ///
-    /// Updates packet timestamp. This method is invoked
-    /// by interface manager just before sending or
-    /// just after receiving it.
-    /// @throw isc::Unexpected if timestamp update failed
-    void updateTimestamp();
-
     /// @brief Return textual type of packet.
     ///
     /// Returns the name of valid packet received by the server (e.g. SOLICIT).
@@ -391,14 +260,6 @@ public:
     ///         be freed by the caller.
     const char* getName() const;
 
-    /// @brief Set callback function to be used to parse options.
-    ///
-    /// @param callback An instance of the callback function or NULL to
-    /// uninstall callback.
-    void setCallback(UnpackOptionsCallback callback) {
-        callback_ = callback;
-    }
-
     /// @brief copies relay information from client's packet to server's response
     ///
     /// This information is not simply copied over. Some parameter are
@@ -417,55 +278,15 @@ public:
     /// (or least bad) solution.
     std::vector<RelayInfo> relay_info_;
 
-
-    /// unparsed data (in received packets)
-    ///
-    /// @warning This public member is accessed by derived
-    /// classes directly. One of such derived classes is
-    /// @ref perfdhcp::PerfPkt6. The impact on derived clasess'
-    /// behavior must be taken into consideration before making
-    /// changes to this member such as access scope restriction or
-    /// data format change etc.
-    OptionBuffer data_;
-
-    /// @brief Checks whether a client belongs to a given class
-    ///
-    /// @param client_class name of the class
-    /// @return true if belongs
-    bool inClass(const std::string& client_class);
-
-    /// @brief Adds packet to a specified class
-    ///
-    /// A packet can be added to the same class repeatedly. Any additional
-    /// attempts to add to a class the packet already belongs to, will be
-    /// ignored silently.
-    ///
-    /// @note It is a matter of naming convention. Conceptually, the server
-    /// processes a stream of packets, with some packets belonging to given
-    /// classes. From that perspective, this method adds a packet to specifed
-    /// class. Implementation wise, it looks the opposite - the class name
-    /// is added to the packet. Perhaps the most appropriate name for this
-    /// method would be associateWithClass()? But that seems overly long,
-    /// so I decided to stick with addClass().
-    ///
-    /// @param client_class name of the class to be added
-    void addClass(const std::string& client_class);
-
-    /// @brief Classes this packet belongs to.
-    ///
-    /// This field is public, so code can iterate over existing classes.
-    /// Having it public also solves the problem of returned reference lifetime.
-    ClientClasses classes_;
-
 protected:
-    /// Builds on wire packet for TCP transmission.
+    /// @brief Builds on wire packet for TCP transmission.
     ///
-    /// TODO This function is not implemented yet.
+    /// @todo This function is not implemented yet.
     ///
     /// @throw NotImplemented, IPv6 over TCP is not yet supported.
     void packTCP();
 
-    /// Builds on wire packet for UDP transmission.
+    /// @brief Builds on wire packet for UDP transmission.
     ///
     /// @throw InvalidOperation if packing fails
     void packUDP();
@@ -477,10 +298,10 @@ protected:
     /// Will create a collection of option objects that will
     /// be stored in options_ container.
     ///
-    /// TODO This function is not implemented yet.
+    /// @todo This function is not implemented yet.
     ///
-    /// @return true, if build was successful
-    bool unpackTCP();
+    /// @throw tbd
+    void unpackTCP();
 
     /// @brief Parses on-wire form of UDP DHCPv6 packet.
     ///
@@ -489,10 +310,10 @@ protected:
     /// Will create a collection of option objects that will
     /// be stored in options_ container.
     ///
-    /// @return true, if build was successful
-    bool unpackUDP();
+    /// @throw tbd
+    void unpackUDP();
 
-    /// @brief unpacks direct (non-relayed) message
+    /// @brief Unpacks direct (non-relayed) message.
     ///
     /// This method unpacks specified buffer range as a direct
     /// (e.g. solicit or request) message. This method is called from
@@ -500,31 +321,31 @@ protected:
     ///
     /// @param begin start of the buffer
     /// @param end end of the buffer
-    /// @return true if parsing was successful and there are no leftover bytes
-    bool unpackMsg(OptionBuffer::const_iterator begin,
+    /// @throw tbd
+    void unpackMsg(OptionBuffer::const_iterator begin,
                    OptionBuffer::const_iterator end);
 
-    /// @brief unpacks relayed message (RELAY-FORW or RELAY-REPL)
+    /// @brief Unpacks relayed message (RELAY-FORW or RELAY-REPL).
     ///
     /// This method is called from unpackUDP() when received message
     /// is detected to be relay-message. It goes iteratively over
     /// all relays (if there are multiple encapsulation levels).
     ///
-    /// @return true if parsing was successful
-    bool unpackRelayMsg();
+    /// @throw tbd
+    void unpackRelayMsg();
 
-    /// @brief calculates overhead introduced in specified relay
+    /// @brief Calculates overhead introduced in specified relay.
     ///
     /// It is used when calculating message size and packing message
     /// @param relay RelayInfo structure that holds information about relay
     /// @return number of bytes needed to store relay information
     uint16_t getRelayOverhead(const RelayInfo& relay) const;
 
-    /// @brief calculates overhead for all relays defined for this message
+    /// @brief Calculates overhead for all relays defined for this message.
     /// @return number of bytes needed to store all relay information
     uint16_t calculateRelaySizes();
 
-    /// @brief calculates size of the message as if it was not relayed at all
+    /// @brief Calculates size of the message as if it was not relayed at all.
     ///
     /// This is equal to len() if the message was not relayed.
     /// @return number of bytes required to store the message
@@ -536,48 +357,6 @@ protected:
     /// DHCPv6 message type
     uint8_t msg_type_;
 
-    /// DHCPv6 transaction-id
-    uint32_t transid_;
-
-    /// name of the network interface the packet was received/to be sent over
-    std::string iface_;
-
-    /// @brief interface index
-    ///
-    /// interface index (each network interface has assigned unique ifindex
-    /// it is functional equivalent of name, but sometimes more useful, e.g.
-    /// when using crazy systems that allow spaces in interface names
-    /// e.g. windows
-    int ifindex_;
-
-    /// local address (dst if receiving packet, src if sending packet)
-    isc::asiolink::IOAddress local_addr_;
-
-    /// remote address (src if receiving packet, dst if sending packet)
-    isc::asiolink::IOAddress remote_addr_;
-
-    /// local TDP or UDP port
-    uint16_t local_port_;
-
-    /// remote TCP or UDP port
-    uint16_t remote_port_;
-
-    /// Output buffer (used during message transmission)
-    ///
-    /// @warning This protected member is accessed by derived
-    /// classes directly. One of such derived classes is
-    /// @ref perfdhcp::PerfPkt6. The impact on derived clasess'
-    /// behavior must be taken into consideration before making
-    /// changes to this member such as access scope restriction or
-    /// data format change etc.
-    isc::util::OutputBuffer buffer_out_;
-
-    /// packet timestamp
-    boost::posix_time::ptime timestamp_;
-
-    /// A callback to be called to unpack options from the packet.
-    UnpackOptionsCallback callback_;
-
 }; // Pkt6 class
 
 } // isc::dhcp namespace

+ 12 - 0
src/lib/dhcp/tests/hwaddr_unittest.cc

@@ -163,4 +163,16 @@ TEST(HWAddrTest, fromText) {
 
 }
 
+// Checks that 16 bits values can be stored in HWaddr
+TEST(HWAddrTest, 16bits) {
+
+    uint8_t data[] = {0, 1, 2, 3, 4, 5};
+    uint16_t htype = 257;
+    HWAddrPtr hw(new HWAddr(data, sizeof(data), htype));
+
+    EXPECT_EQ("hwtype=257 00:01:02:03:04:05", hw->toText());
+
+
+}
+
 } // end of anonymous namespace

+ 27 - 1
src/lib/dhcp/tests/pkt4_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
@@ -851,4 +851,30 @@ TEST_F(Pkt4Test, clientClasses) {
     EXPECT_TRUE(pkt.inClass("foo"));
 }
 
+// Tests whether MAC can be obtained and that MAC sources are not
+// confused.
+TEST_F(Pkt4Test, getMAC) {
+    Pkt4 pkt(DHCPOFFER, 1234);
+
+    // DHCPv4 packet by default doens't have MAC address specified.
+    EXPECT_FALSE(pkt.getMAC(Pkt::MAC_SOURCE_ANY));
+    EXPECT_FALSE(pkt.getMAC(Pkt::MAC_SOURCE_RAW));
+
+    // Let's invent a MAC
+    const uint8_t hw[] = { 2, 4, 6, 8, 10, 12 }; // MAC
+    const uint8_t hw_type = 123; // hardware type
+    HWAddrPtr dummy_hwaddr(new HWAddr(hw, sizeof(hw), hw_type));
+
+    // Now let's pretend that we obtained it from raw sockets
+    pkt.setRemoteHWAddr(dummy_hwaddr);
+
+    // Now we should be able to get something
+    ASSERT_TRUE(pkt.getMAC(Pkt::MAC_SOURCE_ANY));
+    ASSERT_TRUE(pkt.getMAC(Pkt::MAC_SOURCE_RAW));
+
+    // Check that the returned MAC is indeed the expected one
+    ASSERT_TRUE(*dummy_hwaddr == *pkt.getMAC(Pkt::MAC_SOURCE_ANY));
+    ASSERT_TRUE(*dummy_hwaddr == *pkt.getMAC(Pkt::MAC_SOURCE_RAW));
+}
+
 } // end of anonymous namespace

+ 90 - 6
src/lib/dhcp/tests/pkt6_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
@@ -22,6 +22,7 @@
 #include <dhcp/option_int.h>
 #include <dhcp/option_int_array.h>
 #include <dhcp/pkt6.h>
+#include <dhcp/hwaddr.h>
 #include <dhcp/docsis3_option_defs.h>
 #include <util/range_utilities.h>
 
@@ -278,7 +279,7 @@ Pkt6* capture2() {
 TEST_F(Pkt6Test, unpack_solicit1) {
     scoped_ptr<Pkt6> sol(capture1());
 
-    ASSERT_EQ(true, sol->unpack());
+    ASSERT_NO_THROW(sol->unpack());
 
     // Check for length
     EXPECT_EQ(98, sol->len() );
@@ -304,7 +305,7 @@ TEST_F(Pkt6Test, packUnpack) {
     Pkt6Ptr clone = packAndClone();
 
     // Now recreate options list
-    EXPECT_TRUE(clone->unpack());
+    ASSERT_NO_THROW(clone->unpack());
 
     // transid, message-type should be the same as before
     EXPECT_EQ(0x020304, clone->getTransid());
@@ -316,6 +317,63 @@ TEST_F(Pkt6Test, packUnpack) {
     EXPECT_FALSE(clone->getOption(4));
 }
 
+// Checks if the code is able to handle malformed packet
+TEST_F(Pkt6Test, unpackMalformed) {
+    // Get a packet. We're really interested in its on-wire representation only.
+    scoped_ptr<Pkt6> donor(capture1());
+
+    // That's our original content. It should be sane.
+    OptionBuffer orig = donor->data_;
+
+    Pkt6Ptr success(new Pkt6(&orig[0], orig.size()));
+    EXPECT_NO_THROW(success->unpack());
+
+    // Insert trailing garbage.
+    OptionBuffer malform1 = orig;
+    malform1.push_back(123);
+
+    // Let's check a truncated packet. Moderately sane DHCPv6 packet should at
+    // least have four bytes header. Zero bytes is definitely not a valid one.
+    OptionBuffer empty(1); // Let's allocate one byte, so we won't be
+                           // dereferencing and empty buffer.
+
+    Pkt6Ptr empty_pkt(new Pkt6(&empty[0], 0));
+    EXPECT_THROW(empty_pkt->unpack(), isc::BadValue);
+
+    // Neither is 3 bytes long.
+    OptionBuffer shorty;
+    shorty.push_back(DHCPV6_SOLICIT);
+    shorty.push_back(1);
+    shorty.push_back(2);
+    Pkt6Ptr too_short_pkt(new Pkt6(&shorty[0], shorty.size()));
+    EXPECT_THROW(too_short_pkt->unpack(), isc::BadValue);
+
+    // The code should complain about remaining bytes that can't
+    // be parsed.
+    Pkt6Ptr trailing_garbage(new Pkt6(&malform1[0], malform1.size()));
+    EXPECT_NO_THROW(trailing_garbage->unpack());
+
+    // A strict approach would assume the code will reject the whole packet,
+    // but we decided to follow Jon Postel's law and be silent about
+    // received malformed or truncated options.
+
+    // Add an option that is truncated
+    OptionBuffer malform2 = orig;
+    malform2.push_back(0);
+    malform2.push_back(123); // 0, 123 - option code = 123
+    malform2.push_back(0);
+    malform2.push_back(1);   // 0, 1 - option length = 1
+    // Option content would go here, but it's missing
+
+    Pkt6Ptr trunc_option(new Pkt6(&malform2[0], malform2.size()));
+
+    // The unpack() operation should succeed...
+    EXPECT_NO_THROW(trunc_option->unpack());
+
+    // ... but there should be no option 123 as it was malformed.
+    EXPECT_FALSE(trunc_option->getOption(123));
+}
+
 // This test verifies that it is possible to specify custom implementation of
 // the option parsing algorithm by installing a callback function.
 TEST_F(Pkt6Test, packUnpackWithCallback) {
@@ -333,7 +391,7 @@ TEST_F(Pkt6Test, packUnpackWithCallback) {
     ASSERT_FALSE(cb.executed_);
 
     // Now recreate options list
-    EXPECT_TRUE(clone->unpack());
+    ASSERT_NO_THROW(clone->unpack());
 
     // An object which holds a callback should now have a flag set which
     // indicates that callback has been called.
@@ -353,7 +411,7 @@ TEST_F(Pkt6Test, packUnpackWithCallback) {
     // By setting the callback to NULL we effectively uninstall the callback.
     clone->setCallback(NULL);
     // Do another unpack.
-    EXPECT_TRUE(clone->unpack());
+    ASSERT_NO_THROW(clone->unpack());
     // Callback should not be executed.
     EXPECT_FALSE(cb.executed_);
 }
@@ -643,7 +701,7 @@ TEST_F(Pkt6Test, relayPack) {
                                     parent->getBuffer().getLength()));
 
     // Now recreate options list
-    EXPECT_TRUE( clone->unpack() );
+    EXPECT_NO_THROW( clone->unpack() );
 
     // transid, message-type should be the same as before
     EXPECT_EQ(parent->getTransid(), parent->getTransid());
@@ -810,4 +868,30 @@ TEST_F(Pkt6Test, clientClasses) {
     EXPECT_TRUE(pkt.inClass("foo"));
 }
 
+// Tests whether MAC can be obtained and that MAC sources are not
+// confused.
+TEST_F(Pkt6Test, getMAC) {
+    Pkt6 pkt(DHCPV6_ADVERTISE, 1234);
+
+    // DHCPv6 packet by default doens't have MAC address specified.
+    EXPECT_FALSE(pkt.getMAC(Pkt::HWADDR_SOURCE_ANY));
+    EXPECT_FALSE(pkt.getMAC(Pkt::HWADDR_SOURCE_RAW));
+
+    // Let's invent a MAC
+    const uint8_t hw[] = { 2, 4, 6, 8, 10, 12 }; // MAC
+    const uint8_t hw_type = 123; // hardware type
+    HWAddrPtr dummy_hwaddr(new HWAddr(hw, sizeof(hw), hw_type));
+
+    // Now let's pretend that we obtained it from raw sockets
+    pkt.setRemoteHWAddr(dummy_hwaddr);
+
+    // Now we should be able to get something
+    ASSERT_TRUE(pkt.getMAC(Pkt::HWADDR_SOURCE_ANY));
+    ASSERT_TRUE(pkt.getMAC(Pkt::HWADDR_SOURCE_RAW));
+
+    // Check that the returned MAC is indeed the expected one
+    ASSERT_TRUE(*dummy_hwaddr == *pkt.getMAC(Pkt::HWADDR_SOURCE_ANY));
+    ASSERT_TRUE(*dummy_hwaddr == *pkt.getMAC(Pkt::HWADDR_SOURCE_RAW));
+}
+
 }