From 56238e63fc5389daad209c1a9e4558815772f337 Mon Sep 17 00:00:00 2001 From: Jochen Hoenicke Date: Tue, 19 Apr 2016 18:23:12 +0200 Subject: [PATCH 1/2] Added curve type to HD node. Create a different root node for every curve type to separate the key space. --- firmware/crypto.c | 2 +- firmware/fsm.c | 44 +++++++++++---------------- firmware/storage.c | 72 +++++++++++++++++++++++--------------------- firmware/storage.h | 4 ++- vendor/trezor-crypto | 2 +- 5 files changed, 61 insertions(+), 63 deletions(-) diff --git a/firmware/crypto.c b/firmware/crypto.c index 84d611d..27e07fd 100644 --- a/firmware/crypto.c +++ b/firmware/crypto.c @@ -282,7 +282,7 @@ uint8_t *cryptoHDNodePathToPubkey(const HDNodePathType *hdnodepath) { if (!hdnodepath->node.has_public_key || hdnodepath->node.public_key.size != 33) return 0; static HDNode node; - if (hdnode_from_xpub(hdnodepath->node.depth, hdnodepath->node.fingerprint, hdnodepath->node.child_num, hdnodepath->node.chain_code.bytes, hdnodepath->node.public_key.bytes, &node) == 0) { + if (hdnode_from_xpub(hdnodepath->node.depth, hdnodepath->node.fingerprint, hdnodepath->node.child_num, hdnodepath->node.chain_code.bytes, hdnodepath->node.public_key.bytes, SECP256K1_NAME, &node) == 0) { return 0; } layoutProgressUpdate(true); diff --git a/firmware/fsm.c b/firmware/fsm.c index f505c3b..b3466c7 100644 --- a/firmware/fsm.c +++ b/firmware/fsm.c @@ -93,10 +93,10 @@ const CoinType *fsm_getCoin(const char *name) return coin; } -const HDNode *fsm_getDerivedNode(uint32_t *address_n, size_t address_n_count) +const HDNode *fsm_getDerivedNode(const char *curve, uint32_t *address_n, size_t address_n_count) { static HDNode node; - if (!storage_getRootNode(&node)) { + if (!storage_getRootNode(&node, curve)) { fsm_sendFailure(FailureType_Failure_NotInitialized, "Device not initialized or passphrase request cancelled"); layoutHome(); return 0; @@ -292,20 +292,16 @@ void fsm_msgGetPublicKey(GetPublicKey *msg) return; } - const HDNode *node = fsm_getDerivedNode(msg->address_n, msg->address_n_count); + const char *curve = SECP256K1_NAME; + if (msg->has_ecdsa_curve_name) { + curve = msg->ecdsa_curve_name; + } + const HDNode *node = fsm_getDerivedNode(curve, msg->address_n, msg->address_n_count); if (!node) return; uint8_t public_key[33]; // copy public key to temporary buffer memcpy(public_key, node->public_key, sizeof(public_key)); - if (msg->has_ecdsa_curve_name) { - const ecdsa_curve *curve = get_curve_by_name(msg->ecdsa_curve_name); - if (curve) { - // correct public key (since fsm_getDerivedNode uses secp256k1 curve) - ecdsa_get_public_key33(curve, node->private_key, public_key); - } - } - if (msg->has_show_display && msg->show_display) { layoutPublicKey(public_key); if (!protectButton(ButtonRequestType_ButtonRequest_PublicKey, true)) { @@ -401,7 +397,7 @@ void fsm_msgSignTx(SignTx *msg) const CoinType *coin = fsm_getCoin(msg->coin_name); if (!coin) return; - const HDNode *node = fsm_getDerivedNode(0, 0); + const HDNode *node = fsm_getDerivedNode(SECP256K1_NAME, 0, 0); if (!node) return; signing_init(msg->inputs_count, msg->outputs_count, coin, node, msg->version, msg->lock_time); @@ -445,7 +441,7 @@ void fsm_msgCipherKeyValue(CipherKeyValue *msg) layoutHome(); return; } - const HDNode *node = fsm_getDerivedNode(msg->address_n, msg->address_n_count); + const HDNode *node = fsm_getDerivedNode(SECP256K1_NAME, msg->address_n, msg->address_n_count); if (!node) return; bool encrypt = msg->has_encrypt && msg->encrypt; @@ -566,7 +562,7 @@ void fsm_msgGetAddress(GetAddress *msg) const CoinType *coin = fsm_getCoin(msg->coin_name); if (!coin) return; - const HDNode *node = fsm_getDerivedNode(msg->address_n, msg->address_n_count); + const HDNode *node = fsm_getDerivedNode(SECP256K1_NAME, msg->address_n, msg->address_n_count); if (!node) return; if (msg->has_multisig) { @@ -646,7 +642,7 @@ void fsm_msgSignMessage(SignMessage *msg) const CoinType *coin = fsm_getCoin(msg->coin_name); if (!coin) return; - const HDNode *node = fsm_getDerivedNode(msg->address_n, msg->address_n_count); + const HDNode *node = fsm_getDerivedNode(SECP256K1_NAME, msg->address_n, msg->address_n_count); if (!node) return; layoutProgressSwipe("Signing", 0); @@ -724,20 +720,16 @@ void fsm_msgSignIdentity(SignIdentity *msg) address_n[3] = 0x80000000 | hash[ 8] | (hash[ 9] << 8) | (hash[10] << 16) | (hash[11] << 24); address_n[4] = 0x80000000 | hash[12] | (hash[13] << 8) | (hash[14] << 16) | (hash[15] << 24); - const HDNode *node = fsm_getDerivedNode(address_n, 5); + const char *curve = SECP256K1_NAME; + if (msg->has_ecdsa_curve_name) { + curve = msg->ecdsa_curve_name; + } + const HDNode *node = fsm_getDerivedNode(curve, address_n, 5); if (!node) return; uint8_t public_key[33]; // copy public key to temporary buffer memcpy(public_key, node->public_key, sizeof(public_key)); - if (msg->has_ecdsa_curve_name) { - const ecdsa_curve *curve = get_curve_by_name(msg->ecdsa_curve_name); - if (curve) { - // correct public key (since fsm_getDerivedNode uses secp256k1 curve) - ecdsa_get_public_key33(curve, node->private_key, public_key); - } - } - bool sign_ssh = msg->identity.has_proto && (strcmp(msg->identity.proto, "ssh") == 0); int result = 0; @@ -807,7 +799,7 @@ void fsm_msgEncryptMessage(EncryptMessage *msg) layoutHome(); return; } - node = fsm_getDerivedNode(msg->address_n, msg->address_n_count); + node = fsm_getDerivedNode(SECP256K1_NAME, msg->address_n, msg->address_n_count); if (!node) return; uint8_t public_key[33]; ecdsa_get_public_key33(&secp256k1, node->private_key, public_key); @@ -859,7 +851,7 @@ void fsm_msgDecryptMessage(DecryptMessage *msg) layoutHome(); return; } - const HDNode *node = fsm_getDerivedNode(msg->address_n, msg->address_n_count); + const HDNode *node = fsm_getDerivedNode(SECP256K1_NAME, msg->address_n, msg->address_n_count); if (!node) return; layoutProgressSwipe("Decrypting", 0); diff --git a/firmware/storage.c b/firmware/storage.c index 047f110..a2c4ce2 100644 --- a/firmware/storage.c +++ b/firmware/storage.c @@ -45,8 +45,8 @@ Storage storage; uint8_t storage_uuid[12]; char storage_uuid_str[25]; -static bool sessionRootNodeCached; -static HDNode sessionRootNode; +static bool sessionSeedCached; +static uint8_t sessionSeed[64]; static bool sessionPinCached; @@ -126,8 +126,10 @@ void storage_reset(void) void session_clear(bool clear_pin) { - sessionRootNodeCached = false; memset(&sessionRootNode, 0, sizeof(sessionRootNode)); - sessionPassphraseCached = false; memset(&sessionPassphrase, 0, sizeof(sessionPassphrase)); + sessionSeedCached = false; + memset(&sessionSeed, 0, sizeof(sessionSeed)); + sessionPassphraseCached = false; + memset(&sessionPassphrase, 0, sizeof(sessionPassphrase)); if (clear_pin) { sessionPinCached = false; } @@ -186,14 +188,14 @@ void storage_loadDevice(LoadDevice *msg) storage.has_node = true; storage.has_mnemonic = false; memcpy(&storage.node, &(msg->node), sizeof(HDNodeType)); - sessionRootNodeCached = false; - memset(&sessionRootNode, 0, sizeof(sessionRootNode)); + sessionSeedCached = false; + memset(&sessionSeed, 0, sizeof(sessionSeed)); } else if (msg->has_mnemonic) { storage.has_mnemonic = true; storage.has_node = false; strlcpy(storage.mnemonic, msg->mnemonic, sizeof(storage.mnemonic)); - sessionRootNodeCached = false; - memset(&sessionRootNode, 0, sizeof(sessionRootNode)); + sessionSeedCached = false; + memset(&sessionSeed, 0, sizeof(sessionSeed)); } if (msg->has_language) { @@ -224,7 +226,7 @@ void storage_setLanguage(const char *lang) void storage_setPassphraseProtection(bool passphrase_protection) { - sessionRootNodeCached = false; + sessionSeedCached = false; sessionPassphraseCached = false; storage.has_passphrase_protection = true; @@ -249,20 +251,34 @@ void get_root_node_callback(uint32_t iter, uint32_t total) layoutProgress("Waking up", 1000 * iter / total); } -bool storage_getRootNode(HDNode *node) +const uint8_t *storage_getSeed(void) { // root node is properly cached - if (sessionRootNodeCached) { - memcpy(node, &sessionRootNode, sizeof(HDNode)); - return true; + if (sessionSeedCached) { + return sessionSeed; } + // if storage has mnemonic, convert it to node and use it + if (storage.has_mnemonic) { + if (!protectPassphrase()) { + return NULL; + } + mnemonic_to_seed(storage.mnemonic, sessionPassphrase, sessionSeed, get_root_node_callback); // BIP-0039 + sessionSeedCached = true; + return sessionSeed; + } + + return NULL; +} + +bool storage_getRootNode(HDNode *node, const char *curve) +{ // if storage has node, decrypt and use it - if (storage.has_node) { + if (storage.has_node && strcmp(curve, "secp256k1") == 0) { if (!protectPassphrase()) { return false; } - if (hdnode_from_xprv(storage.node.depth, storage.node.fingerprint, storage.node.child_num, storage.node.chain_code.bytes, storage.node.private_key.bytes, &sessionRootNode) == 0) { + if (hdnode_from_xprv(storage.node.depth, storage.node.fingerprint, storage.node.child_num, storage.node.chain_code.bytes, storage.node.private_key.bytes, curve, node) == 0) { return false; } if (storage.has_passphrase_protection && storage.passphrase_protection && sessionPassphraseCached && strlen(sessionPassphrase) > 0) { @@ -273,30 +289,18 @@ bool storage_getRootNode(HDNode *node) pbkdf2_hmac_sha512((const uint8_t *)sessionPassphrase, strlen(sessionPassphrase), salt, 8, BIP39_PBKDF2_ROUNDS, secret, 64, get_root_node_callback); aes_decrypt_ctx ctx; aes_decrypt_key256(secret, &ctx); - aes_cbc_decrypt(sessionRootNode.chain_code, sessionRootNode.chain_code, 32, secret + 32, &ctx); - aes_cbc_decrypt(sessionRootNode.private_key, sessionRootNode.private_key, 32, secret + 32, &ctx); + aes_cbc_decrypt(node->chain_code, node->chain_code, 32, secret + 32, &ctx); + aes_cbc_decrypt(node->private_key, node->private_key, 32, secret + 32, &ctx); } - memcpy(node, &sessionRootNode, sizeof(HDNode)); - sessionRootNodeCached = true; return true; } - // if storage has mnemonic, convert it to node and use it - if (storage.has_mnemonic) { - if (!protectPassphrase()) { - return false; - } - uint8_t seed[64]; - mnemonic_to_seed(storage.mnemonic, sessionPassphrase, seed, get_root_node_callback); // BIP-0039 - if (hdnode_from_seed(seed, sizeof(seed), &sessionRootNode) == 0) { - return false; - } - memcpy(node, &sessionRootNode, sizeof(HDNode)); - sessionRootNodeCached = true; - return true; + const uint8_t *seed = storage_getSeed(); + if (seed == NULL) { + return false; } - - return false; + + return hdnode_from_seed(seed, 64, curve, node); } const char *storage_getLabel(void) diff --git a/firmware/storage.h b/firmware/storage.h index b1041a8..fb7b6b7 100644 --- a/firmware/storage.h +++ b/firmware/storage.h @@ -33,7 +33,9 @@ void session_clear(bool clear_pin); void storage_loadDevice(LoadDevice *msg); -bool storage_getRootNode(HDNode *node); +const uint8_t *storage_getSeed(void); + +bool storage_getRootNode(HDNode *node, const char *curve); const char *storage_getLabel(void); void storage_setLabel(const char *label); diff --git a/vendor/trezor-crypto b/vendor/trezor-crypto index f4dd151..c983afd 160000 --- a/vendor/trezor-crypto +++ b/vendor/trezor-crypto @@ -1 +1 @@ -Subproject commit f4dd151eb9ef989b88dc79218a2b0115934e4268 +Subproject commit c983afd72f40a8c65355af076afd9c132878e8a5 From 03c501d9e3633248bf3423a29827de398e656ac0 Mon Sep 17 00:00:00 2001 From: Jochen Hoenicke Date: Wed, 20 Apr 2016 15:39:15 +0200 Subject: [PATCH 2/2] Do not use hardcoded string for secp256k1. --- firmware/storage.c | 3 ++- vendor/trezor-crypto | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/firmware/storage.c b/firmware/storage.c index a2c4ce2..19e70dd 100644 --- a/firmware/storage.c +++ b/firmware/storage.c @@ -31,6 +31,7 @@ #include "pbkdf2.h" #include "bip32.h" #include "bip39.h" +#include "secp256k1.h" #include "util.h" #include "memory.h" #include "rng.h" @@ -274,7 +275,7 @@ const uint8_t *storage_getSeed(void) bool storage_getRootNode(HDNode *node, const char *curve) { // if storage has node, decrypt and use it - if (storage.has_node && strcmp(curve, "secp256k1") == 0) { + if (storage.has_node && strcmp(curve, SECP256K1_NAME) == 0) { if (!protectPassphrase()) { return false; } diff --git a/vendor/trezor-crypto b/vendor/trezor-crypto index c983afd..d577410 160000 --- a/vendor/trezor-crypto +++ b/vendor/trezor-crypto @@ -1 +1 @@ -Subproject commit c983afd72f40a8c65355af076afd9c132878e8a5 +Subproject commit d577410fc4a87262c107fb25657158f8f8ba720e