axum-live-view icon indicating copy to clipboard operation
axum-live-view copied to clipboard

CSRF tokens

Open davidpdrsn opened this issue 2 years ago • 16 comments

Phoenix uses CSRF tokens, we might need to as well but I don't know a whole lot about CSRF tokens so not sure how to implement them.

https://en.wikipedia.org/wiki/Cross-site_request_forgery

davidpdrsn avatar Dec 21 '21 10:12 davidpdrsn

there are 2 ways you can go about it and I would suggest supporting Both

https://crates.io/crates/axum-csrf-sync-pattern https://crates.io/crates/axum_csrf

As both are used in different ways for different things.

genusistimelord avatar Feb 06 '23 19:02 genusistimelord

At the first glance, these crates meant to work with ajax and fetch requests. I don't see how would we use them, as all the data go through websocket. It seems that we need a csrf check on websockets handshake phase.

Here's an example from the Phoenix docs:

let csrfToken = document.querySelector("meta[name='csrf-token']").getAttribute("content")
let liveSocket = new LiveSocket("/live", Socket, {params: {_csrf_token: csrfToken}})
liveSocket.connect()
  1. they put a random token into html meta tag on the initial render
  2. then they take it from the html and pass the LiveView::mount to keep during the connection
  3. and then I guess they pass it into LiveView::update each time and check if it correspond the handshake version

Here's also from a blogpost people seem to reference with regard to the question:

  1. Check the Origin header of the WebSocket handshake request on the server, since that header was designed to protect the server against attacker-initiated cross-site connections of victim browsers!
  2. Use session-individual random tokens (like CSRF-Tokens) on the handshake request and verify them on the server.

imbolc avatar Feb 11 '23 18:02 imbolc

What do you think on this approach: https://github.com/imbolc/axum-live-view/commit/c3a8d85cb6f3c665c073c4ce4cde746ce73eb5e1

