tenseleyflow/shithub / 6ea11a9

Browse files

ssrf: ValidateWithResolve + use it in webhook Create/Update (SR2 H3)

Pre-fix: webhook.Create / Update called only validateURL (scheme
check) and ssrf.Validate (scheme + port). Loopback / RFC1918 /
CGNAT / multicast hosts were rejected only at delivery time inside
dialContext — admin could persist a hook with http://localhost or
http://192.168.1.1 and only see failures on the deliveries page
after the first attempt. Disallowed ports were caught (port 9090
fails Validate); IPs were not.

Post-fix:
- ssrf.ValidateWithResolve runs Validate plus a DNS lookup +
IsForbiddenIP check on every resolved IP. IP literals are
matched directly without DNS. AllowedHosts and
AllowPrivateNetworks behave the same as in dialContext.
- webhook.Create + Update call ValidateWithResolve. The plain
Validate is left in place as the cheap syntactic gate.
- dialContext keeps re-resolving as defense in depth (DNS
rebinding) — the validate-resolve check is *not* a substitute.

ssrf_create_test pins the table directly: 127.0.0.1, [::1],
192.168.1.1, 10.0.0.1, 172.16.0.1, port 9090 — all rejected.
A public host on a default port still passes.
Authored by espadonne
SHA
6ea11a965c385d245caa7604982cd441fb0b9fdd
Parents
cad7ad5
Tree
f28fae8

3 changed files

StatusFile+-
M internal/security/ssrf/ssrf.go 54 1
M internal/webhook/manage.go 15 0
A internal/webhook/ssrf_create_test.go 66 0
internal/security/ssrf/ssrf.gomodified
@@ -89,10 +89,63 @@ func (c Config) HTTPClient() *http.Client {
8989
 	}
9090
 }
9191
92
+// ValidateWithResolve runs Validate plus a DNS resolve so callers can
93
+// reject loopback/private/CGNAT/multicast hosts at create-time, not
94
+// only at delivery-time. dialContext still re-resolves on each dial
95
+// (DNS rebinding defense) — this is the cheap-but-thorough gate
96
+// admin forms call so the persisted hook can't sit broken-on-arrival
97
+// (SR2 H3). It's NOT a substitute for the dial-time check.
98
+//
99
+// A nil Resolver uses net.DefaultResolver. AllowedHosts (exact match
100
+// case-insensitive) bypasses the IP block-list as in dialContext.
101
+func (c Config) ValidateWithResolve(ctx context.Context, rawURL string) error {
102
+	if err := c.Validate(rawURL); err != nil {
103
+		return err
104
+	}
105
+	cfg := c.applyDefaults()
106
+	u, err := url.Parse(rawURL)
107
+	if err != nil {
108
+		return &Error{URL: rawURL, Reason: "malformed URL"}
109
+	}
110
+	host := u.Hostname()
111
+	if stringSetContainsFold(cfg.AllowedHosts, host) {
112
+		return nil
113
+	}
114
+	if cfg.AllowPrivateNetworks {
115
+		return nil
116
+	}
117
+	// IP literal — check directly without DNS.
118
+	if ip := net.ParseIP(host); ip != nil {
119
+		if IsForbiddenIP(ip) {
120
+			return &Error{URL: rawURL, Reason: "host resolves to forbidden IP " + ip.String()}
121
+		}
122
+		return nil
123
+	}
124
+	// Hostname — resolve and check every result.
125
+	resolver := cfg.Resolver
126
+	if resolver == nil {
127
+		resolver = net.DefaultResolver
128
+	}
129
+	ips, err := resolver.LookupIPAddr(ctx, host)
130
+	if err != nil {
131
+		return &Error{URL: rawURL, Reason: "DNS resolve: " + err.Error()}
132
+	}
133
+	if len(ips) == 0 {
134
+		return &Error{URL: rawURL, Reason: "no IPs resolved"}
135
+	}
136
+	for _, ipa := range ips {
137
+		if IsForbiddenIP(ipa.IP) {
138
+			return &Error{URL: rawURL, Reason: "host resolves to forbidden IP " + ipa.IP.String()}
139
+		}
140
+	}
141
+	return nil
142
+}
143
+
92144
 // Validate runs the syntactic gate (scheme, port, host shape) without
93145
 // resolving DNS. dialContext re-runs the IP-level checks at connect
94146
 // time so a passing Validate doesn't imply the URL is safe to dial —
