From 1a2002e5cd63edd87a076a3ca138db01bed9e645 Mon Sep 17 00:00:00 2001
From: Francis Dupont <fdupont@isc.org>
Date: Mon, 8 Jun 2015 12:55:18 +0200
Subject: [PATCH] [sedhcpv6] fixed multiple verify in HMAC

---
 src/lib/cryptolink/botan_hmac.cc           | 25 +++++++++++++---------
 src/lib/cryptolink/openssl_hmac.cc         | 15 +++++++++++--
 src/lib/cryptolink/tests/hmac_unittests.cc | 10 +++++++++
 3 files changed, 38 insertions(+), 12 deletions(-)

diff --git a/src/lib/cryptolink/botan_hmac.cc b/src/lib/cryptolink/botan_hmac.cc
index 14539c97e7..ce90e9ff3a 100644
--- a/src/lib/cryptolink/botan_hmac.cc
+++ b/src/lib/cryptolink/botan_hmac.cc
@@ -48,11 +48,11 @@ public:
         try {
             hash = Botan::get_hash(btn::getHashAlgorithmName(hash_algorithm));
         } catch (const Botan::Algorithm_Not_Found&) {
-            isc_throw(isc::cryptolink::UnsupportedAlgorithm,
+            isc_throw(UnsupportedAlgorithm,
                       "Unknown hash algorithm: " <<
                       static_cast<int>(hash_algorithm));
         } catch (const Botan::Exception& exc) {
-            isc_throw(isc::cryptolink::LibraryError, exc.what());
+            isc_throw(LibraryError, exc.what());
         }
 
         hmac_.reset(new Botan::HMAC(hash));
@@ -88,7 +88,7 @@ public:
         } catch (const Botan::Invalid_Key_Length& ikl) {
             isc_throw(BadKey, ikl.what());
         } catch (const Botan::Exception& exc) {
-            isc_throw(isc::cryptolink::LibraryError, exc.what());
+            isc_throw(LibraryError, exc.what());
         }
     }
 
@@ -123,7 +123,7 @@ public:
         try {
             hmac_->update(static_cast<const Botan::byte*>(data), len);
         } catch (const Botan::Exception& exc) {
-            isc_throw(isc::cryptolink::LibraryError, exc.what());
+            isc_throw(LibraryError, exc.what());
         }
     }
 
@@ -139,7 +139,7 @@ public:
             }
             result.writeData(b_result.begin(), len);
         } catch (const Botan::Exception& exc) {
-            isc_throw(isc::cryptolink::LibraryError, exc.what());
+            isc_throw(LibraryError, exc.what());
         }
     }
 
@@ -155,7 +155,7 @@ public:
             }
             std::memcpy(result, b_result.begin(), output_size);
         } catch (const Botan::Exception& exc) {
-            isc_throw(isc::cryptolink::LibraryError, exc.what());
+            isc_throw(LibraryError, exc.what());
         }
     }
 
@@ -171,7 +171,7 @@ public:
                 return (std::vector<uint8_t>(b_result.begin(), &b_result[len]));
             }
         } catch (const Botan::Exception& exc) {
-            isc_throw(isc::cryptolink::LibraryError, exc.what());
+            isc_throw(LibraryError, exc.what());
         }
     }
 
@@ -184,7 +184,6 @@ public:
         /// which causes it to fail for truncated signatures, so we do
         /// the check ourselves
         try {
-            Botan::SecureVector<Botan::byte> our_mac = hmac_->final();
             size_t size = getOutputLength();
             if (len < 10 || len < size / 2) {
                 return (false);
@@ -192,11 +191,14 @@ public:
             if (len > size) {
                 len = size;
             }
-            return (Botan::same_mem(&our_mac[0],
+            if (digest_.empty()) {
+                digest_ = hmac_->final();
+            }
+            return (Botan::same_mem(&digest_[0],
                                     static_cast<const unsigned char*>(sig),
                                     len));
         } catch (const Botan::Exception& exc) {
-            isc_throw(isc::cryptolink::LibraryError, exc.what());
+            isc_throw(LibraryError, exc.what());
         }
     }
 
@@ -206,6 +208,9 @@ private:
 
     /// @brief The protected pointer to the Botan HMAC object
     boost::scoped_ptr<Botan::HMAC> hmac_;
+
+    /// @brief The digest cache for multiple verify
+    Botan::SecureVector<Botan::byte> digest_;
 };
 
 HMAC::HMAC(const void* secret, size_t secret_length,
diff --git a/src/lib/cryptolink/openssl_hmac.cc b/src/lib/cryptolink/openssl_hmac.cc
index ed1a618177..baf7fa66e9 100644
--- a/src/lib/cryptolink/openssl_hmac.cc
+++ b/src/lib/cryptolink/openssl_hmac.cc
@@ -76,7 +76,7 @@ public:
     size_t getOutputLength() const {
         int size = HMAC_size(md_.get());
         if (size < 0) {
-            isc_throw(isc::cryptolink::LibraryError, "EVP_MD_CTX_size");
+            isc_throw(LibraryError, "HMAC_size");
         }
         return (static_cast<size_t>(size));
     }
@@ -131,12 +131,23 @@ public:
     ///
     /// See @ref isc::cryptolink::HMAC::verify() for details.
     bool verify(const void* sig, size_t len) {
+        // Check the length
         size_t size = getOutputLength();
         if (len < 10 || len < size / 2) {
             return (false);
         }
+        // Get the digest from a copy of the context
+        HMAC_CTX tmp;
+        HMAC_CTX_init(&tmp);
+        if (!HMAC_CTX_copy(&tmp, md_.get())) {
+            isc_throw(LibraryError, "HMAC_CTX_copy");
+        }
         ossl::SecBuf<unsigned char> digest(size);
-        HMAC_Final(md_.get(), &digest[0], NULL);
+        if (!HMAC_Final(&tmp, &digest[0], NULL)) {
+            HMAC_CTX_cleanup(&tmp);
+            isc_throw(LibraryError, "HMAC_Final");
+        }
+        HMAC_CTX_cleanup(&tmp);
         if (len > size) {
             len = size;
         }
diff --git a/src/lib/cryptolink/tests/hmac_unittests.cc b/src/lib/cryptolink/tests/hmac_unittests.cc
index f148dca59e..6e2af12d74 100644
--- a/src/lib/cryptolink/tests/hmac_unittests.cc
+++ b/src/lib/cryptolink/tests/hmac_unittests.cc
@@ -133,6 +133,13 @@ namespace {
         hmac_sig.writeUint8At(~hmac_sig[0], 0);
         EXPECT_FALSE(hmac_verify->verify(hmac_sig.getData(),
                                          hmac_sig.getLength()));
+
+        // Restore the sig by flipping the first octet, and check
+        // whether verification succeeds then
+        hmac_sig.writeUint8At(~hmac_sig[0], 0);
+        EXPECT_TRUE(hmac_verify->verify(hmac_sig.getData(),
+                                        hmac_sig.getLength()));
+
     }
 
     /// @brief Sign and verify with vector representation of signature
@@ -197,6 +204,9 @@ namespace {
         sig[0] = ~sig[0];
         EXPECT_FALSE(hmac_verify->verify(sig, hmac_len));
 
+        sig[0] = ~sig[0];
+        EXPECT_TRUE(hmac_verify->verify(sig, hmac_len));
+
         delete[] sig;
     }
 
-- 
2.47.2