From: Marcin Siodelski Date: Tue, 17 Nov 2015 17:12:15 +0000 (+0100) Subject: [3874] Reuse existing DUID if not explicitly specified. X-Git-Tag: trac4231_base~39^2~5 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=93a91c8a9fd36a3fa2d45cb5031bab68c588abf7;p=thirdparty%2Fkea.git [3874] Reuse existing DUID if not explicitly specified. --- diff --git a/src/lib/dhcp/duid_factory.cc b/src/lib/dhcp/duid_factory.cc index e77e7b8c5e..43e2906ca9 100644 --- a/src/lib/dhcp/duid_factory.cc +++ b/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& 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 identifier_current; + + // If DUID exists in the file, try to use it as much as possible. + if (duid_) { + std::vector 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(time(NULL) - DUID_TIME_EPOCH); + time_out = (time_current != 0 ? time_current : + static_cast(time(NULL) - DUID_TIME_EPOCH)); } std::vector 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& 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 identifier_current; + + // If DUID exists in the file, try to use it as much as possible. + if (duid_) { + std::vector 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 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& 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 identifier_current; + + // If DUID exists in the file, try to use it as much as possible. + if (duid_) { + std::vector 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 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()); + + } 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()); + } + + 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()); - - } 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()); - } - - return (duid_); + } } + }; // end of isc::dhcp namespace }; // end of isc namespace diff --git a/src/lib/dhcp/duid_factory.h b/src/lib/dhcp/duid_factory.h index db9182b9a3..5718c30d0c 100644 --- a/src/lib/dhcp/duid_factory.h +++ b/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& 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& 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_; diff --git a/src/lib/dhcp/tests/duid_factory_unittest.cc b/src/lib/dhcp/tests/duid_factory_unittest.cc index cb377ab926..7328222202 100644 --- a/src/lib/dhcp/tests/duid_factory_unittest.cc +++ b/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())); + 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())); + 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())); + testLL("0008", "242424242424"); +} + // This test verifies that it is possible to override a DUID. TEST_F(DUIDFactoryTest, override) { // Create default DUID-LLT.