Skip to content

Commit 14c5317

Browse files
committed
🔒 security: harden proxy middleware against SSRF, redirect downgrades, and hop-by-hop smuggling
Security audit of middleware/proxy and the redirect path in client/ identified ten findings. This change implements secure-by-default behavior for all of them. See middleware/proxy/SECURITY_AUDIT.md for the full report and severity table. New API - proxy.SecurityPolicy (AllowedSchemes, AllowPrivateIPs, AllowHTTPSDowngrade, KeepHopByHopHeaders) - proxy.DefaultSecurityPolicy, proxy.WithSecurityPolicy - proxy.Config.SecurityPolicy, MaxResponseBodySize, MaxConnsPerHost - Sentinel errors: ErrUpstreamSchemeNotAllowed, ErrUpstreamHostInvalid, ErrUpstreamHostBlocked, ErrRedirectDowngrade - client.ErrRedirectDowngrade Breaking changes (defaults) - Upstream targets resolving to loopback, RFC 1918, link-local (including 169.254.169.254), multicast, unspecified, or RFC 6598 CGNAT addresses are rejected. Set AllowPrivateIPs to opt in. - Only http/https schemes are accepted; file://, gopher://, ftp://, etc. are rejected. - DoRedirects rejects HTTPS→HTTP redirect downgrades. - RFC 7230 §6.1 hop-by-hop headers (Connection, Keep-Alive, Proxy-Authenticate, Proxy-Authorization, TE, Trailer, Transfer-Encoding, Upgrade) are stripped both ways, plus every header listed in the Connection field. - TLSConfig is cloned with MinVersion: tls.VersionTLS12 when no minimum is set; caller's struct is not mutated. - Fiber HTTP client (client.composeRedirectURL) rejects HTTPS→HTTP redirect downgrades. Bug fixes - DomainForward/BalancerForward previously concatenated `addr + c.OriginalURL()`. A leading `//` in the request path could form a network-path reference that, when re-parsed, changed the upstream host. joinUpstreamPath now rebuilds the URL safely. - Snapshot scheme/target into freshly allocated strings before SetRequestURI to fix an aliasing regression where a caller-supplied addr that was itself a slice of the request buffer got clobbered mid-request (produced "unsupported protocol ttp:" errors). Tests - middleware/proxy/security_test.go: scheme allowlist, private-IP blocking + opt-in, hop-by-hop stripping (request and response, including Connection-listed), file:// rejection, HTTPS→HTTP redirect blocking with a real TLS handshake, path-injection protection, and TLS minimum version cloning. - middleware/proxy/fuzz_test.go: FuzzValidateUpstream, FuzzJoinUpstreamPath, FuzzConnectionListedHeaders (all pass with -fuzztime=10s). - TestMain in proxy_test.go relaxes the policy for loopback tests so the rest of the suite continues to work; security_test.go installs the strict policy where it asserts the new defaults. - client/transport_test.go: TestComposeRedirectURL_RejectsHTTPSDowngrade. Docs - docs/middleware/proxy.md gains a full Security section covering every default, the SecurityPolicy fields, and migration notes for the breaking defaults. https://claude.ai/code/session_01FhTTWhze2oLocDUWAkEet2
1 parent dec796e commit 14c5317

12 files changed

Lines changed: 1819 additions & 47 deletions

File tree

client/errors.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,10 @@ var (
1111
errFileTypeAssertion = errors.New("failed to type-assert to *File")
1212
errCookieJarTypeAssertion = errors.New("failed to type-assert to *CookieJar")
1313
errSyncPoolBuffer = errors.New("failed to retrieve buffer from a sync.Pool")
14+
15+
// ErrRedirectDowngrade is returned when DoRedirects encounters a
16+
// redirect from an HTTPS origin to a plaintext HTTP target.
17+
// Following such a redirect would leak any credentials, cookies, or
18+
// session tokens that the original HTTPS handshake protected.
19+
ErrRedirectDowngrade = errors.New("client: HTTPS to HTTP redirect blocked")
1420
)

