-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
scripts: generator script for key provisioning json files #20369
scripts: generator script for key provisioning json files #20369
Conversation
CI InformationTo view the history of this post, clich the 'edited' button above Inputs:Sources:sdk-nrf: PR head: e6af6d8be3ef5072875337772f0f10e535647c0d more detailssdk-nrf:
Github labels
List of changed files detected by CI (2)
Outputs:ToolchainVersion: 0f22b642ed Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped;
|
cracen_usage=PsaCracenUsageSceme[args.cracen_usage]) | ||
.pack()).decode('utf-8').upper() | ||
|
||
if args.file and Path(args.file).is_file(): |
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.
Make the type an input file, then you don't need this test.
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.
ok
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 struggle to get this to work correctly with the argparse.Filetype, when the file has to be created. I do not have time to debug this more. So keeping it as is.
json_data= json.loads('{ "version": 0, "keyslots": [ ]}') | ||
|
||
|
||
if args.key != "TRNG": |
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 think this should be mandatory, it won't be used for nRF54H right?
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.
It's not mandatory, it's just that if the value is TRNG the value is going to generated. But I could turn it into an argument so that either you have to supply a key or set the --trng_key argument maybe?
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.
Changed, so that --key or --trng-key is mandatory, but they are mutually exclusive.
description="Generate PSA key attributes and write to stdout." | ||
"Use 'xxd' or similar if you want to print it to a terminal:" | ||
f" 'python {Path(__file__).name} --args | xxd'", |
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.
This needs to be updated to match the new functionality and intended use.
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.
ok
) | ||
|
||
parser.add_argument( | ||
"--file", |
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.
type=argparse.FileType(mode="r")
or rw
if its to be created
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.
ok
"--key", | ||
help="Key value", | ||
type=str, | ||
required=True, |
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.
This shouldn't be required I guess?
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.
No, but one of --key, --trng-key or --key-from-file must be set.
value = f'0x{key}' | ||
else: | ||
key = args.key | ||
value = f'{key}:{int(math.ceil(args.size / 8))}' |
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.
What is the purpose of the ceil and division?
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.
The size input is in bits, for instance AES is 256 bit while ED25519 is 255 bit for some reason, but when I generate the key I must still generate a key size divisible by 8 (in bytes). So I need to generate 32 bytes of data for both AES and ED25519. So to make sure I get enough bytes, I need to make sure that I always round up to the closest number.
with open(args.file, "w", encoding="utf-8") as file: | ||
file.write(output) | ||
else: | ||
print(output) |
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.
Should it be conditional if we generate the json format or instead only the binary blob? That way the script could be re-used for other purposes where the PSA attribute binary blob is needed.
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.
Ok
we also need a codewner files to be updated. This should be shared resp for aurora and someone from Lumos team @barsok do we have a name here ? |
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.
code owners to be identified
Isn't it a line manager responsibility rather than project manager? I suggest talking to line management. |
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.
This script simplifies some steps. There is no longer a need to copy and paste values between the command line and the required JSON file. However, the full process is still not very user-friendly. Please take a look at all the steps required to generate and provision the keys:
1. Generate the keys:
openssl genpkey -algorithm Ed25519 -out MANIFEST_APPLICATION_GEN1_priv.pem
openssl genpkey -algorithm Ed25519 -out MANIFEST_RADIOCORE_GEN1_priv.pem
openssl genpkey -algorithm Ed25519 -out MANIFEST_OEM_ROOT_GEN1_priv.pem
2. Extract public keys:
openssl pkey -in MANIFEST_APPLICATION_GEN1_priv.pem -pubout -out MANIFEST_APPLICATION_GEN1_pub.pem
openssl pkey -in MANIFEST_RADIOCORE_GEN1_priv.pem -pubout -out MANIFEST_RADIOCORE_GEN1_pub.pem
openssl pkey -in MANIFEST_OEM_ROOT_GEN1_priv.pem -pubout -out MANIFEST_OEM_ROOT_GEN1_pub.pem
3. Check the required key IDs.
MANIFEST_PUBKEY_APPLICATION_GEN1 = 0x40022100
MANIFEST_PUBKEY_APPLICATION_GEN2 = 0x40022101
MANIFEST_PUBKEY_APPLICATION_GEN3 = 0x40022102
MANIFEST_PUBKEY_OEM_ROOT_GEN1 = 0x4000AA00
MANIFEST_PUBKEY_OEM_ROOT_GEN2 = 0x4000AA01
MANIFEST_PUBKEY_OEM_ROOT_GEN3 = 0x4000AA02
MANIFEST_PUBKEY_RADIOCORE_GEN1 = 0x40032100
MANIFEST_PUBKEY_RADIOCORE_GEN2 = 0x40032101
MANIFEST_PUBKEY_RADIOCORE_GEN3 = 0x40032102
4. Convert the public key to the RAW format/encoding:
from cryptography.hazmat.primitives import serialization
for key_path in ['MANIFEST_APPLICATION_GEN1_pub.pem', 'MANIFEST_RADIOCORE_GEN1_pub.pem', 'MANIFEST_OEM_ROOT_GEN1_pub.pem']:
with open(key_path, 'rb') as key_file:
key_data = key_file.read()
key = serialization.load_pem_public_key(key_data)
pub_key_bytes = key.public_bytes(
encoding=serialization.Encoding.Raw,
format=serialization.PublicFormat.Raw
)
print(f"Key: {key_path}, value: {pub_key_bytes.hex()}")
Key: MANIFEST_APPLICATION_GEN1_pub.pem, value: 5e9c6bc9a4a74a56a54c7b281082619aeaa76cff929d676dc0649d4135bda744
Key: MANIFEST_RADIOCORE_GEN1_pub.pem, value: 8c2c1686a1afe09831a6068f573c08becb345f0add41b87a2d3dca8219c908af
Key: MANIFEST_OEM_ROOT_GEN1_pub.pem, value: 58f3f146a3f26a60e566e23f0d9f8cc01cb2dce5d2ca7413719de58a50ddda63
4. Create JSON input file using generate_psa_key_attributes.py script
#application
python generate_psa_key_attributes.py --usage VERIFY_MESSAGE_EXPORT --id 0x40022100 --type ECC_TWISTED_EDWARDS --size 255 --algorithm EDDSA_PURE --location PERSISTENT_CRACEN --key 5e9c6bc9a4a74a56a54c7b281082619aeaa76cff929d676dc0649d4135bda744 --file all_keys.json --cracen_usage RAW
#radiocore
python generate_psa_key_attributes_new.py --usage VERIFY_MESSAGE_EXPORT --id 0x40032100 --type ECC_TWISTED_EDWARDS --size 255 --algorithm EDDSA_PURE --location PERSISTENT_CRACEN --key 8c2c1686a1afe09831a6068f573c08becb345f0add41b87a2d3dca8219c908af --file all_keys.json --cracen_usage RAW
#radiocore
python generate_psa_key_attributes_new.py --usage VERIFY_MESSAGE_EXPORT --id 0x4000AA00 --type ECC_TWISTED_EDWARDS --size 255 --algorithm EDDSA_PURE --location PERSISTENT_CRACEN --key 58f3f146a3f26a60e566e23f0d9f8cc01cb2dce5d2ca7413719de58a50ddda63 --file all_keys.json --cracen_usage RAW
5. Call nrfutil device command to provision the keys:
export snr=<snr number>
nrfutil device x-provision-nrf54h-keys --serial-number $snr --key-file key_application_gen_1.json
It would be nice to have support for different key types as an input, for example, PEM files, etc. Currently, the user needs to convert PEM keys to RAW encoding/RAW format, which is quite challenging. I do not know of any command-line option for that except for Python code.
It would also be nice to have better error handling and input validation. For example, the key value "0xdeadbeef" should be rejected. This could be addressed by adding support for key loading instead of pasting raw values.
Help message should be also somehow improved, it is not clear what is under particular options:
Generate PSA key attributes and write to stdout.Use 'xxd' or similar if you want to print it to a terminal: 'python generate_psa_key_attributes_new.py --args | xxd'
options:
-h, --help show this help message and exit
--usage {VERIFY_MESSAGE_EXPORT,ENCRYPT_DECRYPT,USAGE_DERIVE}
Key usage
--id ID Key identifier
--type {AES,ECC_TWISTED_EDWARDS,RAW_DATA}
Key type
--size SIZE Key size in bits
--algorithm {NONE,CBC,EDDSA_PURE}
Ket algorithm
--location {PERSISTENT_CRACEN,PERSISTENT_CRACEN_KMU}
Storage location
--key KEY Key value
--file FILE JSON file to create or modify
--cracen_usage {NONE,PROTECTED,SEED,ENCRYPTED,RAW}
CRACEN KMU Slot usage scheme
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 have added an option (--key-from-file) that can read public keys from a PEM file, but cleaning up the help screen was not so easy. It's auto generated from the options I add to the list, but I haven't found any way to format the output.
48ccb9a
to
2f48a36
Compare
2f48a36
to
a138fa8
Compare
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.
NACK-ing since there are issues that should be fixed, but I won't be hurt if you dismiss my review, then its at least documented what I meant should be fixed.
CODEOWNERS
Outdated
@@ -688,6 +688,7 @@ | |||
/scripts/print_toolchain_checksum.sh @nrfconnect/ncs-ci | |||
/scripts/sdp/ @nrfconnect/ncs-ll-ursus | |||
/scripts/twister/alt/zephyr/tests/drivers/mspi/api/testcase.yaml @nrfconnect/ncs-ll-ursus | |||
/scripts/generate_psa_key_attributes.py @hakonfam |
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.
Use a team, not a person
/scripts/generate_psa_key_attributes.py @hakonfam | |
/scripts/generate_psa_key_attributes.py @nrfconnect/ncs-aurora |
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.
Fixed
@@ -0,0 +1,273 @@ | |||
# | |||
# Copyright (c) 2025Nordic Semiconductor ASA |
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.
This should be fixed
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.
Fixed
import sys | ||
from pathlib import Path | ||
from enum import IntEnum | ||
from pathlib import Path |
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.
We should fix the duplicate import
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.
Fixed
choices=[x.name for x in PsaKeyLifetime], | ||
) | ||
|
||
parser.add_argument( |
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 this is mutually exclusive with --key-from-file
, but that is handled in the logic later in the script, not expressed in the parser itself? IMO it should be more clear that its either or. AFAICS its not mentioned in the docs either.
a138fa8
to
fd87c0d
Compare
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.
Approving although we should fix the license to match that of other python files in this repo, and it should be more clear to the user what parameters are mutual exclusive and not.
CODEOWNERS
Outdated
@@ -688,6 +688,7 @@ | |||
/scripts/print_toolchain_checksum.sh @nrfconnect/ncs-ci | |||
/scripts/sdp/ @nrfconnect/ncs-ll-ursus | |||
/scripts/twister/alt/zephyr/tests/drivers/mspi/api/testcase.yaml @nrfconnect/ncs-ll-ursus | |||
/scripts/generate_psa_key_attributes.py @nrfconnect/ncs-aurora TBD |
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.
/scripts/generate_psa_key_attributes.py @nrfconnect/ncs-aurora TBD | |
/scripts/generate_psa_key_attributes.py @nrfconnect/ncs-aurora |
We should remove the TBD as it makes the file unparseable I think
ad8d5de
to
a77f08d
Compare
This python script is used to generate json files that contain key data and metadata that is used by nrfutil provisioning keys into nrf54h and nrf54l devices. Signed-off-by: Even Falch-Larsen <[email protected]> Signed-off-by: Håkon Amundsen <[email protected]>
a77f08d
to
e6af6d8
Compare
This python script is used to generate json files that contain key data and en metadata that is used by nrfutil provisioning keys into nrf54h and nrf54l devices.