[trac781] remove TSIGKey from crypto api

src/lib/crypto/crypto.cc

@@ -14,9 +14,6 @@
 
 #include "crypto.h"
 
-#include <iostream>
-#include <iomanip>
-
 #include <botan/botan.h>
 #include <botan/hmac.h>
 #include <botan/hash.h>
@@ -24,7 +21,6 @@
 
 #include <dns/buffer.h>
 #include <dns/name.h>
-#include <dns/tsigkey.h>
 #include <dns/util/base64.h>
 
 #include <string>
@@ -33,23 +29,25 @@ using namespace std;
 using namespace isc::dns;
 
 namespace {
-Botan::HashFunction*
-getHash(const Name& hash_name) {
-    try {
-        if (hash_name == TSIGKey::HMACMD5_NAME()) {
-            return (Botan::get_hash("MD5"));
-        } else if (hash_name == TSIGKey::HMACSHA1_NAME()) {
-            return (Botan::get_hash("SHA-1"));
-        } else if (hash_name == TSIGKey::HMACSHA256_NAME()) {
-            return (Botan::get_hash("SHA-256"));
-        } else {
-            isc_throw(isc::crypto::UnsupportedAlgorithm,
-                      "Unknown Hash type " + hash_name.toText());
-        }
-    } catch (const Botan::Algorithm_Not_Found) {
-        isc_throw(isc::crypto::UnsupportedAlgorithm,
-                  "Algorithm not supported by boost: " + hash_name.toText());
+const char*
+getBotanHashAlgorithmName(isc::crypto::HMAC::HashAlgorithm algorithm) {
+    switch (algorithm) {
+    case isc::crypto::HMAC::MD5:
+        return "MD5";
+        break;
+    case isc::crypto::HMAC::SHA1:
+        return "SHA-1";
+        break;
+    case isc::crypto::HMAC::SHA256:
+        return "SHA-256";
+        break;
+    case isc::crypto::HMAC::UNKNOWN:
+        return "Unknown";
+        break;
     }
+    // compiler should have prevented us to reach this, since we have
+    // no default. But we need a return value anyway
+    return "Unknown";
 }
 
 // Library needs to have been inited during the entire program
@@ -64,22 +62,31 @@ namespace crypto {
 
 class HMACImpl {
 public:
-    explicit HMACImpl(const TSIGKey& key) {
-        Botan::HashFunction* hash = getHash(key.getAlgorithmName());
+    explicit HMACImpl(const void* secret, size_t secret_len,
+                      const HMAC::HashAlgorithm hash_algorithm) {
+        Botan::HashFunction* hash;
+        try {
+            hash = Botan::get_hash(
+                getBotanHashAlgorithmName(hash_algorithm));
+        } catch (const Botan::Algorithm_Not_Found) {
+            isc_throw(isc::crypto::UnsupportedAlgorithm,
+                      "Unknown hash algorithm: " + hash_algorithm);
+        }
+
         hmac_ = new Botan::HMAC::HMAC(hash);
 
         // Take the 'secret' from the key
         // If the key length is larger than the block size, we hash the
         // key itself first.
         try {
-            if (key.getSecretLength() > hash->HASH_BLOCK_SIZE) {
+            if (secret_len > hash->HASH_BLOCK_SIZE) {
                 Botan::SecureVector<Botan::byte> hashed_key =
-                    hash->process(static_cast<const Botan::byte*>(key.getSecret()),
-                                  key.getSecretLength());
+                    hash->process(static_cast<const Botan::byte*>(secret),
+                                  secret_len);
                 hmac_->set_key(hashed_key.begin(), hashed_key.size());
             } else {
-                hmac_->set_key(static_cast<const Botan::byte*>(key.getSecret()),
-                               key.getSecretLength());
+                hmac_->set_key(static_cast<const Botan::byte*>(secret),
+                               secret_len);
             }
         } catch (const Botan::Invalid_Key_Length& ikl) {
             delete hmac_;
@@ -110,8 +117,10 @@ private:
     Botan::HMAC* hmac_;
 };
 
-HMAC::HMAC(const TSIGKey& key) {
-    impl_ = new HMACImpl(key);
+HMAC::HMAC(const void* secret, size_t secret_length,
+           const HashAlgorithm hash_algorithm)
+{
+    impl_ = new HMACImpl(secret, secret_length, hash_algorithm);
 }
 
 HMAC::~HMAC() {
@@ -134,20 +143,22 @@ HMAC::verify(const void* sig, const size_t len) {
 }
 
 void
-signHMAC(const void* data, size_t data_len, const TSIGKey& key,
+signHMAC(const void* data, size_t data_len, const void* secret,
+         size_t secret_len, const HMAC::HashAlgorithm hash_algorithm,
          isc::dns::OutputBuffer& result)
 {
-    HMAC hmac(key);
+    HMAC hmac(secret, secret_len, hash_algorithm);
     hmac.update(data, data_len);
     hmac.sign(result);
 }
 
 
 bool
-verifyHMAC(const void* data, const size_t data_len, const TSIGKey& key,
+verifyHMAC(const void* data, const size_t data_len, const void* secret,
+           size_t secret_len, const HMAC::HashAlgorithm hash_algorithm,
            const void* sig, const size_t sig_len)
 {
-    HMAC hmac(key);
+    HMAC hmac(secret, secret_len, hash_algorithm);
     hmac.update(data, data_len);
     return (hmac.verify(sig, sig_len));
 }

src/lib/crypto/crypto.h

@@ -14,7 +14,6 @@
 
 #include <string>
 #include <dns/buffer.h>
-#include <dns/tsigkey.h>
 #include <exceptions/exceptions.h>
 
 #include <boost/noncopyable.hpp>
@@ -58,19 +57,29 @@ class HMACImpl;
 ///
 class HMAC : public boost::noncopyable {
 public:
-    /// \brief Constructor from a key
+    enum HashAlgorithm {
+        MD5 = 0,
+        SHA1 = 1,
+        SHA256 = 2,
+        UNKNOWN = 3
+    };
+
+    /// \brief Constructor from a secret and a hash algorithm
     ///
-    /// \exception UnsupportedAlgorithmException if the given key
-    /// is for an algorithm that is not supported by the underlying
-    /// library
-    /// \exception InvalidKeyLength if the given key has a bad length
+    /// \exception UnsupportedAlgorithmException if the given algorithm
+    ///            is unknown or not supported by the underlying library
+    /// \exception InvalidKeyLength if the given key secret_len is bad
     ///
-    /// Notes: if the key is longer than the block size of its
+    /// Notes: if the secret is longer than the block size of its
     /// algorithm, the constructor will run it through the hash
-    /// algorithm, and use the digest as a key for this HMAC operation
+    /// algorithm, and use the digest as the secret for this HMAC
+    /// operation
     ///
-    /// \param key The key to use
-    explicit HMAC(const isc::dns::TSIGKey& key);
+    /// \param secret The secret to sign with
+    /// \param len The length of the secret
+    /// \param hash_algorithm The hash algorithm
+    explicit HMAC(const void* secret, size_t secret_len,
+                  const HashAlgorithm hash_algorithm);
 
     /// \brief Destructor
     ~HMAC();
@@ -106,18 +115,26 @@ private:
 /// creating an HMAC object, feeding it the data, and calculating the
 /// resulting signature.
 ///
-/// \exception UnsupportedAlgorithm if we do not support the given
-/// algorithm.
-/// \exception BadKey if the underlying library cannot handle the
-/// given TSIGKey (for instance if it has a bad length).
+/// \exception UnsupportedAlgorithm if the given algorithm is unknown
+///            or not supported by the underlying library
+/// \exception BadKey if the given key secret_len is bad
+///
+/// Notes: if the secret is longer than the block size of its
+/// algorithm, the constructor will run it through the hash
+/// algorithm, and use the digest as the secret for this HMAC
+/// operation
 ///
 /// \param data The data to sign
 /// \param data_len The length of the data
-/// \param key The TSIGKey to sign with
+/// \param secret The secret to sign with
+/// \param secret_len The length of the secret
+/// \param hash_algorithm The hash algorithm
 /// \param result The signature will be appended to this buffer
 void signHMAC(const void* data,
               const size_t data_len,
-              const isc::dns::TSIGKey& key,
+              const void* secret,
+              size_t secret_len,
+              const HMAC::HashAlgorithm hash_algorithm,
               isc::dns::OutputBuffer& result);
 
 /// \brief Verify an HMAC signature for the given data
@@ -127,10 +144,14 @@ void signHMAC(const void* data,
 /// creating an HMAC object, feeding it the data, and checking the
 /// resulting signature.
 ///
-/// \exception UnsupportedAlgorithm if we do not support the given
-/// algorithm.
-/// \exception BadKey exception if the underlying library cannot
-/// handle the given TSIGKey (for instance if it has a bad length).
+/// \exception UnsupportedAlgorithm if the given algorithm is unknown
+///            or not supported by the underlying library
+/// \exception BadKey if the given key secret_len is bad
+///
+/// Notes: if the secret is longer than the block size of its
+/// algorithm, the constructor will run it through the hash
+/// algorithm, and use the digest as the secret for this HMAC
+/// operation
 ///
 /// \param data The data to verify
 /// \param data_len The length of the data
@@ -139,7 +160,9 @@ void signHMAC(const void* data,
 /// \return True if the signature verifies, false if not
 bool verifyHMAC(const void* data,
                 const size_t data_len,
-                const isc::dns::TSIGKey& key,
+                const void* secret,
+                size_t secret_len,
+                const HMAC::HashAlgorithm hash_algorithm,
                 const void* sig,
                 const size_t sig_len);
 

src/lib/crypto/tests/crypto_unittests.cc

@@ -18,12 +18,27 @@
 #include <crypto/crypto.h>
 #include <dns/buffer.h>
 #include <dns/name.h>
+#include <dns/tsigkey.h>
 #include <exceptions/exceptions.h>
 
 using namespace isc::dns;
 using namespace isc::crypto;
 
 namespace {
+    isc::crypto::HMAC::HashAlgorithm
+    getHashAlgorithm(isc::dns::TSIGKey key) {
+        if (key.getAlgorithmName() == TSIGKey::HMACMD5_NAME()) {
+            return isc::crypto::HMAC::MD5;
+        } else if (key.getAlgorithmName() == TSIGKey::HMACSHA1_NAME()) {
+            return isc::crypto::HMAC::SHA1;
+        } else if (key.getAlgorithmName() == TSIGKey::HMACSHA256_NAME()) {
+            return isc::crypto::HMAC::SHA256;
+        } else {
+            return isc::crypto::HMAC::UNKNOWN;
+        }
+    }
+
+    
     void checkBuffer(const OutputBuffer& buf, uint8_t *data, size_t len) {
         ASSERT_EQ(len, buf.getLength());
         const uint8_t* buf_d = static_cast<const uint8_t*>(buf.getData());
@@ -44,22 +59,27 @@ namespace {
         TSIGKey key(key_str);
 
         // Sign it
-        signHMAC(data_buf.getData(), data_buf.getLength(), key,
-                 hmac_sig);
+        signHMAC(data_buf.getData(), data_buf.getLength(),
+                 key.getSecret(), key.getSecretLength(),
+                 getHashAlgorithm(key), hmac_sig);
 
         // Check if the signature is what we expect
         checkBuffer(hmac_sig, expected_hmac, hmac_len);
 
         // Check whether we can verify it ourselves
         EXPECT_TRUE(verifyHMAC(data_buf.getData(), data_buf.getLength(),
-                               key, hmac_sig.getData(),
+                               key.getSecret(), key.getSecretLength(),
+                               getHashAlgorithm(key),
+                               hmac_sig.getData(),
                                hmac_sig.getLength()));
 
         // Change the sig by flipping the first octet, and check
         // whether verification fails then
         hmac_sig.writeUint8At(~hmac_sig[0], 0);
         EXPECT_FALSE(verifyHMAC(data_buf.getData(), data_buf.getLength(),
-                               key, hmac_sig.getData(),
+                               key.getSecret(), key.getSecretLength(),
+                               getHashAlgorithm(key),
+                               hmac_sig.getData(),
                                hmac_sig.getLength()));
     }
 
@@ -75,7 +95,7 @@ namespace {
         TSIGKey key(key_str);
 
         // Sign it
-        HMAC hmac_sign(key);
+        HMAC hmac_sign(key.getSecret(), key.getSecretLength(), getHashAlgorithm(key));
         hmac_sign.update(data_buf.getData(), data_buf.getLength());
         hmac_sign.sign(hmac_sig);
 
@@ -83,7 +103,7 @@ namespace {
         checkBuffer(hmac_sig, expected_hmac, hmac_len);
 
         // Check whether we can verify it ourselves
-        HMAC hmac_verify(key);
+        HMAC hmac_verify(key.getSecret(), key.getSecretLength(), getHashAlgorithm(key));
         hmac_verify.update(data_buf.getData(), data_buf.getLength());
         EXPECT_TRUE(hmac_verify.verify(hmac_sig.getData(),
                                        hmac_sig.getLength()));
@@ -327,16 +347,23 @@ TEST(CryptoTest, HMAC_SHA256_RFC2202_SIGN) {
 }
 
 TEST(CryptoTest, BadKey) {
-    TSIGKey bad_key = TSIGKey(Name("test.example."), Name("hmac-sha1."),
-                              NULL, 0);
-
     OutputBuffer data_buf(0);
     OutputBuffer hmac_sig(0);
 
-    EXPECT_THROW(new HMAC(bad_key), BadKey);
+    EXPECT_THROW(new HMAC(NULL, 0, HMAC::MD5), BadKey);
+    EXPECT_THROW(new HMAC(NULL, 0, HMAC::UNKNOWN), UnsupportedAlgorithm);
+
+    EXPECT_THROW(signHMAC(data_buf.getData(), data_buf.getLength(),
+                          NULL, 0, HMAC::MD5, hmac_sig), BadKey);
     EXPECT_THROW(signHMAC(data_buf.getData(), data_buf.getLength(),
-                          bad_key, hmac_sig), BadKey);
+                          NULL, 0, HMAC::UNKNOWN, hmac_sig),
+                          UnsupportedAlgorithm);
+
     EXPECT_THROW(verifyHMAC(data_buf.getData(), data_buf.getLength(),
-                            bad_key, hmac_sig.getData(),
+                            NULL, 0, HMAC::MD5, hmac_sig.getData(),
                             hmac_sig.getLength()), BadKey);
+    EXPECT_THROW(verifyHMAC(data_buf.getData(), data_buf.getLength(),
+                            NULL, 0, HMAC::UNKNOWN, hmac_sig.getData(),
+                            hmac_sig.getLength()),
+                            UnsupportedAlgorithm);
 }

src/lib/dns/tsigkey.cc

@@ -18,6 +18,8 @@
 
 #include <exceptions/exceptions.h>
 
+#include <crypto/crypto.h>
+
 #include <dns/name.h>
 #include <dns/util/base64.h>
 #include <dns/tsigkey.h>