From 61501e3b3d42d129be87a280f537290dbf3d6d30 Mon Sep 17 00:00:00 2001 From: Alex Luong Date: Tue, 27 Oct 2026 12:16:50 +0700 Subject: [PATCH 01/13] feat: add netguard package for outbound address checks --- internal/netguard/allowlist.go | 109 ++++++ internal/netguard/allowlist_test.go | 123 ++++++ internal/netguard/errors.go | 49 +++ internal/netguard/export_test.go | 24 ++ internal/netguard/guard.go | 165 ++++++++ internal/netguard/guard_test.go | 365 ++++++++++++++++++ internal/netguard/iana.go | 145 +++++++ internal/netguard/iana_test.go | 365 ++++++++++++++++++ internal/netguard/netguardtest/dns.go | 125 ++++++ .../testdata/iana-ipv4-special-registry-1.csv | 27 ++ .../testdata/iana-ipv6-special-registry-1.csv | 28 ++ internal/netguard/transport.go | 42 ++ internal/netguard/transport_test.go | 167 ++++++++ 13 files changed, 1734 insertions(+) create mode 100644 internal/netguard/allowlist.go create mode 100644 internal/netguard/allowlist_test.go create mode 100644 internal/netguard/errors.go create mode 100644 internal/netguard/export_test.go create mode 100644 internal/netguard/guard.go create mode 100644 internal/netguard/guard_test.go create mode 100644 internal/netguard/iana.go create mode 100644 internal/netguard/iana_test.go create mode 100644 internal/netguard/netguardtest/dns.go create mode 100644 internal/netguard/testdata/iana-ipv4-special-registry-1.csv create mode 100644 internal/netguard/testdata/iana-ipv6-special-registry-1.csv create mode 100644 internal/netguard/transport.go create mode 100644 internal/netguard/transport_test.go diff --git a/internal/netguard/allowlist.go b/internal/netguard/allowlist.go new file mode 100644 index 000000000..fd7ba5f02 --- /dev/null +++ b/internal/netguard/allowlist.go @@ -0,0 +1,109 @@ +package netguard + +import ( + "errors" + "fmt" + "net/netip" + "strings" +) + +// Allowlist holds the address ranges a Guard lets connections reach even +// though they are not globally reachable. It holds IP ranges only. +// A hostname entry would trust whatever DNS answers for that name, which is +// exactly what the guard is there to distrust. A nil *Allowlist is empty. +type Allowlist struct { + prefixes []netip.Prefix +} + +var loopbackPrefixes = []netip.Prefix{ + netip.MustParsePrefix("127.0.0.0/8"), + netip.MustParsePrefix("::1/128"), +} + +// ParseAllowlist parses allowlist entries: an IP address, a CIDR range, or +// "localhost" (127.0.0.0/8 and ::1/128). Blank entries are skipped. A /0 +// range, a range with host bits set, an address with a zone and anything +// unparsable is an error; all bad entries are reported together. +// IPv4-mapped IPv6 entries are converted to IPv4, since the guard compares +// unmapped addresses. +func ParseAllowlist(entries []string) (*Allowlist, error) { + var ( + prefixes []netip.Prefix + errs []error + seen = make(map[netip.Prefix]bool) + ) + add := func(p netip.Prefix) { + if seen[p] { + return + } + seen[p] = true + prefixes = append(prefixes, p) + } + for _, raw := range entries { + entry := strings.TrimSpace(raw) + if entry == "" { + continue + } + if strings.EqualFold(entry, "localhost") { + for _, p := range loopbackPrefixes { + add(p) + } + continue + } + p, err := parseAllowlistEntry(entry) + if err != nil { + errs = append(errs, err) + continue + } + if masked := p.Masked(); masked != p { + errs = append(errs, fmt.Errorf("allowlist entry %q has host bits set; use %s", entry, masked)) + continue + } + add(p) + } + if len(errs) > 0 { + return nil, errors.Join(errs...) + } + return &Allowlist{prefixes: prefixes}, nil +} + +func parseAllowlistEntry(entry string) (netip.Prefix, error) { + var p netip.Prefix + if strings.Contains(entry, "/") { + var err error + if p, err = netip.ParsePrefix(entry); err != nil { + return netip.Prefix{}, fmt.Errorf("allowlist entry %q: not an IP address or CIDR range", entry) + } + } else { + addr, err := netip.ParseAddr(entry) + if err != nil { + return netip.Prefix{}, fmt.Errorf("allowlist entry %q: not an IP address or CIDR range (host names are not supported)", entry) + } + if addr.Zone() != "" { + return netip.Prefix{}, fmt.Errorf("allowlist entry %q: zoned addresses are not supported", entry) + } + p = netip.PrefixFrom(addr, addr.BitLen()) + } + if p.Addr().Is4In6() && p.Bits() >= 96 { + p = netip.PrefixFrom(p.Addr().Unmap(), p.Bits()-96) + } + if p.Bits() == 0 { + return netip.Prefix{}, fmt.Errorf("allowlist entry %q: a /0 range would disable the guard", entry) + } + return p, nil +} + +// Contains reports whether addr falls in an allowlisted range. IPv4-mapped +// addresses are unmapped first; an address with a zone never matches. +func (a *Allowlist) Contains(addr netip.Addr) bool { + if a == nil { + return false + } + addr = addr.Unmap() + for _, p := range a.prefixes { + if p.Contains(addr) { + return true + } + } + return false +} diff --git a/internal/netguard/allowlist_test.go b/internal/netguard/allowlist_test.go new file mode 100644 index 000000000..3e3891de4 --- /dev/null +++ b/internal/netguard/allowlist_test.go @@ -0,0 +1,123 @@ +package netguard_test + +import ( + "net/netip" + "testing" + + "github.com/hookdeck/outpost/internal/netguard" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestParseAllowlist(t *testing.T) { + t.Parallel() + al, err := netguard.ParseAllowlist([]string{ + " 10.0.0.0/8 ", "", " ", "192.168.1.5", "fd00::/8", "localhost", "::ffff:172.16.0.0/108", + }) + require.NoError(t, err) + + assert.Equal(t, []netip.Prefix{ + netip.MustParsePrefix("10.0.0.0/8"), + netip.MustParsePrefix("192.168.1.5/32"), + netip.MustParsePrefix("fd00::/8"), + netip.MustParsePrefix("127.0.0.0/8"), + netip.MustParsePrefix("::1/128"), + // IPv4-mapped entries are stored as IPv4. + netip.MustParsePrefix("172.16.0.0/12"), + }, al.Prefixes()) + + for addr, want := range map[string]bool{ + "10.0.0.0": true, + "10.255.255.255": true, + "11.0.0.0": false, + "192.168.1.5": true, + "192.168.1.6": false, + "fd12::1": true, + "fc00::1": false, + "127.0.0.1": true, + "127.9.9.9": true, + "::1": true, + "::2": false, + "172.20.0.1": true, + "::ffff:10.1.2.3": true, // mapped addresses are unmapped first + "::ffff:11.1.2.3": false, + "fd00::1%eth0": false, // zones never match + "8.8.8.8": false, + "2001:4860::8888": false, + "::ffff:127.0.0.1": true, + } { + assert.Equal(t, want, al.Contains(netip.MustParseAddr(addr)), addr) + } + assert.False(t, al.Contains(netip.Addr{})) +} + +func TestParseAllowlist_Localhost(t *testing.T) { + t.Parallel() + al, err := netguard.ParseAllowlist([]string{"LocalHost", "127.0.0.0/8", "localhost"}) + require.NoError(t, err) + assert.Equal(t, []netip.Prefix{ + netip.MustParsePrefix("127.0.0.0/8"), + netip.MustParsePrefix("::1/128"), + }, al.Prefixes(), "duplicates collapse") +} + +func TestParseAllowlist_Empty(t *testing.T) { + t.Parallel() + al, err := netguard.ParseAllowlist(nil) + require.NoError(t, err) + assert.Empty(t, al.Prefixes()) + assert.False(t, al.Contains(netip.MustParseAddr("127.0.0.1"))) + + var nilList *netguard.Allowlist + assert.False(t, nilList.Contains(netip.MustParseAddr("127.0.0.1"))) + assert.Nil(t, nilList.Prefixes()) +} + +func TestParseAllowlist_Rejects(t *testing.T) { + t.Parallel() + for _, entry := range []string{ + "0.0.0.0/0", + "::/0", + "::ffff:0.0.0.0/96", // all of IPv4, once unmapped + "example.com", + "*.example.com", + "*", + "10.0.0.0/33", + "10.0.0.0/abc", + "10.0.0", + "fe80::1%eth0", + "fe80::1%eth0/64", + "[::1]", + "127.0.0.1:8080", + "localhost:8080", + "0x7f.0.0.1", + "2130706433", + "10.0.0.1/8", + } { + al, err := netguard.ParseAllowlist([]string{"127.0.0.1", entry}) + assert.Error(t, err, entry) + assert.Nil(t, al, entry) + } + + // Every bad entry is reported at once. + _, err := netguard.ParseAllowlist([]string{"bad-one", "10.0.0.0/8", "0.0.0.0/0"}) + require.Error(t, err) + assert.Contains(t, err.Error(), `"bad-one"`) + assert.Contains(t, err.Error(), `"0.0.0.0/0"`) + assert.NotContains(t, err.Error(), `"10.0.0.0/8"`) +} + +func TestParseAllowlist_HostBits(t *testing.T) { + t.Parallel() + for entry, masked := range map[string]string{ + "10.1.2.3/8": "10.0.0.0/8", + "fd00::1/8": "fd00::/8", + "::ffff:10.1.2.3/104": "10.0.0.0/8", + "192.168.1.1/24": "192.168.1.0/24", + } { + al, err := netguard.ParseAllowlist([]string{entry}) + require.Error(t, err, entry) + assert.Nil(t, al, entry) + assert.Contains(t, err.Error(), "use "+masked, entry) + } +} diff --git a/internal/netguard/errors.go b/internal/netguard/errors.go new file mode 100644 index 000000000..063e772c1 --- /dev/null +++ b/internal/netguard/errors.go @@ -0,0 +1,49 @@ +package netguard + +import ( + "errors" + "net/netip" + "strconv" +) + +// Reasons an *AddressNotAllowedError carries. +const ( + // ReasonNotGlobal: an address is neither globally reachable nor + // allowlisted. + ReasonNotGlobal = "not_global" + // ReasonNoAddresses: the host resolved to no address at all. + ReasonNoAddresses = "no_addresses" +) + +// AddressNotAllowedError is returned when the guard refuses a URL or a +// connection. It is for server-side handling and logs only: callers map it +// to a fixed client-visible result and never echo Addr, which can reveal +// internal DNS. Error() leaves Addr out for the same reason. +type AddressNotAllowedError struct { + // Host is the host as given (URL host or dial host), without port. + Host string + // Addr is the refused address; zero when no address is involved + // (ReasonNoAddresses). + Addr netip.Addr + Reason string +} + +func (e *AddressNotAllowedError) Error() string { + return "netguard: address not allowed for host " + strconv.Quote(e.Host) + ": " + e.Reason +} + +// AsAddressNotAllowed returns the *AddressNotAllowedError in err's chain. +func AsAddressNotAllowed(err error) (*AddressNotAllowedError, bool) { + var e *AddressNotAllowedError + if errors.As(err, &e) { + return e, true + } + return nil, false +} + +// IsAddressNotAllowed reports whether err's chain holds an +// *AddressNotAllowedError (through *url.Error and *net.OpError too). +func IsAddressNotAllowed(err error) bool { + _, ok := AsAddressNotAllowed(err) + return ok +} diff --git a/internal/netguard/export_test.go b/internal/netguard/export_test.go new file mode 100644 index 000000000..71baa3b92 --- /dev/null +++ b/internal/netguard/export_test.go @@ -0,0 +1,24 @@ +package netguard + +import "net/netip" + +// RegistryTable returns the embedded registry rows as prefix → global. +func RegistryTable(v6 bool) map[netip.Prefix]bool { + blocks := ipv4Special + if v6 { + blocks = ipv6Special + } + out := make(map[netip.Prefix]bool, len(blocks)) + for _, b := range blocks { + out[b.prefix] = b.global + } + return out +} + +// Prefixes returns a copy of the allowlisted ranges. +func (a *Allowlist) Prefixes() []netip.Prefix { + if a == nil { + return nil + } + return append([]netip.Prefix(nil), a.prefixes...) +} diff --git a/internal/netguard/guard.go b/internal/netguard/guard.go new file mode 100644 index 000000000..bcde05e32 --- /dev/null +++ b/internal/netguard/guard.go @@ -0,0 +1,165 @@ +// Package netguard keeps outbound requests to user-supplied URLs away from +// internal networks. A URL is vetted when it is accepted (Guard.CheckURL) +// and again on every new connection (Guard.DialContext): every address a +// connection is about to be made to must be globally reachable per the IANA +// special-purpose registries or allowlisted, so a DNS answer that changes in +// between (rebinding) can't redirect it. Non-ASCII host names must be given +// in punycode. +package netguard + +import ( + "context" + "errors" + "fmt" + "net" + "net/netip" + "net/url" + "strings" + "syscall" + "time" +) + +const ( + dialTimeout = 30 * time.Second + dialKeepAlive = 30 * time.Second +) + +// Resolver resolves host names. *net.Resolver satisfies it. +type Resolver interface { + LookupNetIP(ctx context.Context, network, host string) ([]netip.Addr, error) +} + +// Guard decides which addresses outbound connections may reach: globally +// reachable ones (IsGlobal) and allowlisted ones. CheckURL fails a host if +// any resolved address is refused; DialContext skips refused addresses and +// fails only if none is left. Scheme and port don't matter. +// +// The zero value is usable: no allowlist and net.DefaultResolver. A Guard is +// safe for concurrent use; don't change its fields once it is in use. +type Guard struct { + Allowlist *Allowlist + // Resolver defaults to net.DefaultResolver. Names are always looked up + // in rooted form ("example.com."), so CheckURL and DialContext resolve + // the same name regardless of resolv.conf search domains. DialContext + // uses it only if it is a *net.Resolver. + Resolver Resolver +} + +// CheckURL vets a URL before it is accepted. IP-literal hosts are checked as +// they are; names are resolved and every address in the answer must pass. +// Passing CheckURL guarantees nothing later, since DNS can change: +// DialContext checks again on every new connection. +// +// A non-ASCII host is refused rather than guessed at. Callers must map every +// error (resolver failure, empty answer, non-global address) to the same +// client-visible result, or the check becomes an oracle for internal DNS. +// Policy refusals are *AddressNotAllowedError; resolver errors pass through +// unchanged. +func (g *Guard) CheckURL(ctx context.Context, u *url.URL) error { + if u == nil { + return errors.New("netguard: nil URL") + } + host := u.Hostname() + if host == "" { + return errors.New("netguard: URL has no host") + } + addrs, err := g.resolve(ctx, host) + if err != nil { + return err + } + return g.checkAddrs(host, addrs) +} + +// DialContext is an http.Transport DialContext: a net.Dialer with the +// default transport's timeouts whose Control hook refuses each address +// before the socket connects to it. The TLS server name and Host header +// stay the original host name, since the transport derives them from the +// request URL. +func (g *Guard) DialContext(ctx context.Context, network, address string) (net.Conn, error) { + switch network { + case "tcp", "tcp4", "tcp6": + default: + return nil, &net.OpError{Op: "dial", Net: network, Err: net.UnknownNetworkError(network)} + } + host, port, err := net.SplitHostPort(address) + if err != nil { + return nil, &net.OpError{Op: "dial", Net: network, Err: err} + } + if _, err := netip.ParseAddr(host); err != nil { + fqdn, err := rootedHost(host) + if err != nil { + return nil, &net.OpError{Op: "dial", Net: network, Err: err} + } + address = net.JoinHostPort(fqdn, port) + } + d := &net.Dialer{ + Timeout: dialTimeout, + KeepAlive: dialKeepAlive, + Control: func(_, address string, _ syscall.RawConn) error { + ap, err := netip.ParseAddrPort(address) + if err != nil { + return &AddressNotAllowedError{Host: host, Reason: ReasonNotGlobal} + } + return g.checkAddr(host, ap.Addr()) + }, + } + if r, ok := g.Resolver.(*net.Resolver); ok { + d.Resolver = r + } + return d.DialContext(ctx, network, address) +} + +// rootedHost returns host in rooted form, refusing non-ASCII names. +func rootedHost(host string) (string, error) { + for i := 0; i < len(host); i++ { + if host[i] >= 0x80 { + return "", fmt.Errorf("netguard: host %q is not ASCII (IDNA-encode it first)", host) + } + } + if !strings.HasSuffix(host, ".") { + host += "." + } + return host, nil +} + +func (g *Guard) resolve(ctx context.Context, host string) ([]netip.Addr, error) { + if host == "" { + return nil, errors.New("netguard: empty host") + } + if addr, err := netip.ParseAddr(host); err == nil { + return []netip.Addr{addr}, nil + } + fqdn, err := rootedHost(host) + if err != nil { + return nil, err + } + var r Resolver = net.DefaultResolver + if g.Resolver != nil { + r = g.Resolver + } + addrs, err := r.LookupNetIP(ctx, "ip", fqdn) + if err != nil { + return nil, err + } + if len(addrs) == 0 { + return nil, &AddressNotAllowedError{Host: host, Reason: ReasonNoAddresses} + } + return addrs, nil +} + +func (g *Guard) checkAddrs(host string, addrs []netip.Addr) error { + for _, addr := range addrs { + if err := g.checkAddr(host, addr); err != nil { + return err + } + } + return nil +} + +func (g *Guard) checkAddr(host string, addr netip.Addr) error { + addr = addr.Unmap() + if g.Allowlist.Contains(addr) || IsGlobal(addr) { + return nil + } + return &AddressNotAllowedError{Host: host, Addr: addr, Reason: ReasonNotGlobal} +} diff --git a/internal/netguard/guard_test.go b/internal/netguard/guard_test.go new file mode 100644 index 000000000..e724ed461 --- /dev/null +++ b/internal/netguard/guard_test.go @@ -0,0 +1,365 @@ +package netguard_test + +import ( + "context" + "errors" + "net" + "net/netip" + "net/url" + "strings" + "sync" + "testing" + + "github.com/hookdeck/outpost/internal/netguard" + "github.com/hookdeck/outpost/internal/netguard/netguardtest" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// fakeResolver answers CheckURL lookups from a table keyed by rooted name +// and records every query. +type fakeResolver struct { + mu sync.Mutex + answers map[string][]string + errs map[string]error + queries []string +} + +func newResolver() *fakeResolver { + return &fakeResolver{answers: map[string][]string{}, errs: map[string]error{}} +} + +// set maps host (without the trailing dot) to its answer. +func (r *fakeResolver) set(host string, answer []string) *fakeResolver { + r.answers[host+"."] = answer + return r +} + +func (r *fakeResolver) LookupNetIP(_ context.Context, _, host string) ([]netip.Addr, error) { + r.mu.Lock() + defer r.mu.Unlock() + r.queries = append(r.queries, host) + if err, ok := r.errs[host]; ok { + return nil, err + } + answer, ok := r.answers[host] + if !ok { + return nil, &net.DNSError{Err: "no such host", Name: host, IsNotFound: true} + } + var out []netip.Addr + for _, s := range answer { + out = append(out, netip.MustParseAddr(s)) + } + return out, nil +} + +func (r *fakeResolver) Queries() []string { + r.mu.Lock() + defer r.mu.Unlock() + return append([]string(nil), r.queries...) +} + +func mustAllowlist(t *testing.T, entries ...string) *netguard.Allowlist { + t.Helper() + al, err := netguard.ParseAllowlist(entries) + require.NoError(t, err) + return al +} + +func requireNotAllowed(t *testing.T, err error, reason string) *netguard.AddressNotAllowedError { + t.Helper() + require.Error(t, err) + e, ok := netguard.AsAddressNotAllowed(err) + require.True(t, ok, "want *AddressNotAllowedError, got %T: %v", err, err) + assert.Equal(t, reason, e.Reason) + assert.True(t, netguard.IsAddressNotAllowed(err)) + return e +} + +func TestCheckURL_NoAllowlist(t *testing.T) { + t.Parallel() + res := newResolver(). + set("public.example", []string{"93.184.216.34", "2606:2800:220:1:248:1893:25c8:1946"}). + set("private.example", []string{"10.0.0.5"}). + set("mixed.example", []string{"93.184.216.34", "10.0.0.5"}). + set("mixed6.example", []string{"2606:2800:220:1::1", "fd00::1"}). + set("metadata.example", []string{"169.254.169.254"}). + set("mapped.example", []string{"::ffff:10.0.0.1"}). + set("nat64.example", []string{"64:ff9b::7f00:1"}). + set("sixtofour.example", []string{"2002:a00:1::1"}). + set("loopback.example", []string{"127.0.0.1"}). + set("empty.example", []string{}). + set("localhost.example", []string{"127.0.0.1"}) + res.errs["servfail.example."] = &net.DNSError{Err: "server misbehaving", Name: "servfail.example.", IsTemporary: true} + g := &netguard.Guard{Resolver: res} + + cases := []struct { + url string + reason string // "" = allowed; "dns" = resolver error passes through + addr string + }{ + {"https://8.8.8.8/", "", ""}, + {"https://[2001:4860:4860::8888]:8443/x", "", ""}, + {"https://[::ffff:8.8.8.8]/", "", ""}, + {"https://public.example/hook", "", ""}, + {"https://public.example./hook", "", ""}, + {"https://10.0.0.1/", netguard.ReasonNotGlobal, "10.0.0.1"}, + {"https://127.0.0.1:8080/", netguard.ReasonNotGlobal, "127.0.0.1"}, + {"https://0.0.0.0/", netguard.ReasonNotGlobal, "0.0.0.0"}, + {"https://[::1]/", netguard.ReasonNotGlobal, "::1"}, + {"https://[::ffff:127.0.0.1]/", netguard.ReasonNotGlobal, "127.0.0.1"}, + {"https://[fe80::1%25eth0]/", netguard.ReasonNotGlobal, "fe80::1%eth0"}, + {"https://[::]/", netguard.ReasonNotGlobal, "::"}, + {"https://255.255.255.255/", netguard.ReasonNotGlobal, "255.255.255.255"}, + {"https://224.0.0.1/", netguard.ReasonNotGlobal, "224.0.0.1"}, + {"https://[ff02::1]/", netguard.ReasonNotGlobal, "ff02::1"}, + {"https://[fec0::1]/", netguard.ReasonNotGlobal, "fec0::1"}, + {"https://100.64.0.1/", netguard.ReasonNotGlobal, "100.64.0.1"}, + {"https://192.0.0.9/", "", ""}, + {"https://[64:ff9b::a00:1]/", netguard.ReasonNotGlobal, "64:ff9b::a00:1"}, + {"https://private.example/", netguard.ReasonNotGlobal, "10.0.0.5"}, + {"https://mixed.example/", netguard.ReasonNotGlobal, "10.0.0.5"}, + {"https://mixed6.example/", netguard.ReasonNotGlobal, "fd00::1"}, + {"https://metadata.example/", netguard.ReasonNotGlobal, "169.254.169.254"}, + {"https://mapped.example/", netguard.ReasonNotGlobal, "10.0.0.1"}, + {"https://nat64.example/", netguard.ReasonNotGlobal, "64:ff9b::7f00:1"}, + {"https://sixtofour.example/", netguard.ReasonNotGlobal, "2002:a00:1::1"}, + {"https://loopback.example/", netguard.ReasonNotGlobal, "127.0.0.1"}, + {"https://empty.example/", netguard.ReasonNoAddresses, ""}, + {"https://nxdomain.example/", "dns", ""}, + {"https://servfail.example/", "dns", ""}, + {"http://127.0.0.1:8080/", netguard.ReasonNotGlobal, "127.0.0.1"}, + {"http://[::1]/", netguard.ReasonNotGlobal, "::1"}, + {"http://[::ffff:127.0.0.1]/", netguard.ReasonNotGlobal, "127.0.0.1"}, + {"http://0.0.0.0/", netguard.ReasonNotGlobal, "0.0.0.0"}, + {"https://localhost.example./", netguard.ReasonNotGlobal, "127.0.0.1"}, + // The scheme plays no part. + {"http://8.8.8.8/", "", ""}, + {"http://[2001:4860:4860::8888]:8443/", "", ""}, + {"http://public.example/", "", ""}, + {"ftp://8.8.8.8/", "", ""}, + {"ftp://public.example/", "", ""}, + } + for _, tc := range cases { + u, err := url.Parse(tc.url) + require.NoError(t, err, tc.url) + err = g.CheckURL(context.Background(), u) + switch tc.reason { + case "": + assert.NoError(t, err, tc.url) + case "dns": + var dnsErr *net.DNSError + assert.True(t, errors.As(err, &dnsErr), "%s: %v", tc.url, err) + assert.False(t, netguard.IsAddressNotAllowed(err), tc.url) + default: + e := requireNotAllowed(t, err, tc.reason) + if tc.addr != "" { + assert.Equal(t, netip.MustParseAddr(tc.addr), e.Addr, tc.url) + } else { + assert.False(t, e.Addr.IsValid(), tc.url) + } + assert.Equal(t, u.Hostname(), e.Host, tc.url) + if _, literal := netip.ParseAddr(e.Host); literal != nil && e.Addr.IsValid() { + assert.NotContains(t, e.Error(), e.Addr.String(), "Error() leaves the resolved address out") + } + } + } + + // Names are looked up rooted; IP literals never reach the resolver. + for _, q := range res.Queries() { + assert.True(t, strings.HasSuffix(q, ".") && !strings.HasSuffix(q, ".."), q) + } + assert.NotContains(t, res.Queries(), "8.8.8.8.") + assert.Equal(t, 4, countOf(res.Queries(), "public.example."), "once per public.example URL") +} + +func countOf(list []string, s string) int { + n := 0 + for _, v := range list { + if v == s { + n++ + } + } + return n +} + +func TestCheckURL_ErrorShapes(t *testing.T) { + t.Parallel() + res := newResolver() + g := &netguard.Guard{Resolver: res} + assert.Error(t, g.CheckURL(context.Background(), nil)) + assert.Error(t, g.CheckURL(context.Background(), &url.URL{Scheme: "https", Path: "/x"})) + + // A non-ASCII host is refused, never handed to the resolver half-encoded. + err := g.CheckURL(context.Background(), &url.URL{Scheme: "https", Host: "bücher.example"}) + require.Error(t, err) + assert.False(t, netguard.IsAddressNotAllowed(err)) + assert.Empty(t, res.Queries()) +} + +func TestCheckURL_Allowlist(t *testing.T) { + t.Parallel() + res := newResolver(). + set("localhost", []string{"127.0.0.1", "::1"}). + set("internal.example", []string{"10.2.2.2"}). + set("mapped.example", []string{"::ffff:10.2.2.2"}). + set("other.example", []string{"10.3.3.3"}) + + exact := &netguard.Guard{Resolver: res, Allowlist: mustAllowlist(t, "127.0.0.1/32")} + assert.NoError(t, exact.CheckURL(context.Background(), mustURL(t, "http://127.0.0.1:9000/"))) + requireNotAllowed(t, exact.CheckURL(context.Background(), mustURL(t, "http://127.0.0.2:9000/")), netguard.ReasonNotGlobal) + + private := &netguard.Guard{Resolver: res, Allowlist: mustAllowlist(t, "10.2.0.0/16")} + assert.NoError(t, private.CheckURL(context.Background(), mustURL(t, "https://10.2.1.1/"))) + assert.NoError(t, private.CheckURL(context.Background(), mustURL(t, "https://internal.example/"))) + assert.NoError(t, private.CheckURL(context.Background(), mustURL(t, "https://mapped.example/"))) + requireNotAllowed(t, private.CheckURL(context.Background(), mustURL(t, "https://other.example/")), netguard.ReasonNotGlobal) + requireNotAllowed(t, private.CheckURL(context.Background(), mustURL(t, "https://10.3.1.1/")), netguard.ReasonNotGlobal) + + loopback := &netguard.Guard{Resolver: res, Allowlist: mustAllowlist(t, "localhost")} + assert.NoError(t, loopback.CheckURL(context.Background(), mustURL(t, "http://[::1]:9000/"))) + assert.NoError(t, loopback.CheckURL(context.Background(), mustURL(t, "http://127.1.2.3/"))) + assert.NoError(t, loopback.CheckURL(context.Background(), mustURL(t, "http://localhost:9000/"))) + assert.NoError(t, loopback.CheckURL(context.Background(), mustURL(t, "http://localhost.:9000/"))) + requireNotAllowed(t, loopback.CheckURL(context.Background(), mustURL(t, "https://10.0.0.1/")), netguard.ReasonNotGlobal) +} + +// Go's URL parser keeps these forms as host names and netip refuses them, +// so they reach the resolver, and whatever the system resolver makes of +// them, its answer is checked. +func TestCheckURL_ShortIPv4Forms(t *testing.T) { + t.Parallel() + hosts := []string{"0x7f.1", "127.1", "2130706433", "0177.0.0.1", "0x7f000001"} + res := newResolver() + for _, h := range hosts { + _, err := netip.ParseAddr(h) + require.Error(t, err, h) + res.set(h, []string{"127.0.0.1"}) + } + g := &netguard.Guard{Resolver: res} + for _, h := range hosts { + requireNotAllowed(t, g.CheckURL(context.Background(), mustURL(t, "http://"+h+"/")), netguard.ReasonNotGlobal) + } + + // An unresolvable name fails closed. + err := (&netguard.Guard{Resolver: newResolver()}).CheckURL(context.Background(), mustURL(t, "http://127.1/")) + require.Error(t, err) + assert.False(t, netguard.IsAddressNotAllowed(err)) +} + +func TestCheckURL_TrailingDot(t *testing.T) { + t.Parallel() + res := newResolver().set("public.example", []string{"8.8.8.8"}).set("localhost", []string{"127.0.0.1"}) + g := &netguard.Guard{Resolver: res} + assert.NoError(t, g.CheckURL(context.Background(), mustURL(t, "https://public.example./"))) + assert.Equal(t, []string{"public.example."}, res.Queries()) + requireNotAllowed(t, g.CheckURL(context.Background(), mustURL(t, "https://localhost./")), netguard.ReasonNotGlobal) +} + +func mustURL(t *testing.T, raw string) *url.URL { + t.Helper() + u, err := url.Parse(raw) + require.NoError(t, err) + return u +} + +// listener accepts and closes connections on 127.0.0.1 and returns its port. +func listener(t *testing.T) string { + t.Helper() + ln, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + t.Cleanup(func() { _ = ln.Close() }) + go func() { + for { + c, err := ln.Accept() + if err != nil { + return + } + _ = c.Close() + } + }() + _, port, _ := net.SplitHostPort(ln.Addr().String()) + return port +} + +func TestDialContext_RefusesPrivateAddresses(t *testing.T) { + t.Parallel() + dns := netguardtest.NewDNS(t).Set("internal.example", []string{"10.0.0.1"}) + g := &netguard.Guard{Resolver: dns.Resolver} + ctx := context.Background() + + for _, address := range []string{"10.0.0.1:443", "[fd00::1]:443", "[::ffff:10.0.0.1]:443", "127.0.0.1:1"} { + _, err := g.DialContext(ctx, "tcp", address) + requireNotAllowed(t, err, netguard.ReasonNotGlobal) + } + + _, err := g.DialContext(ctx, "tcp", "internal.example:443") + e := requireNotAllowed(t, err, netguard.ReasonNotGlobal) + assert.Equal(t, netip.MustParseAddr("10.0.0.1"), e.Addr) + assert.Equal(t, "internal.example", e.Host) + assert.Equal(t, []string{"internal.example."}, dns.Queries(), "names are looked up in rooted form") +} + +func TestDialContext_Rebinding(t *testing.T) { + t.Parallel() + // Public when the URL is checked, private by the time it is dialed. + dns := netguardtest.NewDNS(t).Set("rebind.example", []string{"93.184.216.34"}, []string{"10.0.0.1"}) + g := &netguard.Guard{Resolver: dns.Resolver} + + require.NoError(t, g.CheckURL(context.Background(), mustURL(t, "https://rebind.example/"))) + _, err := g.DialContext(context.Background(), "tcp", "rebind.example:443") + e := requireNotAllowed(t, err, netguard.ReasonNotGlobal) + assert.Equal(t, netip.MustParseAddr("10.0.0.1"), e.Addr) + assert.Equal(t, []string{"rebind.example.", "rebind.example."}, dns.Queries(), "resolved again at dial time") +} + +func TestDialContext_MixedRecordsNeverConnectToPrivate(t *testing.T) { + t.Parallel() + port := listener(t) + dns := netguardtest.NewDNS(t).Set("mixed.example", []string{"10.0.0.1", "127.0.0.1"}) + + // Without the allowlisted address the private one is all there is. + g := &netguard.Guard{Resolver: dns.Resolver} + _, err := g.DialContext(context.Background(), "tcp", "mixed.example:"+port) + requireNotAllowed(t, err, netguard.ReasonNotGlobal) + + g = &netguard.Guard{Resolver: dns.Resolver, Allowlist: mustAllowlist(t, "127.0.0.1/32")} + conn, err := g.DialContext(context.Background(), "tcp", "mixed.example:"+port) + require.NoError(t, err) + defer conn.Close() + assert.Equal(t, "127.0.0.1", conn.RemoteAddr().(*net.TCPAddr).IP.String()) +} + +func TestDialContext_AllowlistedLoopback(t *testing.T) { + t.Parallel() + port := listener(t) + dns := netguardtest.NewDNS(t).Set("hooks.example", []string{"127.0.0.1"}) + g := &netguard.Guard{Resolver: dns.Resolver, Allowlist: mustAllowlist(t, "localhost")} + + for _, address := range []string{"127.0.0.1:" + port, "hooks.example:" + port} { + conn, err := g.DialContext(context.Background(), "tcp", address) + require.NoError(t, err, address) + conn.Close() + } +} + +func TestDialContext_Arguments(t *testing.T) { + t.Parallel() + g := &netguard.Guard{} + ctx := context.Background() + _, err := g.DialContext(ctx, "udp", "8.8.8.8:53") + assert.Error(t, err) + _, err = g.DialContext(ctx, "tcp", "8.8.8.8") + assert.Error(t, err) + _, err = g.DialContext(ctx, "tcp", "bücher.example:443") + assert.Error(t, err) +} + +func TestDialContext_ResolverErrorPassesThrough(t *testing.T) { + t.Parallel() + g := &netguard.Guard{Resolver: netguardtest.NewDNS(t).Resolver} + _, err := g.DialContext(context.Background(), "tcp", "nxdomain.example:443") + var dnsErr *net.DNSError + require.True(t, errors.As(err, &dnsErr), "%v", err) + assert.True(t, dnsErr.IsNotFound) +} diff --git a/internal/netguard/iana.go b/internal/netguard/iana.go new file mode 100644 index 000000000..f821befe8 --- /dev/null +++ b/internal/netguard/iana.go @@ -0,0 +1,145 @@ +package netguard + +import "net/netip" + +// specialBlock is one row of an IANA special-purpose address registry. +// global mirrors the "Globally Reachable" column: only "True" is global. +// "False", "N/A" and the blank columns of terminated blocks all fail closed. +type specialBlock struct { + prefix netip.Prefix + global bool +} + +// The tables below are transcribed from the IANA IPv4 and IPv6 +// Special-Purpose Address Registries, both last updated 2025-10-09 +// (https://www.iana.org/assignments/iana-ipv4-special-registry/ and +// .../iana-ipv6-special-registry/). The CSVs are kept in testdata/ and a test +// checks the tables against them row for row: to refresh, replace the CSVs +// with the current iana-ipv{4,6}-special-registry-1.csv and update the +// tables until the test passes. +// +// A lookup takes the most specific matching block, which is how the +// registry expresses exceptions: 192.0.0.9/32 is global inside the +// non-global 192.0.0.0/24. +var ipv4Special = []specialBlock{ + {netip.MustParsePrefix("0.0.0.0/8"), false}, // This network + {netip.MustParsePrefix("0.0.0.0/32"), false}, // This host on this network + {netip.MustParsePrefix("10.0.0.0/8"), false}, // Private-Use + {netip.MustParsePrefix("100.64.0.0/10"), false}, // Shared Address Space + {netip.MustParsePrefix("127.0.0.0/8"), false}, // Loopback + {netip.MustParsePrefix("169.254.0.0/16"), false}, // Link Local + {netip.MustParsePrefix("172.16.0.0/12"), false}, // Private-Use + {netip.MustParsePrefix("192.0.0.0/24"), false}, // IETF Protocol Assignments + {netip.MustParsePrefix("192.0.0.0/29"), false}, // IPv4 Service Continuity Prefix + {netip.MustParsePrefix("192.0.0.8/32"), false}, // IPv4 dummy address + {netip.MustParsePrefix("192.0.0.9/32"), true}, // Port Control Protocol Anycast + {netip.MustParsePrefix("192.0.0.10/32"), true}, // Traversal Using Relays around NAT Anycast + {netip.MustParsePrefix("192.0.0.170/32"), false}, // NAT64/DNS64 Discovery + {netip.MustParsePrefix("192.0.0.171/32"), false}, // NAT64/DNS64 Discovery + {netip.MustParsePrefix("192.0.2.0/24"), false}, // Documentation (TEST-NET-1) + {netip.MustParsePrefix("192.31.196.0/24"), true}, // AS112-v4 + {netip.MustParsePrefix("192.52.193.0/24"), true}, // AMT + // Terminated 2015-03 (RFC 7526); the registry leaves its columns blank. + {netip.MustParsePrefix("192.88.99.0/24"), false}, // Deprecated (6to4 Relay Anycast) + {netip.MustParsePrefix("192.88.99.2/32"), false}, // 6a44-relay anycast address + {netip.MustParsePrefix("192.168.0.0/16"), false}, // Private-Use + {netip.MustParsePrefix("192.175.48.0/24"), true}, // Direct Delegation AS112 Service + {netip.MustParsePrefix("198.18.0.0/15"), false}, // Benchmarking + {netip.MustParsePrefix("198.51.100.0/24"), false}, // Documentation (TEST-NET-2) + {netip.MustParsePrefix("203.0.113.0/24"), false}, // Documentation (TEST-NET-3) + {netip.MustParsePrefix("240.0.0.0/4"), false}, // Reserved + {netip.MustParsePrefix("255.255.255.255/32"), false}, // Limited Broadcast + // Not in the special-purpose registry: multicast (IANA IPv4 Multicast + // Address Space Registry) is never a unicast destination. + {netip.MustParsePrefix("224.0.0.0/4"), false}, // Multicast +} + +// ipv6Special leaves out three registry rows that IsGlobal judges by the +// IPv4 address they carry instead: ::ffff:0:0/96 (IPv4-mapped; Go dials it as +// plain IPv4), 64:ff9b::/96 (NAT64, "True" in the registry, but a NAT64 +// gateway forwards to whatever IPv4 address is embedded) and 2002::/16 (6to4, +// "N/A"; a 6to4 router tunnels to the embedded IPv4 address). +var ipv6Special = []specialBlock{ + {netip.MustParsePrefix("::1/128"), false}, // Loopback Address + {netip.MustParsePrefix("::/128"), false}, // Unspecified Address + {netip.MustParsePrefix("64:ff9b:1::/48"), false}, // IPv4-IPv6 Translat. + {netip.MustParsePrefix("100::/64"), false}, // Discard-Only Address Block + {netip.MustParsePrefix("100:0:0:1::/64"), false}, // Dummy IPv6 Prefix + {netip.MustParsePrefix("2001::/23"), false}, // IETF Protocol Assignments + {netip.MustParsePrefix("2001::/32"), false}, // TEREDO // "N/A" + {netip.MustParsePrefix("2001:1::1/128"), true}, // Port Control Protocol Anycast + {netip.MustParsePrefix("2001:1::2/128"), true}, // Traversal Using Relays around NAT Anycast + {netip.MustParsePrefix("2001:1::3/128"), true}, // DNS-SD Service Registration Protocol Anycast + {netip.MustParsePrefix("2001:2::/48"), false}, // Benchmarking + {netip.MustParsePrefix("2001:3::/32"), true}, // AMT + {netip.MustParsePrefix("2001:4:112::/48"), true}, // AS112-v6 + // Terminated 2014-03 (RFC 4843); the registry leaves its columns blank. + {netip.MustParsePrefix("2001:10::/28"), false}, // Deprecated (previously ORCHID) + {netip.MustParsePrefix("2001:20::/28"), true}, // ORCHIDv2 + {netip.MustParsePrefix("2001:30::/28"), true}, // Drone Remote ID Protocol Entity Tags (DETs) Prefix + {netip.MustParsePrefix("2001:db8::/32"), false}, // Documentation + {netip.MustParsePrefix("2620:4f:8000::/48"), true}, // Direct Delegation AS112 Service + {netip.MustParsePrefix("3fff::/20"), false}, // Documentation + {netip.MustParsePrefix("5f00::/16"), false}, // Segment Routing (SRv6) SIDs + {netip.MustParsePrefix("fc00::/7"), false}, // Unique-Local + {netip.MustParsePrefix("fe80::/10"), false}, // Link-Local Unicast +} + +var ( + ipv4Mapped = netip.MustParsePrefix("::ffff:0:0/96") + nat64 = netip.MustParsePrefix("64:ff9b::/96") + sixToFour = netip.MustParsePrefix("2002::/16") + // globalUnicast is the only IPv6 space the IANA IPv6 Address Space + // Registry allocates for global unicast; everything outside it (fec0::/10 + // site-local, ff00::/8 multicast, the reserved blocks) is never global. + globalUnicast = netip.MustParsePrefix("2000::/3") +) + +// IsGlobal reports whether addr is globally reachable per the IANA +// special-purpose address registries. Unlike netip.Addr.IsGlobalUnicast it +// rejects shared address space, documentation, benchmarking and reserved +// ranges, and it judges IPv4-mapped (::ffff:0:0/96), NAT64 (64:ff9b::/96) +// and 6to4 (2002::/16) addresses by the IPv4 address they embed. Invalid +// addresses and addresses with a zone are never global. +func IsGlobal(addr netip.Addr) bool { + if !addr.IsValid() || addr.Zone() != "" { + return false + } + if addr.Is4() { + b, ok := mostSpecific(ipv4Special, addr) + return !ok || b.global + } + if v4, ok := embeddedIPv4(addr); ok { + return IsGlobal(v4) + } + if b, ok := mostSpecific(ipv6Special, addr); ok { + return b.global + } + return globalUnicast.Contains(addr) +} + +// embeddedIPv4 extracts the IPv4 address an IPv4-mapped, NAT64 or 6to4 +// address stands for. +func embeddedIPv4(addr netip.Addr) (netip.Addr, bool) { + a := addr.As16() + switch { + case ipv4Mapped.Contains(addr), nat64.Contains(addr): + return netip.AddrFrom4([4]byte(a[12:16])), true + case sixToFour.Contains(addr): + return netip.AddrFrom4([4]byte(a[2:6])), true + } + return netip.Addr{}, false +} + +func mostSpecific(blocks []specialBlock, addr netip.Addr) (specialBlock, bool) { + best := -1 + for i := range blocks { + if blocks[i].prefix.Contains(addr) && (best < 0 || blocks[i].prefix.Bits() > blocks[best].prefix.Bits()) { + best = i + } + } + if best < 0 { + return specialBlock{}, false + } + return blocks[best], true +} diff --git a/internal/netguard/iana_test.go b/internal/netguard/iana_test.go new file mode 100644 index 000000000..c703fc7c2 --- /dev/null +++ b/internal/netguard/iana_test.go @@ -0,0 +1,365 @@ +package netguard_test + +import ( + "encoding/csv" + "net/netip" + "os" + "regexp" + "strings" + "testing" + + "github.com/hookdeck/outpost/internal/netguard" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Every registry entry with its first and last address and, where the +// neighbour isn't itself special, the addresses just outside it. +func TestIsGlobal_Table(t *testing.T) { + t.Parallel() + cases := []struct { + addr string + global bool + }{ + // Plain public addresses. + {"1.1.1.1", true}, + {"8.8.8.8", true}, + {"93.184.216.34", true}, + {"2001:4860:4860::8888", true}, + {"2606:4700:4700::1111", true}, + + // 0.0.0.0/8 "This network", 0.0.0.0/32. + {"0.0.0.0", false}, + {"0.0.0.1", false}, + {"0.255.255.255", false}, + {"1.0.0.0", true}, + // 10.0.0.0/8 Private-Use. + {"9.255.255.255", true}, + {"10.0.0.0", false}, + {"10.255.255.255", false}, + {"11.0.0.0", true}, + // 100.64.0.0/10 Shared Address Space. + {"100.63.255.255", true}, + {"100.64.0.0", false}, + {"100.127.255.255", false}, + {"100.128.0.0", true}, + // 127.0.0.0/8 Loopback. + {"126.255.255.255", true}, + {"127.0.0.0", false}, + {"127.0.0.1", false}, + {"127.255.255.255", false}, + {"128.0.0.0", true}, + // 169.254.0.0/16 Link Local (cloud metadata lives here). + {"169.253.255.255", true}, + {"169.254.0.0", false}, + {"169.254.169.254", false}, + {"169.254.255.255", false}, + {"169.255.0.0", true}, + // 172.16.0.0/12 Private-Use. + {"172.15.255.255", true}, + {"172.16.0.0", false}, + {"172.31.255.255", false}, + {"172.32.0.0", true}, + // 192.0.0.0/24 IETF Protocol Assignments with its /29, /32 rows and + // the two global anycast exceptions. + {"191.255.255.255", true}, + {"192.0.0.0", false}, + {"192.0.0.7", false}, + {"192.0.0.8", false}, + {"192.0.0.9", true}, + {"192.0.0.10", true}, + {"192.0.0.11", false}, + {"192.0.0.170", false}, + {"192.0.0.171", false}, + {"192.0.0.255", false}, + {"192.0.1.0", true}, + // 192.0.2.0/24 TEST-NET-1. + {"192.0.2.0", false}, + {"192.0.2.255", false}, + {"192.0.3.0", true}, + // 192.31.196.0/24 AS112-v4 and 192.52.193.0/24 AMT: globally + // reachable per the registry. + {"192.31.196.0", true}, + {"192.31.196.255", true}, + {"192.52.193.0", true}, + {"192.52.193.255", true}, + // 192.88.99.0/24 deprecated 6to4 relay anycast, 192.88.99.2/32 6a44. + {"192.88.98.255", true}, + {"192.88.99.0", false}, + {"192.88.99.1", false}, + {"192.88.99.2", false}, + {"192.88.99.255", false}, + {"192.88.100.0", true}, + // 192.168.0.0/16 Private-Use. + {"192.167.255.255", true}, + {"192.168.0.0", false}, + {"192.168.255.255", false}, + {"192.169.0.0", true}, + // 192.175.48.0/24 Direct Delegation AS112: globally reachable. + {"192.175.48.0", true}, + {"192.175.48.255", true}, + // 198.18.0.0/15 Benchmarking. + {"198.17.255.255", true}, + {"198.18.0.0", false}, + {"198.19.255.255", false}, + {"198.20.0.0", true}, + // 198.51.100.0/24 TEST-NET-2. + {"198.51.99.255", true}, + {"198.51.100.0", false}, + {"198.51.100.255", false}, + {"198.51.101.0", true}, + // 203.0.113.0/24 TEST-NET-3. + {"203.0.112.255", true}, + {"203.0.113.0", false}, + {"203.0.113.255", false}, + {"203.0.114.0", true}, + // 224.0.0.0/4 multicast (outside the registry), 240.0.0.0/4 Reserved, + // 255.255.255.255/32 Limited Broadcast. + {"223.255.255.255", true}, + {"224.0.0.0", false}, + {"224.0.0.1", false}, + {"100.64.0.1", false}, + {"::127.0.0.1", false}, // IPv4-compatible + {"::ffff:127.0.0.1", false}, + {"239.255.255.255", false}, + {"240.0.0.0", false}, + {"255.255.255.254", false}, + {"255.255.255.255", false}, + + // ::/128, ::1/128 and the rest of ::/8 (outside 2000::/3). + {"::", false}, + {"::1", false}, + {"::2", false}, + // IPv4-compatible (deprecated) and SIIT IPv4-translated addresses + // are not unwrapped: they sit outside 2000::/3. + {"::a00:1", false}, + {"::7f00:1", false}, + {"::808:808", false}, + {"::ffff:0:7f00:1", false}, + {"::ffff:0:808:808", false}, + // ::ffff:0:0/96 IPv4-mapped: judged by the embedded address. + {"::ffff:0.0.0.0", false}, + {"::ffff:10.0.0.1", false}, + {"::ffff:127.0.0.1", false}, + {"::ffff:169.254.169.254", false}, + {"::ffff:8.8.8.8", true}, + {"::ffff:255.255.255.255", false}, + // 64:ff9b::/96 NAT64: judged by the embedded address. + {"64:ff9b::", false}, + {"64:ff9b::a00:1", false}, + {"64:ff9b::7f00:1", false}, + {"64:ff9b::c000:9", true}, + {"64:ff9b::808:808", true}, + {"64:ff9b::ffff:ffff", false}, + {"64:ff9b::1:0:0", false}, + // 64:ff9b:1::/48 local-use NAT64. + {"64:ff9b:1::", false}, + {"64:ff9b:1::808:808", false}, + {"64:ff9b:1:ffff:ffff:ffff:ffff:ffff", false}, + // 100::/64 Discard-Only, 100:0:0:1::/64 Dummy IPv6 Prefix. + {"100::", false}, + {"100::ffff:ffff:ffff:ffff", false}, + {"100:0:0:1::", false}, + {"100:0:0:1:ffff:ffff:ffff:ffff", false}, + // 2000::/3 boundaries. + {"1fff:ffff:ffff:ffff:ffff:ffff:ffff:ffff", false}, + {"2000::", true}, + {"3fff:1000::", true}, + {"4000::", false}, + // 2001::/23 IETF Protocol Assignments, with 2001::/32 TEREDO ("N/A"). + {"2001::", false}, + {"2001:0:4136:e378:8000:63bf:3fff:fdd2", false}, + {"2001:0:ffff:ffff:ffff:ffff:ffff:ffff", false}, + {"2001:1::", false}, + {"2001:1::1", true}, + {"2001:1::2", true}, + {"2001:1::3", true}, + {"2001:1::4", false}, + {"2001:2::", false}, + {"2001:2:0:ffff:ffff:ffff:ffff:ffff", false}, + {"2001:2:1::", false}, + {"2001:3::", true}, + {"2001:3:ffff:ffff:ffff:ffff:ffff:ffff", true}, + {"2001:4:111:ffff:ffff:ffff:ffff:ffff", false}, + {"2001:4:112::", true}, + {"2001:4:112:ffff:ffff:ffff:ffff:ffff", true}, + {"2001:4:113::", false}, + {"2001:10::", false}, + {"2001:1f:ffff:ffff:ffff:ffff:ffff:ffff", false}, + {"2001:20::", true}, + {"2001:2f:ffff:ffff:ffff:ffff:ffff:ffff", true}, + {"2001:30::", true}, + {"2001:3f:ffff:ffff:ffff:ffff:ffff:ffff", true}, + {"2001:40::", false}, + {"2001:1ff:ffff:ffff:ffff:ffff:ffff:ffff", false}, + {"2001:200::", true}, + // 2001:db8::/32 Documentation. + {"2001:db7:ffff:ffff:ffff:ffff:ffff:ffff", true}, + {"2001:db8::", false}, + {"2001:db8:ffff:ffff:ffff:ffff:ffff:ffff", false}, + {"2001:db9::", true}, + // 2002::/16 6to4: judged by the embedded address. + {"2002::", false}, + {"2002:a00:1::", false}, + {"2002:7f00:1::1", false}, + {"2002:c0a8:101::1", false}, + {"2002:808:808::", true}, + {"2002:ffff:ffff:ffff:ffff:ffff:ffff:ffff", false}, + // 2620:4f:8000::/48 Direct Delegation AS112. + {"2620:4f:8000::", true}, + {"2620:4f:8000:ffff:ffff:ffff:ffff:ffff", true}, + // 3fff::/20 Documentation. + {"3ffe:ffff:ffff:ffff:ffff:ffff:ffff:ffff", true}, + {"3fff::", false}, + {"3fff:fff:ffff:ffff:ffff:ffff:ffff:ffff", false}, + // 5f00::/16 SRv6 SIDs. + {"5f00::", false}, + {"5f00:ffff:ffff:ffff:ffff:ffff:ffff:ffff", false}, + // fc00::/7 Unique-Local, fe80::/10 Link-Local, fec0::/10 site-local, + // ff00::/8 multicast. + {"fc00::", false}, + {"fd00::1", false}, + {"fdff:ffff:ffff:ffff:ffff:ffff:ffff:ffff", false}, + {"fe80::", false}, + {"fe80::1", false}, + {"febf:ffff:ffff:ffff:ffff:ffff:ffff:ffff", false}, + {"fec0::1", false}, + {"ff02::1", false}, + {"ff0e::1", false}, + // A zone is never global, even on a global address. + {"fe80::1%eth0", false}, + {"2001:4860:4860::8888%eth0", false}, + } + for _, tc := range cases { + addr := netip.MustParseAddr(tc.addr) + assert.Equal(t, tc.global, netguard.IsGlobal(addr), tc.addr) + } + assert.False(t, netguard.IsGlobal(netip.Addr{}), "zero Addr") +} + +// registryRow is one row of an IANA special-purpose registry CSV. +type registryRow struct { + prefix netip.Prefix + reachable string // "True", "False", "N/A" or "" (terminated) +} + +var footnote = regexp.MustCompile(`\s*\[\d+\]`) + +// loadRegistry reads a registry CSV as published by IANA (copies in +// testdata, fetched 2026-10-09; registry last updated 2025-10-09). +func loadRegistry(t *testing.T, path string) []registryRow { + t.Helper() + f, err := os.Open(path) + require.NoError(t, err) + defer f.Close() + records, err := csv.NewReader(f).ReadAll() + require.NoError(t, err) + require.Equal(t, "Address Block", records[0][0]) + require.Equal(t, "Globally Reachable", records[0][8]) + var rows []registryRow + for _, rec := range records[1:] { + reachable := strings.TrimSpace(footnote.ReplaceAllString(rec[8], "")) + for _, block := range strings.Split(footnote.ReplaceAllString(rec[0], ""), ",") { + rows = append(rows, registryRow{netip.MustParsePrefix(strings.TrimSpace(block)), reachable}) + } + } + require.NotEmpty(t, rows) + return rows +} + +var ( + ipv4MappedPrefix = netip.MustParsePrefix("::ffff:0:0/96") + nat64Prefix = netip.MustParsePrefix("64:ff9b::/96") + sixToFourPrefix = netip.MustParsePrefix("2002::/16") + multicastV4 = netip.MustParsePrefix("224.0.0.0/4") + globalUnicastV6 = netip.MustParsePrefix("2000::/3") +) + +// embedsIPv4 reports whether a registry row is one IsGlobal unwraps. +func embedsIPv4(p netip.Prefix) bool { + return p == ipv4MappedPrefix || p == nat64Prefix || p == sixToFourPrefix +} + +// registryOracle answers IsGlobal straight from the registry rows (most +// specific row wins, only "True" is global) plus the documented departures: +// embedded IPv4 for mapped/NAT64/6to4, multicast for IPv4, and 2000::/3 as +// the only global IPv6 space. +func registryOracle(rows []registryRow, addr netip.Addr) bool { + if addr.Is4In6() { + return registryOracle(rows, addr.Unmap()) + } + a := addr.As16() + if nat64Prefix.Contains(addr) { + return registryOracle(rows, netip.AddrFrom4([4]byte(a[12:16]))) + } + if sixToFourPrefix.Contains(addr) { + return registryOracle(rows, netip.AddrFrom4([4]byte(a[2:6]))) + } + best := -1 + for i, r := range rows { + if r.prefix.Contains(addr) && (best < 0 || r.prefix.Bits() > rows[best].prefix.Bits()) { + best = i + } + } + if best >= 0 { + return rows[best].reachable == "True" + } + if addr.Is4() { + return !multicastV4.Contains(addr) + } + return globalUnicastV6.Contains(addr) +} + +func lastAddr(p netip.Prefix) netip.Addr { + b := p.Masked().Addr().AsSlice() + for i := p.Bits(); i < len(b)*8; i++ { + b[i/8] |= 1 << (7 - i%8) + } + a, _ := netip.AddrFromSlice(b) + return a +} + +// TestIsGlobal_MatchesRegistry checks the embedded tables row for row +// against the published registries, then probes every row's edges (and the +// addresses just outside them) against an oracle built from the CSVs. +func TestIsGlobal_MatchesRegistry(t *testing.T) { + t.Parallel() + v4 := loadRegistry(t, "testdata/iana-ipv4-special-registry-1.csv") + v6 := loadRegistry(t, "testdata/iana-ipv6-special-registry-1.csv") + // The oracle needs both: NAT64, 6to4 and mapped rows defer to IPv4 rows. + all := append(append([]registryRow(nil), v4...), v6...) + for _, tc := range []struct { + name string + v6 bool + rows []registryRow + }{ + {"ipv4", false, v4}, + {"ipv6", true, v6}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + rows := tc.rows + + want := make(map[netip.Prefix]bool) + for _, r := range rows { + if !embedsIPv4(r.prefix) { + want[r.prefix] = r.reachable == "True" + } + } + got := netguard.RegistryTable(tc.v6) + delete(got, multicastV4) // not a special-purpose registry row + assert.Equal(t, want, got, "embedded table must match the registry") + + for _, r := range rows { + first, last := r.prefix.Masked().Addr(), lastAddr(r.prefix) + probes := []netip.Addr{first, first.Next(), last, last.Prev(), first.Prev(), last.Next()} + for _, addr := range probes { + if !addr.IsValid() { + continue + } + assert.Equal(t, registryOracle(all, addr), netguard.IsGlobal(addr), "%s (row %s)", addr, r.prefix) + } + } + }) + } +} diff --git a/internal/netguard/netguardtest/dns.go b/internal/netguard/netguardtest/dns.go new file mode 100644 index 000000000..a871b9eb8 --- /dev/null +++ b/internal/netguard/netguardtest/dns.go @@ -0,0 +1,125 @@ +// Package netguardtest provides an in-process DNS server for tests that +// resolve host names through the standard library. +package netguardtest + +import ( + "context" + "encoding/binary" + "net" + "net/netip" + "strings" + "sync" + "testing" +) + +// DNS answers A queries from a table and AAAA queries with an empty answer. +// Names it doesn't know get NXDOMAIN. +type DNS struct { + // Resolver sends every query to this server. + Resolver *net.Resolver + + mu sync.Mutex + answers map[string][][]netip.Addr + calls map[string]int + queries []string +} + +// NewDNS starts a server that stops with the test. +func NewDNS(t testing.TB) *DNS { + t.Helper() + conn, err := net.ListenPacket("udp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + d := &DNS{answers: map[string][][]netip.Addr{}, calls: map[string]int{}} + d.Resolver = &net.Resolver{ + PreferGo: true, + Dial: func(ctx context.Context, network, _ string) (net.Conn, error) { + return (&net.Dialer{}).DialContext(ctx, network, conn.LocalAddr().String()) + }, + } + t.Cleanup(func() { _ = conn.Close() }) + go d.serve(conn) + return d +} + +// Set maps host (no trailing dot) to IPv4 answers. With several answers, the +// nth A query for the host gets the nth one and the last repeats, which is +// how a test changes DNS between a URL check and a dial. +func (d *DNS) Set(host string, answers ...[]string) *DNS { + seq := make([][]netip.Addr, len(answers)) + for i, set := range answers { + for _, s := range set { + seq[i] = append(seq[i], netip.MustParseAddr(s)) + } + } + d.mu.Lock() + d.answers[strings.ToLower(host)+"."] = seq + d.mu.Unlock() + return d +} + +// Queries returns the A-queried names in rooted form, in order. +func (d *DNS) Queries() []string { + d.mu.Lock() + defer d.mu.Unlock() + return append([]string(nil), d.queries...) +} + +func (d *DNS) serve(conn net.PacketConn) { + buf := make([]byte, 512) + for { + n, from, err := conn.ReadFrom(buf) + if err != nil { + return + } + if resp := d.reply(buf[:n]); resp != nil { + _, _ = conn.WriteTo(resp, from) + } + } +} + +func (d *DNS) reply(q []byte) []byte { + if len(q) < 12 { + return nil + } + var name []string + i := 12 + for i < len(q) && q[i] != 0 { + end := i + 1 + int(q[i]) + if end > len(q) { + return nil + } + name = append(name, string(q[i+1:end])) + i = end + } + if i+5 > len(q) { + return nil + } + rooted := strings.ToLower(strings.Join(name, ".")) + "." + qtype := binary.BigEndian.Uint16(q[i+1:]) + question := q[12 : i+5] + + var ips []netip.Addr + rcode := byte(3) + d.mu.Lock() + if seq, ok := d.answers[rooted]; ok { + rcode = 0 + if qtype == 1 { + d.queries = append(d.queries, rooted) + n := d.calls[rooted] + d.calls[rooted]++ + ips = seq[min(n, len(seq)-1)] + } + } + d.mu.Unlock() + + resp := []byte{q[0], q[1], 0x81, 0x80 | rcode, 0, 1, byte(len(ips) >> 8), byte(len(ips)), 0, 0, 0, 0} + resp = append(resp, question...) + for _, ip := range ips { + resp = append(resp, 0xC0, 0x0C, 0, 1, 0, 1, 0, 0, 0, 0, 0, 4) + a := ip.As4() + resp = append(resp, a[:]...) + } + return resp +} diff --git a/internal/netguard/testdata/iana-ipv4-special-registry-1.csv b/internal/netguard/testdata/iana-ipv4-special-registry-1.csv new file mode 100644 index 000000000..99458ca36 --- /dev/null +++ b/internal/netguard/testdata/iana-ipv4-special-registry-1.csv @@ -0,0 +1,27 @@ +Address Block,Name,RFC,Allocation Date,Termination Date,Source,Destination,Forwardable,Globally Reachable,Reserved-by-Protocol +0.0.0.0/8,"""This network""","[RFC791], Section 3.2",1981-09,N/A,True,False,False,False,True +0.0.0.0/32,"""This host on this network""","[RFC1122], Section 3.2.1.3",1981-09,N/A,True,False,False,False,True +10.0.0.0/8,Private-Use,[RFC1918],1996-02,N/A,True,True,True,False,False +100.64.0.0/10,Shared Address Space,[RFC6598],2012-04,N/A,True,True,True,False,False +127.0.0.0/8,Loopback,"[RFC1122], Section 3.2.1.3",1981-09,N/A,False [1],False [1],False [1],False [1],True +169.254.0.0/16,Link Local,[RFC3927],2005-05,N/A,True,True,False,False,True +172.16.0.0/12,Private-Use,[RFC1918],1996-02,N/A,True,True,True,False,False +192.0.0.0/24 [2],IETF Protocol Assignments,"[RFC6890], Section 2.1",2010-01,N/A,False,False,False,False,False +192.0.0.0/29,IPv4 Service Continuity Prefix,[RFC7335],2011-06,N/A,True,True,True,False,False +192.0.0.8/32,IPv4 dummy address,[RFC7600],2015-03,N/A,True,False,False,False,False +192.0.0.9/32,Port Control Protocol Anycast,[RFC7723],2015-10,N/A,True,True,True,True,False +192.0.0.10/32,Traversal Using Relays around NAT Anycast,[RFC8155],2017-02,N/A,True,True,True,True,False +"192.0.0.170/32, 192.0.0.171/32",NAT64/DNS64 Discovery,"[RFC8880][RFC7050], Section 2.2",2013-02,N/A,False,False,False,False,True +192.0.2.0/24,Documentation (TEST-NET-1),[RFC5737],2010-01,N/A,False,False,False,False,False +192.31.196.0/24,AS112-v4,[RFC7535],2014-12,N/A,True,True,True,True,False +192.52.193.0/24,AMT,[RFC7450],2014-12,N/A,True,True,True,True,False +192.88.99.0/24,Deprecated (6to4 Relay Anycast),[RFC7526],2001-06,2015-03,,,,, +192.88.99.2/32,6a44-relay anycast address,[RFC6751],2012-10,N/A,True,True,True,False,False +192.168.0.0/16,Private-Use,[RFC1918],1996-02,N/A,True,True,True,False,False +192.175.48.0/24,Direct Delegation AS112 Service,[RFC7534],1996-01,N/A,True,True,True,True,False +198.18.0.0/15,Benchmarking,[RFC2544],1999-03,N/A,True,True,True,False,False +198.51.100.0/24,Documentation (TEST-NET-2),[RFC5737],2010-01,N/A,False,False,False,False,False +203.0.113.0/24,Documentation (TEST-NET-3),[RFC5737],2010-01,N/A,False,False,False,False,False +240.0.0.0/4,Reserved,"[RFC1112], Section 4",1989-08,N/A,False,False,False,False,True +255.255.255.255/32,Limited Broadcast,"[RFC8190] + [RFC919], Section 7",1984-10,N/A,False,True,False,False,True diff --git a/internal/netguard/testdata/iana-ipv6-special-registry-1.csv b/internal/netguard/testdata/iana-ipv6-special-registry-1.csv new file mode 100644 index 000000000..f5bf9c073 --- /dev/null +++ b/internal/netguard/testdata/iana-ipv6-special-registry-1.csv @@ -0,0 +1,28 @@ +Address Block,Name,RFC,Allocation Date,Termination Date,Source,Destination,Forwardable,Globally Reachable,Reserved-by-Protocol +::1/128,Loopback Address,[RFC4291],2006-02,N/A,False,False,False,False,True +::/128,Unspecified Address,[RFC4291],2006-02,N/A,True,False,False,False,True +::ffff:0:0/96,IPv4-mapped Address,[RFC4291],2006-02,N/A,False,False,False,False,True +64:ff9b::/96,IPv4-IPv6 Translat.,[RFC6052],2010-10,N/A,True,True,True,True,False +64:ff9b:1::/48,IPv4-IPv6 Translat.,[RFC8215],2017-06,N/A,True,True,True,False,False +100::/64,Discard-Only Address Block,[RFC6666],2012-06,N/A,True,True,True,False,False +100:0:0:1::/64,Dummy IPv6 Prefix,[RFC9780],2025-04,N/A,True,False,False,False,False +2001::/23,IETF Protocol Assignments,[RFC2928],2000-09,N/A,False [1],False [1],False [1],False [1],False +2001::/32,TEREDO,"[RFC4380] + [RFC8190]",2006-01,N/A,True,True,True,N/A [2],False +2001:1::1/128,Port Control Protocol Anycast,[RFC7723],2015-10,N/A,True,True,True,True,False +2001:1::2/128,Traversal Using Relays around NAT Anycast,[RFC8155],2017-02,N/A,True,True,True,True,False +2001:1::3/128,DNS-SD Service Registration Protocol Anycast,[RFC9665],2024-04,N/A,True,True,True,True,False +2001:2::/48,Benchmarking,[RFC5180][RFC Errata 1752],2008-04,N/A,True,True,True,False,False +2001:3::/32,AMT,[RFC7450],2014-12,N/A,True,True,True,True,False +2001:4:112::/48,AS112-v6,[RFC7535],2014-12,N/A,True,True,True,True,False +2001:10::/28,Deprecated (previously ORCHID),[RFC4843],2007-03,2014-03,,,,, +2001:20::/28,ORCHIDv2,[RFC7343],2014-07,N/A,True,True,True,True,False +2001:30::/28,Drone Remote ID Protocol Entity Tags (DETs) Prefix,[RFC9374],2022-12,N/A,True,True,True,True,False +2001:db8::/32,Documentation,[RFC3849],2004-07,N/A,False,False,False,False,False +2002::/16 [3],6to4,[RFC3056],2001-02,N/A,True,True,True,N/A [3],False +2620:4f:8000::/48,Direct Delegation AS112 Service,[RFC7534],2011-05,N/A,True,True,True,True,False +3fff::/20,Documentation,[RFC9637],2024-07,N/A,False,False,False,False,False +5f00::/16,Segment Routing (SRv6) SIDs,[RFC9602],2024-04,N/A,True,True,True,False,False +fc00::/7,Unique-Local,"[RFC4193] + [RFC8190]",2005-10,N/A,True,True,True,False [4],False +fe80::/10,Link-Local Unicast,[RFC4291],2006-02,N/A,True,True,False,False,True diff --git a/internal/netguard/transport.go b/internal/netguard/transport.go new file mode 100644 index 000000000..58da845bf --- /dev/null +++ b/internal/netguard/transport.go @@ -0,0 +1,42 @@ +package netguard + +import ( + "errors" + "net/http" +) + +// WrapTransport returns a RoundTripper that checks each request's target URL +// with CheckURL before it goes to base. It is for a base that connects +// through a forward proxy, which resolves the target itself and whose own +// address is not guarded. The request is never modified. +func (g *Guard) WrapTransport(base http.RoundTripper) http.RoundTripper { + return &guardTransport{base: base, guard: g} +} + +type guardTransport struct { + base http.RoundTripper + guard *Guard +} + +func (t *guardTransport) RoundTrip(req *http.Request) (*http.Response, error) { + closeBody := func() { + if req.Body != nil { + _ = req.Body.Close() + } + } + if req.URL == nil { + closeBody() + return nil, errors.New("netguard: nil Request.URL") + } + if err := t.guard.CheckURL(req.Context(), req.URL); err != nil { + closeBody() + return nil, err + } + return t.base.RoundTrip(req) +} + +func (t *guardTransport) CloseIdleConnections() { + if c, ok := t.base.(interface{ CloseIdleConnections() }); ok { + c.CloseIdleConnections() + } +} diff --git a/internal/netguard/transport_test.go b/internal/netguard/transport_test.go new file mode 100644 index 000000000..5019abc00 --- /dev/null +++ b/internal/netguard/transport_test.go @@ -0,0 +1,167 @@ +package netguard_test + +import ( + "crypto/tls" + "crypto/x509" + "io" + "net" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "sync" + "sync/atomic" + "testing" + + "github.com/hookdeck/outpost/internal/netguard" + "github.com/hookdeck/outpost/internal/netguard/netguardtest" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func get(t *testing.T, rt http.RoundTripper, rawURL string) (*http.Response, error) { + t.Helper() + req, err := http.NewRequest(http.MethodGet, rawURL, nil) + require.NoError(t, err) + return rt.RoundTrip(req) +} + +type roundTripFunc func(*http.Request) (*http.Response, error) + +func (f roundTripFunc) RoundTrip(r *http.Request) (*http.Response, error) { return f(r) } + +type closeTracker struct { + io.Reader + closed atomic.Bool +} + +func (c *closeTracker) Close() error { + c.closed.Store(true) + return nil +} + +// proxyRecorder is an httptest server standing in for a forward proxy: it +// records what it is asked to forward and answers 502. +type proxyRecorder struct { + *httptest.Server + mu sync.Mutex + reqs []string +} + +func newProxy(t *testing.T) *proxyRecorder { + p := &proxyRecorder{} + p.Server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + p.mu.Lock() + p.reqs = append(p.reqs, r.Method+" "+r.Host) + p.mu.Unlock() + w.WriteHeader(http.StatusBadGateway) + })) + t.Cleanup(p.Close) + return p +} + +func (p *proxyRecorder) seen() []string { + p.mu.Lock() + defer p.mu.Unlock() + return append([]string(nil), p.reqs...) +} + +func TestWrapTransport_ProxyMode(t *testing.T) { + t.Parallel() + proxy := newProxy(t) + proxyURL, err := url.Parse(proxy.URL) + require.NoError(t, err) + + // The proxy sits on unallowlisted 127.0.0.1 and its own address is not + // checked. + res := newResolver().set("public.example", []string{"8.8.8.8"}).set("internal.example", []string{"10.0.0.1"}) + g := &netguard.Guard{Resolver: res} + base := &http.Transport{Proxy: http.ProxyURL(proxyURL)} + t.Cleanup(base.CloseIdleConnections) + rt := g.WrapTransport(base) + + // A private target is refused before the proxy sees anything, and the + // request body is closed. + body := &closeTracker{Reader: strings.NewReader("x")} + req, err := http.NewRequest(http.MethodPost, "http://internal.example/hook", body) + require.NoError(t, err) + _, err = rt.RoundTrip(req) + require.Error(t, err) + assert.True(t, netguard.IsAddressNotAllowed(err)) + assert.True(t, body.closed.Load()) + _, err = get(t, rt, "http://10.0.0.1/") + assert.True(t, netguard.IsAddressNotAllowed(err)) + assert.Empty(t, proxy.seen()) + + // A public target is forwarded by name. + resp, err := get(t, rt, "http://public.example/hook") + require.NoError(t, err) + resp.Body.Close() + assert.Equal(t, http.StatusBadGateway, resp.StatusCode) + assert.Equal(t, []string{"GET public.example"}, proxy.seen()) +} + +func TestWrapTransport_NilURL(t *testing.T) { + t.Parallel() + g := &netguard.Guard{} + rt := g.WrapTransport(roundTripFunc(func(*http.Request) (*http.Response, error) { + t.Fatal("must not be reached") + return nil, nil + })) + _, err := rt.RoundTrip(&http.Request{}) + assert.Error(t, err) +} + +// The dial connects to the resolved address while TLS (server name, certificate +// check) and the Host header keep the original name. httptest's certificate +// covers example.com and *.example.com. +func TestDialContext_PreservesSNIAndHost(t *testing.T) { + t.Parallel() + for _, h2 := range []bool{false, true} { + t.Run(map[bool]string{false: "http1", true: "http2"}[h2], func(t *testing.T) { + t.Parallel() + type seen struct{ serverName, host string } + var mu sync.Mutex + var got []seen + srv := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + mu.Lock() + got = append(got, seen{r.TLS.ServerName, r.Host}) + mu.Unlock() + _, _ = io.WriteString(w, r.Proto) + })) + srv.EnableHTTP2 = h2 + srv.StartTLS() + t.Cleanup(srv.Close) + _, port, _ := net.SplitHostPort(srv.Listener.Addr().String()) + roots := x509.NewCertPool() + roots.AddCert(srv.Certificate()) + + dns := netguardtest.NewDNS(t). + Set("hooks.example.com", []string{"127.0.0.1"}). + Set("hooks.other.test", []string{"127.0.0.1"}) + g := &netguard.Guard{Resolver: dns.Resolver, Allowlist: mustAllowlist(t, "127.0.0.0/8")} + base := &http.Transport{ + DialContext: g.DialContext, + ForceAttemptHTTP2: true, + TLSClientConfig: &tls.Config{RootCAs: roots}, + } + t.Cleanup(base.CloseIdleConnections) + rt := base + + resp, err := get(t, rt, "https://hooks.example.com:"+port+"/hook") + require.NoError(t, err) + body, _ := io.ReadAll(resp.Body) + resp.Body.Close() + assert.Equal(t, map[bool]int{false: 1, true: 2}[h2], resp.ProtoMajor, string(body)) + mu.Lock() + require.Len(t, got, 1) + assert.Equal(t, seen{"hooks.example.com", "hooks.example.com:" + port}, got[0]) + mu.Unlock() + + // The certificate is checked against the name, not the IP. + _, err = get(t, rt, "https://hooks.other.test:"+port+"/hook") + require.Error(t, err) + assert.Contains(t, err.Error(), "x509") + }) + } +} From 776b93ac19c5f3331deef169af1d7e2b1187f325 Mon Sep 17 00:00:00 2001 From: Alex Luong Date: Tue, 27 Oct 2026 12:24:02 +0700 Subject: [PATCH 02/13] feat: add address guard option to destination http client --- internal/destregistry/httpclient.go | 16 ++ .../destregistry/httpclient_guard_test.go | 181 ++++++++++++++++++ 2 files changed, 197 insertions(+) create mode 100644 internal/destregistry/httpclient_guard_test.go diff --git a/internal/destregistry/httpclient.go b/internal/destregistry/httpclient.go index a9ffce203..9bb3f1e63 100644 --- a/internal/destregistry/httpclient.go +++ b/internal/destregistry/httpclient.go @@ -7,6 +7,7 @@ import ( "net/url" "time" + "github.com/hookdeck/outpost/internal/netguard" "github.com/hookdeck/outpost/internal/proxychain" ) @@ -34,12 +35,18 @@ type HTTPClientConfig struct { // underlying connection was reused. This is the signal that the pool // ceiling is binding. OnConnection func(reused bool) + + // Guard, if set, limits connections to addresses it allows and ignores + // HTTP_PROXY/HTTPS_PROXY. Through a forward proxy only the target URL is + // checked, so the proxy must refuse private addresses itself. + Guard *netguard.Guard } func (c HTTPClientConfig) needsTransport() bool { return len(c.Proxy) > 0 || c.UserAgent != nil || c.OnConnection != nil || + c.Guard != nil || c.Pool.MaxIdleConns > 0 || c.Pool.MaxIdleConnsPerHost > 0 } @@ -65,6 +72,11 @@ func NewHTTPClient(config HTTPClientConfig) (*http.Client, error) { transport.MaxIdleConnsPerHost = config.Pool.MaxIdleConnsPerHost } + if config.Guard != nil && len(config.Proxy) == 0 { + transport.Proxy = nil + transport.DialContext = config.Guard.DialContext + } + var rt http.RoundTripper = transport if n := len(config.Proxy); n > 0 { @@ -80,6 +92,10 @@ func NewHTTPClient(config HTTPClientConfig) (*http.Client, error) { } } + if config.Guard != nil && len(config.Proxy) > 0 { + rt = config.Guard.WrapTransport(rt) + } + if config.UserAgent != nil { rt = &userAgentTransport{ userAgent: *config.UserAgent, diff --git a/internal/destregistry/httpclient_guard_test.go b/internal/destregistry/httpclient_guard_test.go new file mode 100644 index 000000000..903efa525 --- /dev/null +++ b/internal/destregistry/httpclient_guard_test.go @@ -0,0 +1,181 @@ +package destregistry_test + +import ( + "io" + "net" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "sync/atomic" + "testing" + + "github.com/hookdeck/outpost/internal/destregistry" + "github.com/hookdeck/outpost/internal/netguard" + "github.com/hookdeck/outpost/internal/netguard/netguardtest" + "github.com/hookdeck/outpost/internal/proxychain" + "github.com/hookdeck/outpost/internal/proxychain/proxychaintest" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func guardAllowing(t *testing.T, res *net.Resolver, entries ...string) *netguard.Guard { + t.Helper() + al, err := netguard.ParseAllowlist(entries) + require.NoError(t, err) + return &netguard.Guard{Resolver: res, Allowlist: al} +} + +func newGuardedClient(t *testing.T, cfg destregistry.HTTPClientConfig) *http.Client { + t.Helper() + client, err := destregistry.NewHTTPClient(cfg) + require.NoError(t, err) + t.Cleanup(client.CloseIdleConnections) + return client +} + +func getBody(t *testing.T, client *http.Client, rawURL string) (string, error) { + t.Helper() + resp, err := client.Get(rawURL) + if err != nil { + return "", err + } + defer resp.Body.Close() + b, err := io.ReadAll(resp.Body) + require.NoError(t, err) + return string(b), nil +} + +func TestHTTPClientGuard_DirectDialsThroughGuard(t *testing.T) { + t.Parallel() + dest := newDestination(t, false) + + blocked := newGuardedClient(t, destregistry.HTTPClientConfig{Guard: &netguard.Guard{}}) + _, err := getBody(t, blocked, dest.URL) + require.Error(t, err) + assert.True(t, netguard.IsAddressNotAllowed(err)) + + allowed := newGuardedClient(t, destregistry.HTTPClientConfig{Guard: guardAllowing(t, nil, "127.0.0.1/32")}) + body, err := getBody(t, allowed, dest.URL) + require.NoError(t, err) + assert.Equal(t, "ok", body) +} + +// Not parallel: it sets process environment. +func TestHTTPClientGuard_IgnoresEnvironmentProxy(t *testing.T) { + var proxied atomic.Int32 + proxy := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + proxied.Add(1) + _, _ = io.WriteString(w, "via proxy") + })) + t.Cleanup(proxy.Close) + dest := newDestination(t, false) + for _, k := range []string{"HTTP_PROXY", "HTTPS_PROXY", "http_proxy", "https_proxy", "ALL_PROXY", "all_proxy"} { + t.Setenv(k, proxy.URL) + } + t.Setenv("NO_PROXY", "") + t.Setenv("no_proxy", "") + + // A name, not 127.0.0.1: Go never proxies loopback literals anyway. + _, port, _ := net.SplitHostPort(dest.Listener.Addr().String()) + res := netguardtest.NewDNS(t).Set("hooks.example.com", []string{"127.0.0.1"}).Resolver + client := newGuardedClient(t, destregistry.HTTPClientConfig{Guard: guardAllowing(t, res, "localhost")}) + body, err := getBody(t, client, "http://hooks.example.com:"+port+"/") + require.NoError(t, err) + assert.Equal(t, "ok", body) + assert.Zero(t, proxied.Load()) +} + +func TestHTTPClientGuard_ProxyChain(t *testing.T) { + t.Parallel() + for name, hops := range map[string]int{"one hop": 1, "two hops": 2} { + t.Run(name, func(t *testing.T) { + t.Parallel() + var proxies []*proxychaintest.Proxy + var urls []string + for range hops { + p := proxychaintest.New(t, false) + proxies = append(proxies, p) + urls = append(urls, p.URL) + } + chain, err := proxychain.Parse(strings.Join(urls, " ")) + require.NoError(t, err) + // No target is dialed: the last proxy answers every CONNECT. + proxies[hops-1].Reject = func(string) (int, http.Header) { return http.StatusForbidden, nil } + + res := netguardtest.NewDNS(t).Set("public.example", []string{"8.8.8.8"}).Set("internal.example", []string{"10.0.0.1"}).Resolver + client := newGuardedClient(t, destregistry.HTTPClientConfig{Proxy: chain, Guard: &netguard.Guard{Resolver: res}}) + + // The proxies are on unallowlisted 127.0.0.1 and are reached. + _, err = getBody(t, client, "https://public.example/hook") + require.Error(t, err) + assert.False(t, netguard.IsAddressNotAllowed(err)) + assert.Equal(t, []string{"public.example:443"}, proxies[hops-1].Connects()) + + // A private target is refused before any CONNECT. + for _, target := range []string{"https://internal.example/hook", "https://10.0.0.1/hook", "https://[::1]/hook"} { + _, err = getBody(t, client, target) + require.Error(t, err, target) + assert.True(t, netguard.IsAddressNotAllowed(err), "%s: %v", target, err) + } + assert.Len(t, proxies[hops-1].Connects(), 1) + }) + } +} + +// The guard sits in the transport, so a followed redirect is checked at +// every hop even though the client has no CheckRedirect. +func TestHTTPClientGuard_RedirectToPrivateAddressRefused(t *testing.T) { + t.Parallel() + var landed atomic.Int32 + target := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + landed.Add(1) + })) + t.Cleanup(target.Close) + _, targetPort, _ := net.SplitHostPort(target.Listener.Addr().String()) + redirector := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, "http://internal.example:"+targetPort+"/landed", http.StatusFound) + })) + t.Cleanup(redirector.Close) + + // Only the first hop's address is allowlisted. + res := netguardtest.NewDNS(t).Set("internal.example", []string{"127.0.0.2"}).Resolver + client := newGuardedClient(t, destregistry.HTTPClientConfig{Guard: guardAllowing(t, res, "127.0.0.1/32")}) + _, err := getBody(t, client, redirector.URL) + require.Error(t, err) + assert.True(t, netguard.IsAddressNotAllowed(err)) + assert.Zero(t, landed.Load()) +} + +// With a forward proxy the guard checks every request, so a redirect to a +// private target is refused before it reaches the proxy. +func TestHTTPClientGuard_ProxyRedirectToPrivateAddressRefused(t *testing.T) { + t.Parallel() + var seen atomic.Int32 + proxy := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + seen.Add(1) + http.Redirect(w, r, "http://internal.example/landed", http.StatusFound) + })) + t.Cleanup(proxy.Close) + hop, err := url.Parse(proxy.URL) + require.NoError(t, err) + + res := netguardtest.NewDNS(t).Set("public.example", []string{"8.8.8.8"}).Set("internal.example", []string{"10.0.0.1"}).Resolver + client := newGuardedClient(t, destregistry.HTTPClientConfig{Proxy: []*url.URL{hop}, Guard: &netguard.Guard{Resolver: res}}) + _, err = getBody(t, client, "http://public.example/start") + require.Error(t, err) + assert.True(t, netguard.IsAddressNotAllowed(err)) + assert.EqualValues(t, 1, seen.Load(), "only the first hop reached the proxy") +} + +func TestHTTPClientGuard_NilGuardLeavesTransportAlone(t *testing.T) { + t.Parallel() + client, err := destregistry.NewHTTPClient(destregistry.HTTPClientConfig{}) + require.NoError(t, err) + assert.Nil(t, client.Transport) + + dest := newDestination(t, false) + body, err := getBody(t, newGuardedClient(t, destregistry.HTTPClientConfig{OnConnection: func(bool) {}}), dest.URL) + require.NoError(t, err) + assert.Equal(t, "ok", body) +} From 50a8ba88c0ea4497f689cac6e86bca1713d4254a Mon Sep 17 00:00:00 2001 From: Alex Luong Date: Tue, 27 Oct 2026 12:35:17 +0700 Subject: [PATCH 03/13] feat: block private addresses for webhook destinations --- docs/content/self-hosting/configuration.mdoc | 2 + .../destination_private_address_test.go | 86 ++++++++++ internal/apirouter/errorhandler_middleware.go | 2 + internal/config/config.go | 33 ++-- internal/config/destinations.go | 5 + internal/config/logging.go | 2 + internal/config/private_address_test.go | 79 +++++++++ internal/config/validation.go | 4 + internal/destregistry/providers/default.go | 11 ++ .../destregistry/providers/default_test.go | 17 ++ .../providers/destwebhook/destwebhook.go | 42 ++++- .../destwebhook/destwebhook_guard_test.go | 150 ++++++++++++++++++ .../providers/destwebhook/httphelper.go | 6 + 13 files changed, 422 insertions(+), 17 deletions(-) create mode 100644 internal/apirouter/destination_private_address_test.go create mode 100644 internal/config/private_address_test.go create mode 100644 internal/destregistry/providers/destwebhook/destwebhook_guard_test.go diff --git a/docs/content/self-hosting/configuration.mdoc b/docs/content/self-hosting/configuration.mdoc index 0eadbf7b0..bb9a31bcb 100644 --- a/docs/content/self-hosting/configuration.mdoc +++ b/docs/content/self-hosting/configuration.mdoc @@ -150,6 +150,7 @@ Choose one for event log persistence: | `DESTINATIONS_PROXY_URL` | — | HTTP `CONNECT` forward proxy for destination connections, e.g. `http://user:pass@proxy:8080`. Multiple whitespace-separated URLs are tunneled in order. Applies to webhook, RabbitMQ and Kafka destinations; other destination types connect directly. The deprecated `DESTINATIONS_WEBHOOK_PROXY_URL`, when set, takes precedence for webhooks; there is no way to proxy RabbitMQ and Kafka without also proxying webhooks. See [Webhook Forward Proxy](/docs/outpost/self-hosting/guides/webhook-proxy#other-destination-types). | | `DESTINATIONS_METADATA_PATH` | — | Optional. Filesystem path to a directory of [custom destination metadata](https://github.com/hookdeck/outpost/tree/main/internal/destregistry/metadata/providers) (per-type `metadata.json` and `instructions.md`). Non-core fields such as `label`, `description`, `icon`, and `instructions` can be customized; `config_fields` and `credential_fields` cannot be overridden. | | `DESTINATIONS_AWS_EVENTBRIDGE_SOURCE` | `outpost` | `Source` set on every event published by [AWS EventBridge](/docs/outpost/destinations/aws-eventbridge) destinations. Must not be empty or start with `aws.`. | +| `DESTINATIONS_PRIVATE_ADDRESS_ALLOWLIST` | — | Comma-separated IP addresses and CIDR ranges that destinations may still reach while blocking private addresses (`DESTINATIONS_WEBHOOK_BLOCK_PRIVATE_ADDRESSES`), e.g. `10.0.0.0/8,192.168.1.5`. `localhost` stands for `127.0.0.0/8` and `::1`. Host names, `/0` ranges and ranges with host bits set (`10.0.0.1/8`) fail startup, also while blocking is off. | ## Webhook Behavior @@ -176,6 +177,7 @@ Choose one for event log persistence: | `DESTINATIONS_WEBHOOK_COMPAT_SIGNATURE_SECRET_PREFIX` | — | Prefix stripped from the destination secret before decoding for the compat signature | | `DESTINATIONS_WEBHOOK_COMPAT_HEADERS` | — | Extra headers sent with the compat signature, as comma-separated `name=template` pairs (`{{.EventID}}`, `{{.Topic}}`, `{{.Timestamp}}`). `\,` is a literal comma. | | `DESTINATIONS_WEBHOOK_MAX_RESPONSE_BODY_BYTES` | `131072` (128 KiB) | Max bytes of a destination response body stored on the delivery attempt. Larger responses are replaced with a placeholder so the attempt log stays under the event queue's per-message size limit. Set to `0` to disable the cap. | +| `DESTINATIONS_WEBHOOK_BLOCK_PRIVATE_ADDRESSES` | `false` | If `true`, webhooks connect only to publicly routable addresses (per the IANA special-purpose address registries) or to addresses in `DESTINATIONS_PRIVATE_ADDRESS_ALLOWLIST`. Destination URLs are checked on create and update (validation error) and on every connection; a blocked delivery fails with the code `address_not_allowed`. Host names are resolved as given, without DNS search domains. With a forward proxy (`DESTINATIONS_PROXY_URL`), Outpost checks the target before each request but the proxy resolves it again; configure the proxy to refuse private addresses too. `HTTP_PROXY` and `HTTPS_PROXY` are ignored for webhooks while this is on. Internationalized host names must be given in punycode. | {% callout type="warning" %} The `DESTINATIONS_WEBHOOK_DISABLE_DEFAULT_EVENT_ID_HEADER`, `DESTINATIONS_WEBHOOK_DISABLE_DEFAULT_SIGNATURE_HEADER`, `DESTINATIONS_WEBHOOK_DISABLE_DEFAULT_TIMESTAMP_HEADER`, and `DESTINATIONS_WEBHOOK_DISABLE_DEFAULT_TOPIC_HEADER` flags are deprecated and will be removed in a future version. Disable a header by setting its corresponding `*_HEADER_NAME` variable to an empty string instead. A set `*_HEADER_NAME` always takes precedence over the matching deprecated flag. diff --git a/internal/apirouter/destination_private_address_test.go b/internal/apirouter/destination_private_address_test.go new file mode 100644 index 000000000..609c91918 --- /dev/null +++ b/internal/apirouter/destination_private_address_test.go @@ -0,0 +1,86 @@ +package apirouter_test + +import ( + "encoding/json" + "net/http" + "testing" + + "github.com/hookdeck/outpost/internal/destregistry" + destregistrydefault "github.com/hookdeck/outpost/internal/destregistry/providers" + "github.com/hookdeck/outpost/internal/destregistry/providers/destwebhook" + "github.com/hookdeck/outpost/internal/util/testutil" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func webhookRegistry(t *testing.T, block bool, allowlist ...string) destregistry.Registry { + t.Helper() + reg := destregistry.NewRegistry(&destregistry.Config{}, testutil.CreateTestLogger(t)) + err := destregistrydefault.RegisterDefault(reg, destregistrydefault.RegisterDefaultDestinationOptions{ + Webhook: &destregistrydefault.DestWebhookConfig{ + HeaderPrefix: destwebhook.DefaultHeaderPrefix, + SignatureContentTemplate: destwebhook.DefaultSignatureContentTmpl, + SignatureHeaderTemplate: destwebhook.DefaultSignatureHeaderTmpl, + SignatureEncoding: destwebhook.DefaultEncoding, + SignatureAlgorithm: destwebhook.DefaultAlgorithm, + SigningSecretTemplate: destwebhook.DefaultSigningSecretTmpl, + BlockPrivateAddresses: block, + }, + PrivateAddressAllowlist: allowlist, + AWSEventBridge: &destregistrydefault.DestAWSEventBridgeConfig{Source: "outpost"}, + }) + require.NoError(t, err) + return reg +} + +func TestDestinationPrivateAddresses(t *testing.T) { + post := func(h *apiTest, id, url string) (int, string) { + req := h.jsonReq(http.MethodPost, "/api/v1/tenants/t1/destinations", map[string]any{ + "id": id, + "type": "webhook", + "topics": []string{"user.created"}, + "config": map[string]string{"url": url}, + }) + resp := h.do(h.withAPIKey(req)) + return resp.Code, resp.Body.String() + } + + t.Run("blocked", func(t *testing.T) { + h := newAPITest(t, withDestRegistry(webhookRegistry(t, true))) + h.tenantStore.UpsertTenant(t.Context(), tf.Any(tf.WithID("t1"))) + + code, body := post(h, "d1", "http://10.0.0.1/hook") + require.Equal(t, http.StatusUnprocessableEntity, code, body) + var got struct { + Message string `json:"message"` + Data []string `json:"data"` + } + require.NoError(t, json.Unmarshal([]byte(body), &got)) + assert.Equal(t, "validation error", got.Message) + assert.Equal(t, []string{"config.url must resolve to a public address"}, got.Data) + + code, body = post(h, "d2", "https://8.8.8.8/hook") + require.Equal(t, http.StatusCreated, code, body) + + patch := h.jsonReq(http.MethodPatch, "/api/v1/tenants/t1/destinations/d2", map[string]any{ + "config": map[string]string{"url": "http://127.0.0.1/hook"}, + }) + resp := h.do(h.withAPIKey(patch)) + assert.Equal(t, http.StatusUnprocessableEntity, resp.Code, resp.Body.String()) + assert.Contains(t, resp.Body.String(), "config.url must resolve to a public address") + }) + + t.Run("allowlisted", func(t *testing.T) { + h := newAPITest(t, withDestRegistry(webhookRegistry(t, true, "10.0.0.0/8"))) + h.tenantStore.UpsertTenant(t.Context(), tf.Any(tf.WithID("t1"))) + code, body := post(h, "d1", "http://10.0.0.1/hook") + assert.Equal(t, http.StatusCreated, code, body) + }) + + t.Run("switch off", func(t *testing.T) { + h := newAPITest(t, withDestRegistry(webhookRegistry(t, false))) + h.tenantStore.UpsertTenant(t.Context(), tf.Any(tf.WithID("t1"))) + code, body := post(h, "d1", "http://10.0.0.1/hook") + assert.Equal(t, http.StatusCreated, code, body) + }) +} diff --git a/internal/apirouter/errorhandler_middleware.go b/internal/apirouter/errorhandler_middleware.go index 5ba7ffd36..2df9a8dd6 100644 --- a/internal/apirouter/errorhandler_middleware.go +++ b/internal/apirouter/errorhandler_middleware.go @@ -112,6 +112,8 @@ func formatValidationError(field, tag, param string) string { return fmt.Sprintf("%s must be at most %s characters", field, param) case "email": return fmt.Sprintf("%s must be a valid email address", field) + case "address_not_allowed": + return fmt.Sprintf("%s must resolve to a public address", field) case "url": return fmt.Sprintf("%s must be a valid URL", field) case "oneof": diff --git a/internal/config/config.go b/internal/config/config.go index 990eb4b71..306a27bd6 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -127,22 +127,23 @@ type Config struct { } var ( - ErrMismatchedServiceType = errors.New("config validation error: service type mismatch") - ErrInvalidServiceType = errors.New("config validation error: invalid service type") - ErrMissingRedis = errors.New("config validation error: redis configuration is required") - ErrMissingLogStorage = errors.New("config validation error: log storage must be provided") - ErrMissingMQs = errors.New("config validation error: message queue configuration is required") - ErrMissingAESSecret = errors.New("config validation error: AES encryption secret is required") - ErrInvalidPortalProxyURL = errors.New("config validation error: invalid portal proxy url") - ErrInvalidWebhookProxyURL = errors.New("config validation error: invalid webhook proxy url") - ErrInvalidAWSEventBridgeSource = errors.New("config validation error: invalid aws eventbridge source") - ErrInvalidDestinationsProxyURL = errors.New("config validation error: invalid destinations proxy url") - ErrInvalidPublishProxyURL = errors.New("config validation error: invalid publish proxy url") - ErrInvalidDeploymentID = errors.New("config validation error: deployment_id must contain only alphanumeric characters, hyphens, and underscores (max 64 characters)") - ErrInvalidRedisPoolSize = errors.New("config validation error: redis pool_size must be >= 0") - ErrInvalidPublishMaxRedeliveries = errors.New("config validation error: publish_max_redeliveries must be >= -1") - ErrInvalidSupervisorLimit = errors.New("config validation error: invalid supervisor limit") - ErrInvalidSupervisorWorker = errors.New("config validation error: invalid supervisor restart worker") + ErrMismatchedServiceType = errors.New("config validation error: service type mismatch") + ErrInvalidServiceType = errors.New("config validation error: invalid service type") + ErrMissingRedis = errors.New("config validation error: redis configuration is required") + ErrMissingLogStorage = errors.New("config validation error: log storage must be provided") + ErrMissingMQs = errors.New("config validation error: message queue configuration is required") + ErrMissingAESSecret = errors.New("config validation error: AES encryption secret is required") + ErrInvalidPortalProxyURL = errors.New("config validation error: invalid portal proxy url") + ErrInvalidWebhookProxyURL = errors.New("config validation error: invalid webhook proxy url") + ErrInvalidAWSEventBridgeSource = errors.New("config validation error: invalid aws eventbridge source") + ErrInvalidDestinationsProxyURL = errors.New("config validation error: invalid destinations proxy url") + ErrInvalidPrivateAddressAllowlist = errors.New("config validation error: invalid private address allowlist") + ErrInvalidPublishProxyURL = errors.New("config validation error: invalid publish proxy url") + ErrInvalidDeploymentID = errors.New("config validation error: deployment_id must contain only alphanumeric characters, hyphens, and underscores (max 64 characters)") + ErrInvalidRedisPoolSize = errors.New("config validation error: redis pool_size must be >= 0") + ErrInvalidPublishMaxRedeliveries = errors.New("config validation error: publish_max_redeliveries must be >= -1") + ErrInvalidSupervisorLimit = errors.New("config validation error: invalid supervisor limit") + ErrInvalidSupervisorWorker = errors.New("config validation error: invalid supervisor restart worker") ) func (c *Config) InitDefaults() { diff --git a/internal/config/destinations.go b/internal/config/destinations.go index 87cbbf399..6b09a1dd7 100644 --- a/internal/config/destinations.go +++ b/internal/config/destinations.go @@ -14,6 +14,7 @@ type DestinationsConfig struct { MetadataPath string `yaml:"metadata_path" env:"DESTINATIONS_METADATA_PATH" desc:"Path to the directory containing custom destination type definitions." required:"N"` IncludeMillisecondTimestamp bool `yaml:"include_millisecond_timestamp" env:"DESTINATIONS_INCLUDE_MILLISECOND_TIMESTAMP" desc:"If true, includes a 'timestamp-ms' field with millisecond precision in destination metadata. Useful for load testing and debugging." required:"N"` ProxyURL string `yaml:"proxy_url" env:"DESTINATIONS_PROXY_URL" desc:"HTTP CONNECT forward proxy for destination connections, e.g. 'http://user:pass@proxy:10000'. Multiple whitespace-separated URLs are tunneled in order, nearest first. Applies to webhook, RabbitMQ and Kafka destinations; other types connect directly. DESTINATIONS_WEBHOOK_PROXY_URL, when set, takes precedence for webhooks; webhooks can't opt out while this is set." required:"N"` + PrivateAddressAllowlist []string `yaml:"private_address_allowlist" env:"DESTINATIONS_PRIVATE_ADDRESS_ALLOWLIST" envSeparator:"," desc:"Comma-separated IP addresses and CIDR ranges that destinations may still reach while blocking private addresses (in YAML, a list). 'localhost' stands for 127.0.0.0/8 and ::1. Host names, /0 ranges and ranges with host bits set fail startup, also while blocking is off." required:"N"` Webhook DestinationWebhookConfig `yaml:"webhook" desc:"Configuration specific to webhook destinations."` AWSKinesis DestinationAWSKinesisConfig `yaml:"aws_kinesis" desc:"Configuration specific to AWS Kinesis destinations."` AWSEventBridge DestinationAWSEventBridgeConfig `yaml:"aws_eventbridge" desc:"Configuration specific to AWS EventBridge destinations."` @@ -35,6 +36,7 @@ func (c *DestinationsConfig) ToConfig(cfg *Config) destregistrydefault.RegisterD IncludeMillisecondTimestamp: c.IncludeMillisecondTimestamp, ProxyURL: c.ProxyURL, Webhook: webhook, + PrivateAddressAllowlist: c.PrivateAddressAllowlist, AWSKinesis: c.AWSKinesis.toConfig(), AWSEventBridge: c.AWSEventBridge.toConfig(), DeliveryMaxConcurrency: cfg.DeliveryMaxConcurrency, @@ -94,6 +96,8 @@ type DestinationWebhookConfig struct { SigningSecretTemplate string `yaml:"signing_secret_template" env:"DESTINATIONS_WEBHOOK_SIGNING_SECRET_TEMPLATE" desc:"Go template for generating webhook signing secrets. Available variables: {{.RandomHex}} (64-char hex), {{.RandomBase64}} (base64-encoded), {{.RandomAlphanumeric}} (32-char alphanumeric). Defaults to 'whsec_{{.RandomHex}}'. Only applies to 'default' mode." required:"N"` MaxResponseBodyBytes int `yaml:"max_response_body_bytes" env:"DESTINATIONS_WEBHOOK_MAX_RESPONSE_BODY_BYTES" desc:"Maximum size in bytes of a destination's response body stored on the delivery attempt. Responses larger than this are replaced with a placeholder so the attempt log stays under the event queue's per-message size limit (oversized log messages fail to publish and retry indefinitely). Default: 131072 (128 KiB). Set to 0 to disable the cap." required:"N"` + BlockPrivateAddresses bool `yaml:"block_private_addresses" env:"DESTINATIONS_WEBHOOK_BLOCK_PRIVATE_ADDRESSES" desc:"If true, webhooks connect only to publicly routable addresses (IANA special-purpose registries) or to addresses in DESTINATIONS_PRIVATE_ADDRESS_ALLOWLIST. URLs are checked on create/update and on every connection. Host names are resolved as given, without DNS search domains. With a forward proxy, Outpost checks the target before each request but the proxy resolves it again; configure the proxy to refuse private addresses too. HTTP_PROXY/HTTPS_PROXY are ignored for webhooks while this is on. Defaults to false." required:"N"` + Compat DestinationWebhookCompatConfig `yaml:"compat" desc:"A second signature sent alongside the primary one, so receivers verifying an older scheme keep working while they migrate."` } @@ -180,6 +184,7 @@ func (c *DestinationWebhookConfig) toConfig() *destregistrydefault.DestWebhookCo SignatureAlgorithm: c.SignatureAlgorithm, SigningSecretTemplate: c.SigningSecretTemplate, MaxResponseBodyBytes: c.MaxResponseBodyBytes, + BlockPrivateAddresses: c.BlockPrivateAddresses, } if c.Mode == "standard" { applyStandardWebhooks(cfg) diff --git a/internal/config/logging.go b/internal/config/logging.go index 831a1860f..c7062b1ac 100644 --- a/internal/config/logging.go +++ b/internal/config/logging.go @@ -159,6 +159,8 @@ func (c *Config) LogConfigurationSummary() []zap.Field { zap.Bool("publish_proxy_url_configured", strings.TrimSpace(c.PublishMQ.ProxyURL) != ""), zap.Bool("destinations_proxy_url_configured", strings.TrimSpace(c.Destinations.ProxyURL) != ""), zap.Bool("destinations_webhook_proxied", strings.TrimSpace(webhookCfg.ProxyURL) != ""), + zap.Bool("destinations_webhook_block_private_addresses", webhookCfg.BlockPrivateAddresses), + zap.Int("destinations_private_address_allowlist_entries", len(c.Destinations.PrivateAddressAllowlist)), // Destinations - Webhook (effective header directives after resolving the // three-state name configs and deprecated DISABLE_* flags) diff --git a/internal/config/private_address_test.go b/internal/config/private_address_test.go new file mode 100644 index 000000000..129001393 --- /dev/null +++ b/internal/config/private_address_test.go @@ -0,0 +1,79 @@ +package config_test + +import ( + "testing" + + "github.com/hookdeck/outpost/internal/config" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestDestinationsPrivateAddresses(t *testing.T) { + t.Run("off and empty by default", func(t *testing.T) { + cfg, err := config.ParseWithoutValidation(config.Flags{}, &mockOS{files: map[string][]byte{}, envVars: map[string]string{}}) + require.NoError(t, err) + opts := cfg.Destinations.ToConfig(cfg) + assert.False(t, opts.Webhook.BlockPrivateAddresses) + assert.Empty(t, opts.PrivateAddressAllowlist) + }) + + t.Run("env", func(t *testing.T) { + for _, mode := range []string{"default", "standard"} { + cfg, err := config.ParseWithoutValidation(config.Flags{}, &mockOS{files: map[string][]byte{}, envVars: map[string]string{ + "DESTINATIONS_WEBHOOK_MODE": mode, + "DESTINATIONS_WEBHOOK_BLOCK_PRIVATE_ADDRESSES": "true", + "DESTINATIONS_PRIVATE_ADDRESS_ALLOWLIST": "10.0.0.0/8,localhost", + }}) + require.NoError(t, err, mode) + opts := cfg.Destinations.ToConfig(cfg) + assert.True(t, opts.Webhook.BlockPrivateAddresses, mode) + assert.Equal(t, []string{"10.0.0.0/8", "localhost"}, opts.PrivateAddressAllowlist, mode) + } + }) + + t.Run("yaml", func(t *testing.T) { + cfg, err := config.ParseWithoutValidation(config.Flags{}, &mockOS{ + files: map[string][]byte{"config.yaml": []byte(` +destinations: + private_address_allowlist: + - 10.0.0.0/8 + - 192.168.1.5 + webhook: + block_private_addresses: true +`)}, + envVars: map[string]string{"CONFIG": "config.yaml"}, + }) + require.NoError(t, err) + opts := cfg.Destinations.ToConfig(cfg) + assert.True(t, opts.Webhook.BlockPrivateAddresses) + assert.Equal(t, []string{"10.0.0.0/8", "192.168.1.5"}, opts.PrivateAddressAllowlist) + }) +} + +func TestValidate_PrivateAddressAllowlist(t *testing.T) { + for name, tc := range map[string]struct { + entries []string + block bool + wantErr bool + }{ + "empty": {}, + "valid": {entries: []string{"10.0.0.0/8", "127.0.0.1", "localhost", "fd00::/8"}}, + "host name": {entries: []string{"internal.example"}, wantErr: true}, + "whole internet": {entries: []string{"0.0.0.0/0"}, wantErr: true}, + "host bits": {entries: []string{"10.0.0.1/8"}, wantErr: true}, + "bad entry, block on": {entries: []string{"nope"}, block: true, wantErr: true}, + "bad entry, block off": {entries: []string{"nope"}, wantErr: true}, + } { + t.Run(name, func(t *testing.T) { + c := validConfig() + c.Destinations.PrivateAddressAllowlist = tc.entries + c.Destinations.Webhook.BlockPrivateAddresses = tc.block + err := c.Validate(config.Flags{}) + if tc.wantErr { + assert.ErrorIs(t, err, config.ErrInvalidPrivateAddressAllowlist) + } else { + assert.NoError(t, err) + } + }) + } +} diff --git a/internal/config/validation.go b/internal/config/validation.go index e12e99f26..5b1593b32 100644 --- a/internal/config/validation.go +++ b/internal/config/validation.go @@ -7,6 +7,7 @@ import ( "regexp" "strings" + "github.com/hookdeck/outpost/internal/netguard" "github.com/hookdeck/outpost/internal/proxychain" ) @@ -202,6 +203,9 @@ func (c *Config) validateDestinations() error { if _, err := proxychain.Parse(c.Destinations.Webhook.ProxyURL); err != nil { return fmt.Errorf("%w: %w", ErrInvalidWebhookProxyURL, err) } + if _, err := netguard.ParseAllowlist(c.Destinations.PrivateAddressAllowlist); err != nil { + return fmt.Errorf("%w: %w", ErrInvalidPrivateAddressAllowlist, err) + } switch source := c.Destinations.AWSEventBridge.Source; { case source == "": return fmt.Errorf("%w: DESTINATIONS_AWS_EVENTBRIDGE_SOURCE must not be empty", ErrInvalidAWSEventBridgeSource) diff --git a/internal/destregistry/providers/default.go b/internal/destregistry/providers/default.go index 254dd299f..59ae8522d 100644 --- a/internal/destregistry/providers/default.go +++ b/internal/destregistry/providers/default.go @@ -18,6 +18,7 @@ import ( "github.com/hookdeck/outpost/internal/destregistry/providers/destrabbitmq" "github.com/hookdeck/outpost/internal/destregistry/providers/destwebhook" "github.com/hookdeck/outpost/internal/emetrics" + "github.com/hookdeck/outpost/internal/netguard" "github.com/hookdeck/outpost/internal/proxychain" ) @@ -49,6 +50,7 @@ type DestWebhookConfig struct { MaxResponseBodyBytes int SignatureSecretEncoding string SignatureSecretPrefix string + BlockPrivateAddresses bool Compat *destwebhook.CompatSignatureConfig } @@ -71,6 +73,8 @@ type RegisterDefaultDestinationOptions struct { AWSKinesis *DestAWSKinesisConfig AWSEventBridge *DestAWSEventBridgeConfig + PrivateAddressAllowlist []string + // DeliveryMaxConcurrency is the delivery worker pool size. It bounds how // many deliveries can be in flight, and therefore how many connections a // single destination could need. 0 leaves Go's per-host default in place. @@ -140,6 +144,13 @@ func RegisterDefault(registry destregistry.Registry, opts RegisterDefaultDestina destwebhook.WithSecretEncoding(opts.Webhook.SignatureSecretEncoding, opts.Webhook.SignatureSecretPrefix), destwebhook.WithCompatSignature(opts.Webhook.Compat), ) + if opts.Webhook.BlockPrivateAddresses { + allowlist, err := netguard.ParseAllowlist(opts.PrivateAddressAllowlist) + if err != nil { + return fmt.Errorf("private address allowlist: %w", err) + } + webhookOpts = append(webhookOpts, destwebhook.WithAddressGuard(&netguard.Guard{Allowlist: allowlist})) + } if opts.Webhook.MetadataName != "" { webhookOpts = append(webhookOpts, destwebhook.WithMetadataName(opts.Webhook.MetadataName)) } diff --git a/internal/destregistry/providers/default_test.go b/internal/destregistry/providers/default_test.go index 0fa21a5f8..e086e2a1f 100644 --- a/internal/destregistry/providers/default_test.go +++ b/internal/destregistry/providers/default_test.go @@ -97,3 +97,20 @@ func TestRegisterDefault_WebhookCompatSignature(t *testing.T) { assert.ErrorContains(t, err, "compat signature") }) } + +func TestRegisterDefault_WebhookBlockPrivateAddresses_BadAllowlist(t *testing.T) { + registry := destregistry.NewRegistry(&destregistry.Config{}, testutil.CreateTestLogger(t)) + err := destregistrydefault.RegisterDefault(registry, destregistrydefault.RegisterDefaultDestinationOptions{ + Webhook: &destregistrydefault.DestWebhookConfig{ + HeaderPrefix: destwebhook.DefaultHeaderPrefix, + SignatureContentTemplate: destwebhook.DefaultSignatureContentTmpl, + SignatureHeaderTemplate: destwebhook.DefaultSignatureHeaderTmpl, + SignatureEncoding: destwebhook.DefaultEncoding, + SignatureAlgorithm: destwebhook.DefaultAlgorithm, + SigningSecretTemplate: destwebhook.DefaultSigningSecretTmpl, + BlockPrivateAddresses: true, + }, + PrivateAddressAllowlist: []string{"internal.example"}, + }) + assert.ErrorContains(t, err, "private address allowlist") +} diff --git a/internal/destregistry/providers/destwebhook/destwebhook.go b/internal/destregistry/providers/destwebhook/destwebhook.go index 311ab68df..ebf17a807 100644 --- a/internal/destregistry/providers/destwebhook/destwebhook.go +++ b/internal/destregistry/providers/destwebhook/destwebhook.go @@ -20,8 +20,11 @@ import ( "github.com/hookdeck/outpost/internal/destregistry" "github.com/hookdeck/outpost/internal/destregistry/metadata" "github.com/hookdeck/outpost/internal/models" + "github.com/hookdeck/outpost/internal/netguard" ) +const checkURLTimeout = 5 * time.Second + const ( DefaultEncoding = "hex" DefaultAlgorithm = "hmac-sha256" @@ -111,6 +114,7 @@ type WebhookDestination struct { headerPrefix string userAgent string proxy []*url.URL + guard *netguard.Guard signatureContentTemplate string signatureHeaderTemplate string eventIDHeader headerConfig @@ -205,6 +209,16 @@ func WithProxy(hops []*url.URL) Option { } } +// WithAddressGuard limits webhooks to addresses the guard allows: the URL is +// checked when a destination is created or updated, and every connection is +// checked on delivery. HTTP_PROXY/HTTPS_PROXY are then ignored. See +// destregistry.HTTPClientConfig.Guard for forward proxies. +func WithAddressGuard(g *netguard.Guard) Option { + return func(w *WebhookDestination) { + w.guard = g + } +} + // WithMaxResponseBodyBytes caps how much of the destination response body is // stored on the attempt. 0 (default) disables the cap. func WithMaxResponseBodyBytes(maxBytes int) Option { @@ -418,6 +432,7 @@ func New(loader metadata.MetadataLoader, basePublisherOpts []destregistry.BasePu httpClient, err := destregistry.NewHTTPClient(destregistry.HTTPClientConfig{ UserAgent: &destination.userAgent, Proxy: destination.proxy, + Guard: destination.guard, WrapTransport: WrapTransport, Pool: destination.pool, OnConnection: destination.onConnection, @@ -474,7 +489,7 @@ func (d *WebhookDestination) ObfuscateDestination(destination *models.Destinatio } func (d *WebhookDestination) Validate(ctx context.Context, destination *models.Destination) error { - _, creds, err := d.resolveConfig(ctx, destination) + config, creds, err := d.resolveConfig(ctx, destination) if err != nil { return err } @@ -492,6 +507,31 @@ func (d *WebhookDestination) Validate(ctx context.Context, destination *models.D }}) } } + return d.checkURL(ctx, config.URL) +} + +// checkURL refuses a URL whose host resolves to an address the guard blocks. +// Every failure, resolver errors included, gives the same error, so the check +// can't be used to probe internal DNS. +func (d *WebhookDestination) checkURL(ctx context.Context, rawURL string) error { + if d.guard == nil { + return nil + } + fail := func(errType string) error { + return destregistry.NewErrDestinationValidation([]destregistry.ValidationErrorDetail{{ + Field: "config.url", + Type: errType, + }}) + } + u, err := url.Parse(rawURL) + if err != nil { + return fail("invalid") + } + ctx, cancel := context.WithTimeout(ctx, checkURLTimeout) + defer cancel() + if err := d.guard.CheckURL(ctx, u); err != nil { + return fail("address_not_allowed") + } return nil } diff --git a/internal/destregistry/providers/destwebhook/destwebhook_guard_test.go b/internal/destregistry/providers/destwebhook/destwebhook_guard_test.go new file mode 100644 index 000000000..25727ed72 --- /dev/null +++ b/internal/destregistry/providers/destwebhook/destwebhook_guard_test.go @@ -0,0 +1,150 @@ +package destwebhook_test + +import ( + "context" + "errors" + "net" + "net/http" + "net/http/httptest" + "net/url" + "sync/atomic" + "testing" + + "github.com/hookdeck/outpost/internal/destregistry" + "github.com/hookdeck/outpost/internal/destregistry/providers/destwebhook" + "github.com/hookdeck/outpost/internal/models" + "github.com/hookdeck/outpost/internal/netguard" + "github.com/hookdeck/outpost/internal/netguard/netguardtest" + "github.com/hookdeck/outpost/internal/util/testutil" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func guardWith(t *testing.T, res *net.Resolver, allow ...string) *netguard.Guard { + t.Helper() + al, err := netguard.ParseAllowlist(allow) + require.NoError(t, err) + return &netguard.Guard{Resolver: res, Allowlist: al} +} + +func webhookTo(rawURL string) models.Destination { + return testutil.DestinationFactory.Any( + testutil.DestinationFactory.WithType("webhook"), + testutil.DestinationFactory.WithConfig(map[string]string{"url": rawURL}), + testutil.DestinationFactory.WithCredentials(map[string]string{"secret": "test-secret"}), + ) +} + +func requireURLError(t *testing.T, err error, errType string) { + t.Helper() + var validationErr *destregistry.ErrDestinationValidation + require.ErrorAs(t, err, &validationErr) + require.Len(t, validationErr.Errors, 1) + assert.Equal(t, "config.url", validationErr.Errors[0].Field) + assert.Equal(t, errType, validationErr.Errors[0].Type) +} + +func TestWebhookDestination_Validate_AddressGuard(t *testing.T) { + t.Parallel() + res := netguardtest.NewDNS(t).Set("public.example", []string{"8.8.8.8"}).Set("internal.example", []string{"10.0.0.1"}).Resolver + guarded := NewTestProvider(t, destwebhook.WithAddressGuard(&netguard.Guard{Resolver: res})) + + for _, u := range []string{"http://10.0.0.1/", "http://127.0.0.1/", "http://[::1]/", "https://internal.example/hook", "https://unknown.example/hook"} { + d := webhookTo(u) + requireURLError(t, guarded.Validate(context.Background(), &d), "address_not_allowed") + } + // The schema rejects the second before the guard runs. + for u, errType := range map[string]string{"http://[::1": "invalid", "http://": "pattern"} { + d := webhookTo(u) + requireURLError(t, guarded.Validate(context.Background(), &d), errType) + } + for _, u := range []string{"https://8.8.8.8/hook", "https://public.example/hook"} { + d := webhookTo(u) + assert.NoError(t, guarded.Validate(context.Background(), &d), u) + } + + allowing := NewTestProvider(t, destwebhook.WithAddressGuard(guardWith(t, res, "10.0.0.0/8"))) + d := webhookTo("https://internal.example/hook") + assert.NoError(t, allowing.Validate(context.Background(), &d)) +} + +func TestWebhookDestination_Validate_NoGuardAllowsPrivateAddresses(t *testing.T) { + t.Parallel() + d := webhookTo("http://127.0.0.1/") + assert.NoError(t, NewTestProvider(t).Validate(context.Background(), &d)) +} + +func TestWebhookPublisher_AddressGuard(t *testing.T) { + t.Parallel() + var hits atomic.Int32 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + hits.Add(1) + })) + t.Cleanup(srv.Close) + event := testutil.EventFactory.Any(testutil.EventFactory.WithDataMap(map[string]interface{}{"k": "v"})) + + publish := func(t *testing.T, provider *destwebhook.WebhookDestination, target string) (*destregistry.Delivery, error) { + t.Helper() + d := webhookTo(target) + publisher, err := provider.CreatePublisher(context.Background(), &d) + require.NoError(t, err) + t.Cleanup(func() { _ = publisher.Close() }) + return publisher.Publish(context.Background(), &event) + } + + t.Run("allowlisted address is delivered to", func(t *testing.T) { + provider := NewTestProvider(t, destwebhook.WithAddressGuard(guardWith(t, nil, "127.0.0.1/32"))) + delivery, err := publish(t, provider, srv.URL) + require.NoError(t, err) + assert.Equal(t, "success", delivery.Status) + assert.EqualValues(t, 1, hits.Load()) + }) + + t.Run("private address fails the attempt", func(t *testing.T) { + hits.Store(0) + provider := NewTestProvider(t, destwebhook.WithAddressGuard(&netguard.Guard{})) + delivery, err := publish(t, provider, srv.URL) + require.Error(t, err) + var attemptErr *destregistry.ErrDestinationPublishAttempt + require.True(t, errors.As(err, &attemptErr)) + require.NotNil(t, delivery) + assert.Equal(t, "failed", delivery.Status) + assert.Equal(t, "address_not_allowed", delivery.Code) + assert.Zero(t, hits.Load()) + }) + + t.Run("forward proxy: the target is checked before the request", func(t *testing.T) { + var proxied atomic.Int32 + proxy := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + proxied.Add(1) + w.WriteHeader(http.StatusBadGateway) + })) + t.Cleanup(proxy.Close) + proxyURL, err := url.Parse(proxy.URL) + require.NoError(t, err) + res := netguardtest.NewDNS(t).Set("hooks.example", []string{"8.8.8.8"}).Resolver + provider := NewTestProvider(t, + destwebhook.WithProxy([]*url.URL{proxyURL}), + destwebhook.WithAddressGuard(&netguard.Guard{Resolver: res}), + ) + + delivery, err := publish(t, provider, "http://10.0.0.1/hook") + require.Error(t, err) + require.NotNil(t, delivery) + assert.Equal(t, "address_not_allowed", delivery.Code) + assert.Zero(t, proxied.Load(), "the proxy saw nothing") + + delivery, err = publish(t, provider, "http://hooks.example/hook") + require.Error(t, err) + require.NotNil(t, delivery) + assert.Equal(t, "502", delivery.Code) + assert.EqualValues(t, 1, proxied.Load()) + }) +} + +func TestClassifyNetworkError_AddressNotAllowed(t *testing.T) { + t.Parallel() + err := &netguard.AddressNotAllowedError{Host: "internal.example", Reason: netguard.ReasonNotGlobal} + assert.Equal(t, "address_not_allowed", destwebhook.ClassifyNetworkError(err)) + assert.Equal(t, "address_not_allowed", destwebhook.ClassifyNetworkError(&url.Error{Op: "Get", URL: "http://x", Err: err})) +} diff --git a/internal/destregistry/providers/destwebhook/httphelper.go b/internal/destregistry/providers/destwebhook/httphelper.go index 30239d152..8d1dfa865 100644 --- a/internal/destregistry/providers/destwebhook/httphelper.go +++ b/internal/destregistry/providers/destwebhook/httphelper.go @@ -9,6 +9,7 @@ import ( "strings" "github.com/hookdeck/outpost/internal/destregistry" + "github.com/hookdeck/outpost/internal/netguard" ) // HTTPRequestResult contains the result of an HTTP request execution. @@ -133,6 +134,7 @@ func ExecuteHTTPRequest(ctx context.Context, client *http.Client, req *http.Requ // All errors classified here are destination-level failures (DeliveryError → ack + retry). // // Error codes and their meanings: +// - address_not_allowed: Destination address is blocked by the address guard // - dns_error: Domain doesn't exist or DNS lookup failed // - connection_refused: Server not running or rejecting connections // - connection_reset: Connection was dropped by the server @@ -146,6 +148,10 @@ func ClassifyNetworkError(err error) string { return "unknown" } + if netguard.IsAddressNotAllowed(err) { + return "address_not_allowed" + } + errStr := err.Error() switch { From de04ff1f6e634912afdfa1a9bbf4dea64eda5d07 Mon Sep 17 00:00:00 2001 From: Alex Luong Date: Tue, 27 Oct 2026 20:18:31 +0700 Subject: [PATCH 04/13] docs: clarify private address allowlist wording Co-Authored-By: Claude Opus 5.5 --- docs/content/self-hosting/configuration.mdoc | 2 +- internal/config/destinations.go | 2 +- internal/netguard/guard.go | 2 +- internal/netguard/iana.go | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/content/self-hosting/configuration.mdoc b/docs/content/self-hosting/configuration.mdoc index bb9a31bcb..06d5ae04d 100644 --- a/docs/content/self-hosting/configuration.mdoc +++ b/docs/content/self-hosting/configuration.mdoc @@ -150,7 +150,7 @@ Choose one for event log persistence: | `DESTINATIONS_PROXY_URL` | — | HTTP `CONNECT` forward proxy for destination connections, e.g. `http://user:pass@proxy:8080`. Multiple whitespace-separated URLs are tunneled in order. Applies to webhook, RabbitMQ and Kafka destinations; other destination types connect directly. The deprecated `DESTINATIONS_WEBHOOK_PROXY_URL`, when set, takes precedence for webhooks; there is no way to proxy RabbitMQ and Kafka without also proxying webhooks. See [Webhook Forward Proxy](/docs/outpost/self-hosting/guides/webhook-proxy#other-destination-types). | | `DESTINATIONS_METADATA_PATH` | — | Optional. Filesystem path to a directory of [custom destination metadata](https://github.com/hookdeck/outpost/tree/main/internal/destregistry/metadata/providers) (per-type `metadata.json` and `instructions.md`). Non-core fields such as `label`, `description`, `icon`, and `instructions` can be customized; `config_fields` and `credential_fields` cannot be overridden. | | `DESTINATIONS_AWS_EVENTBRIDGE_SOURCE` | `outpost` | `Source` set on every event published by [AWS EventBridge](/docs/outpost/destinations/aws-eventbridge) destinations. Must not be empty or start with `aws.`. | -| `DESTINATIONS_PRIVATE_ADDRESS_ALLOWLIST` | — | Comma-separated IP addresses and CIDR ranges that destinations may still reach while blocking private addresses (`DESTINATIONS_WEBHOOK_BLOCK_PRIVATE_ADDRESSES`), e.g. `10.0.0.0/8,192.168.1.5`. `localhost` stands for `127.0.0.0/8` and `::1`. Host names, `/0` ranges and ranges with host bits set (`10.0.0.1/8`) fail startup, also while blocking is off. | +| `DESTINATIONS_PRIVATE_ADDRESS_ALLOWLIST` | — | Comma-separated IP addresses and CIDR ranges that webhooks may still reach while `DESTINATIONS_WEBHOOK_BLOCK_PRIVATE_ADDRESSES` is on, e.g. `10.0.0.0/8,192.168.1.5`. `localhost` stands for `127.0.0.0/8` and `::1`. Host names, `/0` ranges and ranges with host bits set (`10.0.0.1/8`) fail startup, also while blocking is off. | ## Webhook Behavior diff --git a/internal/config/destinations.go b/internal/config/destinations.go index 6b09a1dd7..8bc12a613 100644 --- a/internal/config/destinations.go +++ b/internal/config/destinations.go @@ -14,7 +14,7 @@ type DestinationsConfig struct { MetadataPath string `yaml:"metadata_path" env:"DESTINATIONS_METADATA_PATH" desc:"Path to the directory containing custom destination type definitions." required:"N"` IncludeMillisecondTimestamp bool `yaml:"include_millisecond_timestamp" env:"DESTINATIONS_INCLUDE_MILLISECOND_TIMESTAMP" desc:"If true, includes a 'timestamp-ms' field with millisecond precision in destination metadata. Useful for load testing and debugging." required:"N"` ProxyURL string `yaml:"proxy_url" env:"DESTINATIONS_PROXY_URL" desc:"HTTP CONNECT forward proxy for destination connections, e.g. 'http://user:pass@proxy:10000'. Multiple whitespace-separated URLs are tunneled in order, nearest first. Applies to webhook, RabbitMQ and Kafka destinations; other types connect directly. DESTINATIONS_WEBHOOK_PROXY_URL, when set, takes precedence for webhooks; webhooks can't opt out while this is set." required:"N"` - PrivateAddressAllowlist []string `yaml:"private_address_allowlist" env:"DESTINATIONS_PRIVATE_ADDRESS_ALLOWLIST" envSeparator:"," desc:"Comma-separated IP addresses and CIDR ranges that destinations may still reach while blocking private addresses (in YAML, a list). 'localhost' stands for 127.0.0.0/8 and ::1. Host names, /0 ranges and ranges with host bits set fail startup, also while blocking is off." required:"N"` + PrivateAddressAllowlist []string `yaml:"private_address_allowlist" env:"DESTINATIONS_PRIVATE_ADDRESS_ALLOWLIST" envSeparator:"," desc:"Comma-separated IP addresses and CIDR ranges that webhooks may still reach while DESTINATIONS_WEBHOOK_BLOCK_PRIVATE_ADDRESSES is on (in YAML, a list). 'localhost' stands for 127.0.0.0/8 and ::1. Host names, /0 ranges and ranges with host bits set fail startup, also while blocking is off." required:"N"` Webhook DestinationWebhookConfig `yaml:"webhook" desc:"Configuration specific to webhook destinations."` AWSKinesis DestinationAWSKinesisConfig `yaml:"aws_kinesis" desc:"Configuration specific to AWS Kinesis destinations."` AWSEventBridge DestinationAWSEventBridgeConfig `yaml:"aws_eventbridge" desc:"Configuration specific to AWS EventBridge destinations."` diff --git a/internal/netguard/guard.go b/internal/netguard/guard.go index bcde05e32..024c3ba26 100644 --- a/internal/netguard/guard.go +++ b/internal/netguard/guard.go @@ -3,7 +3,7 @@ // and again on every new connection (Guard.DialContext): every address a // connection is about to be made to must be globally reachable per the IANA // special-purpose registries or allowlisted, so a DNS answer that changes in -// between (rebinding) can't redirect it. Non-ASCII host names must be given +// between (rebinding) can't send it to a blocked address. Non-ASCII host names must be given // in punycode. package netguard diff --git a/internal/netguard/iana.go b/internal/netguard/iana.go index f821befe8..5965441f0 100644 --- a/internal/netguard/iana.go +++ b/internal/netguard/iana.go @@ -66,7 +66,7 @@ var ipv6Special = []specialBlock{ {netip.MustParsePrefix("100::/64"), false}, // Discard-Only Address Block {netip.MustParsePrefix("100:0:0:1::/64"), false}, // Dummy IPv6 Prefix {netip.MustParsePrefix("2001::/23"), false}, // IETF Protocol Assignments - {netip.MustParsePrefix("2001::/32"), false}, // TEREDO // "N/A" + {netip.MustParsePrefix("2001::/32"), false}, // TEREDO ("N/A") {netip.MustParsePrefix("2001:1::1/128"), true}, // Port Control Protocol Anycast {netip.MustParsePrefix("2001:1::2/128"), true}, // Traversal Using Relays around NAT Anycast {netip.MustParsePrefix("2001:1::3/128"), true}, // DNS-SD Service Registration Protocol Anycast From 305c91a7c8ab77c9f5362083af0ca71b7c7f43d0 Mon Sep 17 00:00:00 2001 From: Alex Luong Date: Fri, 30 Oct 2026 03:09:05 +0700 Subject: [PATCH 05/13] refactor: rename the private address switch to DESTINATIONS_BLOCK_PRIVATE_ADDRESSES Co-Authored-By: Claude Opus 5.5 --- docs/content/self-hosting/configuration.mdoc | 4 ++-- .../destination_private_address_test.go | 2 +- internal/config/destinations.go | 7 +++---- internal/config/logging.go | 2 +- internal/config/private_address_test.go | 17 ++++++++--------- internal/destregistry/providers/default.go | 4 ++-- internal/destregistry/providers/default_test.go | 4 ++-- 7 files changed, 19 insertions(+), 21 deletions(-) diff --git a/docs/content/self-hosting/configuration.mdoc b/docs/content/self-hosting/configuration.mdoc index 06d5ae04d..8454b7c7d 100644 --- a/docs/content/self-hosting/configuration.mdoc +++ b/docs/content/self-hosting/configuration.mdoc @@ -150,7 +150,8 @@ Choose one for event log persistence: | `DESTINATIONS_PROXY_URL` | — | HTTP `CONNECT` forward proxy for destination connections, e.g. `http://user:pass@proxy:8080`. Multiple whitespace-separated URLs are tunneled in order. Applies to webhook, RabbitMQ and Kafka destinations; other destination types connect directly. The deprecated `DESTINATIONS_WEBHOOK_PROXY_URL`, when set, takes precedence for webhooks; there is no way to proxy RabbitMQ and Kafka without also proxying webhooks. See [Webhook Forward Proxy](/docs/outpost/self-hosting/guides/webhook-proxy#other-destination-types). | | `DESTINATIONS_METADATA_PATH` | — | Optional. Filesystem path to a directory of [custom destination metadata](https://github.com/hookdeck/outpost/tree/main/internal/destregistry/metadata/providers) (per-type `metadata.json` and `instructions.md`). Non-core fields such as `label`, `description`, `icon`, and `instructions` can be customized; `config_fields` and `credential_fields` cannot be overridden. | | `DESTINATIONS_AWS_EVENTBRIDGE_SOURCE` | `outpost` | `Source` set on every event published by [AWS EventBridge](/docs/outpost/destinations/aws-eventbridge) destinations. Must not be empty or start with `aws.`. | -| `DESTINATIONS_PRIVATE_ADDRESS_ALLOWLIST` | — | Comma-separated IP addresses and CIDR ranges that webhooks may still reach while `DESTINATIONS_WEBHOOK_BLOCK_PRIVATE_ADDRESSES` is on, e.g. `10.0.0.0/8,192.168.1.5`. `localhost` stands for `127.0.0.0/8` and `::1`. Host names, `/0` ranges and ranges with host bits set (`10.0.0.1/8`) fail startup, also while blocking is off. | +| `DESTINATIONS_BLOCK_PRIVATE_ADDRESSES` | `false` | Currently applies to webhook destinations. If `true`, destinations connect only to publicly routable addresses (per the IANA special-purpose address registries) or to addresses in `DESTINATIONS_PRIVATE_ADDRESS_ALLOWLIST`. Destination URLs are checked on create and update (validation error) and on every connection; a blocked delivery fails with the code `address_not_allowed`. Host names are resolved as given, without DNS search domains. With a forward proxy (`DESTINATIONS_PROXY_URL`), Outpost checks the target before each request but the proxy resolves it again; configure the proxy to refuse private addresses too. `HTTP_PROXY` and `HTTPS_PROXY` are ignored while this is on. Internationalized host names must be given in punycode. | +| `DESTINATIONS_PRIVATE_ADDRESS_ALLOWLIST` | — | Comma-separated IP addresses and CIDR ranges that webhooks may still reach while `DESTINATIONS_BLOCK_PRIVATE_ADDRESSES` is on, e.g. `10.0.0.0/8,192.168.1.5`. `localhost` stands for `127.0.0.0/8` and `::1`. Host names, `/0` ranges and ranges with host bits set (`10.0.0.1/8`) fail startup, also while blocking is off. | ## Webhook Behavior @@ -177,7 +178,6 @@ Choose one for event log persistence: | `DESTINATIONS_WEBHOOK_COMPAT_SIGNATURE_SECRET_PREFIX` | — | Prefix stripped from the destination secret before decoding for the compat signature | | `DESTINATIONS_WEBHOOK_COMPAT_HEADERS` | — | Extra headers sent with the compat signature, as comma-separated `name=template` pairs (`{{.EventID}}`, `{{.Topic}}`, `{{.Timestamp}}`). `\,` is a literal comma. | | `DESTINATIONS_WEBHOOK_MAX_RESPONSE_BODY_BYTES` | `131072` (128 KiB) | Max bytes of a destination response body stored on the delivery attempt. Larger responses are replaced with a placeholder so the attempt log stays under the event queue's per-message size limit. Set to `0` to disable the cap. | -| `DESTINATIONS_WEBHOOK_BLOCK_PRIVATE_ADDRESSES` | `false` | If `true`, webhooks connect only to publicly routable addresses (per the IANA special-purpose address registries) or to addresses in `DESTINATIONS_PRIVATE_ADDRESS_ALLOWLIST`. Destination URLs are checked on create and update (validation error) and on every connection; a blocked delivery fails with the code `address_not_allowed`. Host names are resolved as given, without DNS search domains. With a forward proxy (`DESTINATIONS_PROXY_URL`), Outpost checks the target before each request but the proxy resolves it again; configure the proxy to refuse private addresses too. `HTTP_PROXY` and `HTTPS_PROXY` are ignored for webhooks while this is on. Internationalized host names must be given in punycode. | {% callout type="warning" %} The `DESTINATIONS_WEBHOOK_DISABLE_DEFAULT_EVENT_ID_HEADER`, `DESTINATIONS_WEBHOOK_DISABLE_DEFAULT_SIGNATURE_HEADER`, `DESTINATIONS_WEBHOOK_DISABLE_DEFAULT_TIMESTAMP_HEADER`, and `DESTINATIONS_WEBHOOK_DISABLE_DEFAULT_TOPIC_HEADER` flags are deprecated and will be removed in a future version. Disable a header by setting its corresponding `*_HEADER_NAME` variable to an empty string instead. A set `*_HEADER_NAME` always takes precedence over the matching deprecated flag. diff --git a/internal/apirouter/destination_private_address_test.go b/internal/apirouter/destination_private_address_test.go index 609c91918..b50aa6711 100644 --- a/internal/apirouter/destination_private_address_test.go +++ b/internal/apirouter/destination_private_address_test.go @@ -24,8 +24,8 @@ func webhookRegistry(t *testing.T, block bool, allowlist ...string) destregistry SignatureEncoding: destwebhook.DefaultEncoding, SignatureAlgorithm: destwebhook.DefaultAlgorithm, SigningSecretTemplate: destwebhook.DefaultSigningSecretTmpl, - BlockPrivateAddresses: block, }, + BlockPrivateAddresses: block, PrivateAddressAllowlist: allowlist, AWSEventBridge: &destregistrydefault.DestAWSEventBridgeConfig{Source: "outpost"}, }) diff --git a/internal/config/destinations.go b/internal/config/destinations.go index 8bc12a613..f010c1689 100644 --- a/internal/config/destinations.go +++ b/internal/config/destinations.go @@ -14,7 +14,8 @@ type DestinationsConfig struct { MetadataPath string `yaml:"metadata_path" env:"DESTINATIONS_METADATA_PATH" desc:"Path to the directory containing custom destination type definitions." required:"N"` IncludeMillisecondTimestamp bool `yaml:"include_millisecond_timestamp" env:"DESTINATIONS_INCLUDE_MILLISECOND_TIMESTAMP" desc:"If true, includes a 'timestamp-ms' field with millisecond precision in destination metadata. Useful for load testing and debugging." required:"N"` ProxyURL string `yaml:"proxy_url" env:"DESTINATIONS_PROXY_URL" desc:"HTTP CONNECT forward proxy for destination connections, e.g. 'http://user:pass@proxy:10000'. Multiple whitespace-separated URLs are tunneled in order, nearest first. Applies to webhook, RabbitMQ and Kafka destinations; other types connect directly. DESTINATIONS_WEBHOOK_PROXY_URL, when set, takes precedence for webhooks; webhooks can't opt out while this is set." required:"N"` - PrivateAddressAllowlist []string `yaml:"private_address_allowlist" env:"DESTINATIONS_PRIVATE_ADDRESS_ALLOWLIST" envSeparator:"," desc:"Comma-separated IP addresses and CIDR ranges that webhooks may still reach while DESTINATIONS_WEBHOOK_BLOCK_PRIVATE_ADDRESSES is on (in YAML, a list). 'localhost' stands for 127.0.0.0/8 and ::1. Host names, /0 ranges and ranges with host bits set fail startup, also while blocking is off." required:"N"` + PrivateAddressAllowlist []string `yaml:"private_address_allowlist" env:"DESTINATIONS_PRIVATE_ADDRESS_ALLOWLIST" envSeparator:"," desc:"Comma-separated IP addresses and CIDR ranges that webhooks may still reach while DESTINATIONS_BLOCK_PRIVATE_ADDRESSES is on (in YAML, a list). 'localhost' stands for 127.0.0.0/8 and ::1. Host names, /0 ranges and ranges with host bits set fail startup, also while blocking is off." required:"N"` + BlockPrivateAddresses bool `yaml:"block_private_addresses" env:"DESTINATIONS_BLOCK_PRIVATE_ADDRESSES" desc:"Currently applies to webhook destinations. If true, destinations connect only to publicly routable addresses (IANA special-purpose registries) or to addresses in DESTINATIONS_PRIVATE_ADDRESS_ALLOWLIST. URLs are checked on create/update and on every connection. Host names are resolved as given, without DNS search domains. With a forward proxy, Outpost checks the target before each request but the proxy resolves it again; configure the proxy to refuse private addresses too. HTTP_PROXY/HTTPS_PROXY are ignored for destinations while this is on. Defaults to false." required:"N"` Webhook DestinationWebhookConfig `yaml:"webhook" desc:"Configuration specific to webhook destinations."` AWSKinesis DestinationAWSKinesisConfig `yaml:"aws_kinesis" desc:"Configuration specific to AWS Kinesis destinations."` AWSEventBridge DestinationAWSEventBridgeConfig `yaml:"aws_eventbridge" desc:"Configuration specific to AWS EventBridge destinations."` @@ -36,6 +37,7 @@ func (c *DestinationsConfig) ToConfig(cfg *Config) destregistrydefault.RegisterD IncludeMillisecondTimestamp: c.IncludeMillisecondTimestamp, ProxyURL: c.ProxyURL, Webhook: webhook, + BlockPrivateAddresses: c.BlockPrivateAddresses, PrivateAddressAllowlist: c.PrivateAddressAllowlist, AWSKinesis: c.AWSKinesis.toConfig(), AWSEventBridge: c.AWSEventBridge.toConfig(), @@ -96,8 +98,6 @@ type DestinationWebhookConfig struct { SigningSecretTemplate string `yaml:"signing_secret_template" env:"DESTINATIONS_WEBHOOK_SIGNING_SECRET_TEMPLATE" desc:"Go template for generating webhook signing secrets. Available variables: {{.RandomHex}} (64-char hex), {{.RandomBase64}} (base64-encoded), {{.RandomAlphanumeric}} (32-char alphanumeric). Defaults to 'whsec_{{.RandomHex}}'. Only applies to 'default' mode." required:"N"` MaxResponseBodyBytes int `yaml:"max_response_body_bytes" env:"DESTINATIONS_WEBHOOK_MAX_RESPONSE_BODY_BYTES" desc:"Maximum size in bytes of a destination's response body stored on the delivery attempt. Responses larger than this are replaced with a placeholder so the attempt log stays under the event queue's per-message size limit (oversized log messages fail to publish and retry indefinitely). Default: 131072 (128 KiB). Set to 0 to disable the cap." required:"N"` - BlockPrivateAddresses bool `yaml:"block_private_addresses" env:"DESTINATIONS_WEBHOOK_BLOCK_PRIVATE_ADDRESSES" desc:"If true, webhooks connect only to publicly routable addresses (IANA special-purpose registries) or to addresses in DESTINATIONS_PRIVATE_ADDRESS_ALLOWLIST. URLs are checked on create/update and on every connection. Host names are resolved as given, without DNS search domains. With a forward proxy, Outpost checks the target before each request but the proxy resolves it again; configure the proxy to refuse private addresses too. HTTP_PROXY/HTTPS_PROXY are ignored for webhooks while this is on. Defaults to false." required:"N"` - Compat DestinationWebhookCompatConfig `yaml:"compat" desc:"A second signature sent alongside the primary one, so receivers verifying an older scheme keep working while they migrate."` } @@ -184,7 +184,6 @@ func (c *DestinationWebhookConfig) toConfig() *destregistrydefault.DestWebhookCo SignatureAlgorithm: c.SignatureAlgorithm, SigningSecretTemplate: c.SigningSecretTemplate, MaxResponseBodyBytes: c.MaxResponseBodyBytes, - BlockPrivateAddresses: c.BlockPrivateAddresses, } if c.Mode == "standard" { applyStandardWebhooks(cfg) diff --git a/internal/config/logging.go b/internal/config/logging.go index c7062b1ac..943207426 100644 --- a/internal/config/logging.go +++ b/internal/config/logging.go @@ -159,7 +159,7 @@ func (c *Config) LogConfigurationSummary() []zap.Field { zap.Bool("publish_proxy_url_configured", strings.TrimSpace(c.PublishMQ.ProxyURL) != ""), zap.Bool("destinations_proxy_url_configured", strings.TrimSpace(c.Destinations.ProxyURL) != ""), zap.Bool("destinations_webhook_proxied", strings.TrimSpace(webhookCfg.ProxyURL) != ""), - zap.Bool("destinations_webhook_block_private_addresses", webhookCfg.BlockPrivateAddresses), + zap.Bool("destinations_block_private_addresses", c.Destinations.BlockPrivateAddresses), zap.Int("destinations_private_address_allowlist_entries", len(c.Destinations.PrivateAddressAllowlist)), // Destinations - Webhook (effective header directives after resolving the diff --git a/internal/config/private_address_test.go b/internal/config/private_address_test.go index 129001393..8515e4950 100644 --- a/internal/config/private_address_test.go +++ b/internal/config/private_address_test.go @@ -13,20 +13,20 @@ func TestDestinationsPrivateAddresses(t *testing.T) { cfg, err := config.ParseWithoutValidation(config.Flags{}, &mockOS{files: map[string][]byte{}, envVars: map[string]string{}}) require.NoError(t, err) opts := cfg.Destinations.ToConfig(cfg) - assert.False(t, opts.Webhook.BlockPrivateAddresses) + assert.False(t, opts.BlockPrivateAddresses) assert.Empty(t, opts.PrivateAddressAllowlist) }) t.Run("env", func(t *testing.T) { for _, mode := range []string{"default", "standard"} { cfg, err := config.ParseWithoutValidation(config.Flags{}, &mockOS{files: map[string][]byte{}, envVars: map[string]string{ - "DESTINATIONS_WEBHOOK_MODE": mode, - "DESTINATIONS_WEBHOOK_BLOCK_PRIVATE_ADDRESSES": "true", - "DESTINATIONS_PRIVATE_ADDRESS_ALLOWLIST": "10.0.0.0/8,localhost", + "DESTINATIONS_WEBHOOK_MODE": mode, + "DESTINATIONS_BLOCK_PRIVATE_ADDRESSES": "true", + "DESTINATIONS_PRIVATE_ADDRESS_ALLOWLIST": "10.0.0.0/8,localhost", }}) require.NoError(t, err, mode) opts := cfg.Destinations.ToConfig(cfg) - assert.True(t, opts.Webhook.BlockPrivateAddresses, mode) + assert.True(t, opts.BlockPrivateAddresses, mode) assert.Equal(t, []string{"10.0.0.0/8", "localhost"}, opts.PrivateAddressAllowlist, mode) } }) @@ -35,17 +35,16 @@ func TestDestinationsPrivateAddresses(t *testing.T) { cfg, err := config.ParseWithoutValidation(config.Flags{}, &mockOS{ files: map[string][]byte{"config.yaml": []byte(` destinations: + block_private_addresses: true private_address_allowlist: - 10.0.0.0/8 - 192.168.1.5 - webhook: - block_private_addresses: true `)}, envVars: map[string]string{"CONFIG": "config.yaml"}, }) require.NoError(t, err) opts := cfg.Destinations.ToConfig(cfg) - assert.True(t, opts.Webhook.BlockPrivateAddresses) + assert.True(t, opts.BlockPrivateAddresses) assert.Equal(t, []string{"10.0.0.0/8", "192.168.1.5"}, opts.PrivateAddressAllowlist) }) } @@ -67,7 +66,7 @@ func TestValidate_PrivateAddressAllowlist(t *testing.T) { t.Run(name, func(t *testing.T) { c := validConfig() c.Destinations.PrivateAddressAllowlist = tc.entries - c.Destinations.Webhook.BlockPrivateAddresses = tc.block + c.Destinations.BlockPrivateAddresses = tc.block err := c.Validate(config.Flags{}) if tc.wantErr { assert.ErrorIs(t, err, config.ErrInvalidPrivateAddressAllowlist) diff --git a/internal/destregistry/providers/default.go b/internal/destregistry/providers/default.go index 59ae8522d..8d757ef75 100644 --- a/internal/destregistry/providers/default.go +++ b/internal/destregistry/providers/default.go @@ -50,7 +50,6 @@ type DestWebhookConfig struct { MaxResponseBodyBytes int SignatureSecretEncoding string SignatureSecretPrefix string - BlockPrivateAddresses bool Compat *destwebhook.CompatSignatureConfig } @@ -73,6 +72,7 @@ type RegisterDefaultDestinationOptions struct { AWSKinesis *DestAWSKinesisConfig AWSEventBridge *DestAWSEventBridgeConfig + BlockPrivateAddresses bool PrivateAddressAllowlist []string // DeliveryMaxConcurrency is the delivery worker pool size. It bounds how @@ -144,7 +144,7 @@ func RegisterDefault(registry destregistry.Registry, opts RegisterDefaultDestina destwebhook.WithSecretEncoding(opts.Webhook.SignatureSecretEncoding, opts.Webhook.SignatureSecretPrefix), destwebhook.WithCompatSignature(opts.Webhook.Compat), ) - if opts.Webhook.BlockPrivateAddresses { + if opts.BlockPrivateAddresses { allowlist, err := netguard.ParseAllowlist(opts.PrivateAddressAllowlist) if err != nil { return fmt.Errorf("private address allowlist: %w", err) diff --git a/internal/destregistry/providers/default_test.go b/internal/destregistry/providers/default_test.go index e086e2a1f..448830c67 100644 --- a/internal/destregistry/providers/default_test.go +++ b/internal/destregistry/providers/default_test.go @@ -98,7 +98,7 @@ func TestRegisterDefault_WebhookCompatSignature(t *testing.T) { }) } -func TestRegisterDefault_WebhookBlockPrivateAddresses_BadAllowlist(t *testing.T) { +func TestRegisterDefault_BlockPrivateAddresses_BadAllowlist(t *testing.T) { registry := destregistry.NewRegistry(&destregistry.Config{}, testutil.CreateTestLogger(t)) err := destregistrydefault.RegisterDefault(registry, destregistrydefault.RegisterDefaultDestinationOptions{ Webhook: &destregistrydefault.DestWebhookConfig{ @@ -108,8 +108,8 @@ func TestRegisterDefault_WebhookBlockPrivateAddresses_BadAllowlist(t *testing.T) SignatureEncoding: destwebhook.DefaultEncoding, SignatureAlgorithm: destwebhook.DefaultAlgorithm, SigningSecretTemplate: destwebhook.DefaultSigningSecretTmpl, - BlockPrivateAddresses: true, }, + BlockPrivateAddresses: true, PrivateAddressAllowlist: []string{"internal.example"}, }) assert.ErrorContains(t, err, "private address allowlist") From 671d956e122b4e51d553893d978542375736e8fc Mon Sep 17 00:00:00 2001 From: Alex Luong Date: Fri, 30 Oct 2026 16:14:46 +0700 Subject: [PATCH 06/13] fix: skip the delivery-time address check behind a forward proxy Co-Authored-By: Claude Opus 5.5 --- docs/content/self-hosting/configuration.mdoc | 2 +- internal/config/destinations.go | 2 +- internal/destregistry/httpclient.go | 9 +- .../destregistry/httpclient_guard_test.go | 28 +-- .../providers/destwebhook/destwebhook.go | 6 +- .../destwebhook/destwebhook_guard_test.go | 12 +- internal/netguard/transport.go | 42 ----- internal/netguard/transport_test.go | 167 ------------------ 8 files changed, 13 insertions(+), 255 deletions(-) delete mode 100644 internal/netguard/transport.go delete mode 100644 internal/netguard/transport_test.go diff --git a/docs/content/self-hosting/configuration.mdoc b/docs/content/self-hosting/configuration.mdoc index 8454b7c7d..38b7cb98b 100644 --- a/docs/content/self-hosting/configuration.mdoc +++ b/docs/content/self-hosting/configuration.mdoc @@ -150,7 +150,7 @@ Choose one for event log persistence: | `DESTINATIONS_PROXY_URL` | — | HTTP `CONNECT` forward proxy for destination connections, e.g. `http://user:pass@proxy:8080`. Multiple whitespace-separated URLs are tunneled in order. Applies to webhook, RabbitMQ and Kafka destinations; other destination types connect directly. The deprecated `DESTINATIONS_WEBHOOK_PROXY_URL`, when set, takes precedence for webhooks; there is no way to proxy RabbitMQ and Kafka without also proxying webhooks. See [Webhook Forward Proxy](/docs/outpost/self-hosting/guides/webhook-proxy#other-destination-types). | | `DESTINATIONS_METADATA_PATH` | — | Optional. Filesystem path to a directory of [custom destination metadata](https://github.com/hookdeck/outpost/tree/main/internal/destregistry/metadata/providers) (per-type `metadata.json` and `instructions.md`). Non-core fields such as `label`, `description`, `icon`, and `instructions` can be customized; `config_fields` and `credential_fields` cannot be overridden. | | `DESTINATIONS_AWS_EVENTBRIDGE_SOURCE` | `outpost` | `Source` set on every event published by [AWS EventBridge](/docs/outpost/destinations/aws-eventbridge) destinations. Must not be empty or start with `aws.`. | -| `DESTINATIONS_BLOCK_PRIVATE_ADDRESSES` | `false` | Currently applies to webhook destinations. If `true`, destinations connect only to publicly routable addresses (per the IANA special-purpose address registries) or to addresses in `DESTINATIONS_PRIVATE_ADDRESS_ALLOWLIST`. Destination URLs are checked on create and update (validation error) and on every connection; a blocked delivery fails with the code `address_not_allowed`. Host names are resolved as given, without DNS search domains. With a forward proxy (`DESTINATIONS_PROXY_URL`), Outpost checks the target before each request but the proxy resolves it again; configure the proxy to refuse private addresses too. `HTTP_PROXY` and `HTTPS_PROXY` are ignored while this is on. Internationalized host names must be given in punycode. | +| `DESTINATIONS_BLOCK_PRIVATE_ADDRESSES` | `false` | Currently applies to webhook destinations. If `true`, destinations connect only to publicly routable addresses (per the IANA special-purpose address registries) or to addresses in `DESTINATIONS_PRIVATE_ADDRESS_ALLOWLIST`. Destination URLs are checked on create and update (validation error) and on every direct connection; a blocked delivery fails with the code `address_not_allowed`. Host names are resolved as given, without DNS search domains. With a forward proxy (`DESTINATIONS_PROXY_URL`), only the create/update check applies, since the proxy resolves and connects to the target; configure the proxy to refuse private addresses. `HTTP_PROXY` and `HTTPS_PROXY` are ignored while this is on. Internationalized host names must be given in punycode. | | `DESTINATIONS_PRIVATE_ADDRESS_ALLOWLIST` | — | Comma-separated IP addresses and CIDR ranges that webhooks may still reach while `DESTINATIONS_BLOCK_PRIVATE_ADDRESSES` is on, e.g. `10.0.0.0/8,192.168.1.5`. `localhost` stands for `127.0.0.0/8` and `::1`. Host names, `/0` ranges and ranges with host bits set (`10.0.0.1/8`) fail startup, also while blocking is off. | ## Webhook Behavior diff --git a/internal/config/destinations.go b/internal/config/destinations.go index f010c1689..3261e5ee8 100644 --- a/internal/config/destinations.go +++ b/internal/config/destinations.go @@ -15,7 +15,7 @@ type DestinationsConfig struct { IncludeMillisecondTimestamp bool `yaml:"include_millisecond_timestamp" env:"DESTINATIONS_INCLUDE_MILLISECOND_TIMESTAMP" desc:"If true, includes a 'timestamp-ms' field with millisecond precision in destination metadata. Useful for load testing and debugging." required:"N"` ProxyURL string `yaml:"proxy_url" env:"DESTINATIONS_PROXY_URL" desc:"HTTP CONNECT forward proxy for destination connections, e.g. 'http://user:pass@proxy:10000'. Multiple whitespace-separated URLs are tunneled in order, nearest first. Applies to webhook, RabbitMQ and Kafka destinations; other types connect directly. DESTINATIONS_WEBHOOK_PROXY_URL, when set, takes precedence for webhooks; webhooks can't opt out while this is set." required:"N"` PrivateAddressAllowlist []string `yaml:"private_address_allowlist" env:"DESTINATIONS_PRIVATE_ADDRESS_ALLOWLIST" envSeparator:"," desc:"Comma-separated IP addresses and CIDR ranges that webhooks may still reach while DESTINATIONS_BLOCK_PRIVATE_ADDRESSES is on (in YAML, a list). 'localhost' stands for 127.0.0.0/8 and ::1. Host names, /0 ranges and ranges with host bits set fail startup, also while blocking is off." required:"N"` - BlockPrivateAddresses bool `yaml:"block_private_addresses" env:"DESTINATIONS_BLOCK_PRIVATE_ADDRESSES" desc:"Currently applies to webhook destinations. If true, destinations connect only to publicly routable addresses (IANA special-purpose registries) or to addresses in DESTINATIONS_PRIVATE_ADDRESS_ALLOWLIST. URLs are checked on create/update and on every connection. Host names are resolved as given, without DNS search domains. With a forward proxy, Outpost checks the target before each request but the proxy resolves it again; configure the proxy to refuse private addresses too. HTTP_PROXY/HTTPS_PROXY are ignored for destinations while this is on. Defaults to false." required:"N"` + BlockPrivateAddresses bool `yaml:"block_private_addresses" env:"DESTINATIONS_BLOCK_PRIVATE_ADDRESSES" desc:"Currently applies to webhook destinations. If true, destinations connect only to publicly routable addresses (IANA special-purpose registries) or to addresses in DESTINATIONS_PRIVATE_ADDRESS_ALLOWLIST. URLs are checked on create/update and on every direct connection. Host names are resolved as given, without DNS search domains. With a forward proxy (DESTINATIONS_PROXY_URL), only the create/update check applies, since the proxy resolves and connects to the target; configure the proxy to refuse private addresses. HTTP_PROXY/HTTPS_PROXY are ignored for destinations while this is on. Defaults to false." required:"N"` Webhook DestinationWebhookConfig `yaml:"webhook" desc:"Configuration specific to webhook destinations."` AWSKinesis DestinationAWSKinesisConfig `yaml:"aws_kinesis" desc:"Configuration specific to AWS Kinesis destinations."` AWSEventBridge DestinationAWSEventBridgeConfig `yaml:"aws_eventbridge" desc:"Configuration specific to AWS EventBridge destinations."` diff --git a/internal/destregistry/httpclient.go b/internal/destregistry/httpclient.go index 9bb3f1e63..ad85c6159 100644 --- a/internal/destregistry/httpclient.go +++ b/internal/destregistry/httpclient.go @@ -37,8 +37,9 @@ type HTTPClientConfig struct { OnConnection func(reused bool) // Guard, if set, limits connections to addresses it allows and ignores - // HTTP_PROXY/HTTPS_PROXY. Through a forward proxy only the target URL is - // checked, so the proxy must refuse private addresses itself. + // HTTP_PROXY/HTTPS_PROXY. With a forward proxy, connections go to the + // proxy and are not checked: the proxy resolves the target and must + // refuse private addresses itself. Guard *netguard.Guard } @@ -92,10 +93,6 @@ func NewHTTPClient(config HTTPClientConfig) (*http.Client, error) { } } - if config.Guard != nil && len(config.Proxy) > 0 { - rt = config.Guard.WrapTransport(rt) - } - if config.UserAgent != nil { rt = &userAgentTransport{ userAgent: *config.UserAgent, diff --git a/internal/destregistry/httpclient_guard_test.go b/internal/destregistry/httpclient_guard_test.go index 903efa525..b93c470c9 100644 --- a/internal/destregistry/httpclient_guard_test.go +++ b/internal/destregistry/httpclient_guard_test.go @@ -5,7 +5,6 @@ import ( "net" "net/http" "net/http/httptest" - "net/url" "strings" "sync/atomic" "testing" @@ -112,13 +111,13 @@ func TestHTTPClientGuard_ProxyChain(t *testing.T) { assert.False(t, netguard.IsAddressNotAllowed(err)) assert.Equal(t, []string{"public.example:443"}, proxies[hops-1].Connects()) - // A private target is refused before any CONNECT. + // Private targets go to the proxy unchecked: it resolves the target and enforces. for _, target := range []string{"https://internal.example/hook", "https://10.0.0.1/hook", "https://[::1]/hook"} { _, err = getBody(t, client, target) require.Error(t, err, target) - assert.True(t, netguard.IsAddressNotAllowed(err), "%s: %v", target, err) + assert.False(t, netguard.IsAddressNotAllowed(err), "%s: %v", target, err) } - assert.Len(t, proxies[hops-1].Connects(), 1) + assert.Equal(t, []string{"public.example:443", "internal.example:443", "10.0.0.1:443", "[::1]:443"}, proxies[hops-1].Connects()) }) } } @@ -147,27 +146,6 @@ func TestHTTPClientGuard_RedirectToPrivateAddressRefused(t *testing.T) { assert.Zero(t, landed.Load()) } -// With a forward proxy the guard checks every request, so a redirect to a -// private target is refused before it reaches the proxy. -func TestHTTPClientGuard_ProxyRedirectToPrivateAddressRefused(t *testing.T) { - t.Parallel() - var seen atomic.Int32 - proxy := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - seen.Add(1) - http.Redirect(w, r, "http://internal.example/landed", http.StatusFound) - })) - t.Cleanup(proxy.Close) - hop, err := url.Parse(proxy.URL) - require.NoError(t, err) - - res := netguardtest.NewDNS(t).Set("public.example", []string{"8.8.8.8"}).Set("internal.example", []string{"10.0.0.1"}).Resolver - client := newGuardedClient(t, destregistry.HTTPClientConfig{Proxy: []*url.URL{hop}, Guard: &netguard.Guard{Resolver: res}}) - _, err = getBody(t, client, "http://public.example/start") - require.Error(t, err) - assert.True(t, netguard.IsAddressNotAllowed(err)) - assert.EqualValues(t, 1, seen.Load(), "only the first hop reached the proxy") -} - func TestHTTPClientGuard_NilGuardLeavesTransportAlone(t *testing.T) { t.Parallel() client, err := destregistry.NewHTTPClient(destregistry.HTTPClientConfig{}) diff --git a/internal/destregistry/providers/destwebhook/destwebhook.go b/internal/destregistry/providers/destwebhook/destwebhook.go index ebf17a807..9f20992d4 100644 --- a/internal/destregistry/providers/destwebhook/destwebhook.go +++ b/internal/destregistry/providers/destwebhook/destwebhook.go @@ -210,9 +210,9 @@ func WithProxy(hops []*url.URL) Option { } // WithAddressGuard limits webhooks to addresses the guard allows: the URL is -// checked when a destination is created or updated, and every connection is -// checked on delivery. HTTP_PROXY/HTTPS_PROXY are then ignored. See -// destregistry.HTTPClientConfig.Guard for forward proxies. +// checked when a destination is created or updated, and every direct +// connection is checked on delivery. HTTP_PROXY/HTTPS_PROXY are then ignored. +// Through a forward proxy only the create/update check applies. func WithAddressGuard(g *netguard.Guard) Option { return func(w *WebhookDestination) { w.guard = g diff --git a/internal/destregistry/providers/destwebhook/destwebhook_guard_test.go b/internal/destregistry/providers/destwebhook/destwebhook_guard_test.go index 25727ed72..6818e3c08 100644 --- a/internal/destregistry/providers/destwebhook/destwebhook_guard_test.go +++ b/internal/destregistry/providers/destwebhook/destwebhook_guard_test.go @@ -112,8 +112,7 @@ func TestWebhookPublisher_AddressGuard(t *testing.T) { assert.Equal(t, "address_not_allowed", delivery.Code) assert.Zero(t, hits.Load()) }) - - t.Run("forward proxy: the target is checked before the request", func(t *testing.T) { + t.Run("forward proxy: the delivery is not checked", func(t *testing.T) { var proxied atomic.Int32 proxy := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { proxied.Add(1) @@ -122,21 +121,14 @@ func TestWebhookPublisher_AddressGuard(t *testing.T) { t.Cleanup(proxy.Close) proxyURL, err := url.Parse(proxy.URL) require.NoError(t, err) - res := netguardtest.NewDNS(t).Set("hooks.example", []string{"8.8.8.8"}).Resolver provider := NewTestProvider(t, destwebhook.WithProxy([]*url.URL{proxyURL}), - destwebhook.WithAddressGuard(&netguard.Guard{Resolver: res}), + destwebhook.WithAddressGuard(&netguard.Guard{}), ) delivery, err := publish(t, provider, "http://10.0.0.1/hook") require.Error(t, err) require.NotNil(t, delivery) - assert.Equal(t, "address_not_allowed", delivery.Code) - assert.Zero(t, proxied.Load(), "the proxy saw nothing") - - delivery, err = publish(t, provider, "http://hooks.example/hook") - require.Error(t, err) - require.NotNil(t, delivery) assert.Equal(t, "502", delivery.Code) assert.EqualValues(t, 1, proxied.Load()) }) diff --git a/internal/netguard/transport.go b/internal/netguard/transport.go deleted file mode 100644 index 58da845bf..000000000 --- a/internal/netguard/transport.go +++ /dev/null @@ -1,42 +0,0 @@ -package netguard - -import ( - "errors" - "net/http" -) - -// WrapTransport returns a RoundTripper that checks each request's target URL -// with CheckURL before it goes to base. It is for a base that connects -// through a forward proxy, which resolves the target itself and whose own -// address is not guarded. The request is never modified. -func (g *Guard) WrapTransport(base http.RoundTripper) http.RoundTripper { - return &guardTransport{base: base, guard: g} -} - -type guardTransport struct { - base http.RoundTripper - guard *Guard -} - -func (t *guardTransport) RoundTrip(req *http.Request) (*http.Response, error) { - closeBody := func() { - if req.Body != nil { - _ = req.Body.Close() - } - } - if req.URL == nil { - closeBody() - return nil, errors.New("netguard: nil Request.URL") - } - if err := t.guard.CheckURL(req.Context(), req.URL); err != nil { - closeBody() - return nil, err - } - return t.base.RoundTrip(req) -} - -func (t *guardTransport) CloseIdleConnections() { - if c, ok := t.base.(interface{ CloseIdleConnections() }); ok { - c.CloseIdleConnections() - } -} diff --git a/internal/netguard/transport_test.go b/internal/netguard/transport_test.go deleted file mode 100644 index 5019abc00..000000000 --- a/internal/netguard/transport_test.go +++ /dev/null @@ -1,167 +0,0 @@ -package netguard_test - -import ( - "crypto/tls" - "crypto/x509" - "io" - "net" - "net/http" - "net/http/httptest" - "net/url" - "strings" - "sync" - "sync/atomic" - "testing" - - "github.com/hookdeck/outpost/internal/netguard" - "github.com/hookdeck/outpost/internal/netguard/netguardtest" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func get(t *testing.T, rt http.RoundTripper, rawURL string) (*http.Response, error) { - t.Helper() - req, err := http.NewRequest(http.MethodGet, rawURL, nil) - require.NoError(t, err) - return rt.RoundTrip(req) -} - -type roundTripFunc func(*http.Request) (*http.Response, error) - -func (f roundTripFunc) RoundTrip(r *http.Request) (*http.Response, error) { return f(r) } - -type closeTracker struct { - io.Reader - closed atomic.Bool -} - -func (c *closeTracker) Close() error { - c.closed.Store(true) - return nil -} - -// proxyRecorder is an httptest server standing in for a forward proxy: it -// records what it is asked to forward and answers 502. -type proxyRecorder struct { - *httptest.Server - mu sync.Mutex - reqs []string -} - -func newProxy(t *testing.T) *proxyRecorder { - p := &proxyRecorder{} - p.Server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - p.mu.Lock() - p.reqs = append(p.reqs, r.Method+" "+r.Host) - p.mu.Unlock() - w.WriteHeader(http.StatusBadGateway) - })) - t.Cleanup(p.Close) - return p -} - -func (p *proxyRecorder) seen() []string { - p.mu.Lock() - defer p.mu.Unlock() - return append([]string(nil), p.reqs...) -} - -func TestWrapTransport_ProxyMode(t *testing.T) { - t.Parallel() - proxy := newProxy(t) - proxyURL, err := url.Parse(proxy.URL) - require.NoError(t, err) - - // The proxy sits on unallowlisted 127.0.0.1 and its own address is not - // checked. - res := newResolver().set("public.example", []string{"8.8.8.8"}).set("internal.example", []string{"10.0.0.1"}) - g := &netguard.Guard{Resolver: res} - base := &http.Transport{Proxy: http.ProxyURL(proxyURL)} - t.Cleanup(base.CloseIdleConnections) - rt := g.WrapTransport(base) - - // A private target is refused before the proxy sees anything, and the - // request body is closed. - body := &closeTracker{Reader: strings.NewReader("x")} - req, err := http.NewRequest(http.MethodPost, "http://internal.example/hook", body) - require.NoError(t, err) - _, err = rt.RoundTrip(req) - require.Error(t, err) - assert.True(t, netguard.IsAddressNotAllowed(err)) - assert.True(t, body.closed.Load()) - _, err = get(t, rt, "http://10.0.0.1/") - assert.True(t, netguard.IsAddressNotAllowed(err)) - assert.Empty(t, proxy.seen()) - - // A public target is forwarded by name. - resp, err := get(t, rt, "http://public.example/hook") - require.NoError(t, err) - resp.Body.Close() - assert.Equal(t, http.StatusBadGateway, resp.StatusCode) - assert.Equal(t, []string{"GET public.example"}, proxy.seen()) -} - -func TestWrapTransport_NilURL(t *testing.T) { - t.Parallel() - g := &netguard.Guard{} - rt := g.WrapTransport(roundTripFunc(func(*http.Request) (*http.Response, error) { - t.Fatal("must not be reached") - return nil, nil - })) - _, err := rt.RoundTrip(&http.Request{}) - assert.Error(t, err) -} - -// The dial connects to the resolved address while TLS (server name, certificate -// check) and the Host header keep the original name. httptest's certificate -// covers example.com and *.example.com. -func TestDialContext_PreservesSNIAndHost(t *testing.T) { - t.Parallel() - for _, h2 := range []bool{false, true} { - t.Run(map[bool]string{false: "http1", true: "http2"}[h2], func(t *testing.T) { - t.Parallel() - type seen struct{ serverName, host string } - var mu sync.Mutex - var got []seen - srv := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - mu.Lock() - got = append(got, seen{r.TLS.ServerName, r.Host}) - mu.Unlock() - _, _ = io.WriteString(w, r.Proto) - })) - srv.EnableHTTP2 = h2 - srv.StartTLS() - t.Cleanup(srv.Close) - _, port, _ := net.SplitHostPort(srv.Listener.Addr().String()) - roots := x509.NewCertPool() - roots.AddCert(srv.Certificate()) - - dns := netguardtest.NewDNS(t). - Set("hooks.example.com", []string{"127.0.0.1"}). - Set("hooks.other.test", []string{"127.0.0.1"}) - g := &netguard.Guard{Resolver: dns.Resolver, Allowlist: mustAllowlist(t, "127.0.0.0/8")} - base := &http.Transport{ - DialContext: g.DialContext, - ForceAttemptHTTP2: true, - TLSClientConfig: &tls.Config{RootCAs: roots}, - } - t.Cleanup(base.CloseIdleConnections) - rt := base - - resp, err := get(t, rt, "https://hooks.example.com:"+port+"/hook") - require.NoError(t, err) - body, _ := io.ReadAll(resp.Body) - resp.Body.Close() - assert.Equal(t, map[bool]int{false: 1, true: 2}[h2], resp.ProtoMajor, string(body)) - mu.Lock() - require.Len(t, got, 1) - assert.Equal(t, seen{"hooks.example.com", "hooks.example.com:" + port}, got[0]) - mu.Unlock() - - // The certificate is checked against the name, not the IP. - _, err = get(t, rt, "https://hooks.other.test:"+port+"/hook") - require.Error(t, err) - assert.Contains(t, err.Error(), "x509") - }) - } -} From fd89fd5578f5976f6d3e1b327fea67ac05bb638f Mon Sep 17 00:00:00 2001 From: Alex Luong Date: Fri, 30 Oct 2026 16:15:19 +0700 Subject: [PATCH 07/13] feat: warn at startup when HTTP_PROXY is ignored for destinations Co-Authored-By: Claude Opus 5.5 --- docs/content/self-hosting/configuration.mdoc | 2 +- internal/config/destinations.go | 2 +- internal/destregistry/providers/default.go | 16 ++++++++++++++ .../destregistry/providers/default_test.go | 21 +++++++++++++++++++ internal/services/builder.go | 6 +++++- 5 files changed, 44 insertions(+), 3 deletions(-) diff --git a/docs/content/self-hosting/configuration.mdoc b/docs/content/self-hosting/configuration.mdoc index 38b7cb98b..50ab3cebc 100644 --- a/docs/content/self-hosting/configuration.mdoc +++ b/docs/content/self-hosting/configuration.mdoc @@ -150,7 +150,7 @@ Choose one for event log persistence: | `DESTINATIONS_PROXY_URL` | — | HTTP `CONNECT` forward proxy for destination connections, e.g. `http://user:pass@proxy:8080`. Multiple whitespace-separated URLs are tunneled in order. Applies to webhook, RabbitMQ and Kafka destinations; other destination types connect directly. The deprecated `DESTINATIONS_WEBHOOK_PROXY_URL`, when set, takes precedence for webhooks; there is no way to proxy RabbitMQ and Kafka without also proxying webhooks. See [Webhook Forward Proxy](/docs/outpost/self-hosting/guides/webhook-proxy#other-destination-types). | | `DESTINATIONS_METADATA_PATH` | — | Optional. Filesystem path to a directory of [custom destination metadata](https://github.com/hookdeck/outpost/tree/main/internal/destregistry/metadata/providers) (per-type `metadata.json` and `instructions.md`). Non-core fields such as `label`, `description`, `icon`, and `instructions` can be customized; `config_fields` and `credential_fields` cannot be overridden. | | `DESTINATIONS_AWS_EVENTBRIDGE_SOURCE` | `outpost` | `Source` set on every event published by [AWS EventBridge](/docs/outpost/destinations/aws-eventbridge) destinations. Must not be empty or start with `aws.`. | -| `DESTINATIONS_BLOCK_PRIVATE_ADDRESSES` | `false` | Currently applies to webhook destinations. If `true`, destinations connect only to publicly routable addresses (per the IANA special-purpose address registries) or to addresses in `DESTINATIONS_PRIVATE_ADDRESS_ALLOWLIST`. Destination URLs are checked on create and update (validation error) and on every direct connection; a blocked delivery fails with the code `address_not_allowed`. Host names are resolved as given, without DNS search domains. With a forward proxy (`DESTINATIONS_PROXY_URL`), only the create/update check applies, since the proxy resolves and connects to the target; configure the proxy to refuse private addresses. `HTTP_PROXY` and `HTTPS_PROXY` are ignored while this is on. Internationalized host names must be given in punycode. | +| `DESTINATIONS_BLOCK_PRIVATE_ADDRESSES` | `false` | Currently applies to webhook destinations. If `true`, destinations connect only to publicly routable addresses (per the IANA special-purpose address registries) or to addresses in `DESTINATIONS_PRIVATE_ADDRESS_ALLOWLIST`. Destination URLs are checked on create and update (validation error) and on every direct connection; a blocked delivery fails with the code `address_not_allowed`. Host names are resolved as given, without DNS search domains. With a forward proxy (`DESTINATIONS_PROXY_URL`), only the create/update check applies, since the proxy resolves and connects to the target; configure the proxy to refuse private addresses. `HTTP_PROXY` and `HTTPS_PROXY` are ignored while this is on (a warning is logged at startup). Internationalized host names must be given in punycode. | | `DESTINATIONS_PRIVATE_ADDRESS_ALLOWLIST` | — | Comma-separated IP addresses and CIDR ranges that webhooks may still reach while `DESTINATIONS_BLOCK_PRIVATE_ADDRESSES` is on, e.g. `10.0.0.0/8,192.168.1.5`. `localhost` stands for `127.0.0.0/8` and `::1`. Host names, `/0` ranges and ranges with host bits set (`10.0.0.1/8`) fail startup, also while blocking is off. | ## Webhook Behavior diff --git a/internal/config/destinations.go b/internal/config/destinations.go index 3261e5ee8..b53eb7134 100644 --- a/internal/config/destinations.go +++ b/internal/config/destinations.go @@ -15,7 +15,7 @@ type DestinationsConfig struct { IncludeMillisecondTimestamp bool `yaml:"include_millisecond_timestamp" env:"DESTINATIONS_INCLUDE_MILLISECOND_TIMESTAMP" desc:"If true, includes a 'timestamp-ms' field with millisecond precision in destination metadata. Useful for load testing and debugging." required:"N"` ProxyURL string `yaml:"proxy_url" env:"DESTINATIONS_PROXY_URL" desc:"HTTP CONNECT forward proxy for destination connections, e.g. 'http://user:pass@proxy:10000'. Multiple whitespace-separated URLs are tunneled in order, nearest first. Applies to webhook, RabbitMQ and Kafka destinations; other types connect directly. DESTINATIONS_WEBHOOK_PROXY_URL, when set, takes precedence for webhooks; webhooks can't opt out while this is set." required:"N"` PrivateAddressAllowlist []string `yaml:"private_address_allowlist" env:"DESTINATIONS_PRIVATE_ADDRESS_ALLOWLIST" envSeparator:"," desc:"Comma-separated IP addresses and CIDR ranges that webhooks may still reach while DESTINATIONS_BLOCK_PRIVATE_ADDRESSES is on (in YAML, a list). 'localhost' stands for 127.0.0.0/8 and ::1. Host names, /0 ranges and ranges with host bits set fail startup, also while blocking is off." required:"N"` - BlockPrivateAddresses bool `yaml:"block_private_addresses" env:"DESTINATIONS_BLOCK_PRIVATE_ADDRESSES" desc:"Currently applies to webhook destinations. If true, destinations connect only to publicly routable addresses (IANA special-purpose registries) or to addresses in DESTINATIONS_PRIVATE_ADDRESS_ALLOWLIST. URLs are checked on create/update and on every direct connection. Host names are resolved as given, without DNS search domains. With a forward proxy (DESTINATIONS_PROXY_URL), only the create/update check applies, since the proxy resolves and connects to the target; configure the proxy to refuse private addresses. HTTP_PROXY/HTTPS_PROXY are ignored for destinations while this is on. Defaults to false." required:"N"` + BlockPrivateAddresses bool `yaml:"block_private_addresses" env:"DESTINATIONS_BLOCK_PRIVATE_ADDRESSES" desc:"Currently applies to webhook destinations. If true, destinations connect only to publicly routable addresses (IANA special-purpose registries) or to addresses in DESTINATIONS_PRIVATE_ADDRESS_ALLOWLIST. URLs are checked on create/update and on every direct connection. Host names are resolved as given, without DNS search domains. With a forward proxy (DESTINATIONS_PROXY_URL), only the create/update check applies, since the proxy resolves and connects to the target; configure the proxy to refuse private addresses. HTTP_PROXY/HTTPS_PROXY are ignored for destinations while this is on (a warning is logged at startup). Defaults to false." required:"N"` Webhook DestinationWebhookConfig `yaml:"webhook" desc:"Configuration specific to webhook destinations."` AWSKinesis DestinationAWSKinesisConfig `yaml:"aws_kinesis" desc:"Configuration specific to AWS Kinesis destinations."` AWSEventBridge DestinationAWSEventBridgeConfig `yaml:"aws_eventbridge" desc:"Configuration specific to AWS EventBridge destinations."` diff --git a/internal/destregistry/providers/default.go b/internal/destregistry/providers/default.go index 8d757ef75..f2ea29713 100644 --- a/internal/destregistry/providers/default.go +++ b/internal/destregistry/providers/default.go @@ -4,6 +4,8 @@ import ( "context" "fmt" "net/url" + "os" + "strings" "github.com/hookdeck/outpost/internal/destregistry" "github.com/hookdeck/outpost/internal/destregistry/providers/destawseventbridge" @@ -81,6 +83,20 @@ type RegisterDefaultDestinationOptions struct { DeliveryMaxConcurrency int } +// WebhookEnvProxyIgnored reports whether HTTP_PROXY/HTTPS_PROXY are set but +// ignored for webhooks: the address guard is on and no webhook proxy is set. +func WebhookEnvProxyIgnored(opts RegisterDefaultDestinationOptions) bool { + if !opts.BlockPrivateAddresses || opts.Webhook == nil || strings.TrimSpace(opts.Webhook.ProxyURL) != "" { + return false + } + for _, k := range []string{"HTTP_PROXY", "http_proxy", "HTTPS_PROXY", "https_proxy"} { + if os.Getenv(k) != "" { + return true + } + } + return false +} + // RegisterDefault registers the default destination providers with the registry. // NOTE: The order of registration will determine the order of the provider array // returned when listing providers. diff --git a/internal/destregistry/providers/default_test.go b/internal/destregistry/providers/default_test.go index 448830c67..c65cb6015 100644 --- a/internal/destregistry/providers/default_test.go +++ b/internal/destregistry/providers/default_test.go @@ -114,3 +114,24 @@ func TestRegisterDefault_BlockPrivateAddresses_BadAllowlist(t *testing.T) { }) assert.ErrorContains(t, err, "private address allowlist") } + +func TestWebhookEnvProxyIgnored(t *testing.T) { + on := func(proxyURL string) destregistrydefault.RegisterDefaultDestinationOptions { + return destregistrydefault.RegisterDefaultDestinationOptions{ + BlockPrivateAddresses: true, + Webhook: &destregistrydefault.DestWebhookConfig{ProxyURL: proxyURL}, + } + } + for _, k := range []string{"HTTP_PROXY", "http_proxy", "HTTPS_PROXY", "https_proxy"} { + t.Setenv(k, "") + } + assert.False(t, destregistrydefault.WebhookEnvProxyIgnored(on(""))) + + t.Setenv("https_proxy", "http://proxy:3128") + assert.True(t, destregistrydefault.WebhookEnvProxyIgnored(on(""))) + assert.False(t, destregistrydefault.WebhookEnvProxyIgnored(on("http://proxy:3128"))) + + off := on("") + off.BlockPrivateAddresses = false + assert.False(t, destregistrydefault.WebhookEnvProxyIgnored(off)) +} diff --git a/internal/services/builder.go b/internal/services/builder.go index 54020beff..316bac686 100644 --- a/internal/services/builder.go +++ b/internal/services/builder.go @@ -570,7 +570,11 @@ func (s *serviceInstance) initDestRegistry(cfg *config.Config, logger *logging.L DestinationMetadataPath: cfg.Destinations.MetadataPath, DeliveryTimeout: time.Duration(cfg.DeliveryTimeoutSeconds) * time.Second, }, logger) - if err := destregistrydefault.RegisterDefault(registry, cfg.Destinations.ToConfig(cfg)); err != nil { + destCfg := cfg.Destinations.ToConfig(cfg) + if destregistrydefault.WebhookEnvProxyIgnored(destCfg) { + logger.Warn("HTTP_PROXY/HTTPS_PROXY are ignored for webhook destinations while DESTINATIONS_BLOCK_PRIVATE_ADDRESSES is on; set DESTINATIONS_PROXY_URL to deliver through a proxy", zap.String("service", s.name)) + } + if err := destregistrydefault.RegisterDefault(registry, destCfg); err != nil { logger.Error("destination registry setup failed", zap.String("service", s.name), zap.Error(err)) return err } From ff6280b487b7e2de81df93844c1a56eee281b47d Mon Sep 17 00:00:00 2001 From: Alex Luong Date: Fri, 30 Oct 2026 16:47:05 +0700 Subject: [PATCH 08/13] chore: tidy address guard comments and pin the ignored env proxy Co-Authored-By: Claude Opus 5.5 --- internal/destregistry/httpclient_guard_test.go | 4 ++++ .../providers/destwebhook/destwebhook.go | 6 ++---- .../destwebhook/destwebhook_guard_test.go | 3 ++- .../providers/destwebhook/httphelper.go | 16 ++++++++-------- internal/netguard/guard.go | 4 ++-- internal/services/builder.go | 2 +- 6 files changed, 19 insertions(+), 16 deletions(-) diff --git a/internal/destregistry/httpclient_guard_test.go b/internal/destregistry/httpclient_guard_test.go index b93c470c9..e07eb0720 100644 --- a/internal/destregistry/httpclient_guard_test.go +++ b/internal/destregistry/httpclient_guard_test.go @@ -79,6 +79,10 @@ func TestHTTPClientGuard_IgnoresEnvironmentProxy(t *testing.T) { _, port, _ := net.SplitHostPort(dest.Listener.Addr().String()) res := netguardtest.NewDNS(t).Set("hooks.example.com", []string{"127.0.0.1"}).Resolver client := newGuardedClient(t, destregistry.HTTPClientConfig{Guard: guardAllowing(t, res, "localhost")}) + // Go caches the proxy environment per process, so also check the setting. + tr, ok := client.Transport.(*http.Transport) + require.True(t, ok) + assert.Nil(t, tr.Proxy) body, err := getBody(t, client, "http://hooks.example.com:"+port+"/") require.NoError(t, err) assert.Equal(t, "ok", body) diff --git a/internal/destregistry/providers/destwebhook/destwebhook.go b/internal/destregistry/providers/destwebhook/destwebhook.go index 9f20992d4..109d67f23 100644 --- a/internal/destregistry/providers/destwebhook/destwebhook.go +++ b/internal/destregistry/providers/destwebhook/destwebhook.go @@ -209,10 +209,8 @@ func WithProxy(hops []*url.URL) Option { } } -// WithAddressGuard limits webhooks to addresses the guard allows: the URL is -// checked when a destination is created or updated, and every direct -// connection is checked on delivery. HTTP_PROXY/HTTPS_PROXY are then ignored. -// Through a forward proxy only the create/update check applies. +// WithAddressGuard limits webhooks to addresses the guard allows: on +// create/update, and on every delivery made without a forward proxy. func WithAddressGuard(g *netguard.Guard) Option { return func(w *WebhookDestination) { w.guard = g diff --git a/internal/destregistry/providers/destwebhook/destwebhook_guard_test.go b/internal/destregistry/providers/destwebhook/destwebhook_guard_test.go index 6818e3c08..88e3338cb 100644 --- a/internal/destregistry/providers/destwebhook/destwebhook_guard_test.go +++ b/internal/destregistry/providers/destwebhook/destwebhook_guard_test.go @@ -53,7 +53,7 @@ func TestWebhookDestination_Validate_AddressGuard(t *testing.T) { d := webhookTo(u) requireURLError(t, guarded.Validate(context.Background(), &d), "address_not_allowed") } - // The schema rejects the second before the guard runs. + // "http://" fails the schema's pattern before the guard runs. for u, errType := range map[string]string{"http://[::1": "invalid", "http://": "pattern"} { d := webhookTo(u) requireURLError(t, guarded.Validate(context.Background(), &d), errType) @@ -112,6 +112,7 @@ func TestWebhookPublisher_AddressGuard(t *testing.T) { assert.Equal(t, "address_not_allowed", delivery.Code) assert.Zero(t, hits.Load()) }) + t.Run("forward proxy: the delivery is not checked", func(t *testing.T) { var proxied atomic.Int32 proxy := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/internal/destregistry/providers/destwebhook/httphelper.go b/internal/destregistry/providers/destwebhook/httphelper.go index 8d1dfa865..e8677df29 100644 --- a/internal/destregistry/providers/destwebhook/httphelper.go +++ b/internal/destregistry/providers/destwebhook/httphelper.go @@ -134,15 +134,15 @@ func ExecuteHTTPRequest(ctx context.Context, client *http.Client, req *http.Requ // All errors classified here are destination-level failures (DeliveryError → ack + retry). // // Error codes and their meanings: -// - address_not_allowed: Destination address is blocked by the address guard -// - dns_error: Domain doesn't exist or DNS lookup failed -// - connection_refused: Server not running or rejecting connections -// - connection_reset: Connection was dropped by the server +// - address_not_allowed: Address is private and not allowlisted +// - dns_error: Domain doesn't exist or DNS lookup failed +// - connection_refused: Server not running or rejecting connections +// - connection_reset: Connection was dropped by the server // - network_unreachable: Network path to destination is unavailable -// - timeout: Request took too long (I/O timeout or context deadline) -// - tls_error: TLS/SSL certificate or handshake failure -// - redirect_error: Too many redirects -// - network_error: Other network-related failures (catch-all) +// - timeout: Request took too long (I/O timeout or context deadline) +// - tls_error: TLS/SSL certificate or handshake failure +// - redirect_error: Too many redirects +// - network_error: Other network-related failures (catch-all) func ClassifyNetworkError(err error) string { if err == nil { return "unknown" diff --git a/internal/netguard/guard.go b/internal/netguard/guard.go index 024c3ba26..a34baf1c4 100644 --- a/internal/netguard/guard.go +++ b/internal/netguard/guard.go @@ -3,8 +3,8 @@ // and again on every new connection (Guard.DialContext): every address a // connection is about to be made to must be globally reachable per the IANA // special-purpose registries or allowlisted, so a DNS answer that changes in -// between (rebinding) can't send it to a blocked address. Non-ASCII host names must be given -// in punycode. +// between (rebinding) can't send it to a blocked address. Non-ASCII host +// names must be given in punycode. package netguard import ( diff --git a/internal/services/builder.go b/internal/services/builder.go index 316bac686..20490d6dc 100644 --- a/internal/services/builder.go +++ b/internal/services/builder.go @@ -572,7 +572,7 @@ func (s *serviceInstance) initDestRegistry(cfg *config.Config, logger *logging.L }, logger) destCfg := cfg.Destinations.ToConfig(cfg) if destregistrydefault.WebhookEnvProxyIgnored(destCfg) { - logger.Warn("HTTP_PROXY/HTTPS_PROXY are ignored for webhook destinations while DESTINATIONS_BLOCK_PRIVATE_ADDRESSES is on; set DESTINATIONS_PROXY_URL to deliver through a proxy", zap.String("service", s.name)) + logger.Warn("HTTP_PROXY/HTTPS_PROXY are ignored for webhook destinations while DESTINATIONS_BLOCK_PRIVATE_ADDRESSES is on; set DESTINATIONS_PROXY_URL to deliver through a proxy (the proxy must then refuse private addresses)", zap.String("service", s.name)) } if err := destregistrydefault.RegisterDefault(registry, destCfg); err != nil { logger.Error("destination registry setup failed", zap.String("service", s.name), zap.Error(err)) From 3e50468da007d625b71ba2d8a66608fa106072f8 Mon Sep 17 00:00:00 2001 From: Alex Luong Date: Fri, 30 Oct 2026 17:26:52 +0700 Subject: [PATCH 09/13] docs: add outbound network access guidance for self-hosted deployments Co-Authored-By: Claude Opus 5.5 --- docs/content/self-hosting/configuration.mdoc | 2 +- docs/content/self-hosting/deployment.mdoc | 22 +++++++++++++++++++ .../self-hosting/guides/webhook-proxy.mdoc | 2 ++ 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/docs/content/self-hosting/configuration.mdoc b/docs/content/self-hosting/configuration.mdoc index 50ab3cebc..cfe7a1614 100644 --- a/docs/content/self-hosting/configuration.mdoc +++ b/docs/content/self-hosting/configuration.mdoc @@ -150,7 +150,7 @@ Choose one for event log persistence: | `DESTINATIONS_PROXY_URL` | — | HTTP `CONNECT` forward proxy for destination connections, e.g. `http://user:pass@proxy:8080`. Multiple whitespace-separated URLs are tunneled in order. Applies to webhook, RabbitMQ and Kafka destinations; other destination types connect directly. The deprecated `DESTINATIONS_WEBHOOK_PROXY_URL`, when set, takes precedence for webhooks; there is no way to proxy RabbitMQ and Kafka without also proxying webhooks. See [Webhook Forward Proxy](/docs/outpost/self-hosting/guides/webhook-proxy#other-destination-types). | | `DESTINATIONS_METADATA_PATH` | — | Optional. Filesystem path to a directory of [custom destination metadata](https://github.com/hookdeck/outpost/tree/main/internal/destregistry/metadata/providers) (per-type `metadata.json` and `instructions.md`). Non-core fields such as `label`, `description`, `icon`, and `instructions` can be customized; `config_fields` and `credential_fields` cannot be overridden. | | `DESTINATIONS_AWS_EVENTBRIDGE_SOURCE` | `outpost` | `Source` set on every event published by [AWS EventBridge](/docs/outpost/destinations/aws-eventbridge) destinations. Must not be empty or start with `aws.`. | -| `DESTINATIONS_BLOCK_PRIVATE_ADDRESSES` | `false` | Currently applies to webhook destinations. If `true`, destinations connect only to publicly routable addresses (per the IANA special-purpose address registries) or to addresses in `DESTINATIONS_PRIVATE_ADDRESS_ALLOWLIST`. Destination URLs are checked on create and update (validation error) and on every direct connection; a blocked delivery fails with the code `address_not_allowed`. Host names are resolved as given, without DNS search domains. With a forward proxy (`DESTINATIONS_PROXY_URL`), only the create/update check applies, since the proxy resolves and connects to the target; configure the proxy to refuse private addresses. `HTTP_PROXY` and `HTTPS_PROXY` are ignored while this is on (a warning is logged at startup). Internationalized host names must be given in punycode. | +| `DESTINATIONS_BLOCK_PRIVATE_ADDRESSES` | `false` | Currently applies to webhook destinations. If `true`, destinations connect only to publicly routable addresses (per the IANA special-purpose address registries) or to addresses in `DESTINATIONS_PRIVATE_ADDRESS_ALLOWLIST`. Destination URLs are checked on create and update (validation error) and on every direct connection; a blocked delivery fails with the code `address_not_allowed`. Host names are resolved as given, without DNS search domains. With a forward proxy (`DESTINATIONS_PROXY_URL`), only the create/update check applies, since the proxy resolves and connects to the target; configure the proxy to refuse private addresses. `HTTP_PROXY` and `HTTPS_PROXY` are ignored while this is on (a warning is logged at startup). Internationalized host names must be given in punycode. See [Outbound Network Access](/docs/outpost/self-hosting/deployment#outbound-network-access). | | `DESTINATIONS_PRIVATE_ADDRESS_ALLOWLIST` | — | Comma-separated IP addresses and CIDR ranges that webhooks may still reach while `DESTINATIONS_BLOCK_PRIVATE_ADDRESSES` is on, e.g. `10.0.0.0/8,192.168.1.5`. `localhost` stands for `127.0.0.0/8` and `::1`. Host names, `/0` ranges and ranges with host bits set (`10.0.0.1/8`) fail startup, also while blocking is off. | ## Webhook Behavior diff --git a/docs/content/self-hosting/deployment.mdoc b/docs/content/self-hosting/deployment.mdoc index e36209e7d..b793a3d48 100644 --- a/docs/content/self-hosting/deployment.mdoc +++ b/docs/content/self-hosting/deployment.mdoc @@ -49,3 +49,25 @@ Outpost requires the following dependencies: - Publish queue (optional) Each of these will need to be provisioned and allocated sufficient resources based on expected usage and load. + +## Outbound Network Access + +Tenants choose where their destinations point, and Outpost stores the response to each webhook delivery. If your tenants are outside your organization, a tenant could point a destination at a service on your internal network, such as an internal API or your cloud provider's metadata endpoint, and read the response in its delivery logs. If only your own team creates destinations, you can skip this section. + +When tenants are outside your organization, we recommend both of the following. + +### Restrict outbound traffic + +Limit where Outpost can connect at the network level: allow the public internet and Outpost's own dependencies (Redis, the database and the message queues), and deny private ranges (`10.0.0.0/8`, `172.16.0.0/12`, `192.168.0.0/16`, `fc00::/7`) and link-local ranges (`169.254.0.0/16`, which includes the cloud metadata endpoint, and `fe80::/10`). On Kubernetes, use a NetworkPolicy on the Outpost pods; elsewhere, use your cloud provider's firewall or security groups. This covers every destination type. + +Network rules don't apply to connections to the same machine or pod (`localhost`), so also enable the setting below. + +### Block private addresses in Outpost + +Set `DESTINATIONS_BLOCK_PRIVATE_ADDRESSES=true`. Outpost then rejects webhook destination URLs that point to a private address when they are created or updated, and fails deliveries to a private address with the error code `address_not_allowed`, including when the host name's DNS changes after the destination was created. + +To let tenants reach specific internal services, list their IPs or ranges in `DESTINATIONS_PRIVATE_ADDRESS_ALLOWLIST`. See [Configuration](/docs/outpost/self-hosting/configuration) for both settings. + +### With a forward proxy + +When `DESTINATIONS_PROXY_URL` is set, the [forward proxy](/docs/outpost/self-hosting/guides/webhook-proxy) connects to destinations instead of Outpost, so Outpost can only check URLs when destinations are created or updated. Apply the outbound rules above to the proxy instead. diff --git a/docs/content/self-hosting/guides/webhook-proxy.mdoc b/docs/content/self-hosting/guides/webhook-proxy.mdoc index 6977d6fd5..0b6a94e3d 100644 --- a/docs/content/self-hosting/guides/webhook-proxy.mdoc +++ b/docs/content/self-hosting/guides/webhook-proxy.mdoc @@ -20,6 +20,8 @@ The proxy applies to every webhook destination served by the deployment. When set, Outpost installs an HTTP proxy on the webhook publisher's transport. HTTPS destinations use the standard `CONNECT` tunneling flow; HTTP destinations are forwarded request-by-request. +If your tenants are outside your organization, configure the proxy to refuse private addresses. Outpost doesn't check delivery addresses when a proxy is set. See [Outbound Network Access](/docs/outpost/self-hosting/deployment#outbound-network-access). + ## Failure attribution Outpost separates failures the proxy is responsible for from failures the destination is responsible for: From f5f229a8aefbff62fc5488138221816c80a64f72 Mon Sep 17 00:00:00 2001 From: Alex Luong Date: Fri, 30 Oct 2026 18:37:50 +0700 Subject: [PATCH 10/13] docs: keep forward proxy note in the proxy guide only Co-Authored-By: Claude Opus 5.5 --- docs/content/self-hosting/deployment.mdoc | 4 ---- docs/content/self-hosting/guides/webhook-proxy.mdoc | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/docs/content/self-hosting/deployment.mdoc b/docs/content/self-hosting/deployment.mdoc index b793a3d48..318e26cd8 100644 --- a/docs/content/self-hosting/deployment.mdoc +++ b/docs/content/self-hosting/deployment.mdoc @@ -67,7 +67,3 @@ Network rules don't apply to connections to the same machine or pod (`localhost` Set `DESTINATIONS_BLOCK_PRIVATE_ADDRESSES=true`. Outpost then rejects webhook destination URLs that point to a private address when they are created or updated, and fails deliveries to a private address with the error code `address_not_allowed`, including when the host name's DNS changes after the destination was created. To let tenants reach specific internal services, list their IPs or ranges in `DESTINATIONS_PRIVATE_ADDRESS_ALLOWLIST`. See [Configuration](/docs/outpost/self-hosting/configuration) for both settings. - -### With a forward proxy - -When `DESTINATIONS_PROXY_URL` is set, the [forward proxy](/docs/outpost/self-hosting/guides/webhook-proxy) connects to destinations instead of Outpost, so Outpost can only check URLs when destinations are created or updated. Apply the outbound rules above to the proxy instead. diff --git a/docs/content/self-hosting/guides/webhook-proxy.mdoc b/docs/content/self-hosting/guides/webhook-proxy.mdoc index 0b6a94e3d..b3dba15d7 100644 --- a/docs/content/self-hosting/guides/webhook-proxy.mdoc +++ b/docs/content/self-hosting/guides/webhook-proxy.mdoc @@ -20,7 +20,7 @@ The proxy applies to every webhook destination served by the deployment. When set, Outpost installs an HTTP proxy on the webhook publisher's transport. HTTPS destinations use the standard `CONNECT` tunneling flow; HTTP destinations are forwarded request-by-request. -If your tenants are outside your organization, configure the proxy to refuse private addresses. Outpost doesn't check delivery addresses when a proxy is set. See [Outbound Network Access](/docs/outpost/self-hosting/deployment#outbound-network-access). +Since the proxy connects to destinations, Outpost can't check delivery addresses while it is set. If your tenants are outside your organization, configure the proxy to refuse private addresses (see [Outbound Network Access](/docs/outpost/self-hosting/deployment#outbound-network-access)). ## Failure attribution From dc0fde495640be190cd69695ea138f221ddf9fec Mon Sep 17 00:00:00 2001 From: Alex Luong Date: Fri, 30 Oct 2026 18:38:21 +0700 Subject: [PATCH 11/13] docs: shorten private address config rows Co-Authored-By: Claude Opus 5.5 --- docs/content/self-hosting/configuration.mdoc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/content/self-hosting/configuration.mdoc b/docs/content/self-hosting/configuration.mdoc index cfe7a1614..b5d89b1a5 100644 --- a/docs/content/self-hosting/configuration.mdoc +++ b/docs/content/self-hosting/configuration.mdoc @@ -150,8 +150,8 @@ Choose one for event log persistence: | `DESTINATIONS_PROXY_URL` | — | HTTP `CONNECT` forward proxy for destination connections, e.g. `http://user:pass@proxy:8080`. Multiple whitespace-separated URLs are tunneled in order. Applies to webhook, RabbitMQ and Kafka destinations; other destination types connect directly. The deprecated `DESTINATIONS_WEBHOOK_PROXY_URL`, when set, takes precedence for webhooks; there is no way to proxy RabbitMQ and Kafka without also proxying webhooks. See [Webhook Forward Proxy](/docs/outpost/self-hosting/guides/webhook-proxy#other-destination-types). | | `DESTINATIONS_METADATA_PATH` | — | Optional. Filesystem path to a directory of [custom destination metadata](https://github.com/hookdeck/outpost/tree/main/internal/destregistry/metadata/providers) (per-type `metadata.json` and `instructions.md`). Non-core fields such as `label`, `description`, `icon`, and `instructions` can be customized; `config_fields` and `credential_fields` cannot be overridden. | | `DESTINATIONS_AWS_EVENTBRIDGE_SOURCE` | `outpost` | `Source` set on every event published by [AWS EventBridge](/docs/outpost/destinations/aws-eventbridge) destinations. Must not be empty or start with `aws.`. | -| `DESTINATIONS_BLOCK_PRIVATE_ADDRESSES` | `false` | Currently applies to webhook destinations. If `true`, destinations connect only to publicly routable addresses (per the IANA special-purpose address registries) or to addresses in `DESTINATIONS_PRIVATE_ADDRESS_ALLOWLIST`. Destination URLs are checked on create and update (validation error) and on every direct connection; a blocked delivery fails with the code `address_not_allowed`. Host names are resolved as given, without DNS search domains. With a forward proxy (`DESTINATIONS_PROXY_URL`), only the create/update check applies, since the proxy resolves and connects to the target; configure the proxy to refuse private addresses. `HTTP_PROXY` and `HTTPS_PROXY` are ignored while this is on (a warning is logged at startup). Internationalized host names must be given in punycode. See [Outbound Network Access](/docs/outpost/self-hosting/deployment#outbound-network-access). | -| `DESTINATIONS_PRIVATE_ADDRESS_ALLOWLIST` | — | Comma-separated IP addresses and CIDR ranges that webhooks may still reach while `DESTINATIONS_BLOCK_PRIVATE_ADDRESSES` is on, e.g. `10.0.0.0/8,192.168.1.5`. `localhost` stands for `127.0.0.0/8` and `::1`. Host names, `/0` ranges and ranges with host bits set (`10.0.0.1/8`) fail startup, also while blocking is off. | +| `DESTINATIONS_BLOCK_PRIVATE_ADDRESSES` | `false` | If `true`, webhook destinations only connect to public addresses and those in `DESTINATIONS_PRIVATE_ADDRESS_ALLOWLIST`. See [Outbound Network Access](/docs/outpost/self-hosting/deployment#outbound-network-access). | +| `DESTINATIONS_PRIVATE_ADDRESS_ALLOWLIST` | — | Comma-separated IPs and CIDR ranges destinations may still reach while `DESTINATIONS_BLOCK_PRIVATE_ADDRESSES` is on, e.g. `10.0.0.0/8,192.168.1.5`. | ## Webhook Behavior From 667a7c70ef8022396361e1e5ec00ea71d8a92ab7 Mon Sep 17 00:00:00 2001 From: Alex Luong Date: Fri, 30 Oct 2026 18:40:44 +0700 Subject: [PATCH 12/13] docs: name SSRF in outbound network access section Co-Authored-By: Claude Opus 5.5 --- docs/content/self-hosting/deployment.mdoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/content/self-hosting/deployment.mdoc b/docs/content/self-hosting/deployment.mdoc index 318e26cd8..36eee3868 100644 --- a/docs/content/self-hosting/deployment.mdoc +++ b/docs/content/self-hosting/deployment.mdoc @@ -52,7 +52,7 @@ Each of these will need to be provisioned and allocated sufficient resources bas ## Outbound Network Access -Tenants choose where their destinations point, and Outpost stores the response to each webhook delivery. If your tenants are outside your organization, a tenant could point a destination at a service on your internal network, such as an internal API or your cloud provider's metadata endpoint, and read the response in its delivery logs. If only your own team creates destinations, you can skip this section. +Tenants choose where their destinations point, and Outpost stores the response to each webhook delivery. If your tenants are outside your organization, this exposes you to [server-side request forgery (SSRF)](https://owasp.org/www-community/attacks/Server_Side_Request_Forgery): a tenant could point a destination at an internal service, such as your cloud provider's metadata endpoint, and read the response in its delivery logs. If only your own team creates destinations, you can skip this section. When tenants are outside your organization, we recommend both of the following. From 779289a57594ed71a1334715092366c974e9073c Mon Sep 17 00:00:00 2001 From: Alex Luong Date: Fri, 30 Oct 2026 18:40:57 +0700 Subject: [PATCH 13/13] docs: use canonical OWASP SSRF link Co-Authored-By: Claude Opus 5.5 --- docs/content/self-hosting/deployment.mdoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/content/self-hosting/deployment.mdoc b/docs/content/self-hosting/deployment.mdoc index 36eee3868..5569aaeb9 100644 --- a/docs/content/self-hosting/deployment.mdoc +++ b/docs/content/self-hosting/deployment.mdoc @@ -52,7 +52,7 @@ Each of these will need to be provisioned and allocated sufficient resources bas ## Outbound Network Access -Tenants choose where their destinations point, and Outpost stores the response to each webhook delivery. If your tenants are outside your organization, this exposes you to [server-side request forgery (SSRF)](https://owasp.org/www-community/attacks/Server_Side_Request_Forgery): a tenant could point a destination at an internal service, such as your cloud provider's metadata endpoint, and read the response in its delivery logs. If only your own team creates destinations, you can skip this section. +Tenants choose where their destinations point, and Outpost stores the response to each webhook delivery. If your tenants are outside your organization, this exposes you to [server-side request forgery (SSRF)](https://community.owasp.org/attacks/Server_Side_Request_Forgery): a tenant could point a destination at an internal service, such as your cloud provider's metadata endpoint, and read the response in its delivery logs. If only your own team creates destinations, you can skip this section. When tenants are outside your organization, we recommend both of the following.