From 3006bae0d8ea9d42ed887862dcb6e56e0b9be081 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 17 Jul 2024 14:11:35 +0200 Subject: [PATCH 1/2] Send client-generated session GUID for identification purposes This is the first half of a change that *may* fix https://github.com/ppy/osu/issues/26338 (it definitely fixes *one case* where the issue happens, but I'm not sure if it will cover all of them). As described in the issue thread, using the `jti` claim from the JWT used for authorisation seemed like a decent idea. However, upon closer inspection the scheme falls over badly in a specific scenario where: 1. A client instance connects to spectator server using JWT A. 2. At some point, JWT A expires, and is silently rotated by the game in exchange for JWT B. The spectator server knows nothing of this, and continues to only track JWT A, including the old `jti` claim in said JWT. 3. At some later point, the client's connection to one of the spectator server hubs drops out. A reconnection is automatically attempted, *but* it is attempted using JWT B. The spectator server was not aware of JWT B until now, and said JWT has a different `jti` claim than the old one, so to the spectator server, it looks like a completely different client connecting, which boots the user out of their account. This PR adds a per-session GUID which is sent in a HTTP header on every connection attempt to spectator server. This GUID will be used instead of the `jti` claim in JWTs as a persistent identifier of a single user's single lazer session, which bypasses the failure scenario described above. I don't think any stronger primitive than this is required. As far as I can tell this is as strong a protection as the JWT was (which is to say, not *very* strong), and doing this removes a lot of weird complexity that would be otherwise incurred by attempting to have client ferry all of its newly issued JWTs to the server so that it can be aware of them. --- osu.Game/Online/API/APIAccess.cs | 2 ++ osu.Game/Online/API/DummyAPIAccess.cs | 2 ++ osu.Game/Online/API/IAPIProvider.cs | 6 ++++++ osu.Game/Online/HubClientConnector.cs | 8 ++++++-- 4 files changed, 16 insertions(+), 2 deletions(-) diff --git a/osu.Game/Online/API/APIAccess.cs b/osu.Game/Online/API/APIAccess.cs index 923f841bd848..0cf344ecafd9 100644 --- a/osu.Game/Online/API/APIAccess.cs +++ b/osu.Game/Online/API/APIAccess.cs @@ -164,6 +164,8 @@ private WebSocketNotificationsClientConnector setUpNotificationsClient() public string AccessToken => authentication.RequestAccessToken(); + public Guid SessionIdentifier { get; } = Guid.NewGuid(); + /// /// Number of consecutive requests which failed due to network issues. /// diff --git a/osu.Game/Online/API/DummyAPIAccess.cs b/osu.Game/Online/API/DummyAPIAccess.cs index 960941fc05f7..0af76537cd76 100644 --- a/osu.Game/Online/API/DummyAPIAccess.cs +++ b/osu.Game/Online/API/DummyAPIAccess.cs @@ -39,6 +39,8 @@ public partial class DummyAPIAccess : Component, IAPIProvider public string AccessToken => "token"; + public Guid SessionIdentifier { get; } = Guid.NewGuid(); + /// public bool IsLoggedIn => State.Value > APIState.Offline; diff --git a/osu.Game/Online/API/IAPIProvider.cs b/osu.Game/Online/API/IAPIProvider.cs index 7b95b68ec3b4..d8194dc32b19 100644 --- a/osu.Game/Online/API/IAPIProvider.cs +++ b/osu.Game/Online/API/IAPIProvider.cs @@ -44,6 +44,12 @@ public interface IAPIProvider /// string AccessToken { get; } + /// + /// Used as an identifier of a single local lazer session. + /// Sent across the wire for the purposes of concurrency control to spectator server. + /// + Guid SessionIdentifier { get; } + /// /// Returns whether the local user is logged in. /// diff --git a/osu.Game/Online/HubClientConnector.cs b/osu.Game/Online/HubClientConnector.cs index 9d414deade75..dc9ed7cc2e40 100644 --- a/osu.Game/Online/HubClientConnector.cs +++ b/osu.Game/Online/HubClientConnector.cs @@ -19,6 +19,9 @@ public class HubClientConnector : PersistentEndpointClientConnector, IHubClientC { public const string SERVER_SHUTDOWN_MESSAGE = "Server is shutting down."; + public const string VERSION_HASH_HEADER = @"OsuVersionHash"; + public const string CLIENT_SESSION_ID_HEADER = @"X-Client-Session-ID"; + /// /// Invoked whenever a new hub connection is built, to configure it before it's started. /// @@ -68,8 +71,9 @@ protected override Task BuildConnectionAsync(Cancellat options.Proxy.Credentials = CredentialCache.DefaultCredentials; } - options.Headers.Add("Authorization", $"Bearer {API.AccessToken}"); - options.Headers.Add("OsuVersionHash", versionHash); + options.Headers.Add(@"Authorization", @$"Bearer {API.AccessToken}"); + options.Headers.Add(VERSION_HASH_HEADER, versionHash); + options.Headers.Add(CLIENT_SESSION_ID_HEADER, API.SessionIdentifier.ToString()); }); if (RuntimeFeature.IsDynamicCodeCompiled && preferMessagePack) From 2a601ce9617d0ef55bcb36b4de4e87278179b72e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bart=C5=82omiej=20Dach?= Date: Wed, 17 Jul 2024 16:21:33 +0200 Subject: [PATCH 2/2] Also send version hash header under more accepted convention of name --- osu.Game/Online/HubClientConnector.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/osu.Game/Online/HubClientConnector.cs b/osu.Game/Online/HubClientConnector.cs index dc9ed7cc2e40..9288a32052dd 100644 --- a/osu.Game/Online/HubClientConnector.cs +++ b/osu.Game/Online/HubClientConnector.cs @@ -19,7 +19,7 @@ public class HubClientConnector : PersistentEndpointClientConnector, IHubClientC { public const string SERVER_SHUTDOWN_MESSAGE = "Server is shutting down."; - public const string VERSION_HASH_HEADER = @"OsuVersionHash"; + public const string VERSION_HASH_HEADER = @"X-Osu-Version-Hash"; public const string CLIENT_SESSION_ID_HEADER = @"X-Client-Session-ID"; /// @@ -72,6 +72,8 @@ protected override Task BuildConnectionAsync(Cancellat } options.Headers.Add(@"Authorization", @$"Bearer {API.AccessToken}"); + // non-standard header name kept for backwards compatibility, can be removed after server side has migrated to `VERSION_HASH_HEADER` + options.Headers.Add(@"OsuVersionHash", versionHash); options.Headers.Add(VERSION_HASH_HEADER, versionHash); options.Headers.Add(CLIENT_SESSION_ID_HEADER, API.SessionIdentifier.ToString()); });