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: Allow configuring west sign similar to west runners #22562

Closed
henrikbrixandersen opened this issue Feb 6, 2020 · 13 comments · Fixed by #27818
Closed

West: Allow configuring west sign similar to west runners #22562

henrikbrixandersen opened this issue Feb 6, 2020 · 13 comments · Fixed by #27818
Assignees
Labels
area: West West utility Enhancement Changes/Updates/Additions to existing features

Comments

@henrikbrixandersen
Copy link
Member

henrikbrixandersen commented Feb 6, 2020

Is your enhancement proposal related to a problem? Please describe.
It is very easy to configure west runner parameters through CMake (either in board.cmake or app CMakeLists.txt). It is not easy to configure parameters for west sign through CMake.

Describe the solution you'd like
It would be nice if there was a similar (to configuring west runners through CMake) for configuring west sign parameters and having CMake generate a sign build target.

Ideally, the west runners could be configured to pick up the signed build artifacts instead of the unsigned one if signing is enabled for a given app/board (keeping the dependency chain correct, so that attempting to flash a signed bin/hex will not only build the unsigned bin/hex but also sign it before running west flash).

Describe alternatives you've considered
I have written a small, local CMakeLists.txt file for providing a sign target, but it is far from flexible enough to be upstreamed - and it lacks a way of passing the signed bin/hex path to the west runner.

@henrikbrixandersen henrikbrixandersen added Enhancement Changes/Updates/Additions to existing features area: West West utility labels Feb 6, 2020
@mbolivar-nordic
Copy link
Contributor

I've switched my work-related account to @mbolivar-nordic (#23530).

@JordanYates
Copy link
Collaborator

+1 for the problem, it feels like signed images are second class citizens in terms of tooling.

The way I see for solving this is for integrating the signing process into CMake.
This would solve the dependency chain, as west flash already ensures the artifacts are up to date.
The locations of the signed output images would also be known by the build system, allowing west build to find them without manually specification.

@mbolivar-nordic
Copy link
Contributor

mbolivar-nordic commented Jul 6, 2020

The way I see for solving this is for integrating the signing process into CMake.

This is done downstream in the Zephyr-based Nordic SDK, but the approach to do it was rejected for upstream for various reasons, mainly having to do with lack of generality in my opinion. I would be interested in seeing this improve in upstream Zephyr, though.

Here are some random design thoughts; I'd be interested in feedback on how useful they sound.

It would be easy enough to add some logic to call imgtool when CONFIG_BOOTLOADER_MCUBOOT=y, especially now that mcuboot is a west.yml project and we know the path to it (this was not the case when west sign was originally written).

The remaining requirement is the path to the key file.

Perhaps we could set that up with another Kconfig option, CONFIG_MCUBOOT_SIGNING_KEY_FILE or so, that depends on BOOTLOADER_MCUBOOT? We could potentially have this default to root-rsa-2048.pem in the mcuboot project, which we can now locate by adding a new helper to kconfigfunctions.py to retrieve the path to a west project.

From there, we can have the build system sign the image by default as a post-processing step after creating the final binary.

That potentially eliminates the need for west sign entirely, and opens up the path to making the build system create a runners.yaml that defaults to flashing the signed file.

@mbolivar-nordic
Copy link
Contributor

mbolivar-nordic commented Jul 6, 2020

Here are some random design thoughts; I'd be interested in feedback on how useful they sound.

CC @hakonfam @oyvindronningstad @tejlmand @nvlsianpu

@henrikbrixandersen
Copy link
Member Author

@mbolivar-nordic You have basically just described how I handle this in our downstream repo (except for having a default for the key file, which I do not think we should have - people might be more inclined to just use the default key if provided). I am all for going for this kind of solution.

@JordanYates
Copy link
Collaborator

It would be easy enough to add some logic to call imgtool when CONFIG_BOOTLOADER_MCUBOOT=y

The tool to sign images with should probably be selected in a similar method to the default west runner.

Perhaps we could set that up with another Kconfig option, CONFIG_MCUBOOT_SIGNING_KEY_FILE

The issue here is that the entire project must be rebuilt in order to sign an image with a different key.
This is probably not acceptable as I imagine a common workflow would be to release an application, then sign it with different keys depending on where it is being deployed.

Maybe a slightly more split setup, where board/soc CMake files specify signing tools and parameters.
Then when CONFIG_BOOTLOADER_MCUBOOT is enabled, west flash expects a --key parameter which is used to sign and then flash.

@mbolivar-nordic
Copy link
Contributor

The issue here is that the entire project must be rebuilt in order to sign an image with a different key.

That's true.

This is probably not acceptable as I imagine a common workflow would be to release an application, then sign it with different keys depending on where it is being deployed.

However, I don't struggle with acceptability as much personally, because the zephyr west extensions are mainly for development, not going to production.

By the time one has dev and release keys and is deploying a release build to production, I would guess that the release flow isn't "take this zephyr build directory and use west to build the signed binaries and flash them", so I'm not sure west commands for signing and flashing signed binaries are the right place to address this use case.

I could imagine making the Kconfig option a CMake ;-list so multiple values could be passed, though, and having some sort of naming convention if there are multiple binaries, but then you need some way to tell west flash which one to use, which kind of takes me back to west sign plus west flash --file as a solution for this use case.

Maybe a slightly more split setup, where board/soc CMake files specify signing tools and parameters.

Does that mean doing this in CMakeLists.txt files, rather than Kconfig? That still seems to require a rebuild to change the keys.