95
-// it's the early-rejection cheap gate.
147
+// it's the early-rejection cheap gate. For full create-time rejection
148
+// (loopback, private IPs, etc.) use ValidateWithResolve.
96149
 func (c Config) Validate(rawURL string) error {
97150
 	cfg := c.applyDefaults()
98151
 	u, err := url.Parse(rawURL)
internal/webhook/manage.gomodified
@@ -5,6 +5,7 @@ package webhook
55
 import (
66
 	"context"
77
 	"errors"
8
+	"fmt"
89
 	"net/url"
910
1011
 	"github.com/jackc/pgx/v5/pgtype"
@@ -51,6 +52,14 @@ func Create(ctx context.Context, deps ManageDeps, params CreateParams) (webhookd
5152
 	if err := validateURL(params.URL); err != nil {
5253
 		return webhookdb.Webhook{}, err
5354
 	}
55
+	// SSRF defense: reject loopback / private / disallowed-port URLs at
56
+	// create time so the form returns synchronously instead of every
57
+	// delivery silently failing later (SR2 H3). ValidateWithResolve
58
+	// runs scheme/port + DNS-resolve + IP-block-list — the delivery
59
+	// path's dialContext re-resolves as defense in depth (DNS rebinding).
60
+	if err := deps.SSRF.ValidateWithResolve(ctx, params.URL); err != nil {
61
+		return webhookdb.Webhook{}, fmt.Errorf("%w: %v", ErrBadURL, err)
62
+	}
5463
 	ownerKind, err := parseOwnerKind(params.OwnerKind)
5564
 	if err != nil {
5665
 		return webhookdb.Webhook{}, err
@@ -113,6 +122,12 @@ func Update(ctx context.Context, deps ManageDeps, hookID int64, params UpdatePar
113122
 	if err := validateURL(params.URL); err != nil {
114123
 		return err
115124
 	}
125
+	// SSRF re-validation at update time mirrors Create (SR2 H3). An
126
+	// admin who flips a previously-valid hook's URL to an internal
127
+	// address gets the same synchronous rejection.
128
+	if err := deps.SSRF.ValidateWithResolve(ctx, params.URL); err != nil {
129
+		return fmt.Errorf("%w: %v", ErrBadURL, err)
130
+	}
116131
 	contentType, err := parseContentType(params.ContentType)
117132
 	if err != nil {
118133
 		return err
internal/webhook/ssrf_create_test.goadded
@@ -0,0 +1,66 @@
1
+// SPDX-License-Identifier: AGPL-3.0-or-later
2
+
3
+package webhook
4
+
5
+import (
6
+	"context"
7
+	"errors"
8
+	"fmt"
9
+	"testing"
10
+)
11
+
12
+// TestSSRFRejectsLoopbackAndPrivateAtCreateTime pins SR2 H3:
13
+// Create/Update must reject loopback / RFC1918 / disallowed-port URLs
14
+// synchronously instead of letting them persist and only fail on
15
+// every delivery attempt.
16
+//
17
+// Production code calls cfg.ValidateWithResolve(ctx, url) — that's
18
+// what Create() and Update() use. The plain Validate() is the cheap
19
+// syntactic gate; ValidateWithResolve adds the IP-block-list check
20
+// that catches loopback + RFC1918 hosts (literal and resolved).
21
+func TestSSRFRejectsLoopbackAndPrivateAtCreateTime(t *testing.T) {
22
+	t.Parallel()
23
+
24
+	cfg := DefaultSSRFConfig()
25
+	rejected := []string{
26
+		"http://127.0.0.1/hook",
27
+		"http://127.0.0.1:8080/hook",
28
+		"http://[::1]/hook",
29
+		"http://192.168.1.1/hook",
30
+		"http://10.0.0.1/hook",
31
+		"http://172.16.0.1/hook",
32
+		// Disallowed port (only 80/443/8080/8443 pass by default).
33
+		"http://example.com:9090/hook",
34
+	}
35
+	ctx := context.Background()
36
+	for _, u := range rejected {
37
+		t.Run(u, func(t *testing.T) {
38
+			t.Parallel()
39
+			err := cfg.ValidateWithResolve(ctx, u)
40
+			if err == nil {
41
+				t.Fatalf("SSRF.ValidateWithResolve(%q) = nil; expected an error", u)
42
+			}
43
+		})
44
+	}
45
+
46
+	// Sanity: a public-looking URL on a default port should pass.
47
+	if err := cfg.ValidateWithResolve(ctx, "https://example.com/hook"); err != nil {
48
+		t.Fatalf("SSRF.ValidateWithResolve(public) = %v; expected nil", err)
49
+	}
50
+}
51
+
52
+// TestCreateUpdateWrapsSSRFErrInBadURL pins the error contract used
53
+// by Create/Update: an SSRF rejection wraps ErrBadURL so callers can
54
+// errors.Is(err, ErrBadURL) for form-shaped feedback. Production code
55
+// uses fmt.Errorf("%w: %v", ErrBadURL, err) — this test pins that the
56
+// wrap shape unwraps correctly.
57
+func TestCreateUpdateWrapsSSRFErrInBadURL(t *testing.T) {
58
+	t.Parallel()
59
+
60
+	inner := errors.New("ssrf: loopback")
61
+	wrapped := fmt.Errorf("%w: %v", ErrBadURL, inner)
62
+
63
+	if !errors.Is(wrapped, ErrBadURL) {
64
+		t.Fatalf("wrapped error is not ErrBadURL: %v", wrapped)
65
+	}
66
+}