From: Razvan Becheriu Date: Sun, 8 May 2022 08:55:02 +0000 (+0300) Subject: [#2381] fixed initialization race X-Git-Tag: Kea-2.1.6~27 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3b0df485cf2a75392a4940ea989e8a629352a6aa;p=thirdparty%2Fkea.git [#2381] fixed initialization race --- diff --git a/src/lib/cryptolink/botan_link.cc b/src/lib/cryptolink/botan_link.cc index 714663d507..41d25b4ba2 100644 --- a/src/lib/cryptolink/botan_link.cc +++ b/src/lib/cryptolink/botan_link.cc @@ -24,7 +24,6 @@ class CryptoLinkImpl { }; CryptoLink::~CryptoLink() { - delete impl_; } /// \brief Botan implementation of RNG. @@ -56,11 +55,10 @@ private: }; void -CryptoLink::initialize() { - CryptoLink& c = getCryptoLinkInternal(); +CryptoLink::initialize(CryptoLink& c) { if (!c.impl_) { try { - c.impl_ = new CryptoLinkImpl(); + c.impl_.reset(new CryptoLinkImpl()); } catch (const Botan::Exception& ex) { isc_throw(InitializationError, "Botan error: " << ex.what()); } diff --git a/src/lib/cryptolink/cryptolink.cc b/src/lib/cryptolink/cryptolink.cc index ed5d8ab709..18033b2d74 100644 --- a/src/lib/cryptolink/cryptolink.cc +++ b/src/lib/cryptolink/cryptolink.cc @@ -15,29 +15,18 @@ namespace cryptolink { CryptoLink& CryptoLink::getCryptoLink() { - CryptoLink& c = getCryptoLinkInternal(); - if (c.impl_ == NULL) { - c.initialize(); - } - return (c); -} - -CryptoLink& -CryptoLink::getCryptoLinkInternal() { static CryptoLink instance; return (instance); } Hash* -CryptoLink::createHash(const HashAlgorithm hash_algorithm) -{ +CryptoLink::createHash(const HashAlgorithm hash_algorithm) { return (new Hash(hash_algorithm)); } HMAC* CryptoLink::createHMAC(const void* secret, size_t secret_len, - const HashAlgorithm hash_algorithm) -{ + const HashAlgorithm hash_algorithm) { return (new HMAC(secret, secret_len, hash_algorithm)); } diff --git a/src/lib/cryptolink/cryptolink.h b/src/lib/cryptolink/cryptolink.h index 41f1b620ff..a7e4a54137 100644 --- a/src/lib/cryptolink/cryptolink.h +++ b/src/lib/cryptolink/cryptolink.h @@ -36,14 +36,16 @@ enum HashAlgorithm { }; -// Forward declaration for createHash() +/// \brief Forward declaration for createHash() class Hash; -// Forward declaration for createHMAC() +/// \brief Forward declaration for createHMAC() class HMAC; -// Forward declaration for getRNG() +/// \brief Forward declaration for getRNG() class RNG; + +/// \brief Type representing the pointer to the RNG. typedef boost::shared_ptr RNGPtr; /// General exception class that is the base for all crypto-related @@ -88,8 +90,13 @@ public: CryptoLinkError(file, line, what) {} }; -/// Forward declarations for pimpl +/// \brief Forward declarations for CryptoLink pimpl. class CryptoLinkImpl; + +/// \brief Type representing the pointer to the CryptoLinkImpl. +typedef boost::shared_ptr CryptoLinkImplPtr; + +/// \brief Forward declarations for RNG pimpl. class RNGImpl; /// \brief Singleton entry point and factory class @@ -100,10 +107,7 @@ class RNGImpl; /// /// There is only one way to access it, through getCryptoLink(), which /// returns a reference to the initialized library. On the first call, -/// it will be initialized automatically. You can however initialize it -/// manually through a call to initialize(), before your first call -/// to getCryptoLink. Any subsequent call to initialize() will be a -/// noop. +/// it will be initialized automatically. /// /// In order for the CryptoLink library to be sure that the underlying /// library has been initialized, and because we do not want to add @@ -145,19 +149,6 @@ public: /// \return Reference to the singleton instance static CryptoLink& getCryptoLink(); - /// \brief Initialize the library manually - /// - /// If the library has already been initialized (either by a call - /// to initialize() or automatically in getCryptoLink()), this - /// function does nothing. - /// - /// \note A call to initialize() is not strictly necessary with - /// the current implementation. - /// - /// \exception InitializationError if initialization fails - /// - static void initialize(); - /// \brief Get version string static std::string getVersion(); @@ -223,16 +214,28 @@ public: virtual RNGPtr& getRNG(); private: - // To enable us to use an optional explicit initialization call, - // the 'real' instance getter is private - static CryptoLink& getCryptoLinkInternal(); + /// \brief Initialize the library + /// + /// If the library has already been initialized (either by a call + /// to initialize() or automatically in getCryptoLink()), this + /// function does nothing. + /// + /// \note A call to initialize() is not strictly necessary with + /// the current implementation. + /// + /// \exception InitializationError if initialization fails + /// + /// \param c the CryptoLink singleton instance which is being initialized. + void initialize(CryptoLink& c); // To prevent people constructing their own, we make the constructor // private too. - CryptoLink() : impl_(NULL) {} + CryptoLink() : impl_(NULL) { + initialize(*this); + } ~CryptoLink(); - CryptoLinkImpl* impl_; + CryptoLinkImplPtr impl_; RNGPtr rng_; }; diff --git a/src/lib/cryptolink/openssl_link.cc b/src/lib/cryptolink/openssl_link.cc index 1a573f7c41..99a2c219ab 100644 --- a/src/lib/cryptolink/openssl_link.cc +++ b/src/lib/cryptolink/openssl_link.cc @@ -22,7 +22,6 @@ class CryptoLinkImpl { }; CryptoLink::~CryptoLink() { - delete impl_; } /// \brief OpenSSL implementation of RNG. @@ -47,11 +46,10 @@ private: }; void -CryptoLink::initialize() { - CryptoLink& c = getCryptoLinkInternal(); +CryptoLink::initialize(CryptoLink& c) { if (!c.impl_) { try { - c.impl_ = new CryptoLinkImpl(); + c.impl_.reset(new CryptoLinkImpl()); } catch (const std::exception &ex) { // Should never happen isc_throw(InitializationError, diff --git a/src/lib/util/strutil.cc b/src/lib/util/strutil.cc index 7eaabdc4d5..02f1723822 100644 --- a/src/lib/util/strutil.cc +++ b/src/lib/util/strutil.cc @@ -309,6 +309,7 @@ decodeFormattedHexString(const std::string& hex_string, class StringSanitizerImpl { public: + /// @brief Constructor. StringSanitizerImpl(const std::string& char_set, const std::string& char_replacement) : char_set_(char_set), char_replacement_(char_replacement) { if (char_set.size() > StringSanitizer::MAX_DATA_SIZE) { @@ -440,7 +441,6 @@ StringSanitizer::StringSanitizer(const std::string& char_set, } StringSanitizer::~StringSanitizer() { - delete impl_; } std::string diff --git a/src/lib/util/strutil.h b/src/lib/util/strutil.h index bdb496fdb0..36fac1fb60 100644 --- a/src/lib/util/strutil.h +++ b/src/lib/util/strutil.h @@ -21,10 +21,10 @@ namespace isc { namespace util { namespace str { -/// \brief A Set of C++ Utilities for Manipulating Strings +/// @brief A Set of C++ Utilities for Manipulating Strings /// -/// \brief A standard string util exception that is thrown if getToken or +/// @brief A standard string util exception that is thrown if getToken or /// numToToken are called with bad input data /// class StringTokenError : public Exception { @@ -33,39 +33,39 @@ public: isc::Exception(file, line, what) {} }; -/// \brief Normalize Backslash +/// @brief Normalize Backslash /// /// Only relevant to Windows, this replaces all "\" in a string with "/" /// and returns the result. On other systems it is a no-op. Note /// that Windows does recognize file names with the "\" replaced by "/" /// (at least in system calls, if not the command line). /// -/// \param name Name to be substituted +/// @param name Name to be substituted void normalizeSlash(std::string& name); -/// \brief Trim Leading and Trailing Spaces +/// @brief Trim Leading and Trailing Spaces /// /// Returns a copy of the input string but with any leading or trailing spaces /// or tabs removed. /// -/// \param instring Input string to modify +/// @param instring Input string to modify /// -/// \return String with leading and trailing spaces removed +/// @return String with leading and trailing spaces removed std::string trim(const std::string& instring); -/// \brief Finds the "trimmed" end of a buffer +/// @brief Finds the "trimmed" end of a buffer /// /// Works backward from the end of the buffer, looking for the first /// character not equal to the trim value, and returns an iterator /// pointing that that position. /// -/// \param begin - Forward iterator pointing to the beginning of the +/// @param begin - Forward iterator pointing to the beginning of the /// buffer to trim -/// \param end - Forward iterator pointing to the untrimmed end of +/// @param end - Forward iterator pointing to the untrimmed end of /// the buffer to trim -/// \param trim_val - byte value to trim off +/// @param trim_val - byte value to trim off /// -/// \return Iterator pointing the first character from the end of the +/// @return Iterator pointing the first character from the end of the /// buffer not equal to the trim value template Iterator @@ -74,7 +74,7 @@ seekTrimmed(Iterator begin, Iterator end, uint8_t trim_val) { return(end); } -/// \brief Split String into Tokens +/// @brief Split String into Tokens /// /// Splits a string into tokens (the tokens being delimited by one or more of /// the delimiter characters) and returns the tokens in a vector array. Note @@ -95,90 +95,90 @@ seekTrimmed(Iterator begin, Iterator end, uint8_t trim_val) { /// We could use Boost for this, but this (simple) function eliminates one /// dependency in the code. /// -/// \param text String to be split. Passed by value as the internal copy is +/// @param text String to be split. Passed by value as the internal copy is /// altered during the processing. -/// \param delim Delimiter characters -/// \param escape Use backslash to escape delimiter characters +/// @param delim Delimiter characters +/// @param escape Use backslash to escape delimiter characters /// -/// \return Vector of tokens. +/// @return Vector of tokens. std::vector tokens(const std::string& text, const std::string& delim = std::string(" \t\n"), bool escape = false); -/// \brief Uppercase Character +/// @brief Uppercase Character /// /// Used in uppercase() to pass as an argument to std::transform(). The /// function std::toupper() can't be used as it takes an "int" as its argument; /// this confuses the template expansion mechanism because dereferencing a /// string::iterator returns a char. /// -/// \param chr Character to be upper-cased. +/// @param chr Character to be upper-cased. /// -/// \return Uppercase version of the argument +/// @return Uppercase version of the argument inline char toUpper(char chr) { return (static_cast(std::toupper(static_cast(chr)))); } -/// \brief Uppercase String +/// @brief Uppercase String /// /// A convenience function to uppercase a string. /// -/// \param text String to be upper-cased. +/// @param text String to be upper-cased. inline void uppercase(std::string& text) { std::transform(text.begin(), text.end(), text.begin(), isc::util::str::toUpper); } -/// \brief Lowercase Character +/// @brief Lowercase Character /// /// Used in lowercase() to pass as an argument to std::transform(). The /// function std::tolower() can't be used as it takes an "int" as its argument; /// this confuses the template expansion mechanism because dereferencing a /// string::iterator returns a char. /// -/// \param chr Character to be lower-cased. +/// @param chr Character to be lower-cased. /// -/// \return Lowercase version of the argument +/// @return Lowercase version of the argument inline char toLower(char chr) { return (static_cast(std::tolower(static_cast(chr)))); } -/// \brief Lowercase String +/// @brief Lowercase String /// /// A convenience function to lowercase a string /// -/// \param text String to be lower-cased. +/// @param text String to be lower-cased. inline void lowercase(std::string& text) { std::transform(text.begin(), text.end(), text.begin(), isc::util::str::toLower); } -/// \brief Apply Formatting +/// @brief Apply Formatting /// /// Given a printf-style format string containing only "%s" place holders /// (others are ignored) and a vector of strings, this produces a single string /// with the placeholders replaced. /// -/// \param format Format string -/// \param args Vector of argument strings +/// @param format Format string +/// @param args Vector of argument strings /// -/// \return Resultant string +/// @return Resultant string std::string format(const std::string& format, const std::vector& args); -/// \brief Returns one token from the given stringstream +/// @brief Returns one token from the given stringstream /// /// Using the >> operator, with basic error checking /// -/// \exception StringTokenError if the token cannot be read from the stream +/// @throw StringTokenError if the token cannot be read from the stream /// -/// \param iss stringstream to read one token from +/// @param iss stringstream to read one token from /// -/// \return the first token read from the stringstream +/// @return the first token read from the stringstream std::string getToken(std::istringstream& iss); -/// \brief Converts a string token to an *unsigned* integer. +/// @brief Converts a string token to an *unsigned* integer. /// /// The value is converted using a lexical cast, with error and bounds /// checking. @@ -193,12 +193,12 @@ std::string getToken(std::istringstream& iss); /// necessary because lexical_cast where T is an unsigned integer type /// doesn't correctly reject negative numbers when compiled with SunStudio. /// -/// \exception StringTokenError if the value is out of range, or if it -/// could not be converted +/// @throw StringTokenError if the value is out of range, or if it +/// could not be converted /// -/// \param num_token the string token to convert +/// @param num_token the string token to convert /// -/// \return the converted value, of type NumType +/// @return the converted value, of type NumType template NumType tokenToNum(const std::string& num_token) { @@ -216,7 +216,7 @@ tokenToNum(const std::string& num_token) { return (num); } -/// \brief Converts a string in quotes into vector. +/// @brief Converts a string in quotes into vector. /// /// A converted string is first trimmed. If a trimmed string is in /// quotes, the quotes are removed and the resulting string is copied @@ -229,13 +229,14 @@ tokenToNum(const std::string& num_token) { /// parsers to convert string values surrounded with quotes into /// binary form. /// -/// \param quoted_string String to be converted. -/// \return Vector containing converted string or empty string if +/// @param quoted_string String to be converted. +/// +/// @return Vector containing converted string or empty string if /// input string didn't contain expected quote characters. std::vector quotedStringToBinary(const std::string& quoted_string); -/// \brief Converts a string of separated hexadecimal digits +/// @brief Converts a string of separated hexadecimal digits /// into a vector. /// /// Octets may contain 1 or 2 digits. For example, using a colon @@ -248,29 +249,31 @@ quotedStringToBinary(const std::string& quoted_string); /// If the decoded string doesn't match any of the supported formats, /// an exception is thrown. /// -/// \param hex_string Input string. -/// \param sep character to use as a separator. -/// \param binary Vector receiving converted string into binary. -/// \throw isc::BadValue if the format of the input string is invalid. +/// @param hex_string Input string. +/// @param sep character to use as a separator. +/// @param binary Vector receiving converted string into binary. +/// +/// @throw isc::BadValue if the format of the input string is invalid. void decodeSeparatedHexString(const std::string& hex_string, const std::string& sep, std::vector& binary); -/// \brief Converts a string of hexadecimal digits with colons into +/// @brief Converts a string of hexadecimal digits with colons into /// a vector. /// /// Convenience method which calls @c decodeSeparatedHexString() passing /// in a colon for the separator. -/// \param hex_string Input string. -/// \param binary Vector receiving converted string into binary. -/// \throw isc::BadValue if the format of the input string is invalid. +/// @param hex_string Input string. +/// @param binary Vector receiving converted string into binary. +/// +/// @throw isc::BadValue if the format of the input string is invalid. void decodeColonSeparatedHexString(const std::string& hex_string, std::vector& binary); -/// \brief Converts a formatted string of hexadecimal digits into +/// @brief Converts a formatted string of hexadecimal digits into /// a vector. /// /// This function supports the following formats: @@ -284,9 +287,10 @@ decodeColonSeparatedHexString(const std::string& hex_string, /// If there is an odd number of hexadecimal digits in the input /// string, the '0' is prepended to the string before decoding. /// -/// \param hex_string Input string. -/// \param binary Vector receiving converted string into binary. -/// \throw isc::BadValue if the format of the input string is invalid. +/// @param hex_string Input string. +/// @param binary Vector receiving converted string into binary. +/// +/// @throw isc::BadValue if the format of the input string is invalid. void decodeFormattedHexString(const std::string& hex_string, std::vector& binary); @@ -294,6 +298,9 @@ decodeFormattedHexString(const std::string& hex_string, /// @brief Forward declaration to the @c StringSanitizer implementation. class StringSanitizerImpl; +/// @brief Type representing the pointer to the @c StringSanitizerImpl. +typedef boost::shared_ptr StringSanitizerImplPtr; + /// @brief Implements a regular expression based string scrubber /// /// The implementation uses C++11 regex IF the environment supports it @@ -303,7 +310,7 @@ class StringSanitizerImpl; class StringSanitizer { public: - /// Constructor + /// @brief Constructor. /// /// Compiles the given character set into a regular expression, and /// retains the given character replacement. Thereafter, the instance @@ -315,6 +322,7 @@ public: /// inversion of valid characters "[^A-Za-z0-9_-]". /// @param char_replacement string of one or more characters to use as the /// replacement for invalid characters. + /// /// @throw BadValue if given an invalid regular expression StringSanitizer(const std::string& char_set, const std::string& char_replacement); @@ -330,6 +338,7 @@ public: /// expression with the character replacement. /// /// @param original the string to scrub + /// /// @throw Unexpected if an error occurs during scrubbing std::string scrub(const std::string& original); @@ -342,15 +351,17 @@ public: private: /// @brief Pointer to the @c StringSanitizerImpl. - StringSanitizerImpl* impl_; + StringSanitizerImplPtr impl_; }; +/// @brief Type representing the pointer to the @c StringSanitizer. typedef boost::shared_ptr StringSanitizerPtr; -/// \brief Check if a string is printable +/// @brief Check if a string is printable /// -/// \param content String to check for printable characters -/// \return True if empty or contains only printable characters, False otherwise +/// @param content String to check for printable characters +/// +/// @return True if empty or contains only printable characters, False otherwise inline bool isPrintable(const std::string& content) { for (const auto& ch : content) { @@ -361,10 +372,11 @@ isPrintable(const std::string& content) { return (true); } -/// \brief Check if a byte vector is printable +/// @brief Check if a byte vector is printable +/// +/// @param content Vector to check for printable characters /// -/// \param content Vector to check for printable characters -/// \return True if empty or contains only printable characters, False otherwise +/// @return True if empty or contains only printable characters, False otherwise inline bool isPrintable(const std::vector& content) { for (const auto& ch : content) {