summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat (limited to '')
-rw-r--r--src/bin/dhcp4/dhcp4_srv.cc13
-rw-r--r--src/bin/dhcp6/dhcp6_srv.cc7
-rw-r--r--src/lib/dhcp/libdhcp++.cc19
-rw-r--r--src/lib/dhcp/pkt.cc6
-rw-r--r--src/lib/dhcp/pkt.h20
-rw-r--r--src/lib/dhcp/pkt4.cc21
-rw-r--r--src/lib/dhcp/pkt6.cc39
-rw-r--r--src/lib/dhcp/tests/pkt4_unittest.cc2
-rw-r--r--src/lib/dhcp/tests/pkt6_unittest.cc6
9 files changed, 88 insertions, 45 deletions
diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc
index 78ac0a7198..3b151d7cde 100644
--- a/src/bin/dhcp4/dhcp4_srv.cc
+++ b/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
diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc
index 5b64119e5b..1b8f372d07 100644
--- a/src/bin/dhcp6/dhcp6_srv.cc
+++ b/src/bin/dhcp6/dhcp6_srv.cc
@@ -2470,7 +2470,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) {
diff --git a/src/lib/dhcp/libdhcp++.cc b/src/lib/dhcp/libdhcp++.cc
index 1e3776a5b7..56b022fee0 100644
--- a/src/lib/dhcp/libdhcp++.cc
+++ b/src/lib/dhcp/libdhcp++.cc
@@ -239,7 +239,11 @@ size_t LibDHCP::unpackOptions6(const OptionBuffer& buf,
if (offset + opt_len > length) {
// @todo: consider throwing exception here.
- // Let's pretend we never parsed those 4 bytes
+
+ // 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);
}
@@ -352,9 +356,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
diff --git a/src/lib/dhcp/pkt.cc b/src/lib/dhcp/pkt.cc
index 1e05da9929..ca19344f31 100644
--- a/src/lib/dhcp/pkt.cc
+++ b/src/lib/dhcp/pkt.cc
@@ -116,10 +116,10 @@ Pkt::setRemoteHWAddr(const HWAddrPtr& hw_addr) {
void
Pkt::setHWAddrMember(const uint8_t htype, const uint8_t,
- const std::vector<uint8_t>& mac_addr,
- HWAddrPtr& hw_addr) {
+ const std::vector<uint8_t>& hw_addr,
+ HWAddrPtr& storage) {
- hw_addr.reset(new HWAddr(mac_addr, htype));
+ storage.reset(new HWAddr(hw_addr, htype));
}
HWAddrPtr
diff --git a/src/lib/dhcp/pkt.h b/src/lib/dhcp/pkt.h
index 4db64deb4f..90d37baf48 100644
--- a/src/lib/dhcp/pkt.h
+++ b/src/lib/dhcp/pkt.h
@@ -56,31 +56,31 @@ public:
/// Extracted from DUID-LL or DUID-LLT (not 100% reliable as the client
/// can send fake DUID).
- static const uint32_t MAC_SOURCE_DUID = 0x0002;
+ //static const uint32_t MAC_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 MAC_SOURCE_IPV6_LINK_LOCAL = 0x0004;
+ //static const uint32_t MAC_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 MAC_SOURCE_CLIENT_ADDR_RELAY_OPTION = 0x0008;
+ //static const uint32_t MAC_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 MAC_SOURCE_REMOTE_ID = 0x0010;
+ //static const uint32_t MAC_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 MAC_SOURCE_SUBSCRIBER_ID = 0x0020;
+ //static const uint32_t MAC_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 MAC_SOURCE_DOCSIS_OPTIONS = 0x0040;
+ //static const uint32_t MAC_SOURCE_DOCSIS_OPTIONS = 0x0040;
/// @}
@@ -572,13 +572,13 @@ private:
///
/// @param htype hardware type.
/// @param hlen hardware length.
- /// @param mac_addr pointer to actual hardware address.
- /// @param [out] hw_addr pointer to a class member to be modified.
+ /// @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>& mac_addr,
- HWAddrPtr& hw_addr);
+ const std::vector<uint8_t>& hw_addr,
+ HWAddrPtr& storage);
};
}; // namespace isc::dhcp
diff --git a/src/lib/dhcp/pkt4.cc b/src/lib/dhcp/pkt4.cc
index 999eace5f6..1d10d416dc 100644
--- a/src/lib/dhcp/pkt4.cc
+++ b/src/lib/dhcp/pkt4.cc
@@ -210,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();
diff --git a/src/lib/dhcp/pkt6.cc b/src/lib/dhcp/pkt6.cc
index a0db0dba11..e3c3187ad2 100644
--- a/src/lib/dhcp/pkt6.cc
+++ b/src/lib/dhcp/pkt6.cc
@@ -31,10 +31,8 @@ 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 */)
@@ -315,24 +313,31 @@ Pkt6::unpackMsg(OptionBuffer::const_iterator begin,
// 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()) {
- size_t offset = LibDHCP::unpackOptions6(opt_buffer, "dhcp6", options_);
- if (offset != size) {
- // Something is wrong here. We either parsed past input buffer
- // (impossible, our code is bug-free ;) 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.
- //isc_throw(BadValue, "Received DHCPv6 buffer of size " << size
- // << ", were able to parse " << offset << " bytes.");
- }
+ 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.
- callback_(opt_buffer, "dhcp6", options_, NULL, NULL);
+ offset = callback_(opt_buffer, "dhcp6", 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;
}
void
@@ -386,7 +391,7 @@ Pkt6::unpackRelayMsg() {
// store relay information parsed so far
addRelayInfo(relay);
- /// @todo: implement ERO here
+ /// @todo: implement ERO (Echo Request Option, RFC 4994) here
if (relay_msg_len >= bufsize) {
// length of the relay_msg option extends beyond end of the message
diff --git a/src/lib/dhcp/tests/pkt4_unittest.cc b/src/lib/dhcp/tests/pkt4_unittest.cc
index 0de4ceb329..aa8aed99a4 100644
--- a/src/lib/dhcp/tests/pkt4_unittest.cc
+++ b/src/lib/dhcp/tests/pkt4_unittest.cc
@@ -856,7 +856,7 @@ TEST_F(Pkt4Test, clientClasses) {
TEST_F(Pkt4Test, getMAC) {
Pkt4 pkt(DHCPOFFER, 1234);
- // DHCPv6 packet by default doens't have MAC address specified.
+ // 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));
diff --git a/src/lib/dhcp/tests/pkt6_unittest.cc b/src/lib/dhcp/tests/pkt6_unittest.cc
index 3cfaff9237..3fc25d3324 100644
--- a/src/lib/dhcp/tests/pkt6_unittest.cc
+++ b/src/lib/dhcp/tests/pkt6_unittest.cc
@@ -354,7 +354,8 @@ TEST_F(Pkt6Test, unpackMalformed) {
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.
+ // 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;
@@ -371,9 +372,6 @@ TEST_F(Pkt6Test, unpackMalformed) {
// ... but there should be no option 123 as it was malformed.
EXPECT_FALSE(trunc_option->getOption(123));
-
- // A strict approach would assume the code will reject the whole packet,
- // but we decided to follow Jon Postel's law.
}
// This test verifies that it is possible to specify custom implementation of
generated by cgit v1.2.3 (git 2.39.1) at 2024-12-14 11:18:38 +0100