]> git.ipfire.org Git - thirdparty/kea.git/commitdiff
[#2381] fixed initialization race
authorRazvan Becheriu <razvan@isc.org>
Sun, 8 May 2022 08:55:02 +0000 (11:55 +0300)
committerRazvan Becheriu <razvan@isc.org>
Fri, 20 May 2022 12:39:40 +0000 (12:39 +0000)
src/lib/cryptolink/botan_link.cc
src/lib/cryptolink/cryptolink.cc
src/lib/cryptolink/cryptolink.h
src/lib/cryptolink/openssl_link.cc
src/lib/util/strutil.cc
src/lib/util/strutil.h

index 714663d507f2d781e12b0b7c39c0622f359a4c6c..41d25b4ba243511353b3cdf973e817bf3924f366 100644 (file)
@@ -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());
         }
index ed5d8ab709712d8b14771231afa0e9ef94d1d3c2..18033b2d749f930c8c195c1d7ad4225af123407d 100644 (file)
@@ -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));
 }
 
index 41f1b620ffb9c687a0243886e026d8860c2ff005..a7e4a54137620d1c849e3adc38dd4c0ad0dca975 100644 (file)
@@ -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<RNG> 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<CryptoLinkImpl> 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_;
 };
index 1a573f7c41756552613eeab9b428656b802ba392..99a2c219abbe88bef79b5efe9e0422573d449902 100644 (file)
@@ -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,
index 7eaabdc4d53ee6485613758592432dc61bd2dffd..02f17238224961192c49cce97fbf67c38ba7c43b 100644 (file)
@@ -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
index bdb496fdb017e9c01713b265eecb213de5c15257..36fac1fb603287dc6de7b4cc3135842109cfa686 100644 (file)
@@ -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<typename Iterator>
 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<std::string> 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<char>(std::toupper(static_cast<int>(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<char>(std::tolower(static_cast<int>(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<std::string>& 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<T> 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 <typename NumType, int BitSize>
 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<uint8_t>
 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<uint8_t>& 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<uint8_t>& 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<uint8_t>& 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<StringSanitizerImpl> 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<StringSanitizer> 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<uint8_t>& content) {
     for (const auto& ch : content) {