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

OpenSSL 3 deprecation warnings #83

Open
pattivacek opened this issue Jul 8, 2022 · 10 comments
Open

OpenSSL 3 deprecation warnings #83

pattivacek opened this issue Jul 8, 2022 · 10 comments

Comments

@pattivacek
Copy link
Collaborator

I recently updated my OS to 22.04, which uses openssl-3 by default. That's excellent, but we get a bunch of deprecation warnings now, such as:

/home/patti/Code/aktualizr/src/libaktualizr/crypto/p11engine.h: In destructor ‘virtual P11Engine::~P11Engine()’:
/home/patti/Code/aktualizr/src/libaktualizr/crypto/p11engine.h:56:20: warning: ‘int ENGINE_finish(ENGINE*)’ is deprecated: Since OpenSSL 3.0 [-Wdeprecated-declarations]
   56 |       ENGINE_finish(ssl_engine_);
      |       ~~~~~~~~~~~~~^~~~~~~~~~~~~
/home/patti/Code/aktualizr/src/libaktualizr/crypto/p11engine.h:57:18: warning: ‘int ENGINE_free(ENGINE*)’ is deprecated: Since OpenSSL 3.0 [-Wdeprecated-declarations]
   57 |       ENGINE_free(ssl_engine_);
      |       ~~~~~~~~~~~^~~~~~~~~~~~~
/home/patti/Code/aktualizr/src/libaktualizr/crypto/crypto.cc: In static member function ‘static std::string Crypto::RSAPSSSign(ENGINE*, const string&, const string&)’:
/home/patti/Code/aktualizr/src/libaktualizr/crypto/crypto.cc:143:33: warning: ‘void RSA_free(RSA*)’ is deprecated: Since OpenSSL 3.0 [-Wdeprecated-declarations]
  143 |   StructGuard<RSA> rsa(nullptr, RSA_free);
      |                                 ^~~~~~~~
/home/patti/Code/aktualizr/src/libaktualizr/crypto/crypto.cc:146:38: warning: ‘EVP_PKEY* ENGINE_load_private_key(ENGINE*, const char*, UI_METHOD*, void*)’ is deprecated: Since OpenSSL 3.0 [-Wdeprecated-declarations]
  146 |     key.reset(ENGINE_load_private_key(engine, private_key.c_str(), nullptr, nullptr));
      |               ~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/patti/Code/aktualizr/src/libaktualizr/crypto/crypto.cc:153:32: warning: ‘rsa_st* EVP_PKEY_get1_RSA(EVP_PKEY*)’ is deprecated: Since OpenSSL 3.0 [-Wdeprecated-declarations]
  153 |     rsa.reset(EVP_PKEY_get1_RSA(key.get()));
      |               ~~~~~~~~~~~~~~~~~^~~~~~~~~~~
/home/patti/Code/aktualizr/src/libaktualizr/crypto/crypto.cc:174:48: warning: ‘const RSA_METHOD* RSA_PKCS1_OpenSSL()’ is deprecated: Since OpenSSL 3.0 [-Wdeprecated-declarations]
  174 |     RSA_set_method(rsa.get(), RSA_PKCS1_OpenSSL());
      |                               ~~~~~~~~~~~~~~~~~^~
/home/patti/Code/aktualizr/src/libaktualizr/crypto/crypto.cc:174:19: warning: ‘int RSA_set_method(RSA*, const RSA_METHOD*)’ is deprecated: Since OpenSSL 3.0 [-Wdeprecated-declarations]
  174 |     RSA_set_method(rsa.get(), RSA_PKCS1_OpenSSL());
      |     ~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/patti/Code/aktualizr/src/libaktualizr/crypto/crypto.cc:178:60: warning: ‘int RSA_size(const RSA*)’ is deprecated: Since OpenSSL 3.0 [-Wdeprecated-declarations]
  178 |   const auto sign_size = static_cast<unsigned int>(RSA_size(rsa.get()));
      |                                                    ~~~~~~~~^~~~~~~~~~~
