Harden app security controls from audit
This commit is contained in:
@@ -2,6 +2,7 @@ using GameList.Data;
|
||||
using GameList.Contracts;
|
||||
using Microsoft.AspNetCore.Mvc;
|
||||
using GameList.Infrastructure;
|
||||
using Microsoft.AspNetCore.RateLimiting;
|
||||
|
||||
namespace GameList.Endpoints;
|
||||
|
||||
@@ -9,7 +10,7 @@ public static class AdminEndpoints
|
||||
{
|
||||
public static void MapAdminEndpoints(this IEndpointRouteBuilder app)
|
||||
{
|
||||
var admin = app.MapGroup("/api/admin").RequireAuthorization().AddEndpointFilter<AdminOnlyFilter>();
|
||||
var admin = app.MapGroup("/api/admin").RequireAuthorization().RequireRateLimiting("admin-sensitive").AddEndpointFilter<AdminOnlyFilter>();
|
||||
|
||||
admin.MapPost("/results", async ([FromBody] ResultsOpenRequest request, AdminWorkflowService service) => await service.SetResultsOpenAsync(request.ResultsOpen));
|
||||
|
||||
@@ -25,7 +26,7 @@ public static class AdminEndpoints
|
||||
if (player is null)
|
||||
return EndpointHelpers.UnauthorizedError();
|
||||
|
||||
return await service.DeletePlayerAsync(playerId, player.Id, request.Password);
|
||||
return await service.DeletePlayerAsync(playerId, player.Id, request.Password, ctx);
|
||||
});
|
||||
|
||||
admin.MapPost("/link-suggestions", async ([FromBody] LinkSuggestionsRequest request, HttpContext ctx, AppDbContext db, AdminWorkflowService service) =>
|
||||
@@ -52,7 +53,7 @@ public static class AdminEndpoints
|
||||
if (player is null)
|
||||
return EndpointHelpers.UnauthorizedError();
|
||||
|
||||
return await service.ResetAsync(player.Id, request.Password);
|
||||
return await service.ResetAsync(player.Id, request.Password, ctx);
|
||||
});
|
||||
|
||||
admin.MapPost("/factory-reset", async ([FromBody] AdminPasswordRequest request, HttpContext ctx, AppDbContext db, AdminWorkflowService service) =>
|
||||
@@ -61,7 +62,7 @@ public static class AdminEndpoints
|
||||
if (player is null)
|
||||
return EndpointHelpers.UnauthorizedError();
|
||||
|
||||
return await service.FactoryResetAsync(player.Id, request.Password);
|
||||
return await service.FactoryResetAsync(player.Id, request.Password, ctx);
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
@@ -87,9 +87,9 @@ internal sealed class AdminWorkflowService(AppDbContext db)
|
||||
return Results.Ok(new AdminSetPlayerPhaseResponse(player.Id, player.CurrentPhase, player.VotesFinal));
|
||||
}
|
||||
|
||||
public async Task<IResult> DeletePlayerAsync(Guid playerId, Guid adminPlayerId, string password)
|
||||
public async Task<IResult> DeletePlayerAsync(Guid playerId, Guid adminPlayerId, string password, HttpContext ctx)
|
||||
{
|
||||
var passwordError = await ValidateAdminPasswordAsync(adminPlayerId, password);
|
||||
var passwordError = await ValidateAdminPasswordAsync(adminPlayerId, password, ctx);
|
||||
if (passwordError is not null)
|
||||
return passwordError;
|
||||
|
||||
@@ -208,9 +208,9 @@ internal sealed class AdminWorkflowService(AppDbContext db)
|
||||
return Results.Ok(new AdminUnlinkSuggestionsResponse(groupIds, await db.Players.CountAsync()));
|
||||
}
|
||||
|
||||
public async Task<IResult> ResetAsync(Guid adminPlayerId, string password)
|
||||
public async Task<IResult> ResetAsync(Guid adminPlayerId, string password, HttpContext ctx)
|
||||
{
|
||||
var passwordError = await ValidateAdminPasswordAsync(adminPlayerId, password);
|
||||
var passwordError = await ValidateAdminPasswordAsync(adminPlayerId, password, ctx);
|
||||
if (passwordError is not null)
|
||||
return passwordError;
|
||||
|
||||
@@ -229,9 +229,9 @@ internal sealed class AdminWorkflowService(AppDbContext db)
|
||||
return Results.Ok(new AdminResetStateResponse(Phase.Suggest, state.ResultsOpen, state.UpdatedAt));
|
||||
}
|
||||
|
||||
public async Task<IResult> FactoryResetAsync(Guid adminPlayerId, string password)
|
||||
public async Task<IResult> FactoryResetAsync(Guid adminPlayerId, string password, HttpContext ctx)
|
||||
{
|
||||
var passwordError = await ValidateAdminPasswordAsync(adminPlayerId, password);
|
||||
var passwordError = await ValidateAdminPasswordAsync(adminPlayerId, password, ctx);
|
||||
if (passwordError is not null)
|
||||
return passwordError;
|
||||
|
||||
@@ -251,7 +251,7 @@ internal sealed class AdminWorkflowService(AppDbContext db)
|
||||
return Results.Ok(new AdminResetStateResponse(Phase.Suggest, fresh.ResultsOpen, fresh.UpdatedAt));
|
||||
}
|
||||
|
||||
private async Task<IResult?> ValidateAdminPasswordAsync(Guid adminPlayerId, string password)
|
||||
private async Task<IResult?> ValidateAdminPasswordAsync(Guid adminPlayerId, string password, HttpContext ctx)
|
||||
{
|
||||
if (string.IsNullOrWhiteSpace(password))
|
||||
return EndpointHelpers.BadRequestError("Admin password is required.");
|
||||
@@ -260,8 +260,15 @@ internal sealed class AdminWorkflowService(AppDbContext db)
|
||||
if (admin is null)
|
||||
return EndpointHelpers.UnauthorizedError();
|
||||
|
||||
return PasswordHasher.Verify(password, admin.PasswordHash, admin.PasswordSalt)
|
||||
? null
|
||||
: EndpointHelpers.BadRequestError("Invalid admin password.");
|
||||
var monitor = ctx.RequestServices.GetRequiredService<AuthAttemptMonitor>();
|
||||
var verified = PasswordHasher.Verify(password, admin.PasswordHash, admin.PasswordSalt);
|
||||
if (!verified)
|
||||
{
|
||||
monitor.RecordFailure(ctx, "admin-password", admin.NormalizedUsername, "invalid-password");
|
||||
return EndpointHelpers.BadRequestError("Invalid admin password.");
|
||||
}
|
||||
|
||||
monitor.RecordSuccess(ctx, "admin-password", admin.NormalizedUsername);
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -3,6 +3,7 @@ using GameList.Data;
|
||||
using GameList.Domain;
|
||||
using GameList.Infrastructure;
|
||||
using Microsoft.AspNetCore.Mvc;
|
||||
using Microsoft.AspNetCore.RateLimiting;
|
||||
using Microsoft.EntityFrameworkCore;
|
||||
|
||||
namespace GameList.Endpoints;
|
||||
@@ -11,12 +12,15 @@ public static class AuthEndpoints
|
||||
{
|
||||
public static void MapAuthEndpoints(this IEndpointRouteBuilder app)
|
||||
{
|
||||
var group = app.MapGroup("/api/auth");
|
||||
var group = app.MapGroup("/api/auth").RequireRateLimiting("auth-sensitive");
|
||||
|
||||
group.MapPost("/register", async ([FromBody] RegisterRequest request, HttpContext ctx, AppDbContext db, IConfiguration config) =>
|
||||
group.MapPost("/register", async ([FromBody] RegisterRequest request, HttpContext ctx, AppDbContext db, IConfiguration config, AuthAttemptMonitor authAttemptMonitor) =>
|
||||
{
|
||||
if (!AuthValidator.TryValidateRegistration(request, out var validated, out var registrationError))
|
||||
{
|
||||
authAttemptMonitor.RecordFailure(ctx, "auth-register", request.Username?.Trim() ?? "unknown", "validation-failed");
|
||||
return EndpointHelpers.BadRequestError(registrationError);
|
||||
}
|
||||
|
||||
var exists = await db.Players.AnyAsync(p => p.NormalizedUsername == validated.NormalizedUsername);
|
||||
if (exists)
|
||||
@@ -28,7 +32,17 @@ public static class AuthEndpoints
|
||||
if (wantsAdmin)
|
||||
{
|
||||
if (string.IsNullOrWhiteSpace(expectedAdminKey) || validated.AdminKey != expectedAdminKey)
|
||||
{
|
||||
authAttemptMonitor.RecordFailure(ctx, "auth-register-admin", validated.NormalizedUsername, "invalid-admin-key");
|
||||
return EndpointHelpers.BadRequestError("Invalid admin key.");
|
||||
}
|
||||
|
||||
var adminExists = await db.Players.AnyAsync(p => p.IsAdmin);
|
||||
if (adminExists)
|
||||
{
|
||||
authAttemptMonitor.RecordFailure(ctx, "auth-register-admin", validated.NormalizedUsername, "bootstrap-admin-disabled");
|
||||
return EndpointHelpers.BadRequestError("Admin registration via admin key is disabled after the first admin account.");
|
||||
}
|
||||
}
|
||||
|
||||
var isAdmin = wantsAdmin;
|
||||
@@ -49,6 +63,9 @@ public static class AuthEndpoints
|
||||
db.Players.Add(player);
|
||||
await db.SaveChangesAsync();
|
||||
|
||||
if (isAdmin)
|
||||
authAttemptMonitor.RecordSuccess(ctx, "auth-register-admin", validated.NormalizedUsername);
|
||||
|
||||
await PlayerIdentityExtensions.SignInPlayerAsync(ctx, player);
|
||||
|
||||
return Results.Ok(new AuthSessionResponse(
|
||||
@@ -59,14 +76,20 @@ public static class AuthEndpoints
|
||||
));
|
||||
});
|
||||
|
||||
group.MapPost("/login", async ([FromBody] LoginRequest request, HttpContext ctx, AppDbContext db) =>
|
||||
group.MapPost("/login", async ([FromBody] LoginRequest request, HttpContext ctx, AppDbContext db, AuthAttemptMonitor authAttemptMonitor) =>
|
||||
{
|
||||
if (!AuthValidator.TryValidateLogin(request, out _, out var normalizedUsername, out var loginError))
|
||||
{
|
||||
authAttemptMonitor.RecordFailure(ctx, "auth-login", request.Username?.Trim() ?? "unknown", "validation-failed");
|
||||
return EndpointHelpers.BadRequestError(loginError);
|
||||
}
|
||||
|
||||
var player = await db.Players.FirstOrDefaultAsync(p => p.NormalizedUsername == normalizedUsername);
|
||||
if (player == null || !PasswordHasher.Verify(request.Password, player.PasswordHash, player.PasswordSalt))
|
||||
{
|
||||
authAttemptMonitor.RecordFailure(ctx, "auth-login", normalizedUsername, "invalid-credentials");
|
||||
return EndpointHelpers.UnauthorizedError("Invalid username or password.");
|
||||
}
|
||||
|
||||
if (string.IsNullOrWhiteSpace(player.DisplayName))
|
||||
{
|
||||
@@ -76,6 +99,7 @@ public static class AuthEndpoints
|
||||
player.LastLoginAt = DateTimeOffset.UtcNow;
|
||||
await db.SaveChangesAsync();
|
||||
|
||||
authAttemptMonitor.RecordSuccess(ctx, "auth-login", normalizedUsername);
|
||||
await PlayerIdentityExtensions.SignInPlayerAsync(ctx, player);
|
||||
|
||||
return Results.Ok(new AuthSessionResponse(
|
||||
|
||||
@@ -7,6 +7,8 @@ internal static class AuthValidator
|
||||
public const int MaxUsernameLength = 24;
|
||||
public const int MaxDisplayNameLength = 16;
|
||||
public const int MaxAdminKeyLength = 128;
|
||||
public const int MinPasswordLength = 8;
|
||||
public const int MaxPasswordLength = 128;
|
||||
|
||||
public static bool TryValidateRegistration(RegisterRequest request, out ValidatedRegistration validated, out string error)
|
||||
{
|
||||
@@ -25,6 +27,25 @@ internal static class AuthValidator
|
||||
return false;
|
||||
}
|
||||
|
||||
var password = request.Password.Trim();
|
||||
if (password.Length < MinPasswordLength || password.Length > MaxPasswordLength)
|
||||
{
|
||||
validated = default;
|
||||
error = $"Password must be between {MinPasswordLength} and {MaxPasswordLength} characters.";
|
||||
return false;
|
||||
}
|
||||
|
||||
var hasUpper = password.Any(char.IsUpper);
|
||||
var hasLower = password.Any(char.IsLower);
|
||||
var hasDigit = password.Any(char.IsDigit);
|
||||
var hasSymbol = password.Any(ch => !char.IsLetterOrDigit(ch));
|
||||
if (!hasUpper || !hasLower || !hasDigit || !hasSymbol)
|
||||
{
|
||||
validated = default;
|
||||
error = "Password must include uppercase, lowercase, number, and symbol.";
|
||||
return false;
|
||||
}
|
||||
|
||||
if ((request.DisplayName ?? string.Empty).Trim().Length > MaxDisplayNameLength)
|
||||
{
|
||||
validated = default;
|
||||
@@ -63,6 +84,12 @@ internal static class AuthValidator
|
||||
return false;
|
||||
}
|
||||
|
||||
if (request.Password.Length > MaxPasswordLength)
|
||||
{
|
||||
error = $"Password must be <= {MaxPasswordLength} characters.";
|
||||
return false;
|
||||
}
|
||||
|
||||
normalizedUsername = username.ToLowerInvariant();
|
||||
error = string.Empty;
|
||||
return true;
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
using GameList.Data;
|
||||
using GameList.Domain;
|
||||
using Microsoft.EntityFrameworkCore;
|
||||
using System.Net;
|
||||
using System.Net.Sockets;
|
||||
using System.Security.Claims;
|
||||
|
||||
namespace GameList.Endpoints;
|
||||
@@ -140,6 +142,36 @@ internal static class EndpointHelpers
|
||||
|| path.EndsWith(".avif", StringComparison.Ordinal);
|
||||
}
|
||||
|
||||
public static HttpMessageHandler CreateImageValidationHandler()
|
||||
{
|
||||
return new SocketsHttpHandler
|
||||
{
|
||||
AllowAutoRedirect = false,
|
||||
ConnectCallback = async (context, cancellationToken) =>
|
||||
{
|
||||
var addresses = await ResolveSafePublicAddressesAsync(context.DnsEndPoint.Host, cancellationToken);
|
||||
if (addresses.Count == 0)
|
||||
throw new HttpRequestException("No safe public IPs found for host.");
|
||||
|
||||
foreach (var ip in addresses)
|
||||
{
|
||||
var socket = new Socket(ip.AddressFamily, SocketType.Stream, ProtocolType.Tcp);
|
||||
try
|
||||
{
|
||||
await socket.ConnectAsync(new IPEndPoint(ip, context.DnsEndPoint.Port), cancellationToken);
|
||||
return new NetworkStream(socket, ownsSocket: true);
|
||||
}
|
||||
catch
|
||||
{
|
||||
socket.Dispose();
|
||||
}
|
||||
}
|
||||
|
||||
throw new HttpRequestException("Unable to connect to validated public IP for host.");
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
public static async Task<bool> IsReachableImageAsync(string? url, IHttpClientFactory httpFactory, HttpMessageHandler? handler = null, CancellationToken ct = default)
|
||||
{
|
||||
if (string.IsNullOrWhiteSpace(url))
|
||||
@@ -148,13 +180,21 @@ internal static class EndpointHelpers
|
||||
return false;
|
||||
if (uri.Scheme is not ("http" or "https"))
|
||||
return false;
|
||||
if (!await IsSafePublicHostAsync(uri, ct))
|
||||
if (handler is null)
|
||||
{
|
||||
if (!await IsSafePublicHostAsync(uri, ct))
|
||||
return false;
|
||||
}
|
||||
else if (IPAddress.TryParse(uri.Host, out var literal) && IsBlockedAddress(literal))
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
using var cts = CancellationTokenSource.CreateLinkedTokenSource(ct);
|
||||
cts.CancelAfter(TimeSpan.FromSeconds(3));
|
||||
|
||||
var client = handler is null ? httpFactory.CreateClient("imageValidation") : new HttpClient(handler, disposeHandler: false);
|
||||
using var fallbackClient = handler is null ? null : new HttpClient(handler, disposeHandler: false);
|
||||
var client = fallbackClient ?? httpFactory.CreateClient("imageValidation");
|
||||
|
||||
try
|
||||
{
|
||||
@@ -234,24 +274,8 @@ internal static class EndpointHelpers
|
||||
{
|
||||
try
|
||||
{
|
||||
var host = uri.Host;
|
||||
if (Uri.CheckHostName(host) == UriHostNameType.Dns || Uri.CheckHostName(host) == UriHostNameType.IPv4 || Uri.CheckHostName(host) == UriHostNameType.IPv6)
|
||||
{
|
||||
var addresses = await System.Net.Dns.GetHostAddressesAsync(host, ct);
|
||||
foreach (var ip in addresses)
|
||||
{
|
||||
if (System.Net.IPAddress.IsLoopback(ip))
|
||||
return false;
|
||||
if (IsPrivate(ip))
|
||||
return false;
|
||||
}
|
||||
}
|
||||
else
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
return true;
|
||||
var addresses = await ResolveSafePublicAddressesAsync(uri.Host, ct);
|
||||
return addresses.Count > 0;
|
||||
}
|
||||
catch
|
||||
{
|
||||
@@ -259,26 +283,90 @@ internal static class EndpointHelpers
|
||||
}
|
||||
}
|
||||
|
||||
private static bool IsPrivate(System.Net.IPAddress ip)
|
||||
private static async Task<IReadOnlyList<IPAddress>> ResolveSafePublicAddressesAsync(string host, CancellationToken ct)
|
||||
{
|
||||
if (ip.AddressFamily == System.Net.Sockets.AddressFamily.InterNetwork)
|
||||
if (!IsSupportedHostType(host))
|
||||
return [];
|
||||
|
||||
IPAddress[] resolved;
|
||||
if (IPAddress.TryParse(host, out var literal))
|
||||
{
|
||||
var bytes = ip.GetAddressBytes();
|
||||
return bytes[0] switch
|
||||
{
|
||||
10 => true,
|
||||
172 when bytes[1] >= 16 && bytes[1] <= 31 => true,
|
||||
192 when bytes[1] == 168 => true,
|
||||
127 => true,
|
||||
_ => false
|
||||
};
|
||||
resolved = [literal];
|
||||
}
|
||||
else
|
||||
{
|
||||
resolved = await Dns.GetHostAddressesAsync(host, ct);
|
||||
}
|
||||
|
||||
if (ip.AddressFamily == System.Net.Sockets.AddressFamily.InterNetworkV6)
|
||||
var safe = new List<IPAddress>(resolved.Length);
|
||||
foreach (var ip in resolved)
|
||||
{
|
||||
return ip.IsIPv6LinkLocal || ip.IsIPv6SiteLocal || ip.IsIPv6Multicast || System.Net.IPAddress.IsLoopback(ip);
|
||||
if (!IsBlockedAddress(ip))
|
||||
safe.Add(ip);
|
||||
}
|
||||
|
||||
return safe.Distinct().ToArray();
|
||||
}
|
||||
|
||||
private static bool IsSupportedHostType(string host)
|
||||
{
|
||||
var type = Uri.CheckHostName(host);
|
||||
return type is UriHostNameType.Dns or UriHostNameType.IPv4 or UriHostNameType.IPv6;
|
||||
}
|
||||
|
||||
private static bool IsBlockedAddress(IPAddress ip)
|
||||
{
|
||||
if (IPAddress.IsLoopback(ip))
|
||||
return true;
|
||||
|
||||
if (ip.IsIPv4MappedToIPv6)
|
||||
return IsBlockedAddress(ip.MapToIPv4());
|
||||
|
||||
if (ip.AddressFamily == AddressFamily.InterNetwork)
|
||||
return IsBlockedIpv4(ip);
|
||||
|
||||
if (ip.AddressFamily == AddressFamily.InterNetworkV6)
|
||||
return IsBlockedIpv6(ip);
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
private static bool IsBlockedIpv4(IPAddress ip)
|
||||
{
|
||||
var b = ip.GetAddressBytes();
|
||||
return b[0] switch
|
||||
{
|
||||
0 => true, // "This network"
|
||||
10 => true, // private
|
||||
100 when b[1] >= 64 && b[1] <= 127 => true, // CGNAT
|
||||
127 => true, // loopback
|
||||
169 when b[1] == 254 => true, // link local
|
||||
172 when b[1] >= 16 && b[1] <= 31 => true, // private
|
||||
192 when b[1] == 0 && b[2] == 0 => true, // IETF protocol assignments
|
||||
192 when b[1] == 0 && b[2] == 2 => true, // documentation
|
||||
192 when b[1] == 88 && b[2] == 99 => true, // 6to4 relay anycast
|
||||
192 when b[1] == 168 => true, // private
|
||||
198 when b[1] is 18 or 19 => true, // benchmarking
|
||||
198 when b[1] == 51 && b[2] == 100 => true, // documentation
|
||||
203 when b[1] == 0 && b[2] == 113 => true, // documentation
|
||||
>= 224 => true, // multicast/reserved/broadcast
|
||||
_ => false
|
||||
};
|
||||
}
|
||||
|
||||
private static bool IsBlockedIpv6(IPAddress ip)
|
||||
{
|
||||
if (ip.Equals(IPAddress.IPv6None))
|
||||
return true;
|
||||
if (ip.IsIPv6Multicast || ip.IsIPv6LinkLocal || ip.IsIPv6SiteLocal)
|
||||
return true;
|
||||
|
||||
var bytes = ip.GetAddressBytes();
|
||||
if ((bytes[0] & 0xFE) == 0xFC) // fc00::/7 unique local
|
||||
return true;
|
||||
if (bytes[0] == 0x20 && bytes[1] == 0x01 && bytes[2] == 0x0D && bytes[3] == 0xB8) // 2001:db8::/32 docs
|
||||
return true;
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user