security(#197): constant-time API-key compare, clamp playout paging, baseline security headers
Build ErsatzTV Image / Docs update reminder (pull_request) Successful in 7s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 5m15s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 6m15s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Build ErsatzTV Image / Docs update reminder (push) Has been skipped
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 8m13s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 9m43s
Build ErsatzTV Image / Build & push image (amd64) (push) Has been cancelled
Build ErsatzTV Image / Docs update reminder (pull_request) Successful in 7s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 5m15s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 6m15s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Build ErsatzTV Image / Docs update reminder (push) Has been skipped
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 8m13s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 9m43s
Build ErsatzTV Image / Build & push image (amd64) (push) Has been cancelled
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) <noreply@anthropic.com>
This commit was merged in pull request #279.
This commit is contained in:
@@ -599,6 +599,34 @@ public class PlayoutControllerTests
|
||||
Arg.Any<CancellationToken>());
|
||||
}
|
||||
|
||||
[Test]
|
||||
public async Task GetAll_Should_Clamp_PageSize_And_PageNum_Before_Query()
|
||||
{
|
||||
_mediator.Send(Arg.Any<GetPagedPlayouts>(), Arg.Any<CancellationToken>())
|
||||
.Returns(new PagedPlayoutsViewModel(0, []));
|
||||
|
||||
await _controller.GetAll("q", -5, 100_000_000, CancellationToken.None);
|
||||
|
||||
await _mediator.Received(1).Send(
|
||||
Arg.Is<GetPagedPlayouts>(q => q.PageNum == 0 && q.PageSize == 100),
|
||||
Arg.Any<CancellationToken>());
|
||||
}
|
||||
|
||||
[Test]
|
||||
public async Task GetItems_Should_Clamp_PageSize_And_PageNum_Before_Query()
|
||||
{
|
||||
_mediator.Send(Arg.Any<GetPlayoutById>(), Arg.Any<CancellationToken>())
|
||||
.Returns(Option<PlayoutNameViewModel>.Some(MakePlayout(9)));
|
||||
_mediator.Send(Arg.Any<GetFuturePlayoutItemsById>(), Arg.Any<CancellationToken>())
|
||||
.Returns(new PagedPlayoutItemsViewModel(0, []));
|
||||
|
||||
await _controller.GetItems(9, false, -5, 100_000_000, CancellationToken.None);
|
||||
|
||||
await _mediator.Received(1).Send(
|
||||
Arg.Is<GetFuturePlayoutItemsById>(q => q.PageNum == 0 && q.PageSize == 100),
|
||||
Arg.Any<CancellationToken>());
|
||||
}
|
||||
|
||||
[Test]
|
||||
public async Task GetAll_Should_Emit_Null_BuildStatus_When_Absent()
|
||||
{
|
||||
|
||||
@@ -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");
|
||||
}
|
||||
}
|
||||
@@ -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);
|
||||
|
||||
@@ -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<SkipApiKeyAuthorizationAttribute>().Any()
|
||||
|
||||
@@ -0,0 +1,23 @@
|
||||
using Microsoft.AspNetCore.Http;
|
||||
|
||||
namespace ErsatzTV.Middleware;
|
||||
|
||||
/// <summary>
|
||||
/// 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).
|
||||
/// <c>nosniff</c> 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.
|
||||
/// </summary>
|
||||
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);
|
||||
}
|
||||
}
|
||||
@@ -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<SecurityHeadersMiddleware>();
|
||||
|
||||
app.UseCors("AllowAll");
|
||||
app.UseForwardedHeaders();
|
||||
|
||||
|
||||
Reference in New Issue
Block a user