[2762] Update tests and implementation (see full log)

* Fix the unittest so that it compares the correct object (tsig6)
* Store canonical long-form HMAC name in case of HMAC-MD5
  + Update TSIG RDATA class (test already present)
  + Update TSIGKey class (new tests added)

src/lib/dns/rdata/any_255/tsig_250.cc

@@ -26,6 +26,7 @@
 #include <dns/rdata.h>
 #include <dns/rdataclass.h>
 #include <dns/rcode.h>
+#include <dns/tsigkey.h>
 #include <dns/tsigerror.h>
 #include <dns/rdata/generic/detail/lexer_util.h>
 
@@ -75,6 +76,9 @@ TSIGImpl*
 TSIG::constructFromLexer(MasterLexer& lexer, const Name* origin) {
     const Name& algorithm =
         createNameFromLexer(lexer, origin ? origin : &Name::ROOT_NAME());
+    const Name& canonical_algorithm_name =
+        (algorithm == TSIGKey::HMACMD5_SHORT_NAME()) ?
+            TSIGKey::HMACMD5_NAME() : algorithm;
 
     const string& time_txt =
         lexer.getNextToken(MasterToken::STRING).getString();
@@ -154,8 +158,8 @@ TSIG::constructFromLexer(MasterLexer& lexer, const Name* origin) {
     // RFC2845 says Other Data is "empty unless Error == BADTIME".
     // However, we don't enforce that.
 
-    return (new TSIGImpl(algorithm, time_signed, fudge, mac, orig_id,
-                         error, other_data));
+    return (new TSIGImpl(canonical_algorithm_name, time_signed, fudge, mac,
+                         orig_id, error, other_data));
 }
 
 /// \brief Constructor from string.
@@ -302,8 +306,11 @@ TSIG::TSIG(InputBuffer& buffer, size_t) :
         buffer.readData(&other_data[0], other_len);
     }
 
-    impl_ = new TSIGImpl(algorithm, time_signed, fudge, mac, original_id,
-                         error, other_data);
+    const Name& canonical_algorithm_name =
+        (algorithm == TSIGKey::HMACMD5_SHORT_NAME()) ?
+            TSIGKey::HMACMD5_NAME() : algorithm;
+    impl_ = new TSIGImpl(canonical_algorithm_name, time_signed, fudge, mac,
+                         original_id, error, other_data);
 }
 
 TSIG::TSIG(const Name& algorithm, uint64_t time_signed, uint16_t fudge,
@@ -324,8 +331,11 @@ TSIG::TSIG(const Name& algorithm, uint64_t time_signed, uint16_t fudge,
         isc_throw(InvalidParameter,
                   "TSIG Other data length and data inconsistent");
     }
-    impl_ = new TSIGImpl(algorithm, time_signed, fudge, mac_size, mac,
-                         original_id, error, other_len, other_data);
+    const Name& canonical_algorithm_name =
+        (algorithm == TSIGKey::HMACMD5_SHORT_NAME()) ?
+            TSIGKey::HMACMD5_NAME() : algorithm;
+    impl_ = new TSIGImpl(canonical_algorithm_name, time_signed, fudge, mac_size,
+                         mac, original_id, error, other_len, other_data);
 }
 
 /// \brief The copy constructor.

src/lib/dns/tests/rdata_tsig_unittest.cc

@@ -145,8 +145,8 @@ TEST_F(Rdata_TSIG_Test, fromText) {
                        "0 16020 BADKEY 0 )");
 
     // short-form HMAC-MD5 name