It also seems like this is a build mechanism that should be provided by the build system "core", then configured by the application or in the build command, not set in board/soc files, no?

Then when CONFIG_BOOTLOADER_MCUBOOT is enabled, west flash expects a --key parameter which is used to sign > and then flash.

Coupling signing to flashing in the west flash command still feels like a layering issue to me.

West flash has no way of knowing if a signed file is up to date unless it implements build-system-like tracking based on timestamps or what have you. The naive implementation creates a new signed binary every single time, even when that is not necessary. The build system knows the dependency graph and how to do updates.

That is why I favored the --file solution instead for use cases like this, so I'm still a bit unclear on why it's a solution for a different problem, sorry. If you can help me be less dense I'd appreciate it. Thanks.

@mbolivar-nordic
Copy link
Contributor

I do get that --file means you have to know if your runner takes a .hex or a .bin, and that is a bit of friction. Perhaps we could allow --file to be given multiple times to specify both, at the runner's discretion, with an error if none of the file types is usable?

@JordanYates
Copy link
Collaborator

However, I don't struggle with acceptability as much personally, because the zephyr west extensions are mainly for development, not going to production.

That's fair, my 'production' is typically no more than 100 devices for a given application, so I normally just use the same tools as development.

Maybe a slightly more split setup, where board/soc CMake files specify signing tools and parameters.

Does that mean doing this in CMakeLists.txt files, rather than Kconfig?

Sorry, I was not very clear. The idea was for the CMake files to specify the parameters which have sensible defaults for all applications on that board. i.e. my custom boards would specify they should be signed using imgtool with an erased-val of 0xFF, etc. Upsquared would specify rimage and whatever other parameters it takes. Basically everything except for the actual keyfile.

Ignoring the layering issues for a moment, when you call west flash the only information missing would be the key that it should be signed with, which must be provided. I'm not sure whether this is actually a good idea or not.

That is why I favored the --file solution instead for use cases like this, so I'm still a bit unclear on why it's a solution for a different problem, sorry. If you can help me be less dense I'd appreciate it. Thanks.

Its a solution for a different problem because if I am not manually changing output file names (outside of the build system), the tooling should be 100% aware of the absolute path to a signed output binary. If the tooling knows where it is, I should not have to specify it.

If the tooling is currently not aware of the path to signed binaries when no custom names are specified, then that should be resolved. In short, every backend supported by west sign should generate the outputs into the same location, the same way that west build always generates ${build_dir}/zephyr/zephyr.hex regardless of whether its built by Ninja or Makefiles.

@mbolivar-nordic
Copy link
Contributor

If the tooling knows where it is, I should not have to specify it.

Amen to that, we are on the exact same page here, the question is just how to set it up.

The idea was for the CMake files to specify the parameters which have sensible defaults for all applications on that board. i.e. my custom boards would specify they should be signed using imgtool with an erased-val of 0xFF, etc. Upsquared would specify rimage and whatever other parameters it takes.

I don't think it is possible in general to specify sensible defaults for bootloaders used by "all applications on that board", except maybe in the case of single-application custom boards, but that's not the only use case that matters here.

This is notably not possible for generic development boards, which may support a variety of bootloaders, might (or might not) have flash devices used to store upgrades on a separate shield depending on application- or even user-specified configuration, etc.

That is why I believe it is better to make this the application's and core build system's responsibility to configure, not the board's. The build system provides mechanism; applications and their users set policy. Boards can certainly make use of the mechanism if they are designed for a single application or small set of them, but that should follow from the way boards already have hooks into the build system that happen before the application's.

Zephyr exposes mechanisms like this via devicetree, CMake variables, and Kconfig. DT is out in my opinion as this is a software thing, not a hardware thing. That leaves "raw" CMake variables and Kconfig, but I believe Kconfig is a better choice here since we are already configuring MCUboot related mechanism with that.

In short, every backend supported by west sign should generate the outputs into the same location, the same way that west build always generates ${build_dir}/zephyr/zephyr.hex regardless of whether its built by Ninja or Makefiles.

This too I think is not possible in general. Not every application configuration generates a .hex; you can even turn this off with CONFIG_BUILD_OUTPUT_HEX. It just so happens that for the mcuboot bootloader, enabling and using it is a logical thing to be doing.

I can similarly imagine that the secure boot environments Zephyr ultimately needs to support might have myriad different types of artifacts. Consider things like IETF SUIT or TUF / Uptane that manage a hierarchy of artifacts and have fancy things like multiple signatures/delegation.

Questions of layering aside, would you be willing to agree to using Kconfig to set the signed artifact locations up in a way that respects dependencies, e.g. on CONFIG_BOOTLOADER_MCUBOOT, in order to set up west flash? If we have agreement there, I am sure we can figure out the rest of the details in a way that works for all.

@mbolivar-nordic
Copy link
Contributor

Questions of layering aside, would you be willing to agree to using Kconfig to set the signed artifact locations up in a way that respects dependencies, e.g. on CONFIG_BOOTLOADER_MCUBOOT, in order to set up west flash? If we have agreement there, I am sure we can figure out the rest of the details in a way that works for all.

@JordanYates ping?

@JordanYates
Copy link
Collaborator

That sounds like a good first step

@mbolivar-nordic
Copy link
Contributor

Great, thanks. I'm off next week but I'll pick this up when I get back.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: West West utility Enhancement Changes/Updates/Additions to existing features
Projects
None yet
4 participants