Compare commits
6 Commits
d188a08db7
...
8c12b27c0a
| Author | SHA1 | Date | |
|---|---|---|---|
| 8c12b27c0a | |||
| c7d163f44a | |||
| 7e4822c021 | |||
| f9b622cb25 | |||
| 9204fa148a | |||
| da0e9f5915 |
128
docs/wallet-hardening.md
Normal file
128
docs/wallet-hardening.md
Normal file
@@ -0,0 +1,128 @@
|
||||
# Wallet Loading & Management — Hardening Plan
|
||||
|
||||
Prioritized, grouped remediation for the wallet loading/management audit (33 verified findings +
|
||||
diagnosability QoL). Companion to the findings artifact. Line references are against `dev`.
|
||||
|
||||
- **Provenance:** 7 parallel subsystem finders, each finding adversarially verified against the
|
||||
code; the 3 highest-impact confirmed findings re-checked by hand. 32 confirmed, 1 refuted
|
||||
(W1-5), 1 raised (W5-3 Low→Med).
|
||||
- **Severity:** 8 High · 12 Medium · 13 Low.
|
||||
|
||||
Status legend: ☐ not started · ◐ in progress · ☑ landed & verified
|
||||
|
||||
---
|
||||
|
||||
## Roadmap (ordered by risk; shared fixes grouped)
|
||||
|
||||
| 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) | ☑ |
|
||||
| **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 | ☐ |
|
||||
| **F** | W7-2, W7-3, W7-4, QoL | Diagnostics foundation + QoL bundle | ☐ |
|
||||
|
||||
---
|
||||
|
||||
## P0-A — Secret hardening
|
||||
|
||||
Shared fix: a `SecureString` RAII buffer (zeroes on destruction) retrofitted onto the un-scrubbed
|
||||
key/passphrase paths, plus console redaction and deleting the plaintext export.
|
||||
|
||||
- **W7-1 (High)** `console_tab.cpp:1419` — RPC console echoes/stores/clipboards raw secrets. Fix: an
|
||||
allowlist of secret-bearing first-tokens (`walletpassphrase`, `walletpassphrasechange`,
|
||||
`encryptwallet`, `importprivkey`, `importwallet`, `z_importkey`, `z_importviewingkey`,
|
||||
`signrawtransaction`, `magicrecoverkey`, lite equivalents); echo `> walletpassphrase ****` and
|
||||
keep the raw text out of `command_history_`. Extract a pure `redactConsoleCommand(cmd)` helper for
|
||||
unit testing. **← implementing first (self-contained + testable).**
|
||||
- **W2-1 (High)** `wallet_security_workflow.cpp:66` — delete the `obsidiandecryptexport<ts>` plaintext
|
||||
key dump after `z_importwallet` succeeds (overwrite-then-unlink).
|
||||
- **W4-3 (High)** `app_network.cpp:4481` — `sodium_memzero` the concatenated all-keys string in
|
||||
`exportAllKeys`; write the backup 0600. (Also unify with `ExportAllKeysDialog` — QoL.)
|
||||
- **W4-1 (High)** `app_network.cpp:3801` — zero the key copies in `importPrivateKey`/`sweepPrivateKey`
|
||||
(local + worker-lambda copies).
|
||||
- **W2-3 (Med)** `app_security.cpp:1481` — zero the passphrase threaded through the decrypt lambda chain.
|
||||
- **W4-5 (Med)** `app.cpp:3577` — the seed-backup `.txt` is a permanent predictable cleartext seed;
|
||||
at minimum warn + offer to delete, ideally discourage file save in favor of the on-screen phrase.
|
||||
- **W5-3 (Med)** `lite_wallet_lifecycle_service.cpp:322` — remove the dead `passphrase` field from the
|
||||
lite create/open/restore requests (unused; a secret copied for nothing).
|
||||
|
||||
## P0-B — Encryption integrity
|
||||
|
||||
- **W2-2 / W4-2 (High)** `wallet_security_controller.h:89` — the wizard's deferred encryption is
|
||||
in-memory only and silently lost if the daemon doesn't connect or the app quits/crashes first, so a
|
||||
wallet the user believes is encrypted stays plaintext. Fix: persist a lightweight
|
||||
`encryption_requested_but_incomplete` settings flag (NEVER the passphrase) when
|
||||
`beginDeferredEncryption` is called; surface a persistent warning banner while it's set; clear it
|
||||
only on confirmed `encryptwallet` success; on next connect, if set, re-prompt for the passphrase to
|
||||
complete it.
|
||||
- **W2-4 (Med)** `app_security.cpp:480` — `lockWallet` only sets `locked` on RPC success; log the
|
||||
failure and notify (currently a silent no-op that can leave the wallet unlocked).
|
||||
|
||||
## P1-A — Migrate-to-seed correctness (fund-adjacent; verify carefully)
|
||||
|
||||
- **W3-1 (High)** `app_network.cpp:4327` — adopt hardcodes `datadir + "/wallet.dat"`; use
|
||||
`settings_->getActiveWalletFile()` so migrating a non-default active wallet swaps the right file.
|
||||
- **W3-2 (High)** `seed_wallet_creator.cpp:57` — `remove_all(<config>/seed-migrate)` unconditionally
|
||||
at Phase-1 start; refuse to wipe if a temp `DRAGONX/wallet.dat` already exists (a prior un-adopted
|
||||
swept wallet) and surface it, so swept funds in the temp wallet can't be destroyed by re-entry.
|
||||
- **W3-4 (Med)** `app_network.cpp:1124` — block wallet switching while a migration is *pending*
|
||||
(`getSeedMigrationPending()`), not only while the dialog is open.
|
||||
- **W3-3 (Med)** `app_network.cpp:4231` — persist the sweep opid so an app-close mid-Sweeping can
|
||||
resume/re-poll it instead of silently dropping the txid.
|
||||
|
||||
## P1-B — Missing/wrong wallet-file safety
|
||||
|
||||
- **W1-1 (High)** `app_network.cpp:1109` — `fs::exists()`-check the target wallet file in
|
||||
`switchToWallet()` and before the first daemon launch at startup; if missing, block with an explicit
|
||||
"Wallet file not found — moved or deleted?" dialog (browse / create-new) instead of letting the
|
||||
daemon fabricate an empty wallet.
|
||||
- **W1-3 (Med)** `app_network.cpp:1095` — defer the `syncedHere=true` stamp to the first successful
|
||||
address/balance readback (idHash non-empty), not bare `onConnected()`.
|
||||
- **W1-2 (Med)** `app_network.cpp:198` — split `DB_CORRUPT`-specific strings from the generic "Error
|
||||
loading wallet" fallback; give `DB_TOO_NEW` its own message/action (not a salvage offer).
|
||||
- **W1-4 (Low)** `wallets_dialog.h:393` — re-`fs::exists()` the in-datadir row before switching (match
|
||||
the out-of-datadir path).
|
||||
|
||||
## P2 — State & lite persistence
|
||||
|
||||
- **W6-2 (Med)** `network_refresh_service.cpp:1183` — record a per-field last-success timestamp / a
|
||||
"refresh failed" flag so the UI can show a staleness badge instead of last-good-as-current.
|
||||
- **W5-1 / W5-2 (Med)** `lite_wallet_controller.cpp:78,603` — `liteLog()` the failed save and bubble a
|
||||
one-shot UI warning (both call sites currently discard the bool).
|
||||
- **W6-1 (Med)** `wallet_state.h:313` — reset `mining`/`pool_mining` in `clear()` (or comment why not).
|
||||
- **W6-3 (Low)** `address_book.cpp:46` — per-entry try/catch: skip + count malformed entries instead
|
||||
of discarding the whole list.
|
||||
|
||||
## F — Diagnostics foundation + QoL
|
||||
|
||||
Land W7-2 first — it unblocks the rest.
|
||||
|
||||
- **W7-2 (Med)** `logger.cpp:31` — call `Logger::instance().init(<config>/dragonx-debug.log)` early in
|
||||
`main()` on all platforms; add an "Open log folder" action.
|
||||
- **W7-3 (Med)** `main.cpp:144` — add a `sigaction`-based crash handler writing `dragonx-crash.log` on
|
||||
POSIX (mirror the Windows SEH path).
|
||||
- **W7-4 (Low)** `logger.cpp:39` — size-cap/rotate the log on `init()`.
|
||||
- **QoL** — "Copy diagnostics for support" bundle; persistent alert history; daemon/RPC error banner;
|
||||
refresh-staleness badge; multi-wallet diagnostic panel; refresh-diagnostics panel; structured
|
||||
switch/migration audit logging; restore-from-seed entry point (W4-4, effort L).
|
||||
|
||||
---
|
||||
|
||||
## 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 <path>". It now reads "**Saved an UNENCRYPTED seed file — move it to secure offline storage and delete this copy**: <path>", 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:
|
||||
- **W4-1** `importPrivateKey`/`sweepPrivateKey`: the spending/viewing key is now scrubbed on all paths — the calling-frame copy (after the worker post), the worker-lambda's captured copy (lambda made `mutable`, zeroed after the request is sent), and the JSON request `params` copy.
|
||||
- **W4-3** `exportAllKeys`/`backupWallet`: the concatenated all-keys buffer is zeroed after the consumer uses it, and the backup file is now written via `Platform::writeFileAtomically(..., restrictPermissions=true)` (atomic + 0600) instead of a umask-default `ofstream`.
|
||||
- **W2-3** decrypt-wallet passphrase: `std::move`-captured into the worker lambda (so no plaintext copy is left in the calling frame) and `sodium_memzero`'d right after `unlockWallet` (its only use).
|
||||
Not unit-testable (the scrubbing has no observable RPC effect — the key value sent to the daemon is unchanged; only post-use memory zeroing is added). Build-clean; `ctest` 1/1 (no regression). **Remaining in P0-A:** W5-3 (remove the dead lite `passphrase` field), W4-5 (predictable plaintext seed-backup file).
|
||||
- **P0-A / W2-1** — ☑ landed: the decrypt-wallet flow now scrubs (best-effort in-place zero-overwrite) and removes the plaintext key export (`obsidiandecryptexport…`) as soon as the `z_importwallet` attempt resolves — success or failure — so a full cleartext dump of every private key is no longer left on disk forever. Recovery remains the encrypted backup (`wallet.dat.encrypted.bak`). `app_security.cpp` (after the import call). Not unit-testable (fs I/O in a deep lambda); build-clean, `ctest` 1/1 (no regression).
|
||||
- **P0-A / W7-1** — ☑ landed: `RedactConsoleCommand`/`ConsoleCommandCarriesSecret` in `console_tab_helpers` redact secret-bearing commands (an allowlist of 13 first-tokens: `walletpassphrase`, `encryptwallet`, `z_importkey`, …) to `> walletpassphrase ****` before they hit the console echo AND the recall history; the real command still executes unredacted. Wired into `submitConsoleCommand` (`console_tab.cpp`). New `testConsoleSecretRedaction` (11 assertions). Clean build; `ctest` 1/1. (Output-secret commands like `z_exportkey` — result redaction — remain a follow-up.)
|
||||
@@ -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;
|
||||
|
||||
@@ -3783,6 +3783,8 @@ void App::exportAllKeys(std::function<void(const std::string&, int, int)> callba
|
||||
(*pending)--;
|
||||
if (*pending == 0 && callback) {
|
||||
callback(*keys_result, *exported, *total);
|
||||
// Scrub the concatenated all-keys buffer once the consumer (backup writer) has used it.
|
||||
if (!keys_result->empty()) sodium_memzero(&(*keys_result)[0], keys_result->size());
|
||||
}
|
||||
});
|
||||
}
|
||||
@@ -3814,7 +3816,7 @@ void App::importPrivateKey(const std::string& rawKey, int startHeight,
|
||||
== services::WalletSecurityController::KeyKind::Shielded;
|
||||
// Run on the worker thread — import requests a full rescan (rescan=true), so the
|
||||
// synchronous curl call can take many seconds; never block the UI thread on it.
|
||||
worker_->post([this, key, viewing, shielded, startHeight, callback]() -> rpc::RPCWorker::MainCb {
|
||||
worker_->post([this, key, viewing, shielded, startHeight, callback]() mutable -> rpc::RPCWorker::MainCb {
|
||||
std::string err, addr;
|
||||
try {
|
||||
rpc::RPCClient::TraceScope trace("Settings / Import key");
|
||||
@@ -3826,6 +3828,11 @@ void App::importPrivateKey(const std::string& rawKey, int startHeight,
|
||||
// A start height (shielded RPCs only) rescans from that block instead of genesis.
|
||||
if (startHeight > 0 && (viewing || shielded)) params.push_back(startHeight);
|
||||
nlohmann::json r = rpc_->call(method, params);
|
||||
// Scrub the key out of the request params (the json holds its own copy of it).
|
||||
if (params.is_array() && !params.empty() && params[0].is_string()) {
|
||||
std::string& pk = params[0].get_ref<std::string&>();
|
||||
if (!pk.empty()) sodium_memzero(&pk[0], pk.size());
|
||||
}
|
||||
// z_import* return {type,address}; importprivkey returns the t-address string.
|
||||
if (r.is_object() && r.contains("address") && r["address"].is_string())
|
||||
addr = r["address"].get<std::string>();
|
||||
@@ -3838,6 +3845,8 @@ void App::importPrivateKey(const std::string& rawKey, int startHeight,
|
||||
// below would never run, leaving a stuck "Importing…" spinner.
|
||||
err = "Import failed (unknown error)";
|
||||
}
|
||||
// Scrub the worker's copy of the key now that the request has been sent (all paths).
|
||||
if (!key.empty()) sodium_memzero(&key[0], key.size());
|
||||
return [this, err, addr, callback]() {
|
||||
if (!err.empty()) {
|
||||
if (callback) callback(false, err, "");
|
||||
@@ -3848,6 +3857,7 @@ void App::importPrivateKey(const std::string& rawKey, int startHeight,
|
||||
if (callback) callback(true, "", addr);
|
||||
};
|
||||
});
|
||||
if (!key.empty()) sodium_memzero(&key[0], key.size()); // scrub the calling-frame copy
|
||||
}
|
||||
|
||||
// Sweep a spending key: import it (a full rescan populates its UTXOs/notes — the stock node has no
|
||||
@@ -3885,7 +3895,7 @@ void App::sweepPrivateKey(const std::string& rawKey, int startHeight, int destMo
|
||||
const bool shielded = services::WalletSecurityController::classifyPrivateKey(key)
|
||||
== services::WalletSecurityController::KeyKind::Shielded;
|
||||
const double fee = DRAGONX_DEFAULT_FEE;
|
||||
worker_->post([this, key, startHeight, destMode, destExisting, shielded, fee]() -> rpc::RPCWorker::MainCb {
|
||||
worker_->post([this, key, startHeight, destMode, destExisting, shielded, fee]() mutable -> rpc::RPCWorker::MainCb {
|
||||
std::string err, dest, sourceAddr, amountStr;
|
||||
double amount = 0.0;
|
||||
try {
|
||||
@@ -3909,6 +3919,11 @@ void App::sweepPrivateKey(const std::string& rawKey, int startHeight, int destMo
|
||||
else { method = "importprivkey"; params = {key, "", true}; }
|
||||
if (startHeight > 0 && shielded) params.push_back(startHeight);
|
||||
nlohmann::json r = rpc_->call(method, params);
|
||||
// Scrub the key out of the request params (the json holds its own copy of it).
|
||||
if (params.is_array() && !params.empty() && params[0].is_string()) {
|
||||
std::string& pk = params[0].get_ref<std::string&>();
|
||||
if (!pk.empty()) sodium_memzero(&pk[0], pk.size());
|
||||
}
|
||||
|
||||
// 2. Determine the swept address. importprivkey returns the t-address string; z_importkey
|
||||
// returns null, so diff the z-address list to find the one the key just added.
|
||||
@@ -3967,6 +3982,8 @@ void App::sweepPrivateKey(const std::string& rawKey, int startHeight, int destMo
|
||||
} catch (...) {
|
||||
err = "Sweep failed (unknown error)";
|
||||
}
|
||||
// Scrub the worker's copy of the spending key now that the request has been sent (all paths).
|
||||
if (!key.empty()) sodium_memzero(&key[0], key.size());
|
||||
return [this, err, sourceAddr, dest, amount, amountStr, fee]() {
|
||||
invalidateAddressValidationCache();
|
||||
refreshAddresses();
|
||||
@@ -4003,6 +4020,7 @@ void App::sweepPrivateKey(const std::string& rawKey, int startHeight, int destMo
|
||||
});
|
||||
};
|
||||
});
|
||||
if (!key.empty()) sodium_memzero(&key[0], key.size()); // scrub the calling-frame copy
|
||||
}
|
||||
|
||||
void App::exportSeedPhrase(std::function<void(bool, bool, const std::string&, const std::string&)> callback)
|
||||
@@ -4478,13 +4496,12 @@ void App::backupWallet(const std::string& destination, std::function<void(bool,
|
||||
return;
|
||||
}
|
||||
|
||||
std::ofstream file(destination);
|
||||
if (!file.is_open()) {
|
||||
if (callback) callback(false, "Could not open file: " + destination);
|
||||
// Write the key backup atomically and owner-only (0600) — it must never be even briefly
|
||||
// world-readable, and the previous std::ofstream left it at the umask default.
|
||||
if (!util::Platform::writeFileAtomically(destination, keys, /*restrictPermissions=*/true)) {
|
||||
if (callback) callback(false, "Could not write file: " + destination);
|
||||
return;
|
||||
}
|
||||
file << keys;
|
||||
file.close();
|
||||
|
||||
std::string msg = "Wallet backup saved to " + destination + " — "
|
||||
+ std::to_string(exported) + " of " + std::to_string(total) + " keys.";
|
||||
|
||||
@@ -33,6 +33,10 @@
|
||||
#include <ctime>
|
||||
#include <cstdint>
|
||||
#include <filesystem>
|
||||
#include <fstream>
|
||||
#include <vector>
|
||||
#include <utility>
|
||||
#include <sodium.h>
|
||||
#include <functional>
|
||||
#include <memory>
|
||||
#include <utility>
|
||||
@@ -483,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);
|
||||
}
|
||||
}
|
||||
};
|
||||
});
|
||||
@@ -560,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()) {
|
||||
@@ -572,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 {
|
||||
@@ -1478,12 +1511,14 @@ void App::renderDecryptWalletDialog() {
|
||||
|
||||
// Run entire decrypt flow on worker thread
|
||||
if (worker_) {
|
||||
worker_->post([this, passphrase]() -> rpc::RPCWorker::MainCb {
|
||||
worker_->post([this, passphrase = std::move(passphrase)]() mutable -> rpc::RPCWorker::MainCb {
|
||||
WalletSecurityDecryptRpcAdapter decryptRpc(rpc_.get(),
|
||||
[this](rpc::RPCClient& client, const char* context) {
|
||||
return sendStopCommandSafely(client, context);
|
||||
});
|
||||
auto unlock = services::WalletSecurityWorkflowExecutor::unlockWallet(passphrase, decryptRpc);
|
||||
// Scrub the passphrase — unlock is its only use in this flow.
|
||||
if (!passphrase.empty()) sodium_memzero(&passphrase[0], passphrase.size());
|
||||
if (!unlock.ok) {
|
||||
return [this]() {
|
||||
wallet_security_workflow_.failEntry("Incorrect passphrase");
|
||||
@@ -1606,6 +1641,27 @@ void App::renderDecryptWalletDialog() {
|
||||
WalletSecurityImportRpcAdapter importAdapter(rpc_.get(), saved_config_);
|
||||
auto importResult = services::WalletSecurityWorkflowExecutor::importWallet(
|
||||
importAdapter, exportPath);
|
||||
|
||||
// The plaintext key export (obsidiandecryptexport…) has served its purpose now
|
||||
// that the import attempt has resolved — scrub and remove it so a full cleartext
|
||||
// dump of every private key isn't left on disk forever. Recovery, if ever needed,
|
||||
// is the encrypted backup (wallet.dat.encrypted.bak), never this file.
|
||||
{
|
||||
std::error_code delEc;
|
||||
const auto sz = std::filesystem::file_size(exportPath, delEc);
|
||||
if (!delEc && sz > 0) {
|
||||
std::fstream scrub(exportPath,
|
||||
std::ios::binary | std::ios::in | std::ios::out);
|
||||
if (scrub) {
|
||||
const std::vector<char> zeros(static_cast<size_t>(sz), 0);
|
||||
scrub.write(zeros.data(), static_cast<std::streamsize>(sz));
|
||||
scrub.flush();
|
||||
}
|
||||
}
|
||||
std::filesystem::remove(exportPath, delEc);
|
||||
DEBUG_LOGF("[decrypt] removed plaintext key export after import\n");
|
||||
}
|
||||
|
||||
if (!importResult.ok) {
|
||||
std::string err = importResult.error;
|
||||
if (worker_) {
|
||||
|
||||
@@ -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_));
|
||||
|
||||
@@ -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_;
|
||||
|
||||
@@ -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<std::string, AddressMeta> 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=<name> the daemon loads (multi-wallet)
|
||||
bool seed_migration_pending_ = false;
|
||||
|
||||
@@ -1416,8 +1416,11 @@ bool ConsoleTab::submitConsoleCommand(ConsoleCommandExecutor& exec, const std::s
|
||||
{
|
||||
if (cmd.empty()) return false;
|
||||
|
||||
addLine("> " + cmd, ConsoleChannel::Command);
|
||||
AppendConsoleHistory(command_history_, cmd, 100);
|
||||
// Redact secret-bearing commands (walletpassphrase, z_importkey, …) before they reach the visible
|
||||
// log and the recall history. The real `cmd` below is still executed unredacted.
|
||||
const std::string display = RedactConsoleCommand(cmd);
|
||||
addLine("> " + display, ConsoleChannel::Command);
|
||||
AppendConsoleHistory(command_history_, display, 100);
|
||||
history_index_ = -1;
|
||||
|
||||
// First token, lowercased, for built-in interception.
|
||||
|
||||
@@ -1,10 +1,34 @@
|
||||
#include "console_tab_helpers.h"
|
||||
|
||||
#include <algorithm>
|
||||
#include <cctype>
|
||||
|
||||
namespace dragonx {
|
||||
namespace ui {
|
||||
|
||||
namespace {
|
||||
// First tokens (lowercase) of console/RPC commands that carry a secret argument on the command line.
|
||||
// Output-secret commands (dumpprivkey / z_exportkey / z_exportmnemonic) are deliberately absent —
|
||||
// their secret is in the RESULT, which is a separate redaction concern.
|
||||
const char* const kSecretConsoleCommands[] = {
|
||||
"walletpassphrase", "walletpassphrasechange", "encryptwallet",
|
||||
"importprivkey", "importwallet", "importmulti",
|
||||
"z_importkey", "z_importviewingkey", "z_importwallet",
|
||||
"signrawtransaction", "magicrecoverkey", "sethdseed", "importmnemonic",
|
||||
};
|
||||
|
||||
std::string firstConsoleTokenLower(const std::string& cmd, size_t& tokenEnd) {
|
||||
size_t b = cmd.find_first_not_of(" \t");
|
||||
if (b == std::string::npos) { tokenEnd = cmd.size(); return {}; }
|
||||
size_t e = cmd.find_first_of(" \t", b);
|
||||
tokenEnd = (e == std::string::npos) ? cmd.size() : e;
|
||||
std::string t = cmd.substr(b, tokenEnd - b);
|
||||
std::transform(t.begin(), t.end(), t.begin(),
|
||||
[](unsigned char c) { return static_cast<char>(std::tolower(c)); });
|
||||
return t;
|
||||
}
|
||||
} // namespace
|
||||
|
||||
float ComputeConsoleInputHeight(float frameHeightWithSpacing,
|
||||
float itemSpacingY,
|
||||
float spacingSm,
|
||||
@@ -27,5 +51,27 @@ float ClampConsoleWrapWidth(float contentWidth, float paddingX)
|
||||
return std::max(50.0f, contentWidth - paddingX * 2.0f);
|
||||
}
|
||||
|
||||
bool ConsoleCommandCarriesSecret(const std::string& cmd)
|
||||
{
|
||||
size_t end = 0;
|
||||
const std::string name = firstConsoleTokenLower(cmd, end);
|
||||
if (name.empty()) return false;
|
||||
for (const char* s : kSecretConsoleCommands) if (name == s) return true;
|
||||
return false;
|
||||
}
|
||||
|
||||
std::string RedactConsoleCommand(const std::string& cmd)
|
||||
{
|
||||
size_t end = 0;
|
||||
const std::string name = firstConsoleTokenLower(cmd, end);
|
||||
if (name.empty()) return cmd;
|
||||
bool secret = false;
|
||||
for (const char* s : kSecretConsoleCommands) if (name == s) { secret = true; break; }
|
||||
if (!secret) return cmd;
|
||||
// Only redact if there are actually arguments after the command name.
|
||||
if (cmd.find_first_not_of(" \t", end) == std::string::npos) return cmd;
|
||||
return cmd.substr(0, end) + " ****";
|
||||
}
|
||||
|
||||
} // namespace ui
|
||||
} // namespace dragonx
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
#pragma once
|
||||
|
||||
#include <string>
|
||||
|
||||
namespace dragonx {
|
||||
namespace ui {
|
||||
|
||||
@@ -14,5 +16,14 @@ float ComputeConsoleOutputHeight(float availableHeight,
|
||||
float minHeightRatio);
|
||||
float ClampConsoleWrapWidth(float contentWidth, float paddingX);
|
||||
|
||||
// True if `cmd`'s first token names a console/RPC command that carries a SECRET on its command line
|
||||
// (passphrase, private/spending/viewing key, mnemonic). Output-secret commands (dumpprivkey,
|
||||
// z_exportkey, z_exportmnemonic) are NOT covered — their secret is in the result, a separate concern.
|
||||
bool ConsoleCommandCarriesSecret(const std::string& cmd);
|
||||
|
||||
// A display/history-safe copy of `cmd`: the command name with its arguments replaced by "****" when
|
||||
// it carries a secret, else `cmd` unchanged. The real command is still executed unredacted.
|
||||
std::string RedactConsoleCommand(const std::string& cmd);
|
||||
|
||||
} // namespace ui
|
||||
} // namespace dragonx
|
||||
|
||||
@@ -320,7 +320,7 @@ void I18n::loadBuiltinEnglish()
|
||||
strings_["seed_backup_load_failed"] = "Could not load the seed phrase.";
|
||||
strings_["seed_backup_copy"] = "Copy";
|
||||
strings_["seed_backup_save"] = "Save to file…";
|
||||
strings_["seed_backup_saved"] = "Saved to ";
|
||||
strings_["seed_backup_saved"] = "Saved an UNENCRYPTED seed file — move it to secure offline storage and delete this copy: ";
|
||||
strings_["seed_backup_save_failed"] = "Could not write ";
|
||||
strings_["seed_backup_close"] = "Close";
|
||||
strings_["seed_backup_reminder"] = "Your wallet has a 24-word recovery seed phrase. Back it up now in Settings → Node & Security.";
|
||||
|
||||
@@ -1172,25 +1172,47 @@ void LiteWalletController::workerLoop()
|
||||
LiteWalletLifecycleResult LiteWalletController::createWallet(LiteWalletCreateRequest request)
|
||||
{
|
||||
auto result = lifecycle_.createWallet(request);
|
||||
secureWipeLiteSecret(request.passphrase);
|
||||
onLifecycleResult(result);
|
||||
// If the user supplied a passphrase, encrypt the brand-new wallet with it now that it's open
|
||||
// (the backend encrypts + locks + saves). Previously this passphrase was collected but never
|
||||
// used (W5-3) — a passphrase field that silently did nothing. encryptWallet() takes its own
|
||||
// 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);
|
||||
}
|
||||
secureWipeLiteSecret(request.passphrase);
|
||||
return result;
|
||||
}
|
||||
|
||||
LiteWalletLifecycleResult LiteWalletController::openWallet(LiteWalletOpenRequest request)
|
||||
{
|
||||
auto result = lifecycle_.openWallet(request);
|
||||
secureWipeLiteSecret(request.passphrase);
|
||||
onLifecycleResult(result);
|
||||
// An existing wallet may be encrypted + locked — use the supplied passphrase to unlock it so it
|
||||
// opens ready to use. Only meaningful when the wallet is actually locked (W5-3).
|
||||
if (walletOpen_.load() && !request.passphrase.empty()) {
|
||||
const auto encStatus = encryptionStatus();
|
||||
if (encStatus.ok && encStatus.encrypted && encStatus.locked) {
|
||||
if (!unlockWallet(request.passphrase))
|
||||
liteLog("wallet opened but unlock failed (wrong passphrase?)");
|
||||
}
|
||||
}
|
||||
secureWipeLiteSecret(request.passphrase);
|
||||
return result;
|
||||
}
|
||||
|
||||
LiteWalletLifecycleResult LiteWalletController::restoreWallet(LiteWalletRestoreRequest request)
|
||||
{
|
||||
auto result = lifecycle_.restoreWallet(request);
|
||||
onLifecycleResult(result);
|
||||
// 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);
|
||||
}
|
||||
secureWipeLiteSecret(request.seedPhrase);
|
||||
secureWipeLiteSecret(request.passphrase);
|
||||
onLifecycleResult(result);
|
||||
return result;
|
||||
}
|
||||
|
||||
|
||||
@@ -2523,6 +2523,29 @@ void testAllowsPlaintextRemote()
|
||||
EXPECT_TRUE(!Connection::usesPlaintextRemote(remoteTls)); // TLS → not plaintext, never refused
|
||||
}
|
||||
|
||||
void testConsoleSecretRedaction()
|
||||
{
|
||||
using dragonx::ui::RedactConsoleCommand;
|
||||
using dragonx::ui::ConsoleCommandCarriesSecret;
|
||||
|
||||
// Secret-bearing commands are recognized (case- and whitespace-insensitive on the name).
|
||||
EXPECT_TRUE(ConsoleCommandCarriesSecret("walletpassphrase myPass 60"));
|
||||
EXPECT_TRUE(ConsoleCommandCarriesSecret("z_importkey SK-secret"));
|
||||
EXPECT_TRUE(ConsoleCommandCarriesSecret(" ENCRYPTWALLET topsecret"));
|
||||
EXPECT_TRUE(!ConsoleCommandCarriesSecret("getinfo"));
|
||||
EXPECT_TRUE(!ConsoleCommandCarriesSecret("getwalletinfo")); // not a false-positive substring match
|
||||
|
||||
// Redaction replaces the arguments with **** but preserves the (original-case) command name.
|
||||
EXPECT_EQ(RedactConsoleCommand("walletpassphrase myPass 60"), std::string("walletpassphrase ****"));
|
||||
EXPECT_EQ(RedactConsoleCommand("z_importkey SK-secret-key"), std::string("z_importkey ****"));
|
||||
EXPECT_EQ(RedactConsoleCommand("ENCRYPTWALLET topsecret"), std::string("ENCRYPTWALLET ****"));
|
||||
// A bare secret command with no argument is left unchanged (nothing to hide).
|
||||
EXPECT_EQ(RedactConsoleCommand("walletpassphrase"), std::string("walletpassphrase"));
|
||||
// Non-secret commands pass through untouched.
|
||||
EXPECT_EQ(RedactConsoleCommand("sendtoaddress addr 1.0"), std::string("sendtoaddress addr 1.0"));
|
||||
EXPECT_EQ(RedactConsoleCommand("getwalletinfo"), std::string("getwalletinfo"));
|
||||
}
|
||||
|
||||
void testConnectHasStalled()
|
||||
{
|
||||
using dragonx::util::connectHasStalled;
|
||||
@@ -4377,7 +4400,6 @@ void testLiteWalletControllerLifecycle()
|
||||
EXPECT_FALSE(controller.walletOpen());
|
||||
|
||||
LiteWalletCreateRequest req;
|
||||
req.passphrase = "hunter2";
|
||||
const auto result = controller.createWallet(req);
|
||||
EXPECT_TRUE(result.ok);
|
||||
EXPECT_TRUE(result.walletReady);
|
||||
@@ -4394,7 +4416,6 @@ void testLiteWalletControllerLifecycle()
|
||||
dragonx::test::g_liteFakeWalletExists = true;
|
||||
LiteWalletController controller(liteCaps, conn, LiteClientBridge::fromApi(dragonx::test::makeFakeLiteApi()));
|
||||
LiteWalletOpenRequest req;
|
||||
req.passphrase = "hunter2";
|
||||
const auto result = controller.openWallet(req);
|
||||
EXPECT_TRUE(result.ok);
|
||||
EXPECT_TRUE(result.walletReady);
|
||||
@@ -4469,7 +4490,6 @@ void testLiteWalletControllerM4()
|
||||
auto c = std::make_unique<LiteWalletController>(
|
||||
liteCaps, conn, LiteClientBridge::fromApi(dragonx::test::makeFakeLiteApi()));
|
||||
LiteWalletCreateRequest req;
|
||||
req.passphrase = "hunter2";
|
||||
(void)c->createWallet(req);
|
||||
return c;
|
||||
};
|
||||
@@ -4597,7 +4617,6 @@ void testLiteWalletControllerM5Persistence()
|
||||
auto c = std::make_unique<LiteWalletController>(
|
||||
liteCaps, conn, LiteClientBridge::fromApi(dragonx::test::makeFakeLiteApi()));
|
||||
LiteWalletCreateRequest req;
|
||||
req.passphrase = "hunter2";
|
||||
(void)c->createWallet(req);
|
||||
return c;
|
||||
};
|
||||
@@ -4679,7 +4698,6 @@ void testLiteWalletControllerEncryption()
|
||||
auto c = std::make_unique<LiteWalletController>(
|
||||
liteCaps, conn, LiteClientBridge::fromApi(dragonx::test::makeFakeLiteApi()));
|
||||
LiteWalletCreateRequest req;
|
||||
req.passphrase = "hunter2";
|
||||
(void)c->createWallet(req);
|
||||
return c;
|
||||
};
|
||||
@@ -4910,6 +4928,33 @@ void testLiteWalletControllerConsoleCommand()
|
||||
// Async FULL lifecycle (Settings-page create/open/restore WITH passphrase/restore params) also
|
||||
// fails over: the request runs off the UI thread against the preferred server, then the other
|
||||
// usable defaults, finalized by pumpLifecycleResult() on the main thread.
|
||||
// W5-3: a create-time passphrase now actually encrypts (and locks) the new lite wallet, and it
|
||||
// unlocks with the same passphrase — previously the field was collected but ignored.
|
||||
void testLiteWalletControllerCreateEncryptsWithPassphrase()
|
||||
{
|
||||
using namespace dragonx::wallet;
|
||||
const auto liteCaps = makeWalletCapabilities(WalletBuildKind::Lite, false, true);
|
||||
const LiteConnectionSettings conn = defaultLiteConnectionSettings();
|
||||
|
||||
dragonx::test::g_liteFakeEncrypted = false;
|
||||
dragonx::test::g_liteFakeLocked = false;
|
||||
auto c = std::make_unique<LiteWalletController>(
|
||||
liteCaps, conn, LiteClientBridge::fromApi(dragonx::test::makeFakeLiteApi()));
|
||||
|
||||
LiteWalletCreateRequest req;
|
||||
req.passphrase = "hunter2";
|
||||
(void)c->createWallet(req);
|
||||
|
||||
const auto s = c->encryptionStatus();
|
||||
EXPECT_TRUE(s.ok);
|
||||
EXPECT_TRUE(s.encrypted); // the create-time passphrase encrypted the new wallet
|
||||
EXPECT_TRUE(s.locked); // encrypt locks immediately
|
||||
|
||||
EXPECT_TRUE(c->unlockWallet("hunter2"));
|
||||
const auto s2 = c->encryptionStatus();
|
||||
EXPECT_FALSE(s2.locked);
|
||||
}
|
||||
|
||||
void testLiteWalletControllerAsyncLifecycleFailover()
|
||||
{
|
||||
using namespace dragonx::wallet;
|
||||
@@ -4938,7 +4983,6 @@ void testLiteWalletControllerAsyncLifecycleFailover()
|
||||
LiteWalletController controller(liteCaps, conn,
|
||||
LiteClientBridge::fromApi(dragonx::test::makeFakeLiteApi()));
|
||||
LiteWalletCreateRequest req;
|
||||
req.passphrase = "hunter2";
|
||||
EXPECT_TRUE(controller.beginCreateWalletAsync(req));
|
||||
drain(controller);
|
||||
EXPECT_TRUE(controller.walletOpen());
|
||||
@@ -6861,6 +6905,7 @@ int main()
|
||||
testConnectHasStalled();
|
||||
testIsLocalHost();
|
||||
testAllowsPlaintextRemote();
|
||||
testConsoleSecretRedaction();
|
||||
testDaemonLifecycleExecution();
|
||||
testDaemonLifecycleAdapters();
|
||||
testConsoleTextLayout();
|
||||
@@ -6895,6 +6940,7 @@ int main()
|
||||
testLiteWalletControllerM4();
|
||||
testLiteWalletControllerM5Persistence();
|
||||
testLiteWalletControllerEncryption();
|
||||
testLiteWalletControllerCreateEncryptsWithPassphrase();
|
||||
testLiteChainNameMigration();
|
||||
testLiteRefreshModelAppliesToWalletState();
|
||||
testLiteSendShowsRecipientFromOutgoing();
|
||||
|
||||
Reference in New Issue
Block a user