diff --git a/docs/authentication.md b/docs/authentication.md index 489d00f8..0c0b0025 100644 --- a/docs/authentication.md +++ b/docs/authentication.md @@ -952,6 +952,9 @@ Define an IdentityResource/`role` claim and add it to the client's allowed scope |---|---| | **Redeem-ticket TTL** | 30s, fixed. The ticket is single-use, session-bound, 128-bit. | | **CSRF on `/_rask/auth/redeem`** | The secret session-bound ticket defeats classic CSRF; the endpoint additionally **rejects cross-origin `Origin`/`Referer`** (HTTP 403). | +| **Session identity trust model** | A live session is keyed by an unguessable 128-bit id embedded in the page (same model as a Blazor circuit). The WS `hello` handler binds the session's user to the principal authenticated on that socket, so security rests on the **secrecy of the session id** — serve over HTTPS, don't log it or leak it via `Referer`. Cross-origin pages can't read it (same-origin policy). | +| **Cross-Site WebSocket Hijacking** | CORS doesn't apply to WS handshakes and the upgrade carries the auth cookie, so the `/rask/ws` endpoint **rejects a cross-origin handshake** (HTTP 403) using the same host-only same-origin check as redeem. Clients sending no `Origin` (non-browser) are allowed. | +| **Session-store growth (DoS)** | Each session pins a component tree + DI scope. Sessions are reclaimed shortly after their socket disconnects; set `RaskLiveOptions.MaxSessions` for a hard ceiling (a GET over the cap gets `503` + `Retry-After`). `0` = unlimited (default). Pair with a reverse-proxy rate limit. | | **Sign-out invalidation** | Redeem clears the cookie; the WS reconnect re-seeds `SessionUserProvider` to anonymous. `SessionUserProvider.Clear()` is available for explicit invalidation. | | **Session expiry → re-auth** | A swept live session pushes `{type:"session",status:"unknown"}`; `rask.js` reloads → fresh GET → route guard challenges to `ChallengePath?returnUrl=…`. | | **JWT on WebSocket** | Token rides `?access_token=` on the WS URL via `window.Rask.authToken`; pair with `AddJwtBearer`'s `OnMessageReceived`. | @@ -967,7 +970,10 @@ Define an IdentityResource/`role` claim and add it to the client's allowed scope - ☑ If a token must be in the browser, store it **encrypted** (`ProtectedTokenStore`), never plaintext. - ☑ Short JWT lifetime + app-driven silent refresh so sessions stay smooth without long-lived tokens. - ☑ Keep `UseAuthentication()` **before** `UseRask()`. -- ☑ Validate redirect targets — Rask sanitizes the `returnUrl` to local paths. +- ☑ Validate redirect targets — Rask sanitizes the `returnUrl` to local same-origin paths (rejects `//`, `/\`, and backslash/control-char variants). +- ☑ Treat the **session id as a bearer secret** — HTTPS only, never logged or placed in URLs that leak via `Referer`. +- ☑ Behind a reverse proxy, wire **ForwardedHeaders** so the host-only same-origin checks (redeem + WS) see the public host. +- ☑ For untrusted-traffic hosts, set `RaskLiveOptions.MaxSessions` and a reverse-proxy rate limit to bound session creation. - ☑ Rotate signing keys; manage the Data Protection key ring (persisted, encrypted at rest). --- diff --git a/samples/Rask.Example.Auth.WasmCookie/Auth/WasmLoginService.cs b/samples/Rask.Example.Auth.WasmCookie/Auth/WasmLoginService.cs index 9e85df73..3aec5a15 100644 --- a/samples/Rask.Example.Auth.WasmCookie/Auth/WasmLoginService.cs +++ b/samples/Rask.Example.Auth.WasmCookie/Auth/WasmLoginService.cs @@ -19,7 +19,9 @@ public async Task LoginAsync(string username, string password, string? ret } await users.RefreshAsync(); - nav.Navigate(returnUrl ?? "/members"); + // Open-redirect guard: never navigate off-origin from an attacker-supplied returnUrl + // (parity with the server's SanitizeReturnUrl). Unsafe values collapse to "/". + nav.Navigate(LocalUrl.Sanitize(returnUrl ?? "/members")); return true; } diff --git a/samples/Rask.Example.Auth.WasmJwt.Host/Program.cs b/samples/Rask.Example.Auth.WasmJwt.Host/Program.cs index cbbde25f..53cc4be0 100644 --- a/samples/Rask.Example.Auth.WasmJwt.Host/Program.cs +++ b/samples/Rask.Example.Auth.WasmJwt.Host/Program.cs @@ -7,8 +7,19 @@ var builder = WebApplication.CreateBuilder(args); -var signingKey = new SymmetricSecurityKey(Encoding.UTF8.GetBytes( - builder.Configuration["Jwt:Key"] ?? JwtIssuer.DevKey)); +// JwtIssuer.DevKey is a public, hardcoded signing key — fine for the demo, but anyone could forge +// tokens with it. Fail fast rather than silently fall back to it outside Development, so a deploy +// that forgets to set Jwt:Key can't ship a forgeable-token endpoint. +var jwtKey = builder.Configuration["Jwt:Key"]; +if (jwtKey is null && !builder.Environment.IsDevelopment()) +{ + throw new InvalidOperationException( + "Jwt:Key is not configured. Set a strong signing key (e.g. via user-secrets, environment, " + + "or a secret store) before running outside Development — the built-in JwtIssuer.DevKey is " + + "public and lets anyone forge tokens."); +} + +var signingKey = new SymmetricSecurityKey(Encoding.UTF8.GetBytes(jwtKey ?? JwtIssuer.DevKey)); builder.Services.AddAuthentication(JwtBearerDefaults.AuthenticationScheme) .AddJwtBearer(o => diff --git a/samples/Rask.Example.Auth.WasmJwt/Auth/JwtLoginService.cs b/samples/Rask.Example.Auth.WasmJwt/Auth/JwtLoginService.cs index 1ebec1b1..4c1db0d7 100644 --- a/samples/Rask.Example.Auth.WasmJwt/Auth/JwtLoginService.cs +++ b/samples/Rask.Example.Auth.WasmJwt/Auth/JwtLoginService.cs @@ -23,7 +23,9 @@ public async Task LoginAsync(string username, string password, string? ret await tokens.SetAsync(dto.Token); await users.RefreshAsync(); - nav.Navigate(returnUrl ?? "/members"); + // Open-redirect guard: never navigate off-origin from an attacker-supplied returnUrl + // (parity with the server's SanitizeReturnUrl). Unsafe values collapse to "/". + nav.Navigate(LocalUrl.Sanitize(returnUrl ?? "/members")); return true; } diff --git a/src/Rask.Core/Components/Context.cs b/src/Rask.Core/Components/Context.cs index 746de0e5..9e18bcaf 100644 --- a/src/Rask.Core/Components/Context.cs +++ b/src/Rask.Core/Components/Context.cs @@ -94,10 +94,16 @@ public static T Required(string? name = null) /// /// true when a value of type (optionally by - /// ) is provided by an enclosing . Does - /// not mark the caller as a consumer. + /// ) is provided by an enclosing . Like + /// , marks the caller a context consumer so it re-renders when an + /// ancestor begins/stops providing the value — otherwise a component that gates purely on + /// Has would be render-cached and show stale UI when the provider appears or leaves. /// - public static bool Has(string? name = null) => ContextStack.TryGet(typeof(T), name, out _); + public static bool Has(string? name = null) + { + MarkConsumer(); + return ContextStack.TryGet(typeof(T), name, out _); + } private static void MarkConsumer() => LiveRenderContext.Current?.MarkCurrentConsumesContext(); } diff --git a/src/Rask.Core/Live/HandlerSyncContext.cs b/src/Rask.Core/Live/HandlerSyncContext.cs index 82553e86..46e57b4c 100644 --- a/src/Rask.Core/Live/HandlerSyncContext.cs +++ b/src/Rask.Core/Live/HandlerSyncContext.cs @@ -11,10 +11,13 @@ internal sealed class HandlerSyncContext : SynchronizationContext public override void Post(SendOrPostCallback d, object? state) { - var task = Task.Run(() => RunWithRendersAsync(d, state)); + // Schedule AND record under the same lock so DrainAsync can't snapshot _pending in the + // gap between Task.Run and the Add — which could otherwise let the drain return before a + // just-posted render completes. Task.Run always schedules on the pool (never inline), so + // holding the lock around it doesn't run user code under the lock. lock (_pending) { - _pending.Add(task); + _pending.Add(Task.Run(() => RunWithRendersAsync(d, state))); } } diff --git a/src/Rask.Core/Live/LiveOptions.cs b/src/Rask.Core/Live/LiveOptions.cs index 4b906c8c..f15503a7 100644 --- a/src/Rask.Core/Live/LiveOptions.cs +++ b/src/Rask.Core/Live/LiveOptions.cs @@ -46,6 +46,19 @@ public sealed class RaskLiveOptions { public LiveDiffMode DiffMode { get; set; } = LiveDiffMode.Auto; + /// + /// Maximum number of concurrent live sessions the server will hold. Each session + /// pins a component tree and a DI scope, so an unbounded count is a memory-exhaustion + /// (DoS) surface for hosts exposed to untrusted traffic. 0 (default) means + /// unlimited, preserving prior behaviour. When set, a GET that would create a session + /// beyond the cap is answered with 503 Service Unavailable + Retry-After; + /// existing sessions and auth challenge/forbid redirects are unaffected. This is a + /// coarse backstop — pair it with a reverse-proxy rate limit for precise control. + /// Sessions are reclaimed shortly after their socket disconnects (the grace-period + /// removal), so the live count tracks active clients, not cumulative visits. + /// + public int MaxSessions { get; set; } + /// /// Per-app URL prefix. Empty (default) keeps every framework URL at the /// origin root. A non-empty value like "/appA" scopes every emitted diff --git a/src/Rask.Core/Routing/LocalUrl.cs b/src/Rask.Core/Routing/LocalUrl.cs new file mode 100644 index 00000000..0905a49d --- /dev/null +++ b/src/Rask.Core/Routing/LocalUrl.cs @@ -0,0 +1,48 @@ +namespace Rask.Core.Routing; + +/// +/// Open-redirect guard shared by every Rask redirect path (server post-sign-in +/// returnUrl, the client route-guard challenge, and the WASM login/logout flows). It +/// mirrors ASP.NET's Url.IsLocalUrl: only a same-origin absolute path may pass through; +/// anything that could navigate off the origin collapses to "/". +/// +public static class LocalUrl +{ + /// + /// Returns unchanged when it is a guaranteed-local absolute path, + /// otherwise "/". Rejects: null/empty; non-rooted values ("evil.com", + /// "https://evil.com", "javascript:…"); protocol-relative URLs + /// ("//evil.com"); the backslash variants browsers normalise into them + /// ("/\evil.com", "\evil.com"); and any value containing a control character + /// (which can smuggle the value past downstream parsers / split headers). + /// + public static string Sanitize(string? url) + { + if (string.IsNullOrEmpty(url)) + { + return "/"; + } + + if (url[0] != '/' + || (url.Length > 1 && (url[1] == '/' || url[1] == '\\')) + || ContainsControlCharacter(url)) + { + return "/"; + } + + return url; + } + + private static bool ContainsControlCharacter(string value) + { + foreach (var c in value) + { + if (char.IsControl(c)) + { + return true; + } + } + + return false; + } +} diff --git a/src/Rask.Core/Routing/Navigator.cs b/src/Rask.Core/Routing/Navigator.cs index 2a60a8de..a947ec05 100644 --- a/src/Rask.Core/Routing/Navigator.cs +++ b/src/Rask.Core/Routing/Navigator.cs @@ -119,6 +119,13 @@ private IDownloadSink ResolveSink() => internal IDisposable EnterHandler() { + // Clear any navigation a prior handler queued but never consumed — e.g. it called + // Navigate(...) and then threw before TryConsumeHistory ran. Resetting on entry (rather + // than on scope dispose) starts each dispatch clean so a faulted handler can't leak its + // pending nav (and _replace flag) into the next one, while still allowing the caller to + // consume the navigation after the scope disposes. + _dirty = false; + _replace = false; _inHandler = true; return new HandlerScope(this); } diff --git a/src/Rask.Generators/ComponentFactoryGenerator.cs b/src/Rask.Generators/ComponentFactoryGenerator.cs index 3b13cad5..cccf3d10 100644 --- a/src/Rask.Generators/ComponentFactoryGenerator.cs +++ b/src/Rask.Generators/ComponentFactoryGenerator.cs @@ -814,7 +814,7 @@ private static void EmitFactory(StringBuilder sb, Candidate c, bool emitNavigati } first = false; - sb.Append(p.TypeFqn).Append(' ').Append(p.Name); + sb.Append(p.TypeFqn).Append(' ').Append(p.Escaped); } foreach (var p in optionalProps) @@ -825,7 +825,7 @@ private static void EmitFactory(StringBuilder sb, Candidate c, bool emitNavigati } first = false; - sb.Append(p.TypeFqn).Append(' ').Append(p.Name).Append(" = ") + sb.Append(p.TypeFqn).Append(' ').Append(p.Escaped).Append(" = ") .Append(DefaultLiteralFor(p)); } @@ -1196,7 +1196,7 @@ private static void EmitInitializerBody(StringBuilder sb, IEnumerable for (var i = 0; i < entries.Count; i++) { var p = entries[i]; - sb.Append(" ").Append(p.Name).Append(" = ").Append(p.Name); + sb.Append(" ").Append(p.Escaped).Append(" = ").Append(p.Escaped); if (i < entries.Count - 1) { sb.Append(','); @@ -1219,19 +1219,21 @@ private static void EmitSnapshotsAndAssignments(StringBuilder sb, // nullable annotations round-trip). foreach (var p in foldProps) { - sb.Append(" var __old_").Append(p.Name).Append(" = __c.").Append(p.Name).AppendLine(";"); + // __old_ is a fresh local (raw Name is a valid identifier even when Name is a + // keyword); the property access __c. must be '@'-escaped. + sb.Append(" var __old_").Append(p.Name).Append(" = __c.").Append(p.Escaped).AppendLine(";"); } // Re-apply ALL params (including Key) so cached instances see fresh values. Event-callback // delegates are wrapped so invoking them re-renders the owning component (see AutoCallback). foreach (var p in assignProps) { - sb.Append(" __c.").Append(p.Name).Append(" = "); + sb.Append(" __c.").Append(p.Escaped).Append(" = "); if (p.IsAutoRerenderDelegate) { // Wrap returns a nullable delegate (null in → null out); a non-nullable prop never // passes null, so the null-forgiving `!` is safe and silences CS8601. - sb.Append("global::Rask.Core.AutoCallback.Wrap(").Append(p.Name).Append(')'); + sb.Append("global::Rask.Core.AutoCallback.Wrap(").Append(p.Escaped).Append(')'); if (!p.IsNullable) { sb.Append('!'); @@ -1239,7 +1241,7 @@ private static void EmitSnapshotsAndAssignments(StringBuilder sb, } else { - sb.Append(p.Name); + sb.Append(p.Escaped); } sb.AppendLine(";"); @@ -1258,7 +1260,7 @@ private static void EmitSnapshotsAndAssignments(StringBuilder sb, { var p = foldProps[0]; sb.Append(" var __propsChanged = !global::System.Collections.Generic.EqualityComparer<") - .Append(p.TypeFqn).Append(">.Default.Equals(__old_").Append(p.Name).Append(", ").Append(p.Name) + .Append(p.TypeFqn).Append(">.Default.Equals(__old_").Append(p.Name).Append(", ").Append(p.Escaped) .AppendLine(");"); return; } @@ -1268,7 +1270,7 @@ private static void EmitSnapshotsAndAssignments(StringBuilder sb, { var p = foldProps[i]; sb.Append(" !global::System.Collections.Generic.EqualityComparer<").Append(p.TypeFqn) - .Append(">.Default.Equals(__old_").Append(p.Name).Append(", ").Append(p.Name).Append(')'); + .Append(">.Default.Equals(__old_").Append(p.Name).Append(", ").Append(p.Escaped).Append(')'); sb.AppendLine(i < foldProps.Count - 1 ? " ||" : ";"); } } @@ -1361,6 +1363,15 @@ private readonly record struct ForwarderParamInfo( string DefaultLiteral, bool IsParams); + // A property/parameter name as a valid C# identifier in emitted code. ISymbol.Name strips the + // leading '@' from a verbatim identifier (a property declared `@event` has Name "event"), so a + // reserved keyword must be re-escaped with '@' wherever it is emitted as an identifier — + // otherwise the generated factory (`string? event = null`, `__c.event = event`) fails to + // compile in the consumer's build. Use this only for emitted identifiers; comparisons against + // metadata names (modelProperty, "Children", typed-delegate sets) keep the raw Name. + internal static string EscapeIdentifier(string name) => + SyntaxFacts.GetKeywordKind(name) != SyntaxKind.None ? "@" + name : name; + private readonly record struct PropInfo( string Name, string TypeFqn, @@ -1371,7 +1382,11 @@ private readonly record struct PropInfo( string DeclaringFilePath, int DeclaringSpanStart, int DeclaringSpanLength, - bool IsAutoRerenderDelegate); + bool IsAutoRerenderDelegate) + { + // The factory-parameter / property identifier, '@'-escaped when Name is a reserved keyword. + public string Escaped => EscapeIdentifier(Name); + } } internal readonly struct EquatableArray : IEquatable>, IEnumerable diff --git a/src/Rask.Generators/RoutesGenerator.cs b/src/Rask.Generators/RoutesGenerator.cs index 0f780e1c..76a16060 100644 --- a/src/Rask.Generators/RoutesGenerator.cs +++ b/src/Rask.Generators/RoutesGenerator.cs @@ -622,7 +622,14 @@ private static void Emit(SourceProductionContext spc, ImmutableArray spc.AddSource(hint, SourceText.From(sb.ToString(), Encoding.UTF8)); } - EmitRegistryInitializer(spc, filtered); + // Deduplicate by fully-qualified type name before emitting the registry. A `partial` + // routed page whose declarations carry attributes on more than one part (e.g. [Route] on + // one and [Obsolete]/a source-gen attribute on another) yields one Candidate per attributed + // declaration — all with the same FQN. Emitting them all produced duplicate + // RouteRegistration entries (competing Route nodes for the same page) and duplicate + // [DynamicDependency] attributes. byFqn already keeps the first Candidate per FQN (its + // Templates reflect every [Route] on the merged symbol), so the registry uses that. + EmitRegistryInitializer(spc, byFqn.Values.ToList()); } private static void EmitRegistryInitializer(SourceProductionContext spc, IReadOnlyList candidates) @@ -905,13 +912,19 @@ private static void EmitFactoryBody(StringBuilder sb, Candidate c, List