yiisoft / yii2

Yii 2: The Fast, Secure and Professional PHP Framework
http://www.yiiframework.com
BSD 3-Clause "New" or "Revised" License
14.23k stars 6.91k forks source link

Unable to verify data submission - csrf token out of date due to additional request forcing a refresh #19351

Closed developedsoftware closed 2 years ago

developedsoftware commented 2 years ago

We are using yii/web/DbSession to manage our sessions. We are getting hundreds of "Unable to verify your data submission" since updating the framework to the latest version.

If we disable CSRF checks - the problem dissapears (obviously). If we set enableCsrfCookie to false in yii/web/Request the issue also dissapears once we clear our cookies.

From what I can tell the cookie value is getting the serialised output from the session table appeneded to the end of it.

At first I thought it was our implementation of the framework but after several hours of trying to pin it down I cannot seem to see anywhere in our code whereby we would alter this.

So now I am thinking it may be a framework issue. But please forgive me because I am not totally sure how I can explain how to replicate the issue.

Does anybody have any ideas that could start me off on the road of discovery?

Screenshot 2022-04-13 at 11 17 08 Screenshot 2022-04-13 at 11 17 03
bizley commented 2 years ago

Is the problem gone when switching back to the previous version? We need to exclude all not-Yii related sources of that issue first.

developedsoftware commented 2 years ago

Having reverted back to several previous versions the problem still persists. So that tells me its got to be my implementation of it. However, I do not alter the csrf cookie in any way and just use the out of the box implementation.

I couldnt figure out where the csrf cookie is set in the framework? If I can tinker with that I may be able to find something.

For clarification I should point out I am using nginx behind a reverse proxy running PHP 8.1.4

I have tried fiddling with max_upload_size and all the other php and nginx variables as found in other issues. But still no joy.

I think if I can stop the serialised data from the session table being appended to the csrf value in the cookie then this should be easy to fix (after almost losing the will the live yesterday lol)

bizley commented 2 years ago

The cookie is configured for the Request component so please check there. My bet would be on sameSite param.

developedsoftware commented 2 years ago

Have traced through the request component - all seems fine! Yet the request cookie csrf (in all browsers) appears to have this serialised data on the end of it.

Chrome encodes it so does not suffer with the issue.

Firefox Dev Edition does not and this is the main browser that is receiving the 400 errors looking at the user agent...

I have altered createCsrfCookie to dump out the value its setting - and it looks fine. But the browser includes this serialised string at the end. Perhaps its even a browser bug!? Wouldnt have thought so though...

bizley commented 2 years ago

This is the proper way of the csrf cookie to look like. We did not get any reports that this fails in the specific browsers so additional investigation is required.

developedsoftware commented 2 years ago
Screenshot 2022-04-13 at 13 51 42

I am fairly confident its the framework causing the issue. If I clear my cookies and send a request to Yii, the response cookies have the serialized data included which is then sent with the following request.

Trying to tap into where that gets set in the framework to see why its sending it with the response.

Yes I agree, it is very odd!

bizley commented 2 years ago

What was your previous Yii version where it worked?

developedsoftware commented 2 years ago

2.0.39 - however I have reverted back to that and the problem persists.

The only other difference in my environment to when it worked OK was that I am not on PHP 8.1.4 and before I was on PHP 7.4

I can roll back to 7.4 to see if the problem is solved? But I am pretty sure it was doing it on this particular browser long before we went to 8.1.4

I will keep digging. The fact that the response cookie is coming back mangled tells me either nginx is modifying it, or that the framework sending it to the browser.

bizley commented 2 years ago

If you can check it on 7.4 it would be great.

bizley commented 2 years ago

I'm not sure what I see exactly on your screens (the source of it) but for me it looks like your cookie is url decoded. If this is not prior to your action this could be the reason - Yii is trying to decode the cookie value and since it's already decoded you see the failed result of comparing two strings.

developedsoftware commented 2 years ago

I can confirm its the same with PHP 7.4. I can also confirm the issue happens on Yii 2.0.39 as well as the latest on both PHP 8 and PHP 7.4.

So I suspect the issue is probably with nginx?

I can confirm that in the below screenshot Yii does indeed set the csrf token to rkqsQGpOMJ4JbolNUgfFJXI25CtTzwyi. Confirmed by dumping out the value on screen.

