Implement Cross-Site Request Forgery Algorithm based on Fetch Metadata headers#66585
Implement Cross-Site Request Forgery Algorithm based on Fetch Metadata headers#66585DeagleGross wants to merge 13 commits intodotnet:mainfrom
Conversation
Implement lightweight cross-origin request protection based on Sec-Fetch-Site and Origin header validation, following the Go 1.25 CSRF protection approach. Architecture: - Interface + Enum in Http.Abstractions (ICrossOriginProtection, CrossOriginAntiforgeryResult) - Options + Implementation in Antiforgery (CrossOriginProtectionOptions, DefaultCrossOriginProtection) - Auto-registered in WebHost.ConfigureWebDefaultsWorker (enabled by default) - Enforced in EndpointMiddleware for endpoints with IAntiforgeryMetadata Key design decisions: - Independent layer alongside token-based antiforgery (not a replacement) - No dependency on DataProtection (trimmer-safe for cross-origin-only usage) - Config-based disable via CrossOriginProtection:Enabled = false - TrustedOrigins configurable via appsettings.json (semicolon-delimited) - Per-endpoint opt-out via existing .DisableAntiforgery() Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Problem: No way to opt out of Sec-Fetch CSRF protection per-endpoint in MVC Our CSRF delegate skips validation when MVC controllers have Impact: An MVC action decorated with Proposed fix: Have Trade-off: This changes the semantics of [IgnoreAntiforgeryToken] from "skip token validation" to "skip all Alternative: Introduce a new attribute (e.g., [AllowCrossOrigin]) that implements IAntiforgeryMetadata { |
There was a problem hiding this comment.
Pull request overview
This PR adds an auto-enabled, tokenless cross-origin request protection mechanism to ASP.NET Core’s default hosting pipeline, using Fetch Metadata (Sec-Fetch-Site) with an Origin/host fallback and optional trust derived from the default CORS policy.
Changes:
- Introduces new public antiforgery API (
ICsrfProtection,CsrfProtectionResult) to support cross-origin request validation and customization. - Implements
DefaultCrossOriginProtectionas shared source and auto-registers it in the default host, optionally trusting origins from the default CORS policy. - Auto-injects an inline middleware in
WebApplicationBuilderto enforce the validation by default, with opt-outs via endpoint antiforgery metadata or configuration.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/Shared/test/Shared.Tests/Microsoft.AspNetCore.Shared.Tests.csproj | Adds Antiforgery reference and compiles shared CSRF protection sources for shared tests. |
| src/Shared/test/Shared.Tests/DefaultCrossOriginProtectionTests.cs | Adds unit coverage for the Fetch Metadata + Origin/Host CSRF decision algorithm. |
| src/Shared/CsrfProtection/DefaultCrossOriginProtection.cs | Implements the core cross-origin protection algorithm and origin normalization helpers. |
| src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs | Updates debug-view middleware expectations to account for the new auto-injected delegate. |
| src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/Microsoft.AspNetCore.Tests.csproj | Adds references needed for CSRF protection + CORS integration tests. |
| src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/CsrfProtectionIntegrationTests.cs | Adds end-to-end tests validating auto-injection, disable paths, customization, and CORS-derived trust. |
| src/DefaultBuilder/src/WebHost.cs | Registers ICsrfProtection by default and derives trusted origins from the default CORS policy. |
| src/DefaultBuilder/src/WebApplicationBuilder.cs | Auto-injects a middleware that enforces ICsrfProtection validation (with opt-out support). |
| src/DefaultBuilder/src/Microsoft.AspNetCore.csproj | Pulls in shared CSRF protection implementation and adds required references. |
| src/Antiforgery/src/PublicAPI.Unshipped.txt | Declares new public API surface for review/baseline updates. |
| src/Antiforgery/src/ICsrfProtection.cs | Adds the new public interface used by the default builder and for customization. |
| src/Antiforgery/src/CsrfProtectionResult.cs | Adds the new public enum representing allow/deny outcomes. |
| // Step 2: Check trusted origins against the Origin header. | ||
| var origin = request.Headers.Origin.ToString(); | ||
| if (!string.IsNullOrEmpty(origin) && _trustedOrigins.Count > 0) | ||
| { | ||
| if (TryNormalizeOrigin(origin, out var normalizedOrigin) && _trustedOrigins.Contains(normalizedOrigin)) | ||
| { | ||
| return CsrfProtectionResult.Allowed; | ||
| } | ||
| } | ||
|
|
||
| // Step 3: Sec-Fetch-Site header (set by browsers per Fetch Metadata spec) | ||
| // https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Sec-Fetch-Site#same-site | ||
| var secFetchSite = request.Headers["Sec-Fetch-Site"].ToString(); | ||
| if (!string.IsNullOrEmpty(secFetchSite)) | ||
| { | ||
| return secFetchSite switch | ||
| { | ||
| "same-origin" => CsrfProtectionResult.Allowed, | ||
| "none" => CsrfProtectionResult.Allowed, | ||
| _ => CsrfProtectionResult.Denied, | ||
| }; | ||
| } | ||
|
|
||
| // Step 4: No Sec-Fetch-Site header. Fall back to Origin vs Host comparison. | ||
| if (!string.IsNullOrEmpty(origin)) | ||
| { |
There was a problem hiding this comment.
Not sure if that is needed, since Go implementation does not check against that:
https://cs.opensource.google/go/go/+/refs/tags/go1.25rc2:src/net/http/csrf.go;l=122
But we can consider adding this. Docs tell about Origin: null:
https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Origin- Requests from file:// URLs (local HTML files)
- Sandboxed iframes (<iframe sandbox> without allow-same-origin)
- data: URIs
- Cross-origin redirects (the redirect hop sets origin to null)
- Referrer-Policy: no-referrer in some cases
Since current implementation normalizes origin to the format "scheme://host:port", then it will fail normalization and will be rejected as "cross-origin", but it probably makes sense to special check this
| if (!_builtApplication.Properties.ContainsKey(CsrfProtectionMiddlewareSetKey)) | ||
| { | ||
| _builtApplication.Properties[CsrfProtectionMiddlewareSetKey] = true; | ||
| app.Use(static async (context, next) => |
There was a problem hiding this comment.
A place where CSRF is used in the request pipeline
| // Always registered; can be disabled at runtime via "CrossOriginProtection": "disable" in config | ||
| // (e.g., builder.WebHost.UseSetting("CrossOriginProtection", "disable")). | ||
| // Trusted origins are pulled from CORS policies if configured. | ||
| services.TryAddSingleton<ICsrfProtection>(sp => |
There was a problem hiding this comment.
where ICsrfProtection is registered
BrennanConroy
left a comment
There was a problem hiding this comment.
There was some concern expressed for HttpRequest.Form as it uses antiforgery internally.
https://source.dot.net/#Microsoft.AspNetCore.Http/Features/FormFeature.cs,140
Should we have a smoke test that it works fine with csrfprotection?
|
|
||
| // Auto-inject cross-origin CSRF protection after auth. | ||
| // Config check during Build, so established mechanisms like builder.WebHost.UseSetting("CrossOriginProtection", "disable") work. | ||
| if (!string.Equals("disable", _builtApplication.Configuration["CrossOriginProtection"], StringComparison.OrdinalIgnoreCase)) |
There was a problem hiding this comment.
DisableCsrfProtection: true is what we decided in API review.
| services.PostConfigure<CsrfProtectionOptions>(options => | ||
| { | ||
| // "CsrfProtection:TrustedOrigins": "https://example.com;https://other.com" | ||
| var origins = hostingContext.Configuration["CsrfProtection:TrustedOrigins"]?.Split(new[] { ';' }, StringSplitOptions.RemoveEmptyEntries); |
There was a problem hiding this comment.
I thought we were removing this for v1.
| /// <returns>The initialized <see cref="IWebHostBuilder"/>.</returns> | ||
| public static IWebHostBuilder CreateDefaultBuilder<[DynamicallyAccessedMembers(StartupLinkerOptions.Accessibility)] TStartup>(string[] args) where TStartup : class => | ||
| CreateDefaultBuilder(args).UseStartup<TStartup>(); | ||
|
|
| /// <summary> | ||
| /// Options for configuring cross-origin CSRF protection. | ||
| /// </summary> | ||
| public class CsrfProtectionOptions |
There was a problem hiding this comment.
Not part of approved API, we were just going to use CorsOptions
| return; | ||
| } | ||
|
|
||
| var csrfProtection = context.RequestServices.GetRequiredService<ICsrfProtection>(); |
There was a problem hiding this comment.
We should be able to resolve this once before creating the middleware.
| /// <summary> | ||
| /// Represents the result of cross-origin antiforgery request validation. | ||
| /// </summary> | ||
| public enum CsrfProtectionResult |
There was a problem hiding this comment.
I don't remember talking about this type much, did we consider a non-enum result type that could carry more information like an error? Sort of similar to IAntiforgeryValidationFeature
| { | ||
| } | ||
|
|
||
| public DefaultCrossOriginProtection(IEnumerable<string> trustedOrigins) |
There was a problem hiding this comment.
From API review, the origin is supposed to be configurable per-endpoint (via cors options).
It looks a bit complicated though, makes me want us to reconsider that decision 😆
https://source.dot.net/#Microsoft.AspNetCore.Cors/Infrastructure/CorsMiddleware.cs,134
https://source.dot.net/#Microsoft.AspNetCore.Cors/Infrastructure/CorsService.cs,232
| // Step 4: No Sec-Fetch-Site header. Fall back to Origin vs Host comparison. | ||
| if (!string.IsNullOrEmpty(origin)) | ||
| { | ||
| var requestOrigin = GetRequestOrigin(request); |
There was a problem hiding this comment.
nit: It'd be nice to try and optimize this to reduce the temporary strings being created.
|
|
||
| <ItemGroup> | ||
| <Compile Include="$(SharedSourceRoot)Obsoletions.cs" LinkBase="Shared" /> | ||
| <Compile Include="$(SharedSourceRoot)CsrfProtection\DefaultCrossOriginProtection.cs" LinkBase="Shared" /> |
There was a problem hiding this comment.
This seems odd, move the file to src/DefaultBuilder/src/internal/ if this is where we want the type to live?
| m => Assert.Equal("Microsoft.AspNetCore.Routing.EndpointRoutingMiddleware", m), | ||
| m => Assert.Equal("Microsoft.AspNetCore.Authentication.AuthenticationMiddleware", m), | ||
| m => Assert.Equal("Microsoft.AspNetCore.Authorization.AuthorizationMiddlewareInternal", m), | ||
| m => Assert.Contains("UseExtensions", m), // CSRF protection inline delegate |
There was a problem hiding this comment.
Wonder if internally we should be using a strongly-typed middleware class so this has a real name.
PR implements lightweight cross-origin request protection based on Sec-Fetch-Site and Origin header validation.
Design Decisions
Auto-enabled, zero config — no
AddCsrfProtection()orUseCsrfProtection(). Works automatically for all apps using WebApplicationBuilder (Minimal APIs, MVC, Razor Pages, Blazor).Public API surface (in Microsoft.AspNetCore.Antiforgery):
ICsrfProtection— interface with single Validate(HttpContext) methodCsrfProtectionResult— enum (Allowed, Denied)Implementation (
DefaultCrossOriginProtection) lives insrc/Shared/CsrfProtectionas shared source, compiled intoMicrosoft.AspNetCore (DefaultBuilder). Not publicly exposed.
Middleware injection — inline app.Use() delegate auto-injected in WebApplicationBuilder after routing/auth (same
pattern as
UseAuthentication/UseAuthorizationauto-injection).Custom implementation injection is supported via replacing the registration. (note that
TryAddSingleton<>will not work here, since it will return false and not replace the registration)That is the reason to resolve
ICsrfProtectionon every user request.Location of
ICsrfProtectionWe could put it in Antiforgery dll, and I tried, but there are problems (confirmed by tests):
ConfigureWebDefaultsWorkernow hasservices.TryAddSingleton<ICsrfProtection>(...)ICsrfProtectioncauses the JIT compiler to load its assembly whenConfigureWebDefaultsWorker is compiled, not when the line executes
Microsoft.AspNetCore.Antiforgery.dll in their bin output
That means we need to add reference to Antiforgery from OpenApi dll for example (which I think we want to avoid). So I moved the
ICsrfProtectiontoHttp.Abstractions- it is another place we discussed and agreed upon in the API review (see linked issue).Algorithm
CORS Integration
Trusted origins for the CSRF check are automatically pulled from the default CORS policy. If AddCors is configured with
specific origins (e.g., policy.WithOrigins("https://sso.example.com")), those origins are trusted for cross-origin POST
requests without requiring separate configuration. Policies using AllowAnyOrigin() are ignored — trusting all origins
would defeat the purpose of CSRF protection. Only the default policy is read since named policies are not publicly
enumerable via CorsOptions.
We can later expand to provide a configuration option to read CSRF trusted origins separately from CORS trusted origins
Disabling
Since
ICsrfProtectionis always registered in the WebHost, there is an option to check if CrossOriginProtection was disabled via config on the build step (WebApplicationBuilder.ConfigureApplication). Such disables work today:WebHostBuilder.UseSetting():builder.Services.Remove<ICsrfProtection>(a hack)Closes #65127