efi: allow other architectures in check_host_security and friends

[alexclewontin]

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).

[alexclewontin]

Ported comments from last PR

[alexclewontin]

Porting @frederic-hoerni's comment from the last PR (this was originally misplaced, and now actually points to the code addressing the comment):

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.

[Copilot]

🔵 Needs a closer look

The security-sensitive architecture dispatch and TPM behavior require final human cross-architecture validation.

Pull request overview

This PR prepares EFI host-security and TPM checks for additional architectures while preserving AMD64 behavior.

Changes:

• Replaces compile-time architecture gating with runtime dispatch.
• Introduces architecture-neutral CPUID wrappers and feature constants.
• Consolidates AMD64 implementations/tests and adds reusable host fixtures.

File summaries

File	Description
internal/efi/export_test.go	Adds architecture and CPUID test hooks.
internal/efi/export_amd64_test.go	Removes superseded AMD64-only hooks.
internal/efi/env.go	Adds portable CPUID feature constants.
internal/efi/default_env.go	Consolidates AMD64 environment logic with runtime gating.
internal/efi/default_env_test.go	Makes AMD64 environment tests architecture-neutral.
internal/efi/default_env_null.go	Removes compile-time non-AMD64 implementation.
internal/efi/default_env_not_amd64_test.go	Removes superseded architecture-gated test.
internal/efi/default_env_amd64.go	Moves AMD64 implementation into shared code.
internal/efi/default_env_amd64_test.go	Moves AMD64 tests into shared tests.
internal/cpuid/cpuid_null.go	Provides non-AMD64 CPUID stubs.
internal/cpuid/cpuid_amd64.go	Wraps the upstream AMD64 CPUID package.
internal/cpuid/cpuid_amd64_test.go	Verifies portable feature constants match upstream.
efi/preinstall/intel_util.go	Makes Intel utility logic cross-compilable.
efi/preinstall/export_test.go	Consolidates test exports and architecture mocking.
efi/preinstall/export_amd64_test.go	Removes redundant AMD64-only exports.
efi/preinstall/cpu_vendor.go	Updates copyright metadata.
efi/preinstall/cpu_vendor_test.go	Updates copyright metadata.
efi/preinstall/checks_fixture_test.go	Adds reusable platform fixtures and capabilities.
efi/preinstall/check_tpm.go	Adds runtime TPM-discreteness dispatch.
efi/preinstall/check_tpm_test.go	Consolidates architecture-specific TPM tests.
efi/preinstall/check_tpm_intel.go	Makes Intel TPM checks cross-compilable.
efi/preinstall/check_tpm_intel_test.go	Runs Intel TPM tests on all build architectures.
efi/preinstall/check_tpm_amd64.go	Removes superseded AMD64-only implementation.
efi/preinstall/check_tpm_amd64_test.go	Removes superseded AMD64-only tests.
efi/preinstall/check_host_security.go	Adds runtime host-security dispatch and shared AMD64 logic.
efi/preinstall/check_host_security_test.go	Consolidates host-security dispatch tests.
efi/preinstall/check_host_security_null.go	Removes compile-time fallback implementation.
efi/preinstall/check_host_security_intel.go	Uses portable CPUID constants.
efi/preinstall/check_host_security_intel_test.go	Removes AMD64-only test gating.
efi/preinstall/check_host_security_intel_csme18.go	Makes CSME18 checks cross-compilable.
efi/preinstall/check_host_security_intel_csme18_test.go	Makes CSME18 tests architecture-neutral.
efi/preinstall/check_host_security_intel_csme11.go	Makes CSME11 checks cross-compilable.
efi/preinstall/check_host_security_intel_csme11_test.go	Makes CSME11 tests architecture-neutral.
efi/preinstall/check_host_security_intel_btgmsr.go	Makes BootGuard MSR checks cross-compilable.
efi/preinstall/check_host_security_intel_btgmsr_test.go	Makes BootGuard MSR tests architecture-neutral.
efi/preinstall/check_host_security_amd64.go	Removes superseded AMD64-only dispatch.
efi/preinstall/check_host_security_amd64_test.go	Removes superseded AMD64-only tests.
efi/preinstall/check_host_security_amd.go	Makes AMD PSP checks cross-compilable.
efi/preinstall/check_host_security_amd_test.go	Makes AMD PSP tests architecture-neutral.

Review details

• Files reviewed: 39/41 changed files
• Comments generated: 1
• Review effort level: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[pedronis]

the basic idea here seems sounds

here are some generally minor issues found by a LLM review:

4. and 6. might definitely be OK as is

1. Unreachable test assertions (additionalExpectedFlags)

The additionalExpectedFlags field is declared in efi/preinstall/checks_fixture_test.go (line 86)
but no fixture ever sets it, so it is always 0. That makes these two blocks dead code:

• efi/preinstall/checks_context_test.go (line 892)
• efi/preinstall/checks_context_test.go (line 2481)

Each contains a full alternative set of assertions (ErrorKindNoSuitablePCRBank, specific PCR0
digests) plus a continue that would skip the trailing c.Assert(errs, HasLen, 0). This is
anticipatory arm64 code that has never been executed and whose hardcoded digests cannot have been
validated. It is also ~75 | fixture.additionalExpectedFlags OR-ins across
efi/preinstall/checks_test.go and efi/preinstall/checks_context_test.go that are all no-ops
today.

Suggestion: drop the mechanism and reintroduce it together with the arm64 fixture that needs it.

2. Accidentally deleted TODO

// TODO: Good test case for invalid PCR1 when we support it. was dropped from
efi/preinstall/checks_context_test.go during the restructuring of TestRunGoodInvalidPCR0Value.
It looks unintentional and should be restored.

3. Layering smell in the CPUID drift guard

CPUIDFeatureSMX and CPUIDFeatureSDBG are declared in internal/efi/env.go (lines 70 and 74),
but the drift-guard test that pins them to the upstream github.com/canonical/cpuid values lives in
internal/cpuid/cpuid_amd64_test.go and therefore has to import internal/efi — the reverse of the
production dependency direction.

Suggestion: declare the constants in internal/cpuid (and reference or re-export them from
internal/efi), keeping the test local to the package that owns the knowledge.

4. CI never exercises the non-amd64 paths

.github/workflows/test.yaml (line 9) runs only on ubuntu-24.04 (x64), so
internal/cpuid/cpuid_null.go and the general arch-neutrality claim are unverified by CI. Since the
point of the PR is "compiled on every architecture", a cheap
GOARCH=arm64 go build ./... && GOARCH=arm64 go vet ./... step would lock that in. It passes today.

5. Silent-skip risk in the fixture selector

runChecksHostFixturesFor in efi/preinstall/checks_fixture_test.go (line 89) calls c.Skip(...)
when no fixture matches the required capabilities. If a capability is mistyped or a fixture is
removed, a test silently stops running rather than failing.

Suggestion: at minimum, make the skip message name the missing capabilities.

6. Stub semantics are silently permissive

internal/cpuid/cpuid_null.go returns false from HasFeature (line 38) and "" from
VendorIdentificator (line 26) on non-amd64. These are unreachable today because
DefaultEnv.AMD64() gates on runtimeGOARCH first, but a future direct caller would silently get a
"feature absent" answer for a security-relevant check. A comment is present; consider whether
panicking would be safer.

7. logRunChecksHostFixture implementation

efi/preinstall/checks_fixture_test.go (line 118) uses fmt.Printf plus runtime.Caller and
panic on name-parse failure. c.Logf would be more idiomatic and avoid the
reflection-on-function-names fragility, though it does not give the always-on visibility that was
requested in review.

[alexclewontin]

1. Unreachable test assertions (additionalExpectedFlags)

The additionalExpectedFlags field is declared in efi/preinstall/checks_fixture_test.go (line 86) but no fixture ever sets it, so it is always 0. That makes these two blocks dead code:

• efi/preinstall/checks_context_test.go (line 892)
• efi/preinstall/checks_context_test.go (line 2481)

Each contains a full alternative set of assertions (ErrorKindNoSuitablePCRBank, specific PCR0 digests) plus a continue that would skip the trailing c.Assert(errs, HasLen, 0). This is anticipatory arm64 code that has never been executed and whose hardcoded digests cannot have been validated. It is also ~75 | fixture.additionalExpectedFlags OR-ins across efi/preinstall/checks_test.go and efi/preinstall/checks_context_test.go that are all no-ops today.

Suggestion: drop the mechanism and reintroduce it together with the arm64 fixture that needs it.

Fixed

2. Accidentally deleted TODO

// TODO: Good test case for invalid PCR1 when we support it. was dropped from efi/preinstall/checks_context_test.go during the restructuring of TestRunGoodInvalidPCR0Value. It looks unintentional and should be restored.

Fixed

3. Layering smell in the CPUID drift guard

CPUIDFeatureSMX and CPUIDFeatureSDBG are declared in internal/efi/env.go (lines 70 and 74), but the drift-guard test that pins them to the upstream github.com/canonical/cpuid values lives in internal/cpuid/cpuid_amd64_test.go and therefore has to import internal/efi — the reverse of the production dependency direction.

Suggestion: declare the constants in internal/cpuid (and reference or re-export them from internal/efi), keeping the test local to the package that owns the knowledge.

Fixed

4. CI never exercises the non-amd64 paths

.github/workflows/test.yaml (line 9) runs only on ubuntu-24.04 (x64), so internal/cpuid/cpuid_null.go and the general arch-neutrality claim are unverified by CI. Since the point of the PR is "compiled on every architecture", a cheap GOARCH=arm64 go build ./... && GOARCH=arm64 go vet ./... step would lock that in. It passes today.

Concur this is a non-issue, as it is addressed in #566 (I like PR stacks, but lack of visibility on things like this is a downside).

5. Silent-skip risk in the fixture selector

runChecksHostFixturesFor in efi/preinstall/checks_fixture_test.go (line 89) calls c.Skip(...) when no fixture matches the required capabilities. If a capability is mistyped or a fixture is removed, a test silently stops running rather than failing.

Suggestion: at minimum, make the skip message name the missing capabilities.

This is a good catch, and after some consideration I think that a test with no fixtures that meet the required capabilities should be considered a fail, not a skip, to avoid the risk of easy-to-overlook regression. Now errors out.

6. Stub semantics are silently permissive

internal/cpuid/cpuid_null.go returns false from HasFeature (line 38) and "" from VendorIdentificator (line 26) on non-amd64. These are unreachable today because DefaultEnv.AMD64() gates on runtimeGOARCH first, but a future direct caller would silently get a "feature absent" answer for a security-relevant check. A comment is present; consider whether panicking would be safer.

I mostly concur this is a non-issue, since this should only be called during testing of amd64 business logic on non-amd64 platforms. The only thing that might make it nicer is if we provided two alternative implementations, one which is silently permissive for testing, and one which panics if this is ever reached during actual operation. Not sure if the complexity is actually worth it though.

7. logRunChecksHostFixture implementation

efi/preinstall/checks_fixture_test.go (line 118) uses fmt.Printf plus runtime.Caller and panic on name-parse failure. c.Logf would be more idiomatic and avoid the reflection-on-function-names fragility, though it does not give the always-on visibility that was requested in review.

This would change the behavior. I have no strong opinion on what the desired behavior here should be, but if we want to implement what @frederic-hoerni suggested I believe this is the right how