Skip to content

wallet.dat holds private keys in plain text: no wallet encryption exists #129

Description

@Bitflash-sh

wallet.dat stores private keys in plain text. There is no passphrase anywhere in this codebase — Bitcoin got wallet encryption in 0.4, and this tree came from 0.1.0, so it was never inherited.

Anyone who can read the file can spend the coins. No cracking, no brute force: the keys are simply there.

What is exposed

The records written today are key, pool, defaultkey, hdmaster, hdnext, hdschema, name, setting, tx. Two of them are the whole wallet:

  • key — the DER-encoded private key of every address the wallet has ever held.
  • hdmaster — the BIP-32 master key and chain code. This one is worse than the individual keys, because it reproduces every address the wallet will ever derive, including ones that do not exist yet.

Worth stating plainly, since we just published it: doc/derivation.md now documents the paths and address encoding. That was the right trade — the people who need it most are the ones recovering a wallet — but it does mean a leaked hdmaster is now trivially usable by anyone, with no reverse engineering.

Why this one matters here

The nodes in this project run headless under scheduled tasks, on machines that do other jobs, and at least one wallet file sits on a host reachable over SMB from the rest of the network. -dumpwallet also writes every key as readable text by design. A recovery phrase protects you from losing the file; nothing protects the file.

Shape of the fix

The Bitcoin 0.4 design is the obvious one and fits here: a random master key encrypted with AES-256-CBC under a key stretched from the passphrase, private keys stored as ckey instead of key, and an mkey record holding the encrypted master key, its salt, and the iteration count. The wallet is locked by default and unlocked into memory for as long as it is needed.

The parts that will bite are not in that summary.

Encrypt hdmaster too, or the rest is theatre. Encrypting the key records while the seed sits in the clear leaves an attacker one derivation away from everything. This is the trap Bitcoin 0.4 could not have had, because HD wallets came much later.

Rewrite the whole file when encrypting. Berkeley DB does not securely delete: overwriting a record leaves the old bytes in slack space, so a file that has been "encrypted" in place can still contain the plaintext keys. Bitcoin rewrites the entire wallet for exactly this reason, and skipping it produces a wallet that looks encrypted and is not.

Regenerate the key pool, and say what that means for backups. Every pooled key predating encryption exists unencrypted in any backup taken before. After encrypting, the old backup is simultaneously worthless (it lacks the new keys) and dangerous (it holds the old ones in the clear). The user has to be told to take a new backup and destroy the old one, in those words.

Decide what mining does with a locked wallet, and never let it fail silently. Mining draws from the key pool, and TopUpKeyPool writes new private keys — which a locked wallet cannot do. So a locked node mines until the pool empties and then cannot continue. Either it stops with a loud message or it refuses to start mining locked; what it must not do is keep running and quietly produce nothing, which is this project's most repeated failure and the reason for half the fixes in the last five releases.

Headless needs a way in that is not the command line. A passphrase in -walletpassphrase=... lands in the process list, in shell history, and in any log that echoes the command. Read it from stdin or prompt for it.

An old binary must refuse an encrypted wallet, loudly. A pre-encryption Bitflash opening a wallet full of ckey records it does not understand would find no keys and show an empty wallet. "Your balance is zero" to someone whose coins are fine is the worst possible failure here, and it is the same shape as the bugs in #40 and #95. This needs an explicit minimum-version marker that makes the old build stop and say why, rather than open successfully and lie.

-dumpwallet must require an unlocked wallet and keep its warning.

What it does not fix

Nothing here protects a wallet that is unlocked on a compromised machine, and nothing protects memory while the node runs. This closes the case of the file being read at rest — copied off a share, restored from a stolen backup, recovered from a decommissioned disk — which is the realistic case for this project.

Bar for merging

The same as the wallet work in #115 and #124: a real-coin round trip, not a self-test. Encrypt a wallet holding real coin, restart, unlock, spend, and restore it from the phrase — plus a check that the rewritten file contains no plaintext key material, since the whole point is what is left on disk.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions