-
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -55,4 +55,9 @@ RUN chmod 755 /home/sdk/sdk_entry.sh | |
| # it's likely that scripts and SDK tarball are out of sync | ||
| RUN /home/sdk/sdk_entry.sh ./update_chroot --toolchain_boards="amd64-usr arm64-usr" | ||
|
|
||
| # Clean up ephemeral key directory variables that were added during build | ||
| 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 | ||
|
Comment on lines
+59
to
+61
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could be a single sed invocation, and I'd add |
||
|
|
||
| ENTRYPOINT ["/home/sdk/sdk_entry.sh"] | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,3 +19,8 @@ RUN /home/sdk/sdk_entry.sh ./setup_board --board="amd64-usr" --regen_configs | |
| # Restore original .bashrc to remove sandbox disablement | ||
| RUN mv /home/sdk/.bashrc.bak /home/sdk/.bashrc | ||
| RUN chown sdk:sdk /home/sdk/.bashrc | ||
|
|
||
| # Clean up ephemeral key directory variables that were added during build | ||
| 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 | ||
|
Comment on lines
+24
to
+26
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could be a single sed invocation, and I'd add |
||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -52,16 +52,36 @@ sed -i -r '/^masters =/s/\bcoreos(\s|$)/coreos-overlay\1/g' /usr/local/portage/c | |||||||||||||
| # SDK container is launched using the su command below, which does not preserve environment | ||||||||||||||
| # moreover, if multiple shells are attached to the same container, | ||||||||||||||
| # we want all of them to share the same value of the variable, therefore we need to save it in .bashrc | ||||||||||||||
| 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 | ||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||
| # Extract the existing path | ||||||||||||||
| EXISTING_DIR=$(grep 'export MODULE_SIGNING_KEY_DIR' /home/sdk/.bashrc | sed "s/.*MODULE_SIGNING_KEY_DIR='\(.*\)'/\1/") | ||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You probably could use
Suggested change
|
||||||||||||||
| # If directory doesn't exist (stale from image build), remove the old entries and recreate | ||||||||||||||
| if [[ ! -d "$EXISTING_DIR" ]]; then | ||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No need to quote variables inside double brackets in bash:
Suggested change
|
||||||||||||||
| echo "Deleting stale module signing directory." | ||||||||||||||
| 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 | ||||||||||||||
|
Comment on lines
+62
to
+64
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Suggested change
|
||||||||||||||
| fi | ||||||||||||||
| fi | ||||||||||||||
|
|
||||||||||||||
| # Create key directory if not already configured in .bashrc | ||||||||||||||
| if ! grep -q 'export MODULE_SIGNING_KEY_DIR' /home/sdk/.bashrc; then | ||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||
| # For official builds, use ephemeral keys. For unofficial builds, use persistent directory | ||||||||||||||
| if [[ ${COREOS_OFFICIAL:-0} -eq 1 ]]; 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'" | ||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use quoting feature of bash (search for
Suggested change
|
||||||||||||||
| fi | ||||||||||||||
| if [[ ! "$MODULE_SIGNING_KEY_DIR" || ! -d "$MODULE_SIGNING_KEY_DIR" ]]; then | ||||||||||||||
| echo "Failed to create temporary directory for secure boot keys." | ||||||||||||||
| echo "Failed to create directory for module signing keys." | ||||||||||||||
| else | ||||||||||||||
| echo "export MODULE_SIGNING_KEY_DIR='$MODULE_SIGNING_KEY_DIR'" >> /home/sdk/.bashrc | ||||||||||||||
| echo "export MODULES_SIGN_KEY='${MODULE_SIGNING_KEY_DIR}/certs/modules.pem'" >> /home/sdk/.bashrc | ||||||||||||||
| echo "export MODULES_SIGN_CERT='${MODULE_SIGNING_KEY_DIR}/certs/modules.pub.pem'" >> /home/sdk/.bashrc | ||||||||||||||
| fi | ||||||||||||||
| } | ||||||||||||||
| fi | ||||||||||||||
|
|
||||||||||||||
| # This is ugly. | ||||||||||||||
| # We need to sudo su - sdk -c so the SDK user gets a fresh login. | ||||||||||||||
|
|
||||||||||||||
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.