sfackler / rust-openssl

OpenSSL bindings for Rust
1.39k stars 744 forks source link

Fixing use of OpenSSL 3 deprecated functions #2047

Open wiktor-k opened 1 year ago

wiktor-k commented 1 year ago

Hi,

I'm using rust-openssl as one of the cryptographic backends in Sequoia and just recently we've got a report that we're using functions deprecated in OpenSSL 3:

I've went through the migration guide and it seems they want to encourage the usage of EVP_PKEY family of functions and quite a bit of them are missing here (EVP_PKEY_CTX_new_from_{name,pkey}) as well as the associated machinery (OSSL_PARAM_BLD, OSSL_PARAM_BLD_push_BN, ...).

Now, before I file a big PR I'd like to verify if my approach is correct:

One way to solve this would be to just expose the missing features and mark them as ossl300 but AFAICT this crate takes extra effort to provide consistent interface regardless of which library is used underneath. I wonder if a better approach would be just to fix the implementation so that it uses new functions when ossl300 is defined and uses the old API otherwise.

An example of existing code:

impl Rsa<Public> {
    pub fn from_public_components(n: BigNum, e: BigNum) -> Result<Rsa<Public>, ErrorStack> {
        unsafe {
            let rsa = cvt_p(ffi::RSA_new())?;
            RSA_set0_key(rsa, n.as_ptr(), e.as_ptr(), ptr::null_mut());
            mem::forget((n, e));
            Ok(Rsa::from_ptr(rsa))
        }
    }

This uses deprecated RSA_set0_key function and I think it should be changed to use EVP_PKEY_CTX_new_from_name passing BigNums via OSSL_PARAM_BLD_push_BN and, at the end, extracting the Rsa object out of the PKey but only when ossl300 is defined.

Not sure if all functions can be adjusted like that but from my casual skim most of them are for creating cryptographic objects.

Please confirm if this is a good approach or if there is a better one I should explore instead.

Thank you for your time! :wave:

sfackler commented 1 year ago

AFAICT this crate takes extra effort to provide consistent interface regardless of which library is used underneath.

To the contrary - we try to provide a pretty raw interface over OpenSSL's APIs. Adding new #[cfg(ossl300)] stuff is the way to go.

Might be worth also adding a CI build against an OpenSSL built without deprecated APIs to ensure we don't regress.

wiktor-k commented 1 year ago

Understood! Thanks for the comment. I'll be back with code.