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

DoS when using Premium Plugins caused by deleteTrackerCache() and getLicenseValidInfo() #14401

Closed
MichaelRoosz opened this issue May 2, 2019 · 3 comments · Fixed by #14405
Closed

DoS when using Premium Plugins caused by deleteTrackerCache() and getLicenseValidInfo() #14401

MichaelRoosz opened this issue May 2, 2019 · 3 comments · Fixed by #14405
Assignees
@tsteur
Labels
Bug For errors / faults / flaws / inconsistencies etc. c: Performance For when we could improve the performance / speed of Matomo.
Milestone
3.10.0

Comments

@MichaelRoosz
Copy link
Contributor

MichaelRoosz commented May 2, 2019 •
edited

This is a pretty nasty denial of service bug:

Some actions, for example:

  • adding a new user
  • updating a user or user rights
  • deleting a user
  • changing global site settings
  • and more

will call core\Tracker\Cache->deleteTrackerCache(), which deletes the whole cache.
This causes core\Plugin\Manager->getLicenseValidInfo() to get called for every activated premium plugin on the next tracking request or admin panel request.
getLicenseValidInfo() will send a http request to the matomo marketplace server.

On big setups like mine (>8000 tracking requests per minute) there are 350+ php workers running at the same time, many of them will call getLicenseValidInfo() roughly at the same time after a deleteTrackerCache() cache.

This means: About every 4th time I edit a users permissions or add a new site, the API request doing this admin operation will take over 60 seconds to complete, if it completes at all.
And during and after this, all 350+ php workers will get stuck, all trying to send a http request to the matomo marketplace. Bascially thousands of http request at the same time.

Our load balancer will then report for about 60s with "502 Bad Gateway" because all web servers are stuck on sending thousands of http requests to the matomo marketplace.

We have about 12 premium plugins activated, and about 700 php workers, this means up to 700*12 = 8400 http requests at the same time caused by a single admin action. These will possibly add up if editing multiple users in a short time period.

During this 60+ seconds we lose thousands of tracking requests.

I think an easy fix for now would be to introduce a new "plugin cache", that is independent from the normal "cache" and not affected by deleteTrackerCache(),

Alternative fix: do not call getLicenseValidInfo() during tracking requests.

I can try to create a PR for this, but I would like to know how you want to proceed.
tsteur added a commit that referenced this issue May 2, 2019
@tsteur
Do not create request to validate license in tracker mode
9cdf052 
fix #14401
@tsteur tsteur mentioned this issue May 2, 2019
Merged
@tsteur
Copy link
Member

tsteur commented May 2, 2019

Quickly looking at the tracker cache the problem looks indeed that deleteTrackerCache deletes all lazy caches in https://github.com/matomo-org/matomo/blob/3.10.0-b1/core/Tracker/Cache.php#L216 might not be too easy to change though as it could break something maybe.

We could in tracker mode only access a cached result. Eventually an archiver or something would create the cache key again. Or if you currently delete a user, the cache would be quickly created again through the UI while you are logged in. Of course if someone uses eg the API to delete a user, and then doesn't issue another API request, and doesn't have any archiving active then it may take a while for the cache to be written again but it should all be fine. In the worst case for a little while a bit more information is tracked.

What do you think about #14405 @MichaelHeerklotz ?

@tsteur tsteur added this to the 3.10.0 milestone May 2, 2019
@tsteur tsteur added the Bug For errors / faults / flaws / inconsistencies etc. label May 2, 2019
@tsteur tsteur self-assigned this May 3, 2019
@MichaelRoosz
Copy link
Contributor Author

MichaelRoosz commented May 3, 2019 •
edited

Thank you for your patch @tsteur.
#14405 looks good, I have tested it on our production setup.

Changing user permissions takes now

  • about 100ms if no license checks are done
  • about 900ms if license checks are done (after a previous user permission change)
  • one time it spiked to about 5.5s but only once in about 20 tries, so this is acceptable I would say

I was not able to make it overload again, so the denial of service / overload problem seems to be gone.

While the problem is gone, maybe it would still be a good idea to separate the plugin license cache in the future, maybe with Matomo 4? Most of the time when I edit users or sites, I edit a bunch of them, and it seems a bit wasteful to check all plugin licenses again for every single action.

But this issue is resolved with your patch, thanks a lot :)

@tsteur
Copy link
Member

tsteur commented May 5, 2019

image

The cache is kind of separate. The problem is really that Tracker\Cache should here instead only deleteCacheGeneral() and have some feature like self::getCache()->flushLike('cacheWebsiteAttributePrefix_').

tsteur added a commit that referenced this issue May 5, 2019
@tsteur
Do not create request to validate license in tracker mode (#14405)
7d97f5d 
* Do not create request to validate license in tracker mode

fix #14401

* be a bit more specific we assume license is not missing in tracker mode
@mattab mattab added the c: Performance For when we could improve the performance / speed of Matomo. label Jun 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tsteur tsteur

Labels
Bug For errors / faults / flaws / inconsistencies etc. c: Performance For when we could improve the performance / speed of Matomo.
Projects
None yet
Milestone
3.10.0
Development

Successfully merging a pull request may close this issue.

Do not create request to validate license in tracker mode matomo-org/matomo
3 participants
@tsteur @mattab @MichaelRoosz