fix(wasm): zeroize transient device-private-key copy in export_state_bytes
Review fix (Important secret-hygiene finding): export_state_bytes built a PersistedDeviceState via state.signing_private.as_str().to_owned(), creating non-zeroized heap copies of the device ed25519 private keys (signing + deploy) that lingered as plaintext residue after the struct dropped. - Cargo.toml: enable zeroize "derive" feature (zeroize_derive already in lock tree) - device.rs: PersistedDeviceState now derives Zeroize + ZeroizeOnDrop, wiping all String fields on every drop path (success, serde_json::to_vec ? error path, panic) - device.rs: import_state_bytes uses std::mem::take per field (ZeroizeOnDrop adds a Drop impl, so fields can no longer be moved out — E0509); mem::take transfers the heap buffers without copying, leaving empty Strings the drop-zeroize no-ops - doc-comments updated to describe the ZeroizeOnDrop / mem::take hygiene Hygiene-only; no behavior change. Gates: cargo test -p relicario-wasm 15/15, clippy -D warnings clean, build --target wasm32-unknown-unknown clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014s527M917W47LDrfQ4t47g
This commit is contained in:
@@ -14,7 +14,7 @@ wasm-bindgen = "0.2"
|
|||||||
serde-wasm-bindgen = "0.6"
|
serde-wasm-bindgen = "0.6"
|
||||||
serde_json = "1"
|
serde_json = "1"
|
||||||
serde = { version = "1", features = ["derive"] }
|
serde = { version = "1", features = ["derive"] }
|
||||||
zeroize = "1"
|
zeroize = { version = "1", features = ["derive"] }
|
||||||
getrandom = { version = "0.2", features = ["js"] }
|
getrandom = { version = "0.2", features = ["js"] }
|
||||||
ed25519-dalek = { version = "2", features = ["rand_core"] }
|
ed25519-dalek = { version = "2", features = ["rand_core"] }
|
||||||
base64 = "0.22"
|
base64 = "0.22"
|
||||||
|
|||||||
@@ -4,7 +4,7 @@ use std::sync::Mutex;
|
|||||||
|
|
||||||
use once_cell::sync::Lazy;
|
use once_cell::sync::Lazy;
|
||||||
use serde::{Deserialize, Serialize};
|
use serde::{Deserialize, Serialize};
|
||||||
use zeroize::Zeroizing;
|
use zeroize::{Zeroize, ZeroizeOnDrop, Zeroizing};
|
||||||
|
|
||||||
use relicario_core::device as core_device;
|
use relicario_core::device as core_device;
|
||||||
|
|
||||||
@@ -30,10 +30,17 @@ struct DeviceState {
|
|||||||
///
|
///
|
||||||
/// # Security note
|
/// # Security note
|
||||||
///
|
///
|
||||||
/// This struct itself is NOT zeroed on drop. The invariant that matters is:
|
/// `ZeroizeOnDrop` wipes every `String` field (including the `signing_private` /
|
||||||
|
/// `deploy_private` device PRIVATE keys) when the struct drops, on EVERY path —
|
||||||
|
/// success, the `?` error path of `serde_json::to_vec`, and panics — so the
|
||||||
|
/// transient plaintext copy created in `export_state_bytes` leaves no residue in
|
||||||
|
/// heap memory. Because the derived `ZeroizeOnDrop` adds a `Drop` impl, fields
|
||||||
|
/// cannot be moved out (Rust E0509); `import_state_bytes` uses `mem::take` to
|
||||||
|
/// transfer the private-key Strings into `Zeroizing<String>` without copying,
|
||||||
|
/// leaving empty Strings that the drop-zeroize no-ops. The remaining invariants:
|
||||||
/// (a) `persist_device_key` in lib.rs encrypts the bytes before returning to JS,
|
/// (a) `persist_device_key` in lib.rs encrypts the bytes before returning to JS,
|
||||||
/// (b) `export_state_bytes` is NOT a `#[wasm_bindgen]` export.
|
/// (b) `export_state_bytes` is NOT a `#[wasm_bindgen]` export.
|
||||||
#[derive(Serialize, Deserialize)]
|
#[derive(Serialize, Deserialize, Zeroize, ZeroizeOnDrop)]
|
||||||
struct PersistedDeviceState {
|
struct PersistedDeviceState {
|
||||||
name: String,
|
name: String,
|
||||||
signing_private: String,
|
signing_private: String,
|
||||||
@@ -97,9 +104,15 @@ pub fn clear_device() {
|
|||||||
/// # Security
|
/// # Security
|
||||||
///
|
///
|
||||||
/// The returned bytes are **plaintext secret material** — they contain the
|
/// The returned bytes are **plaintext secret material** — they contain the
|
||||||
/// ed25519 private key in OpenSSH PEM format. The caller in `lib.rs` encrypts
|
/// ed25519 private keys (signing + deploy) in OpenSSH PEM format. The caller in
|
||||||
/// them immediately under the vault master key before returning anything to JS.
|
/// `lib.rs` encrypts them immediately under the vault master key before returning
|
||||||
/// This function **must never** be exposed as a `#[wasm_bindgen]` export.
|
/// anything to JS. This function **must never** be exposed as a `#[wasm_bindgen]`
|
||||||
|
/// export.
|
||||||
|
///
|
||||||
|
/// The transient `persisted` clone holds plaintext private-key copies, but
|
||||||
|
/// `PersistedDeviceState` derives `ZeroizeOnDrop`, so those copies are wiped when
|
||||||
|
/// `persisted` drops — on the success path, the `?` error path of
|
||||||
|
/// `serde_json::to_vec`, and on any panic — leaving no heap residue.
|
||||||
///
|
///
|
||||||
/// # Errors
|
/// # Errors
|
||||||
///
|
///
|
||||||
@@ -116,23 +129,32 @@ pub fn export_state_bytes() -> Result<Zeroizing<Vec<u8>>, String> {
|
|||||||
};
|
};
|
||||||
let bytes = serde_json::to_vec(&persisted).map_err(|e| e.to_string())?;
|
let bytes = serde_json::to_vec(&persisted).map_err(|e| e.to_string())?;
|
||||||
Ok(Zeroizing::new(bytes))
|
Ok(Zeroizing::new(bytes))
|
||||||
|
// `persisted` (and its plaintext private-key copies) is zeroized here on drop.
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Deserialize a `PersistedDeviceState` from raw bytes and repopulate
|
/// Deserialize a `PersistedDeviceState` from raw bytes and repopulate
|
||||||
/// `DEVICE_STATE`. Private key fields are re-wrapped in `Zeroizing<String>`.
|
/// `DEVICE_STATE`. Private key fields are re-wrapped in `Zeroizing<String>`.
|
||||||
///
|
///
|
||||||
|
/// # Security
|
||||||
|
///
|
||||||
|
/// `PersistedDeviceState` derives `ZeroizeOnDrop`, which adds a `Drop` impl, so
|
||||||
|
/// its fields cannot be moved out (Rust E0509). We use `mem::take` to transfer
|
||||||
|
/// each `String` into the new `DeviceState` (wrapping the private keys in
|
||||||
|
/// `Zeroizing<String>`) — this moves the heap buffer without copying, leaving an
|
||||||
|
/// empty `String` behind whose drop-zeroize is a no-op. No secret copy survives.
|
||||||
|
///
|
||||||
/// # Errors
|
/// # Errors
|
||||||
///
|
///
|
||||||
/// Returns `Err(...)` if `bytes` is not valid JSON matching `PersistedDeviceState`.
|
/// Returns `Err(...)` if `bytes` is not valid JSON matching `PersistedDeviceState`.
|
||||||
pub fn import_state_bytes(bytes: &[u8]) -> Result<(), String> {
|
pub fn import_state_bytes(bytes: &[u8]) -> Result<(), String> {
|
||||||
let persisted: PersistedDeviceState =
|
let mut persisted: PersistedDeviceState =
|
||||||
serde_json::from_slice(bytes).map_err(|e| e.to_string())?;
|
serde_json::from_slice(bytes).map_err(|e| e.to_string())?;
|
||||||
let state = DeviceState {
|
let state = DeviceState {
|
||||||
name: persisted.name,
|
name: std::mem::take(&mut persisted.name),
|
||||||
signing_private: Zeroizing::new(persisted.signing_private),
|
signing_private: Zeroizing::new(std::mem::take(&mut persisted.signing_private)),
|
||||||
signing_public: persisted.signing_public,
|
signing_public: std::mem::take(&mut persisted.signing_public),
|
||||||
deploy_private: Zeroizing::new(persisted.deploy_private),
|
deploy_private: Zeroizing::new(std::mem::take(&mut persisted.deploy_private)),
|
||||||
deploy_public: persisted.deploy_public,
|
deploy_public: std::mem::take(&mut persisted.deploy_public),
|
||||||
};
|
};
|
||||||
*DEVICE_STATE.lock().unwrap() = Some(state);
|
*DEVICE_STATE.lock().unwrap() = Some(state);
|
||||||
Ok(())
|
Ok(())
|
||||||
|
|||||||
Reference in New Issue
Block a user