Skip to content

Commit

Permalink
Implement abililty to use BIP39 password. This means this password ne… (
Browse files Browse the repository at this point in the history
#990)

* Implement abililty to use BIP39 password. This means this password needs to be password through our various projects to be able to correctly generate the key that controls the wallet. This also renames 'CreateKeyManagerApi' -> 'BIP39CreateKeymanagerApi' as the bip39 password is needed when creating the KeyManager

* Add bip39 password to BIP39KeyManager.fromParams(), fix mdocs

* Fix bug in unit test were whe weren't specifying password

* Fix rebase issues
  • Loading branch information
Christewart committed Jan 3, 2020
1 parent d776e1c commit 039722a
Show file tree
Hide file tree
Showing 16 changed files with 214 additions and 90 deletions.
16 changes: 10 additions & 6 deletions app/server/src/main/scala/org/bitcoins/server/Main.scala
Expand Up @@ -38,13 +38,13 @@ object Main extends App {
val peerSocket =
parseInetSocketAddress(nodeConf.peers.head, nodeConf.network.port)
val peer = Peer.fromSocket(peerSocket)

val bip39PasswordOpt = None //todo need to prompt user for this
val startFut = for {
_ <- conf.initialize()

uninitializedNode <- createNode
chainApi <- uninitializedNode.chainApiFromDb()
wallet <- createWallet(uninitializedNode, chainApi)
wallet <- createWallet(uninitializedNode, chainApi, bip39PasswordOpt)
node <- initializeNode(uninitializedNode, wallet)

_ <- node.start()
Expand Down Expand Up @@ -94,22 +94,25 @@ object Main extends App {

private def createWallet(
nodeApi: Node,
chainQueryApi: ChainQueryApi): Future[UnlockedWalletApi] = {
chainQueryApi: ChainQueryApi,
bip39PasswordOpt: Option[String]): Future[UnlockedWalletApi] = {
if (hasWallet()) {
logger.info(s"Using pre-existing wallet")
val locked = LockedWallet(nodeApi, chainQueryApi)

// TODO change me when we implement proper password handling
locked.unlock(BIP39KeyManager.badPassphrase) match {
locked.unlock(BIP39KeyManager.badPassphrase, bip39PasswordOpt) match {
case Right(wallet) =>
Future.successful(wallet)
case Left(kmError) =>
error(kmError)
}
} else {
logger.info(s"Initializing key manager")
val bip39PasswordOpt = None
val keyManagerE: Either[KeyManagerInitializeError, BIP39KeyManager] =
BIP39KeyManager.initialize(walletConf.kmParams)
BIP39KeyManager.initialize(kmParams = walletConf.kmParams,
bip39PasswordOpt = bip39PasswordOpt)

val keyManager = keyManagerE match {
case Right(keyManager) => keyManager
Expand All @@ -120,7 +123,8 @@ object Main extends App {
logger.info(s"Creating new wallet")
val unInitializedWallet = Wallet(keyManager, nodeApi, chainQueryApi)

Wallet.initialize(wallet = unInitializedWallet)
Wallet.initialize(wallet = unInitializedWallet,
bip39PasswordOpt = bip39PasswordOpt)
}
}

Expand Down
7 changes: 5 additions & 2 deletions docs/applications/wallet.md
Expand Up @@ -111,7 +111,10 @@ val syncF: Future[ChainApi] = configF.flatMap { _ =>

//initialize our key manager, where we store our keys
import org.bitcoins.keymanager.bip39._
val keyManager = BIP39KeyManager.initialize(walletConfig.kmParams).getOrElse {
//you can add a password here if you want
//val bip39PasswordOpt = Some("my-password-here")
val bip39PasswordOpt = None
val keyManager = BIP39KeyManager.initialize(walletConfig.kmParams, bip39PasswordOpt).getOrElse {
throw new RuntimeException(s"Failed to initalize key manager")
}

Expand All @@ -135,7 +138,7 @@ val wallet = Wallet(keyManager, new NodeApi {
override def getFiltersBetweenHeights(startHeight: Int, endHeight: Int): Future[Vector[FilterResponse]] = Future.successful(Vector.empty)
})
val walletF: Future[LockedWalletApi] = configF.flatMap { _ =>
Wallet.initialize(wallet)
Wallet.initialize(wallet,bip39PasswordOpt)
}

// when this future completes, ww have sent a transaction
Expand Down
2 changes: 1 addition & 1 deletion docs/key-manager/key-manager.md
Expand Up @@ -78,7 +78,7 @@ val network = RegTest

val kmParams = KeyManagerParams(seedPath, purpose, network)

val km = BIP39KeyManager.initializeWithMnemonic(mnemonic, kmParams)
val km = BIP39KeyManager.initializeWithMnemonic(mnemonic, None, kmParams)

val rootXPub = km.right.get.getRootXPub

Expand Down
Expand Up @@ -38,19 +38,23 @@ class BIP39KeyManagerTest extends KeyManagerUnitTest {
val kmParams = buildParams()

val keyManager = withInitializedKeyManager(kmParams = kmParams,
entropy = mnemonic.toEntropy)
entropy = mnemonic.toEntropy,
bip39PasswordOpt = None)


keyManager.deriveXPub(hdAccount).get.toString must be ("xpub6D36zpm3tLPy3dBCpiScEpmmgsivFBcHxX5oXmPBW982BmLiEkjBEDdswxFUoeXpp272QuSpNKZ3f2TdEMkAHyCz1M7P3gFkYJJVEsM66SE")
}

it must "initialize a key manager to the same xpub if we call constructor directly or use CreateKeyManagerApi" in {
val kmParams = buildParams()
val direct = BIP39KeyManager(mnemonic, kmParams)
val direct = BIP39KeyManager(mnemonic, kmParams, None)

val directXpub = direct.getRootXPub

val api = BIP39KeyManager.initializeWithEntropy(mnemonic.toEntropy, kmParams).right.get
val api = BIP39KeyManager.initializeWithEntropy(
entropy = mnemonic.toEntropy,
bip39PasswordOpt = None,
kmParams = kmParams).right.get

val apiXpub = api.getRootXPub

Expand All @@ -61,9 +65,48 @@ class BIP39KeyManagerTest extends KeyManagerUnitTest {
assert(api.deriveXPub(hdAccount) == direct.deriveXPub(hdAccount))
}

it must "initialize a key manager with a bip39 password to the same xpub if we call constructor directly or use CreateKeyManagerApi" in {
val kmParams = buildParams()
val bip39Pw = KeyManagerTestUtil.bip39Password
val direct = BIP39KeyManager(mnemonic, kmParams,Some(bip39Pw))

val directXpub = direct.getRootXPub

val api = BIP39KeyManager.initializeWithEntropy(
mnemonic.toEntropy,
Some(bip39Pw),
kmParams).right.get

val apiXpub = api.getRootXPub

assert(apiXpub == directXpub, s"We don't have initialization symmetry between our constructors!")


//we should be able to derive the same child xpub
assert(api.deriveXPub(hdAccount) == direct.deriveXPub(hdAccount))
}

it must "NOT initialize a key manager to the same xpub if one has a password and one does not" in {
val kmParams = buildParams()
val bip39Pw = KeyManagerTestUtil.bip39Password

val withPassword = BIP39KeyManager(mnemonic, kmParams, Some(bip39Pw))
val withPasswordXpub = withPassword.getRootXPub

val noPassword = BIP39KeyManager(mnemonic, kmParams, None)

val noPwXpub = noPassword.getRootXPub

assert(withPasswordXpub != noPwXpub, s"A key manager with a BIP39 passwrod should not generate the same xpub as a key manager without a password!")

}


it must "return a mnemonic not found if we have not initialized the key manager" in {
val kmParams = buildParams()
val kmE = BIP39KeyManager.fromParams(kmParams, BIP39KeyManager.badPassphrase)
val kmE = BIP39KeyManager.fromParams(kmParams = kmParams,
password = BIP39KeyManager.badPassphrase,
bip39PasswordOpt = None)

assert(kmE == Left(ReadMnemonicError.NotFoundError))
}
Expand All @@ -80,7 +123,10 @@ class BIP39KeyManagerTest extends KeyManagerUnitTest {
val badEntropy = BitVector.empty


val init = BIP39KeyManager.initializeWithEntropy(badEntropy, buildParams())
val init = BIP39KeyManager.initializeWithEntropy(
entropy = badEntropy,
bip39PasswordOpt = KeyManagerTestUtil.bip39PasswordOpt,
kmParams = buildParams())

assert(init == Left(InitializeKeyManagerError.BadEntropy))
}
Expand Down
Expand Up @@ -6,9 +6,15 @@ import org.bitcoins.keymanager.{KeyManagerTestUtil, KeyManagerUnitTest, KeyManag
class BIP39LockedKeyManagerTest extends KeyManagerUnitTest {

it must "be able to read a locked mnemonic from disk" in {
val km = withInitializedKeyManager()
val bip39PwOpt = KeyManagerTestUtil.bip39PasswordOpt
val km = withInitializedKeyManager(bip39PasswordOpt = bip39PwOpt)

val unlockedE = BIP39LockedKeyManager.unlock(
KeyManagerTestUtil.badPassphrase,
bip39PasswordOpt = bip39PwOpt,
km.kmParams)

val unlockedKm = BIP39LockedKeyManager.unlock(KeyManagerTestUtil.badPassphrase, km.kmParams) match {
val unlockedKm = unlockedE match {
case Right(km) => km
case Left(err) => fail(s"Failed to unlock key manager ${err}")
}
Expand All @@ -20,7 +26,11 @@ class BIP39LockedKeyManagerTest extends KeyManagerUnitTest {
it must "fail to read bad json in the seed file" in {
val km = withInitializedKeyManager()
val badPassword = AesPassword.fromString("other bad password").get
BIP39LockedKeyManager.unlock(passphrase = badPassword, kmParams = km.kmParams) match {
val unlockedE = BIP39LockedKeyManager.unlock(passphrase = badPassword,
bip39PasswordOpt = None,
kmParams = km.kmParams)

unlockedE match {
case Left(KeyManagerUnlockError.BadPassword) => succeed
case result @ (Left(_) | Right(_)) =>
fail(s"Expected to fail test with ${KeyManagerUnlockError.BadPassword} got ${result}")
Expand All @@ -34,7 +44,9 @@ class BIP39LockedKeyManagerTest extends KeyManagerUnitTest {

val badPath = km.kmParams.copy(seedPath = badSeedPath)
val badPassword = AesPassword.fromString("other bad password").get
BIP39LockedKeyManager.unlock(badPassword, badPath) match {
val unlockedE = BIP39LockedKeyManager.unlock(badPassword, None, badPath)

unlockedE match {
case Left(KeyManagerUnlockError.MnemonicNotFound) => succeed
case result @ (Left(_) | Right(_)) =>
fail(s"Expected to fail test with ${KeyManagerUnlockError.MnemonicNotFound} got ${result}")
Expand Down
@@ -1,6 +1,7 @@
package org.bitcoins.keymanager

import org.bitcoins.core.crypto.MnemonicCode
import org.bitcoins.keymanager.bip39.BIP39KeyManager
import scodec.bits.BitVector

/**
Expand All @@ -16,21 +17,29 @@ import scodec.bits.BitVector
* can write it down. They should also be prompted
* to confirm at least parts of the code.
*/
trait KeyManagerCreateApi[T <: KeyManager] {
trait BIP39KeyManagerCreateApi {

/**
* $initialize
*/
final def initialize(
kmParams: KeyManagerParams): Either[KeyManagerInitializeError, T] =
initializeWithEntropy(entropy = MnemonicCode.getEntropy256Bits, kmParams)
kmParams: KeyManagerParams,
bip39PasswordOpt: Option[String]): Either[
KeyManagerInitializeError,
BIP39KeyManager] =
initializeWithEntropy(entropy = MnemonicCode.getEntropy256Bits,
bip39PasswordOpt = bip39PasswordOpt,
kmParams = kmParams)

/**
* $initializeWithEnt
*/
def initializeWithEntropy(
entropy: BitVector,
kmParams: KeyManagerParams): Either[KeyManagerInitializeError, T]
bip39PasswordOpt: Option[String],
kmParams: KeyManagerParams): Either[
KeyManagerInitializeError,
BIP39KeyManager]

/**
* Helper method to initialize a [[KeyManagerCreate$ KeyManager]] with a [[MnemonicCode MnemonicCode]]
Expand All @@ -41,8 +50,13 @@ trait KeyManagerCreateApi[T <: KeyManager] {
*/
final def initializeWithMnemonic(
mnemonicCode: MnemonicCode,
kmParams: KeyManagerParams): Either[KeyManagerInitializeError, T] = {
bip39PasswordOpt: Option[String],
kmParams: KeyManagerParams): Either[
KeyManagerInitializeError,
BIP39KeyManager] = {
val entropy = mnemonicCode.toEntropy
initializeWithEntropy(entropy = entropy, kmParams)
initializeWithEntropy(entropy = entropy,
bip39PasswordOpt = bip39PasswordOpt,
kmParams = kmParams)
}
}
Expand Up @@ -22,11 +22,17 @@ import scala.util.{Failure, Success, Try}
*/
case class BIP39KeyManager(
private val mnemonic: MnemonicCode,
kmParams: KeyManagerParams)
kmParams: KeyManagerParams,
private val bip39PasswordOpt: Option[String])
extends KeyManager {

private val seed = BIP39Seed.fromMnemonic(mnemonic = mnemonic,
password = BIP39Seed.EMPTY_PASSWORD)
private val seed = bip39PasswordOpt match {
case Some(pw) =>
BIP39Seed.fromMnemonic(mnemonic = mnemonic, password = pw)
case None =>
BIP39Seed.fromMnemonic(mnemonic = mnemonic,
password = BIP39Seed.EMPTY_PASSWORD)
}

private val privVersion: ExtKeyPrivVersion =
HDUtil.getXprivVersion(kmParams.purpose, kmParams.network)
Expand All @@ -53,14 +59,13 @@ case class BIP39KeyManager(
}
}

object BIP39KeyManager
extends KeyManagerCreateApi[BIP39KeyManager]
with BitcoinSLogger {
object BIP39KeyManager extends BIP39KeyManagerCreateApi with BitcoinSLogger {
val badPassphrase = AesPassword.fromString("changeMe").get

/** Initializes the mnemonic seed and saves it to file */
override def initializeWithEntropy(
entropy: BitVector,
bip39PasswordOpt: Option[String],
kmParams: KeyManagerParams): Either[
KeyManagerInitializeError,
BIP39KeyManager] = {
Expand Down Expand Up @@ -99,10 +104,17 @@ object BIP39KeyManager
logger.info(s"Saved encrypted wallet mnemonic to $mnemonicPath")
}

} yield BIP39KeyManager(mnemonic = mnemonic, kmParams = kmParams)
} yield {
BIP39KeyManager(mnemonic = mnemonic,
kmParams = kmParams,
bip39PasswordOpt = bip39PasswordOpt)
}

//verify we can unlock it for a sanity check
val unlocked = BIP39LockedKeyManager.unlock(badPassphrase, kmParams)
val unlocked = BIP39LockedKeyManager.unlock(passphrase = badPassphrase,
bip39PasswordOpt =
bip39PasswordOpt,
kmParams = kmParams)

val biasedFinalE: CompatEither[KeyManagerInitializeError, BIP39KeyManager] =
for {
Expand Down Expand Up @@ -133,13 +145,17 @@ object BIP39KeyManager
/** Reads the key manager from disk and decrypts it with the given password */
def fromParams(
kmParams: KeyManagerParams,
password: AesPassword): Either[ReadMnemonicError, BIP39KeyManager] = {
password: AesPassword,
bip39PasswordOpt: Option[String]): Either[
ReadMnemonicError,
BIP39KeyManager] = {
val mnemonicCodeE =
WalletStorage.decryptMnemonicFromDisk(kmParams.seedPath, password)

mnemonicCodeE match {
case Right(mnemonic) => Right(new BIP39KeyManager(mnemonic, kmParams))
case Left(v) => Left(v)
case Right(mnemonic) =>
Right(new BIP39KeyManager(mnemonic, kmParams, bip39PasswordOpt))
case Left(v) => Left(v)
}
}
}
Expand Up @@ -17,15 +17,19 @@ object BIP39LockedKeyManager extends BitcoinSLogger {
* @param kmParams parameters needed to create the key manager
*
* */
def unlock(passphrase: AesPassword, kmParams: KeyManagerParams): Either[
def unlock(
passphrase: AesPassword,
bip39PasswordOpt: Option[String],
kmParams: KeyManagerParams): Either[
KeyManagerUnlockError,
BIP39KeyManager] = {
logger.debug(s"Trying to unlock wallet with seedPath=${kmParams.seedPath}")
val resultE =
WalletStorage.decryptMnemonicFromDisk(kmParams.seedPath, passphrase)
resultE match {
case Right(mnemonicCode) =>
Right(new BIP39KeyManager(mnemonicCode, kmParams))
Right(BIP39KeyManager(mnemonicCode, kmParams, bip39PasswordOpt))

case Left(result) =>
result match {
case DecryptionError =>
Expand Down

0 comments on commit 039722a

Please sign in to comment.