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

west sign: imgtool: zephyr.signed.hex and zephyr.signed.bin do not have the same contents #52271

Closed
JordanYates opened this issue Nov 16, 2022 · 18 comments
Assignees
Labels
area: MCUBoot area: West West utility bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug Stale

Comments

@JordanYates
Copy link
Collaborator

Describe the bug

When creating signed images, the generated files zephyr.signed.hex and zephyr.signed.bin do not have the same logical contents.

west sign works by running two independent calls to imgtool, once to create the .hex, and again to create the .bin.
Unfortunately even running imgtool with the exact same arguments twice does not result in the same binary.

Both files are valid, they are just different due to the different generated values of the IMAGE_TLV_RSA2048_PSS.

To Reproduce

yat033@GRAY-PH zephyr % imgtool sign --version 0.0.0+0 --align 4 --header-size 512 --slot-size 0x60000 --key ~/code/zephyr/bootloader/mcuboot/root-rsa-2048.pem app.hex signed.hex && md5 signed.hex
MD5 (signed.hex) = 6b2ce8658c234cc0008009490d0c012b
yat033@GRAY-PH zephyr % imgtool sign --version 0.0.0+0 --align 4 --header-size 512 --slot-size 0x60000 --key ~/code/zephyr/bootloader/mcuboot/root-rsa-2048.pem app.hex signed.hex && md5 signed.hex
MD5 (signed.hex) = 4ad4834a9de8123134a7cc8a5c4e3e39

Expected behavior

I would expect zephyr.signed.bin and zephyr.signed.hex to have the same contents, differing only in the file format.

Impact

The actual binary on the device depends on which file format was flashed.

Additional context

Given this behaviour is in imgtool, the only solution I see is running imgtool once and then converting the .hex to a .bin

@JordanYates JordanYates added bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug area: MCUBoot labels Nov 16, 2022
@Laczen
Copy link
Collaborator

Laczen commented Nov 16, 2022

@JordanYates, when during the signing a some kind of random is used to generate a signature or to generate an encryption key each call to imgtool will create a different hex/bin. As both are generated by a call to imgtool this behavior is to be expected. Both images however should be ok.

As this behavior is intentionally this is not a bug.

