glm-5.1
9ec7627d80
9 atomic tasks for alknet-secret spec conformance and gap closure, derived from architect's implementation review. Dependencies form a 5-generation graph starting with spec update, then parallel implementation tasks, ending with a review gate. Tasks address: DerivedKey zeroize security, key caching with TTL, irpc protocol integration, password derivation, secp256k1/Ethereum derivation, encryption salt/KDF, crypto test vectors, and final spec conformance review.
104 lines
5.1 KiB
Markdown
104 lines
5.1 KiB
Markdown
---
|
|
id: review-alknet-secret-spec-conformance
|
|
name: Review alknet-secret crate for spec conformance and prepare for Phase A integration
|
|
status: pending
|
|
depends_on: [spec-update-secret-service, derivedkey-zeroize-security, key-caching-ttl, irpc-secret-protocol-integration, derive-password-implementation, secp256k1-ethereum-derivation, encryption-salt-kdf, crypto-test-vectors]
|
|
scope: moderate
|
|
risk: low
|
|
impact: phase
|
|
level: review
|
|
---
|
|
|
|
## Description
|
|
|
|
Review the complete alknet-secret crate for spec conformance, security properties, and readiness for Phase A integration (connecting to alknet-storage via EncryptedData wire format and alknet-core via OperationEnv/irpc).
|
|
|
|
This review covers all tasks in the secret-service task group. It verifies that the implementation matches the updated spec and that the crate is safe and correct for production use.
|
|
|
|
### Review Checklist
|
|
|
|
1. **Spec conformance**: Does every module, type, and method in the implementation match what the updated `secret-service.md` specifies?
|
|
- Mnemonic module matches spec (BIP39, Language, Seed, Zeroize)
|
|
- Derivation module matches spec (SLIP-0010, path constants, ExtendedPrivKey)
|
|
- Encryption module matches spec (AES-256-GCM, EncryptedData, salt purpose)
|
|
- Protocol module matches spec (SecretProtocol, SecretMessage, DerivedKey with Zeroize)
|
|
- Service module matches spec (SecretServiceHandle, SecretServiceActor, lifecycle)
|
|
- Cache module matches spec (KeyCache, TTL, LRU, cleared on Lock)
|
|
|
|
2. **Security properties (ADR-038)**:
|
|
- All sensitive types derive `Zeroize` with `#[zeroize(drop)]`
|
|
- `DerivedKey.private_key` is zeroized on drop (not Clone)
|
|
- `Seed` is zeroized on drop
|
|
- `Mnemonic` is zeroized on drop
|
|
- `CachedKey.private_key` is zeroized on drop
|
|
- `EncryptionKey` is zeroized on drop
|
|
- `Lock` zeroizes all cached keys and the seed
|
|
- No private key material leaks through `Debug`, `Display`, or `Serialize`
|
|
|
|
3. **irpc integration**:
|
|
- `SecretMessage` is properly defined with oneshot channels
|
|
- `SecretServiceActor` processes all message variants
|
|
- Local use via `SecretServiceHandle` still works without irpc overhead
|
|
- The actor model doesn't introduce deadlocks or race conditions
|
|
|
|
4. **Test vectors**:
|
|
- BIP39 test vectors pass
|
|
- SLIP-0010 test vectors pass
|
|
- AES-256-GCM test vectors pass
|
|
- Cross-consistency (mnemonic → seed → key) pass
|
|
|
|
5. **Feature flag**:
|
|
- `secp256k1` feature flag works correctly
|
|
- Without the flag, `derive_ethereum_key()` returns `UnsupportedKeyType`
|
|
- With the flag, BIP-0032 derivation produces correct secp256k1 keys
|
|
|
|
6. **No circular dependencies**:
|
|
- `alknet-secret` does not depend on `alknet-core` or `alknet-storage`
|
|
- Check `Cargo.toml` for any accidental dependencies
|
|
|
|
7. **Wire format compatibility**:
|
|
- `EncryptedData` serialization matches the spec (key_version, salt, iv, data — all Base64)
|
|
- `DerivedKey` serialization redacts `private_key`
|
|
- `SecretProtocol` variants serialize correctly for irpc (postcard format)
|
|
|
|
8. **Public API completeness**:
|
|
- All spec'd re-exports are present in `lib.rs`
|
|
- `SecretService`, `SecretServiceHandle`, `SecretServiceActor`, `SecretMessage` all re-exported
|
|
- `CacheConfig` is available for configuration
|
|
- Feature-gated types (`Secp256k1` items) are behind the `secp256k1` flag
|
|
|
|
## Acceptance Criteria
|
|
|
|
- [ ] All tests pass: `cargo test -p alknet-secret --all-features`
|
|
- [ ] No compiler warnings: `cargo clippy -p alknet-secret --all-features`
|
|
- [ ] Formatting is clean: `cargo fmt -p alknet-secret -- --check`
|
|
- [ ] Every module, type, and method in the implementation matches the updated spec
|
|
- [ ] All sensitive types implement `Zeroize` with `#[zeroize(drop)]`
|
|
- [ ] `DerivedKey` is not `Clone` and private_key is zeroized
|
|
- [ ] `DerivedKey` serialization does not expose `private_key`
|
|
- [ ] Key cache works: TTL eviction, LRU eviction, cleared on Lock
|
|
- [ ] `SecretServiceActor` processes all `SecretMessage` variants correctly
|
|
- [ ] BIP39, SLIP-0010, and AES-256-GCM test vectors pass
|
|
- [ ] `secp256k1` feature flag gates Ethereum derivation correctly
|
|
- [ ] `alknet-secret` has no dependency on `alknet-core` or `alknet-storage`
|
|
- [ ] `EncryptedData` JSON serialization matches the spec format
|
|
- [ ] `SecretProtocol` / `SecretMessage` types are correctly structured for irpc
|
|
- [ ] Public API (`lib.rs` re-exports) matches the spec's crate interface section
|
|
- [ ] Documentation comments on all public items
|
|
|
|
## References
|
|
|
|
- docs/architecture/secret-service.md — Updated spec
|
|
- docs/architecture/decisions/038-seed-lifecycle-memory-security.md — ADR-038
|
|
- docs/architecture/decisions/027-crate-decomposition.md — ADR-027
|
|
- All implementation tasks in this task group
|
|
|
|
## Notes
|
|
|
|
> This review should be thorough but should not block on minor documentation phrasing. Focus on: security properties (zeroize), spec conformance (does the code match what the spec says), and integration readiness (can Phase A wire this crate to alknet-core and alknet-storage?).
|
|
|
|
> If any deviations between spec and implementation are found, document them and create follow-up tasks rather than fixing them during review.
|
|
|
|
## Summary
|
|
|
|
> To be filled on completion |