Skip to content

OOT modules sign - #2636

Merged
danzatt merged 2 commits into
flatcar:mainfrom
danzatt:danzatt/oot-modules-sign
Apr 30, 2025
Merged

danzatt merged 2 commits into
flatcar:mainfrom
danzatt:danzatt/oot-modules-sign

Conversation

@danzatt

@danzatt danzatt commented Feb 5, 2025

Copy link
Copy Markdown
Contributor

[Title: describe the change in one sentence]

For out of tree modules (like ZFS or NVIDIA) to work with secureboot, they need to be signed by the ephemeral kernel modules key. This key is shredded after the upstream-included kernel modules are built, therefore it can't be reused during ZFS module build. This PR moves the key to /tmp, so that it stays in RAM and can be reused by out of tree modules. Moreover, by moving the key to /tmp we improve the security of the ephemeral module signing key (previously we wrote it to disk and then shredded it, but it might still stay in the disk or software cache, compromising the secure boot model).

Currently, this PR works when the packages are built manually in the order coreos-modules, zfs-kmod and coreos-kernel. We need to fix the dependecies, so that we enforce this order.

[ describe the change in 1 - 3 paragraphs ]

How to use

[ describe what reviewers need to do in order to validate this PR ]

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 entries added in the respective changelog/ directory (user-facing change, bug fix, security fix, update)
  • Inspected CI output for image differences: /boot and /usr size, packages, list files for any missing binaries, kernel modules, config files, kernel modules, etc.

@danzatt
danzatt marked this pull request as draft February 6, 2025 14:20

@chewi chewi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mentioned this on Teams, so perhaps you've initially taken a shortcut, but using a deterministic path in /tmp is dangerous. We should generate a random one with mktemp and set an environment variable.

@danzatt
danzatt force-pushed the danzatt/oot-modules-sign branch from a1ce873 to 935efe2 Compare February 13, 2025 11:50
Comment thread sdk_container/src/third_party/coreos-overlay/eclass/coreos-kernel.eclass Outdated
Comment thread sdk_container/src/third_party/coreos-overlay/eclass/coreos-kernel.eclass Outdated
Comment thread sdk_container/src/third_party/coreos-overlay/eclass/coreos-kernel.eclass Outdated
Comment thread sdk_container/src/third_party/coreos-overlay/eclass/coreos-kernel.eclass Outdated
Comment thread sdk_lib/sdk_entry.sh Outdated
Comment thread run_sdk_container Outdated
@danzatt
danzatt force-pushed the danzatt/oot-modules-sign branch from 935efe2 to 17589ba Compare February 13, 2025 14:02
@danzatt
danzatt force-pushed the danzatt/oot-modules-sign branch from 17589ba to 06cda22 Compare February 20, 2025 12:59
@danzatt

danzatt commented Feb 20, 2025

Copy link
Copy Markdown
Contributor Author

I've rebased the PR and added new function which just verifies the conditions (that the key is in /tmp).

@chewi chewi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Arrgh, sorry, just noticed one more thing. It isn't enough for /tmp/$(uuidgen) to be random. It actually needs to be created with mktemp in order to be safe. Hopefully that isn't a problem.

@danzatt danzatt changed the title [WIP/RFC] OOT modules sign OOT modules sign Feb 28, 2025
@danzatt
danzatt marked this pull request as ready for review February 28, 2025 14:03
@danzatt
danzatt requested a review from krnowak March 20, 2025 14:22
@danzatt
danzatt force-pushed the danzatt/oot-modules-sign branch from 06cda22 to f5f7fb2 Compare March 28, 2025 12:46
@danzatt danzatt mentioned this pull request Mar 31, 2025
1 task
@danzatt
danzatt force-pushed the danzatt/oot-modules-sign branch from f5f7fb2 to 729d83c Compare April 29, 2025 08:09
@danzatt

danzatt commented Apr 29, 2025

Copy link
Copy Markdown
Contributor Author

Arrgh, sorry, just noticed one more thing. It isn't enough for /tmp/$(uuidgen) to be random. It actually needs to be created with mktemp in order to be safe. Hopefully that isn't a problem.

I fixed this now

@danzatt
danzatt requested a review from chewi April 29, 2025 08:12
@github-actions

github-actions Bot commented Apr 29, 2025 •

Copy link
Copy Markdown

Build action triggered: https://github.com/flatcar/scripts/actions/runs/14757631459

@chewi chewi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The shell quoting is still a bit wonky, so please use shellcheck in future. It's good enough though. Thanks!

@danzatt
danzatt enabled auto-merge (rebase) April 30, 2025 12:54
danzatt added 2 commits April 30, 2025 14:55
Move module signing key to /tmp, so that it stays in RAM. Disable
shredding signing key after coreos-modules finishes, but rather shred it
after coreos-kernel finishes, so that out of tree modules (like ZFS from
upstream portage) can also use the key before it is shreded.
@danzatt
danzatt force-pushed the danzatt/oot-modules-sign branch from 729d83c to bfb5ec7 Compare April 30, 2025 12:56
@danzatt
danzatt disabled auto-merge April 30, 2025 12:56
@danzatt
danzatt merged commit 37f476e into flatcar:main Apr 30, 2025
@sayanchowdhury

Copy link
Copy Markdown
Member

Can you please create PR for changelog or add an entry with the nvidia sysext PR?

This branch had an error being deployed

1 failed deployment
development — bfb5ec7d Deployed Apr 30, 2025 by danzatt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants