fix(sec): webhook SSRF-guard через Dialer.Control (закрытие DNS-rebinding TOCTOU)

2026-07-04 13:48:22 +07:00
parent 29f448d4b5
commit 070a32717f
3 changed files with 148 additions and 11 deletions
@@ -39,7 +39,10 @@ func NewDispatcher(store ChannelStore, cipher Decryptor) *Dispatcher {
 		cipher: cipher,
 		byType: map[string]Notifier{
 			"telegram": &Telegram{BaseURL: "https://api.telegram.org", HTTP: &http.Client{Timeout: 15 * time.Second}},
-			"webhook":  &Webhook{HTTP: &http.Client{Timeout: 15 * time.Second}},
+			"webhook": &Webhook{HTTP: &http.Client{
+				Timeout:   15 * time.Second,
+				Transport: newWebhookTransport(false),
+			}},
 		},
 	}
 }
@@ -165,6 +165,77 @@ func TestIsAllowedURL(t *testing.T) {
 	}
 }

+func TestDialControlBlocksActualConnectingAddress(t *testing.T) {
+	cases := []struct {
+		name    string
+		address string
+		blocked bool
+	}{
+		{"loopback v4", "127.0.0.1:80", true},
+		{"loopback v6", "[::1]:80", true},
+		{"metadata link-local", "169.254.169.254:80", true},
+		{"private class a", "10.0.0.1:80", true},
+		{"private class b", "172.16.0.1:80", true},
+		{"private class c", "192.168.1.1:80", true},
+		{"unspecified", "0.0.0.0:80", true},
+		{"multicast", "224.0.0.1:80", true},
+		{"public ip", "93.184.216.34:443", false},
+	}
+	control := dialControl(false)
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			err := control("tcp", tc.address, nil)
+			if tc.blocked && err == nil {
+				t.Fatalf("expected %q to be blocked", tc.address)
+			}
+			if !tc.blocked && err != nil {
+				t.Fatalf("expected %q to be allowed, got: %v", tc.address, err)
+			}
+		})
+	}
+}
+
+func TestDialControlAllowsEverythingWhenAllowPrivate(t *testing.T) {
+	control := dialControl(true)
+	if err := control("tcp", "127.0.0.1:80", nil); err != nil {
+		t.Fatalf("expected allowPrivate to skip the dial guard, got: %v", err)
+	}
+}
+
+// TestWebhookControlBlocksConnectionEvenWhenPreCheckPasses simulates the
+// DNS-rebinding TOCTOU: allowPrivate=true skips the pre-request isAllowedURL
+// check (standing in for a rebinding attacker answering a public IP to that
+// lookup), but the Transport's Control func — wired independently of
+// Webhook.allowPrivate — still inspects the literal address the dialer
+// connects to and must still reject it. If Control did not exist, this
+// request would reach the httptest handler; it must not.
+func TestWebhookControlBlocksConnectionEvenWhenPreCheckPasses(t *testing.T) {
+	var handlerCalled bool
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		handlerCalled = true
+		w.WriteHeader(http.StatusOK)
+	}))
+	defer srv.Close()
+
+	wh := &Webhook{
+		HTTP:         &http.Client{Transport: newWebhookTransport(false)},
+		allowPrivate: true, // pre-check bypassed on purpose; Control is not
+	}
+	ev := Event{Project: "proj", Domain: "example.com", Status: "drift", Summary: "x", At: time.Now()}
+	cfg, _ := json.Marshal(map[string]string{"url": srv.URL})
+
+	err := wh.Send(context.Background(), cfg, "", ev)
+	if err == nil {
+		t.Fatal("expected error: Control should have blocked the loopback connection")
+	}
+	if !strings.Contains(err.Error(), "destination not allowed") {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if handlerCalled {
+		t.Fatal("Control should have rejected the dial before the handler ran")
+	}
+}
+
 // --- Dispatcher ---

 type mockChannelStore struct {
@@ -9,6 +9,8 @@ import (
 	"net"
 	"net/http"
 	"net/url"
+	"syscall"
+	"time"
 )

 // Webhook delivers notifications as a JSON POST of the Event to a
@@ -16,17 +18,30 @@ import (
 // unused (reserved for future request signing) and is never logged.
 //
 // The destination URL is project-controlled (any project owner can set it),
-// so it is treated as untrusted input: isAllowedURL blocks requests to
-// loopback/private/link-local/unspecified addresses to prevent SSRF against
-// internal services and cloud metadata endpoints (e.g. 169.254.169.254).
-// Redirects are not followed, since a redirect response could otherwise be
-// used to bypass the destination check.
+// so it is treated as untrusted input. Two layers guard against SSRF:
+//
+//  1. isAllowedURL is a pre-request fast-fail check on the URL's scheme and
+//     (resolved) hostname.
+//  2. HTTP's Transport, when built via newWebhookTransport, wires a
+//     net.Dialer.Control that re-checks the actual "ip:port" being dialed for
+//     every connection net/http opens — including the DNS resolution
+//     http.Client.Do performs internally, independent of (1).
+//
+// Layer (2) is the source of truth: DNS answers are attacker-influenceable
+// (an attacker with authoritative DNS and a low TTL can answer a public IP to
+// a pre-request lookup and a private/loopback IP to the actual connection —
+// DNS rebinding). Relying on (1) alone leaves that TOCTOU window open; (2)
+// closes it because it inspects the address the connection is actually made
+// to, not a name. Redirects are not followed, since a redirect response
+// could otherwise be used to bypass the destination checks.
 type Webhook struct {
 	HTTP *http.Client

-	// allowPrivate disables the SSRF guard. It exists only so tests can
-	// exercise Send happy-paths against httptest servers, which listen on
-	// loopback. Production Dispatchers (NewDispatcher) must never set this.
+	// allowPrivate disables the isAllowedURL pre-check. It exists only so
+	// tests can exercise Send happy-paths against httptest servers, which
+	// listen on loopback. Production Dispatchers (NewDispatcher) must never
+	// set this; they also wire a Transport whose Control func enforces the
+	// same guard at dial time regardless of this flag.
 	allowPrivate bool
 }

@@ -61,14 +76,18 @@ func isAllowedURL(rawurl string) error {
 	}

 	for _, ip := range ips {
-		if isDisallowedIP(ip) {
+		if isBlockedIP(ip) {
 			return errors.New("webhook: destination not allowed")
 		}
 	}
 	return nil
 }

-func isDisallowedIP(ip net.IP) bool {
+// isBlockedIP reports whether ip must never be connected to: loopback,
+// private (RFC1918 etc.), link-local, unspecified, or multicast. Used both
+// by isAllowedURL's pre-request check and by dialControl's per-connection
+// check.
+func isBlockedIP(ip net.IP) bool {
 	return ip.IsLoopback() ||
 		ip.IsPrivate() ||
 		ip.IsLinkLocalUnicast() ||
@@ -77,6 +96,50 @@ func isDisallowedIP(ip net.IP) bool {
 		ip.IsMulticast()
 }

+// dialControl returns a net.Dialer.Control function enforcing the SSRF guard
+// on the literal address ("ip:port") that net/http is about to connect to.
+// It runs after any DNS resolution net/http performs internally — including
+// resolution done independently of, and possibly later than, isAllowedURL's
+// own lookup — so it sees the real connecting IP and closes the DNS-rebinding
+// TOCTOU window described on Webhook.
+//
+// allowPrivate disables the check entirely; it exists so tests can dial
+// httptest servers, which listen on loopback.
+func dialControl(allowPrivate bool) func(network, address string, c syscall.RawConn) error {
+	return func(network, address string, c syscall.RawConn) error {
+		if allowPrivate {
+			return nil
+		}
+		host, _, err := net.SplitHostPort(address)
+		if err != nil {
+			return errors.New("webhook: destination not allowed")
+		}
+		ip := net.ParseIP(host)
+		if ip == nil {
+			return errors.New("webhook: destination not allowed")
+		}
+		if isBlockedIP(ip) {
+			return errors.New("webhook: destination not allowed")
+		}
+		return nil
+	}
+}
+
+// newWebhookTransport builds an http.Transport whose dialer enforces the
+// SSRF guard on the actual address being connected to, for every connection
+// it opens (see dialControl). This is the guard of record; isAllowedURL is
+// only a fast pre-request rejection layered in front of it.
+func newWebhookTransport(allowPrivate bool) *http.Transport {
+	dialer := &net.Dialer{
+		Timeout:   30 * time.Second,
+		KeepAlive: 30 * time.Second,
+		Control:   dialControl(allowPrivate),
+	}
+	t := http.DefaultTransport.(*http.Transport).Clone()
+	t.DialContext = dialer.DialContext
+	return t
+}
+
 func (w *Webhook) Send(ctx context.Context, cfg json.RawMessage, secret string, ev Event) error {
 	var c struct {
 		URL string `json:"url"`