However by the time it reaches my browser, the _csrf value has been prepended with 97e3be9038b0d7c45bacb5a795707fb427dc580ce1f124196eaa96ae133affbba and the original value url encoded like you say.

Screenshot 2022-04-13 at 13 51 42

I will now look at my nginx configuration to see if I can fix it. And then close this issue if it is indeed that!

developedsoftware commented 2 years ago

Does seem identical to this issue - of which it couldnt be replicated either... https://github.com/yiisoft/yii2/issues/18514

Will keep digging...

alex-code commented 2 years ago

The value before the serialised data is a validation hash https://www.yiiframework.com/doc/api/2.0/yii-web-request#$enableCookieValidation-detail

Does your server have separate install locations for the different versions of PHP? Just thinking if installing 8.1 updated the php.ini file so even downgrading to 7.4 still has the changes.

developedsoftware commented 2 years ago

Oh, so is that to be expeccted then? I do indeed have cookie validation enabled. That makes me feel better.

I am running inside a container, so no it would have been a fresh build with PHP 7 on.

Shall I disabled cookie validation to see if that solves it?

My6UoT9 commented 2 years ago

Check what happens there https://github.com/yiisoft/yii2/blob/master/framework/web/Request.php#L1745

developedsoftware commented 2 years ago

Success!

Disabling cookie validation caused everything to work as before!

@My6UoT9 -> that outputs a string such as this 0_ZU0ZbwczKNMos4OLZFPfVDKorkl4rJUwcB0ApbTU-qhg6m4MghfMF342J82XByliJM1YL-2p4naGKIWAwZGg==

I wonder if that is causing the issue?

bizley commented 2 years ago

Disabling it just simply hides your problem, and it is not recommended.

developedsoftware commented 2 years ago

@bizley I realise that, but it does allow our app to function as before.

Will try to figure out why that causes the issue. Could it be the (=) in the string?

bizley commented 2 years ago

Could you re-enable the validation and check this line on runtime because it's obviously false in your case and if confirmed could you provide us with the exact values of $value and $this->cookieValidationKey?

developedsoftware commented 2 years ago

I can confirm that $data = false

$value = 3cf6fotctth4342qtnfh4mjn7a $this->cookieValidationKey = 6256f45694fad

Cleared cookies and tried again....

$value = l52ipf9ej7jmkmqftb65fct2cg $this->cookieValidationkey = 6256f45694fad

bizley commented 2 years ago

Your $value is way too short. It looks like the cookie is never hashed before sending or somehow cookies are truncated when read. As I wrote before if anything is decoding the cookie value there will be a problem since a semicolon will not be encoded anymore and it's not allowed as a cookie value.

Could you please check the code in the lines I've mentioned and also provide us with your nginx config?

ailmanki commented 2 years ago

@My6UoT9 -> that outputs a string such as this 0_ZU0ZbwczKNMos4OLZFPfVDKorkl4rJUwcB0ApbTU-qhg6m4MghfMF342J82XByliJM1YL-2p4naGKIWAwZGg==

That is afaik the string you would want in the cookie, but what you have is something different. Since reverting to an older Yii version did not help, it's most likely something in your code. It seems you are assigning some cached content directly as CSRF token. I think the data looks like session data. So unlikely from nginx.

bizley commented 2 years ago

@developedsoftware I also wonder about the fact that there is once csrf and once _csrf on your screenshots.

developedsoftware commented 2 years ago

Your $value is way too short. It looks like the cookie is never hashed before sending or somehow cookies are truncated when read. As I wrote before if anything is decoding the cookie value there will be a problem since a semicolon will not be encoded anymore and it's not allowed as a cookie value.

Could you please check the code in the lines I've mentioned and also provide us with your nginx config?

Response.php

$value = Raptly::$app->getSecurity()->hashData(serialize([$cookie->name, $value]), $validationKey);
var_dump($value);

cf18c073a386f60b453f6d6f467b2ac6defcf9de7740a65074a0ca7a623790a1a:2:{i:0;s:5:"_csrf";i:1;s:32:"KO46kquCoTackCjpNnHgaK2y-bXyzrmJ";}

Looks good, and is reflected in the next request cookie...

Screenshot 2022-04-13 at 18 17 35

But the server receives the cookie value as follows...

Request.php

