kemalcr/kemal-csrf

Add CSRF protection to your Kemal application

kemal-csrf's Issues

Tests fail on Crystal 1.0.0

Applying this patch on shards.yml to let it compile with Crystal 1.0.0 make some tests fail:

diff --git a/shard.yml b/shard.yml
index 71562b6..4186bcf 100644
--- a/shard.yml
+++ b/shard.yml
@@ -4,7 +4,7 @@ version: 0.7.0
 dependencies:
   kemal-session:
     github: kemalcr/kemal-session
-    version: 0.12.1
+    version: ~>1.0.0
 
 development_dependencies:
   kemal:
@@ -14,6 +14,6 @@ authors:
   - Serdar Dogruyol <[email protected]>
 
 
-crystal: 0.35.0
+crystal: ">= 0.36.1, < 2.0.0"
 
 license: MIT

Crystal spec result:

Failures:

  1) CSRF allows POSTs with the correct token in FORM submit
     Failure/Error: client_response.status_code.should eq 404

       Expected: 404
            got: 403

     # spec/kemal-csrf_spec.cr:38

  2) CSRF allows POSTs with the correct token in HTTP header
     Failure/Error: client_response.status_code.should eq 404

       Expected: 404
            got: 403

     # spec/kemal-csrf_spec.cr:60

Finished in 3.19 milliseconds
9 examples, 2 failures, 0 errors, 0 pending

Failed examples:

crystal spec spec/kemal-csrf_spec.cr:20 # CSRF allows POSTs with the correct token in FORM submit
crystal spec spec/kemal-csrf_spec.cr:41 # CSRF allows POSTs with the correct token in HTTP header

With the same patch but using Crystal 0.36.1 all tests pass.

Use SameSite

SameSite: strict on a cookie prevent it to be sent by requests on other domains. This should probably be used, as I don't see any reason why we would allow requests from other domains (on a CSRF context), and it is supported by recent browsers.

edit : SameSite: lax is probably enough.

Can't install with kemal session

This is tied to kemal session 0.6.0 - so I can't install it.

I'll fork it to get around this. If it works I'll do a quick pull request on version numbers, unless you beat me to it.

Thanks.

Kemal-Session version

Kemal-Session version in the shard.yml is locked to 0.10.0 and is breaking because of it.

csrf cookie

In single page apps, the page hasn't always been refreshed when a session expires.
On the next xhr request the stored csrf token will be invalid, so will trigger a csrf error. As the user may have unsaved work, refreshing the browser is not ideal.

Th node csrf middleware sets a csrf cookie, which contains the csrf token. This is protected in the same way as the session cookie is, but can be read by js to send back as a header.

This article discusses it - https://www.jamesward.com/2013/05/13/securing-single-page-apps-and-rest-services

I'll add this feature to my fork and create a pull request. Let me know if this needs adding somewhere else as there was talk of merging this in with kemal session.

Thanks.

Allowed Routes

I need to allow specific routes to skip past a csrf check. This is for callbacks from a payment gateway.

I plan to add an allowed_routes array to the config object. What do you think?
eg.

add_handler CSRF.new(
  header: "X_CSRF_TOKEN",
  allowed_methods: ["GET", "HEAD", "OPTIONS", "TRACE"],
  parameter_name: "_csrf",
  error: "CSRF Error",
  allowed_routes: ["/api/confirm_payment"]
)

If there is already a way to do this I'm not away of I'd be happy to hear it. Thanks.

use kemal-session 0.12.0

shard.yml is still requiring kemal-session 0.10.0 when kemal-session 0.12.0 is released

CSRF with Multi Part forms

The csrf attribute isn't read with multipart forms.
I'll try and add this as a pull request if I can.

SecureRandom -> Random::Secure crystal 0.24.0

There is a breaking change in crystal 0.24.0, SecureRandom has been renamed.

I'll prepare a pull request when I get a minute, just thought it was worth raising the issue incase anyone is tempted to update and using this.

HTTP::Handler is not a class, it's a module

test.cr:

require "kemal"
require "kemal-csrf"

add_handler CSRF.new

get "/" do
  "Hello World!"
end

Kemal.run

> crystal test.cr

Error in test.cr:2: while requiring "kemal-csrf"

require "kemal-csrf"
^

in lib/kemal-csrf/src/kemal-csrf.cr:13: HTTP::Handler is not a class, it's a module

class CSRF < HTTP::Handler
             ^~~~~~~~~~~~~

What am I doing wrong?

Use httpOnly

I think HTTPOnly should be used on the cookie to prevent attacks like this one. Or is there a good reason to not do it?

Include this in kemal-session itself?

Is there any advantage to having this Handler as a separate shard and not including it in kemal-session itself? I can't imagine anyone who would say, "Well, actually, I would rather like to have sessions that are not CSRF-safe because I like to live dangerously" ;-).
This way with the separate shard it's just some extra complication for people who are new to Kemal.

How to insert the token in the form

I would like to know how to insert the token in the form:
<input type="hidden" name="_csrf" value="<%= env.session.csrf %>">
This doen't work.
Thanks.

The token is not reseted

There is actually a comment saying it is done, but the code doing it is commented, I guess because it is wrong.

I think the code should define a new token, put it in the session and in the cookie.

Is there any reason why there is this comment here?

CSRF clears the env settings

Before I included this library in my codebase I had an api endpoint

/api/login

that would check if username == Jacobsin and if password == password (Dont worry this isnt in production)

However after including the csrf handler I get a 403 code... Simple solve being that the postman request needs the csrf token however after I supply the csrf token and I print out the body of the request using:

username = env.params.body["username"]?
    password = env.params.body["password"]?

    if username.nil? || password.nil?
        puts username
        puts password
...

I get empty strings... Now postman shows they are in the body... So idk what is going on except the the env.params must be being destroyed?

Review CSRF security

Not so much an issue, as something I will try to do.

The below article recommends checking specific headers between the request response cycle, as well as other considerations to further improve security. I'll see if I use this info to improve this shard.

https://www.owasp.org/index.php/Cross-Site_Request_Forgery_(CSRF)_Prevention_Cheat_Sheet