Update: [CSRF] Improving the new Double Submit Cookie sections from #1110

[advename]

What is missing or needs to be updated?

My previous PR to fix #1110 reintroduced an overview of the HMAC CSRF Token and added a Naive Double Submit Cookie and Encrypt Cookie sections.

The PR is merged into main, but there are some improvements left I believe should be addressed.

How should this be resolved?

Improvement 1

Uncertain if the pseudocode should be marked with code. Maybe we should use python since it's close to python syntax, taking advantage of highlighting.

Improvement 2

Personally, the new "Signed Double Submit Cookie" section does still not provide full mitigation against CSRF attacks, since it doesn't involve by default the idea of using session-dependent value. I would suggest instead of the header "Signed Double Submit Cookie", to use "Session-dependent Signed Double Submit Cookie", since a "Session-independent Signed Double Submit Cookie" is only a mitigation technique when combined with Referer validation to protect against:

• Vulnerable subdomains or sibling domains
• Man-in-the-middle (MITM) attack
• Open redirect vulnerability

Actually, the "Double Submit Cookie" & "Signed Double Submit Cookie" pattern without a session-dependent value is only a defense-in-depth (DiD) technique.

Only the following two are true mitigation techniques:

• Signed, session-independent, CSRF Token is combined with Referer validation. This is the CSRF mitigation technique of Django, as they have always utilized a signed, session independent, Double Submit Cookie) which by itself without Referer validation is vulnerable to both previously mentioned attacks.
• Signed, session-dependent, CSRF Token

I'm uncertain how to include easily the idea of "session-dependent" value without overwhelming the less experienced developers.

Furthermore, the "Naive Double Submit Cookie" is a defense-in-depth technique and should probably be moved into the Defense in Depth Techniques section? This would mean we have to restructure the "Double Submit Cookie" to introduce "Signed Double Submit Cookie" as the default from the beginning.

Improvement 3

There is actually a bug I just noticed in the pseudocode. The request.setCookie() should actually be response.setCookie() since we set the CSRF cookie in the response.

[mackowski]

Awesome @advename I agree. I am not sure how much python will help in Improvement 1 but it is worth checking

[jmanico]

I think the double-submit cookie section is really "over built," and would love to see it simplified.

[advename]

@jmanico, do you have any suggestions? I mentioned in #1101 about Collapsible Sections? But how do you see Improvement 2 of my issue? Do you agree that naive Double Submit Cookie should be moved to DiD or what do you suggest?

[kiara-riley]

I like the new double-submit cookie section as someone that is attempting to learn about this. There are a lot of different approaches for different reasons and more information is certainly appreciated.

However, I have a question about the pseudocode. It sets a crsfToken with the sessionID in plaintext so that the server can verify the HMAC hash later. However, wouldn't this add a new vulnerability where the sessionID is now exposed to XSS attacks?

Wouldn't it be better to only expose the random value in the csrfToken and recreate the message with the sessionID from the (hopefully) httpOnly session cookie?

[jmanico]

I think the signed double-cookie submit is very confusing and would like it to go away, I think its very confusing and not necessary.

[jmanico]

I like the new double-submit cookie section as someone that is attempting to learn about this. There are a lot of different approaches for different reasons and more information is certainly appreciated.

However, I have a question about the pseudocode. It sets a crsfToken with the sessionID in plaintext so that the server can verify the HMAC hash later. However, wouldn't this add a new vulnerability where the sessionID is now exposed to XSS attacks?

Wouldn't it be better to only expose the random value in the csrfToken and recreate the message with the sessionID from the (hopefully) httpOnly session cookie?

If you care to submit a PR that removed that section, I'd take it!

[advename]

@jmanico I think the signed double-cookie submit is very confusing and would like it to go away, I think its very confusing and not necessary.

That's what I outlined in this PR, and am still waiting for feedback on PR 1149. Collapsibles should help us to make it more straightforward and "stow away" the extensive contexts.

---

@kiara-riley However, I have a question about the pseudocode. It sets a crsfToken with the sessionID in plaintext so that the server can verify the HMAC hash later. However, wouldn't this add a new vulnerability where the sessionID is now exposed to XSS attacks? Wouldn't it be better to only expose the random value in the csrfToken and recreate the message with the sessionID from the (hopefully) httpOnly session cookie?

I have not yet had the chance to see any XSS vulnerabilities because of the sessionID knowledge. Nevertheless, I agree. My notes, from which I wrote the new sections from, also don't send the sessionID in the CSRF Cookie, but only, as you point out, the random value. Would you mind opening a new issue or PR that fixes this in the pseudocode?

[jmanico]

Hey @advename can you give me a PR for one small thing at a time, maybe on PR for just the removal of signed double submit for start? It's a lot easier for me to process small changes as opposed to big design changes.

And when your collapsable is ready to go live let me know!

[advename]

@jmanico would love to. But i have still not received a reply to my previous questions.

[jmanico]

Hey @advename what questions are not answered? I don't have any thoughts on collapsable but am happy to answer questions about content. I don't think we need the signed cookie section at all, I'd rather just keep it simple and explain the basic ideas.

If there are pending questions you want me to answer, please ask me again here. I'm sorry to have missed them!

[siradam7th]

I like the new double-submit cookie section as someone that is attempting to learn about this. There are a lot of different approaches for different reasons and more information is certainly appreciated.

However, I have a question about the pseudocode. It sets a crsfToken with the sessionID in plaintext so that the server can verify the HMAC hash later. However, wouldn't this add a new vulnerability where the sessionID is now exposed to XSS attacks?

Wouldn't it be better to only expose the random value in the csrfToken and recreate the message with the sessionID from the (hopefully) httpOnly session cookie?

I was about to open an issue about this, then decided to see if anyone else mentioned it, so here I am.

As you mentioned, this way of generating a csrf token leaks the SessionID since it is recommended to set it to HttpOnly, but doing it this way, it makes such action useless, since it will be accessible by javascript easily from the csrf Cookie, by just spliting the csrf token string.

In summary, including the SessionID in the CSRF token generation by concatenating it defies the protections afforded by the actual SessionID cookie being set to HttpOnly.

[jmanico]

I also agree the sessionId derived token is a bad idea. Would someone like to remove this and submit a PR?