From 16a1ca02e8d2e71f6954f0e1b5a26cb57f65b0d8 Mon Sep 17 00:00:00 2001 From: DanS Date: Mon, 19 Oct 2026 15:33:28 -0500 Subject: [PATCH] =?UTF-8?q?fix(lite):=20lock/unlock=20parity=20=E2=80=94?= =?UTF-8?q?=20shared=20attempt-lockout,=20immediate=20unlock=20state,=20no?= =?UTF-8?q?=20dead=20PIN,=20surfaced=20encrypt=20failure?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lite-parity audit, FIX C (gating/validation/presentation only — no encrypt/unlock/send backend call changed): - Lite unlock went straight to the backend with no lockout. New App::unlockLiteWallet() runs the SAME escalating, persisted attempt-lockout as the full node (restoreLockoutState/applyUnlockFailure), wired into the Settings unlock, the send-time prompt and the lock screen. A wrong-passphrase failure counts; an already-unlocked/not-encrypted result does not. - state_.locked is now cleared immediately on a successful unlock (the ~2s status poll lagged, so the UI stayed "locked" after a correct passphrase). - PIN quick-unlock controls are hidden in lite Settings — lite has no PIN vault, so they were dead. - A create/restore whose passphrase encryption FAILS now raises a one-shot visible error (takeEncryptionFailureNotice -> lite_encrypt_failed_unencrypted) instead of only a log line; the wallet is left open but unencrypted. Fixed the flag on BOTH the lifecycle and the direct create/restore encrypt paths. - A background sync save is skipped while an encrypted wallet is unlocked (the backend save re-locks it), resuming once locked. Compile- and unit-verified (testLiteWalletControllerLockUnlockParity covers the save-deferral + encrypt-failure notice, run twice for the flaky guard); the lock-screen lockout/PIN-hiding UX itself is runtime/GUI-only and not exercised here — worth a manual lite GUI pass. Co-Authored-By: Claude Opus 4.8 --- src/app.cpp | 10 +++-- src/app.h | 4 ++ src/app_network.cpp | 63 +++++++++++++++++++++++++++ src/app_security.cpp | 9 ++++ src/ui/pages/settings_page.cpp | 11 +++-- src/util/i18n.cpp | 1 + src/wallet/lite_wallet_controller.cpp | 43 ++++++++++++++++-- src/wallet/lite_wallet_controller.h | 4 ++ tests/fake_lite_backend.h | 3 ++ tests/test_phase4.cpp | 61 ++++++++++++++++++++++++++ 10 files changed, 199 insertions(+), 10 deletions(-) diff --git a/src/app.cpp b/src/app.cpp index 18da0a1..e0e460b 100644 --- a/src/app.cpp +++ b/src/app.cpp @@ -965,6 +965,9 @@ void App::update() lite_wallet_->pumpAsyncOpen(); // Likewise finalize any completed async lifecycle request (Settings-page create/open/restore). lite_wallet_->pumpLifecycleResult(); + // A create/restore whose passphrase encryption failed left the wallet UNENCRYPTED (previously log-only): say so. + if (lite_wallet_->takeEncryptionFailureNotice()) + ui::Notifications::instance().error(TR("lite_encrypt_failed_unencrypted"), 15.0f); // Deliver finished wallet ops (new address / import / encrypt / decrypt) to their UI callbacks. lite_wallet_->pumpWalletOps(); // A server switch deferred behind an in-flight wallet op: retry it. This may replace lite_wallet_; the rest of @@ -3829,12 +3832,13 @@ void App::renderLiteUnlockPrompt() const float btnW = 130.0f * dp; bool doUnlock = ui::material::TactileButton(TR("lite_unlock_btn"), ImVec2(btnW, 0)) || entered; if (doUnlock) { - const bool ok = lite_wallet_->unlockWallet(pass); + std::string unlockErr; + const bool ok = unlockLiteWallet(std::string(pass), &unlockErr); sodium_memzero(pass, sizeof(pass)); if (ok) ui::Notifications::instance().success(TR("lite_unlock_ok"), 5.0f); else { - wallet::liteLog(std::string("Unlock failed: ") + TR("lite_unlock_failed")); - ui::Notifications::instance().error(TR("lite_unlock_failed")); + wallet::liteLog(std::string("Unlock failed: ") + unlockErr); + ui::Notifications::instance().error(unlockErr); } lite_unlock_prompt_ = false; ImGui::CloseCurrentPopup(); diff --git a/src/app.h b/src/app.h index 93af85a..cb15f1a 100644 --- a/src/app.h +++ b/src/app.h @@ -333,6 +333,10 @@ public: // Lock the lite wallet AND immediately tear down the chat session (the lite backend `lock` // doesn't update state_.locked until the next poll, so chat secrets would otherwise linger). bool lockLiteWallet(); + // Unlock the lite wallet with the same escalating attempt-lockout as the full node (persisted across restarts) + // and set state_.locked=false right away on success (the poll lags). Takes the passphrase by value and wipes + // it. On failure *err (if given) holds a user-facing reason ("Unlock failed..." or the lockout wait). + bool unlockLiteWallet(std::string passphrase, std::string* err = nullptr); // (Re)build the lite controller from current settings so a changed lite-server selection // takes effect. No-op on non-lite/unlinked builds; preserves a live wallet (see app.cpp). void rebuildLiteWallet(bool force = false); diff --git a/src/app_network.cpp b/src/app_network.cpp index 9cb977e..6dc4f61 100644 --- a/src/app_network.cpp +++ b/src/app_network.cpp @@ -5427,6 +5427,69 @@ bool App::lockLiteWallet() return ok; } +bool App::unlockLiteWallet(std::string passphrase, std::string* err) +{ + const auto fail = [&](const std::string& m) { if (err) *err = m; }; + const auto waitMsg = [](long long secs) { + char msg[128]; + snprintf(msg, sizeof(msg), TR("sec_too_many_attempts_wait"), (double)secs); + return std::string(msg); + }; + if (!lite_wallet_ || !lite_wallet_->walletOpen() || passphrase.empty()) { + wallet::secureWipeLiteSecret(passphrase); + fail(TR("lite_unlock_failed")); + return false; // nothing to attempt: not a wrong-credential failure, not counted + } + // Restore the persisted attempt count once per session (the full-node lock screen does the same). + if (!lock_state_restored_) { + lock_state_restored_ = true; + restoreLockoutState(); + } + // Enforce the lockout from the persisted wall-clock expiry: lite has no per-frame lockout timer. + if (settings_ && settings_->getLockStateWallet() == settings_->getActiveWalletFile()) { + const long long remaining = settings_->getLockoutUntilEpoch() - (long long)std::time(nullptr); + if (remaining > 0) { + wallet::secureWipeLiteSecret(passphrase); + fail(waitMsg(remaining)); + return false; + } + } + lock_lockout_timer_ = 0.0f; // expired: don't let a stale in-memory value re-arm the curve + + const bool ok = lite_wallet_->unlockWallet(std::move(passphrase)); // wipes the passphrase + if (!ok) { + const auto st = lite_wallet_->encryptionStatus(); + if (st.ok && !(st.encrypted && st.locked)) { + // Not a wrong credential: already unlocked (e.g. a racing prompt) or not encrypted. Don't count it. + state_.encrypted = st.encrypted; + state_.locked = false; + if (st.encrypted) { idle_.noteActivity(); return true; } + fail(TR("lite_unlock_failed")); + return false; + } + applyUnlockFailure(TR("lite_unlock_failed")); // bump counter, arm lockout curve, persist + fail(lock_lockout_timer_ > 0.0f ? waitMsg((long long)std::ceil(lock_lockout_timer_)) + : std::string(TR("lite_unlock_failed"))); + return false; + } + + // Success: clear the attempt state (and its persisted copy) and reflect the unlock NOW — the next poll lags. + lock_unlock_in_progress_ = false; + lock_error_msg_.clear(); + lock_attempts_ = 0; + lock_lockout_timer_ = 0.0f; + if (settings_ && settings_->getLockFailedAttempts() != 0) { + settings_->setLockFailedAttempts(0); + settings_->setLockoutUntilEpoch(0); + settings_->save(); + } + memset(lock_passphrase_buf_, 0, sizeof(lock_passphrase_buf_)); + idle_.noteActivity(); + state_.encrypted = true; + state_.locked = false; + return true; +} + bool App::seedChatDemoData() { if (!chat::hushChatFeatureEnabledAtBuild()) return false; diff --git a/src/app_security.cpp b/src/app_security.cpp index ed74904..cb8b4a3 100644 --- a/src/app_security.cpp +++ b/src/app_security.cpp @@ -548,6 +548,15 @@ void App::applyUnlockNotCounted(const std::string& msg) { } void App::unlockWallet(const std::string& passphrase, int timeout) { + if (lite_wallet_) { // lite: backend unlock with the shared attempt-lockout (no RPC/daemon) + std::string liteErr; + if (!unlockLiteWallet(passphrase, &liteErr)) { + lock_error_msg_ = liteErr; + lock_error_timer_ = 3.0f; + lock_timer_armed_at_ = std::chrono::steady_clock::now(); + } + return; + } if (!rpc_ || !rpc_->isConnected() || !worker_) return; lock_unlock_in_progress_ = true; diff --git a/src/ui/pages/settings_page.cpp b/src/ui/pages/settings_page.cpp index 3da9a67..f393893 100644 --- a/src/ui/pages/settings_page.cpp +++ b/src/ui/pages/settings_page.cpp @@ -1708,7 +1708,11 @@ void RenderSettingsPage(App* app) { // PIN unlock controls, trailing the auto-lock combo on the same row. bool isEncryptedPIN = app->state().isEncrypted(); - if (isEncryptedPIN) { + // Lite has no PIN quick-unlock: its unlock paths (Settings, send-time prompt) take the passphrase + // directly and never consult a PIN vault, so these controls would be dead. Hide them. + if (app->isLiteBuild()) { + // nothing to show + } else if (isEncryptedPIN) { bool hasPIN = app->hasPinVault(); float pinBtnW = std::min(rowBtnW({TR("settings_set_pin"), TR("settings_change_pin"), TR("settings_remove_pin")}), (secColW - Layout::spacingSm()) * 0.5f); @@ -2172,9 +2176,10 @@ void RenderSettingsPage(App* app) { if (ImGui::IsItemHovered()) material::Tooltip("%s", TR("tt_lite_unlock_pass")); ImGui::SetCursorScreenPos(ImVec2(leftX, ImGui::GetCursorScreenPos().y)); if (TactileButton(TrId("lite_unlock", "LiteUnlock").c_str(), ImVec2(0, 0), S.resolveFont("button"))) { - const bool ok = app->liteWallet()->unlockWallet(s_settingsState.lite_enc_pass); + std::string unlockErr; + const bool ok = app->unlockLiteWallet(std::string(s_settingsState.lite_enc_pass), &unlockErr); sodium_memzero(s_settingsState.lite_enc_pass, sizeof(s_settingsState.lite_enc_pass)); - s_settingsState.lite_encryption_status = ok ? TR("lite_wallet_unlocked") : TR("lite_unlock_failed"); + s_settingsState.lite_encryption_status = ok ? std::string(TR("lite_wallet_unlocked")) : unlockErr; } if (ImGui::IsItemHovered()) material::Tooltip("%s", TR("tt_lite_unlock")); } else { diff --git a/src/util/i18n.cpp b/src/util/i18n.cpp index 23dc491..c6b59b7 100644 --- a/src/util/i18n.cpp +++ b/src/util/i18n.cpp @@ -1354,6 +1354,7 @@ void I18n::loadBuiltinEnglish() strings_["lite_unlock_btn"] = "Unlock"; strings_["lite_unlock_ok"] = "Wallet unlocked — you can send now"; strings_["lite_unlock_failed"] = "Unlock failed — wrong passphrase?"; + strings_["lite_encrypt_failed_unencrypted"] = "Wallet encryption failed — your wallet may still be UNENCRYPTED. Open Settings > Security and set the passphrase again."; // Send Tab strings_["pay_from"] = "Pay From"; diff --git a/src/wallet/lite_wallet_controller.cpp b/src/wallet/lite_wallet_controller.cpp index 50e3d8a..e62f990 100644 --- a/src/wallet/lite_wallet_controller.cpp +++ b/src/wallet/lite_wallet_controller.cpp @@ -110,6 +110,21 @@ bool persistAfterBroadcast(LiteClientBridge& bridge) return false; } +// True when the wallet is encrypted AND currently unlocked. The backend's `save` re-locks such a wallet (do_save), +// so a background save must not run under a user who just unlocked. Fail-open: any error reads as "not unlocked" and +// the save proceeds as it always did. +bool liteWalletEncryptedAndUnlocked(LiteClientBridge& bridge) +{ + const auto r = bridge.execute("encryptionstatus", ""); + if (!r.ok) return false; + try { + const auto j = nlohmann::json::parse(r.value); + return j.is_object() && !j.contains("error") && j.value("encrypted", false) && !j.value("locked", false); + } catch (...) { + return false; + } +} + // The detached sync/rescan thread. `firstCommand` is "sync" or "rescan"; every retry is "sync" (a failed rescan has // already cleared the wallet, so re-running it would throw away the partial progress). Partial progress is saved after // every attempt. Captures shared state only. @@ -129,7 +144,12 @@ void runLiteScanThread(const std::shared_ptr& bridge, op.error = call.error.empty() ? "sync failed" : call.error; } // The backend does NOT auto-save after a sync; persist what was scanned even when the attempt failed. - if (!bridge->execute("save", "").ok) // W5-2 + // Not while the user has the wallet unlocked: the backend save re-locks an encrypted+unlocked wallet, which + // would yank the unlock out from under a spend/unlock in progress. The progress is saved by the next scan + // once locked (and by the shutdown save); a crash in between only costs a re-scan from the checkpoint. + if (liteWalletEncryptedAndUnlocked(*bridge)) + liteLog("sync save deferred: wallet is unlocked (a save would re-lock it)"); + else if (!bridge->execute("save", "").ok) // W5-2 liteLog("save failed after sync — the next launch will re-scan from the checkpoint"); if (op.ok) { scan->setError(std::string()); @@ -938,9 +958,11 @@ void LiteWalletController::applyPendingLifecycleSecurity() if (op == PendingSecurityOp::Encrypt) { // encryptWallet() takes its own copy and wipes it; the backend encrypts + locks + saves. const auto enc = encryptWallet(pendingLifecyclePassphrase_); - if (!enc.ok) + if (!enc.ok) { + lifecycleEncryptFailed_ = true; // surfaced to the user by the App (takeEncryptionFailureNotice) liteLog("wallet created/restored but ENCRYPTION FAILED — wallet is NOT encrypted: " + enc.error); + } } else if (op == PendingSecurityOp::Unlock) { if (!unlockWallet(pendingLifecyclePassphrase_)) liteLog("wallet opened but unlock failed (wrong passphrase?)"); @@ -1597,6 +1619,13 @@ bool LiteWalletController::unlockWallet(std::string passphrase) return ok; } +bool LiteWalletController::takeEncryptionFailureNotice() +{ + const bool failed = lifecycleEncryptFailed_; + lifecycleEncryptFailed_ = false; + return failed; +} + bool LiteWalletController::lockWallet() { if (!walletOpen_.load() || !bridge_) return false; @@ -1767,7 +1796,10 @@ LiteWalletLifecycleResult LiteWalletController::createWallet(LiteWalletCreateReq // copy and wipes it. if (walletOpen_.load() && !request.passphrase.empty()) { const auto enc = encryptWallet(request.passphrase); - if (!enc.ok) liteLog("wallet created but encryption failed: " + enc.error); + if (!enc.ok) { + lifecycleEncryptFailed_ = true; // the wallet is open but NOT encrypted; App surfaces this + liteLog("wallet created but encryption failed: " + enc.error); + } } secureWipeLiteSecret(request.passphrase); return result; @@ -1797,7 +1829,10 @@ LiteWalletLifecycleResult LiteWalletController::restoreWallet(LiteWalletRestoreR // If the user supplied a passphrase, encrypt the restored wallet with it now that it's open (W5-3). if (walletOpen_.load() && !request.passphrase.empty()) { const auto enc = encryptWallet(request.passphrase); - if (!enc.ok) liteLog("wallet restored but encryption failed: " + enc.error); + if (!enc.ok) { + lifecycleEncryptFailed_ = true; // the wallet is open but NOT encrypted; App surfaces this + liteLog("wallet restored but encryption failed: " + enc.error); + } } secureWipeLiteSecret(request.seedPhrase); secureWipeLiteSecret(request.passphrase); diff --git a/src/wallet/lite_wallet_controller.h b/src/wallet/lite_wallet_controller.h index 5dc4261..3738fe1 100644 --- a/src/wallet/lite_wallet_controller.h +++ b/src/wallet/lite_wallet_controller.h @@ -363,6 +363,9 @@ public: bool lockWallet(); LiteEncryptionResult decryptWallet(std::string passphrase); LiteEncryptionStatus encryptionStatus(); + // True once (then cleared) if the passphrase encryption of a just-created/restored wallet FAILED, leaving it + // unencrypted. Main thread; the App turns it into a visible error instead of a log line. + bool takeEncryptionFailureNotice(); // Poll sync status + fetch balance/addresses/transactions, and apply the result into the // app's WalletState. Returns true if state was updated. Safe no-op when no wallet is open. @@ -488,6 +491,7 @@ private: enum class PendingSecurityOp { None, Encrypt, Unlock }; std::string pendingLifecyclePassphrase_; // main-thread only; wiped after use PendingSecurityOp pendingLifecycleSecurityOp_ = PendingSecurityOp::None; + bool lifecycleEncryptFailed_ = false; // main-thread only; see takeEncryptionFailureNotice() void stashPendingLifecycleSecurity(const std::string& passphrase, PendingSecurityOp op); void applyPendingLifecycleSecurity(); // encrypt (create/restore) or unlock (open), then wipe void clearPendingLifecycleSecurity(); // wipe + reset (rejected request / failed open) diff --git a/tests/fake_lite_backend.h b/tests/fake_lite_backend.h index f2fac70..7cbd595 100644 --- a/tests/fake_lite_backend.h +++ b/tests/fake_lite_backend.h @@ -52,6 +52,7 @@ inline std::atomic g_liteFakeBlockBalance{false}; // "balance" blocks (a inline std::atomic g_liteFakeBalanceError{false}; // "balance" returns the backend's plain-text sync error inline std::atomic g_liteFakeBalanceEntered{0}; // calls that reached "balance" inline std::atomic g_liteFakeBlockEncrypt{false}; // "encrypt" blocks (before changing state) +inline std::atomic g_liteFakeEncryptFails{false}; // "encrypt" returns an error and changes no state inline std::atomic g_liteFakeBlockImport{false}; // "import"/"timport" block inline std::atomic g_liteFakeImportFails{false}; // "import"/"timport" reject the key (real do_import_key shape) inline std::atomic g_liteFakeImportEncrypted{false}; // "import"/"timport" refuse: the wallet is encrypted @@ -261,6 +262,8 @@ inline char* liteFakeExecute(const char* command, const char* args) // unlock/lock toggle locked; decrypt clears it; encryptionstatus reports {encrypted,locked}. if (std::strcmp(c, "encrypt") == 0) { liteFakeWaitWhile(g_liteFakeBlockEncrypt); + if (g_liteFakeEncryptFails.load()) + return liteFakeDup("{\"result\":\"error\",\"error\":\"encrypt failed\"}"); g_liteFakeEncrypted = true; g_liteFakeLocked = true; return liteFakeDup("{\"result\":\"success\"}"); } diff --git a/tests/test_phase4.cpp b/tests/test_phase4.cpp index 5f381cb..f41df05 100644 --- a/tests/test_phase4.cpp +++ b/tests/test_phase4.cpp @@ -10974,6 +10974,66 @@ void testLiteWalletControllerCreateEncryptsWithPassphrase() EXPECT_FALSE(s2.locked); } +// FIX C: (e) a create-time passphrase whose encryption FAILS is surfaced once via takeEncryptionFailureNotice() and +// leaves the wallet unencrypted; (d) the background scan save is skipped while an encrypted wallet is unlocked (the +// backend save would re-lock it) and resumes once locked. +void testLiteWalletControllerLockUnlockParity() +{ + using namespace dragonx::wallet; + const auto caps = makeWalletCapabilities(WalletBuildKind::Lite, false, true); + const auto conn = defaultLiteConnectionSettings(); + dragonx::test::resetLiteFakeCounters(); + { // (e) + dragonx::test::g_liteFakeEncrypted = false; + dragonx::test::g_liteFakeLocked = false; + dragonx::test::g_liteFakeEncryptFails = true; + auto c = std::make_unique( + caps, conn, LiteClientBridge::fromApi(dragonx::test::makeFakeLiteApi())); + LiteWalletCreateRequest req; + req.passphrase = "hunter2hunter2"; + (void)c->createWallet(req); + EXPECT_TRUE(c->takeEncryptionFailureNotice()); + EXPECT_FALSE(c->takeEncryptionFailureNotice()); // one-shot + EXPECT_FALSE(c->encryptionStatus().encrypted); + dragonx::test::g_liteFakeEncryptFails = false; + } + { // (e) success path raises no notice + dragonx::test::g_liteFakeEncrypted = false; + dragonx::test::g_liteFakeLocked = false; + auto c = std::make_unique( + caps, conn, LiteClientBridge::fromApi(dragonx::test::makeFakeLiteApi())); + LiteWalletCreateRequest req; + req.passphrase = "hunter2hunter2"; + (void)c->createWallet(req); + EXPECT_FALSE(c->takeEncryptionFailureNotice()); + EXPECT_TRUE(c->encryptionStatus().encrypted); + } + { // (d) + dragonx::test::resetLiteFakeCounters(); + dragonx::test::g_liteFakeEncrypted = false; + dragonx::test::g_liteFakeLocked = false; + auto c = std::make_unique( + caps, conn, LiteClientBridge::fromApi(dragonx::test::makeFakeLiteApi())); + EXPECT_TRUE(c->createWallet(LiteWalletCreateRequest{}).walletReady); + for (int i = 0; i < 600 && !c->syncComplete(); ++i) std::this_thread::sleep_for(std::chrono::milliseconds(5)); + EXPECT_TRUE(c->encryptWallet("pw12345678").ok); + EXPECT_TRUE(c->unlockWallet("pw12345678")); + EXPECT_FALSE(c->encryptionStatus().locked); + const long saves0 = dragonx::test::g_liteFakeSaveCount.load(); + EXPECT_TRUE(c->startRescan()); + for (int i = 0; i < 600 && !c->syncComplete(); ++i) std::this_thread::sleep_for(std::chrono::milliseconds(5)); + EXPECT_TRUE(c->syncComplete()); + EXPECT_EQ(dragonx::test::g_liteFakeSaveCount.load(), saves0); // no background save while unlocked + EXPECT_FALSE(c->encryptionStatus().locked); // so the unlock survived the scan + EXPECT_TRUE(c->lockWallet()); + EXPECT_TRUE(c->startRescan()); + for (int i = 0; i < 600 && !c->syncComplete(); ++i) std::this_thread::sleep_for(std::chrono::milliseconds(5)); + EXPECT_TRUE(dragonx::test::g_liteFakeSaveCount.load() > saves0); // saves resume once locked + } + dragonx::test::g_liteFakeEncrypted = false; + dragonx::test::g_liteFakeLocked = false; +} + void testLiteWalletControllerAsyncLifecycleFailover() { using namespace dragonx::wallet; @@ -19608,6 +19668,7 @@ int main() testLiteWalletControllerM5Persistence(); testLiteWalletControllerEncryption(); testLiteWalletControllerCreateEncryptsWithPassphrase(); + testLiteWalletControllerLockUnlockParity(); testLiteWalletControllerAsyncAppliesPassphrase(); testLiteNewPassphraseRules(); testLiteChainNameMigration();