mir/kea
2
0
Fork 0
mirror of https://gitlab.isc.org/isc-projects/kea synced 2025-08-30 13:37:55 +00:00
Code Issues Releases Wiki Activity

[950-harden-deferred-unpack] Hardened deffered unpacking so it no longer throws

Browse Source
This commit is contained in:
Francis Dupont
2019-10-12 22:46:39 +02:00
parent 2321173c63
commit 68df26f36e
5 changed files with 33 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
src/bin/dhcp4/dhcp4_messages.cc
View File

@@ -101,6 +101,7 @@ extern const isc::log::MessageID DHCP4_PACKET_NAK_0002 = "DHCP4_PACKET_NAK_0002"
extern const isc::log::MessageID DHCP4_PACKET_NAK_0003 = "DHCP4_PACKET_NAK_0003";
extern const isc::log::MessageID DHCP4_PACKET_NAK_0004 = "DHCP4_PACKET_NAK_0004";
extern const isc::log::MessageID DHCP4_PACKET_OPTIONS_SKIPPED = "DHCP4_PACKET_OPTIONS_SKIPPED";
extern const isc::log::MessageID DHCP4_PACKET_OPTION_UNPACK_FAIL = "DHCP4_PACKET_OPTION_UNPACK_FAIL";
extern const isc::log::MessageID DHCP4_PACKET_PACK = "DHCP4_PACKET_PACK";
extern const isc::log::MessageID DHCP4_PACKET_PACK_FAIL = "DHCP4_PACKET_PACK_FAIL";
extern const isc::log::MessageID DHCP4_PACKET_PROCESS_EXCEPTION = "DHCP4_PACKET_PROCESS_EXCEPTION";
@@ -238,7 +239,8 @@ const char* values[] = {
"DHCP4_PACKET_NAK_0002", "%1: invalid address %2 requested by INIT-REBOOT",
"DHCP4_PACKET_NAK_0003", "%1: failed to advertise a lease, client sent ciaddr %2, requested-ip-address %3",
"DHCP4_PACKET_NAK_0004", "%1: failed to grant a lease, client sent ciaddr %2, requested-ip-address %3",
"DHCP4_PACKET_OPTIONS_SKIPPED", "An error upacking an option, caused subsequent options to be skipped: %1",
"DHCP4_PACKET_OPTIONS_SKIPPED", "An error unpacking an option, caused subsequent options to be skipped: %1",
"DHCP4_PACKET_OPTION_UNPACK_FAIL", "An error unpacking the option %1: %2",
"DHCP4_PACKET_PACK", "%1: preparing on-wire format of the packet to be sent",
"DHCP4_PACKET_PACK_FAIL", "%1: preparing on-wire-format of the packet to be sent failed %2",
"DHCP4_PACKET_PROCESS_EXCEPTION", "exception occurred during packet processing",

1
src/bin/dhcp4/dhcp4_messages.h
View File

@@ -102,6 +102,7 @@ extern const isc::log::MessageID DHCP4_PACKET_NAK_0002;
extern const isc::log::MessageID DHCP4_PACKET_NAK_0003;
extern const isc::log::MessageID DHCP4_PACKET_NAK_0004;
extern const isc::log::MessageID DHCP4_PACKET_OPTIONS_SKIPPED;
extern const isc::log::MessageID DHCP4_PACKET_OPTION_UNPACK_FAIL;
extern const isc::log::MessageID DHCP4_PACKET_PACK;
extern const isc::log::MessageID DHCP4_PACKET_PACK_FAIL;
extern const isc::log::MessageID DHCP4_PACKET_PROCESS_EXCEPTION;

7
src/bin/dhcp4/dhcp4_messages.mes
View File

@@ -558,7 +558,12 @@ identification information. The second argument contains the IPv4 address
in the ciaddr field. The third argument contains the IPv4 address in the
requested-ip-address option (if present).
% DHCP4_PACKET_OPTIONS_SKIPPED An error upacking an option, caused subsequent options to be skipped: %1
% DHCP4_PACKET_OPTION_UNPACK_FAIL An error unpacking the option %1: %2
A debug message issued when an option failed to unpack correctly, making it
to be left unpacked in the packet. The first argument is the option code,
the second the error.
% DHCP4_PACKET_OPTIONS_SKIPPED An error unpacking an option, caused subsequent options to be skipped: %1
A debug message issued when an option failed to unpack correctly, making it
impossible to unpack the remaining options in the packet. The server will
server will still attempt to service the packet.

18
src/bin/dhcp4/dhcp4_srv.cc
View File

@@ -3598,14 +3598,28 @@ Dhcpv4Srv::deferredUnpack(Pkt4Ptr& query)
OptionPtr opt = query->getOption(code);
if (!opt) {
// should not happen but do not crash anyway
LOG_DEBUG(bad_packet4_logger, DBG_DHCP4_DETAIL,
DHCP4_PACKET_OPTION_UNPACK_FAIL)
.arg(code)
.arg("can't find the option in the query");
continue;
}
const OptionBuffer buf = opt->getData();
try {
// Unpack the option
opt = def->optionFactory(Option::V4, code, buf);
} catch (const std::exception& e) {
// Failed to parse the option.
LOG_DEBUG(bad_packet4_logger, DBG_DHCP4_DETAIL,
DHCP4_PACKET_OPTION_UNPACK_FAIL)
.arg(code)
.arg(e.what());
continue;
}
while (query->delOption(code)) {
// continue
}
// Unpack the option and add it
opt = def->optionFactory(Option::V4, code, buf.cbegin(), buf.cend());
// Add the unpacked option.
query->addOption(opt);
}
}

12
src/bin/dhcp4/tests/vendor_opts_unittest.cc
View File

@@ -684,7 +684,7 @@ TEST_F(VendorOptsTest, option43FailRaw) {
// The vendor-encapsulated-options has an incompatible data
// so won't have the expected content. Here the processing
// of suboptions tries to unpack the uitn32 foo suboption and
// raises an exception.
// raises an exception which is caught so the option stays unpacked.
string config = "{ \"interfaces-config\": {"
" \"interfaces\": [ \"*\" ] }, "
"\"subnet4\": [ "
@@ -742,7 +742,9 @@ TEST_F(VendorOptsTest, option43FailRaw) {
query->addOption(prl);
srv.classifyPacket(query);
EXPECT_THROW(srv.deferredUnpack(query), InvalidOptionValue);
EXPECT_NO_THROW(srv.deferredUnpack(query));
ASSERT_TRUE(query->getOption(vopt->getType()));
EXPECT_EQ(vopt, query->getOption(vopt->getType()));
}
// Verifies raw option 43 can be handled (global)
@@ -1321,7 +1323,7 @@ TEST_F(VendorOptsTest, clientOption43FailRaw) {
// The vendor-encapsulated-options has an incompatible data
// so won't have the expected content. Here the processing
// of suboptions tries to unpack the uint32 foo suboption and
// raises an exception.
// raises an exception which is caught.
string config = "{ \"interfaces-config\": {"
" \"interfaces\": [ \"*\" ] }, "
"\"subnet4\": [ "
@@ -1346,9 +1348,9 @@ TEST_F(VendorOptsTest, clientOption43FailRaw) {
client.addExtraOption(vopt);
// Let's check whether the server is not able to process this packet
// and raises an exception so the response is empty.
// and raises an exception which is caught so the response is not empty.
EXPECT_NO_THROW(client.doDiscover());
EXPECT_FALSE(client.getContext().response_);
EXPECT_TRUE(client.getContext().response_);
}
// Verifies raw option 43 sent by a client can be handled (global)