[4259] kea-dhcp4 now supports replace-client-name modes

    src/bin/dhcp4/dhcp4_messages.mes
        - Added new log message DHCP4_SUPPLY_HOSTNAME

    src/bin/dhcp4/dhcp4_srv.cc
        - Dhcpv4Srv::processClientName() - pushed lack of host option in the
        client request down into processHostoption()

        - Dhcpv4Srv::processHostnameOption() - modified to support the new name
        replacement modes

    src/bin/dhcp4/tests/fqdn_unittest.cc
        - NameDhcpv4SrvTest:: testReplaceClientNameMode() - new method which
        tests a server's handling of a single client packet for a given
        replace-client-name mode.

        - TEST_F(NameDhcpv4SrvTest, replaceClientNameModeTest) - new test which
        exercises the permutations of client packets and replace-client-name
        modes.

src/bin/dhcp4/dhcp4_messages.mes

@@ -1,4 +1,4 @@
-# Copyright (C) 2012-2015 Internet Systems Consortium, Inc. ("ISC")
+# Copyright (C) 2012-2016 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
@@ -672,3 +672,8 @@ will drop its message if the received message was DHCPDISCOVER,
 and will send DHCPNAK if the received message was DHCPREQUEST.
 The argument includes the client and the transaction identification
 information.
+
+% DHCP4_SUPPLY_HOSTNAME %1: client did not send hostname option, will supply onefor them
+This debug message is issued when the server did not receive a Hostname option
+from the client and hostname generation is enabled.  This provides a means to
+create DNS entries for unsophisticated clients.

src/bin/dhcp4/dhcp4_srv.cc

