diff --git a/docs/roadmap.md b/docs/roadmap.md
index 1022c2ab9e2a1..9bbe7ca4c0d4d 100644
--- a/docs/roadmap.md
+++ b/docs/roadmap.md
@@ -179,9 +179,9 @@ of its size, it takes work to keep it that way.
* [Add support for 2-factor authentication on all
platforms](https://github.com/zulip/zulip/pull/1747)
-* [Add support for stronger security controls for uploaded files (The
+* [Add support for stronger security controls for uploaded files (The
LOCAL_UPLOADS_DIR file uploads backend only supports world-readable
- uploads)](https://github.com/zulip/zulip/issues/320)
+ uploads)](https://github.com/zulip/zulip/issues/320)
* [Fix requirement to set a password when creating account via
Google](https://github.com/zulip/zulip/issues/1633)
* [Add a retention policy feature that automatically deletes old
diff --git a/docs/security-model.md b/docs/security-model.md
index 0fe3fcd5e8ff3..d742dcc21ddce 100644
--- a/docs/security-model.md
+++ b/docs/security-model.md
@@ -184,24 +184,9 @@ your organization.
* Zulip supports user-uploaded files; ideally they should be hosted
from a separate domain from the main Zulip server to protect against
various same-domain attacks (e.g. zulip-user-content.example.com)
- using the S3 integration.
-
- The URLs of user-uploaded files are secret; if you are using the
- "local file upload" integration, anyone with the URL of an uploaded
- file can access the file. This means the local uploads integration
- is vulnerable to a subtle attack where if a user clicks on a link in
- a secret .PDF or .HTML file that had been uploaded to Zulip, access
- to the file might be leaked to the other server via the Referrer
- header (see [the "Uploads world readable" issue on
- GitHub](https://github.com/zulip/zulip/issues/320)).
-
- The Zulip S3 file upload integration is relatively safe against that
- attack, because the URLs of files presented to users don't host the
- content. Instead, the S3 integration checks the user has a valid
- Zulip session in the relevant realm, and if so then redirects the
- browser to a one-time S3 URL that expires a short time later.
- Keeping the URL secret is still important to avoid other users in
- the Zulip realm from being able to access the file.
+ using the S3 integration. The uploaded files could be viewed by only
+ those users who have access to them. Simple possession of a URL to
+ the uploaded file doesn't qualify as a right to view such a file.
* Zulip supports using the Camo image proxy to proxy content like
inline image previews that can be inserted into the Zulip message
diff --git a/puppet/zulip/files/nginx/sites-available/zulip-enterprise b/puppet/zulip/files/nginx/sites-available/zulip-enterprise
index b2f3e3a0ecf40..54945171b1890 100644
--- a/puppet/zulip/files/nginx/sites-available/zulip-enterprise
+++ b/puppet/zulip/files/nginx/sites-available/zulip-enterprise
@@ -12,7 +12,8 @@ server {
ssl_certificate /etc/ssl/certs/zulip.combined-chain.crt;
ssl_certificate_key /etc/ssl/private/zulip.key;
- location /user_uploads {
+ location /serve_uploads {
+ internal;
add_header X-Content-Type-Options nosniff;
include /etc/nginx/zulip-include/uploads.types;
alias /home/zulip/uploads/files;
diff --git a/requirements/common.txt b/requirements/common.txt
index 57b831d13d987..c7ef0d774438c 100644
--- a/requirements/common.txt
+++ b/requirements/common.txt
@@ -186,3 +186,8 @@ ijson==2.3
# Needed for link preview
beautifulsoup4==4.5.3
git+https://github.com/rafaelmartins/pyoembed.git@eb9901917c2a44b49e2887c077ead84a722c50dc#egg=pyoembed
+
+# Needed for serving uploaded files from nginx but perform auth checks in django.
+# We are using a commit from a PR https://github.com/johnsensible/django-sendfile/pull/64
+# in Django-sendfile repo as it might be sometime before it gets merged.
+git+https://github.com/adnrs96/django-sendfile.git@f45fac5fdd83e7d42a8d5275038e33f8c2cdb920#egg=django-sendfile==0.3.12
diff --git a/version.py b/version.py
index 9099a73eb0c3c..34f319cbf9c30 100644
--- a/version.py
+++ b/version.py
@@ -1,2 +1,2 @@
ZULIP_VERSION = "1.5.1+git"
-PROVISION_VERSION = '5.0'
+PROVISION_VERSION = '5.01'
diff --git a/zerver/tests/test_upload.py b/zerver/tests/test_upload.py
index bcc24b8915d15..0ca248b45bb8f 100644
--- a/zerver/tests/test_upload.py
+++ b/zerver/tests/test_upload.py
@@ -24,7 +24,7 @@
from zerver.lib.actions import do_delete_old_unclaimed_attachments
from zilencer.models import Deployment
-from zerver.views.upload import upload_file_backend
+from zerver.views.upload import upload_file_backend, serve_local
import ujson
from six.moves import urllib
@@ -43,6 +43,8 @@
import base64
from datetime import timedelta
from django.utils.timezone import now as timezone_now
+from django.http import HttpRequest
+from sendfile import _get_sendfile
from moto import mock_s3
@@ -580,6 +582,21 @@ def test_file_download_authorization_public(self):
self.assertEqual(b"zulip!", data)
self.logout()
+ def test_serve_local(self):
+ # type: () -> None
+ with self.settings(SENDFILE_BACKEND='sendfile.backends.nginx',
+ NGINX_VERSION='1.4.6'):
+ _get_sendfile.clear() # To clearout cached version of backend from djangosendfile
+ filename = os.path.join(settings.LOCAL_UPLOADS_DIR, 'files', 'á.jpg')
+ basedir = os.path.dirname(filename)
+ if not os.path.exists(basedir):
+ os.makedirs(basedir)
+ open(filename, 'w').close()
+ responce = serve_local(HttpRequest(), 'á.jpg')
+ _get_sendfile.clear()
+ test_upload_dir = os.path.split(settings.LOCAL_UPLOADS_DIR)[1]
+ self.assertEqual(responce['X-Accel-Redirect'], '/serve_uploads/../../' + test_upload_dir + '/files/á.jpg')
+
def tearDown(self):
# type: () -> None
destroy_uploads()
diff --git a/zerver/views/upload.py b/zerver/views/upload.py
index 0a5f14f1d7c51..eb38ec66418b3 100644
--- a/zerver/views/upload.py
+++ b/zerver/views/upload.py
@@ -14,24 +14,19 @@
from zerver.lib.validator import check_bool
from zerver.models import UserProfile, validate_attachment_request
from django.conf import settings
+from sendfile import sendfile
def serve_s3(request, url_path):
# type: (HttpRequest, str) -> HttpResponse
uri = get_signed_upload_url(url_path)
return redirect(uri)
-# TODO: Rewrite this once we have django-sendfile
def serve_local(request, path_id):
# type: (HttpRequest, str) -> HttpResponse
- import os
- import mimetypes
local_path = get_local_file_path(path_id)
if local_path is None:
return HttpResponseNotFound('File not found
')
- filename = os.path.basename(local_path)
- response = FileResponse(open(local_path, 'rb'),
- content_type = mimetypes.guess_type(filename))
- return response
+ return sendfile(request, local_path)
@has_request_variables
def serve_file_backend(request, user_profile, realm_id_str, filename):
diff --git a/zproject/dev_settings.py b/zproject/dev_settings.py
index 576762e250794..d32db73b21c0c 100644
--- a/zproject/dev_settings.py
+++ b/zproject/dev_settings.py
@@ -52,3 +52,6 @@
# Don't require anything about password strength in development
PASSWORD_MIN_LENGTH = 0
PASSWORD_MIN_ZXCVBN_QUALITY = 0
+# Make sendfile use django to serve files in development
+SENDFILE_BACKEND = 'sendfile.backends.development'
+SENDFILE_ROOT = os.path.join(LOCAL_UPLOADS_DIR, 'files')
diff --git a/zproject/prod_settings_template.py b/zproject/prod_settings_template.py
index 6c5871d66ce38..399e615554739 100644
--- a/zproject/prod_settings_template.py
+++ b/zproject/prod_settings_template.py
@@ -171,21 +171,20 @@
# directly on the Zulip server. If file storage in Amazon S3 is
# desired, you can configure that as follows:
#
-# (1) Set s3_key and s3_secret_key in /etc/zulip/zulip-secrets.conf to
+# Set s3_key and s3_secret_key in /etc/zulip/zulip-secrets.conf to
# be the S3 access and secret keys that you want to use, and setting
# the S3_AUTH_UPLOADS_BUCKET and S3_AVATAR_BUCKET to be the S3 buckets
# you've created to store file uploads and user avatars, respectively.
# Then restart Zulip (scripts/restart-zulip).
-#
-# (2) Edit /etc/nginx/sites-available/zulip-enterprise to comment out
-# the nginx configuration for /user_uploads and /user_avatars (see
-# https://github.com/zulip/zulip/issues/291 for discussion of a better
-# solution that won't be automatically reverted by the Zulip upgrade
-# script), and then restart nginx.
LOCAL_UPLOADS_DIR = "/home/zulip/uploads"
#S3_AUTH_UPLOADS_BUCKET = ""
#S3_AVATAR_BUCKET = ""
+# Setup the version of NGINX which is being used for this installation.
+# By default a NGINX version is automatically detected from system but it could also be
+# specified here optionally.
+# NGINX_VERSION = '1.4.6'
+
# Maximum allowed size of uploaded files, in megabytes. DO NOT SET
# ABOVE 80MB. The file upload implementation doesn't support chunked
# uploads, so browsers will crash if you try uploading larger files.
diff --git a/zproject/settings.py b/zproject/settings.py
index 315733051f694..d87016484d88d 100644
--- a/zproject/settings.py
+++ b/zproject/settings.py
@@ -15,6 +15,8 @@
import platform
import time
import sys
+import subprocess
+import re
import six.moves.configparser
from zerver.lib.db import TimeTrackingConnection
@@ -93,6 +95,11 @@ def get_secret(key):
else:
from .dev_settings import *
+if PRODUCTION and LOCAL_UPLOADS_DIR and 'NGINX_VERSION' not in vars():
+ NGINX_VERSION = re.search(r'([\d.]+)', subprocess.Popen(['nginx', '-v'],
+ stdout=subprocess.PIPE,
+ stderr=subprocess.STDOUT).stdout.read().decode('utf-8')).group(1)
+
########################################################################
# DEFAULT VALUES FOR SETTINGS
########################################################################
@@ -138,6 +145,10 @@ def get_secret(key):
'RATE_LIMITING': True,
'REDIS_HOST': '127.0.0.1',
'REDIS_PORT': 6379,
+ 'SENDFILE_BACKEND': 'sendfile.backends.nginx',
+ 'SENDFILE_ROOT': '/home/zulip/uploads/files',
+ 'SENDFILE_URL': '/serve_uploads',
+ 'NGINX_VERSION': None,
# The following bots only exist in non-VOYAGER installs
'ERROR_BOT': None,
'NEW_USER_BOT': None,