-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Add (more) CSRF protection #14
Comments
Tying this specifically to forms is a bad idea because it makes supporting Single Page Applications (SPAs) written in JavaScript more difficult and can break support for AJAX requests that use json/xml APIs. It's common for those applications to send the token in an HTTP request header instead of in the form body, especially if the body isn't a standard form encoded value (eg. JSON, XML), because it's easier to simply add a header before a request is sent than have to rewrite the DOM each time a request is made. A common way to handle this is to provide two code paths for the check determined by the presence and value of the X-Requested-With header. jQuery specifically, and other AJAX libraries, set the X-Requested-With header before sending a request and set the value to "XMLHttpRequest". In the case that the header isn't set or it's not that value defaulting to looking in the form payload for the specified value is the correct place to find the token. Otherwise, if the header is set and it denotes an xhr you look in the header for the token. It's also worth noting that a reasonable implementation should depend on sessions existing for storage of the token. I'm happy to help work on this if it's not already being done. |
I'd like to also have CSRF protection by default on when accessing the site with any mutating HTTP method, like POST, PUT or DELETE. At least check the origin + referer by default in these cases, and make it easy to additionally annotate routes that need checking, plus make it easy to have a HTTP header or cookie based check too. |
Is there anyone working on this? If not, I wouldn't mind having a go at it. I believe that checking for a header token and then falling back to the form body would cover most cases (80/20 rule). |
With a rather large sigh of grief, I'm pushing this to Implementing this in a simple, efficient, and straightforward manner has been a challenging endeavor. While it's simple to validate the incoming request against CSRF, it's not simple to emit a response with the appropriate information for later validation. In particular, we'd like to make it easy and efficient to insert a CSRF token into a web form in a template. To complete the defense mechanism, we need to insert the same token into a cookie. This presents a challenge: how do we know that a token has been inserted into a template so that we can then use the same token as a cookie in the response? I have two candidate solutions, each with a somewhat large drawback.
I believe that choice #2 above is a better approach. I'm against adding any kind of global state, though I'll note that this is precisely what other frameworks use to implement this kind of automatic CSRF protection. Any thoughts, suggestions, or ideas on this matter would be greatly appreciated. This is an important feature to have, and I'm excited to really nail down a good solution here. |
I'm not following why global state is needed for implementing the token system or why the design of 1 is being considered. Validation and generation can always happen at request time before a response with the template ever occurs. Typically the way most frameworks handle this is when a request comes in the framework determines if the request requires a token and if it needs to be validated. If not, it's simply a nop and the the request/response cycle goes through as expected. In the case where a token needs to be validated one of two events can happen:
You should never need to know a prior token's value and that information should not need to be kept around or generated at template generation time. Furthermore you also shouldn't need to expose a global, the template's scope just needs to include access to the session where the token can be taken out of it. |
As I said in my previous comment:
Validation is easy, as you've determined. As is generating a new token. The hard part (for us) is inserting that token in two places: a template and a cookie. We want to use a new token for each response requiring one, and we don't want to generate a new token unless we're going to eventually use it. Thus, generating a new token each time a request is received is a non-starter. You say:
But that's exactly part of the issue; how do you do that? You've already assumed that there's some global session that we can simply refer to: there isn't. We also can't simply inject information into a template's scope: the user specifies the scope. It is conceivable that we can extend existing template engines with a second, framework provided scope. This would be equivalent to my second proposal except with information flowing the inverse direction. Let me try to be as clear as possible. In a template, a user should be able to write something like: <form ..>
<!-- in practice, a form helper will generate the form tag AND the token field -->
{% csrf_token %}
</form> Rocket should generate the CSRF token hidden field from the
In my proposal, |
I understand the issue now. One suggestion I have, and this may not be the popular opinion or desired behavior, is to not try and push the token injection into the templates as part of rocket. I think unless rocket is dictating the template/rendering engine. The reason I say this is a couple reasons: First, There's going to be a lot of variation and support needed as well as buy in from all the developers working on the engines. Second, The updating of the tokens itself is going to be very app specific and going to be difficult to impossible to handle without providing a frontend js component like Rails does with it's UJS. If the application is using xhr the template may not be re-rendered and rotating the token on the server side (especially if it wasn't validated) will immediately break the page. In my day job we had to solve this problem for apps that use our security plugin and what we did was append some JavaScript to the end of the any responses that have a handful of characteristics (content-type, etc.). This puts a small wrapper around the xhr code as well as adds an event listener into the dom and delegates to a handful of elements (submit, a, button) to add tokens into xhr requests from a cookie do some clever dom manipulation to drop the token into forms before the request is submitted. This let's us avoid parsing the response and manipulating it to try and put the tokens into the forms. The parsing wont be an issue for Rocket because the location is going to explicitly be set for where insertion is needed but this would solve the issue of tokens updating and the dom not being rewritten. This solution is sort of out of the scope of Rocket as a rust based web framework but it's a solution that we've found solves some of these problems well. |
@SergioBenitez May I ask why? Generating a token is easy. It should be as simple as grabbing a few bytes from an optimized implementation of ChaCha20 or (if AES-NI is available) AES256-CTR. Much, much less effort than the upstream proxy server spent decrypting the incoming HTTPS request. Regarding sessions: Rocket already supports authenticated encryption of cookies. That means that we can store the token client side without any security loss. Client side code can still read the token, but can’t tamper with it without being detected (resulting in a Finally, just checking |
@DemiMarie so I'm sure that Sergio will have more to add to this but it's not so much the generation that's difficult but the submitting the tokens with requests. The problem becomes getting the token into the DOM for form based requests and into XHR requests. If the goal is to provide server side templating with support for csrf tokens (which it appears this is a goal) the hard part is getting the token into the template and updating the template. Rails solved this by providing some helpers with global state on the server side rendering end and some additional JS that was part of their UJS package. Global state is undesired in this case which complicates the problem significantly. One solution that might be worth looking at is providing two separate pieces of middleware. One to handle generation/validation of tokens as well as adding them into responses for JS SPA frameworks, such as angular, to use that already have built in support for this. Then provide a second middleware or component that adds support for the helpers on the server side rendering. If you're looking for more ideas I spoke at DEF CON this year about a solution I outlined above that included some JS that is injected to provide support client side. It seemed that there wasn't interest in a solution like this but just to offer more options. https://www.youtube.com/watch?v=foFf4gMbL2w |
@jrozner What about having some sort of per-request state? I really really really don’t want the solution to be dependent on JS. I personally use NoScript, for instance, and believe that a simple form should not require JS to submit. There are also accessibility issues, if I recall correctly, though I don’t remember what they are. |
@DemiMarie The issue isn't so much about the state at the handler point but providing the state to the templating engine in a way that the templating engine can actually use it. The state in sessions is probably fine at the handler level but getting that into a template is more complicated. Part of the problem is attempting to solve this for all cases and Rocket not being opinionated allowing you to bring basically any templating with you or bring none (eg. no server side rendering and returning JSON). I realistically don't see a possible solution that isn't split into multiple parts because a non-js dependent solution that allows for server side rendering but also for SPAs (or at least requests that don't re-render) is incompatible and solve very different problems. One problem is generation, validation, and storage of tokens while the other is insertion of tokens into requests and updating tokens after responses. Supporting arbitrary templating engines isn't really a csrf problem but is required to be solved for server side rendering in Rocket and providing necessary support. Either adapters likely will need to be provided for specific templating engines to provide a common interface for this or maybe an alternative solutions. What I'm proposing here is build the functionality for dealing with generation, validation, and providing tokens in responses so this becomes available now for a large majority of users and applications that don't have the specific requirements you do. At this point, whether you like it or not, NoScript is not viable for most parts of the web and in most cases JS based SPAs have won and their frameworks handle the token insertion for you. |
Unless you've got some objective research here, I'm going to say we're both suffering from selection bias. I certainly don't see them as having won, nor do I see NoScript as being non-viable. |
@ssokolow I'll cede to that and accept that you might be right here. Given trends in web development I doubt it but it's very possible. However, the earlier point about separate problems I think still holds and is relevant. |
It appears that checking Origin & Referer are sufficient, except when:
|
Here are another two reasons which CSRF protection via the Referer header is inadequate:
|
@seanlinsley suggesting that it's reasonable to simply have an application not work for users behind a proxy is a pretty concerning stance. Especially if the application can't explain why it's not working for the user. We shouldn't make users choose between security and functionality when there are solutions available that don't require that. |
@ssokolow could you link to such browser extensions? Is there data to suggest that they block the referer header indiscriminately on any website, even when navigating from page to page on the same domain? (as opposed to stripping the header once it becomes a cross-domain request) I'm not sure I understand the argument in point 2. Could you elaborate? @jrozner AFAICT proxies in general don't remove the header unless specifically configured to. This is entirely explainable to end users. For example:
|
Firefox has it built in through these
...though I prefer to use an extension like RefControl (legacy) or Smart Referer (WebExt) which implements a whitelist similar to NoScript. (RefControl, which I've used for years and which I'll continue to use until I get off Firefox 52ESR, allows you to choose between Block (no referrer), Forge, and Allow, both for the default policy and for per-domain rules... including ones which only apply to 3rd-party requests.) For Chrome, you either launch it with |
That still means that some users behind corporate proxies cannot use the
app. This is bad!
…On Nov 25, 2017 8:06 PM, "Sean Linsley" ***@***.***> wrote:
@ssokolow <https://github.com/ssokolow> could you link to such browser
extensions? Is there data to suggest that they block the referer header
indiscriminately on any website, even when navigating from page to page on
the same domain? (as opposed to stripping the header once it becomes a
cross-domain request)
I'm not sure I understand the argument in point 2. Could you elaborate?
@jrozner <https://github.com/jrozner> AFAICT proxies in general don't
remove the header unless specifically configured to.
This is entirely explainable to end users. For example:
Unable to verify your request. Please try again after turning off privacy
extensions in your browser, or moving to a different internet connection.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#14 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AGGWBzBez85iSeV0cCq88lBLL-AcQHrQks5s6LmqgaJpZM4KF2dV>
.
|
...as for point 2, my argument is that I don't think it's robust enough to rely exclusively on headers for CSRF protection and the OWASP document seems to agree. The whole point of the token-based approach is twofold:
Heck, I watched a talk where one of the takeaways was "Don't assume your design's custom fonts will load. There are still corporate proxies that don't have headers like |
FWIW the above-linked Smart Referer browser extension only removes the header when navigating cross-domain. From my point of view, if you have corporate customers you can either afford the budget to write a ton of custom code, or you can get them to whitelist your site so the headers aren't stripped. Given @SergioBenitez's desire not to add global state, and the complexity involved in building a common API for multiple possible template engines, this seems like a reasonable solution that will work for almost all users. The perfect is the enemy of the good, etc. |
I think that global (well, request-local) state is not a big deal.
…On Nov 26, 2017 2:09 PM, "Sean Linsley" ***@***.***> wrote:
FWIW the above-linked Smart Referer browser extension only removes the
header when navigating cross-domain.
From my point of view, if you have corporate customers you can either
afford the budget to write a ton of custom code, or you can get them to
whitelist your site so the headers aren't stripped.
Given @SergioBenitez <https://github.com/sergiobenitez>'s desire not to
add global state, and the complexity involved in building a common API for
multiple possible template engines, this seems like a reasonable solution
that will work for almost all users. The perfect is the enemy of the good,
etc.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#14 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AGGWB5HbNAnGBcvnsl2PZa7UMxDGVFCOks5s6bdogaJpZM4KF2dV>
.
|
I've started working on some CSRF protection: https://github.com/kotovalexarian/rocket_csrf It is not safe enough maybe because it doesn't encrypt authenticity token included in forms. I'm not sure why it's needed to encrypt it but Ruby on Rails does this. Also my implementation doesn't provide any template helpers. However it may be a good working prototype to see how CSRF protection can be done in Rocket in idiomatic way. Tell me if it is non-idiomatic. I'm ready to transfer crate name ( Here is an example: #![feature(decl_macro)]
#[macro_use] extern crate rocket;
#[macro_use] extern crate serde_derive;
use rocket::response::{Flash, Redirect};
use rocket::request::{FlashMessage, Form};
use rocket_contrib::templates::Template;
use rocket_csrf::CsrfToken;
#[derive(Serialize)]
struct TemplateContext {
authenticity_token: String,
flash: Option<String>,
}
#[derive(FromForm)]
struct Comment {
authenticity_token: String,
text: String,
}
fn main() {
rocket::ignite()
.attach(rocket_csrf::Fairing::new())
.attach(Template::fairing())
.mount("/", routes![new, create])
.launch();
}
#[get("/comments/new")]
fn new(csrf_token: CsrfToken, flash: Option<FlashMessage>) -> Template {
let template_context = TemplateContext {
authenticity_token: csrf_token.0,
flash: flash.map(|msg| format!("{}! {}", msg.name(), msg.msg())),
};
Template::render("comments/new", &template_context)
}
#[post("/comments", data = "<form>")]
fn create(csrf_token: CsrfToken, form: Form<Comment>) -> Flash<Redirect> {
if let Err(_) = csrf_token.verify(&form.authenticity_token) {
return Flash::error(
Redirect::to(uri!(new)),
"invalid authenticity token",
);
}
Flash::success(
Redirect::to(uri!(new)),
format!("created comment: {:#?}", form.text),
)
} |
Thank you for working on this! :) One improvement could be to create an attribute-like macro which takes care of the token verification and the early return such that it gets harder to introduce errors. Or maybe |
I am not sure about the token encryption, but I'll do some research and get back to you |
This is mentioned in TODO. It requires data guard because usual guards can't extract form data from request AFAIK. Anyway, thank you for mentioning this, I'll try to focus on this task first. |
I think this may still require #775 |
Yes. I just found that only single data guard can be used in a route. So there is no way to avoid authenticity token validation code in a route for now. |
@kotovalexarian it was possible to peek into first bytes of request data and read token value this way, but it's impossible in current master. See #1438 |
Since Rocket doesn't have built in CSRF support yet and this hasn't been mentioned in the comments yet, it's worth noting that a ready-to-go fairing already exists with Plume's csrf_rocket. It can auto-inject CSRF tokens for you into all routes (along with blacklisting, etc) and can handle CSRF verification for you - alternatively, you can use the fairing just for it's CSRF guard and do the injection yourself. I currently have a private fork of my own that relies on the checking mechanism while updating the crypto -- I accidentally converged on Plume's solution after realizing the tradeoff between using Rocket's built in form support and defining my own data guard for CSRF -- but the fairing as it exists is very good and should work as a stopgap until Rocket has built in CSRF utilities. |
As the main developer of rocket_csrf, I'd recommend against using the auto-injecting feature, while it works most of the time, it has history of corrupting content. It is fixed as far as I know, but still quite fragile. |
Hi all, Is this a good way of doing ? Thanks in advance, |
This is long past due. Rocket.rs claims to be ready for production. There's two libraries for CSRF protection so far. I'm hesitant to use both since I'm making a youtube tutorial on how to use templates and forms in Rocket.rs and the last thing I want to do is give people improper information. kotovalexarian/rocket_csrf says the project may not be ready for production due to a simple implementation and Trinitiy's Plume-org/rocket_csrf is undocumented. The way I see it, Rocket explicitly refers to Flask and even support Tera which is inspired from Jinaj2. I really hope someone can implement the Rocket csrf library without a disclaimer. If the implementation might take only a couple days I might try my hand at it. I also found https://www.stackhawk.com/blog/rust-csrf-protection-guide-examples-and-how-to-enable-it/ but it's so maybe there might not be a need to create a new library. |
Note that Rocket readily and automatically leverages the |
I've made some progress on this issue and have implemented the bulk of an advanced CSRF token mechanism for Rocket. As far as I can tell, it offers a level of security and usability unmatched by other frameworks. You can get an idea of how things are shaping up by reading the docstring in Please note that this is still incomplete and cannot yet be used by Rocket applications. What's implemented so far are the core cryptographic operations and mechanisms. What's left is to tie that to Rocket. Furthermore, I would like to thoroughly review the cryptography and security-sensitive components before publishing this. |
Basic recommendation on AEADs: instead of using a randomly-generated nonce, generate a per-process subkey (using HMAC or Blake2b) and then use an incrementing nonce for AEAD encryption. That allows the key to be shared across many machines without needing to synchronize state. Alternatively, it is safe to use XChaCha20-Poly1305, which uses a 192-bit nonce that can be randomly generated each time. |
The key used for the AEAD encryption for CSRF sessions uses the |
How long is the nonce? 96 bits isn’t enough for a random nonce, but 192 bits is. |
The recommended length of the nonce is determined by the underlying algorithm, in our case AES-256-GCM, which recommends a 96-bit nonce. The RustCrypto libraries enforce using a nonce of this size, so no other option is available to us. Irrespective, we could improve our use of AEAD by using a more nonce-resistant algorithm, like AES-256-GCM-SIV, adding a deterministically increasing component to the nonce, or both. Or switching the algorithm to XChaCha20-Poly1305 which as you suggest, gives us a 192-bit nonce. Finally note that none of this happens in Rocket or as part of this CSRF mechanism but rather in |
This makes AES-256-GCM unsuitable for this application, unless subkey derivation is used: generate a new AES-256-GCM key on each machine, using a KDF and a per-machine high-entropy nonce. I’m inclined to declare this a vulnerability in |
This is exactly what is done. |
Laravel's CSRF protection is incredibly well-implemented; it's enabled by default and integrates seamlessly into any form/page meta/headers. I'd really love to have the same level of protection in Rocket by default, without any extra hassle. It's the only thing stopping me from using Rocket. I'm using Laravel instead of Rocket because I'm very concerned about security and I don't want to rely on an unofficial solution for CSRF protection. I expect it from the framework. Thanks! |
CSRF is a thing. Protect against it automatically in forms.
The text was updated successfully, but these errors were encountered: