-
Notifications
You must be signed in to change notification settings - Fork 74
Secure boot state checking for Classic and Core no matter if the enable or disable (New) #1998
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?
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1998 +/- ##
==========================================
+ Coverage 52.84% 53.16% +0.31%
==========================================
Files 395 396 +1
Lines 42623 42941 +318
Branches 7902 7970 +68
==========================================
+ Hits 22526 22831 +305
- Misses 19295 19305 +10
- Partials 802 805 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
a185fd6 to
7fdb457
Compare
7fdb457 to
e1ba8bc
Compare
|
I've tried with my project which is fail result |
rickwu666666
left a comment
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.
LGTM
tomli380576
left a comment
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.
lgtm overall just small things that can help simplify enum usage
Co-authored-by: Zhongning Li <60045212+tomli380576@users.noreply.github.com>
|
Tried with uc24/arm64/secure boot enabled platform. And test result were expected. pass result Failed result |
rickwu666666
left a comment
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.
LGTM
c99c838 to
349d87f
Compare
rickwu666666
left a comment
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.
LGTM
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.
Pull request overview
This PR introduces comprehensive secure boot state checking for both Ubuntu Classic and Core systems, validating both enabled and disabled states against manifest expectations. The implementation adds dual job coverage to prevent certification mismatches where systems might pass despite incorrect secure boot configurations.
Changes:
- Added two new secure boot validation jobs (
secure-boot-enabledandsecure-boot-disabled) that check against manifest expectations - Enhanced secure boot detection with automatic fallback from UEFI (mokutil) to FIT image (dumpimage) methods
- Created comprehensive test coverage (1438 lines) for the new secure boot checking functionality
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| providers/certification-client/units/client-cert-iot-ubuntucore-24.pxu | Added secure-boot-enabled and secure-boot-disabled job references to Ubuntu Core 24 test plan |
| providers/certification-client/units/client-cert-iot-server-24-04.pxu | Added secure-boot-enabled and secure-boot-disabled job references to IoT Server 24.04 test plan |
| providers/certification-client/units/client-cert-iot-desktop-24-04.pxu | Added secure-boot-enabled and secure-boot-disabled job references to IoT Desktop 24.04 test plan |
| providers/base/units/miscellanea/test-plan.pxu | Added both new secure boot jobs to base miscellanea test plan with blocker certification status |
| providers/base/units/miscellanea/jobs.pxu | Defined two new shell jobs for validating secure boot enabled/disabled states with manifest-based requirements |
| providers/base/tests/test_check_secure_boot_state.py | Added comprehensive unit test suite covering all classes, methods, and edge cases for secure boot state checking |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| The test automatically detects and uses the appropriate checking method: | ||
| - UEFI-based secure boot using mokutil | ||
| - FIT image-based secure boot using dumpimage | ||
|
|
Copilot
AI
Jan 23, 2026
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.
Trailing whitespace at the end of the file. Remove the unnecessary whitespace on line 675.
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.
Thanks for this PR. I especially appreciate the explanations and the tests run (so this extends to @rickwu666666, thanks for testing on your project!)
I got a recent certificate request for a project where the secure_boot_mode_* was failing, and suggesting to use this instead, so I had a look.
One actionable request is: since the word disabled is used when running the command, could the opposite be enabled and not enable to be more symmetrical?
I have a few questions:
- Does this come in addition to the
secure_boot_mode_*job, or to replace it? - Is it intended only for IoT projects?
I had a look at the code as well. To be honest I'm worried about PR that add thousands of lines of code. Some parts are re-inventing the wheel (like all the helper functions to log things, or the implementation of a timeout), some don't follow the guidance in place for python tests...
I understand the need to have better secure boot checks, but I'm sure this can be done in a more simple way. Could you revisit this?
WARNING: This modifies com.canonical.certification::sru-server
Description
Current secure boot testing only validates enabled states, missing checkpoints when systems have secure boot disabled. This can allow systems to pass certification with configurations that don't match manifest expectations.
1. Dual Job Coverage
secure-boot-enabled: Validates whenmanifest.has_secure_boot == 'True'secure-boot-disabled: Validates whenmanifest.has_secure_boot != 'True'2. Enhanced Detection
/sys/firmware/efi/path, usesmokutildumpimagefor image signature analysisFiles Modified
providers/base/units/miscellanea/jobs.pxu- Job definitionsproviders/base/bin/check_secure_boot_state.py- Script improvementsUsage
Resolved issues
Documentation
https://warthogs.atlassian.net/browse/OEMQA-6211
Tests
Testing on My Laptop: Ubuntu 22.04 Desktop Image with Secure Boot Enabled
{ "com.canonical.certification::has_secure_boot": True }checkbox-cli run com.canonical.certification::miscellanea/secure-boot-enabled com.canonical.certification::miscellanea/secure-boot-disabled{ "com.canonical.certification::has_secure_boot": False }Testing on Baoshan: UC 22 Image with Secure Boot Disabled (UBOOT + UKI image with UEFI)
checkbox-baoshan.checkbox-cli run com.canonical.certification::miscellanea/secure-boot-enabled com.canonical.certification::miscellanea/secure-boot-disabledVerify secure boot is enabled when manifest indicates supportwill be skipped. It will run now since the manifest requirement is removed.Testing on Baoshan: Ubuntu 22 Image with Secure Boot Enabled
checkbox-baoshan.checkbox-cli run com.canonical.certification::miscellanea/secure-boot-enabled com.canonical.certification::miscellanea/secure-boot-disabledVerify secure boot is disabled when manifest indicates no supportwill be skipped. It will run now since the manifest requirement is removed.