Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Filtering out safe methods and excluded paths #37

Closed
inmylo opened this issue Mar 21, 2017 · 2 comments
Closed

Filtering out safe methods and excluded paths #37

inmylo opened this issue Mar 21, 2017 · 2 comments

Comments

@inmylo
Copy link

inmylo commented Mar 21, 2017

Nosurf excludes safe methods (like GET) and paths (using ExemptPaths) when there is no need to check CSRF token. It's good, but..

In handler.go you have a function ServeHTTP, and after every request even if you are not interested in checking a token, you still do:

  • addNosurfContext(r)
  • w.Header().Add("Vary", "Cookie")
  • tokenCookie, err := r.Cookie(CookieName)
  • realToken = b64decode(tokenCookie.Value)
  • if len(realToken) != tokenLength { ...

.. and this all is useless because then you do:

if sContains(safeMethods, r.Method) || h.IsExempt(r) {
    // short-circuit with a success for safe methods
    h.handleSuccess(w, r)
    return
}

I offer you to move this check to the top of the ServeHTTP function as much as possible, so nosurf can avoid doing useless operations. Performance will be increased

@justinas
Copy link
Owner

Hi,

This is basically all intended behavior, as Exempt...() exempts paths from CSRF checking, but not from regenerating the token in case it does not exist at all or is of invalid format (see previous discussion).

Not regenerating a cookie would mean you would be unable to POST from an exempted or "safe" route to a protected route, unless there already exists a valid token cookie.

@justinas
Copy link
Owner

@Imilo Closing this for now, hopefully my answer was helpful. Feel free to submit any further inquiries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants