uptane / aktualizr

C++ Uptane Client
Mozilla Public License 2.0
15 stars 15 forks source link

OpenSSL 3 deprecation warnings #83

Open pattivacek opened 1 year ago

pattivacek commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

I'm on Debian 11, which still uses 1.1.1

mike-sul commented 1 year ago

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

EddyGharib commented 7 months ago

+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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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 commented 4 months ago

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 commented 4 months ago

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.