-
Notifications
You must be signed in to change notification settings - Fork 67
Rework handling of the Secure Boot keys and certificates #3044
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?
Conversation
We now need the official shim vendor certificate present in the SDK when building the kernel so that it can be inserted and used to verify the verity root hash and signed sysexts. While we're at it, copy the official signing certificate from Azure Key Vault so that we don't need to fetch it every time, simplifying the signing code. This change also partly deals with the eventual expiration of our shim vendor certificate. We cannot simply replace the shim with one containing just the new certificate because it needs to be able to boot kernels from older releases. We therefore now keep all the certificates in the coreos-sb-keys package as separate dated PEM files that then get combined into a single DER ESL that the shim build expects. Note that the shim does not check certificate expiry dates. It is therefore also no longer necessary to manually convert the certificate to DER format. The problem of actually upgrading the shim on user systems remains. Each certificate in the DER ESL requires an owner GUID. We previous used a zero GUID for the DB certificates, but these were only used for testing. I have therefore now generated a static GUID for Flatcar that we should use going forwards. Signed-off-by: James Le Cuirot <[email protected]>
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.
- If this is the official one, why is the issuer: "Flatcar Container Linux Secure Boot Development CA"
- This key is RSA 2K while the signing one is RSA 4K. The CA key must not be weaker than the signing key.
- Naming would be clearer if
shim-20241107.pem
were called "flatcar-sb-ca.pem"
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.
- That's what @sayanchowdhury called it. 🤷♂️ This CA certificate matches the issuer of flatcar-sb-dev-hsm-sign-2025 in AKV, which is the one we've been signing official releases with. I don't think this is the actual certificate we will use in the shim review, which would explain the name, but I've never been entirely clear on this.
- Again, @sayanchowdhury created it, so I'll leave him to answer this point.
- It is the shim vendor certificate, but yeah, that name might be clearer.
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.
Since the current solution was a stop-gap solution, my intention was to keep the name as development. Going foward, I'll post the metadata to be used and once vouched for, I'll proceed.
I'll look into the keysize.
# its validity. Only the newest is needed in PEM format to sign the | ||
# bootloader and the kernel image. Unofficial builds are already covered | ||
# above because the shim vendor cert /is/ the signing cert, not a CA. | ||
openssl x509 -in "${FILESDIR}"/official/signing.pem -inform PEM -out official/signing.pem || die |
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 certificate is only needed during signing - storing it in the repo means we need to update more places. Fetching it from Keyvault when signing seems like the better choice.
|
||
PKCS11_ENV=( | ||
AZURE_CORE_COLLECT_TELEMETRY=no |
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 think we should leave this at the default, though you may want to provide an option for rebuilders to easily override.
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 figured the telemetry might not be that useful when it is coming from an ephemeral container, but I don't feel strongly about it.
else | ||
SBSIGN_KEY="pkcs11:token=flatcar-sb-dev-hsm-sign-2025" | ||
unset SBSIGN_CERT | ||
SBSIGN_CERT="/usr/share/sb_keys/official/signing.pem" |
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.
Since we can fetch it from Keyvault on demand, and this certificate is needed while signing, why store it in the repo. Better keep the way it's done.
I like some of the cleanups here, but not everything is settled yet when it comes to changes to the chain-of-trust. Did you see my comments to the doc? The verity root hash does not need to be signed by a key in the kernel keyring if it is validated by grub, and signed sysexts would prevent users from loading their own sysexts (correct me if i'm wrong here), including those from the sysext-bakery. Even if we would want to sign either of the two, I'm not sure we would want it to be the same key as the one used by shim. Cryptographers prefer to not reuse keys in different contexts like this (eg. if you need to revoke one key, you invalidate unrelated artifacts).
This seems to have a downside - we have to sync another location when the key changes. Why not fetch it at runtime?
Just making sure we're on the same page: the shim vendor certificate is our Flatcar SB CA certificate, which is meant to have a lifetime of ~30 years (compare with Debian https://wiki.debian.org/SecureBoot#Finding_Debian.27s_Secure_Boot_keys). That's not the one we should be worried about expiring. |
Build action triggered: https://github.com/flatcar/scripts/actions/runs/15850281985 |
Actually no, it hadn't emailed any notification.
Our current one has an expiry in 2037, as set by @sayanchowdhury. Still, I thought it best to have something in place in case we need to rotate it sooner. I'll get back to you on the other points. |
I miss things all the time for the opposite reason - too many notifications... I just saw one for a comment you posted on a commit a month back...
No rush, lets get this right :) |
Rework handling of the Secure Boot keys and certificates
We now need the official shim vendor certificate present in the SDK when building the kernel so that it can be inserted and used to verify the verity root hash and signed sysexts.
While we're at it, copy the official signing certificate from Azure Key Vault so that we don't need to fetch it every time, simplifying the signing code.
This change also partly deals with the eventual expiration of our shim vendor certificate. We cannot simply replace the shim with one containing just the new certificate because it needs to be able to boot kernels from older releases. We therefore now keep all the certificates in the coreos-sb-keys package as separate dated PEM files that then get combined into a single DER ESL that the shim build expects. Note that the shim does not check certificate expiry dates. It is therefore also no longer necessary to manually convert the certificate to DER format. The problem of actually upgrading the shim on user systems remains.
Each certificate in the DER ESL requires an owner GUID. We previous used a zero GUID for the DB certificates, but these were only used for testing. I have therefore now generated a static GUID for Flatcar that we should use going forwards.
@sayanchowdhury, you will no longer need to set
SHIM_SIGNING_CERTIFICATE
in your shim Dockerfile¸ but you will need to ensure the SDK you build from already has the certificate in coreos-sb-keys.How to use
Nothing to use here, if it builds and the tests pass, we should be good.
Testing done
I did a Jenkins run, including forcing an official build. It passed, except for qemu_update, but that was due to how I forced it to be official, not due to the change.
changelog/
directory (user-facing change, bug fix, security fix, update) -- N/A/boot
and/usr
size, packages, list files for any missing binaries, kernel modules, config files, kernel modules, etc.