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

Firmware decoding improvements #2576

Merged
merged 10 commits into from Nov 1, 2022
Merged

Conversation

matejcik
Copy link
Contributor

In order to be able to build better tooling for firmware signing, I needed to improve the firmware decoding (and rebuilding). This is done by splitting trezorlib.firmware into multiple independent modules and writing out clearly defined classes for each kind of firmware object.

Remote signing functionality is removed from headertool, as it will live elsewhere and be done differently.

in addition, trezorctl will now warn about using CoSi-insecure firmwares older than 1.11.2

@trezor-ci trezor-ci added this to To be reviewed in Pull Requests via automation Oct 20, 2022
@matejcik matejcik force-pushed the matejcik/remove-signingtools branch 4 times, most recently from 18cc0df to 8071571 Compare October 21, 2022 12:10
@matejcik matejcik marked this pull request as ready for review October 21, 2022 12:10
@matejcik matejcik requested review from grdddj and removed request for prusnak October 21, 2022 12:10
Copy link
Contributor

@grdddj grdddj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine, I appreciate the tests there.

Couple of things I noticed in the comments, some small changes could be made.

python/src/trezorlib/_internal/firmware_headers.py Outdated Show resolved Hide resolved
python/src/trezorlib/_internal/firmware_headers.py Outdated Show resolved Hide resolved
python/src/trezorlib/firmware/core.py Show resolved Hide resolved
python/tests/test_firmware.py Show resolved Hide resolved
core/tools/headertool.py Outdated Show resolved Hide resolved
Copy link
Contributor

@grdddj grdddj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

Pull Requests automation moved this from To be reviewed to Finalizing Nov 1, 2022
@matejcik matejcik merged commit 7c1209e into master Nov 1, 2022
Pull Requests automation moved this from Finalizing to Merged Nov 1, 2022
@matejcik matejcik deleted the matejcik/remove-signingtools branch November 1, 2022 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants