Skip to content

Conversation

@tzerrell
Copy link

@tzerrell tzerrell commented Nov 9, 2024

Make the necessary updates to use the new RSA extern from risc0/risc0#2487

There's a bit of a sharp edge, where the guest crate call this needs to be sure to depend on risc0-circuit-bigint and to make a call directly to modpow_65537 from the rsa module of that crate (even if only in a dead code function that's never used) so the linker doesn't optimize it away. We should either document or fix this sharp edge.

@tzerrell tzerrell self-assigned this Nov 9, 2024
Comment on lines 39 to 48
let base: [u32; WIDTH_WORDS] = base.chunks(4)
.map(|word| u32::from_le_bytes(word.try_into().unwrap()))
.collect::<Vec<u32>>()
.try_into()
.unwrap();
let modulus: [u32; WIDTH_WORDS] = modulus.chunks(4)
.map(|word| u32::from_le_bytes(word.try_into().unwrap()))
.collect::<Vec<u32>>()
.try_into()
.unwrap();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let base: [u32; WIDTH_WORDS] = base.chunks(4)
.map(|word| u32::from_le_bytes(word.try_into().unwrap()))
.collect::<Vec<u32>>()
.try_into()
.unwrap();
let modulus: [u32; WIDTH_WORDS] = modulus.chunks(4)
.map(|word| u32::from_le_bytes(word.try_into().unwrap()))
.collect::<Vec<u32>>()
.try_into()
.unwrap();
let base = base.to_u32_digits();
let modulus = modulus.to_u32_digits();

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to_u32_digits does not exist in num-bigint-dig, it's only in num-bigint

Co-authored-by: Frank Laub <github@frank.laub.io>
tzerrell added a commit to risc0/risc0 that referenced this pull request Nov 12, 2024
We discovered that the approach used in #2481 was not playing well with
linking. This PR redoes the RSA acceleration wrappers to expect outside
crates to interact with an `extern "C"` API utilizing pointers to `[u32;
WIDTH_WORDS]` rather than pulling in Rust wrapping functions, moving the
work of translating bigints into u32 words to the patched crates.

Because this moves wrapping code to patched crates, the
`bigint-dig-shim` feature was dropped as unused and unnecessary.

See also related PRs #2488 and
risc0/RustCrypto-RSA#2 which will follow this
one

---------

Co-authored-by: Frank Laub <flaub@risc0.com>
@tzerrell tzerrell marked this pull request as ready for review November 12, 2024 19:33
@tzerrell tzerrell merged commit d48bf59 into risc0 Nov 12, 2024
@flaub flaub deleted the tzerrell/rsa-extern branch November 13, 2024 08:17
austinabell added a commit that referenced this pull request Dec 13, 2024
Use the new RSA extern (#2)

---------

Co-authored-by: Frank Laub <github@frank.laub.io>
Use risc0-bigint2 (#3)

* Use risc0-bigint2

* Use num-bigint-dig feature

* Update lockfile

* Update ref

* Update ref

* Update git ref
Update bigint2 impl with 4096 bit support (#4)

* update acceleration to use latest version of bigint2 (with 4096 bit support)

* bump version

* bump to 1.2
austinabell added a commit that referenced this pull request Dec 13, 2024
* Add Zirgen-based acceleration (#1)

Use the new RSA extern (#2)

---------

Co-authored-by: Frank Laub <github@frank.laub.io>
Use risc0-bigint2 (#3)

* Use risc0-bigint2

* Use num-bigint-dig feature

* Update lockfile

* Update ref

* Update ref

* Update git ref
Update bigint2 impl with 4096 bit support (#4)

* update acceleration to use latest version of bigint2 (with 4096 bit support)

* bump version

* bump to 1.2

* gate prop tests behind cfg to enable cargo risczero test
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.

3 participants