Problem with Re-using CSRF Token

ive done it like this

if($this->security->getSessionToken() == "") $this->view->token = $this->security->getToken();
else $this->view->token = $this->security->getSessionToken();

getSessionToken returns already set token for a session but getToken generates new one and "sticks" it to the session. Please correct me if this is wrong or can be done better

@Conradaek

Thanks for the input. I like your idea. Do you think its necessary to have both a token key and a token stored? I see a lot of people just using token.

In regard to the form reload problem, after thinking about it I dont think its something my end users would encounter anyway. Its mostly just something that affects me during development. I fill out a (long) form, I hit submit, but If I have an error in my controller method, I fix it and hit reload, but my token has expired, so I hit back and my long form has lost its values.

I usually throw an http exception if a token is invalid, like a 403 usually. Is that what others do?

Do you think its necessary to have both a token key and a token stored? I see a lot of people just using token.

I think it is enough if you use only token but I may be wrong. Using both tokens and token-keys is definietly NOT less secure than using tokens only :)

Hi! I have read some articles about the topic.

I was completely WRONG with my suggestion before. Huge sorry to everyone who took my advice : / I had no fully understanding of the CSRF issues.

So, what we really need to have a secure protection against CSRF attack is only to generate ONE token and use it for ENTIRE session.

1. You simply generate the token when user logs in to the application and store it in $_SESSION.
2. It does NOT matter how many tabs/forms the user have opened.
3. In each form you put hidden element with the same token.
4. Each time one of the forms is being sent you compare the token from hidden element to the token in session. If they match eachother you process the form.

DRY and KISS and so on:)

So Phalcon Security solution has completely no affect to user experience! ;)

Some post from stackoverflow I have read:

1. https://stackoverflow.com/questions/20057225/csrf-token-collisions-with-multiple-tabs
2. https://stackoverflow.com/questions/15829977/too-many-csrf-tokens-generated-php-how-do-i-deal-with-them?rq=1
3. https://stackoverflow.com/questions/10466241/new-csrf-token-per-request-or-not
4. https://stackoverflow.com/questions/20587746/one-token-vs-multiple-tokens-to-prevent-csrf-attacks
5. https://stackoverflow.com/questions/6724365/csrf-protection-question?rq=1
6. https://stackoverflow.com/questions/6929449/are-we-really-secured-from-csrf?rq=1

A quatation from official OWASP document:

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. If the token was not found within the request or the value provided does not match the value within the session, then the request should be aborted, token should be reset and the event logged as a potential CSRF attack in progress.

To further enhance the security of this proposed design, consider randomizing the CSRF token parameter name and or value for each request. Implementing this approach results in the generation of per-request tokens as opposed to per-session tokens. Note, however, that this may result in usability concerns. For example, the "Back" button browser capability is often hindered as the previous page may contain a token that is no longer valid. Interaction with this previous page will result in a CSRF false positive security event at the server. Regardless of the approach taken, developers are encouraged to protect the CSRF token the same way they protect authenticated session identifiers, such as the use of SSLv3/TLS.

Source: https://www.owasp.org/index.php/Cross-Site_Request_Forgery_%28CSRF%29_Prevention_Cheat_Sheet#General_Recommendation:_Synchronizer_Token_Pattern

@Conradaek I tend to agree and have read most of those articles and OWASP many times.... I do one extra step though. I have a session token unique to users, and in my forms I generate a form token. The form token is a hash generated based on user data and the specific form, so storing in session isnt even needed. It also has an expiration built into the hash.

I think the argument for having per-request tokens isn't justified. The method used to get the token (like an XSS vulnerability) makes getting the token trivial in any case, per-request or per-session.

Just my two-cents.

@jymboche, does your token change between sessions? I mean a token for specific user and specific form.

Well I have two tokens. A hash generated when the user logs in (stays the same for life of session), and one for forms. The form token is generated every request since it uses a timestamp in the hash. But since the timestamp and timeout are included with the hash, I can check if the hash is valid. Some people suggests storing tokens in session, and removing them or expiring them as they are used. This method is similar but doesn't require storing lots of tokens in session. All the info is encapsulated in the token itself. The idea came from this article: https://josephscott.org/archives/2013/08/better-stateless-csrf-tokens/

Heres a code sample. getUserKey() returns unique user data from database/session and hash() is just a generic modern hash method:

    public function getFormToken($str = "", $timeout = 900)
    {
        $hash_time = microtime(true);
        $key = $this->getUserKey();
        $toHash = $str . $hash_time . $timeout . $key;
        $hash = $this->hash($toHash);

        return "{$hash}-{$hash_time}-{$timeout}";
    }

    public function verifyFormToken($str = "", $token = "")
    {
        list($hash, $hash_time, $timeout) = explode("-", $token);

        if(empty($hash) || empty($hash_time) || empty($timeout)) {
            return false;
        }

        if( microtime(true) > $hash_time + $timeout) {
            return false;
        }

        $key = $this->getUserKey();
        $checkString = $str . $hash_time . $timeout . $key;
        $checkHash = $this->hash($checkString);
        if($checkHash === $hash) {
            return true;
        }

        return false;
    }