From 92febf38f11d12620b8d75b5f41c40c100903e08 Mon Sep 17 00:00:00 2001 From: adlee-was-taken Date: Thu, 25 Jun 2026 21:13:49 -0400 Subject: [PATCH] refactor(core,wasm): zeroized ed25519 seed decode in core; org_unwrap_key reuses it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review fixes for Task 1 (org_unwrap_key). Public signature unchanged. - Add device::extract_ed25519_seed -> Zeroizing<[u8;32]>: length-checks the slice then copy_from_slice straight into the Zeroizing buffer, so the raw seed never lands in a plain [u8;32] (Important #1). - Refactor device::sign() to call the new helper (pure refactor; device/sign roundtrip tests unchanged and green) (Important #2). - org_unwrap_key now calls relicario_core::device::extract_ed25519_seed directly; drop the duplicated local decode_device_seed helper and remove the redundant ssh-key dep from relicario-wasm (core already pins it) (Important #2). - Device-key-form finding (OpenSSH PEM, not base64; spec name matches reality) moved onto the core helper, condensed copy kept on org_unwrap_key. - Test helper keeps the private key as Zeroizing and passes &priv (derefs to &str) — no plain-String copy (Minor #3). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_014s527M917W47LDrfQ4t47g --- Cargo.lock | 1 - crates/relicario-core/src/device.rs | 35 ++++++++++++++----- crates/relicario-core/src/lib.rs | 2 +- crates/relicario-wasm/Cargo.toml | 1 - crates/relicario-wasm/src/lib.rs | 52 +++++++++-------------------- 5 files changed, 44 insertions(+), 47 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b2d222f..d352788 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2248,7 +2248,6 @@ dependencies = [ "serde", "serde-wasm-bindgen", "serde_json", - "ssh-key", "wasm-bindgen", "wasm-bindgen-test", "zeroize", diff --git a/crates/relicario-core/src/device.rs b/crates/relicario-core/src/device.rs index f2ce780..12dd42d 100644 --- a/crates/relicario-core/src/device.rs +++ b/crates/relicario-core/src/device.rs @@ -54,10 +54,20 @@ pub fn generate_keypair() -> Result<(Zeroizing, String)> { Ok((Zeroizing::new(private_pem.to_string()), public_line)) } -/// Sign data with an OpenSSH private key, returning base64 signature. -pub fn sign(private_key_openssh: &str, data: &[u8]) -> Result { - use base64::Engine; - +/// Extract the raw 32-byte ed25519 seed from an OpenSSH PEM private key. +/// +/// **Device-key form (verified against the producers):** `generate_keypair()` +/// emits the private key as an OpenSSH PEM blob — the multiline +/// `-----BEGIN OPENSSH PRIVATE KEY-----...` form produced by +/// `ssh_key::PrivateKey::to_openssh()` — NOT a base64-encoded raw-bytes string. +/// The org plan's `device_private_key_base64` parameter name was therefore +/// misleading; the design spec's `device_private_openssh` matches reality. The +/// extension SW holds this blob in WASM `DEVICE_STATE` and never exposes it to JS. +/// +/// The seed bytes are copied straight into a `Zeroizing<[u8; 32]>` — no +/// intermediate plain `[u8; 32]` of secret material is ever materialized — so +/// the seed is wiped on drop. +pub fn extract_ed25519_seed(private_key_openssh: &str) -> Result> { let private = PrivateKey::from_openssh(private_key_openssh) .map_err(|e| RelicarioError::DeviceKey(format!("parse private key: {e}")))?; @@ -67,11 +77,20 @@ pub fn sign(private_key_openssh: &str, data: &[u8]) -> Result { .ok_or_else(|| RelicarioError::DeviceKey("not an ed25519 key".into()))?; let secret_slice: &[u8] = key_data.private.as_ref(); - let secret_bytes: [u8; 32] = secret_slice - .try_into() - .map_err(|_| RelicarioError::DeviceKey("invalid key length".into()))?; + if secret_slice.len() != 32 { + return Err(RelicarioError::DeviceKey("invalid key length".into())); + } + let mut seed = Zeroizing::new([0u8; 32]); + seed.copy_from_slice(secret_slice); + Ok(seed) +} - let signing_key = SigningKey::from_bytes(&secret_bytes); +/// Sign data with an OpenSSH private key, returning base64 signature. +pub fn sign(private_key_openssh: &str, data: &[u8]) -> Result { + use base64::Engine; + + let seed = extract_ed25519_seed(private_key_openssh)?; + let signing_key = SigningKey::from_bytes(&seed); let signature = signing_key.sign(data); Ok(base64::engine::general_purpose::STANDARD.encode(signature.to_bytes())) } diff --git a/crates/relicario-core/src/lib.rs b/crates/relicario-core/src/lib.rs index 5c1e607..4dce7a7 100644 --- a/crates/relicario-core/src/lib.rs +++ b/crates/relicario-core/src/lib.rs @@ -91,7 +91,7 @@ pub mod import_lastpass; pub use import_lastpass::{parse_lastpass_csv, ImportWarning}; pub mod device; -pub use device::{fingerprint, DeviceEntry, RevokedEntry, generate_keypair, sign, verify}; +pub use device::{fingerprint, DeviceEntry, RevokedEntry, extract_ed25519_seed, generate_keypair, sign, verify}; pub mod org; pub use org::{ diff --git a/crates/relicario-wasm/Cargo.toml b/crates/relicario-wasm/Cargo.toml index 1c49e95..ffd5fae 100644 --- a/crates/relicario-wasm/Cargo.toml +++ b/crates/relicario-wasm/Cargo.toml @@ -21,7 +21,6 @@ base64 = "0.22" hex = "0.4" rand = "0.8" once_cell = "1" -ssh-key = { version = "0.6", features = ["ed25519", "std"] } [dev-dependencies] wasm-bindgen-test = "0.3" diff --git a/crates/relicario-wasm/src/lib.rs b/crates/relicario-wasm/src/lib.rs index 3a114d9..9e2b438 100644 --- a/crates/relicario-wasm/src/lib.rs +++ b/crates/relicario-wasm/src/lib.rs @@ -570,42 +570,20 @@ pub fn wasm_unwrap_recovery_qr( // ── Org vault WASM bridge ──────────────────────────────────────────────────── -/// Decode an OpenSSH ed25519 private key string into a Zeroizing 32-byte seed. -/// -/// **Key-form finding (Step 1 of the task brief):** -/// `relicario_core::device::generate_keypair()` returns the private key as an -/// OpenSSH PEM blob — the multiline `-----BEGIN OPENSSH PRIVATE KEY-----...` format, -/// the same form the `ssh_key` crate emits via `PrivateKey::to_openssh()`. It is -/// NOT a raw-bytes base64 string. The plan's `device_private_key_base64` parameter -/// name is therefore misleading; the spec name `device_private_openssh` matches -/// reality. The SW stores this blob in WASM `DEVICE_STATE` and never exposes it -/// to JS. If it were ever serialised for cross-session persistence (future work), -/// it would travel as this same PEM string. -/// -/// This mirrors the first half of `relicario_core::device::sign()`. -fn decode_device_seed(private_key_openssh: &str) -> Result, String> { - use ssh_key::PrivateKey; - let private = PrivateKey::from_openssh(private_key_openssh) - .map_err(|e| format!("parse private key: {e}"))?; - let key_data = private - .key_data() - .ed25519() - .ok_or_else(|| "not an ed25519 key".to_string())?; - let secret_slice: &[u8] = key_data.private.as_ref(); - let secret_bytes: [u8; 32] = secret_slice - .try_into() - .map_err(|_| "invalid ed25519 seed length".to_string())?; - let mut seed = Zeroizing::new([0u8; 32]); - seed.copy_from_slice(&secret_bytes); - Ok(seed) -} - /// Unwrap a member's ECIES-wrapped org master key into a session handle. /// /// `keys_blob` is the raw wrapped-key blob at `keys/.enc` in the org /// repo — produced by `relicario_core::org::wrap_org_key`. `device_private_openssh` -/// is the device's ed25519 private key in OpenSSH PEM format (held in WASM memory -/// via `crates/relicario-wasm/src/device.rs`; never exposed to JS). +/// is the device's ed25519 private key. +/// +/// **Device-key form (Step 1 finding):** the key arrives as an OpenSSH PEM blob — +/// the multiline `-----BEGIN OPENSSH PRIVATE KEY-----...` form emitted by +/// `relicario_core::device::generate_keypair()`, NOT a base64-encoded raw-bytes +/// string. The plan's `device_private_key_base64` name was misleading; the spec +/// name `device_private_openssh` matches reality. The SW holds this blob in WASM +/// `DEVICE_STATE` and never exposes it to JS. Decoding to the zeroized 32-byte +/// seed is delegated to `relicario_core::device::extract_ed25519_seed`, the same +/// path `device::sign()` uses, so the secret never lands in a plain `[u8; 32]`. /// /// The org key is held in the same Zeroizing WASM session registry as the personal /// master key; org items share the personal `.enc` AEAD format, so the returned @@ -615,7 +593,7 @@ pub fn org_unwrap_key( keys_blob: &[u8], device_private_openssh: &str, ) -> Result { - let seed = decode_device_seed(device_private_openssh) + let seed = relicario_core::device::extract_ed25519_seed(device_private_openssh) .map_err(|e| JsError::new(&format!("bad device key: {e}")))?; let org_key = relicario_core::org::unwrap_org_key(keys_blob, &seed) .map_err(|e| JsError::new(&format!("org unwrap failed: {e}")))?; @@ -637,10 +615,11 @@ mod org_tests { /// NOT base64-encoded raw bytes. The plan parameter name "device_private_key_base64" /// is misleading; the spec name "device_private_openssh" matches the actual format. /// We call the real generator here (not a hand-rolled helper) so the test exercises - /// the exact production key format and cannot silently drift. - fn test_device_keypair() -> (String, String) { + /// the exact production key format and cannot silently drift. The private key is + /// kept in its `Zeroizing` wrapper — never copied into a plain `String`. + fn test_device_keypair() -> (String, Zeroizing) { let (priv_openssh, pub_openssh) = relicario_core::generate_keypair().unwrap(); - (pub_openssh, priv_openssh.to_string()) + (pub_openssh, priv_openssh) } #[test] @@ -652,6 +631,7 @@ mod org_tests { let (pub_openssh, priv_openssh) = test_device_keypair(); let wrapped = wrap_org_key(&org_key, &pub_openssh).unwrap(); + // &priv_openssh derefs Zeroizing → &str; no plain-String copy. let handle = org_unwrap_key(&wrapped, &priv_openssh).unwrap(); // item_encrypt is native-safe: returns Vec; JsError is only reachable on // the error path, which is not exercised here.