Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Enable by default to store all session data in the database + remove feature of file-based sessions in tmp/sessions/* #12170

Closed
mattab opened this issue Oct 12, 2017 · 5 comments · Fixed by #13540
Closed

Enable by default to store all session data in the database + remove feature of file-based sessions in tmp/sessions/* #12170

mattab opened this issue Oct 12, 2017 · 5 comments · Fixed by #13540
Assignees
@tsteur
Labels
c: Security For issues that make Matomo more secure. Please report issues through HackerOne and not in Github. Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical.
Milestone
3.8.0

Comments

@mattab
Copy link
Member

mattab commented Oct 12, 2017 •
edited

In order to simplify life (for example when we refactor/improve security in our sesions in #12164), I'd like to propose that we remove the File Sessions Handler in Piwik, and default everyone to use the Database session handler.

  • there is no advantage or reason to support the file sessions handler AFAIK.
  • Instead the database session handler works well in all cases, especially in the case where multiple piwik frontends are used and sessions must be shared (load balanced Piwik), or when NFS is used to store Piwik files (in which case file sessions are very slow)
  • We also support the Redis session handler, see redis session faq

Notes:
@mattab
Copy link
Member Author

mattab commented Oct 12, 2017

A more reasonable first step would be to instead, enable database session by default in Piwik, but leave the file session handler in case it is still useful to have.

@mattab mattab added this to the 3.7.0 milestone Oct 1, 2018
@mattab mattab changed the title Remove the file sessions handler and store all sessions in DB Enable by default to store all session data in the database (instead of files in tmp/sessions/*) Oct 1, 2018
@mattab
Copy link
Member Author

mattab commented Oct 1, 2018

It will be great to enable DB based sessions in Matomo 3.7.0. It will improve security because currently only on Apache2 we explicitely disable opening sessions files (on IIS or Nginx session files may be possible to open via direct web access). So once we store sessions in the DB it becomes impossible to directly access the content of the sessions in the tmp/sessions/ folder by guessing (or stealing) session tokens.

@mattab mattab added the c: Security For issues that make Matomo more secure. Please report issues through HackerOne and not in Github. label Oct 1, 2018
@mattab
Copy link
Member Author

mattab commented Oct 2, 2018

On top of activating DB sessions by default, we also want to:

add a system check that Warns people, if the sessions are using files.

It is important that people use DB sessions for maximum security and ensuring session files can't be read (when accessed directly).

@tsteur
Copy link
Member

tsteur commented Oct 4, 2018

If it is so important, why do you keep the functionality in there?

@mattab
Copy link
Member Author

mattab commented Oct 4, 2018

💥 even better! let's remove the feature of file-based sessions 🎉

@tsteur tsteur mentioned this issue Oct 4, 2018
Merged
@tsteur tsteur self-assigned this Oct 4, 2018
@mattab mattab changed the title Enable by default to store all session data in the database (instead of files in tmp/sessions/*) Enable by default to store all session data in the database + remove feature of file-based sessions in tmp/sessions/* Oct 14, 2018
@mattab mattab added the Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical. label Nov 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tsteur tsteur

Labels
c: Security For issues that make Matomo more secure. Please report issues through HackerOne and not in Github. Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical.
Projects
None yet
Milestone
3.8.0
Development

Successfully merging a pull request may close this issue.

Use db sessions by default, deprecate file session handler matomo-org/matomo
2 participants
@tsteur @mattab