+1673. [bug] tmark
+ Fixed a bug introduced in Kea 1.6.0 (see #539) that caused
+ kea-dhcp4 and kea-dhcp6 to discard inbound packets containing
+ string options that consist solely of nulls. The servers
+ will now quiely omit empty or all-null string options from
+ inbound packets.
+ (Gitlab #900, !561 git TBD)
+
1672. [build] fdupont
Deprecated bind1st and bind2nd templates were replaced with
lambda expressions or plain bind templates.
buf.begin() + offset,
buf.begin() + offset + opt_len));
} else {
- // The option definition has been found. Use it to create
- // the option instance from the provided buffer chunk.
- const OptionDefinitionPtr& def = *(range.first);
- assert(def);
- opt = def->optionFactory(Option::V6, opt_type,
- buf.begin() + offset,
- buf.begin() + offset + opt_len);
+ try {
+ // The option definition has been found. Use it to create
+ // the option instance from the provided buffer chunk.
+ const OptionDefinitionPtr& def = *(range.first);
+ assert(def);
+ opt = def->optionFactory(Option::V6, opt_type,
+ buf.begin() + offset,
+ buf.begin() + offset + opt_len);
+ } catch (const SkipThisOptionError& ex) {
+ opt.reset();
+ }
}
+
// add option to options
- options.insert(std::make_pair(opt_type, opt));
+ if (opt) {
+ options.insert(std::make_pair(opt_type, opt));
+ }
+
offset += opt_len;
}
buf.begin() + offset + opt_len));
opt->setEncapsulatedSpace(DHCP4_OPTION_SPACE);
} else {
- // The option definition has been found. Use it to create
- // the option instance from the provided buffer chunk.
- const OptionDefinitionPtr& def = *(range.first);
- assert(def);
- opt = def->optionFactory(Option::V4, opt_type,
- buf.begin() + offset,
- buf.begin() + offset + opt_len);
+ try {
+ // The option definition has been found. Use it to create
+ // the option instance from the provided buffer chunk.
+ const OptionDefinitionPtr& def = *(range.first);
+ assert(def);
+ opt = def->optionFactory(Option::V4, opt_type,
+ buf.begin() + offset,
+ buf.begin() + offset + opt_len);
+ } catch (const SkipThisOptionError& ex) {
+ opt.reset();
+ }
+ }
+
+ // If we have the option, insert it
+ if (opt) {
+ options.insert(std::make_pair(opt_type, opt));
}
- options.insert(std::make_pair(opt_type, opt));
offset += opt_len;
}
last_offset = offset;
isc::Exception(file, line, what) { };
};
+class SkipThisOptionError : public Exception {
+public:
+ SkipThisOptionError (const char* file, size_t line, const char* what) :
+ isc::Exception(file, line, what) { };
+};
+
+
class Option {
public:
/// length of the usual DHCPv4 option header (there are exceptions)
;
}
return (OptionPtr(new OptionCustom(*this, u, begin, end)));
+ } catch (const SkipThisOptionError& ex) {
+ // We need to throw this one as is.
+ throw ex;
} catch (const SkipRemainingOptionsError& ex) {
// We need to throw this one as is.
throw ex;
/// because it is checked in a constructor.
virtual void unpack(OptionBufferConstIter begin, OptionBufferConstIter end) {
if (distance(begin, end) < sizeof(T)) {
- isc_throw(OutOfRange, "Option " << getType() << " truncated");
+ isc_throw(OutOfRange, "OptionInt " << getType() << " truncated");
}
// @todo consider what to do if buffer is longer than data type.
isc_throw(OutOfRange, "option " << getType() << " empty");
}
if (distance(begin, end) % sizeof(T) != 0) {
- isc_throw(OutOfRange, "option " << getType() << " truncated");
+ isc_throw(OutOfRange, "OptionIntArray " << getType() << " truncated");
}
// @todo consider what to do if buffer is longer than data type.
// Trim off trailing null(s)
end = util::str::seekTrimmed(begin, end, 0x0);
if (std::distance(begin, end) == 0) {
- isc_throw(isc::OutOfRange, "failed to parse an option '"
+ isc_throw(isc::dhcp::SkipThisOptionError, "failed to parse an option '"
<< getType() << "' holding string value"
<< "' was empty or contained only NULLs");
}
// an exception.
EXPECT_THROW(
OptionString(Option::V4, 234, buf_.begin(), buf_.begin()),
- isc::OutOfRange
+ isc::dhcp::SkipThisOptionError
);
// NULLs should result in an exception.
std::vector<uint8_t>nulls = { 0, 0, 0 };
EXPECT_THROW(
OptionString(Option::V4, 234, nulls.begin(), nulls.begin()),
- isc::OutOfRange
+ isc::dhcp::SkipThisOptionError
);
// Declare option as a scoped pointer here so as its scope is
// Only nulls should throw.
OptionBuffer buffer = { 0, 0 };
- ASSERT_THROW(optv4.unpack(buffer.begin(), buffer.end()), isc::OutOfRange);
+ ASSERT_THROW(optv4.unpack(buffer.begin(), buffer.end()), isc::dhcp::SkipThisOptionError);
// One trailing null should trim off.
buffer = {'o', 'n', 'e', 0 };
#include <dhcp/option_vendor.h>
#include <dhcp/pkt4.h>
#include <exceptions/exceptions.h>
+#include <testutils/gtest_utils.h>
#include <util/buffer.h>
#include <util/encode/hex.h>
#include <pkt_captures.h>
TEST_F(Pkt4Test, truncatedVendorLength) {
// Build a good discover packet
- Pkt4Ptr pkt = test::PktCaptures::discoverWithValidVIVSO();
+ Pkt4Ptr pkt = dhcp::test::PktCaptures::discoverWithValidVIVSO();
// Unpacking should not throw
ASSERT_NO_THROW(pkt->unpack());
EXPECT_EQ(133+2, vivso->len()); // data + opt code + len
// Build a bad discover packet
- pkt = test::PktCaptures::discoverWithTruncatedVIVSO();
+ pkt = dhcp::test::PktCaptures::discoverWithTruncatedVIVSO();
// Unpack should throw Skip exception
ASSERT_THROW(pkt->unpack(), SkipRemainingOptionsError);
EXPECT_EQ(0, memcmp(&packed[base_size], &packed_opts[0], packed_opts.size()));
}
+// Checks that unpacking correctly handles SkipThisOptionError by
+// omitting the offending option from the unpacked options.
+TEST_F(Pkt4Test, testSkipThisOptionError) {
+ vector<uint8_t> orig = generateTestPacket2();
+
+ orig.push_back(0x63);
+ orig.push_back(0x82);
+ orig.push_back(0x53);
+ orig.push_back(0x63);
+
+ orig.push_back(53); // Message Type
+ orig.push_back(1); // length=1
+ orig.push_back(2); // type=2
+
+ orig.push_back(14); // merit-dump
+ orig.push_back(3); // length=3
+ orig.push_back(0x61); // data="abc"
+ orig.push_back(0x62);
+ orig.push_back(0x63);
+
+ orig.push_back(12); // Hostname
+ orig.push_back(3); // length=3
+ orig.push_back(0); // data= all nulls
+ orig.push_back(0);
+ orig.push_back(0);
+
+ orig.push_back(17); // root-path
+ orig.push_back(3); // length=3
+ orig.push_back(0x64); // data="def"
+ orig.push_back(0x65);
+ orig.push_back(0x66);
+
+ // Unpacking should not throw.
+ Pkt4Ptr pkt(new Pkt4(&orig[0], orig.size()));
+ ASSERT_NO_THROW_LOG(pkt->unpack());
+
+ // We should have option 14 = "abc".
+ OptionPtr opt;
+ OptionStringPtr opstr;
+ ASSERT_TRUE(opt = pkt->getOption(14));
+ ASSERT_TRUE(opstr = boost::dynamic_pointer_cast<OptionString>(opt));
+ EXPECT_EQ(3, opstr->getValue().length());
+ EXPECT_EQ("abc", opstr->getValue());
+
+ // We should not have option 12.
+ EXPECT_FALSE(opt = pkt->getOption(12));
+
+ // We should have option 17 = "def".
+ ASSERT_TRUE(opt = pkt->getOption(17));
+ ASSERT_TRUE(opstr = boost::dynamic_pointer_cast<OptionString>(opt));
+ EXPECT_EQ(3, opstr->getValue().length());
+ EXPECT_EQ("def", opstr->getValue());
+}
+
} // end of anonymous namespace
#include <dhcp/option6_ia.h>
#include <dhcp/option_int.h>
#include <dhcp/option_int_array.h>
+#include <dhcp/option_string.h>
#include <dhcp/option_vendor.h>
#include <dhcp/iface_mgr.h>
#include <dhcp/pkt6.h>
#include <dhcp/hwaddr.h>
#include <dhcp/docsis3_option_defs.h>
#include <dhcp/tests/pkt_captures.h>
+#include <testutils/gtest_utils.h>
#include <util/range_utilities.h>
#include <boost/bind.hpp>
#include <boost/date_time/posix_time/posix_time.hpp>
// (Relay Supplied Options option). This test checks whether it was parsed
// properly. See captureRelayed2xRSOO() description for details.
TEST_F(Pkt6Test, rsoo) {
- Pkt6Ptr msg = test::PktCaptures::captureRelayed2xRSOO();
+ Pkt6Ptr msg = dhcp::test::PktCaptures::captureRelayed2xRSOO();
EXPECT_NO_THROW(msg->unpack());
TEST_F(Pkt6Test, truncatedVendorLength) {
// Build a good Solicit packet
- Pkt6Ptr pkt = test::PktCaptures::captureSolicitWithVIVSO();
+ Pkt6Ptr pkt = dhcp::test::PktCaptures::captureSolicitWithVIVSO();
// Unpacking should not throw
ASSERT_NO_THROW(pkt->unpack());
EXPECT_EQ(8, vivso->len()); // data + opt code + len
// Build a bad Solicit packet
- pkt = test::PktCaptures::captureSolicitWithTruncatedVIVSO();
+ pkt = dhcp::test::PktCaptures::captureSolicitWithTruncatedVIVSO();
// Unpack should throw Skip exception
ASSERT_THROW(pkt->unpack(), SkipRemainingOptionsError);
ASSERT_FALSE(x);
}
+// Checks that unpacking correctly handles SkipThisOptionError by
+// omitting the offending option from the unpacked options.
+TEST_F(Pkt6Test, testSkipThisOptionError) {
+ // Get a packet. We're really interested in its on-wire
+ // representation only.
+ Pkt6Ptr donor(capture1());
+
+ // That's our original content. It should be sane.
+ OptionBuffer orig = donor->data_;
+
+ orig.push_back(0);
+ orig.push_back(41); // new-posix-timezone
+ orig.push_back(0);
+ orig.push_back(3); // length=3
+ orig.push_back(0x61); // data="abc"
+ orig.push_back(0x62);
+ orig.push_back(0x63);
+
+ orig.push_back(0);
+ orig.push_back(59); // bootfile-url
+ orig.push_back(0);
+ orig.push_back(3); // length=3
+ orig.push_back(0); // data= all nulls
+ orig.push_back(0);
+ orig.push_back(0);
+
+ orig.push_back(0);
+ orig.push_back(42); // new-tzdb-timezone
+ orig.push_back(0);
+ orig.push_back(3); // length=3
+ orig.push_back(0x64); // data="def"
+ orig.push_back(0x65);
+ orig.push_back(0x66);
+
+ // Unpacking should not throw.
+ Pkt6Ptr pkt(new Pkt6(&orig[0], orig.size()));
+ ASSERT_NO_THROW_LOG(pkt->unpack());
+
+ // We should have option 41 = "abc".
+ OptionPtr opt;
+ OptionStringPtr opstr;
+ ASSERT_TRUE(opt = pkt->getOption(41));
+ ASSERT_TRUE(opstr = boost::dynamic_pointer_cast<OptionString>(opt));
+ EXPECT_EQ(3, opstr->getValue().length());
+ EXPECT_EQ("abc", opstr->getValue());
+
+ // We should not have option 59.
+ EXPECT_FALSE(opt = pkt->getOption(59));
+
+ // We should have option 42 = "def".
+ ASSERT_TRUE(opt = pkt->getOption(42));
+ ASSERT_TRUE(opstr = boost::dynamic_pointer_cast<OptionString>(opt));
+ EXPECT_EQ(3, opstr->getValue().length());
+ EXPECT_EQ("def", opstr->getValue());
+}
+
}
#include <dhcp/option_string.h>
#include <dhcp/option4_addrlst.h>
#include <dhcpsrv/cfg_option.h>
+#include <testutils/gtest_utils.h>
#include <testutils/test_to_element.h>
#include <boost/foreach.hpp>
#include <boost/pointer_cast.hpp>
// When we attempt to merge, it should fail, recognizing that
// option 2, which has a formatted value, has no definition.
- try {
- this_cfg.merge(defs, other_cfg);
- GTEST_FAIL() << "merge should have thrown";
- } catch (const InvalidOperation& ex) {
- std::string exp_msg = "option: isc.2 has a formatted value: "
- "'one,two,three' but no option definition";
- EXPECT_EQ(std::string(exp_msg), std::string(ex.what()));
- } catch (const std::exception& ex) {
- GTEST_FAIL() << "wrong exception thrown:" << ex.what();
- }
+ ASSERT_THROW_MSG(this_cfg.merge(defs, other_cfg), InvalidOperation,
+ "option: isc.2 has a formatted value: "
+ "'one,two,three' but no option definition");
// Now let's add an option definition that will force data truncated
// error for option 1.
// When we attempt to merge, it should fail because option 1's data
// is not valid per its definition.
- try {
- this_cfg.merge(defs, other_cfg);
- GTEST_FAIL() << "merge should have thrown";
- } catch (const InvalidOperation& ex) {
- std::string exp_msg = "could not create option: isc.1"
- " from data specified, reason:"
- " Option 1 truncated";
- EXPECT_EQ(std::string(exp_msg), std::string(ex.what()));
- } catch (const std::exception& ex) {
- GTEST_FAIL() << "wrong exception thrown:" << ex.what();
- }
+ EXPECT_THROW_MSG(this_cfg.merge(defs, other_cfg), InvalidOperation,
+ "could not create option: isc.1"
+ " from data specified, reason:"
+ " OptionInt 1 truncated");
}
// This test verifies the all of the valid option cases
// License, v. 2.0. If a copy of the MPL was not distributed with this
// file, You can obtain one at http://mozilla.org/MPL/2.0/.
-#ifndef GTEST_UTIL_H
-#define GTEST_UTIL_H
+#ifndef GTEST_UTILS_H
+#define GTEST_UTILS_H
#include <gtest/gtest.h>
} \
} \
+/// @brief Adds a non-fatal failure with exception info, if the given
+/// expression throws
+///
+/// @param statement - statement block to execute
+#define EXPECT_NO_THROW_LOG(statement) \
+{ \
+ try { \
+ statement; \
+ } catch (const std::exception& ex) { \
+ ADD_FAILURE() << #statement << "threw: " << ex.what(); \
+ } catch (...) { \
+ ADD_FAILURE() << #statement << "threw non-std::exception"; \
+ } \
+} \
+
+/// @brief Generates a fatal failure with exception info, if the given
+/// expression throws
+///
+/// @param statement - statement block to execute
+#define ASSERT_NO_THROW_LOG(statement) \
+{ \
+ try { \
+ statement; \
+ } catch (const std::exception& ex) { \
+ GTEST_FAIL() << #statement << "threw: " << ex.what(); \
+ } catch (...) { \
+ GTEST_FAIL() << #statement << "threw non-std::exception"; \
+ } \
+} \
+
}; // end of isc::test namespace
}; // end of isc namespace
-#endif // GTEST_UTIL_H
+#endif // GTEST_UTILS_H
projects / thirdparty / kea.git / commitdiff
