diff --git a/src/lib/dns/tests/tsigkey_unittest.cc b/src/lib/dns/tests/tsigkey_unittest.cc index 22b26690b8..3408d4552d 100644 --- a/src/lib/dns/tests/tsigkey_unittest.cc +++ b/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); - } diff --git a/src/lib/dns/tsigkey.cc b/src/lib/dns/tsigkey.cc index 42ef532437..4082fbec04 100644 --- a/src/lib/dns/tsigkey.cc +++ b/src/lib/dns/tsigkey.cc @@ -42,8 +42,7 @@ namespace { if (name == TSIGKey::HMACSHA256_NAME()) { return (isc::cryptolink::SHA256); } - isc_throw(InvalidParameter, - "Unknown TSIG algorithm is specified: " << name); + return (isc::cryptolink::UNKNOWN_HASH); } } @@ -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 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 diff --git a/src/lib/dns/tsigkey.h b/src/lib/dns/tsigkey.h index 7959514cdf..dd79d1f42a 100644 --- a/src/lib/dns/tsigkey.h +++ b/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 - /// algorithm to this implementation, which are defined via the - /// static const member functions. For other names - /// an exception of class \c InvalidParameter will be thrown. - /// Note: This restriction may be too strict, and we may revisit it - /// later. + /// \c algorithm_name should generally be a known algorithm to this + /// implementation, which are defined via the + /// static const member functions. + /// + /// Other names are still accepted as long as the secret is empty + /// (\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: /// :[:] /// Where is a domain name for the key, 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.