Limit the CSP header size to avoid server errors

[Toflar]

With the latest PRs merged in the 5.3 branch, the probability of generating a large CSP header increased but it was also possible before: the CSP header can get pretty long. If you e.g. have a 50 bytes long inline style, a sha384 hash base64 encoded can get about 130 bytes long. (echo strlen('sha384-' . base64_encode(hash('sha384', random_bytes(50))));).

I found various different sources for the maximum header size of nginx, Apache and Co. but it ranges from 4kb to 16kb. If it is 4kb, you'll end up having a max of about 30 of those hashes and you will end up crashing the webserver for a header too long.

I think, it would be great if Contao could limit this. It should be better to remove hashes and have CSP violations than having a server crash. Or in other words: better to not have a text-decoration: underline than a 500 Server Error.

So here's some proposal reducing the CSP header length automatically, starting with style-src hashes and then script-src hashes until the header size is okay.

[Toflar]

Should be ready. The only thing here missing is a decision for the default length. Looks like nginx is 8kb, as is Apache. So maybe we could increase the default to 8192? /cc @leofeyer

[ausi]

Most information about header size I could find online talks about the size of the whole HTTP header block containing all headers. Do we maybe need to take the size of the other headers in the $response into account when reducing the size?

The only thing here missing is a decision for the default length. Looks like nginx is 8kb, as is Apache. So maybe we could increase the default to 8192?

Nginx writes “By default, the buffer size is equal to one memory page. This is either 4K or 8K, depending on a platform.”
I have no idea what a common memory page size is on most webservers but searching the internet 4K seems to be quite common.

[fritzmg]

Yes, the maximum header size relates to the sum of all headers, not just one single header.

To be honest, I am not sure anymore we should or can implement this. You'll have the same issue if you have too many / too large cookies or may be even too many cache tags.

[Toflar]

Yes, the maximum header size relates to the sum of all headers, not just one single header.

To be honest, I am not sure anymore we should or can implement this. You'll have the same issue if you have too many / too large cookies or may be even too many cache tags.

Then we must implement this PR even more so! I don't think considering the other headers makes any sense. We should just limit the CSP header to a fixed amount of bytes for maximum compatibility. This is just a safety net anyway. Most users won't hit it because they will not have 30 hashes. Remember, if you have multiple tinyMCE values on the same page, all using text-decoration: underline only, you'll end up having only one single hash. Only if you use multiple style different style combinations you might run into this issue. And I think this PR greatly enhances usability. It will log and tell you that the CSP header was too long and what you should do. If you run into a 502 or 500, you'll have to search the server logs and all you'll probably find is that the total header content was too long, not giving you any hints about the CSP header being the culprit or not at all.

[ausi]

We should just limit the CSP header to a fixed amount of bytes for maximum compatibility.

Agree. How many bytes do we want to allow by default? 3072 or 2048?

[Toflar]

Agree. How many bytes do we want to allow by default? 3072 or 2048?

I wrote a quick calculation script with a realistic base CSP header and the options to adjust the SHA algorithm and max header length to see how many hashes would fit: https://3v4l.org/YPirH

I think we should probably switch to SHA256 anyway (secure enough, speed is probably neglectable and should be in cache for public pages anyway) and 3KB. This would allow a max of 29 hashes which I think is easily enough for most use cases.

[leofeyer]

It seems the changes have caused a problem in the applyHeaders() method:

[Toflar]

Ah that's a leftover, fixed in cd6f3f7

[leofeyer]

Thank you @Toflar.