-
Notifications
You must be signed in to change notification settings - Fork 1.9k
fix(random): Enhance security by replacing platform-specific randomizer code with Rust's bindings #4626
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
base: dev
Are you sure you want to change the base?
Conversation
* TODO consider refactoring WASM Random.cpp
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request replaces platform-specific random number generation implementations (JNI, Swift, WASM) with a unified Rust-based solution. The changes consolidate cryptographically secure random number generation into a single implementation using rand::thread_rng(), exposed to C++ via FFI. This simplifies the codebase, improves maintainability, and provides consistent error handling across all platforms.
Changes:
- Removed platform-specific random generators (JNI/Java, Swift/SecRandom, WASM/JavaScript) in favor of Rust's
rand::thread_rng() - Added Rust FFI function
crypto_random_bufferwith comprehensive error handling and tests - Updated C++ random buffer API to return status codes and handle errors properly instead of aborting/terminating
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| wasm/src/Random.cpp | Removed WASM-specific JavaScript-based random implementation |
| swift/Sources/SecRandom.m | Removed Swift-specific SecRandom implementation |
| jni/cpp/Random.cpp | Removed JNI-specific Java SecureRandom implementation |
| src/rand.h | Updated to return status codes and use TW::Random namespace |
| src/rand.cpp | Replaced platform-specific code with Rust FFI wrapper |
| src/interface/TWPrivateKey.cpp | Updated to handle random generation errors gracefully |
| src/Keystore/ScryptParameters.cpp | Added error handling for salt generation |
| src/Keystore/PBKDF2Parameters.cpp | Added error handling for salt generation |
| src/Keystore/AESParameters.cpp | Added error handling for IV generation |
| src/FIO/Encryption.cpp | Added error handling for IV generation |
| rust/tw_memory/src/ffi/mod.rs | Registered new c_byte_array_mut module |
| rust/tw_memory/src/ffi/c_byte_array_mut.rs | Added mutable FFI byte array wrapper |
| rust/tw_crypto/src/ffi/mod.rs | Registered new crypto_random module |
| rust/tw_crypto/src/ffi/crypto_random.rs | Implemented crypto_random_buffer FFI function |
| rust/tw_crypto/src/crypto_mnemonic/mnemonic.rs | Updated to use try_fill_bytes with proper error handling |
| rust/tw_crypto/src/crypto_mnemonic/error.rs | Added RandGeneratorError variant |
| rust/tw_crypto/tests/crypto_random_ffi.rs | Added comprehensive FFI tests |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Binary size comparison➡️ aarch64-apple-ios: - 15.25 MB
+ 15.25 MB +1 KB➡️ aarch64-apple-ios-sim: 15.25 MB ➡️ aarch64-linux-android: - 20.10 MB
+ 20.10 MB +1 KB➡️ armv7-linux-androideabi: 17.31 MB ➡️ wasm32-unknown-emscripten: 14.85 MB |
This pull request refactors and unifies the implementation of cryptographically secure random number generation across the codebase. It removes platform-specific random number generator code (for JNI, Swift, and WASM), and replaces it with a single Rust-backed implementation that is exposed to C++ via FFI. The new approach improves code maintainability, error handling, and testability, and ensures consistent random byte generation across all platforms.
Key changes include:
Core random number generation refactor:
jni/cpp/Random.cpp,swift/Sources/SecRandom.m, andwasm/src/Random.cpp, consolidating all random byte generation through a new Rust FFI function (crypto_random_buffer).random_bufferimplementation with a wrapper that calls the Rust FFI function, and updated the API to return a status code (RandomResult).Rust FFI and memory handling:
crypto_random_bufferin Rust (rust/tw_crypto/src/ffi/crypto_random.rs), which usesrand::thread_rng()to fill buffers securely, and defined error codes for FFI.CByteArrayMutin Rust for safe FFI buffer handling.Error handling and usage updates:
Random::random_bufferAPI and to handle errors more robustly (throwing exceptions or returning null on failure instead of aborting or terminating).Testing:
crypto_random_bufferfunction, including edge cases for buffer validity and randomness.These changes ensure a single, secure, and consistent random number generation mechanism across all supported platforms, simplify the codebase, and improve error handling for cryptographic operations.