diff --git a/docs/subsystems/thumbnailing.md b/docs/subsystems/thumbnailing.md index 516f43a431..1b2ca1c390 100644 --- a/docs/subsystems/thumbnailing.md +++ b/docs/subsystems/thumbnailing.md @@ -16,7 +16,9 @@ Thumbor is responsible for a few things in Zulip: * Serving all image content over HTTPS, even if the original/upstream image was hosted on HTTP (this was previously done by `camo` in - older versions of Zulip). This is important to avoid mixed-content + older versions of Zulip; the `THUMBOR_SERVES_CAMO` setting controls + whether Thumbor will serve the old-style Camo URLs that might be + present in old messages). This is important to avoid mixed-content warnings from browsers (which look very bad), and does have some real security benefit in protecting our users from malicious content. diff --git a/zerver/lib/camo.py b/zerver/lib/camo.py index b506d4c45c..661ea43a7d 100644 --- a/zerver/lib/camo.py +++ b/zerver/lib/camo.py @@ -17,3 +17,8 @@ def get_camo_url(url: str) -> str: if settings.CAMO_URI == '': return url return "%s%s" % (settings.CAMO_URI, generate_camo_url(url)) + +def is_camo_url_valid(digest: str, url: str) -> bool: + camo_url = generate_camo_url(url) + camo_url_digest = camo_url.split('/')[0] + return camo_url_digest == digest diff --git a/zerver/lib/thumbnail.py b/zerver/lib/thumbnail.py index e14b991181..3ff624d8e8 100644 --- a/zerver/lib/thumbnail.py +++ b/zerver/lib/thumbnail.py @@ -30,7 +30,9 @@ def get_source_type(url: str) -> str: return THUMBOR_LOCAL_FILE_TYPE return THUMBOR_S3_TYPE -def generate_thumbnail_url(path: str, size: str='0x0') -> str: +def generate_thumbnail_url(path: str, + size: str='0x0', + is_camo_url: bool=False) -> str: if not (path.startswith('https://') or path.startswith('http://')): path = '/' + path @@ -48,14 +50,18 @@ def generate_thumbnail_url(path: str, size: str='0x0') -> str: width, height = map(int, size.split('x')) crypto = CryptoURL(key=settings.THUMBOR_KEY) + smart_crop_enabled = True apply_filters = ['no_upscale()'] + if is_camo_url: + smart_crop_enabled = False + apply_filters.append('quality(100)') if size != '0x0': apply_filters.append('sharpen(0.5,0.2,true)') encrypted_url = crypto.generate( width=width, height=height, - smart=True, + smart=smart_crop_enabled, filters=apply_filters, image_url=image_url ) diff --git a/zerver/tests/test_camo.py b/zerver/tests/test_camo.py new file mode 100644 index 0000000000..7376fb33c3 --- /dev/null +++ b/zerver/tests/test_camo.py @@ -0,0 +1,19 @@ +# -*- coding: utf-8 -*- +from zerver.lib.test_classes import ZulipTestCase + +class CamoURLTest(ZulipTestCase): + def test_legacy_camo_url(self) -> None: + # Test with valid hex and url pair + result = self.client_get("/external_content/0f50f0bda30b6e65e9442c83ddb4076c74e75f96/687474703a2f2f7777772e72616e646f6d2e736974652f696d616765732f666f6f6261722e6a706567") + self.assertEqual(result.status_code, 302, result) + self.assertIn('/filters:no_upscale():quality(100)/aHR0cDovL3d3dy5yYW5kb20uc2l0ZS9pbWFnZXMvZm9vYmFyLmpwZWc=/source_type/external', result.url) + + # Test with invalid hex and url pair + result = self.client_get("/external_content/074c5e6c9c6d4ce97db1c740d79dc561cf7eb379/687474703a2f2f7777772e72616e646f6d2e736974652f696d616765732f666f6f6261722e6a706567") + self.assertEqual(result.status_code, 403, result) + self.assert_in_response("Not a valid URL.", result) + + def test_with_thumbor_disabled(self) -> None: + with self.settings(THUMBOR_SERVES_CAMO=False): + result = self.client_get("/external_content/074c5e6c9c6d4ce97db1c740d79dc561cf7eb379/687474703a2f2f7777772e72616e646f6d2e736974652f696d616765732f666f6f6261722e6a706567") + self.assertEqual(result.status_code, 404, result) diff --git a/zerver/views/camo.py b/zerver/views/camo.py new file mode 100644 index 0000000000..b82337c0db --- /dev/null +++ b/zerver/views/camo.py @@ -0,0 +1,23 @@ +from django.conf import settings +from django.shortcuts import redirect +from django.utils.translation import ugettext as _ +from django.http import ( + HttpRequest, HttpResponse, HttpResponseForbidden, HttpResponseNotFound +) +from zerver.lib.camo import is_camo_url_valid +from zerver.lib.thumbnail import generate_thumbnail_url + +import codecs + +def handle_camo_url(request: HttpRequest, digest: str, + received_url: str) -> HttpResponse: + if not settings.THUMBOR_SERVES_CAMO: + return HttpResponseNotFound() + + hex_encoded_url = received_url.encode('utf-8') + hex_decoded_url = codecs.decode(hex_encoded_url, 'hex') + original_url = hex_decoded_url.decode('utf-8') # type: ignore # https://github.com/python/typeshed/issues/300 + if is_camo_url_valid(digest, original_url): + return redirect(generate_thumbnail_url(original_url, is_camo_url=True)) + else: + return HttpResponseForbidden(_("
Not a valid URL.
")) diff --git a/zproject/settings.py b/zproject/settings.py index b054df0e64..badd7497f4 100644 --- a/zproject/settings.py +++ b/zproject/settings.py @@ -209,6 +209,7 @@ DEFAULT_SETTINGS = { 'REMOTE_POSTGRES_HOST': '', 'REMOTE_POSTGRES_SSLMODE': '', 'THUMBOR_URL': '', + 'THUMBOR_SERVES_CAMO': False, 'THUMBNAIL_IMAGES': False, 'SENDFILE_BACKEND': None, diff --git a/zproject/test_settings.py b/zproject/test_settings.py index 985d1d8fb6..23fd2a5efb 100644 --- a/zproject/test_settings.py +++ b/zproject/test_settings.py @@ -160,6 +160,7 @@ SLOW_QUERY_LOGS_STREAM = None THUMBOR_URL = 'http://127.0.0.1:9995' THUMBNAIL_IMAGES = True +THUMBOR_SERVES_CAMO = True # Logging the emails while running the tests adds them # to /emails page. diff --git a/zproject/urls.py b/zproject/urls.py index f2461b9f8e..3382bbd890 100644 --- a/zproject/urls.py +++ b/zproject/urls.py @@ -20,6 +20,7 @@ import zerver.tornado.views import zerver.views import zerver.views.auth import zerver.views.archive +import zerver.views.camo import zerver.views.compatibility import zerver.views.home import zerver.views.email_mirror @@ -585,6 +586,14 @@ urls += [ urls += url(r'^report/csp_violations$', zerver.views.report.report_csp_violations, name='zerver.views.report.report_csp_violations'), +# This url serves as a way to provide backward compatibility to messages +# rendered at the time Zulip used camo for doing http -> https conversion for +# such links with images previews. Now thumbor can be used for serving such +# images. +urls += url(r'^external_content/(?P