diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index dfed55e41..68f2c168b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -52,22 +52,26 @@ jobs: with: ref: ${{ github.event.pull_request.head.sha || github.sha }} - - name: Install clang-format - run: sudo apt-get install -y clang-format-14 - - name: Check code formatting run: | + # Use pre-installed clang-format (ubuntu-latest ships with it) + CF=$(command -v clang-format-14 || command -v clang-format || true) + if [ -z "$CF" ]; then + echo "::warning::clang-format not found, skipping lint" + exit 0 + fi + echo "Using: $CF ($($CF --version | head -1))" FAILED=0 for f in $(find include/keepkey lib/firmware lib/board lib/transport/src \ -name '*.c' -o -name '*.h' 2>/dev/null | grep -v generated | grep -v '.pb.'); do - if ! clang-format-14 --style=file --dry-run --Werror "$f" 2>/dev/null; then + if ! $CF --style=file --dry-run --Werror "$f" 2>/dev/null; then echo "::warning file=$f::Formatting differs from .clang-format" FAILED=1 fi done if [ "$FAILED" = "1" ]; then echo "" - echo "Run: clang-format -i to fix formatting" + echo "Run: $CF -i to fix formatting" echo "Note: treating as warning until codebase is reformatted" fi # Warn-only until existing code is reformatted diff --git a/deps/device-protocol b/deps/device-protocol index 323802f17..b07589ff3 160000 --- a/deps/device-protocol +++ b/deps/device-protocol @@ -1 +1 @@ -Subproject commit 323802f17dd44165a5100357df771348c8b49672 +Subproject commit b07589ff386265f894736a490fc163d46f9479b7 diff --git a/deps/python-keepkey b/deps/python-keepkey index f1dd2b684..e89587701 160000 --- a/deps/python-keepkey +++ b/deps/python-keepkey @@ -1 +1 @@ -Subproject commit f1dd2b6847346abe8ea2985b22d688de4911643f +Subproject commit e8958770152a85af4c2a03f9e93b5d98e8dff44b diff --git a/include/keepkey/transport/messages-solana.options b/include/keepkey/transport/messages-solana.options new file mode 100644 index 000000000..645d5cfac --- /dev/null +++ b/include/keepkey/transport/messages-solana.options @@ -0,0 +1,17 @@ +SolanaGetAddress.address_n max_count:8 +SolanaGetAddress.coin_name max_size:21 + +SolanaAddress.address max_size:64 + +SolanaSignTx.address_n max_count:8 +SolanaSignTx.coin_name max_size:21 +SolanaSignTx.raw_tx max_size:2048 + +SolanaSignedTx.signature max_size:64 + +SolanaSignMessage.address_n max_count:8 +SolanaSignMessage.coin_name max_size:21 +SolanaSignMessage.message max_size:1024 + +SolanaMessageSignature.public_key max_size:32 +SolanaMessageSignature.signature max_size:64 diff --git a/include/keepkey/transport/messages-ton.options b/include/keepkey/transport/messages-ton.options new file mode 100644 index 000000000..18c064289 --- /dev/null +++ b/include/keepkey/transport/messages-ton.options @@ -0,0 +1,12 @@ +TonGetAddress.address_n max_count:8 +TonGetAddress.coin_name max_size:21 + +TonAddress.address max_size:50 +TonAddress.raw_address max_size:70 + +TonSignTx.address_n max_count:8 +TonSignTx.coin_name max_size:21 +TonSignTx.raw_tx max_size:1024 +TonSignTx.to_address max_size:50 + +TonSignedTx.signature max_size:64 diff --git a/include/keepkey/transport/messages-tron.options b/include/keepkey/transport/messages-tron.options new file mode 100644 index 000000000..80215e8cf --- /dev/null +++ b/include/keepkey/transport/messages-tron.options @@ -0,0 +1,14 @@ +TronGetAddress.address_n max_count:8 +TronGetAddress.coin_name max_size:21 + +TronAddress.address max_size:35 + +TronSignTx.address_n max_count:8 +TronSignTx.coin_name max_size:21 +TronSignTx.raw_data max_size:2048 +TronSignTx.ref_block_bytes max_size:4 +TronSignTx.ref_block_hash max_size:32 +TronSignTx.contract_type max_size:64 +TronSignTx.to_address max_size:35 + +TronSignedTx.signature max_size:65 diff --git a/include/keepkey/transport/messages.options b/include/keepkey/transport/messages.options index 41e0c377c..ad08bd6ba 100644 --- a/include/keepkey/transport/messages.options +++ b/include/keepkey/transport/messages.options @@ -131,3 +131,5 @@ FlashHash.challenge max_size:32 FlashWrite.data max_size:1024 FlashHashResponse.data max_size:32 + +Bip85Mnemonic.mnemonic max_size:241 diff --git a/lib/board/signatures.c b/lib/board/signatures.c index c6b88026c..7cff0421b 100644 --- a/lib/board/signatures.c +++ b/lib/board/signatures.c @@ -20,7 +20,9 @@ #include "trezor/crypto/sha2.h" #include "trezor/crypto/ecdsa.h" #include "trezor/crypto/secp256k1.h" +#include "trezor/crypto/memzero.h" #include "keepkey/board/memory.h" +#include "keepkey/board/memcmp_s.h" #include "keepkey/board/signatures.h" #include "keepkey/board/pubkeys.h" @@ -68,23 +70,51 @@ int signatures_ok(void) { return KEY_EXPIRED; } /* Expired signing key */ + /* F3 hardening: double-compute SHA-256, compare in constant time */ + uint8_t firmware_fingerprint2[32]; sha256_Raw((uint8_t *)FLASH_APP_START, codelen, firmware_fingerprint); + asm volatile("" ::: "memory"); + sha256_Raw((uint8_t *)FLASH_APP_START, codelen, firmware_fingerprint2); - if (ecdsa_verify_digest(&secp256k1, pubkey[sigindex1 - 1], - (uint8_t *)FLASH_META_SIG1, - firmware_fingerprint) != 0) { /* Failure */ + if (memcmp_s(firmware_fingerprint, firmware_fingerprint2, 32) != 0) { + memzero(firmware_fingerprint, sizeof(firmware_fingerprint)); + memzero(firmware_fingerprint2, sizeof(firmware_fingerprint2)); return SIG_FAIL; } + memzero(firmware_fingerprint2, sizeof(firmware_fingerprint2)); - if (ecdsa_verify_digest(&secp256k1, pubkey[sigindex2 - 1], - (uint8_t *)FLASH_META_SIG2, - firmware_fingerprint) != 0) { /* Failure */ + /* F3 hardening: infective aggregation — accumulate all three ECDSA + * results instead of early-returning on each. Forces attacker to + * corrupt all three verify calls, not just skip one branch. */ + volatile int verify_acc = 0; + volatile int verify_sentinel = 0; + + verify_acc |= ecdsa_verify_digest(&secp256k1, pubkey[sigindex1 - 1], + (uint8_t *)FLASH_META_SIG1, + firmware_fingerprint); + verify_sentinel++; + asm volatile("" ::: "memory"); + + verify_acc |= ecdsa_verify_digest(&secp256k1, pubkey[sigindex2 - 1], + (uint8_t *)FLASH_META_SIG2, + firmware_fingerprint); + verify_sentinel++; + asm volatile("" ::: "memory"); + + verify_acc |= ecdsa_verify_digest(&secp256k1, pubkey[sigindex3 - 1], + (uint8_t *)FLASH_META_SIG3, + firmware_fingerprint); + verify_sentinel++; + asm volatile("" ::: "memory"); + + memzero(firmware_fingerprint, sizeof(firmware_fingerprint)); + + /* All three verifies must have executed and all must have passed */ + if (verify_sentinel != 3) { return SIG_FAIL; } - if (ecdsa_verify_digest(&secp256k1, pubkey[sigindex3 - 1], - (uint8_t *)FLASH_META_SIG3, - firmware_fingerprint) != 0) { /* Failure */ + if (verify_acc != 0) { return SIG_FAIL; } diff --git a/lib/firmware/recovery_cipher.c b/lib/firmware/recovery_cipher.c index 9677db5c6..0c8d4775a 100644 --- a/lib/firmware/recovery_cipher.c +++ b/lib/firmware/recovery_cipher.c @@ -462,6 +462,24 @@ void recovery_character(const char *character) { } } } else { + /* Per-word BIP39 validation: reject immediately if the decoded word + * doesn't match any entry in the wordlist. */ + if (enforce_wordlist && strlen(decoded_word) > 0) { + static CONFIDENTIAL char check_word[CURRENT_WORD_BUF]; + strlcpy(check_word, decoded_word, sizeof(check_word)); + bool valid = attempt_auto_complete(check_word); + memzero(check_word, sizeof(check_word)); + if (!valid) { + memzero(coded_word, sizeof(coded_word)); + memzero(decoded_word, sizeof(decoded_word)); + recovery_cipher_abort(); + fsm_sendFailure(FailureType_Failure_SyntaxError, + "Word not found in BIP39 wordlist"); + layoutHome(); + return; + } + } + memzero(coded_word, sizeof(coded_word)); memzero(decoded_word, sizeof(decoded_word)); @@ -575,7 +593,7 @@ void recovery_cipher_finalize(void) { } memzero(temp_word, sizeof(temp_word)); - if (!auto_completed && !enforce_wordlist) { + if (!auto_completed && enforce_wordlist) { if (!dry_run) { storage_reset(); } diff --git a/lib/firmware/signing.c b/lib/firmware/signing.c index ccc38c226..b6f3d4460 100644 --- a/lib/firmware/signing.c +++ b/lib/firmware/signing.c @@ -624,7 +624,7 @@ void signing_init(const SignTx *msg, const CoinType *_coin, branch_id = 0x5BA81B19; // Overwinter break; case 4: - branch_id = 0x76B809BB; // Sapling + branch_id = 0xC8E71055; // NU6 break; } } diff --git a/lib/transport/CMakeLists.txt b/lib/transport/CMakeLists.txt index 41b8d5864..d42d3e113 100644 --- a/lib/transport/CMakeLists.txt +++ b/lib/transport/CMakeLists.txt @@ -15,6 +15,9 @@ set(protoc_pb_sources ${DEVICE_PROTOCOL}/messages-tendermint.proto ${DEVICE_PROTOCOL}/messages-thorchain.proto ${DEVICE_PROTOCOL}/messages-mayachain.proto + ${DEVICE_PROTOCOL}/messages-solana.proto + ${DEVICE_PROTOCOL}/messages-tron.proto + ${DEVICE_PROTOCOL}/messages-ton.proto ${DEVICE_PROTOCOL}/messages.proto) set(protoc_pb_options @@ -29,6 +32,9 @@ set(protoc_pb_options ${CMAKE_SOURCE_DIR}/include/keepkey/transport/messages-tendermint.options ${CMAKE_SOURCE_DIR}/include/keepkey/transport/messages-thorchain.options ${CMAKE_SOURCE_DIR}/include/keepkey/transport/messages-mayachain.options + ${CMAKE_SOURCE_DIR}/include/keepkey/transport/messages-solana.options + ${CMAKE_SOURCE_DIR}/include/keepkey/transport/messages-tron.options + ${CMAKE_SOURCE_DIR}/include/keepkey/transport/messages-ton.options ${CMAKE_SOURCE_DIR}/include/keepkey/transport/messages.options) set(protoc_c_sources @@ -43,6 +49,9 @@ set(protoc_c_sources ${CMAKE_BINARY_DIR}/lib/transport/messages-tendermint.pb.c ${CMAKE_BINARY_DIR}/lib/transport/messages-thorchain.pb.c ${CMAKE_BINARY_DIR}/lib/transport/messages-mayachain.pb.c + ${CMAKE_BINARY_DIR}/lib/transport/messages-solana.pb.c + ${CMAKE_BINARY_DIR}/lib/transport/messages-tron.pb.c + ${CMAKE_BINARY_DIR}/lib/transport/messages-ton.pb.c ${CMAKE_BINARY_DIR}/lib/transport/messages.pb.c) set(protoc_c_headers @@ -57,6 +66,9 @@ set(protoc_c_headers ${CMAKE_BINARY_DIR}/include/messages-tendermint.pb.h ${CMAKE_BINARY_DIR}/include/messages-thorchain.pb.h ${CMAKE_BINARY_DIR}/include/messages-mayachain.pb.h + ${CMAKE_BINARY_DIR}/include/messages-solana.pb.h + ${CMAKE_BINARY_DIR}/include/messages-tron.pb.h + ${CMAKE_BINARY_DIR}/include/messages-ton.pb.h ${CMAKE_BINARY_DIR}/include/messages.pb.h) set(protoc_pb_sources_moved @@ -71,6 +83,9 @@ set(protoc_pb_sources_moved ${CMAKE_BINARY_DIR}/lib/transport/messages-tendermint.proto ${CMAKE_BINARY_DIR}/lib/transport/messages-thorchain.proto ${CMAKE_BINARY_DIR}/lib/transport/messages-mayachain.proto + ${CMAKE_BINARY_DIR}/lib/transport/messages-solana.proto + ${CMAKE_BINARY_DIR}/lib/transport/messages-tron.proto + ${CMAKE_BINARY_DIR}/lib/transport/messages-ton.proto ${CMAKE_BINARY_DIR}/lib/transport/messages.proto) add_custom_command( @@ -136,6 +151,18 @@ add_custom_command( ${PROTOC_BINARY} -I. -I/usr/include --plugin=nanopb=${NANOPB_DIR}/generator/protoc-gen-nanopb "--nanopb_out=-f messages-mayachain.options:." messages-mayachain.proto + COMMAND + ${PROTOC_BINARY} -I. -I/usr/include + --plugin=nanopb=${NANOPB_DIR}/generator/protoc-gen-nanopb + "--nanopb_out=-f messages-solana.options:." messages-solana.proto + COMMAND + ${PROTOC_BINARY} -I. -I/usr/include + --plugin=nanopb=${NANOPB_DIR}/generator/protoc-gen-nanopb + "--nanopb_out=-f messages-tron.options:." messages-tron.proto + COMMAND + ${PROTOC_BINARY} -I. -I/usr/include + --plugin=nanopb=${NANOPB_DIR}/generator/protoc-gen-nanopb + "--nanopb_out=-f messages-ton.options:." messages-ton.proto COMMAND ${PROTOC_BINARY} -I. -I/usr/include --plugin=nanopb=${NANOPB_DIR}/generator/protoc-gen-nanopb diff --git a/lib/transport/pb_decode.c b/lib/transport/pb_decode.c index 5ff4ecd7e..cff906307 100644 --- a/lib/transport/pb_decode.c +++ b/lib/transport/pb_decode.c @@ -452,14 +452,18 @@ static bool checkreturn decode_static_field(pb_istream_t *stream, } case PB_HTYPE_ONEOF: - *(pb_size_t *)iter->pSize = iter->pos->tag; - if (PB_LTYPE(type) == PB_LTYPE_SUBMESSAGE) { + if (PB_LTYPE(type) == PB_LTYPE_SUBMESSAGE && + *(pb_size_t*)iter->pSize != iter->pos->tag) + { /* We memset to zero so that any callbacks are set to NULL. - * Then set any default values. */ + * This is because the callbacks might otherwise have values + * from some other union field. */ memset(iter->pData, 0, iter->pos->data_size); pb_message_set_to_defaults((const pb_field_t *)iter->pos->ptr, iter->pData); } + *(pb_size_t*)iter->pSize = iter->pos->tag; + return func(stream, iter->pos, iter->pData); default: diff --git a/tools/firmware/keepkey_main.c b/tools/firmware/keepkey_main.c index fcf4aa9cb..95fcf94e3 100644 --- a/tools/firmware/keepkey_main.c +++ b/tools/firmware/keepkey_main.c @@ -174,8 +174,24 @@ int main(void) { _mmhusr_isr = (void *)&mmhisr; { // limit sigRet lifetime to this block + /* F5 hardening: replace full signatures_ok() (~1 sec crypto) with fast + * metadata presence check. The bootloader has already performed the + * authoritative signature verification before jumping here. We only + * need to know whether the bootloader considered us signed, which is + * indicated by valid signature indices in flash metadata. */ int sigRet = SIG_FAIL; - sigRet = signatures_ok(); + + volatile uint8_t si1 = *((volatile uint8_t *)FLASH_META_SIGINDEX1); + volatile uint8_t si2 = *((volatile uint8_t *)FLASH_META_SIGINDEX2); + volatile uint8_t si3 = *((volatile uint8_t *)FLASH_META_SIGINDEX3); + + if (si1 >= 1 && si1 <= PUBKEYS && + si2 >= 1 && si2 <= PUBKEYS && + si3 >= 1 && si3 <= PUBKEYS && + si1 != si2 && si1 != si3 && si2 != si3) { + sigRet = SIG_OK; + } + flash_collectHWEntropy(SIG_OK == sigRet); /* Drop privileges */