[#1111] DHCP Option 43 PAD END bug fix

Backport #950,!552 changes to v1_6_0

Changes:
    src/bin/dhcp4/dhcp4_messages.cc
    src/bin/dhcp4/dhcp4_messages.h
    src/bin/dhcp4/dhcp4_messages.mes
    src/bin/dhcp4/dhcp4_srv.cc
    src/bin/dhcp4/tests/vendor_opts_unittest.cc

diff --git a/src/bin/dhcp4/dhcp4_messages.cc b/src/bin/dhcp4/dhcp4_messages.cc
index 807b8147da134ab3a4cc56877c7e8546a86cea7a..63b6a7e9a4ec74a5315a2237ff23b990027c6492 100644 (file)
--- a/src/bin/dhcp4/dhcp4_messages.cc
+++ b/src/bin/dhcp4/dhcp4_messages.cc
@@ -1,4 +1,4 @@
-// File created from ../../../src/bin/dhcp4/dhcp4_messages.mes on Wed Jul 10 2019 15:10
+// File created from ../../../src/bin/dhcp4/dhcp4_messages.mes on Tue Feb 04 2020 11:15
 
 #include <cstddef>
 #include <log/message_types.h>
@@ -47,6 +47,8 @@ extern const isc::log::MessageID DHCP4_DEACTIVATE_INTERFACE = "DHCP4_DEACTIVATE_
 extern const isc::log::MessageID DHCP4_DECLINE_LEASE = "DHCP4_DECLINE_LEASE";
 extern const isc::log::MessageID DHCP4_DECLINE_LEASE_MISMATCH = "DHCP4_DECLINE_LEASE_MISMATCH";
 extern const isc::log::MessageID DHCP4_DECLINE_LEASE_NOT_FOUND = "DHCP4_DECLINE_LEASE_NOT_FOUND";
+extern const isc::log::MessageID DHCP4_DEFERRED_OPTION_MISSING = "DHCP4_DEFERRED_OPTION_MISSING";
+extern const isc::log::MessageID DHCP4_DEFERRED_OPTION_UNPACK_FAIL = "DHCP4_DEFERRED_OPTION_UNPACK_FAIL";
 extern const isc::log::MessageID DHCP4_DHCP4O6_BAD_PACKET = "DHCP4_DHCP4O6_BAD_PACKET";
 extern const isc::log::MessageID DHCP4_DHCP4O6_PACKET_RECEIVED = "DHCP4_DHCP4O6_PACKET_RECEIVED";
 extern const isc::log::MessageID DHCP4_DHCP4O6_PACKET_SEND = "DHCP4_DHCP4O6_PACKET_SEND";
@@ -100,6 +102,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";
@@ -184,6 +187,8 @@ const char* values[] = {
     "DHCP4_DECLINE_LEASE", "Received DHCPDECLINE for addr %1 from client %2. The lease will be unavailable for %3 seconds.",
     "DHCP4_DECLINE_LEASE_MISMATCH", "Received DHCPDECLINE for addr %1 from client %2, but the data doesn't match: received hwaddr: %3, lease hwaddr: %4, received client-id: %5, lease client-id: %6",
     "DHCP4_DECLINE_LEASE_NOT_FOUND", "Received DHCPDECLINE for addr %1 from client %2, but no such lease found.",
+    "DHCP4_DEFERRED_OPTION_MISSING", "can find deferred option code %1 in the query",
+    "DHCP4_DEFERRED_OPTION_UNPACK_FAIL", "An error unpacking the deferred option %1: %2",
     "DHCP4_DHCP4O6_BAD_PACKET", "received malformed DHCPv4o6 packet: %1",
     "DHCP4_DHCP4O6_PACKET_RECEIVED", "received DHCPv4o6 packet from DHCPv4 server (type %1) for %2 on interface %3",
     "DHCP4_DHCP4O6_PACKET_SEND", "%1: trying to send packet %2 (type %3) to %4 port %5 on interface %6 encapsulating %7: %8 (type %9)",
@@ -236,7 +241,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",

diff --git a/src/bin/dhcp4/dhcp4_messages.h b/src/bin/dhcp4/dhcp4_messages.h
index f58ca25bbe951cda870d194037855ed075dce38f..171ef6590767e22bca1665832ca63bb0d1d39995 100644 (file)
--- a/src/bin/dhcp4/dhcp4_messages.h
+++ b/src/bin/dhcp4/dhcp4_messages.h
@@ -1,4 +1,4 @@
-// File created from ../../../src/bin/dhcp4/dhcp4_messages.mes on Wed Jul 10 2019 15:10
+// File created from ../../../src/bin/dhcp4/dhcp4_messages.mes on Tue Feb 04 2020 11:15
 
 #ifndef DHCP4_MESSAGES_H
 #define DHCP4_MESSAGES_H
@@ -48,6 +48,8 @@ extern const isc::log::MessageID DHCP4_DEACTIVATE_INTERFACE;
 extern const isc::log::MessageID DHCP4_DECLINE_LEASE;
 extern const isc::log::MessageID DHCP4_DECLINE_LEASE_MISMATCH;
 extern const isc::log::MessageID DHCP4_DECLINE_LEASE_NOT_FOUND;
+extern const isc::log::MessageID DHCP4_DEFERRED_OPTION_MISSING;
+extern const isc::log::MessageID DHCP4_DEFERRED_OPTION_UNPACK_FAIL;
 extern const isc::log::MessageID DHCP4_DHCP4O6_BAD_PACKET;
 extern const isc::log::MessageID DHCP4_DHCP4O6_PACKET_RECEIVED;
 extern const isc::log::MessageID DHCP4_DHCP4O6_PACKET_SEND;
@@ -101,6 +103,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;

diff --git a/src/bin/dhcp4/dhcp4_messages.mes b/src/bin/dhcp4/dhcp4_messages.mes
index 7a0b1c19f2e18dbacf75897438fdacd7d2b5ee1f..1ed65de401b199e7badbc0ae4d29ed78c2e8cfd9 100644 (file)
--- a/src/bin/dhcp4/dhcp4_messages.mes
+++ b/src/bin/dhcp4/dhcp4_messages.mes
@@ -241,6 +241,15 @@ detected as a duplicate (i.e. another device in the network is using this addres
 However, the server does not have a record for this address. This may indicate
 a client's error or a server's purged database.
 
+% DHCP4_DEFERRED_OPTION_MISSING can find deferred option code %1 in the query
+This debug message is printed when a deferred option cannot be found in
+the query.
+
+% DHCP4_DEFERRED_OPTION_UNPACK_FAIL An error unpacking the deferred option %1: %2
+A debug message issued when deferred unpacking of an option failed, making it
+to be left unpacked in the packet. The first argument is the option code,
+the second the error.
+
 % DHCP4_DHCP4O6_BAD_PACKET received malformed DHCPv4o6 packet: %1
 A malformed DHCPv4o6 packet was received.
 
@@ -552,7 +561,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.

diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc
index 98f66725234d35183ff4079df7d82a8c29bf198c..fab7cd44058938491677866ff91049c7efb594e9 100644 (file)
--- a/src/bin/dhcp4/dhcp4_srv.cc
+++ b/src/bin/dhcp4/dhcp4_srv.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2011-2019 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-2020 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -3535,14 +3535,27 @@ 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_DEFERRED_OPTION_MISSING)
+                .arg(code);
             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_DEFERRED_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);
     }
 }

diff --git a/src/bin/dhcp4/tests/vendor_opts_unittest.cc b/src/bin/dhcp4/tests/vendor_opts_unittest.cc
index 2f94205a71e28a57a23ca1d1587f2c5a9cc7503a..c13cabe889862fcbed26ca3188f7e0e4c796974c 100644 (file)
--- a/src/bin/dhcp4/tests/vendor_opts_unittest.cc
+++ b/src/bin/dhcp4/tests/vendor_opts_unittest.cc
@@ -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)