feat(nginx): Trust admins to upload large files

[markstos]

This request implements the recommendation in #1661

However, I've done some more follow-up research that leaves me uncertain on whether to recommend merging this.

What's checked first: authentication or the upload size?

In the linked issue, I recommended allowing unlimited upload size, but scoped to a route that only works for admins. That's a sound idea, but it only works to block unwanted large uploads if authentication happens before all that data is sent. So I put together a test:

I submitted an upload to admin-only endpoint as an unauthenticated user, sending a file that was larger than client_max_body_size. So my request has two reasons to fail: It's not authenticated and it's too big. The question is: which problem will problem trigger first?

In my tests, the client_max_body_size limit was always triggered before the request was passed to Ghost and the app had a chance to run the authentication middleware.

http --print=Hh --multipart https://pea-pod.org/ghost/api/admin/posts/62c44ce15e9a84e9ec9948d4 boom@huge-file.mp4
mp4
POST /ghost/api/admin/posts/62c44ce15e9a84e9ec9948d4 HTTP/1.1
Accept: */*
Accept-Encoding: gzip, deflate
Connection: keep-alive
Content-Length: 440882259
Content-Type: multipart/form-data; boundary=3a74970ec30842a88d623d35c2a2fa6a
Host: example.com
User-Agent: HTTPie/3.2.1

HTTP/1.1 413 Request Entity Too Large
Connection: close
Content-Length: 192
Content-Type: text/html
Date: Wed, 06 Jul 2022 00:26:09 GMT
Server: nginx/1.18.0 (Ubuntu)

To try the force the request to get passed through from Ngnix to Ghost before it was fully uploaded, I tried setting these directives:

# In theory, this shoudl stop Nginx from buffering the uploads
proxy_request_buffering off;
# This is supposed to stop Nginx from buffering HTTP/1.1 multi-part uploads
# if the above is set
proxy_http_version 1.1;

To summarize: Since Ghost is open source, it doesn't help security to scope large file uploads to a single authenticated route, because that route is known, and Ghost (apparently) can't check the authentication until the upload is complete. And since it's better to be "secure by default", perhaps the current max upload setting of 50 mb is a reasonable default. Those who need to upload videos can change the default.

Detour: Does proxy_request_buffering perform better on or off?

As part testing this, I ended up testing uploading a 400 Mb file repeatedly, both with `proxy_request_buffering on and off. My hypothesis was that uploads would perform better with the buffering off because Ghost could start processing the first part of the file even while the last part is still uploaded.

The result was instead that it was about 7x faster to leave proxy_request_buffering enabled. I can't explain why. But here you can see in the Ghost logs the different valuels changes as I toggled the directive back and forth and re-tested:

INFO "POST /ghost/api/admin/media/upload/" 201 8730ms
INFO "POST /ghost/api/admin/media/upload/" 201 202634ms
INFO "POST /ghost/api/admin/media/upload/" 201 6576ms
INFO "POST /ghost/api/admin/media/upload/" 201 197616ms
INFO "POST /ghost/api/admin/media/upload/" 201 5521ms
INFO "POST /ghost/api/admin/media/upload/" 201 209844ms

The conclusion there is no change is recommended to the current proxy_request_buffering directive either.

[ErisDS]

Hey @markstos, as I explained on the issue, I think the proper solution here is just to increase client_max_body_size. If there's a strong need to limit uploads based on user role, that's a product concern.

I really appreciate all the research you did here, especially into the buffering config. Would you be happy to update this to just change the limit?

[markstos]

@ErisDS I've forced-pushed an alternate patch raises the client_max_body_size to 1 gig to match Ghost(Pro) and better support video and other large files other self-hosters.

I checked the Nginx dogs that "g" is the right way to express Gigabytes:

https://nginx.org/en/docs/syntax.html

[ErisDS]

@markstos thanks for this! I know we're gearing up to do another release of CLI, so it should be in the wild soon 🎉