wallet: erase the plaintext HD seed record when the wallet is encrypted
CWalletDB::WriteCryptedHDSeed wrote the "chdseed" record and left "hdseed" in place, unlike WriteCryptedKey which erases "key"/"wkey" after writing "ckey". No erase of "hdseed" existed anywhere in src/wallet/. CDB::Rewrite does not save us: EncryptWallet calls it with pszSkip defaulted, so it copies every surviving record verbatim into the new file. The result is that a wallet created unencrypted and later encrypted keeps its raw HD seed in cleartext on disk permanently, and reloads it into memory on every start. Add CWalletDB::EraseHDSeed and call it from CWallet::SetCryptedHDSeed after the encrypted record is written, through the same CWalletDB so it shares EncryptWallet's transaction. Erase returns true on DB_NOTFOUND, so a wallet that was never written in plaintext is unaffected. The erase is deliberately best-effort and only logs on failure. A hard failure here propagates into CCryptoKeyStore::EncryptKeys, which EncryptWallet turns into assert(false) with half the keys encrypted in memory; a warning is strictly better than that. Note this path is only reachable with -developerencryptwallet, which is experimental and off by default on this chain, so this is a latent fix rather than a live one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -2523,10 +2523,33 @@ bool CWallet::SetCryptedHDSeed(const uint256& seedFp, const std::vector<unsigned
|
|||||||
|
|
||||||
{
|
{
|
||||||
LOCK(cs_wallet);
|
LOCK(cs_wallet);
|
||||||
if (pwalletdbEncryption)
|
// Write the encrypted record, then drop the plaintext one. Both go
|
||||||
return pwalletdbEncryption->WriteCryptedHDSeed(seedFp, vchCryptedSecret);
|
// through the same CWalletDB (and therefore the same transaction when
|
||||||
else
|
// EncryptWallet supplied pwalletdbEncryption), because CDB::Rewrite at
|
||||||
return CWalletDB(strWalletFile).WriteCryptedHDSeed(seedFp, vchCryptedSecret);
|
// the end of EncryptWallet copies every surviving record into the fresh
|
||||||
|
// file -- a leftover plaintext "hdseed" would keep the unencrypted seed
|
||||||
|
// on disk for the life of the wallet.
|
||||||
|
//
|
||||||
|
// The erase is deliberately best-effort: a hard failure here propagates
|
||||||
|
// into CCryptoKeyStore::EncryptKeys, which CWallet::EncryptWallet turns
|
||||||
|
// into assert(false) with half the keys encrypted in memory. A logged
|
||||||
|
// warning is strictly better than that.
|
||||||
|
if (pwalletdbEncryption) {
|
||||||
|
if (!pwalletdbEncryption->WriteCryptedHDSeed(seedFp, vchCryptedSecret))
|
||||||
|
return false;
|
||||||
|
if (!pwalletdbEncryption->EraseHDSeed(seedFp))
|
||||||
|
LogPrintf("%s: WARNING: could not erase the plaintext hdseed record; "
|
||||||
|
"the unencrypted HD seed may remain in wallet.dat\n", __func__);
|
||||||
|
return true;
|
||||||
|
} else {
|
||||||
|
CWalletDB walletdb(strWalletFile);
|
||||||
|
if (!walletdb.WriteCryptedHDSeed(seedFp, vchCryptedSecret))
|
||||||
|
return false;
|
||||||
|
if (!walletdb.EraseHDSeed(seedFp))
|
||||||
|
LogPrintf("%s: WARNING: could not erase the plaintext hdseed record; "
|
||||||
|
"the unencrypted HD seed may remain in wallet.dat\n", __func__);
|
||||||
|
return true;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1335,6 +1335,16 @@ bool CWalletDB::WriteCryptedHDSeed(const uint256& seedFp, const std::vector<unsi
|
|||||||
return Write(std::make_pair(std::string("chdseed"), seedFp), vchCryptedSecret);
|
return Write(std::make_pair(std::string("chdseed"), seedFp), vchCryptedSecret);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
bool CWalletDB::EraseHDSeed(const uint256& seedFp)
|
||||||
|
{
|
||||||
|
nWalletDBUpdated++;
|
||||||
|
// CDB::Erase honours activeTxn, so when this runs inside EncryptWallet's
|
||||||
|
// transaction the erase commits or aborts atomically with the chdseed write.
|
||||||
|
// It also returns true for DB_NOTFOUND, so erasing a record that was never
|
||||||
|
// written (e.g. a wallet encrypted at creation time) is not a failure.
|
||||||
|
return Erase(std::make_pair(std::string("hdseed"), seedFp));
|
||||||
|
}
|
||||||
|
|
||||||
bool CWalletDB::WriteHDChain(const CHDChain& chain)
|
bool CWalletDB::WriteHDChain(const CHDChain& chain)
|
||||||
{
|
{
|
||||||
nWalletDBUpdated++;
|
nWalletDBUpdated++;
|
||||||
|
|||||||
@@ -222,6 +222,11 @@ public:
|
|||||||
|
|
||||||
bool WriteHDSeed(const HDSeed& seed);
|
bool WriteHDSeed(const HDSeed& seed);
|
||||||
bool WriteCryptedHDSeed(const uint256& seedFp, const std::vector<unsigned char>& vchCryptedSecret);
|
bool WriteCryptedHDSeed(const uint256& seedFp, const std::vector<unsigned char>& vchCryptedSecret);
|
||||||
|
//! Remove the PLAINTEXT hdseed record. Must be called once the seed has been
|
||||||
|
//! written in encrypted form: CDB::Rewrite (invoked at the end of
|
||||||
|
//! CWallet::EncryptWallet) copies whatever records still exist into the new
|
||||||
|
//! file, so a leftover "hdseed" leaves the unencrypted seed on disk forever.
|
||||||
|
bool EraseHDSeed(const uint256& seedFp);
|
||||||
//! write the hdchain model (external chain child index counter)
|
//! write the hdchain model (external chain child index counter)
|
||||||
bool WriteHDChain(const CHDChain& chain);
|
bool WriteHDChain(const CHDChain& chain);
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user