From db7915e946268631c902be13058c53e7fbd5418d Mon Sep 17 00:00:00 2001 From: Roman Zeyde Date: Tue, 20 Jun 2017 22:49:17 +0300 Subject: [PATCH] Allow testing recovery with an initialized device This would allow safe mnemonic validation by using a dry-run flag. --- firmware/fsm.c | 10 ++++- firmware/recovery.c | 100 ++++++++++++++++++++++++++++++++++---------- firmware/recovery.h | 2 +- 3 files changed, 86 insertions(+), 26 deletions(-) diff --git a/firmware/fsm.c b/firmware/fsm.c index f1d807b..a956025 100644 --- a/firmware/fsm.c +++ b/firmware/fsm.c @@ -988,7 +988,12 @@ void fsm_msgEstimateTxSize(EstimateTxSize *msg) void fsm_msgRecoveryDevice(RecoveryDevice *msg) { - CHECK_NOT_INITIALIZED + const bool dry_run = msg->has_dry_run ? msg->dry_run : false; + if (dry_run) { + CHECK_PIN + } else { + CHECK_NOT_INITIALIZED + } CHECK_PARAM(!msg->has_word_count || msg->word_count == 12 || msg->word_count == 18 || msg->word_count == 24, _("Invalid word count")); @@ -1000,7 +1005,8 @@ void fsm_msgRecoveryDevice(RecoveryDevice *msg) msg->has_label ? msg->label : 0, msg->has_enforce_wordlist && msg->enforce_wordlist, msg->has_type ? msg->type : 0, - msg->has_u2f_counter ? msg->u2f_counter : 0 + msg->has_u2f_counter ? msg->u2f_counter : 0, + dry_run ); } diff --git a/firmware/recovery.c b/firmware/recovery.c index da0a698..4ca56c5 100644 --- a/firmware/recovery.c +++ b/firmware/recovery.c @@ -44,6 +44,11 @@ static uint32_t word_count; */ static int awaiting_word = 0; +/* True if we should not write anything back to storage + * (can be used for testing seed for correctness). + */ +static bool dry_run; + /* True if we should check that seed corresponds to bip39. */ static bool enforce_wordlist; @@ -121,27 +126,73 @@ static void recovery_request(void) { msg_write(MessageType_MessageType_WordRequest, &resp); } +static bool is_same_mnemonic(const char *new_mnemonic) { + /* The execution time of the following code only depends on the + * (public) input. This avoids timing attacks. + */ + char diff = 0; + uint32_t i = 0; + for (; new_mnemonic[i]; i++) { + diff |= (storage.mnemonic[i] - new_mnemonic[i]); + } + diff |= storage.mnemonic[i]; + return diff == 0; +} + /* Called when the last word was entered. * Check mnemonic and send success/failure. */ static void recovery_done(void) { uint32_t i; - strlcpy(storage.mnemonic, words[0], sizeof(storage.mnemonic)); + char new_mnemonic[sizeof(storage.mnemonic)] = {0}; + + strlcpy(new_mnemonic, words[0], sizeof(new_mnemonic)); for (i = 1; i < word_count; i++) { - strlcat(storage.mnemonic, " ", sizeof(storage.mnemonic)); - strlcat(storage.mnemonic, words[i], sizeof(storage.mnemonic)); + strlcat(new_mnemonic, " ", sizeof(new_mnemonic)); + strlcat(new_mnemonic, words[i], sizeof(new_mnemonic)); } - if (!enforce_wordlist || mnemonic_check(storage.mnemonic)) { - storage.has_mnemonic = true; - if (!enforce_wordlist) { - // not enforcing => mark storage as imported - storage.has_imported = true; - storage.imported = true; + if (!enforce_wordlist || mnemonic_check(new_mnemonic)) { + // New mnemonic is valid. + if (!dry_run) { + // Update mnemonic on storage. + storage.has_mnemonic = true; + strlcpy(storage.mnemonic, new_mnemonic, sizeof(new_mnemonic)); + if (!enforce_wordlist) { + // not enforcing => mark storage as imported + storage.has_imported = true; + storage.imported = true; + } + storage_commit(); + fsm_sendSuccess(_("Device recovered")); + } else { + // Inform the user about new mnemonic correctness (as well as whether it is the same as the current one). + const bool same_mnemonic = is_same_mnemonic(new_mnemonic); + if (same_mnemonic) { + layoutDialog(&bmp_icon_ok, NULL, _("Confirm"), NULL, + _("The mnemonic is"), + _("valid and matches"), + _("existing one."), NULL, NULL, NULL); + protectButton(ButtonRequestType_ButtonRequest_Other, true); + fsm_sendSuccess(_("Mnemonic is valid and matches existing one")); + } else { + layoutDialog(&bmp_icon_error, NULL, _("Confirm"), NULL, + _("The mnemonic is"), + _("valid but doesn't"), + _("match existing one."), NULL, NULL, NULL); + protectButton(ButtonRequestType_ButtonRequest_Other, true); + fsm_sendFailure(FailureType_Failure_DataError, + _("Mnemonic is valid but doesn't match existing one")); + } } - storage_commit(); - fsm_sendSuccess(_("Device recovered")); } else { - storage_reset(); + // New mnemonic is invalid. + if (!dry_run) { + storage_reset(); + } else { + layoutDialog(&bmp_icon_error, NULL, _("Confirm"), NULL, + _("The mnemonic is"), _("invalid!"), NULL, NULL, NULL, NULL); + protectButton(ButtonRequestType_ButtonRequest_Other, true); + } fsm_sendFailure(FailureType_Failure_DataError, _("Invalid mnemonic, are words in correct order?")); } awaiting_word = 0; @@ -369,25 +420,28 @@ void next_word(void) { recovery_request(); } -void recovery_init(uint32_t _word_count, bool passphrase_protection, bool pin_protection, const char *language, const char *label, bool _enforce_wordlist, uint32_t type, uint32_t u2f_counter) +void recovery_init(uint32_t _word_count, bool passphrase_protection, bool pin_protection, const char *language, const char *label, bool _enforce_wordlist, uint32_t type, uint32_t u2f_counter, bool _dry_run) { if (_word_count != 12 && _word_count != 18 && _word_count != 24) return; word_count = _word_count; enforce_wordlist = _enforce_wordlist; + dry_run = _dry_run; - if (pin_protection && !protectChangePin()) { - fsm_sendFailure(FailureType_Failure_ActionCancelled, NULL); - layoutHome(); - return; + if (!dry_run) { + if (pin_protection && !protectChangePin()) { + fsm_sendFailure(FailureType_Failure_ActionCancelled, NULL); + layoutHome(); + return; + } + + storage.has_passphrase_protection = true; + storage.passphrase_protection = passphrase_protection; + storage_setLanguage(language); + storage_setLabel(label); + storage_setU2FCounter(u2f_counter); } - storage.has_passphrase_protection = true; - storage.passphrase_protection = passphrase_protection; - storage_setLanguage(language); - storage_setLabel(label); - storage_setU2FCounter(u2f_counter); - if ((type & RecoveryDeviceType_RecoveryDeviceType_Matrix) != 0) { awaiting_word = 2; word_index = 0; diff --git a/firmware/recovery.h b/firmware/recovery.h index ac463d2..fdd61dc 100644 --- a/firmware/recovery.h +++ b/firmware/recovery.h @@ -23,7 +23,7 @@ #include #include -void recovery_init(uint32_t _word_count, bool passphrase_protection, bool pin_protection, const char *language, const char *label, bool _enforce_wordlist, uint32_t type, uint32_t u2f_counter); +void recovery_init(uint32_t _word_count, bool passphrase_protection, bool pin_protection, const char *language, const char *label, bool _enforce_wordlist, uint32_t type, uint32_t u2f_counter, bool _dry_run); void recovery_word(const char *word); void recovery_abort(void); const char *recovery_get_fake_word(void);