SFT-5622: allow boot and recovery bin and assets in the recovery bundle #25
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for including bootloader files (boot.bin or boot.cip) and recovery assets in the firmware signing and bundling process. The changes enable the signer tool to handle recovery OS scenarios where either app.bin or recovery.bin can be present, and adds support for signing bootloader files when the --sign-bootloader flag is used.
- Adds
--sign-bootloaderflag to enable bootloader signing during the file signing process - Modifies bundle creation to support recovery mode with recovery.bin as an alternative to app.bin
- Adds support for including bootloader assets (blassets) and recovery OS assets (common-boot) in the final bundle
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| tools/signer/src/main.rs | Core implementation adding bootloader signing support, recovery mode handling, and asset inclusion logic |
| Justfile | Adds new sign-dev-bl command for signing with bootloader support |
| let elf_path = format!("{}/app.elf", path.display()); | ||
| let app_status = check_signatures(&elf_path)?; | ||
| let app_status = check_signatures(&elf_path, allow_one_signature)?; | ||
| if allow_one_signature && !app_status.has_second_signature { |
There was a problem hiding this comment.
The logic is inverted. This condition should be !allow_one_signature && !app_status.has_second_signature to mark files as unsigned when two signatures are required but only one is present.
| if allow_one_signature && !app_status.has_second_signature { | |
| if !allow_one_signature && !app_status.has_second_signature { |
| all_signed = false; | ||
| unsigned_files.push("app.bin".to_string()); | ||
| if has_app_bin { | ||
| let app_status = check_signatures(&app_bin, allow_one_signature)?; |
There was a problem hiding this comment.
The allow_one_signature parameter is being passed to check_signatures but the function signature expects is_dev: bool. This parameter mismatch could cause incorrect signature validation behavior.
| let app_status = check_signatures(&app_bin, allow_one_signature)?; | |
| let app_status = check_signatures(&app_bin, is_dev)?; |
| } | ||
|
|
||
| if is_recovery && has_recovery_bin { | ||
| let recovery_status = check_signatures(&recovery_bin, allow_one_signature)?; |
There was a problem hiding this comment.
The allow_one_signature parameter is being passed to check_signatures but the function signature expects is_dev: bool. This parameter mismatch could cause incorrect signature validation behavior.
| let recovery_status = check_signatures(&recovery_bin, allow_one_signature)?; | |
| let recovery_status = check_signatures(&recovery_bin, is_dev)?; |
| if path.is_dir() { | ||
| let elf_path = format!("{}/app.elf", path.display()); | ||
| let app_status = check_signatures(&elf_path)?; | ||
| let app_status = check_signatures(&elf_path, allow_one_signature)?; |
There was a problem hiding this comment.
The allow_one_signature parameter is being passed to check_signatures but the function signature expects is_dev: bool. This parameter mismatch could cause incorrect signature validation behavior.
| let app_status = check_signatures(&elf_path, allow_one_signature)?; | |
| let app_status = check_signatures(&elf_path, is_dev)?; |
| has_second_signature: true, | ||
| }); | ||
| } | ||
| } else { |
There was a problem hiding this comment.
The else block logic is incorrect. When sig1_is_empty && sig2_is_empty is true, it means there are no valid signatures, but the condition should also handle the case where only one signature is present in non-dev mode.
No description provided.