flash signed binaries: key path and version

[Delsian]

Related to PR #27818

@mbolivar-nordic Thanks for great functionality, only minor improvement looks necessary to use in projects:

• CONFIG_MCUBOOT_SIGNATURE_KEY_FILE uses relative path to zephyrproject folder. Better to use relative path to user's project to store keys in separate place. Or describe additional shell variable with path to keys folder.

• imgtool.exe sign --version should take version number from Git tag (probably using new setting CONFIG_MCUBOOT_GIT_TAG_VERSION).

Thank you.

[Delsian]

Found another issue:
build/zephyr/runners.yaml should contain updated hex-file and bin-file fields

[mbolivar-nordic]

Hi there, thanks for your feedback!

I'm a bit unclear from your description, but would you like this to be addressed before #27818 is merged?

I'm particularly concerned by these two comments:

only minor improvement looks necessary to use in projects [...] Found another issue: [...]

This makes it sound like the new feature is not useful enough yet.

[nandojve]

Hi @mbolivar-nordic , I think many use cases passes "user" version to MCUboot. If the improvements that you are doing could address that will help in many cases. For instance, any user that rely uses FW update may need set all time the version number manually. Yesterday you answered my question and I'm Ok with that, to change file name, for instance. But I think people realized that --version for MCUboot is rely a important thing. In my case, because of that I asked about how add version to file names. This way people can "see" and know what check.

For instance, my use case uses below mandatory args when using MCUboot

         --key
         --align 8
         --version x.x.x
         --header-size 0x200
         --slot-size {from DT}
         --pad

My intention try to clarify things for you.
Hi @utzig , do you have anything that could help to clarify @mbolivar-nordic possible doubts?

[mbolivar-nordic]

Hi @nandojve -- sure, I get that the version is important. But I am sorry for not understanding whether there is some change needed in pull request #27818 to make it useful, or if the existing code is good enough.

Perhaps if the code is good enough, we need some changes to the documentation to make it clearer how to manage the version number?

Thanks again for helping me to understand.

[mbolivar-nordic]

• CONFIG_MCUBOOT_SIGNATURE_KEY_FILE uses relative path to zephyrproject folder. Better to use relative path to user's project to store keys in separate place. Or describe additional shell variable with path to keys folder.

We could potentially support this without changing the existing PR by adding a choice option as later work controlling how relative paths work. But it might be easier just to put the application inside the workspace, no? That is generally best for making 'west build' work without environment variables, anyway. So I will consider this an enhancement that is out of scope of the existing PR.

• imgtool.exe sign --version should take version number from Git tag (probably using new setting CONFIG_MCUBOOT_GIT_TAG_VERSION).

This is tricky. There are lots of places where we might want to put software versions and lots of opinions about where they come from. I will also consider this out of scope of the initial PR unless people come to consensus on what to do. As you say, we can add a new Kconfig option that covers this at that point.

[mbolivar-nordic]

Found another issue:
build/zephyr/runners.yaml should contain updated hex-file and bin-file fields

Not sure what you mean by this.

[Delsian]

About version. Currently I'm using Python script to take FW version from Git tag, probably it should be useful for others:

# git describe
_git_describe = call_process_out( ['git', 'describe', '--tags', '--long', '--dirty' ] ).decode( 'utf-8' )
print( _git_describe )

# extract version : tags/3.1-25-g4c41456
#_git_describe = 'tags/3.1-35'
#_git_describe = 'tags/3.1-25-g4c41456'
_git_describe_match = re.match( '([0-9]+)\.([0-9]+)\-([0-9]+)([\-\w]+)', _git_describe )
if _git_describe_match != None:
	_ver_patch = _git_describe_match.group(4)
else:
	_git_describe_match = re.match( '([0-9]+)\.([0-9]+)\-([0-9]+)', _git_describe )
	_ver_patch = ''
_ver_major = _git_describe_match.group(1)
_ver_minor = _git_describe_match.group(2)
_ver_build = _git_describe_match.group(3)
_version_num = (int(_ver_major) << 24) + (int(_ver_minor) << 16) + int(_ver_build)
_version_str = _ver_major + '.' + _ver_minor + '.' + _ver_build + _ver_patch
print( 'Version : %s / %08X' % (_version_str, _version_num))

[Delsian]

build/zephyr/runners.yaml contains filenames for flashing.
After signing there is still unsigned filenames:

# Default command line arguments. The ones in "common" are always given.
# The other sub-keys give runner-specific arguments.
args:
  common:
  - --elf-file=...../build/zephyr/zephyr.elf
  - --hex-file=..../build/zephyr/zephyr.hex
  - --bin-file=..../build/zephyr/zephyr.bin

Probably we need another PR with updating this file?

[Delsian]

Just a question - in the debug process I'm using zephyr.elf file.
Do you know any way how to add signing to *.elf? Unfortunately bootloader reports "Incorrect signature" when I start debugging.

[mbolivar-nordic]

Do you know any way how to add signing to *.elf? Unfortunately bootloader reports "Incorrect signature" when I start debugging.

No, imgtool only supports bin and hex.

You can just use 'west flash' followed by 'west attach' to debug the flashed binary without reflashing.

[mbolivar-nordic]

After signing there is still unsigned filenames:

That shouldn't be the case if you follow the workflow in the updated west sign documentation from #27818. If that's what's happening, it's a bug and we'll fix it.

If you are manually calling 'west sign', then that doesn't (and is not meant to) change runners.yaml.