Module may fail to destroy expired sessions

[brynwhyman]

Note: The following is extracted from a commissioned report by Insomnia Security.

Technical details

The functionality of the SilverStripe DynamoDB module is not accurately documented and, without additional configuration, expired sessions may accumulate in the session table.

The SilverStripe DynamoDB module exists to replace the default session PHP handing with an AWS DynamoDB backed session store. By default, on Debian based systems an operating system ‘cron’ is used to clear away expired sessions. Similarly, with SilverStripe DynamoDB the Crontask module was at one point used to remove expired sessions. However, current versions of the Crontask module do not provide this facility.

The current version of the Crontask module no longer implements GarbageCollectSessionCronTask which removes expired sessions. The removal and reasoning are explained in this commit:

d91baf5#diff-003cb34625bfaea8be18fd87cd2d3afc

Yet the silverstripe-dynamodb module readme still states that: "Inactive sessions are garbage collected by GarbageCollectSessionCronTask if silverstripe-crontask is setup on your instance."

The garbage collection task was removed because AWS DynamoDB now provides a facility to automatically remove expired sessions. However, enabling this functionality requires setting a TTL property named ‘expires’ on the AWS DynamoDB table.

There is no explanation of this step in the module documentation and absent this step sessions will accumulate indefinitely in the AWS DynamoDB session table.

It is also noteworthy that in its default configuration SilverStripe does not set an expiry on the session cookie. Some standardised deployments of SilverStripe (such as CWP) do introduce a cookie lifetime with the following directive in config.yml:

SilverStripe\Control\Session:
timeout: 1440

If the cookie is not expired by the browser and the session is not expired by the server then sessions will have an indefinite lifetime.

Affected modules

▪ silverstripe/crontask
▪ silverstripe/dynamodb

Consequence

In the absence of a timeout being set for the session cookie and a TTL value set in DynamoDB it is possible that user sessions may not expire. It is also possible that entries in the session table will be accumulated indefinitely, which could impose unwanted AWS billing costs.

Recommendations

1. Update the documentation to explain that setting the expiry TTL attribute on the AWS DymamoDB session table is required for expired sessions to be removed and refer users to the relevant AWS documentation. This document explains using the DynamoDB TTL expiry to automictically remove expired sessions: https://docs.aws.amazon.com/sdk-for-php/v3/developer-guide/service_dynamodb-session-handler.html

2. Remove the crontask module from the list of DynamoDB dependencies.

3. Consider providing an example AWS CLI command for creating the DynamoDB session table with a correctly configured TTL attribute.

Reference

CWE: CWE-613 Insufficient Session Expiration
▪ https://cwe.mitre.org/data/definitions/613.html
AWS: Documentation on DynamoDB Session Handler
▪ https://docs.aws.amazon.com/sdk-for-php/v3/developer-guide/service_dynamodb-session-handler.html

[dhensby]

If the cookie is not expired by the browser and the session is not expired by the server then sessions will have an indefinite lifetime.

This is not true. A session cookie with 0 lifetime should be delete by the browser when the session ends (window is closed). Sessions with lifetimes are no longer actually "session" cookies they are persistent cookies.

Have you actually seen this behaviour in the wild or is it theoretical? My understanding of the AWS php-sdk was that it actually made a call to clean up sessions when PHP normally would.

The set up of Dynamo DB tables for PHP sessions is also clearly laid out in their docs (https://docs.aws.amazon.com/sdk-for-php/v3/developer-guide/service_dynamodb-session-handler.html) and, again from my previous usage, the php-sdk can set these up for you automatically.

It does appear that dynamo DB recommends deleting session in batches rather than relying on PHP's default behaviour, but it's by no means a requirement.

[dhensby]

tl;dr - not the first time a pen test company will have got something wrong ;)

[chillu]

It's worth noting that this is an audit report passed on by Bryn, the original reporter isn't listening in here :) This is public because we don't consider it a security issue as such.

This is not true. A session cookie with 0 lifetime should be delete by the browser when the session ends (window is closed). Sessions with lifetimes are no longer actually "session" cookies they are persistent cookies.

I think that's what they mean by "if the cookie is not expired by the browser" (through closing the window). This is explained here: https://github.com/silverstripe/cwp/blob/c57ecfc065157d31e3761271dcc2e7b00b5893ce/docs/en/01_Working_with_projects/10_Security.md#user-session-expiration. The CWP specific default simply matches the PHP settings on the server.

My understanding of the AWS php-sdk was that it actually made a call to clean up sessions when PHP normally would.

I'm not sure if it used to do that, but the aws docs are pretty clear on this - they don't do automated GC outside of the DynamoDB TTL setting, or the library feature being triggered externally (e.g. through cron).

So in conclusion, our module README made it a bit too easy to set this up wrong. I've sent a PR at #40. Notably in Silverstripe Cloud, we've been creating the expires attribute in new tables since mid 2019, and at that point retroactively applied to to existing tables as well.