-
Notifications
You must be signed in to change notification settings - Fork 76
Fix kernel module signing with ephemeral keys for official builds #3493
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?
Fix kernel module signing with ephemeral keys for official builds #3493
Conversation
The SDK container build process was persisting temporary directory paths for module signing keys into /home/sdk/.bashrc. This caused all container instances to share the same ephemeral key location. Fixed by: - Runtime check in sdk_entry.sh to recreate stale temp directories - Build-time cleanup in Dockerfiles to remove the variables Each container instance now gets unique temporary directories. Signed-off-by: Daniel Zatovic <[email protected]>
For official builds (COREOS_OFFICIAL=1), continue using ephemeral temporary directories for module signing keys. For unofficial/development builds, use a persistent directory at /mnt/host/source/.module-signing-keys to preserve keys across container restarts. Signed-off-by: Daniel Zatovic <[email protected]>
| # Check if MODULE_SIGNING_KEY_DIR exists in .bashrc and if the directory actually exists | ||
| if grep -q 'export MODULE_SIGNING_KEY_DIR' /home/sdk/.bashrc; then | ||
| # Extract the existing path | ||
| EXISTING_DIR=$(grep 'export MODULE_SIGNING_KEY_DIR' /home/sdk/.bashrc | sed "s/.*MODULE_SIGNING_KEY_DIR='\(.*\)'/\1/") |
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.
You probably could use cut like:
| EXISTING_DIR=$(grep 'export MODULE_SIGNING_KEY_DIR' /home/sdk/.bashrc | sed "s/.*MODULE_SIGNING_KEY_DIR='\(.*\)'/\1/") | |
| EXISTING_DIR=$(grep 'export MODULE_SIGNING_KEY_DIR=' /home/sdk/.bashrc | cut -f2- -d=) |
| # Extract the existing path | ||
| EXISTING_DIR=$(grep 'export MODULE_SIGNING_KEY_DIR' /home/sdk/.bashrc | sed "s/.*MODULE_SIGNING_KEY_DIR='\(.*\)'/\1/") | ||
| # If directory doesn't exist (stale from image build), remove the old entries and recreate | ||
| if [[ ! -d "$EXISTING_DIR" ]]; then |
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 need to quote variables inside double brackets in bash:
| if [[ ! -d "$EXISTING_DIR" ]]; then | |
| if [[ ! -d ${EXISTING_DIR} ]]; then |
| sed -i '/export MODULE_SIGNING_KEY_DIR/d' /home/sdk/.bashrc | ||
| sed -i '/export MODULES_SIGN_KEY/d' /home/sdk/.bashrc | ||
| sed -i '/export MODULES_SIGN_CERT/d' /home/sdk/.bashrc |
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 be a single invocation to sed, like:
(Also I'd add = to the keys to be sure we match the exact key, not like MODULE_SIGNING_KEY_DIR_HAHA_MY_OWN).
| sed -i '/export MODULE_SIGNING_KEY_DIR/d' /home/sdk/.bashrc | |
| sed -i '/export MODULES_SIGN_KEY/d' /home/sdk/.bashrc | |
| sed -i '/export MODULES_SIGN_CERT/d' /home/sdk/.bashrc | |
| sed -i -e '/export MODULE_SIGNING_KEY_DIR=/d' \ | |
| -e '/export MODULES_SIGN_KEY=/d' \ | |
| -e '/export MODULES_SIGN_CERT=/d' /home/sdk/.bashrc |
| RUN sed -i '/export MODULE_SIGNING_KEY_DIR/d' /home/sdk/.bashrc && \ | ||
| sed -i '/export MODULES_SIGN_KEY/d' /home/sdk/.bashrc && \ | ||
| sed -i '/export MODULES_SIGN_CERT/d' /home/sdk/.bashrc |
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 be a single sed invocation, and I'd add = to keys, see other comment below for details.
| RUN sed -i '/export MODULE_SIGNING_KEY_DIR/d' /home/sdk/.bashrc && \ | ||
| sed -i '/export MODULES_SIGN_KEY/d' /home/sdk/.bashrc && \ | ||
| sed -i '/export MODULES_SIGN_CERT/d' /home/sdk/.bashrc |
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 be a single sed invocation, and I'd add = to keys, see other comment below for details.
| RUN sed -i '/export MODULE_SIGNING_KEY_DIR/d' /home/sdk/.bashrc && \ | ||
| sed -i '/export MODULES_SIGN_KEY/d' /home/sdk/.bashrc && \ | ||
| sed -i '/export MODULES_SIGN_CERT/d' /home/sdk/.bashrc |
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 be a single sed invocation, and I'd add = to keys, see other comment below for details.
| grep -q 'export MODULE_SIGNING_KEY_DIR' /home/sdk/.bashrc || { | ||
| MODULE_SIGNING_KEY_DIR=$(su sdk -c "mktemp -d") | ||
| # Check if MODULE_SIGNING_KEY_DIR exists in .bashrc and if the directory actually exists | ||
| if grep -q 'export MODULE_SIGNING_KEY_DIR' /home/sdk/.bashrc; then |
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 grep -q 'export MODULE_SIGNING_KEY_DIR' /home/sdk/.bashrc; then | |
| if grep -q 'export MODULE_SIGNING_KEY_DIR=' /home/sdk/.bashrc; then |
| fi | ||
|
|
||
| # Create key directory if not already configured in .bashrc | ||
| if ! grep -q 'export MODULE_SIGNING_KEY_DIR' /home/sdk/.bashrc; then |
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 ! grep -q 'export MODULE_SIGNING_KEY_DIR' /home/sdk/.bashrc; then | |
| if ! grep -q 'export MODULE_SIGNING_KEY_DIR=' /home/sdk/.bashrc; then |
| MODULE_SIGNING_KEY_DIR=$(su sdk -c "mktemp -d") | ||
| else | ||
| MODULE_SIGNING_KEY_DIR="/home/sdk/.module-signing-keys" | ||
| su sdk -c "mkdir -p '$MODULE_SIGNING_KEY_DIR'" |
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 quoting feature of bash (search for ${parameter@operator} in https://www.gnu.org/software/bash/manual/bash.html#Shell-Parameter-Expansion):
| su sdk -c "mkdir -p '$MODULE_SIGNING_KEY_DIR'" | |
| su sdk -c "mkdir -p ${MODULE_SIGNING_KEY_DIR@Q}" |
Summary
This PR fixes kernel module signing to work correctly with ephemeral keys for official builds while maintaining persistent keys for development builds. It also resolves portage sandbox violations that occurred when building kernel modules.
Problem
After switching to ephemeral keys (in
/tmp) for official builds, two issues emerged:Overly strict validation: The
get_sig_key()function was enforcing/tmp/*paths for ALL builds, but this check should only apply to official builds.Sandbox violations: For development builds using persistent keys in
/home/sdk/.module-signing-keys, portage's sandbox was blocking write access during thesetup_keys()function, causing build failures.Solution
Commit 1: Fix ephemeral key directory paths baked into container images
/tmpenforcement conditional onCOREOS_OFFICIAL=1/tmpsdk_entry.shCommit 2: Add portage sandbox exception for development builds
addwrite "${MODULE_SIGNING_KEY_DIR}"insetup_keys()for unofficial builds/home/sdk/.module-signing-keysduring development/tmpdon't need this exception (already writable)Testing
/tmpsuccessfully/home/sdk/.module-signing-keyswithout sandbox violationsFiles Changed
sdk_container/src/third_party/coreos-overlay/eclass/coreos-kernel.eclass: Added conditional/tmpcheck and sandbox exceptionsdk_lib/sdk_entry.sh: Added stale directory detection and conditional key directory creationsdk_lib/Dockerfile.sdk-build: Clean up ephemeral variables during buildsdk_lib/Dockerfile.sdk-import: Clean up ephemeral variables during buildsdk_lib/Dockerfile.sdk-update: Clean up ephemeral variables during buildSigned-off-by: Daniel Zatovic [email protected]