trezor-firmware icon indicating copy to clipboard operation
trezor-firmware copied to clipboard

remove obsolete bootloader antiglitch protection

Open TychoVrahe opened this issue 6 months ago • 2 comments

As we are now always rebooting mcu when jumping to firmware, this protection is no longer needed as all the checks will be repeated on reboot. in case skipping the reboot due to glitch, there are still integrity checks right before jumping to fw.

fixes #4805

TychoVrahe avatar Jun 10 '25 06:06 TychoVrahe

we got a review comment from @PrisionMike on the original issue but it got lost, I asked him to resubmit it. something to do with moving one of the checks into a different place instead of removing it.

matejcik avatar Jun 18 '25 11:06 matejcik

Hey! Just to add a few points from the time I reviewed the code:

  1. The referred code does seem redundant at that location. There are no plausible reasons to justify protecting the reboot routine. But,
  2. It is strongly recommended to add the integrity check just before the final jump instruction at l241. This is the critical jump and the spread between the last integrity check and this instruction is large.

Other less pressing but still important measures could be:

  1. Moving the random delay from the start to the real_jump . This will ensure that no side channel analysis would leak the exact program location of the sensitive jump. (Optional)
  2. The ensure subroutine is comparing the flags with themselves. The general practise is to compare the flag with its compliment. Marginally safer and faster.

PrisionMike avatar Jun 24 '25 11:06 PrisionMike

IIUC, items 2,3 and 4 are unrelated to 1 (which is handled by this PR) and can be addressed in separate PR without affecting overall security by merging this one, right?

TychoVrahe avatar Jun 24 '25 17:06 TychoVrahe

Agreed.

PrisionMike avatar Jun 27 '25 12:06 PrisionMike

ok then, let's merge this one. @PrisionMike can you please make a separate issue for 2+3+4?

matejcik avatar Jun 27 '25 12:06 matejcik

Sure @matejcik

PrisionMike avatar Jun 27 '25 12:06 PrisionMike