[trac781] input data for sign and verify as void* instead of buffer

src/lib/crypto/crypto.cc

@@ -96,7 +96,7 @@ public:
     }
     
     bool verify(const void* sig, size_t len) {
-        return hmac_->verify_mac(static_cast<const Botan::byte*>(sig), len);
+        return (hmac_->verify_mac(static_cast<const Botan::byte*>(sig), len));
     }
 
 private:
@@ -123,26 +123,26 @@ HMAC::sign(isc::dns::OutputBuffer& result) {
 
 bool
 HMAC::verify(const void* sig, size_t len) {
-    return impl_->verify(sig, len);
+    return (impl_->verify(sig, len));
 }
 
 void
-signHMAC(const OutputBuffer& data, TSIGKey key,
+signHMAC(const void* data, size_t data_len, TSIGKey key,
          isc::dns::OutputBuffer& result)
 {
     HMAC hmac(key);
-    hmac.update(data.getData(), data.getLength());
+    hmac.update(data, data_len);
     hmac.sign(result);
 }
 
 
 bool
-verifyHMAC(const OutputBuffer& data, TSIGKey key,
+verifyHMAC(const void* data, size_t data_len, TSIGKey key,
            const isc::dns::OutputBuffer& result)
 {
     HMAC hmac(key);
-    hmac.update(data.getData(), data.getLength());
-    return hmac.verify(result.getData(), result.getLength());
+    hmac.update(data, data_len);
+    return (hmac.verify(result.getData(), result.getLength()));
 }
 
 } // namespace crypto

src/lib/crypto/crypto.h

@@ -12,19 +12,6 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-// This is an abstraction layer from an actual crypto implementation
-// The abstraction works as follows; we provide abstract base classes,
-// one general one for the entire library, one for a per-operation
-// context, and one for 'keys', in all their forms as we need them
-// for BIND10.
-// Any implementation (we currently use Botan as a backend), should
-// subclass all of these, and provide the necessary translations
-// between our calls and the underlying library. This also includes
-// translating between 'real' algorithm identifiers and ours.
-//
-// For future expansion, we may need to introduce a fourth type; sessions
-//
-
 #include <string>
 #include <dns/buffer.h>
 #include <dns/tsigkey.h>
@@ -60,45 +47,96 @@ public:
         CryptoError(file, line, what) {}
 };
 
+/// Forward declaration, pimpl style
 class HMACImpl;
 
+/// \brief HMAC support
+///
+/// This class is used to create and verify HMAC signatures
+///
 class HMAC {
 public:
+    /// \brief Constructor from a key
+    ///
+    /// Raises an UnsupportedAlgorithmException if the given key
+    /// is for an algorithm that is not supported by the underlying
+    /// library
+    /// Raises an InvalidKeyLength if the given key has a bad length
+    ///
+    /// Notes: if the key 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
+    /// 
+    /// \param key The key to use
     explicit HMAC(const isc::dns::TSIGKey& key);
+
+    /// \brief Destructor
     ~HMAC();
+
+    /// \brief Add data to digest
+    ///
+    /// \param data The data to add
+    /// \param len The size of the data
     void update(const void* data, size_t len);
+
+    /// \brief Calculate the final signature
+    ///
+    /// The result will be appended to the given outputbuffer
+    ///
+    /// \param result The OutputBuffer to append the result to
     void sign(isc::dns::OutputBuffer& result);
+
+    /// \brief Verify an existing signature
+    ///
+    /// \param sig The signature to verify
+    /// \param len The length of the sig
+    /// \return true if the signature is correct, false otherwise
     bool verify(const void* sig, size_t len);
+
 private:
     HMACImpl* impl_;
 };
 
 /// \brief Create an HMAC signature for the given data
 ///
+/// This is a convenience function that calculates the hmac signature,
+/// given a fixed amount of data. Internally it does the same as
+/// creating an HMAC object, feeding it the data, and calculating the
+/// resulting signature.
+///
 /// Raises an UnsupportedAlgorithm if we do not support the given
 /// algorithm. Raises a BadKey exception if the underlying library
 /// cannot handle the given TSIGKey (for instance if it has a bad
 /// length).
 ///
 /// \param data The data to sign
+/// \param data_len The length of the data
 /// \param key The TSIGKey to sign with
-/// \param result The signature will be written to the end of this buffer
-void signHMAC(const isc::dns::OutputBuffer& data,
+/// \param result The signature will be appended to this buffer
+void signHMAC(const void* data,
+              size_t data_len,
               isc::dns::TSIGKey key,
               isc::dns::OutputBuffer& result);
 
 /// \brief Verify an HMAC signature for the given data
 ///
+/// This is a convenience function that verifies an hmac signature,
+/// given a fixed amount of data. Internally it does the same as
+/// creating an HMAC object, feeding it the data, and checking the
+/// resulting signature.
+///
 /// Raises an UnsupportedAlgorithm if we do not support the given
 /// algorithm. Raises a BadKey exception if the underlying library
 /// cannot handle the given TSIGKey (for instance if it has a bad
 /// length).
 ///
 /// \param data The data to verify
+/// \param data_len The length of the data
 /// \param key The TSIGKey to verify with
 /// \param mac The signature to verify
 /// \return True if the signature verifies, false if not
-bool verifyHMAC(const isc::dns::OutputBuffer& data,
+bool verifyHMAC(const void* data,
+                size_t data_len,
                 isc::dns::TSIGKey key,
                 const isc::dns::OutputBuffer& mac);
 

src/lib/crypto/tests/crypto_unittests.cc

@@ -40,9 +40,22 @@ namespace {
 
         TSIGKey key(key_str);
 
-        signHMAC(data_buf, key, hmac_sig);
+        // Sign it
+        signHMAC(data_buf.getData(), data_buf.getLength(), key,
+                 hmac_sig);
+
+        // Check if the signature is what we expect
         checkBuffer(hmac_sig, expected_hmac, hmac_len);
-        EXPECT_TRUE(verifyHMAC(data_buf, key, hmac_sig));
+
+        // Check whether we can verify it ourselves
+        EXPECT_TRUE(verifyHMAC(data_buf.getData(), data_buf.getLength(),
+                               key, hmac_sig));
+
+        // 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));
     }
 }
 
@@ -258,7 +271,9 @@ TEST(CryptoTest, HMAC_SHA256_RFC2202_SIGN) {
                                  0x63, 0x64, 0x4f, 0x07, 0x13, 0x93,
                                  0x8a, 0x7f, 0x51, 0x53, 0x5c, 0x3a,
                                  0x35, 0xe2 };
-    doHMACTest("This is a test using a larger than block-size key and a larger than block-size data. The key needs to be hashed before being used by the HMAC algorithm.",
+    doHMACTest("This is a test using a larger than block-size key and a"
+               " larger than block-size data. The key needs to be hashe"
+               "d before being used by the HMAC algorithm.",
                "test.example:qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq"
                "qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq"
                "qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq"
@@ -273,6 +288,8 @@ TEST(CryptoTest, BadKey) {
     OutputBuffer data_buf(0);
     OutputBuffer hmac_sig(1);
 
-    EXPECT_THROW(signHMAC(data_buf, bad_key, hmac_sig), BadKey);
-    EXPECT_THROW(verifyHMAC(data_buf, bad_key, hmac_sig), BadKey);
+    EXPECT_THROW(signHMAC(data_buf.getData(), data_buf.getLength(),
+                          bad_key, hmac_sig), BadKey);
+    EXPECT_THROW(verifyHMAC(data_buf.getData(), data_buf.getLength(),
+                            bad_key, hmac_sig), BadKey);
 }