diff --git a/docker-compose.yml b/docker-compose.yml index 217330c..3be41ba 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -4,11 +4,16 @@ services: environment: POSTGRES_DB: inboxintel POSTGRES_USER: inboxintel - POSTGRES_PASSWORD: ${POSTGRES_PASSWORD:-inboxintel} + # V-03: require an explicit strong password (fail fast if POSTGRES_PASSWORD is unset) + # rather than silently defaulting to a guessable one. + POSTGRES_PASSWORD: ${POSTGRES_PASSWORD:?set POSTGRES_PASSWORD in deploy/.env} volumes: - pgdata:/var/lib/postgresql/data + # V-03: bind to loopback only so the database is reachable from the host for local + # tooling but NOT from other machines on the network. The api container reaches it + # over the internal compose network regardless of this published port. ports: - - "5432:5432" + - "127.0.0.1:5432:5432" healthcheck: test: ["CMD-SHELL", "pg_isready -U inboxintel"] interval: 5s @@ -22,7 +27,7 @@ services: environment: ASPNETCORE_ENVIRONMENT: Production ASPNETCORE_URLS: http://+:8080 - ConnectionStrings__Postgres: "Host=postgres;Port=5432;Database=inboxintel;Username=inboxintel;Password=${POSTGRES_PASSWORD:-inboxintel}" + ConnectionStrings__Postgres: "Host=postgres;Port=5432;Database=inboxintel;Username=inboxintel;Password=${POSTGRES_PASSWORD:?set POSTGRES_PASSWORD in deploy/.env}" DataProtection__KeyPath: /keys GoogleOAuth__ClientId: ${GOOGLE_CLIENT_ID:-} GoogleOAuth__ClientSecret: ${GOOGLE_CLIENT_SECRET:-} @@ -37,8 +42,11 @@ services: depends_on: postgres: condition: service_healthy + # V-08: bind to loopback so the API is not directly reachable from the network + # (only via the frontend/nginx proxy over the internal compose network). This + # prevents external clients from bypassing the proxy to spoof X-Forwarded-* headers. ports: - - "8080:8080" + - "127.0.0.1:8080:8080" frontend: build: diff --git a/src/InboxIntel.Api/Controllers/AnalyticsController.cs b/src/InboxIntel.Api/Controllers/AnalyticsController.cs index 5487053..43563a4 100644 --- a/src/InboxIntel.Api/Controllers/AnalyticsController.cs +++ b/src/InboxIntel.Api/Controllers/AnalyticsController.cs @@ -16,11 +16,13 @@ public class AnalyticsController : ApiControllerBase [HttpGet("top-senders")] public async Task TopSenders([FromQuery] int take = 20, CancellationToken ct = default) - => Ok(await _analytics.GetTopSendersAsync(UserId, take, ct)); + // V-10: clamp to a sane bound (the SPA legitimately requests up to 5000 to list + // all senders) so a caller cannot force an unbounded scan. + => Ok(await _analytics.GetTopSendersAsync(UserId, Math.Clamp(take, 1, 5000), ct)); [HttpGet("volume")] public async Task Volume([FromQuery] int days = 90, CancellationToken ct = default) - => Ok(await _analytics.GetVolumeOverTimeAsync(UserId, days, ct)); + => Ok(await _analytics.GetVolumeOverTimeAsync(UserId, Math.Clamp(days, 1, 3660), ct)); [HttpGet("heatmap")] public async Task Heatmap(CancellationToken ct) => Ok(await _analytics.GetHeatmapAsync(UserId, ct)); diff --git a/src/InboxIntel.Api/Controllers/AppInfoController.cs b/src/InboxIntel.Api/Controllers/AppInfoController.cs index 8004246..8f03d6e 100644 --- a/src/InboxIntel.Api/Controllers/AppInfoController.cs +++ b/src/InboxIntel.Api/Controllers/AppInfoController.cs @@ -28,10 +28,17 @@ public class AppInfoController : ControllerBase public IActionResult Info() { var devMode = _config.GetValue("App:DevMode") ?? _env.IsDevelopment(); + // V-13: when NOT in dev mode, disclose nothing beyond the flag to anonymous + // callers. The dev banner (the only consumer of environment/maxMessages) only + // renders when devMode is true, so this preserves the feature without leaking + // the environment name or sync cap in production. + if (!devMode) + return Ok(new { devMode = false }); + return Ok(new { + devMode = true, environment = _env.EnvironmentName, - devMode, maxMessages = _config.GetValue("GmailSync:MaxMessages") }); } diff --git a/src/InboxIntel.Api/Controllers/AuthController.cs b/src/InboxIntel.Api/Controllers/AuthController.cs index 778f034..4e16fb6 100644 --- a/src/InboxIntel.Api/Controllers/AuthController.cs +++ b/src/InboxIntel.Api/Controllers/AuthController.cs @@ -17,7 +17,12 @@ public class AuthController : ControllerBase [HttpGet("login")] [AllowAnonymous] public IActionResult Login([FromQuery] string? returnUrl = "/") - => Challenge(new AuthenticationProperties { RedirectUri = returnUrl }, GoogleDefaults.AuthenticationScheme); + { + // V-09: only allow local post-login redirects; reject absolute/off-host targets + // so the OAuth flow can't be abused as an open redirect for phishing. + var safe = !string.IsNullOrEmpty(returnUrl) && Url.IsLocalUrl(returnUrl) ? returnUrl : "/app"; + return Challenge(new AuthenticationProperties { RedirectUri = safe }, GoogleDefaults.AuthenticationScheme); + } [HttpPost("logout")] [Authorize] diff --git a/src/InboxIntel.Api/Dockerfile b/src/InboxIntel.Api/Dockerfile index 65a7424..384d9e0 100644 --- a/src/InboxIntel.Api/Dockerfile +++ b/src/InboxIntel.Api/Dockerfile @@ -16,5 +16,13 @@ RUN dotnet publish src/InboxIntel.Api/InboxIntel.Api.csproj -c Release -o /app/p FROM mcr.microsoft.com/dotnet/aspnet:8.0 AS runtime WORKDIR /app COPY --from=build /app/publish . + +# V-12: run as the non-root 'app' user shipped in the .NET 8 images. Pre-create the +# DataProtection key directory owned by that user so the (initially empty) 'keys' +# volume inherits app ownership on first mount and key persistence still works. +# NOTE: an EXISTING root-owned keys volume must be recreated for this to take effect. +RUN mkdir -p /keys && chown -R app:app /keys /app +USER app + EXPOSE 8080 ENTRYPOINT ["dotnet", "InboxIntel.Api.dll"] diff --git a/src/InboxIntel.Api/Program.cs b/src/InboxIntel.Api/Program.cs index 5f90730..087e06d 100644 --- a/src/InboxIntel.Api/Program.cs +++ b/src/InboxIntel.Api/Program.cs @@ -48,6 +48,12 @@ builder.Services.AddAuthentication(options => { options.Cookie.HttpOnly = true; options.Cookie.SameSite = SameSiteMode.Lax; + // V-04: never emit the session cookie over plain HTTP in non-dev. Behind nginx + // the forwarded proto (now only trusted from known proxies, see below) makes + // Always work; local http://localhost dev still functions via SameAsRequest. + options.Cookie.SecurePolicy = builder.Environment.IsDevelopment() + ? CookieSecurePolicy.SameAsRequest + : CookieSecurePolicy.Always; options.Cookie.Name = "inboxintel.session"; options.ExpireTimeSpan = TimeSpan.FromDays(7); options.SlidingExpiration = true; @@ -94,6 +100,9 @@ builder.Services.AddApiVersioning(o => builder.Services.AddControllers(); builder.Services.AddEndpointsApiExplorer(); builder.Services.AddSwaggerGen(); +// V-06: RFC7807 ProblemDetails so the global exception handler returns a safe, +// generic error body instead of leaking framework stack traces / internal messages. +builder.Services.AddProblemDetails(); builder.Services.AddCors(o => o.AddPolicy("frontend", p => p .WithOrigins(builder.Configuration.GetSection("Cors:Origins").Get() ?? new[] { "http://localhost:5173" }) @@ -109,21 +118,53 @@ using (var scope = app.Services.CreateScope()) await db.Database.MigrateAsync(); } +// Honor X-Forwarded-* from the nginx reverse proxy so OAuth redirect URIs and +// cookie Secure flags reflect the external scheme/host, not the container's. +// V-08: only trust these headers from KNOWN proxy networks (configurable). The +// default covers private/Docker ranges so the compose nginx works, while a client +// reaching the API directly can no longer spoof scheme/host/forwarded-for. +var forwardedOptions = new ForwardedHeadersOptions +{ + ForwardedHeaders = ForwardedHeaders.XForwardedFor | ForwardedHeaders.XForwardedProto | ForwardedHeaders.XForwardedHost, + ForwardLimit = app.Configuration.GetValue("ForwardedHeaders:ForwardLimit") ?? 1 +}; +forwardedOptions.KnownNetworks.Clear(); +forwardedOptions.KnownProxies.Clear(); +var trustedNetworks = app.Configuration.GetSection("ForwardedHeaders:KnownNetworks").Get() + ?? new[] { "10.0.0.0/8", "172.16.0.0/12", "192.168.0.0/16", "127.0.0.0/8", "::1/128" }; +foreach (var cidr in trustedNetworks) +{ + var parts = cidr.Split('/'); + if (parts.Length == 2 && System.Net.IPAddress.TryParse(parts[0], out var prefix) && int.TryParse(parts[1], out var len)) + forwardedOptions.KnownNetworks.Add(new Microsoft.AspNetCore.HttpOverrides.IPNetwork(prefix, len)); +} +app.UseForwardedHeaders(forwardedOptions); + if (app.Environment.IsDevelopment()) { app.UseSwagger(); app.UseSwaggerUI(); } - -// Honor X-Forwarded-* from the nginx reverse proxy so OAuth redirect URIs and -// cookie Secure flags reflect the external scheme/host, not the container's. -var forwardedOptions = new ForwardedHeadersOptions +else { - ForwardedHeaders = ForwardedHeaders.XForwardedFor | ForwardedHeaders.XForwardedProto | ForwardedHeaders.XForwardedHost -}; -forwardedOptions.KnownNetworks.Clear(); -forwardedOptions.KnownProxies.Clear(); -app.UseForwardedHeaders(forwardedOptions); + // V-06: generic ProblemDetails for unhandled exceptions (no stack traces to clients). + app.UseExceptionHandler(); + // V-11: HSTS once TLS is enforced at the proxy (forwarded proto now trustworthy). + app.UseHsts(); +} + +// V-11: baseline security response headers. CSP is report-only for now so it can be +// tuned against the SPA before enforcing (the SPA itself is also served with headers +// by its nginx). Applied to API responses here as defense-in-depth. +app.Use(async (ctx, next) => +{ + var h = ctx.Response.Headers; + h["X-Content-Type-Options"] = "nosniff"; + h["X-Frame-Options"] = "DENY"; + h["Referrer-Policy"] = "no-referrer"; + h["Cross-Origin-Opener-Policy"] = "same-origin"; + await next(); +}); app.UseSerilogRequestLogging(); app.UseCors("frontend"); diff --git a/src/InboxIntel.Api/appsettings.json b/src/InboxIntel.Api/appsettings.json index 0ebdc56..7a049ac 100644 --- a/src/InboxIntel.Api/appsettings.json +++ b/src/InboxIntel.Api/appsettings.json @@ -51,6 +51,10 @@ "FrequencyDays": 7, "SendHourUtc": 8 }, + "ForwardedHeaders": { + "ForwardLimit": 1, + "KnownNetworks": [ "10.0.0.0/8", "172.16.0.0/12", "192.168.0.0/16", "127.0.0.0/8", "::1/128" ] + }, "Cors": { "Origins": [ "http://localhost:5173" ] }, diff --git a/src/InboxIntel.Infrastructure/Cleanup/CleanupService.cs b/src/InboxIntel.Infrastructure/Cleanup/CleanupService.cs index 7112cf8..508fb45 100644 --- a/src/InboxIntel.Infrastructure/Cleanup/CleanupService.cs +++ b/src/InboxIntel.Infrastructure/Cleanup/CleanupService.cs @@ -100,8 +100,9 @@ public class CleanupService : ICleanupService catch (Exception ex) { _logger.LogError(ex, "Cleanup action {Action} failed for user {UserId}", request.Action, userId); - errors.Add(ex.Message); - return Result.Failure(ex.Message); + // V-06: do not echo internal exception detail to the client. + errors.Add("The cleanup action could not be completed."); + return Result.Failure("The cleanup action could not be completed."); } } diff --git a/src/InboxIntel.Infrastructure/Cleanup/UnsubscribeService.cs b/src/InboxIntel.Infrastructure/Cleanup/UnsubscribeService.cs index fa01cb1..ed62e9e 100644 --- a/src/InboxIntel.Infrastructure/Cleanup/UnsubscribeService.cs +++ b/src/InboxIntel.Infrastructure/Cleanup/UnsubscribeService.cs @@ -5,6 +5,7 @@ using InboxIntel.Domain.Entities; using InboxIntel.Domain.Enums; using InboxIntel.Infrastructure.Gmail; using InboxIntel.Infrastructure.Persistence; +using InboxIntel.Infrastructure.Security; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Logging; @@ -120,13 +121,17 @@ public class UnsubscribeService : IUnsubscribeService switch (item.Method) { case UnsubscribeMethod.OneClickPost: - var post = await http.PostAsync(item.UnsubscribeTarget, + // V-01: the target comes from an attacker-authored email header. + // Validate against SSRF (scheme + private/metadata ranges) before fetching. + var postUri = await SafeHttpGuard.ValidateAsync(item.UnsubscribeTarget, ct); + var post = await http.PostAsync(postUri, new StringContent("List-Unsubscribe=One-Click"), ct); SetResult(item, post.IsSuccessStatusCode); if (post.IsSuccessStatusCode) ok++; else fail++; break; case UnsubscribeMethod.HttpLink: - var get = await http.GetAsync(item.UnsubscribeTarget, ct); + var getUri = await SafeHttpGuard.ValidateAsync(item.UnsubscribeTarget, ct); + var get = await http.GetAsync(getUri, ct); SetResult(item, get.IsSuccessStatusCode); if (get.IsSuccessStatusCode) ok++; else fail++; break; @@ -140,12 +145,24 @@ public class UnsubscribeService : IUnsubscribeService break; } } - catch (Exception ex) + catch (SsrfBlockedException ex) { + // Blocked target (e.g. points at an internal/metadata address). Log + // server-side; surface only a generic reason to the client. fail++; item.Status = UnsubscribeStatus.Failed; - item.ResultMessage = ex.Message; - errors.Add($"{item.UnsubscribeTarget}: {ex.Message}"); + item.ResultMessage = "Unsubscribe link was blocked for safety."; + _logger.LogWarning(ex, "Blocked unsubscribe target for user {UserId}", userId); + errors.Add("An unsubscribe link was blocked for safety."); + } + catch (Exception ex) + { + // V-06: log detail, return a generic message. + fail++; + item.Status = UnsubscribeStatus.Failed; + item.ResultMessage = "Unsubscribe request failed."; + _logger.LogWarning(ex, "Unsubscribe request failed for user {UserId}", userId); + errors.Add("An unsubscribe request failed."); } } await _db.SaveChangesAsync(ct); diff --git a/src/InboxIntel.Infrastructure/DependencyInjection.cs b/src/InboxIntel.Infrastructure/DependencyInjection.cs index f52e281..6d5c30f 100644 --- a/src/InboxIntel.Infrastructure/DependencyInjection.cs +++ b/src/InboxIntel.Infrastructure/DependencyInjection.cs @@ -60,7 +60,10 @@ public static class DependencyInjection services.AddHostedService(); // HTTP clients - services.AddHttpClient("unsubscribe", c => c.Timeout = TimeSpan.FromSeconds(15)); + // V-01: do NOT follow redirects — a validated external URL must not be able to + // 3xx-redirect into an internal target after SafeHttpGuard has checked it. + services.AddHttpClient("unsubscribe", c => c.Timeout = TimeSpan.FromSeconds(15)) + .ConfigurePrimaryHttpMessageHandler(() => new HttpClientHandler { AllowAutoRedirect = false }); services.AddHttpClient("ollama"); services.AddHttpClient("openai"); diff --git a/src/InboxIntel.Infrastructure/Search/SearchService.cs b/src/InboxIntel.Infrastructure/Search/SearchService.cs index a51acc6..79b9e91 100644 --- a/src/InboxIntel.Infrastructure/Search/SearchService.cs +++ b/src/InboxIntel.Infrastructure/Search/SearchService.cs @@ -17,8 +17,17 @@ public class SearchService : ISearchService private readonly AppDbContext _db; public SearchService(AppDbContext db) => _db = db; + /// Hard upper bound on a user-facing page of results (V-10: DoS via huge pageSize). + public const int MaxPageSize = 200; + public async Task> SearchAsync(Guid userId, SearchRequestDto r, CancellationToken ct = default) { + // V-10: clamp pagination at the user-facing chokepoint (covers both the GET + // query-string path and the POST body path) so a caller cannot request an + // unbounded materialisation. Internal callers (e.g. cleanup target resolution) + // do not go through this service, so their larger pages are unaffected. + r = r with { Page = Math.Max(1, r.Page), PageSize = Math.Clamp(r.PageSize, 1, MaxPageSize) }; + var q = _db.Emails.AsNoTracking().Where(e => e.UserId == userId); if (!string.IsNullOrWhiteSpace(r.Sender)) diff --git a/src/InboxIntel.Infrastructure/Security/SafeHttpGuard.cs b/src/InboxIntel.Infrastructure/Security/SafeHttpGuard.cs new file mode 100644 index 0000000..fa05986 --- /dev/null +++ b/src/InboxIntel.Infrastructure/Security/SafeHttpGuard.cs @@ -0,0 +1,94 @@ +using System.Net; +using System.Net.Sockets; + +namespace InboxIntel.Infrastructure.Security; + +/// +/// Validates outbound request URLs before the server fetches them, to prevent +/// SSRF (V-01). Used by the unsubscribe processor (URLs come from attacker-authored +/// List-Unsubscribe headers) and intended for any future server-initiated fetch +/// (AI/breach providers). Enforces an http/https scheme allowlist and rejects hosts +/// that resolve to loopback / private / link-local / unique-local / multicast ranges +/// or the cloud metadata address. DNS is resolved and EVERY resolved address is +/// checked, defeating DNS-rebinding to an external name that points at an internal IP. +/// +/// Pair this with an HttpClient configured with AllowAutoRedirect = false so a +/// permitted external URL cannot 3xx-redirect into an internal target post-validation. +/// +public static class SafeHttpGuard +{ + /// + /// Throws if the URL is unsafe to fetch. + /// Returns the validated absolute Uri otherwise. + /// + public static async Task ValidateAsync(string? url, CancellationToken ct = default) + { + if (string.IsNullOrWhiteSpace(url) || !Uri.TryCreate(url, UriKind.Absolute, out var uri)) + throw new SsrfBlockedException("Unsubscribe target is not a valid absolute URL."); + + if (uri.Scheme != Uri.UriSchemeHttp && uri.Scheme != Uri.UriSchemeHttps) + throw new SsrfBlockedException($"Disallowed URL scheme '{uri.Scheme}'."); + + // Resolve the host; if it's already a literal IP, GetHostAddressesAsync returns it. + IPAddress[] addresses; + try + { + addresses = await Dns.GetHostAddressesAsync(uri.DnsSafeHost, ct); + } + catch (Exception ex) when (ex is SocketException or ArgumentException) + { + throw new SsrfBlockedException("Unsubscribe target host could not be resolved."); + } + + if (addresses.Length == 0) + throw new SsrfBlockedException("Unsubscribe target host did not resolve to any address."); + + foreach (var ip in addresses) + if (IsBlocked(ip)) + throw new SsrfBlockedException($"Unsubscribe target resolves to a disallowed address ({ip})."); + + return uri; + } + + /// True if the address is in a range we must never fetch server-side. + public static bool IsBlocked(IPAddress ip) + { + if (IPAddress.IsLoopback(ip)) return true; + + if (ip.AddressFamily == AddressFamily.InterNetwork) + { + var b = ip.GetAddressBytes(); // big-endian + // 0.0.0.0/8 (this host), 10/8, 100.64/10 (CGNAT), 127/8, 169.254/16 (link-local + metadata), + // 172.16/12, 192.0.0/24, 192.168/16, 255.255.255.255 + if (b[0] == 0) return true; + if (b[0] == 10) return true; + if (b[0] == 100 && b[1] >= 64 && b[1] <= 127) return true; + if (b[0] == 127) return true; + if (b[0] == 169 && b[1] == 254) return true; // includes 169.254.169.254 metadata + if (b[0] == 172 && b[1] >= 16 && b[1] <= 31) return true; + if (b[0] == 192 && b[1] == 168) return true; + if (ip.Equals(IPAddress.Broadcast)) return true; + if (b[0] >= 224) return true; // multicast / reserved + return false; + } + + if (ip.AddressFamily == AddressFamily.InterNetworkV6) + { + if (ip.IsIPv6LinkLocal || ip.IsIPv6SiteLocal || ip.IsIPv6Multicast) return true; + // IPv4-mapped (::ffff:a.b.c.d) — re-check the embedded v4 address. + if (ip.IsIPv4MappedToIPv6) return IsBlocked(ip.MapToIPv4()); + var b = ip.GetAddressBytes(); + // Unique-local fc00::/7 + if ((b[0] & 0xFE) == 0xFC) return true; + return false; + } + + return true; // unknown family — fail closed + } +} + +/// Raised when an outbound URL is rejected by . +public class SsrfBlockedException : Exception +{ + public SsrfBlockedException(string message) : base(message) { } +} diff --git a/tests/InboxIntel.UnitTests/SafeHttpGuardTests.cs b/tests/InboxIntel.UnitTests/SafeHttpGuardTests.cs new file mode 100644 index 0000000..6416c5a --- /dev/null +++ b/tests/InboxIntel.UnitTests/SafeHttpGuardTests.cs @@ -0,0 +1,71 @@ +using System.Net; +using FluentAssertions; +using InboxIntel.Infrastructure.Security; +using Xunit; + +namespace InboxIntel.UnitTests; + +/// +/// SSRF guard (V-01): outbound URLs derived from attacker-authored email headers +/// must not be allowed to target internal/metadata/private addresses or non-web schemes. +/// +public class SafeHttpGuardTests +{ + [Theory] + [InlineData("169.254.169.254")] // cloud metadata + [InlineData("127.0.0.1")] // loopback + [InlineData("10.0.0.5")] // private A + [InlineData("172.16.4.4")] // private B + [InlineData("172.31.255.255")] // private B upper bound + [InlineData("192.168.1.1")] // private C + [InlineData("100.64.0.1")] // CGNAT + [InlineData("0.0.0.0")] // this-host + [InlineData("255.255.255.255")] // broadcast + [InlineData("224.0.0.1")] // multicast + public void Blocks_private_and_special_ipv4(string ip) + => SafeHttpGuard.IsBlocked(IPAddress.Parse(ip)).Should().BeTrue(); + + [Theory] + [InlineData("::1")] // loopback + [InlineData("fe80::1")] // link-local + [InlineData("fc00::1")] // unique-local + [InlineData("fd12:3456::1")] // unique-local + [InlineData("::ffff:169.254.169.254")] // v4-mapped metadata + [InlineData("::ffff:10.0.0.1")] // v4-mapped private + public void Blocks_private_and_special_ipv6(string ip) + => SafeHttpGuard.IsBlocked(IPAddress.Parse(ip)).Should().BeTrue(); + + [Theory] + [InlineData("8.8.8.8")] + [InlineData("93.184.216.34")] // example.com + [InlineData("2606:2800:220:1::1")] + public void Allows_public_addresses(string ip) + => SafeHttpGuard.IsBlocked(IPAddress.Parse(ip)).Should().BeFalse(); + + [Theory] + [InlineData("ftp://example.com/x")] + [InlineData("file:///etc/passwd")] + [InlineData("gopher://example.com")] + [InlineData("not-a-url")] + [InlineData("")] + public async Task Rejects_non_http_schemes_and_garbage(string url) + { + var act = async () => await SafeHttpGuard.ValidateAsync(url); + await act.Should().ThrowAsync(); + } + + [Fact] + public async Task Rejects_url_resolving_to_loopback() + { + // localhost resolves to a loopback address and must be blocked. + var act = async () => await SafeHttpGuard.ValidateAsync("http://localhost/unsub"); + await act.Should().ThrowAsync(); + } + + [Fact] + public async Task Rejects_literal_metadata_ip_url() + { + var act = async () => await SafeHttpGuard.ValidateAsync("http://169.254.169.254/latest/meta-data/"); + await act.Should().ThrowAsync(); + } +}