If you really need to have the same images (I don't know why) you should indeed create convert the generated hex to a bin.

@JordanYates
Copy link
Collaborator Author

I agree that the behaviour is expected from the perspective of how the tool is being used, I disagree that it is expected from the perspective of the end user. I spent an hour or so trying to figure out why the two weren't the same before I realised the difference was only in the signed TLV and that the behaviour came from imgtool itself.

Everything I know about build systems would tell me that if I see two files with the same name, one .hex and one .bin, they should have the exact same logical contents if I flashed them to a device. In this case they don't, hence the issue.

@Laczen
Copy link
Collaborator

Laczen commented Nov 17, 2022

The difference is much larger for encrypted images. There has been a similar issue regarding reproducible builds. As soon as the image build process includes some kind of randomness inside (e.g. for signing), repeated builds will generate different artifacts.
I think it is better to leave it as it is, the generation of the bin is for systems that use the bin, the hex is for systems that use the hex. Some build systems might only generate a bin file and it would be needed to provide extra parameters to generate the hex file.

@stephanosio
Copy link
Member

@JordanYates should this issue be moved to mcuboot?

@carlescufi
Copy link
Member

@JordanYates I agree with @stephanosio that this is likely an MCUboot issue? And one more thing, is this not related to the fact that signature is non-deterministic? For example ECDSA will use a random number every time you sign:

https://stackoverflow.com/a/68959662/337201

@Laczen
Copy link
Collaborator

Laczen commented Nov 22, 2022

@stephanosio, @carlescufi, no this is not a mcuboot issue. It is due to how a hex and bin are generated by west. By calling imgtool twice the signature being non-deterministic is different for the hex and the bin. If west would create the bin from the hex this would not be the case.
I would not call this a bug, it is a property of the system. Whether or nor a build system should generate "identical" files in hex or bin format I have no opinion on.

@laurenmurphyx64 laurenmurphyx64 added west area: West West utility and removed west labels Nov 29, 2022
@nordicjm nordicjm removed their assignment Nov 30, 2022
@nordicjm
Copy link
Collaborator

I don't see any fix for this, a build can output a hex file, and a build can output a bin file - those 2 files are optional via Kconfig, unless you force generating both then you have to assume that one might not be there and so both are going to get signed individually, thus have different signature contents.

@marc-hb
Copy link
Collaborator

marc-hb commented Jan 7, 2023

(just spotted this issue by sheer chance while searching something else)

I spent an hour or so trying to figure out why the two weren't the same before I realised the difference was only in the signed TLV and that the behaviour came from imgtool itself.

I experienced basically the same issue with rimage's salt:

As long as salt is required for security reasons there is no "solution" to this problem; only mitigations and education: everyone must simply get used to signatures being not deterministic. C'est la vie. CMake comments and logging can help.

If you're interested in reproducibility then focus on stripped files BEFORE signing, it's been working great for us. As a coincidence I just finished fixing SOF build reproducibility issues, now our Windows CI and Linux CI are about to produce the same binaries BEFORE signing. See early demo at: thesofproject/sof#6920 (comment)

See also #51954 (which I have not forgotten)

@JordanYates
Copy link
Collaborator Author

This issue is slightly different from what you are describing I think.
It makes complete sense that rebuilding and signing an application for a second time will result in a different file due to the salt changing.
Here I am suggesting that a .hex and .bin from the SAME build should not be different, given that one can be generated from the other.

@marc-hb
Copy link
Collaborator

marc-hb commented Jan 9, 2023

This issue is slightly different from what you are describing I think.

Yes rimage and imgtool are very different and meant for very different systems. However some high-level discussions look very similar and they share the same west sign frontend so I think it was worth sharing a link.

given that one can be generated from the other.

Can it really? Looking at Kconfig and CMake (I recommend git grep -i outtarget), CONFIG_OUTPUT_BIN and CONFIG_OUTPUT_HEX are independent from each other, they seem meant for different "consumers". Pardon my ignorance but I'm struggling to understand how signing one output format could be interchangeable with signing the same data in a totally different file format.

@JordanYates
Copy link
Collaborator Author

Can it really? Looking at Kconfig and CMake (I recommend git grep -i outtarget), CONFIG_OUTPUT_BIN and CONFIG_OUTPUT_HEX are independent from each other, they seem meant for different "consumers". Pardon my ignorance but I'm struggling to understand how signing one output format could be interchangeable with signing the same data in a totally different file format.

Sure they can, objcopy --input-target=ihex --output-target=binary input.signed.hex output.signed.bin

@marc-hb
Copy link
Collaborator

marc-hb commented Jan 9, 2023

Sure they can, objcopy --input-target=ihex --output-target=binary input.signed.hex output.signed.bin

Interesting, I didn't expect objcopy to "support" signatures.

I'm struggling to understand how signing one output format could be interchangeable with signing the same data in a totally different file format.

I just had a look at https://github.com/mcu-tools/mcuboot/blob/main/scripts/imgtool/image.py and that helped. The ihex is never signed but always converted to binary on the fly (without depending on objcopy).

I think it is better to leave it as it is, the generation of the bin is for systems that use the bin, the hex is for systems that use the hex.

So the reason while imgtool is called twice is because OUTPUT_BIN and OUTPUT_HEX are mutually exclusive in imgtool - whereas they can be selected independently of each other in KConfig.

Make imgtool able to output 1 or 2 formats at the same time (with the same signature) and I think you have the first step to address this issue.

My 2 cents.

@nordicjm
Copy link
Collaborator

nordicjm commented Jan 9, 2023

Interesting, I didn't expect objcopy to "support" signatures.

The signatures here are nothing special, they are just normal hex/binary data at a specific location, to any hex viewer it is just "program code".

@Laczen
Copy link
Collaborator

Laczen commented Jan 9, 2023

Maybe there is a different reason to modify the generation as proposed by @JordanYates: it is never a good idea to generate a different signature for the same binary data as this might leak the private key used for signing. To mitigate this either create the bin from the hex as proposed (if this is supported by all toolchains) or add an extra random value to the image that ensures the binary data is different (in imgtool).

@marc-hb
Copy link
Collaborator

marc-hb commented Jan 9, 2023

it is never a good idea to generate a different signature for the same binary data

But the idea of cryptographic salt is exactly that?! Creating a different signature for the same binary data for various security reasons. I have no idea "how" but this is "why": https://crypto.stackexchange.com/questions/25182/why-use-randomness-in-digital-signature-algorithms

either create the bin from the hex as proposed

As mentioned above, OUTPUT_BIN and OUTPUT_HEX can be selected independently of each other in KConfig. Are you suggesting a new dependency from one to the other? (In that case creating the hex from the bin would make more sense to me)

(if this is supported by all toolchains)

Unlike GPL projects, the license of the Zephyr project allows re-use in closed source software. So it's very difficult to tell what "all" toolchains support. Here's a quote of the second sentence in the issue that discussed the toolchain abstraction layer that implements these BIN and HEX formats (among many others)

Furthermore, the name of some of these proprietary commercial toolchains cannot be public due to industry competition.

or add an extra random value to the image that ensures the binary data is different (in imgtool).

This is happening already and why this issue was created? You lost me sorry.

@github-actions
Copy link

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

@github-actions
Copy link

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

@github-actions github-actions bot added the Stale label May 26, 2023
@d3zd3z
Copy link
Collaborator

d3zd3z commented May 26, 2023

I'll leave a few comments before closing the issue. Feel free to open if needed. To summarize, several signature and encryption algorithms used by imgtool/mcuboot are non-deterministic. There are situations where west is used to build both a .bin and a .hex file, and because each is a separate invocation of imgtool, the output ends up different.

My question is, what are the situations where there is a need to generate both a .hex and a .bin file? Shouldn't one be the preferred output type for a given build configuration. If this is really important, it might be worthwhile changing west to generate just a hex file, and build a .bin from that, but I'm not sure how well that would fit with the given architecture.

As an aside, if you really want deterministic signatures, Ed25519 should be deterministic. RSA-PSS uses a random value to increase the security properties of the signature. A random value is fundamental to how ECDSA works. Another option would be to add support for rfc6979 to imgtool. The resulting signatures are compatible with existing ECDSA verification, but the signatures are deterministic. Note that there are security implications of deterministic ECDSA, even though there are also benefits.

@d3zd3z d3zd3z closed this as completed May 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: MCUBoot area: West West utility bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug Stale
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants