From: Francis Dupont <fdupont@isc.org>
Date: Wed, 10 Apr 2019 19:29:59 +0000 (+0200)
Subject: [564-customer-request-relax-constraints-on-allowable-option-types-to-permit-option... 
X-Git-Tag: Kea-1.6.0-beta~231^2~5
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=60f099ab7e491673dbd0b7e118db56f585dd87f8;p=thirdparty%2Fkea.git

[564-customer-request-relax-constraints-on-allowable-option-types-to-permit-option-type-0-and-255] checkpoint
---

diff --git a/src/lib/dhcp/libdhcp++.cc b/src/lib/dhcp/libdhcp++.cc
index 4572270de7..81abee5303 100644
--- a/src/lib/dhcp/libdhcp++.cc
+++ b/src/lib/dhcp/libdhcp++.cc
@@ -472,6 +472,9 @@ size_t LibDHCP::unpackOptions4(const OptionBuffer& buf,
     size_t offset = 0;
     size_t last_offset = 0;
 
+    // Special case when option_space is dhcp4.
+    bool space_is_dhcp4 = (option_space == DHCP4_OPTION_SPACE);
+
     // Get the list of standard option definitions.
     const OptionDefContainerPtr& option_defs = LibDHCP::getOptionDefs(option_space);
     // Runtime option definitions for non standard option space and if
@@ -493,7 +496,7 @@ size_t LibDHCP::unpackOptions4(const OptionBuffer& buf,
         uint8_t opt_type = buf[offset++];
 
         // DHO_END is a special, one octet long option
-        if (opt_type == DHO_END) {
+        if (space_is_dhcp4 && (opt_type == DHO_END)) {
             // just return. Don't need to add DHO_END option
             // Don't return offset because it makes this condition
             // and partial parsing impossible to recognize.
@@ -502,8 +505,9 @@ size_t LibDHCP::unpackOptions4(const OptionBuffer& buf,
 
         // DHO_PAD is just a padding after DHO_END. Let's continue parsing
         // in case we receive a message without DHO_END.
-        if (opt_type == DHO_PAD)
+        if (space_is_dhcp4 && (opt_type == DHO_PAD)) {
             continue;
+        }
 
         if (offset + 1 > buf.size()) {
             // We peeked at the option header of the next option, but
@@ -530,7 +534,7 @@ size_t LibDHCP::unpackOptions4(const OptionBuffer& buf,
         // rather than the dropping the whole packet.  We do not have a
         // way to log this from here but meh...  a PCAP will show it arriving,
         // and we know we drop it.
-        if (opt_len == 0 && opt_type == DHO_HOST_NAME) {
+        if (space_is_dhcp4 && opt_len == 0 && opt_type == DHO_HOST_NAME) {
             continue;
         }
 
diff --git a/src/lib/dhcp/option.cc b/src/lib/dhcp/option.cc
index 5d7320780b..1fb18bfe46 100644
--- a/src/lib/dhcp/option.cc
+++ b/src/lib/dhcp/option.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2011-2017 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-2019 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
@@ -35,12 +35,7 @@ Option::factory(Option::Universe u,
 
 Option::Option(Universe u, uint16_t type)
     :universe_(u), type_(type) {
-
-    // END option (type 255 is forbidden as well)
-    if ((u == V4) && ((type == 0) || (type > 254))) {
-        isc_throw(BadValue, "Can't create V4 option of type "
-                  << type << ", V4 options are in range 1..254");
-    }
+    check();
 }
 
 Option::Option(Universe u, uint16_t type, const OptionBuffer& data)
diff --git a/src/lib/dhcp/tests/option_unittest.cc b/src/lib/dhcp/tests/option_unittest.cc
index 0e282072ce..a86a26d882 100644
--- a/src/lib/dhcp/tests/option_unittest.cc
+++ b/src/lib/dhcp/tests/option_unittest.cc
@@ -70,13 +70,7 @@ TEST_F(OptionTest, v4_basic) {
     EXPECT_NO_THROW(opt.reset());
 
     // V4 options have type 0...255
-    EXPECT_THROW(opt.reset(new Option(Option::V4, 256)), BadValue);
-
-    // 0 is a special PAD option
-    EXPECT_THROW(opt.reset(new Option(Option::V4, 0)), BadValue);
-
-    // 255 is a special END option
-    EXPECT_THROW(opt.reset(new Option(Option::V4, 255)), BadValue);
+    EXPECT_THROW(opt.reset(new Option(Option::V4, 256)), OutOfRange);
 }
 
 const uint8_t dummyPayload[] =
diff --git a/src/lib/dhcpsrv/parsers/option_data_parser.cc b/src/lib/dhcpsrv/parsers/option_data_parser.cc
index d6cb037a3f..523ff3560d 100644
--- a/src/lib/dhcpsrv/parsers/option_data_parser.cc
+++ b/src/lib/dhcpsrv/parsers/option_data_parser.cc
@@ -59,14 +59,10 @@ OptionDataParser::extractCode(ConstElementPtr parent) const {
         return (Optional<uint32_t>());
     }
 
-    if (code == 0) {
-        isc_throw(DhcpConfigError, "option code must not be zero "
-                  "(" << getPosition("code", parent) << ")");
-
-    } else if (address_family_ == AF_INET &&
-               code > std::numeric_limits<uint8_t>::max()) {
+    if (address_family_ == AF_INET &&
+        code > std::numeric_limits<uint8_t>::max()) {
         isc_throw(DhcpConfigError, "invalid option code '" << code
-                << "', it must not be greater than '"
+                  << "', it must not be greater than '"
                   << static_cast<int>(std::numeric_limits<uint8_t>::max())
                   << "' (" << getPosition("code", parent)
                   << ")");
@@ -74,7 +70,7 @@ OptionDataParser::extractCode(ConstElementPtr parent) const {
     } else if (address_family_ == AF_INET6 &&
                code > std::numeric_limits<uint16_t>::max()) {
         isc_throw(DhcpConfigError, "invalid option code '" << code
-                << "', it must not exceed '"
+                  << "', it must not exceed '"
                   << std::numeric_limits<uint16_t>::max()
                   << "' (" << getPosition("code", parent)
                   << ")");