search

redwoodjs / redwood

The App Framework for Startups
https://redwoodjs.com
MIT License
16.92k stars 973 forks source link

Implement CSRF checking with dbAuth #3075

Open cannikin opened 2 years ago

cannikin commented 2 years ago

dbAuth is currently generating a CSRF token on each request and adding it to the header/session cookie response. But, to get CSRF protection working for real we need some additional functionality:

  1. web-side: store the latest received CSRF token header and send back up in a csrf-token header for each GraphQL request (mutation for sure, but maybe not query? classically, I believe CSRF tokens are only used when state can be changed on the server, which for the web usually means a form submission)
  2. api-side: Compare the received CSRF token in the header to the one in the decrypted session cookie and reject the request if they don't match

Similar to Rails we could give the developer an option of what to do if the tokens don't match: throw an error or log the user out.

cannikin commented 2 years ago

I won't be able to make any progress on this until I can get time with @peterp or someone else intimately familiar with hooking in to the web side's GraphQL requests, and the api side's GraphQL responses, and then storing data either in cache or React state somehow, with zero config from the author of the app.

I believe hooking into the api side's response was going to be significantly easier once we switch to Envelop/Helix.

pi0neerpat commented 2 years ago

Does this mean dbAuth is not safe to use until this is implemented?

cannikin commented 2 years ago

Well, not any less safe than any of the other 3rd-party auth solutions, none of which prevent CSRF attacks since you’re not checking with them on the validity of the request content on each request.

We set the SameSite attribute on the session cookie, which goes a long way to preventing CSRF. I’m not sure what attack vectors can get around that, however.

I’m not familiar enough with the guts of Apollo Client to get in there and get it sending the token in a header on each GraphQL request, and I haven’t had a chance to look into it. We’re generating a token on each request and setting it in the encrypted cookie, as well as setting in a header in the response back from the auth function. The last piece of the puzzle is saving the last one in state and sending it back UP to the API and checking against the one encrypted in the cookie. This should only be necessary for mutation queries since they should be the only ones capable of changing state on the server.

dthyresson commented 2 years ago

More info on CSRF and cookies can be found in Clerk's documentation: https://docs.clerk.dev/learning-center/security/csrf-protection

dthyresson commented 2 years ago

But, to get CSRF protection working for real we need some additional functionality:

  1. web-side: store the latest received CSRF token header and send back up in a csrf-token header for each GraphQL request (mutation for sure, but maybe not query? classically, I believe CSRF tokens are only used when state can be changed on the server, which for the web usually means a form submission)
  2. api-side: Compare the received CSRF token in the header to the one in the decrypted session cookie and reject the request if they don't match

Some ideas ...

1 - since you sort of ignore the Bearer token (it is the user id), could you store the CSRF as the token such that getToken sends it in each graphql request?

The add a custom envelop plugin to extract and ue that as the csrf to be set in context?

2 - A custom envelop plugin similar to https://github.com/redwoodjs/redwood/blob/main/packages/graphql-server/src/plugins/useRedwoodAuthContext.ts could work here -- or add that feature.

cannikin commented 2 years ago

Hmm, that Clerk doc just talks about setting the SameSite attribute on the cookie (which dbAuth does as well, although we set it Strict by default), but I'm thinking more about the traditional CSRF protection where you've got a unique token going back and forth from the client and server and the server verifies that it's correct.

We could maybe re-use the Bearer auth header, although is that "standard" usage? Most CSRF stuff I've read talks about adding a custom header that contains it. Should the Bearer token be changing on every request?

I'm not 100% sure that traditional CSRF token verification will work with a SPA...with your standard server-rendered site you could set a token on every request, since every request comes the server. But in our case you may not have talked to the server since the last time you logged in, which is the last time we could have sent you a new CSRF token. Although I guess as long as the last one you received matches the one encoded in your cookie it should still be considered valid, no matter how long ago that was...

cannikin commented 2 years ago

I've also thought about changing the content of the Bearer token so that it is NOT the user's ID... that could be potentially sensitive information you don't want leaking to the outside world (like if you're doing some good ol' security-by-obscurity).

pi0neerpat commented 2 years ago

I agree with both of your last comments.