$data = Raptly::$app->getSecurity()->validateData($value, $this->cookieValidationKey);
if ($data === false) {
    var_dump($value, $this->cookieValidationKey); die();
     continue;
}

$value = h9mnsc93rmuiaoqfjedb1l11q3
$this->cookieValidationKey = 62570585b9533

However dumping out $_COOKIE gives me the expected value!!


array (size=2)
  'PHPSESSID' => string 'h9mnsc93rmuiaoqfjedb1l11q3' (length=26)
  '_csrf' => string 'cf18c073a386f60b453f6d6f467b2ac6defcf9de7740a65074a0ca7a623790a1a:2:{i:0;s:5:"_csrf";i:1;s:32:"KO46kquCoTackCjpNnHgaK2y-bXyzrmJ";}' (length=130)

UPDATE -> looks like PHPSESSID what causes $data to return false....

So I will run the same test above but for csrf only. But before I do this, would the fact that PHPSESSID is returning false - could that cause a new token to be generated? Or a new session?

developedsoftware commented 2 years ago

@developedsoftware I also wonder about the fact that there is once csrf and once _csrf on your screenshots.

Yes please ignore this, I was trying to remove underscores and special chars to see if it did anythng - i can confirm I am using the bog standard _csrf

bizley commented 2 years ago

So I will run the same test above but for csrf only. But before I do this, would the fact that PHPSESSID is returning false - could that cause a new token to be generated? Or a new session?

It is normal behavior, this PHPSESSID cookie should be ignored anyway. Please check if csrf cookie is passing the validation.

developedsoftware commented 2 years ago

Sure, thanks for corfirming. OK here goes...

Response.php

$value = Raptly::$app->getSecurity()->hashData(serialize([$cookie->name, $value]), $validationKey);
var_dump($value);

2b6ac333b03f740ee806597f0c82a6dc4942928600941232aed6fca1adbbf884a:2:{i:0;s:5:"_csrf";i:1;s:32:"2lOLs3uxeW0ug4cm7yqQxcBNVKu3xz10";}

Request.php


$data = Raptly::$app->getSecurity()->validateData($value, $this->cookieValidationKey);

array (size=2)
  0 => string '_csrf' (length=5)
  1 => string '2lOLs3uxeW0ug4cm7yqQxcBNVKu3xz10' (length=32)

Passes validation just fine. But the error persists. You'd think that would be fine right?

developedsoftware commented 2 years ago

OK, so heres some progress...

public function validateCsrfToken($clientSuppliedToken = null) returns false

var_dump([$clientSuppliedToken, $trueToken, $this->getBodyParam($this->csrfParam), $this->getCsrfTokenFromHeader()]); die();

array (size=4)
  0 => null
  1 => string 'LPtaA8Sb__iBXdgcr7z4KQOPJbrAsup8yG81gK4IWUUZuDs6paOTv-oa7HDdkahhWvtj_67tqT6ADg2x5l4-LA==' (length=88)
  2 => string 'kKQ4eW882-vTZT47jpLNkW6na5rq2YBkHAZ-HJwAZ6fY4HBKHkb23JIuXAj7zZmpIcZey7yUyQIpZDIq12cu1w==' (length=88)
  3 => null

$this->getCsrfTokenFromHeader() is null

Unsure what $trueToken and $this->getBodyParam($this->csrfParam) should return. But I would assume it should be the same value from the cookie (which it is not)

So my guess is that I need to pass the csrf header - so perhaps servers that always pass this header are masking the issue whereby cookie validation does not appear to work)

developedsoftware commented 2 years ago

Side note: I am running over https. Could the two values above be encrypted? Certaintly doesnt look correct to me...

developedsoftware commented 2 years ago

In Request.php, if I replace the following

public function getCsrfToken($regenerate = false)
    {
        if ($this->_csrfToken === null || $regenerate) {
            $token = $this->loadCsrfToken();
            if ($regenerate || empty($token)) {
                $token = $this->generateCsrfToken();
            }
            $this->_csrfToken = Yii::$app->security->maskToken($token);
        }

        return $this->_csrfToken;
    }

With the following, validateCsrfToken() then returns true and the problem is solved

public function getCsrfToken($regenerate = false)
    {
        if ($this->_csrfToken === null || $regenerate) {
            $token = $this->loadCsrfToken();
            if ($regenerate || empty($token)) {
                $token = $this->generateCsrfToken();
            }
            $this->_csrfToken = $token;
        }

        return $this->_csrfToken;
    }

