Browse Source

[3035] Generate client hostname from IP address acquired.

Marcin Siodelski 11 years ago
parent
commit
9dc963b3f1
3 changed files with 126 additions and 22 deletions
  1. 6 0
      src/bin/dhcp4/dhcp4_messages.mes
  2. 49 16
      src/bin/dhcp4/dhcp4_srv.cc
  3. 71 6
      src/bin/dhcp4/tests/fqdn_unittest.cc

+ 6 - 0
src/bin/dhcp4/dhcp4_messages.mes

@@ -139,6 +139,12 @@ specified client after receiving a REQUEST message from it.  There are many
 possible reasons for such a failure. Additional messages will indicate the
 possible reasons for such a failure. Additional messages will indicate the
 reason.
 reason.
 
 
+% DHCP4_NAME_GEN_UPDATE_FAIL failed to update the lease after generating name for a client: %1
+This message indicates the failure when trying to update the lease and/or
+options in the server's response with the hostname generated by the server
+from the acquired address. The message argument indicates the reason for the
+failure.
+
 % DHCP4_NCR_CREATION_FAILED failed to generate name change requests for DNS: %1
 % DHCP4_NCR_CREATION_FAILED failed to generate name change requests for DNS: %1
 This message indicates that server was unable to generate so called
 This message indicates that server was unable to generate so called
 NameChangeRequests which should be sent to the b10-dhcp_ddns module to create
 NameChangeRequests which should be sent to the b10-dhcp_ddns module to create

+ 49 - 16
src/bin/dhcp4/dhcp4_srv.cc

@@ -89,9 +89,6 @@ const bool FQDN_ALWAYS_INCLUDE = false;
 const bool FQDN_ALLOW_CLIENT_UPDATE = false;
 const bool FQDN_ALLOW_CLIENT_UPDATE = false;
 // Globally enable updates (Enabled).
 // Globally enable updates (Enabled).
 const bool FQDN_ENABLE_UPDATE = true;
 const bool FQDN_ENABLE_UPDATE = true;
-// The partial name generated for the client if empty name has been
-// supplied.
-const char* FQDN_GENERATED_PARTIAL_NAME = "myhost";
 // Do update, even if client requested no updates with N flag (Disabled).
 // Do update, even if client requested no updates with N flag (Disabled).
 const bool FQDN_OVERRIDE_NO_UPDATE = false;
 const bool FQDN_OVERRIDE_NO_UPDATE = false;
 // Server performs an update when client requested delegation (Enabled).
 // Server performs an update when client requested delegation (Enabled).
