From d2bd335e76a6a18634d033b49bf2d36c2494d9cf Mon Sep 17 00:00:00 2001 From: Craig Raw Date: Tue, 19 May 2020 14:02:29 +0200 Subject: [PATCH] securely handle mnemonic seed in memory --- .../sparrowwallet/drongo/SecureString.java | 39 ++++++++ .../java/com/sparrowwallet/drongo/Utils.java | 36 -------- .../drongo/crypto/AESKeyCrypter.java | 4 +- .../drongo/crypto/Argon2KeyDeriver.java | 4 +- .../drongo/crypto/DoubleSha256KeyDeriver.java | 6 +- .../drongo/crypto/EncryptionType.java | 5 +- .../com/sparrowwallet/drongo/crypto/Key.java | 7 ++ .../drongo/crypto/Pbkdf2KeyDeriver.java | 4 +- .../drongo/crypto/ScryptKeyDeriver.java | 4 +- .../drongo/wallet/DeterministicSeed.java | 92 ++++++++++++++----- .../drongo/wallet/DeterministicSeedTest.java | 2 +- 11 files changed, 129 insertions(+), 74 deletions(-) diff --git a/src/main/java/com/sparrowwallet/drongo/SecureString.java b/src/main/java/com/sparrowwallet/drongo/SecureString.java index 80d8edd..ca2b396 100644 --- a/src/main/java/com/sparrowwallet/drongo/SecureString.java +++ b/src/main/java/com/sparrowwallet/drongo/SecureString.java @@ -1,5 +1,8 @@ package com.sparrowwallet.drongo; +import java.nio.ByteBuffer; +import java.nio.CharBuffer; +import java.nio.charset.StandardCharsets; import java.security.SecureRandom; import java.util.Arrays; @@ -97,4 +100,40 @@ public class SecureString implements CharSequence { chars[i] = pad[i] ^ charAt; } } + + public static byte[] toBytesUTF8(CharSequence charSequence) { + CharBuffer charBuffer = CharBuffer.wrap(charSequence); + ByteBuffer byteBuffer = StandardCharsets.UTF_8.encode(charBuffer); + byte[] bytes = Arrays.copyOfRange(byteBuffer.array(), byteBuffer.position(), byteBuffer.limit()); + Arrays.fill(byteBuffer.array(), (byte)0); // clear sensitive data + return bytes; + } + + public static SecureString fromBytesUTF8(byte[] bytes) { + ByteBuffer byteBuffer = ByteBuffer.wrap(bytes); + CharBuffer charBuffer = StandardCharsets.UTF_8.decode(byteBuffer); + SecureString secureString = new SecureString(charBuffer); + Arrays.fill(charBuffer.array(), (char)0); + return secureString; + } + + public static byte[] toBytesUTF16(CharSequence charSequence) { + byte[] byteArray = new byte[charSequence.length() << 1]; + for(int i = 0; i < charSequence.length(); i++) { + int bytePosition = i << 1; + byteArray[bytePosition] = (byte) ((charSequence.charAt(i)&0xFF00)>>8); + byteArray[bytePosition + 1] = (byte) (charSequence.charAt(i)&0x00FF); + } + return byteArray; + } + + public static boolean isValidUTF16(CharSequence charSequence) { + for (int i = 0; i < charSequence.length(); i++) { + if (Character.isLowSurrogate(charSequence.charAt(i)) && (i == 0 || !Character.isHighSurrogate(charSequence.charAt(i - 1))) + || Character.isHighSurrogate(charSequence.charAt(i)) && (i == charSequence.length() -1 || !Character.isLowSurrogate(charSequence.charAt(i + 1)))) { + return false; + } + } + return true; + } } diff --git a/src/main/java/com/sparrowwallet/drongo/Utils.java b/src/main/java/com/sparrowwallet/drongo/Utils.java index 72db457..b7666f3 100644 --- a/src/main/java/com/sparrowwallet/drongo/Utils.java +++ b/src/main/java/com/sparrowwallet/drongo/Utils.java @@ -283,40 +283,4 @@ public class Utils { hmacSha512.doFinal(out, 0); return out; } - - public static byte[] toBytesUTF8(CharSequence charSequence) { - CharBuffer charBuffer = CharBuffer.wrap(charSequence); - ByteBuffer byteBuffer = StandardCharsets.UTF_8.encode(charBuffer); - byte[] bytes = Arrays.copyOfRange(byteBuffer.array(), byteBuffer.position(), byteBuffer.limit()); - Arrays.fill(byteBuffer.array(), (byte)0); // clear sensitive data - return bytes; - } - - public static SecureString fromBytesUTF8(byte[] bytes) { - ByteBuffer byteBuffer = ByteBuffer.wrap(bytes); - CharBuffer charBuffer = StandardCharsets.UTF_8.decode(byteBuffer); - SecureString secureString = new SecureString(charBuffer); - Arrays.fill(charBuffer.array(), (char)0); - return secureString; - } - - public static byte[] toBytesUTF16(CharSequence charSequence) { - byte[] byteArray = new byte[charSequence.length() << 1]; - for(int i = 0; i < charSequence.length(); i++) { - int bytePosition = i << 1; - byteArray[bytePosition] = (byte) ((charSequence.charAt(i)&0xFF00)>>8); - byteArray[bytePosition + 1] = (byte) (charSequence.charAt(i)&0x00FF); - } - return byteArray; - } - - public static boolean isValidUTF16(CharSequence charSequence) { - for (int i = 0; i < charSequence.length(); i++) { - if (Character.isLowSurrogate(charSequence.charAt(i)) && (i == 0 || !Character.isHighSurrogate(charSequence.charAt(i - 1))) - || Character.isHighSurrogate(charSequence.charAt(i)) && (i == charSequence.length() -1 || !Character.isLowSurrogate(charSequence.charAt(i + 1)))) { - return false; - } - } - return true; - } } diff --git a/src/main/java/com/sparrowwallet/drongo/crypto/AESKeyCrypter.java b/src/main/java/com/sparrowwallet/drongo/crypto/AESKeyCrypter.java index 3020dbe..48d2a5a 100644 --- a/src/main/java/com/sparrowwallet/drongo/crypto/AESKeyCrypter.java +++ b/src/main/java/com/sparrowwallet/drongo/crypto/AESKeyCrypter.java @@ -49,7 +49,9 @@ public class AESKeyCrypter implements KeyCrypter { final int length1 = cipher.processBytes(cipherBytes, 0, cipherBytes.length, decryptedBytes, 0); final int length2 = cipher.doFinal(decryptedBytes, length1); - return Arrays.copyOf(decryptedBytes, length1 + length2); + byte[] decrypted = Arrays.copyOf(decryptedBytes, length1 + length2); + Arrays.fill(decryptedBytes, (byte)0); + return decrypted; } catch (InvalidCipherTextException e) { throw new KeyCrypterException.InvalidCipherText("Could not decrypt bytes", e); } catch (RuntimeException e) { diff --git a/src/main/java/com/sparrowwallet/drongo/crypto/Argon2KeyDeriver.java b/src/main/java/com/sparrowwallet/drongo/crypto/Argon2KeyDeriver.java index 7e4c46c..9f9e3fe 100644 --- a/src/main/java/com/sparrowwallet/drongo/crypto/Argon2KeyDeriver.java +++ b/src/main/java/com/sparrowwallet/drongo/crypto/Argon2KeyDeriver.java @@ -1,6 +1,6 @@ package com.sparrowwallet.drongo.crypto; -import com.sparrowwallet.drongo.Utils; +import com.sparrowwallet.drongo.SecureString; import de.mkammerer.argon2.Argon2Advanced; import de.mkammerer.argon2.Argon2Factory; @@ -42,7 +42,7 @@ public class Argon2KeyDeriver implements KeyDeriver, AsymmetricKeyDeriver { @Override public Key deriveKey(CharSequence password) throws KeyCrypterException { Argon2Advanced argon2 = Argon2Factory.createAdvanced(Argon2Factory.Argon2Types.ARGON2id, argon2Parameters.saltLength, argon2Parameters.hashLength); - byte[] hash = argon2.rawHash(argon2Parameters.iterations, argon2Parameters.memory, argon2Parameters.parallelism, Utils.toBytesUTF8(password), salt); + byte[] hash = argon2.rawHash(argon2Parameters.iterations, argon2Parameters.memory, argon2Parameters.parallelism, SecureString.toBytesUTF8(password), salt); return new Key(hash, salt, getDeriverType()); } diff --git a/src/main/java/com/sparrowwallet/drongo/crypto/DoubleSha256KeyDeriver.java b/src/main/java/com/sparrowwallet/drongo/crypto/DoubleSha256KeyDeriver.java index abda797..9d446a2 100644 --- a/src/main/java/com/sparrowwallet/drongo/crypto/DoubleSha256KeyDeriver.java +++ b/src/main/java/com/sparrowwallet/drongo/crypto/DoubleSha256KeyDeriver.java @@ -1,15 +1,13 @@ package com.sparrowwallet.drongo.crypto; -import com.sparrowwallet.drongo.Utils; +import com.sparrowwallet.drongo.SecureString; import com.sparrowwallet.drongo.protocol.Sha256Hash; -import java.nio.charset.StandardCharsets; - public class DoubleSha256KeyDeriver implements KeyDeriver { @Override public Key deriveKey(CharSequence password) throws KeyCrypterException { - byte[] passwordBytes = Utils.toBytesUTF8(password); + byte[] passwordBytes = SecureString.toBytesUTF8(password); byte[] sha256 = Sha256Hash.hash(passwordBytes); byte[] doubleSha256 = Sha256Hash.hash(sha256); return new Key(doubleSha256, null, getDeriverType()); diff --git a/src/main/java/com/sparrowwallet/drongo/crypto/EncryptionType.java b/src/main/java/com/sparrowwallet/drongo/crypto/EncryptionType.java index f137623..ffbecd2 100644 --- a/src/main/java/com/sparrowwallet/drongo/crypto/EncryptionType.java +++ b/src/main/java/com/sparrowwallet/drongo/crypto/EncryptionType.java @@ -1,8 +1,7 @@ package com.sparrowwallet.drongo.crypto; -import com.sparrowwallet.drongo.Utils; +import com.sparrowwallet.drongo.SecureString; -import java.nio.charset.StandardCharsets; import java.util.Objects; public class EncryptionType { @@ -12,7 +11,7 @@ public class EncryptionType { return new KeyDeriver() { @Override public Key deriveKey(CharSequence password) throws KeyCrypterException { - return new Key(Utils.toBytesUTF8(password), null, NONE); + return new Key(SecureString.toBytesUTF8(password), null, NONE); } @Override diff --git a/src/main/java/com/sparrowwallet/drongo/crypto/Key.java b/src/main/java/com/sparrowwallet/drongo/crypto/Key.java index a379153..c240c90 100644 --- a/src/main/java/com/sparrowwallet/drongo/crypto/Key.java +++ b/src/main/java/com/sparrowwallet/drongo/crypto/Key.java @@ -1,5 +1,7 @@ package com.sparrowwallet.drongo.crypto; +import java.util.Arrays; + public class Key { private final byte[] keyBytes; private final byte[] salt; @@ -22,4 +24,9 @@ public class Key { public EncryptionType.Deriver getDeriver() { return deriver; } + + public void clear() { + Arrays.fill(keyBytes, (byte)0); + Arrays.fill(salt, (byte)0); + } } diff --git a/src/main/java/com/sparrowwallet/drongo/crypto/Pbkdf2KeyDeriver.java b/src/main/java/com/sparrowwallet/drongo/crypto/Pbkdf2KeyDeriver.java index 335584a..578b27b 100644 --- a/src/main/java/com/sparrowwallet/drongo/crypto/Pbkdf2KeyDeriver.java +++ b/src/main/java/com/sparrowwallet/drongo/crypto/Pbkdf2KeyDeriver.java @@ -1,6 +1,6 @@ package com.sparrowwallet.drongo.crypto; -import com.sparrowwallet.drongo.Utils; +import com.sparrowwallet.drongo.SecureString; import org.bouncycastle.crypto.digests.SHA512Digest; import org.bouncycastle.crypto.generators.PKCS5S2ParametersGenerator; import org.bouncycastle.crypto.params.KeyParameter; @@ -36,7 +36,7 @@ public class Pbkdf2KeyDeriver implements KeyDeriver, AsymmetricKeyDeriver { @Override public Key deriveKey(CharSequence password) throws KeyCrypterException { PKCS5S2ParametersGenerator gen = new PKCS5S2ParametersGenerator(new SHA512Digest()); - gen.init(Utils.toBytesUTF8(password), salt, iterationCount); + gen.init(SecureString.toBytesUTF8(password), salt, iterationCount); byte[] keyBytes = ((KeyParameter)gen.generateDerivedParameters(512)).getKey(); return new Key(keyBytes, salt, getDeriverType()); } diff --git a/src/main/java/com/sparrowwallet/drongo/crypto/ScryptKeyDeriver.java b/src/main/java/com/sparrowwallet/drongo/crypto/ScryptKeyDeriver.java index c24c4bb..40d70a8 100644 --- a/src/main/java/com/sparrowwallet/drongo/crypto/ScryptKeyDeriver.java +++ b/src/main/java/com/sparrowwallet/drongo/crypto/ScryptKeyDeriver.java @@ -1,6 +1,6 @@ package com.sparrowwallet.drongo.crypto; -import com.sparrowwallet.drongo.Utils; +import com.sparrowwallet.drongo.SecureString; import org.bouncycastle.crypto.generators.SCrypt; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -103,7 +103,7 @@ public class ScryptKeyDeriver implements KeyDeriver { public Key deriveKey(CharSequence password) throws KeyCrypterException { byte[] passwordBytes = null; try { - passwordBytes = Utils.toBytesUTF8(password); + passwordBytes = SecureString.toBytesUTF8(password); byte[] salt = new byte[0]; if (scryptParameters.getSalt() != null) { salt = scryptParameters.getSalt(); diff --git a/src/main/java/com/sparrowwallet/drongo/wallet/DeterministicSeed.java b/src/main/java/com/sparrowwallet/drongo/wallet/DeterministicSeed.java index 04db86b..d2c9469 100644 --- a/src/main/java/com/sparrowwallet/drongo/wallet/DeterministicSeed.java +++ b/src/main/java/com/sparrowwallet/drongo/wallet/DeterministicSeed.java @@ -1,9 +1,9 @@ package com.sparrowwallet.drongo.wallet; +import com.sparrowwallet.drongo.SecureString; import com.sparrowwallet.drongo.Utils; import com.sparrowwallet.drongo.crypto.*; -import java.nio.charset.StandardCharsets; import java.security.SecureRandom; import java.util.*; @@ -20,7 +20,7 @@ public class DeterministicSeed implements EncryptableItem { //Session only storage private transient String passphrase; - public DeterministicSeed(String mnemonicString, String passphrase, long creationTimeSeconds, Type type) { + public DeterministicSeed(CharSequence mnemonicString, String passphrase, long creationTimeSeconds, Type type) { this(decodeMnemonicCode(mnemonicString), passphrase, creationTimeSeconds, type); } @@ -123,15 +123,6 @@ public class DeterministicSeed implements EncryptableItem { return encryptedMnemonicCode != null; } - @Override - public String toString() { - if(isEncrypted()) { - return encryptedMnemonicCode.toString(); - } - - return getMnemonicString(); - } - /** Returns the seed as hex or null if encrypted. */ public String toHexString() throws MnemonicException { byte[] seed = getSeedBytes(); @@ -183,7 +174,10 @@ public class DeterministicSeed implements EncryptableItem { } KeyCrypter keyCrypter = getEncryptionType().getCrypter().getKeyCrypter(); - EncryptedData encryptedMnemonic = keyCrypter.encrypt(getMnemonicAsBytes(), null, key); + byte[] mnemonicBytes = getMnemonicAsBytes(); + EncryptedData encryptedMnemonic = keyCrypter.encrypt(mnemonicBytes, null, key); + Arrays.fill(mnemonicBytes != null ? mnemonicBytes : new byte[0], (byte)0); + DeterministicSeed seed = new DeterministicSeed(encryptedMnemonic, needsPassphrase, creationTimeSeconds, type); seed.setPassphrase(passphrase); @@ -191,12 +185,15 @@ public class DeterministicSeed implements EncryptableItem { } private byte[] getMnemonicAsBytes() { - String mnemonicString = getMnemonicString(); + SecureString mnemonicString = getMnemonicString(); if(mnemonicString == null) { return null; } - return mnemonicString.getBytes(StandardCharsets.UTF_8); + byte[] mnemonicBytes = SecureString.toBytesUTF8(mnemonicString); + mnemonicString.clear(); + + return mnemonicBytes; } public DeterministicSeed decrypt(CharSequence password) { @@ -206,8 +203,10 @@ public class DeterministicSeed implements EncryptableItem { KeyDeriver keyDeriver = getEncryptionType().getDeriver().getKeyDeriver(encryptedMnemonicCode.getKeySalt()); Key key = keyDeriver.deriveKey(password); + DeterministicSeed seed = decrypt(key); + key.clear(); - return decrypt(key); + return seed; } public DeterministicSeed decrypt(Key key) { @@ -216,13 +215,25 @@ public class DeterministicSeed implements EncryptableItem { } KeyCrypter keyCrypter = getEncryptionType().getCrypter().getKeyCrypter(); - List mnemonic = decodeMnemonicCode(keyCrypter.decrypt(encryptedMnemonicCode, key)); + byte[] decrypted = keyCrypter.decrypt(encryptedMnemonicCode, key); + List mnemonic = decodeMnemonicCode(decrypted); + Arrays.fill(decrypted, (byte)0); + DeterministicSeed seed = new DeterministicSeed(mnemonic, needsPassphrase, creationTimeSeconds, type); seed.setPassphrase(passphrase); return seed; } + @Override + public String toString() { + return "DeterministicSeed{" + + "type=" + type + + ", encryptedMnemonicCode=" + encryptedMnemonicCode + + ", needsPassphrase=" + needsPassphrase + + '}'; + } + @Override public boolean equals(Object o) { if (this == o) return true; @@ -250,6 +261,15 @@ public class DeterministicSeed implements EncryptableItem { } } + public void clear() { + if(mnemonicCode != null) { + mnemonicCode.clear(); + } + if(passphrase != null) { + passphrase = ""; + } + } + byte[] getEntropyBytes() throws MnemonicException { return type.getEntropyBytes(mnemonicCode); } @@ -260,25 +280,51 @@ public class DeterministicSeed implements EncryptableItem { } /** Get the mnemonic code as string, or null if unknown. */ - public String getMnemonicString() { - StringJoiner joiner = new StringJoiner(" "); + public SecureString getMnemonicString() { + StringBuilder builder = new StringBuilder(); if(mnemonicCode != null) { for(String word : mnemonicCode) { - joiner.add(word); + builder.append(word); + builder.append(' '); } - return joiner.toString(); + if(builder.length() > 0) { + builder.setLength(builder.length() - 1); + } + + return new SecureString(builder); } return null; } private static List decodeMnemonicCode(byte[] mnemonicCode) { - return decodeMnemonicCode(new String(mnemonicCode, StandardCharsets.UTF_8)); + SecureString secureString = SecureString.fromBytesUTF8(mnemonicCode); + List words = decodeMnemonicCode(secureString); + secureString.clear(); + + return words; } - private static List decodeMnemonicCode(String mnemonicCode) { - return Arrays.asList(mnemonicCode.split(" ")); + private static List decodeMnemonicCode(CharSequence mnemonicCode) { + List words = new ArrayList<>(); + StringBuilder word = new StringBuilder(); + for(int i = 0; i < mnemonicCode.length(); i++) { + char c = mnemonicCode.charAt(i); + if(c != ' ') { + word.append(mnemonicCode.charAt(i)); + } + if(c == ' ' || i == mnemonicCode.length() - 1) { + words.add(word.toString()); + + for(int j = 0; j < word.length(); j++) { + word.setCharAt(j, ' '); + } + word = new StringBuilder(); + } + } + + return words; } public DeterministicSeed copy() { diff --git a/src/test/java/com/sparrowwallet/drongo/wallet/DeterministicSeedTest.java b/src/test/java/com/sparrowwallet/drongo/wallet/DeterministicSeedTest.java index 3c3f0bf..1fa04d0 100644 --- a/src/test/java/com/sparrowwallet/drongo/wallet/DeterministicSeedTest.java +++ b/src/test/java/com/sparrowwallet/drongo/wallet/DeterministicSeedTest.java @@ -15,7 +15,7 @@ public class DeterministicSeedTest { DeterministicSeed encryptedSeed = seed.encrypt(keyDeriver.deriveKey("pass")); DeterministicSeed decryptedSeed = encryptedSeed.decrypt("pass"); - Assert.assertEquals(words, decryptedSeed.getMnemonicString()); + Assert.assertEquals(words, decryptedSeed.getMnemonicString().asString()); } @Test