Rust wrapper: add digest and signature crate trait implementations#10248
Rust wrapper: add digest and signature crate trait implementations#10248holtrop-wolfssl wants to merge 6 commits intowolfSSL:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds RustCrypto digest and signature crate trait implementations to the wolfCrypt Rust wrapper, enabling interoperability with the broader RustCrypto ecosystem.
Changes:
- Implement RustCrypto
digesttraits for SHA/SHA3 hashers (sha_digest.rs) and add corresponding tests. - Add RustCrypto
signaturetrait-based wrappers for ECDSA and RSA PKCS#1 v1.5, plus trait impls for Ed25519/Ed448, with new tests. - Add a build-time configuration guard for an incompatible wolfSSL RNG layout and enable new features in Cargo/Makefile.
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| wrapper/rust/wolfssl-wolfcrypt/src/sha_digest.rs | Implements RustCrypto digest traits for SHA-family hashers. |
| wrapper/rust/wolfssl-wolfcrypt/src/ecdsa.rs | Adds ECDSA signature trait wrappers (P-256/P-384/P-521). |
| wrapper/rust/wolfssl-wolfcrypt/src/rsa_pkcs1v15.rs | Adds RSA PKCS#1 v1.5 signature trait wrappers and fixed-size signature type. |
| wrapper/rust/wolfssl-wolfcrypt/src/ed25519.rs | Adds signature trait impls and exports Signature/VerifyingKey. |
| wrapper/rust/wolfssl-wolfcrypt/src/ed448.rs | Adds signature trait impls and exports Signature/VerifyingKey. |
| wrapper/rust/wolfssl-wolfcrypt/src/rsa.rs | Exposes internal key handle to crate and adds new_public_from_raw(_ex). |
| wrapper/rust/wolfssl-wolfcrypt/src/ecc.rs | Exposes internal key handle to crate for ECDSA wrappers. |
| wrapper/rust/wolfssl-wolfcrypt/src/lib.rs | Wires in new modules behind digest/signature features. |
| wrapper/rust/wolfssl-wolfcrypt/tests/test_sha_digest.rs | Tests digest::Digest behavior for SHA/SHA3 implementations. |
| wrapper/rust/wolfssl-wolfcrypt/tests/test_ecdsa.rs | Tests ECDSA signing/verifying and encoding round-trips via signature traits. |
| wrapper/rust/wolfssl-wolfcrypt/tests/test_rsa_pkcs1v15.rs | Tests RSA PKCS#1 v1.5 sign/verify and key/encoding invariants. |
| wrapper/rust/wolfssl-wolfcrypt/tests/test_ed25519.rs | Adds tests for Ed25519 signature trait interoperability. |
| wrapper/rust/wolfssl-wolfcrypt/tests/test_ed448.rs | Adds tests for Ed448 signature trait interoperability. |
| wrapper/rust/wolfssl-wolfcrypt/build.rs | Adds build-time rejection for self-referential RNG layout. |
| wrapper/rust/wolfssl-wolfcrypt/Cargo.toml | Adds digest/signature features and dependencies. |
| wrapper/rust/wolfssl-wolfcrypt/Makefile | Enables digest and signature features in default feature set. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
| // `rng->drbg = &rng->drbg_data` — a self-referential pointer. Rust | ||
| // moves values by memcpy, which would silently invalidate that pointer. | ||
| // Detect this configuration and refuse to build. | ||
| if binding.contains("drbg_data") { |
There was a problem hiding this comment.
I think this hard-fails the build for the entire crate even when the user has not enabled random/signature/rand_core and doesn't touch WC_RNG at all.
maybe I'd consider gating this on cfg!(feature = "random")
also, not sure about this but, binding.contains("drbg_data") is a raw substring match on the generated bindings any unrelated future symbol containing drbg_data would trip this check.
probably matching on the struct field in WC_RNG specifically (e.g. a regex anchored to the struct) would be more robust.
There was a problem hiding this comment.
You're right, it would break the build even without the traits enabled. I considered this ok though since any use of RNG that moves the struct memory (which Rust compiler can do at will!) will cause a problem in this regard, regardless of whether any of these features are enabled. If we only stored a pointer to the RNG instead of the C struct itself then that probably would not be an issue. Perhaps an improvement for the future.
As far as the name "drbg_data", there are no other occurrences of that in the repository. I'm hesitant to try to come up with a regular expression that anchors the "drbg_data" match to within the RNG struct since Rust grammar is definitely not a "regular" language to match against with a regular expression.
| panic!("wc_RsaFlattenPublicKey failed: {rc}"); | ||
| } | ||
| if (n_len as usize) != N || e_len == 0 || (e_len as usize) > MAX_E_LEN { | ||
| panic!("wc_RsaFlattenPublicKey failed: e_len: {e_len}, n_len: {n_len}"); |
There was a problem hiding this comment.
"wc_RsaFlattenPublicKey returned unexpected lengths: e_len: {e_len}, n_len: {{n_len}" would be a more appropriate message I think, since at this point wc_RsaFlattenPublicKey returned zero.
There was a problem hiding this comment.
Changed as suggested
| } | ||
| } | ||
|
|
||
| const MAX_E_LEN: usize = 8; |
There was a problem hiding this comment.
maybe a documentaation comment on this would be worth adding, since exponents that are longer than 8 bytes are rejected, saying that bad_func_argument I think would be the returned value in case of failure.
There was a problem hiding this comment.
This isn't a pub const so I'm not sure a documentation comment would show up, but I added one!
Description
Rust wrapper: add digest and signature crate trait implementations
Testing
Unit/CI tests
Checklist