[3874] Reuse existing DUID if not explicitly specified.

src/lib/dhcp/duid_factory.cc

@@ -59,10 +59,32 @@ DUIDFactory::isPersisted() const {
 void
 DUIDFactory::createLLT(const uint16_t htype, const uint32_t time_in,
                        const std::vector<uint8_t>& ll_identifier) {
+    // We'll need DUID stored in the file to compare it against the
+    // new configuration. If the new configuration indicates that some
+    // bits of the DUID should be generated we'll first try to use the
+    // values stored in the file to prvent DUID from changing if possible.
+    readFromFile();
+
+    uint16_t htype_current = 0;
+    uint32_t time_current = 0;
+    std::vector<uint8_t> identifier_current;
+
+    // If DUID exists in the file, try to use it as much as possible.
+    if (duid_) {
+        std::vector<uint8_t> duid_vec = duid_->getDuid();
+        if ((duid_->getType() == DUID::DUID_LLT) && (duid_vec.size() > 8)) {
+            htype_current = readUint16(&duid_vec[2], duid_vec.size() - 2);
+            time_current = readUint32(&duid_vec[4], duid_vec.size() - 4);
+            identifier_current.assign(duid_vec.begin() + 8, duid_vec.end());
+        }
+    }
+
     uint32_t time_out = time_in;
-    // If time unspecified, use current time.
+    // If time is unspecified (ANY), then use the time from current DUID or
+    // set it to current time.
     if (time_out == 0) {
-        time_out = static_cast<uint32_t>(time(NULL) - DUID_TIME_EPOCH);
+        time_out = (time_current != 0 ? time_current :
+            static_cast<uint32_t>(time(NULL) - DUID_TIME_EPOCH));
     }
 
     std::vector<uint8_t> ll_identifier_out = ll_identifier;
@@ -72,7 +94,14 @@ DUIDFactory::createLLT(const uint16_t htype, const uint32_t time_in,
     // interfaces present in the system. Also, update the link
     // layer type accordingly.
     if (ll_identifier_out.empty()) {
-        createLinkLayerId(ll_identifier_out, htype_out);
+        // If DUID doesn't exist yet, generate a new identifier.
+        if (identifier_current.empty()) {
+            createLinkLayerId(ll_identifier_out, htype_out);
+        } else {
+            // Use current identifier and hardware type.
+            ll_identifier_out = identifier_current;
+            htype_out = htype_current;
+        }
 
     } else if (htype_out == 0) {
         // If link layer type unspecified and link layer adddress
@@ -97,27 +126,60 @@ DUIDFactory::createLLT(const uint16_t htype, const uint32_t time_in,
 void
 DUIDFactory::createEN(const uint32_t enterprise_id,
                       const std::vector<uint8_t>& identifier) {
-    // Enterprise id 0 means "unspecified". In this case use the ISC
-    // enterprise id.
-    uint32_t enterprise_id_out = enterprise_id != 0 ?
-        enterprise_id : ENTERPRISE_ID_ISC;
+    // We'll need DUID stored in the file to compare it against the
+    // new configuration. If the new configuration indicates that some
+    // bits of the DUID should be generated we'll first try to use the
+    // values stored in the file to prvent DUID from changing if possible.
+    readFromFile();
+
+    uint32_t enterprise_id_current = 0;
+    std::vector<uint8_t> identifier_current;
+
+    // If DUID exists in the file, try to use it as much as possible.
+    if (duid_) {
+        std::vector<uint8_t> duid_vec = duid_->getDuid();
+        if ((duid_->getType() == DUID::DUID_EN) && (duid_vec.size() > 6)) {
+            enterprise_id_current = readUint32(&duid_vec[2], duid_vec.size() - 2);
+            identifier_current.assign(duid_vec.begin() + 6, duid_vec.end());
+        }
+    }
+
+    // Enterprise id 0 means "unspecified". In this case, try to use existing
+    // DUID's enterprise id, or use ISC enterprise id.
+    uint32_t enterprise_id_out = enterprise_id;
+    if (enterprise_id_out == 0) {
+        if (enterprise_id_current != 0) {
+            enterprise_id_out = enterprise_id_current;
+        } else {
+            enterprise_id_out = ENTERPRISE_ID_ISC;
+        }
+    }
 
     // Render DUID.
     std::vector<uint8_t> duid_out(DUID_TYPE_LEN + ENTERPRISE_ID_LEN);
     writeUint16(DUID::DUID_EN, &duid_out[0], 2);
     writeUint32(enterprise_id_out, &duid_out[2], ENTERPRISE_ID_LEN);
 
+    // If no identifier specified, we'll have to use the one from the
+    // DUID file or generate new.
     if (identifier.empty()) {
-        // Identifier is empty, so we have to extend the DUID by 6 bytes
-        // to fit the random identifier.
-        duid_out.resize(DUID_TYPE_LEN + ENTERPRISE_ID_LEN +
-                        DUID_EN_IDENTIFIER_LEN);
-        // Variable length identifier consists of random numbers. The generated
-        // identifier is always 6 bytes long.
-        ::srandom(time(NULL));
-        fillRandom(&duid_out[DUID_TYPE_LEN + ENTERPRISE_ID_LEN],
-                   &duid_out[DUID_TYPE_LEN + ENTERPRISE_ID_LEN +
-                             DUID_EN_IDENTIFIER_LEN]);
+        // No DUID file, so generate new.
+        if (identifier_current.empty()) {
+            // Identifier is empty, so we have to extend the DUID by 6 bytes
+            // to fit the random identifier.
+            duid_out.resize(DUID_TYPE_LEN + ENTERPRISE_ID_LEN +
+                            DUID_EN_IDENTIFIER_LEN);
+            // Variable length identifier consists of random numbers. The generated
+            // identifier is always 6 bytes long.
+            ::srandom(time(NULL));
+            fillRandom(&duid_out[DUID_TYPE_LEN + ENTERPRISE_ID_LEN],
+                       &duid_out[DUID_TYPE_LEN + ENTERPRISE_ID_LEN +
+                                 DUID_EN_IDENTIFIER_LEN]);
+        } else {
+            // Append existing identifier.
+            duid_out.insert(duid_out.end(), identifier_current.begin(),
+                            identifier_current.end());
+        }
 
     } else {
         // Append the specified identifier to the end of DUID.
@@ -131,6 +193,24 @@ DUIDFactory::createEN(const uint32_t enterprise_id,
 void
 DUIDFactory::createLL(const uint16_t htype,
                       const std::vector<uint8_t>& ll_identifier) {
+    // We'll need DUID stored in the file to compare it against the
+    // new configuration. If the new configuration indicates that some
+    // bits of the DUID should be generated we'll first try to use the
+    // values stored in the file to prvent DUID from changing if possible.
+    readFromFile();
+
+    uint16_t htype_current = 0;
+    std::vector<uint8_t> identifier_current;
+
+    // If DUID exists in the file, try to use it as much as possible.
+    if (duid_) {
+        std::vector<uint8_t> duid_vec = duid_->getDuid();
+        if ((duid_->getType() == DUID::DUID_LL) && (duid_vec.size() > 4)) {
+            htype_current = readUint16(&duid_vec[2], duid_vec.size() - 2);
+            identifier_current.assign(duid_vec.begin() + 4, duid_vec.end());
+        }
+    }
+
     std::vector<uint8_t> ll_identifier_out = ll_identifier;
     uint16_t htype_out = htype;
 
@@ -138,7 +218,14 @@ DUIDFactory::createLL(const uint16_t htype,
     // interfaces present in the system. Also, update the link
     // layer type accordingly.
     if (ll_identifier_out.empty()) {
-        createLinkLayerId(ll_identifier_out, htype_out);
+        // If DUID doesn't exist yet, generate a new identifier.
+        if (identifier_current.empty()) {
+            createLinkLayerId(ll_identifier_out, htype_out);
+        } else {
+            // Use current identifier and hardware type.
+            ll_identifier_out = identifier_current;
+            htype_out = htype_current;
+        }
 
     } else if (htype_out == 0) {
         // If link layer type unspecified and link layer adddress
@@ -259,10 +346,39 @@ DUIDFactory::get() {
         return (duid_);
     }
 
-    // If DUID object hasn't been initialized then we need to retrieve a
-    // DUID from the file or create one.
+    // Try to read DUID from file, if it exists.
+    readFromFile();
+    if (duid_) {
+        return (duid_);
+    }
+
+    // DUID doesn't exist, so we need to create it.
+    try {
+        // There is no file with a DUID or the DUID stored in the file is
+        // invalid. We need to generate a new DUID.
+        createLLT(0, 0, std::vector<uint8_t>());
+
+    } catch (...) {
+        // It is possible that the creation of the DUID-LLT failed if there
+        // are no suitable interfaces present in the system.
+    }
+
+    if (!duid_) {
+        // Fall back to creation of DUID enterprise. If that fails we allow
+        // for propagating exception to indicate a fatal error. This may
+        // be the case if we failed to write it to a file.
+        createEN(0, std::vector<uint8_t>());
+    }
+
+    return (duid_);
+}
+
+void
+DUIDFactory::readFromFile() {
+    duid_.reset();
+
     std::ostringstream duid_str;
-    if (isPersisted()) {
+   if (isPersisted()) {
         std::ifstream ifs;
         ifs.open(storage_location_, std::ifstream::in);
         if (ifs.good()) {
@@ -279,35 +395,15 @@ DUIDFactory::get() {
         if (duid_str.tellp() != 0) {
             try {
                 duid_.reset(new DUID(DUID::fromText(duid_str.str())));
-                return (duid_);
 
             } catch (...) {
                 // The contents of this file don't represent a valid DUID.
                 // We'll need to generate it.
             }
         }
-
-    }
-
-    try {
-        // There is no file with a DUID or the DUID stored in the file is
-        // invalid. We need to generate a new DUID.
-        createLLT(0, 0, std::vector<uint8_t>());
-
-    } catch (...) {
-        // It is possible that the creation of the DUID-LLT failed if there
-        // are no suitable interfaces present in the system.
-    }
-
-    if (!duid_) {
-        // Fall back to creation of DUID enterprise. If that fails we allow
-        // for propagating exception to indicate a fatal error. This may
-        // be the case if we failed to write it to a file.
-        createEN(0, std::vector<uint8_t>());
-    }
-
-    return (duid_);
+   }
 }
 
+
 }; // end of isc::dhcp namespace
 }; // end of isc namespace

src/lib/dhcp/duid_factory.h

@@ -87,15 +87,18 @@ public:
     ///
     /// This method generates DUID-LLT.
     ///
-    /// @param htype Link layer type. If this is set to 0 and link layer
-    /// address is empty a default value of @c HTYPE_ETHER is used.
-    /// Otherwise a link layer type of selected interface is used.
+    /// @param htype Hardware type. If this is set to 0 and link layer
+    /// address is empty a value from existing DUID or a default value
+    /// of @c HTYPE_ETHER is used. Otherwise a link layer type of selected
+    /// interface is used.
     /// @param time_in Explicit value of time for the DUID. If this is
-    /// set to 0 a current time is used, otherwise a value specified is
-    /// used.
+    /// set to 0 a value from existing DUOD or current time is used,
+    /// otherwise a value specified is used.
     /// @param ll_identifier Data to be used as link layer address. If
-    /// this is an empty vector this method will iterate over all
-    /// active interfaces and will pick link layer address of one of them.
+    /// this is an empty vector this method will try to use link layer
+    /// address from existing DUID. If there is no DUID yet, it will
+    /// iterate over all active interfaces and will pick link layer
+    /// address of one of them.
     ///
     /// @throw isc::Unexpected if none of the interfaces includes has a
     /// suitable link layer address.
@@ -104,22 +107,27 @@ public:
 
     /// @brief Generates DUID-EN.
     ///
-    /// @param enterprise_id Enterprise id. If this value is 0, the ISC's
-    /// enterprise id is used.
+    /// @param enterprise_id Enterprise id. If this value is 0, a value
+    /// from existing DUID is used or ISC's enterprise id if there is
+    /// no DUID yet.
     /// @param identifier Data to be used as variable length identifier.
-    /// If this is an empty vector, the 6-bytes long vector with random
+    /// If this is an empty vector, an identifier from existing DUID is
+    /// used. If there is no DUID yet, the 6-bytes long vector with random
     /// values is generated.
     void createEN(const uint32_t enterprise_id,
                   const std::vector<uint8_t>& identifier);
 
     /// @brief Generates DUID-LL.
     ///
-    /// @param htype Link layer type. If this is set to 0 and link layer
-    /// address is empty a default value of @c HTYPE_ETHER is used.
-    /// Otherwise a link layer type of selected interface is used.
+    /// @param htype Hardware type. If this is set to 0 and link layer
+    /// address is empty a value from existing DUID or a default value
+    /// of @c HTYPE_ETHER is used. Otherwise a link layer type of selected
+    /// interface is used.
     /// @param ll_identifier Data to be used as link layer address. If
-    /// this is an empty vector this method will iterate over all
-    /// active interfaces and will pick link layer address of one of them.
+    /// this is an empty vector this method will try to use link layer
+    /// address from existing DUID. If there is no DUID yet, it will
+    /// iterate over all active interfaces and will pick link layer
+    /// address of one of them.
     ///
     /// @throw isc::Unexpected if none of the interfaces includes has a
     /// suitable link layer address.
@@ -164,6 +172,9 @@ private:
     /// @param duid_vector New DUID represented as vector of bytes.
     void set(const std::vector<uint8_t>& duid_vector);
 
+    /// @brief Reads DUID from file, if file exists.
+    void readFromFile();
+
     /// @brief Location of the file holding generated DUID (if specified).
     std::string storage_location_;
 

src/lib/dhcp/tests/duid_factory_unittest.cc

@@ -95,6 +95,22 @@ public:
                  const bool time_equal,
                  const std::string& expected_hwaddr);
 
+    /// @brief Tests creation of a DUID-LLT.
+    ///
+    /// @param expected_htype Expected link layer type as string.
+    /// @param expected_time Expected time as string.
+    /// @param time_equal Indicates if @c expected time should be
+    /// compared for equality with the time being part of a DUID
+    /// (if true), or the time being part of the DUID should be
+    /// less or equal current time (if false).
+    /// @param expected_hwaddr Expected link layer type as string.
+    /// @param factory_ref Reference to DUID factory.
+    void testLLT(const std::string& expected_htype,
+                 const std::string& expected_time,
+                 const bool time_equal,
+                 const std::string& expected_hwaddr,
+                 DUIDFactory& factory_ref);
+
     /// @brief Tests creation of a DUID-EN.
     ///
     /// @param expected_enterprise_id Expected enterprise id as string.
@@ -203,7 +219,17 @@ DUIDFactoryTest::testLLT(const std::string& expected_htype,
                          const std::string& expected_time,
                          const bool time_equal,
                          const std::string& expected_hwaddr) {
-    DuidPtr duid = factory().get();
+    testLLT(expected_htype, expected_time, time_equal, expected_hwaddr,
+            factory());
+}
+
+void
+DUIDFactoryTest::testLLT(const std::string& expected_htype,
+                         const std::string& expected_time,
+                         const bool time_equal,
+                         const std::string& expected_hwaddr,
+                         DUIDFactory& factory_ref) {
+    DuidPtr duid = factory_ref.get();
     ASSERT_TRUE(duid);
     ASSERT_GE(duid->getDuid().size(), 14);
     std::string duid_text = toString(duid->getDuid());
@@ -229,6 +255,8 @@ DUIDFactoryTest::testLLT(const std::string& expected_htype,
     EXPECT_EQ(duid->toText(), readDefaultFile());
 }
 
+
+
 void
 DUIDFactoryTest::testEN(const std::string& expected_enterprise_id,
                         const std::string& expected_identifier) {
@@ -321,6 +349,21 @@ TEST_F(DUIDFactoryTest, createLLTAllExplcitParameters) {
     testLLT("0008", "FAFAFAFA", true, "24242424242424242424");
 }
 
+// This test verifies that the createLLT function will try to reuse existing
+// DUID for the non-explicitly specified values.
+TEST_F(DUIDFactoryTest, createLLTReuse) {
+    // Create DUID-LLT and store it in a file.
+    ASSERT_NO_THROW(factory().createLLT(HTYPE_FDDI, 0xFAFAFAFA,
+                                        toVector("242424242424")));
+    // Create another factory class, which uses the same file.
+    DUIDFactory factory2(absolutePath(DEFAULT_DUID_FILE));
+    // Create DUID-LLT without specifying hardware type, time and
+    // link layer address. The factory function should use the
+    // values in the existing DUID.
+    ASSERT_NO_THROW(factory2.createLLT(0, 0, std::vector<uint8_t>()));
+    testLLT("0008", "FAFAFAFA", true, "242424242424");
+}
+
 // This test verifies that the DUID-EN can be generated entirely. Such
 // generated DUID contains ISC enterprise id and the random identifier.
 TEST_F(DUIDFactoryTest, createEN) {
@@ -348,6 +391,17 @@ TEST_F(DUIDFactoryTest, createENAllExplicitParameters) {
     testEN("01020304", "ABCD");
 }
 
+// This test verifies that the createEN function will try to reuse existing
+// DUID for the non-explicitly specified values.
+TEST_F(DUIDFactoryTest, createENReuse) {
+    // Create DUID-EN and store it in a file.
+    ASSERT_NO_THROW(factory().createEN(0xFAFAFAFA, toVector("242424242424")));
+    // Create another factory class, which uses the same file.
+    DUIDFactory factory2(absolutePath(DEFAULT_DUID_FILE));
+    ASSERT_NO_THROW(factory2.createEN(0, std::vector<uint8_t>()));
+    testEN("FAFAFAFA", "242424242424");
+}
+
 // This test verifies that the DUID-LL is generated when neither link layer
 // type nor address is specified.
 TEST_F(DUIDFactoryTest, createLL) {
@@ -393,6 +447,20 @@ TEST_F(DUIDFactoryTest, createENIfNotExists) {
     EXPECT_EQ(DUID::DUID_EN, duid->getType());
 }
 
+// This test verifies that the createLL function will try to reuse existing
+// DUID for the non-explicitly specified values.
+TEST_F(DUIDFactoryTest, createLLReuse) {
+    // Create DUID-EN and store it in a file.
+    ASSERT_NO_THROW(factory().createLL(HTYPE_FDDI, toVector("242424242424")));
+    // Create another factory class, which uses the same file.
+    DUIDFactory factory2(absolutePath(DEFAULT_DUID_FILE));
+    // Create DUID-LL without specifying hardware type, time and
+    // link layer address. The factory function should use the
+    // values in the existing DUID.
+    ASSERT_NO_THROW(factory2.createLL(0, std::vector<uint8_t>()));
+    testLL("0008", "242424242424");
+}
+
 // This test verifies that it is possible to override a DUID.
 TEST_F(DUIDFactoryTest, override) {
     // Create default DUID-LLT.