From 33c941407b72a320a1975947bdc002243af794a0 Mon Sep 17 00:00:00 2001 From: Anders Kaseorg Date: Tue, 25 Jun 2019 23:28:16 -0700 Subject: [PATCH] puppet: Remove legacy unauthenticated local uploads backend. This was only used in Ubuntu 14.04 Trusty. Removing this also finally lets us simplify our security model discussion of uploaded files. Signed-off-by: Anders Kaseorg --- docs/production/security-model.md | 14 ++------------ .../nginx/zulip-include-maybe/uploads-route.direct | 12 ------------ puppet/zulip/manifests/nginx.pp | 13 +------------ 3 files changed, 3 insertions(+), 36 deletions(-) delete mode 100644 puppet/zulip/files/nginx/zulip-include-maybe/uploads-route.direct diff --git a/docs/production/security-model.md b/docs/production/security-model.md index c0af175bd4..44a8d13f2c 100644 --- a/docs/production/security-model.md +++ b/docs/production/security-model.md @@ -227,22 +227,12 @@ strength allowed is controlled by two settings in possessing a URL to a secret file in Zulip does not provide unauthorized users with access to that file. - We have a similar protection for the `LOCAL_UPLOADS_DIR` backend, - that is only unavailable on Ubuntu Trusty (this is the one place - in Zulip where behavior is currently different between different OS - versions). For platforms that are not Ubuntu Trusty, every access + We have a similar protection for the `LOCAL_UPLOADS_DIR` backend. + Every access to an uploaded file has access control verified (confirming that the browser is logged into a Zulip account that has received the uploaded file in question). - On Ubuntu Trusty, because the older version of `nginx` available - there doesn't have proper Unicode support for the `X-Accel-Redirect` - feature, the `LOCAL_UPLOADS_DIR` backend only has the single layer - of security described at the beginning of this section (long, - randomly generated secret URLs). This could be fixed with further - engineering, but given the upcoming end-of-life of Ubuntu Trusty, we - have no plans to do that further work. - * Zulip supports using the Camo image proxy to proxy content like inline image previews that can be inserted into the Zulip message feed by other users over HTTPS. diff --git a/puppet/zulip/files/nginx/zulip-include-maybe/uploads-route.direct b/puppet/zulip/files/nginx/zulip-include-maybe/uploads-route.direct deleted file mode 100644 index 932c9bb3b7..0000000000 --- a/puppet/zulip/files/nginx/zulip-include-maybe/uploads-route.direct +++ /dev/null @@ -1,12 +0,0 @@ -# This Django route not under /api is shared between mobile and web -# and thus needs API headers added, in addition to the configuration -# required to have it serve files directly. - -location /user_uploads { - include /etc/nginx/zulip-include/api_headers; - - add_header X-Content-Type-Options nosniff; - 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; - alias /home/zulip/uploads/files; -} diff --git a/puppet/zulip/manifests/nginx.pp b/puppet/zulip/manifests/nginx.pp index 6e715b766c..8abaebab11 100644 --- a/puppet/zulip/manifests/nginx.pp +++ b/puppet/zulip/manifests/nginx.pp @@ -36,18 +36,7 @@ class zulip::nginx { # If we're not serving uploads locally, set the appropriate API headers for it. $uploads_route = 'puppet:///modules/zulip/nginx/zulip-include-maybe/uploads-route.noserve' } else { - # Nginx versions 1.4.6 and older do not support quoted URLs with the - # X-Accel-Redirect / "sendfile" feature, which are required for - # unicode support in filenames. As a result, we use the fancier - # django-sendfile behavior only when a sufficiently current version - # of nginx is present (e.g.. Xenial). Older versions (e.g. Trusty) - # retain the older, less secure, file upload behavior; we expect - # that this will stop being relevant when we drop Trusty support - # from Zulip altogether, no later than when Trusty reaches EOL in 2019. - $uploads_route = $zulip::base::release_name ? { - 'trusty' => 'puppet:///modules/zulip/nginx/zulip-include-maybe/uploads-route.direct', - default => 'puppet:///modules/zulip/nginx/zulip-include-maybe/uploads-route.internal', - } + $uploads_route = 'puppet:///modules/zulip/nginx/zulip-include-maybe/uploads-route.internal' } file { '/etc/nginx/zulip-include/uploads.route':