🛡️ Sentinel: [HIGH] Configure ForwardedHeaders for correct IP detection#32
🛡️ Sentinel: [HIGH] Configure ForwardedHeaders for correct IP detection#32
Conversation
- Adds `ForwardedHeadersMiddleware` to `Enerflow.API` pipeline. - Configures `ForwardedHeadersOptions` to trust upstream proxies and process `X-Forwarded-For`/`X-Forwarded-Proto`. - Fixes vulnerability where Rate Limiting could be bypassed or cause DoS in containerized environments. Co-authored-by: Abdssamie <182430981+Abdssamie@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
📝 WalkthroughWalkthroughThis pull request documents and implements a security vulnerability fix for API rate limiting by adding ForwardedHeaders middleware configuration. The changes add documentation describing the vulnerability and implement the corresponding fix in the startup pipeline to correctly detect client IPs behind proxies. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@Enerflow.API/Program.cs`:
- Around line 12-19: The current ForwardedHeadersOptions block clears
options.KnownNetworks and options.KnownProxies which allows X-Forwarded headers
from any source; instead, update the ForwardedHeaders configuration
(ForwardedHeadersOptions) to explicitly allowlist trusted proxy IPs/CIDR entries
in options.KnownProxies or options.KnownNetworks and set options.ForwardLimit to
the exact number of proxies in your chain; locate the ForwardedHeadersOptions
setup (the lambda that sets options.ForwardedHeaders,
options.KnownNetworks.Clear(), options.KnownProxies.Clear()) and replace the
Clear() calls with adding the allowed proxy addresses/CIDRs and configure
options.ForwardLimit to an appropriate integer.
🧹 Nitpick comments (2)
Enerflow.API/Program.cs (2)
1-8: Reorder using directives to match project convention.Project namespaces should come after third‑party namespaces.
As per coding guidelines: Organize imports in order: System namespaces, third-party packages, project namespaces, DWSIM namespaces.♻️ Suggested import order
-using Enerflow.API.Extensions; -using Enerflow.API.Middleware; -using Enerflow.API.Services; -using Enerflow.Domain.Interfaces; -using MassTransit; -using StackExchange.Redis; -using Enerflow.Infrastructure; -using Microsoft.AspNetCore.HttpOverrides; +using MassTransit; +using Microsoft.AspNetCore.HttpOverrides; +using StackExchange.Redis; +using Enerflow.API.Extensions; +using Enerflow.API.Middleware; +using Enerflow.API.Services; +using Enerflow.Domain.Interfaces; +using Enerflow.Infrastructure;
30-42: Use explicitstringtypes for primitive configuration values.Guidelines call for explicit types for primitives.
As per coding guidelines: Usevarfor complex object types; use explicit types for primitives and return types.✅ Suggested change
-var redisConfiguration = builder.Configuration["RedisConfiguration"] +string redisConfiguration = builder.Configuration["RedisConfiguration"] ?? throw new InvalidOperationException("RedisConfiguration is not set in configuration"); @@ -var dbConnectionString = builder.Configuration.GetConnectionString("DefaultConnection") +string dbConnectionString = builder.Configuration.GetConnectionString("DefaultConnection") ?? throw new InvalidOperationException("DefaultConnection is not set in configuration");
| // Configure ForwardedHeaders to ensure correct IP detection behind proxies (critical for RateLimiting) | ||
| builder.Services.Configure<ForwardedHeadersOptions>(options => | ||
| { | ||
| options.ForwardedHeaders = ForwardedHeaders.XForwardedFor | ForwardedHeaders.XForwardedProto; | ||
| // In containerized environments, we trust the proxy (e.g. sidecar, ingress, ALB) | ||
| options.KnownNetworks.Clear(); | ||
| options.KnownProxies.Clear(); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
ASP.NET Core ForwardedHeadersOptions KnownProxies KnownNetworks security best practices
💡 Result:
Security best practices for ForwardedHeadersOptions.KnownProxies / KnownNetworks
-
Treat forwarded headers as untrusted input unless they come from a trusted proxy.
Only allow your reverse proxy/load balancer(s) to influenceRequest.Scheme,Request.Host, andRemoteIpAddressby configuringKnownProxiesand/orKnownNetworks(IP allowlisting). Otherwise, attackers can spoofX-Forwarded-*headers. [1] -
Prefer
KnownNetworksfor stable private ranges; useKnownProxiesfor fixed proxy IPs.- Use
KnownNetworkswhen your proxies live in a known CIDR range (common in VPC/VNet setups). - Use
KnownProxieswhen you truly have static, well-known proxy IPs. [1]
- Use
-
Do not “trust everyone” by clearing
KnownProxies/KnownNetworksin production.
Clearing these lists makes the app accept forwarded headers from any source, re-enabling spoofing risk; only consider this if you have a separate, strong network enforcement boundary and fully understand the tradeoff. [2] -
Limit how many hops you trust (
ForwardLimit/ForwardedForHeaderLimit).
Set the limit to the exact number of proxies in front of the app (not “large just in case”) to reduce header-injection surface area. -
Run the middleware early in the pipeline.
CallUseForwardedHeaders()before anything that depends on scheme/host/client IP (e.g., HTTPS redirection, auth, redirects, link generation). [3] -
Only enable the specific headers you need.
Typically:XForwardedForandXForwardedProto. Avoid enabling extra forwarded headers unless your proxy sets them correctly and you rely on them. -
Be aware of recent platform hardening (.NET servicing updates).
Starting in ASP.NET Core 8.0.17 and 9.0.6, forwarded headers are ignored unless the request comes from configured trusted proxies/networks (a security-hardening change). Ensure your real proxy IPs/ranges are configured, or you may see breakages like redirect loops behind TLS termination. [2]
Minimal secure example
using Microsoft.AspNetCore.HttpOverrides;
using System.Net;
builder.Services.Configure<ForwardedHeadersOptions>(o =>
{
o.ForwardedHeaders = ForwardedHeaders.XForwardedFor | ForwardedHeaders.XForwardedProto;
// Choose ONE (or both) based on your topology:
o.KnownProxies.Add(IPAddress.Parse("10.0.0.10")); // single proxy
// o.KnownNetworks.Add(new IPNetwork(IPAddress.Parse("10.0.0.0"), 8)); // CIDR range
o.ForwardLimit = 1; // set to your real proxy hop count
});
var app = builder.Build();
app.UseForwardedHeaders(); // earlySources: Microsoft Learn guidance on trusted proxies/networks and spoofing risk [1]; Microsoft breaking-change note on “unknown proxies” being ignored in 8.0.17/9.0.6 [2]; Microsoft IIS reverse-proxy guidance emphasizing middleware order and KnownProxies/KnownNetworks configuration [3].
[1] (learn.microsoft.com)
[2] (learn.microsoft.com)
[3] (techcommunity.microsoft.com)
Citations:
- 1: https://learn.microsoft.com/en-us/aspnet/core/host-and-deploy/proxy-load-balancer?view=aspnetcore-10.0&utm_source=openai
- 2: https://learn.microsoft.com/en-us/dotnet/core/compatibility/aspnet-core/8.0/forwarded-headers-unknown-proxies?utm_source=openai
- 3: https://techcommunity.microsoft.com/blog/iis-support-blog/how-to-properly-configure-iis-reverse-proxy-for-asp-net-core-applications-secure/4388483?utm_source=openai
Configure KnownProxies or KnownNetworks instead of clearing them.
Clearing both lists causes the app to accept X-Forwarded-* headers from any source, which enables IP spoofing attacks. Even in containerized environments, you must explicitly allowlist trusted proxies by IP or CIDR range. Set ForwardLimit to the exact number of proxies in your chain to reduce the attack surface.
Note: ASP.NET Core 8.0.17+ and 9.0.6+ will ignore forwarded headers from unknown proxies as a security hardening measure, which may cause unexpected behavior if trusted proxy IPs are not configured.
🔒 Correct configuration pattern
builder.Services.Configure<ForwardedHeadersOptions>(options =>
{
options.ForwardedHeaders = ForwardedHeaders.XForwardedFor | ForwardedHeaders.XForwardedProto;
- // In containerized environments, we trust the proxy (e.g. sidecar, ingress, ALB)
- options.KnownNetworks.Clear();
- options.KnownProxies.Clear();
+ // Explicitly allow only known proxies from configuration
+ options.KnownNetworks.Clear();
+ options.KnownProxies.Clear();
+ foreach (var ip in builder.Configuration.GetSection("ForwardedHeaders:KnownProxies").Get<string[]>() ?? [])
+ {
+ options.KnownProxies.Add(System.Net.IPAddress.Parse(ip));
+ }
+ options.ForwardLimit = 1; // set to your proxy chain depth
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Configure ForwardedHeaders to ensure correct IP detection behind proxies (critical for RateLimiting) | |
| builder.Services.Configure<ForwardedHeadersOptions>(options => | |
| { | |
| options.ForwardedHeaders = ForwardedHeaders.XForwardedFor | ForwardedHeaders.XForwardedProto; | |
| // In containerized environments, we trust the proxy (e.g. sidecar, ingress, ALB) | |
| options.KnownNetworks.Clear(); | |
| options.KnownProxies.Clear(); | |
| }); | |
| // Configure ForwardedHeaders to ensure correct IP detection behind proxies (critical for RateLimiting) | |
| builder.Services.Configure<ForwardedHeadersOptions>(options => | |
| { | |
| options.ForwardedHeaders = ForwardedHeaders.XForwardedFor | ForwardedHeaders.XForwardedProto; | |
| // Explicitly allow only known proxies from configuration | |
| options.KnownNetworks.Clear(); | |
| options.KnownProxies.Clear(); | |
| foreach (var ip in builder.Configuration.GetSection("ForwardedHeaders:KnownProxies").Get<string[]>() ?? []) | |
| { | |
| options.KnownProxies.Add(System.Net.IPAddress.Parse(ip)); | |
| } | |
| options.ForwardLimit = 1; // set to your proxy chain depth | |
| }); |
🤖 Prompt for AI Agents
In `@Enerflow.API/Program.cs` around lines 12 - 19, The current
ForwardedHeadersOptions block clears options.KnownNetworks and
options.KnownProxies which allows X-Forwarded headers from any source; instead,
update the ForwardedHeaders configuration (ForwardedHeadersOptions) to
explicitly allowlist trusted proxy IPs/CIDR entries in options.KnownProxies or
options.KnownNetworks and set options.ForwardLimit to the exact number of
proxies in your chain; locate the ForwardedHeadersOptions setup (the lambda that
sets options.ForwardedHeaders, options.KnownNetworks.Clear(),
options.KnownProxies.Clear()) and replace the Clear() calls with adding the
allowed proxy addresses/CIDRs and configure options.ForwardLimit to an
appropriate integer.
1 participant
Security Fix: Forwarded Headers Configuration
🛡️ Sentinel Report
Severity: HIGH
Vulnerability:
The application was missing
ForwardedHeadersMiddlewareconfiguration. In containerized environments (Docker/Kubernetes) or behind reverse proxies (Nginx/AWS ALB), the application sees the proxy's IP address instead of the actual client IP.Impact:
RateLimitingMiddlewareusesRemoteIpAddress. Without this fix, ALL users share the same IP (the proxy's IP). If one user triggers the rate limit, ALL users are blocked.Fix:
using Microsoft.AspNetCore.HttpOverrides;.ForwardedHeadersOptionsto clear known networks/proxies (trusting the upstream proxy) and enabledX-Forwarded-ForandX-Forwarded-Proto.app.UseForwardedHeaders()to the request pipeline before rate limiting and HSTS.Verification:
PR created automatically by Jules for task 10318858718581525389 started by @Abdssamie
Summary by CodeRabbit
Release Notes
Bug Fixes
Documentation