-    const any::TSIG tsig6("hmac-md5 1286779327 300 0 16020 BADKEY 0");
-    EXPECT_EQ(0, tsig1.compare(rdata_tsig));
+    const any::TSIG tsig6("hmac-md5. 1286779327 300 0 16020 BADKEY 0");
+    EXPECT_EQ(0, tsig6.compare(rdata_tsig));
 };
 
 TEST_F(Rdata_TSIG_Test, badText) {

src/lib/dns/tests/tsigkey_unittest.cc

@@ -38,6 +38,7 @@ protected:
 
 TEST_F(TSIGKeyTest, algorithmNames) {
     EXPECT_EQ(Name("hmac-md5.sig-alg.reg.int"), TSIGKey::HMACMD5_NAME());
+    EXPECT_EQ(Name("hmac-md5"), TSIGKey::HMACMD5_SHORT_NAME());
     EXPECT_EQ(Name("hmac-sha1"), TSIGKey::HMACSHA1_NAME());
     EXPECT_EQ(Name("hmac-sha256"), TSIGKey::HMACSHA256_NAME());
     EXPECT_EQ(Name("hmac-sha224"), TSIGKey::HMACSHA224_NAME());
@@ -47,6 +48,9 @@ TEST_F(TSIGKeyTest, algorithmNames) {
     // Also check conversion to cryptolink definitions
     EXPECT_EQ(isc::cryptolink::MD5, TSIGKey(key_name, TSIGKey::HMACMD5_NAME(),
                                             NULL, 0).getAlgorithm());
+    EXPECT_EQ(isc::cryptolink::MD5,
+              TSIGKey(key_name, TSIGKey::HMACMD5_SHORT_NAME(),
+                      NULL, 0).getAlgorithm());
     EXPECT_EQ(isc::cryptolink::SHA1, TSIGKey(key_name, TSIGKey::HMACSHA1_NAME(),
                                              NULL, 0).getAlgorithm());
     EXPECT_EQ(isc::cryptolink::SHA256, TSIGKey(key_name,
@@ -71,6 +75,13 @@ TEST_F(TSIGKeyTest, construct) {
     EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, secret.c_str(),
                         secret.size(), key.getSecret(), key.getSecretLength());
 
+    TSIGKey key_short_md5(key_name, TSIGKey::HMACMD5_SHORT_NAME(),
+                          secret.c_str(), secret.size());
+    EXPECT_EQ(key_name, key.getKeyName());
+    EXPECT_EQ(Name("hmac-md5.sig-alg.reg.int"), key.getAlgorithmName());
+    EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, secret.c_str(),
+                        secret.size(), key.getSecret(), key.getSecretLength());
+
     // "unknown" algorithm is only accepted with empty secret.
     EXPECT_THROW(TSIGKey(key_name, Name("unknown-alg"),
                          secret.c_str(), secret.size()),

src/lib/dns/tsigkey.cc

@@ -31,16 +31,12 @@ using namespace isc::cryptolink;
 namespace isc {
 namespace dns {
 namespace {
-    bool
-    isHMACMD5(const isc::dns::Name& name) {
-        static const Name md5_short_name("hmac-md5");
-        return ((name == TSIGKey::HMACMD5_NAME()) ||
-                (name == md5_short_name));
-    }
-
     HashAlgorithm
     convertAlgorithmName(const isc::dns::Name& name) {
-        if (isHMACMD5(name)) {
+        if (name == TSIGKey::HMACMD5_NAME()) {
+            return (isc::cryptolink::MD5);
+        }
+        if (name == TSIGKey::HMACMD5_SHORT_NAME()) {
             return (isc::cryptolink::MD5);
         }
         if (name == TSIGKey::HMACSHA1_NAME()) {
@@ -75,6 +71,9 @@ TSIGKey::TSIGKeyImpl {
     {
         // Convert the key and algorithm names to the canonical form.
         key_name_.downcase();
+        if (algorithm == isc::cryptolink::MD5) {
+            algorithm_name_ = TSIGKey::HMACMD5_NAME();
+        }
         algorithm_name_.downcase();
     }
     Name key_name_;
@@ -213,6 +212,12 @@ Name& TSIGKey::HMACMD5_NAME() {
 }
 
 const
+Name& TSIGKey::HMACMD5_SHORT_NAME() {
+    static Name alg_name("hmac-md5");
+    return (alg_name);
+}
+
+const
 Name& TSIGKey::HMACSHA1_NAME() {
     static Name alg_name("hmac-sha1");
     return (alg_name);

src/lib/dns/tsigkey.h

@@ -203,6 +203,7 @@ public:
     /// We'll add others as we see the need for them.
     //@{
     static const Name& HMACMD5_NAME();    ///< HMAC-MD5 (RFC2845)
+    static const Name& HMACMD5_SHORT_NAME();
     static const Name& HMACSHA1_NAME();   ///< HMAC-SHA1 (RFC4635)
     static const Name& HMACSHA256_NAME(); ///< HMAC-SHA256 (RFC4635)
     static const Name& HMACSHA224_NAME(); ///< HMAC-SHA256 (RFC4635)