New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Antiforgery: Why is the RequestToken "tailored" to the user? #4078

Closed
ruidfigueiredo opened this Issue Nov 19, 2018 · 3 comments

Comments

Projects
None yet
4 participants
@ruidfigueiredo

ruidfigueiredo commented Nov 19, 2018

The AntiforgeryToken's RequestToken contains not only random data (SecurityToken) but also the Username or a ClaimUid when the user is authenticated.

However this poses a problem when using ASP.NET as a web api for a SPA application while relying on cookies for authentication (and using a header as the RequestToken).

If all requests are performed via AJAX when logging in the following scenario occurs:

  • Ajax POST request to login
    The response to this request will contain the Set-Cookie for the authentication, a Set-Cookie for the antiforgery cookie and a non httponly Set-Cookie for what will be used as the value for the header in subsequent requests (RequestToken).

When calling IAntiforgery.GetAndStoreTokens while handling the login POST, no matter where this is done in the pipeline, the user won't be authenticated (HttpContext.User is not set). As a consequence of this the RequestToken is generated without a Username or ClaimUid.

If another requests happens immediately that requires antiforgery validation then we'll have a situation where the antiforgery "credentials" that we are able to supply in our request(antiforgery cookie + RequestToken) are for an non-authenticated user, but our request will be of an authenticated user (the auth cookie will be present).

The only solution that I found around this is to not rely on the available MVC filters to validate antiforgery (AutoValidateAntiforgeryAttribute and ValidateAntiforgeryAttribute) and manually perform antiforgery checks early in the pipeline, before the auth middleware. This feels a bit hacky.

            app.Use(async (context, next) =>
            {                
                if (context.Request.Path.Value.Equals("/")){
                    await next();
                    return;
                }
                
                if (!context.Request.Path.Value.EndsWith("login", StringComparison.InvariantCultureIgnoreCase))
                {
                    try
                    {                        
                        await antiForgery.ValidateRequestAsync(context);
                    }
                    catch (AntiforgeryValidationException ex)
                    {
                        context.Response.StatusCode = 400;
                        System.Diagnostics.Debug.WriteLine($"Invalid antiforgery token: {ex.Message}");
                        return;                        
                    }
                }
                var tokens = antiForgery.GetAndStoreTokens(context);
                context.Response.Cookies.Append(tokens.HeaderName, tokens.RequestToken, new CookieOptions
                {
                    HttpOnly = false
                });
                await next();

            });

            app.UseAuthentication();

The source of this issue is the inclusion of Username or ClaimUid in the RequestToken.

What is not obvious to me is what problem does that solve. Isn't the SecurityToken from the AntiForgeryCookie and RequestToken matching not enough to prevent CSRF?

Also I couldn't find a description that matched this approach (e.g. in OWASP)

As far as I can tell the inclusion of Username or ClaimUid prevents someone who can somehow steal an antiforgery cookie and request token from using them (to use them they would need to be able to login with the account that was used to create them which makes the exercise pointless). But if they can steal the antiforgery tokens then they can also steal the auth cookie, and in that case there's nothing that can be done to prevent the stolen account from being used.

@blowdart

This comment has been minimized.

Member

blowdart commented Nov 19, 2018

It's to prevent attacks in shared environments where you share a domain name, like azurewebsites.net or cloudapp.net. An attacker here could set a cookie for the "root" domain, which would then affect all subdomains. Hence we try to avoid this by adding in the username.

@ruidfigueiredo

This comment has been minimized.

ruidfigueiredo commented Nov 23, 2018

I tried setting up a proof of concept using the hosts file and nginx as a reverse proxy to two asp.net core applications, one at website.com and another at evil.website.com, both with antiforgery.

I was able to POST from evil.website.com to website.com and have the request be considered valid (I configured antiforgery in evil.website.com to use the website.com domain: services.AddAntiforgery(options => { options.CookieDomain = "website.com"; });). This for a non-authenticated user.

I was also able to "forge" an authenticated POST request from evil.website.com to website.com if the Username matched, i.e. I used the AntiForgery.GetAndStoreTokens with an evil.website.com user that had a username that matched a user from website.com. That was all it took to make the CSRF request valid.

But none of this is applicable in a "real" scenario because the Data Protection API is used to secure the value of the antiforgery cookie and token.

It only works in my scenario because I'm running everything locally and the Data Protection key used in website.com is shared with evil.website.com (I only had to setup data protection in evil.website.com with the same application name as website.com: services.AddDataProtection().SetApplicationName("GoodWebsite");).

All of this to say that including the username or claimuid in the request token doesn't seem to make much (any?) difference, and it makes the scenario I described initially more difficult (login via an ajax call).

For an attack to work, the attacker would need the key used with Data Protection. And with that key I believe they could just forge an auth cookie that would be accepted by the authentication cookie middleware and log in with any user (I actually didn't try this, but as far as I know it's the Data Protection API that is used to encrypt the auth cookie as well, so it should work).

@Eilon Eilon added the area-mvc label Nov 26, 2018

@mkArtakMSFT

This comment has been minimized.

Contributor

mkArtakMSFT commented Nov 28, 2018

Thanks for contacting us, @ruidfigueiredo. We're closing this issue as we have no plans to change the token contents.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment