diff --git a/ChangeLog b/ChangeLog index 15d256e79a..2213b2666d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,7 @@ +848. [func] fdupont + Added truncated HMAC support to TSIG, as per RFC 4635. + (Trac #3593, git ...) + 847. [build] fdupont Removed no longer used configuration option --with-shared-memory and associated files and variables. diff --git a/doc/examples/ddns/sample1.json b/doc/examples/ddns/sample1.json index 922027f515..9a61fb9800 100644 --- a/doc/examples/ddns/sample1.json +++ b/doc/examples/ddns/sample1.json @@ -106,6 +106,12 @@ "algorithm": "HMAC-SHA1", "secret": "hRrp29wzUv3uzSNRLlY68w==" } + { + "name": "d2.sha512.key", + "algorithm": "HMAC-SHA512", + "digest_bits": 256, + "secret": "/4wklkm04jeH4anx2MKGJLcya+ZLHldL5d6mK+4q6UXQP7KJ9mS2QG29hh0SJR4LA0ikxNJTUMvir42gLx6fGQ==" + } ] } diff --git a/doc/examples/ddns/template.json b/doc/examples/ddns/template.json index 19bea2f87c..98a75ed5d7 100644 --- a/doc/examples/ddns/template.json +++ b/doc/examples/ddns/template.json @@ -92,6 +92,9 @@ # Valid values for algorithm are: HMAC-MD5, HMAC-SHA1, # HMAC-SHA224, HMAC-SHA256, # HMAC-SHA384, HMAC-SHA512 +# "digest_bits" : 256, +# Minimum truncated length in bits. +# Default 0 (means truncation is forbidden). "secret" : "" } # , diff --git a/doc/guide/ddns.xml b/doc/guide/ddns.xml index f73eb0f067..04fe06fd3a 100644 --- a/doc/guide/ddns.xml +++ b/doc/guide/ddns.xml @@ -283,6 +283,17 @@ corresponding values in the DHCP servers' "dhcp-ddns" configuration section. This value is not case sensitive. + + + digest_bits - + is used to specify the minimum truncated length in bits. + The default value 0 means truncation is forbidden, not 0 + values must be an integral number of octets, be greater + than 80 and the half of the full length. Note in BIND9 + this parameter is appended after a dash to the algorithm + name. + + secret - diff --git a/src/bin/d2/d2_config.cc b/src/bin/d2/d2_config.cc index 6b1ae252ab..62d3ce21c1 100644 --- a/src/bin/d2/d2_config.cc +++ b/src/bin/d2/d2_config.cc @@ -20,6 +20,7 @@ #include #include +#include #include #include @@ -143,8 +144,9 @@ 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) - :name_(name), algorithm_(algorithm), secret_(secret), tsig_key_() { + const std::string& secret, uint32_t digestbits) + :name_(name), algorithm_(algorithm), secret_(secret), + digestbits_(digestbits), tsig_key_() { remakeKey(); } @@ -180,6 +182,9 @@ TSIGKeyInfo::remakeKey() { stream << dns::Name(name_).toText() << ":" << secret_ << ":" << stringToAlgorithmName(algorithm_); + if (digestbits_ > 0) { + stream << ":" << digestbits_; + } tsig_key_.reset(new dns::TSIGKey(stream.str())); } catch (const std::exception& ex) { @@ -366,14 +371,17 @@ TSIGKeyInfoParser::build(isc::data::ConstElementPtr key_config) { std::string name; std::string algorithm; + uint32_t digestbits = 0; std::string secret; std::map pos; // Fetch the key's parsed scalar values from parser's local storage. - // All are required, if any are missing we'll throw. + // Only digestbits is optional and doesn't throw when missing try { pos["name"] = local_scalars_.getParam("name", name); pos["algorithm"] = local_scalars_.getParam("algorithm", algorithm); + pos["digest_bits"] = local_scalars_.getParam("digest_bits", digestbits, + DCfgContextBase::OPTIONAL); pos["secret"] = local_scalars_.getParam("secret", secret); } catch (const std::exception& ex) { isc_throw(D2CfgError, "TSIG Key incomplete : " << ex.what() @@ -400,6 +408,44 @@ TSIGKeyInfoParser::build(isc::data::ConstElementPtr key_config) { isc_throw(D2CfgError, "TSIG key : " << ex.what() << " (" << pos["algorithm"] << ")"); } + // Not zero Digestbits must be an integral number of octets, greater + // than 80 and the half of the full length + if (digestbits > 0) { + if ((digestbits % 8) != 0) { + isc_throw(D2CfgError, "Invalid TSIG key digest_bits specified : " << + digestbits << " (" << pos["digest_bits"] << ")"); + } + if (digestbits < 80) { + isc_throw(D2CfgError, "TSIG key digest_bits too small : " << + digestbits << " (" << pos["digest_bits"] << ")"); + } + if (boost::iequals(algorithm, TSIGKeyInfo::HMAC_SHA224_STR)) { + if (digestbits < 112) { + isc_throw(D2CfgError, "TSIG key digest_bits too small : " << + digestbits << " (" << pos["digest_bits"] + << ")"); + } + } else if (boost::iequals(algorithm, TSIGKeyInfo::HMAC_SHA256_STR)) { + if (digestbits < 128) { + isc_throw(D2CfgError, "TSIG key digest_bits too small : " << + digestbits << " (" << pos["digest_bits"] + << ")"); + } + } else if (boost::iequals(algorithm, TSIGKeyInfo::HMAC_SHA384_STR)) { + if (digestbits < 192) { + isc_throw(D2CfgError, "TSIG key digest_bits too small : " << + digestbits << " (" << pos["digest_bits"] + << ")"); + } + } else if (boost::iequals(algorithm, TSIGKeyInfo::HMAC_SHA512_STR)) { + if (digestbits < 256) { + isc_throw(D2CfgError, "TSIG key digest_bits too small : " << + digestbits << " (" << pos["digest_bits"] + << ")"); + } + } + } + // Secret cannot be blank. // Cryptolink lib doesn't offer any way to validate these. As long as it // isn't blank we'll accept it. If the content is bad, the call to in @@ -414,7 +460,7 @@ TSIGKeyInfoParser::build(isc::data::ConstElementPtr key_config) { // with an invalid secret content. TSIGKeyInfoPtr key_info; try { - key_info.reset(new TSIGKeyInfo(name, algorithm, secret)); + key_info.reset(new TSIGKeyInfo(name, algorithm, secret, digestbits)); } catch (const std::exception& ex) { isc_throw(D2CfgError, ex.what() << " (" << key_config->getPosition() << ")"); @@ -435,6 +481,9 @@ TSIGKeyInfoParser::createConfigParser(const std::string& config_id, (config_id == "secret")) { parser = new isc::dhcp::StringParser(config_id, local_scalars_.getStringStorage()); + } else if (config_id == "digest_bits") { + parser = new isc::dhcp::Uint32Parser(config_id, + local_scalars_.getUint32Storage()); } else { isc_throw(NotImplemented, "parser error: TSIGKeyInfo parameter not supported: " diff --git a/src/bin/d2/d2_config.h b/src/bin/d2/d2_config.h index 159da45fe9..bb9f7f1c92 100644 --- a/src/bin/d2/d2_config.h +++ b/src/bin/d2/d2_config.h @@ -41,7 +41,8 @@ namespace d2 { /// forward domains and one list for reverse domains. /// /// The key list consists of one or more TSIG keys, each entry described by -/// a name, the algorithm method name, and its secret key component. +/// a name, the algorithm method name, optionally the minimum truncated +/// length, and its secret key component. /// /// @todo NOTE that TSIG configuration parsing is functional, the use of /// TSIG Keys during the actual DNS update transactions is not. This will be @@ -317,33 +318,41 @@ public: /// Activate: 20140515143700 /// @endcode /// where the value the "Key:" entry is the secret component of the key.) + /// @param digestbits the minimum truncated length in bits /// /// @throw D2CfgError if values supplied are invalid: /// name cannot be blank, algorithm must be a supported value, /// secret must be a non-blank, base64 encoded string. TSIGKeyInfo(const std::string& name, const std::string& algorithm, - const std::string& secret); + const std::string& secret, uint32_t digestbits = 0); /// @brief Destructor virtual ~TSIGKeyInfo(); /// @brief Getter which returns the key's name. /// - /// @return returns the name as as std::string. + /// @return returns the name as a std::string. const std::string getName() const { return (name_); } /// @brief Getter which returns the key's algorithm string ID /// - /// @return returns the algorithm as as std::string. + /// @return returns the algorithm as a std::string. const std::string getAlgorithm() const { return (algorithm_); } + /// @brief Getter which returns the key's minimum truncated length + /// + /// @return returns the minimum truncated length or 0 as an uint32_t + uint32_t getDigestbits() const { + return (digestbits_); + } + /// @brief Getter which returns the key's secret. /// - /// @return returns the secret as as std::string. + /// @return returns the secret as a std::string. const std::string getSecret() const { return (secret_); } @@ -376,7 +385,7 @@ private: /// @brief Creates the actual TSIG key instance member /// /// Replaces this tsig_key member with a key newly created using the key - /// name, algorithm id, and secret. + /// name, algorithm id, digest bits, and secret. /// This method is currently only called by the constructor, however it /// could be called post-construction should keys ever support expiration. /// @@ -395,6 +404,10 @@ private: /// @brief The base64 encoded string secret value component of this key. std::string secret_; + /// @brief The minimum truncated length in bits + /// (0 means no truncation is allowed and is the default) + uint32_t digestbits_; + /// @brief The actual TSIG key. dns::TSIGKeyPtr tsig_key_; }; @@ -759,7 +772,8 @@ public: /// The key elements currently supported are(see dhcp-ddns.spec): /// 1. name /// 2. algorithm - /// 3. secret + /// 3. digestbits + /// 4. secret /// /// @param config_id is the "item_name" for a specific member element of /// the "tsig_key" specification. diff --git a/src/bin/d2/dhcp-ddns.spec b/src/bin/d2/dhcp-ddns.spec index 4127f11e55..d126dafa78 100644 --- a/src/bin/d2/dhcp-ddns.spec +++ b/src/bin/d2/dhcp-ddns.spec @@ -58,6 +58,12 @@ "item_optional": false, "item_default": "" }, + { + "item_name": "digest_bits", + "item_type": "integer", + "item_optional": true, + "item_default": 0 + }, { "item_name": "secret", "item_type": "string", diff --git a/src/bin/d2/tests/d2_cfg_mgr_unittests.cc b/src/bin/d2/tests/d2_cfg_mgr_unittests.cc index d5026a043a..538a8011a9 100644 --- a/src/bin/d2/tests/d2_cfg_mgr_unittests.cc +++ b/src/bin/d2/tests/d2_cfg_mgr_unittests.cc @@ -249,11 +249,13 @@ bool checkServer(DnsServerInfoPtr server, const char* hostname, /// @return returns true if there is a match across the board, otherwise it /// returns false. bool checkKey(TSIGKeyInfoPtr key, const std::string& name, - const std::string& algorithm, const std::string& secret) { + const std::string& algorithm, const std::string& secret, + uint32_t digestbits = 0) { // Return value, assume its a match. return (((key) && (key->getName() == name) && (key->getAlgorithm() == algorithm) && + (key->getDigestbits() == digestbits) && (key->getSecret() == secret) && (key->getTSIGKey()))); } @@ -618,6 +620,7 @@ TEST_F(TSIGKeyInfoTest, validEntry) { std::string config = "{" " \"name\": \"d2_key_one\" , " " \"algorithm\": \"HMAC-MD5\" , " + " \"digest_bits\": 120 , " " \"secret\": \"dGhpcyBrZXkgd2lsbCBtYXRjaA==\" " "}"; ASSERT_TRUE(fromJSON(config)); @@ -638,7 +641,7 @@ TEST_F(TSIGKeyInfoTest, validEntry) { // Verify the key contents. EXPECT_TRUE(checkKey(key, "d2_key_one", "HMAC-MD5", - "dGhpcyBrZXkgd2lsbCBtYXRjaA==")); + "dGhpcyBrZXkgd2lsbCBtYXRjaA==", 120)); } /// @brief Verifies that attempting to parse an invalid list of TSIGKeyInfo @@ -649,11 +652,13 @@ TEST_F(TSIGKeyInfoTest, invalidTSIGKeyList) { " { \"name\": \"key1\" , " " \"algorithm\": \"HMAC-MD5\" ," + " \"digest_bits\": 120 , " " \"secret\": \"GWG/Xfbju4O2iXGqkSu4PQ==\" " " }," // this entry has an invalid algorithm " { \"name\": \"key2\" , " " \"algorithm\": \"\" ," + " \"digest_bits\": 120 , " " \"secret\": \"GWG/Xfbju4O2iXGqkSu4PQ==\" " " }," " { \"name\": \"key3\" , " @@ -680,10 +685,12 @@ TEST_F(TSIGKeyInfoTest, duplicateTSIGKey) { " { \"name\": \"key1\" , " " \"algorithm\": \"HMAC-MD5\" ," + " \"digest_bits\": 120 , " " \"secret\": \"GWG/Xfbju4O2iXGqkSu4PQ==\" " " }," " { \"name\": \"key2\" , " " \"algorithm\": \"HMAC-MD5\" ," + " \"digest_bits\": 120 , " " \"secret\": \"GWG/Xfbju4O2iXGqkSu4PQ==\" " " }," " { \"name\": \"key1\" , " @@ -710,26 +717,32 @@ TEST_F(TSIGKeyInfoTest, validTSIGKeyList) { " { \"name\": \"key1\" , " " \"algorithm\": \"HMAC-MD5\" ," + " \"digest_bits\": 80 , " " \"secret\": \"dGhpcyBrZXkgd2lsbCBtYXRjaA==\" " " }," " { \"name\": \"key2\" , " " \"algorithm\": \"HMAC-SHA1\" ," + " \"digest_bits\": 80 , " " \"secret\": \"dGhpcyBrZXkgd2lsbCBtYXRjaA==\" " " }," " { \"name\": \"key3\" , " " \"algorithm\": \"HMAC-SHA256\" ," + " \"digest_bits\": 128 , " " \"secret\": \"dGhpcyBrZXkgd2lsbCBtYXRjaA==\" " " }," " { \"name\": \"key4\" , " " \"algorithm\": \"HMAC-SHA224\" ," + " \"digest_bits\": 112 , " " \"secret\": \"dGhpcyBrZXkgd2lsbCBtYXRjaA==\" " " }," " { \"name\": \"key5\" , " " \"algorithm\": \"HMAC-SHA384\" ," + " \"digest_bits\": 192 , " " \"secret\": \"dGhpcyBrZXkgd2lsbCBtYXRjaA==\" " " }," " { \"name\": \"key6\" , " " \"algorithm\": \"HMAC-SHA512\" ," + " \"digest_bits\": 256 , " " \"secret\": \"dGhpcyBrZXkgd2lsbCBtYXRjaA==\" " " }" " ]"; @@ -754,7 +767,8 @@ TEST_F(TSIGKeyInfoTest, validTSIGKeyList) { TSIGKeyInfoPtr& key = gotit->second; // Verify the key contents. - EXPECT_TRUE(checkKey(key, "key1", TSIGKeyInfo::HMAC_MD5_STR, ref_secret)); + EXPECT_TRUE(checkKey(key, "key1", TSIGKeyInfo::HMAC_MD5_STR, + ref_secret, 80)); // Find the 2nd key and retrieve it. gotit = keys_->find("key2"); @@ -762,7 +776,8 @@ TEST_F(TSIGKeyInfoTest, validTSIGKeyList) { key = gotit->second; // Verify the key contents. - EXPECT_TRUE(checkKey(key, "key2", TSIGKeyInfo::HMAC_SHA1_STR, ref_secret)); + EXPECT_TRUE(checkKey(key, "key2", TSIGKeyInfo::HMAC_SHA1_STR, + ref_secret, 80)); // Find the 3rd key and retrieve it. gotit = keys_->find("key3"); @@ -771,7 +786,7 @@ TEST_F(TSIGKeyInfoTest, validTSIGKeyList) { // Verify the key contents. EXPECT_TRUE(checkKey(key, "key3", TSIGKeyInfo::HMAC_SHA256_STR, - ref_secret)); + ref_secret, 128)); // Find the 4th key and retrieve it. gotit = keys_->find("key4"); @@ -780,7 +795,7 @@ TEST_F(TSIGKeyInfoTest, validTSIGKeyList) { // Verify the key contents. EXPECT_TRUE(checkKey(key, "key4", TSIGKeyInfo::HMAC_SHA224_STR, - ref_secret)); + ref_secret, 112)); // Find the 5th key and retrieve it. gotit = keys_->find("key5"); @@ -789,7 +804,7 @@ TEST_F(TSIGKeyInfoTest, validTSIGKeyList) { // Verify the key contents. EXPECT_TRUE(checkKey(key, "key5", TSIGKeyInfo::HMAC_SHA384_STR, - ref_secret)); + ref_secret, 192)); // Find the 6th key and retrieve it. gotit = keys_->find("key6"); @@ -798,7 +813,7 @@ TEST_F(TSIGKeyInfoTest, validTSIGKeyList) { // Verify the key contents. EXPECT_TRUE(checkKey(key, "key6", TSIGKeyInfo::HMAC_SHA512_STR, - ref_secret)); + ref_secret, 256)); } /// @brief Tests the enforcement of data validation when parsing DnsServerInfos. @@ -1261,6 +1276,7 @@ TEST_F(D2CfgMgrTest, fullConfig) { "{" " \"name\": \"d2_key.billcat.net\" , " " \"algorithm\": \"hmac-md5\" , " + " \"digest_bits\": 120 , " " \"secret\": \"LSWXnfkKZjdPJI5QxlpnfQ==\" " "}" "]," diff --git a/src/bin/d2/tests/testdata/d2_cfg_tests.json b/src/bin/d2/tests/testdata/d2_cfg_tests.json index 4a7bfbde0a..742d525e92 100644 --- a/src/bin/d2/tests/testdata/d2_cfg_tests.json +++ b/src/bin/d2/tests/testdata/d2_cfg_tests.json @@ -479,6 +479,195 @@ } } +#----- D2.tsig_keys, digest_bits tests +,{ +"description" : "D2.tsig_keys, all valid algorthms", +"data" : + { + "forward_ddns" : {}, + "reverse_ddns" : {}, + "tsig_keys" : + [ + { + "name" : "d2.md5.key", + "algorithm" : "HMAC-MD5", + "digest_bits" : 80, + "secret" : "LSWXnfkKZjdPJI5QxlpnfQ==" + }, + { + "name" : "d2.sha1.key", + "algorithm" : "HMAC-SHA1", + "digest_bits" : 80, + "secret" : "hRrp29wzUv3uzSNRLlY68w==" + }, + { + "name" : "d2.sha224.key", + "algorithm" : "HMAC-SHA224", + "digest_bits" : 112, + "secret" : "bZEG7Ow8OgAUPfLWV3aAUQ==" + }, + { + "name" : "d2.sha256.key", + "algorithm" : "hmac-sha256", + "digest_bits" : 128, + "secret" : "bjF4hYhTfQ5MX0siagelsw==" + }, + { + "name" : "d2.sha384.key", + "algorithm" : "hmac-sha384", + "digest_bits" : 192, + "secret" : "Gwk53fvy3CmbupoI9TgigA==" + }, + { + "name" : "d2.sha512.key", + "algorithm" : "hmac-sha512", + "digest_bits" : 256, + "secret" : "wP+5FqMnKXCxBWljU/BZAA==" + } + ] + } +} + +#----- +,{ +"description" : "D2.tsig_keys, invalid digest_bits", +"should_fail" : true, +"data" : + { + "forward_ddns" : {}, + "reverse_ddns" : {}, + "tsig_keys" : + [ + { + "name" : "d2.md5.key", + "algorithm" : "HMAC-MD5", + "digest_bits" : 84, + "secret" : "LSWXnfkKZjdPJI5QxlpnfQ==" + }, + ] + } +} + +#----- +,{ +"description" : "D2.tsig_keys, too small truncated HMAC-MD5", +"should_fail" : true, +"data" : + { + "forward_ddns" : {}, + "reverse_ddns" : {}, + "tsig_keys" : + [ + { + "name" : "d2.md5.key", + "algorithm" : "HMAC-MD5", + "digest_bits" : 72, + "secret" : "LSWXnfkKZjdPJI5QxlpnfQ==" + }, + ] + } +} + +#----- +,{ +"description" : "D2.tsig_keys, too small truncated HMAC-SHA1", +"should_fail" : true, +"data" : + { + "forward_ddns" : {}, + "reverse_ddns" : {}, + "tsig_keys" : + [ + { + "name" : "d2.sha1.key", + "algorithm" : "HMAC-SHA1", + "digest_bits" : 72, + "secret" : "hRrp29wzUv3uzSNRLlY68w==" + }, + ] + } +} + +#----- +,{ +"description" : "D2.tsig_keys, too small truncated HMAC-SHA224", +"should_fail" : true, +"data" : + { + "forward_ddns" : {}, + "reverse_ddns" : {}, + "tsig_keys" : + [ + { + "name" : "d2.sha224.key", + "algorithm" : "HMAC-SHA224", + "digest_bits" : 104, + "secret" : "bZEG7Ow8OgAUPfLWV3aAUQ==" + }, + ] + } +} + +#----- +,{ +"description" : "D2.tsig_keys, too small truncated HMAC-SHA256", +"should_fail" : true, +"data" : + { + "forward_ddns" : {}, + "reverse_ddns" : {}, + "tsig_keys" : + [ + { + "name" : "d2.sha256.key", + "algorithm" : "hmac-sha256", + "digest_bits" : 120, + "secret" : "bjF4hYhTfQ5MX0siagelsw==" + }, + ] + } +} + +#----- +,{ +"description" : "D2.tsig_keys, too small truncated HMAC-SHA384", +"should_fail" : true, +"data" : + { + "forward_ddns" : {}, + "reverse_ddns" : {}, + "tsig_keys" : + [ + { + "name" : "d2.sha384.key", + "algorithm" : "hmac-sha384", + "digest_bits" : 184, + "secret" : "Gwk53fvy3CmbupoI9TgigA==" + }, + ] + } +} + +#----- +,{ +"description" : "D2.tsig_keys, too small truncated HMAC-SHA512", +"should_fail" : true, +"data" : + { + "forward_ddns" : {}, + "reverse_ddns" : {}, + "tsig_keys" : + [ + { + "name" : "d2.sha512.key", + "algorithm" : "hmac-sha512", + "digest_bits" : 248, + "secret" : "wP+5FqMnKXCxBWljU/BZAA==" + } + ] + } +} + #----- D2.tsig_keys, secret tests ,{ "description" : "D2.tsig_keys, missing secret", diff --git a/src/lib/cryptolink/botan_hmac.cc b/src/lib/cryptolink/botan_hmac.cc index 174985ccfe..a6453ef100 100644 --- a/src/lib/cryptolink/botan_hmac.cc +++ b/src/lib/cryptolink/botan_hmac.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2011 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2011, 2014 Internet Systems Consortium, Inc. ("ISC") // // Permission to use, copy, modify, and/or distribute this software for any // purpose with or without fee is hereby granted, provided that the above @@ -178,19 +178,14 @@ public: /// @todo Botan's verify_mac checks if len matches the output_length, /// which causes it to fail for truncated signatures, so we do /// the check ourselves - /// SEE BELOW FOR TEMPORARY CHANGE try { Botan::SecureVector our_mac = hmac_->final(); - if (len < getOutputLength()) { - // Currently we don't support truncated signature in TSIG (see - // #920). To avoid validating too short signature accidently, - // we enforce the standard signature size for the moment. - // Once we support truncation correctly, this if-clause should - // (and the capitalized comment above) be removed. + size_t size = getOutputLength(); + if (len != 0 && (len < 10 || len < size / 2)) { return (false); } - if (len == 0 || len > getOutputLength()) { - len = getOutputLength(); + if (len == 0 || len > size) { + len = size; } return (Botan::same_mem(&our_mac[0], static_cast(sig), diff --git a/src/lib/cryptolink/botan_link.cc b/src/lib/cryptolink/botan_link.cc index c76bbe5196..ede4f33124 100644 --- a/src/lib/cryptolink/botan_link.cc +++ b/src/lib/cryptolink/botan_link.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2011 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2011, 2014 Internet Systems Consortium, Inc. ("ISC") // // Permission to use, copy, modify, and/or distribute this software for any // purpose with or without fee is hereby granted, provided that the above diff --git a/src/lib/cryptolink/crypto_hmac.cc b/src/lib/cryptolink/crypto_hmac.cc index f20a639829..9887855c41 100644 --- a/src/lib/cryptolink/crypto_hmac.cc +++ b/src/lib/cryptolink/crypto_hmac.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2011 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2011, 2014 Internet Systems Consortium, Inc. ("ISC") // // Permission to use, copy, modify, and/or distribute this software for any // purpose with or without fee is hereby granted, provided that the above diff --git a/src/lib/cryptolink/cryptolink.cc b/src/lib/cryptolink/cryptolink.cc index d196f0a19e..799b6710c9 100644 --- a/src/lib/cryptolink/cryptolink.cc +++ b/src/lib/cryptolink/cryptolink.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2011 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2011, 2014 Internet Systems Consortium, Inc. ("ISC") // // Permission to use, copy, modify, and/or distribute this software for any // purpose with or without fee is hereby granted, provided that the above diff --git a/src/lib/cryptolink/cryptolink.h b/src/lib/cryptolink/cryptolink.h index 727c26f78a..69e1cc2d97 100644 --- a/src/lib/cryptolink/cryptolink.h +++ b/src/lib/cryptolink/cryptolink.h @@ -1,4 +1,4 @@ -// Copyright (C) 2011 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2011, 2014 Internet Systems Consortium, Inc. ("ISC") // // Permission to use, copy, modify, and/or distribute this software for any // purpose with or without fee is hereby granted, provided that the above diff --git a/src/lib/cryptolink/openssl_hmac.cc b/src/lib/cryptolink/openssl_hmac.cc index 81ba92386b..411d86949c 100644 --- a/src/lib/cryptolink/openssl_hmac.cc +++ b/src/lib/cryptolink/openssl_hmac.cc @@ -201,7 +201,7 @@ public: /// See @ref isc::cryptolink::HMAC::verify() for details. bool verify(const void* sig, size_t len) { size_t size = getOutputLength(); - if (len != 0 && len < size / 2) { + if (len != 0 && (len < 10 || len < size / 2)) { return (false); } SecBuf digest(size); diff --git a/src/lib/cryptolink/tests/crypto_unittests.cc b/src/lib/cryptolink/tests/crypto_unittests.cc index 7cc5e8baa5..fa5b429c72 100644 --- a/src/lib/cryptolink/tests/crypto_unittests.cc +++ b/src/lib/cryptolink/tests/crypto_unittests.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2011 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2011, 2014 Internet Systems Consortium, Inc. ("ISC") // // Permission to use, copy, modify, and/or distribute this software for any // purpose with or without fee is hereby granted, provided that the above diff --git a/src/lib/cryptolink/tests/hmac_unittests.cc b/src/lib/cryptolink/tests/hmac_unittests.cc index aef63df3db..00d20252c6 100644 --- a/src/lib/cryptolink/tests/hmac_unittests.cc +++ b/src/lib/cryptolink/tests/hmac_unittests.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2011 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2011, 2014 Internet Systems Consortium, Inc. ("ISC") // // Permission to use, copy, modify, and/or distribute this software for any // purpose with or without fee is hereby granted, provided that the above @@ -274,10 +274,8 @@ TEST(HMACTest, HMAC_MD5_RFC2202_SIGN) { 0x4c }; doHMACTest("Test With Truncation", secret5, 16, MD5, hmac_expected5, 16); -#ifndef WITH_BOTAN doHMACTest("Test With Truncation", secret5, 16, MD5, hmac_expected5, 12); -#endif const uint8_t hmac_expected6[] = { 0x6b, 0x1a, 0xb7, 0xfe, 0x4b, 0xd7, 0xbf, 0x8f, 0x0b, 0x62, @@ -306,10 +304,8 @@ TEST(HMACTest, HMAC_MD5_RFC2202_SIGN_TRUNCATED) { 0x4c }; doHMACTest("Test With Truncation", secret5, 16, MD5, hmac_expected5, 16); -#ifndef WITH_BOTAN doHMACTest("Test With Truncation", secret5, 16, MD5, hmac_expected5, 12); -#endif } // @@ -363,10 +359,8 @@ TEST(HMACTest, HMAC_SHA1_RFC2202_SIGN) { 0x32, 0x4a, 0x9a, 0x5a, 0x04 }; doHMACTest("Test With Truncation", secret5, 20, SHA1, hmac_expected5, 20); -#ifndef WITH_BOTAN doHMACTest("Test With Truncation", secret5, 20, SHA1, hmac_expected5, 12); -#endif const uint8_t hmac_expected6[] = { 0xaa, 0x4a, 0xe5, 0xe1, 0x52, 0x72, 0xd0, 0x0e, 0x95, 0x70, @@ -396,10 +390,8 @@ TEST(HMACTest, HMAC_SHA1_RFC2202_SIGN_TRUNCATED) { 0x32, 0x4a, 0x9a, 0x5a, 0x04 }; doHMACTest("Test With Truncation", secret5, 20, SHA1, hmac_expected5, 20); -#ifndef WITH_BOTAN doHMACTest("Test With Truncation", secret5, 20, SHA1, hmac_expected5, 12); -#endif } // @@ -560,11 +552,7 @@ TEST(HMACTest, HMAC_SHA512_RFC4231_SIGN) { doRFC4231Tests(SHA512, hmac_expected_list); } -#ifndef WITH_BOTAN TEST(HMACTest, HMAC_SHA256_RFC2202_SIGN_TRUNCATED) { -#else -TEST(HMACTest, DISABLED_HMAC_SHA256_RFC2202_SIGN_TRUNCATED) { -#endif const uint8_t secret5[] = { 0x0c, 0x0c, 0x0c, 0x0c, 0x0c, 0x0c, 0x0c, 0x0c, 0x0c, 0x0c, 0x0c, 0x0c, 0x0c, 0x0c, 0x0c, 0x0c, 0x0c, 0x0c, diff --git a/src/lib/dns/rdata/any_255/tsig_250.cc b/src/lib/dns/rdata/any_255/tsig_250.cc index 3252cfdb10..1da826de71 100644 --- a/src/lib/dns/rdata/any_255/tsig_250.cc +++ b/src/lib/dns/rdata/any_255/tsig_250.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2010-2013 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2010-2014 Internet Systems Consortium, Inc. ("ISC") // // Permission to use, copy, modify, and/or distribute this software for any // purpose with or without fee is hereby granted, provided that the above @@ -129,6 +129,14 @@ TSIG::constructFromLexer(MasterLexer& lexer, const Name* origin) { error = TSIGError::BAD_KEY_CODE; } else if (error_txt == "BADTIME") { error = TSIGError::BAD_TIME_CODE; + } else if (error_txt == "BADMODE") { + error = TSIGError::BAD_MODE_CODE; + } else if (error_txt == "BADNAME") { + error = TSIGError::BAD_NAME_CODE; + } else if (error_txt == "BADALG") { + error = TSIGError::BAD_ALG_CODE; + } else if (error_txt == "BADTRUNC") { + error = TSIGError::BAD_TRUNC_CODE; } else { /// we cast to uint32_t and range-check, because casting directly to /// uint16_t will convert negative numbers to large positive numbers diff --git a/src/lib/dns/rdataclass.cc b/src/lib/dns/rdataclass.cc index 53ef6a8c02..9e0390f781 100644 --- a/src/lib/dns/rdataclass.cc +++ b/src/lib/dns/rdataclass.cc @@ -5,7 +5,7 @@ /////////////// /////////////// -// Copyright (C) 2010-2013 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2010-2014 Internet Systems Consortium, Inc. ("ISC") // // Permission to use, copy, modify, and/or distribute this software for any // purpose with or without fee is hereby granted, provided that the above @@ -138,6 +138,14 @@ TSIG::constructFromLexer(MasterLexer& lexer, const Name* origin) { error = TSIGError::BAD_KEY_CODE; } else if (error_txt == "BADTIME") { error = TSIGError::BAD_TIME_CODE; + } else if (error_txt == "BADMODE") { + error = TSIGError::BAD_MODE_CODE; + } else if (error_txt == "BADNAME") { + error = TSIGError::BAD_NAME_CODE; + } else if (error_txt == "BADALG") { + error = TSIGError::BAD_ALG_CODE; + } else if (error_txt == "BADTRUNC") { + error = TSIGError::BAD_TRUNC_CODE; } else { /// we cast to uint32_t and range-check, because casting directly to /// uint16_t will convert negative numbers to large positive numbers diff --git a/src/lib/dns/tests/testdata/Makefile.am b/src/lib/dns/tests/testdata/Makefile.am index 0920e10a0a..17f9d9e1fb 100644 --- a/src/lib/dns/tests/testdata/Makefile.am +++ b/src/lib/dns/tests/testdata/Makefile.am @@ -112,7 +112,7 @@ EXTRA_DIST += tsigrecord_toWire1.spec tsigrecord_toWire2.spec EXTRA_DIST += tsig_verify1.spec tsig_verify2.spec tsig_verify3.spec EXTRA_DIST += tsig_verify4.spec tsig_verify5.spec tsig_verify6.spec EXTRA_DIST += tsig_verify7.spec tsig_verify8.spec tsig_verify9.spec -EXTRA_DIST += tsig_verify10.spec +EXTRA_DIST += tsig_verify10.spec tsig_verify11.spec EXTRA_DIST += example.org EXTRA_DIST += broken.zone EXTRA_DIST += origincheck.txt @@ -193,7 +193,7 @@ EXTRA_DIST += tsigrecord_toWire1.wire tsigrecord_toWire2.wire EXTRA_DIST += tsig_verify1.wire tsig_verify2.wire tsig_verify3.wire EXTRA_DIST += tsig_verify4.wire tsig_verify5.wire tsig_verify6.wire EXTRA_DIST += tsig_verify7.wire tsig_verify8.wire tsig_verify9.wire -EXTRA_DIST += tsig_verify10.wire +EXTRA_DIST += tsig_verify10.wire tsig_verify11.wire # We no longer use gen_wiredata.py during build process, so the # dependency is no longer needed. However, we'll keep this dependency diff --git a/src/lib/dns/tests/testdata/tsig_verify11.spec b/src/lib/dns/tests/testdata/tsig_verify11.spec new file mode 100644 index 0000000000..9927b48f64 --- /dev/null +++ b/src/lib/dns/tests/testdata/tsig_verify11.spec @@ -0,0 +1,24 @@ +# +# A simple DNS query message with TSIG signed with truncated MAC +# using common HMAC-SHA512-256 +# + +[custom] +sections: header:question:tsig +[header] +id: 0x2d65 +rd: 1 +arcount: 1 +[question] +name: www.example.com +[tsig] +as_rr: True +# TSIG QNAME won't be compressed +rr_name: www.example.com +algorithm: hmac-sha512 +time_signed: 0x4da8877a +#mac_size: 64 +mac_size: 32 +#mac: 0xc4bc4053572d62dd1b26998111565c18056be773dedc6ecea60dff31db2f25966e5d9bafbaaed56efbd5ee2d7e2a12ede4caad630ddf69846c980409724da34e +mac: 0xc4bc4053572d62dd1b26998111565c18056be773dedc6ecea60dff31db2f2596 +original_id: 0x2d65 diff --git a/src/lib/dns/tests/testdata/tsig_verify11.wire b/src/lib/dns/tests/testdata/tsig_verify11.wire new file mode 100644 index 0000000000..f0a8f4fa9d --- /dev/null +++ b/src/lib/dns/tests/testdata/tsig_verify11.wire @@ -0,0 +1,24 @@ +### +### This data file was auto-generated from tsig_verify11.spec +### + +# Header Section +# ID=11621 QR=Query Opcode=QUERY(0) Rcode=NOERROR(0) RD +2d65 0100 +# QDCNT=1, ANCNT=0, NSCNT=0, ARCNT=1 +0001 0000 0000 0001 + +# Question Section +# QNAME=www.example.com QTYPE=A(1) QCLASS=IN(1) +03777777076578616d706c6503636f6d00 0001 0001 + +# TSIG RR (QNAME=www.example.com Class=ANY(255) TTL=0, RDLEN=61) +03777777076578616d706c6503636f6d00 00fa 00ff 00000000 003d +# Algorithm=hmac-sha512 Time-Signed=1302890362 Fudge=300 +0b686d61632d73686135313200 00004da8877a 012c +# MAC Size=32 MAC=(see hex) +0020 c4bc4053572d62dd1b26998111565c18056be773dedc6ecea60dff31db2f2596 +# Original-ID=11621 Error=0 +2d65 0000 +# Other-Len=0 Other-Data=(see hex) +0000 diff --git a/src/lib/dns/tests/tsig_unittest.cc b/src/lib/dns/tests/tsig_unittest.cc index 6348e9cbb5..a23ec5a291 100644 --- a/src/lib/dns/tests/tsig_unittest.cc +++ b/src/lib/dns/tests/tsig_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2011 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2011, 2014 Internet Systems Consortium, Inc. ("ISC") // // Permission to use, copy, modify, and/or distribute this software for any // purpose with or without fee is hereby granted, provided that the above @@ -951,8 +951,6 @@ TEST_F(TSIGTest, signAfterVerified) { TEST_F(TSIGTest, tooShortMAC) { // Too short MAC should be rejected. - // Note: when we implement RFC4635-based checks, the error code will - // (probably) be FORMERR. isc::util::detail::gettimeFunction = testGetTime<0x4da8877a>; createMessageFromFile("tsig_verify10.wire"); @@ -960,7 +958,35 @@ TEST_F(TSIGTest, tooShortMAC) { SCOPED_TRACE("Verify test for request"); commonVerifyChecks(*tsig_verify_ctx, message.getTSIGRecord(), &received_data[0], received_data.size(), - TSIGError::BAD_SIG(), TSIGContext::RECEIVED_REQUEST); + TSIGError::FORMERR(), TSIGContext::RECEIVED_REQUEST); + } +} + +TEST_F(TSIGTest, truncatedMAC) { + // Check truncated MAC support with HMAC-SHA512-256 + isc::util::detail::gettimeFunction = testGetTime<0x4da8877a>; + + secret.clear(); + decodeBase64("jI/Pa4qRu96t76Pns5Z/Ndxbn3QCkwcxLOgt9vgvnJw5wqTRvNyk3FtD6yIMd1dWVlqZ+Y4fe6Uasc0ckctEmg==", secret); + TSIGContext sha_ctx(TSIGKey(test_name, TSIGKey::HMACSHA512_NAME(), + &secret[0], secret.size(), 256)); + + createMessageFromFile("tsig_verify11.wire"); + { + SCOPED_TRACE("Verify test for request"); + commonVerifyChecks(sha_ctx, message.getTSIGRecord(), + &received_data[0], received_data.size(), + TSIGError::NOERROR(), TSIGContext::RECEIVED_REQUEST); + } + + // Try with HMAC-SHA512-264 (should fail) + TSIGContext bad_sha_ctx(TSIGKey(test_name, TSIGKey::HMACSHA512_NAME(), + &secret[0], secret.size(), 264)); + { + SCOPED_TRACE("Verify test for request"); + commonVerifyChecks(bad_sha_ctx, message.getTSIGRecord(), + &received_data[0], received_data.size(), + TSIGError::BAD_TRUNC(), TSIGContext::RECEIVED_REQUEST); } } @@ -973,6 +999,12 @@ TEST_F(TSIGTest, getTSIGLength) { // hmac-md5.sig-alg.reg.int.: n2=26, x=16 EXPECT_EQ(85, tsig_ctx->getTSIGLength()); + // hmac-md5-80: n2=26, x=10 + tsig_ctx.reset(new TestTSIGContext(TSIGKey(test_name, + TSIGKey::HMACMD5_NAME(), + &dummy_data[0], 10, 80))); + EXPECT_EQ(79, tsig_ctx->getTSIGLength()); + // hmac-sha1: n2=11, x=20 tsig_ctx.reset(new TestTSIGContext(TSIGKey(test_name, TSIGKey::HMACSHA1_NAME(), @@ -1003,6 +1035,12 @@ TEST_F(TSIGTest, getTSIGLength) { &dummy_data[0], 64))); EXPECT_EQ(120, tsig_ctx->getTSIGLength()); + // hmac-sha512-256: n2=13, x=32 + tsig_ctx.reset(new TestTSIGContext(TSIGKey(test_name, + TSIGKey::HMACSHA512_NAME(), + &dummy_data[0], 32, 256))); + EXPECT_EQ(88, tsig_ctx->getTSIGLength()); + // bad key case: n1=len(badkey.example.com)=20, n2=26, x=0 tsig_ctx.reset(new TestTSIGContext(badkey_name, TSIGKey::HMACMD5_NAME(), keyring)); diff --git a/src/lib/dns/tests/tsigerror_unittest.cc b/src/lib/dns/tests/tsigerror_unittest.cc index bb08aef9b1..1b00f9ff45 100644 --- a/src/lib/dns/tests/tsigerror_unittest.cc +++ b/src/lib/dns/tests/tsigerror_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2011 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2011, 2014 Internet Systems Consortium, Inc. ("ISC") // // Permission to use, copy, modify, and/or distribute this software for any // purpose with or without fee is hereby granted, provided that the above @@ -51,6 +51,10 @@ TEST(TSIGErrorTest, constants) { EXPECT_EQ(TSIGError::BAD_SIG_CODE, TSIGError(16).getCode()); EXPECT_EQ(TSIGError::BAD_KEY_CODE, TSIGError(17).getCode()); EXPECT_EQ(TSIGError::BAD_TIME_CODE, TSIGError(18).getCode()); + EXPECT_EQ(TSIGError::BAD_MODE_CODE, TSIGError(19).getCode()); + EXPECT_EQ(TSIGError::BAD_NAME_CODE, TSIGError(20).getCode()); + EXPECT_EQ(TSIGError::BAD_ALG_CODE, TSIGError(21).getCode()); + EXPECT_EQ(TSIGError::BAD_TRUNC_CODE, TSIGError(22).getCode()); EXPECT_EQ(0, TSIGError::NOERROR().getCode()); EXPECT_EQ(9, TSIGError::NOTAUTH().getCode()); @@ -58,6 +62,10 @@ TEST(TSIGErrorTest, constants) { EXPECT_EQ(TSIGError::BAD_SIG_CODE, TSIGError::BAD_SIG().getCode()); EXPECT_EQ(TSIGError::BAD_KEY_CODE, TSIGError::BAD_KEY().getCode()); EXPECT_EQ(TSIGError::BAD_TIME_CODE, TSIGError::BAD_TIME().getCode()); + EXPECT_EQ(TSIGError::BAD_MODE_CODE, TSIGError::BAD_MODE().getCode()); + EXPECT_EQ(TSIGError::BAD_NAME_CODE, TSIGError::BAD_NAME().getCode()); + EXPECT_EQ(TSIGError::BAD_ALG_CODE, TSIGError::BAD_ALG().getCode()); + EXPECT_EQ(TSIGError::BAD_TRUNC_CODE, TSIGError::BAD_TRUNC().getCode()); } TEST(TSIGErrorTest, equal) { @@ -87,9 +95,13 @@ TEST(TSIGErrorTest, toText) { EXPECT_EQ("BADSIG", TSIGError::BAD_SIG().toText()); EXPECT_EQ("BADKEY", TSIGError::BAD_KEY().toText()); EXPECT_EQ("BADTIME", TSIGError::BAD_TIME().toText()); + EXPECT_EQ("BADMODE", TSIGError::BAD_MODE().toText()); + EXPECT_EQ("BADNAME", TSIGError::BAD_NAME().toText()); + EXPECT_EQ("BADALG", TSIGError::BAD_ALG().toText()); + EXPECT_EQ("BADTRUNC", TSIGError::BAD_TRUNC().toText()); // Unknown (or not yet supported) codes. Simply converted as numeric. - EXPECT_EQ("19", TSIGError(19).toText()); + EXPECT_EQ("23", TSIGError(23).toText()); EXPECT_EQ("65535", TSIGError(65535).toText()); } @@ -101,9 +113,13 @@ TEST(TSIGErrorTest, toRcode) { EXPECT_EQ(Rcode::NOTAUTH(), TSIGError::BAD_SIG().toRcode()); EXPECT_EQ(Rcode::NOTAUTH(), TSIGError::BAD_KEY().toRcode()); EXPECT_EQ(Rcode::NOTAUTH(), TSIGError::BAD_TIME().toRcode()); + EXPECT_EQ(Rcode::NOTAUTH(), TSIGError::BAD_MODE().toRcode()); + EXPECT_EQ(Rcode::NOTAUTH(), TSIGError::BAD_NAME().toRcode()); + EXPECT_EQ(Rcode::NOTAUTH(), TSIGError::BAD_ALG().toRcode()); + EXPECT_EQ(Rcode::NOTAUTH(), TSIGError::BAD_TRUNC().toRcode()); // Unknown (or not yet supported) codes are treated as SERVFAIL. - EXPECT_EQ(Rcode::SERVFAIL(), TSIGError(19).toRcode()); + EXPECT_EQ(Rcode::SERVFAIL(), TSIGError(23).toRcode()); EXPECT_EQ(Rcode::SERVFAIL(), TSIGError(65535).toRcode()); } diff --git a/src/lib/dns/tests/tsigkey_unittest.cc b/src/lib/dns/tests/tsigkey_unittest.cc index 8a9d86f73a..f06a67981c 100644 --- a/src/lib/dns/tests/tsigkey_unittest.cc +++ b/src/lib/dns/tests/tsigkey_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2010 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2010, 2014 Internet Systems Consortium, Inc. ("ISC") // // Permission to use, copy, modify, and/or distribute this software for any // purpose with or without fee is hereby granted, provided that the above @@ -105,6 +105,12 @@ TEST_F(TSIGKeyTest, construct) { secret.c_str(), secret.size()).getKeyName().toText()); + // Check digestbits + EXPECT_EQ(key.getDigestbits(), 0); + TSIGKey key_trunc(key_name, TSIGKey::HMACMD5_NAME(), + secret.c_str(), secret.size(), 120); + EXPECT_EQ(key_trunc.getDigestbits(), 120); + // Invalid combinations of secret and secret_len: EXPECT_THROW(TSIGKey(key_name, TSIGKey::HMACSHA1_NAME(), secret.c_str(), 0), isc::InvalidParameter); @@ -116,13 +122,14 @@ void compareTSIGKeys(const TSIGKey& expect, const TSIGKey& actual) { EXPECT_EQ(expect.getKeyName(), actual.getKeyName()); EXPECT_EQ(expect.getAlgorithmName(), actual.getAlgorithmName()); + EXPECT_EQ(expect.getDigestbits(), actual.getDigestbits()); matchWireData(expect.getSecret(), expect.getSecretLength(), actual.getSecret(), actual.getSecretLength()); } TEST_F(TSIGKeyTest, copyConstruct) { const TSIGKey original(key_name, TSIGKey::HMACSHA256_NAME(), - secret.c_str(), secret.size()); + secret.c_str(), secret.size(), 128); const TSIGKey copy(original); compareTSIGKeys(original, copy); @@ -135,7 +142,7 @@ TEST_F(TSIGKeyTest, copyConstruct) { TEST_F(TSIGKeyTest, assignment) { const TSIGKey original(key_name, TSIGKey::HMACSHA256_NAME(), - secret.c_str(), secret.size()); + secret.c_str(), secret.size(), 200); TSIGKey copy = original; compareTSIGKeys(original, copy); @@ -306,9 +313,10 @@ TEST(TSIGStringTest, TSIGKeyFromToString) { TSIGKey k1 = TSIGKey("test.example:MSG6Ng==:hmac-md5.sig-alg.reg.int"); 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); + TSIGKey k4 = TSIGKey("test.example.:MSG6Ng==:hmac-md5.sig-alg.reg.int.:120"); + TSIGKey k5 = TSIGKey(Name("test.example."), Name("hmac-sha1."), NULL, 0); // "Unknown" key with empty secret is okay - TSIGKey k5 = TSIGKey("test.example.::unknown"); + TSIGKey k6 = TSIGKey("test.example.::unknown"); EXPECT_EQ("test.example.:MSG6Ng==:hmac-md5.sig-alg.reg.int.", k1.toText()); @@ -316,9 +324,12 @@ TEST(TSIGStringTest, TSIGKeyFromToString) { k2.toText()); 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_EQ("test.example.:MSG6Ng==:hmac-md5.sig-alg.reg.int.:120", + k4.toText()); + EXPECT_EQ(120, k4.getDigestbits()); + EXPECT_EQ("test.example.::hmac-sha1.", k5.toText()); + EXPECT_EQ(Name("test.example."), k6.getKeyName()); + EXPECT_EQ(Name("unknown"), k6.getAlgorithmName()); EXPECT_THROW(TSIGKey(""), isc::InvalidParameter); EXPECT_THROW(TSIGKey(":"), isc::InvalidParameter); @@ -329,6 +340,10 @@ 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); + EXPECT_THROW(TSIGKey("test.example.:MSG6Ng==:hmac-md5.sig-alg.reg.int.:"), + isc::InvalidParameter); + EXPECT_THROW(TSIGKey("test.example.:MSG6Ng==:hmac-md5.sig-alg.reg.int.:xxx"), + isc::InvalidParameter); } diff --git a/src/lib/dns/tsig.cc b/src/lib/dns/tsig.cc index b6e1716d67..ceb39f4bfa 100644 --- a/src/lib/dns/tsig.cc +++ b/src/lib/dns/tsig.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2011 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2011, 2014 Internet Systems Consortium, Inc. ("ISC") // // Permission to use, copy, modify, and/or distribute this software for any // purpose with or without fee is hereby granted, provided that the above @@ -82,7 +82,20 @@ struct TSIGContext::TSIGContextImpl { } catch (const isc::Exception&) { return; } - digest_len_ = hmac_->getOutputLength(); + size_t digestbits = key_.getDigestbits(); + size_t default_digest_len = hmac_->getOutputLength(); + if (digestbits > 0) { + digest_len_ = (digestbits + 7) / 8; + // sanity (cf. RFC 4635) + if ((digest_len_ < 10) || + (digest_len_ < (default_digest_len / 2)) || + (digest_len_ > default_digest_len)) { + // should emit a warning? + digest_len_ = default_digest_len; + } + } else { + digest_len_ = default_digest_len; + } } } @@ -402,7 +415,7 @@ TSIGContext::sign(const uint16_t qid, const void* const data, impl_->state_ == SENT_RESPONSE); // Get the final digest, update internal state, then finish. - vector digest = hmac->sign(); + vector digest = hmac->sign(impl_->digest_len_); assert(digest.size() <= 0xffff); // cryptolink API should have ensured it. ConstTSIGRecordPtr tsig(new TSIGRecord( impl_->key_.getKeyName(), @@ -490,9 +503,6 @@ TSIGContext::verify(const TSIGRecord* const record, const void* const data, digest_len)); } - // TODO: signature length check based on RFC4635 - // (Right now we enforce the standard signature length in libcryptolink) - // Handling empty MAC. While RFC2845 doesn't explicitly prohibit other // cases, it can only reasonably happen in a response with BADSIG or // BADKEY. We reject other cases as if it were BADSIG to avoid unexpected @@ -514,6 +524,21 @@ TSIGContext::verify(const TSIGRecord* const record, const void* const data, impl_->digestPreviousMAC(hmac); } + // Signature length check based on RFC 4635 3.1 + if (tsig_rdata.getMACSize() > hmac->getOutputLength()) { + // signature length too big + return (impl_->postVerifyUpdate(TSIGError::FORMERR(), NULL, 0)); + } + if ((tsig_rdata.getMACSize() < 10) || + (tsig_rdata.getMACSize() < (hmac->getOutputLength() / 2))) { + // signature length below minimum + return (impl_->postVerifyUpdate(TSIGError::FORMERR(), NULL, 0)); + } + if (tsig_rdata.getMACSize() < impl_->digest_len_) { + // (truncated) signature length too small + return (impl_->postVerifyUpdate(TSIGError::BAD_TRUNC(), NULL, 0)); + } + // // Digest DNS message (excluding the trailing TSIG RR and adjusting the // QID and ARCOUNT header fields) diff --git a/src/lib/dns/tsig.h b/src/lib/dns/tsig.h index 1f50a01a3e..a2cd22704b 100644 --- a/src/lib/dns/tsig.h +++ b/src/lib/dns/tsig.h @@ -1,4 +1,4 @@ -// Copyright (C) 2011 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2011, 2014 Internet Systems Consortium, Inc. ("ISC") // // Permission to use, copy, modify, and/or distribute this software for any // purpose with or without fee is hereby granted, provided that the above @@ -288,6 +288,11 @@ public: /// - \c BAD_SIG: The signature given in the TSIG doesn't match against /// the locally computed digest or is the signature is /// invalid in other way. + /// - \c BAD_MODE: Not yet implemented TKEY error + /// - \c BAD_NAME: Not yet implemented TKEY error + /// - \c BAD_ALG: Not yet implemented TKEY error + /// - \c BAD_TRUNC: The signature or truncated signature length is too + /// small. /// /// If this method is called by a DNS client waiting for a signed /// response and the result is not \c NOERROR, the context can be used diff --git a/src/lib/dns/tsigerror.cc b/src/lib/dns/tsigerror.cc index 36ef47da10..c8524f2b0f 100644 --- a/src/lib/dns/tsigerror.cc +++ b/src/lib/dns/tsigerror.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2011 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2011, 2014 Internet Systems Consortium, Inc. ("ISC") // // Permission to use, copy, modify, and/or distribute this software for any // purpose with or without fee is hereby granted, provided that the above @@ -28,7 +28,11 @@ namespace { const char* const tsigerror_text[] = { "BADSIG", "BADKEY", - "BADTIME" + "BADTIME", + "BADMODE", + "BADNAME", + "BADALG", + "BADTRUNC" }; } @@ -42,7 +46,7 @@ std::string TSIGError::toText() const { if (code_ <= MAX_RCODE_FOR_TSIGERROR) { return (Rcode(code_).toText()); - } else if (code_ <= BAD_TIME_CODE) { + } else if (code_ <= BAD_TRUNC_CODE) { return (tsigerror_text[code_ - (MAX_RCODE_FOR_TSIGERROR + 1)]); } else { return (boost::lexical_cast(code_)); @@ -54,7 +58,7 @@ TSIGError::toRcode() const { if (code_ <= MAX_RCODE_FOR_TSIGERROR) { return (Rcode(code_)); } - if (code_ > BAD_TIME_CODE) { + if (code_ > BAD_TRUNC_CODE) { return (Rcode::SERVFAIL()); } return (Rcode::NOTAUTH()); diff --git a/src/lib/dns/tsigerror.h b/src/lib/dns/tsigerror.h index 5b8056d868..2ef37a0817 100644 --- a/src/lib/dns/tsigerror.h +++ b/src/lib/dns/tsigerror.h @@ -1,4 +1,4 @@ -// Copyright (C) 2011 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2011, 2014 Internet Systems Consortium, Inc. ("ISC") // // Permission to use, copy, modify, and/or distribute this software for any // purpose with or without fee is hereby granted, provided that the above @@ -26,7 +26,7 @@ namespace dns { /// /// The \c TSIGError class objects represent standard errors related to /// TSIG protocol operations as defined in related specifications, mainly -/// in RFC2845. +/// in RFC2845, RFC2930 and RFC4635. class TSIGError { public: /// Constants for pre-defined TSIG error values. @@ -38,9 +38,13 @@ public: /// header file. To avoid conflict with it we add an underscore to our /// definitions. enum CodeValue { - BAD_SIG_CODE = 16, ///< 16: TSIG verification failure - BAD_KEY_CODE = 17, ///< 17: TSIG key is not recognized - BAD_TIME_CODE = 18 ///< 18: Current time and time signed are too different + BAD_SIG_CODE = 16, ///< 16: TSIG verification failure + BAD_KEY_CODE = 17, ///< 17: TSIG key is not recognized + BAD_TIME_CODE = 18, ///< 18: Current time and time signed are too different + BAD_MODE_CODE = 19, ///< 19: Bad TKEY mode + BAD_NAME_CODE = 20, ///< 20: Duplicate TKEY name + BAD_ALG_CODE = 21, ///< 21: TKEY algorithm not supported + BAD_TRUNC_CODE = 22 ///< 22: Bad truncation }; /// \name Constructors @@ -195,6 +199,22 @@ public: /// (see \c TSIGError::BAD_TIME_CODE). static const TSIGError& BAD_TIME(); + /// A constant TSIG error object for the BADMODE code + /// (see \c TSIGError::BAD_MODE_CODE). + static const TSIGError& BAD_MODE(); + + /// A constant TSIG error object for the BADNAME code + /// (see \c TSIGError::BAD_NAME_CODE). + static const TSIGError& BAD_NAME(); + + /// A constant TSIG error object for the BADALG code + /// (see \c TSIGError::BAD_ALG_CODE). + static const TSIGError& BAD_ALG(); + + /// A constant TSIG error object for the BADTRUNC code + /// (see \c TSIGError::BAD_TRUNC_CODE). + static const TSIGError& BAD_TRUNC(); + private: // This is internally used to specify the maximum possible RCODE value // that can be convertible to TSIGErrors. @@ -317,6 +337,30 @@ TSIGError::BAD_TIME() { return (e); } +inline const TSIGError& +TSIGError::BAD_MODE() { + static TSIGError e(BAD_MODE_CODE); + return (e); +} + +inline const TSIGError& +TSIGError::BAD_NAME() { + static TSIGError e(BAD_NAME_CODE); + return (e); +} + +inline const TSIGError& +TSIGError::BAD_ALG() { + static TSIGError e(BAD_ALG_CODE); + return (e); +} + +inline const TSIGError& +TSIGError::BAD_TRUNC() { + static TSIGError e(BAD_TRUNC_CODE); + return (e); +} + /// Insert the \c TSIGError as a string into stream. /// /// This method convert \c tsig_error into a string and inserts it into the diff --git a/src/lib/dns/tsigkey.cc b/src/lib/dns/tsigkey.cc index 862d3005eb..fd055e4233 100644 --- a/src/lib/dns/tsigkey.cc +++ b/src/lib/dns/tsigkey.cc @@ -1,4 +1,4 @@ -// Copyright (C) 2010 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2010, 2014 Internet Systems Consortium, Inc. ("ISC") // // Permission to use, copy, modify, and/or distribute this software for any // purpose with or without fee is hereby granted, provided that the above @@ -25,6 +25,8 @@ #include #include +#include + using namespace std; using namespace isc::cryptolink; @@ -63,9 +65,11 @@ struct TSIGKey::TSIGKeyImpl { TSIGKeyImpl(const Name& key_name, const Name& algorithm_name, isc::cryptolink::HashAlgorithm algorithm, + size_t digestbits, const void* secret, size_t secret_len) : + key_name_(key_name), algorithm_name_(algorithm_name), - algorithm_(algorithm), + algorithm_(algorithm), digestbits_(digestbits), secret_(static_cast(secret), static_cast(secret) + secret_len) { @@ -79,11 +83,13 @@ TSIGKey::TSIGKeyImpl { Name key_name_; Name algorithm_name_; const isc::cryptolink::HashAlgorithm algorithm_; + size_t digestbits_; const vector secret_; }; TSIGKey::TSIGKey(const Name& key_name, const Name& algorithm_name, - const void* secret, size_t secret_len) : impl_(NULL) + const void* secret, size_t secret_len, + size_t digestbits /*= 0*/) : impl_(NULL) { const HashAlgorithm algorithm = convertAlgorithmName(algorithm_name); if ((secret != NULL && secret_len == 0) || @@ -97,8 +103,8 @@ TSIGKey::TSIGKey(const Name& key_name, const Name& algorithm_name, "TSIGKey with unknown algorithm has non empty secret: " << key_name << ":" << algorithm_name); } - impl_ = new TSIGKeyImpl(key_name, algorithm_name, algorithm, secret, - secret_len); + impl_ = new TSIGKeyImpl(key_name, algorithm_name, algorithm, + digestbits, secret, secret_len); } TSIGKey::TSIGKey(const std::string& str) : impl_(NULL) { @@ -119,7 +125,15 @@ TSIGKey::TSIGKey(const std::string& str) : impl_(NULL) { string algo_str; if (!iss.eof()) { - getline(iss, algo_str); + getline(iss, algo_str, ':'); + } + if (iss.fail() || iss.bad()) { + isc_throw(InvalidParameter, "Invalid TSIG key string: " << str); + } + + string dgstbt_str; + if (!iss.eof()) { + getline(iss, dgstbt_str); } if (iss.fail() || iss.bad()) { isc_throw(InvalidParameter, "Invalid TSIG key string: " << str); @@ -128,6 +142,15 @@ TSIGKey::TSIGKey(const std::string& str) : impl_(NULL) { const Name algo_name(algo_str.empty() ? "hmac-md5.sig-alg.reg.int" : algo_str); const HashAlgorithm algorithm = convertAlgorithmName(algo_name); + size_t digestbits = 0; + try { + if (!dgstbt_str.empty()) { + digestbits = boost::lexical_cast(dgstbt_str); + } + } catch (const boost::bad_lexical_cast&) { + isc_throw(InvalidParameter, + "TSIG key with non-numeric digestbits: " << dgstbt_str); + } vector secret; isc::util::encode::decodeBase64(secret_str, secret); @@ -139,6 +162,7 @@ TSIGKey::TSIGKey(const std::string& str) : impl_(NULL) { } impl_ = new TSIGKeyImpl(Name(keyname_str), algo_name, algorithm, + digestbits, secret.empty() ? NULL : &secret[0], secret.size()); } catch (const isc::Exception& e) { @@ -184,6 +208,11 @@ TSIGKey::getAlgorithm() const { return (impl_->algorithm_); } +size_t +TSIGKey::getDigestbits() const { + return (impl_->digestbits_); +} + const void* TSIGKey::getSecret() const { return ((impl_->secret_.size() > 0) ? &impl_->secret_[0] : NULL); @@ -196,13 +225,20 @@ TSIGKey::getSecretLength() const { std::string TSIGKey::toText() const { + size_t digestbits = getDigestbits(); const vector secret_v(static_cast(getSecret()), static_cast(getSecret()) + getSecretLength()); std::string secret_str = isc::util::encode::encodeBase64(secret_v); - return (getKeyName().toText() + ":" + secret_str + ":" + - getAlgorithmName().toText()); + if (digestbits) { + std::string dgstbt_str = boost::lexical_cast(static_cast(digestbits)); + return (getKeyName().toText() + ":" + secret_str + ":" + + getAlgorithmName().toText() + ":" + dgstbt_str); + } else { + return (getKeyName().toText() + ":" + secret_str + ":" + + getAlgorithmName().toText()); + } } const diff --git a/src/lib/dns/tsigkey.h b/src/lib/dns/tsigkey.h index e623be9c94..fe4b4f471d 100644 --- a/src/lib/dns/tsigkey.h +++ b/src/lib/dns/tsigkey.h @@ -1,4 +1,4 @@ -// Copyright (C) 2010 Internet Systems Consortium, Inc. ("ISC") +// Copyright (C) 2010, 2014 Internet Systems Consortium, Inc. ("ISC") // // Permission to use, copy, modify, and/or distribute this software for any // purpose with or without fee is hereby granted, provided that the above @@ -30,6 +30,7 @@ class Name; /// A TSIG key consists of the following attributes: /// - Key name /// - Hash algorithm +/// - Digest bits /// - Shared secret /// /// Implementation Notes @@ -97,6 +98,12 @@ public: /// is 0 if and only if the former is \c NULL; /// otherwise an exception of type \c InvalidParameter will be thrown. /// + /// \c digestbits is the truncated length in bits or 0 which means no + /// truncation and is the default. Constraints for non-zero value + /// are in RFC 4635 section 3.1: minimum 80 or the half of the + /// full (i.e., not truncated) length, integral number of octets + /// (i.e., multiple of 8), and maximum the full length. + /// /// This constructor internally involves resource allocation, and if /// it fails, a corresponding standard exception will be thrown. /// @@ -108,16 +115,18 @@ public: /// used for this key, or \c NULL if the secret is empty. /// \param secret_len The size of the binary %data (\c secret) in bytes. TSIGKey(const Name& key_name, const Name& algorithm_name, - const void* secret, size_t secret_len); + const void* secret, size_t secret_len, size_t digestbits = 0); /// \brief Constructor from an input string /// /// The string must be of the form: - /// name:secret[:algorithm] + /// name:secret[:algorithm][:digestbits] /// 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 RFC 4635. /// The default algorithm is hmac-md5.sig-alg.reg.int. + /// "digestbits" is the minimum truncated length in bits. + /// The default digestbits value is 0 and means truncation is forbidden. /// /// The same restriction about the algorithm name (and secret) as that /// for the other constructor applies. @@ -168,6 +177,9 @@ public: /// Return the hash algorithm name in the form of cryptolink::HashAlgorithm isc::cryptolink::HashAlgorithm getAlgorithm() const; + /// Return the minimum truncated length. + size_t getDigestbits() const; + /// Return the length of the TSIG secret in bytes. size_t getSecretLength() const; @@ -187,10 +199,11 @@ public: /// \brief Converts the TSIGKey to a string value /// /// The resulting string will be of the form - /// name:secret:algorithm + /// name:secret:algorithm[:digestbits] /// Where "name" is a domain name for the key, "secret" is a /// base64 representation of the key secret, and "algorithm" is /// an algorithm identifier as specified in RFC 4635. + /// When not zero, digestbits is appended. /// /// \return The string representation of the given TSIGKey. std::string toText() const;