Echo: HttpOnly cookie of CSRF token

[yaa110]

As mentioned in Wikipedia:

An HttpOnly cookie cannot be accessed by client-side APIs, such as JavaScript.

I updated the echo to last version and now the csrf cookie is HttpOnly enabled. Thus it is not possible to get data using AJAX.

[vishr]

Yes this was changed to fix a bug in CSRF. You probably need to get CSRF token from the context and pass it down to the client via template or response header.

[yaa110]

Thank you @vishr, but it makes it difficult and a little buggy to pass cookies via response header. As you know Django does not use HttpOnly for the csrf token. What bug was fixed using the change?

[vishr]

https://github.com/labstack/echo/issues/597

[yaa110]

Thanks for the link, there is one problem with passing token via response header. Attackers could read the token from response header rather than the cookies. So, I think it is not a good idea to make the cookie HttpOnly as Django did not do the same because it does not address the security issue for sites using AJAX.

Isn't it better to let users change HttpOnly via CSRFConfig?

[vishr]

Getting access of the token is not a security issue. We rely on readonly CSRF token cookie to cross check it on the server during validation.

[yaa110]

@vishr, I tried to get the token using c.Get("csrf").(string) to pass it via view, but its value is different than the token saved in the cookie?

The problem is the token is changed on every Get request including STATIC file request.

[vishr]

@yaa110 Can you provide code to debug?

[yaa110]

type Page struct {
    CsrfToken string
}

e.Static("/", "public")
e.GET("/test", controller.TestGet)

func TestGet(c echo.Context) error {
        view := &Page{
                CsrfToken: c.Get("csrf").(string),
        }

        // the `test` template includes static images, css, etc.
        return c.Render(http.StatusOK, "test", view)
}

I also tried the following code:

func TestGet(c echo.Context) error {
        view := &Page{
                CsrfToken: getCsrfToken(c),
        }

        // the `test` template includes static images, css, etc.
        return c.Render(http.StatusOK, "test", view)
}

func getCsrfToken(c *echo.Context) string {
    csrfToken, err := (*c).Cookie("csrfToken")

    if err == nil {
        return csrfToken.Value()
    }

    return ""
}

[vishr]

I used the following code for testing.

package main

import (
    "github.com/labstack/echo"
    "github.com/labstack/echo/engine/standard"
    "github.com/labstack/echo/middleware"
)

func main() {
    e := echo.New()
    e.Use(middleware.CSRFWithConfig(middleware.CSRFConfig{
        Secret:      []byte("secret"),
        TokenLookup: "form:csrf",
    }))
    e.GET("/", func(c echo.Context) error {
        return c.HTML(200, `
            <form action="/" method="POST">
                <input type="hidden" name="csrf" value="`+c.Get("csrf").(string)+`">
                <input type="submit" value="Save">
            </form>
        `)
    })
    e.POST("/", func(c echo.Context) error {
        return c.String(200, "Hello, World!")
    })
    e.Run(standard.New(":1323"))
}

[yaa110]

@vishr, your example should work without any problem because it does not contain any static request (such as CSS and image files). GET request of each STATIC file is the culprit. Because the token is changed after each request for static files. In the handler function of router, c.Get("csrf").(string) returns the current token, but after loading the page, the token (cookie) is changed for each static file request.

[yaa110]

Isn't it better to disable HttpOnly and save and validate generated cookies via simple and fast NoSQL databases like Tiedot or LedisDB. This approach helps users access cookies via Javascript and also addresses the security issue.

[remylab]

Or you could you could store the serverToken in a session and only renew the value when the session expires. As suggested by wasp :

In general, developers need only generate this token once for the current session. After initial generation of this token, the value is stored in the session and is utilized for each subsequent request until the session expires. When a request is issued by the end-user, the server-side component must verify the existence and validity of the token in the request as compared to the token found in the session.

Src: https://www.owasp.org/index.php/Cross-Site_Request_Forgery_(CSRF)_Prevention_Cheat_Sheet#Synchronizer_.28CSRF.29_Tokens

[remylab]

Same logic Works with cookies too..

[yaa110]

A workaround to the issue of STATIC files is to exclude Path of root directory of static files in csrf.go provided by CSRFConfig.

For example in csrf.go:

1- Add ExcludedPaths to CSRFConfig struct:

type CSRFConfig struct {
    // ...
    ExcludedPaths []string
}

2- Check if the request path is excluded:

func CSRFWithConfig(config CSRFConfig) echo.MiddlewareFunc {
    // ...
    return func(next echo.HandlerFunc) echo.HandlerFunc {
        return func(c echo.Context) error {
            // ...
            case echo.GET, echo.HEAD, echo.OPTIONS, echo.TRACE:
                if isPathExcluded(c.Path(), config.ExcludedPaths) {
                    return next(c)
                }
            // ...
        }
    }
}

func isPathExcluded(path string, excludedPaths []string) bool {
    for _, p := range excludedPaths {
        if p == path {
            return true
        }
    }
    return false
}

then users can exclude their static root directory:

func main() {
    e.Static("/", "public")

    csrfConfig := middleware.CSRFConfig{
        ExcludedPaths: []string{
            "/*",     // The asterisk symbol is important
        },
        // ...
    }

    e.Use(middleware.CSRFWithConfig(csrfConfig))
}