We pass a random token in a cookie (is should be SameSite or something, I haven't dig into this). Js part reads the cookie and passes the token in the uri to the server. At the same time browser resent us back the token as a cookie. Then we compare tokens (from uri and cookies).

imbolc avatar Feb 12 '23 11:02 imbolc

I skimmed it and looks good overall!

davidpdrsn avatar Feb 12 '23 12:02 davidpdrsn

I was thinking on why Phoenix uses html meta tag to pass csrf token and found a little issue with using cookies for this puprose.

There's a little gap between parsing csrf token from cookies and making handshake request. Imagine someone opens the site in another browser tab during this gap and the cookie change. Now, when the first tab finally makes the request, csrf token doesn't match the cookie anymore.

To illustrate this I added an artificial delay in the TS:

function connect(options: LiveViewOptions) {
    const csrf_token = getCookie("_lv_csrf");
    if (!csrf_token) {
        console.error("no live view csrf token");
        return;
    }

    console.log("delaying connection...");
    setTimeout(() => {
        console.log("connecting");
        delayed_connect(options, csrf_token);
    }, 15 * 1000);
}

function delayed_connect(options: LiveViewOptions, csrf_token: string) {
  ...

This way, opening second tab during the delay, I see reconnection in the first one:

bundle.js:1 delaying connection...
bundle.js:1 connecting
bundle.js:1 WebSocket connection to 'ws://localhost:3000/?csrf=1676245839336943659' failed: 
bundle.js:1 delaying connection...
bundle.js:1 connecting

It's hard to imagine the issue manifest itself in reality. And even if it would, it only leads to reconnection. So I'd stick with cookies for the sake of simplicity, other way we would need to change response html. What do you think?

imbolc avatar Feb 13 '23 00:02 imbolc

Hmm... I don't even see how using meta tag would help, as we still need a cookie for the browser to send it back. Meta tag would actually make it worse breaking reconnection, as we always would use a value from the static tag and won't see the change as we do with cookies. So we would need to use something like localStorage to sync changes between tabs. Maybe meta tag is just a legacy thing for Phoenix.

imbolc avatar Feb 13 '23 00:02 imbolc

yeah the design wise is what I was referring too. not to using them directly. To do CSRF correctly you will need some kind of local session storage per user. then to send a hashed CSRF token to the socket request and finally compare the hashes of the original server side with the returned. we really don't have to use a cookie. I just used a cookie in mine as a three prong approach. Also you need to ensure that the connection is generally encrypted.

Also to prevent people who do manage to get the token somehow you would reset the token server side upon each use so it cant be used again.

genusistimelord avatar Feb 13 '23 13:02 genusistimelord

@genusistimelord could elaborate on issues with the approach I'm suggesting?

imbolc avatar Feb 13 '23 13:02 imbolc

the issue is as you say. The cookie itself cant exist for multiple tabs. The best way to go about this would be to implement A session layer that can handle a Per tab bases upon socket connections. This is also more secure than using a cookie. So design wise.

Generate a Session Layer that is linked with a private Cookie containing a UUID to the Session that can be shared per Tab. Then inside of the Session Layer you can have a CSRF Maintainer that holds CSRF codes Per Socket Information which is generated Per Tab. You can then on a Socket Receive check upon this Table Against the Incoming CSRF Hash from within the Page. Once it is Verified you will then generate a new CSRF Token for the connection. This invalidates the old Hash just in case someone else does try to use it.

Also while your at it since its a Session Layer you can also add a Session data container to contain Data per Session and or Per Tab. Based on how you would want to go about this. That way you limit the Saved important data to the Server side and not the client side. Client side is not Secure enough to only use just it. This is how my CSRF system was designed. Other than not using Sockets and using fetch requests instead. The design works with anything just needs to be adapted for it.

genusistimelord avatar Feb 13 '23 14:02 genusistimelord

the issue is as you say

Do I understand correctly that you don't see other issues with the implementation? I mean isn't this one seems negligible? I'm not sure it would ever occur in practice and even if it did, it would just lead to a reconnection isn't it?

Generate a Session Layer that is linked with a private Cookie containing a UUID to the Session that can be shared per Tab.

Could you explain this. Wouldn't the cookie still be shared between tabs? I guess it should be something like a html meta tag then?

Once it is Verified you will then generate a new CSRF Token for the connection. This invalidates the old Hash just in case someone else does try to use it.

Don't we need the csrf token only on the handshake step? Why change it afterwards then?

imbolc avatar Feb 13 '23 17:02 imbolc

@genusistimelord would it be correct to adapt your suggestion like this?

  • on html request:
    • generate a random token
    • keep it in some kind of registry on server
    • add it to html meta tag
  • in browser:
    • parse the html tag
    • pass the token to handshake request
  • on handshake request:
    • check if the token is in the registry
    • confirm handshake if it's there
    • remove the token from the registry

imbolc avatar Feb 13 '23 17:02 imbolc

@imbolc

Do I understand correctly that you don't see other issues with the implementation? I mean isn't this one seems negligible? I'm not sure it would ever occur in practice and even if it did, it would just lead to a reconnection isn't it?

the issue would not be a reconnection it would cause the codes to collide if they were on multiple pages. You can solve this by making a Token per each separate Tab/connection and can store it under the Session. You can use a Vec<Token> and just do a lookup search for it OR my preferred way is to use a HashMap and make the Key the connection information needed to see the difference between each tag. This would be Per Session.

Could you explain this. Wouldn't the cookie still be shared between tabs? I guess it should be something like a html meta tag then?

I would make this a HashMap of UUID's as the key for the user. inside the hashmap I would have it have a storage HashMap for all the Session Data that needs to be used across the Session. But in this Case for the CSRF I would add a 2nd HashMap or Vec that stores and handles these per Connection info or just a simple Lookup if it exists or not. I would Also have the CSRF Tokens set to a Timer so they Expire and can not be used after a set amount of time for Security. For the Session Stuff you can view how I did it in my Session Library.

This is the main Store Per User. https://github.com/AscendingCreations/AxumSessions/blob/main/src/session_data.rs#L23

This is the Storage for All Sessions active. https://github.com/AscendingCreations/AxumSessions/blob/main/src/session_store.rs#L28

Don't we need the csrf token only on the handshake step? Why change it afterwards then?

You can either Remove it or change it if you are going to Use it again. This prevents Replay attacks. Also with this you wont need the Token in a Cookie just the HTML Tag.

would it be correct to adapt your suggestion like this?

Yes that will work. Just make sure on the HTML Meta Tag that you are Hashing it. you don't want to give them the Direct Token only a hash. Also the nice thing about using a Session System is the End user even if they get the Session ID cant see the important server sided Data. Since we hide this. However One thing I did leave out is If you are setting up Session UUID's you will also want the Ability to Swap the UUID when a user goes into a restricted Area Etc. But that is an entirely different Security issue.

If you want to use my stuff as helper Examples Feel free too. If you do get something written up for this feel free to let me know and I can take a look over it and offer and advice or changes that can be made to improve it and the Security of it.

genusistimelord avatar Feb 13 '23 18:02 genusistimelord

the issue would not be a reconnection it would cause the codes to collide if they were on multiple pages

That's right, but wouldn't the collision in turn lead to just reconnection? I mean there's a really small chance for the collision ever occur. For this happening:

  • tab T1 just parsed the cookie C1, and going to make the handshake request
  • at the same time tab T2 loads html and change the cookie to C2
  • tab T1 then passes the outdated cookie C1 in uri and new the cookie C2 in the header
  • server doesn't agree to handshake
  • tab T1 reconnects with the new cookie C2

For example I've tried to open multiple tabs in a browser, close the browser and reopen it back so all the tabs would request the page simultaneously and the issue won't occur.

I mean is it even worth to complicate the solution for in this case?

imbolc avatar Feb 14 '23 03:02 imbolc

if your not worried about it then I would not make it overly complicated. I do know one of the biggest challenges has always been tabbing. It was just to ensure that each part could fully function without being booted out due to another tab. Personally it should never reconnect you unless your switching to a new Secure Area.

For example

You connect to a website, A Session UUID is sent to you. You go to the login page and get a CSRF Token generated. You login using the CSRF Token the Session UUID is then Regenerated. You then go to enter a Admin Area and get a new CSRF Token. You login to the Admin Area and the Session UUID is then Regenerated. You then logout of the Admin Area, Session UUID is then Regenerated Again.

The only time the cookie UUID should ever change is when a user enters a new Secure Zone. This is to avoid others from hijacking the UUID Token and using it themselves. In which cases the CSRF should never invalidate unless it expires or you enter into a new Zone that requires the Sessions UUID to change. the UUID might not always need to be changed for normal Usage and if you have pages that you check for CSRF Tags you want to ensure all tabs can properly communicate.

Just during Regeneration you would want to clear out any CSRF Token's

genusistimelord avatar Feb 14 '23 12:02 genusistimelord

Personally it should never reconnect you

In case of websocket reconnection is rather mundane. Also, as I understand, the token is used only on the handshake phase, there's no need to keep it afterwards.

imbolc avatar Feb 14 '23 14:02 imbolc

The token method I mentioned isn't to Keep the token after its been used but rather Keep the token during the phases in which need it unless you otherwise change to a new Security Zone Or the Token Expires. We don't want to generate and hold onto these forever.

genusistimelord avatar Feb 14 '23 14:02 genusistimelord