added extra IP checks for the connected realtime client

CHANGELOG.md

@@ -3,7 +3,9 @@
 - Added `RealtimeConnectRequestEvent.MaxTimeout` field to specify the absolute max duration a realtime connection can remain open (default to 30mins).
     _This is in addition to the `IdeTimeout` of 5mins in order to prevent misuse and to allow the GC to run more regularly._
 
-- (@todo) Updated all `golang.org/x/` packages containing the [recent security fixes](https://groups.google.com/g/golang-announce/c/PdiGK3xulk4).
+- Added extra checks for the connected user IP in the realtime APIs to prevent bruteforce guest subscription update attempts and to serve as an extra XSRF protection for the "all-in-one" OAuth2 realtime handler.
+
+- (@todo) Updated all `golang.org/x/` packages containing several [security fixes](https://groups.google.com/g/golang-announce/c/PdiGK3xulk4).
 
 
 ## v0.38.1

apis/realtime.go

@@ -28,6 +28,9 @@ const clientsChunkSize = 150
 // RealtimeClientAuthKey is the name of the realtime client store key that holds its auth state.
 const RealtimeClientAuthKey = "auth"
 
+// RealtimeClientIPKey is the name of the realtime client store key that holds the IP of the connected client.
+const RealtimeClientIPKey = "pbRealtimeClientIP"
+
 // bindRealtimeApi registers the realtime api endpoints.
 func bindRealtimeApi(app core.App, rg *router.RouterGroup[*core.RequestEvent]) {
 	sub := rg.Group("/realtime")
@@ -63,9 +66,12 @@ func realtimeConnect(e *core.RequestEvent) error {
 
 	connectEvent := new(core.RealtimeConnectRequestEvent)
 	connectEvent.RequestEvent = e
-	connectEvent.Client = subscriptions.NewDefaultClient()
 	connectEvent.IdleTimeout = 5 * time.Minute
 	connectEvent.MaxTimeout = 30 * time.Minute
+	connectEvent.Client = subscriptions.NewDefaultClient()
+
+	// could be used as an optional cross-reference check in other API endpoints
+	connectEvent.Client.Set(RealtimeClientIPKey, e.RealIP())
 
 	return e.App.OnRealtimeConnectRequest().Trigger(connectEvent, func(ce *core.RealtimeConnectRequestEvent) error {
 		// register new subscription client
@@ -74,7 +80,7 @@ func realtimeConnect(e *core.RequestEvent) error {
 			e.App.SubscriptionsBroker().Unregister(ce.Client.Id())
 		}()
 
-		ce.App.Logger().Debug("Realtime connection established.", slog.String("clientId", ce.Client.Id()))
+		ce.App.Logger().Debug("Realtime connection established", slog.String("clientId", ce.Client.Id()))
 
 		// signalize established connection (aka. fire "connect" message)
 		connectMsgEvent := new(core.RealtimeMessageEvent)
@@ -194,6 +200,21 @@ func realtimeSetSubscriptions(e *core.RequestEvent) error {
 		return e.NotFoundError("Missing or invalid client id.", err)
 	}
 
+	// for just in case to prevent someone changing a guest subscription
+	//
+	// note1: this is an extra precaution against clientId bruteforce attempts
+	// for installations allowing longer realtime connections duration
+	//
+	// note2: custom registered clients (aka. those without IP in the store)
+	// are excluded from the check for backward compatibility
+	clientIP, _ := client.Get(RealtimeClientIPKey).(string)
+	if clientIP != "" && clientIP != e.RealIP() {
+		return e.BadRequestError(
+			"Invalid realtime client.",
+			errors.New("the subscription request IP doesn't match with the realtime client IP"),
+		)
+	}
+
 	// for now allow only guest->auth upgrades and any other auth change is forbidden
 	clientAuth, _ := client.Get(RealtimeClientAuthKey).(*core.Record)
 	if clientAuth != nil && !isSameAuth(clientAuth, e.Auth) {
@@ -216,7 +237,7 @@ func realtimeSetSubscriptions(e *core.RequestEvent) error {
 		e.Client.Subscribe(e.Subscriptions...)
 
 		e.App.Logger().Debug(
-			"Realtime subscriptions updated.",
+			"Realtime subscriptions updated",
 			slog.String("clientId", e.Client.Id()),
 			slog.Any("subscriptions", e.Subscriptions),
 		)

apis/realtime_test.go

@@ -26,6 +26,7 @@ func TestRealtimeConnect(t *testing.T) {
 			Method:         http.MethodGet,
 			URL:            "/api/realtime",
 			Timeout:        100 * time.Millisecond,
+			Headers:        map[string]string{"x-test-ip": "127.0.0.2"},
 			ExpectedStatus: 200,
 			ExpectedContent: []string{
 				`id:`,
@@ -37,6 +38,17 @@ func TestRealtimeConnect(t *testing.T) {
 				"OnRealtimeConnectRequest": 1,
 				"OnRealtimeMessageSend":    1,
 			},
+			BeforeTestFunc: func(t testing.TB, app *tests.TestApp, e *core.ServeEvent) {
+				app.Settings().TrustedProxy.Headers = []string{"x-test-ip"}
+
+				app.OnRealtimeConnectRequest().BindFunc(func(e *core.RealtimeConnectRequestEvent) error {
+					if ip, _ := e.Client.Get(apis.RealtimeClientIPKey).(string); ip != "127.0.0.2" {
+						t.Fatalf("Expected IP %q, got %q", "127.0.0.2", ip)
+					}
+
+					return e.Next()
+				})
+			},
 			AfterTestFunc: func(t testing.TB, app *tests.TestApp, res *http.Response) {
 				if len(app.SubscriptionsBroker().Clients()) != 0 {
 					t.Errorf("Expected the subscribers to be removed after connection close, found %d", len(app.SubscriptionsBroker().Clients()))
@@ -102,7 +114,8 @@ func TestRealtimeSubscribe(t *testing.T) {
 
 	resetClient := func() {
 		client.Unsubscribe()
-		client.Set(apis.RealtimeClientAuthKey, nil)
+		client.Unset(apis.RealtimeClientAuthKey)
+		client.Unset(apis.RealtimeClientIPKey)
 	}
 
 	validSubscriptionsLimit := make([]string, 1000)
@@ -208,6 +221,26 @@ func TestRealtimeSubscribe(t *testing.T) {
 			},
 			ExpectedEvents: map[string]int{"*": 0},
 		},
+		{
+			Name:            "existing client with different IP",
+			Method:          http.MethodPost,
+			URL:             "/api/realtime",
+			Body:            strings.NewReader(`{"clientId":"` + client.Id() + `","subscriptions":["test"]}`),
+			Headers:         map[string]string{"x-test-ip": "127.0.0.2"},
+			ExpectedStatus:  400,
+			ExpectedContent: []string{`"data":{}`},
+			ExpectedEvents:  map[string]int{"*": 0},
+			BeforeTestFunc: func(t testing.TB, app *tests.TestApp, e *core.ServeEvent) {
+				app.Settings().TrustedProxy.Headers = []string{"x-test-ip"}
+
+				client.Set(apis.RealtimeClientIPKey, "127.0.0.1")
+
+				app.SubscriptionsBroker().Register(client)
+			},
+			AfterTestFunc: func(t testing.TB, app *tests.TestApp, res *http.Response) {
+				resetClient()
+			},
+		},
 		{
 			Name:   "existing client with valid topic length",
 			Method: http.MethodPost,

apis/record_auth_with_oauth2_redirect.go

@@ -55,6 +55,16 @@ func oauth2SubscriptionRedirect(e *core.RequestEvent) error {
 	}
 	defer client.Unsubscribe(oauth2SubscriptionTopic)
 
+	// additional check to minimize the risk of XSRF attack vectors
+	//
+	// note: custom registered clients (aka. those without IP in the store)
+	// are excluded from the check for backward compatibility
+	clientIP, _ := client.Get(RealtimeClientIPKey).(string)
+	if clientIP != "" && clientIP != e.RealIP() {
+		e.App.Logger().Debug("The client IP that completed the authentication is different from the one that initialized the OAuth2 realtime connection")
+		return failureRedirect(e)
+	}
+
 	// temporary store the Apple user's name so that it can be later retrieved with the authWithOAuth2 call
 	// (see https://github.com/pocketbase/pocketbase/issues/7090)
 	if data.AppleUser != "" && data.Error == "" && data.Code != "" {

apis/record_auth_with_oauth2_redirect_test.go

@@ -8,6 +8,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/pocketbase/pocketbase/apis"
 	"github.com/pocketbase/pocketbase/core"
 	"github.com/pocketbase/pocketbase/tests"
 	"github.com/pocketbase/pocketbase/tools/subscriptions"
@@ -16,9 +17,9 @@ import (
 func TestRecordAuthWithOAuth2Redirect(t *testing.T) {
 	t.Parallel()
 
-	clientStubs := make([]map[string]subscriptions.Client, 0, 10)
+	clientStubs := make([]map[string]subscriptions.Client, 0, 11)
 
-	for i := 0; i < 10; i++ {
+	for i := 0; i < 11; i++ {
 		c1 := subscriptions.NewDefaultClient()
 
 		c2 := subscriptions.NewDefaultClient()
@@ -335,6 +336,28 @@ func TestRecordAuthWithOAuth2Redirect(t *testing.T) {
 				}
 			},
 		},
+		{
+			Name:    "client with different IP",
+			Method:  http.MethodGet,
+			URL:     "/api/oauth2-redirect?code=123&state=" + clientStubs[10]["c3"].Id(),
+			Headers: map[string]string{"x-test-ip": "127.0.0.2"},
+			BeforeTestFunc: func(t testing.TB, app *tests.TestApp, e *core.ServeEvent) {
+				app.Settings().TrustedProxy.Headers = []string{"x-test-ip"}
+
+				clientStubs[10]["c3"].Set(apis.RealtimeClientIPKey, "127.0.0.1")
+
+				beforeTestFunc(clientStubs[10], map[string][]string{
+					"c3": {`"state":"` + clientStubs[10]["c3"].Id(), `"code":"123"`},
+				})(t, app, e)
+			},
+			ExpectedStatus: http.StatusTemporaryRedirect,
+			ExpectedEvents: map[string]int{"*": 0},
+			AfterTestFunc: func(t testing.TB, app *tests.TestApp, res *http.Response) {
+				app.Store().Get("cancelFunc").(context.CancelFunc)()
+
+				checkFailureRedirect(t, app, res)
+			},
+		},
 	}
 
 	for _, scenario := range scenarios {