DragonX/ObsidianDragon
1
1
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 10 Wiki Activity

fix(security): don't penalise a correct PIN after a passphrase change

Browse Source 
A passphrase change left the PIN vault sealing the old passphrase. The
next PIN unlock opened the vault, walletpassphrase answered RPC -14, and
the generic catch ran the lockout curve on a correct PIN, while Settings
kept showing the PIN as active.

- Passphrase change: when the active wallet's vault exists at completion
  (same vault path as at dispatch, passphrase actually changed), the
  vault is marked out of date for the session and kept on disk: PIN entry
  is no longer offered, a warning toast says so, and Settings shows
  "PIN out of date" with a tooltip on how to fix it. The Change
  Passphrase dialog notes beforehand that a PIN will go out of date.
- PIN unlock: RPC -14 after the vault opened is treated as "PIN out of
  date": passphrase entry, a message, no attempt counted, no lockout, and
  the vault is not deleted (-14 is only the daemon's answer; the vault may
  hold the only copy of a passphrase that opens an older backup). After a
  restart Settings shows the PIN as active until the first PIN attempt.
- A malformed vault (zero-filled by a removeVault() whose unlink failed,
  truncated, unknown version) is treated as missing: passphrase entry,
  "PIN file unreadable", no penalty. New SecureVault::isWellFormed().
- Race guards: Set/Change/Remove PIN are disabled while a passphrase
  change runs, and Change Passphrase while a PIN operation runs.
- Setting or removing the PIN, or switching wallets, clears the mark;
  setting a PIN offers PIN entry again. Change PIN keeps the mark (the
  vault still seals the same passphrase).
- Pure decisions in data/pin_vault_policy.h, unit-tested. New strings in
  all 8 languages.

No vault, wallet or settings format change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
DanS
2026-10-12 13:22:00 -05:00
co-authored by Claude Opus 5.5
parent 62a5577491
commit f6e5038043
18 changed files with 362 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/test_phase4.cpp
+117
View File
@@ -64,6 +64,7 @@
#include "ui/staleness_badge.h"
#include "ui/notifications.h"
#include "data/seed_migration_resume.h"
#include "data/pin_vault_policy.h"
#include "util/address_validation.h"
#include "util/seed_phrase.h"
#include "util/local_time.h"
@@ -2708,6 +2709,118 @@ void testSecureVaultScope()
EXPECT_TRUE(endsWith(vb.getVaultPath(), "vault.dat"));
}
// C87: a passphrase change marks the PIN vault out of date (D9(a)) and never removes it; RPC -14 on a
// PIN unlock whose vault opened is "PIN out of date", not a wrong PIN (D10).
void testPinVaultPolicy()
{
using dragonx::data::VaultAfterChange;
using dragonx::data::decideVaultAfterPassphraseChange;
using dragonx::data::isStaleVaultUnlockFailure;
const std::string pa = "/cfg/vault.dat", pb = "/cfg/vault-wallet-b-1234abcd.dat";
// The spec rows: failure, no vault, same passphrase, path mismatch -> Keep; otherwise MarkStale.
EXPECT_TRUE(decideVaultAfterPassphraseChange(false, true, false, pa, pa) == VaultAfterChange::Keep);
EXPECT_TRUE(decideVaultAfterPassphraseChange(true, false, false, pa, pa) == VaultAfterChange::Keep);
EXPECT_TRUE(decideVaultAfterPassphraseChange(true, true, true, pa, pa) == VaultAfterChange::Keep);
EXPECT_TRUE(decideVaultAfterPassphraseChange(true, true, false, pa, pb) == VaultAfterChange::Keep);
EXPECT_TRUE(decideVaultAfterPassphraseChange(true, true, false, "", "") == VaultAfterChange::Keep);
EXPECT_TRUE(decideVaultAfterPassphraseChange(true, true, false, pa, pa) == VaultAfterChange::MarkStale);
// Full table: MarkStale exactly when all four conditions hold; there is no "remove" outcome.
int stale = 0;
for (int m = 0; m < 16; ++m) {
const bool ok = m & 1, exists = m & 2, same = m & 4, match = m & 8;
const VaultAfterChange r = decideVaultAfterPassphraseChange(ok, exists, same, pa, match ? pa : pb);
const bool want = ok && exists && !same && match;
EXPECT_TRUE(r == (want ? VaultAfterChange::MarkStale : VaultAfterChange::Keep));
if (r == VaultAfterChange::MarkStale) ++stale;
}
EXPECT_EQ(stale, 1);
EXPECT_EQ(dragonx::data::kRpcWalletPassphraseIncorrect, -14);
EXPECT_TRUE(isStaleVaultUnlockFailure(true, -14));
EXPECT_FALSE(isStaleVaultUnlockFailure(true, -28)); // warmup: a transient refusal, not a stale vault
EXPECT_FALSE(isStaleVaultUnlockFailure(false, -14)); // the vault did not open: a wrong PIN
EXPECT_FALSE(isStaleVaultUnlockFailure(true, 0)); // transport / non-RPC failure
EXPECT_FALSE(isStaleVaultUnlockFailure(true, -1));
}
#ifndef _WIN32
// C87 D11: isWellFormed() is a structural check only (size + version byte). A zero-filled file (what a
// removeVault() whose unlink failed leaves) or a truncated one is not well formed, and retrieve() fails
// on it before any key derivation; the PIN unlock then treats it like a missing vault (no penalty).
// Under a temp $HOME: the vault lives in the config dir.
void testSecureVaultWellFormed()
{
using dragonx::util::SecureVault;
const char* oldHome = std::getenv("HOME");
const std::string savedHome = oldHome ? oldHome : "";
const char* oldXdg = std::getenv("XDG_CONFIG_HOME");
const bool hadXdg = oldXdg != nullptr;
const std::string savedXdg = hadXdg ? oldXdg : "";
const fs::path tmp = makeTempDir();
setenv("HOME", tmp.string().c_str(), 1);
setenv("XDG_CONFIG_HOME", (tmp / ".config").string().c_str(), 1);
{
std::error_code ec;
fs::create_directories(dragonx::util::Platform::getConfigDir(), ec);
SecureVault v("wallet-c87.dat");
const std::string path = v.getVaultPath();
EXPECT_TRUE(path.rfind(tmp.string(), 0) == 0); // never the real config dir
EXPECT_FALSE(v.hasVault());
EXPECT_FALSE(v.isWellFormed()); // missing
EXPECT_TRUE(v.store("1234", "correct horse battery"));
EXPECT_TRUE(v.hasVault());
EXPECT_TRUE(v.isWellFormed());
std::string pass;
EXPECT_TRUE(v.retrieve("1234", pass));
EXPECT_EQ(pass, std::string("correct horse battery"));
const std::string good = [&] {
std::ifstream in(path, std::ios::binary);
return std::string(std::istreambuf_iterator<char>(in), std::istreambuf_iterator<char>());
}();
EXPECT_TRUE(good.size() > 41);
auto rewrite = [&](const std::string& bytes) {
std::ofstream out(path, std::ios::binary | std::ios::trunc);
out.write(bytes.data(), static_cast<std::streamsize>(bytes.size()));
};
// Zero-filled, same size.
rewrite(std::string(good.size(), '\0'));
EXPECT_TRUE(v.hasVault());
EXPECT_FALSE(v.isWellFormed());
pass.clear();
EXPECT_FALSE(v.retrieve("1234", pass));
EXPECT_TRUE(pass.empty());
// Truncated (a valid version byte, too short to hold salt + nonce + tag).
rewrite(good.substr(0, 20));
EXPECT_FALSE(v.isWellFormed());
EXPECT_FALSE(v.retrieve("1234", pass));
// One byte short of the minimum (version + salt + nonce + tag + 1).
rewrite(good.substr(0, 1 + 16 + 24 + 16));
EXPECT_FALSE(v.isWellFormed());
// Full size, unknown version byte.
std::string badVersion = good;
badVersion[0] = '\x02';
rewrite(badVersion);
EXPECT_FALSE(v.isWellFormed());
EXPECT_FALSE(v.retrieve("1234", pass));
// Empty file.
rewrite(std::string());
EXPECT_FALSE(v.isWellFormed());
// The original bytes are well formed again (the check is structural only).
rewrite(good);
EXPECT_TRUE(v.isWellFormed());
EXPECT_TRUE(v.removeVault());
EXPECT_FALSE(v.hasVault());
}
if (!savedHome.empty()) setenv("HOME", savedHome.c_str(), 1); else unsetenv("HOME");
if (hadXdg) setenv("XDG_CONFIG_HOME", savedXdg.c_str(), 1); else unsetenv("XDG_CONFIG_HOME");
removeTree(tmp);
}
#endif
void testWalletSecurityController()
{
using dragonx::services::WalletSecurityController;
@@ -15951,6 +16064,10 @@ int main()
testBalanceDisplayLerp();
testOperationStatusPollParsing();
testSecureVaultScope();
testPinVaultPolicy();
#ifndef _WIN32
testSecureVaultWellFormed();
#endif
testWalletSecurityController();
testWalletSecurityWorkflow();
testWalletSecurityWorkflowExecutor();