RFC: Remove west sign support for imgtool

nordicjm commented 1 month ago

Introduction

West has an extension command that allows for signing images using west sign, it has 2 supported tools: rimage and imgtool. This code likely far predates the cmake signing code inside the build system.

Problem description

The problem with this is that we have 2 completely different, incompatible, signing methods, the build system will do signing using a cmake file (which can be replaced out of tree by users if they wish to change it or use their own way of signing) and the west sign command does its complete own thing which has no relation to the build system method of signing as can be seen here: https://github.com/zephyrproject-rtos/zephyr/blob/main/scripts/west_commands/sign.py#L251

This is problematic because the west sign method is wrong as the build system one has been enhanced and it cannot be expanded or changed out of tree.

Proposed change

I therefore propose removing support for west sign using imgtool - note this is for imgtool only, rimage would not be impacted by this change. Since the build system already deals with signing, users should not have to use this feature, the imgtool parameters can be set via Kconfig. This does mean that if the imgtool parameters are changed, cmake needs to reconfigure and then rebuild the project but it means there is a uniform imgtool signing system, and a single codebase (cmake-based) for signing using imgtool instead of 2 completely independent code paths

Concerns and Unresolved Questions

Users would have to change their workflow if they are reliant upon this, including changes in CI if they use it there

Alternatives

None, there should not be duplicated code in 2 different languages which need to be synchronised and maintained

pdgendt commented 1 month ago

Can we redirect west sign to use the CMake version? Or would that be incompatible/too confusing?

nordicjm commented 1 month ago

Can we redirect west sign to use the CMake version? Or would that be incompatible/too confusing?

It would just do the same as a west build, it could be made to do like that, though it would be a silent change of operation because the command would still work so things like CI (if it's being used in CI) where they supply additional tool arguments would still work but would not be applied

carlescufi commented 1 month ago

Architecture WG:

@nordicjm states that the problem is that the code is duplicated between west and the Zephyr CMake code. Also, the west sign implementation is not overridable in downstreams
@tejlmand argues that the west sign operation being separate is sometimes useful, which is something we'd lose by removing it. Also, west sign accepts additional arguments that cannot be passed
Today, using west sign does not yield the same result as letting the build system sign the image. For example, encrypted image

zephyrproject-rtos / zephyr