S3: Warn about and deprecate insecure defaults

jschneier · Sep 2, 2018 · 6ee6a73 · 6ee6a73
1 parent d9015ca
commit 6ee6a73
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 4 deletions.
diff --git a/storages/backends/s3boto.py b/storages/backends/s3boto.py
@@ -1,9 +1,11 @@
 import mimetypes
 import os
+import warnings
 from datetime import datetime
 from gzip import GzipFile
 from tempfile import SpooledTemporaryFile
 
+from django.conf import settings as django_settings
 from django.core.exceptions import ImproperlyConfigured, SuspiciousOperation
 from django.core.files.base import File
 from django.core.files.storage import Storage
@@ -78,6 +80,17 @@ def __init__(self, name, mode, storage, buffer_size=None):
             self.buffer_size = buffer_size
         self._write_counter = 0
 
+        # warn about upcoming change in default AWS_DEFAULT_ACL setting
+        if not hasattr(django_settings, 'AWS_DEFAULT_ACL'):
+            warnings.warn(
+                "The default behavior of S3BotoStorage is insecure and will change "
+                "in django-storages 1.8. By default files and new buckets are saved "
+                "with an ACL of 'public-read' (globally publicly readable). Version 1.8 will "
+                "default to using the bucket's ACL. To opt into the new behavior set "
+                "AWS_DEFAULT_ACL = None, otherwise to silence this warning explicitly "
+                "set AWS_DEFAULT_ACL."
+            )
+
     @property
     def size(self):
         return self.key.size
@@ -113,9 +126,9 @@ def write(self, content, *args, **kwargs):
         self._is_dirty = True
         if self._multipart is None:
             provider = self.key.bucket.connection.provider
-            upload_headers = {
-                provider.acl_header: self._storage.default_acl
-            }
+            upload_headers = {}
+            if self._storage.default_acl:
+                upload_headers[provider.acl_header] = self._storage.default_acl
             upload_headers.update({
                 'Content-Type': mimetypes.guess_type(self.key.name)[0] or self._storage.key_class.DefaultContentType
             })
@@ -318,7 +331,18 @@ def _get_or_create_bucket(self, name):
         except self.connection_response_error:
             if self.auto_create_bucket:
                 bucket = self.connection.create_bucket(name, location=self.origin)
-                bucket.set_acl(self.bucket_acl)
+                if not hasattr(django_settings, 'AWS_BUCKET_ACL'):
+                    warnings.warn(
+                        "The default behavior of S3BotoStorage is insecure and will change "
+                        "in django-storages 1.8. By default new buckets are saved with an ACL of "
+                        "'public-read' (globally publicly readable). Version 1.8 will default to "
+                        "Amazon's default of the bucket owner. To opt into this behavior "
+                        "set AWS_BUCKET_ACL = None, otherwise to silence this warning explicitly set "
+                        "AWS_BUCKET_ACL.",
+                        DeprecationWarning,
+                    )
+                if self.bucket_acl:
+                    bucket.set_acl(self.bucket_acl)
                 return bucket
             raise ImproperlyConfigured('Bucket %s does not exist. Buckets '
                                        'can be automatically created by '

diff --git a/storages/backends/s3boto3.py b/storages/backends/s3boto3.py
@@ -379,6 +379,16 @@ def _get_or_create_bucket(self, name):
                     #
                     # Also note that Amazon specifically disallows "us-east-1" when passing bucket
                     # region names; LocationConstraint *must* be blank to create in US Standard.
+                    if not hasattr(django_settings, 'AWS_BUCKET_ACL'):
+                        warnings.warn(
+                            "The default behavior of S3Boto3Storage is insecure and will change "
+                            "in django-storages 2.0. By default new buckets are saved with an ACL of "
+                            "'public-read' (globally publicly readable). Version 2.0 will default to "
+                            "Amazon's default of the bucket owner. To opt into this behavior this warning "
+                            "set AWS_BUCKET_ACL = None, otherwise to silence this warning explicitly set "
+                            "AWS_BUCKET_ACL.",
+                            DeprecationWarning,
+                        )
                     if self.bucket_acl:
                         bucket_params = {'ACL': self.bucket_acl}
                     else: