efi: allow other architectures in check_host_security and friends - #565
Open
alexclewontin wants to merge 3 commits into
Open
efi: allow other architectures in check_host_security and friends#565alexclewontin wants to merge 3 commits into
alexclewontin wants to merge 3 commits into
Conversation
github.com/canonical/cpuid does not compile on non-x86 architectures, which forced the AMD64 host environment to be split across a filename gated default_env_amd64.go and a default_env_amd64_null.go stub. That made the implementation invisible to the compiler, and untestable, on every other architecture. Confine that dependency to a new internal/cpuid package. It is a deliberately thin wrapper, and the only place in the tree that is gated by architecture at build time. It exposes VendorIdentificator, Family and HasFeature, delegating to the upstream package on amd64 and returning zero values elsewhere. internal/efi is now architecture neutral: it carries no build constraints, the AMD64 implementation collapses into default_env.go, and AMD64() selects on a mockable runtimeGOARCH rather than a build tag. This mirrors the runtime dispatch used elsewhere for host security checks. The CPUIDFeature* constants stay in internal/efi, so callers are unchanged. Their drift guards against the upstream bit positions move to internal/cpuid, which is now the only package able to see both. The AMD64 tests lose their build constraints and drive the CPU identity through new mock hooks instead of mutating upstream package variables, so they run everywhere. TestNotAMD64Host no longer depends on the host architecture, and therefore now runs on amd64 too.
Signed-off-by: Alex Lewontin <alex.lewontin@canonical.com>
alexclewontin
commented
Sep 4, 2026
alexclewontin
left a comment
alexclewontin
left a comment
Member
Author
There was a problem hiding this comment.
Ported comments from last PR
| expectedPcrAlg: tpm2.HashAlgorithmSHA256, | ||
| expectedUsedSecureBootCAs: []*X509CertificateID{NewX509CertificateID(testutil.ParseCertificate(c, msUefiCACert))}, | ||
| expectedFlags: NoPlatformConfigProfileSupport | NoDriversAndAppsConfigProfileSupport | NoBootManagerConfigProfileSupport, | ||
| expectedWarningsMatch: `3 errors detected: |
Member
Author
There was a problem hiding this comment.
Porting @frederic-hoerni's comment from the last PR:
These are worth being displayed when tests are run (for identifying which host fixture raised an error, or for comparison of which host fixtures get tested from a version to another...).
They appear with -check.vv, but this option also activates other cumbersome log messages.
So, is there a way to give better visibility when running tests? And possibly ensure that Makefile and .github/workflows/test.yaml (or run-tests) take advantage of it.
| // add them to the const block at the top of this file and then audit the existing fixtures | ||
| // and add the new capability to any existing fixtures to which it might apply. | ||
| func runChecksPlatformHostFixtures() []runChecksHostFixture { | ||
| intelDevices := func(status []byte, withIOMMU bool) []internal_efi.SysfsDevice { |
Member
Author
There was a problem hiding this comment.
Porting @frederic-hoerni's comment from the last PR:
Instead of embedding a func here, wouldn't it be more legible to have it as a plain regular named function? That would also be easier to document and let runChecksPlatformHostFixtures() focus on the list that it returns?
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.
This is the continuation of #563 as part of a stack
Checking host security today makes assumes in many ways secboot will only be used on amd64. This PR attempts to lay the groundwork to allow other architectures to leverage much of the same logic.
Big themes:
removing architecture-conditional compilation, to allow unit tests to be mocked and run on any host architecture
abstracting the mocked test environment (the "fixture") from the actual code under test. This accounts for the bulk of the PR, as a ton of a couple test files now sit inside loops over fixtures. The whitespace insensitive diff is much smaller.
adding switch/case statements to the business logic of checking host security, so that each architecture can implement its own ecosystem-aware checks (as even within amd64 there doesn't seem to be a platform agnostic way to handle this).