Secureboot: SECURE_UPGRADE_PROD_TOOL_ARGS quoting issue#23733
Merged
lguohan merged 1 commit intosonic-net:masterfrom Oct 18, 2025
Merged
Secureboot: SECURE_UPGRADE_PROD_TOOL_ARGS quoting issue#23733lguohan merged 1 commit intosonic-net:masterfrom
lguohan merged 1 commit intosonic-net:masterfrom
Conversation
If setting: `SECURE_UPGRADE_PROD_TOOL_ARGS="-arg1 data -arg2 data"` The actual tool arguments would be received as just `-arg1` due to quoting issues. This patch fixes the necessary quoting to be able to pass the values from Makefile.work through to slave.mk.
bhouse-nexthop
requested review from
lguohan,
qiluo-msft and
xumia
as code owners
Collaborator
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Collaborator
Author
|
@DavidZagury @davidpil2002 @qiluo-msft please review |
This was referenced Aug 17, 2025
Enhancement: Secureboot production signing of kernel modules using ephemeral keys + bug fixes
#23406
Closed
Collaborator
Author
|
@Yarden-Z any comments? |
lguohan
approved these changes
Oct 18, 2025
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 <[email protected]>
FengPan-Frank
pushed a commit
to FengPan-Frank/sonic-buildimage
that referenced
this pull request
Dec 4, 2025
) If setting: `SECURE_UPGRADE_PROD_TOOL_ARGS="-arg1 data -arg2 data"` The actual tool arguments would be received as just `-arg1` due to quoting issues. This patch fixes the necessary quoting to be able to pass the values from Makefile.work through to slave.mk. Signed-off-by: Feng Pan <[email protected]>
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 <[email protected]> Signed-off-by: Feng Pan <[email protected]>
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 <[email protected]> Signed-off-by: xiaweijiang <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why I did it
If setting:
SECURE_UPGRADE_PROD_TOOL_ARGS="-arg1 data -arg2 data"The actual tool arguments would be received as just
-arg1due to quoting issues.Work item tracking
How I did it
This patch fixes the necessary quoting to be able to pass the values from Makefile.work through to slave.mk.
How to verify it
Pass multiple args during production secureboot signing without any quoting hacks and see it works.
Which release branch to backport (provide reason below if selected)
Tested branch (Please provide the tested image version)
master as of 20250817
Description for the changelog
Secureboot: SECURE_UPGRADE_PROD_TOOL_ARGS quoting issue
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 [email protected]