test: add FIPS provider validation to FIPS scenario tests#8502
Open
Devinwong wants to merge 19 commits into
Open
test: add FIPS provider validation to FIPS scenario tests#8502Devinwong wants to merge 19 commits into
Devinwong wants to merge 19 commits into
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
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
changed the title
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 #