From 538a9db0058b93b02407ed3465599a4af3f19e50 Mon Sep 17 00:00:00 2001 From: matthew-pilot Date: Mon, 29 Jun 2026 21:42:44 +0000 Subject: [PATCH 1/2] fix(identity): harden HTTP clients and add TLS cert pinning for JWKS 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 --- identity/identity.go | 119 +++++++++++++++++++++++++++++++++++++++---- 1 file changed, 110 insertions(+), 9 deletions(-) diff --git a/identity/identity.go b/identity/identity.go index 5ab4895..b2cbeed 100644 --- a/identity/identity.go +++ b/identity/identity.go @@ -13,6 +13,7 @@ import ( "crypto/rsa" "crypto/sha256" "crypto/tls" + "crypto/x509" "encoding/base64" "encoding/hex" "encoding/json" @@ -63,17 +64,31 @@ type WALRecorder func(nodeID uint32, newPubKeyB64, rotatedAt string, clearBadge // concurrent rotation landed between Phase 1 (snapshot) and Phase 3 (commit). var ErrKeyRotatedConcurrently = fmt.Errorf("rotate_key: key rotated concurrently, retry") -// sharedHTTPClient is reused across identity webhook and JWKS fetch calls so -// that the underlying TCP connections are pooled by the transport layer. -var sharedHTTPClient = &http.Client{Timeout: 5 * time.Second} +// sharedHTTPClient is reused across identity webhook calls so that the +// underlying TCP connections are pooled by the transport layer. +// It disables redirects entirely (a redirect during identity webhook calls +// is a protocol anomaly and a supply-chain attack vector) and enforces a +// TLS 1.2 minimum version (PILOT-241). +var sharedHTTPClient = &http.Client{ + Timeout: 5 * time.Second, + CheckRedirect: func(req *http.Request, via []*http.Request) error { + return http.ErrUseLastResponse + }, + Transport: &http.Transport{ + TLSClientConfig: &tls.Config{ + MinVersion: tls.VersionTLS12, + }, + }, +} // jwksHTTPClient is a hardened HTTP client used ONLY for JWKS key-fetch // (fetchJWKSKeys). It disables redirects entirely (a redirect during JWKS // fetch is a protocol anomaly and a supply-chain attack vector) and enforces // a TLS 1.2 minimum version. // -// TLS certificate pinning per-IDP is a follow-up tracked in PILOT-241; it -// requires a new BlueprintIdentityProvider field in common/registry/wire. +// TLS certificate pinning is handled via jwksPinnedHTTPClient which returns +// a client that verifies the server's TLS certificate fingerprint against +// the configured pinned fingerprint (PILOT-241). var jwksHTTPClient = &http.Client{ Timeout: 10 * time.Second, CheckRedirect: func(req *http.Request, via []*http.Request) error { @@ -198,6 +213,7 @@ type Store struct { identityWebhookURL string identityWebhookSecret string idpConfig *BlueprintIdentityProvider + pinnedCertFingerprint string jwksCache *JWKSCache @@ -391,9 +407,32 @@ func (st *Store) ClearIDPConfig() { st.mu.Lock() st.idpConfig = nil st.identityWebhookURL = "" + st.pinnedCertFingerprint = "" st.mu.Unlock() } +// SetPinnedCertFingerprint sets the pinned TLS certificate fingerprint +// (SHA-256 of the DER-encoded certificate) for the IDP. When set, every +// outbound TLS connection to the identity provider will verify that the +// server's certificate matches this fingerprint (PILOT-241). +// +// The fingerprint should be specified as a lowercase hex-encoded SHA-256 +// hash of the DER-encoded certificate, e.g. +// "a4b3c2d1e5f6..." (64 hex chars). +func (st *Store) SetPinnedCertFingerprint(fp string) { + st.mu.Lock() + st.pinnedCertFingerprint = fp + st.mu.Unlock() +} + +// GetPinnedCertFingerprint returns the currently configured pinned +// TLS certificate fingerprint, or empty string if not set. +func (st *Store) GetPinnedCertFingerprint() string { + st.mu.RLock() + defer st.mu.RUnlock() + return st.pinnedCertFingerprint +} + // --------------------------------------------------------------------------- // Protocol handlers // --------------------------------------------------------------------------- @@ -812,7 +851,7 @@ func (st *Store) HandleValidateToken(msg map[string]interface{}) (map[string]int switch header.Alg { case "HS256": - key, err := st.jwksCache.GetKey(idp.URL, header.Kid) + key, err := st.jwksCache.GetKeyWithPinning(idp.URL, header.Kid, st.pinnedCertFingerprint) if err != nil { return nil, fmt.Errorf("JWKS: %w", err) } @@ -835,7 +874,7 @@ func (st *Store) HandleValidateToken(msg map[string]interface{}) (map[string]int } case "RS256": - key, err := st.jwksCache.GetKey(idp.URL, header.Kid) + key, err := st.jwksCache.GetKeyWithPinning(idp.URL, header.Kid, st.pinnedCertFingerprint) if err != nil { return nil, fmt.Errorf("JWKS: %w", err) } @@ -1013,8 +1052,59 @@ func newJWKSCache() *JWKSCache { return NewJWKSCache() } +// jwksPinnedHTTPClient returns an HTTP client for JWKS fetch that enforces +// TLS certificate pinning when a pinnedFingerprint is provided. The +// fingerprint is a lowercase hex-encoded SHA-256 hash of the DER-encoded +// certificate. When no fingerprint is set, the standard jwksHTTPClient +// is returned (PILOT-241). +func jwksPinnedHTTPClient(pinnedFingerprint string) *http.Client { + if pinnedFingerprint == "" { + return jwksHTTPClient + } + return &http.Client{ + Timeout: 10 * time.Second, + CheckRedirect: func(req *http.Request, via []*http.Request) error { + return http.ErrUseLastResponse + }, + Transport: &http.Transport{ + TLSClientConfig: &tls.Config{ + MinVersion: tls.VersionTLS12, + VerifyPeerCertificate: func(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error { + if len(rawCerts) == 0 { + return fmt.Errorf("no peer certificate to pin") + } + cert, err := x509.ParseCertificate(rawCerts[0]) + if err != nil { + return fmt.Errorf("parse peer certificate: %w", err) + } + h := sha256.Sum256(cert.Raw) + got := hex.EncodeToString(h[:]) + if !strings.EqualFold(got, pinnedFingerprint) { + return fmt.Errorf("certificate fingerprint mismatch: got %s, want %s", got, pinnedFingerprint) + } + return nil + }, + // InsecureSkipVerify is true because VerifyPeerCertificate does + // 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, + }, + }, + } +} + // GetKey returns the JWKS key for the given URL and kid, fetching if stale. +// Pinning is not applied; use GetKeyWithPinning for pinned TLS verification. func (c *JWKSCache) GetKey(jwksURL, kid string) (*JwksKey, error) { + return c.GetKeyWithPinning(jwksURL, kid, "") +} + +// GetKeyWithPinning returns the JWKS key for the given URL and kid, fetching +// if stale. When pinnedFingerprint is non-empty, the TLS connection is +// verified against the certificate fingerprint (SHA-256 of DER) so that a +// MITM serving a different certificate is rejected (PILOT-241). +func (c *JWKSCache) GetKeyWithPinning(jwksURL, kid, pinnedFingerprint string) (*JwksKey, error) { c.mu.RLock() if c.URL == jwksURL && time.Since(c.FetchedAt) < c.TTL && len(c.Keys) > 0 { if kid == "" { @@ -1038,7 +1128,7 @@ func (c *JWKSCache) GetKey(jwksURL, kid string) (*JwksKey, error) { } c.mu.RUnlock() - keys, err := FetchJWKSKeys(jwksURL) + keys, err := FetchJWKSKeysWithPinning(jwksURL, pinnedFingerprint) if err != nil { return nil, err } @@ -1074,7 +1164,18 @@ func FetchJWKSKeys(jwksURL string) ([]JwksKey, error) { } func fetchJWKSKeys(jwksURL string) ([]JwksKey, error) { - resp, err := jwksHTTPClient.Get(jwksURL) + return fetchJWKSKeysWithClient(jwksURL, jwksHTTPClient) +} + +// FetchJWKSKeysWithPinning fetches JWKS keys with TLS certificate pinning. +// The pinnedFingerprint is a hex-encoded SHA-256 of the DER certificate. +func FetchJWKSKeysWithPinning(jwksURL, pinnedFingerprint string) ([]JwksKey, error) { + client := jwksPinnedHTTPClient(pinnedFingerprint) + return fetchJWKSKeysWithClient(jwksURL, client) +} + +func fetchJWKSKeysWithClient(jwksURL string, client *http.Client) ([]JwksKey, error) { + resp, err := client.Get(jwksURL) if err != nil { return nil, fmt.Errorf("fetch JWKS: %w", err) } From 88830cb40968923bd189d56815d7cc2b4dce872e Mon Sep 17 00:00:00 2001 From: Teodor Calin Date: Tue, 30 Jun 2026 15:42:57 +0300 Subject: [PATCH 2/2] Replace InsecureSkipVerify with VerifyConnection cert pinning G402/CWE-295: the JWKS pinning client set InsecureSkipVerify:true with a VerifyPeerCertificate callback, which disabled all chain/expiry/hostname verification - the opposite of pinning. gosec G402 and CodeQL go/disabled-certificate-check flagged identity.go:1091. Pin via tls.Config.VerifyConnection, which runs after Go's default chain verification (InsecureSkipVerify stays false). It requires the leaf to match the configured pin by SPKI-SHA256 (preferred, survives key-preserving renewal) or full-cert SHA-256, fail-closed on missing/mismatched cert. Add tests covering SPKI match, cert-fingerprint match, mismatch rejection, empty-pin passthrough, and the no-peer-cert path. --- identity/identity.go | 93 +++++++++++++++++---------- identity/zz_jwks_pinning_test.go | 106 +++++++++++++++++++++++++++++++ 2 files changed, 167 insertions(+), 32 deletions(-) create mode 100644 identity/zz_jwks_pinning_test.go diff --git a/identity/identity.go b/identity/identity.go index b2cbeed..3583d27 100644 --- a/identity/identity.go +++ b/identity/identity.go @@ -411,14 +411,15 @@ func (st *Store) ClearIDPConfig() { st.mu.Unlock() } -// SetPinnedCertFingerprint sets the pinned TLS certificate fingerprint -// (SHA-256 of the DER-encoded certificate) for the IDP. When set, every -// outbound TLS connection to the identity provider will verify that the -// server's certificate matches this fingerprint (PILOT-241). +// SetPinnedCertFingerprint sets the pinned TLS certificate fingerprint for +// the IDP. When set, every outbound JWKS TLS connection to the identity +// provider must, in addition to passing normal chain verification, present a +// leaf certificate whose fingerprint matches this pin (PILOT-241). // -// The fingerprint should be specified as a lowercase hex-encoded SHA-256 -// hash of the DER-encoded certificate, e.g. -// "a4b3c2d1e5f6..." (64 hex chars). +// The fingerprint is a lowercase hex-encoded SHA-256 (64 hex chars) of either +// the certificate's Subject Public Key Info (SPKI, preferred — survives cert +// renewal that reuses the key) or the full DER-encoded certificate. Either +// form is accepted at verification time. func (st *Store) SetPinnedCertFingerprint(fp string) { st.mu.Lock() st.pinnedCertFingerprint = fp @@ -1052,11 +1053,57 @@ func newJWKSCache() *JWKSCache { return NewJWKSCache() } +// spkiFingerprint returns the lowercase hex-encoded SHA-256 hash of the +// certificate's Subject Public Key Info (SPKI). Pinning the SPKI rather than +// the whole certificate means the pin survives certificate renewal as long as +// the issuer reuses the same key pair, which is the standard HPKP/RFC 7469 +// practice. +func spkiFingerprint(cert *x509.Certificate) string { + h := sha256.Sum256(cert.RawSubjectPublicKeyInfo) + return hex.EncodeToString(h[:]) +} + +// certFingerprint returns the lowercase hex-encoded SHA-256 hash of the full +// DER-encoded certificate, supporting operators who pin the leaf certificate +// rather than its public key. +func certFingerprint(cert *x509.Certificate) string { + h := sha256.Sum256(cert.Raw) + return hex.EncodeToString(h[:]) +} + +// pinVerifyConnection returns a tls.Config.VerifyConnection callback that, on +// top of the standard chain verification that Go performs by default, requires +// the presented leaf certificate to match the configured pin. The pin may be +// either the SPKI-SHA256 (preferred) or the full-certificate SHA-256, both +// hex-encoded. Returning a non-nil error aborts the handshake (PILOT-241). +func pinVerifyConnection(pinnedFingerprint string) func(tls.ConnectionState) error { + return func(cs tls.ConnectionState) error { + if len(cs.PeerCertificates) == 0 { + return fmt.Errorf("tls pin: no peer certificate presented") + } + leaf := cs.PeerCertificates[0] + if strings.EqualFold(spkiFingerprint(leaf), pinnedFingerprint) { + return nil + } + if strings.EqualFold(certFingerprint(leaf), pinnedFingerprint) { + return nil + } + return fmt.Errorf("tls pin: leaf certificate fingerprint mismatch (spki=%s cert=%s want=%s)", + spkiFingerprint(leaf), certFingerprint(leaf), pinnedFingerprint) + } +} + // jwksPinnedHTTPClient returns an HTTP client for JWKS fetch that enforces // TLS certificate pinning when a pinnedFingerprint is provided. The -// fingerprint is a lowercase hex-encoded SHA-256 hash of the DER-encoded -// certificate. When no fingerprint is set, the standard jwksHTTPClient -// is returned (PILOT-241). +// fingerprint is a lowercase hex-encoded SHA-256 of either the certificate's +// SPKI (preferred) or the full DER certificate. When no fingerprint is set, +// the standard jwksHTTPClient is returned (PILOT-241). +// +// Pinning is layered ON TOP of normal certificate-chain verification: the +// default Go verifier still validates the chain, expiry, and hostname +// (InsecureSkipVerify stays false), and VerifyConnection then additionally +// requires the leaf to match the configured pin. A MITM presenting a +// different (even otherwise-valid) certificate is rejected. func jwksPinnedHTTPClient(pinnedFingerprint string) *http.Client { if pinnedFingerprint == "" { return jwksHTTPClient @@ -1068,27 +1115,8 @@ func jwksPinnedHTTPClient(pinnedFingerprint string) *http.Client { }, Transport: &http.Transport{ TLSClientConfig: &tls.Config{ - MinVersion: tls.VersionTLS12, - VerifyPeerCertificate: func(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error { - if len(rawCerts) == 0 { - return fmt.Errorf("no peer certificate to pin") - } - cert, err := x509.ParseCertificate(rawCerts[0]) - if err != nil { - return fmt.Errorf("parse peer certificate: %w", err) - } - h := sha256.Sum256(cert.Raw) - got := hex.EncodeToString(h[:]) - if !strings.EqualFold(got, pinnedFingerprint) { - return fmt.Errorf("certificate fingerprint mismatch: got %s, want %s", got, pinnedFingerprint) - } - return nil - }, - // InsecureSkipVerify is true because VerifyPeerCertificate does - // 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, + MinVersion: tls.VersionTLS12, + VerifyConnection: pinVerifyConnection(pinnedFingerprint), }, }, } @@ -1168,7 +1196,8 @@ func fetchJWKSKeys(jwksURL string) ([]JwksKey, error) { } // FetchJWKSKeysWithPinning fetches JWKS keys with TLS certificate pinning. -// The pinnedFingerprint is a hex-encoded SHA-256 of the DER certificate. +// The pinnedFingerprint is a hex-encoded SHA-256 of the leaf certificate's +// SPKI (preferred) or full DER certificate; an empty value disables pinning. func FetchJWKSKeysWithPinning(jwksURL, pinnedFingerprint string) ([]JwksKey, error) { client := jwksPinnedHTTPClient(pinnedFingerprint) return fetchJWKSKeysWithClient(jwksURL, client) diff --git a/identity/zz_jwks_pinning_test.go b/identity/zz_jwks_pinning_test.go new file mode 100644 index 0000000..58216e2 --- /dev/null +++ b/identity/zz_jwks_pinning_test.go @@ -0,0 +1,106 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +package identity + +import ( + "crypto/sha256" + "crypto/tls" + "crypto/x509" + "encoding/hex" + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +const testJWKSBody = `{"keys":[{"kty":"oct","kid":"k1","k":"c2VjcmV0"}]}` + +func newTLSJWKSServer(t *testing.T) *httptest.Server { + t.Helper() + srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(testJWKSBody)) + })) + t.Cleanup(srv.Close) + return srv +} + +func leafCert(t *testing.T, srv *httptest.Server) *x509.Certificate { + t.Helper() + if srv.TLS == nil || len(srv.TLS.Certificates) == 0 || len(srv.TLS.Certificates[0].Certificate) == 0 { + t.Fatal("test server has no TLS leaf certificate") + } + cert, err := x509.ParseCertificate(srv.TLS.Certificates[0].Certificate[0]) + if err != nil { + t.Fatalf("parse leaf cert: %v", err) + } + return cert +} + +// trustServer returns a fetch helper that trusts the test server's self-signed +// CA, so that the test exercises the PIN check on top of a passing chain +// verification rather than failing on the chain itself. +func fetchWithPin(t *testing.T, srv *httptest.Server, pin string) ([]JwksKey, error) { + t.Helper() + client := jwksPinnedHTTPClient(pin) + tr := client.Transport.(*http.Transport) + pool := x509.NewCertPool() + pool.AddCert(srv.Certificate()) + tr.TLSClientConfig.RootCAs = pool + return fetchJWKSKeysWithClient(srv.URL, client) +} + +func TestJWKSPinning_SPKIMatchAccepts(t *testing.T) { + srv := newTLSJWKSServer(t) + pin := spkiFingerprint(leafCert(t, srv)) + + keys, err := fetchWithPin(t, srv, pin) + if err != nil { + t.Fatalf("expected fetch to succeed with matching SPKI pin, got: %v", err) + } + if len(keys) != 1 || keys[0].Kid != "k1" { + t.Fatalf("unexpected keys: %+v", keys) + } +} + +func TestJWKSPinning_CertFingerprintMatchAccepts(t *testing.T) { + srv := newTLSJWKSServer(t) + cert := leafCert(t, srv) + sum := sha256.Sum256(cert.Raw) + pin := strings.ToUpper(hex.EncodeToString(sum[:])) // also checks case-insensitivity + + keys, err := fetchWithPin(t, srv, pin) + if err != nil { + t.Fatalf("expected fetch to succeed with matching cert pin, got: %v", err) + } + if len(keys) != 1 { + t.Fatalf("unexpected keys: %+v", keys) + } +} + +func TestJWKSPinning_MismatchRejected(t *testing.T) { + srv := newTLSJWKSServer(t) + wrongPin := strings.Repeat("ab", 32) // 64 hex chars, not the server's cert + + _, err := fetchWithPin(t, srv, wrongPin) + if err == nil { + t.Fatal("expected fetch to fail when pin does not match presented cert") + } + if !strings.Contains(err.Error(), "fetch JWKS") { + t.Fatalf("expected transport-level failure, got: %v", err) + } +} + +func TestJWKSPinning_EmptyPinUsesSharedClient(t *testing.T) { + if jwksPinnedHTTPClient("") != jwksHTTPClient { + t.Fatal("empty pin should return the shared non-pinning client") + } +} + +func TestPinVerifyConnection_NoPeerCert(t *testing.T) { + // VerifyConnection with no peer certificates must fail closed. + err := pinVerifyConnection(strings.Repeat("00", 32))(tls.ConnectionState{}) + if err == nil { + t.Fatal("expected error when no peer certificate is presented") + } +}