client/transport.go

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -350,7 +350,8 @@ func doRedirectsWithClient(req *fasthttp.Request, resp *fasthttp.Response, maxRe
350350
// composeRedirectURL resolves a redirect target relative to the current request
351351
// URL while rejecting suspicious payloads (e.g. control characters) and
352352
// restricting schemes to HTTP/S so caller-provided Location headers cannot
353-
// trigger arbitrary transports.
353+
// trigger arbitrary transports. Redirects from HTTPS to plaintext HTTP are
354+
// rejected to prevent credentials from leaking after a TLS handshake.
354355
func composeRedirectURL(base string, location []byte, disablePathNormalizing bool) (string, error) {
355356
for _, b := range location {
356357
if b < 0x20 || b == 0x7f {
@@ -362,16 +363,22 @@ func composeRedirectURL(base string, location []byte, disablePathNormalizing boo
362363
defer fasthttp.ReleaseURI(uri)
363364

364365
uri.Update(base)
366+
wasHTTPS := bytes.EqualFold(uri.Scheme(), httpsScheme)
365367
uri.UpdateBytes(location)
366368
uri.DisablePathNormalizing = disablePathNormalizing
367369

368-
if scheme := uri.Scheme(); len(scheme) > 0 && !bytes.EqualFold(scheme, httpScheme) && !bytes.EqualFold(scheme, httpsScheme) {
370+
scheme := uri.Scheme()
371+
if len(scheme) > 0 && !bytes.EqualFold(scheme, httpScheme) && !bytes.EqualFold(scheme, httpsScheme) {
369372
return "", fasthttp.ErrorInvalidURI
370373
}
371374

372-
if len(uri.Scheme()) > 0 && len(uri.Host()) == 0 {
375+
if len(scheme) > 0 && len(uri.Host()) == 0 {
373376
return "", fasthttp.ErrorInvalidURI
374377
}
375378

379+
if wasHTTPS && bytes.EqualFold(scheme, httpScheme) {
380+
return "", ErrRedirectDowngrade
381+
}
382+
376383
return uri.String(), nil
377384
}

client/transport_test.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,30 @@ func TestDoRedirectsWithClientBranches(t *testing.T) {
325325
require.ErrorIs(t, doRedirectsWithClient(req, resp, 1, client), fasthttp.ErrTooManyRedirects)
326326
}
327327

328+
// TestComposeRedirectURL_RejectsHTTPSDowngrade verifies that redirects
329+
// from HTTPS origins to plaintext HTTP targets are refused so cookies
330+
// and credentials never leave the TLS boundary.
331+
func TestComposeRedirectURL_RejectsHTTPSDowngrade(t *testing.T) {
332+
t.Parallel()
333+
_, err := composeRedirectURL("https://example.com/login", []byte("http://example.com/after"), false)
334+
require.ErrorIs(t, err, ErrRedirectDowngrade)
335+
336+
// Same-origin HTTP→HTTP is fine.
337+
out, err := composeRedirectURL("http://example.com/a", []byte("http://example.com/b"), false)
338+
require.NoError(t, err)
339+
require.Equal(t, "http://example.com/b", out)
340+
341+
// HTTPS→HTTPS is fine.
342+
out, err = composeRedirectURL("https://example.com/a", []byte("https://example.com/b"), false)
343+
require.NoError(t, err)
344+
require.Equal(t, "https://example.com/b", out)
345+
346+
// HTTP→HTTPS upgrade is fine.
347+
out, err = composeRedirectURL("http://example.com/a", []byte("https://example.com/b"), false)
348+
require.NoError(t, err)
349+
require.Equal(t, "https://example.com/b", out)
350+
}
351+
328352
func TestDoRedirectsWithClientDefaultLimit(t *testing.T) {
329353
t.Parallel()
330354

docs/middleware/proxy.md

Lines changed: 65 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,16 +29,48 @@ func BalancerForward(servers []string, clients ...*fasthttp.Client) fiber.Handle
2929

3030
## Security
3131

32-
The `Forward`, `DomainForward`, and `BalancerForward` functions automatically set the `X-Real-IP` header to the actual client IP address obtained from `c.IP()` before forwarding the request upstream. This protects against IP spoofing attacks where a malicious client attempts to forge their IP address by sending a fake `X-Real-IP` header. Any existing `X-Real-IP` header on the incoming request is overwritten with the real client IP. Note that `DomainForward` only applies this overwrite when the request host matches the configured hostname; non-matching requests are not forwarded and are passed through unchanged.
32+
The proxy middleware applies several defenses by default. They can be relaxed via `Config.SecurityPolicy` (for `Balancer`) or `proxy.WithSecurityPolicy` (for the runtime helpers `Do`, `Forward`, `DoRedirects`, `DoTimeout`, `DoDeadline`).
3333

34-
If you're using the `Balancer` function with the `Config` struct, you can achieve the same protection by using the `ModifyRequest` callback as shown in the examples below.
34+
### SSRF protection
3535

36-
When using `Do`, `DoRedirects`, `DoDeadline`, or `DoTimeout` directly, the `X-Real-IP` header is not automatically set. You should set it manually if your upstream server requires it:
36+
Upstream addresses that resolve to loopback, RFC 1918 private, link-local (including the `169.254.169.254` cloud-metadata address), multicast, unspecified, or RFC 6598 CGNAT ranges are rejected with `ErrUpstreamHostBlocked`. Hostnames are resolved at validation time; if any returned IP falls in a blocked range the upstream is rejected, mitigating DNS-rebinding attempts that return a mix of public and private answers.
37+
38+
Set `SecurityPolicy.AllowPrivateIPs = true` to opt out — required when proxying to internal services on the same network.
39+
40+
### Scheme allowlist
41+
42+
Only `http` and `https` upstream schemes are accepted by default; `file://`, `gopher://`, `ftp://`, and other schemes are rejected. Override via `SecurityPolicy.AllowedSchemes`.
43+
44+
### HTTPS-to-HTTP redirect downgrades
45+
46+
`DoRedirects` rejects redirects from HTTPS origins to plaintext HTTP targets with `ErrRedirectDowngrade`. Following such a redirect would leak any cookies or `Authorization` headers established under TLS. Set `SecurityPolicy.AllowHTTPSDowngrade = true` to override.
47+
48+
### RFC 7230 hop-by-hop header stripping
49+
50+
`Connection`, `Keep-Alive`, `Proxy-Authenticate`, `Proxy-Authorization`, `TE`, `Trailer`, `Transfer-Encoding`, and `Upgrade` are stripped from both the outbound request and the inbound response, along with every header listed in the `Connection` field per RFC 7230 §6.1. This prevents request smuggling (`TE`/`Transfer-Encoding`), proxy-credential forwarding, and protocol-upgrade leaks. The legacy `KeepConnectionHeader` option preserves only the literal `Connection` header for backwards compatibility; the other hop-by-hop headers are still stripped. To preserve every hop-by-hop header (not recommended), set `SecurityPolicy.KeepHopByHopHeaders = true`.
51+
52+
### TLS minimum version
53+
54+
`Config.TLSConfig` is cloned with `MinVersion: tls.VersionTLS12` if no minimum is configured, so deprecated TLS versions cannot be negotiated by accident.
55+
56+
### Response body size and connection caps
57+
58+
`Config.MaxResponseBodySize` bounds upstream response bodies to protect against memory exhaustion. `Config.MaxConnsPerHost` (default `1024`) caps concurrent connections per upstream to limit fan-out from a single hot host.
59+
60+
### X-Real-IP spoof prevention
61+
62+
`Forward`, `DomainForward`, and `BalancerForward` automatically overwrite the `X-Real-IP` header with `c.IP()` before forwarding, so clients cannot spoof their address. `DomainForward` only applies the overwrite when the request host matches the configured hostname; non-matching requests are passed through unchanged.
63+
64+
If you're using `Balancer` with the `Config` struct, you can replicate the protection in `ModifyRequest`. When using `Do`, `DoRedirects`, `DoDeadline`, or `DoTimeout` directly, the `X-Real-IP` header is not set automatically — set it manually if needed:
3765

3866
```go
3967
c.Request().Header.Set("X-Real-IP", c.IP())
4068
```
4169

70+
### Path concatenation safety
71+
72+
`DomainForward` and `BalancerForward` previously concatenated the configured upstream with `c.OriginalURL()`. Crafted request paths beginning with `//` could exploit URL parsing to redirect the proxy at a different host (network-path reference injection). The proxy now sanitises the joined path so the upstream host pinned in configuration is preserved regardless of the inbound request.
73+
4274
## Examples
4375

4476
Import the middleware package:
@@ -59,11 +91,24 @@ proxy.WithClient(&fasthttp.Client{
5991
DisablePathNormalizing: true,
6092
MaxConnsPerHost: 2048,
6193
// Allow self-signed certificates when proxying to HTTPS targets.
94+
// SECURITY: disables certificate verification — use only when the
95+
// upstream is on a trusted network.
6296
TLSConfig: &tls.Config{
6397
InsecureSkipVerify: true,
98+
MinVersion: tls.VersionTLS12,
6499
},
65100
})
66101

102+
// Relax SSRF protection for local development against loopback servers.
103+
// SECURITY: in production, leave AllowPrivateIPs false (the default) and
104+
// list explicit upstream hosts so the proxy cannot be coerced into
105+
// reaching internal services or cloud-metadata endpoints.
106+
prev := proxy.WithSecurityPolicy(proxy.SecurityPolicy{
107+
AllowedSchemes: []string{"http", "https"},
108+
AllowPrivateIPs: true,
109+
})
110+
defer proxy.WithSecurityPolicy(prev)
111+
67112
// Forward requests for a specific domain with proxy.DomainForward.
68113
app.Get("/payments", proxy.DomainForward("docs.gofiber.io", "http://localhost:8000"))
69114

@@ -181,10 +226,12 @@ app.Use(proxy.Balancer(proxy.Config{
181226
| MaxConnsPerHost | `int` | Maximum number of connections per upstream host. The default proxy client and balancer host clients use this limit unless you override it with `WithClient`, a per-handler client, or `proxy.Config`. | `1024` |
182227
| ReadBufferSize | `int` | Per-connection buffer size for requests' reading. This also limits the maximum header size. Increase this buffer if your clients send multi-KB RequestURIs and/or multi-KB headers (for example, BIG cookies). | (Not specified) |
183228
| WriteBufferSize | `int` | Per-connection buffer size for responses' writing. | (Not specified) |
184-
| KeepConnectionHeader | `bool` | Keeps the `Connection` header when set to `true`. By default the header is removed to comply with RFC 7230 §6.1 and avoid proxy loops. | `false` |
185-
| TLSConfig | `*tls.Config` | TLS config for the HTTP client. | `nil` |
229+
| KeepConnectionHeader | `bool` | Keeps the `Connection` header when set to `true`. By default the header is removed to comply with RFC 7230 §6.1 and avoid proxy loops. Other hop-by-hop headers are still stripped regardless of this setting. | `false` |
230+
| TLSConfig | `*tls.Config` | TLS config for the HTTP client. Cloned with `MinVersion: tls.VersionTLS12` when no minimum is set. | `nil` |
186231
| DialDualStack | `bool` | Client will attempt to connect to both IPv4 and IPv6 host addresses if set to true. | `false` |
187232
| Client | `*fasthttp.LBClient` | Client is a custom client when client config is complex. | `nil` |
233+
| SecurityPolicy | `*SecurityPolicy` | Overrides the default SSRF, redirect, and hop-by-hop header rules for this balancer. When `nil`, the package-level policy set via `WithSecurityPolicy` is used. See [Security](#security). | `nil` |
234+
| MaxResponseBodySize | `int` | Maximum upstream response body size in bytes. `0` keeps fasthttp's unlimited default. | `0` |
188235

189236
## Default Config
190237

@@ -198,3 +245,16 @@ var ConfigDefault = Config{
198245
KeepConnectionHeader: false,
199246
}
200247
```
248+
249+
## Default SecurityPolicy
250+
251+
When `Config.SecurityPolicy` is `nil` (and `proxy.WithSecurityPolicy` has not been called), the package falls back to:
252+
253+
```go
254+
var DefaultSecurityPolicy = proxy.SecurityPolicy{
255+
AllowedSchemes: []string{"http", "https"},
256+
AllowPrivateIPs: false,
257+
AllowHTTPSDowngrade: false,
258+
KeepHopByHopHeaders: false,
259+
}
260+
```

0 commit comments

Comments
 (0)