From b52a7137c326f829e4ae3c00967734d80984b7fe Mon Sep 17 00:00:00 2001 From: Craig Raw Date: Tue, 19 May 2020 14:02:53 +0200 Subject: [PATCH] securely handle mnemonic seed in memory --- drongo | 2 +- .../com/sparrowwallet/sparrow/io/Electrum.java | 2 +- .../com/sparrowwallet/sparrow/io/Storage.java | 17 +++++++++-------- .../sparrow/io/ColdcardMultisigTest.java | 12 ++++++------ .../sparrow/io/ColdcardSinglesigTest.java | 4 ++-- .../sparrowwallet/sparrow/io/ElectrumTest.java | 6 +++--- 6 files changed, 22 insertions(+), 21 deletions(-) diff --git a/drongo b/drongo index 9f5f5689..d2bd335e 160000 --- a/drongo +++ b/drongo @@ -1 +1 @@ -Subproject commit 9f5f5689bbbe85dad51be84d4a5a2e5d31562564 +Subproject commit d2bd335e76a6a18634d033b49bf2d36c2494d9cf diff --git a/src/main/java/com/sparrowwallet/sparrow/io/Electrum.java b/src/main/java/com/sparrowwallet/sparrow/io/Electrum.java index 891890c5..51a38988 100644 --- a/src/main/java/com/sparrowwallet/sparrow/io/Electrum.java +++ b/src/main/java/com/sparrowwallet/sparrow/io/Electrum.java @@ -213,7 +213,7 @@ public class Electrum implements KeystoreFileImport, WalletImport, WalletExport ek.xprv = keystore.getExtendedPrivateKey().toString(xprvHeader); ek.pw_hash_version = 1; if(keystore.getSeed().getType() == DeterministicSeed.Type.ELECTRUM) { - ek.seed = keystore.getSeed().getMnemonicString(); + ek.seed = keystore.getSeed().getMnemonicString().asString(); ek.passphrase = keystore.getSeed().getPassphrase(); } else if(keystore.getSeed().getType() == DeterministicSeed.Type.BIP39) { ew.seed_type = "bip39"; diff --git a/src/main/java/com/sparrowwallet/sparrow/io/Storage.java b/src/main/java/com/sparrowwallet/sparrow/io/Storage.java index 031165a5..980eb3ef 100644 --- a/src/main/java/com/sparrowwallet/sparrow/io/Storage.java +++ b/src/main/java/com/sparrowwallet/sparrow/io/Storage.java @@ -93,14 +93,6 @@ public class Storage { if(wallet.containsSeeds()) { //Derive xpub and master fingerprint from seed, potentially with passphrase Wallet copy = wallet.copy(); - if(wallet.isEncrypted()) { - if(key == null) { - throw new IllegalStateException("Wallet was not encrypted, but seed is"); - } - - copy.decrypt(key); - } - for(Keystore copyKeystore : copy.getKeystores()) { if(copyKeystore.hasSeed()) { if(copyKeystore.getSeed().needsPassphrase()) { @@ -117,6 +109,14 @@ public class Storage { } } + if(wallet.isEncrypted()) { + if(key == null) { + throw new IllegalStateException("Wallet was not encrypted, but seed is"); + } + + copy.decrypt(key); + } + for(int i = 0; i < wallet.getKeystores().size(); i++) { Keystore keystore = wallet.getKeystores().get(i); if(keystore.hasSeed()) { @@ -125,6 +125,7 @@ public class Storage { keystore.setKeyDerivation(derivedKeystore.getKeyDerivation()); keystore.setExtendedPublicKey(derivedKeystore.getExtendedPublicKey()); keystore.getSeed().setPassphrase(copyKeystore.getSeed().getPassphrase()); + copyKeystore.getSeed().clear(); } } } diff --git a/src/test/java/com/sparrowwallet/sparrow/io/ColdcardMultisigTest.java b/src/test/java/com/sparrowwallet/sparrow/io/ColdcardMultisigTest.java index 83b07b88..56466f29 100644 --- a/src/test/java/com/sparrowwallet/sparrow/io/ColdcardMultisigTest.java +++ b/src/test/java/com/sparrowwallet/sparrow/io/ColdcardMultisigTest.java @@ -16,7 +16,7 @@ public class ColdcardMultisigTest extends IoTest { public void importKeystore1() throws ImportException { ColdcardMultisig ccMultisig = new ColdcardMultisig(); Keystore keystore = ccMultisig.getKeystore(ScriptType.P2SH_P2WSH, getInputStream("cc-multisig-keystore-1.json"), null); - Assert.assertEquals("Coldcard 0F056943", keystore.getLabel()); + Assert.assertEquals("Coldcard", keystore.getLabel()); Assert.assertEquals("m/48'/1'/0'/1'", keystore.getKeyDerivation().getDerivationPath()); Assert.assertEquals("0f056943", keystore.getKeyDerivation().getMasterFingerprint()); Assert.assertEquals(ExtendedKey.fromDescriptor("Upub5T4XUooQzDXL58NCHk8ZCw9BsRSLCtnyHeZEExAq1XdnBFXiXVrHFuvvmh3TnCR7XmKHxkwqdACv68z7QKT1vwru9L1SZSsw8B2fuBvtSa6"), keystore.getExtendedPublicKey()); @@ -33,7 +33,7 @@ public class ColdcardMultisigTest extends IoTest { public void importKeystore2() throws ImportException { ColdcardMultisig ccMultisig = new ColdcardMultisig(); Keystore keystore = ccMultisig.getKeystore(ScriptType.P2SH, getInputStream("cc-multisig-keystore-2.json"), null); - Assert.assertEquals("Coldcard 6BA6CFD0", keystore.getLabel()); + Assert.assertEquals("Coldcard", keystore.getLabel()); Assert.assertEquals("m/45'", keystore.getKeyDerivation().getDerivationPath()); Assert.assertEquals("6ba6cfd0", keystore.getKeyDerivation().getMasterFingerprint()); Assert.assertEquals(ExtendedKey.fromDescriptor("tpubD9429UXFGCTKJ9NdiNK4rC5ygqSUkginycYHccqSg5gkmyQ7PZRHNjk99M6a6Y3NY8ctEUUJvCu6iCCui8Ju3xrHRu3Ez1CKB4ZFoRZDdP9"), keystore.getExtendedPublicKey()); @@ -44,7 +44,7 @@ public class ColdcardMultisigTest extends IoTest { public void importKeystore2b() throws ImportException { ColdcardMultisig ccMultisig = new ColdcardMultisig(); Keystore keystore = ccMultisig.getKeystore(ScriptType.P2WSH, getInputStream("cc-multisig-keystore-2.json"), null); - Assert.assertEquals("Coldcard 6BA6CFD0", keystore.getLabel()); + Assert.assertEquals("Coldcard", keystore.getLabel()); Assert.assertEquals("m/48'/1'/0'/2'", keystore.getKeyDerivation().getDerivationPath()); Assert.assertEquals("6ba6cfd0", keystore.getKeyDerivation().getMasterFingerprint()); Assert.assertEquals(ExtendedKey.fromDescriptor("Vpub5nUnvPehg1VYQh13dGznx1P9moac3SNUrn3qhU9r85RhXabYbSSBNsNNwyR7akozAZJw1SZmRRjry1zY8PWMuw8Ga1vQZ5qzPjKyTDQwtzs"), keystore.getExtendedPublicKey()); @@ -59,7 +59,7 @@ public class ColdcardMultisigTest extends IoTest { Assert.assertEquals(PolicyType.MULTI, wallet.getPolicyType()); Assert.assertEquals(ScriptType.P2WSH, wallet.getScriptType()); Assert.assertEquals(2, wallet.getDefaultPolicy().getNumSignaturesRequired()); - Assert.assertEquals("multi(2,coldcard0f056943,coldcard6ba6cfd0,coldcard747b698e,coldcard7bb026be)", wallet.getDefaultPolicy().getMiniscript().getScript()); + Assert.assertEquals("multi(2,coldcard1,coldcard2,coldcard3,coldcard4)", wallet.getDefaultPolicy().getMiniscript().getScript()); Assert.assertTrue(wallet.isValid()); } @@ -71,7 +71,7 @@ public class ColdcardMultisigTest extends IoTest { Assert.assertEquals(PolicyType.MULTI, wallet.getPolicyType()); Assert.assertEquals(ScriptType.P2SH_P2WSH, wallet.getScriptType()); Assert.assertEquals(2, wallet.getDefaultPolicy().getNumSignaturesRequired()); - Assert.assertEquals("multi(2,coldcard0f056943,coldcard6ba6cfd0,coldcard747b698e,coldcard7bb026be)", wallet.getDefaultPolicy().getMiniscript().getScript()); + Assert.assertEquals("multi(2,coldcard1,coldcard2,coldcard3,coldcard4)", wallet.getDefaultPolicy().getMiniscript().getScript()); Assert.assertTrue(wallet.isValid()); } @@ -83,7 +83,7 @@ public class ColdcardMultisigTest extends IoTest { Assert.assertEquals(PolicyType.MULTI, wallet.getPolicyType()); Assert.assertEquals(ScriptType.P2WSH, wallet.getScriptType()); Assert.assertEquals(3, wallet.getDefaultPolicy().getNumSignaturesRequired()); - Assert.assertEquals("multi(3,coldcard06b57041,coldcard4b569672,coldcardca9a2b19)", wallet.getDefaultPolicy().getMiniscript().getScript()); + Assert.assertEquals("multi(3,coldcard1,coldcard2,coldcard3)", wallet.getDefaultPolicy().getMiniscript().getScript()); Assert.assertEquals("06b57041", wallet.getKeystores().get(0).getKeyDerivation().getMasterFingerprint()); Assert.assertEquals("m/48'/0'/0'/2'", wallet.getKeystores().get(0).getKeyDerivation().getDerivationPath()); Assert.assertEquals("xpub6EfEGa5isJbQFSswM5Uptw5BSq2Td1ZDJr3QUNUcMySpC7itZ3ccypVHtLPnvMzKQ2qxrAgH49vhVxRcaQLFbixAVRR8RACrYTp88Uv9h8Z", wallet.getKeystores().get(0).getExtendedPublicKey().toString()); diff --git a/src/test/java/com/sparrowwallet/sparrow/io/ColdcardSinglesigTest.java b/src/test/java/com/sparrowwallet/sparrow/io/ColdcardSinglesigTest.java index 3bcfad07..4c0b9b5c 100644 --- a/src/test/java/com/sparrowwallet/sparrow/io/ColdcardSinglesigTest.java +++ b/src/test/java/com/sparrowwallet/sparrow/io/ColdcardSinglesigTest.java @@ -12,7 +12,7 @@ public class ColdcardSinglesigTest extends IoTest { ColdcardSinglesig ccSingleSig = new ColdcardSinglesig(); Keystore keystore = ccSingleSig.getKeystore(ScriptType.P2SH_P2WPKH, getInputStream("cc-singlesig-keystore-1.json"), null); - Assert.assertEquals("Coldcard 0F056943", keystore.getLabel()); + Assert.assertEquals("Coldcard", keystore.getLabel()); Assert.assertEquals("m/49'/1'/123'", keystore.getKeyDerivation().getDerivationPath()); Assert.assertEquals("0f056943", keystore.getKeyDerivation().getMasterFingerprint()); Assert.assertEquals(ExtendedKey.fromDescriptor("tpubDCDqt7XXvhAdy1MpSze5nMJA9x8DrdRaKALRRPasfxyHpiqWWEAr9cbDBQ9BcX7cB3up98Pk97U2QQ3xrvQsi5dNPmRYYhdcsKY9wwEY87T"), keystore.getExtendedPublicKey()); @@ -24,7 +24,7 @@ public class ColdcardSinglesigTest extends IoTest { ColdcardSinglesig ccSingleSig = new ColdcardSinglesig(); Keystore keystore = ccSingleSig.getKeystore(ScriptType.P2WPKH, getInputStream("cc-singlesig-keystore-1.json"), null); - Assert.assertEquals("Coldcard 0F056943", keystore.getLabel()); + Assert.assertEquals("Coldcard", keystore.getLabel()); Assert.assertEquals("m/84'/1'/123'", keystore.getKeyDerivation().getDerivationPath()); Assert.assertEquals("0f056943", keystore.getKeyDerivation().getMasterFingerprint()); Assert.assertEquals(ExtendedKey.fromDescriptor("tpubDC7jGaaSE66VDB6VhEDFYQSCAyugXmfnMnrMVyHNzW9wryyTxvha7TmfAHd7GRXrr2TaAn2HXn9T8ep4gyNX1bzGiieqcTUNcu2poyntrET"), keystore.getExtendedPublicKey()); diff --git a/src/test/java/com/sparrowwallet/sparrow/io/ElectrumTest.java b/src/test/java/com/sparrowwallet/sparrow/io/ElectrumTest.java index 2d08ab00..338e739b 100644 --- a/src/test/java/com/sparrowwallet/sparrow/io/ElectrumTest.java +++ b/src/test/java/com/sparrowwallet/sparrow/io/ElectrumTest.java @@ -96,7 +96,7 @@ public class ElectrumTest extends IoTest { Assert.assertEquals(PolicyType.SINGLE, wallet.getPolicyType()); Assert.assertEquals(ScriptType.P2WPKH, wallet.getScriptType()); Assert.assertEquals(1, wallet.getDefaultPolicy().getNumSignaturesRequired()); - Assert.assertEquals("pkh(electrumf881eac5)", wallet.getDefaultPolicy().getMiniscript().getScript()); + Assert.assertEquals("pkh(electrum)", wallet.getDefaultPolicy().getMiniscript().getScript()); Assert.assertEquals("f881eac5", wallet.getKeystores().get(0).getKeyDerivation().getMasterFingerprint()); Assert.assertEquals("m/0'", wallet.getKeystores().get(0).getKeyDerivation().getDerivationPath()); Assert.assertEquals("xpub69iSRreMB6fu24sU8Tdxv7yYGqzPkDwPkwqUfKJTxW3p8afW7XvTewVCapuX3dQjdD197iF65WcjYaNpFbwWT3RyuZ1KJ3ToJNVWKWyAJ6f", wallet.getKeystores().get(0).getExtendedPublicKey().toString()); @@ -116,11 +116,11 @@ public class ElectrumTest extends IoTest { Assert.assertEquals(PolicyType.SINGLE, wallet.getPolicyType()); Assert.assertEquals(ScriptType.P2WPKH, wallet.getScriptType()); Assert.assertEquals(1, wallet.getDefaultPolicy().getNumSignaturesRequired()); - Assert.assertEquals("pkh(electrum59c5474f)", wallet.getDefaultPolicy().getMiniscript().getScript()); + Assert.assertEquals("pkh(electrum)", wallet.getDefaultPolicy().getMiniscript().getScript()); Assert.assertEquals("59c5474f", wallet.getKeystores().get(0).getKeyDerivation().getMasterFingerprint()); Assert.assertEquals("m/0'", wallet.getKeystores().get(0).getKeyDerivation().getDerivationPath()); Assert.assertEquals("xpub68YmVxWbxqjpxbUqqaPrgkBQPBSJuq6gEaL22uuytSEojtS2x5eLPN2uspUuyigtnMkoHrFSF1KwoXPwjzuaUjErUwztxfHquAwuaQhSd9J", wallet.getKeystores().get(0).getExtendedPublicKey().toString()); - Assert.assertEquals(wallet.getKeystores().get(0).getSeed().getMnemonicString(), "coach fan denial rifle frost rival join install one wasp cool antique"); + Assert.assertEquals(wallet.getKeystores().get(0).getSeed().getMnemonicString().asString(), "coach fan denial rifle frost rival join install one wasp cool antique"); Assert.assertEquals("e14c40c638e2c83d1f20e5ee9cd744bc2ba1ef64fa939926f3778fc8735e891f56852f687b32bbd044f272d2831137e3eeba61fd1f285fa73dcc97d9f2be3cd1", Utils.bytesToHex(wallet.getKeystores().get(0).getSeed().getSeedBytes())); } } \ No newline at end of file