docs(architecture): resolve review #002 remaining Tier 4 findings
Add ADR-026 (vault key model — HD derivation) recording the foundational HD-derivation decision, 74' coin type reservation, SLIP-0010/Ed25519 default, secp256k1 feature-gating, and AES-256-GCM cipher choice. These were previously inline rationale with no ADR (W9). Extend ADR-018 with an explicit EncryptedData wire format lock — fields, encoding, and semantics are frozen; no removal without a format-version migration (W10). Resolve the remaining guard clauses and spec decisions: - W2: Capabilities must be immutable after construction (no interior mutability). Makes the Arc vs deep-copy clone semantics genuinely two-way. - W5: Published to_* specs are compatibility contracts — best-effort mappings are two-way before first publication, one-way after. Version generated specs. - W6: Salt field clarification — v2 salt is permanently unused; a future KDF is a different derivation family, not a version-indexed path; the field saves a wire-format change only. - W7: unlock_new returns Zeroizing<String> — the mnemonic is the root of trust and must not linger in freed memory. - W17: OQ-09 WASM — server-side dispatch door is honestly closed (Connection is concrete, tokio-bound), not implicitly preserved. - W18: OQ-10 git — composability fork (raw smart protocol vs call-protocol projection) is a separate decision from ERC721 scope. - W20: from_openapi must prefix imported error codes (HTTP_404) to avoid collision with protocol-level codes (NOT_FOUND). Normative rule, not naming convention. - W21: ScopedOperationEnv field is private — construction via new()/ empty(), query via allows(). Makes the future subgraph refactor non-breaking. - C13: Connection::set_identity — the endpoint does not read identity() after handle() returns (Connection is moved into the spawned task). Observability is handler-side logging. Simplest honest answer. - W1: OperationAdapter trait is async, returns Vec<HandlerRegistration>. from_call requires async discovery; ADR-022 changed the return type. - W11: CompositionAuthority::as_identity() defined — constructs a synthetic Identity (label as id, scopes, resources) not resolvable via IdentityProvider. Second Identity construction path, acknowledged. - W14: SecretKey is iroh::SecretKey (Ed25519) — consistent with the endpoint's iroh dependency. - W19: Grandchild abort propagation is inherit-by-default (option a) — invoke() with no explicit policy inherits parent's policy. ContinueRunning auto-propagates to grandchildren unless explicitly overridden.
This commit is contained in:
@@ -510,6 +510,7 @@ The `Capabilities` type holds non-serializable, zeroized secret material. It doe
|
||||
- Capabilities hold secret material that does not implement `Serialize` and does not appear in `EventEnvelope` payloads.
|
||||
- The call protocol carries no secret material. See [call-protocol.md](call-protocol.md) for the wire-level constraint.
|
||||
- **Capabilities are `Clone` and cloned through composition.** `OperationEnv::invoke()` calls `parent.capabilities.clone()` to pass capabilities to nested calls. This is intentional: a child handler needs the same outbound credentials as its parent (e.g., the `/agent/chat` handler composing `/fs/readFile` may need the same API key for an outbound LLM call). The security implication is that each composition step duplicates the secret material reference — but capabilities are scoped (the handler can only use what the assembly layer declared on the registration bundle), and children run under the parent's composition authority (ADR-015, ADR-022). A clone is the same scoped handle, not a widening of scope. The concrete cloning semantics (reference-counted `Arc` vs deep copy of zeroized material) is a two-way door for implementation, but `Capabilities: Clone` is required by the composition model.
|
||||
- **Capabilities must be immutable after construction.** No interior mutability, no `Mutex<Map>`, no `RefCell`. This makes the clone-semantics two-way door genuinely two-way: Arc-based clone (shared immutable state) and deep-copy clone (isolated state) are behaviorally identical when neither supports mutation. Without this guard, a handler that mutates capabilities (e.g., adds a derived key for a child) would make the mutation visible to siblings and the parent under Arc-based clone — shared mutable state across the call tree, a security-relevant behavior. Once shipped, handlers may depend on shared mutation, and switching from Arc-shared to deep-copy-isolated later is a behavior change that breaks them. The immutability guard prevents the "two-way door" from becoming a future one-way door.
|
||||
|
||||
**No vault operations are registered in the call protocol.** The vault is assembly-layer only (ADR-008, ADR-014). A handler that needs a child key for a specific operation (e.g., signing for GitHub auth) receives a scoped capability that performs the derivation in-process — it never holds the master seed and never calls a network-exposed vault operation.
|
||||
|
||||
|
||||
@@ -41,7 +41,11 @@ pub enum TlsIdentity {
|
||||
/// RFC 7250 raw Ed25519 public key.
|
||||
/// No domain, no CA, no cert renewal. Key = identity.
|
||||
/// Same model as iroh's NodeId, but for direct QUIC connections.
|
||||
RawKey(SecretKey),
|
||||
/// `SecretKey` is `iroh::SecretKey` (Ed25519) — re-exported from iroh,
|
||||
/// which alknet-core already depends on (feature-gated, ADR-010). The
|
||||
/// key can be derived from alknet-vault at the assembly layer
|
||||
/// (endpoint.md) or generated fresh. See OQ-12, W14.
|
||||
RawKey(iroh::SecretKey),
|
||||
|
||||
/// Self-signed X.509 cert for development.
|
||||
/// Generated on startup, not validated by external clients.
|
||||
@@ -76,7 +80,7 @@ The reference `StaticConfig` (in `alknet-main/crates/alknet-core/src/config/stat
|
||||
// P2P / key-based identity (default for most nodes)
|
||||
let p2p_config = StaticConfig {
|
||||
listen_addr: Some("0.0.0.0:4433".parse()?),
|
||||
tls_identity: Some(TlsIdentity::RawKey(SecretKey::generate())),
|
||||
tls_identity: Some(TlsIdentity::RawKey(iroh::SecretKey::generate())),
|
||||
iroh_relay: None,
|
||||
drain_timeout: Duration::from_secs(2),
|
||||
};
|
||||
|
||||
@@ -80,8 +80,7 @@ impl Connection {
|
||||
- `remote_alpn()`: The ALPN negotiated for this connection. Always present.
|
||||
- `remote_addr()`: The peer's address, if available. Informational (NAT/proxy).
|
||||
- `close()`: Close the connection with an error code and reason.
|
||||
- `set_identity()`: Store the handler-resolved identity for observability (OQ-11). Write-once-read-many — a second call returns an error. Handlers that resolve identity inside `handle()` call this; the endpoint and observability layers read it via `identity()`.
|
||||
- `identity()`: Read the handler-resolved identity, if set. Returns `None` until `set_identity()` is called.
|
||||
- `set_identity()`: Store the handler-resolved identity for observability (OQ-11). Write-once-read-many — a second call returns an error. Handlers that resolve identity inside `handle()` call this; the identity is read by handler-side logging (the handler logs which identity it resolved) and is available on the `Connection` for any code that holds a reference to it. The endpoint does **not** read `identity()` after `handle()` returns — the `Connection` is moved into the spawned handler task (endpoint.md), so the endpoint no longer has a reference. Connection-level observability (remote addr, ALPN, connection ID) is logged by the endpoint before the move; identity-level observability is logged by the handler. See OQ-11 for the full resolution.
|
||||
|
||||
The `Connection` type does not expose quinn types in its public API. It wraps `quinn::Connection` internally, but the wrapper allows test implementations.
|
||||
|
||||
@@ -187,7 +186,7 @@ connection, ADR-006).
|
||||
| BiStream is a trait | [ADR-007](../../decisions/007-bistream-type-definition.md) | WASM door preserved, test mocks possible |
|
||||
| HandlerError is non-fatal | [ADR-010](../../decisions/010-alpn-router-and-endpoint.md) | Handler errors close the connection, not the endpoint |
|
||||
| SendStream/RecvStream wrap quinn + iroh | [ADR-010](../../decisions/010-alpn-router-and-endpoint.md) | Internal enum dispatch for both QUIC sources |
|
||||
| Connection stores handler-resolved identity | OQ-11 (resolved) | `set_identity` via `OnceLock` — write-once-read-many for observability |
|
||||
| Connection stores handler-resolved identity | OQ-11 (resolved) | `set_identity` via `OnceLock` — write-once-read-many; read by handler-side logging, not by the endpoint (C13 resolved) |
|
||||
|
||||
## Open Questions
|
||||
|
||||
|
||||
@@ -48,6 +48,7 @@ seed and derived private keys never cross the network.
|
||||
| [020](../../decisions/020-hd-derivation-for-encryption-keys.md) | HD Derivation for Encryption Keys | SLIP-0010 derivation, not PBKDF2; salt unused in v2 |
|
||||
| [021](../../decisions/021-key-rotation-via-version-indexed-paths.md) | Key Rotation via Version-Indexed Paths | Version-indexed paths; `rotate` re-encrypts |
|
||||
| [025](../../decisions/025-vault-local-only-dispatch.md) | Vault Local-Only Dispatch | Dropped irpc; direct method calls; local-only by construction |
|
||||
| [026](../../decisions/026-vault-key-model-hd-derivation.md) | Vault Key Model — HD Derivation | HD derivation from BIP39 seed; `74'` coin type; AES-256-GCM |
|
||||
|
||||
## Relevant Open Questions
|
||||
|
||||
@@ -126,6 +127,7 @@ truth for drift tracking — if an item is fixed in source, update this table.
|
||||
| 5 | `DerivedKey` dual serialization | JSON redacts, postcard preserves bytes | Always redact on serialize; reject `"[REDACTED]"` on deserialize with error (ADR-025, resolves W8) | `protocol.rs` | [protocol.md → Serialization Redaction](protocol.md#serialization-redaction), [ADR-025](../../decisions/025-vault-local-only-dispatch.md) |
|
||||
| 6 | `HashMap::clear` zeroization | `KeyCache::clear()` removes entries and relies on `CachedKey`'s `Drop` impl for zeroization | Verify `HashMap::clear()` actually drops values (it does, but worth a test) | `cache.rs` | [service.md → Security Constraints](service.md#security-constraints) |
|
||||
| 7 | `derive_password` / `site_password_path` | `derive_password`, `derive_password_string`, `site_password_path` methods exist | Remove entirely — password-manager pattern not relevant to RPC system's vault (ADR-025, resolves C9) | `service.rs`, `mnemonic-derivation.rs` | [ADR-025](../../decisions/025-vault-local-only-dispatch.md) |
|
||||
| 8 | `unlock_new` return type | Returns `String` (not zeroized on drop) | Return `Zeroizing<String>` — the mnemonic is the root of trust and must not linger in freed memory (resolves W7) | `service.rs` | [service.md → unlock_new](service.md#unlock_newword_count--phrase) |
|
||||
|
||||
## Public API
|
||||
|
||||
|
||||
@@ -218,13 +218,14 @@ not part of ADR-021's rotation scheme.
|
||||
|
||||
| Decision | ADR | Summary |
|
||||
|----------|-----|---------|
|
||||
| AES-256-GCM for credential encryption | — | Authenticated encryption, hardware-accelerated |
|
||||
| AES-256-GCM for credential encryption | [ADR-026](../../decisions/026-vault-key-model-hd-derivation.md) | Authenticated encryption, hardware-accelerated |
|
||||
| HD derivation, not PBKDF2 | [ADR-020](../../decisions/020-hd-derivation-for-encryption-keys.md) | Seed-derived key; no password; deterministic |
|
||||
| Salt unused in v2 (wire-format compat) | [ADR-020](../../decisions/020-hd-derivation-for-encryption-keys.md) | Kept for TS compat; not used in key derivation |
|
||||
| Key derived at `m/74'/2'/0'/0'` | — | Dedicated account for encryption keys |
|
||||
| Key derived at `m/74'/2'/0'/0'` | [ADR-026](../../decisions/026-vault-key-model-hd-derivation.md) | Dedicated account for encryption keys |
|
||||
| Version-indexed paths for rotation | [ADR-021](../../decisions/021-key-rotation-via-version-indexed-paths.md) | `m/74'/2'/0'/{version-2}'` |
|
||||
| Key versioning (v1=TS PBKDF2, v2=vault HD) | [ADR-020](../../decisions/020-hd-derivation-for-encryption-keys.md) | Distinguishes derivation methods |
|
||||
| All fields base64-encoded | — | JSON serialization compatibility |
|
||||
| `EncryptedData` wire format frozen | [ADR-018](../../decisions/018-vault-standalone-crate.md) | Fields, encoding, semantics locked; no removal without migration |
|
||||
|
||||
## Open Questions
|
||||
|
||||
|
||||
@@ -241,10 +241,11 @@ assembly-layer concern.
|
||||
| Decision | ADR | Summary |
|
||||
|----------|-----|---------|
|
||||
| Vault is standalone | [ADR-018](../../decisions/018-vault-standalone-crate.md) | Zero alknet crate dependencies |
|
||||
| HD derivation (not stored keys) | — | One seed, many keys, no key storage |
|
||||
| `74'` coin type reserved for alknet | — | SLIP-0044 unallocated; alknet namespace |
|
||||
| secp256k1 feature-gated | — | Heavy dep; only needed for Ethereum |
|
||||
| HD derivation (not stored keys) | [ADR-026](../../decisions/026-vault-key-model-hd-derivation.md) | One seed, many keys, no key storage; reproducible across nodes |
|
||||
| `74'` coin type reserved for alknet | [ADR-026](../../decisions/026-vault-key-model-hd-derivation.md) | SLIP-0044 unallocated; alknet namespace |
|
||||
| secp256k1 feature-gated | [ADR-026](../../decisions/026-vault-key-model-hd-derivation.md) | Heavy dep; only needed for Ethereum |
|
||||
| Hardened-only for Ed25519 | SLIP-0010 | Ed25519 cannot do public derivation |
|
||||
| Vault is local-only | [ADR-025](../../decisions/025-vault-local-only-dispatch.md) | Direct method calls, no irpc, no remote dispatch |
|
||||
|
||||
## Open Questions
|
||||
|
||||
|
||||
@@ -74,15 +74,23 @@ produce different seeds.
|
||||
### unlock_new(word_count) → phrase
|
||||
|
||||
```rust
|
||||
pub fn unlock_new(&self, word_count: usize) -> Result<String, VaultServiceError>;
|
||||
pub fn unlock_new(&self, word_count: usize) -> Result<Zeroizing<String>, VaultServiceError>;
|
||||
```
|
||||
|
||||
Generate a new random mnemonic, unlock with it, and return the phrase.
|
||||
Store the returned phrase securely — it is the root of trust. Supported
|
||||
word counts: 12, 15, 18, 21, 24.
|
||||
Generate a new random mnemonic, unlock with it, and return the phrase as
|
||||
a `Zeroizing<String>`. The returned phrase is the root of trust — it is
|
||||
heap-allocated and zeroized on drop, so it does not linger in freed
|
||||
memory. The caller should extract the phrase for secure storage (write
|
||||
down, display to user) and let the `Zeroizing<String>` drop when done.
|
||||
Do not clone the returned value or store it in a non-zeroizing container.
|
||||
Supported word counts: 12, 15, 18, 21, 24.
|
||||
|
||||
This is the "first run" path — a new node generates its mnemonic, writes
|
||||
it down, and the vault is unlocked for the process lifetime.
|
||||
it down, and the vault is unlocked for the process lifetime. The
|
||||
`Zeroizing<String>` wrapper (from the `zeroize` crate) ensures the
|
||||
mnemonic is wiped from memory once the caller is done with it, matching
|
||||
the `Mnemonic` type's own `ZeroizeOnDrop` behavior. This resolves review
|
||||
#002 W7.
|
||||
|
||||
### lock()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user