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

Fix openssl3 deprecated functions #376

Merged
merged 18 commits into from
Mar 22, 2024

Conversation

lo-simon
Copy link
Contributor

@lo-simon lo-simon commented Mar 1, 2024

No description provided.

@lo-simon lo-simon changed the title Fix openssl3 desprecated functions Fix openssl3 deprecated functions Mar 1, 2024

if (EC_KEY_set_public_key_affine_coordinates(ec_key.get(), x_coordinate.get(), y_coordinate.get()))
EVP_PKEY* pkey = NULL;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to use a EVP_PKEY_ptr here and avoid the explicit EVP_PKEY_free calls below?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I had the same thought, but couldn't think how it could be done

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done using the boost.ASIO cleanup-struct idiom

}

// find the RSA private key from private key list
utility::string_t found_rsa_key(const std::vector<utility::string_t>& private_keys)
RSA_ptr rsa(EVP_PKEY_get1_RSA(private_key.get()), &RSA_free);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this function is there no difference in control flow, only the difference between these two snippets?

OpenSSL <3.0

            RSA_ptr rsa(EVP_PKEY_get1_RSA(private_key.get()), &RSA_free);
            if (!rsa)
            {
                throw jwk_exception("extract public key error: failed to load RSA key from private key");
            }
            if (bio && PEM_write_bio_RSA_PUBKEY(bio.get(), rsa.get()))

OpenSSL >=3.0

            if (bio && PEM_write_bio_PUBKEY(bio.get(), private_key.get()))

Copy link
Contributor Author

@lo-simon lo-simon Mar 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, you are right, let's change it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@@ -40,16 +45,19 @@ struct evp_pkey_cleanup
~evp_pkey_cleanup() { if (p) ::EVP_PKEY_free(p); }
};

#if OPENSSL_VERSION_NUMBER < 0x30000000L
struct ec_key_cleanup
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lo-simon I can't remember, was this cleanup-struct idiom adopted from the Boost.ASIO code?
It serves the same purpose as your usage of std::unique_ptr in nmos/jwk_utils.cpp, is that right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, it is; see the following example extracted from asio\ssl\impl\contentext.ipp

struct context::bio_cleanup
{
  BIO* p;
  ~bio_cleanup() { if (p) ::BIO_free(p); }
};

struct context::x509_cleanup
{
  X509* p;
  ~x509_cleanup() { if (p) ::X509_free(p); }
};

struct context::evp_pkey_cleanup
{
  EVP_PKEY* p;
  ~evp_pkey_cleanup() { if (p) ::EVP_PKEY_free(p); }
};

#if (OPENSSL_VERSION_NUMBER < 0x30000000L)
struct context::rsa_cleanup
{
  RSA* p;
  ~rsa_cleanup() { if (p) ::RSA_free(p); }
};

struct context::dh_cleanup
{
  DH* p;
  ~dh_cleanup() { if (p) ::DH_free(p); }
};
#endif // (OPENSSL_VERSION_NUMBER < 0x30000000L)

@garethsb
Copy link
Contributor

@lo-simon Is this tested in CI with both OpenSSL 3.0+ and with OpenSSL 1.1.1?

@lo-simon
Copy link
Contributor Author

lo-simon commented Mar 15, 2024

@lo-simon Is this tested in CI with both OpenSSL 3.0+ and with OpenSSL 1.1.1?

We kept Ubuntu14.04 to use OpenSSL 1.0.1f, so CI tests <OpenSSL 3.0 too

@lo-simon lo-simon merged commit 2b53b76 into sony:master Mar 22, 2024
11 checks passed
@lo-simon lo-simon deleted the fix-openssl3-desprecated-functions branch March 22, 2024 18:54
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

Successfully merging this pull request may close these issues.

2 participants