From 024d29a461211e5f460824ffcd6c118474a84d24 Mon Sep 17 00:00:00 2001 From: keepkeyjon Date: Sun, 10 Mar 2019 19:35:30 -0600 Subject: [PATCH 01/29] firmware: Ripple support --- deps/device-protocol | 2 +- deps/python-keepkey | 2 +- fuzzer/firmware/CMakeLists.txt | 3 + fuzzer/firmware/ripple_decode.cpp | 36 +++ include/keepkey/firmware/coins.def | 1 + include/keepkey/firmware/fsm.h | 8 + include/keepkey/firmware/ripple.h | 95 ++++++ include/keepkey/firmware/ripple_base58.h | 47 +++ include/keepkey/transport/interface.h | 1 + .../keepkey/transport/messages-ripple.options | 10 + lib/firmware/CMakeLists.txt | 2 + lib/firmware/fsm.c | 3 + lib/firmware/fsm_msg_ripple.h | 119 ++++++++ lib/firmware/messagemap.def | 6 + lib/firmware/ripple.c | 287 ++++++++++++++++++ lib/firmware/ripple_base58.c | 224 ++++++++++++++ lib/transport/CMakeLists.txt | 9 + unittests/firmware/CMakeLists.txt | 1 + unittests/firmware/ripple.cpp | 124 ++++++++ 19 files changed, 978 insertions(+), 2 deletions(-) create mode 100644 fuzzer/firmware/ripple_decode.cpp create mode 100644 include/keepkey/firmware/ripple.h create mode 100644 include/keepkey/firmware/ripple_base58.h create mode 100644 include/keepkey/transport/messages-ripple.options create mode 100644 lib/firmware/fsm_msg_ripple.h create mode 100644 lib/firmware/ripple.c create mode 100644 lib/firmware/ripple_base58.c create mode 100644 unittests/firmware/ripple.cpp diff --git a/deps/device-protocol b/deps/device-protocol index 4b8bfdcd3..df02dfd83 160000 --- a/deps/device-protocol +++ b/deps/device-protocol @@ -1 +1 @@ -Subproject commit 4b8bfdcd37638e0ba6bf6b82ddefd7c595412241 +Subproject commit df02dfd839f4eaf990dc37f529b0335262b43899 diff --git a/deps/python-keepkey b/deps/python-keepkey index 4b2cc4e7a..4b4254ec3 160000 --- a/deps/python-keepkey +++ b/deps/python-keepkey @@ -1 +1 @@ -Subproject commit 4b2cc4e7a0c95e30df5efdbd05a789549986f6b7 +Subproject commit 4b4254ec35e3f468effc4379666707b34438c795 diff --git a/fuzzer/firmware/CMakeLists.txt b/fuzzer/firmware/CMakeLists.txt index 80bbd4e72..5d8b7f1b5 100644 --- a/fuzzer/firmware/CMakeLists.txt +++ b/fuzzer/firmware/CMakeLists.txt @@ -23,3 +23,6 @@ target_link_libraries(fuzz-eos_formatName ${libraries}) add_executable(fuzz-eos_formatAsset eos_formatAsset.cpp) target_link_libraries(fuzz-eos_formatAsset ${libraries}) + +add_executable(fuzz-ripple_decode ripple_decode.cpp) +target_link_libraries(fuzz-ripple_decode ${libraries}) diff --git a/fuzzer/firmware/ripple_decode.cpp b/fuzzer/firmware/ripple_decode.cpp new file mode 100644 index 000000000..d673c2b02 --- /dev/null +++ b/fuzzer/firmware/ripple_decode.cpp @@ -0,0 +1,36 @@ +extern "C" { +#include "keepkey/firmware/coins.h" +#include "keepkey/firmware/ripple.h" +#include "keepkey/firmware/ripple_base58.h" +#include "trezor/crypto/hasher.h" +} + +extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { + if (size != 33) + return 0; + + uint8_t buff[64]; + memset(buff, 0, sizeof(buff)); + + Hasher hasher; + hasher_Init(&hasher, HASHER_SHA2_RIPEMD); + hasher_Update(&hasher, data, 33); + hasher_Final(&hasher, buff + 1); + + char address[56]; + if (!ripple_encode_check(buff, 21, HASHER_SHA2D, + address, MAX_ADDR_SIZE)) + return 1; + + uint8_t addr_raw[MAX_ADDR_RAW_SIZE]; + memset(addr_raw, 0, sizeof(addr_raw)); + uint32_t addr_raw_len = ripple_decode_check(address, HASHER_SHA2D, + addr_raw, MAX_ADDR_RAW_SIZE); + if (addr_raw_len != 21) + return 2; + + if (memcmp(buff, addr_raw, 21) != 0) + return 3; + + return 0; +} diff --git a/include/keepkey/firmware/coins.def b/include/keepkey/firmware/coins.def index c9dae87bb..993a85fc4 100644 --- a/include/keepkey/firmware/coins.def +++ b/include/keepkey/firmware/coins.def @@ -38,5 +38,6 @@ X(true, "BCH Testnet", true, "tBCH", true, 0, true, 500000, true, X(true, "LTC Testnet", true, "tLTC", true, 48, true, 1000000, true, 50, true, "Litecoin Signed Message:\n", true, 0x80000001, false, 0, true, 8, false, NO_CONTRACT, true, 27108450, true, true, true, false, true, SECP256K1_STRING, false, "", true, "ltc", false, false, false, 0, false, 0, false, "" ) X(true, ETHEREUM_TST, true, "tETH", true, NA, true, 100000, true, NA, true, "Ethereum Signed Message:\n", true, 0x80000001, true, 1, true, 18, false, NO_CONTRACT, false, 0, true, false, true, false, true, SECP256K1_STRING, false, "", false, "", false, false, false, 0, false, 0, false, "" ) X(true, "Cosmos", true, "ATOM", false, NA, false, NA, false, NA, false, {0}, true, 0x80000076, false, 0, true, 6, false, NO_CONTRACT, false, 0, false, false, false, false, true, SECP256K1_STRING, false, "", false, "cosmos", false, false, false, 0, false, 0, false, "" ) +X(true, "Ripple", true, "Ripple",false, 0, false, 0, false, 0, false, {0}, true, 0x80000090, false, 0, false, 0, false, NO_CONTRACT, false, 0, false, false, false, false, true, SECP256K1_STRING, false, "", false, "", false, false, true, 77429938, true, 78792518, false, "" ) #undef X #undef NO_CONTRACT diff --git a/include/keepkey/firmware/fsm.h b/include/keepkey/firmware/fsm.h index c331afec2..cebb3aa6f 100644 --- a/include/keepkey/firmware/fsm.h +++ b/include/keepkey/firmware/fsm.h @@ -78,16 +78,24 @@ void fsm_msgDecryptMessage(DecryptMessage *msg); //void fsm_msgPassphraseAck(PassphraseAck *msg); void fsm_msgRecoveryDevice(RecoveryDevice *msg); void fsm_msgWordAck(WordAck *msg); + void fsm_msgEthereumGetAddress(EthereumGetAddress *msg); void fsm_msgEthereumSignTx(EthereumSignTx *msg); void fsm_msgEthereumTxAck(EthereumTxAck *msg); void fsm_msgEthereumSignMessage(EthereumSignMessage *msg); void fsm_msgEthereumVerifyMessage(const EthereumVerifyMessage *msg); + void fsm_msgCharacterAck(CharacterAck *msg); void fsm_msgApplyPolicies(ApplyPolicies *msg); + void fsm_msgNanoGetAddress(NanoGetAddress *msg); void fsm_msgNanoSignTx(NanoSignTx *msg); +/// Modifies the RippleSignTx, setting the flag to indicate +/// that the ECDSA sig is canonical. +void fsm_msgRippleSignTx(RippleSignTx *msg); +void fsm_msgRippleGetAddress(const RippleGetAddress *msg); + void fsm_msgEosGetPublicKey(const EosGetPublicKey *msg); void fsm_msgEosSignTx(const EosSignTx *msg); void fsm_msgEosTxActionAck(const EosTxActionAck *msg); diff --git a/include/keepkey/firmware/ripple.h b/include/keepkey/firmware/ripple.h new file mode 100644 index 000000000..0696d32ce --- /dev/null +++ b/include/keepkey/firmware/ripple.h @@ -0,0 +1,95 @@ +/* + * This file is part of the KeepKey project. + * + * Copyright (C) 2019 ShapeShift + * + * This library is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this library. If not, see . + */ + +#ifndef KEEPKEY_FIRMWARE_RIPPLE_H +#define KEEPKEY_FIRMWARE_RIPPLE_H + +#include "trezor/crypto/bip32.h" + +#include "messages-ripple.pb.h" + +#define RIPPLE_MIN_FEE 10 +#define RIPPLE_MAX_FEE 1000000 + +#define RIPPLE_DECIMALS 6 + +#define RIPPLE_FLAG_FULLY_CANONICAL 0x80000000 + +typedef enum { + RFT_INT16 = 1, + RFT_INT32 = 2, + RFT_AMOUNT = 6, + RFT_VL = 7, + RFT_ACCOUNT = 8, +} RippleFieldType; + +typedef struct _RippleFieldMapping { + RippleFieldType type; + int key; +} RippleFieldMapping; + +extern const RippleFieldMapping RFM_account; +extern const RippleFieldMapping RFM_amount; +extern const RippleFieldMapping RFM_destination; +extern const RippleFieldMapping RFM_fee; +extern const RippleFieldMapping RFM_sequence; +extern const RippleFieldMapping RFM_type; +extern const RippleFieldMapping RFM_signingPubKey; +extern const RippleFieldMapping RFM_flags; +extern const RippleFieldMapping RFM_txnSignature; +extern const RippleFieldMapping RFM_lastLedgerSequence; +extern const RippleFieldMapping RFM_destinationTag; + +bool ripple_getAddress(const uint8_t public_key[33], char address[MAX_ADDR_SIZE]); + +void ripple_formatAmount(char *buf, size_t len, uint64_t amount); + +void ripple_serializeType(bool *ok, uint8_t **buf, const uint8_t *end, + const RippleFieldMapping *m); + +void ripple_serializeInt16(bool *ok, uint8_t **buf, const uint8_t *end, + const RippleFieldMapping *m, int16_t val); + +void ripple_serializeInt32(bool *ok, uint8_t **buf, const uint8_t *end, + const RippleFieldMapping *m, int32_t val); + +void ripple_serializeAmount(bool *ok, uint8_t **buf, const uint8_t *end, + const RippleFieldMapping *m, int64_t amount); + +void ripple_serializeVarint(bool *ok, uint8_t **buf, const uint8_t *end, int val); + +void ripple_serializeBytes(bool *ok, uint8_t **buf, const uint8_t *end, + const uint8_t *bytes, size_t count); + +void ripple_serializeAddress(bool *ok, uint8_t **buf, const uint8_t *end, + const RippleFieldMapping *m, const char *address); + +void ripple_serializeVL(bool *ok, uint8_t **buf, const uint8_t *end, + const RippleFieldMapping *m, + const uint8_t *bytes, size_t count); + +bool ripple_serialize(uint8_t **buf, const uint8_t *end, const RippleSignTx *tx, + const char *source_address, + const uint8_t *pubkey, const uint8_t *sig, + size_t sig_len); + +void ripple_signTx(const HDNode *node, RippleSignTx *tx, + RippleSignedTx *resp); + +#endif diff --git a/include/keepkey/firmware/ripple_base58.h b/include/keepkey/firmware/ripple_base58.h new file mode 100644 index 000000000..98c9e83fe --- /dev/null +++ b/include/keepkey/firmware/ripple_base58.h @@ -0,0 +1,47 @@ +/** + * Copyright (c) 2013-2014 Tomas Dzetkulic + * Copyright (c) 2013-2014 Pavol Rusnak + * + * Permission is hereby granted, free of charge, to any person obtaining + * a copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included + * in all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS + * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES + * OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR + * OTHER DEALINGS IN THE SOFTWARE. + */ + +#ifndef KEEPKEY_FIRMWARE_RIPPLE_BASE58_H +#define KEEPKEY_FIRMWARE_RIPPLE_BASE58_H + +#include "trezor/crypto/hasher.h" +#include "trezor/crypto/options.h" + +#include +#include + +extern const char ripple_b58digits_ordered[]; +extern const int8_t ripple_b58digits_map[]; + +int ripple_encode_check(const uint8_t *data, int len, HasherType hasher_type, + char *str, int strsize); +int ripple_decode_check(const char *str, HasherType hasher_type, uint8_t *data, + int datalen); + +// Private +bool ripple_b58tobin(void *bin, size_t *binszp, const char *b58); +int ripple_b58check(const void *bin, size_t binsz, HasherType hasher_type, + const char *base58str); +bool ripple_b58enc(char *b58, size_t *b58sz, const void *data, size_t binsz); + +#endif diff --git a/include/keepkey/transport/interface.h b/include/keepkey/transport/interface.h index a0966729a..e09eb2ae3 100644 --- a/include/keepkey/transport/interface.h +++ b/include/keepkey/transport/interface.h @@ -28,6 +28,7 @@ #include "messages-eos.pb.h" #include "messages-cosmos.pb.h" +#include "messages-ripple.pb.h" #include "types.pb.h" #include "trezor_transport.h" diff --git a/include/keepkey/transport/messages-ripple.options b/include/keepkey/transport/messages-ripple.options new file mode 100644 index 000000000..b3f2e1987 --- /dev/null +++ b/include/keepkey/transport/messages-ripple.options @@ -0,0 +1,10 @@ +RippleGetAddress.address_n max_count:8 + +RippleAddress.address max_size:36 + +RippleSignTx.address_n max_count:8 + +RipplePayment.destination max_size:36 + +RippleSignedTx.signature max_size:75 +RippleSignedTx.serialized_tx max_size:1024 diff --git a/lib/firmware/CMakeLists.txt b/lib/firmware/CMakeLists.txt index d8d021175..b6d09ca22 100644 --- a/lib/firmware/CMakeLists.txt +++ b/lib/firmware/CMakeLists.txt @@ -20,6 +20,8 @@ set(sources policy.c recovery_cipher.c reset.c + ripple.c + ripple_base58.c signing.c storage.c transaction.c diff --git a/lib/firmware/fsm.c b/lib/firmware/fsm.c index 141f6fe63..dd1711a6a 100644 --- a/lib/firmware/fsm.c +++ b/lib/firmware/fsm.c @@ -49,6 +49,7 @@ #include "keepkey/firmware/policy.h" #include "keepkey/firmware/recovery_cipher.h" #include "keepkey/firmware/reset.h" +#include "keepkey/firmware/ripple.h" #include "keepkey/firmware/signing.h" #include "keepkey/firmware/storage.h" #include "keepkey/firmware/transaction.h" @@ -70,6 +71,7 @@ #include "messages-cosmos.pb.h" #include "messages-eos.pb.h" #include "messages-nano.pb.h" +#include "messages-ripple.pb.h" #include @@ -296,3 +298,4 @@ void fsm_msgClearSession(ClearSession *msg) #include "fsm_msg_debug.h" #include "fsm_msg_eos.h" #include "fsm_msg_cosmos.h" +#include "fsm_msg_ripple.h" diff --git a/lib/firmware/fsm_msg_ripple.h b/lib/firmware/fsm_msg_ripple.h new file mode 100644 index 000000000..3a94f0e40 --- /dev/null +++ b/lib/firmware/fsm_msg_ripple.h @@ -0,0 +1,119 @@ +/* + * This file is part of the KeepKey project. + * + * Copyright (C) 2019 ShapeShift + * + * This library is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this library. If not, see . + */ + +void fsm_msgRippleGetAddress(const RippleGetAddress *msg) +{ + RESP_INIT(RippleAddress); + + CHECK_INITIALIZED + + CHECK_PIN + + HDNode *node = fsm_getDerivedNode(SECP256K1_NAME, msg->address_n, msg->address_n_count, NULL); + if (!node) return; + hdnode_fill_public_key(node); + + const CoinType *coin = fsm_getCoin(true, "Ripple"); + + if (!ripple_getAddress(node->public_key, resp->address)) { + fsm_sendFailure(FailureType_Failure_Other, _("Address derivation failed")); + layoutHome(); + return; + } + + resp->has_address = true; + + if (msg->has_show_display && msg->show_display) { + char node_str[NODE_STRING_LENGTH]; + if (!(bip32_node_to_string(node_str, sizeof(node_str), coin, + msg->address_n, + msg->address_n_count, + /*whole_account=*/false, + /*show_addridx=*/false)) && + !bip32_path_to_string(node_str, sizeof(node_str), + msg->address_n, msg->address_n_count)) { + memset(node_str, 0, sizeof(node_str)); + } + + if (!confirm_ethereum_address(node_str, resp->address)) { + fsm_sendFailure(FailureType_Failure_ActionCancelled, _("Show address cancelled")); + layoutHome(); + return; + } + } + + msg_write(MessageType_MessageType_RippleAddress, resp); + layoutHome(); +} + +void fsm_msgRippleSignTx(RippleSignTx *msg) +{ + RESP_INIT(RippleSignedTx); + + CHECK_INITIALIZED + + CHECK_PIN + + bool needs_confirm = true; + + // TODO: handle trades and transfers + + HDNode *node = fsm_getDerivedNode(SECP256K1_NAME, msg->address_n, + msg->address_n_count, NULL); + if (!node) return; + hdnode_fill_public_key(node); + + if (!msg->has_fee || msg->fee < RIPPLE_MIN_FEE || msg->fee > RIPPLE_MAX_FEE) { + fsm_sendFailure(FailureType_Failure_SyntaxError, + _("Fee must be between 10 and 1,000,000 drops")); + return; + } + + char amount_string[20 + 4 + 1]; + ripple_formatAmount(amount_string, sizeof(amount_string), msg->payment.amount); + + char fee_string[20 + 4 + 1]; + ripple_formatAmount(fee_string, sizeof(fee_string), msg->fee); + + if (needs_confirm) { + if (!confirm(ButtonRequestType_ButtonRequest_ConfirmOutput, + "Send", msg->payment.has_destination_tag + ? "Send %s to %s, with destination tag %" PRIu32 "?" + : "Send %s to %s?", + amount_string, + msg->payment.destination, + msg->payment.destination_tag)) { + fsm_sendFailure(FailureType_Failure_ActionCancelled, "Signing cancelled"); + layoutHome(); + return; + } + } + + if (!confirm(ButtonRequestType_ButtonRequest_SignTx, + "Transaction", "Really send %s, with a transaction fee of %s?", + amount_string, fee_string)) { + fsm_sendFailure(FailureType_Failure_ActionCancelled, "Signing cancelled"); + layoutHome(); + return; + } + + ripple_signTx(node, msg, resp); + msg_write(MessageType_MessageType_RippleSignedTx, resp); + layoutHome(); +} diff --git a/lib/firmware/messagemap.def b/lib/firmware/messagemap.def index 98ae5391b..9ccd67961 100644 --- a/lib/firmware/messagemap.def +++ b/lib/firmware/messagemap.def @@ -48,6 +48,9 @@ MSG_IN(MessageType_MessageType_EosSignTx, EosSignTx, fsm_msgEosSignTx) MSG_IN(MessageType_MessageType_EosTxActionAck, EosTxActionAck, fsm_msgEosTxActionAck) + MSG_IN(MessageType_MessageType_RippleGetAddress, RippleGetAddress, fsm_msgRippleGetAddress) + MSG_IN(MessageType_MessageType_RippleSignTx, RippleSignTx, fsm_msgRippleSignTx) + /* Normal Out Messages */ MSG_OUT(MessageType_MessageType_Success, Success, NO_PROCESS_FUNC) MSG_OUT(MessageType_MessageType_Failure, Failure, NO_PROCESS_FUNC) @@ -84,6 +87,9 @@ MSG_OUT(MessageType_MessageType_EosTxActionRequest, EosTxActionRequest, NO_PROCESS_FUNC) MSG_OUT(MessageType_MessageType_EosSignedTx, EosSignedTx, NO_PROCESS_FUNC) + MSG_OUT(MessageType_MessageType_RippleAddress, RippleAddress, NO_PROCESS_FUNC) + MSG_OUT(MessageType_MessageType_RippleSignedTx, RippleSignedTx, NO_PROCESS_FUNC) + #if DEBUG_LINK /* Debug Messages */ DEBUG_IN(MessageType_MessageType_DebugLinkDecision, DebugLinkDecision, NO_PROCESS_FUNC) diff --git a/lib/firmware/ripple.c b/lib/firmware/ripple.c new file mode 100644 index 000000000..bf1ad0d12 --- /dev/null +++ b/lib/firmware/ripple.c @@ -0,0 +1,287 @@ +/* + * This file is part of the KeepKey project. + * + * Copyright (C) 2019 ShapeShift + * + * This library is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this library. If not, see . + */ + +#include "keepkey/firmware/ripple.h" + +#include "keepkey/firmware/ripple_base58.h" +#include "trezor/crypto/base58.h" +#include "trezor/crypto/secp256k1.h" + +#include + +const RippleFieldMapping RFM_account = { .type = RFT_ACCOUNT, .key = 1 }; +const RippleFieldMapping RFM_amount = { .type = RFT_AMOUNT, .key = 1 }; +const RippleFieldMapping RFM_destination = { .type = RFT_ACCOUNT, .key = 3 }; +const RippleFieldMapping RFM_fee = { .type = RFT_AMOUNT, .key = 8 }; +const RippleFieldMapping RFM_sequence = { .type = RFT_INT32, .key = 4 }; +const RippleFieldMapping RFM_type = { .type = RFT_INT16, .key = 2 }; +const RippleFieldMapping RFM_signingPubKey = { .type = RFT_VL, .key = 3 }; +const RippleFieldMapping RFM_flags = { .type = RFT_INT32, .key = 2 }; +const RippleFieldMapping RFM_txnSignature = { .type = RFT_VL, .key = 4 }; +const RippleFieldMapping RFM_lastLedgerSequence = { .type = RFT_INT32, .key = 27 }; +const RippleFieldMapping RFM_destinationTag = { .type = RFT_INT32, .key = 14 }; + +bool ripple_getAddress(const uint8_t public_key[33], char address[MAX_ADDR_SIZE]) +{ + uint8_t buff[64]; + memset(buff, 0, sizeof(buff)); + + Hasher hasher; + hasher_Init(&hasher, HASHER_SHA2_RIPEMD); + hasher_Update(&hasher, public_key, 33); + hasher_Final(&hasher, buff + 1); + + if (!ripple_encode_check(buff, 21, HASHER_SHA2D, + address, MAX_ADDR_SIZE)) { + assert(false && "can't encode address"); + return false; + } + + return true; +} + +void ripple_formatAmount(char *buf, size_t len, uint64_t amount) +{ + bignum256 val; + bn_read_uint64(amount, &val); + bn_format(&val, NULL, " XRP", RIPPLE_DECIMALS, 0, false, buf, len); +} + +static void append_u8(bool *ok, uint8_t **buf, const uint8_t *end, uint8_t val) +{ + if (!*ok) { + return; + } + + if (*buf + 1 > end) { + *ok = false; + return; + } + + **buf = val; + *buf += 1; +} + +void ripple_serializeType(bool *ok, uint8_t **buf, const uint8_t *end, const RippleFieldMapping *m) +{ + if (m->key <= 0xf) { + append_u8(ok, buf, end, m->type << 4 | m->key); + return; + } + + append_u8(ok, buf, end, m->type << 4); + append_u8(ok, buf, end, m->key); +} + +void ripple_serializeInt16(bool *ok, uint8_t **buf, const uint8_t *end, + const RippleFieldMapping *m, int16_t val) +{ + assert(m->type == FT_INT16 && "wrong type?"); + + ripple_serializeType(ok, buf, end, m); + append_u8(ok, buf, end, (val >> 8) & 0xff); + append_u8(ok, buf, end, val & 0xff); +} + +void ripple_serializeInt32(bool *ok, uint8_t **buf, const uint8_t *end, + const RippleFieldMapping *m, int32_t val) +{ + assert(m->type == FT_INT32 && "wrong type?"); + + ripple_serializeType(ok, buf, end, m); + append_u8(ok, buf, end, (val >> 24) & 0xff); + append_u8(ok, buf, end, (val >> 16) & 0xff); + append_u8(ok, buf, end, (val >> 8) & 0xff); + append_u8(ok, buf, end, val & 0xff); +} + +void ripple_serializeAmount(bool *ok, uint8_t **buf, const uint8_t *end, + const RippleFieldMapping *m, int64_t amount) +{ + ripple_serializeType(ok, buf, end, m); + + assert(amount >= 0 && "amounts cannot be negative"); + assert(amount <= 100000000000 && "larger amounts not supported"); + uint8_t msb = (amount >> (7 * 8)) & 0xff; + msb &= 0x7f; // Clear first bit, indicating XRP + msb |= 0x40; // Clear second bit, indicating value is positive + + append_u8(ok, buf, end, msb); + append_u8(ok, buf, end, (amount >> (6 * 8)) & 0xff); + append_u8(ok, buf, end, (amount >> (5 * 8)) & 0xff); + append_u8(ok, buf, end, (amount >> (4 * 8)) & 0xff); + append_u8(ok, buf, end, (amount >> (3 * 8)) & 0xff); + append_u8(ok, buf, end, (amount >> (2 * 8)) & 0xff); + append_u8(ok, buf, end, (amount >> (1 * 8)) & 0xff); + append_u8(ok, buf, end, amount & 0xff); +} + +void ripple_serializeVarint(bool *ok, uint8_t **buf, const uint8_t *end, int val) +{ + if (val < 0) { + assert(false && "can't serialize 0-valued varint"); + *ok = false; + return; + } + + if (val < 192) { + append_u8(ok, buf, end, val); + return; + } + + if (val <= 12480) { + val -= 193; + append_u8(ok, buf, end, 193 + (val >> 8)); + append_u8(ok, buf, end, val & 0xff); + return; + } + + if (val < 918744) { + assert(*buf + 3 < end && "buffer not long enough"); + val -= 12481; + append_u8(ok, buf, end, 241 + (val >> 16)); + append_u8(ok, buf, end, (val >> 8) & 0xff); + append_u8(ok, buf, end, val & 0xff); + return; + } + + assert(false && "value too large"); + *ok = false; +} + +void ripple_serializeBytes(bool *ok, uint8_t **buf, const uint8_t *end, + const uint8_t *bytes, size_t count) +{ + ripple_serializeVarint(ok, buf, end, count); + + if (!*ok || *buf + count > end) { + *ok = false; + assert(false && "buffer not long enough"); + return; + } + + memcpy(*buf, bytes, count); + *buf += count; +} + +void ripple_serializeAddress(bool *ok, uint8_t **buf, const uint8_t *end, + const RippleFieldMapping *m, const char *address) +{ + ripple_serializeType(ok, buf, end, m); + + uint8_t addr_raw[MAX_ADDR_RAW_SIZE]; + uint32_t addr_raw_len = ripple_decode_check(address, HASHER_SHA2D, + addr_raw, MAX_ADDR_RAW_SIZE); + if (addr_raw_len != 21) { + assert(false && "address has wrong length?"); + *ok = false; + return; + } + + ripple_serializeBytes(ok, buf, end, addr_raw + 1, addr_raw_len - 1); +} + +void ripple_serializeVL(bool *ok, uint8_t **buf, const uint8_t *end, const RippleFieldMapping *m, + const uint8_t *bytes, size_t count) +{ + ripple_serializeType(ok, buf, end, m); + ripple_serializeBytes(ok, buf, end, bytes, count); +} + +bool ripple_serialize(uint8_t **buf, const uint8_t *end, const RippleSignTx *tx, + const char *source_address, + const uint8_t *pubkey, const uint8_t *sig, size_t sig_len) +{ + bool ok = true; + ripple_serializeInt16(&ok, buf, end, &RFM_type, /*Payment*/0); + if (tx->has_flags) + ripple_serializeInt32(&ok, buf, end, &RFM_flags, tx->flags); + if (tx->has_sequence) + ripple_serializeInt32(&ok, buf, end, &RFM_sequence, tx->sequence); + if (tx->payment.has_destination_tag) + ripple_serializeInt32(&ok, buf, end, &RFM_destinationTag, tx->payment.destination_tag); + if (tx->has_last_ledger_sequence) + ripple_serializeInt32(&ok, buf, end, &RFM_lastLedgerSequence, tx->last_ledger_sequence); + if (tx->payment.has_amount) + ripple_serializeAmount(&ok, buf, end, &RFM_amount, tx->payment.amount); + if (tx->has_fee) + ripple_serializeAmount(&ok, buf, end, &RFM_fee, tx->fee); + if (pubkey) + ripple_serializeVL(&ok, buf, end, &RFM_signingPubKey, pubkey, 33); + if (sig) + ripple_serializeVL(&ok, buf, end, &RFM_txnSignature, sig, sig_len); + if (source_address) + ripple_serializeAddress(&ok, buf, end, &RFM_account, source_address); + if (tx->payment.has_destination) + ripple_serializeAddress(&ok, buf, end, &RFM_destination, tx->payment.destination); + return ok; +} + +void ripple_signTx(const HDNode *node, RippleSignTx *tx, + RippleSignedTx *resp) { + const curve_info *curve = get_curve_by_name("secp256k1"); + if (!curve) return; + + // Set canonical flag, since trezor-crypto ECDSA implementation returns + // fully-canonical signatures, thereby enforcing it in the transaction + // using the designated flag. + // See: https://github.com/trezor/trezor-crypto/blob/3e8974ff8871263a70b7fbb9a27a1da5b0d810f7/ecdsa.c#L791 + if (!tx->has_flags) { + tx->flags = 0; + tx->has_flags = true; + } + tx->flags |= RIPPLE_FLAG_FULLY_CANONICAL; + + memset(resp->serialized_tx.bytes, 0, sizeof(resp->serialized_tx.bytes)); + + // 'STX' + memcpy(resp->serialized_tx.bytes, "\x53\x54\x58\x00", 4); + + char source_address[MAX_ADDR_SIZE]; + if (!ripple_getAddress(node->public_key, source_address)) + return; + + uint8_t *buf = resp->serialized_tx.bytes + 4; + size_t len = sizeof(resp->serialized_tx.bytes) - 4; + if (!ripple_serialize(&buf, buf + len, tx, source_address, node->public_key, NULL, 0)) + return; + + // Ripple uses the first half of SHA512 + uint8_t hash[64]; + sha512_Raw(resp->serialized_tx.bytes, buf - resp->serialized_tx.bytes, hash); + + uint8_t sig[64]; + if (ecdsa_sign_digest(&secp256k1, node->private_key, hash, sig, NULL, NULL) != 0) { + // Failure + } + + resp->signature.size = ecdsa_sig_to_der(sig, resp->signature.bytes); + resp->has_signature = true; + + memset(resp->serialized_tx.bytes, 0, sizeof(resp->serialized_tx.bytes)); + + buf = resp->serialized_tx.bytes; + len = sizeof(resp->serialized_tx); + if (!ripple_serialize(&buf, buf + len, tx, source_address, node->public_key, + resp->signature.bytes, resp->signature.size)) + return; + + resp->has_serialized_tx = true; + resp->serialized_tx.size = buf - resp->serialized_tx.bytes; +} diff --git a/lib/firmware/ripple_base58.c b/lib/firmware/ripple_base58.c new file mode 100644 index 000000000..2bc221020 --- /dev/null +++ b/lib/firmware/ripple_base58.c @@ -0,0 +1,224 @@ +/** + * Copyright (c) 2012-2014 Luke Dashjr + * Copyright (c) 2013-2014 Pavol Rusnak + * + * Permission is hereby granted, free of charge, to any person obtaining + * a copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included + * in all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS + * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES + * OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR + * OTHER DEALINGS IN THE SOFTWARE. + */ + +#include "keepkey/firmware/ripple_base58.h" + +#include "trezor/crypto/memzero.h" +#include "trezor/crypto/ripemd160.h" +#include "trezor/crypto/sha2.h" + +#include +#include + +// https://developers.ripple.com/base58-encodings.html +const char ripple_b58digits_ordered[] = + "rpshnaf39wBUDNEGHJKLM4PQRST7VWXYZ2bcdeCg65jkm8oFqi1tuvAxyz"; +const int8_t ripple_b58digits_map[] = { + -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, 50, 33, 7, 21, 41, 40, 27, 45, 8, + -1, -1, -1, -1, -1, -1, -1, 54, 10, 38, 12, 14, 47, 15, 16, -1, 17, 18, 19, + 20, 13, -1, 22, 23, 24, 25, 26, 11, 28, 29, 30, 31, 32, -1, -1, -1, -1, -1, + -1, 5, 34, 35, 36, 37, 6, 39, 3, 49, 42, 43, -1, 44, 4, 46, 1, 48, 0, 2, 51, + 52, 53, 9, 55, 56, 57, -1, -1, -1, -1, -1 +}; + +typedef uint64_t b58_maxint_t; +typedef uint32_t b58_almostmaxint_t; +#define b58_almostmaxint_bits (sizeof(b58_almostmaxint_t) * 8) +static const b58_almostmaxint_t b58_almostmaxint_mask = + ((((b58_maxint_t)1) << b58_almostmaxint_bits) - 1); + +bool ripple_b58tobin(void *bin, size_t *binszp, const char *b58) { + size_t binsz = *binszp; + + if (binsz == 0) { + return false; + } + + const unsigned char *b58u = (const unsigned char *)b58; + unsigned char *binu = bin; + size_t outisz = + (binsz + sizeof(b58_almostmaxint_t) - 1) / sizeof(b58_almostmaxint_t); + b58_almostmaxint_t outi[outisz]; + b58_maxint_t t = 0; + b58_almostmaxint_t c = 0; + size_t i = 0, j = 0; + uint8_t bytesleft = binsz % sizeof(b58_almostmaxint_t); + b58_almostmaxint_t zeromask = + bytesleft ? (b58_almostmaxint_mask << (bytesleft * 8)) : 0; + unsigned zerocount = 0; + + size_t b58sz = strlen(b58); + + memzero(outi, sizeof(outi)); + + // Leading zeros, just count + for (i = 0; i < b58sz && b58u[i] == 'r'; ++i) ++zerocount; + + for (; i < b58sz; ++i) { + if (b58u[i] & 0x80) + // High-bit set on invalid digit + return false; + if (ripple_b58digits_map[b58u[i]] == -1) + // Invalid base58 digit + return false; + c = (unsigned)ripple_b58digits_map[b58u[i]]; + for (j = outisz; j--;) { + t = ((b58_maxint_t)outi[j]) * 58 + c; + c = t >> b58_almostmaxint_bits; + outi[j] = t & b58_almostmaxint_mask; + } + if (c) + // Output number too big (carry to the next int32) + return false; + if (outi[0] & zeromask) + // Output number too big (last int32 filled too far) + return false; + } + + j = 0; + if (bytesleft) { + for (i = bytesleft; i > 0; --i) { + *(binu++) = (outi[0] >> (8 * (i - 1))) & 0xff; + } + ++j; + } + + for (; j < outisz; ++j) { + for (i = sizeof(*outi); i > 0; --i) { + *(binu++) = (outi[j] >> (8 * (i - 1))) & 0xff; + } + } + + // Count canonical base58 byte count + binu = bin; + for (i = 0; i < binsz; ++i) { + if (binu[i]) { + if (zerocount > i) { + /* result too large */ + return false; + } + + break; + } + --*binszp; + } + *binszp += zerocount; + + return true; +} + +int ripple_b58check(const void *bin, size_t binsz, HasherType hasher_type, + const char *base58str) { + unsigned char buf[32] = {0}; + const uint8_t *binc = bin; + unsigned i = 0; + if (binsz < 4) return -4; + hasher_Raw(hasher_type, bin, binsz - 4, buf); + if (memcmp(&binc[binsz - 4], buf, 4)) return -1; + + // Check number of zeros is correct AFTER verifying checksum (to avoid + // possibility of accessing base58str beyond the end) + for (i = 0; binc[i] == '\0' && base58str[i] == 'r'; ++i) { + } // Just finding the end of zeros, nothing to do in loop + if (binc[i] == '\0' || base58str[i] == 'r') return -3; + + return binc[0]; +} + +bool ripple_b58enc(char *b58, size_t *b58sz, const void *data, size_t binsz) { + const uint8_t *bin = data; + int carry = 0; + size_t i = 0, j = 0, high = 0, zcount = 0; + size_t size = 0; + + while (zcount < binsz && !bin[zcount]) ++zcount; + + size = (binsz - zcount) * 138 / 100 + 1; + uint8_t buf[size]; + memzero(buf, size); + + for (i = zcount, high = size - 1; i < binsz; ++i, high = j) { + for (carry = bin[i], j = size - 1; (j > high) || carry; --j) { + carry += 256 * buf[j]; + buf[j] = carry % 58; + carry /= 58; + if (!j) { + // Otherwise j wraps to maxint which is > high + break; + } + } + } + + for (j = 0; j < size && !buf[j]; ++j) + ; + + if (*b58sz <= zcount + size - j) { + *b58sz = zcount + size - j + 1; + return false; + } + + if (zcount) memset(b58, 'r', zcount); + for (i = zcount; j < size; ++i, ++j) b58[i] = ripple_b58digits_ordered[buf[j]]; + b58[i] = '\0'; + *b58sz = i + 1; + + return true; +} + +int ripple_encode_check(const uint8_t *data, int datalen, + HasherType hasher_type, char *str, int strsize) { + if (datalen > 128) { + return 0; + } + uint8_t buf[datalen + 32]; + memset(buf, 0, sizeof(buf)); + uint8_t *hash = buf + datalen; + memcpy(buf, data, datalen); + hasher_Raw(hasher_type, data, datalen, hash); + size_t res = strsize; + bool success = ripple_b58enc(str, &res, buf, datalen + 4); + memzero(buf, sizeof(buf)); + return success ? res : 0; +} + +int ripple_decode_check(const char *str, HasherType hasher_type, uint8_t *data, + int datalen) { + if (datalen > 128) { + return 0; + } + uint8_t d[datalen + 4]; + memset(d, 0, sizeof(d)); + size_t res = datalen + 4; + if (ripple_b58tobin(d, &res, str) != true) { + return 0; + } + uint8_t *nd = d + datalen + 4 - res; + if (ripple_b58check(nd, res, hasher_type, str) < 0) { + return 0; + } + memcpy(data, nd, res - 4); + return res - 4; +} + diff --git a/lib/transport/CMakeLists.txt b/lib/transport/CMakeLists.txt index 4d1acac54..ba8b6eea4 100644 --- a/lib/transport/CMakeLists.txt +++ b/lib/transport/CMakeLists.txt @@ -9,6 +9,7 @@ set(protoc_pb_sources ${DEVICE_PROTOCOL}/messages-eos.proto ${DEVICE_PROTOCOL}/messages-nano.proto ${DEVICE_PROTOCOL}/messages-cosmos.proto + ${DEVICE_PROTOCOL}/messages-ripple.proto ${DEVICE_PROTOCOL}/messages.proto) set(protoc_pb_options @@ -17,6 +18,7 @@ set(protoc_pb_options ${CMAKE_SOURCE_DIR}/include/keepkey/transport/messages-eos.options ${CMAKE_SOURCE_DIR}/include/keepkey/transport/messages-nano.options ${CMAKE_SOURCE_DIR}/include/keepkey/transport/messages-cosmos.options + ${CMAKE_SOURCE_DIR}/include/keepkey/transport/messages-ripple.options ${CMAKE_SOURCE_DIR}/include/keepkey/transport/messages.options) set(protoc_c_sources @@ -25,6 +27,7 @@ set(protoc_c_sources ${CMAKE_BINARY_DIR}/lib/transport/messages-eos.pb.c ${CMAKE_BINARY_DIR}/lib/transport/messages-nano.pb.c ${CMAKE_BINARY_DIR}/lib/transport/messages-cosmos.pb.c + ${CMAKE_BINARY_DIR}/lib/transport/messages-ripple.pb.c ${CMAKE_BINARY_DIR}/lib/transport/messages.pb.c) set(protoc_c_headers @@ -33,6 +36,7 @@ set(protoc_c_headers ${CMAKE_BINARY_DIR}/include/messages-eos.pb.h ${CMAKE_BINARY_DIR}/include/messages-nano.pb.h ${CMAKE_BINARY_DIR}/include/messages-cosmos.pb.h + ${CMAKE_BINARY_DIR}/include/messages-ripple.pb.h ${CMAKE_BINARY_DIR}/include/messages.pb.h) set(protoc_pb_sources_moved @@ -41,6 +45,7 @@ set(protoc_pb_sources_moved ${CMAKE_BINARY_DIR}/lib/transport/messages-eos.proto ${CMAKE_BINARY_DIR}/lib/transport/messages-nano.proto ${CMAKE_BINARY_DIR}/lib/transport/messages-cosmos.proto + ${CMAKE_BINARY_DIR}/lib/transport/messages-ripple.proto ${CMAKE_BINARY_DIR}/lib/transport/messages.proto) add_custom_command( @@ -82,6 +87,10 @@ add_custom_command( ${PROTOC_BINARY} -I. -I/usr/include --plugin=nanopb=${NANOPB_DIR}/generator/protoc-gen-nanopb "--nanopb_out=-f messages-cosmos.options:." messages-cosmos.proto + COMMAND + ${PROTOC_BINARY} -I. -I/usr/include + --plugin=nanopb=${NANOPB_DIR}/generator/protoc-gen-nanopb + "--nanopb_out=-f messages-ripple.options:." messages-ripple.proto COMMAND ${PROTOC_BINARY} -I. -I/usr/include --plugin=nanopb=${NANOPB_DIR}/generator/protoc-gen-nanopb diff --git a/unittests/firmware/CMakeLists.txt b/unittests/firmware/CMakeLists.txt index 7d055b883..21825ebbe 100644 --- a/unittests/firmware/CMakeLists.txt +++ b/unittests/firmware/CMakeLists.txt @@ -5,6 +5,7 @@ set(sources ethereum.cpp nano.cpp recovery.cpp + ripple.cpp storage.cpp usb_rx.cpp u2f.cpp) diff --git a/unittests/firmware/ripple.cpp b/unittests/firmware/ripple.cpp new file mode 100644 index 000000000..c6d8abae9 --- /dev/null +++ b/unittests/firmware/ripple.cpp @@ -0,0 +1,124 @@ +extern "C" { +#include "keepkey/firmware/coins.h" +#include "keepkey/firmware/ripple.h" +#include "keepkey/firmware/ripple_base58.h" +#include "trezor/crypto/hasher.h" +} + +#include "gtest/gtest.h" +#include +#include + +TEST(Ripple, AddressEncodeDecode) { + // https://xrpl.org/accounts.html#address-encoding + uint8_t public_key[33 + 1] = + "\xED\x94\x34\x79\x92\x26\x37\x49\x26\xED" + "\xA3\xB5\x4B\x1B\x46\x1B\x4A\xBF\x72\x37" + "\x96\x2E\xAE\x18\x52\x8F\xEA\x67\x59\x53" + "\x97\xFA\x32"; + + uint8_t buff[64]; + memset(buff, 0, sizeof(buff)); + + Hasher hasher; + hasher_Init(&hasher, HASHER_SHA2_RIPEMD); + hasher_Update(&hasher, public_key, 33); + hasher_Final(&hasher, buff + 1); + + EXPECT_TRUE(memcmp(buff, "\x00\x88\xa5\xa5\x7c\x82\x9f\x40\xf2\x5e" + "\xa8\x33\x85\xbb\xde\x6c\x3d\x8b\x4c\xa0\x82", 21) == 0); + + char address[56]; + ASSERT_TRUE(ripple_encode_check(buff, 21, HASHER_SHA2D, + address, MAX_ADDR_SIZE)); + + EXPECT_EQ(std::string(address), "rDTXLQ7ZKZVKz33zJbHjgVShjsBnqMBhmN"); + + uint8_t addr_raw[MAX_ADDR_RAW_SIZE]; + memset(addr_raw, 0, sizeof(addr_raw)); + uint32_t addr_raw_len = ripple_decode_check(address, HASHER_SHA2D, + addr_raw, MAX_ADDR_RAW_SIZE); + EXPECT_EQ(addr_raw_len, 21); + + EXPECT_TRUE(memcmp(buff, addr_raw, 21) == 0); + + EXPECT_TRUE(memcmp(addr_raw, "\x00\x88\xa5\xa5\x7c\x82\x9f\x40\xf2\x5e" + "\xa8\x33\x85\xbb\xde\x6c\x3d\x8b\x4c\xa0\x82", 21) == 0); +} + +TEST(Ripple, SerializeAddress) { + uint8_t buffer[22]; + memset(buffer, 0, sizeof(buffer)); + + uint8_t *buf = buffer; + bool ok = true; + ripple_serializeAddress(&ok, &buf, buf + sizeof(buffer), &RFM_account, + "rNaqKtKrMSwpwZSzRckPf7S96DkimjkF4H"); + + ASSERT_TRUE(ok); + + EXPECT_TRUE(memcmp(buffer, "\x81\x14\x8f\xb4\x0e\x1f\xfa\x5d\x55\x7c\xe9" + "\x85\x1a\x53\x5a\xf9\x49\x65\xe0\xdd\x09\x88", 22) == 0); +} + +TEST(Ripple, Serialize) { + RippleSignTx tx; + memset(&tx, 0, sizeof(tx)); + + tx.address_n_count = 0; + + tx.has_fee = true; + tx.fee = 100000; + + tx.has_flags = true; + tx.flags = 0x80000000; + + tx.has_sequence = true; + tx.sequence = 25; + + tx.has_payment = true; + tx.payment.has_amount = true; + tx.payment.amount = 100000000ULL; + tx.payment.has_destination = true; + strcpy(tx.payment.destination, "rBKz5MC2iXdoS3XgnNSYmF69K1Yo4NS3Ws"); + + uint8_t serialized[183]; + memset(serialized, 0, sizeof(serialized)); + + const uint8_t *public_key = (const uint8_t*) + "\x02\x13\x1f\xac\xd1\xea\xb7\x48\xd6\xcd\xdc\x49\x2f\x54\xb0\x4e" + "\x8c\x35\x65\x88\x94\xf4\xad\xd2\x23\x2e\xbc\x5a\xfe\x75\x21\xdb\xe4"; + + const size_t sig_len = 71; + const uint8_t *sig = (const uint8_t*) + "\x30" // DER Type + "\x45" // Length of rest of payload + "\x02" // r value Marker + "\x21" // r value Length + "\x00\xe2\x43\xef\x62\x36\x75\xee\xeb\x95\x96" // r value + "\x5c\x35\xc3\xe0\x6d\x63\xa9\xfc\x68\xbb\x37" + "\xe1\x7d\xc8\x7a\xf9\xc0\xaf\x83\xec\x05\x7e" + "\x02" // s value Marker + "\x20" // s value Length + "\x6c\xa8\xaa\x5e\xaa\xb8\x39\x63\x97\xae\xf6\xd3\x8d\x25\x71\x04" // s value + "\x41\xfa\xf7\xc7\x9d\x29\x2e\xe1\xd6\x27\xdf\x15\xad\x93\x46\xc0"; + + uint8_t *buf = serialized; + EXPECT_TRUE(ripple_serialize(&buf, buf + sizeof(serialized), &tx, + "rNaqKtKrMSwpwZSzRckPf7S96DkimjkF4H", + public_key, sig, sig_len)); + + const uint8_t *expected = (const uint8_t*) + "\x12\x00\x00\x22\x80\x00\x00\x00\x24\x00\x00\x00\x19\x61\x40\x00\x00\x00\x05\xf5" + "\xe1\x00\x68\x40\x00\x00\x00\x00\x01\x86\xa0\x73\x21\x02\x13\x1f\xac\xd1\xea\xb7" + "\x48\xd6\xcd\xdc\x49\x2f\x54\xb0\x4e\x8c\x35\x65\x88\x94\xf4\xad\xd2\x23\x2e\xbc" + "\x5a\xfe\x75\x21\xdb\xe4\x74\x47\x30\x45\x02\x21\x00\xe2\x43\xef\x62\x36\x75\xee" + "\xeb\x95\x96\x5c\x35\xc3\xe0\x6d\x63\xa9\xfc\x68\xbb\x37\xe1\x7d\xc8\x7a\xf9\xc0" + "\xaf\x83\xec\x05\x7e\x02\x20\x6c\xa8\xaa\x5e\xaa\xb8\x39\x63\x97\xae\xf6\xd3\x8d" + "\x25\x71\x04\x41\xfa\xf7\xc7\x9d\x29\x2e\xe1\xd6\x27\xdf\x15\xad\x93\x46\xc0\x81" + "\x14\x8f\xb4\x0e\x1f\xfa\x5d\x55\x7c\xe9\x85\x1a\x53\x5a\xf9\x49\x65\xe0\xdd\x09" + "\x88\x83\x14\x71\x48\xeb\xeb\xf7\x30\x4c\xcd\xf1\x67\x6f\xef\xcf\x97\x34\xcf\x1e" + "\x78\x08\x26"; + + ASSERT_TRUE(memcmp(serialized, expected, sizeof(serialized)) == 0); +} From 28480c9e5b9632863ba7244b1ef457fb4163fa3e Mon Sep 17 00:00:00 2001 From: keepkeyjon Date: Mon, 27 Jan 2020 14:20:33 -0700 Subject: [PATCH 02/29] xrp: python tests --- deps/python-keepkey | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/deps/python-keepkey b/deps/python-keepkey index 4b4254ec3..879d59690 160000 --- a/deps/python-keepkey +++ b/deps/python-keepkey @@ -1 +1 @@ -Subproject commit 4b4254ec35e3f468effc4379666707b34438c795 +Subproject commit 879d59690a2fe92207a22d5425110fa0d4785542 From e34277f3031a0d67de41b45665b16619448ae395 Mon Sep 17 00:00:00 2001 From: keepkeyjon Date: Tue, 28 Jan 2020 09:05:46 -0700 Subject: [PATCH 03/29] firmware: exchanges from/to XRP not yet supported --- lib/firmware/exchange.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/firmware/exchange.c b/lib/firmware/exchange.c index 117ff2980..032000cc3 100644 --- a/lib/firmware/exchange.c +++ b/lib/firmware/exchange.c @@ -308,6 +308,9 @@ static bool verify_exchange_contract(const CoinType *coin, void *vtx_out, const exchange = &((EthereumSignTx *)vtx_out)->exchange_type; } else if (strcmp("Cosmos", coin->coin_name) == 0) { exchange = &((CosmosMsgSend *)vtx_out)->exchange_type; + } else if (strcmp("Ripple", coin->coin_name) == 0) { + // TODO: Support Ripple exchanges. + return false; } else { exchange = &((TxOutputType *)vtx_out)->exchange_type; } @@ -553,6 +556,9 @@ bool process_exchange_contract(const CoinType *coin, void *vtx_out, const HDNode } else if (strcmp("Cosmos", coin->coin_name) == 0) { tx_exchange = &((CosmosMsgSend *)vtx_out)->exchange_type; deposit_coin = coinByName(coin->coin_name); + } else if (strcmp("Ripple", coin->coin_name) == 0) { + // TODO: Support Ripple exchanges. + return false; } else { tx_exchange = &((TxOutputType *)vtx_out)->exchange_type; deposit_coin = coinByName(coin->coin_name); From c290f0abe2bb260d6e29be7affcbd9f459e3f1fa Mon Sep 17 00:00:00 2001 From: keepkeyjon Date: Mon, 3 Feb 2020 09:49:02 -0700 Subject: [PATCH 04/29] firmware: fix build --- lib/firmware/binance.c | 19 ++++++++----------- lib/firmware/cosmos.c | 1 - lib/firmware/fsm_msg_binance.h | 2 -- lib/firmware/tendermint.c | 2 +- 4 files changed, 9 insertions(+), 15 deletions(-) diff --git a/lib/firmware/binance.c b/lib/firmware/binance.c index f6bffedc3..bb382833d 100644 --- a/lib/firmware/binance.c +++ b/lib/firmware/binance.c @@ -65,9 +65,6 @@ bool binance_serializeCoin(const BinanceCoin *coin) bool binance_serializeInputOutput(const BinanceInputOutput *io) { - bool success = true; - char buffer[64 + 1]; - size_t decoded_len; char hrp[45]; uint8_t decoded[38]; @@ -79,6 +76,7 @@ bool binance_serializeInputOutput(const BinanceInputOutput *io) sha256_Update(&ctx, (const uint8_t*)io->address, strlen(io->address)); sha256_Update(&ctx, (const uint8_t*)"\",\"coins\":[", 11); + bool success = true; for (int i = 0; i < io->coins_count; i++) { success &= binance_serializeCoin(&io->coins[i]); if (i + 1 != io->coins_count) @@ -90,23 +88,23 @@ bool binance_serializeInputOutput(const BinanceInputOutput *io) return success; } -bool binance_signTxUpdateTransfer(const BinanceTransferMsg *msg) +bool binance_signTxUpdateTransfer(const BinanceTransferMsg *_msg) { bool success = true; sha256_Update(&ctx, (const uint8_t*)"{\"inputs\":[", 11); - for (int i = 0; i < msg->inputs_count; i++) { - success &= binance_serializeInputOutput(&msg->inputs[i]); - if (i + 1 != msg->inputs_count) + for (int i = 0; i < _msg->inputs_count; i++) { + success &= binance_serializeInputOutput(&_msg->inputs[i]); + if (i + 1 != _msg->inputs_count) sha256_Update(&ctx, (const uint8_t*)",", 1); } sha256_Update(&ctx, (const uint8_t*)"],\"outputs\":[", 13); - for (int i = 0; i < msg->outputs_count; i++) { - success &= binance_serializeInputOutput(&msg->outputs[i]); - if (i + 1 != msg->outputs_count) + for (int i = 0; i < _msg->outputs_count; i++) { + success &= binance_serializeInputOutput(&_msg->outputs[i]); + if (i + 1 != _msg->outputs_count) sha256_Update(&ctx, (const uint8_t*)",", 1); } @@ -119,7 +117,6 @@ bool binance_signTxUpdateTransfer(const BinanceTransferMsg *msg) bool binance_signTxFinalize(uint8_t *public_key, uint8_t *signature) { - bool success = true; char buffer[64 + 1]; if (!tendermint_snprintf(&ctx, buffer, sizeof(buffer), diff --git a/lib/firmware/cosmos.c b/lib/firmware/cosmos.c index f1a3f0634..5378358a1 100644 --- a/lib/firmware/cosmos.c +++ b/lib/firmware/cosmos.c @@ -116,7 +116,6 @@ bool cosmos_signTxUpdateMsgSend(const uint64_t amount, bool cosmos_signTxFinalize(uint8_t* public_key, uint8_t* signature) { - bool success = true; char buffer[64 + 1]; // 16 + ^20 = ^36 diff --git a/lib/firmware/fsm_msg_binance.h b/lib/firmware/fsm_msg_binance.h index cadd3c4ab..cb53a06b0 100644 --- a/lib/firmware/fsm_msg_binance.h +++ b/lib/firmware/fsm_msg_binance.h @@ -100,8 +100,6 @@ void fsm_msgBinanceTransferMsg(const BinanceTransferMsg *msg) { const CoinType *coin = fsm_getCoin(true, "Binance"); if (!coin) { return; } - const BinanceSignTx *sign_tx = binance_getBinanceSignTx(); - switch (msg->outputs[0].address_type) { case OutputAddressType_EXCHANGE: { HDNode *root_node = fsm_getDerivedNode(SECP256K1_NAME, 0, 0, NULL); diff --git a/lib/firmware/tendermint.c b/lib/firmware/tendermint.c index 2c078fffa..891fc6fe5 100644 --- a/lib/firmware/tendermint.c +++ b/lib/firmware/tendermint.c @@ -44,7 +44,7 @@ bool tendermint_getAddress(const HDNode *node, const char *prefix, char *address void tendermint_sha256UpdateEscaped(SHA256_CTX *ctx, const char *s, size_t len) { - for (int i = 0; i < len; i++) { + for (size_t i = 0; i != len; i++) { if (s[i] == '"') { sha256_Update(ctx, (const uint8_t *)"\\\"", 2); } else if (s[i] == '\\') { From 44a9e48bb406d681782354bf458e752592ddf6a3 Mon Sep 17 00:00:00 2001 From: keepkeyjon Date: Mon, 3 Feb 2020 10:10:27 -0700 Subject: [PATCH 05/29] fix test --- deps/python-keepkey | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/deps/python-keepkey b/deps/python-keepkey index fa84bd61c..8f0a5a084 160000 --- a/deps/python-keepkey +++ b/deps/python-keepkey @@ -1 +1 @@ -Subproject commit fa84bd61cec5897af406cb0bc3b9eefdef951cd7 +Subproject commit 8f0a5a0842b661f650bf6f0d707803c47a1ec6a8 From c0895e79f722aebd1ef210564c7e13f8d43eb4f1 Mon Sep 17 00:00:00 2001 From: keepkeyjon Date: Tue, 28 Jan 2020 10:07:11 -0700 Subject: [PATCH 06/29] board: basic constant-time implementation of memcmp --- include/keepkey/board/memcmp_s.h | 13 +++++ lib/board/CMakeLists.txt | 3 +- lib/board/memcmp_s.S | 96 ++++++++++++++++++++++++++++++++ lib/firmware/storage.c | 7 ++- lib/firmware/u2f.c | 3 +- 5 files changed, 117 insertions(+), 5 deletions(-) create mode 100644 include/keepkey/board/memcmp_s.h create mode 100644 lib/board/memcmp_s.S diff --git a/include/keepkey/board/memcmp_s.h b/include/keepkey/board/memcmp_s.h new file mode 100644 index 000000000..63cd33404 --- /dev/null +++ b/include/keepkey/board/memcmp_s.h @@ -0,0 +1,13 @@ +#ifndef KEEPKEY_BOARD_MEMCMPS_H +#define KEEPKEY_BOARD_MEMCMPS_H + +#ifndef EMULATOR +/// Compare two memory regions for equality in constant time. +int memcmp_s(const void *lhs, const void *rhs, size_t len); +#else +# include +# warning "memcmp_s is not guaranteed to be constant-time on this architecture" +# define memcmp_s(lhs, rhs, len) memcmp((lhs), (rhs), (len)) +#endif + +#endif diff --git a/lib/board/CMakeLists.txt b/lib/board/CMakeLists.txt index 01c1e3773..79fe524e6 100644 --- a/lib/board/CMakeLists.txt +++ b/lib/board/CMakeLists.txt @@ -42,7 +42,8 @@ else() set(sources ${sources} usb21_standard.c webusb.c - winusb.c) + winusb.c + memcmp_s.S) endif() include_directories( diff --git a/lib/board/memcmp_s.S b/lib/board/memcmp_s.S new file mode 100644 index 000000000..29c3a504d --- /dev/null +++ b/lib/board/memcmp_s.S @@ -0,0 +1,96 @@ +.text + .syntax unified + + // Compares up to 256 bytes in constant time. + // Roughly: + // int memcmp_s(const void *lhs, const void *rhs, size_t len) { + // char permute[len]; + // + // for (int i = 0; i < len; i++) { + // permute[i] = i; + // } + // + // random_permute_char(permute, len); + // + // char res = 0; + // for (int i = 0; i < len; i++) { + // res |= lhs[permute[i]] ^ rhs[permute[i]]; + // } + // + // memzero(permute, len); + // + // return res == 0; + // } + .global memcmp_s +memcmp_s: + push {r4, r5, r6, r7, lr} + add r7, sp, #12 + push.w {r8, r9, r10} + mov r6, r0 + adds r0, r2, #7 + bic r0, r0, #7 + sub.w r5, sp, r0 + mov r4, r2 + mov r8, r1 + mov sp, r5 + + mov r0, r5 + mov r1, r2 + bl fill // fill(permute, len) + + mov r0, r5 + mov r1, r4 + bl random_permute_char // random_permute_char(permute, len) + + mov r0, r6 + mov r1, r8 + mov r2, r5 + mov r3, r4 + bl permuted_compare // res = permuted_compare(lhs, rhs, permute, len) + + mov r6, r0 + mov r0, r5 + mov r1, r4 + bl memzero // memzero(permute, len) + mov r0, r6 + + sub.w r4, r7, #24 + mov sp, r4 + pop.w {r8, r9, r10} + pop {r4, r5, r6, r7, pc} + + // Compare two arrays in permuted order + // int permuted_compare( + // const char *lhs, + // const char *rhs, + // const char *permute, + // size_t len) +permuted_compare: + push {r4, r5, r7, r8, lr} + add r7, sp, #12 + mov r12, r0 + movs r0, #0 +.permuted_compare.loop: + ldrb r8, [r2], #1 // r8 = permute[i] + subs r3, #1 + ldrb.w r5, [r12, r8] // r5 = lhs[r8] + ldrb.w r4, [r1, r8] // r4 = rhs[r8] + eor.w r5, r5, r4 // r5 = r5 ^ r4 + orr.w r0, r0, r5 // r0 = r0 | r5 + bne .permuted_compare.loop + + clz r0, r0 + lsrs r0, r0, #5 // r0 = r0 == 0 + + pop {r4, r5, r7, r8, pc} + + // Fill an array with the numbers [0, 1, 2, ... len) + // void fill(const char *array, size_t len) +fill: + movs r2, #0 +.fill.loop: + strb r2, [r0, r2] + adds r2, #1 + cmp r1, r2 + bne .fill.loop + bx lr diff --git a/lib/firmware/storage.c b/lib/firmware/storage.c index 89a1f8ba9..aeed275e3 100644 --- a/lib/firmware/storage.c +++ b/lib/firmware/storage.c @@ -31,6 +31,7 @@ #include "keepkey/board/supervise.h" #include "keepkey/board/keepkey_board.h" #include "keepkey/board/keepkey_flash.h" +#include "keepkey/board/memcmp_s.h" #include "keepkey/board/memory.h" #include "keepkey/board/util.h" #include "keepkey/board/variant.h" @@ -305,7 +306,7 @@ bool storage_isPinCorrect_impl(const char *pin, const uint8_t wrapped_key[64], c uint8_t fp[32]; storage_keyFingerprint(key, fp); - bool ret = memcmp(fp, fingerprint, 32) == 0; + bool ret = memcmp_s(fp, fingerprint, 32) == 0; if (!ret) memzero(key, 64); memzero(wrapping_key, 64); @@ -371,8 +372,8 @@ void storage_secMigrate(SessionState *ss, Storage *storage, bool encrypt) { uint8_t sec_fingerprint[32]; sha256_Raw((const uint8_t *)scratch, sizeof(scratch), sec_fingerprint); if (storage->has_sec_fingerprint) { - if (memcmp(storage->sec_fingerprint, sec_fingerprint, - sizeof(sec_fingerprint)) != 0) { + if (memcmp_s(storage->sec_fingerprint, sec_fingerprint, + sizeof(sec_fingerprint)) != 0) { memzero(scratch, sizeof(scratch)); storage_wipe(); layout_warning_static("Storage decrypt failure. Reboot device!"); diff --git a/lib/firmware/u2f.c b/lib/firmware/u2f.c index 39b699ab6..9ad9db613 100644 --- a/lib/firmware/u2f.c +++ b/lib/firmware/u2f.c @@ -25,6 +25,7 @@ #include "keepkey/board/keepkey_button.h" #include "keepkey/board/confirm_sm.h" #include "keepkey/board/layout.h" +#include "keepkey/board/memcmp_s.h" #include "keepkey/board/timer.h" #include "keepkey/board/u2f_hid.h" #include "keepkey/board/usb.h" @@ -576,7 +577,7 @@ static const HDNode *validateKeyHandle(const uint8_t app_id[], const uint8_t key hmac_sha256(node->private_key, sizeof(node->private_key), keybase, sizeof(keybase), hmac); - if (memcmp(&key_handle[KEY_PATH_LEN], hmac, SHA256_DIGEST_LENGTH) != 0) + if (memcmp_s(&key_handle[KEY_PATH_LEN], hmac, SHA256_DIGEST_LENGTH) != 0) return NULL; // Done! From 3f67e92cb1a29584d0ff18dd2b27770074cf56b0 Mon Sep 17 00:00:00 2001 From: keepkeyjon Date: Mon, 3 Feb 2020 12:01:59 -0700 Subject: [PATCH 07/29] board: use 1-of-N decoy compares ... to further obfuscate the power profile and timing of memcmp_s. --- include/keepkey/board/memcmp_s.h | 11 ++-- lib/board/CMakeLists.txt | 3 +- lib/board/{memcmp_s.S => memcmp_cst.S} | 22 ++++--- lib/board/memcmp_s.c | 90 ++++++++++++++++++++++++++ unittests/board/CMakeLists.txt | 1 + unittests/board/memcmp_s.cpp | 34 ++++++++++ 6 files changed, 144 insertions(+), 17 deletions(-) rename lib/board/{memcmp_s.S => memcmp_cst.S} (85%) create mode 100644 lib/board/memcmp_s.c create mode 100644 unittests/board/memcmp_s.cpp diff --git a/include/keepkey/board/memcmp_s.h b/include/keepkey/board/memcmp_s.h index 63cd33404..1fb6ea072 100644 --- a/include/keepkey/board/memcmp_s.h +++ b/include/keepkey/board/memcmp_s.h @@ -1,13 +1,12 @@ #ifndef KEEPKEY_BOARD_MEMCMPS_H #define KEEPKEY_BOARD_MEMCMPS_H -#ifndef EMULATOR +#include + /// Compare two memory regions for equality in constant time. +/// \param lhs must be an array of at least `len` bytes +/// \param rhs must be an array of at least `len` bytes +/// \param len must be in [32, 255), otherwise behavior is undefined. int memcmp_s(const void *lhs, const void *rhs, size_t len); -#else -# include -# warning "memcmp_s is not guaranteed to be constant-time on this architecture" -# define memcmp_s(lhs, rhs, len) memcmp((lhs), (rhs), (len)) -#endif #endif diff --git a/lib/board/CMakeLists.txt b/lib/board/CMakeLists.txt index 79fe524e6..32ff5622b 100644 --- a/lib/board/CMakeLists.txt +++ b/lib/board/CMakeLists.txt @@ -14,6 +14,7 @@ set(sources keepkey_usart.c layout.c memory.c + memcmp_s.c mmhusr.c messages.c pin.c @@ -43,7 +44,7 @@ else() usb21_standard.c webusb.c winusb.c - memcmp_s.S) + memcmp_cst.S) endif() include_directories( diff --git a/lib/board/memcmp_s.S b/lib/board/memcmp_cst.S similarity index 85% rename from lib/board/memcmp_s.S rename to lib/board/memcmp_cst.S index 29c3a504d..272ed5d30 100644 --- a/lib/board/memcmp_s.S +++ b/lib/board/memcmp_cst.S @@ -3,7 +3,7 @@ // Compares up to 256 bytes in constant time. // Roughly: - // int memcmp_s(const void *lhs, const void *rhs, size_t len) { + // int memcmp_cst(const void *lhs, const void *rhs, size_t len) { // char permute[len]; // // for (int i = 0; i < len; i++) { @@ -21,8 +21,8 @@ // // return res == 0; // } - .global memcmp_s -memcmp_s: + .global memcmp_cst +memcmp_cst: push {r4, r5, r6, r7, lr} add r7, sp, #12 push.w {r8, r9, r10} @@ -36,7 +36,7 @@ memcmp_s: mov r0, r5 mov r1, r2 - bl fill // fill(permute, len) + bl asc_fill // asc_fill(permute, len) mov r0, r5 mov r1, r4 @@ -79,18 +79,20 @@ permuted_compare: orr.w r0, r0, r5 // r0 = r0 | r5 bne .permuted_compare.loop - clz r0, r0 - lsrs r0, r0, #5 // r0 = r0 == 0 + cmp r0, #0 + it ne + movne r0, #1 // r0 = !!r0 pop {r4, r5, r7, r8, pc} // Fill an array with the numbers [0, 1, 2, ... len) - // void fill(const char *array, size_t len) -fill: + // void asc_fill(const char *array, size_t len) + .global asc_fill +asc_fill: movs r2, #0 -.fill.loop: +.asc_fill.loop: strb r2, [r0, r2] adds r2, #1 cmp r1, r2 - bne .fill.loop + bne .asc_fill.loop bx lr diff --git a/lib/board/memcmp_s.c b/lib/board/memcmp_s.c new file mode 100644 index 000000000..7c2587b6a --- /dev/null +++ b/lib/board/memcmp_s.c @@ -0,0 +1,90 @@ +#include "keepkey/board/memcmp_s.h" + +#include "keepkey/rand/rng.h" +#include "trezor/crypto/rand.h" +#include "trezor/crypto/memzero.h" + +#include +#include +#include +#include +#include + +#define DECOY_COUNT 8 + +#ifndef EMULATOR +/// \brief Compare two memory regions for equality in constant time. +/// +/// Note: Don't mark lhs/rhs as const, even though they are, as this forces the +/// compiler to assume the underlying fuction might have changed them. +int memcmp_cst(void *lhs, void *rhs, size_t len); +#else +# warning "memcmp_s is not guaranteed to be constant-time on this architecture" +# define memcmp_cst(lhs, rhs, len) memcmp((lhs), (rhs), (len)) +#endif + +void asc_fill(char *permute, size_t len); + +#ifdef EMULATOR +void asc_fill(char *permute, size_t len) +{ + for (size_t i = 0; i != len; i++) { + permute[i] = i; + } +} +#endif + +int memcmp_s(const void *lhs, const void *rhs, size_t len) +{ + if (len < 32 || len > 255) { + // Setting the floor on length to 32 is a simple way to guarantee that + // all the decoy buffers we end up will never compare equal with either + // lhs/rhs, since the chances of randomly generating the same 32 byte + // sequence twice is astronomically small... you're just as likely to + // guess Satoshi's private keys. + assert(false && "unsupported memcmp_s length"); + abort(); + } + + static uint8_t decoys[DECOY_COUNT][255]; + random_buffer(&decoys[0][0], sizeof(decoys)); + + static void *permuted[DECOY_COUNT + 2]; + for (size_t i = 0; i != DECOY_COUNT; i++) { + permuted[i] = &decoys[i]; + } + permuted[DECOY_COUNT] = (void*)lhs; + permuted[DECOY_COUNT + 1] = (void*)rhs; + + static uint8_t permute[DECOY_COUNT + 2]; + asc_fill((char*)permute, DECOY_COUNT + 2); + random_permute_char((char*)permute, DECOY_COUNT + 2); + + // Compare every pair of buffers once, and count how many match. We should + // get exactly one match from the comparison of lhs with rhs, assuming they + // matched to begin with. Use the permutation array as a random indirection + // so that an attacker measuring power draw can't know which pair of arrays + // we're actually comparing at any given moment. + + // d d d d l r + // d 1 1 1 1 1 + // d 1 1 1 1 + // d 1 1 1 + // d 1 1 + // l 0 + + int diffs = 0; + for (size_t y = 0; y != DECOY_COUNT + 2 - 1; y++) { + for (size_t x = y + 1; x != DECOY_COUNT + 2; x++) { + diffs += !!memcmp_cst(permuted[permute[x]], + permuted[permute[y]], + len); + } + } + + memzero(&decoys[0][0], sizeof(decoys)); + memzero(permuted, sizeof(permuted)); + memzero(permute, sizeof(permute)); + + return diffs != (DECOY_COUNT + 2 - 1) * (DECOY_COUNT + 2) / 2 - 1; +} diff --git a/unittests/board/CMakeLists.txt b/unittests/board/CMakeLists.txt index 40a1a5033..e2f5adb00 100644 --- a/unittests/board/CMakeLists.txt +++ b/unittests/board/CMakeLists.txt @@ -1,4 +1,5 @@ set(sources + memcmp_s.cpp board.cpp) include_directories( diff --git a/unittests/board/memcmp_s.cpp b/unittests/board/memcmp_s.cpp new file mode 100644 index 000000000..f52ec2ba0 --- /dev/null +++ b/unittests/board/memcmp_s.cpp @@ -0,0 +1,34 @@ +extern "C" { +#include "keepkey/board/memcmp_s.h" +} + +#include "gtest/gtest.h" + +TEST(Board, Memcmps) { + struct { + const char *lhs; + const char *rhs; + size_t len; + int expected; + } vec[] = { + { "A1234567890123456789012345678901", + "A1234567890123456789012345678901", 32, 0 }, + { "B123456789012345678901234567890101234567890123456789012345678901", + "B123456789012345678901234567890101234567890123456789012345678901", 63, 0 }, + { "C1234567890123456789012345678901", + "C123456789012345678901234567890A", 32, 1 }, + { "D ", + "D F", 32, 1 }, + { "E ", + "E ", 32, 0 }, + }; + + for (const auto &v : vec) { + for (size_t i = 0; i < 1000; i++) { + ASSERT_EQ(memcmp_s(v.lhs, v.rhs, v.len), v.expected) + << "lhs: " << v.lhs << "\n" + << "rhs: " << v.rhs << "\n" + << "len: " << v.len << "\n"; + } + } +} From 937d556abe1f2fa5d12d49321fca3e9213601d18 Mon Sep 17 00:00:00 2001 From: keepkeyjon Date: Wed, 5 Feb 2020 11:34:12 -0700 Subject: [PATCH 08/29] update python-keepkey --- deps/python-keepkey | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/deps/python-keepkey b/deps/python-keepkey index 350815c85..e619b65f5 160000 --- a/deps/python-keepkey +++ b/deps/python-keepkey @@ -1 +1 @@ -Subproject commit 350815c8532d1ff8cf4bc8aed67eae3d13654119 +Subproject commit e619b65f5a506b0136ecc1ff2044c615c25a60fa From 139d0fdd766492a56e67ad67ef079ade6fe3f8d0 Mon Sep 17 00:00:00 2001 From: keepkeyjon Date: Wed, 5 Feb 2020 15:38:12 -0700 Subject: [PATCH 09/29] firmware: don't return a signature if signing failed --- lib/firmware/ripple.c | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/firmware/ripple.c b/lib/firmware/ripple.c index bf1ad0d12..b569fd1bd 100644 --- a/lib/firmware/ripple.c +++ b/lib/firmware/ripple.c @@ -269,6 +269,7 @@ void ripple_signTx(const HDNode *node, RippleSignTx *tx, uint8_t sig[64]; if (ecdsa_sign_digest(&secp256k1, node->private_key, hash, sig, NULL, NULL) != 0) { // Failure + return; } resp->signature.size = ecdsa_sig_to_der(sig, resp->signature.bytes); From b6ca757ff59a65a1a9418c63f05522bad922cf6e Mon Sep 17 00:00:00 2001 From: keepkeyjon Date: Wed, 5 Feb 2020 16:03:21 -0700 Subject: [PATCH 10/29] firmware: reduce lifetime of secrets ... by memzeroing them at earliest convenience. C++-style RAII would be sooooo helpful here. --- lib/firmware/fsm_msg_cosmos.h | 11 +++++++++-- lib/firmware/fsm_msg_eos.h | 6 +++++- lib/firmware/fsm_msg_ethereum.h | 14 ++++++++++---- lib/firmware/fsm_msg_nano.h | 9 +++++++++ lib/firmware/fsm_msg_ripple.h | 7 +++++++ 5 files changed, 40 insertions(+), 7 deletions(-) diff --git a/lib/firmware/fsm_msg_cosmos.h b/lib/firmware/fsm_msg_cosmos.h index b9a089c73..5640e2758 100644 --- a/lib/firmware/fsm_msg_cosmos.h +++ b/lib/firmware/fsm_msg_cosmos.h @@ -15,6 +15,7 @@ void fsm_msgCosmosGetAddress(const CosmosGetAddress *msg) hdnode_fill_public_key(node); if (!tendermint_getAddress(node, "cosmos", resp->address)) { + memzero(node, sizeof(*node)); fsm_sendFailure(FailureType_Failure_FirmwareError, _("Can't encode address")); layoutHome(); return; @@ -28,6 +29,7 @@ void fsm_msgCosmosGetAddress(const CosmosGetAddress *msg) !bip32_path_to_string(node_str, sizeof(node_str), msg->address_n, msg->address_n_count)) { memset(node_str, 0, sizeof(node_str)); + memzero(node, sizeof(*node)); fsm_sendFailure(FailureType_Failure_FirmwareError, _("Can't create Bip32 Path String")); layoutHome(); } @@ -36,6 +38,7 @@ void fsm_msgCosmosGetAddress(const CosmosGetAddress *msg) if (mismatch) { if (!confirm(ButtonRequestType_ButtonRequest_Other, "WARNING", "Wrong address path for selected coin. Continue at your own risk!")) { + memzero(node, sizeof(*node)); fsm_sendFailure(FailureType_Failure_ActionCancelled, NULL); layoutHome(); return; @@ -43,6 +46,7 @@ void fsm_msgCosmosGetAddress(const CosmosGetAddress *msg) } if(!confirm_ethereum_address(node_str, resp->address)) { + memzero(node, sizeof(*node)); fsm_sendFailure(FailureType_Failure_ActionCancelled, "Show address cancelled"); layoutHome(); return; @@ -51,8 +55,9 @@ void fsm_msgCosmosGetAddress(const CosmosGetAddress *msg) resp->has_address = true; - layoutHome(); + memzero(node, sizeof(*node)); msg_write(MessageType_MessageType_CosmosAddress, resp); + layoutHome(); } void fsm_msgCosmosSignTx(const CosmosSignTx *msg) @@ -81,14 +86,16 @@ void fsm_msgCosmosSignTx(const CosmosSignTx *msg) if (!cosmos_signTxInit(node, msg)) { cosmos_signAbort(); + memzero(node, sizeof(*node)); fsm_sendFailure(FailureType_Failure_FirmwareError, _("Failed to initialize transaction signing")); layoutHome(); return; } - layoutHome(); + memzero(node, sizeof(*node)); msg_write(MessageType_MessageType_CosmosMsgRequest, resp); + layoutHome(); } void fsm_msgCosmosMsgAck(const CosmosMsgAck* msg) { diff --git a/lib/firmware/fsm_msg_eos.h b/lib/firmware/fsm_msg_eos.h index 213302952..cc6be83d6 100644 --- a/lib/firmware/fsm_msg_eos.h +++ b/lib/firmware/fsm_msg_eos.h @@ -37,6 +37,7 @@ void fsm_msgEosGetPublicKey(const EosGetPublicKey *msg) { if (!eos_getPublicKey(node, curve, msg->kind, resp->wif_public_key, sizeof(resp->wif_public_key))) { + memzero(node, sizeof(*node)); fsm_sendFailure(FailureType_Failure_Other, "Could not derive EOS pubkey"); layoutHome(); return; @@ -62,6 +63,7 @@ void fsm_msgEosGetPublicKey(const EosGetPublicKey *msg) { if (!confirm(ButtonRequestType_ButtonRequest_Address, node_str, "%s", resp->wif_public_key)) { + memzero(node, sizeof(*node)); fsm_sendFailure(FailureType_Failure_ActionCancelled, "Show EOS public key cancelled."); layoutHome(); @@ -69,8 +71,9 @@ void fsm_msgEosGetPublicKey(const EosGetPublicKey *msg) { } } - layoutHome(); + memzero(node, sizeof(*node)); msg_write(MessageType_MessageType_EosPublicKey, resp); + layoutHome(); } void fsm_msgEosSignTx(const EosSignTx *msg) { @@ -95,6 +98,7 @@ void fsm_msgEosSignTx(const EosSignTx *msg) { eos_signingInit(msg->chain_id.bytes, msg->num_actions, &msg->header, root, msg->address_n, msg->address_n_count); + memzero(root, sizeof(*root)); RESP_INIT(EosTxActionRequest); msg_write(MessageType_MessageType_EosTxActionRequest, resp); } diff --git a/lib/firmware/fsm_msg_ethereum.h b/lib/firmware/fsm_msg_ethereum.h index 445addd8c..f8a9b735c 100644 --- a/lib/firmware/fsm_msg_ethereum.h +++ b/lib/firmware/fsm_msg_ethereum.h @@ -107,10 +107,11 @@ void fsm_msgEthereumSignTx(EthereumSignTx *msg) return; } - const HDNode *node = fsm_getDerivedNode(SECP256K1_NAME, msg->address_n, msg->address_n_count, NULL); + HDNode *node = fsm_getDerivedNode(SECP256K1_NAME, msg->address_n, msg->address_n_count, NULL); if (!node) return; ethereum_signing_init(msg, node, needs_confirm); + memzero(node, sizeof(*node)); } void fsm_msgEthereumTxAck(EthereumTxAck *msg) @@ -126,13 +127,15 @@ void fsm_msgEthereumGetAddress(EthereumGetAddress *msg) CHECK_PIN - const HDNode *node = fsm_getDerivedNode(SECP256K1_NAME, msg->address_n, msg->address_n_count, NULL); + HDNode *node = fsm_getDerivedNode(SECP256K1_NAME, msg->address_n, msg->address_n_count, NULL); if (!node) return; resp->address.size = 20; - if (!hdnode_get_ethereum_pubkeyhash(node, resp->address.bytes)) + if (!hdnode_get_ethereum_pubkeyhash(node, resp->address.bytes)) { + memzero(node, sizeof(*node)); return; + } const CoinType *coin = NULL; bool rskip60 = false; @@ -168,12 +171,14 @@ void fsm_msgEthereumGetAddress(EthereumGetAddress *msg) } if (!confirm_ethereum_address(node_str, address)) { + memzero(node, sizeof(*node)); fsm_sendFailure(FailureType_Failure_ActionCancelled, _("Show address cancelled")); layoutHome(); return; } } + memzero(node, sizeof(*node)); msg_write(MessageType_MessageType_EthereumAddress, resp); layoutHome(); } @@ -193,10 +198,11 @@ void fsm_msgEthereumSignMessage(EthereumSignMessage *msg) CHECK_PIN - const HDNode *node = fsm_getDerivedNode(SECP256K1_NAME, msg->address_n, msg->address_n_count, NULL); + HDNode *node = fsm_getDerivedNode(SECP256K1_NAME, msg->address_n, msg->address_n_count, NULL); if (!node) return; ethereum_message_sign(msg, node, resp); + memzero(node, sizeof(*node)); layoutHome(); } diff --git a/lib/firmware/fsm_msg_nano.h b/lib/firmware/fsm_msg_nano.h index d51d49e87..922fae0f0 100644 --- a/lib/firmware/fsm_msg_nano.h +++ b/lib/firmware/fsm_msg_nano.h @@ -20,6 +20,7 @@ void fsm_msgNanoGetAddress(NanoGetAddress *msg) &node->public_key[1], coin->nanoaddr_prefix, strlen(coin->nanoaddr_prefix), address, sizeof(address))) { + memzero(node, sizeof(*node)); fsm_sendFailure(FailureType_Failure_Other, _("Can't encode address")); layoutHome(); return; @@ -38,6 +39,7 @@ void fsm_msgNanoGetAddress(NanoGetAddress *msg) if (mismatch) { if (!confirm(ButtonRequestType_ButtonRequest_Other, "WARNING", "Wrong address path for selected coin. Continue at your own risk!")) { + memzero(node, sizeof(*node)); fsm_sendFailure(FailureType_Failure_ActionCancelled, NULL); layoutHome(); return; @@ -45,6 +47,7 @@ void fsm_msgNanoGetAddress(NanoGetAddress *msg) } if(!confirm_nano_address(node_str, address)) { + memzero(node, sizeof(*node)); fsm_sendFailure(FailureType_Failure_ActionCancelled, "Show address cancelled"); layoutHome(); return; @@ -53,6 +56,7 @@ void fsm_msgNanoGetAddress(NanoGetAddress *msg) resp->has_address = true; strlcpy(resp->address, address, sizeof(resp->address)); + memzero(node, sizeof(*node)); msg_write(MessageType_MessageType_NanoAddress, resp); layoutHome(); } @@ -73,6 +77,7 @@ void fsm_msgNanoSignTx(NanoSignTx *msg) hdnode_fill_public_key(node); if (!nano_signingInit(msg, node, coin)) { + memzero(node, sizeof(*node)); fsm_sendFailure(FailureType_Failure_Other, _("Block data invalid")); layoutHome(); return; @@ -82,6 +87,7 @@ void fsm_msgNanoSignTx(NanoSignTx *msg) if (!nano_parentHash(msg)) { nano_signingAbort(); + memzero(node, sizeof(*node)); fsm_sendFailure(FailureType_Failure_Other, _("Parent block data invalid")); layoutHome(); return; @@ -95,6 +101,7 @@ void fsm_msgNanoSignTx(NanoSignTx *msg) NULL); if (!recip) { nano_signingAbort(); + memzero(node, sizeof(*node)); fsm_sendFailure(FailureType_Failure_Other, _("Could not derive node")); layoutHome(); return; @@ -104,6 +111,7 @@ void fsm_msgNanoSignTx(NanoSignTx *msg) if (!nano_currentHash(msg, recip)) { nano_signingAbort(); + memzero(node, sizeof(*node)); fsm_sendFailure(FailureType_Failure_Other, _("Current block data invalid")); layoutHome(); return; @@ -116,6 +124,7 @@ void fsm_msgNanoSignTx(NanoSignTx *msg) if (!nano_sanityCheck(msg)) { nano_signingAbort(); + memzero(node, sizeof(*node)); fsm_sendFailure(FailureType_Failure_Other, _("Failed sanity check")); layoutHome(); return; diff --git a/lib/firmware/fsm_msg_ripple.h b/lib/firmware/fsm_msg_ripple.h index 3a94f0e40..1a4ed4f4a 100644 --- a/lib/firmware/fsm_msg_ripple.h +++ b/lib/firmware/fsm_msg_ripple.h @@ -32,6 +32,7 @@ void fsm_msgRippleGetAddress(const RippleGetAddress *msg) const CoinType *coin = fsm_getCoin(true, "Ripple"); if (!ripple_getAddress(node->public_key, resp->address)) { + memzero(node, sizeof(*node)); fsm_sendFailure(FailureType_Failure_Other, _("Address derivation failed")); layoutHome(); return; @@ -52,12 +53,14 @@ void fsm_msgRippleGetAddress(const RippleGetAddress *msg) } if (!confirm_ethereum_address(node_str, resp->address)) { + memzero(node, sizeof(*node)); fsm_sendFailure(FailureType_Failure_ActionCancelled, _("Show address cancelled")); layoutHome(); return; } } + memzero(node, sizeof(*node)); msg_write(MessageType_MessageType_RippleAddress, resp); layoutHome(); } @@ -80,6 +83,7 @@ void fsm_msgRippleSignTx(RippleSignTx *msg) hdnode_fill_public_key(node); if (!msg->has_fee || msg->fee < RIPPLE_MIN_FEE || msg->fee > RIPPLE_MAX_FEE) { + memzero(node, sizeof(*node)); fsm_sendFailure(FailureType_Failure_SyntaxError, _("Fee must be between 10 and 1,000,000 drops")); return; @@ -99,6 +103,7 @@ void fsm_msgRippleSignTx(RippleSignTx *msg) amount_string, msg->payment.destination, msg->payment.destination_tag)) { + memzero(node, sizeof(*node)); fsm_sendFailure(FailureType_Failure_ActionCancelled, "Signing cancelled"); layoutHome(); return; @@ -108,12 +113,14 @@ void fsm_msgRippleSignTx(RippleSignTx *msg) if (!confirm(ButtonRequestType_ButtonRequest_SignTx, "Transaction", "Really send %s, with a transaction fee of %s?", amount_string, fee_string)) { + memzero(node, sizeof(*node)); fsm_sendFailure(FailureType_Failure_ActionCancelled, "Signing cancelled"); layoutHome(); return; } ripple_signTx(node, msg, resp); + memzero(node, sizeof(*node)); msg_write(MessageType_MessageType_RippleSignedTx, resp); layoutHome(); } From cb08b887d5a0b211e7be992562fd7eb9a5b133ec Mon Sep 17 00:00:00 2001 From: keepkeyjon Date: Wed, 5 Feb 2020 16:11:17 -0700 Subject: [PATCH 11/29] firmware: moar memzero --- lib/firmware/fsm_msg_coin.h | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/lib/firmware/fsm_msg_coin.h b/lib/firmware/fsm_msg_coin.h index 81b46b947..14c59213e 100644 --- a/lib/firmware/fsm_msg_coin.h +++ b/lib/firmware/fsm_msg_coin.h @@ -44,6 +44,7 @@ void fsm_msgGetPublicKey(GetPublicKey *msg) if (coin->has_segwit && coin->xpub_magic_segwit_native && script_type == InputScriptType_SPENDWITNESS) { hdnode_serialize_public(node, fingerprint, coin->xpub_magic_segwit_native, resp->xpub, sizeof(resp->xpub)); } else { + memzero(node, sizeof(*node)); fsm_sendFailure(FailureType_Failure_Other, _("Invalid combination of coin and script_type")); layoutHome(); return; @@ -64,17 +65,16 @@ void fsm_msgGetPublicKey(GetPublicKey *msg) if (!confirm_xpub(node_str, resp->xpub)) { + memzero(node, sizeof(*node)); fsm_sendFailure(FailureType_Failure_ActionCancelled, "Show extended public key cancelled"); layoutHome(); return; } } + memzero(node, sizeof(*node)); msg_write(MessageType_MessageType_PublicKey, resp); layoutHome(); - - if (node) - memzero(node, sizeof(node)); } void fsm_msgSignTx(SignTx *msg) @@ -95,6 +95,7 @@ void fsm_msgSignTx(SignTx *msg) layout_simple_message("Preparing Transaction..."); signing_init(msg, coin, node); + memzero(node, sizeof(*node)); } void fsm_msgTxAck(TxAck *msg) @@ -203,6 +204,7 @@ void fsm_msgGetAddress(GetAddress *msg) animating_progress_handler(_("Computing address"), 0); } if (!compute_address(coin, msg->script_type, node, msg->has_multisig, &msg->multisig, address)) { + memzero(node, sizeof(*node)); fsm_sendFailure(FailureType_Failure_Other, _("Can't encode address")); layoutHome(); return; @@ -227,6 +229,7 @@ void fsm_msgGetAddress(GetAddress *msg) if (mismatch) { if (!confirm(ButtonRequestType_ButtonRequest_Other, "WARNING", "Wrong address path for selected coin. Continue at your own risk!")) { + memzero(node, sizeof(*node)); fsm_sendFailure(FailureType_Failure_ActionCancelled, NULL); layoutHome(); return; @@ -239,12 +242,14 @@ void fsm_msgGetAddress(GetAddress *msg) if(!confirm_address(node_str, address + prefix_len)) { + memzero(node, sizeof(*node)); fsm_sendFailure(FailureType_Failure_ActionCancelled, "Show address cancelled"); layoutHome(); return; } } + memzero(node, sizeof(*node)); strlcpy(resp->address, address, sizeof(resp->address)); msg_write(MessageType_MessageType_Address, resp); layoutHome(); @@ -276,14 +281,17 @@ void fsm_msgSignMessage(SignMessage *msg) resp->has_address = true; hdnode_fill_public_key(node); if (!compute_address(coin, msg->script_type, node, false, NULL, resp->address)) { + memzero(node, sizeof(*node)); fsm_sendFailure(FailureType_Failure_Other, _("Error computing address")); layoutHome(); return; } resp->has_signature = true; resp->signature.size = 65; + memzero(node, sizeof(*node)); msg_write(MessageType_MessageType_MessageSignature, resp); } else { + memzero(node, sizeof(*node)); fsm_sendFailure(FailureType_Failure_Other, _("Error signing message")); } layoutHome(); From 3b3dfbd69e8b047ff4d0d8305305c9e53720e6c1 Mon Sep 17 00:00:00 2001 From: keepkeyjon Date: Wed, 5 Feb 2020 16:18:26 -0700 Subject: [PATCH 12/29] firmware: bitcoin signing still uses the root node --- lib/firmware/fsm_msg_coin.h | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/firmware/fsm_msg_coin.h b/lib/firmware/fsm_msg_coin.h index 14c59213e..7609e51a0 100644 --- a/lib/firmware/fsm_msg_coin.h +++ b/lib/firmware/fsm_msg_coin.h @@ -95,7 +95,6 @@ void fsm_msgSignTx(SignTx *msg) layout_simple_message("Preparing Transaction..."); signing_init(msg, coin, node); - memzero(node, sizeof(*node)); } void fsm_msgTxAck(TxAck *msg) From 05e44eab170013034f61ba3dce6d8491323ec8a8 Mon Sep 17 00:00:00 2001 From: keepkeyjon Date: Thu, 6 Feb 2020 10:57:16 -0700 Subject: [PATCH 13/29] update device-protocol --- deps/device-protocol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/deps/device-protocol b/deps/device-protocol index 4ef20fbba..62f3d3a3a 160000 --- a/deps/device-protocol +++ b/deps/device-protocol @@ -1 +1 @@ -Subproject commit 4ef20fbba4a0e2eeb1ddd110b07f818a3cfde94b +Subproject commit 62f3d3a3a602c0355007aba634b0a907b7097d1c From 2a6d3ad8ad6611ceb705be0a1c33e92f11884095 Mon Sep 17 00:00:00 2001 From: keepkeyjon Date: Thu, 6 Feb 2020 11:12:48 -0700 Subject: [PATCH 14/29] firmware: xrp consistent formatting --- lib/firmware/fsm_msg_ripple.h | 200 +++++++++++++++++----------------- 1 file changed, 100 insertions(+), 100 deletions(-) diff --git a/lib/firmware/fsm_msg_ripple.h b/lib/firmware/fsm_msg_ripple.h index 1a4ed4f4a..2535a268b 100644 --- a/lib/firmware/fsm_msg_ripple.h +++ b/lib/firmware/fsm_msg_ripple.h @@ -19,108 +19,108 @@ void fsm_msgRippleGetAddress(const RippleGetAddress *msg) { - RESP_INIT(RippleAddress); - - CHECK_INITIALIZED - - CHECK_PIN - - HDNode *node = fsm_getDerivedNode(SECP256K1_NAME, msg->address_n, msg->address_n_count, NULL); - if (!node) return; - hdnode_fill_public_key(node); - - const CoinType *coin = fsm_getCoin(true, "Ripple"); - - if (!ripple_getAddress(node->public_key, resp->address)) { - memzero(node, sizeof(*node)); - fsm_sendFailure(FailureType_Failure_Other, _("Address derivation failed")); - layoutHome(); - return; - } - - resp->has_address = true; - - if (msg->has_show_display && msg->show_display) { - char node_str[NODE_STRING_LENGTH]; - if (!(bip32_node_to_string(node_str, sizeof(node_str), coin, - msg->address_n, - msg->address_n_count, - /*whole_account=*/false, - /*show_addridx=*/false)) && - !bip32_path_to_string(node_str, sizeof(node_str), - msg->address_n, msg->address_n_count)) { - memset(node_str, 0, sizeof(node_str)); - } - - if (!confirm_ethereum_address(node_str, resp->address)) { - memzero(node, sizeof(*node)); - fsm_sendFailure(FailureType_Failure_ActionCancelled, _("Show address cancelled")); - layoutHome(); - return; - } - } - - memzero(node, sizeof(*node)); - msg_write(MessageType_MessageType_RippleAddress, resp); - layoutHome(); + RESP_INIT(RippleAddress); + + CHECK_INITIALIZED + + CHECK_PIN + + HDNode *node = fsm_getDerivedNode(SECP256K1_NAME, msg->address_n, msg->address_n_count, NULL); + if (!node) return; + hdnode_fill_public_key(node); + + const CoinType *coin = fsm_getCoin(true, "Ripple"); + + if (!ripple_getAddress(node->public_key, resp->address)) { + memzero(node, sizeof(*node)); + fsm_sendFailure(FailureType_Failure_Other, _("Address derivation failed")); + layoutHome(); + return; + } + + resp->has_address = true; + + if (msg->has_show_display && msg->show_display) { + char node_str[NODE_STRING_LENGTH]; + if (!(bip32_node_to_string(node_str, sizeof(node_str), coin, + msg->address_n, + msg->address_n_count, + /*whole_account=*/false, + /*show_addridx=*/false)) && + !bip32_path_to_string(node_str, sizeof(node_str), + msg->address_n, msg->address_n_count)) { + memset(node_str, 0, sizeof(node_str)); + } + + if (!confirm_ethereum_address(node_str, resp->address)) { + memzero(node, sizeof(*node)); + fsm_sendFailure(FailureType_Failure_ActionCancelled, _("Show address cancelled")); + layoutHome(); + return; + } + } + + memzero(node, sizeof(*node)); + msg_write(MessageType_MessageType_RippleAddress, resp); + layoutHome(); } void fsm_msgRippleSignTx(RippleSignTx *msg) { - RESP_INIT(RippleSignedTx); - - CHECK_INITIALIZED - - CHECK_PIN - - bool needs_confirm = true; - - // TODO: handle trades and transfers - - HDNode *node = fsm_getDerivedNode(SECP256K1_NAME, msg->address_n, - msg->address_n_count, NULL); - if (!node) return; - hdnode_fill_public_key(node); - - if (!msg->has_fee || msg->fee < RIPPLE_MIN_FEE || msg->fee > RIPPLE_MAX_FEE) { - memzero(node, sizeof(*node)); - fsm_sendFailure(FailureType_Failure_SyntaxError, - _("Fee must be between 10 and 1,000,000 drops")); - return; - } - - char amount_string[20 + 4 + 1]; - ripple_formatAmount(amount_string, sizeof(amount_string), msg->payment.amount); - - char fee_string[20 + 4 + 1]; - ripple_formatAmount(fee_string, sizeof(fee_string), msg->fee); - - if (needs_confirm) { - if (!confirm(ButtonRequestType_ButtonRequest_ConfirmOutput, - "Send", msg->payment.has_destination_tag - ? "Send %s to %s, with destination tag %" PRIu32 "?" - : "Send %s to %s?", - amount_string, - msg->payment.destination, - msg->payment.destination_tag)) { - memzero(node, sizeof(*node)); - fsm_sendFailure(FailureType_Failure_ActionCancelled, "Signing cancelled"); - layoutHome(); - return; - } - } - - if (!confirm(ButtonRequestType_ButtonRequest_SignTx, - "Transaction", "Really send %s, with a transaction fee of %s?", - amount_string, fee_string)) { - memzero(node, sizeof(*node)); - fsm_sendFailure(FailureType_Failure_ActionCancelled, "Signing cancelled"); - layoutHome(); - return; - } - - ripple_signTx(node, msg, resp); - memzero(node, sizeof(*node)); - msg_write(MessageType_MessageType_RippleSignedTx, resp); - layoutHome(); + RESP_INIT(RippleSignedTx); + + CHECK_INITIALIZED + + CHECK_PIN + + bool needs_confirm = true; + + // TODO: handle trades and transfers + + HDNode *node = fsm_getDerivedNode(SECP256K1_NAME, msg->address_n, + msg->address_n_count, NULL); + if (!node) return; + hdnode_fill_public_key(node); + + if (!msg->has_fee || msg->fee < RIPPLE_MIN_FEE || msg->fee > RIPPLE_MAX_FEE) { + memzero(node, sizeof(*node)); + fsm_sendFailure(FailureType_Failure_SyntaxError, + _("Fee must be between 10 and 1,000,000 drops")); + return; + } + + char amount_string[20 + 4 + 1]; + ripple_formatAmount(amount_string, sizeof(amount_string), msg->payment.amount); + + char fee_string[20 + 4 + 1]; + ripple_formatAmount(fee_string, sizeof(fee_string), msg->fee); + + if (needs_confirm) { + if (!confirm(ButtonRequestType_ButtonRequest_ConfirmOutput, + "Send", msg->payment.has_destination_tag + ? "Send %s to %s, with destination tag %" PRIu32 "?" + : "Send %s to %s?", + amount_string, + msg->payment.destination, + msg->payment.destination_tag)) { + memzero(node, sizeof(*node)); + fsm_sendFailure(FailureType_Failure_ActionCancelled, "Signing cancelled"); + layoutHome(); + return; + } + } + + if (!confirm(ButtonRequestType_ButtonRequest_SignTx, + "Transaction", "Really send %s, with a transaction fee of %s?", + amount_string, fee_string)) { + memzero(node, sizeof(*node)); + fsm_sendFailure(FailureType_Failure_ActionCancelled, "Signing cancelled"); + layoutHome(); + return; + } + + ripple_signTx(node, msg, resp); + memzero(node, sizeof(*node)); + msg_write(MessageType_MessageType_RippleSignedTx, resp); + layoutHome(); } From e78371716b81c4b4839c588261ad7f18e2c24fb7 Mon Sep 17 00:00:00 2001 From: markrpto Date: Wed, 22 Jan 2020 12:02:49 -0700 Subject: [PATCH 15/29] firmware: SCA hardening of wrapped storage key --- .gitmodules | 3 + CMakeLists.txt | 4 +- deps/sca-hardening/CMakeLists.txt | 12 ++ deps/sca-hardening/SecAESSTM32 | 1 + deps/sca-hardening/aes128_cbc.c | 92 ++++++++++++ deps/sca-hardening/include/aes_sca/aes.h | 1 + .../include/aes_sca/aes128_cbc.h | 20 +++ .../include/aes_sca/affine_aes.h | 1 + .../include/aes_sca/compiler_abstraction.h | 1 + docs/Storage.md | 3 +- fuzzer/firmware/CMakeLists.txt | 1 + include/aes_sca | 1 + include/keepkey/firmware/storage.h | 2 +- lib/firmware/storage.c | 134 +++++++++++++++--- lib/firmware/storage.h | 27 +++- lib/firmware/storage_versions.inc | 4 +- tools/emulator/CMakeLists.txt | 1 + tools/firmware/CMakeLists.txt | 1 + unittests/board/CMakeLists.txt | 1 + unittests/firmware/CMakeLists.txt | 1 + unittests/firmware/storage.cpp | 127 ++++++++++++----- 21 files changed, 376 insertions(+), 62 deletions(-) create mode 100644 deps/sca-hardening/CMakeLists.txt create mode 160000 deps/sca-hardening/SecAESSTM32 create mode 100644 deps/sca-hardening/aes128_cbc.c create mode 120000 deps/sca-hardening/include/aes_sca/aes.h create mode 100644 deps/sca-hardening/include/aes_sca/aes128_cbc.h create mode 120000 deps/sca-hardening/include/aes_sca/affine_aes.h create mode 120000 deps/sca-hardening/include/aes_sca/compiler_abstraction.h create mode 120000 include/aes_sca diff --git a/.gitmodules b/.gitmodules index 30597ede5..3bdf70d95 100644 --- a/.gitmodules +++ b/.gitmodules @@ -16,3 +16,6 @@ [submodule "deps/qrenc/QR-Code-generator"] path = deps/qrenc/QR-Code-generator url = https://github.com/keepkey/QR-Code-generator.git +[submodule "deps/sca-hardening/SecAESSTM32"] + path = deps/sca-hardening/SecAESSTM32 + url = https://github.com/keepkey/SecAESSTM32.git diff --git a/CMakeLists.txt b/CMakeLists.txt index 4958d1c59..46129b1cc 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -86,6 +86,8 @@ add_definitions(-DNDEBUG) add_definitions(-DBIP39_WORDLIST_PADDED=1) +add_definitions(-DAES_128=1) + if(${KK_DEBUG_LINK}) add_definitions(-DDEBUG_LINK=1) else() @@ -136,6 +138,7 @@ add_subdirectory(lib) add_subdirectory(tools) add_subdirectory(deps/crypto) add_subdirectory(deps/qrenc) +add_subdirectory(deps/sca-hardening) if(${KK_EMULATOR}) add_subdirectory(deps/googletest) @@ -155,4 +158,3 @@ if(${KK_EMULATOR}) COMMAND ${CMAKE_BINARY_DIR}/bin/crypto-unit --gtest_output=xml:${CMAKE_BINARY_DIR}/unittests/crypto.xml) endif() - diff --git a/deps/sca-hardening/CMakeLists.txt b/deps/sca-hardening/CMakeLists.txt new file mode 100644 index 000000000..1cb1dfee9 --- /dev/null +++ b/deps/sca-hardening/CMakeLists.txt @@ -0,0 +1,12 @@ +set(sources aes128_cbc.c) + +if(NOT ${KK_EMULATOR}) + set(sources ${sources} + SecAESSTM32/src/aes/affine_aes.S + SecAESSTM32/src/aes/aes.c) +endif() + +include_directories( + ${CMAKE_CURRENT_SOURCE_DIR}/SecAESSTM32/src/aes) + +add_library(SecAESSTM32 ${sources}) diff --git a/deps/sca-hardening/SecAESSTM32 b/deps/sca-hardening/SecAESSTM32 new file mode 160000 index 000000000..71d356a11 --- /dev/null +++ b/deps/sca-hardening/SecAESSTM32 @@ -0,0 +1 @@ +Subproject commit 71d356a1141624994cf613bd2d2583892e8e6d5a diff --git a/deps/sca-hardening/aes128_cbc.c b/deps/sca-hardening/aes128_cbc.c new file mode 100644 index 000000000..31d9bfce2 --- /dev/null +++ b/deps/sca-hardening/aes128_cbc.c @@ -0,0 +1,92 @@ +#include "aes_sca/aes.h" +#include "trezor/crypto/aes/aes.h" +#include "trezor/crypto/memzero.h" + +#include + +AES_RETURN aes128_cbc_sca_encrypt(const unsigned char *key, const unsigned char *ibuf, unsigned char *obuf, + int len, unsigned char *iv) +{ +#ifdef EMULATOR +# warning "aes128 encryption is not SCA-hardened in this build." + aes_encrypt_ctx ctx; + aes_encrypt_key128(key, &ctx); + aes_cbc_encrypt(ibuf, obuf, len, iv, &ctx); + memzero(&ctx, sizeof(ctx)); + return EXIT_SUCCESS; +#else + int nb = len >> AES_BLOCK_SIZE_P2; + STRUCT_AES fctx; + unsigned char out[16]; + + if(len & (AES_BLOCK_SIZE - 1)) + return EXIT_FAILURE; + + while(nb--) + { + iv[ 0] ^= ibuf[ 0]; iv[ 1] ^= ibuf[ 1]; + iv[ 2] ^= ibuf[ 2]; iv[ 3] ^= ibuf[ 3]; + iv[ 4] ^= ibuf[ 4]; iv[ 5] ^= ibuf[ 5]; + iv[ 6] ^= ibuf[ 6]; iv[ 7] ^= ibuf[ 7]; + iv[ 8] ^= ibuf[ 8]; iv[ 9] ^= ibuf[ 9]; + iv[10] ^= ibuf[10]; iv[11] ^= ibuf[11]; + iv[12] ^= ibuf[12]; iv[13] ^= ibuf[13]; + iv[14] ^= ibuf[14]; iv[15] ^= ibuf[15]; + +// if(aes_encrypt(iv, iv, ctx) != EXIT_SUCCESS) + if(aes(MODE_KEYINIT|MODE_AESINIT_ENC|MODE_ENC, &fctx, key, iv, out, NULL, NULL) != NO_ERROR) + return EXIT_FAILURE; + memcpy(iv, out, AES_BLOCK_SIZE); + + memcpy(obuf, iv, AES_BLOCK_SIZE); + ibuf += AES_BLOCK_SIZE; + obuf += AES_BLOCK_SIZE; + } + + return EXIT_SUCCESS; +#endif +} + +AES_RETURN aes128_cbc_sca_decrypt(const unsigned char *key, const unsigned char *ibuf, unsigned char *obuf, + int len, unsigned char *iv) +{ +#ifdef EMULATOR +# warning "aes128 decryption is not SCA-hardened in this build." + aes_decrypt_ctx ctx; + aes_decrypt_key128(key, &ctx); + aes_cbc_decrypt(ibuf, obuf, len, iv, &ctx); + memzero(&ctx, sizeof(ctx)); + return EXIT_SUCCESS; +#else + unsigned char tmp[AES_BLOCK_SIZE]; + int nb = len >> AES_BLOCK_SIZE_P2; + STRUCT_AES fctx; + + + if(len & (AES_BLOCK_SIZE - 1)) + return EXIT_FAILURE; + + while(nb--) + { + memcpy(tmp, ibuf, AES_BLOCK_SIZE); + +// if(aes_decrypt(ibuf, obuf, ctx) != EXIT_SUCCESS) + if(aes(MODE_KEYINIT|MODE_AESINIT_DEC|MODE_DEC, &fctx, key, ibuf, obuf, NULL, NULL) != NO_ERROR) + return EXIT_FAILURE; + + obuf[ 0] ^= iv[ 0]; obuf[ 1] ^= iv[ 1]; + obuf[ 2] ^= iv[ 2]; obuf[ 3] ^= iv[ 3]; + obuf[ 4] ^= iv[ 4]; obuf[ 5] ^= iv[ 5]; + obuf[ 6] ^= iv[ 6]; obuf[ 7] ^= iv[ 7]; + obuf[ 8] ^= iv[ 8]; obuf[ 9] ^= iv[ 9]; + obuf[10] ^= iv[10]; obuf[11] ^= iv[11]; + obuf[12] ^= iv[12]; obuf[13] ^= iv[13]; + obuf[14] ^= iv[14]; obuf[15] ^= iv[15]; + memcpy(iv, tmp, AES_BLOCK_SIZE); + ibuf += AES_BLOCK_SIZE; + obuf += AES_BLOCK_SIZE; + } + + return EXIT_SUCCESS; +#endif +} \ No newline at end of file diff --git a/deps/sca-hardening/include/aes_sca/aes.h b/deps/sca-hardening/include/aes_sca/aes.h new file mode 120000 index 000000000..222d7cbe8 --- /dev/null +++ b/deps/sca-hardening/include/aes_sca/aes.h @@ -0,0 +1 @@ +../../SecAESSTM32/src/aes/aes.h \ No newline at end of file diff --git a/deps/sca-hardening/include/aes_sca/aes128_cbc.h b/deps/sca-hardening/include/aes_sca/aes128_cbc.h new file mode 100644 index 000000000..97a3d1d99 --- /dev/null +++ b/deps/sca-hardening/include/aes_sca/aes128_cbc.h @@ -0,0 +1,20 @@ +#ifndef AES128_CBC_H +#define AES128_CBC_H + +#include "trezor/crypto/aes/aes.h" + +AES_RETURN aes128_cbc_sca_encrypt( + const unsigned char *key, + const unsigned char *ibuf, + unsigned char *obuf, + int len, + unsigned char *iv); + +AES_RETURN aes128_cbc_sca_decrypt( + const unsigned char *key, + const unsigned char *ibuf, + unsigned char *obuf, + int len, + unsigned char *iv); + +#endif \ No newline at end of file diff --git a/deps/sca-hardening/include/aes_sca/affine_aes.h b/deps/sca-hardening/include/aes_sca/affine_aes.h new file mode 120000 index 000000000..5e1f35e98 --- /dev/null +++ b/deps/sca-hardening/include/aes_sca/affine_aes.h @@ -0,0 +1 @@ +../../SecAESSTM32/src/aes/affine_aes.h \ No newline at end of file diff --git a/deps/sca-hardening/include/aes_sca/compiler_abstraction.h b/deps/sca-hardening/include/aes_sca/compiler_abstraction.h new file mode 120000 index 000000000..dc45cd249 --- /dev/null +++ b/deps/sca-hardening/include/aes_sca/compiler_abstraction.h @@ -0,0 +1 @@ +../../SecAESSTM32/src/aes/compiler_abstraction.h \ No newline at end of file diff --git a/docs/Storage.md b/docs/Storage.md index 373251914..5142bd562 100644 --- a/docs/Storage.md +++ b/docs/Storage.md @@ -98,7 +98,8 @@ it easier to extend for new features later on. | AdvancedMode policy | bit 12 | | | | no backup (seedless) | bit 13 | | | | has_sec_fingerprint | bit 14 | | | -| reserved | bits 15 - 31 | | | +| sca_hardened | bit 15 | | | +| reserved | bits 16 - 31 | | | | pin_failed_attempts | u32 | 4 | 8 | | auto_lock_delay_ms | u32 | 4 | 12 | | language | char[16] | 16 | 16 | diff --git a/fuzzer/firmware/CMakeLists.txt b/fuzzer/firmware/CMakeLists.txt index 80bbd4e72..d4d31f7d5 100644 --- a/fuzzer/firmware/CMakeLists.txt +++ b/fuzzer/firmware/CMakeLists.txt @@ -15,6 +15,7 @@ set(libraries kkemulator trezorcrypto qrcodegenerator + SecAESSTM32 kkrand kktransport) diff --git a/include/aes_sca b/include/aes_sca new file mode 120000 index 000000000..60d375bdc --- /dev/null +++ b/include/aes_sca @@ -0,0 +1 @@ +../deps/sca-hardening/include/aes_sca \ No newline at end of file diff --git a/include/keepkey/firmware/storage.h b/include/keepkey/firmware/storage.h index 8768a01f1..66ebc5cf3 100644 --- a/include/keepkey/firmware/storage.h +++ b/include/keepkey/firmware/storage.h @@ -23,7 +23,7 @@ #include "trezor/crypto/bip32.h" #include "keepkey/board/memory.h" -#define STORAGE_VERSION 14 /* Must add case fallthrough in storage_fromFlash after increment*/ +#define STORAGE_VERSION 15 /* Must add case fallthrough in storage_fromFlash after increment*/ #define STORAGE_RETRIES 3 #define STORAGE_DEFAULT_SCREENSAVER_TIMEOUT (10U * 60U * 1000U) /* 10 minutes */ diff --git a/lib/firmware/storage.c b/lib/firmware/storage.c index 89a1f8ba9..1f848c885 100644 --- a/lib/firmware/storage.c +++ b/lib/firmware/storage.c @@ -28,6 +28,10 @@ # include #endif +#ifndef EMULATOR +# include "aes_sca/aes128_cbc.h" +#endif + #include "keepkey/board/supervise.h" #include "keepkey/board/keepkey_board.h" #include "keepkey/board/keepkey_flash.h" @@ -276,14 +280,18 @@ void storage_deriveWrappingKey(const char *pin, uint8_t wrapping_key[64]) { void storage_wrapStorageKey(const uint8_t wrapping_key[64], const uint8_t key[64], uint8_t wrapped_key[64]) { uint8_t iv[64]; memcpy(iv, wrapping_key, sizeof(iv)); - aes_encrypt_ctx ctx; - aes_encrypt_key256(wrapping_key, &ctx); - aes_cbc_encrypt(key, wrapped_key, 64, iv + 32, &ctx); - memzero(&ctx, sizeof(ctx)); + aes128_cbc_sca_encrypt(wrapping_key, key, wrapped_key, 64, iv + 32); memzero(iv, sizeof(iv)); } void storage_unwrapStorageKey(const uint8_t wrapping_key[64], const uint8_t wrapped_key[64], uint8_t key[64]) { + uint8_t iv[64]; + memcpy(iv, wrapping_key, sizeof(iv)); + aes128_cbc_sca_decrypt(wrapping_key, wrapped_key, key, 64, iv + 32); + memzero(iv, sizeof(iv)); +} + +void storage_unwrapStorageKey256(const uint8_t wrapping_key[64], const uint8_t wrapped_key[64], uint8_t key[64]) { uint8_t iv[64]; memcpy(iv, wrapping_key, sizeof(iv)); aes_decrypt_ctx ctx; @@ -297,15 +305,45 @@ void storage_keyFingerprint(const uint8_t key[64], uint8_t fingerprint[32]) { sha256_Raw(key, 64, fingerprint); } -bool storage_isPinCorrect_impl(const char *pin, const uint8_t wrapped_key[64], const uint8_t fingerprint[32], uint8_t key[64]) { +pintest_t storage_isPinCorrect_impl(const char *pin, uint8_t wrapped_key[64], const uint8_t fingerprint[32], bool *sca_hardened, uint8_t key[64]) { +/* + This function tests whether the PIN is correct. It will return + PIN_WRONG - PIN is incorrect + PIN_GOOD - PIN is correct + PIN_REWRAP -> PIN is correct, storage key was rewrapped, CALLING FUNCTION SHOULD storage_commit() + + If the pin is correct, the storage key may have been rewrapped and returned in the wrapped_key parameter. + Since the *_impl functions are assumed to not touch the stored config state, the + rewrapped storage key will not have been saved to flash on exit from this function. Thus, if the return is + PIN_REWRAP, then the calling function is required to update the flash with a storage_commit(). +*/ uint8_t wrapping_key[64]; storage_deriveWrappingKey(pin, wrapping_key); - storage_unwrapStorageKey(wrapping_key, wrapped_key, key); + + // unwrap the storage key for fingerprint test + if (*sca_hardened) { + // key was wrapped using the sca-hardened method + storage_unwrapStorageKey(wrapping_key, wrapped_key, key); + } else { + // key was wrapped using deprecated method, unwrap for test + storage_unwrapStorageKey256(wrapping_key, wrapped_key, key); + } uint8_t fp[32]; storage_keyFingerprint(key, fp); - bool ret = memcmp(fp, fingerprint, 32) == 0; + pintest_t ret = PIN_WRONG; + if (memcmp(fp, fingerprint, 32) == 0) + ret = PIN_GOOD; + if (ret == PIN_GOOD) { + if (!*sca_hardened) { + // PIN is correct but storage key needs rewrap + storage_wrapStorageKey(wrapping_key, key, wrapped_key); + *sca_hardened = true; + ret = PIN_REWRAP; + } + } + if (!ret) memzero(key, 64); memzero(wrapping_key, 64); @@ -463,6 +501,7 @@ void storage_readStorageV1(SessionState *ss, Storage *storage, const char *ptr, if (storage->pub.has_pin) { session_clear_impl(ss, storage, /*clear_pin=*/true); + // No need to storage_commit() here, it will get done } } @@ -487,7 +526,8 @@ void storage_writeStorageV11(char *ptr, size_t len, const Storage *storage) { (storage_isPolicyEnabled("AdvancedMode") ? (1u << 12) : 0) | (storage->pub.no_backup ? (1u << 13) : 0) | (storage->has_sec_fingerprint ? (1u << 14) : 0) | - /* reserved 31:15 */ 0; + (storage->pub.sca_hardened ? (1u << 15) : 0) | + /* reserved 31:16 */ 0; write_u32_le(ptr + 4, flags); write_u32_le(ptr + 8, storage->pub.pin_failed_attempts); @@ -541,6 +581,8 @@ void storage_readStorageV11(Storage *storage, const char *ptr, size_t len) { storage_readPolicyV2(&storage->pub.policies[3], "AdvancedMode", flags & (1u << 12)); storage->pub.no_backup = flags & (1u << 13); storage->has_sec_fingerprint = flags & (1u << 14); + storage->pub.sca_hardened = flags & (1u << 15); + storage->pub.policies_count = POLICY_COUNT; storage->pub.pin_failed_attempts = read_u32_le(ptr + 8); @@ -668,6 +710,7 @@ StorageUpdateStatus storage_fromFlash(SessionState *ss, ConfigFlash *dst, const case StorageVersion_12: case StorageVersion_13: case StorageVersion_14: + case StorageVersion_15: storage_readV11(dst, flash, STORAGE_SECTOR_LEN); dst->storage.version = STORAGE_VERSION; return dst->storage.version == version @@ -874,11 +917,27 @@ void storage_wipe(void) } void session_clear(bool clear_pin) { - session_clear_impl(&session, &shadow_config.storage, clear_pin); + if (PIN_REWRAP == session_clear_impl(&session, &shadow_config.storage, clear_pin)) { + storage_commit(); + } } -void session_clear_impl(SessionState *ss, Storage *storage, bool clear_pin) +pintest_t session_clear_impl(SessionState *ss, Storage *storage, bool clear_pin) { +/* + This is a *_impl() function that is assumed to not modify the flash storage config state. Because this function + calls storage_isPinCorrect_impl(), the storage config may need updating: + This function will return + PIN_WRONG - PIN is incorrect + PIN_GOOD - PIN is correct + PIN_REWRAP -> PIN is correct, storage key was rewrapped, CALLING FUNCTION SHOULD storage_commit() + + If the pin is correct, the shadow config may be out of sync with the storage config in flash. + Thus, if the return is PIN_REWRAP, then the calling function is required to update the flash with a + storage_commit(). +*/ + pintest_t ret = PIN_WRONG; + ss->seedCached = false; memset(&ss->seed, 0, sizeof(ss->seed)); @@ -886,21 +945,27 @@ void session_clear_impl(SessionState *ss, Storage *storage, bool clear_pin) memset(&ss->passphrase, 0, sizeof(ss->passphrase)); if (!storage_hasPin_impl(storage)) { - ss->pinCached = - storage_isPinCorrect_impl("", + ret = storage_isPinCorrect_impl("", storage->pub.wrapped_storage_key, storage->pub.storage_key_fingerprint, + &storage->pub.sca_hardened, ss->storageKey); + if (ret == PIN_WRONG) { + ss->pinCached = false; + } else { + ss->pinCached = true; + } + if (!ss->pinCached) goto clear; storage_secMigrate(ss, storage, /*encrypt=*/false); - return; + return(ret); } if (!clear_pin) { - return; + return(ret); } clear: @@ -908,6 +973,7 @@ void session_clear_impl(SessionState *ss, Storage *storage, bool clear_pin) ss->pinCached = false; storage->has_sec = false; memzero(&storage->sec, sizeof(storage->sec)); + return(ret); } void storage_commit(void) @@ -1139,10 +1205,17 @@ const char *storage_getLanguage(void) bool storage_isPinCorrect(const char *pin) { uint8_t storage_key[64]; - bool ret = storage_isPinCorrect_impl(pin, + + pintest_t ret = storage_isPinCorrect_impl(pin, shadow_config.storage.pub.wrapped_storage_key, shadow_config.storage.pub.storage_key_fingerprint, + &shadow_config.storage.pub.sca_hardened, storage_key); + + if (ret == PIN_REWRAP) { + storage_commit(); + } + memzero(storage_key, 64); return ret; } @@ -1181,6 +1254,7 @@ void storage_setPin_impl(SessionState *ss, Storage *storage, const char *pin) // Wrap the new storageKey. storage_wrapStorageKey(wrapping_key, ss->storageKey, storage->pub.wrapped_storage_key); + storage->pub.sca_hardened = true; // Fingerprint the storageKey. storage_keyFingerprint(ss->storageKey, @@ -1196,23 +1270,45 @@ void storage_setPin_impl(SessionState *ss, Storage *storage, const char *pin) void session_cachePin(const char *pin) { - session_cachePin_impl(&session, &shadow_config.storage, pin); + if (PIN_REWRAP == session_cachePin_impl(&session, &shadow_config.storage, pin)) { + storage_commit(); + } } -void session_cachePin_impl(SessionState *ss, Storage *storage, const char *pin) +pintest_t session_cachePin_impl(SessionState *ss, Storage *storage, const char *pin) { - ss->pinCached = +/* + This is a *_impl() function that is assumed to not modify the flash storage config state. Because this function + calls storage_isPinCorrect_impl(), the storage config may need updating: + This function will return + PIN_WRONG - PIN is incorrect + PIN_GOOD - PIN is correct + PIN_REWRAP -> PIN is correct, storage key was rewrapped, CALLING FUNCTION SHOULD storage_commit() + + If the pin is correct, the shadow config may be out of sync with the storage config in flash. + Thus, if the return is PIN_REWRAP, then the calling function is required to update the flash with a + storage_commit(). +*/ + pintest_t ret = storage_isPinCorrect_impl(pin, storage->pub.wrapped_storage_key, storage->pub.storage_key_fingerprint, + &storage->pub.sca_hardened, ss->storageKey); + if (ret == PIN_WRONG) { + ss->pinCached = false; + } else { + ss->pinCached = true; + } + if (!ss->pinCached) { session_clear_impl(ss, storage, /*clear_pin=*/true); - return; + return(ret); } storage_secMigrate(ss, storage, /*encrypt=*/false); + return(ret); } bool session_isPinCached(void) diff --git a/lib/firmware/storage.h b/lib/firmware/storage.h index 773c93189..ff77f928e 100644 --- a/lib/firmware/storage.h +++ b/lib/firmware/storage.h @@ -24,6 +24,7 @@ #include "keepkey/board/keepkey_board.h" #include "keepkey/firmware/policy.h" + typedef struct _Storage { uint32_t version; struct Public { @@ -48,6 +49,7 @@ typedef struct _Storage { HDNodeType u2froot; uint32_t u2f_counter; bool no_backup; + bool sca_hardened; } pub; bool has_sec; @@ -82,6 +84,12 @@ typedef struct _SessionState { char passphrase[51]; } SessionState; +typedef enum { + PIN_WRONG, // PIN incorrect + PIN_GOOD, // PIN correct + PIN_REWRAP // PIN correct but storage key rewrapped, requires storage update +} pintest_t; + #define MAX_MNEMONIC_LEN 240 void storage_loadNode(HDNode *dst, const HDNodeType *src); @@ -95,12 +103,17 @@ void storage_wrapStorageKey(const uint8_t wrapping_key[64], const uint8_t key[64 /// Attempt to unnwrap the storage key. void storage_unwrapStorageKey(const uint8_t wrapping_key[64], const uint8_t wrapped_key[64], uint8_t key[64]); +/// Attempt to unnwrap the storage key using aes256. +void storage_unwrapStorageKey256(const uint8_t wrapping_key[64], const uint8_t wrapped_key[64], uint8_t key[64]); + /// Get the fingerprint for an unwrapped storage key. void storage_keyFingerprint(const uint8_t key[64], uint8_t fingerprint[32]); /// Check whether a pin is correct. -/// \returns true iff the pin was correct. -bool storage_isPinCorrect_impl(const char *pin, const uint8_t wrapped_key[64], const uint8_t fingerprint[32], uint8_t key[64]); +/// \return: PIN_WRONG - PIN is incorrect +/// PIN_GOOD - PIN is correct +/// PIN_REWRAPPED -> PIN is correct, storage key was rewrapped, CALLING FUNCTION SHOULD storage_commit() +pintest_t storage_isPinCorrect_impl(const char *pin, uint8_t wrapped_key[64], const uint8_t fingerprint[32], bool *sca_hardened, uint8_t key[64]); /// Migrate data in Storage to/from sec/encrypted_sec. void storage_secMigrate(SessionState *state, Storage *storage, bool encrypt); @@ -113,9 +126,15 @@ void storage_setPin_impl(SessionState *session, Storage *storage, const char *pi bool storage_hasPin_impl(const Storage *storage); -void session_cachePin_impl(SessionState *session, Storage *storage, const char *pin); +/// \return: PIN_WRONG - PIN is incorrect +/// PIN_GOOD - PIN is correct +/// PIN_REWRAP -> PIN is correct, storage key was rewrapped, CALLING FUNCTION SHOULD storage_commit() +pintest_t session_cachePin_impl(SessionState *session, Storage *storage, const char *pin); -void session_clear_impl(SessionState *session, Storage *storage, bool clear_pin); +/// \return: PIN_WRONG - PIN is incorrect +/// PIN_GOOD - PIN is correct +/// PIN_REWRAP -> PIN is correct, storage key was rewrapped, CALLING FUNCTION SHOULD storage_commit() +pintest_t session_clear_impl(SessionState *session, Storage *storage, bool clear_pin); /// \brief Get user private seed. /// \returns NULL on error, otherwise \returns the private seed. diff --git a/lib/firmware/storage_versions.inc b/lib/firmware/storage_versions.inc index d87e3c22c..23963d8aa 100644 --- a/lib/firmware/storage_versions.inc +++ b/lib/firmware/storage_versions.inc @@ -19,7 +19,9 @@ STORAGE_VERSION_ENTRY(10) STORAGE_VERSION_ENTRY(11) STORAGE_VERSION_ENTRY(12) STORAGE_VERSION_ENTRY(13) -STORAGE_VERSION_LAST(14) +STORAGE_VERSION_ENTRY(14) +STORAGE_VERSION_LAST(15) + #undef STORAGE_VERSION_ENTRY #undef STORAGE_VERSION_LAST diff --git a/tools/emulator/CMakeLists.txt b/tools/emulator/CMakeLists.txt index e44138c8f..7b0574e65 100644 --- a/tools/emulator/CMakeLists.txt +++ b/tools/emulator/CMakeLists.txt @@ -20,6 +20,7 @@ if(${KK_EMULATOR}) kktransport trezorcrypto qrcodegenerator + SecAESSTM32 kkrand kkemulator) endif() diff --git a/tools/firmware/CMakeLists.txt b/tools/firmware/CMakeLists.txt index 358afdb80..18c50e411 100644 --- a/tools/firmware/CMakeLists.txt +++ b/tools/firmware/CMakeLists.txt @@ -36,6 +36,7 @@ if(NOT ${KK_EMULATOR}) kktransport trezorcrypto qrcodegenerator + SecAESSTM32 kkrand -lopencm3_stm32f2 -lc diff --git a/unittests/board/CMakeLists.txt b/unittests/board/CMakeLists.txt index 40a1a5033..a02b1d2b6 100644 --- a/unittests/board/CMakeLists.txt +++ b/unittests/board/CMakeLists.txt @@ -18,6 +18,7 @@ target_link_libraries(board-unit kkboard trezorcrypto qrcodegenerator + SecAESSTM32 kkemulator kkrand kktransport) diff --git a/unittests/firmware/CMakeLists.txt b/unittests/firmware/CMakeLists.txt index 7d055b883..a6a2a4d00 100644 --- a/unittests/firmware/CMakeLists.txt +++ b/unittests/firmware/CMakeLists.txt @@ -28,5 +28,6 @@ target_link_libraries(firmware-unit kkemulator trezorcrypto qrcodegenerator + SecAESSTM32 kkrand kktransport) diff --git a/unittests/firmware/storage.cpp b/unittests/firmware/storage.cpp index 017fc7e74..411d86654 100644 --- a/unittests/firmware/storage.cpp +++ b/unittests/firmware/storage.cpp @@ -3,6 +3,7 @@ extern "C" { #include "keepkey/firmware/policy.h" #include "keepkey/board/keepkey_board.h" #include "trezor/crypto/memzero.h" +#include "trezor/crypto/aes/aes.h" #include "types.pb.h" #include "storage.h" } @@ -481,11 +482,12 @@ TEST(Storage, StorageRoundTrip) { printf("\n"); #endif + const uint8_t expected_flash[] = { 0x73, 0x74, 0x6f, 0x72, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0x00, 0x00, 0x00, STORAGE_VERSION, 0x00, 0x00, 0x00, - 0xff, 0x42, 0x00, 0x00, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, + 0xff, 0xc2, 0x00, 0x00, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, @@ -514,38 +516,38 @@ TEST(Storage, StorageRoundTrip) { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, STORAGE_VERSION, 0x00, 0x00, 0x00, - 0x94, 0x61, 0x3e, 0xe6, 0x6f, 0x79, 0xe8, 0x29, 0xba, 0x49, 0x41, 0xeb, 0x80, 0x8e, 0x4c, 0x2a, - 0x4a, 0x2b, 0x37, 0x08, 0xf5, 0x95, 0x61, 0x89, 0x57, 0xef, 0x98, 0xa1, 0x45, 0x19, 0x78, 0xe7, - 0x57, 0x58, 0xfa, 0x53, 0x1a, 0x10, 0x9a, 0x8b, 0xa4, 0x30, 0xcb, 0xd2, 0x41, 0x94, 0xeb, 0x23, - 0x74, 0xed, 0xb5, 0x5c, 0xce, 0xc9, 0xa4, 0x82, 0x86, 0xed, 0x27, 0x4c, 0x17, 0xe2, 0xd0, 0x51, - 0x57, 0x6e, 0xe5, 0x2e, 0xc3, 0x02, 0xa0, 0x08, 0xdc, 0x4f, 0x51, 0xcd, 0xee, 0x24, 0x84, 0xe4, - 0xc2, 0xb7, 0xed, 0x60, 0xf4, 0x2d, 0xf9, 0x7b, 0x9f, 0x04, 0x8d, 0x0b, 0xc5, 0x69, 0xbd, 0xc4, - 0x9a, 0x87, 0x2a, 0xdd, 0xfc, 0xa9, 0x13, 0xac, 0x93, 0x96, 0xb0, 0xb7, 0x6f, 0x18, 0x57, 0xab, - 0x76, 0x94, 0xaa, 0xe2, 0x82, 0x42, 0xaa, 0x84, 0x74, 0xfb, 0x77, 0x7a, 0x68, 0x6e, 0xd9, 0xcf, - 0x97, 0x55, 0x4d, 0xd3, 0xa3, 0x29, 0xf4, 0xc7, 0x75, 0xee, 0x36, 0x67, 0xc6, 0x97, 0x4a, 0x6e, - 0xf7, 0x4e, 0xab, 0xdb, 0x43, 0xcb, 0x50, 0xac, 0x8c, 0x75, 0xef, 0x7a, 0x30, 0x45, 0x11, 0x65, - 0xa2, 0x61, 0x34, 0x3c, 0x7d, 0x0c, 0x91, 0xee, 0xb3, 0x99, 0x86, 0xc3, 0x06, 0x7b, 0x82, 0xef, - 0xc5, 0xe4, 0x03, 0x3d, 0xf2, 0xc0, 0x03, 0xfe, 0xea, 0x54, 0x7b, 0x7e, 0x8c, 0x01, 0x26, 0x7c, - 0x94, 0x20, 0x99, 0x50, 0xcb, 0xd2, 0x2d, 0xaf, 0x74, 0x92, 0x23, 0x2c, 0xa6, 0xf7, 0x0a, 0x5f, - 0x37, 0x80, 0x03, 0x57, 0xb1, 0x1b, 0x25, 0x95, 0x42, 0x63, 0x7e, 0xf2, 0x76, 0xf5, 0x90, 0xb2, - 0x3f, 0x68, 0x61, 0xab, 0xf4, 0xfb, 0x44, 0x99, 0x44, 0x4d, 0x66, 0x9f, 0x01, 0xd7, 0xab, 0x67, - 0x93, 0x5c, 0x72, 0x94, 0xed, 0x85, 0x25, 0x26, 0xd9, 0xf6, 0xe4, 0x8a, 0x76, 0x8a, 0x1f, 0x24, - 0xad, 0x94, 0x40, 0xdf, 0xec, 0x88, 0x20, 0x6c, 0x05, 0xce, 0xdf, 0x5d, 0x8d, 0xf5, 0xa6, 0xfc, - 0x86, 0x8e, 0xe8, 0x36, 0x8e, 0xd5, 0xe2, 0xbb, 0xe6, 0x3d, 0x6b, 0x5b, 0xe8, 0xcd, 0x11, 0x94, - 0x8d, 0x3f, 0x25, 0xeb, 0xd9, 0x7d, 0x0f, 0xc4, 0xcc, 0xad, 0xe1, 0x91, 0xc4, 0xa7, 0x2e, 0xdd, - 0x11, 0xf5, 0xc7, 0x71, 0xa8, 0xd1, 0xfa, 0x35, 0x69, 0xcc, 0x98, 0x8e, 0xf8, 0x86, 0xfb, 0x8e, - 0xbb, 0x39, 0xf0, 0xf9, 0xc3, 0xda, 0x94, 0x09, 0xf2, 0xc7, 0xd6, 0xa9, 0xf7, 0x40, 0xbe, 0x69, - 0x74, 0xc9, 0xc6, 0xe0, 0xa6, 0x7a, 0x30, 0x4f, 0xa6, 0xe7, 0x13, 0x5e, 0x3a, 0xf1, 0x78, 0xb9, - 0x5c, 0x3b, 0x5d, 0x9f, 0xc0, 0x8d, 0xe9, 0x39, 0xe8, 0xf0, 0x83, 0x73, 0xf5, 0x83, 0xe9, 0xbb, - 0x36, 0x6b, 0xea, 0x8f, 0xd6, 0x4f, 0xab, 0xe6, 0xa0, 0xf1, 0x78, 0x12, 0x90, 0xef, 0x77, 0xf9, - 0x9f, 0xa1, 0x87, 0x29, 0xce, 0xa7, 0xa0, 0x1b, 0x0c, 0xd6, 0xcb, 0x06, 0x34, 0xe2, 0x11, 0xf3, - 0xb7, 0xbc, 0xb6, 0x91, 0x80, 0xf4, 0xed, 0x46, 0x7a, 0x36, 0x4b, 0x45, 0x50, 0xd0, 0x65, 0x2f, - 0x7a, 0x38, 0x52, 0xcb, 0x0e, 0x64, 0x79, 0x76, 0x9c, 0x41, 0x36, 0xcc, 0xc4, 0x11, 0x85, 0xb3, - 0x7d, 0x0f, 0xea, 0x95, 0x22, 0x95, 0x08, 0x3e, 0xbe, 0xc5, 0xed, 0x00, 0x6f, 0xca, 0x51, 0x37, - 0x34, 0x07, 0x33, 0xa2, 0xbb, 0x91, 0x51, 0x60, 0x8c, 0x5c, 0x0f, 0xd7, 0x05, 0x5b, 0x3a, 0x61, - 0x30, 0xef, 0xa3, 0x0c, 0xa9, 0x15, 0xd9, 0x37, 0x6a, 0xa7, 0x57, 0xeb, 0xc3, 0xb1, 0x5a, 0x82, - 0xdf, 0xd6, 0x67, 0x46, 0x9c, 0xdc, 0xaa, 0x51, 0x1b, 0x43, 0xc2, 0xf3, 0x8a, 0xfd, 0x7b, 0xba, - 0x59, 0x08, 0x70, 0x8a, 0x71, 0x9f, 0x18, 0x50, 0x87, 0x6a, 0x16, 0x1f, 0x14, 0xbd, 0xfe, 0x19, + 0xe4, 0x8d, 0xfe, 0xcf, 0xd0, 0x54, 0x71, 0x50, 0xcb, 0x12, 0x84, 0xfa, 0x5f, 0xbf, 0xcb, 0x09, + 0xca, 0x00, 0xf1, 0x37, 0xe4, 0x8f, 0x5e, 0xf9, 0x81, 0x57, 0x26, 0xb6, 0x7b, 0x8e, 0x03, 0x44, + 0x9a, 0x2a, 0x7c, 0xf4, 0x3c, 0x79, 0x87, 0x5d, 0x26, 0xae, 0x9b, 0x4b, 0xb4, 0xd2, 0xc4, 0x67, + 0x97, 0xe7, 0x6b, 0x6c, 0x4c, 0xbe, 0x68, 0x19, 0x4c, 0x28, 0xf0, 0x5d, 0xef, 0xe8, 0x36, 0x20, + 0x0e, 0x4e, 0x0d, 0x5a, 0x9a, 0xc6, 0x09, 0x37, 0x38, 0xcb, 0x70, 0xb1, 0x90, 0x9d, 0xe7, 0x2c, + 0x4b, 0x32, 0x91, 0x41, 0x1d, 0xda, 0x38, 0x8c, 0xc7, 0x7b, 0x24, 0xc2, 0x5f, 0x40, 0xae, 0xfb, + 0xf8, 0xe7, 0xcd, 0x9e, 0xb5, 0x85, 0x29, 0xdb, 0xa3, 0x70, 0xc3, 0x1b, 0x56, 0xf2, 0x03, 0xfb, + 0xf8, 0xbe, 0xf6, 0x0d, 0x08, 0x00, 0xb4, 0xf8, 0x3b, 0x28, 0xdc, 0x9e, 0x56, 0xf4, 0x86, 0x0f, + 0x86, 0xbb, 0x54, 0xb6, 0x0e, 0x03, 0x78, 0x98, 0x53, 0xeb, 0xbc, 0xe7, 0xb4, 0x5f, 0xd6, 0x3a, + 0x7a, 0xc3, 0xfd, 0x7a, 0x1e, 0xe5, 0x8b, 0x55, 0x03, 0x3d, 0x32, 0x9a, 0x9c, 0x1b, 0x58, 0xdd, + 0xca, 0x23, 0x8d, 0x3b, 0x52, 0x71, 0x7c, 0x66, 0x2f, 0x83, 0xaa, 0xc8, 0xc9, 0xb3, 0xcc, 0xb3, + 0x82, 0x88, 0xac, 0x65, 0x1c, 0xf2, 0xe9, 0x0e, 0x94, 0xff, 0xeb, 0xfb, 0xbe, 0x71, 0x3f, 0x53, + 0x79, 0x73, 0x49, 0x9c, 0x25, 0xe5, 0xf1, 0xe4, 0xf6, 0xcf, 0x29, 0x80, 0x17, 0x6f, 0x1e, 0x94, + 0xf3, 0x79, 0x7f, 0xb0, 0x31, 0x75, 0xeb, 0xf6, 0xd3, 0x11, 0xf3, 0xb1, 0xea, 0xbe, 0x7d, 0x7c, + 0x39, 0xd9, 0x59, 0x7a, 0x30, 0x76, 0xce, 0xc5, 0x20, 0x63, 0xc2, 0x42, 0x95, 0xa5, 0xbc, 0xf3, + 0xe6, 0x39, 0x13, 0xd6, 0xea, 0x7a, 0xf5, 0xf3, 0x68, 0xed, 0x34, 0x63, 0x76, 0xf9, 0xf6, 0x28, + 0x9e, 0x5a, 0x79, 0x76, 0xa7, 0xbd, 0x67, 0x7f, 0xcc, 0xbc, 0xeb, 0x8d, 0x70, 0xbf, 0x4b, 0xaf, + 0xe9, 0x60, 0xb2, 0x90, 0x7d, 0xed, 0x98, 0x6e, 0x35, 0x64, 0x64, 0xdc, 0xf9, 0x79, 0xcc, 0x2c, + 0xfb, 0x94, 0x25, 0xbe, 0xb3, 0xc0, 0x12, 0xc2, 0x5e, 0xb0, 0x8e, 0x5c, 0x4a, 0x92, 0x2a, 0x71, + 0x87, 0xc1, 0x21, 0x6a, 0xb3, 0xed, 0x87, 0x7c, 0xfa, 0xff, 0xc0, 0xcd, 0x6c, 0xd4, 0xf7, 0x54, + 0xe9, 0x54, 0xdc, 0xa7, 0xb3, 0x8a, 0xa5, 0x0a, 0xd4, 0x02, 0xe1, 0xdf, 0x4c, 0xdf, 0x6c, 0xeb, + 0x97, 0xd3, 0x97, 0x29, 0x68, 0xde, 0x50, 0x2f, 0x7c, 0xeb, 0xc4, 0x1a, 0x40, 0x7f, 0x69, 0x4f, + 0xb5, 0x4f, 0x81, 0x64, 0x30, 0x49, 0xd7, 0x01, 0x7a, 0xd7, 0x55, 0x19, 0xb6, 0x33, 0xde, 0x0d, + 0x13, 0x75, 0xf7, 0x57, 0xd3, 0x81, 0xb8, 0xdd, 0x8c, 0x67, 0x73, 0xe0, 0x76, 0xb6, 0x44, 0x9f, + 0x6a, 0x33, 0x7b, 0x60, 0x60, 0x0b, 0xef, 0x23, 0x8b, 0x9d, 0x8c, 0x17, 0x1b, 0x02, 0xef, 0xf5, + 0x10, 0xcf, 0x5a, 0x8e, 0x3b, 0x00, 0x12, 0xb0, 0x45, 0x8e, 0x12, 0x57, 0x81, 0x5f, 0xfb, 0xfd, + 0x04, 0xff, 0xbc, 0xf4, 0x4c, 0xbf, 0xb5, 0x06, 0x10, 0xc1, 0xd8, 0x5e, 0x80, 0x17, 0x06, 0xda, + 0x37, 0x11, 0xba, 0x77, 0x61, 0xe5, 0xd6, 0x4a, 0x0d, 0xd8, 0x6f, 0xd1, 0xd4, 0x57, 0xba, 0xe6, + 0x8f, 0xd8, 0x7a, 0xfa, 0x4f, 0x35, 0x12, 0xdb, 0xbb, 0x34, 0xdf, 0x3f, 0x24, 0x30, 0x95, 0x42, + 0xe9, 0xc4, 0x71, 0xed, 0x0b, 0x4a, 0x9a, 0x61, 0xfa, 0x79, 0xc6, 0x5d, 0x1b, 0x0d, 0x34, 0x61, + 0x5f, 0x45, 0xc9, 0xa1, 0x01, 0xdf, 0x19, 0x3e, 0x7f, 0x3e, 0xb7, 0x2f, 0x0f, 0xdc, 0x26, 0x45, + 0xa2, 0xa0, 0xaa, 0xf9, 0xe1, 0x2b, 0x6b, 0x23, 0x19, 0xb7, 0x03, 0xd0, 0xa1, 0xb9, 0x88, 0x11, }; EXPECT_TRUE(memcmp(&flash[0], expected_flash, sizeof(expected_flash)) == 0); @@ -609,13 +611,66 @@ TEST(Storage, IsPinCorrect) { uint8_t fingerprint[32]; storage_keyFingerprint(storage_key, fingerprint); - uint8_t key_out[64]; - EXPECT_TRUE(storage_isPinCorrect_impl("1234", wrapped_key, fingerprint, key_out)); + uint8_t key_out[64]; bool sca_hardened = true; + EXPECT_TRUE(storage_isPinCorrect_impl("1234", wrapped_key, fingerprint, &sca_hardened, key_out)); EXPECT_TRUE(memcmp(key_out, storage_key, 64) == 0); } +TEST(Storage, Vuln1996) { + ConfigFlash config; + SessionState session; + uint8_t wrapping_key[64], wrapped_key1[64], storage_key[64]; + + struct { + const char *pin; + } vec[] = { + { "" }, + { "1234" }, + { "000000000" }, // etc + }; + + for (const auto &v : vec) { + memset(&session, 0, sizeof(session)); + memset(&config, 0, sizeof(config)); + storage_reset_impl(&session, &config); + + storage_setPin_impl(&session, &config.storage, v.pin); + + ASSERT_TRUE(PIN_GOOD == storage_isPinCorrect_impl(v.pin, + config.storage.pub.wrapped_storage_key, + config.storage.pub.storage_key_fingerprint, + &config.storage.pub.sca_hardened, + storage_key)); + ASSERT_TRUE(config.storage.pub.sca_hardened == true); + memcpy(wrapped_key1, config.storage.pub.wrapped_storage_key, sizeof(wrapped_key1)); + + // Check storage wrapping update by starting with unhardened aes256 version + storage_deriveWrappingKey(v.pin, wrapping_key); + storage_unwrapStorageKey(wrapping_key, config.storage.pub.wrapped_storage_key, storage_key); + uint8_t iv[64]; + memcpy(iv, wrapping_key, sizeof(iv)); + aes_encrypt_ctx ctx; + aes_encrypt_key256(wrapping_key, &ctx); + aes_cbc_encrypt(storage_key, config.storage.pub.wrapped_storage_key, 64, iv + 32, &ctx); + config.storage.pub.sca_hardened = false; + + // wrapped_key1 under aes128. Config version is wrapped under aes256 for test. This check ensures + // that test conditions are correct. + ASSERT_TRUE(memcmp(wrapped_key1, config.storage.pub.wrapped_storage_key, sizeof(wrapped_key1)) != 0); + + ASSERT_TRUE(storage_isPinCorrect_impl(v.pin, + config.storage.pub.wrapped_storage_key, + config.storage.pub.storage_key_fingerprint, + &config.storage.pub.sca_hardened, + storage_key)); + ASSERT_TRUE(memcmp(wrapped_key1, config.storage.pub.wrapped_storage_key, sizeof(wrapped_key1)) == 0); + ASSERT_TRUE(config.storage.pub.sca_hardened == true); + } +} + TEST(Storage, Pin) { + storage_setPin(""); ASSERT_TRUE(storage_isPinCorrect("")); @@ -660,6 +715,7 @@ TEST(Storage, Reset) { ASSERT_TRUE(storage_isPinCorrect_impl("", config.storage.pub.wrapped_storage_key, config.storage.pub.storage_key_fingerprint, + &config.storage.pub.sca_hardened, session.storageKey)); uint8_t old_storage_key[64]; @@ -673,6 +729,7 @@ TEST(Storage, Reset) { ASSERT_TRUE(storage_isPinCorrect_impl("1234", config.storage.pub.wrapped_storage_key, config.storage.pub.storage_key_fingerprint, + &config.storage.pub.sca_hardened, new_storage_key)); ASSERT_TRUE(memcmp(session.storageKey, new_storage_key, 64) == 0); From 24330a664398062b4f4fe352eda3365173e67cfe Mon Sep 17 00:00:00 2001 From: keepkeyjon Date: Thu, 13 Feb 2020 08:57:32 -0700 Subject: [PATCH 16/29] firmware: key stretch the user's pin with pbkdf2 ... salted with permanent entropy in the device's OTP block, and semi-permanent entropy placed in the storage sector. --- docs/Storage.md | 3 +- include/keepkey/board/keepkey_flash.h | 3 + include/keepkey/board/otp.h | 38 ++++++++++++ include/keepkey/firmware/storage.h | 2 + lib/board/CMakeLists.txt | 1 + lib/board/common.c | 9 ++- lib/board/keepkey_flash.c | 51 ++++++++++++---- lib/board/otp.c | 68 +++++++++++++++++++++ lib/firmware/storage.c | 85 +++++++++++++++++++++------ lib/firmware/storage.h | 11 ++-- tools/emulator/main.cpp | 1 + tools/firmware/keepkey_main.c | 2 + unittests/firmware/storage.cpp | 68 +++++++++++---------- 13 files changed, 276 insertions(+), 66 deletions(-) create mode 100644 include/keepkey/board/otp.h create mode 100644 lib/board/otp.c diff --git a/docs/Storage.md b/docs/Storage.md index 5142bd562..f35cb5e24 100644 --- a/docs/Storage.md +++ b/docs/Storage.md @@ -109,7 +109,8 @@ it easier to extend for new features later on. | u2froot | StorageHDNode | 129 | 176 | | u2f_counter | u32 | 4 | 305 | | sec_fingerprint | char[32] | 32 | 309 | -| reserved | char[123] | 123 | 341 | +| random_salt | char[32] | 32 | 341 | +| reserved | char[91] | 91 | 373 | | encrypted_secrets_version | u32 | 4 | 464 | | encrypted_secrets | char[512] | 512 | 468 | diff --git a/include/keepkey/board/keepkey_flash.h b/include/keepkey/board/keepkey_flash.h index 574fa58e8..c504f1abe 100644 --- a/include/keepkey/board/keepkey_flash.h +++ b/include/keepkey/board/keepkey_flash.h @@ -40,4 +40,7 @@ bool set_mfg_mode_off(void); const char *flash_getModel(void); bool flash_setModel(const char (*model)[32]); const char *flash_programModel(void); + +void flash_collectHWEntropy(bool privileged); +void flash_readHWEntropy(uint8_t *buff, size_t size); #endif diff --git a/include/keepkey/board/otp.h b/include/keepkey/board/otp.h new file mode 100644 index 000000000..2e25f1158 --- /dev/null +++ b/include/keepkey/board/otp.h @@ -0,0 +1,38 @@ +/* + * This file is part of the Trezor project, https://trezor.io/ + * + * Copyright (C) 2019 Pavol Rusnak + * + * This library is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this library. If not, see . + */ + +#ifndef __OTP_H__ +#define __OTP_H__ + +#include +#include + +#define FLASH_OTP_NUM_BLOCKS 16 +#define FLASH_OTP_BLOCK_SIZE 32 + +#define FLASH_OTP_BLOCK_RANDOMNESS 3 + +bool flash_otp_is_locked(uint8_t block); +bool flash_otp_lock(uint8_t block); +bool flash_otp_read(uint8_t block, uint8_t offset, uint8_t *data, + uint8_t datalen); +bool flash_otp_write(uint8_t block, uint8_t offset, const uint8_t *data, + uint8_t datalen); + +#endif diff --git a/include/keepkey/firmware/storage.h b/include/keepkey/firmware/storage.h index 66ebc5cf3..e47b25672 100644 --- a/include/keepkey/firmware/storage.h +++ b/include/keepkey/firmware/storage.h @@ -26,6 +26,8 @@ #define STORAGE_VERSION 15 /* Must add case fallthrough in storage_fromFlash after increment*/ #define STORAGE_RETRIES 3 +#define RANDOM_SALT_LEN 32 + #define STORAGE_DEFAULT_SCREENSAVER_TIMEOUT (10U * 60U * 1000U) /* 10 minutes */ #define STORAGE_MIN_SCREENSAVER_TIMEOUT ( 30U * 1000U) /* 30 seconds */ diff --git a/lib/board/CMakeLists.txt b/lib/board/CMakeLists.txt index 01c1e3773..cebca149d 100644 --- a/lib/board/CMakeLists.txt +++ b/lib/board/CMakeLists.txt @@ -40,6 +40,7 @@ if(${KK_EMULATOR}) udp.c) else() set(sources ${sources} + otp.c usb21_standard.c webusb.c winusb.c) diff --git a/lib/board/common.c b/lib/board/common.c index 07c672a66..85b3cb554 100644 --- a/lib/board/common.c +++ b/lib/board/common.c @@ -19,13 +19,18 @@ #include "keepkey/board/common.h" +#include "keepkey/board/otp.h" #include "keepkey/rand/rng.h" #include "trezor/crypto/hmac_drbg.h" #include "trezor/crypto/rand.h" -#include +#ifndef EMULATOR +# include +#endif -uint8_t HW_ENTROPY_DATA[HW_ENTROPY_LEN]; +#include +#include +#include static HMAC_DRBG_CTX drbg_ctx; diff --git a/lib/board/keepkey_flash.c b/lib/board/keepkey_flash.c index 6e335f5ce..7cc07025b 100644 --- a/lib/board/keepkey_flash.c +++ b/lib/board/keepkey_flash.c @@ -20,19 +20,26 @@ #ifndef EMULATOR # include +# include #else # include # include #endif -#include "keepkey/board/supervise.h" -#include "keepkey/board/keepkey_flash.h" - #include "keepkey/board/check_bootloader.h" +#include "keepkey/board/common.h" +#include "keepkey/board/otp.h" +#include "keepkey/board/keepkey_flash.h" +#include "keepkey/board/supervise.h" +#include "keepkey/board/util.h" +#include "keepkey/rand/rng.h" +#include "trezor/crypto/memzero.h" +#include "trezor/crypto/rand.h" #include #include +uint8_t HW_ENTROPY_DATA[HW_ENTROPY_LEN]; /* * flash_write_helper() - Helper function to locate starting address of @@ -105,9 +112,6 @@ void flash_erase_word(Allocation group) #endif } - - - /* * flash_write_word() - Flash write in word (32bit) size * @@ -244,8 +248,6 @@ bool set_mfg_mode_off(void) return(ret_val); } - - const char *flash_getModel(void) { #ifndef EMULATOR if (*((uint8_t*)OTP_MODEL_ADDR) == 0xFF) @@ -258,9 +260,6 @@ const char *flash_getModel(void) { #endif } - - - bool flash_setModel(const char (*model)[MODEL_STR_SIZE]) { #ifndef EMULATOR // Check OTP lock state before updating @@ -316,3 +315,33 @@ const char *flash_programModel(void) { return "Unknown"; #endif } + +void flash_collectHWEntropy(bool privileged) { +#ifdef EMULATOR + (void)privileged; + memzero(HW_ENTROPY_DATA, HW_ENTROPY_LEN); +#else + if (privileged) { + desig_get_unique_id((uint32_t *)HW_ENTROPY_DATA); + // set entropy in the OTP randomness block + if (!flash_otp_is_locked(FLASH_OTP_BLOCK_RANDOMNESS)) { + uint8_t entropy[FLASH_OTP_BLOCK_SIZE] = {0}; + random_buffer(entropy, FLASH_OTP_BLOCK_SIZE); + flash_otp_write(FLASH_OTP_BLOCK_RANDOMNESS, 0, entropy, + FLASH_OTP_BLOCK_SIZE); + flash_otp_lock(FLASH_OTP_BLOCK_RANDOMNESS); + } + // collect entropy from OTP randomness block + flash_otp_read(FLASH_OTP_BLOCK_RANDOMNESS, 0, HW_ENTROPY_DATA + 12, + FLASH_OTP_BLOCK_SIZE); + } else { + // unprivileged mode => use fixed HW_ENTROPY + memset(HW_ENTROPY_DATA, 0x3C, HW_ENTROPY_LEN); + } +#endif +} + +void flash_readHWEntropy(uint8_t *buff, size_t size) +{ + memcpy(buff, HW_ENTROPY_DATA, MIN(sizeof(HW_ENTROPY_DATA), size)); +} diff --git a/lib/board/otp.c b/lib/board/otp.c new file mode 100644 index 000000000..ffd6d46f9 --- /dev/null +++ b/lib/board/otp.c @@ -0,0 +1,68 @@ +/* + * This file is part of the Trezor project, https://trezor.io/ + * + * Copyright (C) 2019 Pavol Rusnak + * + * This library is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this library. If not, see . + */ + +#include "keepkey/board/otp.h" + +#include + +#define FLASH_OTP_BASE 0x1FFF7800U +#define FLASH_OTP_LOCK_BASE 0x1FFF7A00U + +bool flash_otp_is_locked(uint8_t block) { + return 0x00 == *(volatile uint8_t *)(FLASH_OTP_LOCK_BASE + block); +} + +bool flash_otp_lock(uint8_t block) { + if (block >= FLASH_OTP_NUM_BLOCKS) { + return false; + } + flash_unlock(); + flash_program_byte(FLASH_OTP_LOCK_BASE + block, 0x00); + flash_lock(); + return true; +} + +bool flash_otp_read(uint8_t block, uint8_t offset, uint8_t *data, + uint8_t datalen) { + if (block >= FLASH_OTP_NUM_BLOCKS || + offset + datalen > FLASH_OTP_BLOCK_SIZE) { + return false; + } + for (uint8_t i = 0; i < datalen; i++) { + data[i] = *(volatile uint8_t *)(FLASH_OTP_BASE + + block * FLASH_OTP_BLOCK_SIZE + offset + i); + } + return true; +} + +bool flash_otp_write(uint8_t block, uint8_t offset, const uint8_t *data, + uint8_t datalen) { + if (block >= FLASH_OTP_NUM_BLOCKS || + offset + datalen > FLASH_OTP_BLOCK_SIZE) { + return false; + } + flash_unlock(); + for (uint8_t i = 0; i < datalen; i++) { + uint32_t address = + FLASH_OTP_BASE + block * FLASH_OTP_BLOCK_SIZE + offset + i; + flash_program_byte(address, data[i]); + } + flash_lock(); + return true; +} diff --git a/lib/firmware/storage.c b/lib/firmware/storage.c index 1f848c885..3a48eb5fa 100644 --- a/lib/firmware/storage.c +++ b/lib/firmware/storage.c @@ -32,6 +32,7 @@ # include "aes_sca/aes128_cbc.h" #endif +#include "keepkey/board/common.h" #include "keepkey/board/supervise.h" #include "keepkey/board/keepkey_board.h" #include "keepkey/board/keepkey_flash.h" @@ -55,6 +56,14 @@ #include #include +#ifdef EMULATOR +# define PIN_ITER_COUNT 1000 +# define PIN_ITER_CHUNK 10 +#else +# define PIN_ITER_COUNT 100000 +# define PIN_ITER_CHUNK 1000 +#endif + #define U2F_KEY_PATH 0x80553246 #define _(X) (X) @@ -273,8 +282,38 @@ void storage_writeHDNode(char *ptr, size_t len, const HDNodeType *node) { memcpy(ptr + 96, node->public_key.bytes, 33); } -void storage_deriveWrappingKey(const char *pin, uint8_t wrapping_key[64]) { - sha512_Raw((const uint8_t*)pin, strlen(pin), wrapping_key); +void storage_deriveWrappingKey( + const char *pin, uint8_t wrapping_key[64], bool sca_hardened, + uint8_t random_salt[RANDOM_SALT_LEN]) +{ + size_t pin_len = strlen(pin); + if (sca_hardened && pin_len > 0) { + uint8_t salt[HW_ENTROPY_LEN + RANDOM_SALT_LEN]; + memset(salt, 0, sizeof(salt)); + flash_readHWEntropy(salt, sizeof(salt)); + memcpy(salt + HW_ENTROPY_LEN, random_salt, RANDOM_SALT_LEN); + + PBKDF2_HMAC_SHA256_CTX ctx = {0}; + pbkdf2_hmac_sha256_Init(&ctx, (const uint8_t*)pin, pin_len, salt, sizeof(salt), 1); + for (int i = 0; i < PIN_ITER_COUNT; i += PIN_ITER_CHUNK) { + layoutProgress(_("Verifying PIN"), 1000 * i / (PIN_ITER_COUNT * 2)); + pbkdf2_hmac_sha256_Update(&ctx, PIN_ITER_CHUNK); + } + pbkdf2_hmac_sha256_Final(&ctx, wrapping_key); + + memzero(&ctx, sizeof(ctx)); + pbkdf2_hmac_sha256_Init(&ctx, (const uint8_t*)pin, pin_len, salt, sizeof(salt), 2); + for (int i = 0; i < PIN_ITER_COUNT; i += PIN_ITER_CHUNK) { + layoutProgress(_("Verifying PIN"), 1000 * (i + PIN_ITER_COUNT) / (PIN_ITER_COUNT * 2)); + pbkdf2_hmac_sha256_Update(&ctx, PIN_ITER_CHUNK); + } + layoutProgress(_("Verifying PIN"), 1000); + pbkdf2_hmac_sha256_Final(&ctx, wrapping_key + 32); + + memzero(salt, sizeof(salt)); + } else { + sha512_Raw((const uint8_t*)pin, strlen(pin), wrapping_key); + } } void storage_wrapStorageKey(const uint8_t wrapping_key[64], const uint8_t key[64], uint8_t wrapped_key[64]) { @@ -305,7 +344,10 @@ void storage_keyFingerprint(const uint8_t key[64], uint8_t fingerprint[32]) { sha256_Raw(key, 64, fingerprint); } -pintest_t storage_isPinCorrect_impl(const char *pin, uint8_t wrapped_key[64], const uint8_t fingerprint[32], bool *sca_hardened, uint8_t key[64]) { +pintest_t storage_isPinCorrect_impl( + const char *pin, uint8_t wrapped_key[64], const uint8_t fingerprint[32], + bool *sca_hardened, uint8_t key[64], uint8_t random_salt[RANDOM_SALT_LEN]) +{ /* This function tests whether the PIN is correct. It will return PIN_WRONG - PIN is incorrect @@ -318,7 +360,7 @@ pintest_t storage_isPinCorrect_impl(const char *pin, uint8_t wrapped_key[64], co PIN_REWRAP, then the calling function is required to update the flash with a storage_commit(). */ uint8_t wrapping_key[64]; - storage_deriveWrappingKey(pin, wrapping_key); + storage_deriveWrappingKey(pin, wrapping_key, sca_hardened, random_salt); // unwrap the storage key for fingerprint test if (*sca_hardened) { @@ -335,6 +377,7 @@ pintest_t storage_isPinCorrect_impl(const char *pin, uint8_t wrapped_key[64], co pintest_t ret = PIN_WRONG; if (memcmp(fp, fingerprint, 32) == 0) ret = PIN_GOOD; + if (ret == PIN_GOOD) { if (!*sca_hardened) { // PIN is correct but storage key needs rewrap @@ -487,6 +530,8 @@ void storage_readStorageV1(SessionState *ss, Storage *storage, const char *ptr, _Static_assert(sizeof(storage->pub.storage_key_fingerprint) == 32, "key fingerprint must be 32 bytes"); + random_buffer(storage->pub.random_salt, 32); + storage->has_sec = true; storage_setPin_impl(ss, storage, storage->pub.has_pin ? storage->sec.pin : ""); @@ -546,7 +591,9 @@ void storage_writeStorageV11(char *ptr, size_t len, const Storage *storage) { memcpy(ptr + 309, storage->sec_fingerprint, 32); } - // 123 reserved bytes + memcpy(ptr + 341, storage->pub.random_salt, 32); + + // 91 reserved bytes // Ignore whatever was in storage->sec. Only encrypted_sec can be committed. // Yes, this is a potential footgun. No, there's nothing we can do about it here. @@ -607,7 +654,9 @@ void storage_readStorageV11(Storage *storage, const char *ptr, size_t len) { memset(storage->sec_fingerprint, 0, sizeof(storage->sec_fingerprint)); } - // 123 reserved bytes + memcpy(storage->pub.random_salt, ptr + 341, 32); + + // 91 reserved bytes storage->has_sec = false; memzero(&storage->sec, sizeof(storage->sec)); @@ -928,8 +977,8 @@ pintest_t session_clear_impl(SessionState *ss, Storage *storage, bool clear_pin) This is a *_impl() function that is assumed to not modify the flash storage config state. Because this function calls storage_isPinCorrect_impl(), the storage config may need updating: This function will return - PIN_WRONG - PIN is incorrect - PIN_GOOD - PIN is correct + PIN_WRONG - PIN is incorrect + PIN_GOOD - PIN is correct PIN_REWRAP -> PIN is correct, storage key was rewrapped, CALLING FUNCTION SHOULD storage_commit() If the pin is correct, the shadow config may be out of sync with the storage config in flash. @@ -945,11 +994,12 @@ pintest_t session_clear_impl(SessionState *ss, Storage *storage, bool clear_pin) memset(&ss->passphrase, 0, sizeof(ss->passphrase)); if (!storage_hasPin_impl(storage)) { - ret = storage_isPinCorrect_impl("", - storage->pub.wrapped_storage_key, - storage->pub.storage_key_fingerprint, - &storage->pub.sca_hardened, - ss->storageKey); + ret = storage_isPinCorrect_impl("", + storage->pub.wrapped_storage_key, + storage->pub.storage_key_fingerprint, + &storage->pub.sca_hardened, + ss->storageKey, + shadow_config.storage.pub.random_salt); if (ret == PIN_WRONG) { ss->pinCached = false; @@ -1210,7 +1260,8 @@ bool storage_isPinCorrect(const char *pin) { shadow_config.storage.pub.wrapped_storage_key, shadow_config.storage.pub.storage_key_fingerprint, &shadow_config.storage.pub.sca_hardened, - storage_key); + storage_key, + shadow_config.storage.pub.random_salt); if (ret == PIN_REWRAP) { storage_commit(); @@ -1239,14 +1290,13 @@ void storage_setPin(const char *pin) #if DEBUG_LINK strncpy(debuglink_pin, pin, sizeof(debuglink_pin)); #endif - } void storage_setPin_impl(SessionState *ss, Storage *storage, const char *pin) { // Derive the wrapping key for the new pin uint8_t wrapping_key[64]; - storage_deriveWrappingKey(pin, wrapping_key); + storage_deriveWrappingKey(pin, wrapping_key, /*sca_hardened=*/true, storage->pub.random_salt); // Derive a new storageKey. random_buffer(ss->storageKey, 64); @@ -1294,7 +1344,8 @@ pintest_t session_cachePin_impl(SessionState *ss, Storage *storage, const char * storage->pub.wrapped_storage_key, storage->pub.storage_key_fingerprint, &storage->pub.sca_hardened, - ss->storageKey); + ss->storageKey, + shadow_config.storage.pub.random_salt); if (ret == PIN_WRONG) { ss->pinCached = false; diff --git a/lib/firmware/storage.h b/lib/firmware/storage.h index ff77f928e..28c36133f 100644 --- a/lib/firmware/storage.h +++ b/lib/firmware/storage.h @@ -22,9 +22,9 @@ #define STORAGEPB_H #include "keepkey/board/keepkey_board.h" +#include "keepkey/firmware/storage.h" #include "keepkey/firmware/policy.h" - typedef struct _Storage { uint32_t version; struct Public { @@ -50,6 +50,7 @@ typedef struct _Storage { uint32_t u2f_counter; bool no_backup; bool sca_hardened; + uint8_t random_salt[32]; } pub; bool has_sec; @@ -95,7 +96,7 @@ typedef enum { void storage_loadNode(HDNode *dst, const HDNodeType *src); /// Derive the wrapping key from the user's pin. -void storage_deriveWrappingKey(const char *pin, uint8_t wrapping_key[64]); +void storage_deriveWrappingKey(const char *pin, uint8_t wrapping_key[64], bool sca_hardened, uint8_t random_salt[RANDOM_SALT_LEN]); /// Wrap the storage key. void storage_wrapStorageKey(const uint8_t wrapping_key[64], const uint8_t key[64], uint8_t wrapped_key[64]); @@ -109,11 +110,11 @@ void storage_unwrapStorageKey256(const uint8_t wrapping_key[64], const uint8_t w /// Get the fingerprint for an unwrapped storage key. void storage_keyFingerprint(const uint8_t key[64], uint8_t fingerprint[32]); -/// Check whether a pin is correct. /// \return: PIN_WRONG - PIN is incorrect -/// PIN_GOOD - PIN is correct +/// PIN_GOOD - PIN is correct /// PIN_REWRAPPED -> PIN is correct, storage key was rewrapped, CALLING FUNCTION SHOULD storage_commit() -pintest_t storage_isPinCorrect_impl(const char *pin, uint8_t wrapped_key[64], const uint8_t fingerprint[32], bool *sca_hardened, uint8_t key[64]); +pintest_t storage_isPinCorrect_impl(const char *pin, uint8_t wrapped_key[64], const uint8_t fingerprint[32], bool *sca_hardened, uint8_t key[64], + uint8_t random_salt[RANDOM_SALT_LEN]); /// Migrate data in Storage to/from sec/encrypted_sec. void storage_secMigrate(SessionState *state, Storage *storage, bool encrypt); diff --git a/tools/emulator/main.cpp b/tools/emulator/main.cpp index 8587bb909..b907dbf2b 100644 --- a/tools/emulator/main.cpp +++ b/tools/emulator/main.cpp @@ -70,6 +70,7 @@ static void sigintHandler(int sig_num) { int main(void) { setup(); + flash_collectHWEntropy(false); kk_board_init(); drbg_init(); diff --git a/tools/firmware/keepkey_main.c b/tools/firmware/keepkey_main.c index 99281eb78..b25117a41 100644 --- a/tools/firmware/keepkey_main.c +++ b/tools/firmware/keepkey_main.c @@ -171,6 +171,8 @@ int main(void) _timerusr_isr = (void *)&timerisr_usr; _mmhusr_isr = (void *)&mmhisr; + flash_collectHWEntropy(SIG_OK == signatures_ok()); + /* Drop privileges */ drop_privs(); diff --git a/unittests/firmware/storage.cpp b/unittests/firmware/storage.cpp index 411d86654..4b79a4e9d 100644 --- a/unittests/firmware/storage.cpp +++ b/unittests/firmware/storage.cpp @@ -154,7 +154,7 @@ TEST(Storage, ReadStorageV1) { // Decrypt upgraded storage. uint8_t wrapping_key[64]; - storage_deriveWrappingKey("123456789", wrapping_key); // strongest pin evar + storage_deriveWrappingKey("123456789", wrapping_key, dst.pub.sca_hardened, dst.pub.random_salt); // strongest pin evar storage_unwrapStorageKey(wrapping_key, dst.pub.wrapped_storage_key, session.storageKey); storage_secMigrate(&session, &dst, /*encrypt=*/false); @@ -403,7 +403,7 @@ TEST(Storage, StorageUpgrade_Normal) { // Decrypt upgraded storage. uint8_t wrapping_key[64]; - storage_deriveWrappingKey("123456789", wrapping_key); // strongest pin evar + storage_deriveWrappingKey("123456789", wrapping_key, shadow.storage.pub.sca_hardened, shadow.storage.pub.random_salt); // strongest pin evar storage_unwrapStorageKey(wrapping_key, shadow.storage.pub.wrapped_storage_key, session.storageKey); @@ -448,6 +448,7 @@ TEST(Storage, StorageRoundTrip) { start.storage.pub.has_u2froot = false; start.storage.pub.u2froot.has_public_key = false; start.storage.pub.u2froot.has_private_key = true; + start.storage.pub.sca_hardened = false; start.storage.has_sec = true; start.storage.sec.pin[0] = '\0'; start.storage.sec.cache.root_seed_cache_status = 0xEC; @@ -458,7 +459,7 @@ TEST(Storage, StorageRoundTrip) { memset(&session, 0, sizeof(session)); uint8_t wrapping_key[64]; - storage_deriveWrappingKey("", wrapping_key); + storage_deriveWrappingKey("", wrapping_key, start.storage.pub.sca_hardened, start.storage.pub.random_salt); storage_unwrapStorageKey(wrapping_key, start.storage.pub.wrapped_storage_key, session.storageKey); storage_secMigrate(&session, &start.storage, /*encrypt=*/true); @@ -482,12 +483,11 @@ TEST(Storage, StorageRoundTrip) { printf("\n"); #endif - const uint8_t expected_flash[] = { 0x73, 0x74, 0x6f, 0x72, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0x00, 0x00, 0x00, STORAGE_VERSION, 0x00, 0x00, 0x00, - 0xff, 0xc2, 0x00, 0x00, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, + 0xff, 0x42, 0x00, 0x00, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, @@ -508,9 +508,9 @@ TEST(Storage, StorageRoundTrip) { 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0x68, 0x9a, 0xf4, 0xaa, 0x5f, 0x36, 0xb1, 0x9c, 0x8c, 0x5a, 0xfb, 0xaa, 0x6e, 0xc3, 0xd9, 0xfb, 0x6c, 0xee, 0x31, 0xed, 0xf2, 0xb3, 0x08, 0x53, 0x19, 0x8b, 0x20, 0xf1, 0x15, 0x02, - 0x73, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x73, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, + 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, + 0xab, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, @@ -600,9 +600,12 @@ TEST(Storage, UpgradePolicies) { } TEST(Storage, IsPinCorrect) { + bool sca_hardened = true; uint8_t wrapping_key[64]; - storage_deriveWrappingKey("1234", wrapping_key); + uint8_t random_salt[32]; + memset(random_salt, 0, sizeof(random_salt)); + storage_deriveWrappingKey("1234", wrapping_key, sca_hardened, random_salt); const uint8_t storage_key[64] = "Quick blue fox"; uint8_t wrapped_key[64]; @@ -611,8 +614,8 @@ TEST(Storage, IsPinCorrect) { uint8_t fingerprint[32]; storage_keyFingerprint(storage_key, fingerprint); - uint8_t key_out[64]; bool sca_hardened = true; - EXPECT_TRUE(storage_isPinCorrect_impl("1234", wrapped_key, fingerprint, &sca_hardened, key_out)); + uint8_t key_out[64]; + EXPECT_TRUE(storage_isPinCorrect_impl("1234", wrapped_key, fingerprint, &sca_hardened, key_out, random_salt)); EXPECT_TRUE(memcmp(key_out, storage_key, 64) == 0); } @@ -620,33 +623,35 @@ TEST(Storage, IsPinCorrect) { TEST(Storage, Vuln1996) { ConfigFlash config; SessionState session; - uint8_t wrapping_key[64], wrapped_key1[64], storage_key[64]; + uint8_t wrapping_key[64], wrapped_key1[64], storage_key[64], random_salt[32]; struct { const char *pin; } vec[] = { { "" }, { "1234" }, - { "000000000" }, // etc + { "000000000" }, }; for (const auto &v : vec) { memset(&session, 0, sizeof(session)); memset(&config, 0, sizeof(config)); + memset(random_salt, 0, sizeof(random_salt)); storage_reset_impl(&session, &config); storage_setPin_impl(&session, &config.storage, v.pin); ASSERT_TRUE(PIN_GOOD == storage_isPinCorrect_impl(v.pin, - config.storage.pub.wrapped_storage_key, - config.storage.pub.storage_key_fingerprint, - &config.storage.pub.sca_hardened, - storage_key)); + config.storage.pub.wrapped_storage_key, + config.storage.pub.storage_key_fingerprint, + &config.storage.pub.sca_hardened, + storage_key, + random_salt)); ASSERT_TRUE(config.storage.pub.sca_hardened == true); memcpy(wrapped_key1, config.storage.pub.wrapped_storage_key, sizeof(wrapped_key1)); // Check storage wrapping update by starting with unhardened aes256 version - storage_deriveWrappingKey(v.pin, wrapping_key); + storage_deriveWrappingKey(v.pin, wrapping_key, config.storage.pub.sca_hardened, random_salt); storage_unwrapStorageKey(wrapping_key, config.storage.pub.wrapped_storage_key, storage_key); uint8_t iv[64]; memcpy(iv, wrapping_key, sizeof(iv)); @@ -660,10 +665,11 @@ TEST(Storage, Vuln1996) { ASSERT_TRUE(memcmp(wrapped_key1, config.storage.pub.wrapped_storage_key, sizeof(wrapped_key1)) != 0); ASSERT_TRUE(storage_isPinCorrect_impl(v.pin, - config.storage.pub.wrapped_storage_key, - config.storage.pub.storage_key_fingerprint, - &config.storage.pub.sca_hardened, - storage_key)); + config.storage.pub.wrapped_storage_key, + config.storage.pub.storage_key_fingerprint, + &config.storage.pub.sca_hardened, + storage_key, + random_salt)); ASSERT_TRUE(memcmp(wrapped_key1, config.storage.pub.wrapped_storage_key, sizeof(wrapped_key1)) == 0); ASSERT_TRUE(config.storage.pub.sca_hardened == true); } @@ -713,10 +719,11 @@ TEST(Storage, Reset) { storage_reset_impl(&session, &config); ASSERT_TRUE(storage_isPinCorrect_impl("", - config.storage.pub.wrapped_storage_key, - config.storage.pub.storage_key_fingerprint, - &config.storage.pub.sca_hardened, - session.storageKey)); + config.storage.pub.wrapped_storage_key, + config.storage.pub.storage_key_fingerprint, + &config.storage.pub.sca_hardened, + session.storageKey, + config.storage.pub.random_salt)); uint8_t old_storage_key[64]; memcpy(old_storage_key, session.storageKey, sizeof(old_storage_key)); @@ -727,10 +734,11 @@ TEST(Storage, Reset) { uint8_t new_storage_key[64]; ASSERT_TRUE(storage_isPinCorrect_impl("1234", - config.storage.pub.wrapped_storage_key, - config.storage.pub.storage_key_fingerprint, - &config.storage.pub.sca_hardened, - new_storage_key)); + config.storage.pub.wrapped_storage_key, + config.storage.pub.storage_key_fingerprint, + &config.storage.pub.sca_hardened, + new_storage_key, + config.storage.pub.random_salt)); ASSERT_TRUE(memcmp(session.storageKey, new_storage_key, 64) == 0); } From 0152bdfdc9e66a180430c37253dce1a4d88ed260 Mon Sep 17 00:00:00 2001 From: keepkeyjon Date: Wed, 12 Feb 2020 11:10:07 -0700 Subject: [PATCH 17/29] firmware: speed up device tests --- lib/firmware/storage.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/firmware/storage.c b/lib/firmware/storage.c index 3a48eb5fa..b5d59c747 100644 --- a/lib/firmware/storage.c +++ b/lib/firmware/storage.c @@ -56,7 +56,7 @@ #include #include -#ifdef EMULATOR +#if defined(EMULATOR) || defined(DEBUG_ON) # define PIN_ITER_COUNT 1000 # define PIN_ITER_CHUNK 10 #else From bcfdb6710c00337814893018d1b93d545fa12a52 Mon Sep 17 00:00:00 2001 From: keepkeyjon Date: Tue, 11 Feb 2020 16:39:30 -0700 Subject: [PATCH 18/29] firmware: better lock/unlock messaging --- lib/firmware/storage.c | 15 +++++++++------ lib/firmware/storage.h | 4 +++- unittests/firmware/storage.cpp | 10 +++++----- 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/lib/firmware/storage.c b/lib/firmware/storage.c index b5d59c747..0b1db1bca 100644 --- a/lib/firmware/storage.c +++ b/lib/firmware/storage.c @@ -284,7 +284,8 @@ void storage_writeHDNode(char *ptr, size_t len, const HDNodeType *node) { void storage_deriveWrappingKey( const char *pin, uint8_t wrapping_key[64], bool sca_hardened, - uint8_t random_salt[RANDOM_SALT_LEN]) + uint8_t random_salt[RANDOM_SALT_LEN], + const char *message) { size_t pin_len = strlen(pin); if (sca_hardened && pin_len > 0) { @@ -296,7 +297,7 @@ void storage_deriveWrappingKey( PBKDF2_HMAC_SHA256_CTX ctx = {0}; pbkdf2_hmac_sha256_Init(&ctx, (const uint8_t*)pin, pin_len, salt, sizeof(salt), 1); for (int i = 0; i < PIN_ITER_COUNT; i += PIN_ITER_CHUNK) { - layoutProgress(_("Verifying PIN"), 1000 * i / (PIN_ITER_COUNT * 2)); + layoutProgress(message, 1000 * i / (PIN_ITER_COUNT * 2)); pbkdf2_hmac_sha256_Update(&ctx, PIN_ITER_CHUNK); } pbkdf2_hmac_sha256_Final(&ctx, wrapping_key); @@ -304,10 +305,10 @@ void storage_deriveWrappingKey( memzero(&ctx, sizeof(ctx)); pbkdf2_hmac_sha256_Init(&ctx, (const uint8_t*)pin, pin_len, salt, sizeof(salt), 2); for (int i = 0; i < PIN_ITER_COUNT; i += PIN_ITER_CHUNK) { - layoutProgress(_("Verifying PIN"), 1000 * (i + PIN_ITER_COUNT) / (PIN_ITER_COUNT * 2)); + layoutProgress(message, 1000 * (i + PIN_ITER_COUNT) / (PIN_ITER_COUNT * 2)); pbkdf2_hmac_sha256_Update(&ctx, PIN_ITER_CHUNK); } - layoutProgress(_("Verifying PIN"), 1000); + layoutProgress(message, 1000); pbkdf2_hmac_sha256_Final(&ctx, wrapping_key + 32); memzero(salt, sizeof(salt)); @@ -360,7 +361,8 @@ pintest_t storage_isPinCorrect_impl( PIN_REWRAP, then the calling function is required to update the flash with a storage_commit(). */ uint8_t wrapping_key[64]; - storage_deriveWrappingKey(pin, wrapping_key, sca_hardened, random_salt); + storage_deriveWrappingKey(pin, wrapping_key, sca_hardened, random_salt, + _("Verifying PIN")); // unwrap the storage key for fingerprint test if (*sca_hardened) { @@ -1296,7 +1298,8 @@ void storage_setPin_impl(SessionState *ss, Storage *storage, const char *pin) { // Derive the wrapping key for the new pin uint8_t wrapping_key[64]; - storage_deriveWrappingKey(pin, wrapping_key, /*sca_hardened=*/true, storage->pub.random_salt); + storage_deriveWrappingKey(pin, wrapping_key, /*sca_hardened=*/true, + storage->pub.random_salt, _("Encrypting Secrets")); // Derive a new storageKey. random_buffer(ss->storageKey, 64); diff --git a/lib/firmware/storage.h b/lib/firmware/storage.h index 28c36133f..a6b709506 100644 --- a/lib/firmware/storage.h +++ b/lib/firmware/storage.h @@ -96,7 +96,9 @@ typedef enum { void storage_loadNode(HDNode *dst, const HDNodeType *src); /// Derive the wrapping key from the user's pin. -void storage_deriveWrappingKey(const char *pin, uint8_t wrapping_key[64], bool sca_hardened, uint8_t random_salt[RANDOM_SALT_LEN]); +void storage_deriveWrappingKey(const char *pin, uint8_t wrapping_key[64], + bool sca_hardened, uint8_t random_salt[RANDOM_SALT_LEN], + const char *message); /// Wrap the storage key. void storage_wrapStorageKey(const uint8_t wrapping_key[64], const uint8_t key[64], uint8_t wrapped_key[64]); diff --git a/unittests/firmware/storage.cpp b/unittests/firmware/storage.cpp index 4b79a4e9d..33205ebe6 100644 --- a/unittests/firmware/storage.cpp +++ b/unittests/firmware/storage.cpp @@ -154,7 +154,7 @@ TEST(Storage, ReadStorageV1) { // Decrypt upgraded storage. uint8_t wrapping_key[64]; - storage_deriveWrappingKey("123456789", wrapping_key, dst.pub.sca_hardened, dst.pub.random_salt); // strongest pin evar + storage_deriveWrappingKey("123456789", wrapping_key, dst.pub.sca_hardened, dst.pub.random_salt, ""); // strongest pin evar storage_unwrapStorageKey(wrapping_key, dst.pub.wrapped_storage_key, session.storageKey); storage_secMigrate(&session, &dst, /*encrypt=*/false); @@ -403,7 +403,7 @@ TEST(Storage, StorageUpgrade_Normal) { // Decrypt upgraded storage. uint8_t wrapping_key[64]; - storage_deriveWrappingKey("123456789", wrapping_key, shadow.storage.pub.sca_hardened, shadow.storage.pub.random_salt); // strongest pin evar + storage_deriveWrappingKey("123456789", wrapping_key, shadow.storage.pub.sca_hardened, shadow.storage.pub.random_salt, ""); // strongest pin evar storage_unwrapStorageKey(wrapping_key, shadow.storage.pub.wrapped_storage_key, session.storageKey); @@ -459,7 +459,7 @@ TEST(Storage, StorageRoundTrip) { memset(&session, 0, sizeof(session)); uint8_t wrapping_key[64]; - storage_deriveWrappingKey("", wrapping_key, start.storage.pub.sca_hardened, start.storage.pub.random_salt); + storage_deriveWrappingKey("", wrapping_key, start.storage.pub.sca_hardened, start.storage.pub.random_salt, ""); storage_unwrapStorageKey(wrapping_key, start.storage.pub.wrapped_storage_key, session.storageKey); storage_secMigrate(&session, &start.storage, /*encrypt=*/true); @@ -605,7 +605,7 @@ TEST(Storage, IsPinCorrect) { uint8_t wrapping_key[64]; uint8_t random_salt[32]; memset(random_salt, 0, sizeof(random_salt)); - storage_deriveWrappingKey("1234", wrapping_key, sca_hardened, random_salt); + storage_deriveWrappingKey("1234", wrapping_key, sca_hardened, random_salt, ""); const uint8_t storage_key[64] = "Quick blue fox"; uint8_t wrapped_key[64]; @@ -651,7 +651,7 @@ TEST(Storage, Vuln1996) { memcpy(wrapped_key1, config.storage.pub.wrapped_storage_key, sizeof(wrapped_key1)); // Check storage wrapping update by starting with unhardened aes256 version - storage_deriveWrappingKey(v.pin, wrapping_key, config.storage.pub.sca_hardened, random_salt); + storage_deriveWrappingKey(v.pin, wrapping_key, config.storage.pub.sca_hardened, random_salt, ""); storage_unwrapStorageKey(wrapping_key, config.storage.pub.wrapped_storage_key, storage_key); uint8_t iv[64]; memcpy(iv, wrapping_key, sizeof(iv)); From 674f82dc0146cf4bf37b3be2c005b57524363095 Mon Sep 17 00:00:00 2001 From: keepkeyjon Date: Wed, 12 Feb 2020 10:48:15 -0700 Subject: [PATCH 19/29] build: only do NDEBUG in release builds --- CMakeLists.txt | 3 +-- deps/qrenc/QR-Code-generator | 2 +- lib/firmware/storage.c | 2 ++ 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 46129b1cc..8b3b82b11 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -82,8 +82,6 @@ add_definitions(-DBOOTLOADER_MAJOR_VERSION=${BOOTLOADER_MAJOR_VERSION}) add_definitions(-DBOOTLOADER_MINOR_VERSION=${BOOTLOADER_MINOR_VERSION}) add_definitions(-DBOOTLOADER_PATCH_VERSION=${BOOTLOADER_PATCH_VERSION}) -add_definitions(-DNDEBUG) - add_definitions(-DBIP39_WORDLIST_PADDED=1) add_definitions(-DAES_128=1) @@ -100,6 +98,7 @@ if("${CMAKE_BUILD_TYPE}" STREQUAL "Debug") elseif("${CMAKE_BUILD_TYPE}" STREQUAL "Release" OR "${CMAKE_BUILD_TYPE}" STREQUAL "MinSizeRel" OR "${CMAKE_BUILD_TYPE}" STREQUAL "") + add_definitions(-DNDEBUG) add_definitions(-DMEMORY_PROTECT=1) if(NOT ${KK_EMULATOR}) message(WARNING diff --git a/deps/qrenc/QR-Code-generator b/deps/qrenc/QR-Code-generator index 40d24f38a..6dfbfdad5 160000 --- a/deps/qrenc/QR-Code-generator +++ b/deps/qrenc/QR-Code-generator @@ -1 +1 @@ -Subproject commit 40d24f38aa0a8180b271b6c88be8633f842ed9d4 +Subproject commit 6dfbfdad5d9303ed190d1c3cb7bec34b565b6ce8 diff --git a/lib/firmware/storage.c b/lib/firmware/storage.c index 0b1db1bca..2970af71e 100644 --- a/lib/firmware/storage.c +++ b/lib/firmware/storage.c @@ -55,6 +55,7 @@ #include #include +#include #if defined(EMULATOR) || defined(DEBUG_ON) # define PIN_ITER_COUNT 1000 @@ -456,6 +457,7 @@ void storage_secMigrate(SessionState *ss, Storage *storage, bool encrypt) { if (storage->has_sec_fingerprint) { if (memcmp(storage->sec_fingerprint, sec_fingerprint, sizeof(sec_fingerprint)) != 0) { + assert(false && "storage decrypt failure"); memzero(scratch, sizeof(scratch)); storage_wipe(); layout_warning_static("Storage decrypt failure. Reboot device!"); From c42d73aa22fe0864da7263a3278cafa3ac925ebb Mon Sep 17 00:00:00 2001 From: keepkeyjon Date: Wed, 12 Feb 2020 10:49:16 -0700 Subject: [PATCH 20/29] board: avoid nullptr deref in unittests --- lib/board/draw.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/board/draw.c b/lib/board/draw.c index 45a9a4419..a8494ddc8 100644 --- a/lib/board/draw.c +++ b/lib/board/draw.c @@ -118,6 +118,10 @@ bool draw_char_with_shift(Canvas *canvas, DrawableParams *p, void draw_string(Canvas *canvas, const Font *font, const char *str_write, DrawableParams *p, uint16_t width, uint16_t line_height) { + if (!canvas) { + return; + } + bool have_space = true; uint16_t x_offset = 0; DrawableParams char_params = *p; @@ -296,7 +300,7 @@ void draw_box_simple(Canvas *canvas, uint8_t color, uint16_t x, uint16_t y, uint */ bool draw_bitmap_mono_rle(Canvas *canvas, const AnimationFrame *frame, bool erase) { - if (frame == NULL) { + if (!frame || !canvas) { return false; } From c101c4a942dcb2f64c451329091d8a93b33491a0 Mon Sep 17 00:00:00 2001 From: keepkeyjon Date: Wed, 12 Feb 2020 10:49:45 -0700 Subject: [PATCH 21/29] firmware: combine pin checking / pin cacheing ... to avoid duplicate "Unlocking" UX problem during pin changes. --- lib/firmware/pin_sm.c | 1 - lib/firmware/storage.c | 85 ++++++++++++---------------------- lib/firmware/storage.h | 5 -- unittests/firmware/storage.cpp | 36 -------------- 4 files changed, 29 insertions(+), 98 deletions(-) diff --git a/lib/firmware/pin_sm.c b/lib/firmware/pin_sm.c index 79b75c48a..3493825c7 100644 --- a/lib/firmware/pin_sm.c +++ b/lib/firmware/pin_sm.c @@ -283,7 +283,6 @@ bool pin_protect(const char *prompt) return false; } - session_cachePin(pin_info.pin); storage_resetPinFails(); return true; } diff --git a/lib/firmware/storage.c b/lib/firmware/storage.c index 2970af71e..d83f21a26 100644 --- a/lib/firmware/storage.c +++ b/lib/firmware/storage.c @@ -920,8 +920,14 @@ void storage_init(void) break; } - if (!storage_hasPin()) - session_cachePin(""); + if (!storage_hasPin()) { + // Cache the PIN +#ifndef NDEBUG + bool ret = +#endif + storage_isPinCorrect(""); + assert(ret && "Empty PIN not cached?"); + } } void storage_resetUuid(void) @@ -1258,20 +1264,31 @@ const char *storage_getLanguage(void) } bool storage_isPinCorrect(const char *pin) { - uint8_t storage_key[64]; - pintest_t ret = storage_isPinCorrect_impl(pin, - shadow_config.storage.pub.wrapped_storage_key, - shadow_config.storage.pub.storage_key_fingerprint, - &shadow_config.storage.pub.sca_hardened, - storage_key, - shadow_config.storage.pub.random_salt); - - if (ret == PIN_REWRAP) { + shadow_config.storage.pub.wrapped_storage_key, + shadow_config.storage.pub.storage_key_fingerprint, + &shadow_config.storage.pub.sca_hardened, + session.storageKey, + shadow_config.storage.pub.random_salt); + + switch (ret) { + case PIN_REWRAP: + session.pinCached = true; storage_commit(); + storage_secMigrate(&session, &shadow_config.storage, /*encrypt=*/false); + break; + case PIN_GOOD: + session.pinCached = true; + storage_secMigrate(&session, &shadow_config.storage, /*encrypt=*/false); + break; + case PIN_WRONG: + default: + session.pinCached = false; + session_clear_impl(&session, &shadow_config.storage, /*clear_pin=*/true); + memzero(session.storageKey, sizeof(session.storageKey)); + break; } - memzero(storage_key, 64); return ret; } @@ -1323,50 +1340,6 @@ void storage_setPin_impl(SessionState *ss, Storage *storage, const char *pin) storage_secMigrate(ss, storage, /*encrypt=*/true); } -void session_cachePin(const char *pin) -{ - if (PIN_REWRAP == session_cachePin_impl(&session, &shadow_config.storage, pin)) { - storage_commit(); - } -} - -pintest_t session_cachePin_impl(SessionState *ss, Storage *storage, const char *pin) -{ -/* - This is a *_impl() function that is assumed to not modify the flash storage config state. Because this function - calls storage_isPinCorrect_impl(), the storage config may need updating: - This function will return - PIN_WRONG - PIN is incorrect - PIN_GOOD - PIN is correct - PIN_REWRAP -> PIN is correct, storage key was rewrapped, CALLING FUNCTION SHOULD storage_commit() - - If the pin is correct, the shadow config may be out of sync with the storage config in flash. - Thus, if the return is PIN_REWRAP, then the calling function is required to update the flash with a - storage_commit(). -*/ - pintest_t ret = - storage_isPinCorrect_impl(pin, - storage->pub.wrapped_storage_key, - storage->pub.storage_key_fingerprint, - &storage->pub.sca_hardened, - ss->storageKey, - shadow_config.storage.pub.random_salt); - - if (ret == PIN_WRONG) { - ss->pinCached = false; - } else { - ss->pinCached = true; - } - - if (!ss->pinCached) { - session_clear_impl(ss, storage, /*clear_pin=*/true); - return(ret); - } - - storage_secMigrate(ss, storage, /*encrypt=*/false); - return(ret); -} - bool session_isPinCached(void) { return session.pinCached; diff --git a/lib/firmware/storage.h b/lib/firmware/storage.h index a6b709506..584d30b51 100644 --- a/lib/firmware/storage.h +++ b/lib/firmware/storage.h @@ -129,11 +129,6 @@ void storage_setPin_impl(SessionState *session, Storage *storage, const char *pi bool storage_hasPin_impl(const Storage *storage); -/// \return: PIN_WRONG - PIN is incorrect -/// PIN_GOOD - PIN is correct -/// PIN_REWRAP -> PIN is correct, storage key was rewrapped, CALLING FUNCTION SHOULD storage_commit() -pintest_t session_cachePin_impl(SessionState *session, Storage *storage, const char *pin); - /// \return: PIN_WRONG - PIN is incorrect /// PIN_GOOD - PIN is correct /// PIN_REWRAP -> PIN is correct, storage key was rewrapped, CALLING FUNCTION SHOULD storage_commit() diff --git a/unittests/firmware/storage.cpp b/unittests/firmware/storage.cpp index 33205ebe6..fe6d450db 100644 --- a/unittests/firmware/storage.cpp +++ b/unittests/firmware/storage.cpp @@ -675,42 +675,6 @@ TEST(Storage, Vuln1996) { } } -TEST(Storage, Pin) { - - storage_setPin(""); - ASSERT_TRUE(storage_isPinCorrect("")); - - storage_setPin("1234"); - ASSERT_TRUE(storage_isPinCorrect("1234")); - ASSERT_FALSE(storage_isPinCorrect("")); - ASSERT_FALSE(storage_isPinCorrect("9876")); - - storage_setPin("987654321"); - ASSERT_FALSE(storage_isPinCorrect("")); - ASSERT_TRUE(storage_isPinCorrect("987654321")); - - storage_setPin(""); - ASSERT_TRUE(storage_isPinCorrect("")); -} - -TEST(Storage, CacheWrongPin) { - ConfigFlash config; - SessionState session; - memset(&session, 0, sizeof(session)); - - storage_reset_impl(&session, &config); - config.storage.has_sec = true; - strcpy(config.storage.sec.mnemonic, "sekret"); - storage_setPin_impl(&session, &config.storage, "1234"); - - // Attempt to cache the wrong pin - session_cachePin_impl(&session, &config.storage, "1111"); - - // Check that secrets were wiped from the session - ASSERT_FALSE(config.storage.has_sec); - ASSERT_EQ(std::string(config.storage.sec.mnemonic), ""); -} - TEST(Storage, Reset) { ConfigFlash config; SessionState session; From 1701cff862969769c775f4d2f509cba5a9b4e628 Mon Sep 17 00:00:00 2001 From: keepkeyjon Date: Thu, 13 Feb 2020 09:15:15 -0700 Subject: [PATCH 22/29] firmware: memzero more aggressively --- lib/firmware/storage.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/firmware/storage.c b/lib/firmware/storage.c index d83f21a26..767c65dfa 100644 --- a/lib/firmware/storage.c +++ b/lib/firmware/storage.c @@ -302,8 +302,8 @@ void storage_deriveWrappingKey( pbkdf2_hmac_sha256_Update(&ctx, PIN_ITER_CHUNK); } pbkdf2_hmac_sha256_Final(&ctx, wrapping_key); - memzero(&ctx, sizeof(ctx)); + pbkdf2_hmac_sha256_Init(&ctx, (const uint8_t*)pin, pin_len, salt, sizeof(salt), 2); for (int i = 0; i < PIN_ITER_COUNT; i += PIN_ITER_CHUNK) { layoutProgress(message, 1000 * (i + PIN_ITER_COUNT) / (PIN_ITER_COUNT * 2)); @@ -311,6 +311,7 @@ void storage_deriveWrappingKey( } layoutProgress(message, 1000); pbkdf2_hmac_sha256_Final(&ctx, wrapping_key + 32); + memzero(&ctx, sizeof(ctx)); memzero(salt, sizeof(salt)); } else { From b38ef4dc7ffe27f6c4a775e50c64fa98616cfad9 Mon Sep 17 00:00:00 2001 From: keepkeyjon Date: Thu, 13 Feb 2020 09:16:42 -0700 Subject: [PATCH 23/29] firmware: eliminate strlen common subexpr --- lib/firmware/storage.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/firmware/storage.c b/lib/firmware/storage.c index 767c65dfa..e1358da47 100644 --- a/lib/firmware/storage.c +++ b/lib/firmware/storage.c @@ -315,7 +315,7 @@ void storage_deriveWrappingKey( memzero(salt, sizeof(salt)); } else { - sha512_Raw((const uint8_t*)pin, strlen(pin), wrapping_key); + sha512_Raw((const uint8_t*)pin, pin_len, wrapping_key); } } From 55b08037e0c48b50562f18ae8a7fd7d57aecacb8 Mon Sep 17 00:00:00 2001 From: keepkeyjon Date: Thu, 13 Feb 2020 10:55:23 -0700 Subject: [PATCH 24/29] firmware: fix indentation --- lib/firmware/fsm_msg_coin.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/firmware/fsm_msg_coin.h b/lib/firmware/fsm_msg_coin.h index 7609e51a0..aa9f3f78d 100644 --- a/lib/firmware/fsm_msg_coin.h +++ b/lib/firmware/fsm_msg_coin.h @@ -203,7 +203,7 @@ void fsm_msgGetAddress(GetAddress *msg) animating_progress_handler(_("Computing address"), 0); } if (!compute_address(coin, msg->script_type, node, msg->has_multisig, &msg->multisig, address)) { - memzero(node, sizeof(*node)); + memzero(node, sizeof(*node)); fsm_sendFailure(FailureType_Failure_Other, _("Can't encode address")); layoutHome(); return; @@ -228,7 +228,7 @@ void fsm_msgGetAddress(GetAddress *msg) if (mismatch) { if (!confirm(ButtonRequestType_ButtonRequest_Other, "WARNING", "Wrong address path for selected coin. Continue at your own risk!")) { - memzero(node, sizeof(*node)); + memzero(node, sizeof(*node)); fsm_sendFailure(FailureType_Failure_ActionCancelled, NULL); layoutHome(); return; @@ -241,7 +241,7 @@ void fsm_msgGetAddress(GetAddress *msg) if(!confirm_address(node_str, address + prefix_len)) { - memzero(node, sizeof(*node)); + memzero(node, sizeof(*node)); fsm_sendFailure(FailureType_Failure_ActionCancelled, "Show address cancelled"); layoutHome(); return; @@ -280,17 +280,17 @@ void fsm_msgSignMessage(SignMessage *msg) resp->has_address = true; hdnode_fill_public_key(node); if (!compute_address(coin, msg->script_type, node, false, NULL, resp->address)) { - memzero(node, sizeof(*node)); + memzero(node, sizeof(*node)); fsm_sendFailure(FailureType_Failure_Other, _("Error computing address")); layoutHome(); return; } resp->has_signature = true; resp->signature.size = 65; - memzero(node, sizeof(*node)); + memzero(node, sizeof(*node)); msg_write(MessageType_MessageType_MessageSignature, resp); } else { - memzero(node, sizeof(*node)); + memzero(node, sizeof(*node)); fsm_sendFailure(FailureType_Failure_Other, _("Error signing message")); } layoutHome(); From e27f3df6a691330b6283cf094d3006a20fb2df3e Mon Sep 17 00:00:00 2001 From: keepkeyjon Date: Thu, 13 Feb 2020 13:22:51 -0700 Subject: [PATCH 25/29] bump version: v6.4.0 --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 8b3b82b11..89d091329 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -2,7 +2,7 @@ cmake_minimum_required(VERSION 3.7.2) project(KeepKeyFirmware - VERSION 6.3.0 + VERSION 6.4.0 LANGUAGES C CXX ASM) From e368a906606f4517e4b9638ca41f9955422e742f Mon Sep 17 00:00:00 2001 From: keepkeyjon Date: Thu, 13 Feb 2020 13:50:25 -0700 Subject: [PATCH 26/29] update python-keepkey (xrp) --- deps/python-keepkey | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/deps/python-keepkey b/deps/python-keepkey index 9177139bd..28ef3fbe8 160000 --- a/deps/python-keepkey +++ b/deps/python-keepkey @@ -1 +1 @@ -Subproject commit 9177139bdd7d36fe5818d3ecfd1f093768657f35 +Subproject commit 28ef3fbe890b8feb6c94d9094b4532c9bcb363a2 From a0bb83157924253260ac4f3c02cdef50caa2f175 Mon Sep 17 00:00:00 2001 From: markrpto Date: Fri, 14 Feb 2020 15:38:52 -0700 Subject: [PATCH 27/29] fixed some minor include file bugs --- lib/firmware/storage.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/firmware/storage.c b/lib/firmware/storage.c index 839ec463c..bec9b5ff8 100644 --- a/lib/firmware/storage.c +++ b/lib/firmware/storage.c @@ -28,9 +28,8 @@ # include #endif -#ifndef EMULATOR -# include "aes_sca/aes128_cbc.h" -#endif +#include "aes_sca/aes128_cbc.h" + #include "keepkey/board/common.h" #include "keepkey/board/supervise.h" From 829d8cb63842366306f70c0845ba869433d0b09d Mon Sep 17 00:00:00 2001 From: markrpto Date: Fri, 14 Feb 2020 15:53:05 -0700 Subject: [PATCH 28/29] fixed includes --- lib/firmware/storage.c | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/firmware/storage.c b/lib/firmware/storage.c index bec9b5ff8..5ff19d75d 100644 --- a/lib/firmware/storage.c +++ b/lib/firmware/storage.c @@ -30,7 +30,6 @@ #include "aes_sca/aes128_cbc.h" - #include "keepkey/board/common.h" #include "keepkey/board/supervise.h" #include "keepkey/board/keepkey_board.h" From 4e4c14773f344a4274bc0e0b5b02ef7a100e0163 Mon Sep 17 00:00:00 2001 From: markrpto Date: Wed, 19 Feb 2020 10:42:24 -0700 Subject: [PATCH 29/29] fix to pin stretching and unittest --- lib/firmware/storage.c | 7 +++++-- unittests/firmware/storage.cpp | 26 ++++++++++++++++++-------- 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/lib/firmware/storage.c b/lib/firmware/storage.c index 5ff19d75d..c54f40dd0 100644 --- a/lib/firmware/storage.c +++ b/lib/firmware/storage.c @@ -362,7 +362,7 @@ pintest_t storage_isPinCorrect_impl( PIN_REWRAP, then the calling function is required to update the flash with a storage_commit(). */ uint8_t wrapping_key[64]; - storage_deriveWrappingKey(pin, wrapping_key, sca_hardened, random_salt, + storage_deriveWrappingKey(pin, wrapping_key, *sca_hardened, random_salt, _("Verifying PIN")); // unwrap the storage key for fingerprint test @@ -383,7 +383,10 @@ pintest_t storage_isPinCorrect_impl( if (ret == PIN_GOOD) { if (!*sca_hardened) { - // PIN is correct but storage key needs rewrap + // PIN is correct but: + // 1. wrapping key needs to be regenerated using stretched key + // 2. storage key needs a rewrap with new wrapping key and algorithm + storage_deriveWrappingKey(pin, wrapping_key, true/* sca_hardened */, random_salt, _("Verifying PIN")); storage_wrapStorageKey(wrapping_key, key, wrapped_key); *sca_hardened = true; ret = PIN_REWRAP; diff --git a/unittests/firmware/storage.cpp b/unittests/firmware/storage.cpp index fe6d450db..bbf7b27cf 100644 --- a/unittests/firmware/storage.cpp +++ b/unittests/firmware/storage.cpp @@ -623,7 +623,7 @@ TEST(Storage, IsPinCorrect) { TEST(Storage, Vuln1996) { ConfigFlash config; SessionState session; - uint8_t wrapping_key[64], wrapped_key1[64], storage_key[64], random_salt[32]; + uint8_t wrapping_key[64], wrapped_key1[64], wrapping_key_upin[64], storage_key[64], random_salt[32]; struct { const char *pin; @@ -649,21 +649,31 @@ TEST(Storage, Vuln1996) { random_salt)); ASSERT_TRUE(config.storage.pub.sca_hardened == true); memcpy(wrapped_key1, config.storage.pub.wrapped_storage_key, sizeof(wrapped_key1)); - - // Check storage wrapping update by starting with unhardened aes256 version + + // wrapped_key1 should be wrapped with aes128-pinstretch + + // Check storage wrapping update by generating parameters consistent with aes256-pinnostretch + + // first obtain the storage key generated above storage_deriveWrappingKey(v.pin, wrapping_key, config.storage.pub.sca_hardened, random_salt, ""); storage_unwrapStorageKey(wrapping_key, config.storage.pub.wrapped_storage_key, storage_key); + + // now derive a wrapping key from unstretched pin and wrap the storage key with it + storage_deriveWrappingKey(v.pin, wrapping_key_upin, false, random_salt, ""); uint8_t iv[64]; - memcpy(iv, wrapping_key, sizeof(iv)); + memcpy(iv, wrapping_key_upin, sizeof(iv)); aes_encrypt_ctx ctx; - aes_encrypt_key256(wrapping_key, &ctx); + aes_encrypt_key256(wrapping_key_upin, &ctx); aes_cbc_encrypt(storage_key, config.storage.pub.wrapped_storage_key, 64, iv + 32, &ctx); - config.storage.pub.sca_hardened = false; + config.storage.pub.sca_hardened = false; // indicate this is an unhardened wrap key - // wrapped_key1 under aes128. Config version is wrapped under aes256 for test. This check ensures - // that test conditions are correct. + // wrapped_key1 is aes128-pinstretch. Config version is wrapped aes256-pinnostretch + // for test. + + // This check ensures that test conditions are correct. ASSERT_TRUE(memcmp(wrapped_key1, config.storage.pub.wrapped_storage_key, sizeof(wrapped_key1)) != 0); + // now check that aes256-nopinstretch turns into aes128-pinstretched wrapping key ASSERT_TRUE(storage_isPinCorrect_impl(v.pin, config.storage.pub.wrapped_storage_key, config.storage.pub.storage_key_fingerprint,