test: add FIPS provider validation to FIPS scenario tests#8502
Merged
Conversation
Devinwong
requested review from
AbelHu,
SriHarsha001,
awesomenix,
calvin197,
cameronmeissner,
djsly,
ganeshkumarashok,
junjiezhang1997,
lilypan26,
mxj220,
pdamianov-dev,
phealy,
r2k1,
sulixu,
surajssd,
timmy-wright and
zachary-bailey
as code owners
djsly
approved these changes
May 13, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new E2E validator to assert Azure Linux 3 OS Guard (FIPS) nodes have a functioning FIPS/OpenSSL provider setup, and wires that validator into the existing Azl3 OS Guard scenario tests to catch regressions like the referenced portmap/OpenSSL provider failure.
Changes:
- Added
ValidateFIPSProvidervalidator to check kernel FIPS mode, OpenSSL provider state, and guard againstportmappanics. - Enabled the new validator in the
Test_AzureLinux3OSGuardandTest_AzureLinux3OSGuard_PMC_Installscenarios.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| e2e/validators.go | Introduces ValidateFIPSProvider validator (kernel FIPS, OpenSSL providers, portmap panic guard). |
| e2e/scenario_test.go | Invokes ValidateFIPSProvider in Azl3 OS Guard (FIPS) scenarios. |
djsly
reviewed
May 13, 2026
Devinwong
changed the title
djsly
approved these changes
May 20, 2026
The OpenSSL provider on AzureLinux V3 / ACL FIPS images is exposed as 'symcryptprovider', not 'symcrypt'. ValidateFIPSProvider was doing an exact-string match and so failed all Azl3/ACL FIPS scenarios even when SymCrypt was loaded and active. Switch opensslProviderActive to prefix-match the provider name so a single call covers 'fips', 'symcrypt', and 'symcryptprovider'. While here, address review feedback: - Loosen the provider-header regex to tolerate any leading whitespace (spaces or tabs) instead of exactly two spaces. - Drop the throwaway '_ = portmapExists' assignment. - Stop merging stderr into stdout and forcing 'exit 0' when running portmap; capture streams separately and preserve the real exit code so a Go runtime panic remains observable. - Mirror the same prefix match and regex tolerance in vhdbuilder/packer/test/linux-vhd-content-test.sh, and derive the OpenSSL 3.x gate from 'openssl version' instead of a hand-maintained OS allowlist. Add a Go unit test for opensslProviderActive covering the symcryptprovider, inactive-provider, missing-provider and tab-indented-output cases. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add ValidateFIPSProvider to Test_Ubuntu2004FIPS so all FIPS scenarios exercise the same check (djsly: 'should verify on all nodes'). The validator already skips the OpenSSL providers block on OpenSSL 1.1.x, so Ubuntu 20.04 still uses the legacy FIPS module path. - Add an /etc/os-release content assertion to Test_AzureLinuxV3Gen2FIPS so the scenario meaningfully validates the V3 FIPS VHD bootstrap, not just FIPS provider state. - Add an explicit os_version allowlist guard at the top of testFips in linux-vhd-content-test.sh. Unknown distros invoked with enable_fips=true now fail loudly instead of silently falling through the OpenSSL 1.1.x legacy path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace the tautological /etc/os-release ID=azurelinux check with the same containerd2 package-version assertion used by other AzureLinuxV3 scenarios. The VHD selector already pins the image to azurelinux, so checking /etc/os-release adds no signal; verifying the FIPS-baked containerd package version actually catches VHD build regressions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Revert the containerd2 package-version assertion. Non-FIPS Test_AzureLinuxV3_* scenarios already cover that, and package-version verification is the job of the build-time linux-vhd-content-test.sh, not e2e. This scenario exists specifically to validate FIPS, so keep it tightly scoped. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
In Test_ACLGen2FIPSTL we call ValidateACLFIPSEnabled immediately before ValidateFIPSProvider, and both validators independently cat /proc/sys/crypto/fips_enabled. Narrow ValidateACLFIPSEnabled to just the ACL-specific /etc/system-fips marker so each helper has a non-overlapping responsibility and we save one SSH round-trip per scenario run. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Address review feedback on the FIPS provider validation work: e2e/validators.go - Revert ValidateACLFIPSEnabled to also assert /proc/sys/crypto/fips_enabled == 1. Removing it created a fragile cross-helper contract; one duplicate SSH cat is preferable. - Treat malformed 'openssl version' output as a hard failure in ValidateFIPSProvider instead of silently logging 'skipped' — FIPS VHDs are expected to ship a functioning OpenSSL. - Broaden portmap failure-marker detection to also catch 'fatal error:' and 'runtime error:' alongside 'panic:'. - Promote provider header / status line regexes to package-level vars compiled once at init. e2e/validators_test.go - Add a case asserting that multiple active providers (default+legacy) with no fips/symcrypt header at all is still rejected. vhdbuilder/packer/test/linux-vhd-content-test.sh - Restore 'V2' to the FIPS os_version allowlist (was previously accepted, dropping it broke any caller passing V2). - Make the unrecognized-os_version branch 'return' after err so the FIPS body doesn't run with bogus inputs and emit follow-on errors. - Fail loudly (err) when openssl is missing or its version is unparsable, instead of silently logging 'skipped: detected version ''' and passing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Previously the OpenSSL providers check skipped silently for any version that didn't start with '3.'. That means a future 'OpenSSL 4.x' or a malformed/unrecognized version string would silently pass on a FIPS VHD. Switch to a strict allowlist: 3.x runs the providers check, 1.1.x skips (legacy FIPS module), anything else is a hard failure that forces explicit handling when a new openssl branch is supported. Applied symmetrically to e2e/validators.go (ValidateFIPSProvider) and vhdbuilder/packer/test/linux-vhd-content-test.sh (testFips). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Cover the case where an inactive symcrypt provider block is followed by an active default provider block. This is the most likely real-world misparse and explicitly locks in the 'status scoped to enclosing provider block' guarantee documented on the parser. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
cse_helpers.sh (sourced by linux-vhd-content-test.sh) overwrites the
global OS_VERSION from /etc/os-release VERSION_ID, so the call site
`testFips $OS_VERSION ...` passes the sourced value (e.g. "3.0", "2.0"),
not the pipeline-style value ("V3", "OSGuardV3", "acl"). The strict
allowlist introduced in 4564ee6 therefore rejected AzureLinux V3 /
OSGuard / ACL FIPS images at runtime (build 164255293, OSGuard job:
"testFips invoked with enable_fips=true on unrecognized os_version '3.0'").
This also reveals that the pre-PR `os_version = acl` branch was dead
code; switch it to OS_SKU (which is not clobbered) so the ACL FIPS
marker / UKI addon checks actually run.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…s, tighten indent rule - e2e/validators.go: capture combined stdout+stderr for `openssl version` so a future build that prints to stderr doesn't yield an empty parse target (review #9). - e2e/validators.go: replace the bare `runtime error:` substring with regex markers (`^panic:`, `^fatal error:`, `runtime.gopanic`, `goroutine N [running]`) so CNI plugins that mention "runtime error:" in their usage text don't trip a false failure (review #8). - e2e/validators.go: track each provider header's indent and require status lines to be strictly more indented, closing the same-indent mis-attribution hole. Also strip trailing \r so CRLF-terminated remote output still matches the header regex (review #7, #10). - e2e/validators_test.go: add two regression cases — a "Providers:" header with no entries below, and a same-indent `status: active` line that must not be attributed to the prior header (review #17). - vhdbuilder/packer/test/linux-vhd-content-test.sh: collapse the two adjacent `if [[ enable_fips == true ]]` blocks into one early-return guard, dedent the body, and dedent the 3.* case arm so it visually belongs to the case (reviews #5, #6). Mirror the Go parser improvements: `[ \t]` rather than `[[:space:]]` in the header/status regexes, explicit \r stripping, and strict "status more indented than header" check (review #7, #10). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ACL ships /etc/os-release VERSION_ID as e.g. 3.0.20260506. The exact-match allowlist 2.0|3.0 rejected ACL FIPS VHDs, breaking buildaclfipstlgen2 and buildaclarm64fipstlgen2 (ADO build 164284819). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ValidateACLFIPSEnabled now only asserts the ACL-specific /etc/system-fips marker. Kernel FIPS (/proc/sys/crypto/fips_enabled == 1) is universal and already covered by ValidateFIPSProvider, which Test_ACLGen2FIPSTL calls immediately after. Removes the duplicated SSH round-trip and gives each helper a single, clear responsibility. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ACL FIPS TL VHDs do not deploy /opt/cni/bin/portmap by default (ACL ships the cni-plugins tarball staged under /opt/cni/downloads/ and only promotes binaries to /opt/cni/bin/ via installCNILegacy when bridge/host-local/ loopback are absent), so the hard 'must exist' precondition fails the validator on Test_ACLGen2FIPSTL even when FIPS posture is correct. Switch portmap to best-effort: skip with a log when not present, otherwise exec and assert no Go runtime panic markers. Kernel-FIPS and OpenSSL provider checks remain authoritative; portmap is corroborating only. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove e2e/validators_test.go: the parser is e2e-only (no production impact), and synthetic inputs can drift from real OpenSSL output. The e2e FIPS scenarios remain the authoritative check. - Tighten verbose comments in e2e/validators.go and the testFips shell function to keep the rationale without restating obvious mechanics. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Devinwong
force-pushed
the
devinwong/add-fips-provider-validation
branch
from
c2af5e0 to
4978851
Compare
awesomenix
approved these changes
May 20, 2026
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
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.
What this PR does / why we need it:
test: add FIPS provider validation to FIPS scenario tests. This can help us to catch OpenSSL provider FIPS error as described in ICM 51000001009688
Which issue(s) this PR fixes:
Fixes #