[trac893] changed the behavior against "unknown" TSIG key algorithm: accept

unknown names as long as the secret is empty.  This will be useful in handling
the BADKEY situation.

src/lib/dns/tests/tsigkey_unittest.cc

@@ -59,9 +59,13 @@ TEST_F(TSIGKeyTest, construct) {
     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()),
                  isc::InvalidParameter);
+    TSIGKey key2(key_name, Name("unknown-alg"), NULL, 0);
+    EXPECT_EQ(key_name, key2.getKeyName());
+    EXPECT_EQ(Name("unknown-alg"), key2.getAlgorithmName());
 
     // The algorithm name should be converted to the canonical form.
     EXPECT_EQ("hmac-sha1.",
@@ -69,6 +73,7 @@ TEST_F(TSIGKeyTest, construct) {
                       secret.c_str(),
                       secret.size()).getAlgorithmName().toText());
 
+    // Same for key name
     EXPECT_EQ("example.com.",
               TSIGKey(Name("EXAMPLE.CoM."), TSIGKey::HMACSHA256_NAME(),
                       secret.c_str(),
@@ -270,6 +275,8 @@ TEST(TSIGStringTest, TSIGKeyFromToString) {
     TSIGKey k2 = TSIGKey("test.example.:MSG6Ng==:hmac-md5.sig-alg.reg.int.");
     TSIGKey k3 = TSIGKey("test.example:MSG6Ng==");
     TSIGKey k4 = TSIGKey(Name("test.example."), Name("hmac-sha1."), NULL, 0);
+    // "Unknown" key with empty secret is okay
+    TSIGKey k5 = TSIGKey("test.example.::unknown");
 
     EXPECT_EQ("test.example.:MSG6Ng==:hmac-md5.sig-alg.reg.int.",
               k1.toText());
@@ -278,6 +285,8 @@ TEST(TSIGStringTest, TSIGKeyFromToString) {
     EXPECT_EQ("test.example.:MSG6Ng==:hmac-md5.sig-alg.reg.int.",
               k3.toText());
     EXPECT_EQ("test.example.::hmac-sha1.", k4.toText());
+    EXPECT_EQ(Name("test.example."), k5.getKeyName());
+    EXPECT_EQ(Name("unknown"), k5.getAlgorithmName());
 
     EXPECT_THROW(TSIGKey(""), isc::InvalidParameter);
     EXPECT_THROW(TSIGKey(":"), isc::InvalidParameter);
@@ -288,7 +297,6 @@ TEST(TSIGStringTest, TSIGKeyFromToString) {
     EXPECT_THROW(TSIGKey("test.example.:"), isc::InvalidParameter);
     EXPECT_THROW(TSIGKey("test.example.:MSG6Ng==:"), isc::InvalidParameter);
     EXPECT_THROW(TSIGKey("test.example.:MSG6Ng==:unknown"), isc::InvalidParameter);
-
 }
 
 

src/lib/dns/tsigkey.cc

@@ -42,8 +42,7 @@ namespace {
         if (name == TSIGKey::HMACSHA256_NAME()) {
             return (isc::cryptolink::SHA256);
         }
-        isc_throw(InvalidParameter,
+        return (isc::cryptolink::UNKNOWN_HASH);
-                  "Unknown TSIG algorithm is specified: " << name);
     }
 }
 
@@ -74,7 +73,13 @@ TSIGKey::TSIGKey(const Name& key_name, const Name& algorithm_name,
     if ((secret != NULL && secret_len == 0) ||
         (secret == NULL && secret_len != 0)) {
         isc_throw(InvalidParameter,
-                  "TSIGKey secret and its length are inconsistent");
+                  "TSIGKey secret and its length are inconsistent: " <<
+                  key_name << ":" << algorithm_name);
+    }
+    if (algorithm == isc::cryptolink::UNKNOWN_HASH && secret_len != 0) {
+        isc_throw(InvalidParameter,
+                  "TSIGKey with unknown algorithm has non empty secret: " <<
+                  key_name << ":" << algorithm_name);
     }
     impl_ = new TSIGKeyImpl(key_name, algorithm_name, algorithm, secret,
                             secret_len);
@@ -111,8 +116,15 @@ TSIGKey::TSIGKey(const std::string& str) : impl_(NULL) {
         vector<uint8_t> secret;
         isc::util::encode::decodeBase64(secret_str, secret);
 
+        if (algorithm == isc::cryptolink::UNKNOWN_HASH && !secret.empty()) {
+            isc_throw(InvalidParameter,
+                      "TSIG key with unknown algorithm has non empty secret: "
+                      << str);
+        }
+
         impl_ = new TSIGKeyImpl(Name(keyname_str), algo_name, algorithm,
-                                &secret[0], secret.size());
+                                secret.empty() ? NULL : &secret[0],
+                                secret.size());
     } catch (const Exception& e) {
         // 'reduce' the several types of exceptions name parsing and
         // Base64 decoding can throw to just the InvalidParameter

src/lib/dns/tsigkey.h

@@ -68,12 +68,21 @@ public:
     //@{
     /// \brief Constructor from key parameters
     ///
-    /// In the current implementation, \c algorithm_name must be a known
+    /// \c algorithm_name should generally be a known algorithm to this
-    /// algorithm to this implementation, which are defined via the
+    /// implementation, which are defined via the
-    /// <code>static const</code> member functions.  For other names
+    /// <code>static const</code> member functions.
-    /// an exception of class \c InvalidParameter will be thrown.
+    ///
-    /// Note: This restriction may be too strict, and we may revisit it
+    /// Other names are still accepted as long as the secret is empty
-    /// later.
+    /// (\c secret is \c NULL and \c secret_len is 0), however; in some cases
+    /// we might want to treat just the pair of key name and algorithm name
+    /// opaquely, e.g., when generating a response TSIG with a BADKEY error.
+    /// It is unlikely that a TSIG key with an unknown algorithm is of any
+    /// use with actual crypto operation, so care must be taken when dealing
+    /// with such keys. (The restriction for the secret will prevent
+    /// accidental creation of such a dangerous key, e.g., due to misspelling
+    /// in a configuration file).
+    /// If the given algorithm name is unknown and non empty secret is
+    /// specified, an exception of type \c InvalidParameter will be thrown.
     ///
     /// \c secret and \c secret_len must be consistent in that the latter
     /// is 0 if and only if the former is \c NULL;
@@ -98,9 +107,12 @@ public:
     /// <name>:<secret>[:<algorithm>]
     /// Where <name> is a domain name for the key, <secret> is a
     /// base64 representation of the key secret, and the optional
-    /// algorithm is an algorithm identifier as specified in RFC4635
+    /// algorithm is an algorithm identifier as specified in RFC4635.
     /// The default algorithm is hmac-md5.sig-alg.reg.int.
     ///
+    /// The same restriction about the algorithm name (and secret) as that
+    /// for the other constructor applies.
+    ///
     /// Since ':' is used as a separator here, it is not possible to
     /// use this constructor to create keys with a ':' character in
     /// their name.