rust-openssl is vulnerable to the Marvin Attack

[tomato42]

I've reviewed the code responsible for doing RSA PKCS#1v1.5 decryption:
https://github.com/sfackler/rust-openssl/blob/a644ec2542473c854a02b7fe642621e813517979/openssl/src/encrypt.rs#L440-L453

and I'm pretty sure that it is vulnerable to the Marvin Attack as it will perform a jump/branch based on the error value returned from OpenSSL.

If you'd like to perform actual test for the leakage (to confirm the review and measure the size of the side-channel), I can run the test, but I'd like to ask for help in writing a test harness. Example test harnesses are available in the marvin-toolkit repo, the one for rust-crypto would most likely be the easiest one to adapt for this package.

May I also ask for assigning a CVE to this issue? As a repo owner you can create a security issue here in github and ask for a CVE assignment.

[alex]

This crate is a fairly minimal wrapper around OpenSSL. I'm not sure there's any way to branchless-ly handle the OpenSSL error stack.

As a result, I'm not sure it makes sense to consider this project as impacted, or not. Rather the underlying OpenSSL that is used is either impacted or not.

[tomato42]

This crate is a fairly minimal wrapper around OpenSSL. I'm not sure there's any way to branchless-ly handle the OpenSSL error stack.

there is:
https://github.com/tomato42/marvin-toolkit/blob/master/example/openssl/time_decrypt_legacy.c#L235-L240
you simply ignore the error stack

As a result, I'm not sure it makes sense to consider this project as impacted, or not. Rather the underlying OpenSSL that is used is either impacted or not.

unfortunately no, I've tested openssl extensively, and the OpenSSL API can be used safely, it's just very hard and unintuitive to do

[alex]

ERR_pop_to_mark is variable time in the number of errors on the stack.

[tomato42]

ERR_pop_to_mark is variable time in the number of errors on the stack.

and you base this statement on what? because I have tested it and verified that it is not

[alex]

Reading it: https://github.com/openssl/openssl/blob/master/crypto/err/err_mark.c#L42-L60

[tomato42]

If you think this comment in openssl is incorrect, feel free to raise an issue in openssl project: https://github.com/openssl/openssl/blob/master/crypto/rsa/rsa_pk1.c#L266-L272

but I have verified that the ERR_pop_to_mark doesn't have a side-channel leakage that depends on PKCS#1 padding being correct or not. If you don't believe me, feel free to either test, or debug it yourself. I don't have time to do it myself and explain line by line why it's not leaky, sorry.

[tomato42]

CVE-2024-3296 has been assigned for this issue.

[Shnatsel]

I understand a proof of concept has not yet been demonstrated for this issue, so its existence is still disputed. As such no version with a fix has been published.

I believe the best next step is to wire up this function to the marvin-toolkit test suite, and the test harness for rust-crypto could serve as an example.

[tomato42]

@Shnatsel yes, if somebody would create a test harness for rust-openssl, I should be able to rather quickly run it and provide conclusive evidence for existence of leakage.

[sfackler]

This is unambiguously a problem in OpenSSL, and has apparently been fixed since 3.2.0:

Since version 3.2.0, the EVP_PKEY_decrypt() method when used with PKCS#1 v1.5 padding doesn't return an error in case it detects an error in padding, instead it returns a pseudo-randomly generated message, removing the need of side-channel secure code from applications using OpenSSL.

[tomato42]

@sfackler it's not a problem in OpenSSL. If you use OpenSSL 3.2 with a PKCS#11 module you still have a problem

[alex]

Surely that's a problem in the PKCS#11 module then?

As I've expressed before, this problem cannot be properly solved external to OpenSSL, because all of these APIs involving manipulating the error stack, for which there are no constant time APIs.

[tomato42]

because all of these APIs involving manipulating the error stack

You have provided no evidence what so ever that this is actually happening. I have tested it in practice, and have found that openssl does manipulate error stack in a way that is safe.

So, it being an issue in applications that call the API is not just my opinion about it, it's also OpenSSL's official stance.

[alex]

Notwithstanding your inability to measure it, the branches exist.

If the standard is "can it be measured", that's fine. But no one has measured this issue with rust-openssl!

[simo5]

@alex in fairness every other user like rust-openssl we measured does show a leak, I do not think there are rel doubts we can show it.
Do you insist on having a measurement first?