Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

T1 downgrade to firmware not compatible with current bootloader is allowed #3706

Closed
alex-jerechinsky opened this issue May 4, 2021 · 11 comments
Assignees
Labels
bug Something isn't working as expected

Comments

@alex-jerechinsky
Copy link
Member

T1 downgrade to firmware with lower bootloader required (1.10.0 -> 1.8.0) is allowed, which results in a "Unknown bootloader detected" error and info to contact support (which is unneccessary).

Expected:
Downgrade to firmware mentioned above is not allowed, the same way downgrade to 1.7.3 is not (Firmware is too old for your device. Aborting.)
Actual:
"Unknown bootloader detected. Unplug your TREZOR contact our support" is displayed.

image

Note:
attempt to downgrade to 1.7.3 from 1.10.0 in trezoctl

$ trezorctl firmware-update -v 1.7.3
Downloading from https://wallet.trezor.io/data/firmware/1/trezor-1.7.3.bin
Trezor One firmware image.
Signatures are valid.
Firmware fingerprint: 10acc6aa4f24aff36627473b98c23dc4f6d0220d33bc1e09cb572f02410ffdaf
Firmware is too old for your device. Aborting.

Originally posted by @sorooris in #1871 (comment)

@alex-jerechinsky alex-jerechinsky changed the title T1 downgrade to firmware with incompatible bootloader is allowed T1 downgrade to firmware not compatible with current bootloader is allowed May 4, 2021
@alex-jerechinsky alex-jerechinsky added this to 🎯 To do in Suite · June 9 🌴 via automation May 4, 2021
@alex-jerechinsky alex-jerechinsky added the bug Something isn't working as expected label May 4, 2021
@alex-jerechinsky alex-jerechinsky removed this from 🎯 To do in Suite · June 9 🌴 May 4, 2021
@alex-jerechinsky alex-jerechinsky added this to 🎯 To do in Suite · July 14 🌊 via automation May 7, 2021
@matejzak matejzak removed this from 🎯 To do in Suite · July 14 🌊 Jun 3, 2021
@tsusanka
Copy link
Contributor

tsusanka commented Jun 3, 2021

We should consider doing this along with #1871.

@alex-jerechinsky
Copy link
Member Author

@marekrjpolak do you need some product input on how to handle this or is there a clear way to abort the firmware downgrade and explain it to the user?

@marekrjpolak
Copy link
Contributor

Currently I handle situations like this in #4170:
image

Unfortunately, 1.8.0 installation from 1.10.0 is allowed in that PR, so I have to come with some new validation rule.

@marekrjpolak
Copy link
Contributor

I've tested it and Suite currently behaves almost exactly as trezorctl: doesn't allow to install 1.7.3 on 1.10.0 but allows to install 1.8.0 which results in Unknown bootloader detected error.

@tsusanka any idea how to detect this error from the firmware binary in advance?

@tsusanka
Copy link
Contributor

tsusanka commented Sep 7, 2021

@matejcik will be able to answer your question properly. My few cents are that I think we should do this only if it is simple enough. We can also hardcode some table against which we would check..

@matejcik
Copy link

matejcik commented Sep 7, 2021

any idea how to detect this error from the firmware binary in advance?

There is no reasonably easy way to do it.

What you can do is look up firmware fingerprint in releases.json and check if the bootloader is older than what's currently on device.

@marekrjpolak
Copy link
Contributor

AFAIK the functionality to obtain fingerprints of firmware binaries is currently not implemented in Suite, and if I'm looking correctly, in trezorctl there are at least 3 different ways how to calculate them based on firmware version. Moreover, it requires advanced parsing, so it would be rather complex task in javascript. Should I implement it anyway, @alex-jerechinsky?

@matejcik
Copy link

The easy way to go about is to do only the one method that is relevant here:

  1. strip first 256 bytes
  2. take a sha256 of the result

I also realized that we only care about bootloaders 1.8.0 and up, correct? There is a reasonably easy algorithm to either get the firmware version, or know that it's incompatible regardless of version.

  1. If firmware[:4] == "TRZR" then strip first 256 bytes: firmware = firmware[256:].
  2. If now firmware[:4] != "TRZF", you can't even install it on BL 1.8.0 and up. Fail here.
  3. version_bytes = firmware[128 : 128 + 4]

version_bytes are four int8 values: major, minor, fix, build. Look up major.minor.fix in releases.json, see what bootloader version is there, compare to what you have now.

@marekrjpolak
Copy link
Contributor

  1. Could it be version_bytes = firmware[16 : 16 + 4] instead? There seems to be nothing useful at indexes you wrote.
  2. This method could only eliminate our 'official' firmware incompatibilities between 1.8.0 and 1.9.4, right? There is no way how to prevent bootloader version incompatibility for 'truly' custom firmware?

@matejcik
Copy link

  1. Could it be version_bytes = firmware[16 : 16 + 4] instead?

yes, you're right, i was wrongly counting in bits :)

  1. There is no way how to prevent bootloader version incompatibility for 'truly' custom firmware?

The message "Unknown bootloader detected" is coming from the firmware newly installed on the device, not from the bootloader. If someone builds a heavily customized firmware based on our sources, they are most likely going to
(a) either keep the bootloader cross-check as is, in which case the detection works exactly as advertised,
(b) or remove the cross-check entirely, in which case there is no incompatibility anymore.

If someone modifies the firmware to the point that they keep the cross-check but remove known official bootloader versions from the allowed list, then this won't work. But that is unlikely to actually happen.

@marekrjpolak marekrjpolak moved this from 🎯 To do to 🏃‍♀️ In progress in Suite · October 13 🍊 Sep 27, 2021
@alex-jerechinsky alex-jerechinsky moved this from Inbox 📥 to Bug in Suite · Backlog 🗂 Oct 4, 2021
@alex-jerechinsky alex-jerechinsky removed this from 🏃‍♀️ In progress in Suite · October 13 🍊 Oct 5, 2021
@alex-jerechinsky alex-jerechinsky added this to 🎯 To do in Suite · November 10 🥫 via automation Oct 5, 2021
@tsusanka
Copy link
Contributor

tsusanka commented Oct 7, 2021

Upon further discussion we have decided this is not worth the effort. It requires complex implementation and solves minor use case.

@tsusanka tsusanka closed this as completed Oct 7, 2021
Suite · Backlog 🗂 automation moved this from 🐛 Bug to ❌ Closed Oct 7, 2021
Suite · November 10 🥫 automation moved this from 🎯 To do to 🤝 Needs QA Oct 7, 2021
@marekrjpolak marekrjpolak removed this from 🤝 Needs QA in Suite · November 10 🥫 Oct 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working as expected
Projects
No open projects
Development

No branches or pull requests

5 participants