Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[backups] Automatic backups basics #5266

Closed
wants to merge 2 commits into from

Conversation

jonasschnelli
Copy link
Contributor

  • allow to do automatic backups when keypool gets refilled
  • by default backups are turned off
  • by default only encrypted wallets are allowed for backup (configurable)

This could be the basic for later adding backup possibilities for Bitcoin-Qt including visual elements for latest and amount of backups as also validity-check-display of backup files.

@sipa
Copy link
Member

sipa commented Nov 12, 2014

Would it be possible to use the dumpwallet RPC output instead, it's much easier to parse and likely to remain usable in more software longer. On the downside, it's not natively encrypted.

@jonasschnelli
Copy link
Contributor Author

@sipa hmm... it would be more open to other wallets. correct. On the other hand; how easy would it be for endusers to restore a backup? I think there is no easy way to import a RPC dumped wallet.
IMO, endusers should just have valid file(s) where they can restore. But a reasonable question is, how users could load a bitcoin-core wallet into other wallet-apps. But this might be out of scope for the backup-process because it affects also the non-backup wallet.dat.

@sipa
Copy link
Member

sipa commented Nov 12, 2014

Yes there is a way to import it, using importwallet.

@jonasschnelli
Copy link
Contributor Author

@sipa: ah. right. Somebody should update https://en.bitcoin.it/wiki/Original_Bitcoin_client/API_calls_list :)
But the enduser then could not just restore the wallet.dat file... or we could detect if the wallet.dat is RPC dumped file and import it straight.
I would prefer to have a restore possibility on file level.

@sipa
Copy link
Member

sipa commented Nov 12, 2014

@jonasschnelli the GUI has no way to restore a backup at all - whether it's done using backupwallet or dumpwallet. That definitely needs fixing, but I would really advise against using the BDB-based format for anything but legacy.

@jonasschnelli
Copy link
Contributor Author

@sipa i just thought through the whole thing again. Your right. Restore per file won't work anyway without rescan everything.
I'll change it to use RPC walletdumped format.
Enduser with limited IT understanding should import it through the Qt client (where i will focus on as soon as the basic things are sorted out).

@jonasschnelli
Copy link
Contributor Author

@sipa i only see a huge drawback when using the walletdumps-txt: encryption. Unencrypted wallet dumps is IMO very risky. Of course we could encrypt the dumps before the go to the filesystem. But this lead to the question what passphrase/key we use for encrypting the backups dumps. Maybe adding a "backup-key" to the wallet would be a overhead?
Any ideas how we could solve that in a nice way?

@laanwj
Copy link
Member

laanwj commented Nov 13, 2014

Good point about dumpwallet format not being encrypted. We should definitely support that.

For example Schildback's Bitcoin Wallet uses a format similar to our dumpwallet format, one line per key, but encrypts the file before writing. This can be decrypted from the command line using openssl enc -d -aes-256-cbc -a -in (see https://github.com/schildbach/bitcoin-wallet/blob/master/wallet/README.recover ), or of course imported back into the application.

As for the key, would be multiple options there:

  • Encrypt the file with the same key as used for the wallet (easy for the user, but not very secure)
  • Ask the user for a password every time backing up, and add a "password" parameter to the dumpwallet RPC call (easiest to implement, most transparent to the user)
  • As you say, store a "backup key" in the settings

@laanwj laanwj added this to the 0.11.0 milestone Nov 13, 2014
@jonasschnelli jonasschnelli force-pushed the walletbackup branch 2 times, most recently from 776a9b8 to 2fb996d Compare November 13, 2014 15:20
@jonasschnelli
Copy link
Contributor Author

I think we should encrypt the dumps with a key derived from the same password the wallet is encrypted.

Additional to that, for experience users, we could allow to set a base64 encoded EC PubKey in the bitcoinconf. The dumps then could be encrypted aes-256-cbc creating a random key with EVP_Seal. The encrypted key length and data will be stored in the dump.
With that, experience users could do encrypted backups with keeping their backup key secret.

@gmaxwell
Copy link
Contributor

Please don't introduce yet another encryption format in the system if it can all be avoided, analyizing and maintaining two is additional work I'd rather avoid. The simpler thing is to just backup the existing files, then we're sure to get all the data, sure that it's already encrypted and not creating additional exposure, etc.

@jonasschnelli
Copy link
Contributor Author

@gmaxwell agreed. Than i keep the pull request as it is. For now we backup just the wallet.dat. Dumping is IMO not a backup because it lacks of metadata (dump != backup). A backup should be somehow a 1:1 copy.
If a user enables -backupsdumps AND -backupsallowunencrypted he can do unencrypted dumps as backups.

Before working on encrypted dumps as backup possibilities i would prefer to implement the/a backup mechanism in the GUI.

std::sort(vKeyBirth.begin(), vKeyBirth.end());

