From 0e55f5ac7d98f3c852ace7b4622f40cfa60b629f Mon Sep 17 00:00:00 2001 From: Jeffrey Walton Date: Sat, 25 Mar 2017 16:38:42 -0400 Subject: [PATCH] Remove g_pAssignIntToInteger pointer, add CRYPTOPP_NO_ASSIGN_TO_INTEGER (Issue 389) This effectively decouples Integer and Public Key from the rest of the library. The change means a compile time define is used rather than a runtime pointer. It avoids the race with Issue 389. The Public Key algorithms will fail if you use them. For example, running the self tests with CRYPTOPP_NO_ASSIGN_TO_INTEGER in effect results in "CryptoPP::Exception caught: NameValuePairs: type mismatch for 'EquivalentTo', stored 'i', trying to retrieve 'N8CryptoPP7IntegerE'". The exception is expected, and the same happend when g_pAssignIntToInteger was present. --- algparam.cpp | 2 -- algparam.h | 14 +++++++----- config.h | 7 ++++++ integer.cpp | 60 ++++++++++++++++++++++++++++------------------------ integer.h | 7 +++++- 5 files changed, 54 insertions(+), 36 deletions(-) diff --git a/algparam.cpp b/algparam.cpp index b6b0b34e..ff394f21 100644 --- a/algparam.cpp +++ b/algparam.cpp @@ -9,8 +9,6 @@ NAMESPACE_BEGIN(CryptoPP) -PAssignIntToInteger g_pAssignIntToInteger = NULLPTR; - bool CombinedNameValuePairs::GetVoidValue(const char *name, const std::type_info &valueType, void *pValue) const { if (strcmp(name, "ValueNames") == 0) diff --git a/algparam.h b/algparam.h index 9e274f49..c66fe984 100644 --- a/algparam.h +++ b/algparam.h @@ -298,9 +298,11 @@ AssignFromHelperClass AssignFromHelper(T *pObject, const NameValuePairs &s // ******************************************************** -// to allow the linker to discard Integer code if not needed. -typedef bool (CRYPTOPP_API * PAssignIntToInteger)(const std::type_info &valueType, void *pInteger, const void *pInt); -CRYPTOPP_DLL extern PAssignIntToInteger g_pAssignIntToInteger; +#ifndef CRYPTOPP_NO_ASSIGN_TO_INTEGER +// Allow the linker to discard Integer code if not needed. +// Also see http://github.com/weidai11/cryptopp/issues/389. +bool AssignIntToInteger(const std::type_info &valueType, void *pInteger, const void *pInt); +#endif CRYPTOPP_DLL const std::type_info & CRYPTOPP_API IntegerTypeId(); @@ -386,8 +388,10 @@ public: void AssignValue(const char *name, const std::type_info &valueType, void *pValue) const { - // special case for retrieving an Integer parameter when an int was passed in - if (!(g_pAssignIntToInteger != NULLPTR && typeid(T) == typeid(int) && g_pAssignIntToInteger(valueType, pValue, &m_value))) +#ifndef CRYPTOPP_NO_ASSIGN_TO_INTEGER + // Special case for retrieving an Integer parameter when an int was passed in + if (!(typeid(T) == typeid(int) && AssignIntToInteger(valueType, pValue, &m_value))) +#endif { NameValuePairs::ThrowIfTypeMismatch(name, typeid(T), valueType); *reinterpret_cast(pValue) = m_value; diff --git a/config.h b/config.h index cfa3a9b6..f3f37d3e 100644 --- a/config.h +++ b/config.h @@ -103,6 +103,13 @@ // of 'b', 'o', 'h' or '.' (the last for decimal). // #define CRYPTOPP_USE_STD_SHOWBASE +// Define this if you want to decouple AlgorithmParameters and Integer +// The decoupling should make it easier for the linker to remove Integer +// related code for those who do not need Integer, and avoid a potential +// race during AssignIntToInteger pointer initialization. Also +// see http://github.com/weidai11/cryptopp/issues/389. +// #define CRYPTOPP_NO_ASSIGN_TO_INTEGER + // choose which style of sockets to wrap (mostly useful for MinGW which has both) #if !defined(NO_BERKELEY_STYLE_SOCKETS) && !defined(PREFER_BERKELEY_STYLE_SOCKETS) # define PREFER_BERKELEY_STYLE_SOCKETS diff --git a/integer.cpp b/integer.cpp index 700e8902..8df0b567 100644 --- a/integer.cpp +++ b/integer.cpp @@ -72,6 +72,12 @@ // ***************** C++ Static Initialization ******************** NAMESPACE_BEGIN(CryptoPP) +static void SetFunctionPointers(); +InitializeInteger::InitializeInteger() +{ + SetFunctionPointers(); +} + template struct NewInteger { @@ -80,54 +86,28 @@ struct NewInteger return new Integer(i); } }; - -static void SetFunctionPointers(); -bool AssignIntToInteger(const std::type_info &valueType, void *pInteger, const void *pInt); NAMESPACE_END ANONYMOUS_NAMESPACE_BEGIN -struct InitializeInteger -{ - InitializeInteger() - { - CryptoPP::SetFunctionPointers(); - CryptoPP::g_pAssignIntToInteger = (CryptoPP::PAssignIntToInteger)CryptoPP::AssignIntToInteger; - } -}; - #if HAVE_GCC_INIT_PRIORITY -const InitializeInteger s_init __attribute__ ((init_priority (CRYPTOPP_INIT_PRIORITY + 30))) = InitializeInteger(); +const CryptoPP::InitializeInteger s_init __attribute__ ((init_priority (CRYPTOPP_INIT_PRIORITY + 30))) = CryptoPP::InitializeInteger(); const CryptoPP::Integer s_zero __attribute__ ((init_priority (CRYPTOPP_INIT_PRIORITY + 31))) = CryptoPP::Integer(0L); const CryptoPP::Integer s_one __attribute__ ((init_priority (CRYPTOPP_INIT_PRIORITY + 32))) = CryptoPP::Integer(1L); const CryptoPP::Integer s_two __attribute__ ((init_priority (CRYPTOPP_INIT_PRIORITY + 33))) = CryptoPP::Integer(2L); #elif HAVE_MSC_INIT_PRIORITY #pragma warning(disable: 4075) #pragma init_seg(".CRT$XCU-030") -const InitializeInteger s_init; +const CryptoPP::InitializeInteger s_init; const CryptoPP::Integer s_zero(0L); const CryptoPP::Integer s_one(1L); const CryptoPP::Integer s_two(2L); #pragma warning(default: 4075) -#else -const InitializeInteger& s_init = CryptoPP::Singleton().Ref(); -const CryptoPP::Integer& s_zero = CryptoPP::Singleton >().Ref(); -const CryptoPP::Integer& s_one = CryptoPP::Singleton >().Ref(); -const CryptoPP::Integer& s_two = CryptoPP::Singleton >().Ref(); #endif ANONYMOUS_NAMESPACE_END // ***************** Library code ******************** NAMESPACE_BEGIN(CryptoPP) - -bool AssignIntToInteger(const std::type_info &valueType, void *pInteger, const void *pInt) -{ - if (valueType != typeid(Integer)) - return false; - *reinterpret_cast(pInteger) = *reinterpret_cast(pInt); - return true; -} - inline static int Compare(const word *A, const word *B, size_t N) { while (N--) @@ -3045,17 +3025,29 @@ Integer Integer::Power2(size_t e) const Integer &Integer::Zero() { +#if HAVE_GCC_INIT_PRIORITY || HAVE_MSC_INIT_PRIORITY return s_zero; +#else + return Singleton >().Ref(); +#endif } const Integer &Integer::One() { +#if HAVE_GCC_INIT_PRIORITY || HAVE_MSC_INIT_PRIORITY return s_one; +#else + return Singleton >().Ref(); +#endif } const Integer &Integer::Two() { +#if HAVE_GCC_INIT_PRIORITY || HAVE_MSC_INIT_PRIORITY return s_two; +#else + return Singleton >().Ref(); +#endif } bool Integer::operator!() const @@ -4749,6 +4741,18 @@ std::string IntToString(word64 value, unsigned int base) return result; } +#ifndef CRYPTOPP_NO_ASSIGN_TO_INTEGER +// Allow the linker to discard Integer code if not needed. +// Also see http://github.com/weidai11/cryptopp/issues/389. +bool AssignIntToInteger(const std::type_info &valueType, void *pInteger, const void *pInt) +{ + if (valueType != typeid(Integer)) + return false; + *reinterpret_cast(pInteger) = *reinterpret_cast(pInt); + return true; +} +#endif + NAMESPACE_END #endif diff --git a/integer.h b/integer.h index 1ab5e98c..6e639b2b 100644 --- a/integer.h +++ b/integer.h @@ -40,7 +40,12 @@ typedef SecBlock > IntegerSecBlock; //! used to track the sign of the Integer. //! \since Crypto++ 1.0 //! \nosubgrouping -class CRYPTOPP_DLL Integer : public ASN1Object +class CRYPTOPP_DLL Integer +#if HAVE_GCC_INIT_PRIORITY || HAVE_MSC_INIT_PRIORITY + : private InitializeInteger, public ASN1Object +#else + : public ASN1Object +#endif { public: //! \name ENUMS, EXCEPTIONS, and TYPEDEFS