From f6bb6c142e21c626851007745e17715a8a8a06a5 Mon Sep 17 00:00:00 2001 From: telli Date: Mon, 28 Sep 2026 16:06:41 -0700 Subject: [PATCH 1/2] perf(gateway): hash each account token with PBKDF2 once per process Every account-token request ran PBKDF2 (120,000 iterations, about 18 ms) inside OperatorAccountService's global lock and then rewrote the accounts file, capping gateway-wide account-token throughput. After a token's first successful verification, later uses find it by SHA-256 digest (the tokens are 192-bit random secrets) and skip PBKDF2. Each use still re-checks the token's revocation and expiry and the account's enabled state and role, so none of those needs cache invalidation. Token use now updates lastLoginAtUtc at most once a minute instead of on every request. The service takes an optional TimeProvider and hash function so tests can control time and count hashes. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 1 + .../OperatorAccountService.cs | 90 +++++++--- .../OperatorAccountServiceTests.cs | 158 ++++++++++++++++++ 3 files changed, 229 insertions(+), 20 deletions(-) create mode 100644 src/OpenClaw.Tests/OperatorAccountServiceTests.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index 5411dcda..dfa0603b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,6 +64,7 @@ All notable changes to this project are tracked in this file. - Added Vault / OpenBao secret resolver backend (`OpenClaw.Security.Vault`): KV v2 read with token auth, TTL cache with single-flight and refresh-ahead, startup pre-warm via `IHostedService`, and `vault:/data/#` reference syntax. See `docs/security/vault.md`. - Added `Security.Vault.Tls.CaCertPath` support: load a custom CA bundle (PEM file or directory) for Vault TLS validation; mutually exclusive with `Tls.SkipVerify`. - Made `vault:` references fail closed: resolving a `vault:` ref while the Vault backend is disabled now throws `VaultNotConfiguredException` instead of silently falling back to the literal string. +- Account tokens now run PBKDF2 only on their first use in each gateway process, cutting a repeat verification from about 18 ms under the account store's global lock to microseconds. Later uses match the token by SHA-256 digest and still re-check revocation, expiry, the account's enabled state, and its role. Token use updates `lastLoginAtUtc` at most once a minute instead of rewriting the accounts file on every request. ### Memory Retention and Hardening diff --git a/src/OpenClaw.Gateway/OperatorAccountService.cs b/src/OpenClaw.Gateway/OperatorAccountService.cs index ab01ffe2..1736ac52 100644 --- a/src/OpenClaw.Gateway/OperatorAccountService.cs +++ b/src/OpenClaw.Gateway/OperatorAccountService.cs @@ -45,18 +45,30 @@ internal sealed class StoredToken private const string DirectoryName = "admin"; private const string FileName = "operator-accounts.json"; + // Token use is recorded as a login at most this often, so authenticated requests don't each rewrite the accounts file. + private static readonly TimeSpan TokenLastLoginInterval = TimeSpan.FromMinutes(1); + private readonly string _path; private readonly Lock _gate = new(); private readonly ILogger _logger; + private readonly TimeProvider _clock; + private readonly Func _hashSecret; + private readonly Dictionary _verifiedTokens = new(StringComparer.Ordinal); private StoreState? _cached; - public OperatorAccountService(string storagePath, ILogger logger) + public OperatorAccountService( + string storagePath, + ILogger logger, + TimeProvider? clock = null, + Func? hashSecret = null) { var rootedStoragePath = Path.IsPathRooted(storagePath) ? storagePath : Path.GetFullPath(storagePath); _path = Path.Combine(rootedStoragePath, DirectoryName, FileName); _logger = logger; + _clock = clock ?? TimeProvider.System; + _hashSecret = hashSecret ?? Pbkdf2Hash; } public IReadOnlyList List() @@ -108,8 +120,8 @@ public OperatorAccountSummary Create(OperatorAccountCreateRequest request) DisplayName = request.DisplayName?.Trim() ?? "", Role = OperatorRoleNames.Normalize(request.Role), Enabled = request.Enabled, - CreatedAtUtc = DateTimeOffset.UtcNow, - UpdatedAtUtc = DateTimeOffset.UtcNow, + CreatedAtUtc = _clock.GetUtcNow(), + UpdatedAtUtc = _clock.GetUtcNow(), PasswordSalt = salt, PasswordHash = HashSecret(password, salt) }; @@ -142,7 +154,7 @@ public OperatorAccountSummary Create(OperatorAccountCreateRequest request) account.PasswordHash = HashSecret(NormalizePassword(request.Password), salt); } - account.UpdatedAtUtc = DateTimeOffset.UtcNow; + account.UpdatedAtUtc = _clock.GetUtcNow(); SaveUnsafe(state); return MapSummary(account); } @@ -176,8 +188,8 @@ public bool Delete(string id) return null; var created = CreateTokenUnsafe(account, request.Label, request.ExpiresAtUtc); - account.LastLoginAtUtc = DateTimeOffset.UtcNow; - account.UpdatedAtUtc = DateTimeOffset.UtcNow; + account.LastLoginAtUtc = _clock.GetUtcNow(); + account.UpdatedAtUtc = _clock.GetUtcNow(); SaveUnsafe(state); return new OperatorTokenExchangeResponse @@ -199,7 +211,7 @@ public bool Delete(string id) return null; var created = CreateTokenUnsafe(account, request.Label, request.ExpiresAtUtc); - account.UpdatedAtUtc = DateTimeOffset.UtcNow; + account.UpdatedAtUtc = _clock.GetUtcNow(); SaveUnsafe(state); return new OperatorAccountTokenCreateResponse @@ -246,9 +258,9 @@ public bool RevokeToken(string accountId, string tokenId) if (token.RevokedAtUtc is null) { - token.RevokedAtUtc = DateTimeOffset.UtcNow; + token.RevokedAtUtc = _clock.GetUtcNow(); if (account is not null) - account.UpdatedAtUtc = DateTimeOffset.UtcNow; + account.UpdatedAtUtc = _clock.GetUtcNow(); SaveUnsafe(state); } @@ -271,7 +283,7 @@ public bool TryAuthenticatePassword(string username, string password, out Operat if (!SecretMatches(NormalizePassword(password), account.PasswordSalt, account.PasswordHash)) return false; - account.LastLoginAtUtc = DateTimeOffset.UtcNow; + account.LastLoginAtUtc = _clock.GetUtcNow(); SaveUnsafe(state); identity = MapIdentity(account, OrganizationAuthModeNames.BrowserSession); return true; @@ -285,9 +297,27 @@ public bool TryAuthenticateToken(string token, out OperatorIdentitySnapshot? ide return false; var tokenPrefix = GetTokenPrefix(token); + var tokenDigest = DigestToken(token); lock (_gate) { var state = LoadUnsafe(); + var now = _clock.GetUtcNow(); + + // A token verified earlier skips PBKDF2, but its account and record are re-checked on every use. + if (_verifiedTokens.TryGetValue(tokenDigest, out var verified)) + { + var verifiedAccount = state.Accounts.FirstOrDefault(item => string.Equals(item.Id, verified.AccountId, StringComparison.Ordinal)); + var verifiedRecord = verifiedAccount?.Tokens.FirstOrDefault(item => string.Equals(item.Id, verified.TokenId, StringComparison.Ordinal)); + if (verifiedAccount is not { Enabled: true } || verifiedRecord is null || !IsUsable(verifiedRecord, now)) + { + _verifiedTokens.Remove(tokenDigest); + return false; + } + + identity = AcceptTokenUnsafe(state, verifiedAccount, now); + return true; + } + foreach (var account in state.Accounts) { if (!account.Enabled) @@ -295,18 +325,15 @@ public bool TryAuthenticateToken(string token, out OperatorIdentitySnapshot? ide foreach (var tokenRecord in account.Tokens) { - if (tokenRecord.RevokedAtUtc is not null) - continue; - if (tokenRecord.ExpiresAtUtc is not null && tokenRecord.ExpiresAtUtc <= DateTimeOffset.UtcNow) + if (!IsUsable(tokenRecord, now)) continue; if (!string.Equals(tokenRecord.TokenPrefix, tokenPrefix, StringComparison.Ordinal)) continue; if (!SecretMatches(token, tokenRecord.SecretSalt, tokenRecord.SecretHash)) continue; - account.LastLoginAtUtc = DateTimeOffset.UtcNow; - SaveUnsafe(state); - identity = MapIdentity(account, OrganizationAuthModeNames.AccountToken); + _verifiedTokens[tokenDigest] = new VerifiedToken(account.Id, tokenRecord.Id); + identity = AcceptTokenUnsafe(state, account, now); return true; } } @@ -315,6 +342,24 @@ public bool TryAuthenticateToken(string token, out OperatorIdentitySnapshot? ide return false; } + private static bool IsUsable(StoredToken token, DateTimeOffset now) + => token.RevokedAtUtc is null && (token.ExpiresAtUtc is null || token.ExpiresAtUtc > now); + + private OperatorIdentitySnapshot AcceptTokenUnsafe(StoreState state, StoredAccount account, DateTimeOffset now) + { + if (account.LastLoginAtUtc is not { } lastLogin || now - lastLogin >= TokenLastLoginInterval) + { + account.LastLoginAtUtc = now; + SaveUnsafe(state); + } + + return MapIdentity(account, OrganizationAuthModeNames.AccountToken); + } + + // Account tokens are 192-bit random secrets, so a fast digest identifies one safely; PBKDF2 adds nothing on a repeat use. + private static string DigestToken(string token) + => Convert.ToHexString(SHA256.HashData(System.Text.Encoding.UTF8.GetBytes(token))); + private StoreState LoadUnsafe() { if (_cached is not null) @@ -374,7 +419,7 @@ private static OperatorIdentitySnapshot MapIdentity(StoredAccount account, strin IsBootstrapAdmin = false }; - private static CreatedToken CreateTokenUnsafe(StoredAccount account, string? label, DateTimeOffset? expiresAtUtc) + private CreatedToken CreateTokenUnsafe(StoredAccount account, string? label, DateTimeOffset? expiresAtUtc) { var token = $"oca_{Convert.ToHexString(RandomNumberGenerator.GetBytes(24)).ToLowerInvariant()}"; var salt = GenerateSalt(); @@ -385,7 +430,7 @@ private static CreatedToken CreateTokenUnsafe(StoredAccount account, string? lab TokenPrefix = GetTokenPrefix(token), SecretSalt = salt, SecretHash = HashSecret(token, salt), - CreatedAtUtc = DateTimeOffset.UtcNow, + CreatedAtUtc = _clock.GetUtcNow(), ExpiresAtUtc = expiresAtUtc }; @@ -417,14 +462,17 @@ private static string GenerateSalt() private static string GetTokenPrefix(string token) => token[..Math.Min(token.Length, TokenPrefixLength)]; - private static string HashSecret(string secret, string saltHex) + private string HashSecret(string secret, string saltHex) + => _hashSecret(secret, saltHex); + + private static string Pbkdf2Hash(string secret, string saltHex) { var secretBytes = System.Text.Encoding.UTF8.GetBytes(secret); var saltBytes = Convert.FromHexString(saltHex); return Convert.ToHexString(Rfc2898DeriveBytes.Pbkdf2(secretBytes, saltBytes, Pbkdf2Iterations, HashAlgorithmName.SHA256, HashBytes)); } - private static bool SecretMatches(string candidate, string saltHex, string expectedHashHex) + private bool SecretMatches(string candidate, string saltHex, string expectedHashHex) { var actual = Convert.FromHexString(HashSecret(candidate, saltHex)); var expected = Convert.FromHexString(expectedHashHex); @@ -432,4 +480,6 @@ private static bool SecretMatches(string candidate, string saltHex, string expec } private sealed record CreatedToken(string Secret, StoredToken Record); + + private sealed record VerifiedToken(string AccountId, string TokenId); } diff --git a/src/OpenClaw.Tests/OperatorAccountServiceTests.cs b/src/OpenClaw.Tests/OperatorAccountServiceTests.cs new file mode 100644 index 00000000..8c8b07f7 --- /dev/null +++ b/src/OpenClaw.Tests/OperatorAccountServiceTests.cs @@ -0,0 +1,158 @@ +using System.Security.Cryptography; +using System.Text; +using Microsoft.Extensions.Logging.Abstractions; +using OpenClaw.Core.Models; +using OpenClaw.Gateway; +using Xunit; + +namespace OpenClaw.Tests; + +public sealed class OperatorAccountServiceTests : IDisposable +{ + private readonly string _storagePath = Path.Combine(Path.GetTempPath(), "openclaw-operator-account-tests", Guid.NewGuid().ToString("N")); + private readonly Clock _clock = new(); + private int _hashes; + + public void Dispose() + { + try { Directory.Delete(_storagePath, recursive: true); } + catch { } + } + + [Fact] + public void TryAuthenticateToken_WhenSameTokenIsPresentedAgain_DoesNotHashItAgain() + { + var (accounts, _, token) = CreateOperatorToken(); + Assert.True(accounts.TryAuthenticateToken(token.Token, out _)); + var hashesAfterFirstUse = _hashes; + + Assert.True(accounts.TryAuthenticateToken(token.Token, out var identity)); + + Assert.Equal(hashesAfterFirstUse, _hashes); + Assert.Equal(OperatorRoleNames.Operator, identity!.Role); + } + + [Fact] + public void TryAuthenticateToken_WhenVerifiedTokenIsRevoked_RejectsIt() + { + var (accounts, accountId, token) = CreateOperatorToken(); + Assert.True(accounts.TryAuthenticateToken(token.Token, out _)); + + Assert.True(accounts.RevokeToken(accountId, token.TokenInfo!.Id)); + + Assert.False(accounts.TryAuthenticateToken(token.Token, out _)); + } + + [Fact] + public void TryAuthenticateToken_WhenVerifiedTokenExpires_RejectsIt() + { + var (accounts, accountId, _) = CreateOperatorToken(); + var token = accounts.CreateToken(accountId, new OperatorAccountTokenCreateRequest { ExpiresAtUtc = _clock.Now.AddMinutes(5) })!; + Assert.True(accounts.TryAuthenticateToken(token.Token, out _)); + + _clock.Now = _clock.Now.AddMinutes(6); + + Assert.False(accounts.TryAuthenticateToken(token.Token, out _)); + } + + [Fact] + public void TryAuthenticateToken_WhenAccountIsDisabledAfterVerification_RejectsIt() + { + var (accounts, accountId, token) = CreateOperatorToken(); + Assert.True(accounts.TryAuthenticateToken(token.Token, out _)); + + accounts.Update(accountId, new OperatorAccountUpdateRequest { Enabled = false }); + + Assert.False(accounts.TryAuthenticateToken(token.Token, out _)); + } + + [Fact] + public void TryAuthenticateToken_WhenAccountIsDeletedAfterVerification_RejectsIt() + { + var (accounts, accountId, token) = CreateOperatorToken(); + Assert.True(accounts.TryAuthenticateToken(token.Token, out _)); + + Assert.True(accounts.Delete(accountId)); + + Assert.False(accounts.TryAuthenticateToken(token.Token, out _)); + } + + [Fact] + public void TryAuthenticateToken_WhenRoleChangesAfterVerification_ReturnsCurrentRole() + { + var (accounts, accountId, token) = CreateOperatorToken(); + Assert.True(accounts.TryAuthenticateToken(token.Token, out _)); + + accounts.Update(accountId, new OperatorAccountUpdateRequest { Role = OperatorRoleNames.Viewer }); + + Assert.True(accounts.TryAuthenticateToken(token.Token, out var identity)); + Assert.Equal(OperatorRoleNames.Viewer, identity!.Role); + } + + [Fact] + public void TryAuthenticateToken_WhenTokenDiffersFromVerifiedOne_RejectsIt() + { + var (accounts, _, token) = CreateOperatorToken(); + Assert.True(accounts.TryAuthenticateToken(token.Token, out _)); + + // Same prefix, different secret. + var forged = token.Token[..^1] + (token.Token[^1] == '0' ? '1' : '0'); + + Assert.False(accounts.TryAuthenticateToken(forged, out _)); + } + + [Fact] + public void TryAuthenticateToken_WithinLastLoginInterval_DoesNotRewriteLastLogin() + { + var (accounts, accountId, token) = CreateOperatorToken(); + Assert.True(accounts.TryAuthenticateToken(token.Token, out _)); + var firstUse = PersistedLastLogin(accountId); + + _clock.Now = _clock.Now.AddSeconds(30); + Assert.True(accounts.TryAuthenticateToken(token.Token, out _)); + + Assert.Equal(firstUse, PersistedLastLogin(accountId)); + } + + [Fact] + public void TryAuthenticateToken_AfterLastLoginInterval_RecordsNewLastLogin() + { + var (accounts, accountId, token) = CreateOperatorToken(); + Assert.True(accounts.TryAuthenticateToken(token.Token, out _)); + + _clock.Now = _clock.Now.AddMinutes(2); + Assert.True(accounts.TryAuthenticateToken(token.Token, out _)); + + Assert.Equal(_clock.Now, PersistedLastLogin(accountId)); + } + + private (OperatorAccountService Accounts, string AccountId, OperatorAccountTokenCreateResponse Token) CreateOperatorToken() + { + var accounts = CreateService(); + var account = accounts.Create(new OperatorAccountCreateRequest + { + Username = "cache-operator", + Password = "P@ssw0rd123!", + Role = OperatorRoleNames.Operator + }); + return (accounts, account.Id, accounts.CreateToken(account.Id, new OperatorAccountTokenCreateRequest { Label = "cache" })!); + } + + // A fresh instance reads what was written to disk, not the first instance's in-memory state. + private DateTimeOffset? PersistedLastLogin(string accountId) + => CreateService().Get(accountId)!.Account!.LastLoginAtUtc; + + // A fast stand-in for PBKDF2 that counts how often a secret is hashed. + private OperatorAccountService CreateService() + => new(_storagePath, NullLogger.Instance, _clock, (secret, saltHex) => + { + Interlocked.Increment(ref _hashes); + return Convert.ToHexString(SHA256.HashData(Encoding.UTF8.GetBytes(saltHex + secret))); + }); + + private sealed class Clock : TimeProvider + { + public DateTimeOffset Now { get; set; } = DateTimeOffset.UtcNow; + public override DateTimeOffset GetUtcNow() => Now; + } +} From 9e8143f3a955a94fed6b58a015704ed4d9d6aa3a Mon Sep 17 00:00:00 2001 From: telli Date: Tue, 29 Sep 2026 05:07:39 -0700 Subject: [PATCH 2/2] fix(gateway): count a token login toward the interval only once it is saved The last-login throttle set the in-memory timestamp before saving. If the save failed, that request threw, but the next minute of requests saw a recent timestamp, skipped the write, and authenticated without recording the login. Restore the previous timestamp when the save fails, so every request keeps trying, as before the throttle. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../OperatorAccountService.cs | 12 ++++- .../OperatorAccountServiceTests.cs | 47 +++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/src/OpenClaw.Gateway/OperatorAccountService.cs b/src/OpenClaw.Gateway/OperatorAccountService.cs index 1736ac52..04050014 100644 --- a/src/OpenClaw.Gateway/OperatorAccountService.cs +++ b/src/OpenClaw.Gateway/OperatorAccountService.cs @@ -349,8 +349,18 @@ private OperatorIdentitySnapshot AcceptTokenUnsafe(StoreState state, StoredAccou { if (account.LastLoginAtUtc is not { } lastLogin || now - lastLogin >= TokenLastLoginInterval) { + var previous = account.LastLoginAtUtc; account.LastLoginAtUtc = now; - SaveUnsafe(state); + try + { + SaveUnsafe(state); + } + catch + { + // Only a saved login starts the interval; otherwise the next minute of requests would skip the write. + account.LastLoginAtUtc = previous; + throw; + } } return MapIdentity(account, OrganizationAuthModeNames.AccountToken); diff --git a/src/OpenClaw.Tests/OperatorAccountServiceTests.cs b/src/OpenClaw.Tests/OperatorAccountServiceTests.cs index 8c8b07f7..79560c3c 100644 --- a/src/OpenClaw.Tests/OperatorAccountServiceTests.cs +++ b/src/OpenClaw.Tests/OperatorAccountServiceTests.cs @@ -126,6 +126,38 @@ public void TryAuthenticateToken_AfterLastLoginInterval_RecordsNewLastLogin() Assert.Equal(_clock.Now, PersistedLastLogin(accountId)); } + [Fact] + public void TryAuthenticateToken_WhenLastLoginWriteFails_TriesAgainOnTheNextRequest() + { + if (OperatingSystem.IsWindows()) + { + Assert.Skip("Uses Unix directory permissions to make the accounts file unwritable."); + return; + } + + var (accounts, _, token) = CreateOperatorToken(); + Assert.True(accounts.TryAuthenticateToken(token.Token, out _)); + _clock.Now = _clock.Now.AddMinutes(2); + + var adminDirectory = Path.Combine(_storagePath, "admin"); + var originalMode = File.GetUnixFileMode(adminDirectory); + File.SetUnixFileMode(adminDirectory, UnixFileMode.UserRead | UnixFileMode.UserExecute); + try + { + if (DirectoryIsWritable(adminDirectory)) + Assert.Skip("Directory permissions are not enforced for this user."); + + Assert.Throws(() => accounts.TryAuthenticateToken(token.Token, out _)); + + // A write that failed must not count as recorded, or requests for the next minute would skip it. + Assert.Throws(() => accounts.TryAuthenticateToken(token.Token, out _)); + } + finally + { + File.SetUnixFileMode(adminDirectory, originalMode); + } + } + private (OperatorAccountService Accounts, string AccountId, OperatorAccountTokenCreateResponse Token) CreateOperatorToken() { var accounts = CreateService(); @@ -138,6 +170,21 @@ public void TryAuthenticateToken_AfterLastLoginInterval_RecordsNewLastLogin() return (accounts, account.Id, accounts.CreateToken(account.Id, new OperatorAccountTokenCreateRequest { Label = "cache" })!); } + private static bool DirectoryIsWritable(string directory) + { + var probe = Path.Combine(directory, $".probe-{Guid.NewGuid():N}"); + try + { + File.WriteAllText(probe, ""); + File.Delete(probe); + return true; + } + catch (UnauthorizedAccessException) + { + return false; + } + } + // A fresh instance reads what was written to disk, not the first instance's in-memory state. private DateTimeOffset? PersistedLastLogin(string accountId) => CreateService().Get(accountId)!.Account!.LastLoginAtUtc;