From 5e94de9ed25358b6b5ef45d400e0b837a5294592 Mon Sep 17 00:00:00 2001 From: Andrei Pavel Date: Fri, 14 May 2021 16:38:01 +0300 Subject: [PATCH] [#1860] lenient parsing logic for option 16 --- src/bin/dhcp4/json_config_parser.cc | 10 ++++++++++ src/bin/dhcp6/json_config_parser.cc | 10 ++++++++++ src/lib/dhcp/opaque_data_tuple.h | 24 ++++++++++++++++-------- src/lib/dhcp/option.cc | 2 ++ src/lib/dhcp/option.h | 3 +++ src/lib/dhcpsrv/cfgmgr.cc | 4 ++-- src/lib/dhcpsrv/srv_config.cc | 8 +++++++- src/lib/dhcpsrv/srv_config.h | 27 +++++++++++++++++++++++++++ 8 files changed, 77 insertions(+), 11 deletions(-) diff --git a/src/bin/dhcp4/json_config_parser.cc b/src/bin/dhcp4/json_config_parser.cc index a8ea78fd59..977c16201a 100644 --- a/src/bin/dhcp4/json_config_parser.cc +++ b/src/bin/dhcp4/json_config_parser.cc @@ -578,6 +578,16 @@ configureDhcp4Server(Dhcpv4Srv& server, isc::data::ConstElementPtr config_set, CfgMgr::instance().getStagingCfg()->setConfigControlInfo(config_ctl_info); } + ConstElementPtr compatibility = mutable_cfg->get("compatibility"); + if (compatibility) { + for (auto kv : compatibility->mapValue()) { + if (kv.first == "lenient-option-parsing") { + CfgMgr::instance().getStagingCfg()->setLenientOptionParsing( + kv.second->boolValue()); + } + } + } + // Make parsers grouping. ConfigPair config_pair; const std::map& values_map = diff --git a/src/bin/dhcp6/json_config_parser.cc b/src/bin/dhcp6/json_config_parser.cc index c6a31a8a63..9daf670f2c 100644 --- a/src/bin/dhcp6/json_config_parser.cc +++ b/src/bin/dhcp6/json_config_parser.cc @@ -711,6 +711,16 @@ configureDhcp6Server(Dhcpv6Srv& server, isc::data::ConstElementPtr config_set, parser.parse(srv_config, rsoo_list); } + ConstElementPtr compatibility = mutable_cfg->get("compatibility"); + if (compatibility) { + for (auto kv : compatibility->mapValue()) { + if (kv.first == "lenient-option-parsing") { + CfgMgr::instance().getStagingCfg()->setLenientOptionParsing( + kv.second->boolValue()); + } + } + } + // Make parsers grouping. ConfigPair config_pair; const std::map& values_map = diff --git a/src/lib/dhcp/opaque_data_tuple.h b/src/lib/dhcp/opaque_data_tuple.h index b20ab6b1b5..6554155c28 100644 --- a/src/lib/dhcp/opaque_data_tuple.h +++ b/src/lib/dhcp/opaque_data_tuple.h @@ -7,8 +7,10 @@ #ifndef OPAQUE_DATA_TUPLE_H #define OPAQUE_DATA_TUPLE_H +#include #include #include + #include #include #include @@ -78,8 +80,9 @@ public: /// @param end Iterator pointing to the end of the buffer holding wire data. /// @tparam InputIterator Type of the iterators passed to this function. /// @throw It may throw an exception if the @c unpack throws. - template - OpaqueDataTuple(LengthFieldType length_field_type, InputIterator begin, + template + OpaqueDataTuple(LengthFieldType length_field_type, + InputIterator begin, InputIterator end) : length_field_type_(length_field_type) { unpack(begin, end); @@ -211,7 +214,6 @@ public: /// @tparam InputIterator Type of the iterators passed to this function. template void unpack(InputIterator begin, InputIterator end) { - Buffer buf(begin, end); // The buffer must at least hold the size of the data. if (std::distance(begin, end) < getDataFieldSize()) { isc_throw(OpaqueDataTupleError, @@ -226,12 +228,17 @@ public: // Now that we have the expected data size, let's check that the // reminder of the buffer is long enough. begin += getDataFieldSize(); + // Attempt to parse as a length-value pair. if (std::distance(begin, end) < len) { - isc_throw(OpaqueDataTupleError, - "unable to parse the opaque data tuple, the buffer" - " length is " << std::distance(begin, end) - << ", but the length of the tuple in the length field" - " is " << len); + if (Option::lenient_parsing_) { + // Fallback to parsing the rest of the option as a single value. + len = std::distance(begin, end); + } else { + isc_throw(OpaqueDataTupleError, + "unable to parse the opaque data tuple, " + "the buffer length is " << std::distance(begin, end) + << ", but the tuple length is " << len); + } } // The buffer length is long enough to read the desired amount of data. assign(begin, len); @@ -277,6 +284,7 @@ private: /// @brief Buffer which holds the opaque tuple data. Buffer data_; + /// @brief Holds a type of tuple size field (1 byte long or 2 bytes long). LengthFieldType length_field_type_; }; diff --git a/src/lib/dhcp/option.cc b/src/lib/dhcp/option.cc index e8f26f3da9..6ffb323cb8 100644 --- a/src/lib/dhcp/option.cc +++ b/src/lib/dhcp/option.cc @@ -402,5 +402,7 @@ Option::~Option() { } +bool Option::lenient_parsing_; + } // end of isc::dhcp namespace } // end of isc namespace diff --git a/src/lib/dhcp/option.h b/src/lib/dhcp/option.h index b270be5335..25b4891604 100644 --- a/src/lib/dhcp/option.h +++ b/src/lib/dhcp/option.h @@ -451,6 +451,9 @@ public: /// @return true if options are equal, false otherwise. virtual bool equals(const Option& other) const; + /// @brief Governs whether options should be parsed less strictly. + static bool lenient_parsing_; + protected: /// @brief Copies this option and returns a pointer to the copy. diff --git a/src/lib/dhcpsrv/cfgmgr.cc b/src/lib/dhcpsrv/cfgmgr.cc index 955e4a6405..be1952e113 100644 --- a/src/lib/dhcpsrv/cfgmgr.cc +++ b/src/lib/dhcpsrv/cfgmgr.cc @@ -88,8 +88,6 @@ CfgMgr::clear() { void CfgMgr::commit() { - - ensureCurrentAllocated(); // First we need to remove statistics. The new configuration can have fewer @@ -114,6 +112,8 @@ CfgMgr::commit() { // Now we need to set the statistics back. configuration_->updateStatistics(); + + configuration_->injectIntoDependencies(); } void diff --git a/src/lib/dhcpsrv/srv_config.cc b/src/lib/dhcpsrv/srv_config.cc index 0f4bc1785f..80d6afdb10 100644 --- a/src/lib/dhcpsrv/srv_config.cc +++ b/src/lib/dhcpsrv/srv_config.cc @@ -45,7 +45,8 @@ SrvConfig::SrvConfig() decline_timer_(0), echo_v4_client_id_(true), dhcp4o6_port_(0), d2_client_config_(new D2ClientConfig()), configured_globals_(Element::createMap()), - cfg_consist_(new CfgConsistency()) { + cfg_consist_(new CfgConsistency()), + lenient_option_parsing_(false) { } SrvConfig::SrvConfig(const uint32_t sequence) @@ -890,6 +891,11 @@ SrvConfig::setIPReservationsUnique(const bool unique) { getCfgDbAccess()->setIPReservationsUnique(unique); } +void +SrvConfig::injectIntoDependencies() const { + Option::lenient_parsing_ = lenient_option_parsing_; +} + bool DdnsParams::getEnableUpdates() const { if (!subnet_) { diff --git a/src/lib/dhcpsrv/srv_config.h b/src/lib/dhcpsrv/srv_config.h index a6ee6455f5..0d2af4caba 100644 --- a/src/lib/dhcpsrv/srv_config.h +++ b/src/lib/dhcpsrv/srv_config.h @@ -847,6 +847,28 @@ public: /// @return a pointer to unparsed configuration virtual isc::data::ElementPtr toElement() const; + /// @brief Set lenient option parsing compatibility flag. + /// + /// @param value the boolean value to be set when configuring lenient option + /// parsing + void setLenientOptionParsing(bool const value) { + lenient_option_parsing_ = value; + } + + /// @brief Get lenient option parsing compatibility flag. + /// + /// @return the configured value for lenient option parsing + bool getLenientOptionParsing() const { + return lenient_option_parsing_; + } + + /// @brief Convenience method to inject configuration parameters into + /// internal libraries that don't have access to the server's current + /// configuration + /// + /// Happen on configuration commit. + void injectIntoDependencies() const; + private: /// @brief Merges the DHCPv4 configuration specified as a parameter into @@ -998,6 +1020,11 @@ private: /// @brief Pointer to the configuration consistency settings CfgConsistencyPtr cfg_consist_; + + /// @brief Compatibility flags + /// @{ + bool lenient_option_parsing_; + /// @} }; /// @name Pointers to the @c SrvConfig object. -- 2.47.3