/home/patti/Code/aktualizr/src/libaktualizr/crypto/crypto.cc:183:41: warning: ‘int RSA_padding_add_PKCS1_PSS(RSA*, unsigned char*, const unsigned char*, const EVP_MD*, int)’ is deprecated: Since OpenSSL 3.0 [-Wdeprecated-declarations]
  183 |   int status = RSA_padding_add_PKCS1_PSS(rsa.get(), EM.get(), reinterpret_cast<const unsigned char *>(digest.c_str()),
      |                ~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  184 |                                          EVP_sha256(), -1 /* maximum salt length*/);
      |                                          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/openssl/rsa.h:428:5: note: declared here
  428 | int RSA_padding_add_PKCS1_PSS(RSA *rsa, unsigned char *EM,
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~
/home/patti/Code/aktualizr/src/libaktualizr/crypto/crypto.cc:191:40: warning: ‘int RSA_size(const RSA*)’ is deprecated: Since OpenSSL 3.0 [-Wdeprecated-declarations]
/home/patti/Code/aktualizr/src/libaktualizr/crypto/crypto.cc:191:31: warning: ‘int RSA_private_encrypt(int, const unsigned char*, unsigned char*, RSA*, int)’ is deprecated: Since OpenSSL 3.0 [-Wdeprecated-declarations]
  191 |   status = RSA_private_encrypt(RSA_size(rsa.get()), EM.get(), pSignature.get(), rsa.get(), RSA_NO_PADDING);
      |            ~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/patti/Code/aktualizr/src/libaktualizr/crypto/crypto.cc: In static member function ‘static bool Crypto::RSAPSSVerify(const string&, const string&, const string&)’:
/home/patti/Code/aktualizr/src/libaktualizr/crypto/crypto.cc:215:33: warning: ‘void RSA_free(RSA*)’ is deprecated: Since OpenSSL 3.0 [-Wdeprecated-declarations]
  215 |   StructGuard<RSA> rsa(nullptr, RSA_free);
      |                                 ^~~~~~~~
/home/patti/Code/aktualizr/src/libaktualizr/crypto/crypto.cc:220:32: warning: ‘RSA* PEM_read_bio_RSA_PUBKEY(BIO*, RSA**, int (*)(char*, int, int, void*), void*)’ is deprecated: Since OpenSSL 3.0 [-Wdeprecated-declarations]
  220 |     if (PEM_read_bio_RSA_PUBKEY(bio.get(), &r, nullptr, nullptr) == nullptr) {
      |         ~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

... and so on. There are also a couple boost errors I'm seeing (with 1.74.0) like this:

/usr/include/boost/bind.hpp:36:1: note: ‘#pragma message: The practice of declaring the Bind placeholders (_1, _2, ...) in the global namespace is deprecated. Please use <boost/bind/bind.hpp> + using namespace boost::placeholders, or define BOOST_BIND_GLOBAL_PLACEHOLDERS to retain the current behavior.’
   36 | BOOST_PRAGMA_MESSAGE(
      | ^~~~~~~~~~~~~~~~~~~~
[144/190] Building CXX object src/sota_tools/CMakeFiles/sota_tools_lib.dir/server_credentials.cc.o
@pattivacek
Copy link
Collaborator Author

Actually, these might be worse than just warnings. I'm getting consistent failures in PKCS12_parse that are preventing me from provisioning. @cajun-rat @mike-sul have you seen this?

@cajun-rat
Copy link
Collaborator

I'm on Debian 11, which still uses 1.1.1

@mike-sul
Copy link
Collaborator

Our dev&test containers/env is still on OpenSSL 1.1.1f.

@EddyGharib
Copy link

+1 Same issue on arch linux
An add_definitions(-Wno-deprecated-declarations) to the project solves the declaration warnings momentary

I think the best way to solve this problem is to force users of libp11 to install it as an engine on their system, and configure openssl to use it. Then using the high level EVP API from OpenSSL should be fine. That would reduce the complexity of the project. Not sure if it works but worth a try

@cajun-rat
Copy link
Collaborator

I'm on Debian 11, which still uses 1.1.1

I've just upgraded to Debian 12, which does use OpenSSL 3 :( Turning off the warnings-as-errors gives me a build that fails quite a lot of unit tests, so I agree, there is something bigger that needs looking into here.

@cajun-rat
Copy link
Collaborator

I found the OpenSSL 3 migration guide . It looks like a good chunk of crypto.cc will need updating to move away from the deprecated APIs, but on the bright side at least most of the crypto stuff is already behind a wrapper.

@cajun-rat
Copy link
Collaborator

I think I've figured out why removing the warning about deprecated declarations isn't enough. OpenSSL now doesn't load 'legacy' providers by default. This rather horrible static-initialization hack on the end of crypto.cc gets most of the tests passing on my machine.

class CryptoOpenSSlInit {
  public:
    CryptoOpenSSlInit() {
      OSSL_PROVIDER *legacy = OSSL_PROVIDER_try_load(NULL, "legacy", 1);
      if (legacy == nullptr) {
        std::cout << "Warning: could not load 'legacy' OpenSSL provider";
      }
      OSSL_PROVIDER *default_p = OSSL_PROVIDER_try_load(NULL, "default", 1);
      if (default_p == nullptr) {
        std::cout << "Warning: could not load 'default' OpenSSL provider";
      }
    }
};


CryptoOpenSSlInit const CryptoIniter{};

I'm still thinking that biting the bullet and re-writing crypto.cc to use the new APIs is the better solution.

@pattivacek
Copy link
Collaborator Author

I'm still thinking that biting the bullet and re-writing crypto.cc to use the new APIs is the better solution.

Yeah, I agree. Your trick looks fine for a dirty hack but that's not The Way.

@cajun-rat
Copy link
Collaborator

I think we might have a problem supporting key storage in PKCS#11 and not using the deprecated APIs. The cause is that the 'engine' APIs are used to talk to the HSM via PKCS#11, and the replacement 'provider' APIs would need an alternative to the 'PKCS#11 engine library' that talks the provider API rather than the engine API. There are such things on Github (example) but I can't find one in Debian stable right now.

I think the option space is:

  1. Tidy up the code but keep it compatible with openssl1.1 and 3. Build the relevant source files with -Wno-deprecated-declarations.
  2. Use the new APIs only when BUILD_P11 is disabled and openssl 3 is available, and disable the warnings in the otherwise.
  3. Pull in one of those openssl engine libraries and build it as part of Aktualizr

Right now I think we should go with the first. I'm not a big fan of differing code paths under #defines (option 2), and building 3rd party libraries is an upgrade treadmill.

@pattivacek
Copy link
Collaborator Author

I agree. If the deprecated APIs still work, there's no urgency to find a new solution, and my memory of the initial PKCS#11 support was that it was quite tedious and took a lot of effort to get working just right. I wouldn't want to go through that again with a library that might disappear or get replaced by something better down the line. Obviously using deprecated APIs isn't great but I think the ecosystem hasn't quite caught up yet.

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

No branches or pull requests

4 participants