Mitiate SSRF attack

Signed-off-by: Bryan Frimin <bryan@getprobo.com>
2026-04-20 12:10:56 +02:00
parent 5b8918bd5a
commit 472ca703b5
9 changed files with 83 additions and 8 deletions
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -8,6 +8,7 @@ Detailed guides for specific subsystems live in `contrib/claude/`:
 - [`contrib/claude/go-testing.md`](contrib/claude/go-testing.md) — Go test conventions (parallel, require vs assert, naming)
 - [`contrib/claude/go-service.md`](contrib/claude/go-service.md) — Go service orchestration (Run, graceful shutdown, crash propagation)
 - [`contrib/claude/go-worker.md`](contrib/claude/go-worker.md) — Go worker pattern (poll-based, bounded concurrency, FOR UPDATE SKIP LOCKED)
+- [`contrib/claude/httpclient.md`](contrib/claude/httpclient.md) — HTTP client (kit/httpclient, SSRF protection by default, connector wiring)
 - [`contrib/claude/gid.md`](contrib/claude/gid.md) — Global identifiers (GID layout, TenantID, entity type registry)
 - [`contrib/claude/coredata.md`](contrib/claude/coredata.md) — Data access layer (Scoper, SQL patterns, filters, order fields, migrations)
 - [`contrib/claude/logging.md`](contrib/claude/logging.md) — Structured logging (PII-free rules, field helpers, logger wiring)
