Repository navigation
Conversation
mcanevet
force-pushed
the
fix/secure-boot-policy-always-write
branch
5 times, most recently
from
September 17, 2026 06:25
0c3174a to
a47f069
Compare
…ceptance UEFI platform mode (Setup/Audit/User/Deployed) and this new capability are two independent channels and must not be conflated. Platform mode is a standardized, PK-presence-derived concept already expressible via SecureBootKeysResetter (DeletePK) and SecureBootCertificateImporter (PK import) - no new setter needed. SecureBootKeyManagementSetter instead gates whether the platform accepts *out-of-band* modification of the key stores at all, independent of mode or current key contents. Implemented for Dell only; other providers correctly surface ErrProviderImplementation by not implementing the interface. Lenovo has a direct analog (SecureBootConfiguration.SecureBootPolicy) that Lenovo's own docs confirm gates ImportSecureBootCertificate the same way, but it's not implemented here: unconfirmed whether the attribute is reachable via the generic /Bios PATCH this provider already uses, or requires Lenovo's proprietary OneCLI/XCC transport - untestable without real hardware. Recorded in providers/lenovo/secure_boot.go so it isn't rediscovered from scratch. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The old name read like it managed something generic; per review feedback, rename to make the actual behavior - toggling whether the platform accepts non-factory UEFI Secure Boot keys - clear from the signature alone. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Mickael Canevet <mickael.canevet@proton.ch>
…atches AllowCustomSecureBootKeys read the attribute's currently applied value and returned early when it already matched what was requested. Confirmed live, that's unsafe: currently-applied state can match while a different value is genuinely pending from an earlier call in the same boot cycle (e.g. a stale pending PATCH left staged by an interrupted, unrelated caller). Skipping the write left that stale pending value in place, silently reverting an explicit request on the next reboot - observed as a certificate import rejected because SecureBootPolicy committed as Standard despite requesting Custom moments earlier. It's the same class of bug stmcginnis/gofish#571 fixes one layer down, in SetBiosConfiguration's own diff baseline - but this early return happens before SetBiosConfiguration is ever called, so #571 can't reach it. The attribute is still read first, but only to reject platforms that don't expose it at all. The write itself is now unconditional. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Mickaël Canévet <mickael.canevet@proton.ch>
mcanevet
force-pushed
the
fix/secure-boot-policy-always-write
branch
from
September 17, 2026 14:04
a47f069 to
3f78ec4
Compare
Contributor
Author
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
1 participant
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.
Stacked on #466 and #468. This PR's diff currently bundles their commits in; it'll shrink to just the last commit once they merge.
AllowCustomSecureBootKeysread the attribute's currently applied value and returned early when it already matched what was requested. Confirmed live, that's unsafe: currently-applied state can match while a different value is genuinely pending from an earlier call in the same boot cycle (e.g. a stale pending PATCH left staged by an interrupted, unrelated caller). Skipping the write left that stale pending value in place, silently reverting an explicit request on the next reboot — observed as a certificate import rejected becauseSecureBootPolicycommitted asStandarddespite requestingCustommoments earlier.This is the same class of bug
stmcginnis/gofish#571(merged, picked up by #468) fixes one layer down, inSetBiosConfiguration's own diff baseline — but #468 alone doesn't cover this case, since the early return here skipped the call toSetBiosConfigurationentirely, before #468's fix ever gets a chance to run.The attribute is still read first, but only to reject platforms that don't expose it at all. The write itself is now unconditional.
Depends on #466 and #468.