From: Thomas Markwalder Date: Thu, 15 May 2014 13:51:46 +0000 (-0400) Subject: [3432] Modify NameChangeTransaction to get its TSIGKey from current DdnsDomain X-Git-Tag: trac3434_base~20^2~6 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=ca968c5c71132c17de0682bcafddd260b1de5e7e;p=thirdparty%2Fkea.git [3432] Modify NameChangeTransaction to get its TSIGKey from current DdnsDomain NameChangeTransaction now sets its TSIGKeyPtr inside initServeSelection() from the domain pased into this method. This makes TSIG fully fucntionally end-to-end. --- diff --git a/src/bin/d2/nc_trans.cc b/src/bin/d2/nc_trans.cc index 2dfaa775c8..6200d1c603 100644 --- a/src/bin/d2/nc_trans.cc +++ b/src/bin/d2/nc_trans.cc @@ -429,6 +429,14 @@ NameChangeTransaction::initServerSelection(const DdnsDomainPtr& domain) { "initServerSelection called with an empty domain"); } + // Set the tsig_key to that of the DdnsDomain. + TSIGKeyInfoPtr tsig_key_info = domain->getTSIGKeyInfo(); + if (tsig_key_info) { + tsig_key_ = tsig_key_info->getTSIGKey(); + } else { + tsig_key_.reset(); + } + current_server_list_ = domain->getServers(); next_server_pos_ = 0; current_server_.reset(); @@ -455,11 +463,6 @@ NameChangeTransaction::selectNextServer() { return (false); } -void -NameChangeTransaction::setTSIGKey(const dns::TSIGKeyPtr& tsig_key) { - tsig_key_ = tsig_key; -} - const DNSClientPtr& NameChangeTransaction::getDNSClient() const { return (dns_client_); diff --git a/src/bin/d2/nc_trans.h b/src/bin/d2/nc_trans.h index 10d16a27bc..c093c00b8b 100644 --- a/src/bin/d2/nc_trans.h +++ b/src/bin/d2/nc_trans.h @@ -349,12 +349,6 @@ protected: /// servers from which to select. bool selectNextServer(); - /// @brief Sets the TSIG key to be used. - /// - /// @param tsig_key TSIG Key value to be used for signing and verifying - /// messages. If set to an emtpy pointer, TSIG will not be used. - void setTSIGKey(const dns::TSIGKeyPtr& tsig_key); - /// @brief Sets the update attempt count to the given value. /// /// @param value is the new value to assign. diff --git a/src/bin/d2/tests/nc_test_utils.cc b/src/bin/d2/tests/nc_test_utils.cc index f12ef9f1de..6eee0419ea 100644 --- a/src/bin/d2/tests/nc_test_utils.cc +++ b/src/bin/d2/tests/nc_test_utils.cc @@ -236,7 +236,8 @@ TransactionTest::~TransactionTest() { void TransactionTest::setupForIPv4Transaction(dhcp_ddns::NameChangeType chg_type, - int change_mask) { + int change_mask, + const std::string& key_name) { const char* msg_str = "{" " \"change_type\" : 0 , " @@ -262,7 +263,7 @@ TransactionTest::setupForIPv4Transaction(dhcp_ddns::NameChangeType chg_type, forward_domain_.reset(); } else { // Create the forward domain and then its servers. - forward_domain_ = makeDomain("example.com."); + forward_domain_ = makeDomain("example.com.", key_name); addDomainServer(forward_domain_, "forward.example.com", "127.0.0.1", 5301); addDomainServer(forward_domain_, "forward2.example.com", @@ -276,7 +277,7 @@ TransactionTest::setupForIPv4Transaction(dhcp_ddns::NameChangeType chg_type, reverse_domain_.reset(); } else { // Create the reverse domain and its server. - reverse_domain_ = makeDomain("2.168.192.in.addr.arpa."); + reverse_domain_ = makeDomain("2.168.192.in.addr.arpa.", key_name); addDomainServer(reverse_domain_, "reverse.example.com", "127.0.0.1", 5301); addDomainServer(reverse_domain_, "reverse2.example.com", @@ -286,7 +287,8 @@ TransactionTest::setupForIPv4Transaction(dhcp_ddns::NameChangeType chg_type, void TransactionTest::setupForIPv6Transaction(dhcp_ddns::NameChangeType chg_type, - int change_mask) { + int change_mask, + const std::string& key_name) { const char* msg_str = "{" " \"change_type\" : 0 , " @@ -312,7 +314,7 @@ TransactionTest::setupForIPv6Transaction(dhcp_ddns::NameChangeType chg_type, forward_domain_.reset(); } else { // Create the forward domain and then its servers. - forward_domain_ = makeDomain("example.com."); + forward_domain_ = makeDomain("example.com.", key_name); addDomainServer(forward_domain_, "fwd6-server.example.com", "::1", 5301); addDomainServer(forward_domain_, "fwd6-server2.example.com", @@ -326,7 +328,7 @@ TransactionTest::setupForIPv6Transaction(dhcp_ddns::NameChangeType chg_type, reverse_domain_.reset(); } else { // Create the reverse domain and its server. - reverse_domain_ = makeDomain("1.2001.ip6.arpa."); + reverse_domain_ = makeDomain("1.2001.ip6.arpa.", key_name); addDomainServer(reverse_domain_, "rev6-server.example.com", "::1", 5301); addDomainServer(reverse_domain_, "rev6-server2.example.com", @@ -418,11 +420,11 @@ DdnsDomainPtr makeDomain(const std::string& zone_name, const std::string& key_name) { DdnsDomainPtr domain; DnsServerInfoStoragePtr servers(new DnsServerInfoStorage()); - domain.reset(new DdnsDomain(zone_name, servers, makeTSIGKey(key_name))); + domain.reset(new DdnsDomain(zone_name, servers, makeTSIGKeyInfo(key_name))); return (domain); } -TSIGKeyInfoPtr makeTSIGKey(const std::string& key_name, +TSIGKeyInfoPtr makeTSIGKeyInfo(const std::string& key_name, const std::string& secret, const std::string& algorithm) { TSIGKeyInfoPtr key_info; diff --git a/src/bin/d2/tests/nc_test_utils.h b/src/bin/d2/tests/nc_test_utils.h index c32c518167..190b87e7dc 100644 --- a/src/bin/d2/tests/nc_test_utils.h +++ b/src/bin/d2/tests/nc_test_utils.h @@ -200,8 +200,11 @@ public: /// CHG_REMOVE. /// @param change_mask determines which change directions are requested /// FORWARD_CHG, REVERSE_CHG, or FWD_AND_REV_CHG. + /// @param key_name value to use to create TSIG key, if blank TSIG will not + /// be used. void setupForIPv4Transaction(dhcp_ddns::NameChangeType change_type, - int change_mask); + int change_mask, + const std::string& key_name = ""); /// @brief Creates a transaction which requests an IPv6 DNS update. /// @@ -214,8 +217,11 @@ public: /// CHG_REMOVE. /// @param change_mask determines which change directions are requested /// FORWARD_CHG, REVERSE_CHG, or FWD_AND_REV_CHG. + /// @param key_name value to use to create TSIG key, if blank TSIG will not + /// be used. void setupForIPv6Transaction(dhcp_ddns::NameChangeType change_type, - int change_mask); + int change_mask, + const std::string& key_name = ""); }; @@ -365,9 +371,10 @@ extern DdnsDomainPtr makeDomain(const std::string& zone_name, /// the pointer will be empty. /// @throw Underlying methods may throw. extern -TSIGKeyInfoPtr makeTSIGKey(const std::string& key_name, - const std::string& secret = "", - const std::string& algorithm = TSIGKeyInfo::MD5_STR); +TSIGKeyInfoPtr makeTSIGKeyInfo(const std::string& key_name, + const std::string& secret = "", + const std::string& algorithm + = TSIGKeyInfo::MD5_STR); /// @brief Creates a DnsServerInfo and adds it to the given DdnsDomain. /// diff --git a/src/bin/d2/tests/nc_trans_unittests.cc b/src/bin/d2/tests/nc_trans_unittests.cc index 6386dd567c..463c1c0ecd 100644 --- a/src/bin/d2/tests/nc_trans_unittests.cc +++ b/src/bin/d2/tests/nc_trans_unittests.cc @@ -264,7 +264,6 @@ public: using NameChangeTransaction::addPtrRdata; using NameChangeTransaction::responseString; using NameChangeTransaction::transactionOutcomeString; - using NameChangeTransaction::setTSIGKey; }; // Declare them so Gtest can see them. @@ -290,9 +289,11 @@ public: /// The transaction is constructed around a predefined (i.e "canned") /// NameChangeRequest. The request has both forward and reverse DNS /// changes requested, and both forward and reverse domains are populated. - NameChangeStubPtr makeCannedTransaction() { + /// @param key_name value to use to create TSIG key, if blank TSIG will not + /// be used. + NameChangeStubPtr makeCannedTransaction(const std::string& key_name = "") { // Creates IPv4 remove request, forward, and reverse domains. - setupForIPv4Transaction(dhcp_ddns::CHG_ADD, FWD_AND_REV_CHG); + setupForIPv4Transaction(dhcp_ddns::CHG_ADD, FWD_AND_REV_CHG, key_name); // Now create the test transaction as would occur in update manager. // Instantiate the transaction as would be done by update manager. @@ -1008,7 +1009,7 @@ TEST_F(NameChangeTransactionTest, sendUpdate) { /// @brief Tests that an unsigned response to a signed request is an error TEST_F(NameChangeTransactionTest, tsigUnsignedResponse) { NameChangeStubPtr name_change; - ASSERT_NO_THROW(name_change = makeCannedTransaction()); + ASSERT_NO_THROW(name_change = makeCannedTransaction("key_one")); ASSERT_NO_THROW(name_change->initDictionaries()); ASSERT_TRUE(name_change->selectFwdServer()); @@ -1016,14 +1017,7 @@ TEST_F(NameChangeTransactionTest, tsigUnsignedResponse) { FauxServer server(*io_service_, *(name_change->getCurrentServer())); server.receive (FauxServer::USE_RCODE, dns::Rcode::NOERROR()); - // Create a key and manually set it as the transaction's TSIG key - std::string secret ("key number one"); - dns::TSIGKeyPtr key_one; - ASSERT_NO_THROW(key_one.reset(new - dns::TSIGKey(dns::Name("one.com"), - dns::TSIGKey::HMACMD5_NAME(), - secret.c_str(), secret.size()))); - name_change->setTSIGKey(key_one); + // Do the udpate. ASSERT_NO_FATAL_FAILURE(doOneExchange(name_change)); // Verify that next event is IO_COMPLETED_EVT and DNS status is @@ -1045,7 +1039,7 @@ TEST_F(NameChangeTransactionTest, tsigUnsignedResponse) { /// @brief Tests that a response signed with the wrong key is an error TEST_F(NameChangeTransactionTest, tsigInvalidResponse) { NameChangeStubPtr name_change; - ASSERT_NO_THROW(name_change = makeCannedTransaction()); + ASSERT_NO_THROW(name_change = makeCannedTransaction("key_one")); ASSERT_NO_THROW(name_change->initDictionaries()); ASSERT_TRUE(name_change->selectFwdServer()); @@ -1054,14 +1048,7 @@ TEST_F(NameChangeTransactionTest, tsigInvalidResponse) { FauxServer server(*io_service_, *(name_change->getCurrentServer())); server.receive (FauxServer::INVALID_TSIG, dns::Rcode::NOERROR()); - // Create a key and manually set it as the transaction's TSIG key - std::string secret ("key number one"); - dns::TSIGKeyPtr key_one; - ASSERT_NO_THROW(key_one.reset(new - dns::TSIGKey(dns::Name("one.com"), - dns::TSIGKey::HMACMD5_NAME(), - secret.c_str(), secret.size()))); - name_change->setTSIGKey(key_one); + // Do the udpate. ASSERT_NO_FATAL_FAILURE(doOneExchange(name_change)); // Verify that next event is IO_COMPLETED_EVT and DNS status is @@ -1116,26 +1103,16 @@ TEST_F(NameChangeTransactionTest, tsigUnexpectedSignedResponse) { /// the right key. TEST_F(NameChangeTransactionTest, tsigValidExchange) { NameChangeStubPtr name_change; - ASSERT_NO_THROW(name_change = makeCannedTransaction()); + ASSERT_NO_THROW(name_change = makeCannedTransaction("key_one")); ASSERT_NO_THROW(name_change->initDictionaries()); ASSERT_TRUE(name_change->selectFwdServer()); - // Create a key - std::string secret ("key number one"); - dns::TSIGKeyPtr key_one; - ASSERT_NO_THROW(key_one.reset(new - dns::TSIGKey(dns::Name("one.com"), - dns::TSIGKey::HMACMD5_NAME(), - secret.c_str(), secret.size()))); - // Create a server, set its TSIG key, and then start it listening. FauxServer server(*io_service_, *(name_change->getCurrentServer())); - server.setTSIGKey(key_one); + TSIGKeyInfoPtr key_one = makeTSIGKeyInfo("key_one"); + server.setTSIGKey(key_one->getTSIGKey()); server.receive (FauxServer::USE_RCODE, dns::Rcode::NOERROR()); - // Manually set the transaction's key to the same key as the server. - name_change->setTSIGKey(key_one); - // Do the update. ASSERT_NO_FATAL_FAILURE(doOneExchange(name_change));