Form tokens not required?

[carsten.klein@…]

I propose the following:

With each page delivered by the server, a token will be included as a cookie.

That way, the post processor, which will induce the form token to forms included in a page, will be made obsolete.

The token cookie should be made http only, and, in case of the https protocol, also be made secure.

So, upon processing a request, except for the initial request to the server, a token will be expected, that, if it does not match the previously issued token, will cause the server to except, e.g. http forbidden or something like that.

RATIONALE

in the current implementation, all forms will be assigned the same form token. this can be consolidated into a single token cookie, optimizing response time by not having to iterate over the genshi stream on every request-response cycle.

Embedded part applications, e.g. plugins/extensions that require different tokens should then either use their own mechanisms or set up a different cookie for their personal use.

What do you think?

[osimons]

We already have the token as a cookie. That is how the posted value is compared - by reading the token you have been assigned. We can't just have one of them - what would we check it against? It is much more expensive to store it in the database - with fetching, updating and invalidating logic.

I'm not sure what the actual performance cost of doing the form insertion is, but my suggestion would be to remove this functionality for all but 'anonymous' users. It I have logged in with a username and password, that should really be sufficient for letting me post a form.

A simple if req.authname == 'anonymous': #... token stuff... check for adding the token + checking it on incoming form data posts.

[Remy Blank]

Replying to osimons:

I'm not sure what the actual performance cost of doing the form insertion is, but my suggestion would be to remove this functionality for all but 'anonymous' users. It I have logged in with a username and password, that should really be sufficient for letting me post a form.

Please no! The whole point of this anti-CSRF scheme is to prevent a third party from using the session of an authenticated user to submit requests impersonating that user, not to authenticate requests.

See #4049 and ​Cross-site_request_forgery for more details.

[osimons]

Ah. Been a few years since this issue was active in Trac conext, so thanks for finding the info. Sounds like we can skip it for anonymous users then :-) Nah, seeing how keen my browsers are at offering creds even if not yet offered, that can never be safe either…

Oki. It all 'worksforme' then.

[carsten.klein@…]

Replying to osimons:

We already have the token as a cookie. That is how the posted value is compared - by reading the token you have been assigned. We can't just have one of them - what would we check it against? It is much more expensive to store it in the database - with fetching, updating and invalidating logic.

That is the reason why I suggested it in the first place.

And, I do not see any side effects regarding CSRF attacks, because the main request handler which then dispatches the request to the individual request handlers of the application, will already check for the availability of the either form token or the cookie. Since the cookie is resend by the client on every request, we can simply scrap the form token inserted for each form on a page.

This would actually give us a few cycles back on each request, especially for more complex pages with lots of content and lots of structural elements.

Reopening and will give it a shot evaluating with real life examples, e.g. browse tickets, browse repository etc.

[anonymous]

The main drawback I see is with AJAX requests, here, the COOKIE might not be included with the request, but then again, it would be against the rules set forth in the RFCs concerning HTTP and COOKIES in general which state that on every request to a given host, all available cookies need to be send back to the host with a few additional constraints concerning the path.

[Christian Boos]

I still have the impression that you don't fully understand the problematic as explained by Remy (comment:3).

If you are really convinced you could avoid CSRF attacks using a different scheme, then feel free to propose a patch (that we could shoot down ;-) ).

In my understanding, we might get away from the filtering if we would explicitly add the token each <form>, in the templates. But that would be error prone, at least until some kind of support for tag libraries (​#G395, which you're probably aware of ;-) ). Note that even in that case, people could still use <form> by mistake instead of <trac:form>, so it's probably not worth it.

[Remy Blank]

More specifically, you got the CSRF prevention trick backwards. We transmit a token as a cookie and as a form value, and the latter is mandatory, not the former. That is, an attacker trying to submit a form through CSRF must have to guess the value of the token and submit it with the form. Using only the cookie wouldn't prevent the attack, as it would be transmitted by the browser with every request anyway, same as the authentication cookie.

The cookie could be avoided by storing the form token in the session data, but as Simon wrote in comment:2, this would require a database access, which we would rather avoid.

So if the goal is to avoid using a stream filter, the only solution is to place the form token explicitly in all forms. Considering how often I forgot to import any(), all() or set() from trac.util.compat on 0.11-stable, I prefer having this taken care of automatically.

Here's a vote for "wontfix".

[Carsten Klein <carsten.klein@…>]

Replying to rblank:

More specifically, you got the CSRF prevention trick backwards. We transmit a token as a cookie and as a form value, and the latter is mandatory, not the former. That is, an attacker trying to submit a

You are totally right here, sorry for causing disturbance. Must have been out of my mind lately. Closing, this time for sure.

[Carsten Klein <carsten.klein@…>]

Replying to cboos:

(​#G395, which you're probably aware of ;-) ). Note that even in that case, people could still

Good thing you reminded me of that, needed to close it anyways since in practice it would be doable, but the effort outweighs the benefit. See the other ticket for more info on that.