feat: opt-in password-based encryption for FilesystemKeyStore by WiktorStarczewski · Pull Request #1835 · 0xMiden/rust-sdk

WiktorStarczewski · 2026-02-26T21:25:18Z

Summary

Resolves #30.

Secret keys stored by FilesystemKeyStore are currently written as plaintext files on disk. Any process with filesystem access can read them. This PR adds opt-in password-based encryption so keys are encrypted at rest, while preserving full backward compatibility with existing plaintext key files.

Design

Encryption scheme

KDF: Argon2id with OWASP-recommended parameters (19 MiB memory, 2 iterations, 1 parallelism)
AEAD: ChaCha20-Poly1305
Each key file gets a unique random salt and nonce, so a single password suffices for all files
Encrypted files use a binary format: [4B: "MENC"] [1B: version] [16B: salt] [12B: nonce] [NB: ciphertext + 16B auth tag]

Architecture

New KeyEncryptor trait (object-safe, Send + Sync) with encrypt/decrypt methods
PasswordEncryptor struct implementing the trait using Argon2id + ChaCha20-Poly1305
FilesystemKeyStore gains an optional Arc<dyn KeyEncryptor> field
No changes to the Keystore trait — encryption is internal to FilesystemKeyStore

Auto-detection & backward compatibility

Encrypted files start with MENC magic header; plaintext files start with AuthSecretKey discriminant (0x00 or 0x01), so the two formats are unambiguous
An encrypted keystore transparently reads pre-existing plaintext files
A plaintext keystore encountering an encrypted file returns a clear error message suggesting the user configure an encryptor
key_index.json is not encrypted (it contains only public info: account IDs and key commitments)

Migration support

migrate_to_encrypted() re-encrypts all plaintext key files in-place using atomic writes (write to temp, fsync, rename)
Deduplicates file paths to avoid re-encrypting shared keys across accounts
Already-encrypted files are skipped, making the operation idempotent

Memory hygiene

Password bytes wrapped in Zeroizing<Vec<u8>> (cleared on drop)
Derived key wrapped in Zeroizing<[u8; 32]> (cleared on drop)
Dependencies (argon2, chacha20poly1305, zeroize) are optional and gated behind the std feature flag — no impact on no_std builds

CLI integration

Set MIDEN_KEYSTORE_PASSWORD environment variable to a non-empty value to enable encryption
Unset or empty → plaintext (existing behavior, fully backward compatible)
Invalid Unicode → warning logged, falls back to plaintext

Programmatic API

FilesystemKeyStore::new(path)?.with_encryption(PasswordEncryptor::new("password")) — builder pattern
ClientBuilder::filesystem_keystore_encrypted(path, encryptor) — convenience method

Files changed

File	Change
`crates/rust-client/Cargo.toml`	Add `argon2`, `chacha20poly1305`, `zeroize` optional deps; `tempfile` dev-dep
`crates/rust-client/src/keystore/encryption.rs`	New — `KeyEncryptor` trait, `PasswordEncryptor`, constants, 7 unit tests
`crates/rust-client/src/keystore/mod.rs`	Module declaration + re-exports
`crates/rust-client/src/keystore/fs_keystore.rs`	Encryption support in struct, `atomic_write` helper, `migrate_to_encrypted`, 15 integration tests
`crates/rust-client/src/builder.rs`	`filesystem_keystore_encrypted()` convenience method
`bin/miden-cli/src/lib.rs`	`create_keystore()` helper reading `MIDEN_KEYSTORE_PASSWORD` env var
`CHANGELOG.md`	Feature entry

Test plan

7 encryption unit tests: roundtrip, wrong password, unique ciphertexts, truncation, tampering, magic detection
15 integration tests: plaintext baseline, encrypted roundtrip, magic header verification, wrong password, backward compat (both directions), migration (convert/skip/error/dedup), multiple keys, remove, clone preservation, debug output
cargo check -p miden-client (std)
cargo check -p miden-client --no-default-features (no_std)
cargo check -p miden-client-cli
Manual CLI test: create account with/without MIDEN_KEYSTORE_PASSWORD, verify key files, verify signing works