@@ -810,19 +807,21 @@ Dhcpv4Srv::processClientFqdnOption(const Option4ClientFqdnPtr& fqdn,
     if (fqdn->getDomainNameType() == Option4ClientFqdn::PARTIAL) {
     if (fqdn->getDomainNameType() == Option4ClientFqdn::PARTIAL) {
         std::ostringstream name;
         std::ostringstream name;
         if (fqdn->getDomainName().empty() || FQDN_REPLACE_CLIENT_NAME) {
         if (fqdn->getDomainName().empty() || FQDN_REPLACE_CLIENT_NAME) {
-            name << FQDN_GENERATED_PARTIAL_NAME;
+            fqdn_resp->setDomainName("", Option4ClientFqdn::PARTIAL);
+
         } else {
         } else {
             name << fqdn->getDomainName();
             name << fqdn->getDomainName();
+            name << "." << FQDN_PARTIAL_SUFFIX;
+            fqdn_resp->setDomainName(name.str(), Option4ClientFqdn::FULL);
+
         }
         }
-        name << "." << FQDN_PARTIAL_SUFFIX;
-        fqdn_resp->setDomainName(name.str(), Option4ClientFqdn::FULL);
 
 
     // Server may be configured to replace a name supplied by a client, even if
     // Server may be configured to replace a name supplied by a client, even if
-    // client supplied fully qualified domain-name.
+    // client supplied fully qualified domain-name. The empty domain-name is
+    // is set to indicate that the name must be generated when the new lease
+    // is acquired.
     } else if(FQDN_REPLACE_CLIENT_NAME) {
     } else if(FQDN_REPLACE_CLIENT_NAME) {
-        std::ostringstream name;
-        name << FQDN_GENERATED_PARTIAL_NAME << "." << FQDN_PARTIAL_SUFFIX;
-        fqdn_resp->setDomainName(name.str(), Option4ClientFqdn::FULL);
+        fqdn_resp->setDomainName("", Option4ClientFqdn::PARTIAL);
     }
     }
 
 
     // Add FQDN option to the response message. Note that, there may be some
     // Add FQDN option to the response message. Note that, there may be some
@@ -866,10 +865,7 @@ Dhcpv4Srv::processHostnameOption(const OptionCustomPtr& opt_hostname,
     // If there is only one label, it is a root. We will have to generate
     // If there is only one label, it is a root. We will have to generate
     // the whole domain name for the client.
     // the whole domain name for the client.
     if (FQDN_REPLACE_CLIENT_NAME || (label_count < 2)) {
     if (FQDN_REPLACE_CLIENT_NAME || (label_count < 2)) {
-        std::ostringstream resp_hostname;
-        resp_hostname << FQDN_GENERATED_PARTIAL_NAME << "."
-                      << FQDN_PARTIAL_SUFFIX << ".";
-        opt_hostname_resp->writeString(resp_hostname.str());
+        opt_hostname_resp->writeString("");
     // If there are two labels, it means that the client has specified
     // If there are two labels, it means that the client has specified
     // the unqualified name. We have to concatenate the unqalified name
     // the unqualified name. We have to concatenate the unqalified name
     // with the domain name.
     // with the domain name.
@@ -1022,6 +1018,7 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) {
     std::string hostname;
     std::string hostname;
     bool fqdn_fwd = false;
     bool fqdn_fwd = false;
     bool fqdn_rev = false;
     bool fqdn_rev = false;
+    OptionCustomPtr opt_hostname;
     Option4ClientFqdnPtr fqdn = boost::dynamic_pointer_cast<
     Option4ClientFqdnPtr fqdn = boost::dynamic_pointer_cast<
         Option4ClientFqdn>(answer->getOption(DHO_FQDN));
         Option4ClientFqdn>(answer->getOption(DHO_FQDN));
     if (fqdn) {
     if (fqdn) {
@@ -1029,8 +1026,8 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) {
         fqdn_fwd = fqdn->getFlag(Option4ClientFqdn::FLAG_S);
         fqdn_fwd = fqdn->getFlag(Option4ClientFqdn::FLAG_S);
         fqdn_rev = !fqdn->getFlag(Option4ClientFqdn::FLAG_N);
         fqdn_rev = !fqdn->getFlag(Option4ClientFqdn::FLAG_N);
     } else {
     } else {
-        OptionCustomPtr opt_hostname = boost::dynamic_pointer_cast<
-            OptionCustom>(answer->getOption(DHO_HOST_NAME));
+        opt_hostname = boost::dynamic_pointer_cast<OptionCustom>
+            (answer->getOption(DHO_HOST_NAME));
         if (opt_hostname) {
         if (opt_hostname) {
             hostname = opt_hostname->readString();
             hostname = opt_hostname->readString();
             // @todo It could be configurable what sort of updates the server
             // @todo It could be configurable what sort of updates the server
@@ -1064,6 +1061,42 @@ Dhcpv4Srv::assignLease(const Pkt4Ptr& question, Pkt4Ptr& answer) {
 
 
         answer->setYiaddr(lease->addr_);
         answer->setYiaddr(lease->addr_);
 
 
+        // If there has been Client FQDN or Hostname option sent, but the
+        // hostname is empty, it means that server is responsible for
+        // generating the entire hostname for the client. The example of the
+        // client's name, generated from the IP address is: host-192-0-2-3.
+        if ((fqdn || opt_hostname) && lease->hostname_.empty()) {
+            hostname = lease->addr_.toText();
+            // Replace dots with hyphens.
+            std::replace(hostname.begin(), hostname.end(), '.', '-');
+            ostringstream stream;
+            // The partial suffix will need to be replaced with the actual
+            // domain-name for the client when configuration is implemented.
+            stream << "host-" << hostname << "." << FQDN_PARTIAL_SUFFIX << ".";
+            lease->hostname_ = stream.str();
+            // The operations below are rather safe, but we want to catch
+            // any potential exceptions (e.g. invalid lease database backend
+            // implementation) and log an error.
+            try {
+                // The lease update should be safe, because the lease should
+                // be already in the database. In most cases the exception
+                // would be thrown if the lease was missing.
+                LeaseMgrFactory::instance().updateLease4(lease);
+                // The name update in the option should be also safe,
+                // because the generated name is well formed.
+                if (fqdn) {
+                    fqdn->setDomainName(lease->hostname_,
+                                        Option4ClientFqdn::FULL);
+                } else if (opt_hostname) {
+                    opt_hostname->writeString(lease->hostname_);
+                }
+
+            } catch (const Exception& ex) {
+                LOG_ERROR(dhcp4_logger, DHCP4_NAME_GEN_UPDATE_FAIL)
+                    .arg(ex.what());
+            }
+        }
+
         // IP Address Lease time (type 51)
         // IP Address Lease time (type 51)
         opt = OptionPtr(new Option(Option::V4, DHO_DHCP_LEASE_TIME));
         opt = OptionPtr(new Option(Option::V4, DHO_DHCP_LEASE_TIME));
         opt->setUint32(lease->valid_lft_);
         opt->setUint32(lease->valid_lft_);

+ 71 - 6
src/bin/dhcp4/tests/fqdn_unittest.cc

@@ -76,6 +76,17 @@ public:
         return (opt_hostname);
         return (opt_hostname);
     }
     }
 
 
+    // Generates partial hostname from the address. The format of the
+    // generated address is: host-A-B-C-D, where A.B.C.D is an IP
+    // address.
+    std::string generatedNameFromAddress(const IOAddress& addr) {
+        std::string gen_name = addr.toText();
+        std::replace(gen_name.begin(), gen_name.end(), '.', '-');
+        std::ostringstream hostname;
+        hostname << "host-" << gen_name;
+        return (hostname.str());
+    }
+
     // Get the Client FQDN Option from the given message.
     // Get the Client FQDN Option from the given message.
     Option4ClientFqdnPtr getClientFqdnOption(const Pkt4Ptr& pkt) {
     Option4ClientFqdnPtr getClientFqdnOption(const Pkt4Ptr& pkt) {
         return (boost::dynamic_pointer_cast<
         return (boost::dynamic_pointer_cast<
@@ -153,7 +164,9 @@ public:
     // Test that server generates the appropriate FQDN option in response to
     // Test that server generates the appropriate FQDN option in response to
     // client's FQDN option.
     // client's FQDN option.
     void testProcessFqdn(const Pkt4Ptr& query, const uint8_t exp_flags,
     void testProcessFqdn(const Pkt4Ptr& query, const uint8_t exp_flags,
-                         const std::string& exp_domain_name) {
+                         const std::string& exp_domain_name,
+                         const Option4ClientFqdn::DomainNameType
+                         exp_domain_type = Option4ClientFqdn::FULL) {
         ASSERT_TRUE(getClientFqdnOption(query));
         ASSERT_TRUE(getClientFqdnOption(query));
 
 
         Pkt4Ptr answer;
         Pkt4Ptr answer;
@@ -180,7 +193,7 @@ public:
         EXPECT_EQ(flag_e, fqdn->getFlag(Option4ClientFqdn::FLAG_E));
         EXPECT_EQ(flag_e, fqdn->getFlag(Option4ClientFqdn::FLAG_E));
 
 
         EXPECT_EQ(exp_domain_name, fqdn->getDomainName());
         EXPECT_EQ(exp_domain_name, fqdn->getDomainName());
-        EXPECT_EQ(Option4ClientFqdn::FULL, fqdn->getDomainNameType());
+        EXPECT_EQ(exp_domain_type, fqdn->getDomainNameType());
 
 
     }
     }
 
 
@@ -253,7 +266,11 @@ public:
         EXPECT_EQ(reverse, ncr.isReverseChange());
         EXPECT_EQ(reverse, ncr.isReverseChange());
         EXPECT_EQ(addr, ncr.getIpAddress());
         EXPECT_EQ(addr, ncr.getIpAddress());
         EXPECT_EQ(fqdn, ncr.getFqdn());
         EXPECT_EQ(fqdn, ncr.getFqdn());
-        EXPECT_EQ(dhcid, ncr.getDhcid().toStr());
+        // Compare dhcid if it is not empty. In some cases, the DHCID is
+        // not known in advance and can't be compared.
+        if (!dhcid.empty()) {
+            EXPECT_EQ(dhcid, ncr.getDhcid().toStr());
+        }
         EXPECT_EQ(expires, ncr.getLeaseExpiresOn());
         EXPECT_EQ(expires, ncr.getLeaseExpiresOn());
         EXPECT_EQ(len, ncr.getLeaseLength());
         EXPECT_EQ(len, ncr.getLeaseLength());
         EXPECT_EQ(isc::dhcp_ddns::ST_NEW, ncr.getStatus());
         EXPECT_EQ(isc::dhcp_ddns::ST_NEW, ncr.getStatus());
@@ -369,8 +386,10 @@ TEST_F(NameDhcpv4SrvTest, serverUpdateUnqualifiedHostname) {
     testProcessHostname(query, "myhost.example.com.");
     testProcessHostname(query, "myhost.example.com.");
 }
 }
 
 
-// Test that server generates the fully qualified domain name for the client
-// if clietn supplies empty domain name.
+// Test that server sets empty domain-name in the FQDN option when client
+// supplied no domain-name. The domain-name is supposed to be set after the
+// lease is acquired. The domain-name is then generated from the IP address
+// assigned to a client.
 TEST_F(NameDhcpv4SrvTest, serverUpdateForwardNoNameFqdn) {
 TEST_F(NameDhcpv4SrvTest, serverUpdateForwardNoNameFqdn) {
     Pkt4Ptr query = generatePktWithFqdn(DHCPREQUEST,
     Pkt4Ptr query = generatePktWithFqdn(DHCPREQUEST,
                                         Option4ClientFqdn::FLAG_E |
                                         Option4ClientFqdn::FLAG_E |
@@ -381,7 +400,7 @@ TEST_F(NameDhcpv4SrvTest, serverUpdateForwardNoNameFqdn) {
 
 
     testProcessFqdn(query,
     testProcessFqdn(query,
                     Option4ClientFqdn::FLAG_E | Option4ClientFqdn::FLAG_S,
                     Option4ClientFqdn::FLAG_E | Option4ClientFqdn::FLAG_S,
-                    "myhost.example.com.");
+                    "", Option4ClientFqdn::PARTIAL);
 
 
 }
 }
 
 
@@ -525,6 +544,52 @@ TEST_F(NameDhcpv4SrvTest, processDiscover) {
     EXPECT_TRUE(srv_->name_change_reqs_.empty());
     EXPECT_TRUE(srv_->name_change_reqs_.empty());
 }
 }
 
 
+// Test that server generates client's hostname from the IP address assigned
+// to it when DHCPv4 Client FQDN option specifies an empty domain-name.
+TEST_F(NameDhcpv4SrvTest, processRequestFqdnEmptyDomainName) {
+    Pkt4Ptr req = generatePktWithFqdn(DHCPREQUEST, Option4ClientFqdn::FLAG_S |
+                                      Option4ClientFqdn::FLAG_E,
+                                      "", Option4ClientFqdn::PARTIAL, true);
+
+    Pkt4Ptr reply;
+    ASSERT_NO_THROW(reply = srv_->processRequest(req));
+
+    checkResponse(reply, DHCPACK, 1234);
+
+    // Verify that there is one NameChangeRequest generated.
+    ASSERT_EQ(1, srv_->name_change_reqs_.size());
+    // The hostname is generated from the IP address acquired (yiaddr).
+    std::ostringstream hostname;
+    hostname << generatedNameFromAddress(reply->getYiaddr())
+             << ".example.com.";
+    verifyNameChangeRequest(isc::dhcp_ddns::CHG_ADD, true, true,
+                            reply->getYiaddr().toText(), hostname.str(),
+                            "", // empty DHCID forces that it is not checked
+                            0, subnet_->getValid());
+}
+
+// Test that server generates client's hostname from the IP address assigned
+// to it when Hostname option carries the top level domain-name.
+TEST_F(NameDhcpv4SrvTest, processRequestEmptyHostname) {
+    Pkt4Ptr req = generatePktWithHostname(DHCPREQUEST, ".");
+
+    Pkt4Ptr reply;
+    ASSERT_NO_THROW(reply = srv_->processRequest(req));
+
+    checkResponse(reply, DHCPACK, 1234);
+
+    // Verify that there is one NameChangeRequest generated.
+    ASSERT_EQ(1, srv_->name_change_reqs_.size());
+    // The hostname is generated from the IP address acquired (yiaddr).
+    std::ostringstream hostname;
+    hostname << generatedNameFromAddress(reply->getYiaddr()) << ".example.com.";
+    verifyNameChangeRequest(isc::dhcp_ddns::CHG_ADD, true, true,
+                            reply->getYiaddr().toText(), hostname.str(),
+                            "", // empty DHCID forces that it is not checked
+                            0, subnet_->getValid());
+}
+
+
 // Test that client may send two requests, each carrying FQDN option with
 // Test that client may send two requests, each carrying FQDN option with
 // a different domain-name. Server should use existing lease for the second
 // a different domain-name. Server should use existing lease for the second
 // request but modify the DNS entries for the lease according to the contents
 // request but modify the DNS entries for the lease according to the contents