From: Tomek Mrugalski Date: Thu, 11 Dec 2014 12:03:53 +0000 (+0100) Subject: [3548] Fixes, cleanups and unit-tests in Pkt6::getMACFromDUID() X-Git-Tag: trac3504_base~14 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=905d44b0bccc8fb7a87e1d0966babab6d149acf1;p=thirdparty%2Fkea.git [3548] Fixes, cleanups and unit-tests in Pkt6::getMACFromDUID() --- diff --git a/src/lib/dhcp/dhcp6.h b/src/lib/dhcp/dhcp6.h index 4dc485efd1..9b45307c58 100644 --- a/src/lib/dhcp/dhcp6.h +++ b/src/lib/dhcp/dhcp6.h @@ -104,13 +104,8 @@ extern const char *dhcpv6_type_names[]; extern const int dhcpv6_type_name_max; -/* DUID type definitions (RFC3315 section 9). - */ +// DUID type definitions (RFC3315 section 9). // see isc::dhcp::DUID::DUIDType enum in dhcp/duid.h -// #define DUID_LLT 1 -// #define DUID_EN 2 -// #define DUID_LL 3 -// #define DUID_UUID 4 // Define hardware types // Taken from http://www.iana.org/assignments/arp-parameters/ diff --git a/src/lib/dhcp/pkt.cc b/src/lib/dhcp/pkt.cc index 61ee750472..70a17deb64 100755 --- a/src/lib/dhcp/pkt.cc +++ b/src/lib/dhcp/pkt.cc @@ -144,9 +144,13 @@ Pkt::getMAC(uint32_t hw_addr_src) { // Method 2: Extracted from DUID-LLT or DUID-LL if(hw_addr_src & HWADDR_SOURCE_DUID) { mac = getMACFromDUID(); - if (mac){ + if (mac) { return (mac); - } else return (HWAddrPtr()); + } else if (hw_addr_src == HWADDR_SOURCE_DUID) { + // If the only source allowed is DUID then we can skip the other + // methods. + return (HWAddrPtr()); + } } // Method 3: Extracted from source IPv6 link-local address diff --git a/src/lib/dhcp/pkt.h b/src/lib/dhcp/pkt.h index 4321b67645..a07cfa0626 100755 --- a/src/lib/dhcp/pkt.h +++ b/src/lib/dhcp/pkt.h @@ -547,6 +547,19 @@ protected: /// @return hardware address (or NULL) virtual HWAddrPtr getMACFromIPv6RelayOpt() = 0; + + /// @brief Attempts to obtain MAC address from DUID-LL or DUID-LLT + /// + /// This method is called from getMAC(HWADDR_SOURCE_DUID) and should not be + /// called directly. It will attempt to extract MAC address information + /// from DUID if its type is LLT or LL. If this method fails, it will + /// return NULL. + /// + /// @note This is a pure virtual method and must be implemented in + /// the derived classes. The @c Pkt6 class have respective implementation. + /// This method is not applicable to DHCPv4. + /// + /// @return hardware address (or NULL) virtual HWAddrPtr getMACFromDUID() = 0; /// @brief Attempts to convert IPv6 address into MAC. @@ -565,8 +578,6 @@ protected: HWAddrPtr getMACFromIPv6(const isc::asiolink::IOAddress& addr); - - /// Transaction-id (32 bits for v4, 24 bits for v6) uint32_t transid_; diff --git a/src/lib/dhcp/pkt4.h b/src/lib/dhcp/pkt4.h index 43430695b8..b4b191b5fa 100755 --- a/src/lib/dhcp/pkt4.h +++ b/src/lib/dhcp/pkt4.h @@ -414,6 +414,13 @@ protected: return (HWAddrPtr()); } + /// @brief No-op + /// + /// This method returns hardware address extracted from DUID. + /// Currently it is a no-op, even though there's RFC that defines how to + /// use DUID in DHCPv4 (see RFC4361). We may implement it one day. + /// + /// @return always NULL virtual HWAddrPtr getMACFromDUID(){ return (HWAddrPtr()); } diff --git a/src/lib/dhcp/pkt6.cc b/src/lib/dhcp/pkt6.cc index adf4799983..69e3f1faac 100755 --- a/src/lib/dhcp/pkt6.cc +++ b/src/lib/dhcp/pkt6.cc @@ -442,35 +442,50 @@ Pkt6::unpackTCP() { HWAddrPtr Pkt6::getMACFromDUID() { - OptionPtr opt_duid = getOption(D6O_CLIENTID); - uint8_t hlen = opt_duid->getData().size(); - vector hw_addr(hlen, 0); - std::vector duid_data = opt_duid->getData(); - + OptionPtr opt_duid = getOption(D6O_CLIENTID); + if (!opt_duid) { + return (HWAddrPtr()); + } - uint16_t hwtype = 0; - std::memcpy(&hwtype, &duid_data[2], 2); // TODO nie jestem tego pewien + uint8_t hlen = opt_duid->getData().size(); + vector hw_addr(hlen, 0); + std::vector duid_data = opt_duid->getData(); + // Read the first two bytes. That duid type. + uint16_t duid_type = util::readUint16(&duid_data[0], duid_data.size()); - // checking if DUID is LLT or LL type - if ((duid_data[0] == 0x00) && (duid_data[1]== 0x01)){ - hlen -= 8; - std::memcpy(&hw_addr, &duid_data[8], hlen); + switch (duid_type) { + case DUID::DUID_LL: + { + // 2 bytes of duid type, 2 bytes of hardware type and at least + // 1 byte of actual identification + if (duid_data.size() < 5) { + // This duid is truncated. We can't extract anything from it. + return (HWAddrPtr()); + } - } - else if ((duid_data[0] == 0x00) && (duid_data[1]== 0x03)){ - hlen -= 4; - std::memcpy(&hw_addr, &duid_data[4], hlen); - } - // if none of them return NULL - else return HWAddrPtr(); + uint16_t hwtype = util::readUint16(&duid_data[2], duid_data.size() - 2); + return (HWAddrPtr(new HWAddr(&duid_data[4], duid_data.size() - 4, + hwtype))); + } + case DUID::DUID_LLT: + { + // 2 bytes of duid type, 2 bytes of hardware, 4 bytes for timestamp, + // and at least 1 byte of actual identification + if (duid_data.size() < 9) { + // This duid is truncated. We can't extract anything from it. + return (HWAddrPtr()); + } - hw_addr.resize(hlen); - hwaddr_= HWAddrPtr(new HWAddr(hw_addr, hwtype)); - return (hwaddr_); + uint16_t hwtype = util::readUint16(&duid_data[2], duid_data.size() - 2); + return (HWAddrPtr(new HWAddr(&duid_data[8], duid_data.size() - 8, + hwtype))); + } + default: + return (HWAddrPtr()); + } } - std::string Pkt6::toText() { stringstream tmp; diff --git a/src/lib/dhcp/tests/pkt6_unittest.cc b/src/lib/dhcp/tests/pkt6_unittest.cc index a015c04f3b..fd61999df9 100644 --- a/src/lib/dhcp/tests/pkt6_unittest.cc +++ b/src/lib/dhcp/tests/pkt6_unittest.cc @@ -1069,7 +1069,7 @@ TEST_F(Pkt6Test, getMACFromIPv6RelayOpt_singleRelay) { 0x0a, 0x1b, 0x0b, 0x01, 0xca, 0xfe // MAC }; OptionPtr relay_opt(new Option(Option::V6, 79, - OptionBuffer(opt_data, opt_data + sizeof(opt_data)))); + OptionBuffer(opt_data, opt_data + sizeof(opt_data)))); info.options_.insert(make_pair(relay_opt->getType(), relay_opt)); pkt.addRelayInfo(info); @@ -1098,7 +1098,7 @@ TEST_F(Pkt6Test, getMACFromIPv6RelayOpt_multipleRelay) { 0x1a, 0x30, 0x0b, 0xfa, 0xc0, 0xfe // MAC }; OptionPtr relay_opt1(new Option(Option::V6, D6O_CLIENT_LINKLAYER_ADDR, - OptionBuffer(opt_data, opt_data + sizeof(opt_data)))); + OptionBuffer(opt_data, opt_data + sizeof(opt_data)))); info1.options_.insert(make_pair(relay_opt1->getType(), relay_opt1)); pkt.addRelayInfo(info1); @@ -1117,7 +1117,7 @@ TEST_F(Pkt6Test, getMACFromIPv6RelayOpt_multipleRelay) { // We reuse the option and modify the MAC to be sure we get the right address opt_data[2] = 0xfa; OptionPtr relay_opt3(new Option(Option::V6, D6O_CLIENT_LINKLAYER_ADDR, - OptionBuffer(opt_data, opt_data + sizeof(opt_data)))); + OptionBuffer(opt_data, opt_data + sizeof(opt_data)))); info3.options_.insert(make_pair(relay_opt3->getType(), relay_opt3)); pkt.addRelayInfo(info3); ASSERT_EQ(3, pkt.relay_info_.size()); @@ -1131,4 +1131,56 @@ TEST_F(Pkt6Test, getMACFromIPv6RelayOpt_multipleRelay) { EXPECT_EQ(tmp.str(), found->toText(true)); } +TEST_F(Pkt6Test, getMACFromDUID) { + Pkt6 pkt(DHCPV6_ADVERTISE, 1234); + + // Although MACs are typically 6 bytes long, let's make this test a bit + // more challenging and use odd MAC lengths. + + uint8_t duid_llt[] = { 0, 1, // type (DUID-LLT) + 0, 7, // hwtype (7 - just a randomly picked value) + 1, 2, 3, 4, // timestamp + 0xa, 0xb, 0xc, 0xd, 0xe, 0xf, 0x10 // MAC address (7 bytes) + }; + + uint8_t duid_ll[] = { 0, 3, // type (DUID-LL) + 0, 11, // hwtype (11 - just a randomly picked value) + 0xa, 0xb, 0xc, 0xd, 0xe // MAC address (5 bytes) + }; + + uint8_t duid_en[] = { 0, 2, // type (DUID-EN) + 1, 2, 3, 4, // enterprise-id + 0xa, 0xb, 0xc // opaque data + }; + + OptionPtr clientid1(new Option(Option::V6, D6O_CLIENTID, OptionBuffer( + duid_llt, duid_llt + sizeof(duid_llt)))); + OptionPtr clientid2(new Option(Option::V6, D6O_CLIENTID, OptionBuffer( + duid_ll, duid_ll + sizeof(duid_ll)))); + OptionPtr clientid3(new Option(Option::V6, D6O_CLIENTID, OptionBuffer( + duid_en, duid_en + sizeof(duid_en)))); + + // Packet does not have any client-id, this call should fail + EXPECT_FALSE(pkt.getMAC(Pkt::HWADDR_SOURCE_DUID)); + + // Let's test DUID-LLT. This should work. + pkt.addOption(clientid1); + HWAddrPtr mac = pkt.getMAC(Pkt::HWADDR_SOURCE_DUID); + ASSERT_TRUE(mac); + EXPECT_EQ("htype=7 0a:0b:0c:0d:0e:0f:10", mac->toText(true)); + + // Let's test DUID-LL. This should work. + ASSERT_TRUE(pkt.delOption(D6O_CLIENTID)); + pkt.addOption(clientid2); + mac = pkt.getMAC(Pkt::HWADDR_SOURCE_DUID); + ASSERT_TRUE(mac); + EXPECT_EQ("htype=11 0a:0b:0c:0d:0e", mac->toText(true)); + + // Finally, let's try DUID-EN. This should fail, as EN type does not + // contain any MAC address information. + ASSERT_TRUE(pkt.delOption(D6O_CLIENTID)); + pkt.addOption(clientid3); + EXPECT_FALSE(pkt.getMAC(Pkt::HWADDR_SOURCE_DUID)); +} + }