Wrong Calculation of Session Timeout Can Lead to Constant Session Checks

[TB-effective]

Session timeouts in php.ini are seconds, browser extension assumes minutes

• Passbolt Version: 2.8.3/2.11.1
• Platform and Target: -- Browser: at least Chrome and Firefox -- Web server: nginx -- Windows and macOS

What you did

We increased the php.ini setting session.gc_maxlifetime = 43200 for a 12 hour lifetime (i.a. always stay logged in the full workday).

What happened

Browser plugins started to constantly do session checks, ~ 10 times per second for every active user, quickly filling up the DB. The action_logs table grew by several gigabytes (!) every day, due to the constant AuthCheckSession.checkSessionGet and SettingsIndex.index actions getting logged.

What you expected to happen

Session checks should have only happened at most every 12 hours for each user.

Additional information

After a quick look at the code in gpgauth.js, it looks like this assumes that the timeout is set in minutes, as the default is 24, and later on it gets multiplied by 60 to get seconds and 1000 to get milliseconds for the JS setTimeout():

GpgAuth.prototype.getCheckAuthStatusTimeoutPeriod = async function() {
// [...]
    // By default a default php session expires after 24 min.
    let sessionTimeout = 24;
// [...]
    // Convert the timeout in millisecond and add 1 second to ensure the session is well expired
    // when the request is made.
    timeoutPeriod = ((sessionTimeout * 60) + 1) * 1000;
  }

(I have also checked that sessionTimeout does in fact come from the php.ini value, this would appear to be happening in src/Controller/Settings/SettingsIndexController.php on the back end.)

However, this php.ini setting is in seconds already, see session.gc_maxlifetime docs. (That's why the default is in fact 1440, leading to the above 24 minutes.)

So the final result is too large by a factor of 60, which is the first bug.

This may seem like not a big deal, but it is once combined with this other bug: The result gets ultimately passed to JS's setTimeout() function, without checking if it's out of bounds.

The valid argument range for setTimeout() is not officially documented in any standard (to my knowledge), but is well known to be 32 bit signed, i.e. a maximum of 2 ^ 31 - 1 = 2147483647. See for example this SO question. It is also well known that passing any number larger than that will result in a timeout of 0 (or 1 according to some sources – in practice that doesn't make a difference) being set. The only JS engine that I have seen that actually documents this behavior is node.js.

In our concrete case of 12 hours, the final result is 2592001000, which is just outside that range. And the result was every client calling back 10 times per second.

We can reproduce that the problem goes away when decreasing the session.gc_maxlifetime value in php.ini to, say, 6 hours.

AFAICT, fixing either one of the two bugs should get rid of the drastic misbehavior (filling the DB quickly), but only fixing both will achieve completely correct behavior. (At least as long as someone doesn't set a GC lifetime of > ~ 25 days... ;-) )

[stripthis]

Thanks for the report @TB-effective, i'll investigate and work on a fix.

[squalou]

Hi, just to mention, exact same issue here. Set session.gc_maxlifetime = 43200, same misbehaviour.

[TB-effective]

@squalou: Workaround as long as they don't fix the bug: Set to anything <= 35791 (around 9.9 hours).

[stripthis]

@TB-effective @squalou you can also update to v2.11 which doesn't rely on the server settings anymore. I'll include a fix in the next release though.

[TB-effective]

Ah I wasn't aware. We are in fact on 2.11 by now, but haven't fiddled with the gc_maxlifetime any further since I hadn't seen any news on this issue. (And it wasn't urgent for us because of the existing workaround.) Thanks for the note.

[stripthis]

This is fixed with 2.12 (Chrome webextension is still in google purgatory though).

[TB-effective]

Thank you!