fix possible ssrf in test notifications

- Validate resolved IPs immediately before connecting.
- Guard Shoutrrr requests during initialization and delivery.
- Require admins for services without HTTP client support.
- Test DNS rebinding, redirects, internal hosts, and authorization.
This commit is contained in:
henrygd
2026-09-06 12:47:40 -04:00
parent 266a74bab8
commit aefb917a08
7 changed files with 423 additions and 119 deletions
+22 -6
View File
@@ -20,10 +20,10 @@ type hubLike interface {
}
type AlertManager struct {
hub hubLike
stopOnce sync.Once
pendingAlerts sync.Map
alertsCache *AlertsCache
hub hubLike
stopOnce sync.Once
pendingAlerts sync.Map
alertsCache *AlertsCache
}
type AlertMessageData struct {
@@ -231,8 +231,20 @@ func (am *AlertManager) SendAlert(data AlertMessageData) error {
am.hub.Logger().Error("Failed to unmarshal user settings", "err", err)
}
// send alerts via webhooks
send := sendPublicNotification
if len(userAlertSettings.Webhooks) > 0 {
// Read the owner's current role at delivery time, including for URLs
// saved before an admin was demoted. Never fall back on lookup failure.
owner, err := am.hub.FindRecordById("users", data.UserID)
if err != nil {
return fmt.Errorf("load notification owner: %w", err)
}
if owner.GetString("role") == "admin" {
send = shoutrrr.Send
}
}
for _, webhook := range userAlertSettings.Webhooks {
if err := am.SendShoutrrrAlert(webhook, data.Title, data.Message, data.Link, data.LinkText); err != nil {
if err := am.sendShoutrrrAlert(webhook, data.Title, data.Message, data.Link, data.LinkText, send); err != nil {
am.hub.Logger().Error("Failed to send shoutrrr alert", "err", err)
}
}
@@ -263,6 +275,10 @@ func (am *AlertManager) SendAlert(data AlertMessageData) error {
// SendShoutrrrAlert sends an alert via a Shoutrrr URL
func (am *AlertManager) SendShoutrrrAlert(notificationUrl, title, message, link, linkText string) error {
return am.sendShoutrrrAlert(notificationUrl, title, message, link, linkText, shoutrrr.Send)
}
func (am *AlertManager) sendShoutrrrAlert(notificationUrl, title, message, link, linkText string, send func(string, string) error) error {
// Parse the URL
parsedURL, err := url.Parse(notificationUrl)
if err != nil {
@@ -305,7 +321,7 @@ func (am *AlertManager) SendShoutrrrAlert(notificationUrl, title, message, link,
parsedURL.RawQuery = queryParams.Encode()
// log.Println("URL after modification:", parsedURL.String())
err = shoutrrr.Send(parsedURL.String(), message)
err = send(parsedURL.String(), message)
if err == nil {
am.hub.Logger().Info("Sent shoutrrr alert", "title", title)
+7 -65
View File
@@ -3,13 +3,11 @@ package alerts
import (
"database/sql"
"errors"
"net"
"net/http"
"net/url"
"slices"
"strings"
"github.com/henrygd/beszel/internal/hub/utils"
"github.com/nicholas-fedor/shoutrrr"
"github.com/pocketbase/dbx"
"github.com/pocketbase/pocketbase/core"
)
@@ -147,72 +145,16 @@ func (am *AlertManager) SendTestNotification(e *core.RequestEvent) error {
if err != nil || data.URL == "" {
return e.BadRequestError("URL is required", err)
}
// Only allow admins to send test notifications to internal URLs
send := shoutrrr.Send
if !e.Auth.IsSuperuser() && e.Auth.GetString("role") != "admin" {
internalURL, err := isInternalURL(data.URL)
if err != nil {
return e.BadRequestError(err.Error(), nil)
}
if internalURL {
return e.ForbiddenError("Only admins can send to internal destinations", nil)
}
send = sendPublicNotification
}
err = am.sendShoutrrrAlert(data.URL, "Test Alert", "This is a notification from Beszel.", am.hub.Settings().Meta.AppURL, "View Beszel", send)
if errors.Is(err, errInternalDestination) || errors.Is(err, errUnrestrictedService) {
return e.ForbiddenError(err.Error(), nil)
}
err = am.SendShoutrrrAlert(data.URL, "Test Alert", "This is a notification from Beszel.", am.hub.Settings().Meta.AppURL, "View Beszel")
if err != nil {
return e.JSON(200, map[string]string{"err": err.Error()})
}
return e.JSON(200, map[string]bool{"err": false})
}
// isInternalURL checks if the given shoutrrr URL points to an internal destination (localhost or private IP)
func isInternalURL(rawURL string) (bool, error) {
parsedURL, err := url.Parse(rawURL)
if err != nil {
return false, err
}
host := parsedURL.Hostname()
if host == "" {
return false, nil
}
if strings.EqualFold(host, "localhost") {
return true, nil
}
if ip := net.ParseIP(host); ip != nil {
return isInternalIP(ip), nil
}
// Some Shoutrrr URLs use the host position for service identifiers rather than a
// network hostname (for example, discord://token@webhookid). Restrict DNS lookups
// to names that look like actual hostnames so valid service URLs keep working.
if !strings.Contains(host, ".") {
return false, nil
}
ips, err := net.LookupIP(host)
if err != nil {
return false, nil
}
if slices.ContainsFunc(ips, isInternalIP) {
return true, nil
}
return false, nil
}
var cgnatNetwork = &net.IPNet{
IP: net.IPv4(100, 64, 0, 0),
Mask: net.CIDRMask(10, 32),
}
func isInternalIP(ip net.IP) bool {
return ip.IsPrivate() ||
ip.IsLoopback() ||
ip.IsUnspecified() ||
ip.IsLinkLocalUnicast() ||
ip.IsMulticast() ||
cgnatNetwork.Contains(ip)
}
+33 -44
View File
@@ -7,10 +7,11 @@ import (
"encoding/json"
"io"
"net/http"
"net/http/httptest"
"strings"
"sync/atomic"
"testing"
"github.com/henrygd/beszel/internal/alerts"
beszelTests "github.com/henrygd/beszel/internal/tests"
pbTests "github.com/pocketbase/pocketbase/tests"
@@ -29,43 +30,6 @@ func jsonReader(v any) io.Reader {
return bytes.NewReader(data)
}
func TestIsInternalURL(t *testing.T) {
testCases := []struct {
name string
url string
internal bool
}{
{name: "loopback ipv4", url: "generic://127.0.0.1", internal: true},
{name: "private ipv4", url: "generic://10.0.0.1", internal: true},
{name: "localhost hostname", url: "generic://localhost", internal: true},
{name: "localhost with path", url: "generic+http://localhost/api/v1/postStuff", internal: true},
{name: "loopback with port and path", url: "generic+http://127.0.0.1:8080/api/v1/postStuff", internal: true},
{name: "public hostname", url: "generic+https://beszel.dev/api/v1/postStuff", internal: false},
{name: "cloud metadata ipv4", url: "generic://169.254.169.254", internal: true},
{name: "link-local ipv4", url: "generic://169.254.1.1", internal: true},
{name: "link-local ipv6", url: "generic://[fe80::1]", internal: true},
{name: "mapped link-local ipv4", url: "generic://[::ffff:169.254.169.254]", internal: true},
{name: "cgnat lower boundary", url: "generic://100.64.0.0", internal: true},
{name: "cgnat upper boundary", url: "generic://100.127.255.255", internal: true},
{name: "below cgnat", url: "generic://100.63.255.255", internal: false},
{name: "above cgnat", url: "generic://100.128.0.0", internal: false},
{name: "multicast ipv4", url: "generic://224.0.0.1", internal: true},
{name: "multicast ipv6", url: "generic://[ff02::1]", internal: true},
{name: "public ipv4", url: "generic://8.8.8.8", internal: false},
{name: "public ipv6", url: "generic://[2001:4860:4860::8888]", internal: false},
{name: "token style service url", url: "discord://abc123@123456789", internal: false},
{name: "single label service url", url: "slack://token@team/channel", internal: false},
}
for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
internal, err := alerts.IsInternalURL(testCase.url)
assert.NoError(t, err)
assert.Equal(t, testCase.internal, internal)
})
}
}
func TestUserAlertsApi(t *testing.T) {
hub, _ := beszelTests.NewTestHub(t.TempDir())
defer hub.Cleanup()
@@ -457,6 +421,17 @@ func TestSendTestNotification(t *testing.T) {
hub, user := beszelTests.GetHubWithUser(t)
defer hub.Cleanup()
var delivered atomic.Int32
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
delivered.Add(1)
}))
defer server.Close()
localURL := "generic+" + server.URL
readonlyUser, err := beszelTests.CreateUserWithRole(hub, "readonly@example.com", "password123", "readonly")
assert.NoError(t, err)
readonlyToken, err := readonlyUser.NewAuthToken()
assert.NoError(t, err)
userToken, err := user.NewAuthToken()
adminUser, err := beszelTests.CreateUserWithRole(hub, "admin@example.com", "password123", "admin")
@@ -481,11 +456,11 @@ func TestSendTestNotification(t *testing.T) {
ExpectedContent: []string{"requires valid"},
TestAppFactory: testAppFactory,
Body: jsonReader(map[string]any{
"url": "generic://127.0.0.1",
"url": localURL,
}),
},
{
Name: "POST /test-notification - with external auth should succeed",
Name: "POST /test-notification - invalid service reports error",
Method: http.MethodPost,
URL: "/api/beszel/test-notification",
TestAppFactory: testAppFactory,
@@ -493,7 +468,7 @@ func TestSendTestNotification(t *testing.T) {
"Authorization": userToken,
},
Body: jsonReader(map[string]any{
"url": "generic://8.8.8.8",
"url": "unknown://example.com",
}),
ExpectedStatus: 200,
ExpectedContent: []string{"\"err\":"},
@@ -535,10 +510,10 @@ func TestSendTestNotification(t *testing.T) {
"Authorization": adminUserToken,
},
Body: jsonReader(map[string]any{
"url": "generic://127.0.0.1",
"url": localURL,
}),
ExpectedStatus: 200,
ExpectedContent: []string{"\"err\":"},
ExpectedContent: []string{"\"err\":false"},
},
{
Name: "POST /test-notification - internal url with superuser auth should succeed",
@@ -549,14 +524,28 @@ func TestSendTestNotification(t *testing.T) {
"Authorization": superuserToken,
},
Body: jsonReader(map[string]any{
"url": "generic://127.0.0.1",
"url": localURL,
}),
ExpectedStatus: 200,
ExpectedContent: []string{"\"err\":"},
},
}
for _, url := range []string{localURL, "smtp://user:pass@consul", "mqtt://consul/topic"} {
scenarios = append(scenarios, beszelTests.ApiScenario{
Name: "readonly cannot send to " + url,
Method: http.MethodPost,
URL: "/api/beszel/test-notification",
TestAppFactory: testAppFactory,
Headers: map[string]string{"Authorization": readonlyToken},
Body: jsonReader(map[string]any{"url": url}),
ExpectedStatus: 403,
ExpectedContent: []string{"Only admins"},
})
}
for _, scenario := range scenarios {
scenario.Test(t)
}
assert.EqualValues(t, 2, delivered.Load(), "only admin and superuser requests should reach the server")
}
-4
View File
@@ -100,10 +100,6 @@ func (am *AlertManager) SetAlertTriggered(alert CachedAlertData, triggered bool)
return am.setAlertTriggered(alert, triggered)
}
func IsInternalURL(rawURL string) (bool, error) {
return isInternalURL(rawURL)
}
// BuildContainerLogExcerpt exposes buildContainerLogExcerpt for testing.
func BuildContainerLogExcerpt(raw string) string {
return buildContainerLogExcerpt(raw)
+66
View File
@@ -0,0 +1,66 @@
//go:build testing
package alerts_test
import (
"net/http"
"net/http/httptest"
"sync/atomic"
"testing"
"github.com/henrygd/beszel/internal/alerts"
beszelTests "github.com/henrygd/beszel/internal/tests"
"github.com/pocketbase/dbx"
"github.com/stretchr/testify/require"
)
func TestPersistedWebhooksUseCurrentOwnerRole(t *testing.T) {
hub, user := beszelTests.GetHubWithUser(t)
defer hub.Cleanup()
am := alerts.NewTestAlertManagerWithoutWorker(hub)
var delivered atomic.Int32
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
delivered.Add(1)
}))
defer server.Close()
settings, err := hub.FindFirstRecordByFilter("user_settings", "user={:user}", dbx.Params{"user": user.Id})
require.NoError(t, err)
settings.Set("settings", alerts.UserNotificationSettings{Webhooks: []string{"generic+" + server.URL}})
require.NoError(t, hub.Save(settings))
message := alerts.AlertMessageData{UserID: user.Id, Title: "Test", Message: "Persisted webhook"}
// Keep the same URL and manager while changing roles, so cached privileges
// or treating previously saved URLs as trusted would fail this test.
for _, tc := range []struct {
name string
role string
want int32
}{
{"regular user", "user", 0},
{"readonly user", "readonly", 0},
{"promoted admin", "admin", 1},
{"demoted admin", "user", 1},
} {
t.Run(tc.name, func(t *testing.T) {
user.Set("role", tc.role)
require.NoError(t, hub.Save(user))
// Webhook errors are logged; SendAlert continues to email delivery.
require.NoError(t, am.SendAlert(message))
require.Equal(t, tc.want, delivered.Load())
})
}
t.Run("missing owner fails closed", func(t *testing.T) {
// Model an orphaned settings record without deleting it through the
// normal user deletion cascade.
const missingOwner = "missingowner123"
settings.Set("user", missingOwner)
require.NoError(t, hub.SaveNoValidate(settings))
message.UserID = missingOwner
err := am.SendAlert(message)
require.ErrorContains(t, err, "load notification owner")
require.EqualValues(t, 1, delivered.Load())
})
}
+129
View File
@@ -0,0 +1,129 @@
package alerts
import (
"errors"
"fmt"
"net"
"net/http"
"net/netip"
"sync/atomic"
"syscall"
"time"
"github.com/nicholas-fedor/shoutrrr/pkg/router"
"github.com/nicholas-fedor/shoutrrr/pkg/types"
)
var (
errInternalDestination = errors.New("Only admins can send to internal destinations")
errUnrestrictedService = errors.New("Only admins can use notification services without HTTP client support")
publicNotificationClient = newPublicNotificationClient()
)
func newPublicNotificationClient() *http.Client {
dialer := &net.Dialer{
Timeout: 10 * time.Second,
// Control receives the resolved IP, immediately before connect. Every
// address attempted (including DNS retries and redirects) is checked.
Control: func(_, address string, _ syscall.RawConn) error {
return checkNotificationAddress(address)
},
}
return &http.Client{
Timeout: 15 * time.Second,
Transport: &http.Transport{
// Do not use proxies: they can resolve the target themselves and
// bypass the destination check on our socket.
DialContext: dialer.DialContext,
TLSHandshakeTimeout: 10 * time.Second,
IdleConnTimeout: 90 * time.Second,
},
}
}
func checkNotificationAddress(address string) error {
addr, err := netip.ParseAddrPort(address)
if err != nil || addr.Addr().Zone() != "" {
return errInternalDestination
}
ip := net.IP(addr.Addr().AsSlice())
if !ip.IsGlobalUnicast() || isInternalIP(ip) {
return errInternalDestination
}
return nil
}
func sendPublicNotification(rawURL, message string) error {
client := &notificationClient{Client: publicNotificationClient}
service, err := newPublicNotificationService(rawURL, client)
if err == nil {
err = service.Send(message, &types.Params{})
}
// Some services format errors without preserving their error chain.
if client.blocked.Load() {
return errInternalDestination
}
return err
}
type notificationClient struct {
*http.Client
blocked atomic.Bool
}
func (c *notificationClient) Do(req *http.Request) (*http.Response, error) {
response, err := c.Client.Do(req)
if errors.Is(err, errInternalDestination) {
c.blocked.Store(true)
}
return response, err
}
func newPublicNotificationService(rawURL string, client types.HTTPClient) (types.Service, error) {
r := &router.ServiceRouter{}
scheme, serviceURL, err := r.ExtractServiceName(rawURL)
if err != nil {
return nil, err
}
service, err := r.NewService(scheme)
if err != nil {
return nil, err
}
setter, ok := service.(types.HTTPClientSetter)
if !ok {
return nil, errUnrestrictedService
}
if serviceURL.Scheme != scheme {
custom, ok := service.(types.CustomURLService)
if !ok {
return nil, fmt.Errorf("%w: %s", router.ErrCustomURLsNotSupported, scheme)
}
serviceURL, err = custom.GetServiceURLFromCustom(serviceURL)
if err != nil {
return nil, err
}
}
// Shoutrrr v0.19.0 CreateSenderWithOptions injects only AFTER Initialize.
// Matrix can log in during Initialize, so inject before it as well.
setter.SetHTTPClient(client)
if err := service.Initialize(serviceURL, nil); err != nil {
return nil, err
}
// Some initializers replace their HTTP client with a default client.
setter.SetHTTPClient(client)
return service, nil
}
var cgnatNetwork = &net.IPNet{
IP: net.IPv4(100, 64, 0, 0),
Mask: net.CIDRMask(10, 32),
}
func isInternalIP(ip net.IP) bool {
return ip.IsPrivate() ||
ip.IsLoopback() ||
ip.IsUnspecified() ||
ip.IsLinkLocalUnicast() ||
ip.IsMulticast() ||
cgnatNetwork.Contains(ip)
}
+166
View File
@@ -0,0 +1,166 @@
package alerts
import (
"context"
"encoding/binary"
"errors"
"io"
"net"
"net/http"
"net/http/httptest"
"strings"
"sync/atomic"
"testing"
"github.com/nicholas-fedor/shoutrrr/pkg/types"
"golang.org/x/net/dns/dnsmessage"
)
func TestCheckNotificationAddress(t *testing.T) {
for _, host := range []string{"127.0.0.1", "10.0.0.1", "172.16.0.1", "192.168.0.1", "169.254.169.254", "100.64.0.0", "100.127.255.255", "0.0.0.0", "224.0.0.1", "255.255.255.255", "::1", "::", "fc00::1", "fe80::1", "ff02::1", "::ffff:127.0.0.1", "::ffff:169.254.169.254", "fe80::1%lo", "localhost", "consul"} {
t.Run(host, func(t *testing.T) {
if err := checkNotificationAddress(net.JoinHostPort(host, "80")); !errors.Is(err, errInternalDestination) {
t.Fatalf("expected blocked address, got %v", err)
}
})
}
for _, host := range []string{"8.8.8.8", "100.63.255.255", "100.128.0.0", "2001:4860:4860::8888"} {
if err := checkNotificationAddress(net.JoinHostPort(host, "443")); err != nil {
t.Errorf("public address %s: %v", host, err)
}
}
}
func TestPublicNotificationBlocksInternalRequests(t *testing.T) {
var hits atomic.Int32
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
hits.Add(1)
}))
defer server.Close()
host := strings.TrimPrefix(server.URL, "http://")
for _, rawURL := range []string{
"generic+http://" + host,
"generic+https://" + host,
"generic+http://localhost:" + strings.Split(host, ":")[1],
"matrix://user:password@" + host + "/room?disabletls=yes",
"mattermost://" + host + "/token?disabletls=yes",
} {
t.Run(rawURL, func(t *testing.T) {
if err := sendPublicNotification(rawURL, "test"); !errors.Is(err, errInternalDestination) {
t.Fatalf("expected internal destination error, got %v", err)
}
})
}
if hits.Load() != 0 {
t.Fatal("internal server received a request")
}
for _, rawURL := range []string{"smtp://user:pass@consul", "mqtt://consul/topic", "mqtts://consul/topic"} {
if err := sendPublicNotification(rawURL, "test"); !errors.Is(err, errUnrestrictedService) {
t.Errorf("expected unsupported transport rejection for %s, got %v", rawURL, err)
}
}
}
type notificationRoundTripper func(*http.Request) (*http.Response, error)
func (f notificationRoundTripper) RoundTrip(r *http.Request) (*http.Response, error) { return f(r) }
func TestPublicNotificationRedirect(t *testing.T) {
client := newPublicNotificationClient()
defer client.CloseIdleConnections()
transport := client.Transport
client.Transport = notificationRoundTripper(func(r *http.Request) (*http.Response, error) {
if r.URL.Host == "public.example" {
return &http.Response{StatusCode: 307, Header: http.Header{"Location": {"http://127.0.0.1/"}}, Body: io.NopCloser(strings.NewReader("")), Request: r}, nil
}
return transport.RoundTrip(r)
})
_, err := client.Get("http://public.example/")
if !errors.Is(err, errInternalDestination) {
t.Fatalf("expected redirect to be blocked, got %v", err)
}
}
func TestPublicNotificationServiceClient(t *testing.T) {
for _, rawURL := range []string{"generic+http://public.example/path", "discord://token@123456789", "slack://hook:AAAAAAAAA-BBBBBBBBB-123456789123456789123456@webhook"} {
t.Run(rawURL, func(t *testing.T) {
var hits int
client := &http.Client{Transport: notificationRoundTripper(func(r *http.Request) (*http.Response, error) {
hits++
body := `{"ok":true}`
if strings.HasPrefix(rawURL, "slack:") {
body = "ok"
}
return &http.Response{StatusCode: 200, Header: make(http.Header), Body: io.NopCloser(strings.NewReader(body)), Request: r}, nil
})}
service, err := newPublicNotificationService(rawURL, client)
if err != nil {
t.Fatal(err)
}
if err := service.Send("test", &types.Params{}); err != nil {
t.Fatal(err)
}
if hits == 0 {
t.Fatal("injected client was not used")
}
})
}
}
func TestPublicNotificationDNS(t *testing.T) {
// Supply deterministic DNS responses over an in-memory TCP connection.
// The first lookup sees a public IP; subsequent lookups see loopback.
var rebound atomic.Bool
resolver := &net.Resolver{PreferGo: true, Dial: func(ctx context.Context, network, address string) (net.Conn, error) {
client, server := net.Pipe()
go func() {
defer server.Close()
var size [2]byte
if _, err := io.ReadFull(server, size[:]); err != nil {
return
}
buf := make([]byte, binary.BigEndian.Uint16(size[:]))
if _, err := io.ReadFull(server, buf); err != nil {
return
}
var msg dnsmessage.Message
if err := msg.Unpack(buf); err != nil {
return
}
msg.Header.Response = true
msg.Header.RecursionAvailable = true
q := msg.Questions[0]
if q.Type == dnsmessage.TypeA {
ip := [4]byte{8, 8, 8, 8}
if rebound.Load() {
ip = [4]byte{127, 0, 0, 1}
}
msg.Answers = []dnsmessage.Resource{{Header: dnsmessage.ResourceHeader{Name: q.Name, Type: q.Type, Class: dnsmessage.ClassINET}, Body: &dnsmessage.AResource{A: ip}}}
}
buf, err := msg.Pack()
if err != nil {
return
}
binary.BigEndian.PutUint16(size[:], uint16(len(buf)))
server.Write(append(size[:], buf...))
}()
return client, nil
}}
// These tests do not run in parallel; restore the process resolver afterward.
previous := net.DefaultResolver
net.DefaultResolver = resolver
t.Cleanup(func() { net.DefaultResolver = previous })
ips, err := resolver.LookupIP(context.Background(), "ip4", "rebind.example")
if err != nil || len(ips) != 1 || !ips[0].Equal(net.IPv4(8, 8, 8, 8)) {
t.Fatalf("initial DNS lookup: %v, %v", ips, err)
}
rebound.Store(true)
client := newPublicNotificationClient()
defer client.CloseIdleConnections()
for _, host := range []string{"rebind.example", "consul"} {
_, err := client.Get("http://" + host + "/")
if !errors.Is(err, errInternalDestination) {
t.Errorf("expected dial-time rejection for %s, got %v", host, err)
}
}
}