rfc: CSRF protection

[ematipico]

Summary

CSRF - Cross-site Request forgery - protection

Links

• Rendered RFC
• Stage 2 Proposal
• Stage 1 Proposal

[florian-lefebvre]

I wonder if it would be better to make the option available under security, in case we want to add more security features (eg. honeypot) to core. So

// astro.config.mjs
export default defineConfig({
  security: {
    csrfProtection: ["origin", "cookie"]
  }
})

[natemoo-re]

@florian-lefebvre that's a good suggestion. We do try to avoid nesting too early since we don't necessarily know if we'll ever add something like a security.honeypot option, but if we already see lots of potential for expanding the APIs, adding a top-level category is a good suggestion.

[florian-lefebvre]

I think CORS could be supported in the future as well but i can't see anything else rn

[ematipico]

@lilnasy do you envision some option for the CSP support? If so, we could add a top-level security option.

[lilnasy]

Not necessarily. It's too soon to say because it's only stage 1, but it's possible a solution could just work. I only envision a flag for backwards compatibility, and that's only if the picked solution can't be retrofitted.

[ematipico]

In this commit 4d37d51 (#879) I changed the shape of the configuration due to some requirements that are needed for the cookie/token strategy.

[septatrix]

This will break form submissions when using curl for example or pretty much any other tool which is not a modern browser such as http clients in most programming languages. Another example would be load-testing tools. While some like k6 support setting custom headers this is cumbersome and not supported by everything.

A cookie based implementation would at least work with everything which has a sense of session. So not curl but at least the other stuff I listed.

Originally posted by @septatrix in https://github.com/withastro/astro/issues/10678#issuecomment-2051402692

[ematipico]

@septatrix: have you read the RFC? The RFC proposes a cookie solution, it would be great if you could share your thoughts about that.

[matthewp]

@septatrix can you explain why this will break curl usage? Does curl not allow you to sit the origin header?

[septatrix]

Curl does allow setting headers but this has to be done manually, existing automation might fail and - as said above - this is not the case for everything. E.g. many load testing tools are very simplistic and do not allow setting headers.

The cookie solution would still require changes to existing curl scripts and require one additional initial request to fetch the cookie. However, cookies are automatically handled by many other tools, especially client libraries in many programming languages establish session which store cookies between requests.

[septatrix]

One might also consider adding a configuration to the origin checking of how to treat requests with missing origin headers. To my knowledge this would apply to old browsers and most of non-browser tools. This would allow tools like curl et al to keep working while omitting the security checks for the fraction of users running very old browsers

[ematipico]

We decided to reduce the scope of the protection to the origin check only, hence the proposed configuration is also reduced. Changed applied in bc87ea7 (#879)

Also, this is a call for consensus! The RFC is ready to be shipped.

[florian-lefebvre]

Is it possible to opt-out of CSRF on a route basis? For example for webhooks like stripe

[ematipico]

Is it possible to opt-out of CSRF on a route basis? For example for webhooks like stripe

No, it isn't possible at the moment. Do you have webhooks that send posts like they were forms?

[florian-lefebvre]

Not sure I understand what you mean. In the case of stripe, a webhook is a post route that accepts some data (through request.text()). Stripe docs recommend disabling cors for the webhook specifically

[ematipico]

I understand now. I think it's a legitimate request, however, It can be implemented after this RFC lands

[florian-lefebvre]

I created a RFC so we don't don't forget #923

[matthewp]

The final comment period has ended and this RFC is accepted.