@@ -1074,13 +1074,10 @@ Dhcpv4Srv::processClientName(Dhcpv4Exchange& ex) {
             processClientFqdnOption(ex);
 
         } else {
-            OptionStringPtr hostname = boost::dynamic_pointer_cast<OptionString>
-                (ex.getQuery()->getOption(DHO_HOST_NAME));
-            if (hostname) {
-                LOG_DEBUG(ddns4_logger, DBG_DHCP4_DETAIL, DHCP4_CLIENT_HOSTNAME_PROCESS)
+            LOG_DEBUG(ddns4_logger, DBG_DHCP4_DETAIL,
+                      DHCP4_CLIENT_HOSTNAME_PROCESS)
                     .arg(ex.getQuery()->getLabel());
-                processHostnameOption(ex);
-            }
+            processHostnameOption(ex);
         }
     } catch (const Exception& e) {
         // In some rare cases it is possible that the client's name processing
@@ -1151,14 +1148,6 @@ Dhcpv4Srv::processClientFqdnOption(Dhcpv4Exchange& ex) {
 
 void
 Dhcpv4Srv::processHostnameOption(Dhcpv4Exchange& ex) {
-    // Obtain the Hostname option from the client's message.
-    OptionStringPtr opt_hostname = boost::dynamic_pointer_cast<OptionString>
-        (ex.getQuery()->getOption(DHO_HOST_NAME));
-
-    LOG_DEBUG(ddns4_logger, DBG_DHCP4_DETAIL_DATA, DHCP4_CLIENT_HOSTNAME_DATA)
-        .arg(ex.getQuery()->getLabel())
-        .arg(opt_hostname->getValue());
-
     // Fetch D2 configuration.
     D2ClientMgr& d2_mgr = CfgMgr::instance().getD2ClientMgr();
 
@@ -1167,6 +1156,36 @@ Dhcpv4Srv::processHostnameOption(Dhcpv4Exchange& ex) {
         return;
     }
 
+    D2ClientConfig::ReplaceClientNameMode replace_name_mode =
+            d2_mgr.getD2ClientConfig()->getReplaceClientNameMode();
+
+    // Obtain the Hostname option from the client's message.
+    OptionStringPtr opt_hostname = boost::dynamic_pointer_cast<OptionString>
+        (ex.getQuery()->getOption(DHO_HOST_NAME));
+
+    // If we don't have a hostname then either we'll supply it or do nothing.
+    if (!opt_hostname) {
+        // If we're configured to supply it then add it to the response.
+        // Use the root domain to signal later on that we should replace it.
+        if (replace_name_mode == D2ClientConfig::RCM_ALWAYS ||
+            replace_name_mode == D2ClientConfig::RCM_WHEN_NOT_PRESENT) {
+            LOG_DEBUG(ddns4_logger, DBG_DHCP4_DETAIL_DATA,
+                      DHCP4_SUPPLY_HOSTNAME)
+                .arg(ex.getQuery()->getLabel());
+            OptionStringPtr opt_hostname_resp(new OptionString(Option::V4,
+                                                               DHO_HOST_NAME,
+                                                               "."));
+            ex.getResponse()->addOption(opt_hostname_resp);
+        }
+
+        return;
+    }
+
+    // Client sent us a hostname option so figure out what to do with it.
+    LOG_DEBUG(ddns4_logger, DBG_DHCP4_DETAIL_DATA, DHCP4_CLIENT_HOSTNAME_DATA)
+        .arg(ex.getQuery()->getLabel())
+        .arg(opt_hostname->getValue());
+
     std::string hostname = isc::util::str::trim(opt_hostname->getValue());
     unsigned int label_count = OptionDataTypeUtil::getLabelCount(hostname);
     // The hostname option sent by the client should be at least 1 octet long.
@@ -1196,14 +1215,16 @@ Dhcpv4Srv::processHostnameOption(Dhcpv4Exchange& ex) {
         opt_hostname_resp->setValue(d2_mgr.qualifyName(ex.getContext()->host_->getHostname(),
                                                        false));
 
-    } else if ((d2_mgr.getD2ClientConfig()->getReplaceClientNameMode()
-                != D2ClientConfig::RCM_NEVER) || (label_count < 2)) {
+    } else if ((replace_name_mode == D2ClientConfig::RCM_ALWAYS ||
+               replace_name_mode == D2ClientConfig::RCM_WHEN_PRESENT)
+               || label_count < 2) {
         // Set to root domain to signal later on that we should replace it.
         // DHO_HOST_NAME is a string option which cannot be empty.
         /// @todo We may want to reconsider whether it is appropriate for the
         /// client to send a root domain name as a Hostname. There are
         /// also extensions to the auto generation of the client's name,
-        /// e.g. conversion to the puny code which may be considered at some point.
+        /// e.g. conversion to the puny code which may be considered at some
+        /// point.
         /// For now, we just remain liberal and expect that the DNS will handle
         /// conversion if needed and possible.
         opt_hostname_resp->setValue(".");

src/bin/dhcp4/tests/fqdn_unittest.cc

@@ -128,6 +128,20 @@ public:
     static const uint16_t OVERRIDE_CLIENT_UPDATE = 4;
     static const uint16_t REPLACE_CLIENT_NAME = 8;
 
+    // Enum used to specify whether a client (packet) should include
+    // the hostname option
+    enum ClientNameFlag {
+        CLIENT_NAME_PRESENT,
+        CLIENT_NAME_NOT_PRESENT
+    };
+
+    // Enum used to specify whether the server should replace/supply
+    // the hostname or not
+    enum ReplacementFlag {
+        NAME_REPLACED,
+        NAME_NOT_REPLACED
+    };
+
     NameDhcpv4SrvTest()
         : Dhcpv4SrvTest(),
           d2_mgr_(CfgMgr::instance().getD2ClientMgr()),
@@ -173,7 +187,8 @@ public:
                                   (mask & OVERRIDE_NO_UPDATE),
                                   (mask & OVERRIDE_CLIENT_UPDATE),
                                   ((mask & REPLACE_CLIENT_NAME) ?
-                                    D2ClientConfig::RCM_WHEN_PRESENT : D2ClientConfig::RCM_NEVER),
+                                    D2ClientConfig::RCM_WHEN_PRESENT
+                                   : D2ClientConfig::RCM_NEVER),
                                   "myhost", "example.com")));
         ASSERT_NO_THROW(CfgMgr::instance().setD2ClientConfig(cfg));
         ASSERT_NO_THROW(srv_->startD2());
@@ -185,7 +200,8 @@ public:
                           const bool fqdn_fwd,
                           const bool fqdn_rev) {
         const uint8_t hwaddr_data[] = { 0, 1, 2, 3, 4, 5, 6 };
-        HWAddrPtr hwaddr(new HWAddr(hwaddr_data, sizeof(hwaddr_data), HTYPE_ETHER));
+        HWAddrPtr hwaddr(new HWAddr(hwaddr_data, sizeof(hwaddr_data),
+                                    HTYPE_ETHER));
         Lease4Ptr lease(new Lease4(addr, hwaddr,
                                    &generateClientId()->getData()[0],
                                    generateClientId()->getData().size(),
@@ -310,6 +326,20 @@ public:
 
     }
 
+    // Create a message holding of a given type
+    Pkt4Ptr generatePkt(const uint8_t msg_type) {
+        Pkt4Ptr pkt = Pkt4Ptr(new Pkt4(msg_type, 1234));
+        pkt->setRemoteAddr(IOAddress("192.0.2.3"));
+        // For DISCOVER we don't include server id, because client broadcasts
+        // the message to all servers.
+        if (msg_type != DHCPDISCOVER) {
+            pkt->addOption(srv_->getServerID());
+        }
+
+        pkt->addOption(generateClientId());
+        return (pkt);
+    }
+
     // Test that server generates the appropriate FQDN option in response to
     // client's FQDN option.
     void testProcessFqdn(const Pkt4Ptr& query, const uint8_t exp_flags,
@@ -339,6 +369,75 @@ public:
 
     }
 
+    // Test that the server's processes the hostname (or lack thereof)
+    // in a client request correctly, according to the replace-client-name
+    // mode configuration parameter.
+    //
+    // @param mode - value to use client-name-replacment parameter - for
+    // mode labels such as NEVER and ALWAYS must incluce enclosing quotes:
+    // "\"NEVER\"".  This allows us to also pass in boolean literals which
+    // are unquoted.
+    // @param client_name_flag - specifies whether or not the client request
+    // should contain a hostname option
+    // @param exp_replacement_flag - specifies whether or not the server is
+    // expected to replace (or supply) the hostname in its response
+    void testReplaceClientNameMode(const char* mode,
+                                   enum ClientNameFlag client_name_flag,
+                                   enum ReplacementFlag exp_replacement_flag) {
+        // Configuration "template" with a replaceable mode parameter
+        const char* config_template =
+            "{ \"interfaces-config\": {"
+            "      \"interfaces\": [ \"*\" ]"
+            "},"
+            "\"valid-lifetime\": 3000,"
+            "\"subnet4\": [ { "
+            "    \"subnet\": \"10.0.0.0/24\", "
+            "    \"id\": 1,"
+            "    \"pools\": [ { \"pool\": \"10.0.0.10-10.0.0.10\" } ]"
+            " }],"
+            "\"dhcp-ddns\": {"
+            "\"enable-updates\": true,"
+            "\"qualifying-suffix\": \"fake-suffix.isc.org.\","
+            "\"replace-client-name\": %s"
+            "}}";
+
+        // Create the configuration and configure the server
+        char config_buf[1024];
+        sprintf(config_buf, config_template, mode);
+        configure(config_buf, srv_);
+
+        // Build our client packet
+        Pkt4Ptr query;
+        if (client_name_flag == CLIENT_NAME_PRESENT) {
+            ASSERT_NO_THROW(query = generatePktWithHostname(DHCPREQUEST,
+                                                            "my.example.com."));
+        } else {
+            ASSERT_NO_THROW(query = generatePkt(DHCPREQUEST));
+        }
+
+        // Run the packet through the server, extracting the hostname option
+        // from the response.  If the option isn't present the returned pointer
+        // will be null.
+        OptionStringPtr hostname;
+        ASSERT_NO_THROW(
+            hostname = processHostname(query,
+                                       client_name_flag == CLIENT_NAME_PRESENT)
+        );
+
+        // Verify the contents (or lack thereof) of the hostname
+        if (exp_replacement_flag == NAME_REPLACED) {
+            ASSERT_TRUE(hostname);
+            EXPECT_EQ(".", hostname->getValue());
+        } else {
+            if (client_name_flag == CLIENT_NAME_PRESENT) {
+                ASSERT_TRUE(hostname);
+                EXPECT_EQ("my.example.com.", hostname->getValue());
+            } else {
+                ASSERT_FALSE(hostname);
+            }
+        }
+    }
+
     /// @brief Checks the packet's FQDN option flags against a given mask
     ///
     /// @param pkt IPv4 packet whose FQDN flags should be checked.
@@ -363,8 +462,8 @@ public:
     // the hostname option which would be sent to the client. It will
     // throw NULL pointer if the hostname option is not to be included
     // in the response.
-    OptionStringPtr processHostname(const Pkt4Ptr& query) {
-        if (!getHostnameOption(query)) {
+    OptionStringPtr processHostname(const Pkt4Ptr& query, bool must_have_host = true) {
+        if (!getHostnameOption(query) && must_have_host) {
             ADD_FAILURE() << "Hostname option not carried in the query";
         }
 
@@ -1282,4 +1381,43 @@ TEST_F(NameDhcpv4SrvTest, emptyFqdn) {
 
 }
 
+// Verifies that the replace-client-name behavior is correct for each of
+// the supported modes.
+TEST_F(NameDhcpv4SrvTest, replaceClientNameModeTest) {
+
+    // We pass mode labels in with enclosing quotes so we can also test
+    // unquoted boolean literals true/false
+    testReplaceClientNameMode("\"NEVER\"",
+                              CLIENT_NAME_NOT_PRESENT, NAME_NOT_REPLACED);
+    testReplaceClientNameMode("\"NEVER\"",
+                              CLIENT_NAME_PRESENT, NAME_NOT_REPLACED);
+
+    testReplaceClientNameMode("\"ALWAYS\"",
+                              CLIENT_NAME_NOT_PRESENT, NAME_REPLACED);
+    testReplaceClientNameMode("\"ALWAYS\"",
+                              CLIENT_NAME_PRESENT, NAME_REPLACED);
+
+    testReplaceClientNameMode("\"WHEN_PRESENT\"",
+                              CLIENT_NAME_NOT_PRESENT, NAME_NOT_REPLACED);
+    testReplaceClientNameMode("\"WHEN_PRESENT\"",
+                              CLIENT_NAME_PRESENT, NAME_REPLACED);
+
+    testReplaceClientNameMode("\"WHEN_NOT_PRESENT\"",
+                              CLIENT_NAME_NOT_PRESENT, NAME_REPLACED);
+    testReplaceClientNameMode("\"WHEN_NOT_PRESENT\"",
+                              CLIENT_NAME_PRESENT, NAME_NOT_REPLACED);
+
+    // Verify that boolean false produces the same result as RCM_NEVER
+    testReplaceClientNameMode("false",
+                              CLIENT_NAME_NOT_PRESENT, NAME_NOT_REPLACED);
+    testReplaceClientNameMode("false",
+                              CLIENT_NAME_PRESENT, NAME_NOT_REPLACED);
+
+    // Verify that boolean true produces the same result as RCM_WHEN_PRESENT
+    testReplaceClientNameMode("true",
+                              CLIENT_NAME_NOT_PRESENT, NAME_NOT_REPLACED);
+    testReplaceClientNameMode("true",
+                              CLIENT_NAME_PRESENT, NAME_REPLACED);
+}
+
 } // end of anonymous namespace