Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CS9 OpenSSL deprecated functions warnings and thread safety #37523

Closed
smuzaffar opened this issue Apr 11, 2022 · 7 comments · Fixed by #37531
Closed

CS9 OpenSSL deprecated functions warnings and thread safety #37523

smuzaffar opened this issue Apr 11, 2022 · 7 comments · Fixed by #37531

Comments

@smuzaffar
Copy link
Contributor

For CS9 IBs (where we have OpenSSL 3.0) we have many warnings like [a]. I opened a PR #37490 to fix it. I think we can fix allof these by using EVP_Digest* function. But I just noticed that the following code is not thread safe

#if OPENSSL_API_COMPAT < 0x10100000L
  OpenSSL_add_all_digests();
  EVP_MD_CTX* mdctx = EVP_MD_CTX_create();
#else
  OPENSSL_init_crypto();
  EVP_MD_CTX* mdctx = EVP_MD_CTX_new();
#endif

Specially call to OpenSSL_add_all_digests() or OPENSSL_init_crypto() which should be call once . @makortel any idea how to achieve it properly?

[a]

src/L1TriggerConfig/Utilities/src/L1TCaloParamsViewer.cc:84:17: warning: 'int SHA1_Init(SHA_CTX*)' is deprecated: Since OpenSSL 3.0 [-Wdeprecated-declarations]
    84 |   if (!SHA1_Init(&ctx))
      |        ~~~~~~~~~^~~~~~
In file included from src/L1TriggerConfig/Utilities/src/L1TCaloParamsViewer.cc:75:
/usr/include/openssl/sha.h:49:27: note: declared here
   49 | OSSL_DEPRECATEDIN_3_0 int SHA1_Init(SHA_CTX *c);
      |                           ^~~~~~~~~
  src/L1TriggerConfig/Utilities/src/L1TCaloParamsViewer.cc:87:19: warning: 'int SHA1_Update(SHA_CTX*, const void*, size_t)' is deprecated: Since OpenSSL 3.0 [-Wdeprecated-declarations]
    87 |   if (!SHA1_Update(&ctx, buf, len))
      |        ~~~~~~~~~~~^~~~~~~~~~~~~~~~
In file included from src/L1TriggerConfig/Utilities/src/L1TCaloParamsViewer.cc:75:
/usr/include/openssl/sha.h:50:27: note: declared here
   50 | OSSL_DEPRECATEDIN_3_0 int SHA1_Update(SHA_CTX *c, const void *data, size_t len);
      |                           ^~~~~~~~~~~
  src/L1TriggerConfig/Utilities/src/L1TCaloParamsViewer.cc:91:18: warning: 'int SHA1_Final(unsigned char*, SHA_CTX*)' is deprecated: Since OpenSSL 3.0 [-Wdeprecated-declarations]
    91 |   if (!SHA1_Final(hash, &ctx))
@smuzaffar
Copy link
Contributor Author

assign core

@cmsbuild
Copy link
Contributor

New categories assigned: core

@Dr15Jones,@smuzaffar,@makortel you have been requested to review this Pull request/Issue and eventually sign? Thanks

@cmsbuild
Copy link
Contributor

A new Issue was created by @smuzaffar Malik Shahzad Muzaffar.

@Dr15Jones, @perrotta, @dpiparo, @makortel, @smuzaffar, @qliphy can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

@makortel
Copy link
Contributor

One option would be to add following kind of utility function to some package (currently unclear to me what would be the best place when taking into account dependencies)

// openssl_init.h
namespace cms {
  void openssl_init();
}

// openssl_init.cc
#include <mutex>

namespace cms {
  void openssl_init() {
    static std::once_flag flag;
    std::call_once(flag, []() {
#if OPENSSL_API_COMPAT < 0x10100000L
      OpenSSL_add_all_digests();
#else
      OPENSSL_init_crypto();
#endif
    });
  }
}

@Dr15Jones
Copy link
Contributor

Given the dependency, maybe just a whole new package? The wrapper for the XML parser is also in its own package and that package doesn't contain much.

@smuzaffar
Copy link
Contributor Author

How about Utilities/OpenSSL package and we also move newly added openssl unit tests in this package?

@makortel
Copy link
Contributor

How about Utilities/OpenSSL package and we also move newly added openssl unit tests in this package?

Sounds good to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants