Skip to content

question about signing performance #339

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

Closed
wez opened this issue Jun 15, 2023 · 3 comments
Closed

question about signing performance #339

wez opened this issue Jun 15, 2023 · 3 comments

Comments

@wez
Copy link

wez commented Jun 15, 2023

In my application, a single private key is loaded and used to calculate DKIM signatures for a large number of messages.

Comparing the signing performance against a ring based implementation, the rsa crate is ~3x slower than the ring implementation. That seems to line up with the commentary in #144

Looking at the perf data for signing I was a bit surprised to see encryption and decryption both showing up
in the trace; I've included an abridged view of the relevant parts here:

--99.01%--rsa::key::RsaPrivateKey::sign
          |
           --99.01%--<rsa::pkcs1v15::Pkcs1v15Sign as rsa::traits::padding::SignatureScheme>::sign (inlined)
                     |
                      --99.01%--rsa::pkcs1v15::sign (inlined)
                                |
                                 --98.97%--rsa::algorithms::rsa::rsa_decrypt_and_check (inlined)
                                           |
                                           |--87.35%--rsa::algorithms::rsa::rsa_decrypt (inlined)
                                           |          |
                                           |           --87.11%--num_bigint_dig::biguint::BigUint::modpow
                                           |
                                            --11.62%--rsa::algorithms::rsa::rsa_encrypt (inlined)
                                                      |
                                                       --11.62%--num_bigint_dig::biguint::BigUint::modpow
                                                                 |
                                                                  --11.61%--num_bigint_dig::biguint::monty::monty_modpow

I don't really know anything about RSA in particular, so I don't know if that is expected and the terminology is a bit misleading, or is somehow a redundant sanity check that perhaps isn't needed?

I can see from the readme that making it fast is for a later phase of development, but I wonder if there are any recommendations for more efficient usage in repeated signing scenarios such as mine?

@tarcieri
Copy link
Member

The "decrypt" operation in use here is actually just the description of the private key half of the RSA cryptosystem. In this case it's being used to produce a signature. The terminology is a bit confusing (although relatively standard when describing RSA) in that you don't think of decryption as being the first operation you perform, but really the public and private key operations are reciprocals of each other.

The corresponding "encrypt" operation is the public key component. In this case it's being used to verify the signature was generated correctly.

@wez
Copy link
Author

wez commented Jun 16, 2023

Thanks; so it sounds like there is no low hanging fruit and improved performance will be part of a later phase.

@wez wez closed this as completed Jun 16, 2023
@tarcieri
Copy link
Member

Nope, those are all the core mathematical operations of RSA.

It's possible we'll be able to improve performance with a hypothetical migration to crypto-bigint

wez added a commit to KumoCorp/kumomta that referenced this issue Apr 10, 2024
We keep getting asked about
https://rustsec.org/advisories/RUSTSEC-2023-0071.html and how it impacts
kumomta.

The answer to that question is: in the default build configuration, we
use openssl's RSA signing implementation rather than that of the rsa
crate.  The reason for this is that OpenSSL's RSA implementation is due
to the performance gap between the two implementations
(RustCrypto/RSA#339). The result of this is
that the problematic code and attack vector described in the security
advisory does not apply to KumoMTA, because it is not used to compute
any signatures.

In the interest of not raising any false alarms as more and more people
perform security analyses on kumomta, this commit removes the `rsa`
crate from the build graph. In order to do so, we need to port
verification over to the openssl RSA implementation which is what this
commit does.

I look forward to a future version of the `rsa` crate being published
that has this issue resolved, and that closes the performance gap!

refs: RustCrypto/RSA#390
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

No branches or pull requests

2 participants