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

Use the new OpenSSL 3.* API for managing EVP_PKEY objects (RSA, ECDSA) #2380

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Jakuje
Copy link
Contributor

@Jakuje Jakuje commented Mar 5, 2025

The OpenSSL 3.* users now do not have a way to use non-deprecated API by using this rust bindings, which is not sustainable in the long term as either distributions will stop building with the deprecated API or it will be eventually removed.

This is now mostly PoC on using RSA keys using the new API. It does not expose OSSL_PARAM API as that is considered fragile, therefore hidden (as discussed in #2368).

This is partially based on #2051 which was abandoned.

Fixes: #2047

@Jakuje Jakuje force-pushed the openssl-3-evp_pkey_v2 branch 2 times, most recently from c138e59 to c401e7b Compare March 5, 2025 16:53
@Jakuje Jakuje force-pushed the openssl-3-evp_pkey_v2 branch from c401e7b to 54d5669 Compare March 6, 2025 10:15
@alex
Copy link
Collaborator

alex commented Mar 6, 2025

Thanks for trying this approach. I think it's a clear improvement over just exposing arbitrary OSSL_PARAM to users.

However, I still have pretty major concerns, which really come in two buckets:

  1. We're going to have to maintain this in parallel with the existing Rsa type, because the new stuff doesn't exist on LibreSSL/BoringSSL (also because we still support OpenSSL 0.9.8 for reasons).
  2. This isn't actually any better for our users. Don't get me wrong, there's a ton of deficiencies of the RSA_* APIs, but because this library handles Rsa as immutable, we hide a lot of those deficiencies.

Indeed, the combination of these may be that it becomes incredibly difficult for users to maintain code that works on both no-compat OpenSSL 3.x and also on LibreSSL, which would be a really crappy result.

I wonder if the thing that doesn't make the most sense is to have the Rsa API simply backed by something like this on OpenSSL 3.x? That at least means end users don't have to care. (It still leaves us with the frustration of maintaining 2x codepaths.)

@Jakuje
Copy link
Contributor Author

Jakuje commented Mar 7, 2025

I wonder if the thing that doesn't make the most sense is to have the Rsa API simply backed by something like this on OpenSSL 3.x? That at least means end users don't have to care. (It still leaves us with the frustration of maintaining 2x codepaths.)

Thank you for the suggestion! I think this would be doable, even though there are some rough edges to cover. Just to make sure I understand it correctly, you suggest to implement the same api as the RSA that we have now, that will basically wrap the OSSL_PARAM operations as implemented in this PR?

The rough cases include that the Rsa API works directly with the RSA objects that is later on most of the times assigned to the PKey object. We could theoretically do the same with some abstractions (creating the RSA object, that will internally have PKey object in it and that "pops up" once it is converted to the PKey object?). The RSA API is also made in the way that it takes the BigNums passed as params to construct the resulting RSA objects, while the OSSL_PARAMS takes just a reference and copies it when the resulting object is built. This can be probably hidden too.

Let me have a look how it will turn out.

@alex
Copy link
Collaborator

alex commented Mar 9, 2025

That's the idea, basically no changes for end consumers.

It's still 2xing our code with no possibility of reducing that, which is just a massive bummer and demotivater for me.

We should probably get @sfackler's views before investing a ton of time into this.

@Jakuje
Copy link
Contributor Author

Jakuje commented Mar 12, 2025

I spent some time trying to put this together, but it looks it will require too much bending of the existing crates, reimplementing the whole ForeignTypes trait as it really requires to work on the legacy RSA object, which would be very inefficient and would not solve the goal of using the new API at all.

The whole RSA API is also based tightly around the assumptions of the old RSA API, which really do not hold for the usage through the new API (need to keep around the bignums when returning them from pkey params, no good error handling when we do not return Result, but just the bignum or panic, ...).

So given that I do not think this approach is doable (or I am not proficient in the rust enough to be able to work it out).

Given that we need to use the algorithms through the new API, I guess adding a new API probably makes more sense than trying to stay within of the old one.

Indeed, the combination of these may be that it becomes incredibly difficult for users to maintain code that works on both no-compat OpenSSL 3.x and also on LibreSSL, which would be a really crappy result.

I see only one way out from there -- implementing new API that will wrap the old API operations for older OpenSSL or forks.

For the record, this is base for adding new API that will come with OpenSSL 3.5 for handling PQC algorithms, that we need to fit in. And if we do not want to expose the OSSL_PARAMS, we will have to wrap them in some new rust structures too.

@Jakuje Jakuje force-pushed the openssl-3-evp_pkey_v2 branch from 54d5669 to 26e0a36 Compare March 13, 2025 10:56
@Jakuje Jakuje marked this pull request as ready for review March 13, 2025 10:56
Jakuje added 4 commits March 13, 2025 13:10
We discussed that this API is not well suitable for the end users
but still, it required for several operations in OpenSSL 3.* so
instead of calling to FFI for every use of this API, this
introduces simple wrappers that allow building of the params
and their usage.

Signed-off-by: Jakub Jelen <[email protected]>
The OpenSSL 3.* users now do not have a way to use non-deprecated
API by using this rust bindings, which is not sustainable in the
long term as either distributions will stop building with the
deprecated API or it will be eventually removed.

This is now mostly PoC on using RSA keys using the new API. It does
not expose OSSL_PARAM API as that is considered fragile.

This is partially based on sfackler#2051 which was abandoned.

Fixes: sfackler#2047
Signed-off-by: Jakub Jelen <[email protected]>
@Jakuje Jakuje force-pushed the openssl-3-evp_pkey_v2 branch from 26e0a36 to 6fb3d94 Compare March 13, 2025 12:12
@Jakuje Jakuje changed the title Use the new OpenSSL 3.* API for managing EVP_PKEY (RSA draft) Use the new OpenSSL 3.* API for managing EVP_PKEY objects (RSA, ECDSA) Mar 13, 2025
@teythoon
Copy link

teythoon commented Apr 9, 2025

Just wanted to chime in that we're really looking forward to using the PQC algorithms coming with OpenSSL 3.5: https://github.com/openssl/openssl/releases/tag/openssl-3.5.0

@Jakuje
Copy link
Contributor Author

Jakuje commented Apr 10, 2025

The PQC algorithms will need new definitions anyway, but could reuse some of the internals handling of the OSSL_PARAM and EVP_PKEY from here. I wanted to get back into this to see if I will be able to wrap this into the old API, but did not manage to do so 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

Successfully merging this pull request may close these issues.

Fixing use of OpenSSL 3 deprecated functions
3 participants