dotnet / dotnet/aspnetcore

AntiforgeryMiddleware needs an escape-hatch for troubleshooting and logging, and customizing the final response

Open
#68,054 0 comments 0 reactions 0 assignees View on GitHub
area-security
Dominant language
C#
Stars
38.4k
Forks
10.9k
Avg merge
2d 10h
Merged PRs (30d)
281

Description

### Is there an existing issue for this?

- [x] I have searched the existing issues

Searching existing issues found these which are somewhat relevant, but ultimately unhelpful for this specific problem:

* https://github.com/dotnet/aspnetcore/pull/66772
* https://github.com/dotnet/aspnetcore/pull/51962

### Is your feature request related to a problem? Please describe the problem.

When `AntiforgeryMiddleware` is active, it becomes impossible to inspect `HttpRequest.Form` for logging or troubleshooting purposes, which frustrates attempts to resolve (non-malicious) inadvertent CSRF failures. Additionally, there are plenty of locations within AspNetCore itself where `HttpRequest.Form` or `HasFormContentType` is used such that `HandleUncheckedAntiforgeryValidationFeature()` will throw.

Relatedly, I noticed that there doesn't seem to be a way to return a user-friendly error-page or non-500 response when there is a CSRF failure.

For example, because `Microsoft.AspNetCore.Mvc.ModelBinding.FormValueProviderFactory` accesses `HttpRequest.HasFormContentType` it will throw-up the `InvalidOperationException` caused by the `AntiforgeryMiddleware`:

```
[12:56:19 Error] Microsoft.AspNetCore.Server.Kestrel
Connection id "0HNNC321O3812", Request id "0HNNC321O3812:00000035": An unhandled exception was thrown by the application.
System.InvalidOperationException: This form is being accessed with an invalid anti-forgery token. Validate the `IAntiforgeryValidationFeature` on the request before reading from the form.
at Microsoft.AspNetCore.Http.Features.FormFeature.HandleUncheckedAntiforgeryValidationFeature()
at Microsoft.AspNetCore.Http.Features.FormFeature.get_HasFormContentType()
at Microsoft.AspNetCore.Mvc.ModelBinding.FormValueProviderFactory.CreateValueProviderAsync(ValueProviderFactoryContext context)
at Microsoft.AspNetCore.Mvc.ModelBinding.CompositeValueProvider.CreateAsync(ActionContext actionContext, IList`1 factories)
at Microsoft.AspNetCore.Mvc.ModelBinding.CompositeValueProvider.TryCreateAsync(ActionContext actionContext, IList`1 factories)
at Microsoft.AspNetCore.Mvc.Controllers.ControllerBinderDelegateProvider.<>c__DisplayClass0_0.<g__Bind|0>d.MoveNext()
--- End of stack trace from previous location ---
at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.g__Awaited|13_0(ControllerActionInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.g__Awaited|25_0(ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.Rethrow(ResourceExecutedContextSealed context)
at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.Next(State& next, Scope& scope, Object& state, Boolean& isCompleted)
at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.InvokeFilterPipelineAsync()
--- End of stack trace from previous location ---
at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.g__Awaited|17_0(ResourceInvoker invoker, Task task, IDisposable scope)
at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.g__Awaited|17_0(ResourceInvoker invoker, Task task, IDisposable scope)
at Microsoft.AspNetCore.Authorization.AuthorizationMiddleware.Invoke(HttpContext context)
```

Furthermore, because it throws the generic exception-type, `InvalidOperationException` with no specific information carried with it, it's impractical to intercept it in middleware and return a more appropriate response - and as said, it's impossible to log the _actual_ parsed `Request.Form` values (if any) when they may aid troubleshooting.

### Describe the solution you'd like

`IAntiforgeryValidationFeature` should expose `RequestHasFormContentType` and the `IFormCollection` that `HttpRequest.Form` would otherwise return without throwing `InvalidOperationException`, for use by logging/debugging/troubleshooters.

### Workaround

This is what I'm currently doing as a hack workaround to enable logging of `Request.Form`, though it uses an undocumented `HttpContext.Items` key - and I'm unsure how robust or reliable it is under other CSRF scenarios:

```
private static String GetRequestFormForLogScope( HttpRequest req )
{
if( !"POST".EqualsIns( req.Method ) ) return "(Non-POST request)";

//

// AspNetCore's AntiForgery feature blocks access to `HttpRequest.Form`, including even checking `HasFormContentType` - which means we can't log those details (grrr).
// I filed this issue concerning this problem: https://github.com/dotnet/aspnetcore/issues/68054

IAntiforgeryValidationFeature? csrfFeature = req.HttpContext.Features.Get();
if( csrfFeature is null || csrfFeature.IsValid )
{
if( req.HasFormContentType ) // <-- Accessing this property will throw if `csrfFeature.IsValid == false`
{
return StringBuilderLoggingExtensions.FlattenRequestForm( req.Form );
}
else
{
return KnownSerilogProperties.AspNetRequestForm_InvalidContentType;
}
}
else
{
HttpContext httpContext = req.HttpContext;

// HACK:
const String FLAG_ITEM_KEY = @"__AntiforgeryMiddlewareWithEndpointInvoked";
if( httpContext.Items.ContainsKey( FLAG_ITEM_KEY ) )
{
Object? item = httpContext.Items[ FLAG_ITEM_KEY ];
_ = httpContext.Items.Remove( FLAG_ITEM_KEY );
try
{
if( req.HasFormContentType )
{
return StringBuilderLoggingExtensions.FlattenRequestForm( req.Form );
}
else
{
return KnownSerilogProperties.AspNetRequestForm_InvalidContentType;
}
}
finally
{
httpContext.Items[ FLAG_ITEM_KEY ] = item;
}
}
else
{
return "CSRF Antiforgery error: " + csrfFeature.Error?.ToString();
}
}
}
```

### Additional context

_No response_

Contributor guide

Open the contributing guide

Research direction

Start by reading AntiforgeryMiddleware, IAntiforgeryValidationFeature, and FormFeature, then review the related pull requests linked in the issue. Define the API and behavior for inspecting form data after validation failure and customizing the resulting response. Done should include coverage for invalid tokens, form-content checks, and avoiding the unhandled InvalidOperationException path.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
api, backend, security
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.