From c55a7fda36b40aece3193fc2da42ed90aefdb317 Mon Sep 17 00:00:00 2001 From: Timothy Date: Sat, 11 Jul 2026 23:17:21 +0200 Subject: [PATCH] security(#197): constant-time API-key compare, clamp playout paging, baseline security headers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Posture-independent safe hardening from the #197 cold API security review (the clear-cut fixes that don't depend on the fail-closed/CORS/versioning posture design, which is tracked separately): - ApiKeyAuthorizationFilter: compare X-Api-Key with CryptographicOperations.FixedTimeEquals instead of ordinal string.Equals (removes the response-timing oracle on the write key). [S10] - PlayoutController: clamp pageNum/pageSize on GET /api/playouts and /api/playouts/{id}/items to Math.Clamp(_, 1, 100), matching the documented api-conventions ยง1 convention every other paged endpoint already follows โ€” these two were passing the raw value straight to EF Take(). [S8] - SecurityHeadersMiddleware: emit X-Content-Type-Options: nosniff, X-Frame-Options: DENY, Referrer-Policy: strict-origin-when-cross-origin on every response (nosniff backstops the artwork content-type MIME-sniffing risk). CSP/HSTS deferred to the #197 posture design (CSP needs SPA validation; HSTS is proxy/TLS-owned). [S10] Refs #197. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../Controllers/PlayoutControllerTests.cs | 28 ++++++++++++++++++ .../SecurityHeadersMiddlewareTests.cs | 29 +++++++++++++++++++ ErsatzTV/Controllers/Api/PlayoutController.cs | 4 +++ ErsatzTV/Filters/ApiKeyAuthorizationFilter.cs | 12 +++++++- .../Middleware/SecurityHeadersMiddleware.cs | 23 +++++++++++++++ ErsatzTV/Startup.cs | 3 ++ 6 files changed, 98 insertions(+), 1 deletion(-) create mode 100644 ErsatzTV.Tests/Middleware/SecurityHeadersMiddlewareTests.cs create mode 100644 ErsatzTV/Middleware/SecurityHeadersMiddleware.cs diff --git a/ErsatzTV.Tests/Controllers/PlayoutControllerTests.cs b/ErsatzTV.Tests/Controllers/PlayoutControllerTests.cs index 7eb4e769d..a56964a9c 100644 --- a/ErsatzTV.Tests/Controllers/PlayoutControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/PlayoutControllerTests.cs @@ -599,6 +599,34 @@ public class PlayoutControllerTests Arg.Any()); } + [Test] + public async Task GetAll_Should_Clamp_PageSize_And_PageNum_Before_Query() + { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(new PagedPlayoutsViewModel(0, [])); + + await _controller.GetAll("q", -5, 100_000_000, CancellationToken.None); + + await _mediator.Received(1).Send( + Arg.Is(q => q.PageNum == 0 && q.PageSize == 100), + Arg.Any()); + } + + [Test] + public async Task GetItems_Should_Clamp_PageSize_And_PageNum_Before_Query() + { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(MakePlayout(9))); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(new PagedPlayoutItemsViewModel(0, [])); + + await _controller.GetItems(9, false, -5, 100_000_000, CancellationToken.None); + + await _mediator.Received(1).Send( + Arg.Is(q => q.PageNum == 0 && q.PageSize == 100), + Arg.Any()); + } + [Test] public async Task GetAll_Should_Emit_Null_BuildStatus_When_Absent() { diff --git a/ErsatzTV.Tests/Middleware/SecurityHeadersMiddlewareTests.cs b/ErsatzTV.Tests/Middleware/SecurityHeadersMiddlewareTests.cs new file mode 100644 index 000000000..0f2d40452 --- /dev/null +++ b/ErsatzTV.Tests/Middleware/SecurityHeadersMiddlewareTests.cs @@ -0,0 +1,29 @@ +using ErsatzTV.Middleware; +using Microsoft.AspNetCore.Http; +using NUnit.Framework; +using Shouldly; + +namespace ErsatzTV.Tests.Middleware; + +[TestFixture] +public class SecurityHeadersMiddlewareTests +{ + [Test] + public async Task Should_Set_Baseline_Security_Headers() + { + var nextCalled = false; + var middleware = new SecurityHeadersMiddleware(_ => + { + nextCalled = true; + return Task.CompletedTask; + }); + var context = new DefaultHttpContext(); + + await middleware.InvokeAsync(context); + + nextCalled.ShouldBeTrue(); + context.Response.Headers["X-Content-Type-Options"].ToString().ShouldBe("nosniff"); + context.Response.Headers["X-Frame-Options"].ToString().ShouldBe("DENY"); + context.Response.Headers["Referrer-Policy"].ToString().ShouldBe("strict-origin-when-cross-origin"); + } +} diff --git a/ErsatzTV/Controllers/Api/PlayoutController.cs b/ErsatzTV/Controllers/Api/PlayoutController.cs index 42eb40043..1a00436fd 100644 --- a/ErsatzTV/Controllers/Api/PlayoutController.cs +++ b/ErsatzTV/Controllers/Api/PlayoutController.cs @@ -45,6 +45,8 @@ public class PlayoutController(IMediator mediator, IEntityLocker entityLocker) : [FromQuery] int pageSize = 100, CancellationToken cancellationToken = default) { + pageNum = Math.Max(0, pageNum); + pageSize = Math.Clamp(pageSize, 1, MaxPageSize); PagedPlayoutsViewModel result = await mediator.Send(new GetPagedPlayouts(query, pageNum, pageSize), cancellationToken); return new PagedPlayoutsResponseModel( @@ -91,6 +93,8 @@ public class PlayoutController(IMediator mediator, IEntityLocker entityLocker) : return ApiResults.NotFoundProblem(); } + pageNum = Math.Max(0, pageNum); + pageSize = Math.Clamp(pageSize, 1, MaxPageSize); PagedPlayoutItemsViewModel result = await mediator.Send( new GetFuturePlayoutItemsById(id, showFiller, pageNum, pageSize), cancellationToken); diff --git a/ErsatzTV/Filters/ApiKeyAuthorizationFilter.cs b/ErsatzTV/Filters/ApiKeyAuthorizationFilter.cs index 8c0643d5c..821a4fe08 100644 --- a/ErsatzTV/Filters/ApiKeyAuthorizationFilter.cs +++ b/ErsatzTV/Filters/ApiKeyAuthorizationFilter.cs @@ -1,3 +1,5 @@ +using System.Security.Cryptography; +using System.Text; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Filters; @@ -45,7 +47,7 @@ public class ApiKeyAuthorizationFilter(IConfiguration configuration) : IAuthoriz } if (!context.HttpContext.Request.Headers.TryGetValue(HeaderName, out StringValues provided) - || !string.Equals(provided.ToString(), configuredKey, StringComparison.Ordinal)) + || !KeysMatch(provided.ToString(), configuredKey)) { context.Result = new UnauthorizedObjectResult(new ProblemDetails { @@ -56,6 +58,14 @@ public class ApiKeyAuthorizationFilter(IConfiguration configuration) : IAuthoriz } } + // Compare in constant time so a remote attacker cannot use response-timing to recover the + // key prefix-by-prefix. FixedTimeEquals also short-circuits length differences without + // leaking anything beyond "lengths differ" (still not the matching-prefix length). + private static bool KeysMatch(string provided, string configured) => + CryptographicOperations.FixedTimeEquals( + Encoding.UTF8.GetBytes(provided), + Encoding.UTF8.GetBytes(configured)); + private static bool ShouldSkipApiKeyAuthorization(AuthorizationFilterContext context) { if (context.Filters.OfType().Any() diff --git a/ErsatzTV/Middleware/SecurityHeadersMiddleware.cs b/ErsatzTV/Middleware/SecurityHeadersMiddleware.cs new file mode 100644 index 000000000..3ce81b0ba --- /dev/null +++ b/ErsatzTV/Middleware/SecurityHeadersMiddleware.cs @@ -0,0 +1,23 @@ +using Microsoft.AspNetCore.Http; + +namespace ErsatzTV.Middleware; + +/// +/// Adds baseline security response headers to every response (API, IPTV, artwork, static, and +/// error responses alike โ€” which is why this is middleware rather than an MVC filter). +/// nosniff in particular blunts the MIME-sniffing half of the artwork content-type +/// reflection risk (issue #197). CSP and HSTS are intentionally left out here: CSP needs to be +/// validated against the ChicoryTV SPA's inline assets, and HSTS is a deployment/TLS decision โ€” +/// both are part of the #197 posture design, not this baseline. +/// +public class SecurityHeadersMiddleware(RequestDelegate next) +{ + public Task InvokeAsync(HttpContext context) + { + IHeaderDictionary headers = context.Response.Headers; + headers["X-Content-Type-Options"] = "nosniff"; + headers["X-Frame-Options"] = "DENY"; + headers["Referrer-Policy"] = "strict-origin-when-cross-origin"; + return next(context); + } +} diff --git a/ErsatzTV/Startup.cs b/ErsatzTV/Startup.cs index 47bf89ba8..e2053ac93 100644 --- a/ErsatzTV/Startup.cs +++ b/ErsatzTV/Startup.cs @@ -73,6 +73,7 @@ using ErsatzTV.Infrastructure.Sqlite.Data; using ErsatzTV.Infrastructure.Streaming; using ErsatzTV.Infrastructure.Streaming.Graphics; using ErsatzTV.Infrastructure.Trakt; +using ErsatzTV.Middleware; using ErsatzTV.Serialization; using ErsatzTV.Services; using ErsatzTV.Services.RunOnce; @@ -567,6 +568,8 @@ public class Startup } } + app.UseMiddleware(); + app.UseCors("AllowAll"); app.UseForwardedHeaders();