-
Notifications
You must be signed in to change notification settings - Fork 7.6k
scripts: west: sign: Add support for image signing for Silabs SiWx91x #87669
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,7 +13,6 @@ | |
import sys | ||
|
||
from elftools.elf.elffile import ELFFile | ||
|
||
from west import manifest | ||
from west.commands import Verbosity | ||
from west.util import quote_sh_list | ||
|
@@ -79,6 +78,20 @@ | |
[rimage] sections in your west config file(s); this is especially useful | ||
when invoking west sign _indirectly_ through CMake/ninja. See how at | ||
https://docs.zephyrproject.org/latest/develop/west/sign.html | ||
|
||
silabs_commander | ||
----- | ||
|
||
To create a signed binary with the silabs_commander tool, run this from your build directory: | ||
|
||
west sign -t silabs_commander | ||
|
||
For this to work, Silabs Commander tool must be installed in your PATH. | ||
|
||
The input binary (zephyr.bin.rps) is signed using the specified OTA key and private key, | ||
producing zephyr_signed.bin.rps by default. If CONFIG_SIWX91X_OTA_ENCRYPT_KEY and CONFIG_SIWX91X_OTA_SIGN_KEY are set | ||
in your build configuration, they will be used unless overridden by command-line arguments. | ||
Additional arguments after '--' are passed to silabs_commander directly. | ||
''' | ||
|
||
class ToggleAction(argparse.Action): | ||
|
@@ -112,8 +125,8 @@ def do_add_parser(self, parser_adder): | |
|
||
# general options | ||
group = parser.add_argument_group('tool control options') | ||
group.add_argument('-t', '--tool', choices=['imgtool', 'rimage'], | ||
help='''image signing tool name; imgtool and rimage | ||
group.add_argument('-t', '--tool', choices=['imgtool', 'rimage', 'silabs_commander'], | ||
help='''image signing tool name; imgtool , rimage and silabs_commander | ||
are currently supported (imgtool is deprecated)''') | ||
group.add_argument('-p', '--tool-path', default=None, | ||
help='''path to the tool itself, if needed''') | ||
|
@@ -195,6 +208,8 @@ def do_run(self, args, ignored): | |
signer = ImgtoolSigner() | ||
elif args.tool == 'rimage': | ||
signer = RimageSigner() | ||
elif args.tool == 'silabs_commander': | ||
signer = SilabsRPSSigner() | ||
# (Add support for other signers here in elif blocks) | ||
else: | ||
if args.tool is None: | ||
|
@@ -633,3 +648,83 @@ def sign(self, command, build_dir, build_conf, formats): | |
|
||
os.remove(out_bin) | ||
os.rename(out_tmp, out_bin) | ||
|
||
class SilabsRPSSigner(Signer): | ||
# Signer class for Silicon Labs Commander tool via west sign -t silabs_commander. | ||
def find_commandertool(self, cmd, args): | ||
if args.tool_path: | ||
commandertool = args.tool_path | ||
if not os.path.isfile(commandertool): | ||
cmd.die(f'--tool-path {commandertool}: no such file') | ||
else: | ||
commandertool = shutil.which('commander') | ||
if not commandertool: | ||
cmd.die(f'SILABS "commander" not found in PATH, try "--tool-path"? Cannot sign {args}') | ||
return commandertool | ||
|
||
def get_security_configs(self, build_conf, args, command): | ||
|
||
# Retrieve configurations, prioritizing command-line args, then build_conf | ||
siwx91x_ota_encrypt_key = getattr(args, 'siwx91x-ota-encrypt-key', build_conf.get('CONFIG_SIWX91X_OTA_ENCRYPT_KEY')) | ||
siwx91x_ota_sign_key = getattr(args, 'siwx91x-ota-sign-key', build_conf.get('CONFIG_SIWX91X_OTA_SIGN_KEY')) | ||
siwx91x_ota_mic_enable = getattr(args, 'siwx91x-mic-enable', build_conf.get('CONFIG_SIWX91X_SILABS_OTA_MIC_ENABLE', False)) | ||
siwx91x_ota_encrypt_enable = getattr(args, 'siwx91x-encrypt-enable', build_conf.get('CONFIG_SIWX91X_SILABS_OTA_ENCRYPT_ENABLE', False)) | ||
siwx91x_ota_sign_enable = getattr(args, 'siwx91x-sign-enable', build_conf.get('CONFIG_SIWX91X_SILABS_OTA_SIGN_ENABLE', False)) | ||
|
||
# Refer find_commandertool() function to identify the Commander tool. | ||
commander_path = self.find_commandertool(command, args) | ||
|
||
# Validate required settings | ||
if siwx91x_ota_encrypt_enable and not siwx91x_ota_encrypt_key: | ||
command.die("SIWX91X_OTA_ENCRYPT_KEY not provided. Use --siwx91x-ota-encrypt-key=your_key or add CONFIG_SIWX91X_OTA_ENCRYPT_KEY in prj.conf.") | ||
if siwx91x_ota_sign_enable and not siwx91x_ota_sign_key: | ||
command.die("SIWX91X_OTA_SIGN_KEY not provided. Use --siwx91x-ota-sign-key=/path/to/key or add CONFIG_SIWX91X_OTA_SIGN_KEY in prj.conf.") | ||
|
||
# Validate private key path | ||
if not os.path.isfile(siwx91x_ota_sign_key): | ||
command.die(f"Private key not found at {siwx91x_ota_sign_key}. Please ensure the path is correct.") | ||
|
||
return commander_path,siwx91x_ota_encrypt_key, siwx91x_ota_sign_key, siwx91x_ota_mic_enable, siwx91x_ota_encrypt_enable, siwx91x_ota_sign_enable | ||
|
||
def sign(self, command, build_dir, build_conf, formats): | ||
"""Sign the Zephyr binary using Silicon Labs Commander. | ||
commander_path | ||
:param command: The Sign instance (provides args and utility methods) | ||
:param build_dir: The build directory path | ||
:param build_conf: BuildConfiguration object for the build directory | ||
:param formats: List of formats to generate (e.g., ['bin', 'rps']) | ||
""" | ||
self.command = command | ||
args = command.args | ||
b = pathlib.Path(build_dir) | ||
kernel_name = build_conf.get('CONFIG_KERNEL_BIN_NAME') | ||
# Setting the input and output paths | ||
input_rps = b / 'zephyr' / f'{kernel_name}.bin.rps' | ||
output_rps = args.sbin or (b / 'zephyr' / f'{kernel_name}_secure.bin.rps') | ||
# Check if input binary exists | ||
if not input_rps.is_file(): | ||
command.die(f"No .rps found at {input_rps}. Ensure the build generated kernel_name.bin.rps in {b / 'zephyr'}") | ||
# Load configuration | ||
commander_path,siwx91x_ota_encrypt_key, siwx91x_ota_sign_key, siwx91x_ota_mic_enable, siwx91x_ota_encrypt_enable, siwx91x_ota_sign_enable = self.get_security_configs(build_conf, args, command) | ||
siwx91x_ota_mic_key = siwx91x_ota_encrypt_key | ||
sign_base = [ | ||
commander_path, | ||
"rps", | ||
"convert", | ||
str(output_rps), | ||
"--app", str(input_rps) | ||
] | ||
marc-hb marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
if siwx91x_ota_mic_enable: | ||
sign_base.extend(["--mic", str(siwx91x_ota_mic_key)]) | ||
|
||
if siwx91x_ota_encrypt_enable: | ||
sign_base.extend(["--encrypt", str(siwx91x_ota_encrypt_key)]) | ||
|
||
if siwx91x_ota_sign_enable: | ||
sign_base.extend(["--sign", str(siwx91x_ota_sign_key)]) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is "OTA" everywhere in the key names but no "OTA" in the name of the booleans? This inconsistency gives the impression that there is some subtle difference between the keys and the booleans. But this code looks like there is none. Confusing. |
||
|
||
sign_base.extend(args.tool_args) | ||
|
||
command.inf("Running command:", ' '.join(sign_base)) | ||
subprocess.run(sign_base, check=True) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,3 +28,39 @@ config SIWX91X_NWP_INIT_PRIORITY | |
Wi-Fi and crypto. | ||
|
||
endif # SOC_FAMILY_SILABS_SIWX91X | ||
|
||
if SOC_FAMILY_SILABS_SIWX91X | ||
config SIWX91X_SILABS_OTA_SIGN_ENABLE | ||
bool "Silicon Labs Commander Signing Support" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In fact, the use can still launch |
||
depends on BUILD_OUTPUT_BIN | ||
help | ||
Activates signing of Zephyr binary with Silicon Labs Commander during build process. | ||
For more information refer this | ||
"https://www.silabs.com/documents/public/user-guides/ug574-siwx917-soc-manufacturing-utility-user-guide.pdf" | ||
|
||
config SIWX91X_SILABS_OTA_MIC_ENABLE | ||
bool "Silicon Labs Commander MIC Support" | ||
depends on BUILD_OUTPUT_BIN | ||
help | ||
Activates mic of Zephyr binary with Silicon Labs Commander during build process. | ||
For more information refer this | ||
"https://www.silabs.com/documents/public/user-guides/ug574-siwx917-soc-manufacturing-utility-user-guide.pdf" | ||
|
||
config SIWX91X_SILABS_OTA_ENCRYPT_ENABLE | ||
bool "Silicon Labs Commander Encryption Support" | ||
depends on BUILD_OUTPUT_BIN | ||
help | ||
Activates encryption of Zephyr binary with Silicon Labs Commander during build process. | ||
For more information refer this | ||
"https://www.silabs.com/documents/public/user-guides/ug574-siwx917-soc-manufacturing-utility-user-guide.pdf" | ||
|
||
config SIWX91X_OTA_ENCRYPT_KEY | ||
string "OTA Key for Encryption." | ||
help | ||
Specifies Over the Air key (hex) for encryption with Silicon Labs Commander. | ||
|
||
config SIWX91X_OTA_SIGN_KEY | ||
string "Private Key Path for Signing." | ||
help | ||
Path to private key file for signing with Silicon Labs Commander. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't you think we should mention "UG574: SiWx917 SoC Manufacturing Utility User Guide" somewhere? |
||
endif |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my experience, a build system that produces outputs with changing filenames tends to be really painful to maintain and to use, I don't recommend it. Among others, this variability may require extra logic in a CI building and consuming this output. Is it really important to have a different filename? Instead you could have
KERNEL_NAME_pre.rps
andKERNEL_NAME.rps
. When you don't sign, the latter is either a mere copy of the former, or signed with a development, pseudo-private key.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So FLASH_FILE has a
_signed
suffix even whenCONFIG_SIWX91X_SILABS_SIGN
is false? That does not seem to make sense.Just drop
_signed
and keep the filenames constant and CONFIG-independent, see #87669 (comment)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In fact, I believe the others tools change property
bin_file
of the targetrunners_yaml_props_target
(seecmake/mcuboot.cmake:16
orsoc/st/stm32/stm32n6x/CMakeLists.txt:35
). Probably we should adopt the same design.Then, we could probably remove the
--file=${FLASH_FILE}
from our command line.@marc-hb any objection?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have any blocking objection here, never had. But I've always thought and still think that the simplest and cleanest is to have something like:
KERNEL_BIN_NAME_unsigned.bin
orKERNEL_BIN_NAME_pre.bin
aswest sign
input.KERNEL_BIN_NAME.bin
aswest sign
outputWhen there is no signing, just copy the file. Print some warning that this is a mere copy.
This keeps everything "immutable": files and filenames. It's the best design from a ninja perspective and build troubleshooting perspective.
This area has always been a mess and I don't think there is a need to be "consistent" with other products there. Are they even consistent with each other in the first place? Consistency helps because it reduces cognitive load. But how many people sign different products on a regular basis? Very few if any.
BTW, for another example of a similar problem please search
_prebuilt
in zephyr/CMakeLists.txt