[3231] Removed fixed server identifier option from the DHCPv4 server.

src/bin/dhcp4/dhcp4_messages.mes

@@ -289,26 +289,6 @@ both clones use the same client-id.
 % DHCP4_RESPONSE_DATA responding with packet type %1, data is <%2>
 A debug message listing the data returned to the client.
 
-% DHCP4_SERVERID_GENERATED server-id %1 has been generated and will be stored in %2
-This informational messages indicates that the server was not able to
-read its server identifier and has generated a new one. This server-id
-will be stored in a file and will be read (and used) whenever the server
-is restarted. This is normal behavior when the server is started for the
-first time. If this message is printed every time the server is started,
-please check that the server has sufficient permission to write its
-server-id file and that the file is not corrupt.
-
-% DHCP4_SERVERID_LOADED server-id %1 has been loaded from file %2
-This debug message indicates that the server loaded its server identifier.
-That value is sent in all server responses and clients use it to
-discriminate between servers. This is a part of normal startup or
-reconfiguration procedure.
-
-% DHCP4_SERVERID_WRITE_FAIL server was not able to write its ID to file %1
-This warning message indicates that server was not able to write its
-server identifier to a file. The most likely cause is is that the server
-does not have permissions to write the server id file.
-
 % DHCP4_SERVER_FAILED server failed: %1
 The IPv4 DHCP server has encountered a fatal error and is terminating.
 The reason for the failure is included in the message.

src/bin/dhcp4/dhcp4_srv.cc

@@ -37,12 +37,10 @@
 #include <hooks/hooks_manager.h>
 #include <util/strutil.h>
 
-#include <boost/algorithm/string/erase.hpp>
 #include <boost/bind.hpp>
 #include <boost/foreach.hpp>
 
 #include <iomanip>
-#include <fstream>
 
 using namespace isc;
 using namespace isc::asiolink;
@@ -111,21 +109,9 @@ const bool FQDN_REPLACE_CLIENT_NAME = false;
 
 }
 
-/// @brief file name of a server-id file
-///
-/// Server must store its server identifier in persistent storage that must not
-/// change between restarts. This is name of the file that is created in dataDir
-/// (see isc::dhcp::CfgMgr::getDataDir()). It is a text file that uses
-/// regular IPv4 address, e.g. 192.0.2.1. Server will create it during
-/// first run and then use it afterwards.
-static const char* SERVER_ID_FILE = "b10-dhcp4-serverid";
-
-// These are hardcoded parameters. Currently this is a skeleton server that only
-// grants those options and a single, fixed, hardcoded lease.
-
 Dhcpv4Srv::Dhcpv4Srv(uint16_t port, const char* dbconfig, const bool use_bcast,
                      const bool direct_response_desired)
