uploads: Set Content-Type and -Disposition from Django for local files.

Similar to the previous commit, Django was responsible for setting the
Content-Disposition based on the filename, whereas the Content-Type
was set by nginx based on the filename.  This difference is not
exploitable, as even if they somehow disagreed with Django's expected
Content-Type, nginx will only ever respond with Content-Types found in
`uploads.types` -- none of which are unsafe for user-supplied content.

However, for consistency, have Django provide both Content-Type and
Content-Disposition headers.
This commit is contained in:
Alex Vandiver
2023-01-11 16:43:14 +00:00
parent 2f6c5a883e
commit 23894fc9a3
2 changed files with 24 additions and 11 deletions

View File

@@ -50,7 +50,9 @@ location /internal/local/uploads {
internal; internal;
include /etc/nginx/zulip-include/headers; include /etc/nginx/zulip-include/headers;
add_header Content-Security-Policy "default-src 'none'; style-src 'self' 'unsafe-inline'; img-src 'self'; object-src 'self'; plugin-types application/pdf;"; add_header Content-Security-Policy "default-src 'none'; style-src 'self' 'unsafe-inline'; img-src 'self'; object-src 'self'; plugin-types application/pdf;";
include /etc/nginx/zulip-include/uploads.types;
# Django handles setting Content-Type, Content-Disposition, and Cache-Control.
alias /home/zulip/uploads/files; alias /home/zulip/uploads/files;
} }

View File

@@ -68,7 +68,7 @@ def patch_disposition_header(response: HttpResponse, url: str, is_attachment: bo
response.headers["Content-Disposition"] = f"{disposition}; {file_expr}" response.headers["Content-Disposition"] = f"{disposition}; {file_expr}"
def internal_nginx_redirect(internal_path: str) -> HttpResponse: def internal_nginx_redirect(internal_path: str, content_type: Optional[str] = None) -> HttpResponse:
# The following headers from this initial response are # The following headers from this initial response are
# _preserved_, if present, and sent unmodified to the client; # _preserved_, if present, and sent unmodified to the client;
# all other headers are overridden by the redirected URL: # all other headers are overridden by the redirected URL:
@@ -78,13 +78,16 @@ def internal_nginx_redirect(internal_path: str) -> HttpResponse:
# - Set-Cookie # - Set-Cookie
# - Cache-Control # - Cache-Control
# - Expires # - Expires
# As such, we unset the Content-type header to allow nginx to set # As such, we default to unsetting the Content-type header to
# it from the static file; the caller can set Content-Disposition # allow nginx to set it from the static file; the caller can set
# and Cache-Control on this response as they desire, and the # Content-Disposition and Cache-Control on this response as they
# client will see those values. # desire, and the client will see those values. In some cases
response = HttpResponse() # (local files) we do wish to control the Content-Type, so also
# support setting it explicitly.
response = HttpResponse(content_type=content_type)
response["X-Accel-Redirect"] = internal_path response["X-Accel-Redirect"] = internal_path
del response["Content-Type"] if content_type is None:
del response["Content-Type"]
return response return response
@@ -133,15 +136,23 @@ def serve_local(
if settings.DEVELOPMENT: if settings.DEVELOPMENT:
# In development, we do not have the nginx server to offload # In development, we do not have the nginx server to offload
# the response to; serve it directly ourselves. # the response to; serve it directly ourselves. FileResponse
# FileResponse handles setting Content-Disposition, etc. # handles setting Content-Type, Content-Disposition, etc.
response: HttpResponseBase = FileResponse( response: HttpResponseBase = FileResponse(
open(local_path, "rb"), as_attachment=download # noqa: SIM115 open(local_path, "rb"), as_attachment=download # noqa: SIM115
) )
patch_cache_control(response, private=True, immutable=True) patch_cache_control(response, private=True, immutable=True)
return response return response
response = internal_nginx_redirect(quote(f"/internal/local/uploads/{path_id}")) # For local responses, we are in charge of generating both
# Content-Type and Content-Disposition headers; unlike with S3
# storage, the Content-Type is not stored with the file in any
# way, so Django makes the determination of it, and thus as well
# if that type is safe to have a Content-Disposition of "inline".
# nginx respects the values we send.
response = internal_nginx_redirect(
quote(f"/internal/local/uploads/{path_id}"), content_type=mimetype
)
patch_disposition_header(response, local_path, download) patch_disposition_header(response, local_path, download)
patch_cache_control(response, private=True, immutable=True) patch_cache_control(response, private=True, immutable=True)
return response return response