--- a/contrib/claude/httpclient.md
+++ b/contrib/claude/httpclient.md
@@ -0,0 +1,42 @@
+# HTTP Client
+
+Use `go.gearno.de/kit/httpclient` for every outbound HTTP call. Never use `http.DefaultClient` or a bare `&http.Client{}`.
+
+## SSRF protection is the default
+
+Every call goes through `httpclient.DefaultClient(...)` / `httpclient.DefaultPooledClient(...)` / `httpclient.DefaultPooledTransport(...)` with `httpclient.WithSSRFProtection()` enabled. This applies whenever the destination URL is:
+
+- Customer-supplied (webhook endpoint, OAuth2 token URL, SCIM bridge URL, connector-provided base URL)
+- Reached through a customer-supplied connector (OAuth2/APIKey connection clients)
+- A hardcoded third-party provider host (Slack, Linear, GitHub, Google Workspace, Sentry, …) — defense in depth, and public IPs pass the check unchanged
+
+```go
+client := httpclient.DefaultPooledClient(
+    httpclient.WithLogger(logger),
+    httpclient.WithSSRFProtection(),
+)
+```
+
+What the option does:
+
+- Rejects dials to loopback, RFC 1918 private, RFC 6598 CGNAT, link-local, multicast, unspecified, ULA, IPv4-mapped IPv6, and IETF-reserved ranges. Check runs on the resolved peer IP at connect time, defeating DNS rebinding.
+- On `DefaultClient` / `DefaultPooledClient`, also refuses redirects whose scheme, host, or port differs from the original.
+
+## When to omit SSRF protection
+
+Only when the target is an **internal service you actively intend to reach** (sandbox-local service, sidecar, in-cluster endpoint with a known private IP). These cases are rare in this codebase — confirm the intent in code review. Do not disable it "just to make a test pass."
+
+For tests that need to hit an `httptest` server on loopback, add `httpclient.WithSSRFAllowLoopback()` on top of `WithSSRFProtection()` (or inject a loopback-friendly client into the component under test). Production callers must not use the loopback exemption.
+
+## Connector wiring
+
+`OAuth2Connector.HTTPClient` is a required field for the token-exchange request. Set it via `connector.ApplyProviderDefaults`, which wires an SSRF-protected client. Don't re-introduce a nil fallback in `CompleteWithState` — callers must provide the client explicitly.
+
+`OAuth2Connection.ClientWithOptions`, `RefreshableClient`, `clientCredentialsClient` and `APIKeyConnection.Client` already append `WithSSRFProtection()` internally; additional caller options layer on top.
+
+## Summary
+
+- Customer-reachable URL → `WithSSRFProtection()`, always.
+- Third-party SaaS → `WithSSRFProtection()`, always (public IPs pass).
+- Local/internal service you meant to dial → document why and omit.
+- Test against `httptest` → `WithSSRFProtection()` + `WithSSRFAllowLoopback()`.
--- a/go.mod
+++ b/go.mod
@@ -35,7 +35,7 @@ require (
 	github.com/vikstrous/dataloadgen v0.0.10
 	github.com/yuin/goldmark v1.4.13
 	go.gearno.de/crypto/uuid v0.1.1-0.20251208105319-3f587312a712
-	go.gearno.de/kit v0.5.0
+	go.gearno.de/kit v0.6.0
 	go.gearno.de/x/ref v0.0.0-20260216110753-a700c951377c
 	go.opentelemetry.io/otel v1.43.0
 	go.opentelemetry.io/otel/trace v1.43.0
--- a/go.sum
+++ b/go.sum
@@ -334,8 +334,8 @@ github.com/yuin/goldmark v1.4.13 h1:fVcFKWvrslecOb/tg+Cc05dkeYx540o0FuFt3nUVDoE=
 github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY=
 go.gearno.de/crypto/uuid v0.1.1-0.20251208105319-3f587312a712 h1:J5ccbcxFuwxe6Oa9fVi9FqQOo+n17ni4wbl9t4NuEzc=
 go.gearno.de/crypto/uuid v0.1.1-0.20251208105319-3f587312a712/go.mod h1:fnIIvKO9QnsyLO3ZJLJT3r8KZv/p0FOeT5eZKilYWXg=
-go.gearno.de/kit v0.5.0 h1:rtxlR3LPi7Cq0SZurOuv+OgTVZ2/PSTG6PVtlMAndoM=
-go.gearno.de/kit v0.5.0/go.mod h1:jWrI/mxd0F4GZApL0HgMextcEQoiy2YA1JVamSA/G0E=
+go.gearno.de/kit v0.6.0 h1:2O7Gdi8DCt6sWUKXDq+SJPHjNnV7N+pYHpsBMhfFqmU=
+go.gearno.de/kit v0.6.0/go.mod h1:jWrI/mxd0F4GZApL0HgMextcEQoiy2YA1JVamSA/G0E=
 go.gearno.de/x/panicf v0.1.1 h1:E3Cr9NB8Ry2EsvEG/1eHr7kplP3tEjTf5d56dTX64VQ=
 go.gearno.de/x/panicf v0.1.1/go.mod h1:VnB8oF0UefMZcYeD4v+Wk4U5Z1uza7PHLlhT2CbNEbU=
 go.gearno.de/x/ref v0.0.0-20260216110753-a700c951377c h1:rIVWwnNxHYu9aZhHkptXlNYTBJbY4ccaIAYjztVeaDc=
--- a/pkg/connector/apikey.go
+++ b/pkg/connector/apikey.go
@@ -40,7 +40,7 @@ func (c *APIKeyConnection) Client(ctx context.Context) (*http.Client, error) {
 	transport := &oauth2Transport{
 		token:      c.APIKey,
 		tokenType:  "Bearer",
-		underlying: httpclient.DefaultPooledTransport(),
+		underlying: httpclient.DefaultPooledTransport(httpclient.WithSSRFProtection()),
 	}
 	return &http.Client{Transport: transport}, nil
 }
--- a/pkg/connector/oauth2.go
+++ b/pkg/connector/oauth2.go
@@ -49,6 +49,12 @@ type (
 		ExtraAuthParams         map[string]string // Optional: extra params for auth URL (e.g., access_type=offline for Google)
 		TokenEndpointAuth       string            // "post-form" (default), "basic-form", or "basic-json"
 		SupportsIncrementalAuth bool
+
+		// HTTPClient is used for the OAuth2 token-exchange request
+		// issued from CompleteWithState. It must be set by callers;
+		// ApplyProviderDefaults assigns an SSRF-protected client for
+		// production use. Tests may inject a loopback-friendly one.
+		HTTPClient *http.Client
 	}

 	OAuth2State struct {
@@ -208,7 +214,7 @@ func (c *OAuth2Connector) CompleteWithState(ctx context.Context, r *http.Request
 		return nil, nil, err
 	}

-	tokenResp, err := http.DefaultClient.Do(tokenRequest)
+	tokenResp, err := c.HTTPClient.Do(tokenRequest)
 	if err != nil {
 		return nil, nil, fmt.Errorf("cannot post token URL: %w", err)
 	}
@@ -363,7 +369,14 @@ func (c *OAuth2Connection) Client(ctx context.Context) (*http.Client, error) {

 // ClientWithOptions returns an HTTP client with the given options.
 // Use this to add logging and tracing to the HTTP client.
+//
+// SSRF protection is always enabled: the underlying connector URL
+// (for example a 1Password SCIM bridge URL) is customer-supplied,
+// so dials to private, loopback, or other reserved address ranges
+// are refused. Hardcoded provider hosts on public IPs are
+// unaffected.
 func (c *OAuth2Connection) ClientWithOptions(ctx context.Context, opts ...httpclient.Option) (*http.Client, error) {
+	opts = append(opts, httpclient.WithSSRFProtection())
 	transport := &oauth2Transport{
 		token:      c.AccessToken,
 		tokenType:  c.TokenType,
@@ -389,6 +402,11 @@ func (c *OAuth2Connection) RefreshableClient(ctx context.Context, cfg OAuth2Refr
 		return c.ClientWithOptions(ctx, opts...)
 	}

+	// All HTTP traffic on this path (token refresh + API calls)
+	// must reject private/loopback/reserved peer IPs because the
+	// configured TokenURL or API host can be customer-influenced.
+	opts = append(opts, httpclient.WithSSRFProtection())
+
 	// Determine auth style based on TokenEndpointAuth
 	authStyle := oauth2.AuthStyleInParams
 	switch cfg.TokenEndpointAuth {
@@ -462,6 +480,10 @@ func (c *OAuth2Connection) clientCredentialsClient(ctx context.Context, opts ...
 		return c.ClientWithOptions(ctx, opts...)
 	}

+	// TokenURL is stored from customer-supplied connector settings;
+	// reject dials to private/loopback/reserved peer IPs.
+	opts = append(opts, httpclient.WithSSRFProtection())
+
 	formData := url.Values{}
 	formData.Set("grant_type", "client_credentials")
 	if c.Scope != "" {
--- a/pkg/connector/oauth2_test.go
+++ b/pkg/connector/oauth2_test.go
@@ -27,6 +27,7 @@ import (

 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"go.gearno.de/kit/httpclient"
 	"go.probo.inc/probo/pkg/gid"
 	"go.probo.inc/probo/pkg/statelesstoken"
 )
@@ -228,7 +229,9 @@ func TestClientCredentialsClient(t *testing.T) {
 		TokenURL:     server.URL,
 	}

-	client, err := conn.clientCredentialsClient(context.Background())
+	// httptest binds to loopback, which the SSRF-protected default
+	// transport refuses; relax just for this test.
+	client, err := conn.clientCredentialsClient(context.Background(), httpclient.WithSSRFAllowLoopback())
 	require.NoError(t, err)
 	require.NotNil(t, client)

@@ -518,6 +521,9 @@ func TestCompleteWithState_ScopeFallback(t *testing.T) {
 		RedirectURI:  "https://example.com/cb",
 		AuthURL:      "https://provider.example.com/authorize",
 		TokenURL:     server.URL,
+		// httptest binds to loopback, which the SSRF-protected
+		// default client refuses; inject a permissive client.
+		HTTPClient: httpclient.DefaultClient(httpclient.WithSSRFProtection(), httpclient.WithSSRFAllowLoopback()),
 	}

 	orgID := gid.New(gid.NewTenantID(), 0)
--- a/pkg/connector/providers.go
+++ b/pkg/connector/providers.go
@@ -14,6 +14,8 @@

 package connector

+import "go.gearno.de/kit/httpclient"
+
 // CallbackPath is the HTTP path for the OAuth2 callback endpoint.
 const CallbackPath = "/api/console/v1/connectors/complete"

@@ -87,9 +89,11 @@ var (

 // ApplyProviderDefaults sets the redirect URI and applies static provider
 // defaults (auth URL, token URL, extra params, token endpoint auth) onto
-// an OAuth2Connector. Call this before registering the connector.
+// an OAuth2Connector, and wires an SSRF-protected HTTP client for the
+// token exchange request. Call this before registering the connector.
 func ApplyProviderDefaults(provider string, redirectURI string, c *OAuth2Connector) {
 	c.RedirectURI = redirectURI
+	c.HTTPClient = httpclient.DefaultClient(httpclient.WithSSRFProtection())

 	if def, ok := providerDefinitions[provider]; ok {
 		c.AuthURL = def.AuthURL
--- a/pkg/webhook/sender.go
+++ b/pkg/webhook/sender.go
@@ -88,7 +88,7 @@ func NewSender(pg *pg.Client, logger *log.Logger, cfg Config) *Sender {
 	return &Sender{
 		pg:             pg,
 		logger:         logger,
-		httpClient:     httpclient.DefaultPooledClient(httpclient.WithLogger(logger)),
+		httpClient:     httpclient.DefaultPooledClient(httpclient.WithLogger(logger), httpclient.WithSSRFProtection()),
 		encryptionKey:  cfg.EncryptionKey,
 		host:           cfg.Host,
 		cacheCreatedAt: time.Now(),