-: serverid_(), shutdown_(true), alloc_engine_(), port_(port),
+: shutdown_(true), alloc_engine_(), port_(port),
     use_bcast_(use_bcast), hook_index_pkt4_receive_(-1),
     hook_index_subnet4_select_(-1), hook_index_pkt4_send_(-1) {
 
@@ -153,24 +139,6 @@ Dhcpv4Srv::Dhcpv4Srv(uint16_t port, const char* dbconfig, const bool use_bcast,
             IfaceMgr::instance().openSockets4(port_, use_bcast_, error_handler);
         }
 
-        string srvid_file = CfgMgr::instance().getDataDir() + "/" + string(SERVER_ID_FILE);
-        if (loadServerID(srvid_file)) {
-            LOG_DEBUG(dhcp4_logger, DBG_DHCP4_START, DHCP4_SERVERID_LOADED)
-                .arg(srvidToString(getServerID()))
-                .arg(srvid_file);
-        } else {
-            generateServerID();
-            LOG_INFO(dhcp4_logger, DHCP4_SERVERID_GENERATED)
-                .arg(srvidToString(getServerID()))
-                .arg(srvid_file);
-
-            if (!writeServerID(srvid_file)) {
-                LOG_WARN(dhcp4_logger, DHCP4_SERVERID_WRITE_FAIL)
-                    .arg(srvid_file);
-            }
-
-        }
-
         // Instantiate LeaseMgr
         LeaseMgrFactory::create(dbconfig);
         LOG_INFO(dhcp4_logger, DHCP4_DB_BACKEND_STARTED)
@@ -476,90 +444,6 @@ Dhcpv4Srv::run() {
     return (true);
 }
 
-bool
-Dhcpv4Srv::loadServerID(const std::string& file_name) {
-
-    // load content of the file into a string
-    fstream f(file_name.c_str(), ios::in);
-    if (!f.is_open()) {
-        return (false);
-    }
-
-    string hex_string;
-    f >> hex_string;
-    f.close();
-
-    // remove any spaces
-    boost::algorithm::erase_all(hex_string, " ");
-
-    try {
-        IOAddress addr(hex_string);
-
-        if (!addr.isV4()) {
-            return (false);
-        }
-
-        // Now create server-id option
-        serverid_.reset(new Option4AddrLst(DHO_DHCP_SERVER_IDENTIFIER, addr));
-
-    } catch(...) {
-        // any kind of malformed input (empty string, IPv6 address, complete
-        // garbate etc.)
-        return (false);
-    }
-
-    return (true);
-}
-
-void
-Dhcpv4Srv::generateServerID() {
-
-    const IfaceMgr::IfaceCollection& ifaces = IfaceMgr::instance().getIfaces();
-
-    // Let's find suitable interface.
-    for (IfaceMgr::IfaceCollection::const_iterator iface = ifaces.begin();
-         iface != ifaces.end(); ++iface) {
-
-        // Let's don't use loopback.
-        if (iface->flag_loopback_) {
-            continue;
-        }
-
-        // Let's skip downed interfaces. It is better to use working ones.
-        if (!iface->flag_up_) {
-            continue;
-        }
-
-        const Iface::AddressCollection addrs = iface->getAddresses();
-
-        for (Iface::AddressCollection::const_iterator addr = addrs.begin();
-             addr != addrs.end(); ++addr) {
-            if (addr->getFamily() != AF_INET) {
-                continue;
-            }
-
-            serverid_ = OptionPtr(new Option4AddrLst(DHO_DHCP_SERVER_IDENTIFIER,
-                                                     *addr));
-            return;
-        }
-
-
-    }
-
-    isc_throw(BadValue, "No suitable interfaces for server-identifier found");
-}
-
-bool
-Dhcpv4Srv::writeServerID(const std::string& file_name) {
-    fstream f(file_name.c_str(), ios::out | ios::trunc);
-    if (!f.good()) {
-        return (false);
-    }
-    f << srvidToString(getServerID());
-    f.close();
-    return (true);
-}
-
 string
 Dhcpv4Srv::srvidToString(const OptionPtr& srvid) {
     if (!srvid) {

src/bin/dhcp4/dhcp4_srv.h

@@ -175,7 +175,7 @@ protected:
     /// @param pkt packet to be checked
     /// @param serverid expectation regarding server-id option
     /// @throw RFCViolation if any issues are detected
-    void sanityCheck(const Pkt4Ptr& pkt, RequirementLevel serverid);
+    static void sanityCheck(const Pkt4Ptr& pkt, RequirementLevel serverid);
 
     /// @brief Processes incoming DISCOVER and returns response.
     ///
@@ -469,39 +469,6 @@ protected:
     /// @param [out] response response packet which addresses are to be adjusted.
     static void adjustRemoteAddr(const Pkt4Ptr& question, const Pkt4Ptr& response);
 
-    /// @brief Returns server-identifier option
-    ///
-    /// @return server-id option
-    OptionPtr getServerID() { return serverid_; }
-
-    /// @brief Sets server-identifier.
-    ///
-    /// This method attempts to set server-identifier DUID. It tries to
-    /// load previously stored IP from configuration. If there is no previously
-    /// stored server identifier, it will pick up one address from configured
-    /// and supported network interfaces.
-    ///
-    /// @throws isc::Unexpected Failed to obtain server identifier (i.e. no
-    //          previously stored configuration and no network interfaces available)
-    void generateServerID();
-
-    /// @brief attempts to load server-id from a file
-    ///
-    /// Tries to load duid from a text file. If the load is successful,
-    /// it creates server-id option and stores it in serverid_ (to be used
-    /// later by getServerID()).
-    ///
-    /// @param file_name name of the server-id file to load
-    /// @return true if load was successful, false otherwise
-    bool loadServerID(const std::string& file_name);
-
-    /// @brief attempts to write server-id to a file
-    /// Tries to write server-id content (stored in serverid_) to a text file.
-    ///
-    /// @param file_name name of the server-id file to write
-    /// @return true if write was successful, false otherwise
-    bool writeServerID(const std::string& file_name);
-
     /// @brief converts server-id to text
     /// Converts content of server-id option to a text representation, e.g.
     /// "192.0.2.1"
@@ -538,9 +505,6 @@ protected:
     /// @return selected subnet (or NULL if no suitable subnet was found)
     isc::dhcp::Subnet4Ptr selectSubnet(const Pkt4Ptr& question);
 
-    /// server DUID (to be sent in server-identifier option)
-    OptionPtr serverid_;
-
     /// indicates if shutdown is in progress. Setting it to true will
     /// initiate server shutdown procedure.
     volatile bool shutdown_;

src/bin/dhcp4/tests/dhcp4_srv_unittest.cc

@@ -44,7 +44,6 @@
 
 #include <boost/scoped_ptr.hpp>
 
-#include <fstream>
 #include <iostream>
 
 #include <arpa/inet.h>
@@ -59,9 +58,6 @@ using namespace isc::dhcp::test;
 
 namespace {
 
-/// dummy server-id file location
-const char* SRVID_FILE = "server-id-test.txt";
-
 // This test verifies that the destination address of the response
 // message is set to giaddr, when giaddr is set to non-zero address
 // in the received message.
@@ -371,12 +367,10 @@ TEST_F(Dhcpv4SrvTest, basic) {
     boost::scoped_ptr<NakedDhcpv4Srv> naked_srv;
     ASSERT_NO_THROW(
         naked_srv.reset(new NakedDhcpv4Srv(DHCP4_SERVER_PORT + 10000)));
-    EXPECT_TRUE(naked_srv->getServerID());
     // Close sockets again for the next test.
     IfaceMgr::instance().closeSockets();
 
     ASSERT_NO_THROW(naked_srv.reset(new NakedDhcpv4Srv(0)));
-    EXPECT_TRUE(naked_srv->getServerID());
 }
 
 // This test verifies that exception is not thrown when an error occurs during
@@ -962,24 +956,26 @@ TEST_F(Dhcpv4SrvTest, sanityCheck) {
     pkt->setHWAddr(generateHWAddr(6));
 
     // Server-id is optional for information-request, so
-    EXPECT_NO_THROW(srv->sanityCheck(pkt, Dhcpv4Srv::OPTIONAL));
+    EXPECT_NO_THROW(NakedDhcpv4Srv::sanityCheck(pkt, Dhcpv4Srv::OPTIONAL));
 
     // Empty packet, no server-id
-    EXPECT_THROW(srv->sanityCheck(pkt, Dhcpv4Srv::MANDATORY), RFCViolation);
+    EXPECT_THROW(NakedDhcpv4Srv::sanityCheck(pkt, Dhcpv4Srv::MANDATORY),
+                 RFCViolation);
 
     pkt->addOption(srv->getServerID());
 
     // Server-id is mandatory and present = no exception
-    EXPECT_NO_THROW(srv->sanityCheck(pkt, Dhcpv4Srv::MANDATORY));
+    EXPECT_NO_THROW(NakedDhcpv4Srv::sanityCheck(pkt, Dhcpv4Srv::MANDATORY));
 
     // Server-id is forbidden, but present => exception
-    EXPECT_THROW(srv->sanityCheck(pkt, Dhcpv4Srv::FORBIDDEN),
+    EXPECT_THROW(NakedDhcpv4Srv::sanityCheck(pkt, Dhcpv4Srv::FORBIDDEN),
                  RFCViolation);
 
     // There's no client-id and no HWADDR. Server needs something to
     // identify the client
     pkt->setHWAddr(generateHWAddr(0));
-    EXPECT_THROW(srv->sanityCheck(pkt, Dhcpv4Srv::MANDATORY), RFCViolation);
+    EXPECT_THROW(NakedDhcpv4Srv::sanityCheck(pkt, Dhcpv4Srv::MANDATORY),
+                 RFCViolation);
 }
 
 // This test verifies that incoming (positive) RELEASE can be handled properly.
@@ -1145,36 +1141,6 @@ TEST_F(Dhcpv4SrvFakeIfaceTest, ReleaseReject) {
     EXPECT_FALSE(l);
 }
 
-// This test verifies if the server-id disk operations (read, write) are
-// working properly.
-TEST_F(Dhcpv4SrvFakeIfaceTest, ServerID) {
-    NakedDhcpv4Srv srv(0);
-
-    string srvid_text = "192.0.2.100";
-    IOAddress srvid(srvid_text);
-
-    fstream file1(SRVID_FILE, ios::out | ios::trunc);
-    file1 << srvid_text;
-    file1.close();
-
-    // Test reading from a file
-    EXPECT_TRUE(srv.loadServerID(SRVID_FILE));
-    ASSERT_TRUE(srv.getServerID());
-    EXPECT_EQ(srvid_text, srv.srvidToString(srv.getServerID()));
-
-    // Now test writing to a file
-    EXPECT_EQ(0, unlink(SRVID_FILE));
-    EXPECT_NO_THROW(srv.writeServerID(SRVID_FILE));
-
-    fstream file2(SRVID_FILE, ios::in);
-    ASSERT_TRUE(file2.good());
-    string text;
-    file2 >> text;
-    file2.close();
-
-    EXPECT_EQ(srvid_text, text);
-}
-
 // Checks if received relay agent info option is echoed back to the client
 TEST_F(Dhcpv4SrvFakeIfaceTest, relayAgentInfoEcho) {
 

src/bin/dhcp4/tests/dhcp4_test_utils.h

@@ -21,6 +21,7 @@
 
 #include <gtest/gtest.h>
 #include <dhcp/iface_mgr.h>
+#include <dhcp/option4_addrlst.h>
 #include <dhcp/pkt4.h>
 #include <dhcp/pkt_filter.h>
 #include <dhcp/pkt_filter_inet.h>
@@ -359,6 +360,15 @@ public:
     /// that sockets should not be opened.
     NakedDhcpv4Srv(uint16_t port = 0)
         : Dhcpv4Srv(port, "type=memfile", false, false) {
+        // Create fixed server id.
+        server_id_.reset(new Option4AddrLst(DHO_DHCP_SERVER_IDENTIFIER,
+                                            asiolink::IOAddress("192.0.3.1")));
+    }
+
+    /// @brief Returns fixed server identifier assigned to the naked server
+    /// instance.
+    OptionPtr getServerID() const {
+        return (server_id_);
     }
 
     /// @brief fakes packet reception
@@ -401,6 +411,9 @@ public:
     virtual ~NakedDhcpv4Srv() {
     }
 
+    /// @brief Dummy server identifier option used by various tests.
+    OptionPtr server_id_;
+
     /// @brief packets we pretend to receive
     ///
     /// Instead of setting up sockets on interfaces that change between OSes, it
@@ -421,10 +434,6 @@ public:
     using Dhcpv4Srv::processClientName;
     using Dhcpv4Srv::computeDhcid;
     using Dhcpv4Srv::createNameChangeRequests;
-    using Dhcpv4Srv::getServerID;
-    using Dhcpv4Srv::loadServerID;
-    using Dhcpv4Srv::generateServerID;
-    using Dhcpv4Srv::writeServerID;
     using Dhcpv4Srv::sanityCheck;
     using Dhcpv4Srv::srvidToString;
     using Dhcpv4Srv::unpackOptions;