All checks were successful
check / check (push) Successful in 5m25s
The SSRF blocklist had no escape hatch, so the thing webhooker is mostly for — taking a public webhook and forwarding it to something on your own network — could not be configured at all. Every private address, Docker sibling and loopback service was permanently unreachable as a delivery destination. ALLOWED_EGRESS_CIDRS (default empty) names blocks that delivery targets may reach despite the default blocklist. It is an allowlist and only ever adds destinations: there is no boolean, and no value disables SSRF protection wholesale. Empty, the guard behaves exactly as before. Link-local (169.254.0.0/16, fe80::/10) is refused before the allowlist is consulted, so no supplied CIDR can open it — not the exact address, not a supernet, not 0.0.0.0/0. Reaching cloud instance metadata is credential theft rather than delivery to an internal service. The policy now lives in one function, Guard.checkIP, which both target-creation validation and the delivery dialer call. The two paths previously decided separately, which is how they came to disagree about a destination. The guard is built once from config and injected via fx into both the handlers and the delivery engine, so there is a single instance and a single answer. A set-but-unparseable value aborts startup naming the variable, reusing the existing envPrefixList parser. A non-empty list is logged at startup with the blocks spelled out, not counted, so the hole is visible in the log of any deployment that has one. Tests: an allowlisted loopback CIDR both validates and delivers to a live server (and the same URL still fails without the allowlist); a private address outside the listed block stays refused on both paths; metadata stays refused under six different covering CIDRs; public addresses are unaffected either way; and config coverage for parsing, startup abort, and the warning's contents.
371 lines
9.2 KiB
Go
371 lines
9.2 KiB
Go
package delivery_test
|
|
|
|
import (
|
|
"context"
|
|
"net"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"net/netip"
|
|
"net/url"
|
|
"testing"
|
|
"time"
|
|
|
|
"github.com/stretchr/testify/assert"
|
|
"github.com/stretchr/testify/require"
|
|
"sneak.berlin/go/webhooker/internal/delivery"
|
|
)
|
|
|
|
// Addresses the SSRF tests in this package share.
|
|
const (
|
|
// metadataIP is the cloud instance metadata address, and
|
|
// metadataURL an endpoint on it. The guard must never reach
|
|
// either, whatever an operator lists.
|
|
metadataIP = "169.254.169.254"
|
|
metadataURL = "http://" + metadataIP + "/latest/meta-data/"
|
|
|
|
// loopbackHookURL is a target on this host: blocked by
|
|
// default, reachable only once an operator allowlists
|
|
// loopback.
|
|
loopbackHookURL = "http://127.0.0.1/hook"
|
|
|
|
// publicIP is an ordinary public address, which the guard
|
|
// permits with or without an allowlist.
|
|
publicIP = "93.184.216.34"
|
|
)
|
|
|
|
// TestGuardAllowlist_PermittedCIDRDelivers proves the escape
|
|
// hatch actually works end to end: with 127.0.0.0/8 allowed, the
|
|
// guard's own transport connects to a loopback server and gets a
|
|
// response back. The default guard, given the identical URL,
|
|
// refuses it — so the delivery succeeds because of the allowlist
|
|
// and nothing else.
|
|
func TestGuardAllowlist_PermittedCIDRDelivers(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
srv := httptest.NewServer(http.HandlerFunc(
|
|
func(w http.ResponseWriter, _ *http.Request) {
|
|
w.WriteHeader(http.StatusNoContent)
|
|
},
|
|
))
|
|
t.Cleanup(srv.Close)
|
|
|
|
// httptest listens on loopback, which the default blocklist
|
|
// covers: exactly the "forward to a service on this host"
|
|
// case the allowlist exists for.
|
|
requireLoopback(t, srv.URL)
|
|
|
|
guard := delivery.NewTestGuard(
|
|
netip.MustParsePrefix("127.0.0.0/8"),
|
|
)
|
|
|
|
require.NoError(t,
|
|
guard.ValidateTargetURL(context.Background(), srv.URL),
|
|
"an allowlisted loopback target must pass validation",
|
|
)
|
|
|
|
client := &http.Client{
|
|
Timeout: 5 * time.Second,
|
|
Transport: guard.NewSSRFSafeTransport(),
|
|
}
|
|
|
|
req, err := http.NewRequestWithContext(
|
|
context.Background(), http.MethodPost, srv.URL, nil,
|
|
)
|
|
require.NoError(t, err)
|
|
|
|
resp, err := client.Do(req)
|
|
require.NoError(t,
|
|
err, "an allowlisted loopback target must be deliverable",
|
|
)
|
|
|
|
defer func() { _ = resp.Body.Close() }()
|
|
|
|
assert.Equal(t, http.StatusNoContent, resp.StatusCode)
|
|
|
|
// The same URL through the default guard must still fail, or
|
|
// this test would pass without the allowlist doing anything.
|
|
assert.Error(t,
|
|
delivery.NewTestGuard().ValidateTargetURL(
|
|
context.Background(), srv.URL,
|
|
),
|
|
"without the allowlist the same target must be refused",
|
|
)
|
|
}
|
|
|
|
// TestGuardAllowlist_UnlistedPrivateStillRefused proves the
|
|
// allowlist grants only what it names. A guard that opens one
|
|
// private block must keep refusing every other one, at both the
|
|
// validation and the delivery entry point.
|
|
func TestGuardAllowlist_UnlistedPrivateStillRefused(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
// Only 10.1.0.0/16 is open — a narrow block inside a much
|
|
// wider private range, so the test can tell "permits the
|
|
// listed block" from "permits anything private".
|
|
guard := delivery.NewTestGuard(
|
|
netip.MustParsePrefix("10.1.0.0/16"),
|
|
)
|
|
|
|
refused := []string{
|
|
"http://192.168.1.10/hook",
|
|
"http://172.16.0.1/hook",
|
|
loopbackHookURL,
|
|
"http://[fc00::1]/hook",
|
|
"http://100.64.0.1/hook",
|
|
// Private, adjacent to the allowed block, outside it.
|
|
"http://10.2.0.1/hook",
|
|
}
|
|
|
|
for _, target := range refused {
|
|
t.Run(target, func(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
err := guard.ValidateTargetURL(
|
|
context.Background(), target,
|
|
)
|
|
require.Error(t,
|
|
err, "%s is not allowlisted and must be refused",
|
|
target,
|
|
)
|
|
assert.Contains(t, err.Error(), "blocked")
|
|
|
|
assertDialRefused(t, guard, target)
|
|
})
|
|
}
|
|
|
|
// The block that is listed must in fact be permitted, so the
|
|
// refusals above are selective rather than a guard that
|
|
// ignores its allowlist entirely.
|
|
assert.NoError(t,
|
|
guard.ValidateTargetURL(
|
|
context.Background(), "http://10.1.2.3/hook",
|
|
),
|
|
"the allowlisted block must be permitted",
|
|
)
|
|
}
|
|
|
|
// TestGuardAllowlist_MetadataAlwaysRefused is the load-bearing
|
|
// case: link-local carries cloud instance metadata, so reaching
|
|
// it is credential theft rather than delivery to an internal
|
|
// service. Every one of these guards names a CIDR that covers
|
|
// 169.254.169.254 — including 0.0.0.0/0 — and the address must
|
|
// stay refused anyway, on both paths.
|
|
func TestGuardAllowlist_MetadataAlwaysRefused(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
tests := []struct {
|
|
name string
|
|
allow string
|
|
target string
|
|
}{
|
|
{
|
|
name: "exact metadata host",
|
|
allow: "169.254.169.254/32",
|
|
target: metadataURL,
|
|
},
|
|
{
|
|
name: "whole link-local block",
|
|
allow: "169.254.0.0/16",
|
|
target: metadataURL,
|
|
},
|
|
{
|
|
name: "supernet covering link-local",
|
|
allow: "169.0.0.0/8",
|
|
target: metadataURL,
|
|
},
|
|
{
|
|
name: "the entire IPv4 internet",
|
|
allow: "0.0.0.0/0",
|
|
target: metadataURL,
|
|
},
|
|
{
|
|
name: "other link-local address",
|
|
allow: "0.0.0.0/0",
|
|
target: "http://169.254.1.1/",
|
|
},
|
|
{
|
|
name: "IPv6 link-local",
|
|
allow: "::/0",
|
|
target: "http://[fe80::1]/",
|
|
},
|
|
}
|
|
|
|
for _, tt := range tests {
|
|
t.Run(tt.name, func(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
guard := delivery.NewTestGuard(
|
|
netip.MustParsePrefix(tt.allow),
|
|
)
|
|
|
|
err := guard.ValidateTargetURL(
|
|
context.Background(), tt.target,
|
|
)
|
|
require.Error(t,
|
|
err,
|
|
"%s must stay blocked even though %s covers it",
|
|
tt.target, tt.allow,
|
|
)
|
|
assert.Contains(t, err.Error(), "link-local",
|
|
"the refusal must say why it cannot be opened",
|
|
)
|
|
|
|
assertDialRefused(t, guard, tt.target)
|
|
})
|
|
}
|
|
}
|
|
|
|
// TestGuardAllowlist_PublicUnaffected asserts the allowlist does
|
|
// not narrow anything: public addresses were reachable before it
|
|
// existed and stay reachable, whether or not a list is set.
|
|
func TestGuardAllowlist_PublicUnaffected(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
guards := map[string]*delivery.Guard{
|
|
"default": delivery.NewTestGuard(),
|
|
"with allowlist": delivery.NewTestGuard(
|
|
netip.MustParsePrefix("10.0.0.0/8"),
|
|
),
|
|
}
|
|
|
|
for name, guard := range guards {
|
|
t.Run(name, func(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
assert.NoError(t,
|
|
guard.ValidateTargetURL(
|
|
context.Background(),
|
|
"http://"+publicIP+"/webhook",
|
|
),
|
|
)
|
|
})
|
|
}
|
|
}
|
|
|
|
// TestGuardCheckIP_BothPathsShareOneDecision asserts that the
|
|
// validator and the dialer are not two policies that happen to
|
|
// agree: both are defined in terms of checkIP, so the exported
|
|
// decision function is the whole answer for a given address.
|
|
func TestGuardCheckIP_BothPathsShareOneDecision(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
guard := delivery.NewTestGuard(
|
|
netip.MustParsePrefix("10.0.0.0/8"),
|
|
)
|
|
|
|
tests := []struct {
|
|
ip string
|
|
allowed bool
|
|
}{
|
|
{"10.1.2.3", true},
|
|
{publicIP, true},
|
|
{"192.168.1.1", false},
|
|
{"127.0.0.1", false},
|
|
{metadataIP, false},
|
|
}
|
|
|
|
for _, tt := range tests {
|
|
t.Run(tt.ip, func(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
ip := net.ParseIP(tt.ip)
|
|
require.NotNil(t, ip)
|
|
|
|
decision := guard.ExportCheckIP(ip)
|
|
|
|
validation := guard.ValidateTargetURL(
|
|
context.Background(), "http://"+hostFor(tt.ip)+"/x",
|
|
)
|
|
|
|
if tt.allowed {
|
|
require.NoError(t, decision)
|
|
require.NoError(t, validation)
|
|
|
|
return
|
|
}
|
|
|
|
require.Error(t, decision)
|
|
require.Error(t, validation,
|
|
"validation must refuse what checkIP refuses",
|
|
)
|
|
})
|
|
}
|
|
}
|
|
|
|
// TestAlwaysBlockedNetworks_AreLinkLocal pins the unconditional
|
|
// set to link-local only. It is deliberately small: everything
|
|
// else in the default blocklist is an operator's own network and
|
|
// must remain openable, or the escape hatch would not work.
|
|
func TestAlwaysBlockedNetworks_AreLinkLocal(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
nets := delivery.ExportAlwaysBlockedNetworks()
|
|
|
|
got := make([]string, 0, len(nets))
|
|
for _, n := range nets {
|
|
got = append(got, n.String())
|
|
}
|
|
|
|
assert.Equal(t,
|
|
[]string{"169.254.0.0/16", "fe80::/10"}, got,
|
|
)
|
|
}
|
|
|
|
// requireLoopback fails the test unless rawURL's host is a
|
|
// loopback address, so the allowlist test cannot silently stop
|
|
// exercising a blocked range.
|
|
func requireLoopback(t *testing.T, rawURL string) {
|
|
t.Helper()
|
|
|
|
parsed, err := url.Parse(rawURL)
|
|
require.NoError(t, err)
|
|
|
|
ip := net.ParseIP(parsed.Hostname())
|
|
require.NotNil(t, ip, "test server host must be an IP literal")
|
|
require.True(t, ip.IsLoopback(),
|
|
"test server must listen on loopback, got %s", ip,
|
|
)
|
|
}
|
|
|
|
// assertDialRefused asserts the guard's transport refuses to
|
|
// connect to target, which is the delivery-time half of the
|
|
// policy. It never reaches the network: the guard checks the
|
|
// resolved address before dialling.
|
|
func assertDialRefused(
|
|
t *testing.T, guard *delivery.Guard, target string,
|
|
) {
|
|
t.Helper()
|
|
|
|
client := &http.Client{
|
|
Timeout: 5 * time.Second,
|
|
Transport: guard.NewSSRFSafeTransport(),
|
|
}
|
|
|
|
req, err := http.NewRequestWithContext(
|
|
context.Background(), http.MethodPost, target, nil,
|
|
)
|
|
require.NoError(t, err)
|
|
|
|
resp, err := client.Do(req)
|
|
if resp != nil {
|
|
_ = resp.Body.Close()
|
|
}
|
|
|
|
require.Error(t, err,
|
|
"delivery to %s must be refused by the dialer", target,
|
|
)
|
|
assert.Contains(t, err.Error(), "blocked",
|
|
"the refusal must come from the SSRF guard",
|
|
)
|
|
}
|
|
|
|
// hostFor renders an IP as it appears in a URL host, bracketing
|
|
// IPv6 literals.
|
|
func hostFor(ip string) string {
|
|
if net.ParseIP(ip).To4() == nil {
|
|
return "[" + ip + "]"
|
|
}
|
|
|
|
return ip
|
|
}
|