From 924576d3d4c5f05b3aabcb9365cb50a646d9eaf6 Mon Sep 17 00:00:00 2001 From: Julien Neuhart Date: Sat, 11 Apr 2026 13:05:05 +0200 Subject: [PATCH] fix(outboundURLs): better detaults --- pkg/gotenberg/outbound.go | 325 ++++++++++++++++++++++++ pkg/gotenberg/outbound_test.go | 308 ++++++++++++++++++++++ pkg/modules/api/context.go | 6 +- pkg/modules/chromium/browser.go | 5 +- pkg/modules/chromium/events.go | 11 +- pkg/modules/webhook/middleware.go | 13 +- test/integration/features/debug.feature | 12 +- test/integration/scenario/containers.go | 43 ++++ 8 files changed, 703 insertions(+), 20 deletions(-) create mode 100644 pkg/gotenberg/outbound.go create mode 100644 pkg/gotenberg/outbound_test.go diff --git a/pkg/gotenberg/outbound.go b/pkg/gotenberg/outbound.go new file mode 100644 index 00000000..7d9707e9 --- /dev/null +++ b/pkg/gotenberg/outbound.go @@ -0,0 +1,325 @@ +package gotenberg + +import ( + "context" + "errors" + "fmt" + "net" + "net/http" + "net/netip" + "net/url" + "strings" + "time" + + "github.com/dlclark/regexp2" +) + +// ErrNonPublicIP indicates that an outbound URL targets an IP address that +// is not reachable on the public internet. This covers loopback, RFC1918 +// private, link-local, unspecified, multicast, and IPv6 unique-local +// (fc00::/7) addresses, as well as their IPv4-mapped IPv6 wrappers (for +// example [::ffff:127.0.0.1]). +var ErrNonPublicIP = errors.New("non-public IP") + +// netipResolver is the subset of [net.Resolver] used by +// [ResolveAndCheckPublic]. Defining it as an interface allows tests to +// substitute a stub resolver. +type netipResolver interface { + LookupNetIP(ctx context.Context, network, host string) ([]netip.Addr, error) +} + +// outboundResolver is the resolver used by [ResolveAndCheckPublic]. It is a +// package-level variable so that tests can substitute a stub resolver. +var outboundResolver netipResolver = net.DefaultResolver + +// outboundDialer is the underlying dialer used by [secureDialContext]. It is +// a package-level variable so that tests can replace it. +var outboundDialer = &net.Dialer{ + Timeout: 30 * time.Second, + KeepAlive: 30 * time.Second, +} + +// IsPublicIP reports whether addr is reachable on the public internet. It +// returns false for loopback, private (RFC1918), link-local, unspecified, +// multicast, and unique-local addresses. IPv4-mapped IPv6 addresses are +// unmapped before evaluation so that [::ffff:127.0.0.1] is correctly +// identified as loopback. +func IsPublicIP(addr netip.Addr) bool { + if !addr.IsValid() { + return false + } + addr = addr.Unmap() + switch { + case addr.IsLoopback(), + addr.IsPrivate(), + addr.IsLinkLocalUnicast(), + addr.IsLinkLocalMulticast(), + addr.IsMulticast(), + addr.IsUnspecified(), + addr.IsInterfaceLocalMulticast(): + return false + } + return true +} + +// ResolveAndCheckPublic resolves host and returns the resolved addresses, +// or an error if any resolved address fails [IsPublicIP]. If host is itself +// an IP literal, it is checked directly without performing a DNS lookup. +// The returned slice can be used to pin a subsequent dial to a specific IP +// and prevent DNS rebinding between this validation and the connect. +func ResolveAndCheckPublic(ctx context.Context, host string) ([]netip.Addr, error) { + if host == "" { + return nil, errors.New("empty host") + } + if addr, err := netip.ParseAddr(host); err == nil { + if !IsPublicIP(addr) { + return nil, fmt.Errorf("%q: %w", addr, ErrNonPublicIP) + } + return []netip.Addr{addr}, nil + } + addrs, err := outboundResolver.LookupNetIP(ctx, "ip", host) + if err != nil { + return nil, fmt.Errorf("resolve %q: %w", host, err) + } + if len(addrs) == 0 { + return nil, fmt.Errorf("resolve %q: no addresses returned", host) + } + for _, a := range addrs { + if !IsPublicIP(a) { + return nil, fmt.Errorf("%q resolves to non-public address %q: %w", host, a, ErrNonPublicIP) + } + } + return addrs, nil +} + +// outboundDecision is the result of validating an outbound URL. It is +// stashed in the request context by [outboundRoundTripper] so that +// [secureDialContext] can either bypass the IP check (allow-list match) or +// pin the dial to the IPs that were resolved at validation time. +type outboundDecision struct { + // bypass is true when an allow-list pattern matched the URL. In that + // case the operator has explicitly opted into the destination and the + // dial should proceed without an IP check. + bypass bool + + // pinned holds the IPs resolved by [ResolveAndCheckPublic] for the URL + // host. The dial should be pinned to one of these to prevent DNS + // rebinding between validation and connect. + pinned []netip.Addr +} + +// outboundDecisionKey is the context key under which an [outboundDecision] +// is stored. +type outboundDecisionKey struct{} + +// httpLikeScheme reports whether scheme is one of http, https, ws, or wss. +// Only these schemes go through the IP-based public-address check; data, +// blob, file, and other schemes are filtered by the regex layer alone. +func httpLikeScheme(scheme string) bool { + switch scheme { + case "http", "https", "ws", "wss": + return true + } + return false +} + +// decideOutbound parses rawURL, runs the regex allow/deny lists against the +// normalized form, and (when no allow-list match) resolves the host and +// rejects any non-public address. It returns the resulting +// [outboundDecision] which the caller can stash in a context for the dial. +func decideOutbound(ctx context.Context, rawURL string, allowList, denyList []*regexp2.Regexp, deadline time.Time) (outboundDecision, error) { + parsed, err := url.Parse(rawURL) + if err != nil { + return outboundDecision{}, fmt.Errorf("parse URL %q: %w", rawURL, ErrFiltered) + } + parsed.Scheme = strings.ToLower(parsed.Scheme) + parsed.Host = strings.ToLower(parsed.Host) + normalized := parsed.String() + + allowMatched := false + if len(allowList) > 0 { + for _, pattern := range allowList { + clone := regexp2.MustCompile(pattern.String(), 0) + clone.MatchTimeout = time.Until(deadline) + + ok, err := clone.MatchString(normalized) + if err != nil { + if time.Now().After(deadline) { + return outboundDecision{}, context.DeadlineExceeded + } + return outboundDecision{}, fmt.Errorf("'%s' cannot handle '%s': %w", clone.String(), normalized, err) + } + + if ok { + allowMatched = true + break + } + } + + if !allowMatched { + return outboundDecision{}, fmt.Errorf("'%s' does not match any expression from the allowed list: %w", normalized, ErrFiltered) + } + } + + for _, pattern := range denyList { + clone := regexp2.MustCompile(pattern.String(), 0) + clone.MatchTimeout = time.Until(deadline) + + ok, err := clone.MatchString(normalized) + if err != nil { + if time.Now().After(deadline) { + return outboundDecision{}, context.DeadlineExceeded + } + return outboundDecision{}, fmt.Errorf("'%s' cannot handle '%s': %w", clone.String(), normalized, err) + } + + if ok { + return outboundDecision{}, fmt.Errorf("'%s' matches the expression from the denied list: %w", normalized, ErrFiltered) + } + } + + if allowMatched { + return outboundDecision{bypass: true}, nil + } + + if !httpLikeScheme(parsed.Scheme) { + return outboundDecision{}, nil + } + + host := parsed.Hostname() + if host == "" { + return outboundDecision{}, fmt.Errorf("URL %q has no host: %w", rawURL, ErrFiltered) + } + + addrs, err := ResolveAndCheckPublic(ctx, host) + if err != nil { + if errors.Is(err, ErrNonPublicIP) { + return outboundDecision{}, fmt.Errorf("'%s' targets a non-public address: %w", normalized, ErrFiltered) + } + return outboundDecision{}, fmt.Errorf("validate '%s' host: %w", normalized, err) + } + + return outboundDecision{pinned: addrs}, nil +} + +// FilterOutboundURL validates that rawURL is acceptable for an outbound +// request from Gotenberg. It is the URL-aware replacement for +// [FilterDeadline] and should be preferred for any new code that filters a +// URL before issuing or instructing an outbound request. +// +// The function: +// +// 1. Parses rawURL with [net/url] and lowercases the scheme and host. This +// prevents case-variant bypasses such as HTTP://127.0.0.1 from evading +// case-sensitive deny-list regexes. +// 2. Applies allowList and denyList against the normalized form using the +// same OR semantics as [FilterDeadline]. +// 3. When no allow-list entry explicitly matched and the scheme is one of +// http, https, ws, or wss, resolves the host and verifies every +// resolved address with [IsPublicIP]. This blocks loopback, private, +// link-local, and other internal targets even when the regex layer +// does not cover the textual form (for example IPv4-mapped IPv6 like +// [::ffff:127.0.0.1], or hostnames that resolve to a private address). +// +// An allow-list match bypasses the IP check, allowing operators to opt +// into specific internal destinations via --*-allow-list flags. The +// deny-list always applies and cannot be bypassed by an allow-list match. +func FilterOutboundURL(ctx context.Context, rawURL string, allowList, denyList []*regexp2.Regexp, deadline time.Time) error { + _, err := decideOutbound(ctx, rawURL, allowList, denyList, deadline) + return err +} + +// outboundRoundTripper is an [http.RoundTripper] that validates each request +// URL via [decideOutbound] and stashes the resulting [outboundDecision] in +// the request context so that [secureDialContext] can pin the dial or +// bypass the IP check as appropriate. Because the http.Client invokes +// RoundTrip again for each redirect hop, this also re-validates redirect +// targets without a separate CheckRedirect. +type outboundRoundTripper struct { + base http.RoundTripper + allowList []*regexp2.Regexp + denyList []*regexp2.Regexp +} + +// RoundTrip validates req.URL and delegates to the base transport. +func (rt *outboundRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { + deadline, ok := req.Context().Deadline() + if !ok { + deadline = time.Now().Add(30 * time.Second) + } + + decision, err := decideOutbound(req.Context(), req.URL.String(), rt.allowList, rt.denyList, deadline) + if err != nil { + return nil, err + } + + ctx := context.WithValue(req.Context(), outboundDecisionKey{}, decision) + return rt.base.RoundTrip(req.WithContext(ctx)) +} + +// NewOutboundHttpClient returns an [http.Client] that validates every +// outbound request URL via the same logic as [FilterOutboundURL] and pins +// the resulting dial to a resolved public IP. An allow-list match +// (operator opt-in to a specific destination) bypasses the IP check. +// +// The client re-validates redirect targets automatically because the +// underlying [http.Client] invokes the wrapping [http.RoundTripper] once +// per hop. This closes the redirect-based SSRF bypass that affects raw +// [http.Client] usage when no CheckRedirect is set. +func NewOutboundHttpClient(timeout time.Duration, allowList, denyList []*regexp2.Regexp) *http.Client { + base := http.DefaultTransport.(*http.Transport).Clone() + base.DialContext = secureDialContext + return &http.Client{ + Timeout: timeout, + Transport: &outboundRoundTripper{ + base: base, + allowList: allowList, + denyList: denyList, + }, + } +} + +// secureDialContext consumes the [outboundDecision] stashed in ctx by +// [outboundRoundTripper]. When the decision is to bypass (allow-list +// match), it dials directly. When the decision contains pinned IPs, it +// dials each in turn until one connects. When no decision is present (the +// dialer was used outside of [outboundRoundTripper]), it falls back to +// resolving and checking the destination itself. +func secureDialContext(ctx context.Context, network, addr string) (net.Conn, error) { + host, port, err := net.SplitHostPort(addr) + if err != nil { + return nil, fmt.Errorf("split host:port %q: %w", addr, err) + } + + if decision, ok := ctx.Value(outboundDecisionKey{}).(outboundDecision); ok { + if decision.bypass { + return outboundDialer.DialContext(ctx, network, addr) + } + if len(decision.pinned) > 0 { + return dialPinned(ctx, network, decision.pinned, port) + } + } + + addrs, err := ResolveAndCheckPublic(ctx, host) + if err != nil { + return nil, err + } + return dialPinned(ctx, network, addrs, port) +} + +// dialPinned dials each addr in turn until one connects, returning the +// first successful connection or the last error. +func dialPinned(ctx context.Context, network string, addrs []netip.Addr, port string) (net.Conn, error) { + var lastErr error + for _, a := range addrs { + conn, err := outboundDialer.DialContext(ctx, network, net.JoinHostPort(a.String(), port)) + if err == nil { + return conn, nil + } + lastErr = err + } + if lastErr == nil { + return nil, errors.New("no addresses to dial") + } + return nil, lastErr +} diff --git a/pkg/gotenberg/outbound_test.go b/pkg/gotenberg/outbound_test.go new file mode 100644 index 00000000..efc24849 --- /dev/null +++ b/pkg/gotenberg/outbound_test.go @@ -0,0 +1,308 @@ +package gotenberg + +import ( + "context" + "errors" + "net/netip" + "testing" + "time" + + "github.com/dlclark/regexp2" +) + +func TestIsPublicIP(t *testing.T) { + for _, tc := range []struct { + addr string + public bool + }{ + // Public. + {"1.1.1.1", true}, + {"8.8.8.8", true}, + {"2606:4700:4700::1111", true}, + + // Loopback. + {"127.0.0.1", false}, + {"127.255.255.254", false}, + {"::1", false}, + + // IPv4-mapped IPv6 (Issue 2). + {"::ffff:127.0.0.1", false}, + {"::ffff:10.0.0.1", false}, + {"::ffff:169.254.169.254", false}, + + // RFC1918. + {"10.0.0.1", false}, + {"172.16.0.1", false}, + {"172.31.255.254", false}, + {"192.168.1.1", false}, + + // Link-local. + {"169.254.169.254", false}, + {"fe80::1", false}, + + // Unique-local. + {"fc00::1", false}, + {"fd12:3456:789a::1", false}, + + // Unspecified. + {"0.0.0.0", false}, + {"::", false}, + + // Multicast. + {"224.0.0.1", false}, + {"ff02::1", false}, + } { + t.Run(tc.addr, func(t *testing.T) { + addr, err := netip.ParseAddr(tc.addr) + if err != nil { + t.Fatalf("parse %q: %v", tc.addr, err) + } + if got := IsPublicIP(addr); got != tc.public { + t.Fatalf("IsPublicIP(%q) = %v, want %v", tc.addr, got, tc.public) + } + }) + } +} + +// stubResolver lets tests fake DNS lookups in [ResolveAndCheckPublic]. +type stubResolver struct { + lookup func(host string) ([]netip.Addr, error) +} + +func (s stubResolver) LookupNetIP(_ context.Context, _, host string) ([]netip.Addr, error) { + return s.lookup(host) +} + +func withStubResolver(t *testing.T, fn func(host string) ([]netip.Addr, error)) { + t.Helper() + prev := outboundResolver + outboundResolver = stubResolver{lookup: fn} + t.Cleanup(func() { outboundResolver = prev }) +} + +func mustAddrs(t *testing.T, ss ...string) []netip.Addr { + t.Helper() + out := make([]netip.Addr, 0, len(ss)) + for _, s := range ss { + a, err := netip.ParseAddr(s) + if err != nil { + t.Fatalf("parse %q: %v", s, err) + } + out = append(out, a) + } + return out +} + +func TestFilterOutboundURL(t *testing.T) { + defaultDeny := []*regexp2.Regexp{ + regexp2.MustCompile(`^https?://(10\.|172\.(1[6-9]|2[0-9]|3[01])\.|192\.168\.|169\.254\.|0\.0\.0\.0|127\.|localhost|\[::1\]|\[fd)`, 0), + } + chromiumDeny := []*regexp2.Regexp{ + regexp2.MustCompile(`^file:(?!//\/tmp/).*`, 0), + } + + for _, tc := range []struct { + scenario string + rawURL string + allow []*regexp2.Regexp + deny []*regexp2.Regexp + stub func(host string) ([]netip.Addr, error) + expectErr bool + expectIs error + expectErrMsg string + }{ + { + scenario: "public IP literal passes", + rawURL: "https://1.1.1.1/", + deny: defaultDeny, + expectErr: false, + }, + { + scenario: "loopback IP literal blocked by default deny-list", + rawURL: "http://127.0.0.1:8080/", + deny: defaultDeny, + expectErr: true, + expectIs: ErrFiltered, + }, + { + scenario: "Issue 4: uppercase scheme normalized then blocked by deny-list", + rawURL: "HTTP://127.0.0.1:8080/", + deny: defaultDeny, + expectErr: true, + expectIs: ErrFiltered, + }, + { + scenario: "Issue 2: IPv4-mapped IPv6 evades deny-list but blocked by IP check", + rawURL: "http://[::ffff:127.0.0.1]:8080/page.pdf", + deny: defaultDeny, + expectErr: true, + expectIs: ErrFiltered, + }, + { + scenario: "Issue 2: IPv4-mapped IPv6 to RFC1918 blocked by IP check", + rawURL: "http://[::ffff:10.0.0.1]/", + deny: defaultDeny, + expectErr: true, + expectIs: ErrFiltered, + }, + { + scenario: "hostname resolving to public IP passes", + rawURL: "https://example.com/", + deny: defaultDeny, + stub: func(string) ([]netip.Addr, error) { return mustAddrs(t, "93.184.216.34"), nil }, + expectErr: false, + }, + { + scenario: "hostname resolving to loopback blocked", + rawURL: "https://rebind.example/", + deny: defaultDeny, + stub: func(string) ([]netip.Addr, error) { return mustAddrs(t, "127.0.0.1"), nil }, + expectErr: true, + expectIs: ErrFiltered, + }, + { + scenario: "hostname resolving to mixed public+private blocked", + rawURL: "https://mixed.example/", + deny: defaultDeny, + stub: func(string) ([]netip.Addr, error) { return mustAddrs(t, "1.1.1.1", "10.0.0.1"), nil }, + expectErr: true, + expectIs: ErrFiltered, + }, + { + scenario: "allow-list match bypasses IP check", + rawURL: "http://internal.service/api", + allow: []*regexp2.Regexp{regexp2.MustCompile(`^http://internal\.service`, 0)}, + deny: defaultDeny, + stub: func(string) ([]netip.Addr, error) { return mustAddrs(t, "10.0.0.1"), nil }, + expectErr: false, + }, + { + scenario: "deny-list still wins over allow-list match", + rawURL: "http://internal.service/api", + allow: []*regexp2.Regexp{regexp2.MustCompile(`^http://internal`, 0)}, + deny: []*regexp2.Regexp{regexp2.MustCompile(`/api$`, 0)}, + expectErr: true, + expectIs: ErrFiltered, + }, + { + scenario: "allow-list non-empty and no match rejects", + rawURL: "https://other.example/", + allow: []*regexp2.Regexp{regexp2.MustCompile(`^https://allowed\.example`, 0)}, + expectErr: true, + expectIs: ErrFiltered, + }, + { + scenario: "file:// allowed under tmp passes Chromium default", + rawURL: "file:///tmp/index.html", + deny: chromiumDeny, + expectErr: false, + }, + { + scenario: "file:// outside tmp blocked by Chromium default", + rawURL: "file:///etc/passwd", + deny: chromiumDeny, + expectErr: true, + expectIs: ErrFiltered, + }, + { + scenario: "Issue 1: Chromium default does not block http to public host (regex layer)", + rawURL: "https://example.com/", + deny: chromiumDeny, + stub: func(string) ([]netip.Addr, error) { return mustAddrs(t, "93.184.216.34"), nil }, + expectErr: false, + }, + { + scenario: "Issue 1: Chromium default now blocks http to loopback via IP layer", + rawURL: "http://127.0.0.1:3000/health", + deny: chromiumDeny, + expectErr: true, + expectIs: ErrFiltered, + }, + { + scenario: "Issue 1: Chromium default now blocks cloud metadata via IP layer", + rawURL: "http://169.254.169.254/latest/meta-data/", + deny: chromiumDeny, + expectErr: true, + expectIs: ErrFiltered, + }, + { + scenario: "data: URL passes (non-network scheme)", + rawURL: "data:text/html;base64,PGgxPmhpPC9oMT4=", + expectErr: false, + }, + { + scenario: "URL with no host rejected", + rawURL: "http:///path", + expectErr: true, + expectIs: ErrFiltered, + }, + { + scenario: "userinfo cannot mask host", + rawURL: "http://example.com@127.0.0.1/", + deny: defaultDeny, + expectErr: true, + expectIs: ErrFiltered, + }, + } { + t.Run(tc.scenario, func(t *testing.T) { + if tc.stub != nil { + withStubResolver(t, tc.stub) + } else { + // Default: any DNS lookup in a non-stubbed test is a bug. + withStubResolver(t, func(host string) ([]netip.Addr, error) { + t.Fatalf("unexpected DNS lookup for %q", host) + return nil, nil + }) + } + + err := FilterOutboundURL(context.Background(), tc.rawURL, tc.allow, tc.deny, time.Now().Add(5*time.Second)) + + if tc.expectErr && err == nil { + t.Fatalf("expected error, got nil") + } + if !tc.expectErr && err != nil { + t.Fatalf("expected no error, got: %v", err) + } + if tc.expectIs != nil && !errors.Is(err, tc.expectIs) { + t.Fatalf("expected error to wrap %v, got: %v", tc.expectIs, err) + } + }) + } +} + +func TestResolveAndCheckPublic_IPLiteralLoopback(t *testing.T) { + withStubResolver(t, func(host string) ([]netip.Addr, error) { + t.Fatalf("unexpected DNS lookup for %q", host) + return nil, nil + }) + + _, err := ResolveAndCheckPublic(context.Background(), "127.0.0.1") + if !errors.Is(err, ErrNonPublicIP) { + t.Fatalf("expected ErrNonPublicIP, got: %v", err) + } +} + +func TestResolveAndCheckPublic_HostResolvesToLoopback(t *testing.T) { + withStubResolver(t, func(host string) ([]netip.Addr, error) { + return mustAddrs(t, "127.0.0.1"), nil + }) + + _, err := ResolveAndCheckPublic(context.Background(), "rebind.example") + if !errors.Is(err, ErrNonPublicIP) { + t.Fatalf("expected ErrNonPublicIP, got: %v", err) + } +} + +func TestResolveAndCheckPublic_HostResolvesToPublic(t *testing.T) { + withStubResolver(t, func(host string) ([]netip.Addr, error) { + return mustAddrs(t, "1.1.1.1"), nil + }) + + addrs, err := ResolveAndCheckPublic(context.Background(), "example.com") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(addrs) != 1 || addrs[0].String() != "1.1.1.1" { + t.Fatalf("expected [1.1.1.1], got: %v", addrs) + } +} diff --git a/pkg/modules/api/context.go b/pkg/modules/api/context.go index a4c76925..802923dd 100644 --- a/pkg/modules/api/context.go +++ b/pkg/modules/api/context.go @@ -232,7 +232,7 @@ func newContext(echoCtx echo.Context, logger *slog.Logger, fs *gotenberg.FileSys ) } - err := gotenberg.FilterDeadline(downloadFromCfg.allowList, downloadFromCfg.denyList, dl.Url, deadline) + err := gotenberg.FilterOutboundURL(ctx, dl.Url, downloadFromCfg.allowList, downloadFromCfg.denyList, deadline) if err != nil { return fmt.Errorf("filter URL: %w", err) } @@ -268,9 +268,7 @@ func newContext(echoCtx echo.Context, logger *slog.Logger, fs *gotenberg.FileSys } client := &retryablehttp.Client{ - HTTPClient: &http.Client{ - Timeout: time.Until(deadline), - }, + HTTPClient: gotenberg.NewOutboundHttpClient(time.Until(deadline), downloadFromCfg.allowList, downloadFromCfg.denyList), RetryMax: downloadFromCfg.maxRetry, RetryWaitMin: time.Duration(1) * time.Second, RetryWaitMax: time.Until(deadline), diff --git a/pkg/modules/chromium/browser.go b/pkg/modules/chromium/browser.go index cc758180..93d46ac0 100644 --- a/pkg/modules/chromium/browser.go +++ b/pkg/modules/chromium/browser.go @@ -336,8 +336,9 @@ func (b *chromiumBrowser) do(ctx context.Context, logger *slog.Logger, url strin return errors.New("context has no deadline") } - // We validate the "main" URL against our allowed / deny lists. - err := gotenberg.FilterDeadline(b.arguments.allowList, b.arguments.denyList, url, deadline) + // We validate the "main" URL against our allowed / deny lists, and + // against the IP-based outbound URL guard. See [gotenberg.FilterOutboundURL]. + err := gotenberg.FilterOutboundURL(ctx, url, b.arguments.allowList, b.arguments.denyList, deadline) if err != nil { return fmt.Errorf("filter URL: %w", err) } diff --git a/pkg/modules/chromium/events.go b/pkg/modules/chromium/events.go index fdac71bb..356e5af7 100644 --- a/pkg/modules/chromium/events.go +++ b/pkg/modules/chromium/events.go @@ -52,7 +52,7 @@ func listenForEventRequestPaused(ctx context.Context, logger *slog.Logger, optio return } - err := gotenberg.FilterDeadline(options.allowList, options.denyList, e.Request.URL, deadline) + err := gotenberg.FilterOutboundURL(ctx, e.Request.URL, options.allowList, options.denyList, deadline) if err != nil { logger.WarnContext(ctx, err.Error()) allow = false @@ -81,6 +81,15 @@ func listenForEventRequestPaused(ctx context.Context, logger *slog.Logger, optio executorCtx := cdp.WithExecutor(ctx, cctx.Target) if !allow { + // Use AccessDenied so Chromium emits net::ERR_ACCESS_DENIED, + // which is intentionally absent from the EventLoadingFailed + // known-errors list. Routing through BlockedByClient would + // surface the failure, but the Document-type dispatcher in + // listenForEventLoadingFailed cannot distinguish a blocked + // iframe (sub-frame Document) from a main-page Document, and + // would attribute the iframe failure to the main page. + // Filter-block observability is provided by the warn log + // above instead. req := fetch.FailRequest(e.RequestID, network.ErrorReasonAccessDenied) err = req.Do(executorCtx) if err != nil { diff --git a/pkg/modules/webhook/middleware.go b/pkg/modules/webhook/middleware.go index c6cd7b5c..c305a81f 100644 --- a/pkg/modules/webhook/middleware.go +++ b/pkg/modules/webhook/middleware.go @@ -127,14 +127,15 @@ func webhookMiddleware(w *Webhook) api.Middleware { } // Let's check if the webhook URLs are acceptable according to our - // allowed/denied lists. - err := gotenberg.FilterDeadline(w.allowList, w.denyList, webhookUrl, deadline) + // allowed/denied lists, and against the IP-based outbound URL + // guard. See [gotenberg.FilterOutboundURL]. + err := gotenberg.FilterOutboundURL(ctx, webhookUrl, w.allowList, w.denyList, deadline) if err != nil { return fmt.Errorf("filter webhook URL: %w", err) } if webhookErrorUrl != "" { - err = gotenberg.FilterDeadline(w.errorAllowList, w.errorDenyList, webhookErrorUrl, deadline) + err = gotenberg.FilterOutboundURL(ctx, webhookErrorUrl, w.errorAllowList, w.errorDenyList, deadline) if err != nil { return fmt.Errorf("filter webhook error URL: %w", err) } @@ -197,7 +198,7 @@ func webhookMiddleware(w *Webhook) api.Middleware { // Filter the events URL if provided. if webhookEventsUrl != "" { - err = gotenberg.FilterDeadline(w.allowList, w.denyList, webhookEventsUrl, deadline) + err = gotenberg.FilterOutboundURL(ctx, webhookEventsUrl, w.allowList, w.denyList, deadline) if err != nil { return fmt.Errorf("filter webhook events URL: %w", err) } @@ -219,9 +220,7 @@ func webhookMiddleware(w *Webhook) api.Middleware { startTime: startTime, client: &retryablehttp.Client{ - HTTPClient: &http.Client{ - Timeout: w.clientTimeout, - }, + HTTPClient: gotenberg.NewOutboundHttpClient(w.clientTimeout, w.allowList, w.denyList), RetryMax: w.maxRetry, RetryWaitMin: w.retryMinWait, RetryWaitMax: w.retryMaxWait, diff --git a/test/integration/features/debug.feature b/test/integration/features/debug.feature index b762f847..92b030e5 100644 --- a/test/integration/features/debug.feature +++ b/test/integration/features/debug.feature @@ -62,7 +62,7 @@ Feature: /debug "api-disable-health-check-route-telemetry": "true", "api-disable-root-route-telemetry": "true", "api-disable-version-route-telemetry": "true", - "api-download-from-allow-list": "[]", + "api-download-from-allow-list": "[.+]", "api-download-from-deny-list": "[^https?://(10\\.|172\\.(1[6-9]|2[0-9]|3[01])\\.|192\\.168\\.|169\\.254\\.|0\\.0\\.0\\.0|127\\.|localhost|\\[::1\\]|\\[fd)]", "api-download-from-max-retry": "4", "api-enable-basic-auth": "false", @@ -77,7 +77,7 @@ Feature: /debug "api-trace-header": "Gotenberg-Trace", "chromium-allow-file-access-from-files": "false", "chromium-allow-insecure-localhost": "false", - "chromium-allow-list": "[]", + "chromium-allow-list": "[.+]", "chromium-auto-start": "false", "chromium-clear-cache": "false", "chromium-clear-cookies": "false", @@ -124,7 +124,7 @@ Feature: /debug "prometheus-disable-route-telemetry": "true", "prometheus-namespace": "gotenberg", "prometheus-metrics-path": "/prometheus/metrics", - "webhook-allow-list": "[]", + "webhook-allow-list": "[.+]", "webhook-client-timeout": "30s", "webhook-deny-list": "[^https?://(10\\.|172\\.(1[6-9]|2[0-9]|3[01])\\.|192\\.168\\.|169\\.254\\.|0\\.0\\.0\\.0|127\\.|localhost|\\[::1\\]|\\[fd)]", "webhook-disable": "false", @@ -194,7 +194,7 @@ Feature: /debug "api-disable-health-check-route-telemetry": "true", "api-disable-root-route-telemetry": "true", "api-disable-version-route-telemetry": "true", - "api-download-from-allow-list": "[]", + "api-download-from-allow-list": "[.+]", "api-download-from-deny-list": "[^https?://(10\\.|172\\.(1[6-9]|2[0-9]|3[01])\\.|192\\.168\\.|169\\.254\\.|0\\.0\\.0\\.0|127\\.|localhost|\\[::1\\]|\\[fd)]", "api-download-from-max-retry": "4", "api-enable-basic-auth": "false", @@ -209,7 +209,7 @@ Feature: /debug "api-trace-header": "Gotenberg-Trace", "chromium-allow-file-access-from-files": "false", "chromium-allow-insecure-localhost": "false", - "chromium-allow-list": "[]", + "chromium-allow-list": "[.+]", "chromium-auto-start": "false", "chromium-clear-cache": "false", "chromium-clear-cookies": "false", @@ -256,7 +256,7 @@ Feature: /debug "prometheus-disable-route-telemetry": "true", "prometheus-namespace": "gotenberg", "prometheus-metrics-path": "/prometheus/metrics", - "webhook-allow-list": "[]", + "webhook-allow-list": "[.+]", "webhook-client-timeout": "30s", "webhook-deny-list": "[^https?://(10\\.|172\\.(1[6-9]|2[0-9]|3[01])\\.|192\\.168\\.|169\\.254\\.|0\\.0\\.0\\.0|127\\.|localhost|\\[::1\\]|\\[fd)]", "webhook-disable": "false", diff --git a/test/integration/scenario/containers.go b/test/integration/scenario/containers.go index cb388174..90abab44 100644 --- a/test/integration/scenario/containers.go +++ b/test/integration/scenario/containers.go @@ -25,10 +25,53 @@ func (n *noopLogger) Printf(format string, v ...any) { // NOOP } +// integrationAllowList is the default allow-list pattern injected into +// every Gotenberg container started by the integration tests. The outbound +// URL guard introduced for SSRF protection rejects URLs whose host +// resolves to a non-public IP, which would block: +// +// - host.docker.internal (Docker host gateway, RFC1918) +// - The static helper server running inside the test network +// - file:// URIs created in /tmp by the API context +// +// Setting the allow-list to a permissive pattern flips the URL guard into +// "allow-list match bypasses the IP check" mode for every URL the tests +// touch. Operator-supplied deny-lists still apply, so deny-list scenarios +// keep working. Test scenarios that exercise allow-list semantics +// explicitly override this default in their environment table. +// +// Production operators wanting a similar bypass for trusted internal +// destinations should set their own --*-allow-list with a tighter regex +// (for example ^https?://internal\.svc(:|/|$)). +const integrationAllowList = `.+` + +// applyDefaultEnv merges baseline environment variables that the +// integration tests rely on into env, without overwriting values supplied +// by the test scenario itself. Tests can clear a default by setting it to +// the empty string in their scenario table. +func applyDefaultEnv(env map[string]string) map[string]string { + if env == nil { + env = make(map[string]string) + } + defaults := map[string]string{ + "CHROMIUM_ALLOW_LIST": integrationAllowList, + "API_DOWNLOAD_FROM_ALLOW_LIST": integrationAllowList, + "WEBHOOK_ALLOW_LIST": integrationAllowList, + } + for k, v := range defaults { + if _, ok := env[k]; !ok { + env[k] = v + } + } + return env +} + func startGotenbergContainer(ctx context.Context, env map[string]string) (*testcontainers.DockerNetwork, testcontainers.Container, error) { ctx, cancel := context.WithTimeout(ctx, 2*time.Minute) defer cancel() + env = applyDefaultEnv(env) + n, err := network.New(ctx) if err != nil { return nil, nil, fmt.Errorf("create Gotenberg container network: %w", err)