diff --git a/pkg/appview/handlers/webhook_url_test.go b/pkg/appview/handlers/webhook_url_test.go new file mode 100644 index 0000000..006b7b6 --- /dev/null +++ b/pkg/appview/handlers/webhook_url_test.go @@ -0,0 +1,125 @@ +package handlers + +import ( + "html/template" + "net/http/httptest" + "net/url" + "strings" + "testing" + + "atcr.io/pkg/appview/db" + "atcr.io/pkg/appview/middleware" +) + +// Finding 35: the creation-time check said "must be https" but accepted +// http://, so POST /api/webhooks with http://127.0.0.1:9/hook returned 200 and +// created a webhook the appview would then dial on demand. + +// webhookTestTemplates provides the two templates the webhook handlers render. +// The bodies only need to be identifiable, not faithful. +func webhookTestTemplates(t *testing.T) *template.Template { + t.Helper() + tmpl := template.Must(template.New("alert").Parse(`ALERT:{{ .Message }}`)) + template.Must(tmpl.New("webhooks_list").Parse( + `LIST:{{ range .Webhooks }}{{ .URL }};{{ end }}`)) + return tmpl +} + +func postWebhook(t *testing.T, h *AddWebhookHandler, webhookURL string) string { + t.Helper() + form := url.Values{} + form.Set("url", webhookURL) + form.Set("trigger_push", "on") + + req := httptest.NewRequest("POST", "/api/webhooks", strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req = middleware.WithUser(req, &db.User{DID: "did:plc:webhooktester", Handle: "wh.test"}) + + rr := httptest.NewRecorder() + h.ServeHTTP(rr, req) + return rr.Body.String() +} + +func TestAddWebhook_RejectsNonHTTPSAndPrivateDestinations(t *testing.T) { + database := setupTestDB(t) + defer database.Close() + + h := &AddWebhookHandler{BaseUIHandler: BaseUIHandler{ + Templates: webhookTestTemplates(t), + DB: database, + ReadOnlyDB: database, + }} + + tests := []struct { + name string + url string + wantMsg string + }{ + {"plain http loopback", "http://127.0.0.1:9/hook", "must be https"}, + {"plain http public host", "http://example.com/hook", "must be https"}, + {"no scheme", "example.com/hook", "must be https"}, + {"file scheme", "file:///etc/passwd", "must be https"}, + {"https loopback", "https://127.0.0.1:9/hook", "private or loopback"}, + {"https loopback v6", "https://[::1]/hook", "private or loopback"}, + {"https rfc1918", "https://10.0.0.1/hook", "private or loopback"}, + {"https link-local metadata", "https://169.254.169.254/latest/meta-data/", "private or loopback"}, + {"https ipv4-mapped loopback", "https://[::ffff:127.0.0.1]/hook", "private or loopback"}, + {"https localhost name", "https://localhost:9/hook", "private or loopback"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + body := postWebhook(t, h, tt.url) + if !strings.HasPrefix(body, "ALERT:") { + t.Fatalf("expected an error alert, got %q", body) + } + if !strings.Contains(body, tt.wantMsg) { + t.Errorf("error message %q does not mention %q", body, tt.wantMsg) + } + }) + } + + // Nothing above may have been stored. + stored, err := db.ListWebhooks(database, "did:plc:webhooktester") + if err != nil { + t.Fatalf("list webhooks: %v", err) + } + if len(stored) != 0 { + t.Fatalf("rejected URLs were persisted anyway: %+v", stored) + } +} + +func TestAddWebhook_AcceptsPublicHTTPS(t *testing.T) { + database := setupTestDB(t) + defer database.Close() + + h := &AddWebhookHandler{BaseUIHandler: BaseUIHandler{ + Templates: webhookTestTemplates(t), + DB: database, + ReadOnlyDB: database, + }} + + // The webhooks table has a FK on the owning user. + if err := db.InsertUserIfNotExists(database, &db.User{DID: "did:plc:webhooktester", Handle: "wh.test"}); err != nil { + t.Fatalf("seed user: %v", err) + } + + const good = "https://hooks.example.com/services/abc" + body := postWebhook(t, h, good) + if !strings.HasPrefix(body, "LIST:") { + t.Fatalf("expected the webhook list to render, got %q", body) + } + // ListWebhooks masks the path for display, so match on the host prefix. + const displayPrefix = "https://hooks.example.com/" + if !strings.Contains(body, displayPrefix) { + t.Errorf("rendered list %q does not contain the new webhook", body) + } + + stored, err := db.ListWebhooks(database, "did:plc:webhooktester") + if err != nil { + t.Fatalf("list webhooks: %v", err) + } + if len(stored) != 1 || !strings.HasPrefix(stored[0].URL, displayPrefix) { + t.Fatalf("expected the public https webhook to be stored, got %+v", stored) + } +} diff --git a/pkg/appview/handlers/webhooks.go b/pkg/appview/handlers/webhooks.go index 1b1a402..98ae55e 100644 --- a/pkg/appview/handlers/webhooks.go +++ b/pkg/appview/handlers/webhooks.go @@ -81,9 +81,13 @@ func (h *AddWebhookHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } - // Validate URL scheme - if !strings.HasPrefix(webhookURL, "https://") && !strings.HasPrefix(webhookURL, "http://") { - h.renderWebhookError(w, "Invalid webhook URL: must be https") + // Validate the destination: https only, and no non-public address. This is + // the friendly half of the SSRF guard; the half that actually holds is the + // dial-time check on the delivery client (pkg/appview/webhooks/ssrf.go), + // since an attacker controlling DNS can make a hostname that looks public + // here resolve to loopback at delivery. + if err := webhooks.ValidateWebhookURL(webhookURL); err != nil { + h.renderWebhookError(w, "Invalid webhook URL: "+err.Error()) return } diff --git a/pkg/appview/webhooks/dispatch.go b/pkg/appview/webhooks/dispatch.go index 5714bab..61c6598 100644 --- a/pkg/appview/webhooks/dispatch.go +++ b/pkg/appview/webhooks/dispatch.go @@ -31,20 +31,33 @@ type WebhookLimiter func(userDID string) (maxWebhooks int, allTriggers bool) // It reads webhooks from the appview DB and delivers payloads // with Discord/Slack formatting and HMAC signing. type Dispatcher struct { - db db.DBTX - meta atproto.AppviewMetadata + db db.DBTX + meta atproto.AppviewMetadata + // httpClient talks to internal services (hold quota stats). Holds may + // legitimately live on a private address, so it is deliberately NOT the + // SSRF-guarded client. httpClient *http.Client - limits WebhookLimiter + // deliveryClient is the only client that ever dials a user-supplied webhook + // URL. Its dialer refuses non-public addresses at connect time, which is + // what stops DNS rebinding and redirect chains from reaching loopback, + // RFC1918 or the cloud metadata endpoint. + deliveryClient *http.Client + // validateURL is the destination policy applied before every attempt. Nil + // means ValidateWebhookURL. Only tests replace it, so that they can deliver + // to their own loopback httptest servers. + validateURL func(string) error + limits WebhookLimiter } // NewDispatcher creates a new webhook dispatcher. limits may be nil (treated as // unlimited), but production wires it to the billing manager's GetWebhookLimits. func NewDispatcher(database db.DBTX, meta atproto.AppviewMetadata, limits WebhookLimiter) *Dispatcher { return &Dispatcher{ - db: database, - meta: meta, - httpClient: http.DefaultClient, - limits: limits, + db: database, + meta: meta, + httpClient: http.DefaultClient, + deliveryClient: NewSafeClient(deliveryTimeout), + limits: limits, } } @@ -361,6 +374,13 @@ func (d *Dispatcher) DeliverTest(ctx context.Context, webhookID, userDID, userHa // deliverWithRetry attempts to deliver a webhook with exponential backoff func (d *Dispatcher) deliverWithRetry(webhookURL, secret string, payload []byte) { + // A destination that fails validation will never become valid, so don't + // hold a goroutine open through the backoff schedule for it. + if err := d.validateDestination(webhookURL); err != nil { + slog.Warn("Refusing webhook delivery to disallowed URL", "url", maskURL(webhookURL), "error", err) + return + } + delays := []time.Duration{0, 30 * time.Second, 2 * time.Minute, 8 * time.Minute} for attempt, delay := range delays { if attempt > 0 { @@ -373,9 +393,30 @@ func (d *Dispatcher) deliverWithRetry(webhookURL, secret string, payload []byte) slog.Warn("Webhook delivery failed after retries", "url", maskURL(webhookURL)) } +// validateDestination applies the destination policy. Tests override +// validateURL so they can deliver to their own loopback httptest servers; +// production always gets ValidateWebhookURL. +func (d *Dispatcher) validateDestination(webhookURL string) error { + if d.validateURL != nil { + return d.validateURL(webhookURL) + } + return ValidateWebhookURL(webhookURL) +} + +// deliveryTimeout bounds a single webhook POST, including redirects. +const deliveryTimeout = 10 * time.Second + // attemptDelivery sends a single webhook HTTP POST func (d *Dispatcher) attemptDelivery(webhookURL, secret string, payload []byte) bool { - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + // Re-validate on every attempt, not just at creation. Rows predating the + // guard (or written by any future path that skips the handler) are refused + // here rather than dialed. + if err := d.validateDestination(webhookURL); err != nil { + slog.Warn("Refusing webhook delivery to disallowed URL", "url", maskURL(webhookURL), "error", err) + return false + } + + ctx, cancel := context.WithTimeout(context.Background(), deliveryTimeout) defer cancel() // Reformat payload for platform-specific webhook APIs @@ -404,7 +445,12 @@ func (d *Dispatcher) attemptDelivery(webhookURL, secret string, payload []byte) req.Header.Set("X-Webhook-Signature-256", "sha256="+sig) } - client := &http.Client{Timeout: 10 * time.Second} + client := d.deliveryClient + if client == nil { + // Zero-value Dispatchers (tests) must not fall back to an unguarded + // client. + client = NewSafeClient(deliveryTimeout) + } resp, err := client.Do(req) if err != nil { slog.Warn("Webhook delivery attempt failed", "url", maskURL(webhookURL), "error", err) diff --git a/pkg/appview/webhooks/dispatch_entitlement_test.go b/pkg/appview/webhooks/dispatch_entitlement_test.go index 3c84299..e1b4743 100644 --- a/pkg/appview/webhooks/dispatch_entitlement_test.go +++ b/pkg/appview/webhooks/dispatch_entitlement_test.go @@ -62,6 +62,10 @@ func TestDispatchForScan_EntitlementGate(t *testing.T) { // Free tier: max 1 webhook, no paid triggers. The cap keeps only the oldest // webhook, and scan:all (paid) is masked out — leaving just scan:first. free := NewDispatcher(conn, meta, func(string) (int, bool) { return 1, false }) + // The delivery guard refuses http and loopback addresses; the receiver here + // is an httptest server on 127.0.0.1, so relax it for this test. The guard + // itself is covered in ssrf_test.go. + free.allowLoopbackDeliveryForTest() free.DispatchForScan(context.Background(), scanForUser(userDID), nil, "se.test", "latest", "https://hold") if !receiver.waitFor(1, 2*time.Second) { t.Fatalf("free tier: expected 1 delivery (scan:first on oldest hook), got %d", receiver.count()) @@ -75,6 +79,7 @@ func TestDispatchForScan_EntitlementGate(t *testing.T) { // scan:first AND scan:all = 4 deliveries (regression guard that the gate // doesn't over-suppress). entitled := NewDispatcher(conn, meta, func(string) (int, bool) { return -1, true }) + entitled.allowLoopbackDeliveryForTest() entitled.DispatchForScan(context.Background(), scanForUser(userDID), nil, "se.test", "latest", "https://hold") if !receiver.waitFor(1+4, 2*time.Second) { t.Fatalf("entitled: expected 4 more deliveries (2 hooks x scan:first+scan:all), total got %d", receiver.count()) diff --git a/pkg/appview/webhooks/dispatch_quota_test.go b/pkg/appview/webhooks/dispatch_quota_test.go index 4a6d950..67ac41b 100644 --- a/pkg/appview/webhooks/dispatch_quota_test.go +++ b/pkg/appview/webhooks/dispatch_quota_test.go @@ -137,6 +137,9 @@ func TestDispatchForQuotaEdgeTriggered(t *testing.T) { } d := NewDispatcher(conn, atproto.AppviewMetadata{ClientShortName: "ATCR", BaseURL: "https://atcr.test"}, nil) + // The receiver is an httptest server on loopback, which the delivery guard + // would otherwise refuse. See ssrf_test.go for the guard's own coverage. + d.allowLoopbackDeliveryForTest() event := storage.QuotaWebhookEvent{ UserDID: userDID, @@ -253,6 +256,7 @@ func TestDispatchForQuotaUnlimitedHold(t *testing.T) { } d := NewDispatcher(conn, atproto.AppviewMetadata{}, nil) + d.allowLoopbackDeliveryForTest() d.DispatchForQuota(context.Background(), storage.QuotaWebhookEvent{ UserDID: userDID, HoldDID: holdSrv.URL, HoldEndpoint: holdSrv.URL, }) diff --git a/pkg/appview/webhooks/ssrf.go b/pkg/appview/webhooks/ssrf.go new file mode 100644 index 0000000..bd6b6b4 --- /dev/null +++ b/pkg/appview/webhooks/ssrf.go @@ -0,0 +1,236 @@ +package webhooks + +import ( + "errors" + "fmt" + "net" + "net/http" + "net/url" + "strings" + "syscall" + "time" +) + +// Webhook URLs are attacker-supplied: any authenticated user can register one +// and then make the appview dial it on demand with the Test button. Without a +// destination guard that is a server-side request forgery primitive against +// everything the appview host can reach, including the cloud metadata endpoint +// at 169.254.169.254 and anything bound to loopback. +// +// The guard has two layers: +// +// 1. ValidateWebhookURL, applied at creation time and again before every +// delivery attempt. It enforces https and rejects URLs that name a +// non-public IP literal. Its real job is giving the user an immediate, +// comprehensible error instead of a mysterious delivery failure later. +// +// 2. A net.Dialer Control hook on the delivery HTTP client, which inspects the +// *resolved* address on every connection attempt. This is the layer that +// actually holds: the attacker controls DNS for their own hostname, so a +// name that resolves public at creation can resolve to 127.0.0.1 at +// delivery (DNS rebinding), and creation-time validation cannot see a +// redirect target at all. +// +// Both the scheduled dispatcher and the synchronous Test button deliver through +// attemptDelivery, which uses the guarded client, so neither path can dial a +// private address. + +// User-facing validation failures. The messages are written to be shown +// verbatim in the settings UI after an "Invalid webhook URL: " prefix. +var ( + // ErrMalformedURL means the string did not parse as an absolute URL. + ErrMalformedURL = errors.New("could not be parsed as a URL") + + // ErrSchemeNotHTTPS means the URL used something other than https. + // Webhook payloads carry repository names and HMAC signatures, so plaintext + // http is refused outright rather than merely discouraged. + ErrSchemeNotHTTPS = errors.New("must be https") + + // ErrPrivateAddress means the URL resolved to, or literally named, an + // address that is not publicly routable. + ErrPrivateAddress = errors.New("cannot point at a private or loopback address") +) + +// extraBlocked covers ranges that have no net.IP helper. Everything with a +// stdlib predicate (loopback, RFC1918 + IPv6 ULA, link-local, multicast, +// unspecified, broadcast) is handled by blockedReason instead of being +// re-derived here. +var extraBlocked = []struct { + net *net.IPNet + reason string +}{ + {mustCIDR("100.64.0.0/10"), "carrier-grade NAT address"}, + {mustCIDR("192.0.0.0/24"), "IETF protocol assignment address"}, + {mustCIDR("198.18.0.0/15"), "benchmarking address"}, + {mustCIDR("240.0.0.0/4"), "reserved address"}, + // IPv4-compatible IPv6 (::a.b.c.d). To4 does not unmap these, and + // IsLoopback on ::127.0.0.1 is false, so they need an explicit range. + {mustCIDR("::/96"), "IPv4-compatible IPv6 address"}, + {mustCIDR("64:ff9b::/96"), "NAT64 address"}, + {mustCIDR("64:ff9b:1::/48"), "local-use NAT64 address"}, + {mustCIDR("100::/64"), "discard-only address"}, + {mustCIDR("2001:db8::/32"), "documentation address"}, +} + +func mustCIDR(s string) *net.IPNet { + _, n, err := net.ParseCIDR(s) + if err != nil { + panic("webhooks: bad CIDR " + s + ": " + err.Error()) + } + return n +} + +// blockedReason reports why an address must not be dialed, or "" if it is a +// public destination. IPv4-mapped IPv6 forms (::ffff:127.0.0.1) are unmapped +// first, so every IPv4 rule below applies to them too. +func blockedReason(ip net.IP) string { + if ip == nil { + return "not a valid IP address" + } + if v4 := ip.To4(); v4 != nil { + ip = v4 + } + + switch { + case ip.IsUnspecified(): + return "unspecified address" + case ip.IsLoopback(): + // 127.0.0.0/8 and ::1 + return "loopback address" + case ip.IsPrivate(): + // 10/8, 172.16/12, 192.168/16 and IPv6 unique-local fc00::/7 + return "private address" + case ip.IsLinkLocalUnicast(): + // 169.254/16 (covers the 169.254.169.254 metadata endpoint) and fe80::/10 + return "link-local address" + case ip.IsInterfaceLocalMulticast(), ip.IsLinkLocalMulticast(), ip.IsMulticast(): + return "multicast address" + case !ip.IsGlobalUnicast(): + // Catches the IPv4 broadcast address and anything else the stdlib does + // not consider globally routable. + return "non-global address" + } + + for _, b := range extraBlocked { + if b.net.Contains(ip) { + return b.reason + } + } + return "" +} + +// ValidateWebhookURL checks a user-supplied webhook URL at rest: https scheme, +// a host, and no non-public IP literal. It does not resolve DNS. A hostname +// that resolves to a private address is caught at dial time by the Control hook +// instead, which is the only check an attacker who controls DNS cannot dodge. +func ValidateWebhookURL(raw string) error { + u, err := url.Parse(strings.TrimSpace(raw)) + if err != nil { + return ErrMalformedURL + } + if !strings.EqualFold(u.Scheme, "https") { + return ErrSchemeNotHTTPS + } + return validateParsedURL(u) +} + +// validateParsedURL is the scheme + host check applied both at creation and to +// every redirect hop. +func validateParsedURL(u *url.URL) error { + if !strings.EqualFold(u.Scheme, "https") { + return ErrSchemeNotHTTPS + } + host := u.Hostname() + if host == "" { + return ErrMalformedURL + } + if ip := net.ParseIP(host); ip != nil { + if reason := blockedReason(ip); reason != "" { + return fmt.Errorf("%w (%s is a %s)", ErrPrivateAddress, host, reason) + } + return nil + } + // Names are resolved at dial time, but reject the obvious ones up front so + // the user gets a real message instead of a delivery that just never works. + lower := strings.ToLower(host) + if lower == "localhost" || strings.HasSuffix(lower, ".localhost") { + return fmt.Errorf("%w (%s is a loopback name)", ErrPrivateAddress, host) + } + return nil +} + +// checkDialAddr inspects a resolved "host:port" address just before the socket +// is connected. This runs for every attempt, every redirect hop, and every +// address the resolver returns, which is what makes DNS rebinding ineffective. +func checkDialAddr(network, address string) error { + switch network { + case "tcp", "tcp4", "tcp6": + default: + return fmt.Errorf("%w (network %q is not allowed)", ErrPrivateAddress, network) + } + + host, _, err := net.SplitHostPort(address) + if err != nil { + host = address + } + ip := net.ParseIP(host) + if ip == nil { + // Control receives an already-resolved literal. Anything else is + // unexpected, so refuse rather than guess. + return fmt.Errorf("%w (%q did not resolve to an IP)", ErrPrivateAddress, address) + } + if reason := blockedReason(ip); reason != "" { + return fmt.Errorf("%w (%s is a %s)", ErrPrivateAddress, ip, reason) + } + return nil +} + +// safeDialControl is the net.Dialer Control hook. The signature is fixed by +// net.Dialer. +func safeDialControl(network, address string, _ syscall.RawConn) error { + return checkDialAddr(network, address) +} + +// maxRedirects bounds how far a webhook endpoint can bounce us. Each hop is +// re-validated, and the dial guard applies to every hop regardless. +const maxRedirects = 3 + +// NewSafeClient builds the HTTP client used for delivering webhook payloads to +// user-supplied URLs. It must not be reused for talking to holds or any other +// internal service, which may legitimately live on a private address. +func NewSafeClient(timeout time.Duration) *http.Client { + dialer := &net.Dialer{ + Timeout: 5 * time.Second, + KeepAlive: 30 * time.Second, + Control: safeDialControl, + } + transport := &http.Transport{ + // Deliberately no Proxy: honoring HTTP(S)_PROXY would route the request + // through a proxy that the Control hook cannot see past, handing the + // bypass straight back. + Proxy: nil, + DialContext: dialer.DialContext, + ForceAttemptHTTP2: true, + MaxIdleConns: 10, + IdleConnTimeout: 30 * time.Second, + TLSHandshakeTimeout: 10 * time.Second, + ResponseHeaderTimeout: timeout, + ExpectContinueTimeout: 1 * time.Second, + } + return &http.Client{ + Timeout: timeout, + Transport: transport, + CheckRedirect: func(req *http.Request, via []*http.Request) error { + if len(via) >= maxRedirects { + return fmt.Errorf("stopped after %d redirects", maxRedirects) + } + // A public https endpoint that 302s to http://169.254.169.254/ is + // refused here on scheme alone; the dial guard would refuse the + // address anyway. + if err := validateParsedURL(req.URL); err != nil { + return fmt.Errorf("refusing webhook redirect to %s: %w", req.URL.Redacted(), err) + } + return nil + }, + } +} diff --git a/pkg/appview/webhooks/ssrf_test.go b/pkg/appview/webhooks/ssrf_test.go new file mode 100644 index 0000000..c365958 --- /dev/null +++ b/pkg/appview/webhooks/ssrf_test.go @@ -0,0 +1,358 @@ +package webhooks + +import ( + "context" + "errors" + "net" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "atcr.io/pkg/appview/db" + "atcr.io/pkg/atproto" +) + +// Finding 35: the creation-time check claimed "must be https" while accepting +// http, and nothing anywhere looked at the destination address. Both the +// scheduled dispatcher and the synchronous Test button would dial whatever the +// user typed, including 127.0.0.1 and 169.254.169.254. + +func TestValidateWebhookURL_Scheme(t *testing.T) { + tests := []struct { + name string + url string + wantErr error + }{ + {"plain http is refused", "http://example.com/hook", ErrSchemeNotHTTPS}, + {"http to a public host is still refused", "http://93.184.216.34/hook", ErrSchemeNotHTTPS}, + {"uppercase scheme is still http", "HTTP://example.com/hook", ErrSchemeNotHTTPS}, + {"no scheme", "example.com/hook", ErrSchemeNotHTTPS}, + {"file scheme", "file:///etc/passwd", ErrSchemeNotHTTPS}, + {"gopher scheme", "gopher://example.com/", ErrSchemeNotHTTPS}, + {"https with no host", "https://", ErrMalformedURL}, + + {"public https host", "https://example.com/hook", nil}, + {"public https host with port and path", "https://hooks.slack.com:443/services/T/B/x", nil}, + {"public https IP literal", "https://93.184.216.34/hook", nil}, + {"public https IPv6 literal", "https://[2606:2800:220:1:248:1893:25c8:1946]/hook", nil}, + {"uppercase HTTPS is accepted", "HTTPS://example.com/hook", nil}, + {"leading and trailing space tolerated", " https://example.com/hook ", nil}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateWebhookURL(tt.url) + if tt.wantErr == nil { + if err != nil { + t.Fatalf("ValidateWebhookURL(%q) = %v, want nil", tt.url, err) + } + return + } + if !errors.Is(err, tt.wantErr) { + t.Fatalf("ValidateWebhookURL(%q) = %v, want %v", tt.url, err, tt.wantErr) + } + }) + } +} + +func TestValidateWebhookURL_PrivateLiterals(t *testing.T) { + urls := []string{ + "https://127.0.0.1/hook", + "https://127.1.2.3:8443/hook", + "https://[::1]/hook", + "https://10.0.0.5/hook", + "https://172.16.0.1/hook", + "https://192.168.1.1/hook", + "https://169.254.169.254/latest/meta-data/", + "https://[fe80::1]/hook", + "https://[fd00::1]/hook", + "https://[::ffff:127.0.0.1]/hook", + "https://[::ffff:169.254.169.254]/hook", + "https://0.0.0.0/hook", + "https://localhost/hook", + "https://api.localhost/hook", + } + for _, u := range urls { + t.Run(u, func(t *testing.T) { + err := ValidateWebhookURL(u) + if !errors.Is(err, ErrPrivateAddress) { + t.Fatalf("ValidateWebhookURL(%q) = %v, want ErrPrivateAddress", u, err) + } + }) + } +} + +// blockedReason is the single decision the dial guard makes. Table-driven so a +// range that stops being blocked has to come past this test. +func TestBlockedReason(t *testing.T) { + tests := []struct { + ip string + blocked bool + }{ + // Loopback + {"127.0.0.1", true}, + {"127.255.255.254", true}, + {"::1", true}, + {"::ffff:127.0.0.1", true}, // IPv4-mapped IPv6 + {"::127.0.0.1", true}, // IPv4-compatible IPv6 + + // RFC1918 + {"10.0.0.1", true}, + {"10.255.255.255", true}, + {"172.16.0.1", true}, + {"172.31.255.255", true}, + {"192.168.0.1", true}, + {"::ffff:10.0.0.1", true}, + {"::ffff:192.168.1.1", true}, + {"::ffff:172.20.0.1", true}, + + // Link-local, including the cloud metadata endpoint + {"169.254.169.254", true}, + {"169.254.0.1", true}, + {"::ffff:169.254.169.254", true}, + {"fe80::1", true}, + + // IPv6 unique-local + {"fc00::1", true}, + {"fd12:3456:789a::1", true}, + + // Unspecified / multicast / broadcast + {"0.0.0.0", true}, + {"::", true}, + {"224.0.0.1", true}, + {"239.255.255.250", true}, + {"ff02::1", true}, + {"255.255.255.255", true}, + + // Ranges with no stdlib helper + {"100.64.0.1", true}, // CGNAT + {"192.0.0.1", true}, // IETF protocol assignments + {"198.18.0.1", true}, // benchmarking + {"240.0.0.1", true}, // reserved + {"64:ff9b::1", true}, // NAT64 + {"100::1", true}, // discard-only + {"2001:db8::1", true}, // documentation + + // Public destinations must keep working + {"93.184.216.34", false}, + {"1.1.1.1", false}, + {"8.8.8.8", false}, + {"172.32.0.1", false}, // just outside 172.16/12 + {"172.15.0.1", false}, // just below 172.16/12 + {"100.63.255.255", false}, // just below CGNAT + {"100.128.0.1", false}, // just above CGNAT + {"169.253.0.1", false}, // just below link-local + {"2606:2800:220:1:248:1893:25c8:1946", false}, + {"::ffff:93.184.216.34", false}, + } + + for _, tt := range tests { + t.Run(tt.ip, func(t *testing.T) { + ip := net.ParseIP(tt.ip) + if ip == nil { + t.Fatalf("test bug: %q is not a valid IP", tt.ip) + } + reason := blockedReason(ip) + if tt.blocked && reason == "" { + t.Errorf("blockedReason(%s) allowed a non-public address", tt.ip) + } + if !tt.blocked && reason != "" { + t.Errorf("blockedReason(%s) blocked a public address: %s", tt.ip, reason) + } + }) + } +} + +func TestBlockedReason_NilIP(t *testing.T) { + if blockedReason(nil) == "" { + t.Error("blockedReason(nil) must not report the address as allowed") + } +} + +// checkDialAddr is what the Control hook runs on the resolved address, so it is +// the check DNS rebinding has to get past. +func TestCheckDialAddr(t *testing.T) { + tests := []struct { + name string + network string + address string + blocked bool + }{ + {"loopback v4", "tcp", "127.0.0.1:80", true}, + {"loopback v6", "tcp", "[::1]:443", true}, + {"rfc1918", "tcp4", "10.1.2.3:443", true}, + {"link-local metadata", "tcp4", "169.254.169.254:80", true}, + {"ipv4-mapped loopback", "tcp6", "[::ffff:127.0.0.1]:443", true}, + {"ipv4-mapped rfc1918", "tcp6", "[::ffff:192.168.0.9]:443", true}, + {"unix socket network", "unix", "/var/run/docker.sock", true}, + {"unresolved name", "tcp", "example.com:443", true}, + {"public v4", "tcp", "93.184.216.34:443", false}, + {"public v6", "tcp6", "[2606:2800:220:1:248:1893:25c8:1946]:443", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := checkDialAddr(tt.network, tt.address) + if tt.blocked && err == nil { + t.Errorf("checkDialAddr(%q, %q) allowed the connection", tt.network, tt.address) + } + if !tt.blocked && err != nil { + t.Errorf("checkDialAddr(%q, %q) = %v, want nil", tt.network, tt.address, err) + } + }) + } +} + +// End to end: the guarded client must refuse a loopback server even when the +// URL is handed to it directly, which is what the Test button effectively does. +func TestSafeClient_RefusesLoopbackServer(t *testing.T) { + reached := false + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + reached = true + w.WriteHeader(http.StatusOK) + })) + defer srv.Close() + + client := NewSafeClient(5 * time.Second) + resp, err := client.Get(srv.URL) //nolint:noctx // short-lived guard check + if err == nil { + resp.Body.Close() + t.Fatal("guarded client connected to a loopback server") + } + if reached { + t.Fatal("request reached the loopback server despite the dial guard") + } + if !strings.Contains(err.Error(), ErrPrivateAddress.Error()) { + t.Errorf("error %v does not mention the private-address refusal", err) + } +} + +// A public endpoint that redirects to the metadata address must not be +// followed. The redirect target is checked before the hop is made, and the dial +// guard would refuse the address anyway. +func TestSafeClient_RefusesRedirectToPrivate(t *testing.T) { + err := redirectCheck(t, "http://169.254.169.254/latest/meta-data/") + if err == nil { + t.Fatal("CheckRedirect followed a redirect to the metadata endpoint") + } + if !errors.Is(err, ErrSchemeNotHTTPS) && !errors.Is(err, ErrPrivateAddress) { + t.Errorf("unexpected refusal reason: %v", err) + } + + err = redirectCheck(t, "https://127.0.0.1/hook") + if !errors.Is(err, ErrPrivateAddress) { + t.Errorf("https redirect to loopback: got %v, want ErrPrivateAddress", err) + } + + if err := redirectCheck(t, "https://example.org/next"); err != nil { + t.Errorf("redirect to a public https URL was refused: %v", err) + } +} + +// redirectCheck exercises the client's CheckRedirect hook directly; dialing a +// real redirect chain would need network access. +func redirectCheck(t *testing.T, target string) error { + t.Helper() + client := NewSafeClient(5 * time.Second) + req, err := http.NewRequest(http.MethodGet, target, nil) //nolint:noctx // not sent + if err != nil { + t.Fatalf("building request: %v", err) + } + via, err := http.NewRequest(http.MethodPost, "https://example.com/hook", nil) //nolint:noctx // not sent + if err != nil { + t.Fatalf("building via request: %v", err) + } + return client.CheckRedirect(req, []*http.Request{via}) +} + +func TestSafeClient_StopsRedirectLoop(t *testing.T) { + client := NewSafeClient(5 * time.Second) + req, err := http.NewRequest(http.MethodGet, "https://example.com/hook", nil) //nolint:noctx // not sent + if err != nil { + t.Fatalf("building request: %v", err) + } + via := make([]*http.Request, maxRedirects) + for i := range via { + via[i] = req + } + if err := client.CheckRedirect(req, via); err == nil { + t.Errorf("CheckRedirect allowed hop %d, want a stop after %d", len(via)+1, maxRedirects) + } +} + +// The delivery path re-validates every attempt, so a row written before the +// guard (or by any path that skips the handler) is refused instead of dialed. +func TestAttemptDelivery_RefusesDisallowedURLs(t *testing.T) { + reached := false + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + reached = true + w.WriteHeader(http.StatusOK) + })) + defer srv.Close() + + d := &Dispatcher{deliveryClient: NewSafeClient(5 * time.Second)} + + for _, u := range []string{srv.URL, "http://example.com/hook", "https://127.0.0.1:9/hook"} { + if d.attemptDelivery(u, "", []byte(`{}`)) { + t.Errorf("attemptDelivery(%q) reported success", u) + } + } + if reached { + t.Fatal("a delivery attempt reached the loopback server") + } +} + +// allowLoopbackDeliveryForTest relaxes the destination guard so a test can +// deliver to its own httptest server. It exists only in the test build: there +// is no way to reach it from production code. +func (d *Dispatcher) allowLoopbackDeliveryForTest() { + d.validateURL = func(string) error { return nil } + d.deliveryClient = &http.Client{Timeout: deliveryTimeout} +} + +// The Test button is the more dangerous of the two delivery paths: it is +// immediate and attacker-timed. DeliverTest must go through the same guard as +// the scheduled dispatcher. +func TestDeliverTest_RefusesLoopbackWebhook(t *testing.T) { + conn, err := db.InitDB(t.TempDir()+"/test.db", db.LibsqlConfig{}) + if err != nil { + t.Fatalf("init db: %v", err) + } + defer conn.Close() + + const userDID = "did:plc:ssrftester" + if err := db.UpsertUser(conn, &db.User{ + DID: userDID, Handle: "ssrf.test", PDSEndpoint: "https://pds", LastSeen: time.Now(), + }); err != nil { + t.Fatalf("upsert user: %v", err) + } + + reached := false + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + reached = true + w.WriteHeader(http.StatusOK) + })) + defer srv.Close() + + // A row created before the guard existed, pointing at loopback over http. + if err := db.InsertWebhook(conn, &db.Webhook{ + ID: "wh-loopback", UserDID: userDID, URL: srv.URL, + Triggers: PackTriggers(TriggerFirst, 0), CreatedAt: time.Now().UTC(), + }); err != nil { + t.Fatalf("insert webhook: %v", err) + } + + d := NewDispatcher(conn, atproto.AppviewMetadata{ClientShortName: "ATCR"}, nil) + ok, err := d.DeliverTest(context.Background(), "wh-loopback", userDID, "ssrf.test") + if err != nil { + t.Fatalf("DeliverTest returned an error: %v", err) + } + if ok { + t.Error("DeliverTest reported success for a loopback destination") + } + if reached { + t.Error("the Test button reached a loopback server") + } +}