So it appears that inside validateCsrfToken() a masked "true token" is being compared with an unmasked submitted token from either $clientSuppliedToken, $this->getBodyParam($this->csrfParam)or $this->getCsrfTokenFromHeader()

Does this help in any way?

bizley commented 2 years ago

$trueToken is taken from the cookie and $this->getBodyParam($this->csrfParam) is what has been sent with POST. In your case those two values are different after unmasking.

I've got hard time helping here since there is no clear cause of the problem:

The common cause of CSRF issues is that something is changing the token in the background, so when the original one is sent those two don't match anymore.

Anyway, I'm not sure if can help with the stuff so far, please try to pin-point the root of it.

alex-code commented 2 years ago

Would you be able to do a minimal site to see if that still has issues. See if it's a config thing or a server thing.

developedsoftware commented 2 years ago

Of course!

However, I have just spotted something on the login page - which may cause the issue I am experiencing.

On Firefox only, if a favico is not specified in the DOM it will try to load favicon.ico from the root on the site using its own FavIconLoader.jsm

We have rules in our app with pdf, css, ico suffixes (we generate the assets dynamically) so we did change the server config to only try files and then 404 if the url started with /assets/ (yii assets)

As favicon was being requested at root we was then, on the login page, hitting the login page again (because we redirect to it when unauthenticated) when firefox tries to load the favicon.ico - which would cause the csrf to renew (when does this happen by the way?). And then when we submit the original login form (with the csrf token in the form as a hidden input) this would obviously now be out of date and thus cause the 400 error.

Screenshot 2022-04-14 at 09 17 58 Screenshot 2022-04-13 at 22 08 06

I need to verify the above. I almost hope its not true because then I have wasted all of our time on a bloody favicon!!!

This also explains why we was only seeing this on Firefox user agents!

samdark commented 2 years ago

If that's true, we need to mention it in the docs :)

developedsoftware commented 2 years ago

When does the csrf token get refreshed ? Every single page load or just when a controller is accessed ?

It does appear to have solved the problem (no issues with the framework at all). The CSRF protection is working exactly as designed.

I was just showing a login page with an old csrf token because of the additional request by the favicon loader.

But i can’t work out when the token and under what conditions the token is refreshed…

WinterSilence commented 2 years ago

@developedsoftware if Request::$enableCsrfCookie is true, then token refreshed at every request. if Controller::$enableCsrfCookie is true, token validated before action.

proditis commented 2 years ago

I am trying to track this down, since ~2.0.14 :sob:, hopefully the following will help.

Our setups are very similar with the exception that we're using memcache sessions instead of db.

I have tried a myriad of fixes over time, including combinations of settings mentioned above with no success.

I've tried to reproduce with the favicon mentioned above, but all our tests, with all the browsers request the favicon last, so it didnt seem to be the reason that caused for the CSRF validation to fail for me.

For my setup, the error can be reproduced when a page with a form stays inactive for a short period of time (1440 seconds which is the default value for session.gc_maxlifetime). Upon submission of the form, after this many seconds of inactivity, we get a CSRF validation error.

I played around with the timeout session setting on web.php to test and was able to reproduce the error i was getting

        'session'=>[
          'name' => 'red',
          'timeout'=>10, <--- Set to 10 seconds for test
          'cookieParams'=>[
            'httpOnly'=>true
          ],
        ],

I have confirmed that setting timeout or gc_maxlifetime to 0 (not recommended) makes the problem go away.

There may be a way to also tune session.gc_divisor and session.gc_probability values to achieve a similar result without extending the lifetime of the session data, but these settings have no effect when memcache is used as session storage (not sure how these effect DbSession though)

I hope this helps in tracking down this beast :pray:

WinterSilence commented 2 years ago

@proditis

1440 seconds

and who fills form more one day? + by default, probability of deleting outdated session is 1% (gc_probability/gc_divisor).

most likely cause of this problem is AJAX POST requests to page from this page - something like autocomplete for fill form field

proditis commented 2 years ago

@proditis

1440 seconds

and who fills form more one day? + by default, probability of deleting outdated session is 1% (gc_probability/gc_divisor).

