From: Francis Dupont <fdupont@isc.org>
Date: Sat, 6 Apr 2024 22:03:01 +0000 (+0200)
Subject: [#3289] Added v6 part
X-Git-Tag: Kea-2.5.8~82
X-Git-Url: http://git.ipfire.org/gitweb/?a=commitdiff_plain;h=a7391768fd0a6f0cc48cb6bde249ce4b8429e757;p=thirdparty%2Fkea.git

[#3289] Added v6 part
---

diff --git a/ChangeLog b/ChangeLog
index d63eb3a33e..f362c674fe 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,7 +1,8 @@
 2217.	[func]		fdupont
 	Extended the lenient-option-parsing compatibility
-	flag to ignore DHCPv4 fqdn (81) option with some
-	invalid domain names (e.g. beginning with an empty label).
+	flag to ignore DHCPv4 fqdn (81) and DHCPv6 client-fqdn
+	(39) options with some invalid domain names (e.g.
+	beginning with an empty label).
 	(Gitlab 3289)
 
 2216.	[func]		tmark
diff --git a/doc/sphinx/arm/dhcp6-srv.rst b/doc/sphinx/arm/dhcp6-srv.rst
index f2f64de053..14ee63a6da 100644
--- a/doc/sphinx/arm/dhcp6-srv.rst
+++ b/doc/sphinx/arm/dhcp6-srv.rst
@@ -7996,6 +7996,9 @@ MiNID.
       }
     }
 
+Starting with Kea version 2.5.8 this is extended to silently ignore
+client-fqdn (39) options with some invalid domain names.
+
 .. _dhcp6_allocation_strategies:
 
 Allocation Strategies in DHCPv6
diff --git a/src/lib/dhcp/option6_client_fqdn.cc b/src/lib/dhcp/option6_client_fqdn.cc
index 9aa9c13a67..bc754f5e3f 100644
--- a/src/lib/dhcp/option6_client_fqdn.cc
+++ b/src/lib/dhcp/option6_client_fqdn.cc
@@ -243,8 +243,13 @@ Option6ClientFqdnImpl::parseWireData(OptionBufferConstIter first,
             try {
                 domain_name_.reset(new isc::dns::Name(name_buf, true));
             } catch (const Exception&) {
-                isc_throw(InvalidOption6FqdnDomainName, "failed to parse "
-                          "partial domain-name from wire format");
+                if (Option::lenient_parsing_) {
+                    isc_throw(SkipThisOptionError, "failed to parse "
+                              "partial domain-name from wire format");
+                } else {
+                    isc_throw(InvalidOption6FqdnDomainName, "failed to parse "
+                              "partial domain-name from wire format");
+                }
             }
             // Terminating zero was missing, so set the domain-name type
             // to partial.
@@ -258,8 +263,13 @@ Option6ClientFqdnImpl::parseWireData(OptionBufferConstIter first,
             try {
                 domain_name_.reset(new isc::dns::Name(name_buf, true));
             } catch (const Exception&) {
-                isc_throw(InvalidOption6FqdnDomainName, "failed to parse "
-                          "fully qualified domain-name from wire format");
+                if (Option::lenient_parsing_) {
+                    isc_throw(SkipThisOptionError, "failed to parse "
+                              "fully qualified domain-name from wire format");
+                } else {
+                    isc_throw(InvalidOption6FqdnDomainName, "failed to parse "
+                              "fully qualified domain-name from wire format");
+                }
             }
             // Set the domain-type to fully qualified domain name.
             domain_name_type_ = Option6ClientFqdn::FULL;
diff --git a/src/lib/dhcp/tests/option6_client_fqdn_unittest.cc b/src/lib/dhcp/tests/option6_client_fqdn_unittest.cc
index c2934899cc..2eadd6b7ee 100644
--- a/src/lib/dhcp/tests/option6_client_fqdn_unittest.cc
+++ b/src/lib/dhcp/tests/option6_client_fqdn_unittest.cc
@@ -16,6 +16,18 @@ namespace {
 using namespace isc;
 using namespace isc::dhcp;
 
+// RAII device to make sure that lenient parsing flag is reset to false on exit.
+class LenientOptionParsing {
+public:
+    LenientOptionParsing(bool value) {
+        Option::lenient_parsing_ = value;
+    }
+
+    ~LenientOptionParsing() {
+        Option::lenient_parsing_ = false;
+    }
+};
+
 // This test verifies that constructor accepts empty partial domain-name but
 // does not accept empty fully qualified domain name.
 TEST(Option6ClientFqdnTest, constructEmptyName) {
@@ -173,10 +185,14 @@ TEST(Option6ClientFqdnTest, constructFromWireTooLongLabel) {
     OptionBuffer in_buf(Option6ClientFqdn::FLAG_S);
     in_buf.push_back(70);
     in_buf.insert(in_buf.end(), 70, 109);
-    in_buf.push_back(0);
+    // Don't add a 0 i.e. use a partial name (vs fully qualified).
 
+    LenientOptionParsing(false);
     EXPECT_THROW(Option6ClientFqdn(in_buf.begin(), in_buf.end()),
                  InvalidOption6FqdnDomainName);
+    Option::lenient_parsing_ = true;
+    EXPECT_THROW(Option6ClientFqdn(in_buf.begin(), in_buf.end()),
+                 SkipThisOptionError);
 }
 
 // Verify that exception is thrown if the overall length of the domain-name
@@ -193,8 +209,12 @@ TEST(Option6ClientFqdnTest, constructFromWireTooLongDomainName) {
     // Terminate FQDN with a dot.
     in_buf.push_back(0);
 
+    LenientOptionParsing(false);
     EXPECT_THROW(Option6ClientFqdn(in_buf.begin(), in_buf.end()),
                  InvalidOption6FqdnDomainName);
+    Option::lenient_parsing_ = true;
+    EXPECT_THROW(Option6ClientFqdn(in_buf.begin(), in_buf.end()),
+                 SkipThisOptionError);
 }
 
 // This test verifies that truncated option is rejected.
@@ -403,6 +423,8 @@ TEST(Option6ClientFqdnTest, constructInvalidName) {
     ASSERT_NO_THROW(Option6ClientFqdn(0, "myhost.example.com"));
 
     // Specify invalid domain name and expect that exception is thrown.
+    // Note in v6 the domain name is always encoded so this is not
+    // covered by lenient-option-parsing.
     EXPECT_THROW(Option6ClientFqdn(0, "my...host.example.com"),
                  InvalidOption6FqdnDomainName);
 }