-
Notifications
You must be signed in to change notification settings - Fork 68
Add support for generating signed sysexts #175
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
lib/generate.sh
Outdated
erofs) | ||
mkfs.erofs "${fname}" "${basedir}" | ||
;; | ||
( |
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 make signing opt-in, so wrap the the direct invocations in one if branch?
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 could also reuse the exported repart flag options env var.
FYI: I also noticed problems with erofs and have filed a PR to fix this and if merged earlier you might need to adapt the settings when rebasing here #176
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 make signing opt-in, so wrap the the direct invocations in one if branch?
I've just modified the PR to include both raw squashfs and signed DDI in the release (see releases in my fork).
.github/workflows/release.yaml
Outdated
run: | | ||
pushd bakery | ||
echo "${{ secrets.SYSEXT_PRIVATE_KEY }}" > sysext.key | ||
echo "${{ secrets.SYSEXT_CERTIFICATE }}" > sysext.crt |
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 publish the cert as release artifact - and maybe include it in the sysupdate definition?
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.
Yes, we definitely should publish it as a release. However, I'm not so sure about updating it using sysupdate. If I understand correctly, sysupdate cannot cryptographically verify the integrity of the update (besides using HTTPS). So it could potentially be used to inject a malicious certificate during sysupdate. Maybe we could ship up-to date sysext bakery certificate in OS image (e.g. in /opt
) and the user would just symlink the cert to /etc/verity.d
to trust it. This way the bakery cert would be updated with OS update (that is verified using Flatcar release 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.
Sysupdate has an option to verify with GPG (which is recommended) but anyway, this then would have to be part of the Ignition config. I think for now we can also provision the cert via Ignition. If the cert doesn't change it sounds ok to have it in the OS but if it's expiring every now and then, having it in the OS might be more complex for managing the lifecycle?
lib/generate.sh
Outdated
systemd-repart \ | ||
--empty=create \ | ||
--size=auto \ | ||
--private-key=sysext.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.
does systemd-repart
support pkcs11 tokens? that would be more secure than making the private key accessible to github actions workers
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.
Yes, it does. I've even tested it with Keyvault PKCS11 token. I'll try to figure out how to use keyvault from github actions.
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've just reworked the PR to use PKCS11 token. I've also included script to set up Azure keyvault and managed identity for GiHub Action.
1f2a6ab
to
e6114b7
Compare
b668782
to
73241ab
Compare
Build a custom Ubuntu 24.10 image with PKCS11 Keyavault token. This is required because GitHUb runners only offer Ubuntu 24.04, which has systemd 255. We use PKCS11 signing feature of systemd-repart, which is only avalable since systemd 256 (in Ubuntu 24.10). The other reason is, we use PKCS11 Azure Keyvault token. It takes a non-trivial amount of time to compile and install it, which would have to be performed during every sysext build. Using a Docker image with a prebuild token significantly speeds-up the build process.
Use systemd-repart for creating signed DDI sysexts. Signing is performed via Azure Keyvault PKCS11 token, which is installed in the Docker image. We build both, signed discoverable disk image and raw unsigned image.
73241ab
to
e3b017d
Compare
I've just updated the PR. It now:
|
Are you sure we actually need Azure CLI? This is normally only needed when signing manually. In CI, you're more likely to use a managed identity or a token set in an environment variable. We currently used a managed identity for signing the boot binaries in CI. You may see errors about the |
SQUASHFS_ARG+=('-xattrs-exclude' "'^btrfs.'") | ||
fi | ||
|
||
# Note: We didn't chown to root:root, meaning that the file ownership is left as is |
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.
# Note: We didn't chown to root:root, meaning that the file ownership is left as is | |
# Note: We didn't chown to root:root, meaning that the file ownership is left as is with btrfs |
Maybe unpriv. Podman could help here because it can be nested many times (with the right invocation options). |
SYSTEMD_REPART_MKFS_OPTIONS_SQUASHFS="${SQUASHFS_ARG[*]}" | ||
# Compression recommendations from https://erofs.docs.kernel.org/en/latest/mkfs.html | ||
# and forcing a zero UUID for reproducibility (maybe we could also hash the name and version) | ||
SYSTEMD_REPART_MKFS_OPTIONS_EROFS="-zlz4hc,12 -C65536 -Efragments,ztailpacking -Uclear --all-root" |
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.
SYSTEMD_REPART_MKFS_OPTIONS_EROFS="-zlz4hc,12 -C65536 -Efragments,ztailpacking -Uclear --all-root" | |
SYSTEMD_REPART_MKFS_OPTIONS_EROFS="-zlz4hc,12 -C65536 -Efragments,ztailpacking -U00000000-0000-0000-0000-000000000000 --all-root" |
From #181
@chewi We need it because the job isn't running on an Azure VM with attached managed identity (like in our CI). As the job is running in GitHub it's not possible to directly assign an Azure identity to the job/worker. Instead I configure the managed identity to trust this github repo to provide federated identity to Azure (I basically followed this guide https://learn.microsoft.com/en-us/azure/developer/github/connect-from-azure-openid-connect). Azure then gives the github action a one-time token to login into the managed identity. The PKCS11 token than uses az to authenticate (using the |
I figured managed identities wouldn't be possible, but I thought environment variables might be. They are, but service principal secrets are not recommended. However! I chatted with Copilot, and it has told me that if azure-keyvault-pkcs11 adds support for workload identities (one extra line), then it should work without Azure CLI. The only clunky part is you have to write the token out to a file first. It admits this part could be better, but it's still preferable over installing Azure CLI. Azure::Identity::ChainedTokenCredential::Sources{
std::make_shared<Azure::Identity::EnvironmentCredential>(),
std::make_shared<Azure::Identity::WorkloadIdentityCredential>(),
getClientSecretCredential(),
std::make_shared<Azure::Identity::AzureCliCredential>(),
std::make_shared<Azure::Identity::ManagedIdentityCredential>()}); permissions:
id-token: write
contents: read
jobs:
authenticate:
runs-on: ubuntu-latest
steps:
- name: Request OIDC token and save to file
id: oidc-token
uses: actions/github-script@v7
with:
script: |
const fs = require('fs');
const token = await github.actions.getIDToken();
fs.writeFileSync('/tmp/azure-oidc-token', token);
return '/tmp/azure-oidc-token';
- name: Set AZURE_FEDERATED_TOKEN_FILE
run: echo "AZURE_FEDERATED_TOKEN_FILE=${{ steps.oidc-token.outputs.result }}" >> $GITHUB_ENV I'm also wondering whether you really need a Ubuntu image for this. The Flatcar SDK already has everything needed... except Docker? I'm confused about why that is needed though. |
Sign published sysexts
Use systemd-repart for creating signed DDI sysexts. The keys are expected to be stored in GitHub secrets SYSEXT_PRIVATE_KEY and SYSEXT_CERTIFICATE.
Note/TODO: this needs additional changes in the GH repo setup, there needs to be a signing key and certificate in GH secrets. Additionally, the signing certificate should be published as a release, so that consumers can use it to verify the sysexts. This can be automated through new GH actions, so that the signing key is would be generated and uploaded right in GH actions.
How to use
The following butane config can be used for testing (pulls wasmedge sysext as an example, puts signing certificate into
/etc/verity.d
and forces sysext signature verification using service dropin). For the sysext verification to work, you need to recompile kernel withCONFIG_DM_VERITY_VERIFY_ROOTHASH_SIG
(implemented in a separate PR flatcar/scripts#3162).Note: it uses releases from my fork of sysext-bakery
Testing done
[Describe the testing you have done before submitting this PR. Please include both the commands you issued as well as the output you got.]
changelog/
directory (user-facing change, bug fix, security fix, update)/boot
and/usr
size, packages, list files for any missing binaries, kernel modules, config files, kernel modules, etc.