Note

The web-sdk parts of this PR have been migrated to 0xMiden/web-sdk#27 as part of the web-sdk split (#1992 / #2135). The miden-client side stays here. Note: the migration has 3-way merge conflicts that need manual resolution.

SantiagoPittella · 2026-02-27T15:54:01Z

+    /// # Errors
+    ///
+    /// Returns an error if no encryptor is configured, or if any file cannot be read/written.
+    pub fn migrate_to_encrypted(&self) -> Result<usize, KeyStoreError> {


Considering that we aren't supporting migrations yet, and that the state of the clients (and network in general) will be destroyed in the next release: do we need this migration? O can we asume that the user will have the keys encrypted for the genesis

Good question. I added it here mainly for the case where, someone sets up their app without encryption (it is essentially optional), but then decides to add it later for whatever reason (didn't realize the consequences, made a design mistake, or just elevated the general security level because the app scaled up for example). Is that convincing? I can live without it, just thought it would be nice to support such cases ootb.

I think I disagree with adding this here as well. At the very least this should be an app-level concern (so, maybe it belongs in the CLI). Worst-case, users can re-create the keystore by instantiating an encrypted one and moving keys there, right?

Also, should encryption not be optional at all?

Good question. I would argue keeping encryption optional is the right call for these reasons:

Test and dev friction. All integration tests, benchmarks, and the ClientBuilder::filesystem_keystore() convenience method create plaintext keystores. Making encryption mandatory would mean every test and dev scenario needs a password wired up for no real security gain on a local machine.
It's a library-level building block, not an app. The Keystore trait is where apps enforce policy. The CLI already does
this, create_keystore() opts in via the env var. Other users embedding FilesystemKeyStore might have full-disk encryption or other protections that make Argon2id redundant overhead.
If we wanted a middle ground, we could log a warning when writing a key without encryption configured, nudging users toward enabling it - without forcing it. How does that sound?

igamigo

Leaving some comments. Seems like there are some opportunities to remove complexity from the implementation

igamigo · 2026-03-03T14:15:28Z

+    keys_directory: std::path::PathBuf,
+) -> Result<CliKeyStore, miden_client::keystore::KeyStoreError> {
+    let keystore = CliKeyStore::new(keys_directory)?;
+    match std::env::var("MIDEN_KEYSTORE_PASSWORD") {


Is using an environment variable safe? I'm not entirely sure this is the case. Enforcing this is complicated due to them being visible by the users, being logged by process managers/shell histories, inherited by child processes, etc. At the very least there should be alternatives

Well, environment variables are the standard mechanism for passing secrets to CLI tools - AWS_SECRET_ACCESS_KEY, VAULT_TOKEN, PGPASSWORD, GPG_PASSPHRASE - the entire ecosystem has settled on that being the go-to. The common concerns (visibility in /proc, child process inheritance, shell history) are worth understanding but don't really apply here:

/proc/pid/environ requires same-UID or root access. At that point an attacker can read the key files directly.

The CLI reads the env var once at startup - we're not spawning untrusted child processes that would inherit it.

Env vars don't appear in shell history (unlike --password flags, which show up in both history and ps output).

The main alternatives each have equal or worse tradeoffs: a CLI flag leaks into process listings and history, a password file moves the problem, and a TTY prompt breaks automation/scripting.

That said, happy to add an interactive prompt as a fallback for when the env var isn't set and stdin is a TTY - that would cover the interactive use case without hurting scriptability. Would that address your concern?

igamigo · 2026-03-03T14:24:43Z

+    /// # Errors
+    ///
+    /// Returns an error if no encryptor is configured, or if any file cannot be read/written.
+    pub fn migrate_to_encrypted(&self) -> Result<usize, KeyStoreError> {


I think I disagree with adding this here as well. At the very least this should be an app-level concern (so, maybe it belongs in the CLI). Worst-case, users can re-create the keystore by instantiating an encrypted one and moving keys there, right?

igamigo · 2026-03-03T14:25:28Z

+    /// # Errors
+    ///
+    /// Returns an error if no encryptor is configured, or if any file cannot be read/written.
+    pub fn migrate_to_encrypted(&self) -> Result<usize, KeyStoreError> {


Also, should encryption not be optional at all?

juan518munoz · 2026-03-06T16:13:35Z

Why not use a standarized crate like orion::pwhash instead of implementing this ourselves?

Does 0xMiden/crypto repo expose some of this crypto primitives? Maybe we can use something from there. Also, might be good to ask for a review from that team here wdyt @WiktorStarczewski ?

Re orion::pwhash, the current code does two distinct things:

Key derivation — Argon2id password → 256-bit symmetric key

Authenticated encryption — ChaCha20-Poly1305 with that derived key

orion::pwhash only covers step 1 (and uses Argon2i, not Argon2id which is the recommended variant for key derivation). It doesn't provide the AEAD encryption step at all, so we'd still need a separate crate for that.

Re 0xMiden/crypto, it has ChaCha20-Poly1305 but no password hashing (Argon2/PBKDF2). We'd still need the argon2 crate regardless, and pulling in all of miden-crypto just to share the ChaCha20 dependency I think isn't worth the weight. I'd argue that the two lightweight RustCrypto crates (argon2 + chacha20poly1305) are the right fit here.

orion::aead implements XChaCha20-Poly1305 with automatic nonce handling.

https://docs.rs/orion/latest/orion/aead/index.html

Good point - orion::aead does provide XChaCha20-Poly1305 with automatic nonce handling, so orion::kdf + orion::aead would cover both steps with a single crate. But I think the catch is (iiuc) that orion::kdf only supports Argon2i, not Argon2id. Argon2id is the recommended variant for key derivation.

The auto-nonce is a nice ergonomic win but only saves a few lines, we still need a custom header for the KDF salt either way. Overall, the two crate approach seems like a better tradeoff, no?

Add opt-in encryption support for secret keys at rest using Argon2id (KDF) + ChaCha20-Poly1305 (AEAD). The KeyEncryptor trait is object-safe so FilesystemKeyStore can store it as Arc<dyn>. Encrypted file format: [4B: MENC] [1B: version] [16B: salt] [12B: nonce] [ciphertext+tag] New dependencies (argon2, chacha20poly1305, zeroize) are gated behind the `std` feature so no_std builds are unaffected.

- Add optional `encryptor: Option<Arc<dyn KeyEncryptor>>` field - Add `with_encryption()` builder method for opt-in encryption - Convert read/write helpers to methods that auto-detect format - Encrypted files (MENC header) are decrypted; plaintext files are read directly — backward compatible - Add `migrate_to_encrypted()` to re-encrypt existing plaintext keys - Replace derived Debug with manual impl (dyn trait isn't Debug)

Add `ClientBuilder::filesystem_keystore_encrypted()` that creates a FilesystemKeyStore with encryption in a single builder call.

Extract `create_keystore()` helper that reads the MIDEN_KEYSTORE_PASSWORD env var and enables PasswordEncryptor when set. Both keystore construction sites now share the same encryption configuration.

15 tests covering: - Plaintext baseline (add/get key, file format) - Encrypted roundtrip (add/get, magic header, wrong password) - Backward compat (encrypted reads plaintext, plaintext errors on encrypted) - Migration (convert, skip already encrypted, no-encryptor error, dedup) - Multiple keys, remove key, clone preserves encryption, Debug impl

- Release the index read lock before the encrypt/write loop in migrate_to_encrypted so concurrent keystore operations aren't blocked during slow Argon2id derivations. - Use rand::random() for both salt and nonce generation instead of mixing rand and OsRng sources. - Fix doc on PasswordEncryptor::new (accepts bytes, not just strings).

Write key files via a temp file + rename instead of direct fs::write. This prevents a crash mid-write from leaving a corrupted key file, which is especially important during migrate_to_encrypted where the original plaintext would be lost.

Call sync_all() on the temp file before renaming to ensure data is flushed to disk. Without this, a system crash between write and rename could leave the temp file with incomplete data. Matches the pattern already used by KeyIndex::write_to_file.

Use finish_non_exhaustive() in Debug impl and clone() instead of to_vec().

Co-authored-by: Santiago Pittella <87827390+SantiagoPittella@users.noreply.github.com>

…directly Replace the object-safe KeyEncryptor trait with concrete PasswordEncryptor methods. This reduces unnecessary abstraction since there is only one encryptor implementation. The encrypt/decrypt methods are now inherent methods on PasswordEncryptor instead of trait methods. Updates builder, mod.rs re-exports, and CHANGELOG accordingly.

- Use PasswordEncryptor directly instead of dyn KeyEncryptor - Wrap key bytes in Zeroizing<Vec<u8>> in read/write methods - Remove migrate_to_encrypted() and its tests - Replace tempfile dev-dependency with std::env::temp_dir in tests

* feat(web-client): add WebAuthn PRF-based key encryption module Add passkey-keystore.js implementing opt-in encryption for secret keys at rest using WebAuthn PRF (Touch ID, Face ID, Windows Hello). Keys are wrapped with AES-256-GCM derived from the authenticator's PRF output via HKDF-SHA256. - MWEB envelope format (distinct from native MENC/ChaCha20-Poly1305) - Non-extractable CryptoKey held in closure, never exposed to JS - AAD binding prevents ciphertext-swapping attacks in IndexedDB - Separate MidenKeystore_* Dexie DB avoids schema conflicts - Migration fallback reads plaintext from main DB and re-encrypts - Feature detection via getClientCapabilities + UA heuristic fallback - Playwright tests with CDP virtual authenticator (8 test cases) Update api-types.d.ts with PasskeyEncryptionOptions, PasskeyKeystore interfaces, optional keystore.sign, and function declarations. * feat(web-client): integrate passkeyEncryption into MidenClient.create() Add passkey resolution in MidenClient.create() before the keystore branch. When passkeyEncryption is set, dynamically imports the passkey module and creates encrypted keystore callbacks. - Dynamic import() for tree-shaking (module only loaded when needed) - console.warn when both passkeyEncryption and keystore are provided - Export isPasskeyPrfSupported and createPasskeyKeystore from index.js * feat(react-sdk): add passkeyEncryption support to MidenProvider Wire passkey encryption into the React SDK's MidenProvider. When config.passkeyEncryption is set and no SignerContext is active, the provider dynamically imports createPasskeyKeystore and creates an encrypted client via createClientWithExternalKeystore. - Add PasskeyEncryptionOptions and storeName to MidenConfig - Re-export isPasskeyPrfSupported and PasskeyEncryptionOptions - Passkey encryption ignored when external signer is active * docs: add passkey encryption documentation Add Docusaurus page for passkey encryption covering usage for both web SDK and React SDK, security properties, browser support, migration, export/import limitations, cross-device behavior, and credential loss. Update React SDK README with passkey encryption section including basic usage, feature detection, advanced options, and important notes. Update React SDK CLAUDE.md with config example and quick reference. * feat: add vite-plugin, fix StrictMode init, enable passkey in wallet example - Add @miden-sdk/vite-plugin (from main) for COOP/COEP headers, worker format, and WASM resolve aliases - Fix React StrictMode double-mount bug in MidenProvider that prevented client initialization (reset isInitializedRef in cleanup) - Update wallet example to use local packages and vite-plugin - Enable passkeyEncryption in wallet example config - Add vite-plugin to .gitignore allowlist * chore: update yarn lockfiles * chore: remove debug logs from MidenProvider * docs: add changelog entry for passkey encryption feature * fix: review fixes — migration round-trip verification, passkey fallback, input validation * fix: add passkey fallback to MidenClient.create() for unsupported browsers * fix: review round 2 — verify migration bytes, fix storeName mismatch, remove dead code * fix: pass storeName to fallback createClient path in MidenProvider * docs: fill changelog PR number (#1836) * fix(ci): run prettier, regenerate typedoc, remove version sync script * fix(ci): ignore vite-plugin in root eslint config * fix(web-client): add passkey-keystore as standalone rollup entry point The passkey-keystore.js module was only bundled into index.js but tests import it as a standalone module from http://localhost:8080/passkey-keystore.js. Add it as a separate rollup entry so it outputs to dist/passkey-keystore.js. * fix(web-client): use native IndexedDB API instead of Dexie in passkey tests Bare module specifiers like 'dexie' cannot be resolved inside page.evaluate() in the browser context. Replace with native IndexedDB API calls for the 3 tests that read/write raw keystore records. * fix(web-client): address PR review feedback - Add console.warn when WebAuthn PRF is unsupported instead of silently falling through to standard keystore - Revert wallet example back to devnet config * fix(rust-client): merge use statements to satisfy nightly fmt * docs(wallet-example): add key recovery warning for passkey encryption * fix(web-client): fix accountAuth table name in migration and add migration tests The migration fallback used `accountAuths` (plural) but the actual IndexedDB table created by idxdb-store is `accountAuth` (singular), causing migration to silently fail. Also adds three tests covering the migration path: successful migrate-and-delete, subsequent reads from encrypted keystore, and no-op when key is not found. * fix(web-client): format passkey-keystore test with prettier * docs(web-client): mention seed phrase recovery in credential loss section

WiktorStarczewski · 2026-04-28T19:05:41Z

Heads-up: the web-sdk-related changes from this PR have been migrated to a new PR on the web-sdk repo: 0xMiden/web-sdk#27.

This is part of the ongoing split of web/WASM components from miden-client into a dedicated repo (#1992 / #2135). Once that split lands, the crates/web-client/ and crates/idxdb-store/ directories will no longer exist in miden-client, so the migration moves those changes to the right home now.

Please continue this PR with the miden-client-only changes; the web-sdk-side changes are tracked in 0xMiden/web-sdk#27. If you have write access to your branch, the cleanest follow-up is to drop the web-sdk file changes from this PR. Otherwise they'll naturally fall out when you rebase after #1992 / #2135 merges.

Note: the migrated web-sdk PR contains 3-way merge conflicts (the original miden-client PR was opened against an older next, and web-sdk's next has drifted via the #13 sync). Resolution needed before the web-sdk PR can land.

…nto wiktor-storekeys Brings the keystore-encryption work up to the snapshot of next that web-sdk currently pins (miden-client@dab6cf7b in web-sdk's Cargo.lock). Stops short of #2100 (peaks removal) which would require web-sdk's idxdb-store to migrate its Store impl — that's a separate workstream on web-sdk. Conflict resolutions: - 7 web-side files (crates/web-client/js/*, docs/typedoc/web-client/*, packages/react-sdk/*) — take 'ours' (the passkey modifications from this branch). These files don't affect downstream Rust consumers (web-sdk consumes miden-client + miden-client-sqlite-store only). - scripts/check-react-sdk-sync.js: HEAD deleted, dab6cf7 modified — take HEAD's deletion. - CHANGELOG.md: keep all entries from both sides. - crates/rust-client/Cargo.toml: merge std-feature additions (argon2/chacha20poly1305/zeroize from HEAD + tempfile/tokio from dab6cf7) and combine the [dependencies] block with both sets of additions. cargo check -p miden-client -p miden-client-sqlite-store is clean (the two packages web-sdk actually consumes). The miden-client workspace's own idxdb-store crate fails to compile against the merged tree because its Store impl is older than dab6cf7's trait — that's expected and doesn't affect external consumers.

SantiagoPittella · 2026-04-30T20:40:30Z

@WiktorStarczewski this PR needs to remove the web-client parts. Let us know if you prefer that we continue with it

WiktorStarczewski added a commit that referenced this pull request Feb 26, 2026

docs: fill changelog PR number (#1835)

74ecbdb

WiktorStarczewski requested review from SantiagoPittella and igamigo February 26, 2026 21:29

WiktorStarczewski mentioned this pull request Feb 27, 2026

feat(web): WebAuthn PRF-based key encryption for web client #1836

Merged

7 tasks

SantiagoPittella reviewed Feb 27, 2026

View reviewed changes

igamigo requested a review from juan518munoz March 2, 2026 17:55

igamigo reviewed Mar 3, 2026

View reviewed changes

WiktorStarczewski requested review from SantiagoPittella and igamigo March 3, 2026 15:59

juan518munoz reviewed Mar 6, 2026

View reviewed changes

WiktorStarczewski added a commit that referenced this pull request Mar 9, 2026

docs: fill changelog PR number (#1835)

852d0aa

WiktorStarczewski force-pushed the wiktor-storekeys branch from 4256a8e to 7a2045c Compare March 9, 2026 16:46

WiktorStarczewski closed this Mar 9, 2026

WiktorStarczewski reopened this Mar 9, 2026

WiktorStarczewski added a commit that referenced this pull request Mar 9, 2026

docs: fill changelog PR number (#1835)

ffd7f2c

WiktorStarczewski force-pushed the wiktor-storekeys branch from 45d76b0 to c0544bb Compare March 9, 2026 17:08

WiktorStarczewski requested a review from juan518munoz March 9, 2026 20:55

WiktorStarczewski added 13 commits March 9, 2026 22:59

feat: add filesystem_keystore_encrypted builder convenience

9475413

Add `ClientBuilder::filesystem_keystore_encrypted()` that creates a FilesystemKeyStore with encryption in a single builder call.

feat: add MIDEN_KEYSTORE_PASSWORD env var support to CLI

2be0c69

Extract `create_keystore()` helper that reads the MIDEN_KEYSTORE_PASSWORD env var and enables PasswordEncryptor when set. Both keystore construction sites now share the same encryption configuration.

docs: add changelog entry for keystore encryption feature

da9f579

docs: fill changelog PR number (#1835)

34af117

fix: apply nightly rustfmt and taplo formatting

6fb409f

docs: add keystore encryption documentation for CLI and library

a8a52a9

docs: note keystore encryption is native-only, not WASM

422610a

WiktorStarczewski and others added 12 commits March 9, 2026 22:59

fix(keystore): address clippy lints in FilesystemKeyStore

d5bb1ef

Use finish_non_exhaustive() in Debug impl and clone() instead of to_vec().

fix(keystore): inline format args in debug test

e8c990e

Update crates/rust-client/src/keystore/fs_keystore.rs

7b12ddd

Co-authored-by: Santiago Pittella <87827390+SantiagoPittella@users.noreply.github.com>

chore(keystore): remove tempfile dev-dependency

aa8dcb8

docs(keystore): remove custom encryptors and migration sections

3cc9cb4

fix(cli): remove redundant wording in keystore doc comment

8a07b24

fix: update Cargo.lock to match origin/next after rebase

4537049

ci: trigger workflow runs

734412f

ci: re-trigger workflows

fbf6eeb

WiktorStarczewski force-pushed the wiktor-storekeys branch from c0544bb to fbf6eeb Compare March 9, 2026 22:00

WiktorStarczewski mentioned this pull request Apr 28, 2026

feat(web-client): opt-in passkey-based keystore 0xMiden/web-sdk#27

Open

WiktorStarczewski force-pushed the wiktor-storekeys branch from fde5668 to 5608802 Compare April 30, 2026 12:30

Uh oh!

Conversation

WiktorStarczewski commented Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Design

Encryption scheme

Architecture

Auto-detection & backward compatibility

Migration support

Memory hygiene

CLI integration

Programmatic API

Files changed

Test plan

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

igamigo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WiktorStarczewski Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SantiagoPittella Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

WiktorStarczewski commented Apr 28, 2026

Uh oh!

SantiagoPittella commented Apr 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

WiktorStarczewski commented Feb 26, 2026 •

edited

Loading

WiktorStarczewski Mar 3, 2026 •

edited

Loading

SantiagoPittella Mar 6, 2026 •

edited

Loading