[master] merge cryptolink API cleanup (trac3606a)

ChangeLog

@@ -1,3 +1,8 @@
+870.	[func]		fdupont
+	Cleanup the cryptolink API (e.g., removing spurious 'magic'
+	zero length parameters).
+	(Trac #3606, git ...)
+
 869.	[func]		tomek
 	'mac-sources' configuration parameter added. The DHCPv6 server
 	can now be configured to use various MAC/Hardware address

src/lib/cryptolink/botan_hash.cc

@@ -115,7 +115,7 @@ public:
         try {
             Botan::SecureVector<Botan::byte> b_result(hash_->final());
 
-            if (len == 0 || len > b_result.size()) {
+            if (len > b_result.size()) {
                 len = b_result.size();
             }
             result.writeData(b_result.begin(), len);
@@ -146,7 +146,7 @@ public:
     std::vector<uint8_t> final(size_t len) {
         try {
             Botan::SecureVector<Botan::byte> b_result(hash_->final());
-            if (len == 0 || len > b_result.size()) {
+            if (len > b_result.size()) {
                 return (std::vector<uint8_t>(b_result.begin(), b_result.end()));
             } else {
                 return (std::vector<uint8_t>(b_result.begin(), &b_result[len]));

src/lib/cryptolink/botan_hmac.cc

@@ -128,7 +128,7 @@ public:
         try {
             Botan::SecureVector<Botan::byte> b_result(hmac_->final());
 
-            if (len == 0 || len > b_result.size()) {
+            if (len > b_result.size()) {
                 len = b_result.size();
             }
             result.writeData(b_result.begin(), len);
@@ -159,7 +159,7 @@ public:
     std::vector<uint8_t> sign(size_t len) {
         try {
             Botan::SecureVector<Botan::byte> b_result(hmac_->final());
-            if (len == 0 || len > b_result.size()) {
+            if (len > b_result.size()) {
                 return (std::vector<uint8_t>(b_result.begin(), b_result.end()));
             } else {
                 return (std::vector<uint8_t>(b_result.begin(), &b_result[len]));
@@ -180,10 +180,10 @@ public:
         try {
             Botan::SecureVector<Botan::byte> our_mac = hmac_->final();
             size_t size = getOutputLength();
-            if (len != 0 && (len < 10 || len < size / 2)) {
+            if (len < 10 || len < size / 2) {
                 return (false);
             }
-            if (len == 0 || len > size) {
+            if (len > size) {
                 len = size;
             }
             return (Botan::same_mem(&our_mac[0],

src/lib/cryptolink/crypto_hash.cc

@@ -30,6 +30,9 @@ digest(const void* data, const size_t data_len,
     boost::scoped_ptr<Hash> hash(
         CryptoLink::getCryptoLink().createHash(hash_algorithm));
     hash->update(data, data_len);
+    if (len == 0) {
+        len = hash->getOutputLength();
+    }
     hash->final(result, len);
 }
 

src/lib/cryptolink/crypto_hash.h

@@ -74,9 +74,9 @@ public:
     /// \param result The OutputBuffer to append the result to
     /// \param len The number of bytes from the result to copy. If this
     ///        value is smaller than the algorithms output size, the
-    ///        result will be truncated. If this value is larger, or 0
-    ///        (the default), it will be ignored
-    void final(isc::util::OutputBuffer& result, size_t len = 0);
+    ///        result will be truncated. If this value is larger,
+    ///        only output size bytes will be copied
+    void final(isc::util::OutputBuffer& result, size_t len);
 
     /// \brief Calculate the final digest
     ///
@@ -103,10 +103,10 @@ public:
     ///
     /// \param len The number of bytes from the result to copy. If this
     ///        value is smaller than the algorithms output size, the
-    ///        result will be truncated. If this value is larger, or 0
-    ///        (the default), it will be ignored
+    ///        result will be truncated. If this value is larger,
+    ///        only output size bytes will be copied
     /// \return a vector containing the signature
-    std::vector<uint8_t> final(size_t len = 0);
+    std::vector<uint8_t> final(size_t len);
 
 private:
     HashImpl* impl_;
@@ -128,8 +128,9 @@ private:
 /// \param data_len The length of the data
 /// \param hash_algorithm The hash algorithm
 /// \param result The digest will be appended to this buffer
-/// \param len If this is non-zero and less than the output size,
-///            the result will be truncated to len bytes
+/// \param len If this is non-zero and less than the output size, the result
+///            will be truncated to len bytes. If greater than output size
+///            (or equal to zero) only output size bytes are written
 void digest(const void* data,
             const size_t data_len,
             const HashAlgorithm hash_algorithm,

src/lib/cryptolink/crypto_hmac.cc

@@ -32,6 +32,9 @@ signHMAC(const void* data, const size_t data_len, const void* secret,
                                                secret_len,
                                                hash_algorithm));
     hmac->update(data, data_len);
+    if (len == 0) {
+        len = hmac->getOutputLength();
+    }
     hmac->sign(result, len);
 }
 
@@ -46,7 +49,11 @@ verifyHMAC(const void* data, const size_t data_len, const void* secret,
                                                secret_len,
                                                hash_algorithm));
     hmac->update(data, data_len);
-    return (hmac->verify(sig, sig_len));
+    size_t len = sig_len;
+    if (len == 0) {
+        len = hmac->getOutputLength();
+    }
+    return (hmac->verify(sig, len));
 }
 
 void

src/lib/cryptolink/crypto_hmac.h

@@ -84,9 +84,9 @@ public:
     /// \param result The OutputBuffer to append the result to
     /// \param len The number of bytes from the result to copy. If this
     ///        value is smaller than the algorithms output size, the
-    ///        result will be truncated. If this value is larger, or 0
-    ///        (the default), it will be ignored
-    void sign(isc::util::OutputBuffer& result, size_t len = 0);
+    ///        result will be truncated. If this value is larger,
+    ///        only output size bytes will be copied
+    void sign(isc::util::OutputBuffer& result, size_t len);
 
     /// \brief Calculate the final signature
     ///
@@ -110,10 +110,10 @@ public:
     ///
     /// \param len The number of bytes from the result to copy. If this
     ///        value is smaller than the algorithms output size, the
-    ///        result will be truncated. If this value is larger, or 0
-    ///        (the default), it will be ignored
+    ///        result will be truncated. If this value is larger,
+    ///        only output size bytes will be copied
     /// \return a vector containing the signature
-    std::vector<uint8_t> sign(size_t len = 0);
+    std::vector<uint8_t> sign(size_t len);
 
     /// \brief Verify an existing signature
     ///
@@ -121,8 +121,8 @@ public:
     ///                         in the underlying library
     ///
     /// \param sig The signature to verify
-    /// \param len The length of the signature. If this is non-zero,
-    ///            and smaller than the output length of the algorithm,
+    /// \param len The length of the signature. If this is smaller
+    ///            than the output length of the algorithm,
     ///            only len bytes will be checked
     /// \return true if the signature is correct, false otherwise
     ///
@@ -136,7 +136,7 @@ private:
 
 /// \brief Create an HMAC signature for the given data
 ///
-/// This is a convenience function that calculates the hmac signature,
+/// 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.
@@ -158,8 +158,9 @@ private:
 /// \param secret_len The length of the secret
 /// \param hash_algorithm The hash algorithm
 /// \param result The signature will be appended to this buffer
-/// \param len If this is non-zero and less than the output size,
-///            the result will be truncated to len bytes
+/// \param len If this is non-zero and less than the output size, the result
+///            will be truncated to len bytes. If greater than output size
+///            (or equal to zero) only output size bytes are written
 void signHMAC(const void* data,
               const size_t data_len,
               const void* secret,
@@ -173,7 +174,8 @@ void signHMAC(const void* 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.
+/// resulting signature at the exception a zero sig_len is
+/// internally replaced by the output size.
 ///
 /// \exception UnsupportedAlgorithm if the given algorithm is unknown
 ///            or not supported by the underlying library

src/lib/cryptolink/openssl_hash.cc

@@ -105,7 +105,7 @@ public:
         size_t size = getOutputLength();
         std::vector<unsigned char> digest(size);
         EVP_DigestFinal_ex(md_.get(), &digest[0], NULL);
-        if (len == 0 || len > size) {
+        if (len > size) {
              len = size;
         }
         result.writeData(&digest[0], len);
@@ -131,7 +131,7 @@ public:
         size_t size = getOutputLength();
         std::vector<unsigned char> digest(size);
         EVP_DigestFinal_ex(md_.get(), &digest[0], NULL);
-        if (len != 0 && len < size) {
+        if (len < size) {
             digest.resize(len);
         }
         return (std::vector<uint8_t>(digest.begin(), digest.end()));

src/lib/cryptolink/openssl_hmac.cc

@@ -89,7 +89,7 @@ public:
         size_t size = getOutputLength();
         ossl::SecBuf<unsigned char> digest(size);
         HMAC_Final(md_.get(), &digest[0], NULL);
-        if (len == 0 || len > size) {
+        if (len > size) {
             len = size;
         }
         result.writeData(&digest[0], len);
@@ -115,7 +115,7 @@ public:
         size_t size = getOutputLength();
         ossl::SecBuf<unsigned char> digest(size);
         HMAC_Final(md_.get(), &digest[0], NULL);
-        if (len != 0 && len < size) {
+        if (len < size) {
             digest.resize(len);
         }
         return (std::vector<uint8_t>(digest.begin(), digest.end()));
@@ -126,12 +126,12 @@ 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 < 10 || len < size / 2)) {
+        if (len < 10 || len < size / 2) {
             return (false);
         }
         ossl::SecBuf<unsigned char> digest(size);
         HMAC_Final(md_.get(), &digest[0], NULL);
-        if (len == 0 || len > size) {
+        if (len > size) {
             len = size;
         }
         return (digest.same(sig, len));

src/lib/cryptolink/tests/hash_unittests.cc

@@ -130,7 +130,7 @@ namespace {
         // note: this is not exception-safe, and can leak, but
         // if there is an unexpected exception in the code below we
         // have more important things to fix.
-	boost::scoped_array<uint8_t> result(new uint8_t[hash_len]);
+        boost::scoped_array<uint8_t> result(new uint8_t[hash_len]);
 
         hash_digest->final(result.get(), hash_len);
         checkData(result.get(), expected_hash, hash_len);
@@ -562,37 +562,31 @@ namespace {
 TEST(HashTest, HashLength) {
     std::vector<uint8_t> result;
 
-    EXPECT_EQ(16, digestVectorLength(MD5, 0));
     EXPECT_EQ(8, digestVectorLength(MD5, 8));
     EXPECT_EQ(16, digestVectorLength(MD5, 16));
     EXPECT_EQ(16, digestVectorLength(MD5, 40));
     EXPECT_EQ(16, digestVectorLength(MD5, 2000));
 
-    EXPECT_EQ(20, digestBufferLength(SHA1, 0));
     EXPECT_EQ(8, digestBufferLength(SHA1, 8));
     EXPECT_EQ(20, digestBufferLength(SHA1, 20));
     EXPECT_EQ(20, digestBufferLength(SHA1, 40));
     EXPECT_EQ(20, digestBufferLength(SHA1, 2000));
 
-    EXPECT_EQ(32, digestBufferLength(SHA256, 0));
     EXPECT_EQ(8, digestBufferLength(SHA256, 8));
     EXPECT_EQ(32, digestBufferLength(SHA256, 32));
     EXPECT_EQ(32, digestBufferLength(SHA256, 40));
     EXPECT_EQ(32, digestBufferLength(SHA256, 3200));
 
-    EXPECT_EQ(16, digestBufferLength(MD5, 0));
     EXPECT_EQ(8, digestBufferLength(MD5, 8));
     EXPECT_EQ(16, digestBufferLength(MD5, 16));
     EXPECT_EQ(16, digestBufferLength(MD5, 40));
     EXPECT_EQ(16, digestBufferLength(MD5, 2000));
 
-    EXPECT_EQ(20, digestBufferLength(SHA1, 0));
     EXPECT_EQ(8, digestBufferLength(SHA1, 8));
     EXPECT_EQ(20, digestBufferLength(SHA1, 20));
     EXPECT_EQ(20, digestBufferLength(SHA1, 40));
     EXPECT_EQ(20, digestBufferLength(SHA1, 2000));
 
-    EXPECT_EQ(32, digestBufferLength(SHA256, 0));
     EXPECT_EQ(8, digestBufferLength(SHA256, 8));
     EXPECT_EQ(32, digestBufferLength(SHA256, 32));
     EXPECT_EQ(32, digestBufferLength(SHA256, 40));

src/lib/cryptolink/tests/hmac_unittests.cc

@@ -602,37 +602,31 @@ namespace {
 TEST(HMACTest, HMACSigLengthArgument) {
     std::vector<uint8_t> sig;
 
-    EXPECT_EQ(16, sigVectorLength(MD5, 0));
     EXPECT_EQ(8, sigVectorLength(MD5, 8));
     EXPECT_EQ(16, sigVectorLength(MD5, 16));
     EXPECT_EQ(16, sigVectorLength(MD5, 40));
     EXPECT_EQ(16, sigVectorLength(MD5, 2000));
 
-    EXPECT_EQ(20, sigBufferLength(SHA1, 0));
     EXPECT_EQ(8, sigBufferLength(SHA1, 8));
     EXPECT_EQ(20, sigBufferLength(SHA1, 20));
     EXPECT_EQ(20, sigBufferLength(SHA1, 40));
     EXPECT_EQ(20, sigBufferLength(SHA1, 2000));
 
-    EXPECT_EQ(32, sigBufferLength(SHA256, 0));
     EXPECT_EQ(8, sigBufferLength(SHA256, 8));
     EXPECT_EQ(32, sigBufferLength(SHA256, 32));
     EXPECT_EQ(32, sigBufferLength(SHA256, 40));
     EXPECT_EQ(32, sigBufferLength(SHA256, 3200));
 
-    EXPECT_EQ(16, sigBufferLength(MD5, 0));
     EXPECT_EQ(8, sigBufferLength(MD5, 8));
     EXPECT_EQ(16, sigBufferLength(MD5, 16));
     EXPECT_EQ(16, sigBufferLength(MD5, 40));
     EXPECT_EQ(16, sigBufferLength(MD5, 2000));
 
-    EXPECT_EQ(20, sigBufferLength(SHA1, 0));
     EXPECT_EQ(8, sigBufferLength(SHA1, 8));
     EXPECT_EQ(20, sigBufferLength(SHA1, 20));
     EXPECT_EQ(20, sigBufferLength(SHA1, 40));
     EXPECT_EQ(20, sigBufferLength(SHA1, 2000));
 
-    EXPECT_EQ(32, sigBufferLength(SHA256, 0));
     EXPECT_EQ(8, sigBufferLength(SHA256, 8));
     EXPECT_EQ(32, sigBufferLength(SHA256, 32));
     EXPECT_EQ(32, sigBufferLength(SHA256, 40));

src/lib/dns/nsec3hash.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012,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
@@ -120,7 +120,7 @@ iterateSHA1(const uint8_t* input, size_t inlength,
     boost::scoped_ptr<Hash> hash(CryptoLink::getCryptoLink().createHash(SHA1));
     hash->update(input, inlength);
     hash->update(salt, saltlen); // this works whether saltlen == or > 0
-    hash->final(output);
+    hash->final(output, hash->getOutputLength());
 }
 
 string

src/lib/dns/tsig.cc

@@ -78,7 +78,7 @@ struct TSIGContext::TSIGContextImpl {
                 hmac_.reset(CryptoLink::getCryptoLink().createHMAC(
                                 key_.getSecret(), key_.getSecretLength(),
                                 key_.getAlgorithm()),
-                            deleteHMAC);
+                                deleteHMAC);
             } catch (const isc::Exception&) {
                 return;
             }