[vishr]

I just pushed a fix on https://github.com/labstack/echo/tree/issue-600. Please check.

[yaa110]

Thank you @vishr, it works, but what if the cookie is expired between the request of main page and one of its static file (I know it is very rare but it is possible)?

[vishr]

That means the token is expired, moreover the token verification is done only on POST DELETE like methods.

[yaa110]

@vishr, please consider the following scenario:
1- User requests the "/login" at time A
2- The handler function renders its template and passes the current token via view using c.Get("csrf").(string) at time B
3- The cookie (or token) expires (at time B + epsilon)
4- The rendered view contains STATIC files such as CSS files, so it sends GET requests at time C
5- Since the cookie (or token) has expired at time B + epsilon, a new cookie is generated which I can't access it in the pre-rendered view
6- The login form has a hidden input for csrf token with the value passed in step 2
7- However, the token has changed after a static request in step 5
8- The submission of form will be failed.

There should be an approach to exclude _static files GET requests_ from token generation, as I previously suggested.

[vishr]

I think if you are using static middleware on top, it will never reach csrf.

This is a rare case possible with insane short cookie expiry time or requests made during expiry. Just curios to know how other implementation handle it.

[yaa110]

I think if you are using static middleware on top, it will never reach csrf.

No, I've used static router on top and it changed csrf.

I checked Gorilla/CSRF:

• HttpOnly is optional
• Renews token if only it was expired
• It Enforce an origin check (referer) for HTTPS connections
• It sets the Vary: Cookie header to protect clients from caching the response.

[vishr]

HttpOnly is optional

I have set it to true as I don't want client to modify it. But I think if cross-domain cookie modification is not-allowed, we are good. (Just double check)

Renews token if only it was expired

We do it now

It Enforce an origin check (referer) for HTTPS connections

Not doing

It sets the Vary: Cookie header to protect clients from caching the response.

Will do

[yaa110]

I checked Django implementation of csrf and I'll open a new Pull Request based on that.

[vishr]

How does django csrf handle the case you mentioned earlier?

[yaa110]

it sets the CSRF cookie even if it's already set, so they renew the expiry timer (i.e. it does not change the token but renews the timer for every request)

[vishr]

Does that mean the token never expires?

[yaa110]

No, token expires when users leave the site (send no requests) for a long time (longer than the expiration time).

The final implementation should be as the following:

• For safe requests (GET, HEAD, etc.) renew the token only if there is no valid token in the cookie.
• For each unsafe request (POST, DELETE, etc.) renew the token.
• For all requests (if there is a valid token) renew the timer of the token (not the token itself).

[vishr]

Let me take a stab at it. You can review it?

[yaa110]

Off course, I do.

[yaa110]

Dear @vishr, I also opened a new pull request #602 at which the CSRF was implemented based on Django implementation.

[vishr]

@yaa110 I took changes from your PR and applied to mine. Please verify https://github.com/labstack/echo/tree/issue-600

[yaa110]

Thank you @vishr for the fix and sorry for the late reply. Just a few more things to go:

• the token should (must) be regenerated after successful unsafe requests such as POST (after line#136).
• it is not a good idea to pass error messages to the client in production mode (line#135).
• it is better (more performant) to move csrfTokenFromHeader to the default case of switch, because when users used form or query for TokenLookup there is no need to extract token from the header (line#104-110):

var extractor csrfTokenExtractor
switch parts[0] {
case "form":
    extractor = csrfTokenFromForm(parts[1])
case "query":
    extractor = csrfTokenFromQuery(parts[1])
default:
    extractor = csrfTokenFromHeader(parts[1])
}

[vishr]

the token should (must) be changed after successful unsafe requests such as POST (line#136).

I am interested in knowing the reason. I did not see that in other implementation including django.

it is not a good idea to pass error messages to the client in production mode (line#135).

Error of type HTTPError are meant for the clients across the project. All other errors are filtered in the default error handler.

it is better (more performant) to move csrfTokenFromHeader to the default case of switch, because when users used form or query for TokenLookup there is no need to extract token from the header (line#104):

It is not an actual function call but just a return of internal function (closure)

[yaa110]

I am interested in knowing the reason. I did not see that in other implementation including django.

I checked Django again, they regenerate the csrf token only after login, so we need a public method to reset csrf manually.

It is not an actual function call but just a return of internal function (closure)

I know it, but it is not necessary, is it?

[vishr]

I don't think we have a way to expose a function from middleware.

I know it, but it is not necessary, is it?

To be honest, I don't like initializing via var, I prefer := unless there is no way out and moreover it wasn't a performance issue in this case, lets leave it like that.

[vishr]

The current implementation has improved a lot. Do we really need it now?

[yaa110]

so we need a public method to reset csrf manually.

We do not need it, we could delete the current token after login.

[vishr]

Cool.

[yaa110]

The current implementation has improved a lot. Do we really need it now?

No we don't, the current implementation is good and safe, Thank you.

[siredwin]

@vishr , why is e.POST not secure? On a comment commented on Jul 15, 2016 your e.POST did not carry any csrf data. Is that the way it is supposed to be setup? My POST methods definitely fail and i am wondering if i should split it into 2 parts like yours for POST & GET?