diff --git a/tiny-firmware/bootloader/usb.c b/tiny-firmware/bootloader/usb.c index ffd9472..cafab80 100644 --- a/tiny-firmware/bootloader/usb.c +++ b/tiny-firmware/bootloader/usb.c @@ -178,6 +178,7 @@ static int hid_control_request(usbd_device* dev, struct usb_setup_data* req, uin enum { STATE_READY, STATE_OPEN, + STATE_ABOUT_TO_FLASHSTART, STATE_FLASHSTART, STATE_FLASHING, STATE_CHECK, @@ -481,59 +482,65 @@ static void hid_rx_callback(usbd_device* dev, uint8_t ep) if (flash_state == STATE_OPEN) { if (msg_id == 0x0006) { // FirmwareErase message (id 6) if (!brand_new_firmware) { - layoutDialog(&bmp_icon_question, "Abort", "Continue", NULL, "Install new", "firmware?", NULL, "Never do this without", "your recovery card!", NULL); - do { - delay(100000); - buttonUpdate(); - } while (!button.YesUp && !button.NoUp); + flash_state = STATE_ABOUT_TO_FLASHSTART; + send_msg_buttonrequest_firmwarecheck(dev); + return; } - if (brand_new_firmware || button.YesUp) { - // check whether current firmware is signed - if (!brand_new_firmware && SIG_OK == signatures_ok(NULL)) { - old_was_unsigned = false; - // backup metadata - backup_metadata(meta_backup); - } else { - old_was_unsigned = true; - } - flash_wait_for_last_operation(); - flash_clear_status_flags(); - flash_unlock(); - // erase metadata area - for (int i = FLASH_META_SECTOR_FIRST; i <= FLASH_META_SECTOR_LAST; i++) { - layoutProgress("ERASING ... Please wait", 1000 * (i - FLASH_META_SECTOR_FIRST) / (FLASH_CODE_SECTOR_LAST - FLASH_META_SECTOR_FIRST)); - flash_erase_sector(i, FLASH_CR_PROGRAM_X32); - } - // erase code area - for (int i = FLASH_CODE_SECTOR_FIRST; i <= FLASH_CODE_SECTOR_LAST; i++) { - layoutProgress("ERASING ... Please wait", 1000 * (i - FLASH_META_SECTOR_FIRST) / (FLASH_CODE_SECTOR_LAST - FLASH_META_SECTOR_FIRST)); - flash_erase_sector(i, FLASH_CR_PROGRAM_X32); - } - layoutProgress("INSTALLING ... Please wait", 0); - flash_wait_for_last_operation(); - flash_lock(); - - // check that metadata was succesfully erased - // flash status register should show now error and - // the config block should contain only \xff. - uint8_t hash[32]; - sha256_Raw((unsigned char*)FLASH_META_START, FLASH_META_LEN, hash); - if ((FLASH_SR & (FLASH_SR_PGAERR | FLASH_SR_PGPERR | FLASH_SR_PGSERR | FLASH_SR_WRPERR)) != 0 || memcmp(hash, "\x2d\x86\x4c\x0b\x78\x9a\x43\x21\x4e\xee\x85\x24\xd3\x18\x20\x75\x12\x5e\x5c\xa2\xcd\x52\x7f\x35\x82\xec\x87\xff\xd9\x40\x76\xbc", 32) != 0) { - send_msg_failure(dev); - flash_state = STATE_END; - layoutDialog(&bmp_icon_error, NULL, NULL, NULL, "Error installing ", "firmware.", NULL, "Unplug your Skywallet", "and try again.", NULL); - return; - } + } + } + if (flash_state == STATE_ABOUT_TO_FLASHSTART) { + if (!brand_new_firmware) { + layoutDialog(&bmp_icon_question, "Abort", "Continue", NULL, "Install new", "firmware?", NULL, "Never do this without", "your recovery card!", NULL); + do { + delay(100000); + buttonUpdate(); + } while (!button.YesUp && !button.NoUp); + } + if (brand_new_firmware || button.YesUp) { + // check whether current firmware is signed + if (!brand_new_firmware && SIG_OK == signatures_ok(NULL)) { + old_was_unsigned = false; + // backup metadata + backup_metadata(meta_backup); + } else { + old_was_unsigned = true; + } + flash_wait_for_last_operation(); + flash_clear_status_flags(); + flash_unlock(); + // erase metadata area + for (int i = FLASH_META_SECTOR_FIRST; i <= FLASH_META_SECTOR_LAST; i++) { + layoutProgress("ERASING ... Please wait", 1000 * (i - FLASH_META_SECTOR_FIRST) / (FLASH_CODE_SECTOR_LAST - FLASH_META_SECTOR_FIRST)); + flash_erase_sector(i, FLASH_CR_PROGRAM_X32); + } + // erase code area + for (int i = FLASH_CODE_SECTOR_FIRST; i <= FLASH_CODE_SECTOR_LAST; i++) { + layoutProgress("ERASING ... Please wait", 1000 * (i - FLASH_META_SECTOR_FIRST) / (FLASH_CODE_SECTOR_LAST - FLASH_META_SECTOR_FIRST)); + flash_erase_sector(i, FLASH_CR_PROGRAM_X32); + } + layoutProgress("INSTALLING ... Please wait", 0); + flash_wait_for_last_operation(); + flash_lock(); - send_msg_success(dev); - flash_state = STATE_FLASHSTART; + // check that metadata was succesfully erased + // flash status register should show now error and + // the config block should contain only \xff. + uint8_t hash[32]; + sha256_Raw((unsigned char*)FLASH_META_START, FLASH_META_LEN, hash); + if ((FLASH_SR & (FLASH_SR_PGAERR | FLASH_SR_PGPERR | FLASH_SR_PGSERR | FLASH_SR_WRPERR)) != 0 || memcmp(hash, "\x2d\x86\x4c\x0b\x78\x9a\x43\x21\x4e\xee\x85\x24\xd3\x18\x20\x75\x12\x5e\x5c\xa2\xcd\x52\x7f\x35\x82\xec\x87\xff\xd9\x40\x76\xbc", 32) != 0) { + send_msg_failure(dev); + flash_state = STATE_END; + layoutDialog(&bmp_icon_error, NULL, NULL, NULL, "Error installing ", "firmware.", NULL, "Unplug your Skywallet", "and try again.", NULL); return; } - send_msg_failure(dev); - flash_state = STATE_END; - layoutDialog(&bmp_icon_warning, NULL, NULL, NULL, "Firmware installation", "aborted.", NULL, "You may now", "unplug your Skywallet.", NULL); + + send_msg_success(dev); + flash_state = STATE_FLASHSTART; return; } + send_msg_failure(dev); + flash_state = STATE_END; + layoutDialog(&bmp_icon_warning, NULL, NULL, NULL, "Firmware installation", "aborted.", NULL, "You may now", "unplug your Skywallet.", NULL); return; } diff --git a/tiny-firmware/firmware/fsm_impl.c b/tiny-firmware/firmware/fsm_impl.c index e3aa50b..0db0482 100644 --- a/tiny-firmware/firmware/fsm_impl.c +++ b/tiny-firmware/firmware/fsm_impl.c @@ -535,18 +535,24 @@ ErrCode_t msgPingImpl(Ping* msg) return ErrOk; } -ErrCode_t msgChangePinImpl(ChangePin* msg, const char* (*funcRequestPin)(PinMatrixRequestType, const char*)) +ErrCode_t msgChangePinImpl(ChangePin* msg, ErrCode_t (*funcRequestPin)(PinMatrixRequestType, const char*, char*)) { bool removal = msg->has_remove && msg->remove; if (removal) { storage_setPin(""); storage_update(); - } else { - if (!protectChangePinEx(funcRequestPin)) { - return ErrPinMismatch; - } + return ErrOk; + } + ErrCode_t err = protectChangePinEx(funcRequestPin); + switch (err) { + case ErrPinRequired: + case ErrPinCancelled: + case ErrPinMismatch: + case ErrOk: + return err; + default: + return ErrUnexpectedMessage; } - return ErrOk; } ErrCode_t msgWipeDeviceImpl(WipeDevice* msg) diff --git a/tiny-firmware/firmware/fsm_impl.h b/tiny-firmware/firmware/fsm_impl.h index 4c63ae6..7a29a04 100644 --- a/tiny-firmware/firmware/fsm_impl.h +++ b/tiny-firmware/firmware/fsm_impl.h @@ -192,7 +192,7 @@ ErrCode_t msgGetFeaturesImpl(Features* resp); ErrCode_t msgPingImpl(Ping* msg); -ErrCode_t msgChangePinImpl(ChangePin* msg, const char* (*)(PinMatrixRequestType, const char*)); +ErrCode_t msgChangePinImpl(ChangePin* msg, ErrCode_t (*)(PinMatrixRequestType, const char*, char*)); ErrCode_t msgWipeDeviceImpl(WipeDevice* msg); diff --git a/tiny-firmware/firmware/protect.c b/tiny-firmware/firmware/protect.c index 8af30ac..0a20f1a 100644 --- a/tiny-firmware/firmware/protect.c +++ b/tiny-firmware/firmware/protect.c @@ -112,7 +112,7 @@ bool protectButton(ButtonRequestType type, bool confirm_only) return result; } -const char* requestPin(PinMatrixRequestType type, const char* text) +ErrCode_t requestPin(PinMatrixRequestType type, const char* text, char* out_pin) { PinMatrixRequest resp; memset(&resp, 0, sizeof(PinMatrixRequest)); @@ -128,16 +128,23 @@ const char* requestPin(PinMatrixRequestType type, const char* text) PinMatrixAck* pma = (PinMatrixAck*)msg_tiny; pinmatrix_done(pma->pin); // convert via pinmatrix usbTiny(0); - return pma->pin; + memcpy(out_pin, pma->pin, sizeof(pma->pin)); + return ErrOk; } if (msg_tiny_id == MessageType_MessageType_Cancel || msg_tiny_id == MessageType_MessageType_Initialize) { pinmatrix_done(0); if (msg_tiny_id == MessageType_MessageType_Initialize) { protectAbortedByInitialize = true; + msg_tiny_id = 0xFFFF; + usbTiny(0); + // TODO what does means Initialize here? + return ErrOk; + } + if (msg_tiny_id == MessageType_MessageType_Cancel) { + msg_tiny_id = 0xFFFF; + usbTiny(0); + return ErrPinCancelled; } - msg_tiny_id = 0xFFFF; - usbTiny(0); - return 0; } #if DEBUG_LINK if (msg_tiny_id == MessageType_MessageType_DebugLinkGetState) { @@ -195,11 +202,20 @@ bool protectPin(bool use_cached) wait--; } usbTiny(0); - const char* pin; - pin = requestPin(PinMatrixRequestType_PinMatrixRequestType_Current, _("Please enter current PIN:")); - if (!pin) { + char pin[10] = {0}; + { + PinMatrixAck pm; + _Static_assert(sizeof(pin) == sizeof(pm.pin), "invalid pin buffer size"); + } + switch (requestPin(PinMatrixRequestType_PinMatrixRequestType_Current, _("Please enter current PIN:"), pin)) { + case ErrOk: + break; + case ErrPinCancelled: fsm_sendFailure(FailureType_Failure_PinCancelled, NULL, 0); return false; + default: + fsm_sendFailure(FailureType_Failure_UnexpectedMessage, NULL, 0); + return false; } if (!storage_increasePinFails(fails)) { fsm_sendFailure(FailureType_Failure_PinInvalid, NULL, 0); @@ -221,33 +237,60 @@ bool protectChangePin() return protectChangePinEx(NULL); } -bool protectChangePinEx(const char* (*funcRequestPin)(PinMatrixRequestType, const char*)) +ErrCode_t protectChangePinEx(ErrCode_t (*funcRequestPin)(PinMatrixRequestType, const char*, char*)) { static CONFIDENTIAL char pin_compare[17]; + memset(pin_compare, 0, sizeof(pin_compare)); if (funcRequestPin == NULL) { funcRequestPin = requestPin; } - - const char* pin = funcRequestPin(PinMatrixRequestType_PinMatrixRequestType_NewFirst, _("Please enter new PIN:")); - - if (!pin) { - return false; + static CONFIDENTIAL char pin[10] = {0}; + memset(pin, 0, sizeof(pin)); + { + PinMatrixAck pm; + _Static_assert(sizeof(pin) == sizeof(pm.pin), "invalid pin buffer size"); + } + ErrCode_t err = funcRequestPin(PinMatrixRequestType_PinMatrixRequestType_NewFirst, _("Please enter new PIN:"), pin); + if (err != ErrOk) { + memset(pin_compare, 0, sizeof(pin_compare)); + memset(pin, 0, sizeof(pin)); + return err; + } + { + char empty_pin[sizeof(pin)] = {0}; + if (!memcmp(pin, empty_pin, sizeof(pin))) { + memset(pin_compare, 0, sizeof(pin_compare)); + memset(pin, 0, sizeof(pin)); + return ErrPinRequired; + } } - strlcpy(pin_compare, pin, sizeof(pin_compare)); - - pin = funcRequestPin(PinMatrixRequestType_PinMatrixRequestType_NewSecond, _("Please re-enter new PIN:")); - - const bool result = pin && *pin && (strncmp(pin_compare, pin, sizeof(pin_compare)) == 0); - - if (result) { + memset(pin, 0, sizeof(pin)); + err = funcRequestPin(PinMatrixRequestType_PinMatrixRequestType_NewSecond, _("Please re-enter new PIN:"), pin); + if (err != ErrOk) { + memset(pin_compare, 0, sizeof(pin_compare)); + memset(pin, 0, sizeof(pin)); + return err; + } + { + char empty_pin[sizeof(pin)] = {0}; + if (!memcmp(pin, empty_pin, sizeof(pin))) { + memset(pin_compare, 0, sizeof(pin_compare)); + memset(pin, 0, sizeof(pin)); + return ErrPinRequired; + } + } + if (strncmp(pin_compare, pin, sizeof(pin_compare)) == 0) { storage_setPin(pin_compare); storage_update(); + } else { + memset(pin_compare, 0, sizeof(pin_compare)); + memset(pin, 0, sizeof(pin)); + return ErrPinMismatch; } - - memzero(pin_compare, sizeof(pin_compare)); - - return result; + memset(pin_compare, 0, sizeof(pin_compare)); + memset(pin, 0, sizeof(pin)); + return ErrOk; } bool protectPassphrase(void) diff --git a/tiny-firmware/firmware/protect.h b/tiny-firmware/firmware/protect.h index 5a413c7..2fc621c 100644 --- a/tiny-firmware/firmware/protect.h +++ b/tiny-firmware/firmware/protect.h @@ -18,9 +18,10 @@ * along with this library. If not, see . */ -#ifndef __PROTECT_H__ -#define __PROTECT_H__ +#ifndef __PROTECT_HANDLER_H__ +#define __PROTECT_HANDLER_H__ +#include "tiny-firmware/firmware/error.h" #include "types.pb.h" #include @@ -32,7 +33,7 @@ bool protectPassphrase(void); extern bool protectAbortedByInitialize; // Symbols exported for testing -bool protectChangePinEx(const char* (*)(PinMatrixRequestType, const char*)); -const char* requestPin(PinMatrixRequestType type, const char* text); +ErrCode_t protectChangePinEx(ErrCode_t (*)(PinMatrixRequestType, const char*, char*)); +ErrCode_t requestPin(PinMatrixRequestType type, const char* text, char* out_pin); -#endif +#endif // __PROTECT_HANDLER_H__ diff --git a/tiny-firmware/firmware/reset.c b/tiny-firmware/firmware/reset.c index 34b18a6..c688992 100644 --- a/tiny-firmware/firmware/reset.c +++ b/tiny-firmware/firmware/reset.c @@ -41,7 +41,7 @@ void reset_init(bool display_random, uint32_t _strength, bool passphrase_protect reset_init_ex(display_random, _strength, passphrase_protection, pin_protection, language, label, _skip_backup, NULL); } -void reset_init_ex(bool display_random, uint32_t _strength, bool passphrase_protection, bool pin_protection, const char* language, const char* label, bool _skip_backup, const char* (*funcRequestPin)(PinMatrixRequestType, const char*)) +void reset_init_ex(bool display_random, uint32_t _strength, bool passphrase_protection, bool pin_protection, const char* language, const char* label, bool _skip_backup, ErrCode_t (*funcRequestPin)(PinMatrixRequestType, const char*, char*)) { if (funcRequestPin == NULL) { funcRequestPin = requestPin; @@ -70,13 +70,20 @@ void reset_init_ex(bool display_random, uint32_t _strength, bool passphrase_prot return; } } - - if (pin_protection && !protectChangePinEx(funcRequestPin)) { - fsm_sendFailure(FailureType_Failure_PinMismatch, NULL, 0); - layoutHome(); - return; + if (pin_protection) { + ErrCode_t err = protectChangePinEx(funcRequestPin); + switch (err) { + case ErrOk: + break; + case ErrPinMismatch: + fsm_sendFailure(FailureType_Failure_PinMismatch, NULL, 0); + layoutHome(); + return; + default: + fsm_sendFailure(FailureType_Failure_UnexpectedMessage, NULL, 0); + return; + } } - storage_setPassphraseProtection(passphrase_protection); storage_setLanguage(language); if (label != NULL && strcmp("", label) != 0) { diff --git a/tiny-firmware/firmware/reset.h b/tiny-firmware/firmware/reset.h index 679a2ad..38d8dbe 100644 --- a/tiny-firmware/firmware/reset.h +++ b/tiny-firmware/firmware/reset.h @@ -40,6 +40,6 @@ uint32_t reset_get_int_entropy(uint8_t* entropy); const char* reset_get_word(void); // Functions exported or testing purposes -void reset_init_ex(bool display_random, uint32_t _strength, bool passphrase_protection, bool pin_protection, const char* language, const char* label, bool _skip_backup, const char* (*funcRequestPin)(PinMatrixRequestType mt, const char* msg)); +void reset_init_ex(bool display_random, uint32_t _strength, bool passphrase_protection, bool pin_protection, const char* language, const char* label, bool _skip_backup, ErrCode_t (*funcRequestPin)(PinMatrixRequestType mt, const char* msg, char* out_pin)); #endif diff --git a/tiny-firmware/tests/test_fsm.c b/tiny-firmware/tests/test_fsm.c index 272c4d3..d5f05ba 100644 --- a/tiny-firmware/tests/test_fsm.c +++ b/tiny-firmware/tests/test_fsm.c @@ -416,6 +416,36 @@ START_TEST(test_msgChangePinSecondRejected) } END_TEST +START_TEST(test_msgChangePinShouldReturnCanceledAccordingToPinReaderNewPin) +{ + ChangePin msg = ChangePin_init_zero; + storage_wipe(); + ck_assert_int_eq(storage_hasPin(), false); + ck_assert_int_eq(msgChangePinImpl(&msg, &pin_reader_new_canceled), ErrPinCancelled); + ck_assert_int_eq(storage_hasPin(), false); +} +END_TEST + +START_TEST(test_msgChangePinShouldReturnCanceledAccordingToPinReaderButPreserveAPreviuosOne) +{ + ErrCode_t (*pin_readers[2])(PinMatrixRequestType, const char*, char*) = { + &pin_reader_new_canceled, &pin_reader_confirm_canceled}; + for (size_t i = 0; i < sizeof(pin_readers) / sizeof(*pin_readers); ++i) { + ChangePin msg = ChangePin_init_zero; + storage_wipe(); + ck_assert_int_eq(storage_hasPin(), false); + + ck_assert_int_eq(msgChangePinImpl(&msg, &pin_reader_ok), ErrOk); + ck_assert_int_eq(storage_hasPin(), true); + ck_assert_str_eq(storage_getPin(), TEST_PIN1); + + ck_assert_int_eq(msgChangePinImpl(&msg, pin_readers[i]), ErrPinCancelled); + ck_assert_int_eq(storage_hasPin(), true); + ck_assert_str_eq(storage_getPin(), TEST_PIN1); + } +} +END_TEST + START_TEST(test_msgSkycoinCheckMessageSignatureBip44Ok) { for (size_t wi = 0; wi < sizeof(wcs) / sizeof(*wcs); ++wi) { @@ -532,6 +562,8 @@ TCase* add_fsm_tests(TCase* tc) tcase_add_test(tc, test_msgEntropyAckChgMixerNotInternal); tcase_add_test(tc, test_msgChangePinSuccess); tcase_add_test(tc, test_msgChangePinSecondRejected); + tcase_add_test(tc, test_msgChangePinShouldReturnCanceledAccordingToPinReaderNewPin); + tcase_add_test(tc, test_msgChangePinShouldReturnCanceledAccordingToPinReaderButPreserveAPreviuosOne); tcase_add_test(tc, test_msgChangePinEditSuccess); tcase_add_test(tc, test_msgChangePinRemoveSuccess); tcase_add_test(tc, test_isSha256DigestHex); diff --git a/tiny-firmware/tests/test_pin.c b/tiny-firmware/tests/test_pin.c index 8471429..82fd36f 100644 --- a/tiny-firmware/tests/test_pin.c +++ b/tiny-firmware/tests/test_pin.c @@ -14,30 +14,65 @@ char* TEST_PIN1 = "123"; char* TEST_PIN2 = "246"; -const char* pin_reader_ok(PinMatrixRequestType pinReqType, const char* text) +ErrCode_t pin_reader_ok(PinMatrixRequestType pinReqType, const char* text, char* out_pin) { (void)text; (void)pinReqType; - return TEST_PIN1; + strcpy(out_pin, TEST_PIN1); + return ErrOk; } -const char* pin_reader_alt(PinMatrixRequestType pinReqType, const char* text) +ErrCode_t pin_reader_alt(PinMatrixRequestType pinReqType, const char* text, char* pin_out) { (void)text; (void)pinReqType; - return TEST_PIN2; + strcpy(pin_out, TEST_PIN2); + return ErrOk; } -const char* pin_reader_wrong(PinMatrixRequestType pinReqType, const char* text) +ErrCode_t pin_reader_wrong(PinMatrixRequestType pinReqType, const char* text, char* pin_out) { (void)text; switch (pinReqType) { case PinMatrixRequestType_PinMatrixRequestType_NewFirst: - return TEST_PIN1; + strcpy(pin_out, TEST_PIN1); + break; case PinMatrixRequestType_PinMatrixRequestType_NewSecond: - return "456"; + strcpy(pin_out, "456"); + break; default: break; } - return "789"; + strcpy(pin_out, "789"); + return ErrPinMismatch; +} + +ErrCode_t pin_reader_new_canceled(PinMatrixRequestType pinReqType, const char* text, char* pin_out) +{ + (void)text; + switch (pinReqType) { + case PinMatrixRequestType_PinMatrixRequestType_NewFirst: + return ErrPinCancelled; + case PinMatrixRequestType_PinMatrixRequestType_Current: + case PinMatrixRequestType_PinMatrixRequestType_NewSecond: + strcpy(pin_out, TEST_PIN1); + return ErrOk; + default: + return ErrInvalidArg; + } +} + +ErrCode_t pin_reader_confirm_canceled(PinMatrixRequestType pinReqType, const char* text, char* pin_out) +{ + (void)text; + switch (pinReqType) { + case PinMatrixRequestType_PinMatrixRequestType_NewSecond: + return ErrPinCancelled; + case PinMatrixRequestType_PinMatrixRequestType_Current: + case PinMatrixRequestType_PinMatrixRequestType_NewFirst: + strcpy(pin_out, TEST_PIN1); + return ErrOk; + default: + return ErrInvalidArg; + } } diff --git a/tiny-firmware/tests/test_pin.h b/tiny-firmware/tests/test_pin.h index df42300..7dcc1d6 100644 --- a/tiny-firmware/tests/test_pin.h +++ b/tiny-firmware/tests/test_pin.h @@ -14,8 +14,12 @@ extern char* TEST_PIN1; extern char* TEST_PIN2; -const char* pin_reader_ok(PinMatrixRequestType pinReqType, const char* text); +ErrCode_t pin_reader_ok(PinMatrixRequestType pinReqType, const char* text, char* out_pin); -const char* pin_reader_alt(PinMatrixRequestType pinReqType, const char* text); +ErrCode_t pin_reader_alt(PinMatrixRequestType pinReqType, const char* text, char* pin_out); -const char* pin_reader_wrong(PinMatrixRequestType pinReqType, const char* text); +ErrCode_t pin_reader_wrong(PinMatrixRequestType pinReqType, const char* text, char* pin_out); + +ErrCode_t pin_reader_new_canceled(PinMatrixRequestType pinReqType, const char* text, char* pin_out); + +ErrCode_t pin_reader_confirm_canceled(PinMatrixRequestType pinReqType, const char* text, char* pin_out);