This is seconds 1440/60 = 24 minutes not hours. Also if you read my explanation there is no gc_probability or gc_divisor in effect when libmemcached is used as session storage (i've checked the source code). The only value that effects libmemcache expiration is the gc_maxlifetime

most likely cause of this problem is AJAX POST requests to page from this page - something like autocomplete for fill form field

Nothing to do with ajax or simple post, i've tested ALL possible combinations of settings and forms :rofl:

I can reproduce this error at will by simply changing the timeout value so i think what i've said before still stands.

edit: Confirmed that the same change also worked on another of my applications which is using DbSession

samdark commented 2 years ago

@proditis You seem to have enableCsrfCookie disabled, right?

proditis commented 2 years ago

@samdark

You seem to have enableCsrfCookie disabled, right?

Yes but only as part of the "fixes" i was trying throughout. Most of the "extra" settings got added as part of hunting down this error.

Snip from web.php

        'session'=>[
          'name' => 'red',
          'timeout'=>3600 * 12, /* <---- This is what i'm testing atm
          'cookieParams'=>[
            'sameSite'=> 'Strict',
            'httpOnly'=>true
          ],
        ],
...
        'request' => [
            'csrfParam' => '_csrf-red',
            'enableCsrfValidation' => true,
            'enableCsrfCookie'=>false,
            'csrfCookie'=>['httpOnly'=>true],
            'cookieValidationKey' => $cookieValidationKey,
            'parsers' => [
                'application/json' => 'yii\web\JsonParser',
            ]
WinterSilence commented 2 years ago

@proditis

The only value that effects libmemcache expiration is the gc_maxlifetime

you can change memcache.session_redundancy or look to custom session handlers in net.

Nothing to do with ajax or simple post, i've tested ALL possible combinations of settings and forms 🤣

many ways to generate this error, your version not 🥇

'enableCsrfCookie'=>false,
'csrfCookie'=>['httpOnly'=>true],

looks not so good

developedsoftware commented 2 years ago

My issue does appear to be resolved in that it IS an additional request that is causing the CSRF value to get changed on the server but not the client.

Sounds like your issue may be slightly different to mine. But one thing we can agree on is that it’s extremely frustrating.

But I do think the framework is OK and that it’s the browsers doing additional requests that cause this.

Perhaps doing the cookie check, header check and form input check in the same method is what’s confusing it?

WinterSilence commented 2 years ago

@developedsoftware Yii not refresh CSRF at GET requests

developedsoftware commented 2 years ago

That’s why I asked. Because $forceRefresh is set to true on my get requests !

WinterSilence commented 2 years ago

@developedsoftware where is $forceRefresh?

developedsoftware commented 2 years ago

Apologies, I meant $generate on function getCsrfToken

The favicon loader causes a 301 redirect to the login page (get request) and the csrf token is regenerated. Causing the token in the form hidden input to be out of date…

public function getCsrfToken($regenerate = false)
    {
        if ($this->_csrfToken === null || $regenerate) {
            $token = $this->loadCsrfToken();
            if ($regenerate || empty($token)) {
                $token = $this->generateCsrfToken();
            }
            $this->_csrfToken = Yii::$app->security->maskToken($token);
        }

        return $this->_csrfToken;
    }
proditis commented 2 years ago

@developedsoftware

My issue does appear to be resolved in that it IS an additional request that is causing the CSRF value to get changed on the server but not the client.

Oh thats good then. I used both browsers chrome/firefox just to confirm. Had the same results with both of them.

Sounds like your issue may be slightly different to mine. But one thing we can agree on is that it’s extremely frustrating.

Sorry didnt mean to highjack your thread, i really thought we were having the same issue. On your screenshot it seemed as if the favicon request was after the GET so i got confused there.

But I do think the framework is OK and that it’s the browsers doing additional requests that cause this.

I dont think its a framework issue either. I think its just a matter of documenting these details and particularly how certain settings effect each other.

In any case sorry @developedsoftware and devs for mixing my issue with this, I hope didnt cause too much confusion.

@WinterSilence What does session redundancy have to do with this? In any case as i said this happens with DbSession also.

WinterSilence commented 2 years ago

@developedsoftware I'm not sure, but in theory it should create cookie available only on login page redirect not existed file is incorrect behavior

WinterSilence commented 2 years ago

@proditis garbage collection does not affect session lifetime. I just pointed out to speed up/slow down GC.