// produce output
stream << strprintf("# Wallet dump created by Bitcoin %s (%s)\n", CLIENT_BUILD, CLIENT_DATE);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By "Bitcoin Core" :)?

@jonasschnelli
Copy link
Contributor Author

@Diapolo okay, did overhaul the code, added some comments, etc.
You might check it again.

strUsage += "\n" + _("Backup options:") + "\n";
strUsage += " -backups " + strprintf(_("Enable auto creation of UNENCRYPTED backups (dumps) (default: %u)"), DEFAULT_BACKUPS_ENABLED) + "\n";
strUsage += " -backupsallowunencrypted " + strprintf(_("Allow backups of unencrypted wallets (default: %u)"), DEFAULT_ALLOW_UNENCRYPTED_BACKUPS) + "\n";
strUsage += " -backupspath=<dir> " + strprintf(_("Specify absolut path to backups directory (default: %s)"), strprintf("<datadir>/%s", DEFAULT_BACKUPS_DIR)) + "\n";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"absolute"

@jonasschnelli
Copy link
Contributor Author

Any NACKs ACKs?
I would like to extend this to the GUI.

@laanwj
Copy link
Member

laanwj commented Dec 17, 2014

@jonasschnelli Concept ACK, haven't looked at the code. I promise I will, but right now I cannot keep up with the number of pulls anymore.

@laanwj laanwj added the GUI label Jan 8, 2015
@sipa sipa removed the GUI label Jan 9, 2015
@jonasschnelli
Copy link
Contributor Author

rebased

@jonasschnelli jonasschnelli force-pushed the walletbackup branch 2 times, most recently from 1663543 to c1b1504 Compare March 5, 2015 08:23
- allow to do automatic backups (wallet.dat copy or dumps) when keypool gets refilled
- by default backups are turned off
- by default only encrypted wallets are allowed for backup
- user can choose to backup dumps instead of the wallet.dat file
@jonasschnelli
Copy link
Contributor Author

rebased.

@laanwj laanwj added the Wallet label Mar 27, 2015
@jonasschnelli
Copy link
Contributor Author

I'm no longer convinced that this PRs solution is going into the right direction. As example: what means a encrypted wallet? Why are metadata like labels and comments (which could harm privacy) not encrypted in a encrypted wallet?
How would this perform after there is support for Bip32?

@gmaxwell
Copy link
Contributor

@jonasschnelli Things other than private keys are not encrypted because otherwise the user is forced to enter their key at every startup or use, which is inconvenient and would punish users for using encryption and would leave their key more vulnerable to observation by malware or people around them. The entry of the key also becomes an important consent point: a user of an encrypted wallet can never accidentally send money no matter how stupidly they click or how far they explore.

Encryption of a wallet is not usually adequate for privacy on your local computer in any case: your logs, browser cache, etc all leak much of the same data... When we talk about backups the situation is somewhat different, and I can agree it may make sense to have all the data encrypted there. If I were to do it again I would have an master key encrypted with the hardened user passphrase used to derive separate keys for encrypting the whole wallet and the private keys. At the first use, I'd cache the whole wallet subkey (but not the private key one) in a separate file. This way backups would preserve privacy, so long as they didn't backup the cache.

BIP32 is not at all a replacement for backups; at most it protects key material. It does nothing to preserve metadata which can be absolutely critical (keeping in mind that in much of the developed world if your taxes are audited its up to you to prove you were paying what you owed). I somewhat regret inventing the public derivation thats most commonly used now-- it's overused, and in ways that harm security (often by people who do not understand the implications).

@laanwj laanwj removed this from the 0.11.0 milestone May 18, 2015
@jgarzik
Copy link
Contributor

jgarzik commented Jul 23, 2015

Closing for this older PR. It sounds like there is general agreement that something like this is a good direction, but it's doesn't seem to be coalescing into a specific solution that is liked.

My own criticism: I don't think this low level functionality is needed at all for non-GUI daemon. It can be implemented with a simple, external cron script that applies well to the local site.

Suggestions:

  • Add such a script to contrib/
  • Directly implement a GUI-specific, user-friendly backup method in the GUI. Don't try to match bitcoind and GUI backup behaviors, as the two sets of users can be quite different. Do what makes sense for the GUI.

@jgarzik jgarzik closed this Jul 23, 2015
@jonasschnelli
Copy link
Contributor Author

Agree with @jgarzik in general.
What made me stop woking on this: as long as an "encrypted wallet" contains unencrypted sensitive data (like transactions [unencrypted], labels [unencrypted] and wtx comments [unencrypted]) it is to risky to auto-backup a wallet.
Users might start to think they can store the backup in a insecure space (like dropbox, etc.) because they are assuming the backup is encrypted (which is a false assumption, only the private keys are encrypted).

@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants