Browse Source

[trac3593] added truncated HMAC support to TSIG

Francis Dupont 10 years ago
parent
commit
ae3a9cd1a0

+ 4 - 0
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.

+ 6 - 0
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=="
+        }
     ]
 }
 

+ 3 - 0
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" : "<shared secret value>"
         }
 #       ,

+ 11 - 0
doc/guide/ddns.xml

@@ -285,6 +285,17 @@ corresponding values in the DHCP servers' "dhcp-ddns" configuration section.
 	  </listitem>
 	  <listitem>
 	    <simpara>
+	      <command>digest_bits</command> -
+	      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.
+	    </simpara>
+	  </listitem>
+	  <listitem>
+	    <simpara>
 	      <command>secret</command> -
 	      is used to specify the shared secret key code for this key.  This value is
 	      case sensitive and must exactly match the value specified on the DNS server(s).

+ 53 - 4
src/bin/d2/d2_config.cc

@@ -20,6 +20,7 @@
 
 #include <boost/foreach.hpp>
 #include <boost/lexical_cast.hpp>
+#include <boost/scoped_ptr.hpp>
 #include <boost/algorithm/string/predicate.hpp>
 
 #include <sstream>
@@ -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<std::string, isc::data::Element::Position> 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: "

+ 21 - 7
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.

+ 6 - 0
src/bin/d2/dhcp-ddns.spec

@@ -59,6 +59,12 @@
                 "item_default": ""
             },
             {
+                "item_name": "digest_bits",
+                "item_type": "integer",
+                "item_optional": true,
+                "item_default": 0
+            },
+            {
                 "item_name": "secret",
                 "item_type": "string",
                 "item_optional": false,

+ 24 - 8
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==\" "
                         "}"
                         "],"

+ 189 - 0
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",

+ 5 - 10
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<Botan::byte> 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<const unsigned char*>(sig),

+ 1 - 1
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

+ 1 - 1
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

+ 1 - 1
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

+ 1 - 1
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

+ 1 - 1
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<unsigned char> digest(size);

+ 1 - 1
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

+ 1 - 13
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,

+ 9 - 1
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

+ 9 - 1
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

+ 2 - 2
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

+ 24 - 0
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

+ 24 - 0
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

+ 42 - 4
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));

+ 19 - 3
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());
 }
 

+ 23 - 8
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);
 }
 
 

+ 31 - 6
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<uint8_t> digest = hmac->sign();
+    vector<uint8_t> 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)

+ 6 - 1
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

+ 8 - 4
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<std::string>(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());

+ 49 - 5
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

+ 44 - 8
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 <util/encode/base64.h>
 #include <dns/tsigkey.h>
 
+#include <boost/lexical_cast.hpp>
+
 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<const uint8_t*>(secret),
                 static_cast<const uint8_t*>(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<uint8_t> 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<size_t>(dgstbt_str);
+            }
+        } catch (const boost::bad_lexical_cast&) {
+            isc_throw(InvalidParameter,
+                      "TSIG key with non-numeric digestbits: " << dgstbt_str);
+        }
 
         vector<uint8_t> 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<uint8_t> secret_v(static_cast<const uint8_t*>(getSecret()),
                                    static_cast<const uint8_t*>(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<std::string>(static_cast<int>(digestbits));
+        return (getKeyName().toText() + ":" + secret_str + ":" +
+                getAlgorithmName().toText() + ":" + dgstbt_str);
+    } else {
+        return (getKeyName().toText() + ":" + secret_str + ":" +
+                getAlgorithmName().toText());
+    }
 }
 
 const

+ 17 - 4
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
 ///
 /// <b>Implementation Notes</b>
@@ -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;