Skip to content

Commit fa37d5f

Browse files
security: improve proxy warning to check resolved IOptions, unify unknown-user keys
- Replace raw config-length check with IOptions<ForwardedHeadersOptions> post-Build DI resolution so warning fires for both 'not configured' and 'all CIDRs failed to parse' - Normalize ContentRateLimiterPolicy fallback to 'unknown-user' for consistency
1 parent 07e287f commit fa37d5f

4 files changed

Lines changed: 17 additions & 4 deletions

File tree

EssentialCSharp.Web/Controllers/ChatMessageRequest.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,5 +10,4 @@ public class ChatMessageRequest
1010
[StringLength(200)]
1111
public string? PreviousResponseId { get; set; }
1212
public bool EnableContextualSearch { get; set; } = true;
13-
public string? CaptchaResponse { get; set; } // hCaptcha token; validated server-side when chat captcha enforcement is wired up
1413
}

EssentialCSharp.Web/Program.cs

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,8 @@ private static void Main(string[] args)
128128
{
129129
if (System.Net.IPNetwork.TryParse(cidr, out var network))
130130
options.KnownIPNetworks.Add(network);
131+
else
132+
Console.Error.WriteLine($"[WARN] ForwardedHeaders:TrustedProxyCidrs: could not parse '{cidr}' as an IP network — entry skipped. Check your configuration.");
131133
}
132134
});
133135

@@ -320,7 +322,7 @@ private static void Main(string[] args)
320322
return RateLimitPartition.GetNoLimiter("mcp-transport");
321323

322324
var partitionKey = httpContext.User.Identity?.IsAuthenticated == true
323-
? httpContext.User.FindFirst(ClaimTypes.NameIdentifier)?.Value ?? "unknown-user"
325+
? httpContext.User.FindFirstValue(ClaimTypes.NameIdentifier) ?? "unknown-user"
324326
: httpContext.Connection.RemoteIpAddress?.ToString() ?? "unknown-ip";
325327

326328
return RateLimitPartition.GetFixedWindowLimiter(
@@ -437,6 +439,16 @@ await context.HttpContext.Response.WriteAsync(
437439

438440
WebApplication app = builder.Build();
439441

442+
// Warn if the effective trusted-proxy set is empty in non-Development — this fires for
443+
// both "no CIDRs configured" and "all configured CIDRs failed to parse" cases, ensuring
444+
// X-Forwarded-For spoofing protection (F4) is visibly inactive until properly configured.
445+
if (!app.Environment.IsDevelopment())
446+
{
447+
var fwdOpts = app.Services.GetRequiredService<IOptions<ForwardedHeadersOptions>>().Value;
448+
if (fwdOpts.KnownIPNetworks.Count == 0 && fwdOpts.KnownProxies.Count == 0)
449+
LogTrustedProxyCidrsNotConfigured(app.Logger);
450+
}
451+
440452
// Configure the HTTP request pipeline.
441453
if (!app.Environment.IsDevelopment())
442454
{
@@ -623,4 +635,7 @@ private static bool IsMcpTransportRequest(HttpRequest request) =>
623635

624636
[LoggerMessage(Level = LogLevel.Warning, Message = "Ignoring invalid TryDotNet origin in CSP: {Origin}")]
625637
private static partial void LogIgnoringInvalidTryDotNetOrigin(ILogger logger, string origin);
638+
639+
[LoggerMessage(Level = LogLevel.Warning, Message = "SECURITY: ForwardedHeaders:TrustedProxyCidrs is not configured. All X-Forwarded-For values are trusted, enabling IP spoofing against rate limits. Set this to your load-balancer CIDR (e.g., Azure Container Apps ingress range) to harden this endpoint.")]
640+
private static partial void LogTrustedProxyCidrsNotConfigured(ILogger logger);
626641
}

EssentialCSharp.Web/Services/ContentRateLimiterPolicy.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ public RateLimitPartition<string> GetPartition(HttpContext httpContext)
2626
// Use stable user ID (GUID) for authenticated users so the bucket survives
2727
// username changes and doesn't conflate login/logout with scraping.
2828
string partitionKey = isAuthenticated
29-
? $"user:{httpContext.User.FindFirstValue(ClaimTypes.NameIdentifier) ?? httpContext.User.Identity!.Name ?? "unknown"}"
29+
? $"user:{httpContext.User.FindFirstValue(ClaimTypes.NameIdentifier) ?? "unknown-user"}"
3030
: $"ip:{httpContext.Connection.RemoteIpAddress?.ToString() ?? "unknown-ip"}";
3131

3232
int perMinuteLimit = isAuthenticated ? AuthenticatedPerMinute : AnonymousPerMinute;

EssentialCSharp.Web/Services/McpRateLimiterPolicy.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ public RateLimitPartition<string> GetPartition(HttpContext httpContext)
2121
if (httpContext.User.Identity?.IsAuthenticated == true)
2222
{
2323
string userId = httpContext.User.FindFirstValue(ClaimTypes.NameIdentifier)
24-
?? httpContext.User.Identity?.Name
2524
?? "unknown-user";
2625

2726
return RateLimitPartition.GetTokenBucketLimiter(

0 commit comments

Comments
 (0)