http-rs / tide

Fast and friendly HTTP server framework for async Rust
https://docs.rs/tide
Apache License 2.0
5.07k stars 321 forks source link

CSRF middleware #143

Open mozmark opened 5 years ago

mozmark commented 5 years ago

Feature Request

Detailed Description

I'd like to see tide have a feature for ensuring that requests are not forged from other origins. There are several mechanisms by which this can be achieved (and some of these are pretty robust).

Context

Security is hard. Sensible defaults and robust tooling mean that users can at least have a chance. There are several things that Tide could add to help users build secure applications, CSRF is a sensible first effort.

Possible Implementation

I don't have concrete ideas (yet) but I've talked to @yoshuawuyts who has some, I think.

jonathanKingston commented 5 years ago

@yoshuawuyts had a great suggestion of a test suite for generalised problems also. I don't know the feasibility of this but it certainly sounds worthy of exploring.

yoshuawuyts commented 5 years ago

We talked about this last week, and we ended up with some interesting points:

  1. it'd be great to have dedicated middleware per security concern (CORS, CSRF, etc.)
  2. then in turn those modules could be combined into a single "security" middleware
  3. once we feel the middleware is stable, we can merge it into Tide itself, and turn it on by default
  4. in addition having a test suite to automate security auditing would be great, and would allow testing the whole Rust async ecosystem (https://github.com/rustasync/tide/issues/143#issuecomment-469236092)

We've laid out a similar plan in https://github.com/rustasync/tide/issues/8#issuecomment-466456771 for logging. Proceeding with a similar approach for Security might work well also!


@mozmark @jonathanKingston what do you think of this approach? Are you interested in picking up any of this?

jonathanKingston commented 5 years ago

Sorry for the delay here, some of it was down to wanting to get used to Tide.

I think it makes sense to have a Response type specific to HTML that can accept DOMTree that way we have the ability to have hooks into rewriting it easily. For example we could randomly generate a nonce for script tags on the page easily solving XSS issues and also detect inline scripts and add them to the hash list for the CSP of that request. The same solution would be usable for nonces, if there was a simple way to get all the forms on the page the response type can inject the CSRF token into the form.

This could equally be done with HTML Strings too however requiring to parse the HTML or require the user to inject types is a little more cumbersome (and essentially why these flaws in security come about).

Is having a response type using DOMTree as a type a workable solution?

yoshuawuyts commented 5 years ago

@jonathanKingston I think that's a really cool proposal! -- it def feels like this is a good direction. A few thoughts tho:

  1. Tide probably wants to be generic over rendering frameworks. Rendering seems like something that people might have opinions about, and probably finding a common interface over multiple rendering backends seems like the right approach. This means that it'll be tricky to use something like a common DOMTree type.
  2. For https://choo.io we found that concatenating strings was about 100x faster than constructing DOM trees on the server-side. React found something similar. It seems likely that Rust rendering frameworks might not all want to construct DOMTree-like types on the server.

Perhaps it might be interesting to start by adding a layer that operates on HTML strings first, so it's compatible with any rendering framework, and we don't need to worry about compatibility issues. From there we can measure & consider how to improve it.

Does that approach seem good?

jonathanKingston commented 5 years ago

I agree with depending on DOMTree isn't agnostic enough, we could expose the CSRF token into the context via an extension.

Implementing String parsing likely will remove the performance benefits of pure strings. So the CSRF code will likely just have to rely on authors doing the right thing rather than have compile time types saving them from forgetting to tag a form.

I think the base API can expose the following for String usage:

async fn my_form_page(mut ctx: AppContext) -> String {
    format!(r#
      <form action="/boop" method="post">
        <input type="hidden" name="_CSRF_token" value="" />
...
        <input type="submit" class="submit" />
      </form>
    #, ctx.csrf_token())
}

And the equiv in DOMTree probably could be:

async fn my_form_page(mut ctx: AppContext) -> HTMLOutput {
    html!(<form action="/boop" method="post">
...
        <input type="submit" class="submit" />
      </form>)
}

For 2. I'm interested if the cost of type creation is as costly in Rust. For example bodils lib is compile time types using the macro so it should be just memory init of the types themselves that will be slower (we should run some analysis if possible as the compile time safety here gives us many potential security benefits with near zero cost if the consumer of tide is already using that macro).

Another idea: Potentially tide could support a pattern for build time created routes where authors don't have to pay the DOMTree creation cost at all and instead the build script flattens it into the most efficient template/route?

yoshuawuyts commented 5 years ago

@jonathanKingston oh, I like the approach you're proposing a lot. Given some magic string (e.g. _CSRF_TOKEN or format string {:csrf_token}) we can quickly do a find/replace on it!

Potentially tide could support a pattern for build time created routes where authors don't have to pay the DOMTree creation cost at all and instead the build script flattens it into the most efficient template/route?

We did similar things in Choo + Bankai, by keeping a hash-addressed LRU cache around to store individual templates. I feel this can be useful, but feels like a slightly different concern. I think given Rust deals with statics, and has support for const fns, a lot of the benefits caching introduces might already be happening during compilation.

I think before introducing caching we should probably measure performance to see how, and if we should model a design (:

lpil commented 4 years ago

Hello! Is there a recommended approach for this today? As a stop gap while the ideal API is being developed.

Thanks, Louis

malyn commented 3 years ago

I just published a tide-csrf crate that provides CSRF middleware (inspired by iron-csrf; I used the same csrf crate for the underlying primitives).

I hope this helps, and either way, please let me know if you have any feedback on the crate. This is my first real Rust project, and I am sure that there are some rough edges that more experienced folks could help me address.