diff --git a/Cargo.lock b/Cargo.lock index 1550c74..135cbad 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -136,7 +136,10 @@ dependencies = [ "ed25519-bip32", "hex", "hmac", + "irpc", + "irpc-derive", "rand 0.8.6", + "secp256k1", "serde", "serde_json", "sha2", @@ -793,6 +796,15 @@ version = "0.9.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2459377285ad874054d797f3ccebf984978aa39129f6eafde5cdc8315b612f8" +[[package]] +name = "convert_case" +version = "0.10.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "633458d4ef8c78b72454de2d54fd6ab2e60f9e02be22f3c6104cdc8a4e0fceb9" +dependencies = [ + "unicode-segmentation", +] + [[package]] name = "convert_case" version = "0.11.0" @@ -1102,7 +1114,16 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4a9b99b9cbbe49445b21764dc0625032a89b145a2642e67603e1c936f5458d05" dependencies = [ - "derive_more-impl", + "derive_more-impl 1.0.0", +] + +[[package]] +name = "derive_more" +version = "2.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d751e9e49156b02b44f9c1815bcb94b984cdcc4396ecc32521c739452808b134" +dependencies = [ + "derive_more-impl 2.1.1", ] [[package]] @@ -1117,6 +1138,20 @@ dependencies = [ "unicode-xid", ] +[[package]] +name = "derive_more-impl" +version = "2.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "799a97264921d8623a957f6c3b9011f3b5492f557bbb7a5a19b7fa6d06ba8dcb" +dependencies = [ + "convert_case 0.10.0", + "proc-macro2", + "quote", + "rustc_version", + "syn", + "unicode-xid", +] + [[package]] name = "des" version = "0.8.1" @@ -1281,6 +1316,17 @@ dependencies = [ "syn", ] +[[package]] +name = "enum-assoc" +version = "1.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3ed8956bd5c1f0415200516e78ff07ec9e16415ade83c056c230d7b7ea0d55b7" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "enumflags2" version = "0.7.12" @@ -1621,11 +1667,13 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0de51e6874e94e7bf76d726fc5d13ba782deca734ff60d5bb2fb2607c7406555" dependencies = [ "cfg-if", + "js-sys", "libc", "r-efi 6.0.0", "rand_core 0.10.1", "wasip2", "wasip3", + "wasm-bindgen", ] [[package]] @@ -2078,6 +2126,12 @@ version = "2.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3d3067d79b975e8844ca9eb072e16b31c3c1c36928edf9c6789548c524d0d954" +[[package]] +name = "identity-hash" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dfdd7caa900436d8f13b2346fe10257e0c05c1f1f9e351f4f5d57c03bd5f45da" + [[package]] name = "idna" version = "1.1.0" @@ -2195,7 +2249,7 @@ dependencies = [ "crypto_box", "data-encoding", "der", - "derive_more", + "derive_more 1.0.0", "ed25519-dalek", "futures-util", "hickory-resolver", @@ -2208,7 +2262,7 @@ dependencies = [ "iroh-quinn-proto", "iroh-quinn-udp", "iroh-relay", - "n0-future", + "n0-future 0.1.3", "netdev", "netwatch", "pin-project", @@ -2246,7 +2300,7 @@ checksum = "3cd952d9e25e521d6aeb5b79f2fe32a0245da36aae3569e50f6010b38a5f0923" dependencies = [ "curve25519-dalek", "data-encoding", - "derive_more", + "derive_more 1.0.0", "ed25519-dalek", "rand_core 0.6.4", "serde", @@ -2332,7 +2386,7 @@ dependencies = [ "bytes", "cfg_aliases", "data-encoding", - "derive_more", + "derive_more 1.0.0", "hickory-resolver", "http 1.4.1", "http-body-util", @@ -2343,7 +2397,7 @@ dependencies = [ "iroh-quinn", "iroh-quinn-proto", "lru", - "n0-future", + "n0-future 0.1.3", "num_enum", "pin-project", "pkarr", @@ -2366,6 +2420,39 @@ dependencies = [ "z32", ] +[[package]] +name = "irpc" +version = "0.16.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a05799eb70acd04843c327ef939233ccf80f607d30e9ca94857ac7f3d8f18b46" +dependencies = [ + "futures-buffered", + "futures-util", + "irpc-derive", + "n0-error", + "n0-future 0.3.2", + "noq", + "postcard", + "rcgen 0.14.8", + "rustls", + "serde", + "smallvec", + "tokio", + "tokio-util", + "tracing", +] + +[[package]] +name = "irpc-derive" +version = "0.16.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "445d81dbc1eed4dab6379bf7f97d12ac28ce8e6f3f7d6660c9f333b7b5d8d03b" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "is_terminal_polyfill" version = "1.70.2" @@ -2575,6 +2662,27 @@ dependencies = [ "uuid", ] +[[package]] +name = "n0-error" +version = "1.0.0-rc.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "223e946a84aa91644507a6b7865cfebbb9a231ace499041c747ab0fd30408212" +dependencies = [ + "n0-error-macros", + "spez", +] + +[[package]] +name = "n0-error-macros" +version = "1.0.0-rc.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "565305a21e6b3bf26640ad98f05a0fda12d3ab4315394566b52a7bddb8b34828" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "n0-future" version = "0.1.3" @@ -2582,7 +2690,28 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7bb0e5d99e681ab3c938842b96fcb41bf8a7bb4bfdb11ccbd653a7e83e06c794" dependencies = [ "cfg_aliases", - "derive_more", + "derive_more 1.0.0", + "futures-buffered", + "futures-lite", + "futures-util", + "js-sys", + "pin-project", + "send_wrapper", + "tokio", + "tokio-util", + "wasm-bindgen", + "wasm-bindgen-futures", + "web-time", +] + +[[package]] +name = "n0-future" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e2ab99dfb861450e68853d34ae665243a88b8c493d01ba957321a1e9b2312bbe" +dependencies = [ + "cfg_aliases", + "derive_more 2.1.1", "futures-buffered", "futures-lite", "futures-util", @@ -2634,7 +2763,7 @@ version = "3.5.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "89b3f766e04667e6da0e181e2da4f85475d5a6513b7cf6a80bea184e224a5b42" dependencies = [ - "convert_case", + "convert_case 0.11.0", "ctor", "napi-derive-backend", "proc-macro2", @@ -2648,7 +2777,7 @@ version = "5.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0d5af30503edf933ce7377cf6d4c877a62b0f1107ea05585f1b5e430e88d5baf" dependencies = [ - "convert_case", + "convert_case 0.11.0", "proc-macro2", "quote", "semver", @@ -2768,11 +2897,11 @@ dependencies = [ "atomic-waker", "bytes", "cfg_aliases", - "derive_more", + "derive_more 1.0.0", "iroh-quinn-udp", "js-sys", "libc", - "n0-future", + "n0-future 0.1.3", "netdev", "netlink-packet-core", "netlink-packet-route 0.19.0", @@ -2836,6 +2965,68 @@ dependencies = [ "minimal-lexical", ] +[[package]] +name = "noq" +version = "1.0.0-rc.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "198b99fc085a5db1f7d259edb5ede8311e59f28cdd2687920b4313613d21a73f" +dependencies = [ + "bytes", + "cfg_aliases", + "derive_more 2.1.1", + "noq-proto", + "noq-udp", + "pin-project-lite", + "rustc-hash", + "rustls", + "socket2 0.6.4", + "thiserror 2.0.18", + "tokio", + "tokio-stream", + "tracing", + "web-time", +] + +[[package]] +name = "noq-proto" +version = "1.0.0-rc.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1ab0ac774795ce1e42a7e61266e71f3be8110210630441169ac8dda403dd23f1" +dependencies = [ + "aes-gcm", + "bytes", + "derive_more 2.1.1", + "enum-assoc", + "getrandom 0.4.2", + "identity-hash", + "lru-slab", + "rand 0.10.1", + "rand_pcg", + "ring", + "rustc-hash", + "rustls", + "rustls-pki-types", + "slab", + "sorted-index-buffer", + "thiserror 2.0.18", + "tinyvec", + "tracing", + "web-time", +] + +[[package]] +name = "noq-udp" +version = "1.0.0-rc.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b3c1520eacd33fd6b009e2e70116b05508ade51db5e0d315ff8bf6b702148c2b" +dependencies = [ + "cfg_aliases", + "libc", + "socket2 0.6.4", + "tracing", + "windows-sys 0.61.2", +] + [[package]] name = "nu-ansi-term" version = "0.50.3" @@ -3328,7 +3519,7 @@ checksum = "247dcb75747c53cc433d6d8963a064187eec4a676ba13ea33143f1c9100e754f" dependencies = [ "base64", "bytes", - "derive_more", + "derive_more 1.0.0", "futures-lite", "futures-util", "igd-next", @@ -3652,6 +3843,15 @@ version = "0.10.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "63b8176103e19a2643978565ca18b50549f6101881c443590420e4dc998a3c69" +[[package]] +name = "rand_pcg" +version = "0.10.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "caa0f4137e1c0a72f4c651489402276c8e8e1cf081f3b0ba156d2cbeef09e86a" +dependencies = [ + "rand_core 0.10.1", +] + [[package]] name = "rcgen" version = "0.13.2" @@ -4179,6 +4379,24 @@ dependencies = [ "zeroize", ] +[[package]] +name = "secp256k1" +version = "0.29.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9465315bc9d4566e1724f0fffcbcc446268cb522e60f9a27bcded6b19c108113" +dependencies = [ + "secp256k1-sys", +] + +[[package]] +name = "secp256k1-sys" +version = "0.10.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d4387882333d3aa8cb20530a17c69a3752e97837832f34f6dccc760e715001d9" +dependencies = [ + "cc", +] + [[package]] name = "self_cell" version = "1.2.2" @@ -4387,6 +4605,23 @@ dependencies = [ "windows-sys 0.61.2", ] +[[package]] +name = "sorted-index-buffer" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ea06cc588e43c632923a55450401b8f25e628131571d4e1baea1bdfdb2b5ed06" + +[[package]] +name = "spez" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c87e960f4dca2788eeb86bbdde8dd246be8948790b7618d656e68f9b720a86e8" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "spin" version = "0.9.8" diff --git a/Cargo.toml b/Cargo.toml index daa93d0..71c2fa0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,4 +11,8 @@ resolver = "2" version = "0.1.0" edition = "2021" license = "MIT OR Apache-2.0" -repository = "https://git.alk.dev/alkdev/alknet" \ No newline at end of file +repository = "https://git.alk.dev/alkdev/alknet" + +[workspace.dependencies] +irpc = "0.16" +irpc-derive = "0.16" \ No newline at end of file diff --git a/crates/alknet-secret/Cargo.toml b/crates/alknet-secret/Cargo.toml index 01f33dc..1d81b2e 100644 --- a/crates/alknet-secret/Cargo.toml +++ b/crates/alknet-secret/Cargo.toml @@ -9,6 +9,10 @@ repository.workspace = true [lib] name = "alknet_secret" +[features] +default = [] +secp256k1 = ["dep:secp256k1"] + [dependencies] bip39 = { version = "2", features = ["rand"] } ed25519-bip32 = "0.4" @@ -21,6 +25,9 @@ zeroize = { version = "1", features = ["derive"] } hmac = "0.12" rand = "0.8" base64 = "0.22" +irpc = { workspace = true } +irpc-derive = { workspace = true } +secp256k1 = { version = "0.29", optional = true } [dev-dependencies] hex = "0.4" \ No newline at end of file diff --git a/tasks/integration/phase3/secret-service/encryption-salt-kdf.md b/tasks/integration/phase3/secret-service/encryption-salt-kdf.md index d6d0026..150d97c 100644 --- a/tasks/integration/phase3/secret-service/encryption-salt-kdf.md +++ b/tasks/integration/phase3/secret-service/encryption-salt-kdf.md @@ -1,6 +1,6 @@ --- id: encryption-salt-kdf -name: Clarify and fix EncryptedData salt usage — use HKDF for key derivation or document as reserved +name: Document EncryptedData salt as reserved for future KDF-based key derivation status: pending depends_on: [spec-update-secret-service] scope: narrow @@ -20,83 +20,34 @@ The `EncryptedData` struct has a `salt` field that is generated randomly during 5. Encrypt with AES-256-GCM using the derived key + random IV 6. Store `{key_version, salt, iv, data}` as `EncryptedData` -The salt is stored but serves no purpose. This is a gap because: +The salt is stored but serves no purpose. The spec update (spec-update-secret-service) resolves this by documenting the salt as reserved for future KDF-based key rotation. In v1, the encryption key is derived directly from the seed at path `m/74'/2'/0'/0'` without a salt-based KDF. HKDF-based key derivation is deferred to Phase B. -- Without a KDF, the same derived key is used for every encryption operation (different IVs provide per-message randomness, but the key itself is static) -- Salt-based key derivation would add an additional security layer: even if the derivation path is known, the salt provides per-encryption diversity -- The `key_version` field exists for rotation but without KDF-based key derivation, there's no mechanism to rotate to a stronger key +**Decision: Option B — Document salt as reserved.** The spec update has already made this decision. This task implements Option B only. -**The spec update (spec-update-secret-service task) decides one of two paths:** +## Implementation (Option B only) -### Option A: Use HKDF for key derivation (recommended for v1) - -Replace the direct "first 32 bytes of derived key" approach with: -1. Derive master key from seed at path `m/74'/2'/0'/0'` -2. Use HKDF-SHA256 with `salt` and `info = "alknet-encryption-v{key_version}"` to derive the actual AES-256-GCM key -3. This means: same seed + same path + different salt = different AES key - -Benefits: Each encryption uses a unique derived key (even with the same master key), providing forward security and key diversity. The salt is now purposeful. - -### Option B: Document salt as reserved (Phase B) - -Keep the current approach (direct key from derivation path) and document the salt field as "reserved for future KDF-based key derivation." Add a comment explaining that v1 doesn't use the salt. - -This is simpler in v1 but defers the security improvement. - -**This task implements whichever option the spec update chooses.** If the spec says "use HKDF now," implement Option A. If it says "document as reserved," implement Option B. - -**If Option A (HKDF):** - -1. Add `hkdf` dependency to `Cargo.toml` -2. Modify `encryption::encrypt()`: - - Generate random salt (32 bytes) - - Use HKDF-SHA256 to derive AES key from: `master_key + salt + info` - - The `info` string includes the key version for forward compatibility -3. Modify `encryption::decrypt()`: - - Use HKDF-SHA256 with the stored salt to re-derive the AES key - - Decrypt ciphertext with the derived key + stored IV -4. **Backward compatibility**: Add an `EncryptedData::version` or check if salt is empty/all-zeros to detect v1 (direct key) vs v2 (HKDF) format. Or, since key_version=1 is already in use, bump key_version to 2 for HKDF-derived keys and support both in decrypt. - -**If Option B (reserved):** - -1. Add documentation/comments to `encryption.rs` and `EncryptedData` explaining that the salt is reserved for future KDF -2. Add a `// TODO(Phase B)` comment on the salt generation -3. No code behavior changes +1. Add documentation to `encryption.rs` explaining that the `salt` field in `EncryptedData` is reserved for future KDF-based key derivation (Phase B). In v1, the encryption key is derived directly from the seed at path `m/74'/2'/0'/0'` without using the salt. +2. Add a doc comment on the `EncryptedData.salt` field explaining its reserved purpose and that it is not used in v1 key derivation. +3. Add a `// TODO(Phase B): Use salt in HKDF-based key derivation` comment on the salt generation in `encrypt()`. +4. No code behavior changes — existing tests must pass unchanged. ## Acceptance Criteria -**If Option A (HKDF — recommended):** - -- [ ] `hkdf` dependency added to `Cargo.toml` -- [ ] `encrypt()` uses HKDF-SHA256 with `salt + info = "alknet-encryption-v{key_version}"` to derive AES key -- [ ] `decrypt()` uses HKDF-SHA256 with stored `salt` to re-derive AES key -- [ ] `EncryptedData` with `key_version >= 2` uses HKDF -- [ ] `EncryptedData` with `key_version == 1` uses direct key (backward compat) -- [ ] Backward compatibility: data encrypted with v1 format can still be decrypted -- [ ] `CURRENT_KEY_VERSION` bumped to 2 -- [ ] Unit test: encrypt/decrypt round-trip with HKDF (key_version 2) -- [ ] Unit test: decrypt v1-encrypted data (direct key) still works -- [ ] Unit test: different salts produce different ciphertext keys (even with same master key) -- [ ] `EncryptionKey` struct updated to carry HKDF info if needed - -**If Option B (reserved):** - -- [ ] `encryption.rs` has documentation explaining salt is reserved for future KDF -- [ ] `EncryptedData` struct has doc comment on `salt` field explaining reserved purpose +- [ ] `encryption.rs` module-level documentation explains that the salt field is reserved for future KDF-based key derivation +- [ ] `EncryptedData` struct has doc comment on `salt` field explaining reserved purpose and that it is not used in v1 key derivation - [ ] `// TODO(Phase B)` comment on salt generation in `encrypt()` -- [ ] No behavior changes — existing tests pass unchanged +- [ ] No behavior changes — all existing tests pass unchanged ## References -- docs/architecture/secret-service.md — Encryption section (after spec update) +- docs/architecture/secret-service.md — Encryption section (after spec update, which specifies "salt is reserved for future KDF-based key rotation") - crates/alknet-secret/src/encryption.rs — Current encrypt/decrypt implementation -- HKDF (RFC 5869): https://tools.ietf.org/html/rfc5869 ## Notes -> My recommendation is Option A (HKDF). It's a small amount of additional code (the `hkdf` crate is tiny and well-tested), it makes the `salt` field purposeful, and it provides per-encryption key diversity. The backward compatibility concern is manageable: decrypt based on `key_version` (v1 = direct, v2 = HKDF). +> The spec update task already decided on Option B. HKDF-based key derivation is deferred to Phase B. This task only documents the salt as reserved and adds TODO comments. -> The architect's message specifically called out: "The EncryptedData struct has a salt field but the encryption function generates a random salt per encryption without using it for key derivation. Either the salt should be used in a KDF, or the field should be documented as reserved." This task resolves that ambiguity. +> The architect's message specifically called out: "The EncryptedData struct has a salt field but the encryption function generates a random salt per encryption without using it for key derivation. Either the salt should be used in a KDF, or the field should be documented as reserved." The spec update chose "document as reserved" for v1. ## Summary diff --git a/tasks/integration/phase3/secret-service/irpc-secret-protocol-integration.md b/tasks/integration/phase3/secret-service/irpc-secret-protocol-integration.md index c328ebb..309ac06 100644 --- a/tasks/integration/phase3/secret-service/irpc-secret-protocol-integration.md +++ b/tasks/integration/phase3/secret-service/irpc-secret-protocol-integration.md @@ -16,54 +16,117 @@ The `SecretProtocol` enum in `protocol.rs` currently has a placeholder `SecretMe 1. **Local dispatch (in-process)**: `SecretServiceHandle` — async methods that directly call into `SecretServiceInner`. No serialization overhead. 2. **Remote dispatch (in-cluster)**: `SecretProtocol` irpc client — sends `SecretMessage` via mpsc (local node) or QUIC stream (remote worker). The service runs on a head node; workers request derived keys via irpc. -Per ADR-027, irpc is always a dependency in alknet-secret (not feature-gated). Per ADR-033, irpc is one dispatch backend for OperationEnv. +Per ADR-027, irpc is always-on in alknet-secret (not feature-gated). Per ADR-033, irpc is one dispatch backend for OperationEnv. -**What needs to happen:** +### irpc Crate Details -1. **irpc crate integration**: The `irpc` crate needs to be added as a dependency to `alknet-secret`. The `#[rpc_requests]` macro must be applied to `SecretProtocol` to generate `SecretMessage` with oneshot channels for the response types. +The `irpc` crate (version 0.16.0 on crates.io) provides the `#[rpc_requests]` derive macro that generates message enums with channel types. This is the same pattern used by n0/iroh projects. -2. **SecretMessage definition**: Replace `pub type SecretMessage = SecretProtocol;` with the irpc-generated message type. Each variant gets a `oneshot::Sender` for the response: - - `DeriveEd25519 { path: String, tx: oneshot::Sender }` → `SecretMessage::DeriveEd25519` - - `DeriveEncryptionKey { path: String, tx: oneshot::Sender }` - - `DeriveEthereumKey { path: String, tx: oneshot::Sender }` - - `DerivePassword { path: String, length: usize, tx: oneshot::Sender> }` - - `Encrypt { plaintext: String, key_version: u32, tx: oneshot::Sender }` - - `Decrypt { encrypted: EncryptedData, tx: oneshot::Sender }` - - `Lock { tx: oneshot::Sender<()> }` - - `Unlock { passphrase: String, tx: oneshot::Sender<()> }` +**Key irpc concepts:** +- `#[rpc_requests(message = SecretMessage)]` on the `SecretProtocol` enum generates a `SecretMessage` enum where each variant wraps the inner type in `WithChannels` +- Each variant annotated with `#[rpc(tx=oneshot::Sender)]` gets a `oneshot::Sender` channel for responses +- `Client` is the client type that can send messages locally (via mpsc) or remotely (via noq/QUIC) +- The `rpc` feature in irpc is enabled by default and includes the remote transport (postcard + noq) +- The `derive` feature in irpc enables the `#[rpc_requests]` macro - **Note**: If the `irpc` crate's `#[rpc_requests]` macro generates this automatically, use it. If irpc doesn't exist as a crate yet (it may still be in design), create the `SecretMessage` enum manually with the oneshot channels, following the pattern used by `AuthProtocol` in alknet-core. +**Current pattern in alknet-core for reference:** +- `AuthProtocol` in alknet-core (`crates/alknet-core/src/auth/auth_protocol.rs`) is currently a plain enum with synchronous methods on `AuthServiceImpl` — it does NOT use `#[rpc_requests]` yet. The alknet-core irpc feature flag exists but is empty. This is because alknet-core's irpc integration hasn't been implemented yet. +- alknet-secret should use the actual `irpc` crate with `#[rpc_requests]` since it's the first crate to do the irpc integration properly. -3. **SecretServiceHandle dispatch**: The `SecretServiceHandle` methods should dispatch through the irpc channel. When assembled locally, the handle sends `SecretMessage` variants to a `tokio::sync::mpsc` channel; a task runs `SecretServiceInner` and processes messages. This replaces the current `RwLock` pattern with an actor-model pattern. +**Workspace configuration:** +- `irpc = "0.16"` needs to be added to the workspace `Cargo.toml` `[workspace.dependencies]` section +- `alknet-secret/Cargo.toml` needs `irpc = { workspace = true }` and `irpc-derive = { workspace = true }` (the derive macro is in a separate crate) - **OR** keep the current `RwLock` for local use and add a separate `SecretServiceActor` that wraps the handle in an mpsc-based message loop. The `SecretServiceHandle` stays as the primary local API. The actor is the irpc entry point. +**irpc dependency requirements:** +- `irpc` with default features brings in `noq` (QUIC transport), `postcard` (serialization), and `tokio`. These are acceptable for alknet-secret. +- The `derive` feature is needed for `#[rpc_requests]`. - **Prefer the simpler approach**: Keep `SecretServiceHandle` with `RwLock` for direct local use (current code). Add a `SecretServiceActor` that: - - Holds a `SecretServiceHandle` - - Runs a message loop: receives `SecretMessage`, dispatches to `SecretServiceHandle` methods, sends responses through oneshot channels - - Can be spawned as a `tokio::task` for in-process irpc - - Exposes a `tokio::sync::mpsc::Sender` as the client handle +### What needs to happen -4. **irpc feature**: Per ADR-027, irpc is always-on in alknet-secret. No feature flag needed. If the `irpc` crate exists, depend on it directly. If not, the `SecretMessage` type can be defined locally following the irpc pattern. +1. **Add irpc as a workspace dependency**: Add `irpc = "0.16"` and `irpc-derive = "0.16"` to the workspace `Cargo.toml` `[workspace.dependencies]` section. Add `irpc = { workspace = true }` and `irpc-derive = { workspace = true }` to `alknet-secret/Cargo.toml`. -**Current state**: `irpc` is listed as `"0.x"` in the spec's dependencies but is not in `Cargo.toml`. The current code doesn't import irpc at all. Check whether the `irpc` crate exists in the workspace or if it needs to be defined locally. +2. **Replace `SecretMessage` type alias with irpc-generated type**: Apply `#[rpc_requests(message = SecretMessage)]` to `SecretProtocol` with appropriate `#[rpc(tx=oneshot::Sender)]` attributes on each variant. This generates: + - `SecretMessage` enum with `WithChannels` wrappers + - `Channels` impl for each variant type + - `From for SecretProtocol` impls + - `Service` and `RemoteService` impls for `SecretProtocol` -**Critical dependency**: This task cannot proceed until we know the irpc crate's API. If it doesn't exist yet, we should define `SecretMessage` manually following the same pattern as `AuthProtocol` in alknet-core (which also uses irpc behind a feature flag). +3. **Update SecretProtocol enum for irpc**: The current enum has plain variants like `DeriveEd25519 { path: String }`. With irpc's `#[wrap]` attribute, each variant gets a wrapper struct: + ```rust + #[rpc_requests(message = SecretMessage)] + #[derive(Debug, Serialize, Deserialize)] + pub enum SecretProtocol { + #[rpc(tx=oneshot::Sender)] + #[wrap(DeriveEd25519)] + DeriveEd25519 { path: String }, + + #[rpc(tx=oneshot::Sender)] + #[wrap(DeriveEncryptionKey)] + DeriveEncryptionKey { path: String }, + + #[rpc(tx=oneshot::Sender)] + #[wrap(DeriveEthereumKey)] + DeriveEthereumKey { path: String }, + + #[rpc(tx=oneshot::Sender>)] + #[wrap(DerivePassword)] + DerivePassword { path: String, length: usize }, + + #[rpc(tx=oneshot::Sender)] + #[wrap(Encrypt)] + Encrypt { plaintext: String, key_version: u32 }, + + #[rpc(tx=oneshot::Sender)] + #[wrap(Decrypt)] + Decrypt { encrypted: EncryptedData }, + + #[rpc(tx=oneshot::Sender<()>)] + #[wrap(Lock)] + Lock, + + #[rpc(tx=oneshot::Sender<()>)] + #[wrap(Unlock)] + Unlock { passphrase: String }, + } + ``` + +4. **Create SecretServiceActor**: Wrap `SecretServiceHandle` in an actor that processes `SecretMessage` variants and sends responses through the oneshot channels. The actor runs as a `tokio::task`: + ```rust + pub struct SecretServiceActor { + handle: SecretServiceHandle, + } + + impl SecretServiceActor { + pub async fn run(mut self, mut rx: tokio::sync::mpsc::Receiver) { ... } + pub fn handle(&self) -> &SecretServiceHandle { &self.handle } + } + ``` + +5. **Keep SecretServiceHandle as primary local API**: The `RwLock` pattern stays for direct in-process use. The actor wraps it for irpc dispatch. + +6. **Update public API**: Re-export `SecretMessage`, `SecretServiceActor`, and `Client` from `lib.rs`. + +7. **Handle DerivedKey serialization for irpc**: Since `DerivedKey` will become non-Clone (per `derivedkey-zeroize-security` task) and needs custom serialization that redacts `private_key`, ensure the irpc wire format works correctly. The `#[wrap]` structs and `SecretMessage` need to serialize/deserialize `DerivedKey` — since irpc uses `postcard` for remote transport, the `Serialize`/`Deserialize` impls must handle the redacted `private_key` field appropriately. For local (mpsc) transport, `DerivedKey` is sent through oneshot channels without serialization, so the redacted serialization only matters for remote transport. ## Acceptance Criteria -- [ ] `irpc` dependency added to `Cargo.toml` (or `SecretMessage` defined manually if irpc doesn't exist yet) -- [ ] `SecretMessage` enum defined with oneshot channels for each `SecretProtocol` variant's response type +- [ ] `irpc` and `irpc-derive` added as workspace dependencies in root `Cargo.toml` +- [ ] `irpc` and `irpc-derive` added to `alknet-secret/Cargo.toml` as workspace dependencies +- [ ] `SecretProtocol` enum annotated with `#[rpc_requests(message = SecretMessage)]` and `#[rpc(tx=...)]` attributes +- [ ] `SecretMessage` is no longer a type alias — it's the irpc-generated message type - [ ] `SecretServiceActor` struct that wraps `SecretServiceHandle` and processes `SecretMessage` variants - [ ] `SecretServiceActor::run()` method that spawns a message loop as a `tokio::task` -- [ ] `SecretServiceActor::handle(&self) -> mpsc::Sender` returns a client handle for sending messages -- [ ] Each `SecretMessage` variant dispatches to the corresponding `SecretServiceHandle` method +- [ ] `SecretServiceActor::spawn()` method that returns a `Client` for sending messages +- [ ] Each `SecretMessage` variant dispatches to the corresponding `SecretServiceHandle` method and sends response through oneshot channel - [ ] `SecretServiceHandle` remains the primary local API (RwLock-based, unchanged for direct use) - [ ] Unit test: `SecretServiceActor` processes `SecretMessage::Unlock` and responds successfully - [ ] Unit test: `SecretMessage::DeriveEd25519` dispatched through actor returns `DerivedKey` - [ ] Unit test: `SecretMessage::Lock` clears state and subsequent derive calls fail -- [ ] `protocol.rs` updated: `SecretMessage` is no longer a type alias, it's the irpc message type -- [ ] `lib.rs` re-exports updated to include `SecretServiceActor` and `SecretMessage` +- [ ] `protocol.rs` updated: `SecretMessage` is the irpc-generated message type, not a type alias +- [ ] `lib.rs` re-exports updated to include `SecretServiceActor` and `Client` +- [ ] `cargo test -p alknet-secret` passes with all existing tests +- [ ] `cargo clippy -p alknet-secret -- -D warnings` passes +- [ ] `cargo fmt -p alknet-secret -- --check` passes ## References @@ -72,16 +135,19 @@ Per ADR-027, irpc is always a dependency in alknet-secret (not feature-gated). P - docs/architecture/decisions/033-operationenv-irpc-call-protocol.md — ADR-033 (irpc as dispatch backend) - crates/alknet-secret/src/protocol.rs — Current SecretProtocol with placeholder SecretMessage - crates/alknet-secret/src/service.rs — SecretServiceHandle and SecretService -- crates/alknet-core/src/auth/ — AuthProtocol pattern (reference for irpc integration) +- irpc crate (crates.io v0.16) — `#[rpc_requests]` derive macro, `Client` type, `WithChannels`, `Channels` trait +- crates/alknet-core/src/auth/auth_protocol.rs — AuthProtocol pattern (reference, but note: NOT using irpc yet) ## Notes -> This is the biggest gap identified by the architect. The spec says `#[rpc_requests]` but that macro doesn't exist in the codebase yet. Check whether `irpc` is a workspace crate or an external dependency. +> The irpc crate is on crates.io at version 0.16.0. Use `irpc = "0.16"` and `irpc-derive = "0.16"` as workspace dependencies. Do NOT use a local path dependency. -> If `irpc` doesn't exist yet, create a local `SecretMessage` type following the same channel-based pattern that alknet-core uses for its irpc services. The key pattern is: each protocol variant has a corresponding message variant with a `oneshot::Sender` for the response. The service actor receives messages, processes them, and sends responses. +> The `#[rpc_requests]` macro generates: (1) a `SecretMessage` enum with `WithChannels` wrappers for each variant, (2) `Channels` impls, (3) `From` impls, (4) `Service` and `RemoteService` impls. See the irpc crate docs and examples for the exact generated code structure. > The `SecretServiceHandle` with `RwLock` should remain as the primary local API. It's simpler, faster, and works well for single-process use. The `SecretServiceActor` wraps it for irpc dispatch. This two-API pattern matches the spec's "minimal deployment (local handle) vs production deployment (irpc service)" distinction. +> Since `DerivedKey` is becoming non-Clone with redacted serialization (per `derivedkey-zeroize-security`), the irpc integration needs to handle this. For local (mpsc) transport, `DerivedKey` moves through oneshot channels without serialization — no issue. For remote (postcard) transport, `DerivedKey` needs proper Serialize/Deserialize. The custom serialization should serialize `private_key` as bytes (not redacted) for postcard since it's a binary format used for in-cluster Rust-to-Rust communication — the redaction is for JSON/debug output only. + ## Summary > To be filled on completion \ No newline at end of file diff --git a/tasks/integration/phase3/secret-service/secp256k1-ethereum-derivation.md b/tasks/integration/phase3/secret-service/secp256k1-ethereum-derivation.md index 28dacd3..4ed21a8 100644 --- a/tasks/integration/phase3/secret-service/secp256k1-ethereum-derivation.md +++ b/tasks/integration/phase3/secret-service/secp256k1-ethereum-derivation.md @@ -24,16 +24,18 @@ The Ethereum path `m/44'/60'/0'/0/0` has **unhardened** indices at positions 4 a **Implementation:** -1. Add `libsecp256k1` as an optional dependency behind a `secp256k1` feature flag: +1. Add the `secp256k1` crate (Rust bindings to libsecp256k1) as an optional dependency behind a `secp256k1` feature flag: ```toml [features] - secp256k1 = ["dep:libsecp256k1"] + secp256k1 = ["dep:secp256k1"] [dependencies] - libsecp256k1 = { version = "0.7", optional = true } + secp256k1 = { version = "0.29", optional = true } ``` + **Note**: The Rust crate is named `secp256k1` on crates.io (it wraps the C library `libsecp256k1`). Do not use `libsecp256k1` — that is the C library name, not the Rust crate name. + 2. Add a `ethereum.rs` module (behind `secp256k1` feature flag) that implements BIP-0032 secp256k1 derivation: - `derive_secp256k1_master_key(seed: &[u8]) -> Result` - `derive_secp256k1_path(seed: &[u8], path: &str]) -> Result` @@ -53,7 +55,7 @@ The Ethereum path `m/44'/60'/0'/0/0` has **unhardened** indices at positions 4 a ## Acceptance Criteria -- [ ] `libsecp256k1` dependency added behind `secp256k1` feature flag in `Cargo.toml` +- [ ] `secp256k1` crate (Rust bindings to libsecp256k1) added behind `secp256k1` feature flag in `Cargo.toml` - [ ] `ethereum.rs` module added (behind `secp256k1` feature flag) with BIP-0032 secp256k1 derivation - [ ] `derive_secp256k1_master_key()` uses HMAC-SHA512 with "Bitcoin seed" key - [ ] `derive_secp256k1_path()` supports both hardened and unhardened indices @@ -82,7 +84,7 @@ The Ethereum path `m/44'/60'/0'/0/0` has **unhardened** indices at positions 4 a > This task should be done after `derivedkey-zeroize-security` since `derive_ethereum_key` returns `DerivedKey` which will have the zeroize changes applied. -> The `secp256k1` crate in Rust is `libsecp256k1` (crate name `secp256k1`). The `ed25519-bip32` crate handles SLIP-0010 Ed25519. These are different algorithms and must not be mixed. +> The `secp256k1` crate on crates.io (version 0.29+) provides Rust bindings to the C library `libsecp256k1`. The `ed25519-bip32` crate handles SLIP-0010 Ed25519. These are different algorithms and must not be mixed. > For the Ethereum public key: BIP-0032 secp256k1 produces 33-byte compressed public keys. The `public_key` field in `DerivedKey` is `Vec`, so 33 bytes is fine. Document this size difference from Ed25519 (32-byte public keys).