From 24e41f01c6fade83cccd923affc8216157618767 Mon Sep 17 00:00:00 2001 From: Mike Hearn Date: Tue, 22 Apr 2014 19:25:11 +0200 Subject: [PATCH] Refactorings. Make a convenience ECKey.decrypt(KeyParameter) that doesn't require the keycrypter to be manually specified, as often (always?) the key knows it already. Introduce a KeyBag interface that just contains findKeyBy* methods, then make Wallet implement it and change Transaction.signInputs to use it. Take out the encrypted-key specific stuff here: Transaction now requires unencrypted keys. Create a DecryptingKeyBag class that just forwards calls to Wallet and decrypts the returned keys. This decouples the signing code from Wallet a bit. Should be all API compatible. --- .../java/com/google/bitcoin/core/ECKey.java | 17 +++++- .../com/google/bitcoin/core/Transaction.java | 38 +++++++++---- .../bitcoin/core/TransactionOutPoint.java | 8 ++- .../java/com/google/bitcoin/core/Wallet.java | 2 +- .../bitcoin/crypto/DeterministicKey.java | 5 ++ .../google/bitcoin/wallet/BasicKeyChain.java | 4 +- .../bitcoin/wallet/DecryptingKeyBag.java | 55 +++++++++++++++++++ .../bitcoin/wallet/DeterministicKeyChain.java | 2 +- .../com/google/bitcoin/wallet/KeyBag.java | 40 ++++++++++++++ .../com/google/bitcoin/wallet/KeyChain.java | 16 +----- .../com/google/bitcoin/core/ECKeyTest.java | 8 +-- .../crypto/ChildKeyDerivationTest.java | 4 +- 12 files changed, 155 insertions(+), 44 deletions(-) create mode 100644 core/src/main/java/com/google/bitcoin/wallet/DecryptingKeyBag.java create mode 100644 core/src/main/java/com/google/bitcoin/wallet/KeyBag.java diff --git a/core/src/main/java/com/google/bitcoin/core/ECKey.java b/core/src/main/java/com/google/bitcoin/core/ECKey.java index 189cc6f74..b54d2b7d1 100644 --- a/core/src/main/java/com/google/bitcoin/core/ECKey.java +++ b/core/src/main/java/com/google/bitcoin/core/ECKey.java @@ -567,7 +567,7 @@ public class ECKey implements EncryptableItem, Serializable { if (crypter != null) { if (aesKey == null) throw new KeyIsEncryptedException(); - return decrypt(crypter, aesKey).sign(input); + return decrypt(aesKey).sign(input); } else { // No decryption of private key required. if (priv == null) @@ -966,7 +966,6 @@ public class ECKey implements EncryptableItem, Serializable { * * @param keyCrypter The keyCrypter that specifies exactly how the decrypted bytes are created. * @param aesKey The KeyParameter with the AES encryption key (usually constructed with keyCrypter#deriveKey and cached). - * @return unencryptedKey */ public ECKey decrypt(KeyCrypter keyCrypter, KeyParameter aesKey) throws KeyCrypterException { checkNotNull(keyCrypter); @@ -984,6 +983,20 @@ public class ECKey implements EncryptableItem, Serializable { return key; } + /** + * Create a decrypted private key with AES key. Note that if the AES key is wrong, this + * has some chance of throwing KeyCrypterException due to the corrupted padding that will result, but it can also + * just yield a garbage key. + * + * @param aesKey The KeyParameter with the AES encryption key (usually constructed with keyCrypter#deriveKey and cached). + */ + public ECKey decrypt(KeyParameter aesKey) throws KeyCrypterException { + final KeyCrypter crypter = getKeyCrypter(); + if (crypter == null) + throw new KeyCrypterException("No key crypter available"); + return decrypt(crypter, aesKey); + } + /** *

Check that it is possible to decrypt the key with the keyCrypter and that the original key is returned.

