From ea7c29e38b90ad337b66084b0b9449d10385da9b Mon Sep 17 00:00:00 2001 From: Mike Hearn Date: Wed, 22 Oct 2014 19:29:54 +0200 Subject: [PATCH] Key rotation: construct new HD chain based on the oldest possible key, a la upgrade, with a fresh random HD chain only being created if all random keys are rotating. --- .../main/java/org/bitcoinj/core/Wallet.java | 25 ++++++----- .../wallet/AllRandomKeysRotating.java | 7 +++ .../org/bitcoinj/wallet/KeyChainGroup.java | 17 +++++--- .../java/org/bitcoinj/core/WalletTest.java | 43 ++++++++++++++++--- 4 files changed, 71 insertions(+), 21 deletions(-) create mode 100644 core/src/main/java/org/bitcoinj/wallet/AllRandomKeysRotating.java diff --git a/core/src/main/java/org/bitcoinj/core/Wallet.java b/core/src/main/java/org/bitcoinj/core/Wallet.java index a4baf892f..5581cc69c 100644 --- a/core/src/main/java/org/bitcoinj/core/Wallet.java +++ b/core/src/main/java/org/bitcoinj/core/Wallet.java @@ -4273,15 +4273,12 @@ public class Wallet extends BaseTaggableObject implements Serializable, BlockCha } /** - *

When a key rotation time is set, and money controlled by keys created before the given timestamp T will be + *

When a key rotation time is set, any money controlled by keys created before the given timestamp T will be * automatically respent to any key that was created after T. This can be used to recover from a situation where * a set of keys is believed to be compromised. You can stop key rotation by calling this method again with zero - * as the argument, or by using {@link #setKeyRotationEnabled(boolean)}. Once set up, calling - * {@link #maybeDoMaintenance(org.spongycastle.crypto.params.KeyParameter, boolean)} will create and possibly - * send rotation transactions: but it won't be done automatically (because you might have to ask for the users - * password).

- * - *

Note that this method won't do anything unless you call {@link #setKeyRotationEnabled(boolean)} first.

+ * as the argument. Once set up, calling {@link #maybeDoMaintenance(org.spongycastle.crypto.params.KeyParameter, boolean)} + * will create and possibly send rotation transactions: but it won't be done automatically (because you might have + * to ask for the users password).

* *

The given time cannot be in the future.

*/ @@ -4311,7 +4308,7 @@ public class Wallet extends BaseTaggableObject implements Serializable, BlockCha * @param andSend if true, send the transactions via the tx broadcaster and return them, if false just return them. * @return A list of transactions that the wallet just made/will make for internal maintenance. Might be empty. */ - public ListenableFuture> maybeDoMaintenance(@Nullable KeyParameter aesKey, boolean andSend) { + public ListenableFuture> maybeDoMaintenance(@Nullable KeyParameter aesKey, boolean andSend) throws DeterministicUpgradeRequiresPassword { List txns; lock.lock(); try { @@ -4347,7 +4344,7 @@ public class Wallet extends BaseTaggableObject implements Serializable, BlockCha } // Checks to see if any coins are controlled by rotating keys and if so, spends them. - private List maybeRotateKeys(@Nullable KeyParameter aesKey) { + private List maybeRotateKeys(@Nullable KeyParameter aesKey) throws DeterministicUpgradeRequiresPassword { checkState(lock.isHeldByCurrentThread()); List results = Lists.newLinkedList(); // TODO: Handle chain replays here. @@ -4363,8 +4360,14 @@ public class Wallet extends BaseTaggableObject implements Serializable, BlockCha } } if (allChainsRotating) { - log.info("All HD chains are currently rotating, creating a new one"); - keychain.createAndActivateNewHDChain(); + log.info("All HD chains are currently rotating, attempting to create a new one from the next oldest non-rotating key material ..."); + try { + keychain.upgradeToDeterministic(keyRotationTimestamp, aesKey); + log.info(" ... upgraded to HD again, based on next best oldest key."); + } catch (AllRandomKeysRotating rotating) { + log.info(" ... no non-rotating random keys available, generating entirely new HD tree: backup required after this."); + keychain.createAndActivateNewHDChain(); + } } // Because transactions are size limited, we might not be able to re-key the entire wallet in one go. So diff --git a/core/src/main/java/org/bitcoinj/wallet/AllRandomKeysRotating.java b/core/src/main/java/org/bitcoinj/wallet/AllRandomKeysRotating.java new file mode 100644 index 000000000..b7a60aced --- /dev/null +++ b/core/src/main/java/org/bitcoinj/wallet/AllRandomKeysRotating.java @@ -0,0 +1,7 @@ +package org.bitcoinj.wallet; + +/** + * Indicates that an attempt was made to upgrade a random wallet to deterministic, but there were no non-rotating + * random keys to use as source material for the seed. Add a non-compromised key first! + */ +public class AllRandomKeysRotating extends RuntimeException {} diff --git a/core/src/main/java/org/bitcoinj/wallet/KeyChainGroup.java b/core/src/main/java/org/bitcoinj/wallet/KeyChainGroup.java index 1a6ff59e1..cf49060c7 100644 --- a/core/src/main/java/org/bitcoinj/wallet/KeyChainGroup.java +++ b/core/src/main/java/org/bitcoinj/wallet/KeyChainGroup.java @@ -630,12 +630,14 @@ public class KeyChainGroup implements KeyBag { * and you should provide the users encryption key. * @return the DeterministicKeyChain that was created by the upgrade. */ - public DeterministicKeyChain upgradeToDeterministic(long keyRotationTimeSecs, @Nullable KeyParameter aesKey) throws DeterministicUpgradeRequiresPassword { - checkState(chains.isEmpty()); + public DeterministicKeyChain upgradeToDeterministic(long keyRotationTimeSecs, @Nullable KeyParameter aesKey) throws DeterministicUpgradeRequiresPassword, AllRandomKeysRotating { checkState(basic.numKeys() > 0); checkArgument(keyRotationTimeSecs >= 0); - ECKey keyToUse = basic.findOldestKeyAfter(keyRotationTimeSecs); - checkArgument(keyToUse != null, "All keys are considered rotating, so we cannot upgrade deterministically."); + // Subtract one because the key rotation time might have been set to the creation time of the first known good + // key, in which case, that's the one we want to find. + ECKey keyToUse = basic.findOldestKeyAfter(keyRotationTimeSecs - 1); + if (keyToUse == null) + throw new AllRandomKeysRotating(); if (keyToUse.isEncrypted()) { if (aesKey == null) { @@ -658,7 +660,12 @@ public class KeyChainGroup implements KeyBag { throw new IllegalStateException("AES Key was provided but wallet is not encrypted."); } - log.info("Auto-upgrading pre-HD wallet using oldest non-rotating private key"); + if (chains.isEmpty()) { + log.info("Auto-upgrading pre-HD wallet to HD!"); + } else { + log.info("Wallet with existing HD chain is being re-upgraded due to change in key rotation time."); + } + log.info("Instantiating new HD chain using oldest non-rotating private key (address: {})", keyToUse.toAddress(params)); byte[] entropy = checkNotNull(keyToUse.getSecretBytes()); // Private keys should be at least 128 bits long. checkState(entropy.length >= DeterministicSeed.DEFAULT_SEED_ENTROPY_BITS / 8); diff --git a/core/src/test/java/org/bitcoinj/core/WalletTest.java b/core/src/test/java/org/bitcoinj/core/WalletTest.java index 4b2ebc366..f2f76f1d7 100644 --- a/core/src/test/java/org/bitcoinj/core/WalletTest.java +++ b/core/src/test/java/org/bitcoinj/core/WalletTest.java @@ -78,6 +78,7 @@ public class WalletTest extends TestWithWallet { @Override public void setUp() throws Exception { super.setUp(); + // TODO: Move these fields into the right tests so we don't create two wallets for every test case. encryptedWallet = new Wallet(params); myEncryptedAddress = encryptedWallet.freshReceiveKey().toAddress(params); encryptedWallet.encrypt(PASSWORD1); @@ -96,7 +97,6 @@ public class WalletTest extends TestWithWallet { createMarriedWallet(threshold, numKeys, true); } - private void createMarriedWallet(int threshold, int numKeys, boolean addSigners) throws BlockStoreException { wallet = new Wallet(params); blockStore = new MemoryBlockStore(params); @@ -2302,11 +2302,8 @@ public class WalletTest extends TestWithWallet { assertEquals(0, broadcaster.size()); assertFalse(wallet.isKeyRotating(key1)); - // We got compromised! We have an old style random-only wallet. So let's upgrade to HD: for that we need a fresh - // random key that's not rotating as the wallet won't create a new seed for us, it'll just refuse to upgrade. + // We got compromised! Utils.rollMockClock(1); - ECKey key3 = new ECKey(); - wallet.importKey(key3); wallet.setKeyRotationTime(compromiseTime); assertTrue(wallet.isKeyRotating(key1)); wallet.maybeDoMaintenance(null, true); @@ -2381,6 +2378,42 @@ public class WalletTest extends TestWithWallet { assertNotEquals(watchKey1, watchKey2); } + @SuppressWarnings("ConstantConditions") + @Test + public void keyRotationHD2() throws Exception { + // Check we handle the following scenario: a weak random key is created, then some good random keys are created + // but the weakness of the first isn't known yet. The wallet is upgraded to HD based on the weak key. Later, we + // find out about the weakness and set the rotation time to after the bad key's creation date. A new HD chain + // should be created based on the oldest known good key and the old chain + bad random key should rotate to it. + + // We fix the private keys just to make the test deterministic (last byte differs). + Utils.setMockClock(); + ECKey badKey = ECKey.fromPrivate(Utils.HEX.decode("00905b93f990267f4104f316261fc10f9f983551f9ef160854f40102eb71cffdbb")); + badKey.setCreationTimeSeconds(Utils.currentTimeSeconds()); + Utils.rollMockClock(86400); + ECKey goodKey = ECKey.fromPrivate(Utils.HEX.decode("00905b93f990267f4104f316261fc10f9f983551f9ef160854f40102eb71cffdcc")); + goodKey.setCreationTimeSeconds(Utils.currentTimeSeconds()); + + // Do an upgrade based on the bad key. + KeyChainGroup kcg = new KeyChainGroup(params); + kcg.importKeys(badKey, goodKey); + Utils.rollMockClock(86400); + wallet = new Wallet(params, kcg); // This avoids the automatic HD initialisation + wallet.upgradeToDeterministic(null); + DeterministicKey badWatchingKey = wallet.getWatchingKey(); + assertEquals(badKey.getCreationTimeSeconds(), badWatchingKey.getCreationTimeSeconds()); + sendMoneyToWallet(wallet, CENT, badWatchingKey.toAddress(params), AbstractBlockChain.NewBlockType.BEST_CHAIN); + + // Now we set the rotation time to the time we started making good keys. This should create a new HD chain. + wallet.setKeyRotationTime(goodKey.getCreationTimeSeconds()); + List txns = wallet.maybeDoMaintenance(null, false).get(); + assertEquals(1, txns.size()); + Address output = txns.get(0).getOutput(0).getAddressFromP2PKHScript(params); + ECKey usedKey = wallet.findKeyFromPubHash(output.getHash160()); + assertEquals(goodKey.getCreationTimeSeconds(), usedKey.getCreationTimeSeconds()); + assertEquals("mrM3TpCnav5YQuVA1xLercCGJH4DXujMtv", usedKey.toAddress(params).toString()); + } + @Test(expected = IllegalArgumentException.class) public void importOfHDKeyForbidden() throws Exception { wallet.importKey(wallet.freshReceiveKey());