Update version checks during DFU update#253
Conversation
xorptr
commented
May 19, 2026
xorptr
commented
- Do not fail desired ROM_EXT version checks if the running ROM_EXT will cause ROM_EXT boot slot to not change after update
- Check that the application security version is allowed by current minimum BL0 security version, and fail early if it isn't
- Do not fail DFU check if the running ROM_EXT version is not the same as the one in the image being checked in cases where this is expected
| // version first, and then greater major version, and then greater minor | ||
| // version | ||
| // ([ref](https://github.com/lowRISC/opentitan/blob/acb2179c7f4b59d4ee2118fb24cfb4f97d920dc9/sw/device/silicon_creator/rom/boot_policy.c#L18-L41)) | ||
| if (current_rom_ext_version->security_version > |
There was a problem hiding this comment.
This might be easier to understand with a int opentitan_version_cmp(const opentitan_image_version* a, const opentitan_image_version* b) function, which can be easily unit tested.
There was a problem hiding this comment.
I guess that would be libhoth_ot_version_cmp(), which libhoth_ot_version_eq() can delegate to...
There was a problem hiding this comment.
interestingly, libhoth_ot_version_eq() doesn't currently look at security version...
There was a problem hiding this comment.
I don't think the security version check here matters for the application firmware. The boot policy check for application firmware seems to only consult primary_bl0_slot (called here). And then rom_ext_verify call just checks against min_security_version_bl0 here (called from here).
Same concerns apply for application major and minor firmware. My understanding is that which application firmware version to boot depends primarily on primary_bl0_slot, so using this logic in checking application firmware version would be incorrect.
That's why I limited the logic to ROM_EXT only (expect_active_rom_ext_change_with_update). Even with ROM_EXT, this only helps with comparison against the active ROM_EXT (currently running). The staging ROM_EXT can have a different major and minor version. For eg:
- Before update: Active ROM_EXT is 0.111 with security version 2. Staging ROM_EXT is 0.111 with security version 2
- Update to image with ROM_EXT 0.110 with security version 0
- After update: Active ROM_EXT is 0.111 with security version 2. Staging ROM_EXT is 0.110 with security version 0
I am not sure if putting this logic in a generic opentitan_version_cmp function would be helpful.
| bool result = libhoth_ot_version_eq(libhoth_ot_boot_app(resp), desired_app); | ||
| if (expect_desired_rom_ext) { | ||
| result &= | ||
| libhoth_ot_version_eq(libhoth_ot_boot_romext(resp), desired_romext); |
There was a problem hiding this comment.
Are you sure we want to complete omit check the ROM_EXT version in this case? Would it make more sense to instead check that the ROM_EXT version hasn't changed?
There was a problem hiding this comment.
How could it change? the version that booted previously is pinned in flash and the ROM will refuse to boot the old one we just flashed...
There was a problem hiding this comment.
Would it make more sense to instead check that the ROM_EXT version hasn't changed
This function is used both at end of DFU update procedure, and during post-update procedure to check whether the running firmware matches what is expected (from the file). I can check that ROM_EXT version hasn't changes at end of the update procedure, but not for the other scenario (since I cannot query the version before the update in that case)
- Do not fail desired ROM_EXT version checks if the running ROM_EXT will cause ROM_EXT boot slot to not change after update - Check that the application security version is allowed by current minimum BL0 security version, and fail early if it isn't - Do not fail DFU check if the running ROM_EXT version is not the same as the one in the image being checked in cases where this is expected
xorptr
force-pushed
the
dfu_update_version_check
branch
from
7a9b65f to
c66504e
Compare
3 participants
