Browse Source

[3432] Changed algorithm strings to match dnsssec-keygen labels

Changed the string values used to configure a TSIG key's algorithm to
match those used by Bind9's dnssec-keygen.
Added tests for all supported algorithms.
Thomas Markwalder 11 years ago
parent
commit
64bf00162a

+ 14 - 13
src/bin/d2/d2_config.cc

@@ -28,13 +28,14 @@ namespace isc {
 namespace d2 {
 
 // *********************** TSIGKeyInfo  *************************
-
-const char* TSIGKeyInfo::MD5_STR = "MD5";
-const char* TSIGKeyInfo::SHA1_STR = "SHA1";
-const char* TSIGKeyInfo::SHA224_STR = "SHA224";
-const char* TSIGKeyInfo::SHA256_STR = "SHA256";
-const char* TSIGKeyInfo::SHA384_STR = "SHA384";
-const char* TSIGKeyInfo::SHA512_STR = "SHA512";
+// Note these values match correpsonding values for Bind9's
+// dnssec-keygen
+const char* TSIGKeyInfo::HMAC_MD5_STR = "HMAC-MD5";
+const char* TSIGKeyInfo::HMAC_SHA1_STR = "HMAC-SHA1";
+const char* TSIGKeyInfo::HMAC_SHA224_STR = "HMAC-SHA224";
+const char* TSIGKeyInfo::HMAC_SHA256_STR = "HMAC-SHA256";
+const char* TSIGKeyInfo::HMAC_SHA384_STR = "HMAC-SHA384";
+const char* TSIGKeyInfo::HMAC_SHA512_STR = "HMAC-SHA512";
 
 TSIGKeyInfo::TSIGKeyInfo(const std::string& name, const std::string& algorithm,
                          const std::string& secret)
@@ -47,22 +48,22 @@ TSIGKeyInfo::~TSIGKeyInfo() {
 
 const dns::Name&
 TSIGKeyInfo::stringToAlgorithmName(const std::string& algorithm_id) {
-    if (boost::iequals(algorithm_id, MD5_STR)) {
+    if (boost::iequals(algorithm_id, HMAC_MD5_STR)) {
         return (dns::TSIGKey::HMACMD5_NAME());
     }
-    if (boost::iequals(algorithm_id, SHA1_STR)) {
+    if (boost::iequals(algorithm_id, HMAC_SHA1_STR)) {
         return (dns::TSIGKey::HMACSHA1_NAME());
     }
-    if (boost::iequals(algorithm_id, SHA224_STR)) {
+    if (boost::iequals(algorithm_id, HMAC_SHA224_STR)) {
         return (dns::TSIGKey::HMACSHA224_NAME());
     }
-    if (boost::iequals(algorithm_id, SHA256_STR)) {
+    if (boost::iequals(algorithm_id, HMAC_SHA256_STR)) {
         return (dns::TSIGKey::HMACSHA256_NAME());
     }
-    if (boost::iequals(algorithm_id, SHA384_STR)) {
+    if (boost::iequals(algorithm_id, HMAC_SHA384_STR)) {
         return (dns::TSIGKey::HMACSHA384_NAME());
     }
-    if (boost::iequals(algorithm_id, SHA512_STR)) {
+    if (boost::iequals(algorithm_id, HMAC_SHA512_STR)) {
         return (dns::TSIGKey::HMACSHA512_NAME());
     }
 

+ 18 - 18
src/bin/d2/d2_config.h

@@ -155,12 +155,12 @@ class TSIGKeyInfo {
 public:
     /// @brief Defines string values for the supported TSIG algorithms
     //@{
-    static const char* MD5_STR;
-    static const char* SHA1_STR;
-    static const char* SHA256_STR;
-    static const char* SHA224_STR;
-    static const char* SHA384_STR;
-    static const char* SHA512_STR;
+    static const char* HMAC_MD5_STR;
+    static const char* HMAC_SHA1_STR;
+    static const char* HMAC_SHA256_STR;
+    static const char* HMAC_SHA224_STR;
+    static const char* HMAC_SHA384_STR;
+    static const char* HMAC_SHA512_STR;
     //}@
 
     /// @brief Constructor
@@ -168,12 +168,12 @@ public:
     /// @param name the unique label used to identify this key
     /// @param algorithm the id of the encryption alogirthm this key uses.
     /// Currently supported values are (case insensitive):
-    /// -# "MD5"
-    /// -# "SHA1"
-    /// -# "SHA224"
-    /// -# "SHA256"
-    /// -# "SHA384"
-    /// -# "SHA512"
+    /// -# "HMAC-MD5"
+    /// -# "HMAC-SHA1"
+    /// -# "HMAC-SHA224"
+    /// -# "HMAC-SHA256"
+    /// -# "HMAC-SHA384"
+    /// -# "HMAC-SHA512"
     ///
     /// @param secret the base-64 encoded secret component for this key
     /// such as one would typically see in a key file for the entry "Key"
@@ -230,12 +230,12 @@ public:
     ///
     /// @param algorithm_id string value to translate into an algorithm name.
     /// Currently supported values are (case insensitive):
-    /// -# "MD5"
-    /// -# "SHA1"
-    /// -# "SHA224"
-    /// -# "SHA256"
-    /// -# "SHA384"
-    /// -# "SHA512"
+    /// -# "HMAC-MD5"
+    /// -# "HMAC-SHA1"
+    /// -# "HMAC-SHA224"
+    /// -# "HMAC-SHA256"
+    /// -# "HMAC-SHA384"
+    /// -# "HMAC-SHA512"
     ///
     /// @return const reference to a dns::Name containing the alogorithm name
     /// @throw BadValue if ID isn't recognized.

+ 31 - 27
src/bin/d2/tests/d2_cfg_mgr_unittests.cc

@@ -260,7 +260,7 @@ TEST_F(TSIGKeyInfoTest, invalidEntry) {
     // Config with a blank name entry.
     std::string config = "{"
                          " \"name\": \"\" , "
-                         " \"algorithm\": \"MD5\" , "
+                         " \"algorithm\": \"HMAC-MD5\" , "
                          "   \"secret\": \"LSWXnfkKZjdPJI5QxlpnfQ==\" "
                          "}";
     ASSERT_TRUE(fromJSON(config));
@@ -295,7 +295,7 @@ TEST_F(TSIGKeyInfoTest, invalidEntry) {
     // Config with a blank secret entry.
     config = "{"
                          " \"name\": \"d2_key_one\" , "
-                         " \"algorithm\": \"MD5\" , "
+                         " \"algorithm\": \"HMAC-MD5\" , "
                          " \"secret\": \"\" "
                          "}";
 
@@ -307,7 +307,7 @@ TEST_F(TSIGKeyInfoTest, invalidEntry) {
     // Config with an invalid secret entry.
     config = "{"
                          " \"name\": \"d2_key_one\" , "
-                         " \"algorithm\": \"MD5\" , "
+                         " \"algorithm\": \"HMAC-MD5\" , "
                          " \"secret\": \"bogus\" "
                          "}";
 
@@ -323,7 +323,7 @@ TEST_F(TSIGKeyInfoTest, validEntry) {
     // Valid entries for TSIG key, all items are required.
     std::string config = "{"
                          " \"name\": \"d2_key_one\" , "
-                         " \"algorithm\": \"MD5\" , "
+                         " \"algorithm\": \"HMAC-MD5\" , "
                          " \"secret\": \"dGhpcyBrZXkgd2lsbCBtYXRjaA==\" "
                          "}";
     ASSERT_TRUE(fromJSON(config));
@@ -343,7 +343,7 @@ TEST_F(TSIGKeyInfoTest, validEntry) {
     TSIGKeyInfoPtr& key = gotit->second;
 
     // Verify the key contents.
-    EXPECT_TRUE(checkKey(key, "d2_key_one", "MD5",
+    EXPECT_TRUE(checkKey(key, "d2_key_one", "HMAC-MD5",
                          "dGhpcyBrZXkgd2lsbCBtYXRjaA=="));
 }
 
@@ -354,7 +354,7 @@ TEST_F(TSIGKeyInfoTest, invalidTSIGKeyList) {
     std::string config = "["
 
                          " { \"name\": \"key1\" , "
-                         "   \"algorithm\": \"MD5\" ,"
+                         "   \"algorithm\": \"HMAC-MD5\" ,"
                          "   \"secret\": \"GWG/Xfbju4O2iXGqkSu4PQ==\" "
                          " },"
                          // this entry has an invalid algorithm
@@ -363,7 +363,7 @@ TEST_F(TSIGKeyInfoTest, invalidTSIGKeyList) {
                          "   \"secret\": \"GWG/Xfbju4O2iXGqkSu4PQ==\" "
                          " },"
                          " { \"name\": \"key3\" , "
-                         "   \"algorithm\": \"MD5\" ,"
+                         "   \"algorithm\": \"HMAC-MD5\" ,"
                          "   \"secret\": \"GWG/Xfbju4O2iXGqkSu4PQ==\" "
                          " }"
                          " ]";
@@ -385,15 +385,15 @@ TEST_F(TSIGKeyInfoTest, duplicateTSIGKey) {
     std::string config = "["
 
                          " { \"name\": \"key1\" , "
-                         "   \"algorithm\": \"MD5\" ,"
+                         "   \"algorithm\": \"HMAC-MD5\" ,"
                          "   \"secret\": \"GWG/Xfbju4O2iXGqkSu4PQ==\" "
                          " },"
                          " { \"name\": \"key2\" , "
-                         "   \"algorithm\": \"MD5\" ,"
+                         "   \"algorithm\": \"HMAC-MD5\" ,"
                          "   \"secret\": \"GWG/Xfbju4O2iXGqkSu4PQ==\" "
                          " },"
                          " { \"name\": \"key1\" , "
-                         "   \"algorithm\": \"MD5\" ,"
+                         "   \"algorithm\": \"HMAC-MD5\" ,"
                          "   \"secret\": \"GWG/Xfbju4O2iXGqkSu4PQ==\" "
                          " }"
                          " ]";
@@ -415,27 +415,27 @@ TEST_F(TSIGKeyInfoTest, validTSIGKeyList) {
     std::string config = "["
 
                          " { \"name\": \"key1\" , "
-                         "   \"algorithm\": \"MD5\" ,"
+                         "   \"algorithm\": \"HMAC-MD5\" ,"
                          "  \"secret\": \"dGhpcyBrZXkgd2lsbCBtYXRjaA==\" "
                          " },"
                          " { \"name\": \"key2\" , "
-                         "   \"algorithm\": \"SHA1\" ,"
+                         "   \"algorithm\": \"HMAC-SHA1\" ,"
                          "  \"secret\": \"dGhpcyBrZXkgd2lsbCBtYXRjaA==\" "
                          " },"
                          " { \"name\": \"key3\" , "
-                         "   \"algorithm\": \"SHA256\" ,"
+                         "   \"algorithm\": \"HMAC-SHA256\" ,"
                          "  \"secret\": \"dGhpcyBrZXkgd2lsbCBtYXRjaA==\" "
                          " },"
                          " { \"name\": \"key4\" , "
-                         "   \"algorithm\": \"SHA224\" ,"
+                         "   \"algorithm\": \"HMAC-SHA224\" ,"
                          "  \"secret\": \"dGhpcyBrZXkgd2lsbCBtYXRjaA==\" "
                          " },"
                          " { \"name\": \"key5\" , "
-                         "   \"algorithm\": \"SHA384\" ,"
+                         "   \"algorithm\": \"HMAC-SHA384\" ,"
                          "  \"secret\": \"dGhpcyBrZXkgd2lsbCBtYXRjaA==\" "
                          " },"
                          " { \"name\": \"key6\" , "
-                         "   \"algorithm\": \"SHA512\" ,"
+                         "   \"algorithm\": \"HMAC-SHA512\" ,"
                          "   \"secret\": \"dGhpcyBrZXkgd2lsbCBtYXRjaA==\" "
                          " }"
                          " ]";
@@ -460,7 +460,7 @@ TEST_F(TSIGKeyInfoTest, validTSIGKeyList) {
     TSIGKeyInfoPtr& key = gotit->second;
 
     // Verify the key contents.
-    EXPECT_TRUE(checkKey(key, "key1", TSIGKeyInfo::MD5_STR, ref_secret));
+    EXPECT_TRUE(checkKey(key, "key1", TSIGKeyInfo::HMAC_MD5_STR, ref_secret));
 
     // Find the 2nd key and retrieve it.
     gotit = keys_->find("key2");
@@ -468,7 +468,7 @@ TEST_F(TSIGKeyInfoTest, validTSIGKeyList) {
     key = gotit->second;
 
     // Verify the key contents.
-    EXPECT_TRUE(checkKey(key, "key2", TSIGKeyInfo::SHA1_STR, ref_secret));
+    EXPECT_TRUE(checkKey(key, "key2", TSIGKeyInfo::HMAC_SHA1_STR, ref_secret));
 
     // Find the 3rd key and retrieve it.
     gotit = keys_->find("key3");
@@ -476,7 +476,8 @@ TEST_F(TSIGKeyInfoTest, validTSIGKeyList) {
     key = gotit->second;
 
     // Verify the key contents.
-    EXPECT_TRUE(checkKey(key, "key3", TSIGKeyInfo::SHA256_STR, ref_secret));
+    EXPECT_TRUE(checkKey(key, "key3", TSIGKeyInfo::HMAC_SHA256_STR,
+                         ref_secret));
 
     // Find the 4th key and retrieve it.
     gotit = keys_->find("key4");
@@ -484,7 +485,8 @@ TEST_F(TSIGKeyInfoTest, validTSIGKeyList) {
     key = gotit->second;
 
     // Verify the key contents.
-    EXPECT_TRUE(checkKey(key, "key4", TSIGKeyInfo::SHA224_STR, ref_secret));
+    EXPECT_TRUE(checkKey(key, "key4", TSIGKeyInfo::HMAC_SHA224_STR,
+                         ref_secret));
 
     // Find the 5th key and retrieve it.
     gotit = keys_->find("key5");
@@ -492,7 +494,8 @@ TEST_F(TSIGKeyInfoTest, validTSIGKeyList) {
     key = gotit->second;
 
     // Verify the key contents.
-    EXPECT_TRUE(checkKey(key, "key5", TSIGKeyInfo::SHA384_STR, ref_secret));
+    EXPECT_TRUE(checkKey(key, "key5", TSIGKeyInfo::HMAC_SHA384_STR,
+                         ref_secret));
 
     // Find the 6th key and retrieve it.
     gotit = keys_->find("key6");
@@ -500,7 +503,8 @@ TEST_F(TSIGKeyInfoTest, validTSIGKeyList) {
     key = gotit->second;
 
     // Verify the key contents.
-    EXPECT_TRUE(checkKey(key, "key6", TSIGKeyInfo::SHA512_STR, ref_secret));
+    EXPECT_TRUE(checkKey(key, "key6", TSIGKeyInfo::HMAC_SHA512_STR,
+                         ref_secret));
 }
 
 /// @brief Tests the enforcement of data validation when parsing DnsServerInfos.
@@ -736,7 +740,7 @@ TEST_F(DdnsDomainTest, ddnsDomainParsing) {
     ASSERT_TRUE(fromJSON(config));
 
     // Add a TSIG key to the test key map, so key validation will pass.
-    addKey("d2_key.tmark.org", "md5", "GWG/Xfbju4O2iXGqkSu4PQ==");
+    addKey("d2_key.tmark.org", "HMAC-MD5", "GWG/Xfbju4O2iXGqkSu4PQ==");
 
     // Verify that the domain configuration builds and commits without error.
     ASSERT_NO_THROW(parser_->build(config_set_));
@@ -814,8 +818,8 @@ TEST_F(DdnsDomainTest, DdnsDomainListParsing) {
     ASSERT_TRUE(fromJSON(config));
 
     // Add keys to key map so key validation passes.
-    addKey("d2_key.tmark.org", "MD5", "GWG/Xfbju4O2iXGqkSu4PQ==");
-    addKey("d2_key.billcat.net", "MD5", "GWG/Xfbju4O2iXGqkSu4PQ==");
+    addKey("d2_key.tmark.org", "HMAC-MD5", "GWG/Xfbju4O2iXGqkSu4PQ==");
+    addKey("d2_key.billcat.net", "HMAC-MD5", "GWG/Xfbju4O2iXGqkSu4PQ==");
 
     // Create the list parser
     isc::dhcp::ParserPtr list_parser;
@@ -954,12 +958,12 @@ TEST_F(D2CfgMgrTest, fullConfig) {
                         "\"tsig_keys\": ["
                         "{"
                         "  \"name\": \"d2_key.tmark.org\" , "
-                        "  \"algorithm\": \"md5\" , "
+                        "  \"algorithm\": \"hmac-md5\" , "
                         "   \"secret\": \"LSWXnfkKZjdPJI5QxlpnfQ==\" "
                         "},"
                         "{"
                         "  \"name\": \"d2_key.billcat.net\" , "
-                        "  \"algorithm\": \"md5\" , "
+                        "  \"algorithm\": \"hmac-md5\" , "
                         "   \"secret\": \"LSWXnfkKZjdPJI5QxlpnfQ==\" "
                         "}"
                         "],"

+ 1 - 1
src/bin/d2/tests/d2_process_unittests.cc

@@ -40,7 +40,7 @@ const char* bad_ip_d2_config = "{ "
                         "\"port\" : 5031, "
                         "\"tsig_keys\": ["
                         "{ \"name\": \"d2_key.tmark.org\" , "
-                        "   \"algorithm\": \"md5\" ,"
+                        "   \"algorithm\": \"HMAC-MD5\" ,"
                         "   \"secret\": \"LSWXnfkKZjdPJI5QxlpnfQ==\" "
                         "} ],"
                         "\"forward_ddns\" : {"

+ 49 - 0
src/bin/d2/tests/d2_update_message_unittests.cc

@@ -14,6 +14,7 @@
 
 #include <config.h>
 
+#include <d2/d2_config.h>
 #include <d2/d2_update_message.h>
 #include <d2/d2_zone.h>
 #include <dns/messagerenderer.h>
@@ -653,4 +654,52 @@ TEST_F(D2UpdateMessageTest, validTSIG) {
                  InvalidQRFlag);
 }
 
+// Tests message signing and verification for all supported algorithms.
+TEST_F(D2UpdateMessageTest, allValidTSIG) {
+    std::vector<std::string>algorithms;
+    algorithms.push_back(TSIGKeyInfo::HMAC_MD5_STR);
+    algorithms.push_back(TSIGKeyInfo::HMAC_SHA1_STR);
+    algorithms.push_back(TSIGKeyInfo::HMAC_SHA224_STR);
+    algorithms.push_back(TSIGKeyInfo::HMAC_SHA256_STR);
+    algorithms.push_back(TSIGKeyInfo::HMAC_SHA384_STR);
+    algorithms.push_back(TSIGKeyInfo::HMAC_SHA512_STR);
+
+    dns::Name key_name("test_key");
+    std::string secret("random text for secret");
+    for (int i = 0; i < algorithms.size(); ++i) {
+        dns::TSIGKey key(key_name,
+                         TSIGKeyInfo::stringToAlgorithmName(algorithms[i]),
+                         secret.c_str(), secret.size());
+
+        // Build a request message
+        D2UpdateMessage msg;
+        msg.setId(0x1234);
+        msg.setRcode(Rcode(Rcode::NOERROR_CODE));
+        msg.setZone(Name("example.com"), RRClass::IN());
+
+        // Make a context to send the message with and use it to render
+        // the message into the wire format.
+        TSIGContextPtr context;
+        ASSERT_NO_THROW(context.reset(new TSIGContext(key)));
+        MessageRenderer renderer;
+        ASSERT_NO_THROW(msg.toWire(renderer, context.get()));
+
+        // Grab the wire data from the signed message.
+        const void* wire_data = renderer.getData();
+        const size_t wire_size = renderer.getLength();
+
+        // Create a fresh context to "receive" the message. (We can't use the
+        // one we signed it with, as its expecting a signed response to its
+        // request. Here we are acting like the server).
+        // If the message passes TSIG verification, then the QR Flag test in
+        // the subsequent call to D2UpdateMessage::validateResponse should
+        // fail because this isn't really received message.
+        ASSERT_NO_THROW(context.reset(new TSIGContext(key)));
+        D2UpdateMessage msg2(D2UpdateMessage::INBOUND);
+        ASSERT_THROW(msg2.fromWire(wire_data, wire_size, context.get()),
+                                   InvalidQRFlag);
+    }
+}
+
+
 } // End of anonymous namespace

+ 1 - 1
src/bin/d2/tests/d_test_stubs.cc

@@ -27,7 +27,7 @@ const char* valid_d2_config = "{ "
                         "\"port\" : 5031, "
                         "\"tsig_keys\": ["
                         "{ \"name\": \"d2_key.tmark.org\" , "
-                        "   \"algorithm\": \"md5\" ,"
+                        "   \"algorithm\": \"HMAC-MD5\" ,"
                         "   \"secret\": \"LSWXnfkKZjdPJI5QxlpnfQ==\" "
                         "} ],"
                         "\"forward_ddns\" : {"

+ 29 - 7
src/bin/d2/tests/nc_test_utils.cc

@@ -91,7 +91,7 @@ FauxServer::requestHandler(const asio::error_code& error,
     // If we encountered an error or received no data then fail.
     // We expect the client to send good requests.
     if (error.value() != 0 || bytes_recvd < 1) {
-        ADD_FAILURE() << "FauxServer receive failed" << error.message();
+        ADD_FAILURE() << "FauxServer receive failed: " << error.message();
         receive_pending_ = false;
         return;
     }
@@ -238,7 +238,7 @@ TransactionTest::~TransactionTest() {
 void
 TransactionTest::setupForIPv4Transaction(dhcp_ddns::NameChangeType chg_type,
                                          int change_mask,
-                                         const std::string& key_name) {
+                                         const TSIGKeyInfoPtr& tsig_key_info) {
     const char* msg_str =
         "{"
         " \"change_type\" : 0 , "
@@ -264,7 +264,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.", key_name);
+        forward_domain_ = makeDomain("example.com.", tsig_key_info);
         addDomainServer(forward_domain_, "forward.example.com",
                         "127.0.0.1", 5301);
         addDomainServer(forward_domain_, "forward2.example.com",
@@ -278,7 +278,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.", key_name);
+        reverse_domain_ = makeDomain("2.168.192.in.addr.arpa.", tsig_key_info);
         addDomainServer(reverse_domain_, "reverse.example.com",
                         "127.0.0.1", 5301);
         addDomainServer(reverse_domain_, "reverse2.example.com",
@@ -287,9 +287,16 @@ TransactionTest::setupForIPv4Transaction(dhcp_ddns::NameChangeType chg_type,
 }
 
 void
-TransactionTest::setupForIPv6Transaction(dhcp_ddns::NameChangeType chg_type,
+TransactionTest::setupForIPv4Transaction(dhcp_ddns::NameChangeType chg_type,
                                          int change_mask,
                                          const std::string& key_name) {
+    setupForIPv4Transaction(chg_type, change_mask, makeTSIGKeyInfo(key_name));
+}
+
+void
+TransactionTest::setupForIPv6Transaction(dhcp_ddns::NameChangeType chg_type,
+                                         int change_mask,
+                                         const TSIGKeyInfoPtr& tsig_key_info) {
     const char* msg_str =
         "{"
         " \"change_type\" : 0 , "
@@ -315,7 +322,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.", key_name);
+        forward_domain_ = makeDomain("example.com.", tsig_key_info);
         addDomainServer(forward_domain_, "fwd6-server.example.com",
                         "::1", 5301);
         addDomainServer(forward_domain_, "fwd6-server2.example.com",
@@ -329,7 +336,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.", key_name);
+        reverse_domain_ = makeDomain("1.2001.ip6.arpa.", tsig_key_info);
         addDomainServer(reverse_domain_, "rev6-server.example.com",
                         "::1", 5301);
         addDomainServer(reverse_domain_, "rev6-server2.example.com",
@@ -337,6 +344,13 @@ TransactionTest::setupForIPv6Transaction(dhcp_ddns::NameChangeType chg_type,
     }
 }
 
+void
+TransactionTest::setupForIPv6Transaction(dhcp_ddns::NameChangeType chg_type,
+                                         int change_mask,
+                                         const std::string& key_name) {
+    setupForIPv6Transaction(chg_type, change_mask, makeTSIGKeyInfo(key_name));
+}
+
 
 //********************** Functions ****************************
 
@@ -425,6 +439,14 @@ DdnsDomainPtr makeDomain(const std::string& zone_name,
     return (domain);
 }
 
+DdnsDomainPtr makeDomain(const std::string& zone_name,
+                         const TSIGKeyInfoPtr &tsig_key_info) {
+    DdnsDomainPtr domain;
+    DnsServerInfoStoragePtr servers(new DnsServerInfoStorage());
+    domain.reset(new DdnsDomain(zone_name, servers, tsig_key_info));
+    return (domain);
+}
+
 TSIGKeyInfoPtr makeTSIGKeyInfo(const std::string& key_name,
                            const std::string& secret,
                            const std::string& algorithm) {

+ 55 - 9
src/bin/d2/tests/nc_test_utils.h

@@ -200,11 +200,28 @@ 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.
+    /// @param tsig_key_info pointer to the TSIGKeyInfo to assign to both
+    /// domains in the transaction.  This will cause the transaction to
+    /// use TSIG.  If the pointer is empty, TSIG will not be used.
     void setupForIPv4Transaction(dhcp_ddns::NameChangeType change_type,
                                  int change_mask,
-                                 const std::string& key_name = "");
+                                 const TSIGKeyInfoPtr& tsig_key_info =
+                                 TSIGKeyInfoPtr());
+
+    /// @brief Creates a transaction which requests an IPv4 DNS update.
+    ///
+    /// Convenience wrapper around the above method which accepts a string
+    /// key_name from which the TSIGKeyInfo is constructed.  Note the string
+    /// may not be blank.
+    ///
+    /// @param change_type selects the type of change requested, CHG_ADD or
+    /// 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. The value may not
+    /// be blank.
+    void setupForIPv4Transaction(dhcp_ddns::NameChangeType change_type,
+                                 int change_mask, const std::string& key_name);
 
     /// @brief Creates a transaction which requests an IPv6 DNS update.
     ///
@@ -217,11 +234,29 @@ public:
     /// CHG_REMOVE.
     /// @param change_mask determines which change directions are requested
     /// FORWARD_CHG, REVERSE_CHG, or FWD_AND_REV_CHG.
+    /// @param tsig_key_info pointer to the TSIGKeyInfo to assign to both
+    /// domains in the transaction.  This will cause the transaction to
+    /// use TSIG.  If the pointer is empty, TSIG will not be used.
+    void setupForIPv6Transaction(dhcp_ddns::NameChangeType change_type,
+                                 int change_mask,
+                                 const TSIGKeyInfoPtr& tsig_key_info =
+                                 TSIGKeyInfoPtr());
+
+    /// @brief Creates a transaction which requests an IPv6 DNS update.
+    ///
+    /// Convenience wrapper around the above method which accepts a string
+    /// key_name from which the TSIGKeyInfo is constructed.  Note the string
+    /// may not be blank.
+    ///
+    /// @param change_type selects the type of change requested, CHG_ADD or
+    /// 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,
-                                 const std::string& key_name = "");
+                                 int change_mask, const std::string& key_name);
+
 };
 
 
@@ -355,12 +390,23 @@ dhcp_ddns::NameChangeRequestPtr makeNcrFromString(const std::string& ncr_str);
 /// @brief Creates a DdnsDomain with the one server.
 ///
 /// @param zone_name zone name of the domain
-/// @param key_name TSIG key name of the TSIG key for this domain. Defaults to
-/// blank which means the DdnsDomain does not have a key.
+/// @param key_name TSIG key name of the TSIG key for this domain. It will
+/// create a TSIGKeyInfo based on the key_name and assign it to the domain.
+///
+/// @throw Underlying methods may throw.
+extern DdnsDomainPtr makeDomain(const std::string& zone_name,
+                                const std::string& key_name);
+
+/// @brief Creates a DdnsDomain with the one server.
+///
+/// @param zone_name zone name of the domain
+/// @param tsig_key_info pointer to the TSIGInfog key for this domain.
+/// Defaults to an empty pointer, meaning this domain has no key.
 ///
 /// @throw Underlying methods may throw.
 extern DdnsDomainPtr makeDomain(const std::string& zone_name,
-                                const std::string& key_name = "");
+                                const TSIGKeyInfoPtr&
+                                tsig_key_info = TSIGKeyInfoPtr());
 
 /// @brief Creates a TSIGKeyInfo
 ///
@@ -375,7 +421,7 @@ extern
 TSIGKeyInfoPtr makeTSIGKeyInfo(const std::string& key_name,
                                const std::string& secret = "",
                                const std::string& algorithm
-                               = TSIGKeyInfo::MD5_STR);
+                               = TSIGKeyInfo::HMAC_MD5_STR);
 
 /// @brief Creates a DnsServerInfo and adds it to the given DdnsDomain.
 ///

+ 61 - 34
src/bin/d2/tests/nc_trans_unittests.cc

@@ -75,15 +75,11 @@ public:
     /// sendUpdate without incorporating exectution of the state model
     /// into the test.
     /// It sets the DNS status update and posts IO_COMPLETED_EVT as does
-    /// the normal callback, but rather than invoking runModel it stops
-    /// the IO service.  This allows tests to be constructed that consisted
-    /// of generating a DNS request and invoking sendUpdate to post it and
-    /// wait for response.
+    /// the normal callback.
     virtual void operator()(DNSClient::Status status) {
         if (use_stub_callback_) {
             setDnsUpdateStatus(status);
             postNextEvent(IO_COMPLETED_EVT);
-            getIOService()->stop();
         } else {
             // For tests which need to use the real callback.
             NameChangeTransaction::operator()(status);
@@ -285,13 +281,26 @@ public:
     virtual ~NameChangeTransactionTest() {
     }
 
+
     /// @brief  Instantiates a NameChangeStub test transaction
     /// 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.
     /// @param key_name value to use to create TSIG key, if blank TSIG will not
     /// be used.
-    NameChangeStubPtr makeCannedTransaction(const std::string& key_name = "") {
+    NameChangeStubPtr makeCannedTransaction(const TSIGKeyInfoPtr&
+                                            tsig_key_info = TSIGKeyInfoPtr()) {
+        // Creates IPv4 remove request, forward, and reverse domains.
+        setupForIPv4Transaction(dhcp_ddns::CHG_ADD, FWD_AND_REV_CHG,
+                                tsig_key_info);
+
+        // Now create the test transaction as would occur in update manager.
+        // Instantiate the transaction as would be done by update manager.
+        return (NameChangeStubPtr(new NameChangeStub(io_service_, ncr_,
+                                  forward_domain_, reverse_domain_)));
+    }
+
+    NameChangeStubPtr makeCannedTransaction(const std::string& key_name) {
         // Creates IPv4 remove request, forward, and reverse domains.
         setupForIPv4Transaction(dhcp_ddns::CHG_ADD, FWD_AND_REV_CHG, key_name);
 
@@ -299,6 +308,7 @@ public:
         // Instantiate the transaction as would be done by update manager.
         return (NameChangeStubPtr(new NameChangeStub(io_service_, ncr_,
                                   forward_domain_, reverse_domain_)));
+
     }
 
     /// @brief Builds and then sends an update request
@@ -1009,7 +1019,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("key_one"));
+    ASSERT_NO_THROW(name_change = makeCannedTransaction("key_one"));
     ASSERT_NO_THROW(name_change->initDictionaries());
     ASSERT_TRUE(name_change->selectFwdServer());
 
@@ -1098,38 +1108,55 @@ TEST_F(NameChangeTransactionTest, tsigUnexpectedSignedResponse) {
     EXPECT_EQ("response.example.com.", zone->getName().toText());
 }
 
-
 /// @brief Tests that a TSIG udpate succeeds when client and server both use
-/// the right key.
-TEST_F(NameChangeTransactionTest, tsigValidExchange) {
-    NameChangeStubPtr name_change;
-    ASSERT_NO_THROW(name_change = makeCannedTransaction("key_one"));
-    ASSERT_NO_THROW(name_change->initDictionaries());
-    ASSERT_TRUE(name_change->selectFwdServer());
-
-    // Create a server, set its TSIG key, and then start it listening.
-    FauxServer server(*io_service_, *(name_change->getCurrentServer()));
-    TSIGKeyInfoPtr key_one = makeTSIGKeyInfo("key_one");
-    server.setTSIGKey(key_one->getTSIGKey());
-    server.receive (FauxServer::USE_RCODE, dns::Rcode::NOERROR());
-
-    // Do the update.
-    ASSERT_NO_FATAL_FAILURE(doOneExchange(name_change));
-
-    // Verify that next event is IO_COMPLETED_EVT and DNS status is SUCCESS.
-    ASSERT_EQ(NameChangeTransaction::IO_COMPLETED_EVT,
-              name_change->getNextEvent());
+/// the right key.  Runs the test for all supported algorithms.
+TEST_F(NameChangeTransactionTest, tsigAllValid) {
+    std::vector<std::string>algorithms;
+    algorithms.push_back(TSIGKeyInfo::HMAC_MD5_STR);
+    algorithms.push_back(TSIGKeyInfo::HMAC_SHA1_STR);
+    algorithms.push_back(TSIGKeyInfo::HMAC_SHA224_STR);
+    algorithms.push_back(TSIGKeyInfo::HMAC_SHA256_STR);
+    algorithms.push_back(TSIGKeyInfo::HMAC_SHA384_STR);
+    algorithms.push_back(TSIGKeyInfo::HMAC_SHA512_STR);
+
+    for (int i = 0; i < algorithms.size(); ++i) {
+        TSIGKeyInfoPtr key;
+        ASSERT_NO_THROW(key.reset(new TSIGKeyInfo("test_key",
+                                                  algorithms[i],
+                                                  "GWG/Xfbju4O2iXGqkSu4PQ==")));
+        NameChangeStubPtr name_change;
+        ASSERT_NO_THROW(name_change = makeCannedTransaction(key));
+        ASSERT_NO_THROW(name_change->initDictionaries());
+        ASSERT_TRUE(name_change->selectFwdServer());
+
+        // Create a server, set its TSIG key, and then start it listening.
+        FauxServer server(*io_service_, *(name_change->getCurrentServer()));
+        // Since we create a new server instance each time we need to tell
+        // it not reschedule receives automatically.
+        server.perpetual_receive_ = false;
+        server.setTSIGKey(key->getTSIGKey());
+        server.receive (FauxServer::USE_RCODE, dns::Rcode::NOERROR());
+
+        // Do the update.
+        ASSERT_NO_FATAL_FAILURE(doOneExchange(name_change));
+
+        // Verify that next event is IO_COMPLETED_EVT and DNS status is SUCCESS.
+        ASSERT_EQ(NameChangeTransaction::IO_COMPLETED_EVT,
+                  name_change->getNextEvent());
 
-    ASSERT_EQ(DNSClient::SUCCESS, name_change->getDnsUpdateStatus());
+        ASSERT_EQ(DNSClient::SUCCESS, name_change->getDnsUpdateStatus());
 
-    D2UpdateMessagePtr response = name_change->getDnsUpdateResponse();
-    ASSERT_TRUE(response);
-    ASSERT_EQ(dns::Rcode::NOERROR().getCode(), response->getRcode().getCode());
-    D2ZonePtr zone = response->getZone();
-    EXPECT_TRUE(zone);
-    EXPECT_EQ("response.example.com.", zone->getName().toText());
+        D2UpdateMessagePtr response = name_change->getDnsUpdateResponse();
+        ASSERT_TRUE(response);
+        ASSERT_EQ(dns::Rcode::NOERROR().getCode(),
+                  response->getRcode().getCode());
+        D2ZonePtr zone = response->getZone();
+        EXPECT_TRUE(zone);
+        EXPECT_EQ("response.example.com.", zone->getName().toText());
+    }
 }
 
+
 /// @brief Tests the prepNewRequest method
 TEST_F(NameChangeTransactionTest, prepNewRequest) {
     NameChangeStubPtr name_change;