From 8c3ab55fd2a78b6e2a76b4230d2a6cc9221b33df Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 11 Jun 2026 01:02:17 -0700 Subject: [PATCH 1/3] fix(scheduledauth): use hardened HTTP client for JWKS warmup and fetch The scheduled-task OIDC validator issued its JWKS warmup probe via http.DefaultClient and let go-oidc's RemoteKeySet fetch keys over the default transport. The JWKS URL is operator-supplied (SCHEDULED_TASK_OIDC_JWKS_URL), so a misconfigured or compromised value could reach internal/metadata endpoints, bypassing the IMDS-blocking hardening the Azure provider clients already use. Move the hardened client (IMDS blocking, dial/TLS/overall timeouts) from providers/azure/internal/httpclient to the shared pkg/httpclient package, since the Azure-internal package is unimportable across the module boundary; the Azure package now delegates to it so there is a single implementation. The validator constructs this client once in configureOIDC, uses it for the warmup probe, and supplies it to go-oidc via oidc.ClientContext so key fetches and rotation refreshes ride the same hardened transport. Regression tests guard all paths: the constructed client must not be http.DefaultClient and must carry a timeout and dedicated transport; Warmup and token-verification key fetches are run with a guarded http.DefaultClient that fails the test if used (both fail pre-fix); pkg/httpclient gets direct IMDS-blocking coverage (IPv4 + IPv6). Closes #1145 --- internal/server/scheduledauth/validator.go | 26 ++++-- .../server/scheduledauth/validator_test.go | 91 +++++++++++++++++++ pkg/httpclient/httpclient.go | 68 ++++++++++++++ pkg/httpclient/httpclient_test.go | 64 +++++++++++++ .../azure/internal/httpclient/httpclient.go | 48 ++-------- 5 files changed, 249 insertions(+), 48 deletions(-) create mode 100644 pkg/httpclient/httpclient.go create mode 100644 pkg/httpclient/httpclient_test.go diff --git a/internal/server/scheduledauth/validator.go b/internal/server/scheduledauth/validator.go index 0e036f9c3..e5308a2a1 100644 --- a/internal/server/scheduledauth/validator.go +++ b/internal/server/scheduledauth/validator.go @@ -15,6 +15,8 @@ import ( "time" "github.com/coreos/go-oidc/v3/oidc" + + "github.com/LeanerCloud/CUDly/pkg/httpclient" ) // Mode is the authentication mode used for scheduled-task requests. @@ -66,8 +68,10 @@ type Validator struct { verifier *oidc.IDTokenVerifier // nil unless mode == ModeOIDC; backed by go-oidc's single-flight RemoteKeySet verMu sync.RWMutex // guards verifier and lastRebuild during rotation-recovery rebuilds lastRebuild time.Time // last verifier rebuild; rate-limits attacker-driven JWKS refetches + keySet *oidc.RemoteKeySet // nil unless mode == ModeOIDC; powered by go-oidc's single-flight cache jwksURL string // remembered for Warmup and rotation rebuild; empty unless mode == ModeOIDC issuer string // OIDC issuer URL; empty unless mode == ModeOIDC; needed to rebuild verifier + httpClient *http.Client // hardened client for all JWKS fetches; nil unless mode == ModeOIDC audiences map[string]struct{} subjects map[string]struct{} skew time.Duration @@ -167,8 +171,15 @@ func configureOIDC(v *Validator, cfg Config) (*Validator, error) { v.subjects = subs v.jwksURL = cfg.JWKSURL v.issuer = cfg.Issuer - keySet := oidc.NewRemoteKeySet(context.Background(), cfg.JWKSURL) - v.verifier = oidc.NewVerifier(cfg.Issuer, keySet, &oidc.Config{ + // All JWKS traffic (warmup probe and go-oidc key fetches) goes + // through the hardened shared client: IMDS/metadata endpoints are + // blocked and dial/TLS/overall timeouts apply. The JWKS URL is + // operator-supplied (SCHEDULED_TASK_OIDC_JWKS_URL), so a + // misconfigured or compromised value must not be able to reach + // internal/metadata endpoints via the default transport. + v.httpClient = httpclient.New() + v.keySet = oidc.NewRemoteKeySet(oidc.ClientContext(context.Background(), v.httpClient), cfg.JWKSURL) + v.verifier = oidc.NewVerifier(cfg.Issuer, v.keySet, &oidc.Config{ // Pin to RS256. Google's tokens are RS256; rejecting anything // else closes the alg=none / alg=HS256 confusion family. SupportedSigningAlgs: []string{string(oidc.RS256)}, @@ -220,9 +231,10 @@ func (v *Validator) Mode() Mode { } // warmupTimeout is the fallback deadline for the JWKS warmup probe -// when the caller passes a context without one. http.DefaultClient -// has no timeout, so without this guard a misconfigured / unreachable -// JWKS endpoint would block startup indefinitely. +// when the caller passes a context without one. The hardened client +// has its own overall timeout, but startup should fail the probe much +// faster than that when the JWKS endpoint is misconfigured or +// unreachable. const warmupTimeout = 5 * time.Second // Warmup performs a best-effort sanity check on the JWKS endpoint at @@ -254,7 +266,7 @@ func (v *Validator) Warmup(ctx context.Context) { log.Printf("scheduledauth: WARN — JWKS warmup request build failed: %v", err) return } - resp, err := http.DefaultClient.Do(req) //nolint:gosec // G704: unsafe operation intentional + resp, err := v.httpClient.Do(req) if err != nil { log.Printf("scheduledauth: WARN — JWKS warmup fetch failed for %s: %v "+ "(validator will retry on first request)", v.jwksURL, err) @@ -438,7 +450,7 @@ func (v *Validator) verifyWithRotationRetry(ctx context.Context, rawToken string return nil, err } log.Printf("scheduledauth: oidc signature verification failed; rebuilding key set for rotation retry") - newKS := oidc.NewRemoteKeySet(context.Background(), v.jwksURL) + newKS := oidc.NewRemoteKeySet(oidc.ClientContext(context.Background(), v.httpClient), v.jwksURL) v.verifier = oidc.NewVerifier(v.issuer, newKS, &oidc.Config{ SupportedSigningAlgs: []string{string(oidc.RS256)}, SkipClientIDCheck: true, diff --git a/internal/server/scheduledauth/validator_test.go b/internal/server/scheduledauth/validator_test.go index 31f126351..da3cc623d 100644 --- a/internal/server/scheduledauth/validator_test.go +++ b/internal/server/scheduledauth/validator_test.go @@ -1043,6 +1043,97 @@ func TestWarmup_HitsJWKSEndpoint(t *testing.T) { } } +// defaultClientGuard is a RoundTripper installed into http.DefaultClient +// to prove JWKS traffic does NOT flow through the default client +// (SEC-04 regression guard: warmup and key fetches must use the +// hardened client). Any request through it is counted and rejected. +type defaultClientGuard struct { + calls atomic.Int64 +} + +func (g *defaultClientGuard) RoundTrip(*http.Request) (*http.Response, error) { + g.calls.Add(1) + return nil, errors.New("http.DefaultClient must not be used for JWKS traffic") +} + +// swapDefaultClient replaces http.DefaultClient with a guarded client +// for the duration of the test. No tests in this package use +// t.Parallel() (same precedent as the global log-writer swap in +// TestWarmup_LoggedAndNonFatal_OnDeadEndpoint), so mutating the global +// is safe; t.Cleanup restores it. +func swapDefaultClient(t *testing.T) *defaultClientGuard { + t.Helper() + guard := &defaultClientGuard{} + orig := http.DefaultClient + http.DefaultClient = &http.Client{Transport: guard} + t.Cleanup(func() { http.DefaultClient = orig }) + return guard +} + +// SEC-04 regression: the OIDC validator must construct its own hardened +// HTTP client rather than relying on http.DefaultClient / the default +// transport for JWKS traffic. +func TestNew_OIDC_UsesHardenedHTTPClient(t *testing.T) { + v := newOIDCValidator(t, "http://127.0.0.1:1/jwks") + + if v.httpClient == nil { + t.Fatalf("oidc validator must carry a hardened HTTP client") + } + if v.httpClient == http.DefaultClient { + t.Fatalf("oidc validator must not use http.DefaultClient") + } + if v.httpClient.Timeout == 0 { + t.Fatalf("hardened client must have an overall timeout") + } + if v.httpClient.Transport == nil || v.httpClient.Transport == http.DefaultTransport { + t.Fatalf("hardened client must not ride the default transport") + } +} + +// SEC-04 regression: Warmup must probe the JWKS endpoint through the +// hardened client, never http.DefaultClient. Pre-fix this fails: the +// guard intercepts the probe and the JWKS server is never reached. +func TestWarmup_BypassesDefaultClient(t *testing.T) { + key := newTestKey(t, "kid-1") + srv := newJWKSServer(t, jwks(key)) + v := newOIDCValidator(t, srv.URL) + guard := swapDefaultClient(t) + + before := srv.hits.Load() + v.Warmup(context.Background()) + + if got := guard.calls.Load(); got != 0 { + t.Fatalf("Warmup made %d request(s) through http.DefaultClient; want 0", got) + } + if after := srv.hits.Load(); after <= before { + t.Fatalf("Warmup never reached the JWKS endpoint (before=%d after=%d)", before, after) + } +} + +// SEC-04 regression: go-oidc's RemoteKeySet fetch (triggered by token +// verification) must also go through the hardened client supplied via +// oidc.ClientContext. Pre-fix this fails: go-oidc falls back to +// http.DefaultClient, the guard rejects the fetch, and validation +// errors out. +func TestValidate_OIDC_KeyFetchBypassesDefaultClient(t *testing.T) { + key := newTestKey(t, "kid-1") + srv := newJWKSServer(t, jwks(key)) + v := newOIDCValidator(t, srv.URL) + guard := swapDefaultClient(t) + + tok := signToken(t, key, baseClaims(time.Now(), + testSchedulerSubject, + "https://api.example.com", + "https://accounts.example.com")) + + if err := v.Validate(context.Background(), "Bearer "+tok); err != nil { + t.Fatalf("expected valid token, got: %v", err) + } + if got := guard.calls.Load(); got != 0 { + t.Fatalf("key fetch made %d request(s) through http.DefaultClient; want 0", got) + } +} + func TestValidateJWKSBody_RequiresKeysArray(t *testing.T) { tests := []struct { name string diff --git a/pkg/httpclient/httpclient.go b/pkg/httpclient/httpclient.go new file mode 100644 index 000000000..b428bbfc4 --- /dev/null +++ b/pkg/httpclient/httpclient.go @@ -0,0 +1,68 @@ +// Package httpclient provides a hardened HTTP client for outbound +// requests. It blocks connections to the cloud Instance Metadata +// Service (IMDS) endpoints to prevent SSRF attacks that could leak +// cloud credentials, and applies sane dial/TLS/overall timeouts so a +// misbehaving endpoint cannot hang a caller indefinitely. +// +// This is the single shared implementation; the Azure provider's +// internal httpclient package delegates here so every module uses the +// same hardening. +package httpclient + +import ( + "context" + "fmt" + "net" + "net/http" + "time" +) + +// Timeouts applied by New. Exported indirectly via the constructed +// client; named here so the values are not magic numbers. +const ( + dialTimeout = 10 * time.Second + keepAliveInterval = 30 * time.Second + tlsHandshakeTimeout = 10 * time.Second + requestTimeout = 30 * time.Second +) + +// imdsAddresses are the well-known metadata service addresses that must never +// be reachable from application-level HTTP clients. +var imdsAddresses = map[string]bool{ + "169.254.169.254": true, // AWS/Azure/GCP link-local IMDS (IPv4) + "fd00:ec2::254": true, // AWS IMDS (IPv6) +} + +// blockIMDSDialer wraps net.Dialer and rejects connections to IMDS addresses. +type blockIMDSDialer struct { + inner net.Dialer +} + +func (d *blockIMDSDialer) DialContext(ctx context.Context, network, addr string) (net.Conn, error) { + host, _, err := net.SplitHostPort(addr) + if err != nil { + host = addr + } + if imdsAddresses[host] { + return nil, fmt.Errorf("connection to metadata endpoint %s is blocked", host) + } + return d.inner.DialContext(ctx, network, addr) +} + +// New returns an *http.Client with a 30-second timeout and IMDS blocking. +func New() *http.Client { + dialer := &blockIMDSDialer{ + inner: net.Dialer{ + Timeout: dialTimeout, + KeepAlive: keepAliveInterval, + }, + } + transport := &http.Transport{ + DialContext: dialer.DialContext, + TLSHandshakeTimeout: tlsHandshakeTimeout, + } + return &http.Client{ + Timeout: requestTimeout, + Transport: transport, + } +} diff --git a/pkg/httpclient/httpclient_test.go b/pkg/httpclient/httpclient_test.go new file mode 100644 index 000000000..43ae3d89e --- /dev/null +++ b/pkg/httpclient/httpclient_test.go @@ -0,0 +1,64 @@ +package httpclient + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +// New must hand back a dedicated hardened client, never the process-wide +// default client/transport (SEC-04 regression guard). +func TestNew_NotDefaultClient(t *testing.T) { + c := New() + + if c == http.DefaultClient { + t.Fatalf("New() must not return http.DefaultClient") + } + if c.Timeout == 0 { + t.Fatalf("New() must set an overall request timeout") + } + if c.Transport == nil || c.Transport == http.DefaultTransport { + t.Fatalf("New() must install a dedicated hardened transport") + } +} + +func TestNew_BlocksIMDS(t *testing.T) { + tests := []struct { + name string + url string + }{ + {name: "ipv4 link-local IMDS", url: "http://169.254.169.254/latest/meta-data/"}, + {name: "ipv6 AWS IMDS", url: "http://[fd00:ec2::254]/latest/meta-data/"}, + } + + c := New() + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + resp, err := c.Get(tt.url) + if err == nil { + resp.Body.Close() + t.Fatalf("request to %s must be blocked", tt.url) + } + if !strings.Contains(err.Error(), "blocked") { + t.Fatalf("expected IMDS-blocked error, got: %v", err) + } + }) + } +} + +func TestNew_AllowsRegularEndpoints(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + t.Cleanup(srv.Close) + + resp, err := New().Get(srv.URL) + if err != nil { + t.Fatalf("request to non-IMDS endpoint failed: %v", err) + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + t.Fatalf("status = %d, want %d", resp.StatusCode, http.StatusOK) + } +} diff --git a/providers/azure/internal/httpclient/httpclient.go b/providers/azure/internal/httpclient/httpclient.go index 3d33a60a7..dc51e1d37 100644 --- a/providers/azure/internal/httpclient/httpclient.go +++ b/providers/azure/internal/httpclient/httpclient.go @@ -1,53 +1,19 @@ // Package httpclient provides a hardened HTTP client for Azure provider use. // It blocks requests to the Instance Metadata Service (IMDS) endpoints to // prevent SSRF attacks that could leak cloud credentials. +// +// The implementation lives in the shared pkg module so the root module +// (which cannot import this internal package across the module boundary) +// uses the exact same hardening; this package only delegates. package httpclient import ( - "context" - "fmt" - "net" "net/http" - "time" -) - -// imdsAddresses are the well-known metadata service addresses that must never -// be reachable from application-level HTTP clients. -var imdsAddresses = map[string]bool{ - "169.254.169.254": true, // AWS/Azure/GCP link-local IMDS (IPv4) - "fd00:ec2::254": true, // AWS IMDS (IPv6) -} -// blockIMDSDialer wraps net.Dialer and rejects connections to IMDS addresses. -type blockIMDSDialer struct { - inner net.Dialer -} - -func (d *blockIMDSDialer) DialContext(ctx context.Context, network, addr string) (net.Conn, error) { - host, _, err := net.SplitHostPort(addr) - if err != nil { - host = addr - } - if imdsAddresses[host] { - return nil, fmt.Errorf("connection to metadata endpoint %s is blocked", host) - } - return d.inner.DialContext(ctx, network, addr) -} + "github.com/LeanerCloud/CUDly/pkg/httpclient" +) // New returns an *http.Client with a 30-second timeout and IMDS blocking. func New() *http.Client { - dialer := &blockIMDSDialer{ - inner: net.Dialer{ - Timeout: 10 * time.Second, - KeepAlive: 30 * time.Second, - }, - } - transport := &http.Transport{ - DialContext: dialer.DialContext, - TLSHandshakeTimeout: 10 * time.Second, - } - return &http.Client{ - Timeout: 30 * time.Second, - Transport: transport, - } + return httpclient.New() } From e47ba685bcfe625bd78c1768bb15f3ef32eaf1dc Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sat, 20 Jun 2026 00:08:10 +0200 Subject: [PATCH 2/3] fix(lint): use NewRequestWithContext in httpclient tests to satisfy noctx --- pkg/httpclient/httpclient_test.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/pkg/httpclient/httpclient_test.go b/pkg/httpclient/httpclient_test.go index 43ae3d89e..d08f01ab7 100644 --- a/pkg/httpclient/httpclient_test.go +++ b/pkg/httpclient/httpclient_test.go @@ -1,6 +1,7 @@ package httpclient import ( + "context" "net/http" "net/http/httptest" "strings" @@ -35,7 +36,11 @@ func TestNew_BlocksIMDS(t *testing.T) { c := New() for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - resp, err := c.Get(tt.url) + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, tt.url, nil) + if err != nil { + t.Fatalf("build request: %v", err) + } + resp, err := c.Do(req) if err == nil { resp.Body.Close() t.Fatalf("request to %s must be blocked", tt.url) @@ -53,7 +58,11 @@ func TestNew_AllowsRegularEndpoints(t *testing.T) { })) t.Cleanup(srv.Close) - resp, err := New().Get(srv.URL) + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, srv.URL, nil) + if err != nil { + t.Fatalf("build request: %v", err) + } + resp, err := New().Do(req) if err != nil { t.Fatalf("request to non-IMDS endpoint failed: %v", err) } From 772a66c531eec3975d437fcbd4e02b7205d70148 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 17 Jul 2026 19:45:55 +0300 Subject: [PATCH 3/3] fix(lint): suppress G704 SSRF finding on JWKS warmup httpClient.Do gosec's taint analysis flags v.httpClient.Do(req) because v.jwksURL comes from operator-supplied configuration (SCHEDULED_TASK_OIDC_JWKS_URL). The call is safe: jwksURL is operator config, not request-tainted user input, and v.httpClient is the hardened client from pkg/httpclient that blocks IMDS/link-local endpoints and enforces dial/TLS/overall timeouts. Add #nosec G704 with a full justification naming both guards. --- internal/server/scheduledauth/validator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/server/scheduledauth/validator.go b/internal/server/scheduledauth/validator.go index e5308a2a1..109af0ef0 100644 --- a/internal/server/scheduledauth/validator.go +++ b/internal/server/scheduledauth/validator.go @@ -266,7 +266,7 @@ func (v *Validator) Warmup(ctx context.Context) { log.Printf("scheduledauth: WARN — JWKS warmup request build failed: %v", err) return } - resp, err := v.httpClient.Do(req) + resp, err := v.httpClient.Do(req) // #nosec G704 -- jwksURL is operator-supplied config (SCHEDULED_TASK_OIDC_JWKS_URL), not request-tainted user input; v.httpClient is the hardened client from pkg/httpclient that blocks IMDS/link-local endpoints and enforces dial/TLS/overall timeouts if err != nil { log.Printf("scheduledauth: WARN — JWKS warmup fetch failed for %s: %v "+ "(validator will retry on first request)", v.jwksURL, err)