dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.42k stars 10k forks source link

Antiforgery should be useable without views #22189

Open jods4 opened 4 years ago

jods4 commented 4 years ago

Using ASP.NET Core 3.1 I am creating a new API for a single page application that is built separately and served as static files.

I thought that I would be served well with .AddControllers() as I'm just doing an API, no views.

Naturally I wanted to add CSRF protection. I started with .AddAntiforgery() and in the controllers options:

options.Filters.Add<AutoValidateAntiforgeryTokenAttribute>();

But that failed, because the attributes requires a service AutoValidateAntiforgeryTokenAuthorizationFilter, which wasn't registered and is an internal type.

This is where things started going bad.

First, I would suggest improving the docs and error messages: the antiforgery docs should clearly say what are the precise requirements for making it work. Ideally when an internal service is not found, the error message should point out what configuration is missing. Even if it's just a generic link to a page where all internal services are listed with their matching configuration call(s).

Googling did not bring up anything useful so I started looking at the source code on Github.

It seems that this internal type is only added to services if you use AddMvcCore and then AddViews (I found two other methods but they're even less appropriate).

Unless I missed something (please let me know), it feels wrong that to use a controller attribute you have to opt into the full views/templating stuff. Antiforgery is a security practice that every web api should include, it should be usable with .AddControllers or even .AddMvcCore without views.

davidfowl commented 4 years ago

The reasoning is that anti forgery isn’t required if you aren’t doing form posts from a browser client (which your rarely are when doing APIs). What’s the scenario? What type of API are you building that you need anti forgery without a browser client and views?

jods4 commented 4 years ago

It's a REST-like API that is the back-end of a SPA application (built with Vue) that performs GET, POST and all kind of calls using fetch.

From a security perspective, I don't understand your comment. What I do doesn't matter much. It's what an attacker could do from another site.

My GET don't modify state (although that could be someone else's case) but I have POST operations exposed so I'd better have a way to prevent CSRF, haven't I?

If you rely on the format of the payload to prevent the attack:

Also: security in depth, I'm not trying to be smart with how I prevent CSRF.

EDIT: I shall add that ASP.NET antiforgery doc specifically suggests sending the csrf token to SPA clients through a cookie and indicates that Angular will automatically reflect the value in a header. This sounds like implicit support for API without forms scenarios, which doesn't work end-to-end as you can't validate the token.

davidfowl commented 4 years ago

We’ve been pushing users towards JWT tokens for APIs and not cookies (if you’re using cookies with APIs the it makes sense wanting anti forgery to be enabled).

How is does the client get the anti forgery token? Are you custom rendering it and parsing it out of the body before sending the request?

cc @javiercn @blowdart

jods4 commented 4 years ago

