diff --git a/docs/wallet-hardening.md b/docs/wallet-hardening.md index 4f7285d..18bedaf 100644 --- a/docs/wallet-hardening.md +++ b/docs/wallet-hardening.md @@ -17,7 +17,7 @@ Status legend: ☐ not started · ◐ in progress · ☑ landed & verified | Phase | Findings | Theme | Status | |-------|----------|-------|--------| | **P0-A** | W7-1, W2-1, W4-1, W4-3, W2-3, W4-5, W5-3 ✓ | Secret hardening (console redaction + delete-export + memzero + lite encrypt-at-create) | ☑ 7/7 | -| **P0-B** | W2-2/W4-2, W2-4 | Encryption integrity (never silently unencrypted) | ☐ | +| **P0-B** | W2-2/W4-2, W2-4 | Encryption integrity (never silently unencrypted) | ☑ | | **P1-A** | W3-1, W3-2, W3-4, W3-3 | Migrate-to-seed correctness (fund-adjacent) | ☐ | | **P1-B** | W1-1, W1-3, W1-2, W1-4 | Missing/wrong wallet-file safety | ☐ | | **P2** | W6-2, W5-1, W5-2, W6-1, W6-3 | Stale state & lite save-failure surfacing | ☐ | @@ -112,6 +112,11 @@ Land W7-2 first — it unblocks the rest. ## Progress log +- **P0-B / W2-2 (deferred encryption silently lost) + W2-4 (auto-lock silent-fail)** — ☑ landed: + - **W2-2:** the wizard's deferred encryption was stored only in memory, so a quit/crash or a failed daemon connect before it applied left the wallet unencrypted with **no record it was ever requested** — the user believing it was encrypted. Now a persisted `encryption_pending` settings flag is set the moment encryption is requested (**never the passphrase** — only the fact). `refreshWalletEncryptionState()` reconciles it on every connect: wallet observed **encrypted** → clear the flag; wallet **not** encrypted while the flag is set and no deferred encryption is pending/in-flight → a once-per-session **"your wallet is NOT encrypted — open Settings to finish"** warning (the flag stays set, so it recurs each launch until resolved). We deliberately don't persist the passphrase to auto-complete — surfacing it is the secure choice. + - **W2-4:** `lockWallet()`'s continuation only handled success — a failed `walletlock` silently left the wallet **unlocked** (an unfulfilled auto-lock). It now logs and warns once (reset on the next successful lock), so a failing auto-lock is visible instead of leaving the wallet exposed. + Touches `settings.{h,cpp}`, `app_wizard.cpp`, `app_security.cpp`, `app.h`. Not unit-testable at this layer (RPC/connect-driven state machine); build-clean, `ctest` 1/1. + - **P0-A / W5-3 (lite create-time passphrase)** — ☑ landed (chose option **(b) wire it up**). The lite create/open/restore passphrase was collected but never consumed by the backend — a "passphrase" field that did nothing. It now has a real meaning for all three operations, in `LiteWalletController`: **create/restore** → `encryptWallet(passphrase)` (the backend encrypts + locks + saves the brand-new wallet); **open** → `unlockWallet(passphrase)`, but only when `encryptionStatus()` reports the existing wallet is actually encrypted+locked (skips a spurious unlock otherwise). Encrypt/unlock take their own copy and wipe it; a post-create encrypt failure is `liteLog`'d (the wallet still exists — the create isn't failed). Six existing lite-controller tests carried an incidental `hunter2` create passphrase from the dead-field era; removed (they test non-encryption flows and want an unencrypted wallet), and added `testLiteWalletControllerCreateEncryptsWithPassphrase` to prove the new behavior. Build-clean; `ctest` 1/1. *(Follow-up UX polish: `settings_page` could show the passphrase field's meaning per operation — "encrypt" for create/restore vs "unlock" for open.)* - **P0-A / W4-5 (seed-backup file)** — ☑ landed (proportionate): the seed "Save" already wrote 0600 + zeroed the in-memory buffer, but the success message was a bare "Saved to ". It now reads "**Saved an UNENCRYPTED seed file — move it to secure offline storage and delete this copy**: ", so the plaintext-on-disk risk is called out. `i18n.cpp` (English source; `res/lang` back-fill of this changed key is deferred to the batch i18n pass). A stronger fix (pre-save confirmation, or dropping the file-save in favor of on-screen + Copy) is a follow-up UX decision. - **P0-A / W4-1 · W4-3 · W2-3 (memzero cluster)** — ☑ landed, using the file's established `sodium_memzero` pattern (matching the existing lambda-capture scrub at app_network.cpp:2885 and JSON scrub at :4025) rather than a new type, since this is fund-moving code: diff --git a/src/app.h b/src/app.h index e47ea13..d0a65a2 100644 --- a/src/app.h +++ b/src/app.h @@ -1024,6 +1024,8 @@ private: double clipboard_clear_deadline_ = 0.0; float loading_timer_ = 0.0f; // spinner animation for loading overlay double connect_stall_since_ = 0.0; // ImGui::GetTime() when the daemon first went "reachable but not ready"; 0 = not stalling (see util/connect_stall.h) + bool encryption_incomplete_warned_ = false; // W2-2: once-per-session guard for the "encryption didn't complete" warning + bool lock_failure_warned_ = false; // W2-4: guard so a repeatedly-failing auto-lock warns once, not every retry // Current page (sidebar navigation) ui::NavPage current_page_ = ui::NavPage::Overview; diff --git a/src/app_security.cpp b/src/app_security.cpp index 9019feb..15c8217 100644 --- a/src/app_security.cpp +++ b/src/app_security.cpp @@ -487,7 +487,17 @@ void App::lockWallet() { state_.locked = true; state_.unlocked_until = 0; resetTransactionHistoryCacheSession(); + lock_failure_warned_ = false; DEBUG_LOGF("[App] Wallet locked\n"); + } else { + // The walletlock RPC failed — the wallet is still UNLOCKED. Surface it (once) rather + // than silently leaving an auto-lock unfulfilled and the wallet exposed (W2-4). + DEBUG_LOGF("[App] walletlock failed — wallet remains unlocked\n"); + if (!lock_failure_warned_) { + lock_failure_warned_ = true; + ui::Notifications::instance().warning( + "Couldn't lock the wallet — it is still unlocked. Check the daemon connection.", 12.0f); + } } }; }); @@ -564,6 +574,12 @@ void App::refreshWalletEncryptionState() { state_.unlocked_until = until; state_.locked = (until == 0); state_.encryption_state_known = true; + // Wallet is encrypted — any pending deferred-encryption request has now been + // satisfied (however it completed). Clear the persisted flag (W2-2). + if (settings_ && settings_->getEncryptionPending()) { + settings_->setEncryptionPending(false); + settings_->save(); + } if (state_.locked) { resetTransactionHistoryCacheSession(); } else if (state_.transactions.empty()) { @@ -576,6 +592,19 @@ void App::refreshWalletEncryptionState() { state_.locked = false; state_.unlocked_until = 0; state_.encryption_state_known = true; + // W2-2: encryption was requested (persisted flag) but the wallet is NOT encrypted, + // and no deferred encryption is pending/in-flight — it was lost to a quit/crash or a + // failed connect before it applied. Warn (once/session) instead of silently leaving + // an unencrypted wallet the user believes is protected. The flag stays set until the + // wallet is actually encrypted, so the warning recurs each launch until resolved. + if (settings_ && settings_->getEncryptionPending() && + !wallet_security_.hasDeferredEncryption() && !encrypt_in_progress_ && + !encryption_incomplete_warned_) { + encryption_incomplete_warned_ = true; + ui::Notifications::instance().warning( + "Wallet encryption did not complete — your wallet is NOT encrypted. " + "Open Settings to finish encrypting it.", 30.0f); + } if (state_.transactions.empty()) { loadTransactionHistoryCacheIfAvailable(); } else { diff --git a/src/app_wizard.cpp b/src/app_wizard.cpp index d36c08c..84f8be1 100644 --- a/src/app_wizard.cpp +++ b/src/app_wizard.cpp @@ -1338,6 +1338,10 @@ void App::renderFirstRunWizard() { wallet_security_.beginDeferredEncryption( std::string(encrypt_pass_buf_), (pinEntered && pinOk) ? pinStr : std::string()); + // Persist that encryption was requested (never the passphrase) so a quit/crash or + // failed daemon connect before it applies isn't silent — reconciled on the next + // connect in refreshWalletEncryptionState (W2-2). Saved with the wizard state below. + settings_->setEncryptionPending(true); // Clear sensitive buffers memset(encrypt_pass_buf_, 0, sizeof(encrypt_pass_buf_)); diff --git a/src/config/settings.cpp b/src/config/settings.cpp index 852c4c5..612b609 100644 --- a/src/config/settings.cpp +++ b/src/config/settings.cpp @@ -231,6 +231,7 @@ bool Settings::load(const std::string& path) } loadScalar(j, "wizard_completed", wizard_completed_); loadScalar(j, "seed_backup_reminded", seed_backup_reminded_); + loadScalar(j, "encryption_pending", encryption_pending_); loadScalar(j, "daemon_update_prompted_size", daemon_update_prompted_size_); loadScalar(j, "active_wallet_file", active_wallet_file_); loadScalar(j, "seed_migration_pending", seed_migration_pending_); @@ -497,6 +498,7 @@ bool Settings::save(const std::string& path) } j["wizard_completed"] = wizard_completed_; j["seed_backup_reminded"] = seed_backup_reminded_; + j["encryption_pending"] = encryption_pending_; j["daemon_update_prompted_size"] = daemon_update_prompted_size_; j["active_wallet_file"] = active_wallet_file_; j["seed_migration_pending"] = seed_migration_pending_; diff --git a/src/config/settings.h b/src/config/settings.h index 8cdd124..072cb7e 100644 --- a/src/config/settings.h +++ b/src/config/settings.h @@ -327,6 +327,12 @@ public: bool getSeedBackupReminded() const { return seed_backup_reminded_; } void setSeedBackupReminded(bool v) { seed_backup_reminded_ = v; } + // Persisted the moment deferred (wizard) encryption is requested; cleared only once the wallet is + // observed to be actually encrypted. Lets a quit/crash/failed-connect before it applies be detected + // and surfaced (W2-2). NEVER stores the passphrase — only the fact that encryption was requested. + bool getEncryptionPending() const { return encryption_pending_; } + void setEncryptionPending(bool v) { encryption_pending_ = v; } + // Bundled-daemon size we last prompted to install (see App::renderDaemonUpdatePrompt). Lets the // "a newer node is bundled — update?" prompt fire once per wallet version, never re-nagging. long long getDaemonUpdatePromptedSize() const { return daemon_update_prompted_size_; } @@ -574,6 +580,7 @@ private: std::map address_meta_; bool wizard_completed_ = false; bool seed_backup_reminded_ = false; + bool encryption_pending_ = false; long long daemon_update_prompted_size_ = 0; // bundled daemon size last offered via the update prompt std::string active_wallet_file_ = "wallet.dat"; // -wallet= the daemon loads (multi-wallet) bool seed_migration_pending_ = false;