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

Author: cristim

internal/server/scheduledauth/validator.go

@@ -14,6 +14,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.
@@ -61,15 +63,16 @@ type Config struct {
 
 // Validator authenticates inbound requests against the configured mode.
 type Validator struct {
-	mode      Mode
-	verifier  *oidc.IDTokenVerifier // nil unless mode == ModeOIDC
-	keySet    *oidc.RemoteKeySet    // nil unless mode == ModeOIDC; powered by go-oidc's single-flight cache
-	jwksURL   string                // remembered for Warmup; empty unless mode == ModeOIDC
-	audiences map[string]struct{}
-	subjects  map[string]struct{}
-	skew      time.Duration
-	bearer    []byte
-	now       func() time.Time // pluggable for tests
+	mode       Mode
+	verifier   *oidc.IDTokenVerifier // nil unless mode == ModeOIDC
+	keySet     *oidc.RemoteKeySet    // nil unless mode == ModeOIDC; powered by go-oidc's single-flight cache
+	jwksURL    string                // remembered for Warmup; empty unless mode == ModeOIDC
+	httpClient *http.Client          // hardened client for all JWKS fetches; nil unless mode == ModeOIDC
+	audiences  map[string]struct{}
+	subjects   map[string]struct{}
+	skew       time.Duration
+	bearer     []byte
+	now        func() time.Time // pluggable for tests
 }
 
 // claims captures the timestamp claims we need beyond what go-oidc's
@@ -163,7 +166,14 @@ func configureOIDC(v *Validator, cfg Config) (*Validator, error) {
 	v.audiences = auds
 	v.subjects = subs
 	v.jwksURL = cfg.JWKSURL
-	v.keySet = oidc.NewRemoteKeySet(context.Background(), cfg.JWKSURL)
+	// 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.
@@ -216,9 +226,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
@@ -250,7 +261,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)
+	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)

internal/server/scheduledauth/validator_test.go

@@ -723,6 +723,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

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,
+	}
+}

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)
+	}
+}

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()
 }