Skip to content

Conversation

@fjarri
Copy link
Contributor

@fjarri fjarri commented Apr 23, 2025

Fixes #490

  • Bump crypto-bigint dependency to the current trunk.
  • Use the width of a single prime instead of the full modulus where possible. Brings bench_rsa_2048_pkcsv1_decrypt to pre-crypto-bigint values.
  • Add a check to RsaPrivateKey::from_components() to ensure consistency between the primes and the modulus.
  • Remove zero padding limbs from the primes and the modulus in RsaPrivateKey::from_components().
  • Add a check to rsa_decrypt() to ensure the bit precision of the ciphertext is the same as that of the modulus.

Notes:

  • bench_rsa_2048_pkcsv1_sign_blinded can be restored to the original performance by using variable-time inversion in algorithms::rsa::blind() (as it was during num-bigint times), but it seems to me that the blinding factor must be kept secret, so we have to use the constant-time inversion. This leads to about 5x slowdown compared to pre-crypto-bigint performance.
  • The changes in test vectors are due to Make random_bits_core platform independent crypto-bigint#781

Possible further improvements

@fjarri fjarri force-pushed the widths branch 3 times, most recently from cbc5789 to 7305628 Compare April 24, 2025 17:07
@fjarri fjarri marked this pull request as ready for review April 24, 2025 17:16
@tarcieri
Copy link
Member

The r^(-1) mod n was variable-time before crypto-bigint; is that secure?

Everything was variable-time before crypto-bigint, and to my knowledge it's insecure. It would probably be best to make it constant time despite the performance hit.

Hopefully we'll get better performance after a migration to bingcd

@tarcieri
Copy link
Member

  • The new try_set_precision() may be a method in BoxedUint
  • shorten/widen may need some fallible equivalents

So if I understand the goal of these, it's to prevent truncating the contents of a value, correct?

If so, adding them upstream seems fine. Perhaps they could be made constant-time?

@fjarri
Copy link
Contributor Author

fjarri commented Apr 25, 2025

Everything was variable-time before crypto-bigint, and to my knowledge it's insecure. It would probably be best to make it constant time despite the performance hit.

That's unfortunate, it's like a 5x slowdown.

So if I understand the goal of these, it's to prevent truncating the contents of a value, correct?

Yep.

@tarcieri
Copy link
Member

tarcieri commented Apr 25, 2025

@fjarri do you want to try to upstream try_set_precision and e.g. try_shorten first, or merge this as-is and follow up on that later?

@fjarri
Copy link
Contributor Author

fjarri commented Apr 25, 2025

Let me try it out, I'll see if I can distill some required functionality into general-use methods.

As for the inversion, I need to check if preparing the inverter in advance makes things any better.

@tarcieri
Copy link
Member

It’s unlikely to impact performance aside from optimizing away a single allocation

@fjarri
Copy link
Contributor Author

fjarri commented Apr 25, 2025

Converting to draft until we merge RustCrypto/crypto-bigint#809 in some form

tarcieri pushed a commit to RustCrypto/crypto-bigint that referenced this pull request Apr 27, 2025
This PR attempts to simplify some scenarios that were encountered during
the work on RustCrypto/RSA#506.

- Instead of having `BoxedUint::shorten()`/`widen()` methods, introduce
the `Resize` trait with more convenience methods. Implemented for
`BoxedUint`, `NonZero<BoxedUint>`, `Odd<BoxedUint>`, and their
references.
- `BoxedUint::shorten()` and `widen()` are marked as deprecated.

Note that `try_resize` fails if `self.bits() > at_least_bits_precision`,
unlike the more relaxed `shorten()` behavior.
@tarcieri tarcieri merged commit 96e059c into RustCrypto:master Apr 28, 2025
11 checks passed
scv35 pushed a commit to scv35/Slibs-crypto-bigint that referenced this pull request Jul 4, 2025
This PR attempts to simplify some scenarios that were encountered during
the work on RustCrypto/RSA#506.

- Instead of having `BoxedUint::shorten()`/`widen()` methods, introduce
the `Resize` trait with more convenience methods. Implemented for
`BoxedUint`, `NonZero<BoxedUint>`, `Odd<BoxedUint>`, and their
references.
- `BoxedUint::shorten()` and `widen()` are marked as deprecated.

Note that `try_resize` fails if `self.bits() > at_least_bits_precision`,
unlike the more relaxed `shorten()` behavior.
zhifenxing071346lorrainewyatt added a commit to zhifenxing071346lorrainewyatt/crypto-bigint that referenced this pull request Oct 4, 2025
This PR attempts to simplify some scenarios that were encountered during
the work on RustCrypto/RSA#506.

- Instead of having `BoxedUint::shorten()`/`widen()` methods, introduce
the `Resize` trait with more convenience methods. Implemented for
`BoxedUint`, `NonZero<BoxedUint>`, `Odd<BoxedUint>`, and their
references.
- `BoxedUint::shorten()` and `widen()` are marked as deprecated.

Note that `try_resize` fails if `self.bits() > at_least_bits_precision`,
unlike the more relaxed `shorten()` behavior.
charlizemorgenbriancox added a commit to charlizemorgenbriancox/crypto-bigint that referenced this pull request Oct 10, 2025
This PR attempts to simplify some scenarios that were encountered during
the work on RustCrypto/RSA#506.

- Instead of having `BoxedUint::shorten()`/`widen()` methods, introduce
the `Resize` trait with more convenience methods. Implemented for
`BoxedUint`, `NonZero<BoxedUint>`, `Odd<BoxedUint>`, and their
references.
- `BoxedUint::shorten()` and `widen()` are marked as deprecated.

Note that `try_resize` fails if `self.bits() > at_least_bits_precision`,
unlike the more relaxed `shorten()` behavior.
shell-rider609bn added a commit to shell-rider609bn/crypto-bigint that referenced this pull request Oct 21, 2025
This PR attempts to simplify some scenarios that were encountered during
the work on RustCrypto/RSA#506.

- Instead of having `BoxedUint::shorten()`/`widen()` methods, introduce
the `Resize` trait with more convenience methods. Implemented for
`BoxedUint`, `NonZero<BoxedUint>`, `Odd<BoxedUint>`, and their
references.
- `BoxedUint::shorten()` and `widen()` are marked as deprecated.

Note that `try_resize` fails if `self.bits() > at_least_bits_precision`,
unlike the more relaxed `shorten()` behavior.
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.

Slowdown after switch to crypto-bigint

2 participants