D.O.S. Protection Limits

[gogobd]

Hello, every 01!

I just came across

FORM_MEMORY_LIMIT = 2 ** 20   # memory limit for forms
FORM_DISK_LIMIT = 2 ** 30     # disk limit for forms
FORM_MEMFILE_LIMIT = 2 ** 12  # limit for `BytesIO` -> temporary file switch

In ZPublisher/HTTPRequest.py and I have a question: how do these values relate, "memory limit" is responsible for limiting the request "stream", but I don't understand the documentation of "disk limit" and "memfile limit".

    <key name="form-memory-limit" datatype="byte-size" default="1MB">
      <description>
       The maximum size for each part in a multipart post request,
       for the complete body in an urlencoded post request
       and for the complete request body when accessed as bytes
       (rather than a file).
      </description>
    </key>

    <key name="form-disk-limit" datatype="byte-size" default="1GB">
      <description>
       The maximum size of a POST request body
      </description>
    </key>

    <key name="form-memfile-limit" datatype="byte-size" default="4KB">
      <description>
       The value of form variables of type file with larger size 
       are stored on disk rather than in memory.
      </description>
    </key>

If "disk limit" is the request body size, why is it called "disk" limit?

Do I understand it right that "memfile limit" is a "switch" so that smaller data are stored in memory whereas data larger than 4k are being written to disk?

Additionally I wanted to report that our employees were unable to upload image files with the very restrictive default values after an update; could we consider to raise the form memory limit to say 10MB or more? Will the "memfile" limit of 4k lead to problems with uploads?

Thank you for clarification!

[d-maurer]

Georg Gogo. BERNHARD wrote at 2023-10-25 03:05 -0700:

... If "disk limit" is the request body size, why is it called "disk" limit?

The names come originally from `multipart`. `multipart` maintains parts in memory when they are sufficiently small (controlled by `form_memory_limit`) and stores them on disk otherwise. The `form-disk-limit` is the maximal amount of disk space usable for a form.

Do I understand it right that "memfile limit" is a "switch" so that smaller data are stored in memory whereas data larger than 4k are being written to disk?

Right.

Additionally I wanted to report that our employees were unable to upload image files with the very restrictive default values after an update; could we consider to raise the form memory limit to say 10MB or more? Will the "memfile" limit of 4k lead to problems with uploads?

The `form-memory-limit` limits access when a request variable of type `file` or the request body is not accessed via the "file API" but as `bytes`. E.g. `request["BODYFILE"]` accesses the request body as a file (`form-memory-limit` has no effect) while `request["BODY"]` accesses it as `bytes` (`form-memory-limit` applies). Thus, `form-memory-limit` is used for 2 purposes: 1. control when a part is moved to disk 2. limit the part size when accessed as `bytes` If you are (or are ready to become) a Zope contributer, you could make a "pull request" to provide separate configuration options for those purposes. I would be in favor thereof.

[gogobd]

Thank you so much for shedding some light on this!

We ran into this problem when we came across this traceback:

Module ZPublisher.WSGIPublisher, line 181, in transaction_pubevents
Module ZPublisher.WSGIPublisher, line 391, in publish_module
Module ZPublisher.WSGIPublisher, line 269, in publish
Module ZPublisher.BaseRequest, line 619, in traverse
Module Products.PluggableAuthService.PluggableAuthService, line 244, in validate
Module Products.PluggableAuthService.PluggableAuthService, line 560, in _extractUserIds
Module plone.restapi.pas.plugin, line 100, in extractCredentials
Module plone.restapi.deserializer, line 8, in json_body
Module ZPublisher.HTTPRequest, line 1058, in get
Module ZPublisher.HTTPRequest, line 1370, in __get__

I found that in ZPublisher.HTTPRequest the value from the configuration is used

Zope/src/ZPublisher/HTTPRequest.py

Line 1427 in 8fdd567

VALUE_LIMIT = Global("FORM_MEMORY_LIMIT")

and then, at

Zope/src/ZPublisher/HTTPRequest.py

Line 1370 in 8fdd567

raise BadRequest("data exceeds memory limit")

the setting from the configuration is used and the "get" method from

Zope/src/ZPublisher/HTTPRequest.py

Line 1058 in 8fdd567

v = self.other[key] = self._fs.value

fails.

Plone just wanted to read the value like this: data = json.loads(request.get("BODY") or "{}") https://github.com/plone/plone.restapi/blob/5f9214950a50d4728d7a245bec1358a68c8f2316/src/plone/restapi/deserializer/__init__.py#L8, not knowing that there are two ways request data are being handled.

Now I wonder how I could make this more convenient. Changing the default values would of course help, but the actual problem is the request.get method that should return a value no matter how it was being stored.

Any thoughts on this?

[d-maurer]

Georg Gogo. BERNHARD wrote at 2023-10-30 02:46 -0700:

... Plone just wanted to read the value like this: `data = json.loads(request.get("BODY") or "{}")` <https://github.com/plone/plone.restapi/blob/5f9214950a50d4728d7a245bec1358a68c8f2316/src/plone/restapi/deserializer/__init__.py#L8>, not knowing that there are two ways request data are being handled. Now I wonder how I could make this more convenient. Changing the default values would of course help, but the actual problem is the `request.get` method that should return a value no matter how it was being stored. Any thoughts on this?

`Zope` should not use `FORM_MEMORY_LIMIT` to limit how much data can be returned from `request["BODY"]`. It should use a separate configuration parameter for this (which could be larger than `FORM_MEMORY_LIMIT` as it affects only a single value, not potentially a whole set). Would you like to contribute this change? Of course, `plone.restapi` could use `data = {} if "BODYFILE" not in request else json.load(request["BODYFILE"])` instead of `data = json.loads(request.get("BODY") or "{}")`. This assumes that `BODYFILE` is not an empty file, if it exists. A more conservative solutions would be ```python body = request.get("BODYFILE") if body is not None: body = body.read() data = json.loads(body or "{}") ``` I prefer the `Zope` change. Should you fell uncomfortable with the change contribution, I will do it.

[d-maurer]

multipart maintains parts in memory when they are sufficiently small (controlled by form_memory_limit) and stores them on disk otherwise.

@gogobd I must revert this statement: having reread the documentation in Zope2.Startup:wsgischema.xml, I recognize that form_memory_limit is not used to switch from an in memory representation to a disk representation. Instead, form-memfile-limit is used for this. form_memory_limit is passed as mem_limit to multipart and limits there the total memory used for all request parameter values not stored on disk (thus, the Zope documentation is not correct in this regard).

I no longer think that Zope needs an additional configuration option. Should the default 1MB for form-memory-limit be too small in exceptional cases, reconfigure. Should plone regularly require a larger limit, we can increase the default.

I will create a PR which improves/corrects the DOS protection related Zope documentation.