From: Francis Dupont Date: Tue, 18 Nov 2014 08:08:53 +0000 (+0100) Subject: [master] merge cryptolink cleanups #3602 X-Git-Tag: kea-eng-20141219~20 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=0c1f433da650330b40fe1a67bae4716c9184f636;p=thirdparty%2Fkea.git [master] merge cryptolink cleanups #3602 --- diff --git a/ChangeLog b/ChangeLog index 142c778077..3c85a35a78 100644 --- a/ChangeLog +++ b/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. diff --git a/src/lib/cryptolink/botan_common.h b/src/lib/cryptolink/botan_common.h index 0434c3a71d..ced6d8b360 100644 --- a/src/lib/cryptolink/botan_common.h +++ b/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 diff --git a/src/lib/cryptolink/botan_hash.cc b/src/lib/cryptolink/botan_hash.cc index e033b93301..d0bd9f1792 100644 --- a/src/lib/cryptolink/botan_hash.cc +++ b/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: " << diff --git a/src/lib/cryptolink/botan_hmac.cc b/src/lib/cryptolink/botan_hmac.cc index a6453ef100..bb5b7562a1 100644 --- a/src/lib/cryptolink/botan_hmac.cc +++ b/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: " << diff --git a/src/lib/cryptolink/crypto_hmac.h b/src/lib/cryptolink/crypto_hmac.h index ac827857b7..12908372eb 100644 --- a/src/lib/cryptolink/crypto_hmac.h +++ b/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: diff --git a/src/lib/cryptolink/openssl_common.h b/src/lib/cryptolink/openssl_common.h index fb6ed4b40d..0652037247 100644 --- a/src/lib/cryptolink/openssl_common.h +++ b/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 +class SecBuf { +public: + typedef typename std::vector::iterator iterator; + + typedef typename std::vector::const_iterator const_iterator; + + explicit SecBuf() : vec_(std::vector()) {} + + explicit SecBuf(size_t n, const T& value = T()) : + vec_(std::vector(n, value)) + {} + + SecBuf(iterator first, iterator last) : + vec_(std::vector(first, last)) + {} + + SecBuf(const_iterator first, const_iterator last) : + vec_(std::vector(first, last)) + {} + + SecBuf(const std::vector& 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(x); + for (size_t i = 0; i < len; ++i) + ret = ret && (vec_[i] == p[i]); + return ret; + }; + +private: + std::vector vec_; +}; + +} // namespace ossl } // namespace cryptolink } // namespace isc diff --git a/src/lib/cryptolink/openssl_hash.cc b/src/lib/cryptolink/openssl_hash.cc index dfc870c400..483a61fde2 100644 --- a/src/lib/cryptolink/openssl_hash.cc +++ b/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: " << diff --git a/src/lib/cryptolink/openssl_hmac.cc b/src/lib/cryptolink/openssl_hmac.cc index 411d86949c..711cc44a89 100644 --- a/src/lib/cryptolink/openssl_hmac.cc +++ b/src/lib/cryptolink/openssl_hmac.cc @@ -23,81 +23,6 @@ #include -namespace { - -/// Secure Buffers which are wiped out when released. -template -struct SecBuf { -public: - typedef typename std::vector::iterator iterator; - - typedef typename std::vector::const_iterator const_iterator; - - explicit SecBuf() : vec_(std::vector()) {} - - explicit SecBuf(size_t n, const T& value = T()) : - vec_(std::vector(n, value)) - {} - - SecBuf(iterator first, iterator last) : - vec_(std::vector(first, last)) - {} - - SecBuf(const_iterator first, const_iterator last) : - vec_(std::vector(first, last)) - {} - - SecBuf(const std::vector& 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 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 digest(size); + ossl::SecBuf 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 digest(size); + ossl::SecBuf 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 sign(size_t len) { size_t size = getOutputLength(); - SecBuf digest(size); + ossl::SecBuf 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 digest(size); + ossl::SecBuf 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: