[#4567] Fixed readFqdn with raw input

diff --git a/changelog_unreleased/4567-bad-FQDN-length b/changelog_unreleased/4567-bad-FQDN-length
new file mode 100644 (file)
index 0000000..4de2d2d
--- /dev/null
+++ b/changelog_unreleased/4567-bad-FQDN-length
@@ -0,0 +1,5 @@
+[bug]          fdupont
+       Fixed the length of FQDN data fields when parsing
+       custom options. Thank you to Qifan Zhang from
+       Palo Alto Networks for reporting the issue.
+       (Gitlab #4567)

diff --git a/src/lib/dhcp/option_custom.cc b/src/lib/dhcp/option_custom.cc
index fc7e02b25447c28810c4d25f148665cbc9f6b226..88e2b9767f1d89cb771e6f5574adb8b2b2f12e69 100644 (file)
--- a/src/lib/dhcp/option_custom.cc
+++ b/src/lib/dhcp/option_custom.cc
@@ -228,8 +228,10 @@ OptionCustom::bufferLength(const OptionDataType data_type, bool in_array,
         // utility function will return the size of the buffer on success.
         if (data_type == OPT_FQDN_TYPE) {
             try {
+                // Set the raw optional falg as we do not want escapes.
                 std::string fqdn =
-                    OptionDataTypeUtil::readFqdn(OptionBuffer(begin, end));
+                    OptionDataTypeUtil::readFqdn(
+                        OptionBuffer(begin, end), true);
                 // The size of the buffer holding an FQDN is always
                 // 1 byte larger than the size of the string
                 // representation of this FQDN.

diff --git a/src/lib/dhcp/option_data_types.cc b/src/lib/dhcp/option_data_types.cc
index ea49933b9bcbe15eb7bfa8565fe336becb40b687..59633ee952995ef72972f7a03ef98828bcfe7a1c 100644 (file)
--- a/src/lib/dhcp/option_data_types.cc
+++ b/src/lib/dhcp/option_data_types.cc
@@ -335,7 +335,8 @@ OptionDataTypeUtil::writeBool(const bool value,
 }
 
 std::string
-OptionDataTypeUtil::readFqdn(const std::vector<uint8_t>& buf) {
+OptionDataTypeUtil::readFqdn(const std::vector<uint8_t>& buf,
+                             bool raw /* = false */) {
     // If buffer is empty emit an error.
     if (buf.empty()) {
         isc_throw(BadDataTypeCast, "unable to read FQDN from a buffer."
@@ -348,7 +349,11 @@ OptionDataTypeUtil::readFqdn(const std::vector<uint8_t>& buf) {
         // it means that the buffer doesn't hold a valid domain name (invalid
         // syntax).
         isc::dns::Name name(in_buf);
-        return (name.toText());
+        if (!raw) {
+            return (name.toText());
+        } else {
+            return (name.toRawText());
+        }
     } catch (const isc::Exception& ex) {
         // Unable to convert the data in the buffer into FQDN.
         isc_throw(BadDataTypeCast, ex.what());

diff --git a/src/lib/dhcp/option_data_types.h b/src/lib/dhcp/option_data_types.h
index fc4e193a41c94c8748f6d2a852ad048d246d72e4..5b4b29bd74469ade44052c76333197eb0db5efe2 100644 (file)
--- a/src/lib/dhcp/option_data_types.h
+++ b/src/lib/dhcp/option_data_types.h
@@ -538,11 +538,13 @@ public:
     /// section 3.1.
     ///
     /// @param buf input buffer holding a FQDN.
+    /// @param raw raw (i.e. not escape) output.
     ///
     /// @throw BadDataTypeCast if a FQDN stored within a buffer is
     /// invalid (e.g. empty, contains invalid characters, truncated).
     /// @return fully qualified domain name in a text form.
-    static std::string readFqdn(const std::vector<uint8_t>& buf);
+    static std::string readFqdn(const std::vector<uint8_t>& buf,
+                                bool raw = false);
 
     /// @brief Append FQDN into a buffer.
     ///

diff --git a/src/lib/dhcp/tests/option_custom_unittest.cc b/src/lib/dhcp/tests/option_custom_unittest.cc
index 780065650f6b4aa06fcec8ed30c9687a4f6cd281..9f5a2af804f9f94bc46bbc033e92d0a25b44d2c3 100644 (file)
--- a/src/lib/dhcp/tests/option_custom_unittest.cc
+++ b/src/lib/dhcp/tests/option_custom_unittest.cc
@@ -458,6 +458,25 @@ TEST_F(OptionCustomTest, fqdnData) {
     Option::lenient_parsing_ = false;
 }
 
+// Same as the previous test but with characters which could be escaped.
+TEST_F(OptionCustomTest, fqdnRawData) {
+    OptionDefinition opt_def("option-foo", 1000, "my-space", "fqdn",
+                             "option-foo-space");
+
+    const char data[] = { 1, 9, 0 }; // "<tab>."
+
+    std::vector<uint8_t> buf(data, data + sizeof(data));
+
+    boost::scoped_ptr<OptionCustom> option;
+    ASSERT_NO_THROW(option.reset(new OptionCustom(opt_def, Option::V6,
+                                                  buf.begin(), buf.end())));
+    ASSERT_TRUE(option);
+    ASSERT_EQ(1U, option->getDataFieldsNum());
+
+    std::string domain0 = option->readFqdn(0);
+    EXPECT_EQ("\\009.", domain0);
+}
+
 // The purpose of this test is to verify that the option definition comprising
 // 16-bit signed integer value can be used to create an instance of custom option.
 TEST_F(OptionCustomTest, int16Data) {

diff --git a/src/lib/dhcp/tests/option_data_types_unittest.cc b/src/lib/dhcp/tests/option_data_types_unittest.cc
index f4bfd663bdd1722a9cbb42b05fbd824e656b4dd9..f9b85b115d2450ee805528cbfc122fbf6ed3c563 100644 (file)
--- a/src/lib/dhcp/tests/option_data_types_unittest.cc
+++ b/src/lib/dhcp/tests/option_data_types_unittest.cc
@@ -524,6 +524,25 @@ TEST_F(OptionDataTypesTest, readFqdn) {
     );
 }
 
+// Same as the previous test but with characters which could be escaped.
+TEST_F(OptionDataTypesTest, readFqdnRaw) {
+    // The binary representation of the "<tab>.".
+    const char data[] = { 1, 9, 0 };
+
+    // Make a vector out of the data.
+    std::vector<uint8_t> buf(data, data + sizeof(data));
+
+    // Read the buffer as FQDN and verify its correctness.
+    std::string fqdn;
+    EXPECT_NO_THROW(fqdn = OptionDataTypeUtil::readFqdn(buf));
+    EXPECT_EQ("\\009.", fqdn);
+    std::string raw;
+    EXPECT_NO_THROW(raw = OptionDataTypeUtil::readFqdn(buf, true));
+    EXPECT_EQ(2U, raw.size());
+    EXPECT_EQ(9, raw[0]);
+    EXPECT_EQ('.', raw[1]);
+}
+
 // The purpose of this test is to verify that FQDN's syntax is validated
 // and that FQDN is correctly written to a buffer in a format described
 // in RFC1035 section 3.1.