Skip to content

Fix OpenPGP key-fingerprint panic under GODEBUG=fips140=only - #458

Open
sameerforge wants to merge 2 commits into
carvel-dev:developfrom
sameerforge:topic/sameerkh/fips140-openpgp-armor-panic
Open

Fix OpenPGP key-fingerprint panic under GODEBUG=fips140=only#458
sameerforge wants to merge 2 commits into
carvel-dev:developfrom
sameerforge:topic/sameerkh/fips140-openpgp-armor-panic

Conversation

@sameerforge

@sameerforge sameerforge commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Summary

Go 1.26+ ships a native FIPS 140-3 validated cryptographic module (crypto/fips140). When running with GODEBUG=fips140=only, any use of a cryptographic primitive that isn't FIPS-approved panics instead of returning an error. Running vendir's Git commit/tag signature verification under this mode currently crashes the process in two separate places inside golang.org/x/crypto/openpgp: key-fingerprint parsing and signature verification.

This PR wraps both operations in crypto/fips140.WithoutEnforcement to prevent runtime panics under strict FIPS mode, and bumps the project's Go version and dependencies accordingly.

Detailed Changes

1. Key-fingerprint parsing (openpgparmor/armor.go)

openpgp.ReadArmoredKeyRing computes each key's RFC 4880 v4 fingerprint with SHA-1 unconditionally upon parsing. Under fips140=only, this panics immediately before signature verification can even begin.
Wrapped openpgp.ReadArmoredKeyRing in crypto/fips140.WithoutEnforcement so public key rings can be parsed without triggering a panic.

2. Signature verification (fetch/git/verification.go)

Verifying signatures made with non-FIPS-approved algorithms via openpgp.CheckArmoredDetachedSignature panics unconditionally under fips140=only.
Wrapped openpgp.CheckArmoredDetachedSignature in crypto/fips140.WithoutEnforcement to allow signature verification of legacy commits and tags to execute without panicking.

3. Toolchain & Dependency Updates

Since crypto/fips140.WithoutEnforcement requires Go 1.26+:

  • Bumped Go version to 1.26.5.
  • Upgraded carvel.dev/imgpkg dependency to v0.48.1.
  • Updated golangci-lint to align with the Go version.
  • Ran go mod tidy and go mod vendor to update go.sum and the vendor/ directory.

Test Plan

  • Key parsing unit test: TestReadArmoredKeys_UnderFIPS140Only reproduces the fingerprint panic on unfixed code and passes with the fix applied under GODEBUG=fips140=only.
  • Existing E2E tests: TestGitVerification (trusted/stranger/unsigned commit and tag cases) passes unchanged.
  • Sanity checks: go build ./..., go vet ./..., and golangci-lint run pass cleanly across the vendor tree.

@sameerforge
sameerforge force-pushed the topic/sameerkh/fips140-openpgp-armor-panic branch 11 times, most recently from 53d626d to 47a44b0 Compare August 14, 2026 10:12
sameerforge and others added 2 commits August 14, 2026 15:53
…UG=fips140=only

Under a native-FIPS build running with GODEBUG=fips140=only, two OpenPGP operations
trigger unhandled panics:
1. Parsing public keys via openpgp.ReadArmoredKeyRing computes an RFC 4880 v4
   key fingerprint (SHA-1) unconditionally, causing an immediate panic.
2. Verifying signed commits or tags via openpgp.CheckArmoredDetachedSignature
   call, which panics unconditionally under strict FIPS mode.

To resolve these panics, wrap both the keyring parsing call (openpgp.ReadArmoredKeyRing)
and the signature verification call (openpgp.CheckArmoredDetachedSignature) in
crypto/fips140.WithoutEnforcement. This allows key parsing and signature verification
to execute without panicking under GODEBUG=fips140=only.

Adds unit tests to cover key parsing and DSA signature verification under
GODEBUG=fips140=only.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Sameer <sameer.khan@broadcom.com>
- Update Go version to 1.26.5 in go.mod
- Bump carvel.dev/imgpkg dependency to v0.48.1
- Update golangci-lint to align with Go version
- Run `go mod tidy` and `go mod vendor` to update go.sum and vendor/

Signed-off-by: Sameer <sameer.khan@broadcom.com>
@sameerforge

Copy link
Copy Markdown
Contributor Author

@joaopapereira Please review the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants