From e5b68b2df7b1120c20b3f5ad441df3aae3d993af Mon Sep 17 00:00:00 2001 From: clawbot Date: Thu, 1 Oct 2026 20:39:44 +0000 Subject: [PATCH] Say how to allow a refused private target address (closes #398) Adding or editing an http or slack target whose address is private or reserved was refused with no hint that the refusal is deliberate or that it can be lifted. The refusal now adds that such addresses are refused by default and that the server's ALLOWED_EGRESS_CIDRS setting allows named networks, naming the README section "Allowing egress to your own network". Metadata refusals do not get it: link-local and the other unconditional metadata addresses cannot be opened, and Azure's WireServer, which listing does open, serves VM credentials. The delivery package refuses WireServer with its own error and exports the private-or-reserved one as ErrBlockedIP, so the handler can tell them apart. Model: opus-5-5 --- internal/delivery/ssrf.go | 27 +++- internal/handlers/source_management.go | 21 +++- .../handlers/target_private_refusal_test.go | 116 ++++++++++++++++++ 3 files changed, 154 insertions(+), 10 deletions(-) create mode 100644 internal/handlers/target_private_refusal_test.go diff --git a/internal/delivery/ssrf.go b/internal/delivery/ssrf.go index 0fa73b3..d923bdc 100644 --- a/internal/delivery/ssrf.go +++ b/internal/delivery/ssrf.go @@ -17,6 +17,10 @@ const ( // dnsResolutionTimeout is the maximum time to wait for // DNS resolution during SSRF validation. dnsResolutionTimeout = 5 * time.Second + + // azureWireServer is Azure's WireServer, a public address that + // serves VM credentials. + azureWireServer = "168.63.129.16" ) // Sentinel errors for SSRF validation. @@ -25,8 +29,14 @@ var ( errNoIPs = errors.New( "hostname resolved to no IP addresses", ) - errBlockedIP = errors.New( - "blocked private, reserved or cloud metadata address", + // ErrBlockedIP reports a private or reserved address the + // default blocklist refuses, one that ALLOWED_EGRESS_CIDRS + // can open. + ErrBlockedIP = errors.New( + "blocked private or reserved address", + ) + errBlockedWireServer = errors.New( + "blocked cloud metadata address", ) errBlockedMetadata = errors.New( "blocked link-local or cloud instance metadata " + @@ -123,8 +133,7 @@ func init() { "::1/128", "fc00::/7", "fe80::/10", - // Azure WireServer, a public address that serves VM credentials. - "168.63.129.16/32", + azureWireServer + "/32", }) // Every entry is named. The set must not grow or shrink @@ -338,9 +347,17 @@ func (g *Guard) checkIP(ip net.IP) error { return nil } + // WireServer is on the default blocklist but is a public + // address, so its refusal does not call it private or reserved. + if ip.Equal(net.ParseIP(azureWireServer)) { + return fmt.Errorf( + "target IP %s: %w", ip, errBlockedWireServer, + ) + } + if isBlockedIP(ip) { return fmt.Errorf( - "target IP %s: %w", ip, errBlockedIP, + "target IP %s: %w", ip, ErrBlockedIP, ) } diff --git a/internal/handlers/source_management.go b/internal/handlers/source_management.go index 9147c1f..08c4137 100644 --- a/internal/handlers/source_management.go +++ b/internal/handlers/source_management.go @@ -1577,11 +1577,22 @@ func (h *Handlers) validateTargetURL( "url", delivery.MaskURL(targetURL), "error", err, ) - http.Error( - w, - "Invalid target URL: "+err.Error(), - http.StatusBadRequest, - ) + + msg := "Invalid target URL: " + err.Error() + + // Only a private or reserved address's refusal says how + // to allow it. Metadata refusals never do: link-local and + // the other unconditional metadata addresses cannot be + // opened, and Azure's WireServer, which listing does + // open, hands out VM credentials. + if errors.Is(err, delivery.ErrBlockedIP) { + msg += ". Private and reserved addresses are refused " + + "by default; the server's ALLOWED_EGRESS_CIDRS " + + "setting allows named networks (see \"Allowing " + + "egress to your own network\" in the README)." + } + + http.Error(w, msg, http.StatusBadRequest) return err } diff --git a/internal/handlers/target_private_refusal_test.go b/internal/handlers/target_private_refusal_test.go new file mode 100644 index 0000000..2d80731 --- /dev/null +++ b/internal/handlers/target_private_refusal_test.go @@ -0,0 +1,116 @@ +package handlers_test + +import ( + "net/http" + "net/url" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "sneak.berlin/go/webhooker/internal/database" +) + +// privateRefusalHint is the sentence that tells an operator a private +// destination is refused on purpose, and how to allow one. +const privateRefusalHint = "Private and reserved addresses are " + + "refused by default; the server's ALLOWED_EGRESS_CIDRS setting " + + "allows named networks (see \"Allowing egress to your own " + + "network\" in the README)." + +// TestTargetRefusal_PrivateDestinationSaysHowToAllowIt covers both +// target types that take a URL, on add and on edit. +func TestTargetRefusal_PrivateDestinationSaysHowToAllowIt( + t *testing.T, +) { + t.Parallel() + + env := setupSourceTest(t) + + targetTypes := []database.TargetType{ + database.TargetTypeHTTP, + database.TargetTypeSlack, + } + + for _, targetType := range targetTypes { + t.Run(string(targetType), func(t *testing.T) { + t.Parallel() + + webhook := seedWebhookWithRetention(t, env.db, 30) + targetsPath := "/source/" + webhook.ID + "/targets" + + form := url.Values{} + form.Set("name", "private") + form.Set("type", string(targetType)) + form.Set("url", editBlockedURL) + + added := serveTarget( + env, http.MethodPost, targetsPath, form, + ) + assert.Equal(t, http.StatusBadRequest, added.Code) + assert.Contains( + t, added.Body.String(), privateRefusalHint, + ) + + form.Set("url", editOriginalURL) + + created := serveTarget( + env, http.MethodPost, targetsPath, form, + ) + require.Equal( + t, http.StatusSeeOther, created.Code, + created.Body.String(), + ) + + targets := targetsForWebhook(t, env.db, webhook.ID) + require.Len(t, targets, 1) + + form.Set("url", editBlockedURL) + + edited := submitTargetEdit( + env, webhook.ID, targets[0].ID, form, + ) + assert.Equal(t, http.StatusBadRequest, edited.Code) + assert.Contains( + t, edited.Body.String(), privateRefusalHint, + ) + }) + } +} + +// TestTargetRefusal_MetadataDestinationDoesNotSayHowToAllowIt: no +// setting opens a link-local address, and Azure's WireServer hands out +// VM credentials, so neither refusal points at the setting. +func TestTargetRefusal_MetadataDestinationDoesNotSayHowToAllowIt( + t *testing.T, +) { + t.Parallel() + + env := setupSourceTest(t) + + metadataURLs := map[string]string{ + "link-local": "http://169.254.169.254/latest/meta-data/", + "wireserver": "http://168.63.129.16/?comp=versions", + } + + for name, metadataURL := range metadataURLs { + t.Run(name, func(t *testing.T) { + t.Parallel() + + webhook := seedWebhookWithRetention(t, env.db, 30) + + form := url.Values{} + form.Set("name", "metadata") + form.Set("type", string(database.TargetTypeHTTP)) + form.Set("url", metadataURL) + + w := serveTarget( + env, http.MethodPost, + "/source/"+webhook.ID+"/targets", form, + ) + assert.Equal(t, http.StatusBadRequest, w.Code) + assert.NotContains( + t, w.Body.String(), privateRefusalHint, + ) + }) + } +}