* diff --git a/core/src/main/java/com/google/bitcoin/core/Transaction.java b/core/src/main/java/com/google/bitcoin/core/Transaction.java index 5cd934454..5d9092c0f 100644 --- a/core/src/main/java/com/google/bitcoin/core/Transaction.java +++ b/core/src/main/java/com/google/bitcoin/core/Transaction.java @@ -22,7 +22,8 @@ import com.google.bitcoin.crypto.TransactionSignature; import com.google.bitcoin.script.Script; import com.google.bitcoin.script.ScriptBuilder; import com.google.bitcoin.script.ScriptOpCodes; -import com.google.common.collect.ImmutableList; +import com.google.bitcoin.wallet.DecryptingKeyBag; +import com.google.bitcoin.wallet.KeyBag; import com.google.common.collect.ImmutableMap; import com.google.common.primitives.Ints; import com.google.common.primitives.Longs; @@ -849,7 +850,22 @@ public class Transaction extends ChildMessage implements Serializable { * @param wallet A wallet is required to fetch the keys needed for signing. * @param aesKey The AES key to use to decrypt the key before signing. Null if no decryption is required. */ - public synchronized void signInputs(SigHash hashType, Wallet wallet, @Nullable KeyParameter aesKey) throws ScriptException { + public void signInputs(SigHash hashType, Wallet wallet, @Nullable KeyParameter aesKey) throws ScriptException { + if (aesKey == null) { + signInputs(hashType, false, wallet); + } else { + signInputs(hashType, false, new DecryptingKeyBag(wallet, aesKey)); + } + } + + /** + * Signs as many inputs as possible using keys from the given key bag, which are expected to be usable for + * signing, i.e. not encrypted and not missing the private key part. + * + * @param hashType This should always be set to SigHash.ALL currently. Other types are unused. + * @param keyBag a provider of keys that are usable as-is for signing. + */ + public synchronized void signInputs(SigHash hashType, boolean anyoneCanPay, KeyBag keyBag) throws ScriptException { checkState(inputs.size() > 0); checkState(outputs.size() > 0); @@ -885,16 +901,15 @@ public class Transaction extends ChildMessage implements Serializable { if (input.getScriptBytes().length != 0) log.warn("Re-signing an already signed transaction! Be sure this is what you want."); // Find the signing key we'll need to use. - ECKey key = input.getOutpoint().getConnectedKey(wallet); + ECKey key = input.getOutpoint().getConnectedKey(keyBag); // This assert should never fire. If it does, it means the wallet is inconsistent. checkNotNull(key, "Transaction exists in wallet that we cannot redeem: %s", input.getOutpoint().getHash()); // Keep the key around for the script creation step below. signingKeys[i] = key; // The anyoneCanPay feature isn't used at the moment. - boolean anyoneCanPay = false; byte[] connectedPubKeyScript = input.getOutpoint().getConnectedPubKeyScript(); try { - signatures[i] = calculateSignature(i, key, aesKey, connectedPubKeyScript, hashType, anyoneCanPay); + signatures[i] = calculateSignature(i, key, connectedPubKeyScript, hashType, anyoneCanPay); } catch (ECKey.KeyIsEncryptedException e) { throw e; } catch (ECKey.MissingPrivateKeyException e) { @@ -935,22 +950,21 @@ public class Transaction extends ChildMessage implements Serializable { /** * Calculates a signature that is valid for being inserted into the input at the given position. This is simply * a wrapper around calling {@link Transaction#hashForSignature(int, byte[], com.google.bitcoin.core.Transaction.SigHash, boolean)} - * followed by {@link ECKey#sign(Sha256Hash, org.spongycastle.crypto.params.KeyParameter)} and then returning - * a new {@link TransactionSignature}. + * followed by {@link ECKey#sign(Sha256Hash)} and then returning a new {@link TransactionSignature}. The key + * must be usable for signing as-is: if the key is encrypted it must be decrypted first external to this method. * * @param inputIndex Which input to calculate the signature for, as an index. * @param key The private key used to calculate the signature. - * @param aesKey If not null, this will be used to decrypt the key. * @param connectedPubKeyScript Byte-exact contents of the scriptPubKey that is being satisified. * @param hashType Signing mode, see the enum for documentation. * @param anyoneCanPay Signing mode, see the SigHash enum for documentation. * @return A newly calculated signature object that wraps the r, s and sighash components. */ - public synchronized TransactionSignature calculateSignature(int inputIndex, ECKey key, @Nullable KeyParameter aesKey, - byte[] connectedPubKeyScript, - SigHash hashType, boolean anyoneCanPay) { + public synchronized TransactionSignature calculateSignature(int inputIndex, ECKey key, + byte[] connectedPubKeyScript, + SigHash hashType, boolean anyoneCanPay) { Sha256Hash hash = hashForSignature(inputIndex, connectedPubKeyScript, hashType, anyoneCanPay); - return new TransactionSignature(key.sign(hash, aesKey), hashType, anyoneCanPay); + return new TransactionSignature(key.sign(hash), hashType, anyoneCanPay); } /** diff --git a/core/src/main/java/com/google/bitcoin/core/TransactionOutPoint.java b/core/src/main/java/com/google/bitcoin/core/TransactionOutPoint.java index c9d781344..dacc84391 100644 --- a/core/src/main/java/com/google/bitcoin/core/TransactionOutPoint.java +++ b/core/src/main/java/com/google/bitcoin/core/TransactionOutPoint.java @@ -17,6 +17,7 @@ package com.google.bitcoin.core; import com.google.bitcoin.script.Script; +import com.google.bitcoin.wallet.KeyBag; import javax.annotation.Nullable; import java.io.IOException; @@ -134,19 +135,20 @@ public class TransactionOutPoint extends ChildMessage implements Serializable { /** * Returns the ECKey identified in the connected output, for either pay-to-address scripts or pay-to-key scripts. * If the script forms cannot be understood, throws ScriptException. + * * @return an ECKey or null if the connected key cannot be found in the wallet. */ @Nullable - public ECKey getConnectedKey(Wallet wallet) throws ScriptException { + public ECKey getConnectedKey(KeyBag keyBag) throws ScriptException { TransactionOutput connectedOutput = getConnectedOutput(); checkNotNull(connectedOutput, "Input is not connected so cannot retrieve key"); Script connectedScript = connectedOutput.getScriptPubKey(); if (connectedScript.isSentToAddress()) { byte[] addressBytes = connectedScript.getPubKeyHash(); - return wallet.findKeyFromPubHash(addressBytes); + return keyBag.findKeyFromPubHash(addressBytes); } else if (connectedScript.isSentToRawPubKey()) { byte[] pubkeyBytes = connectedScript.getPubKey(); - return wallet.findKeyFromPubKey(pubkeyBytes); + return keyBag.findKeyFromPubKey(pubkeyBytes); } else { throw new ScriptException("Could not understand form of connected output script: " + connectedScript); } diff --git a/core/src/main/java/com/google/bitcoin/core/Wallet.java b/core/src/main/java/com/google/bitcoin/core/Wallet.java index bb1ebf581..cb88aa892 100644 --- a/core/src/main/java/com/google/bitcoin/core/Wallet.java +++ b/core/src/main/java/com/google/bitcoin/core/Wallet.java @@ -99,7 +99,7 @@ import static com.google.common.base.Preconditions.*; * {@link Wallet#autosaveToFile(java.io.File, long, java.util.concurrent.TimeUnit, com.google.bitcoin.wallet.WalletFiles.Listener)} * for more information about this.

*/ -public class Wallet extends BaseTaggableObject implements Serializable, BlockChainListener, PeerFilterProvider { +public class Wallet extends BaseTaggableObject implements Serializable, BlockChainListener, PeerFilterProvider, KeyBag { private static final Logger log = LoggerFactory.getLogger(Wallet.class); private static final long serialVersionUID = 2L; private static final int MINIMUM_BLOOM_DATA_LENGTH = 8; diff --git a/core/src/main/java/com/google/bitcoin/crypto/DeterministicKey.java b/core/src/main/java/com/google/bitcoin/crypto/DeterministicKey.java index 735100728..f59b666b9 100644 --- a/core/src/main/java/com/google/bitcoin/crypto/DeterministicKey.java +++ b/core/src/main/java/com/google/bitcoin/crypto/DeterministicKey.java @@ -240,6 +240,11 @@ public class DeterministicKey extends ECKey { return key; } + @Override + public DeterministicKey decrypt(KeyParameter aesKey) throws KeyCrypterException { + return (DeterministicKey) super.decrypt(aesKey); + } + // For when a key is encrypted, either decrypt our encrypted private key bytes, or work up the tree asking parents // to decrypt and re-derive. private BigInteger findOrDeriveEncryptedPrivateKey(KeyCrypter keyCrypter, KeyParameter aesKey) { diff --git a/core/src/main/java/com/google/bitcoin/wallet/BasicKeyChain.java b/core/src/main/java/com/google/bitcoin/wallet/BasicKeyChain.java index 77bc32e48..b413ecee3 100644 --- a/core/src/main/java/com/google/bitcoin/wallet/BasicKeyChain.java +++ b/core/src/main/java/com/google/bitcoin/wallet/BasicKeyChain.java @@ -424,7 +424,7 @@ public class BasicKeyChain implements EncryptableKeyChain { throw new KeyCrypterException("Password/key was incorrect."); BasicKeyChain decrypted = new BasicKeyChain(); for (ECKey key : hashToKeys.values()) { - decrypted.importKeyLocked(key.decrypt(keyCrypter, aesKey)); + decrypted.importKeyLocked(key.decrypt(aesKey)); } return decrypted; } finally { @@ -467,7 +467,7 @@ public class BasicKeyChain implements EncryptableKeyChain { checkState(first != null, "No encrypted keys in the wallet"); try { - ECKey rebornKey = first.decrypt(keyCrypter, aesKey); + ECKey rebornKey = first.decrypt(aesKey); return Arrays.equals(first.getPubKey(), rebornKey.getPubKey()); } catch (KeyCrypterException e) { // The AES key supplied is incorrect. diff --git a/core/src/main/java/com/google/bitcoin/wallet/DecryptingKeyBag.java b/core/src/main/java/com/google/bitcoin/wallet/DecryptingKeyBag.java new file mode 100644 index 000000000..3ddd5263b --- /dev/null +++ b/core/src/main/java/com/google/bitcoin/wallet/DecryptingKeyBag.java @@ -0,0 +1,55 @@ +/** + * Copyright 2014 The bitcoinj authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.bitcoin.wallet; + +import com.google.bitcoin.core.ECKey; +import org.spongycastle.crypto.params.KeyParameter; + +import javax.annotation.Nullable; + +import static com.google.common.base.Preconditions.checkNotNull; + +/** + * A DecryptingKeyBag filters a pre-existing key bag, decrypting keys as they are requested using the provided + * AES key. + */ +public class DecryptingKeyBag implements KeyBag { + protected final KeyBag target; + protected final KeyParameter aesKey; + + public DecryptingKeyBag(KeyBag target, KeyParameter aesKey) { + this.target = checkNotNull(target); + this.aesKey = checkNotNull(aesKey); + } + + @Nullable + private ECKey maybeDecrypt(ECKey key) { + return key == null ? null : key.decrypt(aesKey); + } + + @Nullable + @Override + public ECKey findKeyFromPubHash(byte[] pubkeyHash) { + return maybeDecrypt(target.findKeyFromPubHash(pubkeyHash)); + } + + @Nullable + @Override + public ECKey findKeyFromPubKey(byte[] pubkey) { + return maybeDecrypt(target.findKeyFromPubKey(pubkey)); + } +} diff --git a/core/src/main/java/com/google/bitcoin/wallet/DeterministicKeyChain.java b/core/src/main/java/com/google/bitcoin/wallet/DeterministicKeyChain.java index 70e693502..b39ddd7b0 100644 --- a/core/src/main/java/com/google/bitcoin/wallet/DeterministicKeyChain.java +++ b/core/src/main/java/com/google/bitcoin/wallet/DeterministicKeyChain.java @@ -583,7 +583,7 @@ public class DeterministicKeyChain implements EncryptableKeyChain { checkNotNull(aesKey); checkState(getKeyCrypter() != null, "Key chain not encrypted"); try { - return rootKey.decrypt(getKeyCrypter(), aesKey).getPubKeyPoint().equals(rootKey.getPubKeyPoint()); + return rootKey.decrypt(aesKey).getPubKeyPoint().equals(rootKey.getPubKeyPoint()); } catch (KeyCrypterException e) { return false; } diff --git a/core/src/main/java/com/google/bitcoin/wallet/KeyBag.java b/core/src/main/java/com/google/bitcoin/wallet/KeyBag.java new file mode 100644 index 000000000..215aebbe9 --- /dev/null +++ b/core/src/main/java/com/google/bitcoin/wallet/KeyBag.java @@ -0,0 +1,40 @@ +/** + * Copyright 2014 The bitcoinj authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.bitcoin.wallet; + +import com.google.bitcoin.core.ECKey; + +/** + * A KeyBag is simply an object that can map public keys and their 160-bit hashes to ECKey objects. All + * {@link com.google.bitcoin.wallet.KeyChain}s are key bags. + */ +public interface KeyBag { + /** + * Locates a keypair from the keychain given the hash of the public key. This is needed when finding out which + * key we need to use to redeem a transaction output. + * + * @return ECKey object or null if no such key was found. + */ + public ECKey findKeyFromPubHash(byte[] pubkeyHash); + + /** + * Locates a keypair from the keychain given the raw public key bytes. + * + * @return ECKey or null if no such key was found. + */ + public ECKey findKeyFromPubKey(byte[] pubkey); +} diff --git a/core/src/main/java/com/google/bitcoin/wallet/KeyChain.java b/core/src/main/java/com/google/bitcoin/wallet/KeyChain.java index 6d2ecf6ea..e9d2d452c 100644 --- a/core/src/main/java/com/google/bitcoin/wallet/KeyChain.java +++ b/core/src/main/java/com/google/bitcoin/wallet/KeyChain.java @@ -34,21 +34,7 @@ import java.util.concurrent.Executor; * restrictions is to support key chains that may be handled by external hardware or software, or which are derived * deterministically from a seed (and thus the notion of importing a key is meaningless).

*/ -public interface KeyChain { - /** - * Locates a keypair from the keychain given the hash of the public key. This is needed when finding out which - * key we need to use to redeem a transaction output. - * - * @return ECKey object or null if no such key was found. - */ - public ECKey findKeyFromPubHash(byte[] pubkeyHash); - - /** - * Locates a keypair from the keychain given the raw public key bytes. - * @return ECKey or null if no such key was found. - */ - public ECKey findKeyFromPubKey(byte[] pubkey); - +public interface KeyChain extends KeyBag { /** Returns true if the given key is in the chain. */ public boolean hasKey(ECKey key); diff --git a/core/src/test/java/com/google/bitcoin/core/ECKeyTest.java b/core/src/test/java/com/google/bitcoin/core/ECKeyTest.java index 6f3a952c5..ad28708c8 100644 --- a/core/src/test/java/com/google/bitcoin/core/ECKeyTest.java +++ b/core/src/test/java/com/google/bitcoin/core/ECKeyTest.java @@ -32,16 +32,12 @@ import com.google.common.util.concurrent.MoreExecutors; import com.google.protobuf.ByteString; import org.bitcoinj.wallet.Protos; import org.bitcoinj.wallet.Protos.ScryptParameters; -import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.spongycastle.crypto.params.ECDomainParameters; import org.spongycastle.crypto.params.KeyParameter; import org.spongycastle.util.encoders.DecoderException; -import org.spongycastle.math.ec.ECCurve; -import org.spongycastle.math.ec.ECPoint; import org.spongycastle.util.encoders.Hex; import java.io.InputStream; @@ -273,7 +269,7 @@ public class ECKeyTest { assertEquals(time, encryptedKey.getCreationTimeSeconds()); assertTrue(encryptedKey.isEncrypted()); assertNull(encryptedKey.getSecretBytes()); - key = encryptedKey.decrypt(keyCrypter, keyCrypter.deriveKey(PASSWORD1)); + key = encryptedKey.decrypt(keyCrypter.deriveKey(PASSWORD1)); assertTrue(!key.isEncrypted()); assertArrayEquals(originalPrivateKeyBytes, key.getPrivKeyBytes()); } @@ -287,7 +283,7 @@ public class ECKeyTest { ECKey encryptedKey = ECKey.fromEncrypted(encryptedPrivateKey, keyCrypter, unencryptedKey.getPubKey()); assertTrue(encryptedKey.isEncrypted()); assertNull(encryptedKey.getSecretBytes()); - ECKey rebornUnencryptedKey = encryptedKey.decrypt(keyCrypter, keyCrypter.deriveKey(PASSWORD1)); + ECKey rebornUnencryptedKey = encryptedKey.decrypt(keyCrypter.deriveKey(PASSWORD1)); assertTrue(!rebornUnencryptedKey.isEncrypted()); assertArrayEquals(originalPrivateKeyBytes, rebornUnencryptedKey.getPrivKeyBytes()); } diff --git a/core/src/test/java/com/google/bitcoin/crypto/ChildKeyDerivationTest.java b/core/src/test/java/com/google/bitcoin/crypto/ChildKeyDerivationTest.java index b2c9ce97e..97d2f8b64 100644 --- a/core/src/test/java/com/google/bitcoin/crypto/ChildKeyDerivationTest.java +++ b/core/src/test/java/com/google/bitcoin/crypto/ChildKeyDerivationTest.java @@ -133,13 +133,13 @@ public class ChildKeyDerivationTest { DeterministicKey key1 = HDKeyDerivation.createMasterPrivateKey("it was all a hoax".getBytes()); DeterministicKey encryptedKey1 = key1.encrypt(scrypter, aesKey, null); - DeterministicKey decryptedKey1 = encryptedKey1.decrypt(scrypter, aesKey); + DeterministicKey decryptedKey1 = encryptedKey1.decrypt(aesKey); assertEquals(key1, decryptedKey1); DeterministicKey key2 = HDKeyDerivation.deriveChildKey(key1, ChildNumber.ZERO); DeterministicKey derivedKey2 = HDKeyDerivation.deriveChildKey(encryptedKey1, ChildNumber.ZERO); assertTrue(derivedKey2.isEncrypted()); // parent is encrypted. - DeterministicKey decryptedKey2 = derivedKey2.decrypt(scrypter, aesKey); + DeterministicKey decryptedKey2 = derivedKey2.decrypt(aesKey); assertFalse(decryptedKey2.isEncrypted()); assertEquals(key2, decryptedKey2);