[master] merge cryptolink cleanups #3602

ChangeLog

@@ -1,3 +1,8 @@
+857.	[func]		fdupont
+	Improve the cryptolink code, for instance use a constant
+	time comparison.
+	(Trac #3602, git ...)
+
 856.	[build]		marcinw
 	callout_manager.h and server_hooks.h headers are now exported,
 	so statically linked libraries can be tested.

src/lib/cryptolink/botan_common.h

@@ -14,13 +14,15 @@
 
 namespace isc {
 namespace cryptolink {
+namespace btn {
 
 /// @brief Decode the HashAlgorithm enum into a name usable by Botan
 ///
 /// @param algorithm algorithm to be converted
 /// @return static text representation of the algorithm name
 const char*
-getBotanHashAlgorithmName(isc::cryptolink::HashAlgorithm algorithm);
+getHashAlgorithmName(isc::cryptolink::HashAlgorithm algorithm);
 
+} // namespace btn
 } // namespace cryptolink
 } // namespace isc

src/lib/cryptolink/botan_hash.cc

@@ -34,7 +34,7 @@ namespace cryptolink {
 /// @param algorithm algorithm to be converted
 /// @return text representation of the algorithm name
 const char*
-getBotanHashAlgorithmName(isc::cryptolink::HashAlgorithm algorithm) {
+btn::getHashAlgorithmName(HashAlgorithm algorithm) {
     switch (algorithm) {
     case isc::cryptolink::MD5:
         return ("MD5");
@@ -67,8 +67,7 @@ public:
     explicit HashImpl(const HashAlgorithm hash_algorithm) {
         Botan::HashFunction* hash;
         try {
-            hash = Botan::get_hash(
-                getBotanHashAlgorithmName(hash_algorithm));
+            hash = Botan::get_hash(btn::getHashAlgorithmName(hash_algorithm));
         } catch (const Botan::Algorithm_Not_Found&) {
             isc_throw(isc::cryptolink::UnsupportedAlgorithm,
                       "Unknown hash algorithm: " <<

src/lib/cryptolink/botan_hmac.cc

@@ -45,8 +45,7 @@ public:
                       const HashAlgorithm hash_algorithm) {
         Botan::HashFunction* hash;
         try {
-            hash = Botan::get_hash(
-                getBotanHashAlgorithmName(hash_algorithm));
+            hash = Botan::get_hash(btn::getHashAlgorithmName(hash_algorithm));
         } catch (const Botan::Algorithm_Not_Found&) {
             isc_throw(isc::cryptolink::UnsupportedAlgorithm,
                       "Unknown hash algorithm: " <<

src/lib/cryptolink/crypto_hmac.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
@@ -125,6 +125,9 @@ public:
     ///            and smaller than the output length of the algorithm,
     ///            only len bytes will be checked
     /// \return true if the signature is correct, false otherwise
+    ///
+    /// \note verify() does not destroy its context so it can be
+    /// called multiple times with different signatures.
     bool verify(const void* sig, size_t len);
 
 private:

src/lib/cryptolink/openssl_common.h

@@ -14,6 +14,7 @@
 
 namespace isc {
 namespace cryptolink {
+namespace ossl {
 
 /// @brief Decode the HashAlgorithm enum into an EVP_MD pointer (or 0)
 ///
@@ -21,7 +22,91 @@ namespace cryptolink {
 /// @param algorithm algorithm to be converted
 /// @return pointer to a static EVP_MD which identifies the algorithm
 const EVP_MD*
-getOpenSSLHashAlgorithm(isc::cryptolink::HashAlgorithm algorithm);
+getHashAlgorithm(isc::cryptolink::HashAlgorithm algorithm);
 
+/// Secure Buffers which are wiped out when released.
+/// Subset of the std::vector interface but not derived from
+/// to avoid unwanted inheritance.
+template<typename T>
+class SecBuf {
+public:
+    typedef typename std::vector<T>::iterator iterator;
+
+    typedef typename std::vector<T>::const_iterator const_iterator;
+
+    explicit SecBuf() : vec_(std::vector<T>()) {}
+
+    explicit SecBuf(size_t n, const T& value = T()) :
+        vec_(std::vector<T>(n, value))
+    {}
+
+    SecBuf(iterator first, iterator last) :
+        vec_(std::vector<T>(first, last))
+    {}
+
+    SecBuf(const_iterator first, const_iterator last) :
+        vec_(std::vector<T>(first, last))
+    {}
+
+    SecBuf(const std::vector<T>& x) : vec_(x) {}
+
+    ~SecBuf() {
+        std::memset(&vec_[0], 0, vec_.capacity() * sizeof(T));
+    };
+
+    iterator begin() {
+        return (vec_.begin());
+    };
+
+    const_iterator begin() const {
+        return (vec_.begin());
+    };
+
+    iterator end() {
+        return (vec_.end());
+    };
+
+    const_iterator end() const {
+        return (vec_.end());
+    };
+
+    size_t size() const {
+        return (vec_.size());
+    };
+
+    void resize(size_t sz) {
+        vec_.resize(sz);
+    };
+
+    SecBuf& operator=(const SecBuf& x) {
+        if (&x != *this) {
+            vec_ = x.vec_;
+        }
+        return (*this);
+    };
+
+    T& operator[](size_t n) {
+        return (vec_[n]);
+    };
+
+    const T& operator[](size_t n) const {
+        return (vec_[n]);
+    };
+
+    // constant time comparison against timing attacks
+    // (same type than XXX::verify() so const void* (vs. const T*) x)
+    bool same(const void* x, size_t len) const {
+        bool ret = true;
+        const T* p = static_cast<const T*>(x);
+        for (size_t i = 0; i < len; ++i)
+            ret = ret && (vec_[i] == p[i]);
+        return ret;
+    };
+
+private:
+    std::vector<T> vec_;
+};
+
+} // namespace ossl
 } // namespace cryptolink
 } // namespace isc

src/lib/cryptolink/openssl_hash.cc

@@ -32,7 +32,7 @@ namespace cryptolink {
 /// @param algorithm algorithm to be converted
 /// @return pointer to EVP_MD which identifies the algorithm
 const EVP_MD*
-getOpenSSLHashAlgorithm(isc::cryptolink::HashAlgorithm algorithm) {
+ossl::getHashAlgorithm(HashAlgorithm algorithm) {
     switch (algorithm) {
     case isc::cryptolink::MD5:
         return (EVP_md5());
@@ -63,7 +63,7 @@ public:
     ///
     /// @param hash_algorithm The hash algorithm
     explicit HashImpl(const HashAlgorithm hash_algorithm) {
-        const EVP_MD* algo = getOpenSSLHashAlgorithm(hash_algorithm);
+        const EVP_MD* algo = ossl::getHashAlgorithm(hash_algorithm);
         if (algo == 0) {
             isc_throw(isc::cryptolink::UnsupportedAlgorithm,
                       "Unknown hash algorithm: " <<

src/lib/cryptolink/openssl_hmac.cc

@@ -23,81 +23,6 @@
 
 #include <cstring>
 
-namespace {
-
-/// Secure Buffers which are wiped out when released.
-template<typename T>
-struct SecBuf {
-public:
-    typedef typename std::vector<T>::iterator iterator;
-
-    typedef typename std::vector<T>::const_iterator const_iterator;
-
-    explicit SecBuf() : vec_(std::vector<T>()) {}
-
-    explicit SecBuf(size_t n, const T& value = T()) :
-        vec_(std::vector<T>(n, value))
-    {}
-
-    SecBuf(iterator first, iterator last) :
-        vec_(std::vector<T>(first, last))
-    {}
-
-    SecBuf(const_iterator first, const_iterator last) :
-        vec_(std::vector<T>(first, last))
-    {}
-
-    SecBuf(const std::vector<T>& x) : vec_(x) {}
-
-    ~SecBuf() {
-        std::memset(&vec_[0], 0, vec_.capacity() * sizeof(T));
-    };
-
-    iterator begin() {
-        return (vec_.begin());
-    };
-
-    const_iterator begin() const {
-        return (vec_.begin());
-    };
-
-    iterator end() {
-        return (vec_.end());
-    };
-
-    const_iterator end() const {
-        return (vec_.end());
-    };
-
-    size_t size() const {
-        return (vec_.size());
-    };
-
-    void resize(size_t sz) {
-        vec_.resize(sz);
-    };
-
-    SecBuf& operator=(const SecBuf& x) {
-        if (&x != *this) {
-            vec_ = x.vec_;
-        }
-        return (*this);
-    };
-
-    T& operator[](size_t n) {
-        return (vec_[n]);
-    };
-
-    const T& operator[](size_t n) const {
-        return (vec_[n]);
-    };
-
-private:
-    std::vector<T> vec_;
-};
-
-} // local namespace
-
 namespace isc {
 namespace cryptolink {
 
@@ -114,7 +39,7 @@ public:
     /// @param hash_algorithm The hash algorithm
     explicit HMACImpl(const void* secret, size_t secret_len,
                       const HashAlgorithm hash_algorithm) {
-        const EVP_MD* algo = getOpenSSLHashAlgorithm(hash_algorithm);
+        const EVP_MD* algo = ossl::getHashAlgorithm(hash_algorithm);
         if (algo == 0) {
             isc_throw(UnsupportedAlgorithm,
                       "Unknown hash algorithm: " <<
@@ -162,7 +87,7 @@ public:
     /// See @ref isc::cryptolink::HMAC::sign() for details.
     void sign(isc::util::OutputBuffer& result, size_t len) {
         size_t size = getOutputLength();
-        SecBuf<unsigned char> digest(size);
+        ossl::SecBuf<unsigned char> digest(size);
         HMAC_Final(md_.get(), &digest[0], NULL);
         if (len == 0 || len > size) {
             len = size;
@@ -175,7 +100,7 @@ public:
     /// See @ref isc::cryptolink::HMAC::sign() for details.
     void sign(void* result, size_t len) {
         size_t size = getOutputLength();
-        SecBuf<unsigned char> digest(size);
+        ossl::SecBuf<unsigned char> digest(size);
         HMAC_Final(md_.get(), &digest[0], NULL);
         if (len > size) {
             len = size;
@@ -188,7 +113,7 @@ public:
     /// See @ref isc::cryptolink::HMAC::sign() for details.
     std::vector<uint8_t> sign(size_t len) {
         size_t size = getOutputLength();
-        SecBuf<unsigned char> digest(size);
+        ossl::SecBuf<unsigned char> digest(size);
         HMAC_Final(md_.get(), &digest[0], NULL);
         if (len != 0 && len < size) {
             digest.resize(len);
@@ -204,12 +129,12 @@ public:
         if (len != 0 && (len < 10 || len < size / 2)) {
             return (false);
         }
-        SecBuf<unsigned char> digest(size);
+        ossl::SecBuf<unsigned char> digest(size);
         HMAC_Final(md_.get(), &digest[0], NULL);
         if (len == 0 || len > size) {
             len = size;
         }
-        return (std::memcmp(&digest[0], sig, len) == 0);
+        return (digest.same(sig, len));
     }
 
 private: