Skip to content

Secureboot: Fix base paths for kernel module signing and verification#23734

Merged
lguohan merged 1 commit intosonic-net:masterfrom
bhouse-nexthop:secureboot-module-paths
Oct 18, 2025
Merged

Secureboot: Fix base paths for kernel module signing and verification#23734
lguohan merged 1 commit intosonic-net:masterfrom
bhouse-nexthop:secureboot-module-paths

Conversation

@bhouse-nexthop
Copy link
Collaborator

Why I did it

Without this change, additional files like usr/share/vim/vim90/tutor/tutor.ko end up getting signed which is inappropriate.

This also skips attempting to verify kernel module signatures if the module path is not specified. Previously it would verify the kernel module signatures twice.

Work item tracking

How I did it

Updated paths specified to signing and verification scripts.

How to verify it

Build with secureboot and observe usr/share/vim/vim90/tutor/tutor.ko is no longer signed.

Which release branch to backport (provide reason below if selected)

  • 202205
  • 202211
  • 202305
  • 202311
  • 202405
  • 202411
  • 202505

Tested branch (Please provide the tested image version)

master as of 20250817

Description for the changelog

Secureboot: Fix base paths for kernel module signing and verification

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

Fixes: #23406
Signed-off-by: Brad House bhouse@nexthop.ai

Without this change, additional files like `usr/share/vim/vim90/tutor/tutor.ko`
end up getting signed which is inappropriate.

This also skips attempting to verify kernel module signatures if the module
path is not specified.  Previously it would verify the kernel module signatures
twice.
@mssonicbld
Copy link
Collaborator

/azp run Azure.sonic-buildimage

@bhouse-nexthop
Copy link
Collaborator Author

@DavidZagury @davidpil2002 @qiluo-msft please review

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@bhouse-nexthop
Copy link
Collaborator Author

@Yarden-Z any comments?

@lguohan
Copy link
Collaborator

lguohan commented Oct 18, 2025

Here’s a review of the pull request based on the provided diff:

Summary

This PR updates paths and conditions related to kernel modules and secure boot signature verification in several scripts. The main change is switching the kernel modules path from the filesystem root ($FILESYSTEM_ROOT) to specifically ${FILESYSTEM_ROOT}/usr/lib/modules, and tightening a condition to avoid empty directory variables.

Detailed Review

1. build_debian.sh

  • Verification Path Update:
    • The script now verifies kernel modules in ${FILESYSTEM_ROOT}/usr/lib/modules instead of the entire $FILESYSTEM_ROOT.
    • For the vmlinuz file, the -k argument has been removed from the verification command, suggesting it is no longer needed for this file.
  • Impact:
    • This focuses the verification on the actual location of kernel modules, making the process more precise and possibly faster.
    • Removing -k for vmlinuz could be cleaning up an unnecessary argument.

2. scripts/secure_boot_signature_verification.sh

  • Condition Enhancement:
    • The check for KERNEL_MODULES_DIR now ensures it is not empty and exists as a directory:
      if [ ! -z "$KERNEL_MODULES_DIR" -a -d "$KERNEL_MODULES_DIR" ]; then
    • The comment is updated to clarify that kernel modules will be checked only if the directory is specified.
  • Impact:
    • This prevents errors when the variable is empty or not a directory, increasing robustness.

3. scripts/signing_secure_boot_dev.sh

  • Signing Path Update:
    • The signing script is directed to sign kernel modules specifically in ${FS_ROOT}/usr/lib/modules instead of the whole ${FS_ROOT}.
  • Impact:
    • Ensures that only the intended kernel modules are signed, not other files that might be under the filesystem root.

General Assessment

  • Code Quality:
    • The changes are clean and concise, improving specificity and error handling.
    • Comments are updated for clarity.
  • Functionality:
    • These updates should make secure boot signing and verification more robust by targeting the correct paths and handling empty variables gracefully.
  • Risk:
    • Minimal risk, as the changes restrict functionality to more specific paths and add safer checks.

Suggestions / Questions

  • If there are cases where kernel modules are stored outside usr/lib/modules, consider documenting or handling those cases.
  • Double-check that all downstream scripts and processes expect the updated paths and conditions.

Conclusion:
This PR is a good improvement for secure boot signing and verification scripts. The changes are well-targeted and should help prevent errors and improve performance.
If all tests pass and the affected scripts work as expected, I would recommend merging.

@lguohan lguohan merged commit 1d88b9c into sonic-net:master Oct 18, 2025
20 checks passed
lguohan pushed a commit that referenced this pull request Oct 27, 2025
…s and bugfixes) (#23405)

#### Why I did it

The current sonic secureboot implementation assumes all assets are signed with the DB Key.  There are roughly 3600 kernel modules that get signed in the process.

Organizations may have varying security policy requirements where any signing requests may require signoff by one or more parties, this may simply be infeasible for that many requests.

Given that the DB key may be a long-lived key, and only RSA 2048 is often used or available, it is a security best-practice to sign as few assets as possible with such a key.

This PR is fully backwards compatible with any pre-existing usages.  It introduces a new `rules/config` setting of `SECURE_UPGRADE_KERNEL_CAFILE` which is a path to a file for all trusted keys to embed into the kernel.  If that setting is not specified, it defaults to `SECURE_UPGRADE_SIGNING_CERT`.  There are no other infrastructure changes to support ephemeral keys for kernel module signing.  It is up to the organization to generate the ephemeral keys and pass them into their own signing scripts as required.

Dependent PR:
 * sonic-net/sonic-linux-kernel#499

Related PRs:
 * sonic-net/sonic-utilities#3989
 * #23732
 * #23733
 * #23734
 * #23735
 * #23736

Fixes #23406

##### Work item tracking

#### How I did it

Worked through production signing steps and hit issues when requiring signing approval with our HSM.  This was the most elegant solution that required the minimal changes.

This has been fully tested.

#### How to verify it

Since there are no provided production signing script examples this would require a vendor with an existing implementation to validate.  Nexthop has validated this internally.

#### Which release branch to backport (provide reason below if selected)


#### Tested branch (Please provide the tested image version)

master as of 20250721

#### Description for the changelog
Secureboot: Production signing enhancement to allow ephemeral kernel module keys

#### Link to config_db schema for YANG module changes
N/A

#### A picture of a cute animal (not mandatory but encouraged)

Signed-off-by: Brad House <bhouse@nexthop.ai>
bhouse-nexthop added a commit to bhouse-nexthop/sonic-buildimage that referenced this pull request Nov 7, 2025
* simplify logic, don't sprinkle +deb13 everywhere
* secureboot requires kbuild to be installed
* Merge conflict bad resolution from PR sonic-net#23734
saiarcot895 pushed a commit to saiarcot895/sonic-buildimage that referenced this pull request Nov 8, 2025
* simplify logic, don't sprinkle +deb13 everywhere
* secureboot requires kbuild to be installed
* Merge conflict bad resolution from PR sonic-net#23734
saiarcot895 pushed a commit to saiarcot895/sonic-buildimage that referenced this pull request Nov 17, 2025
* simplify logic, don't sprinkle +deb13 everywhere
* secureboot requires kbuild to be installed
* Merge conflict bad resolution from PR sonic-net#23734
saiarcot895 pushed a commit to saiarcot895/sonic-buildimage that referenced this pull request Nov 23, 2025
* simplify logic, don't sprinkle +deb13 everywhere
* secureboot requires kbuild to be installed
* Merge conflict bad resolution from PR sonic-net#23734
FengPan-Frank pushed a commit to FengPan-Frank/sonic-buildimage that referenced this pull request Dec 4, 2025
…sonic-net#23734)

Without this change, additional files like `usr/share/vim/vim90/tutor/tutor.ko`
end up getting signed which is inappropriate.

This also skips attempting to verify kernel module signatures if the module
path is not specified.  Previously it would verify the kernel module signatures
twice.

Signed-off-by: Feng Pan <fenpan@microsoft.com>
FengPan-Frank pushed a commit to FengPan-Frank/sonic-buildimage that referenced this pull request Dec 4, 2025
…s and bugfixes) (sonic-net#23405)

#### Why I did it

The current sonic secureboot implementation assumes all assets are signed with the DB Key.  There are roughly 3600 kernel modules that get signed in the process.

Organizations may have varying security policy requirements where any signing requests may require signoff by one or more parties, this may simply be infeasible for that many requests.

Given that the DB key may be a long-lived key, and only RSA 2048 is often used or available, it is a security best-practice to sign as few assets as possible with such a key.

This PR is fully backwards compatible with any pre-existing usages.  It introduces a new `rules/config` setting of `SECURE_UPGRADE_KERNEL_CAFILE` which is a path to a file for all trusted keys to embed into the kernel.  If that setting is not specified, it defaults to `SECURE_UPGRADE_SIGNING_CERT`.  There are no other infrastructure changes to support ephemeral keys for kernel module signing.  It is up to the organization to generate the ephemeral keys and pass them into their own signing scripts as required.

Dependent PR:
 * sonic-net/sonic-linux-kernel#499

Related PRs:
 * sonic-net/sonic-utilities#3989
 * sonic-net#23732
 * sonic-net#23733
 * sonic-net#23734
 * sonic-net#23735
 * sonic-net#23736

Fixes sonic-net#23406

##### Work item tracking

#### How I did it

Worked through production signing steps and hit issues when requiring signing approval with our HSM.  This was the most elegant solution that required the minimal changes.

This has been fully tested.

#### How to verify it

Since there are no provided production signing script examples this would require a vendor with an existing implementation to validate.  Nexthop has validated this internally.

#### Which release branch to backport (provide reason below if selected)

#### Tested branch (Please provide the tested image version)

master as of 20250721

#### Description for the changelog
Secureboot: Production signing enhancement to allow ephemeral kernel module keys

#### Link to config_db schema for YANG module changes
N/A

#### A picture of a cute animal (not mandatory but encouraged)

Signed-off-by: Brad House <bhouse@nexthop.ai>
Signed-off-by: Feng Pan <fenpan@microsoft.com>
xwjiang-ms pushed a commit to xwjiang-ms/sonic-buildimage that referenced this pull request Dec 22, 2025
…s and bugfixes) (sonic-net#23405)

#### Why I did it

The current sonic secureboot implementation assumes all assets are signed with the DB Key.  There are roughly 3600 kernel modules that get signed in the process.

Organizations may have varying security policy requirements where any signing requests may require signoff by one or more parties, this may simply be infeasible for that many requests.

Given that the DB key may be a long-lived key, and only RSA 2048 is often used or available, it is a security best-practice to sign as few assets as possible with such a key.

This PR is fully backwards compatible with any pre-existing usages.  It introduces a new `rules/config` setting of `SECURE_UPGRADE_KERNEL_CAFILE` which is a path to a file for all trusted keys to embed into the kernel.  If that setting is not specified, it defaults to `SECURE_UPGRADE_SIGNING_CERT`.  There are no other infrastructure changes to support ephemeral keys for kernel module signing.  It is up to the organization to generate the ephemeral keys and pass them into their own signing scripts as required.

Dependent PR:
 * sonic-net/sonic-linux-kernel#499

Related PRs:
 * sonic-net/sonic-utilities#3989
 * sonic-net#23732
 * sonic-net#23733
 * sonic-net#23734
 * sonic-net#23735
 * sonic-net#23736

Fixes sonic-net#23406

##### Work item tracking

#### How I did it

Worked through production signing steps and hit issues when requiring signing approval with our HSM.  This was the most elegant solution that required the minimal changes.

This has been fully tested.

#### How to verify it

Since there are no provided production signing script examples this would require a vendor with an existing implementation to validate.  Nexthop has validated this internally.

#### Which release branch to backport (provide reason below if selected)

#### Tested branch (Please provide the tested image version)

master as of 20250721

#### Description for the changelog
Secureboot: Production signing enhancement to allow ephemeral kernel module keys

#### Link to config_db schema for YANG module changes
N/A

#### A picture of a cute animal (not mandatory but encouraged)

Signed-off-by: Brad House <bhouse@nexthop.ai>
Signed-off-by: xiaweijiang <xiaweijiang@microsoft.com>
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.

Enhancement: Secureboot production signing of kernel modules using ephemeral keys + bug fixes

3 participants