Conversation
Signed-off-by: Johann Queuniet <sub_code.git@queuniet.fr>
446609e to
d3e982b
Compare
chewi
left a comment
There was a problem hiding this comment.
Thank you very much for this! Apart from the one suggestion and some overquoting (no quotes needed in [[ ]]), this looks fine. I think using environment variables is perfectly valid here, otherwise we'll need to add and remember to use arguments in several places. I ran this under GHA, which had some unrelated issues, so I tried it locally, including Kola's cl.basic test, and it just worked. If you can make that change, I'd be happy to merge without doing another build.
For us, we need to do things differently for official vs unofficial builds because our sources are public, and we obviously cannot publish the official private key. You won't have this problem, but I wondered whether you were satisfied with keeping the private key in a file, as opposed to using a safer PKCS11-based mechanism.
| if ! grep -q 'export MODULE_SIGNING_KEY_DIR=' /home/sdk/.bashrc; then | ||
| # For official builds, use ephemeral keys. For unofficial builds, use persistent directory | ||
| if [[ ${COREOS_OFFICIAL:-0} -eq 1 ]]; then | ||
| if [[ -n "${MODULE_SIGNING_KEY_DIR:-}" && -d "${MODULE_SIGNING_KEY_DIR}" ]]; then |
There was a problem hiding this comment.
I would drop the existence check and let it fail below. You don't want to specify MODULE_SIGNING_KEY_DIR and do a whole build, only to realise it used the wrong key at the end. Same below.
| if [[ -n "${MODULE_SIGNING_KEY_DIR:-}" && -d "${MODULE_SIGNING_KEY_DIR}" ]]; then | |
| if [[ -n ${MODULE_SIGNING_KEY_DIR:-} ]]; then |
Parameterize Secure Boot keys
The current Secure Boot pipeline uses two different set of keys, one is public, hard-coded in the SDK container filesystem, and intended for test builds, the other is pulled from an Azure KMS and intended for release builds. Neither can be meaningfully parameterized to sign builds with custom keys.
This merge request attempts to parameterize the test keys to allow using custom keys instead. I tried to minimize actual changes to the codebase and as such opted for environment variables, at least to validate the initial approach. I feel they're a bit too "magic" and obscure though, and CLI arguments could be added to the necessary scripts if preferred.
Also, the generation, env preparation and artefact verification could be offloaded to proper scripts (ie,
verify_sbkey_images,generate_sbkeysandset_sbkeys_env) to ease custom key workflow as well as act as documentation for the process.How to use
Generate keys:
Set a proper environment:
Build Flatcar as usual:
Testing done
Verify artefacts have been signed with the correct keys: