fix(identity): harden HTTP clients and add TLS cert pinning for JWKS fetch (PILOT-241)#88
Open
matthew-pilot wants to merge 1 commit into
Open
fix(identity): harden HTTP clients and add TLS cert pinning for JWKS fetch (PILOT-241)#88matthew-pilot wants to merge 1 commit into
matthew-pilot wants to merge 1 commit into
Conversation
…fetch (PILOT-241) Harden sharedHTTPClient used for identity webhook calls: disable redirects (redirect is a protocol anomaly) and enforce TLS 1.2 minimum version. Add TLS certificate pinning support for JWKS key fetch: - New jwksPinnedHTTPClient helper returns a client that verifies the server's TLS certificate fingerprint (SHA-256 of DER) against a configured pin, rejecting MITM who serve a different certificate. - New GetKeyWithPinning method on JWKSCache accepts a pinning fingerprint. - New FetchJWKSKeysWithPinning exported function for external callers. - HandleValidateToken uses the Store's pinned fingerprint when fetching JWKS keys. - SetPinnedCertFingerprint / GetPinnedCertFingerprint on Store for runtime configuration (future: migrate to BlueprintIdentityProvider field). Previously sharedHTTPClient had no redirect protection and no TLS minimum. JWKS fetch was already hardened with jwksHTTPClient but lacked certificate pinning. Closes PILOT-241
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| // the actual cert verification via fingerprint check. The standard | ||
| // chain verification is bypassed to allow self-signed or | ||
| // non-CA-signed certs that match the pinned fingerprint. | ||
| InsecureSkipVerify: true, |
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
2 participants
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 changed
sharedHTTPClienthardeningTLS certificate pinning for JWKS fetch (PILOT-241)
jwksPinnedHTTPClient()helper that returns a hardened HTTP client which verifies the server TLS certificate fingerprint (SHA-256 of DER-encoded cert) against a configured pinGetKeyWithPinning()method onJWKSCache— fetches JWKS keys with optional TLS fingerprint verificationFetchJWKSKeysWithPinning()exported function for direct use by external callersHandleValidateTokennow uses the Store’s pinned fingerprint (when configured) during JWKS key fetchStore.SetPinnedCertFingerprint()/Store.GetPinnedCertFingerprint()for runtime configurationWhy
PILOT-241 reported that the OIDC/JWKS fetch path followed up to 10 redirects and had no TLS certificate pinning. The redirect + TLS minimum version parts were already addressed via
jwksHTTPClientin the original implementation. This PR:sharedHTTPClient(used by the identity webhook path)Verification
go build ./...— cleango vet ./...— cleango test ./...— all passing (includingidentitypackage)Closes PILOT-241