From bc9ed71ff8d686f07f1e5a41d11925daa64d00d8 Mon Sep 17 00:00:00 2001 From: Kenneth Skovhede Date: Sat, 31 Aug 2024 11:22:10 +0200 Subject: [PATCH 1/2] Added support for drifts in JWT refresh tokens. This can happen if the user refreshes the browser during the login. --- Duplicati/WebserverCore/Middlewares/JWTProvider.cs | 1 + Duplicati/WebserverCore/Services/LoginProvider.cs | 6 ++++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/Duplicati/WebserverCore/Middlewares/JWTProvider.cs b/Duplicati/WebserverCore/Middlewares/JWTProvider.cs index 789abe6e2..50f0d56b9 100644 --- a/Duplicati/WebserverCore/Middlewares/JWTProvider.cs +++ b/Duplicati/WebserverCore/Middlewares/JWTProvider.cs @@ -17,6 +17,7 @@ public record JWTConfig public int RefreshTokenDurationInMinutes { get; init; } = 60 * 24 * 30; public int SigninTokenDurationInMinutes { get; init; } = 5; public int SingleOperationTokenDurationInMinutes { get; init; } = 1; + public int MaxRefreshTokenDrift { get; init; } = 1; public SymmetricSecurityKey SymmetricSecurityKey() => new(Encoding.UTF8.GetBytes(SigningKey)); public static JWTConfig Create() => new() diff --git a/Duplicati/WebserverCore/Services/LoginProvider.cs b/Duplicati/WebserverCore/Services/LoginProvider.cs index e19fe4fae..a60446bac 100644 --- a/Duplicati/WebserverCore/Services/LoginProvider.cs +++ b/Duplicati/WebserverCore/Services/LoginProvider.cs @@ -1,10 +1,11 @@ using Duplicati.Library.Logging; using Duplicati.Server.Database; using Duplicati.WebserverCore.Abstractions; +using Duplicati.WebserverCore.Middlewares; namespace Duplicati.WebserverCore.Services; -public class LoginProvider(ITokenFamilyStore repo, IJWTTokenProvider tokenProvider, Connection connection) : ILoginProvider +public class LoginProvider(ITokenFamilyStore repo, IJWTTokenProvider tokenProvider, JWTConfig jwtConfig, Connection connection) : ILoginProvider { private static readonly string LOGTAG = Log.LogTagFromType(); @@ -29,7 +30,8 @@ public class LoginProvider(ITokenFamilyStore repo, IJWTTokenProvider tokenProvid var tokenFamily = await repo.GetTokenFamily(refreshToken.UserId, refreshToken.TokenFamilyId, ct) ?? throw new UnauthorizedAccessException("Invalid refresh token"); - if (tokenFamily.Counter != refreshToken.Counter) + var counterDiff = tokenFamily.Counter - refreshToken.Counter; + if (counterDiff < 0 || counterDiff > jwtConfig.MaxRefreshTokenDrift) { Log.WriteWarningMessage(LOGTAG, "TokenFamilyReuse", null, $"Invalid refresh token counter: {tokenFamily.Counter} != {refreshToken.Counter}"); await repo.InvalidateTokenFamily(tokenFamily.UserId, tokenFamily.Id, ct); From f962bfc89bf818d80b96fc71ec559724bea37da9 Mon Sep 17 00:00:00 2001 From: Kenneth Skovhede Date: Mon, 2 Sep 2024 11:40:26 +0200 Subject: [PATCH 2/2] Added constraint on the issue timestamp to limit the window where token drift is considered acceptable --- .../Abstractions/ITokenFamilyStore.cs | 5 +++-- Duplicati/WebserverCore/Middlewares/JWTProvider.cs | 1 + Duplicati/WebserverCore/Services/LoginProvider.cs | 7 ++++++- .../WebserverCore/Services/TokenFamilyStore.cs | 14 ++++++++------ 4 files changed, 18 insertions(+), 9 deletions(-) diff --git a/Duplicati/WebserverCore/Abstractions/ITokenFamilyStore.cs b/Duplicati/WebserverCore/Abstractions/ITokenFamilyStore.cs index 5ea055c0a..1aa510df3 100644 --- a/Duplicati/WebserverCore/Abstractions/ITokenFamilyStore.cs +++ b/Duplicati/WebserverCore/Abstractions/ITokenFamilyStore.cs @@ -53,6 +53,7 @@ public interface ITokenFamilyStore /// /// The ID. /// The user ID. - /// The counter. - public record TokenFamily(string Id, string UserId, int Counter); + /// The counter. + /// The last updated timestamp. + public record TokenFamily(string Id, string UserId, int Counter, DateTime LastUpdated); } diff --git a/Duplicati/WebserverCore/Middlewares/JWTProvider.cs b/Duplicati/WebserverCore/Middlewares/JWTProvider.cs index 50f0d56b9..e05feaf2b 100644 --- a/Duplicati/WebserverCore/Middlewares/JWTProvider.cs +++ b/Duplicati/WebserverCore/Middlewares/JWTProvider.cs @@ -18,6 +18,7 @@ public record JWTConfig public int SigninTokenDurationInMinutes { get; init; } = 5; public int SingleOperationTokenDurationInMinutes { get; init; } = 1; public int MaxRefreshTokenDrift { get; init; } = 1; + public int MaxRefreshTokenDriftSeconds { get; init; } = 30; public SymmetricSecurityKey SymmetricSecurityKey() => new(Encoding.UTF8.GetBytes(SigningKey)); public static JWTConfig Create() => new() diff --git a/Duplicati/WebserverCore/Services/LoginProvider.cs b/Duplicati/WebserverCore/Services/LoginProvider.cs index a60446bac..59be211ac 100644 --- a/Duplicati/WebserverCore/Services/LoginProvider.cs +++ b/Duplicati/WebserverCore/Services/LoginProvider.cs @@ -30,8 +30,13 @@ public class LoginProvider(ITokenFamilyStore repo, IJWTTokenProvider tokenProvid var tokenFamily = await repo.GetTokenFamily(refreshToken.UserId, refreshToken.TokenFamilyId, ct) ?? throw new UnauthorizedAccessException("Invalid refresh token"); + // Allow slight drift to adjust for cases where the browser refreshes + // just before the token is received, so the server is ahead var counterDiff = tokenFamily.Counter - refreshToken.Counter; - if (counterDiff < 0 || counterDiff > jwtConfig.MaxRefreshTokenDrift) + var maxDrift = (DateTime.UtcNow - tokenFamily.LastUpdated).TotalSeconds > jwtConfig.MaxRefreshTokenDriftSeconds + ? 0 + : jwtConfig.MaxRefreshTokenDrift; + if (counterDiff < 0 || counterDiff > maxDrift) { Log.WriteWarningMessage(LOGTAG, "TokenFamilyReuse", null, $"Invalid refresh token counter: {tokenFamily.Counter} != {refreshToken.Counter}"); await repo.InvalidateTokenFamily(tokenFamily.UserId, tokenFamily.Id, ct); diff --git a/Duplicati/WebserverCore/Services/TokenFamilyStore.cs b/Duplicati/WebserverCore/Services/TokenFamilyStore.cs index 0a5f5c59c..ae6ecf8c4 100644 --- a/Duplicati/WebserverCore/Services/TokenFamilyStore.cs +++ b/Duplicati/WebserverCore/Services/TokenFamilyStore.cs @@ -11,17 +11,18 @@ public class TokenFamilyStore(Connection connection) : ITokenFamilyStore { var familyId = System.Security.Cryptography.RandomNumberGenerator.GetHexString(16); var counter = System.Security.Cryptography.RandomNumberGenerator.GetInt32(1024) % 1024; + var lastUpdated = DateTime.UtcNow; connection.ExecuteWithCommand(cmd => { cmd.CommandText = @"INSERT INTO TokenFamily (""Id"", ""UserId"", ""Counter"", ""LastUpdated"") VALUES (?, ?, ?, ?)"; cmd.AddParameter(familyId); cmd.AddParameter(userId); cmd.AddParameter(counter); - cmd.AddParameter(DateTime.UtcNow.Ticks); + cmd.AddParameter(lastUpdated.Ticks); cmd.ExecuteNonQuery(); }); - return Task.FromResult(new ITokenFamilyStore.TokenFamily(familyId, userId, counter)); + return Task.FromResult(new ITokenFamilyStore.TokenFamily(familyId, userId, counter, lastUpdated)); } public Task GetTokenFamily(string userId, string familyId, CancellationToken ct) @@ -29,14 +30,14 @@ public class TokenFamilyStore(Connection connection) : ITokenFamilyStore ITokenFamilyStore.TokenFamily? family = null; connection.ExecuteWithCommand(cmd => { - cmd.CommandText = @"SELECT ""Id"", ""UserId"", ""Counter"" FROM ""TokenFamily"" WHERE ""Id"" = ? AND ""UserId"" = ?"; + cmd.CommandText = @"SELECT ""Id"", ""UserId"", ""Counter"", ""LastUpdated"" FROM ""TokenFamily"" WHERE ""Id"" = ? AND ""UserId"" = ?"; cmd.AddParameter(familyId); cmd.AddParameter(userId); using var reader = cmd.ExecuteReader(); if (!reader.Read()) return; - family = new ITokenFamilyStore.TokenFamily(reader.GetString(0), reader.GetString(1), reader.GetInt32(2)); + family = new ITokenFamilyStore.TokenFamily(reader.GetString(0), reader.GetString(1), reader.GetInt32(2), new DateTime(reader.GetInt64(3))); }); return Task.FromResult(family ?? throw new Exceptions.UnauthorizedException("Token family not found")); } @@ -44,11 +45,12 @@ public class TokenFamilyStore(Connection connection) : ITokenFamilyStore public Task IncrementTokenFamily(ITokenFamilyStore.TokenFamily tokenFamily, CancellationToken ct) { var nextCounter = tokenFamily.Counter + 1; + var lastUpdated = DateTime.UtcNow; connection.ExecuteWithCommand(cmd => { cmd.CommandText = @"UPDATE ""TokenFamily"" SET ""Counter"" = ?, ""LastUpdated"" = ? WHERE ""Id"" = ? AND ""UserId"" = ? AND ""Counter"" = ?"; cmd.AddParameter(nextCounter); - cmd.AddParameter(DateTime.UtcNow.Ticks); + cmd.AddParameter(lastUpdated.Ticks); cmd.AddParameter(tokenFamily.Id); cmd.AddParameter(tokenFamily.UserId); cmd.AddParameter(tokenFamily.Counter); @@ -56,7 +58,7 @@ public class TokenFamilyStore(Connection connection) : ITokenFamilyStore throw new Exceptions.ConflictException("Token family counter mismatch or not found"); }); - return Task.FromResult(new ITokenFamilyStore.TokenFamily(tokenFamily.Id, tokenFamily.UserId, nextCounter)); + return Task.FromResult(new ITokenFamilyStore.TokenFamily(tokenFamily.Id, tokenFamily.UserId, nextCounter, lastUpdated)); } public Task InvalidateTokenFamily(string userId, string familyId, CancellationToken ct)