diff --git a/src/DodoSSH.Api/Features/Events/VaultEventFanOut.cs b/src/DodoSSH.Api/Features/Events/VaultEventFanOut.cs new file mode 100644 index 0000000..c63aaf6 --- /dev/null +++ b/src/DodoSSH.Api/Features/Events/VaultEventFanOut.cs @@ -0,0 +1,24 @@ +using DodoSSH.Api.Features.Relay; + +namespace DodoSSH.Api.Features.Events; + +/// +/// Announces a change to everything on this node that acts on one: the event sockets, and the relay. +/// +/// +/// What the write paths are given as their . They announce a change once and +/// do not know who listens, which is the point: the relay learning about removals from teams did not have to +/// touch any of the places a removal happens. +/// +internal sealed class VaultEventFanOut(VaultEventHub hub, RelayAccessWatch relay) : IVaultEventPublisher +{ + /// + public void VaultChanged(Guid vaultId, long sequence) => hub.VaultChanged(vaultId, sequence); + + /// + public void VaultAccessChanged(Guid userId) + { + hub.VaultAccessChanged(userId); + _ = relay.AccessChangedAsync(userId); + } +} diff --git a/src/DodoSSH.Api/Features/Relay/RelayAccessWatch.cs b/src/DodoSSH.Api/Features/Relay/RelayAccessWatch.cs new file mode 100644 index 0000000..72fe818 --- /dev/null +++ b/src/DodoSSH.Api/Features/Relay/RelayAccessWatch.cs @@ -0,0 +1,62 @@ +using DodoSSH.Api.Authorization; +using DodoSSH.Domain.Authorization; + +namespace DodoSSH.Api.Features.Relay; + +/// +/// Ends a user's relay sessions to vaults they can no longer reach, when their access changes. +/// +/// +/// +/// A relay session is authorised once, when its ticket is issued, and used to run on for as long as it +/// carried bytes — up to the maximum lifetime, twelve hours by default. Removing somebody from a team took +/// their keys and their vault list away at once, and left any shell they had open through the relay running +/// into the team's machines until it went idle. +/// +/// +/// Told by the same announcement that tells their event socket to re-read its vaults — see +/// — and it asks the same question the ticket +/// endpoint asked, against the database as it now stands. Off the announcing request, because that request +/// has already committed and answered, and a slow re-check must not hold it. +/// +/// +internal sealed class RelayAccessWatch( + RelaySessionRegistry sessions, + IServiceScopeFactory scopes, + ILogger logger) +{ + /// Re-checks every session the user has open, in the background. + /// The re-check, for a caller that wants to wait for it; the announcement does not. + internal Task AccessChangedAsync(Guid userId) + { + var open = sessions.SessionsOf(userId); + + return open.Count == 0 ? Task.CompletedTask : Task.Run(() => RecheckAsync(userId, open)); + } + + private async Task RecheckAsync(Guid userId, IReadOnlyList open) + { + try + { + var scope = scopes.CreateAsyncScope(); + await using var _ = scope.ConfigureAwait(false); + + var access = scope.ServiceProvider.GetRequiredService(); + + foreach (var lease in open) + { + var resolved = await access.ResolveAsync(userId, lease.VaultId, CancellationToken.None) + .ConfigureAwait(false); + + if (!resolved.Granted || !resolved.Permissions.HasFlag(PermissionFlags.Connect)) + { + lease.Revoke(); + } + } + } + catch (Exception exception) when (exception is not OutOfMemoryException) + { + RelayLog.AccessRecheckFailed(logger, userId, exception); + } + } +} diff --git a/src/DodoSSH.Api/Features/Relay/RelayConnectEndpoint.cs b/src/DodoSSH.Api/Features/Relay/RelayConnectEndpoint.cs index d440f87..a964adb 100644 --- a/src/DodoSSH.Api/Features/Relay/RelayConnectEndpoint.cs +++ b/src/DodoSSH.Api/Features/Relay/RelayConnectEndpoint.cs @@ -83,7 +83,7 @@ internal sealed class RelayConnectEndpoint( return; } - var lease = sessions.TryAdmit(ticket.UserId); + var lease = sessions.TryAdmit(ticket.UserId, ticket.VaultId); try { @@ -113,7 +113,7 @@ internal sealed class RelayConnectEndpoint( return; } - await RelayAsync(socket, ticket, ct).ConfigureAwait(false); + await RelayAsync(socket, ticket, lease.Revoked, ct).ConfigureAwait(false); } finally { @@ -239,7 +239,11 @@ internal sealed class RelayConnectEndpoint( } /// Dials the target, pipes until something ends it, and writes down what happened. - private async Task RelayAsync(WebSocket socket, RelayTicket ticket, CancellationToken requestAborted) + private async Task RelayAsync( + WebSocket socket, + RelayTicket ticket, + CancellationToken revoked, + CancellationToken requestAborted) { var started = clock.GetUtcNow(); @@ -269,7 +273,7 @@ internal sealed class RelayConnectEndpoint( { RelayLog.SessionOpened(logger, ticket.SessionId, ticket.UserId, ticket.HostId, sessions.Count); - ending = await PumpAsync(socket, target, counters, started, requestAborted).ConfigureAwait(false); + ending = await PumpAsync(socket, target, counters, started, revoked, requestAborted).ConfigureAwait(false); } RelayLog.SessionClosed( @@ -338,13 +342,14 @@ internal sealed class RelayConnectEndpoint( Stream target, Counters counters, DateTimeOffset started, + CancellationToken revoked, CancellationToken requestAborted) { using var pump = CancellationTokenSource.CreateLinkedTokenSource(requestAborted); var upstream = ClientToTargetAsync(socket, target, counters, pump.Token); var downstream = TargetToClientAsync(target, socket, counters, pump.Token); - var minding = MindAsync(counters, started, pump.Token); + var minding = MindAsync(counters, started, revoked, pump.Token); var first = await Task.WhenAny(upstream, downstream, minding).ConfigureAwait(false); var ending = await first.ConfigureAwait(false) ?? new Ending( @@ -512,7 +517,11 @@ internal sealed class RelayConnectEndpoint( /// count; SSH's own keepalives do, which is how a client that wants an idle shell kept open says so. /// /// - private async Task MindAsync(Counters counters, DateTimeOffset started, CancellationToken cancellationToken) + private async Task MindAsync( + Counters counters, + DateTimeOffset started, + CancellationToken revoked, + CancellationToken cancellationToken) { var settings = options.Value; var hardDeadline = started + settings.MaxSessionDuration; @@ -527,27 +536,11 @@ internal sealed class RelayConnectEndpoint( drainDeadline = now + settings.DrainTimeout; } - if (now >= hardDeadline) - { - return new Ending( - RelayCloseReason.DurationLimit, - RelayProtocol.DurationLimitCloseCode, - "This relay session reached its maximum lifetime."); - } - - if (drainDeadline is { } drain && now >= drain) - { - return ShuttingDown; - } - var idleDeadline = counters.LastActivity + settings.IdleTimeout; - if (now >= idleDeadline) + if (Due(now, revoked.IsCancellationRequested, hardDeadline, drainDeadline, idleDeadline) is { } ending) { - return new Ending( - RelayCloseReason.IdleTimeout, - RelayProtocol.IdleTimeoutCloseCode, - "This relay session carried nothing for the idle timeout."); + return ending; } var next = new[] { hardDeadline, idleDeadline, drainDeadline ?? DateTimeOffset.MaxValue }.Min(); @@ -555,8 +548,8 @@ internal sealed class RelayConnectEndpoint( // Woken early by the start of a shutdown, so the drain is measured from when it began rather // than from whenever the idle clock next came round. using var wake = drainDeadline is null - ? CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, lifetime.ApplicationStopping) - : CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + ? CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, revoked, lifetime.ApplicationStopping) + : CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, revoked); try { @@ -564,7 +557,7 @@ internal sealed class RelayConnectEndpoint( } catch (OperationCanceledException) when (!cancellationToken.IsCancellationRequested) { - // The shutdown began. Round again to start the drain. + // The shutdown began, or access was withdrawn. Round again to act on whichever it was. } catch (OperationCanceledException) { @@ -575,6 +568,50 @@ internal sealed class RelayConnectEndpoint( return null; } + /// Whichever reason to end the session has come due, or null while none has. + /// + /// Revocation first, because it is the one reason here that is about the user rather than the session: + /// they were removed from the team, or the vault was taken from them, after this session was admitted. + /// It used to run on for as long as it carried bytes, up to the maximum lifetime. + /// + private static Ending? Due( + DateTimeOffset now, + bool revoked, + DateTimeOffset hardDeadline, + DateTimeOffset? drainDeadline, + DateTimeOffset idleDeadline) + { + if (revoked) + { + return AccessRevoked; + } + + if (now >= hardDeadline) + { + return new Ending( + RelayCloseReason.DurationLimit, + RelayProtocol.DurationLimitCloseCode, + "This relay session reached its maximum lifetime."); + } + + if (drainDeadline is { } drain && now >= drain) + { + return ShuttingDown; + } + + return now >= idleDeadline + ? new Ending( + RelayCloseReason.IdleTimeout, + RelayProtocol.IdleTimeoutCloseCode, + "This relay session carried nothing for the idle timeout.") + : null; + } + + private static Ending AccessRevoked { get; } = new( + RelayCloseReason.AccessRevoked, + RelayProtocol.AccessRevokedCloseCode, + "You no longer have access to this host."); + private static Ending ClientGone { get; } = new(RelayCloseReason.ClientClosed, (int)WebSocketCloseStatus.NormalClosure, string.Empty); diff --git a/src/DodoSSH.Api/Features/Relay/RelayLog.cs b/src/DodoSSH.Api/Features/Relay/RelayLog.cs index 3ea1b46..463faf1 100644 --- a/src/DodoSSH.Api/Features/Relay/RelayLog.cs +++ b/src/DodoSSH.Api/Features/Relay/RelayLog.cs @@ -91,4 +91,14 @@ internal static partial class RelayLog Level = LogLevel.Warning, Message = "Relay session {SessionId} ended but its audit record could not be closed.")] internal static partial void AuditCloseFailed(ILogger logger, Guid sessionId, Exception exception); + + /// + /// A warning: the sessions it was checking stay open, which is how they all behaved before the check + /// existed, and each still ends at its idle timeout or its lifetime. + /// + [LoggerMessage( + EventId = 2309, + Level = LogLevel.Warning, + Message = "Re-checking relay access for user {UserId} after an access change failed.")] + internal static partial void AccessRecheckFailed(ILogger logger, Guid userId, Exception exception); } diff --git a/src/DodoSSH.Api/Features/Relay/RelaySessionRegistry.cs b/src/DodoSSH.Api/Features/Relay/RelaySessionRegistry.cs index a2b3514..68c2a6d 100644 --- a/src/DodoSSH.Api/Features/Relay/RelaySessionRegistry.cs +++ b/src/DodoSSH.Api/Features/Relay/RelaySessionRegistry.cs @@ -22,6 +22,7 @@ internal sealed class RelaySessionRegistry(IOptions options) { private readonly Lock gate = new(); private readonly Dictionary perUser = []; + private readonly HashSet open = []; private int total; /// How many sessions are open on this node. @@ -55,7 +56,16 @@ internal sealed class RelaySessionRegistry(IOptions options) /// Admits a session, or answers null when a limit is met. /// A lease that gives the place back when disposed. - internal Lease? TryAdmit(Guid userId) + /// The sessions a user has open now, for re-checking their access. + internal IReadOnlyList SessionsOf(Guid userId) + { + lock (gate) + { + return [.. open.Where(lease => lease.UserId == userId)]; + } + } + + internal Lease? TryAdmit(Guid userId, Guid vaultId) { var settings = options.Value; @@ -71,14 +81,18 @@ internal sealed class RelaySessionRegistry(IOptions options) perUser[userId] = held + 1; total++; - return new Lease(this, userId); + var lease = new Lease(this, userId, vaultId); + open.Add(lease); + + return lease; } } - private void Release(Guid userId) + private void Release(Lease lease, Guid userId) { lock (gate) { + open.Remove(lease); total--; // Removed at zero rather than left behind, so the dictionary is the size of the users with a @@ -95,16 +109,41 @@ internal sealed class RelaySessionRegistry(IOptions options) } /// One admitted session's place. Idempotent to dispose. - internal sealed class Lease(RelaySessionRegistry registry, Guid userId) : IDisposable + internal sealed class Lease(RelaySessionRegistry registry, Guid userId, Guid vaultId) : IDisposable { + private readonly CancellationTokenSource revoked = new(); + private int released; + /// Whose session this is. + internal Guid UserId { get; } = userId; + + /// The vault the ticket's host is filed in, which is what the session's access hangs on. + internal Guid VaultId { get; } = vaultId; + + /// Cancelled when the session's user loses access to . + internal CancellationToken Revoked => revoked.Token; + + /// Ends the session, because its user can no longer reach the vault it was opened for. + internal void Revoke() + { + try + { + revoked.Cancel(); + } + catch (ObjectDisposedException) + { + // Already over. + } + } + /// public void Dispose() { if (Interlocked.Exchange(ref released, 1) == 0) { - registry.Release(userId); + registry.Release(this, UserId); + revoked.Dispose(); } } } diff --git a/src/DodoSSH.Api/Program.cs b/src/DodoSSH.Api/Program.cs index a1a99dd..fa5bc2e 100644 --- a/src/DodoSSH.Api/Program.cs +++ b/src/DodoSSH.Api/Program.cs @@ -41,13 +41,12 @@ builder.Services.AddScoped(); builder.Services.AddScoped(); builder.Services.AddSingleton(); -// A singleton, because the sockets it holds outlive the requests that opened them. Registered twice -// resolving to the same instance: the endpoint needs the whole hub — admit, remove, count — while the -// write paths that announce a change need only the two methods that announce one, and should not gain -// a reference to connection management to get them. +// A singleton, because the sockets it holds outlive the requests that opened them. The endpoint needs the +// whole hub — admit, remove, count — while the write paths that announce a change need only the two methods +// that announce one, and get them from a fan-out that also tells the relay: a user who loses access to a +// vault loses their relay sessions into it too. See VaultEventFanOut and RelayAccessWatch. builder.Services.AddSingleton(); -builder.Services.AddSingleton( - provider => provider.GetRequiredService()); +builder.Services.AddSingleton(); // The relay's services, its ticket key ring, and a shutdown long enough for its drain. See ADR 0004. builder.Services.AddDodoRelay(); diff --git a/src/DodoSSH.Api/Setup/RelaySetup.cs b/src/DodoSSH.Api/Setup/RelaySetup.cs index cee1ca0..68ccb22 100644 --- a/src/DodoSSH.Api/Setup/RelaySetup.cs +++ b/src/DodoSSH.Api/Setup/RelaySetup.cs @@ -24,6 +24,7 @@ internal static class RelaySetup // the network seam holds no state. services.AddSingleton(); services.AddSingleton(); + services.AddSingleton(); services.AddSingleton(); // The application name isolates this deployment's payloads from any other ASP.NET application diff --git a/src/DodoSSH.Client.Session/ServerRelay.cs b/src/DodoSSH.Client.Session/ServerRelay.cs index c2f44c3..25289e9 100644 --- a/src/DodoSSH.Client.Session/ServerRelay.cs +++ b/src/DodoSSH.Client.Session/ServerRelay.cs @@ -80,6 +80,9 @@ public sealed class RelayRoute : IConnectionRoute RelayProtocol.DurationLimitCloseCode => "The server relay closed the connection: it reached the longest session the server allows.", + RelayProtocol.AccessRevokedCloseCode => + "The server relay closed the connection: you no longer have access to this host's vault.", + _ => null, }; } diff --git a/src/DodoSSH.Contracts/PublicAPI.Unshipped.txt b/src/DodoSSH.Contracts/PublicAPI.Unshipped.txt index e45a481..2320a5f 100644 --- a/src/DodoSSH.Contracts/PublicAPI.Unshipped.txt +++ b/src/DodoSSH.Contracts/PublicAPI.Unshipped.txt @@ -382,6 +382,7 @@ DodoSSH.Contracts.RelayConfiguration.RelayConfiguration(bool Enabled, System.Uri DodoSSH.Contracts.RelayConfiguration.WebSocketUrl.get -> System.Uri? DodoSSH.Contracts.RelayConfiguration.WebSocketUrl.init -> void DodoSSH.Contracts.RelaySessionCloseReason +DodoSSH.Contracts.RelaySessionCloseReason.AccessRevoked = 8 -> DodoSSH.Contracts.RelaySessionCloseReason DodoSSH.Contracts.RelaySessionCloseReason.ClientClosed = 1 -> DodoSSH.Contracts.RelaySessionCloseReason DodoSSH.Contracts.RelaySessionCloseReason.DurationLimit = 4 -> DodoSSH.Contracts.RelaySessionCloseReason DodoSSH.Contracts.RelaySessionCloseReason.Error = 7 -> DodoSSH.Contracts.RelaySessionCloseReason @@ -969,6 +970,7 @@ static DodoSSH.Contracts.VaultKeyWrap.operator ==(DodoSSH.Contracts.VaultKeyWrap static DodoSSH.Contracts.VaultSummary.operator !=(DodoSSH.Contracts.VaultSummary? left, DodoSSH.Contracts.VaultSummary? right) -> bool static DodoSSH.Contracts.VaultSummary.operator ==(DodoSSH.Contracts.VaultSummary? left, DodoSSH.Contracts.VaultSummary? right) -> bool const DodoSSH.Contracts.ProblemCodes.RelayUnavailable = "relay-unavailable" -> string! +const DodoSSH.Contracts.RelayProtocol.AccessRevokedCloseCode = 4403 -> int const DodoSSH.Contracts.RelayProtocol.ConnectPath = "/api/v1/relay/connect" -> string! const DodoSSH.Contracts.RelayProtocol.DurationLimitCloseCode = 4409 -> int const DodoSSH.Contracts.RelayProtocol.Feature = "relay" -> string! diff --git a/src/DodoSSH.Contracts/Relay.cs b/src/DodoSSH.Contracts/Relay.cs index 41e361b..2d9fec2 100644 --- a/src/DodoSSH.Contracts/Relay.cs +++ b/src/DodoSSH.Contracts/Relay.cs @@ -70,6 +70,15 @@ public static class RelayProtocol /// Close code for a session that reached the maximum lifetime this server allows. public const int DurationLimitCloseCode = 4409; + + /// + /// Close code for a session whose user lost access to the host's vault while it was open. + /// + /// + /// Removed from the team, or the vault's key withdrawn. Reconnecting will not help: a new ticket is + /// refused for the same reason. + /// + public const int AccessRevokedCloseCode = 4403; } @@ -143,6 +152,9 @@ public enum RelaySessionCloseReason /// An error ended the session. Error = 7, + + /// The user lost access to the host's vault while the session was open. + AccessRevoked = 8, } /// diff --git a/src/DodoSSH.Domain/Relay.cs b/src/DodoSSH.Domain/Relay.cs index 4df2620..f10260d 100644 --- a/src/DodoSSH.Domain/Relay.cs +++ b/src/DodoSSH.Domain/Relay.cs @@ -120,4 +120,7 @@ public enum RelayCloseReason /// An error ended the session. Error = 7, + + /// The user lost access to the host's vault while the session was open. + AccessRevoked = 8, } diff --git a/tests/DodoSSH.Api.Tests/RelayEndpointTests.cs b/tests/DodoSSH.Api.Tests/RelayEndpointTests.cs index 30dc6ff..ca83b80 100644 --- a/tests/DodoSSH.Api.Tests/RelayEndpointTests.cs +++ b/tests/DodoSSH.Api.Tests/RelayEndpointTests.cs @@ -2,6 +2,7 @@ using System.Net; using System.Net.Sockets; using System.Net.WebSockets; using System.Text; +using DodoSSH.Api.Features.Events; using DodoSSH.Contracts; using DodoSSH.Domain; using DodoSSH.Infrastructure; @@ -266,6 +267,56 @@ public sealed class RelayEndpointTests(ApiFixture fixture) .ShouldBe(RelayCloseReason.IdleTimeout); } + /// + /// + /// A session is authorised once, when its ticket is issued, and used to run on for as long as it carried + /// bytes — removing somebody from a team left their shells into its machines open for up to the session's + /// maximum lifetime. The announcement that tells their event socket to re-read its vaults now has the + /// relay ask again too. + /// + /// + /// Access is withdrawn here by deleting the vault rather than by a team removal, because what is under + /// test is the relay hearing the announcement and asking the access model again — any way of making the + /// answer change will do, and this one needs no team. The removal path announces through the same call; + /// see TeamService.RemoveMemberAsync. + /// + /// + [Fact] + public async Task ASessionWhoseUserLosesAccess_IsClosedWithItsOwnCode() + { + using var target = LoopbackTarget.Echo(); + var (subject, hostId) = await SeedRoutedHostAsync(target, "10.40.8.1"); + var ticket = await IssueAsync(subject, hostId); + + using var timeout = Timeout(); + using var socket = await ConnectAsync(ticket, timeout.Token); + + await RoundTripAsync(socket, "still here", timeout.Token); + + Guid userId; + + await using (var scope = fixture.CreateScope()) + { + var database = scope.ServiceProvider.GetRequiredService(); + + userId = (await database.Users.SingleAsync(u => u.Subject == subject, timeout.Token)).Id; + + var vaultId = (await database.Hosts.SingleAsync(h => h.Id == hostId, timeout.Token)).VaultId; + var vault = await database.Vaults.SingleAsync(v => v.Id == vaultId, timeout.Token); + + vault.DeletedAtUtc = Now; + await database.SaveChangesAsync(timeout.Token); + + scope.ServiceProvider.GetRequiredService().VaultAccessChanged(userId); + } + + await ReadUntilCloseAsync(socket, timeout.Token); + + socket.CloseStatus.ShouldBe((WebSocketCloseStatus)RelayProtocol.AccessRevokedCloseCode); + (await WaitForClosedAuditAsync(ticket.SessionId, timeout.Token)).CloseReason + .ShouldBe(RelayCloseReason.AccessRevoked); + } + [Fact] public async Task AUserAtTheCap_IsRefusedATicket_AndASocketOverItIsClosed() {