From e1d714e5e0aa85c00777c3f562ca052182acb4da Mon Sep 17 00:00:00 2001 From: Julien Neuhart Date: Tue, 13 Oct 2026 19:39:50 +0200 Subject: [PATCH] fix(chromium): route loopback WebSockets through the pinning proxy when an outbound policy is set --- pkg/modules/chromium/browser.go | 33 +++++++++++------ pkg/modules/chromium/browser_test.go | 35 +++++++++++++++++++ pkg/modules/chromium/chromium.go | 6 +++- .../features/chromium_convert_html.feature | 21 ++++++++++- .../chromium_convert_markdown.feature | 5 ++- .../features/chromium_convert_url.feature | 5 ++- 6 files changed, 90 insertions(+), 15 deletions(-) diff --git a/pkg/modules/chromium/browser.go b/pkg/modules/chromium/browser.go index ed523eec..f1ff5621 100644 --- a/pkg/modules/chromium/browser.go +++ b/pkg/modules/chromium/browser.go @@ -75,6 +75,20 @@ type browserArguments struct { disableJavaScript bool } +// restrictsNetwork reports whether the outbound policy can reject a network +// URL. The default deny-list only rejects file:// URLs, so it does not count. +func (a browserArguments) restrictsNetwork() bool { + if a.denyPrivateIPs || a.denyPublicIPs || len(a.allowList) > 0 { + return true + } + for _, pattern := range a.denyList { + if pattern.String() != defaultDenyListPattern { + return true + } + } + return false +} + type chromiumBrowser struct { initialCtx context.Context ctx context.Context @@ -205,7 +219,7 @@ func (b *chromiumBrowser) Start(logger *slog.Logger) error { } opts = append(opts, chromedp.ProxyServer(b.pinningProxy.URL())) - if b.arguments.denyPrivateIPs || b.arguments.denyPublicIPs { + if b.arguments.restrictsNetwork() { // Chromium implicitly bypasses the proxy for loopback and // link-local destinations. A WebSocket handshake is never surfaced // as a fetch.EventRequestPaused, so listenForEventRequestPaused @@ -213,17 +227,14 @@ func (b *chromiumBrowser) Start(logger *slog.Logger) error { // it. Left alone, a page could open a WebSocket to 127.0.0.1, ::1, // localhost, or the link-local cloud metadata endpoint // (169.254.169.254) and reach it unfiltered. "<-loopback>" removes - // the implicit bypass so those handshakes also traverse the pinning - // proxy and go through [gotenberg.DecideOutbound] like every other - // request. + // the implicit bypass so those handshakes also traverse the + // pinning proxy and go through [gotenberg.DecideOutbound] like + // every other request. // - // Gated on the IP-class policy: it is the control this closes, and - // under it loopback and link-local HTTP sub-resources are already - // blocked by listenForEventRequestPaused before they would reach - // the proxy, so this adds only the missing WebSocket coverage. When - // the policy is off, loopback is not restricted, and routing it - // through the proxy would merely change how an unreachable loopback - // sub-resource reports its failure. + // Gated on a policy that can reject a network URL: without one, + // loopback is not restricted, and routing it through the proxy + // would only turn a refused loopback sub-resource into a 502 + // response instead of a loading failure. opts = append(opts, chromedp.Flag("proxy-bypass-list", "<-loopback>")) } } diff --git a/pkg/modules/chromium/browser_test.go b/pkg/modules/chromium/browser_test.go index 0c25f5e0..7d9c06b3 100644 --- a/pkg/modules/chromium/browser_test.go +++ b/pkg/modules/chromium/browser_test.go @@ -6,6 +6,8 @@ import ( "slices" "strings" "testing" + + "github.com/dlclark/regexp2/v2" ) // TestChromiumBrowser_Start_rejectsOverlappingStart guards against the latch @@ -59,3 +61,36 @@ func TestChromiumDisableFeatures(t *testing.T) { } } } + +// A WebSocket the outbound policy rejects must reach the pinning proxy, so +// the implicit loopback proxy bypass is removed whenever a policy can reject +// a network URL. See GHSA-73vq-mh8c-27wg. +func TestBrowserArguments_restrictsNetwork(t *testing.T) { + re := func(patterns ...string) []*regexp2.Regexp { + var res []*regexp2.Regexp + for _, p := range patterns { + res = append(res, regexp2.MustCompile(p, regexp2.None)) + } + return res + } + + for _, tc := range []struct { + scenario string + args browserArguments + expect bool + }{ + {scenario: "default deny-list only", args: browserArguments{denyList: re(defaultDenyListPattern)}, expect: false}, + {scenario: "no lists", args: browserArguments{}, expect: false}, + {scenario: "allow-list", args: browserArguments{allowList: re(`^https://example\.com`), denyList: re(defaultDenyListPattern)}, expect: true}, + {scenario: "custom deny-list", args: browserArguments{denyList: re(`^http://internal`)}, expect: true}, + {scenario: "default and custom deny-list", args: browserArguments{denyList: re(defaultDenyListPattern, `^http://internal`)}, expect: true}, + {scenario: "deny private IPs", args: browserArguments{denyList: re(defaultDenyListPattern), denyPrivateIPs: true}, expect: true}, + {scenario: "deny public IPs", args: browserArguments{denyPublicIPs: true}, expect: true}, + } { + t.Run(tc.scenario, func(t *testing.T) { + if got := tc.args.restrictsNetwork(); got != tc.expect { + t.Fatalf("restrictsNetwork() = %t, want %t", got, tc.expect) + } + }) + } +} diff --git a/pkg/modules/chromium/chromium.go b/pkg/modules/chromium/chromium.go index a8eb7a6d..8d42f8f1 100644 --- a/pkg/modules/chromium/chromium.go +++ b/pkg/modules/chromium/chromium.go @@ -97,6 +97,10 @@ var ( ErrPageRangesExceedsPageCount = errors.New("page ranges exceeds page count") ) +// defaultDenyListPattern is the default --chromium-deny-list entry. It only +// rejects file:// URLs outside /tmp. +const defaultDenyListPattern = `^file:(?!//\/tmp/).*` + // Chromium is a module that provides both an [Api] and routes for converting // an HTML document to PDF. type Chromium struct { @@ -476,7 +480,7 @@ func (mod *Chromium) Descriptor() gotenberg.ModuleDescriptor { fs.String("chromium-proxy-server", "", "Set the outbound proxy server; this switch only affects HTTP and HTTPS requests") fs.Bool("chromium-enable-environment-proxy", false, "Route Chromium's outbound requests through the proxy defined by the standard HTTP_PROXY, HTTPS_PROXY, and NO_PROXY variables, including credentials. Use this instead of --chromium-proxy-server for authenticated proxies, and leave --chromium-proxy-server and --chromium-host-resolver-rules unset") fs.StringSlice("chromium-allow-list", []string{}, `Set the allowed URLs for Chromium using regular expressions - supports multiple values. A match bypasses --chromium-deny-private-ips (CHROMIUM_DENY_PRIVATE_IPS) and --chromium-deny-public-ips (CHROMIUM_DENY_PUBLIC_IPS), so terminate the host or the pattern also matches suffix hosts, for example ^https?://internal\.svc(:|/|$)`) - fs.StringSlice("chromium-deny-list", []string{`^file:(?!//\/tmp/).*`}, "Set the denied URLs for Chromium using regular expressions - supports multiple values") + fs.StringSlice("chromium-deny-list", []string{defaultDenyListPattern}, "Set the denied URLs for Chromium using regular expressions - supports multiple values") fs.Bool("chromium-deny-private-ips", false, "Reject URLs whose host resolves to a non-public IP address (loopback, RFC1918, link-local, unique-local). Enable on deployments that accept untrusted form input to mitigate SSRF against internal services") fs.Bool("chromium-deny-public-ips", false, "Reject URLs whose host resolves to a public IP address. Enable on air-gapped or data-governed deployments to prevent outbound traffic from leaving a private network") fs.Bool("chromium-shared-browser-context", false, "Run every conversion in one shared browser context, so cookies, cache, and storage carry over between conversions. Only enable it when all callers trust each other: a conversion can then reuse cookies another one set") diff --git a/test/integration/features/chromium_convert_html.feature b/test/integration/features/chromium_convert_html.feature index 11c6f0f9..5405b61a 100644 --- a/test/integration/features/chromium_convert_html.feature +++ b/test/integration/features/chromium_convert_html.feature @@ -521,6 +521,22 @@ Feature: /forms/chromium/convert/html | 'wss://127.0.0.1:9999/wss-test' targets a non-public address | | CONNECT blocked for '127.0.0.1:9999' | + # An allow-list is enforced on its own, without the IP-class flags. Chromium + # skips the proxy for loopback and link-local destinations unless told + # otherwise, so a WebSocket the allow-list rejects must still reach the + # pinning proxy to be severed. + @chromium-ssrf + Scenario: POST /forms/chromium/convert/html (WebSocket not in the allow-list is filtered) + Given I have a Gotenberg container with the following environment variable(s): + | CHROMIUM_ALLOW_LIST | ^file:///tmp/.* | + When I make a "POST" request to Gotenberg at the "/forms/chromium/convert/html" endpoint with the following form data and header(s): + | files | testdata/ssrf-websocket-html/index.html | file | + | waitDelay | 1s | field | + Then the response status code should be 200 + Then the Gotenberg container should log the following entries: + | 'ws://127.0.0.1:9999/ssrf-websocket' does not match any expression from the allowed list | + | CONNECT blocked for '127.0.0.1:9999' | + # EventSource issues an ordinary HTTP GET, so unlike a WebSocket it IS surfaced # as a fetch.EventRequestPaused and blocked by listenForEventRequestPaused. @chromium-ssrf @@ -629,7 +645,10 @@ Feature: /forms/chromium/convert/html Then there should be 1 PDF(s) in the response Scenario: POST /forms/chromium/convert/html (Fail On Resource Loading Failed) - Given I have a default Gotenberg container + # A configured allow-list routes loopback through the pinning proxy, which + # reports a refused connection as a 502 response, not a loading failure. + Given I have a Gotenberg container with the following environment variable(s): + | CHROMIUM_ALLOW_LIST | | When I make a "POST" request to Gotenberg at the "/forms/chromium/convert/html" endpoint with the following form data and header(s): | files | testdata/feature-rich-html/index.html | file | | failOnResourceLoadingFailed | true | field | diff --git a/test/integration/features/chromium_convert_markdown.feature b/test/integration/features/chromium_convert_markdown.feature index ffaf5963..0f4b8e8a 100644 --- a/test/integration/features/chromium_convert_markdown.feature +++ b/test/integration/features/chromium_convert_markdown.feature @@ -410,7 +410,10 @@ Feature: /forms/chromium/convert/markdown """ Scenario: POST /forms/chromium/convert/markdown (Fail On Resource Loading Failed) - Given I have a default Gotenberg container + # A configured allow-list routes loopback through the pinning proxy, which + # reports a refused connection as a 502 response, not a loading failure. + Given I have a Gotenberg container with the following environment variable(s): + | CHROMIUM_ALLOW_LIST | | When I make a "POST" request to Gotenberg at the "/forms/chromium/convert/markdown" endpoint with the following form data and header(s): | files | testdata/feature-rich-markdown/index.html | file | | files | testdata/feature-rich-markdown/table.md | file | diff --git a/test/integration/features/chromium_convert_url.feature b/test/integration/features/chromium_convert_url.feature index 18c338be..b2d801e9 100644 --- a/test/integration/features/chromium_convert_url.feature +++ b/test/integration/features/chromium_convert_url.feature @@ -711,7 +711,10 @@ Feature: /forms/chromium/convert/url """ Scenario: POST /forms/chromium/convert/url (Fail On Resource Loading Failed) - Given I have a default Gotenberg container + # A configured allow-list routes loopback through the pinning proxy, which + # reports a refused connection as a 502 response, not a loading failure. + Given I have a Gotenberg container with the following environment variable(s): + | CHROMIUM_ALLOW_LIST | | Given I have a static server When I make a "POST" request to Gotenberg at the "/forms/chromium/convert/url" endpoint with the following form data and header(s): | url | http://host.docker.internal:%d/html/testdata/feature-rich-html-remote/index.html | field |