[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.

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_);

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.

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;

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.
 ///

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));