traefik-modsecurity-plugin icon indicating copy to clipboard operation
traefik-modsecurity-plugin copied to clipboard

Default for maxBodySize is too low

Open tspearconquest opened this issue 1 year ago • 2 comments

Hi,

We recently added this plugin to our services and had no issues until we tried to upload a large file, upon which we were getting the error body max limit reached: http: request body too large

It took quite some googling to find out that it was the default value of this plugin causing the problem, especially considering that Traefik themselves has stated that there is no hard coded limits in Traefik.

This changes their default from no limit to a pre-set limit which, while configurable, is too low for todays modern web application requests in the enterprise, wherein docker images and other files that are passed into an API for automation purposes are hundreds of MB.

It would be appreciated if this plugin were to follow more closely with upstream Traefik's defaults by raising the maxBodyLimit default value to at least 1GB.

tspearconquest avatar Jun 06 '23 16:06 tspearconquest

As stated in the readme:

Note: body of every request will be buffered in memory while the request is in-flight (i.e.: during the security check and during the request processing by traefik and the backend), so you may want to tune maxBodySize depending on how much RAM you have.

Im not sure setting the default value to 1gb would be a good idea. Think of low powered devices such as rpi. Some just have 1gb of ram.

What do you think ?

acouvreur avatar Jun 07 '23 04:06 acouvreur

Yes I agree for a setup where a single person implements this on their rpi, 1gb would be too high.

The problem was caused for us because we work in a team scenario where anyone may implement a change but someone else later may have to troubleshoot when the implementer isn't available.

When my colleague added this plugin, it's clear he must have missed that info in the readme because we missed to test with a large request after the implementation.

By the time another colleague spotted issues with large requests, this plugin was already implemented for over a month and no longer considered a recent change. The implementer is on his honeymoon this week and since the max body size option is optional, it took quite a bit of effort for the rest of us to isolate the cause via a Google search that landed us in the code for this plugin, which is poor UX.

So I have a couple of requests here, and mostly it's just around providing a better UX in team working environments.

1: Please add something to the error message that this plugin outputs when the buffer is full to make it clear that this plugin is the one generating the error message, instead of traefik itself.

This would solve the first problem which is that we could see the error message in traefik logs but it wasn't immediately clear that this plugin was generating it; we looked around the traefik configuration and saw that the traefik buffering plugin wasn't configured so that's when we turned to Google as mentioned above.

2: Not sure if it would be possible but I would like to reconfigure traefik to use the aforementioned buffering plugin in our setup and have that plugin handle the buffering itself. Could you add an option to disable the builtin buffering in this plugin so that we can rely on traefik's buffering plugin to handle it instead?

3: Would you consider making maxBodySize mandatory if using this plugin's buffering? Doing this would provide clear documentation in the traefik configuration itself that there is some sort of buffering in use that isn't the traefik buffering plugin.

Thanks in advance!

tspearconquest avatar Jun 07 '23 11:06 tspearconquest