From d06d539d34c143d842b91e2a64326e8c8f9bc405 Mon Sep 17 00:00:00 2001 From: naiba Date: Sun, 10 May 2026 02:53:24 +0000 Subject: [PATCH] fix(notification): harden webhook request handling Co-authored-by: naiba/CloudCode --- model/notification.go | 152 ++++++++++++++++++++++++++++++++++--- model/notification_test.go | 68 +++++++++++++++++ 2 files changed, 211 insertions(+), 9 deletions(-) diff --git a/model/notification.go b/model/notification.go index db32e6bf..a2304cf0 100644 --- a/model/notification.go +++ b/model/notification.go @@ -1,10 +1,14 @@ package model import ( + "context" + "crypto/tls" "errors" "fmt" "io" + "net" "net/http" + "net/netip" "net/url" "strings" "time" @@ -25,6 +29,35 @@ const ( NotificationRequestMethodPOST ) +var errNotificationURLNotAllowed = errors.New("notification URL target is not allowed") + +var notificationBlockedCIDRs = mustParseNotificationCIDRs([]string{ + "0.0.0.0/8", + "10.0.0.0/8", + "100.64.0.0/10", + "127.0.0.0/8", + "169.254.0.0/16", + "172.16.0.0/12", + "192.0.0.0/24", + "192.0.2.0/24", + "192.168.0.0/16", + "198.18.0.0/15", + "198.51.100.0/24", + "203.0.113.0/24", + "224.0.0.0/4", + "240.0.0.0/4", + "::/128", + "::1/128", + "::ffff:0:0/96", + "64:ff9b::/96", + "100::/64", + "2001::/23", + "2001:db8::/32", + "fc00::/7", + "fe80::/10", + "ff00::/8", +}) + type NotificationServerBundle struct { Notification *Notification Server *Server @@ -111,13 +144,8 @@ func (n *Notification) setRequestHeader(req *http.Request) error { } func (ns *NotificationServerBundle) Send(message string) error { - var client *http.Client n := ns.Notification - if n.VerifyTLS != nil && *n.VerifyTLS { - client = utils.HttpClient - } else { - client = utils.HttpClientSkipTlsVerify - } + verifyTLS := n.VerifyTLS != nil && *n.VerifyTLS reqBody, err := ns.reqBody(message) if err != nil { @@ -129,7 +157,13 @@ func (ns *NotificationServerBundle) Send(message string) error { return err } - req, err := http.NewRequest(reqMethod, ns.reqURL(message), strings.NewReader(reqBody)) + reqURL := ns.reqURL(message) + client, err := newNotificationHTTPClient(reqURL, verifyTLS) + if err != nil { + return err + } + + req, err := http.NewRequest(reqMethod, reqURL, strings.NewReader(reqBody)) if err != nil { return err } @@ -149,8 +183,7 @@ func (ns *NotificationServerBundle) Send(message string) error { }() if resp.StatusCode < 200 || resp.StatusCode > 299 { - body, _ := io.ReadAll(resp.Body) - return fmt.Errorf("%d@%s %s", resp.StatusCode, resp.Status, string(body)) + return notificationResponseError(resp) } else { _, _ = io.Copy(io.Discard, resp.Body) } @@ -158,6 +191,107 @@ func (ns *NotificationServerBundle) Send(message string) error { return nil } +func newNotificationHTTPClient(rawURL string, verifyTLS bool) (*http.Client, error) { + parsedURL, ip, err := resolveNotificationTarget(rawURL) + if err != nil { + return nil, err + } + + port := parsedURL.Port() + if port == "" { + if parsedURL.Scheme == "https" { + port = "443" + } else { + port = "80" + } + } + targetAddress := net.JoinHostPort(ip.String(), port) + dialer := &net.Dialer{} + + return &http.Client{ + Transport: &http.Transport{ + DialContext: func(ctx context.Context, network, address string) (net.Conn, error) { + return dialer.DialContext(ctx, network, targetAddress) + }, + TLSClientConfig: &tls.Config{InsecureSkipVerify: !verifyTLS, ServerName: parsedURL.Hostname()}, + }, + CheckRedirect: func(req *http.Request, via []*http.Request) error { + return http.ErrUseLastResponse + }, + Timeout: time.Minute * 10, + }, nil +} + +func notificationResponseError(resp *http.Response) error { + _, _ = io.CopyN(io.Discard, resp.Body, 4096) + return fmt.Errorf("%d@%s", resp.StatusCode, resp.Status) +} + +func resolveNotificationTarget(rawURL string) (*url.URL, net.IP, error) { + parsedURL, err := url.Parse(rawURL) + if err != nil { + return nil, nil, err + } + if parsedURL.Scheme != "http" && parsedURL.Scheme != "https" { + return nil, nil, errNotificationURLNotAllowed + } + + host := parsedURL.Hostname() + if host == "" { + return nil, nil, errNotificationURLNotAllowed + } + if ip := net.ParseIP(host); ip != nil { + if !notificationIPAllowed(ip) { + return nil, nil, errNotificationURLNotAllowed + } + return parsedURL, ip, nil + } + + ips, err := net.LookupIP(host) + if err != nil { + return nil, nil, err + } + if len(ips) == 0 { + return nil, nil, errNotificationURLNotAllowed + } + for _, ip := range ips { + if !notificationIPAllowed(ip) { + return nil, nil, errNotificationURLNotAllowed + } + } + + return parsedURL, ips[0], nil +} + +func notificationIPAllowed(ip net.IP) bool { + parsedIP, ok := netipFromIP(ip) + if !ok { + return false + } + for _, cidr := range notificationBlockedCIDRs { + if cidr.Contains(parsedIP) { + return false + } + } + return parsedIP.IsGlobalUnicast() +} + +func netipFromIP(ip net.IP) (netip.Addr, bool) { + parsedIP, ok := netip.AddrFromSlice(ip) + if !ok { + return netip.Addr{}, false + } + return parsedIP.Unmap(), true +} + +func mustParseNotificationCIDRs(cidrs []string) []netip.Prefix { + prefixes := make([]netip.Prefix, 0, len(cidrs)) + for _, cidr := range cidrs { + prefixes = append(prefixes, netip.MustParsePrefix(cidr)) + } + return prefixes +} + // replaceParamInString 替换字符串中的占位符 func (ns *NotificationServerBundle) replaceParamsInString(str string, message string, mod func(string) string) string { if mod == nil { diff --git a/model/notification_test.go b/model/notification_test.go index 5b7c1979..aee11446 100644 --- a/model/notification_test.go +++ b/model/notification_test.go @@ -1,6 +1,7 @@ package model import ( + "io" "net/http" "strings" "testing" @@ -234,3 +235,70 @@ func TestNotification(t *testing.T) { execCase(t, c) } } + +func TestNotificationResponseErrorDoesNotReflectNonSuccessResponseBody(t *testing.T) { + const internalResponseBody = "internal service says token=secret" + + resp := &http.Response{ + StatusCode: http.StatusTeapot, + Status: "418 I'm a teapot", + Body: io.NopCloser(strings.NewReader(internalResponseBody)), + } + + err := notificationResponseError(resp) + if strings.Contains(err.Error(), internalResponseBody) { + t.Fatalf("expected upstream response body to be hidden from error, got %q", err.Error()) + } +} + +func TestNotificationSendRejectsLoopbackTarget(t *testing.T) { + verifyTLS := true + notification := &Notification{ + URL: "http://127.0.0.1/internal", + RequestMethod: NotificationRequestMethodGET, + VerifyTLS: &verifyTLS, + } + + bundle := NotificationServerBundle{ + Notification: notification, + Loc: time.Local, + } + + err := bundle.Send("probe") + if err == nil { + t.Fatal("expected loopback notification URL to be rejected") + } + if !strings.Contains(err.Error(), "not allowed") { + t.Fatalf("expected not allowed error, got %q", err.Error()) + } +} + +func TestNotificationTargetRejectsSpecialUseAddresses(t *testing.T) { + cases := []string{ + "http://100.64.0.1/", // CGNAT + "http://192.0.2.1/", // documentation range + "http://[fc00::1]/", // IPv6 unique local + "http://[2001:db8::1]/", // IPv6 documentation range + "http://[::ffff:127.0.0.1]/", // IPv4-mapped loopback + } + + for _, rawURL := range cases { + if _, _, err := resolveNotificationTarget(rawURL); err == nil { + t.Fatalf("expected %s to be rejected", rawURL) + } + } +} + +func TestNotificationHTTPClientPreservesTLSServerName(t *testing.T) { + client, err := newNotificationHTTPClient("https://example.com/webhook", true) + if err != nil { + t.Fatalf("expected public HTTPS URL to create client: %v", err) + } + transport, ok := client.Transport.(*http.Transport) + if !ok { + t.Fatalf("expected http.Transport, got %T", client.Transport) + } + if transport.TLSClientConfig == nil || transport.TLSClientConfig.ServerName != "example.com" { + t.Fatalf("expected TLS ServerName example.com, got %#v", transport.TLSClientConfig) + } +}