I guess it also makes sense if using automatic NTLM? (I'm also using Cookies on other apps)

There are plenty of ways to send the token and I've used several in my apps:

javiercn commented 4 years ago

You should be able to register antiforgery on its own and to use the antiforgery token to generate the state required for this to work either on your initial page render or later.

I don't believe that we have a specific sample of cookies + antiforgery because it is something we discourage in favor of using bearer tokens. Specially for SPA scenarios.

The antiforgery token has two pieces, the request token and the cookie token. The request token can be sent inside a response header or inside a JS accessible cookie or transferred to the client through any other mechanism.

The filters for antiforgery are defined within the ViewFeatures assembly, but they should be usable outside of them.

jods4 commented 4 years ago

The filters for antiforgery are defined within the ViewFeatures assembly, but they should be usable outside of them.

This is the issue. Filters are internals and only added to services when you add views.

If I missed something, can you please provide a small ConfigureServices sample demonstrating antiforgery filters registration with AddControllers?

javiercn commented 4 years ago

I believe this is because we separated things into AddMvcCore and AddMvc. I don't think we consider antiforgery a piece of AddMvcCore and that's not something we are likely going to change, since using antiforgery with APIs is something we discourage.

What you can do is, either write your own antiforgery filter to apply to your api controllers:

public class ApiAntiforgeryTokenAuthorizationFilter : IAsyncAuthorizationFilter, IAntiforgeryPolicy
{
    private readonly IAntiforgery _antiforgery;
    public ApiAntiforgeryTokenAuthorizationFilter(IAntiforgery antiforgery) => _antiforgery = antiforgery;

    public async Task OnAuthorizationAsync(AuthorizationFilterContext context)
    {
        if (!context.IsEffectivePolicy<IAntiforgeryPolicy>(this))
        {
            return;
        }

        try
        {
            await _antiforgery.ValidateRequestAsync(context.HttpContext);
        }
        catch (AntiforgeryValidationException exception)
        {
            context.Result = new AntiforgeryValidationFailedResult();
        }
     }
}

or use the IAntiforgery service within your method body to validate the token.

jods4 commented 4 years ago

@javiercn

I don't think we consider antiforgery a piece of AddMvcCore

Actually you can add Antiforgery to AddMvcCore if you chain AddViews to it. Not ideal but possible.

AddMvcCore and AddMvc are not the only ways. I'm specifically mentionning AddControllers (new in 3.0) in which case adding the antiforgery filters is impossible.

since using antiforgery with APIs is something we discourage.

Fair enough but there's a difference between discouraging and not supporting.

We have security practices that have been validated by external reviewers and I can't change them in a blink, even if a bearer token would be better. I'm sure I'm not the only enterprise developer still using CSRF tokens.

Out of curiosity: my api uses automatic NTLM authentication. I have no need for a JWT token for auth, would you say CSRF is discouraged in this case? I'm curious about what you'd say is a security best practice here.

What you can do is, either write your own antiforgery filter

I can do this but wow that's really disappointing. Everything is avail. in asp.net but you just don't expose it.

Can't you add a new API .AddAntiforgery that can be chained to AddControllers? How does this non-view filter not make sense here? Or can't you register the antiforgery filters with .AddControllers? It's a controller thing, heck you do register them automatically with .AddMvcCore().AddViews(). Or make those filters public.

or use the IAntiforgery service within your method body to validate the token.

This is not a prototype but an enterprise app. Hundreds of methods, not gonna happen.

In any case, you may want to review your docs and API a bit. I read the docs, then put [ValidateAntiforgeryToken] on my controller... and boom I get a hard to debug exception referencing an internal type. Makes no sense and bad experience. Googling didnt' help.

As for the docs: please read the 3.1 docs Javascript, Ajax and SPAs in Anti-request forgery. It's quite long and gives various options to integrate CSRF with your SPA:

damienbod commented 4 years ago

@davidfowl @javiercn @blowdart

Would you not consider pushing towards cookies again with strict same domain protection? Renewing / refreshing tokens in the browser seems like a problem in the near future and using a backend for security would be the safest way. Using refresh tokens seems a bit dangerous at the moment, especially if no support exists for the revocation endpoint and most secure token server implementations don’t have the protections to make refresh tokens in the browser less dangerous, or the guidelines for using refresh tokens in browsers are still in progress. With YARP coming, cookies would make even more sense.

Greetings Damien

blowdart commented 4 years ago

Would you not consider pushing towards cookies again with strict same domain protection

Maybe, but the problem is all those older machines, with older browsers which still don't support any version of same site, let alone the new spec. Like IE, older versions of Safari on iPads and iPhones which will never get updated, or older versions of Android.

We cannot just limit ourselves to supporting the latest and greatest in this regard unfortunately, and there's no easy way to test browser support as the user agent sniffing we have to implement for SameSite shows.

damienbod commented 4 years ago

@blowdart Would the anti forgery token not be enough for the old browsers? The modern browser have both, this and same site. The question is "Which is less evil, refresh tokens in the browser, or only anti-forgery tokens for old browsers" Then the user has a choice. But I understand supporting only tokens, supporting 2 security architechtures is double the effort.

mcardoalm commented 4 years ago

I agree with @jods4 that having someway of registering or chaining .AddAntiForgery onto .AddControllers would be a great solution and allow developers to add the functionality if their situation called for it.

In our case we are going an Angular app with cookies, too large to convert to using JWT tokens currently, and everything works great, we use services.AddAntiforgery(options => options.HeaderName = "X-XSRF-TOKEN"); and it checks and validates the antiforgery token as needed using all the built-in attributes. We pass down the token in the initial call for SPA html page.

The only problem is the only way to get it to work now is to call .AddControllersWithViews instead of .AddControllers. If there was a way to chain the antiforgery internal functionality onto this, or even an example of how to register the needed pieces ourselves that would be great and solve our situation.

CrossBound commented 4 years ago

We’ve been pushing users towards JWT tokens for APIs and not cookies (if you’re using cookies with APIs the it makes sense wanting anti forgery to be enabled).

How is does the client get the anti forgery token? Are you custom rendering it and parsing it out of the body before sending the request?

cc @javiercn @blowdart

I believe this is because we separated things into AddMvcCore and AddMvc. I don't think we consider antiforgery a piece of AddMvcCore and that's not something we are likely going to change, since using antiforgery with APIs is something we discourage.

This seems like a very heavy handed tactic for a framework that is supposed to be customizable for your unique situation. Believe it or not Bearer tokens are not the safest option in all cases and there are legitimate reasons to want to authenticate an API with cookies (http://cryto.net/~joepie91/blog/2016/06/19/stop-using-jwt-for-sessions-part-2-why-your-solution-doesnt-work/) . In our case I'm running an ASP.NET Core 3.1 API that I am authenticating with cookies and I'm running into the same issue trying to get anti-forgery tokens working without having to add the entire MVC framework just to support this.

Trying to force bearer tokens on everybody is not the solution. We should be able to use the anti-forgery token code without needing to add all of MVC (which BTW security wise is a bad idea to have to add a lot of unneeded code that increases your attack surface).

In regard to how the client receives the token, you can send it back with a login action (https://odetocode.com/blogs/scott/archive/2017/02/06/anti-forgery-tokens-and-asp-net-core-apis.aspx).

CrossBound commented 4 years ago

I agree with @jods4 that having someway of registering or chaining .AddAntiForgery onto .AddControllers would be a great solution and allow developers to add the functionality if their situation called for it.

In our case we are going an Angular app with cookies, too large to convert to using JWT tokens currently, and everything works great, we use services.AddAntiforgery(options => options.HeaderName = "X-XSRF-TOKEN"); and it checks and validates the antiforgery token as needed using all the built-in attributes. We pass down the token in the initial call for SPA html page.

The only problem is the only way to get it to work now is to call .AddControllersWithViews instead of .AddControllers. If there was a way to chain the antiforgery internal functionality onto this, or even an example of how to register the needed pieces ourselves that would be great and solve our situation.

Thank you @mcardoalm , using AddControllersWithViews instead of just AddControllers fixed the error.

On that note, however, I see that it is automatically sending back the value in a cookie as well... does anyone know of a way to disable this cookie so that the server does not send it with each response?

sayedgt commented 3 years ago

If web apis doesn't support anti forgery tokens, why you allow it to generate. Why you are not mentioning we don't support anti forgery checking in web apis using default attributes? It would save a lot of time for many peoples.

coddo commented 3 years ago

I'm in a similar situation where I'm developing a SPA in Vuejs and want to have an aspnet core backend for it (without Razor Views, so we just call AddControllers() extension method), authenticated with cookies. The backend itself authenticates and authorizez the user through OpenID Connect, and this way of authenticating requests is preferred over using the PKCE flow + refresh tokens directly in the SPA.

This means that we we need to improve security by adding CSRF tokens to protect all HTTP requests between the front-end and back-end.

Indeed calling AddControllersWithViews() instead of just AddControllers() fixes the issue, but the effect of having the entire Razor Views pipeline is not really desired in this case.

Indeed, this kind of setup is not that widespread maybe, but I don't see why it shouldn't be possible to register the entire Antiforgery pipeline in APIs as well. Having the flexibility to configure the MVC pipeline as it fits for anyone and any scenario is one of the strong points of aspnet core, but this small part of it seems to have been omitted.

javiercn commented 3 years ago

Indeed calling AddControllersWithViews() instead of just AddControllers() fixes the issue, but the effect of having the entire Razor Views pipeline is not really desired in this case.

Can you give a concrete problem that you are running into by using AddControllersWithViews over AddControllers?

coddo commented 3 years ago

Can you give a concrete problem that you are running into by using AddControllersWithViews over AddControllers?

There's no technical issue per se, as it just works, but it's more a problem of expected behavior and time consumption to find out the issue when trying such a combination. A few hours were lost debugging until this issue page was reached with an explanation.

This is more of a personal example, though it might be more of an edge-case and might indicate some issues in my design as well, but I work with microservices and have core module that was made to automatically configure the startup of one type of application (API or Web App with Razor Views). For Razor Views I was calling AddControllersWithViews() and then configuring the behavior of the Razor engine, while for APIs I was using AddControllers() and then plugging in needed dependencies only. I was expecting to configure my SPA backend as an actual API + antiforgery plugin, but now I had to add another logical configuration branch that does somewhat of a hybrid configuration, so some time was spent there as well.

Why would I want to enable plugins in the pipeline that are not needed? That usually means also expanding the possible attack surface of the application. If I don't need the Razor engine in my API, why should I have it enabled?

All the extra development time added by this could maybe have been avoided if there was some clear indication in the docs that trying to use Antiforgery with APIs is no go and why?

undrivendev commented 3 years ago

We are in the same situation: we are developing a cookie-authenticated API (with only the AddControllers() services) with an Angular frontend. We cannot change the authentication mechanism, so using JWTs is out of question. We followed the documentation for Angular here, but we got a runtime error when calling the controllers decorated with the ValidateAntiForgeryTokenAttribute because of the missing registration of the ValidateAntiforgeryTokenAuthorizationFilter and AutoValidateAntiforgeryTokenAuthorizationFilter. The crux of the problem for us was that we couldn't register them manually because they are required but they are internal (source code). We ended up registering manually the two necessary filters using reflection, bypassing the internal access modifier and everything seems to be working just fine. Wouldn't be better for these filters to be marked as public? Or, better still, shouldn't be enough for the framework to provide an extension method like the one we did below in order to register the antiforgery dependencies for API-only projects?

public static IServiceCollection AddApiAntiforgery(
    this IServiceCollection services,
    Action<AntiforgeryOptions> setupAction)
{
    var types = Assembly.Load("Microsoft.AspNetCore.Mvc.ViewFeatures")
        .GetTypes();
    var autoType = types.First(t => t.Name == "AutoValidateAntiforgeryTokenAuthorizationFilter"); // necessary for the AutoValidateAntiforgeryTokenAttribute
    var defaultType = types.First(t => t.Name == "ValidateAntiforgeryTokenAuthorizationFilter"); // necessary for the ValidateAntiforgeryTokenAttribute
    services.AddSingleton(autoType);
    services.AddSingleton(defaultType);
    services.AddAntiforgery(setupAction);
    return services;
}
Quentinb commented 3 years ago

stumbled onto this as I've had the same issue and thought to see if there is a way to use AddAntiforgery without views. (I have stayed away from MVC and views for a very long time now), but having the anti-forgery tokens I feel is a must. I'm surprised by the Microsoft team discouraging the use of these outside of anything with forms.

Having these on a banking app for example, that calls to a banking API would be super useful. The first thing I normally do is run chrome dev tools and replay the request and with anti-forgery tokens, this help protect. Perhaps a bit of a lame example, but still. Keeping this feature to MVC, server rendered views only I feel is heavy handed.

But, I'm open to be corrected. I'll take a look at the "write your own" example above in the meantime to get around this for now.

Shayan-To commented 2 years ago

I don't believe that we have a specific sample of cookies + antiforgery because it is something we discourage in favor of using bearer tokens. Specially for SPA scenarios.

Where should I store the token in my SPA? In the localStorage so that every JS library I use can read it with ease? The only alternative to storing the token in the localStorage that I found is to use cookies.

seekingtheoptimal commented 1 year ago

@javiercn regarding the question of @Shayan-To , can you please provide the reasoning behind JWT vs Cookie for SPA from the .net team's perspective? There are a few pros and cons from both sides, but actually very few resources I have found tackle them one by one. The same is true for .net docs where this topic is mentioned, other than "we encourage to use X over Y" or "discourage to use Z" I haven't found a more in depth explanation behind the preferred direction. It would be good to understand the thinking from the developers of the .net platform (aka Microsoft?). On one hand it would reassure devs that the reasons are clear - as opposed to many online search hits when digging into this topic of "which one is better/safer/etc for SPA scenario (where it seems the majority actually "votes" for the "safer" cookie approach). And on the other hand it might also clear the fog on the design decisions on why some feature is shipped and the other not in the dotnet framework. Looking forward for any official docs or links in this topic. And thanks in advance.

Andrzej-W commented 1 year ago

@seekingtheoptimal you can read this https://github.com/dotnet/aspnetcore/issues/42158#issuecomment-1397140670 It is not .NET team's perspective but useful JWT vs Cookie summary. When you read about JWT people usually write that you should have refresh tokens to make it more secure. Every Blazor+JWT+refresh token example I have seen so far is not secure at all! Read this about refresh tokens: https://stackoverflow.com/questions/69800098/whats-the-whole-point-of-a-jwt-refresh-token Refresh tokens are useful only when identity provider is on a second server (different domain).

gcacars commented 3 months ago

Same here. Nothing was changed after 4+ years??

gcacars commented 3 months ago

By the way, I create a solution:

  1. Generate token
[HttpGet("/antiforgery/token")]
public ActionResult Get()
{
    if (!Request.Headers.ContainsKey("Origin")) return Forbid();

    var tokens = _forgeryService.GetAndStoreTokens(HttpContext);
    Response.Cookies.Append(
        "XSRF-TOKEN", tokens.RequestToken!, new CookieOptions { HttpOnly = false });

    return Ok(new SuccessResponseModel<object>(new
    {
        CSRFToken = tokens.RequestToken,
    }));
}
  1. Create a filter in a Controller to check the token
[AttributeUsage(AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = false, Inherited = true)]
public sealed class ValidateCsrfTokenAttribute : Attribute, IAuthorizationFilter
{
    private IAntiforgery? _antiforgeryService;

    public void OnAuthorization(AuthorizationFilterContext context)
    {
        if (context is null) throw new ArgumentNullException(nameof(context));

        _antiforgeryService ??= context.HttpContext.RequestServices
                .GetRequiredService<IAntiforgery>();

        if (!_antiforgeryService.IsRequestValidAsync(context.HttpContext).Result)
        {
            var response = new ErrorResponseModel()
            {
                Errors = new List<ProblemDetails>(),
            };

            response.Errors.Add(new ProblemDetails()
            {
                Title = "Token CSRF inválido",
                Detail = "Token CSRF não informado ou é inválido.",
                Instance = context.HttpContext.Request.Path,
                Status = StatusCodes.Status401Unauthorized,
            });

            context.Result = new JsonResult(response)
            {
                StatusCode = StatusCodes.Status401Unauthorized,
            };

            return;
        }
    }
}
  1. Use the filter in some actions in your API:
[HttpPost]
[ValidateCsrfToken]
public async Task<ActionResult> Post([FromQuery] AuthorizeRequestModel options)
{
   ...