Merge pull request #1597 from lbeltrame/fix-webroot-permissions

Fix webroot permissions [mergeable, but enhancements also requested]

letsencrypt/plugins/webroot.py

@@ -2,6 +2,7 @@
 import errno
 import logging
 import os
+import stat
 
 import zope.interface
 
@@ -60,6 +61,17 @@ to serve all files under specified web root ({0})."""
                          self.full_roots[name])
             try:
                 os.makedirs(self.full_roots[name])
+                # Set permissions as parent directory (GH #1389)
+                # We don't use the parameters in makedirs because it
+                # may not always work
+                # https://stackoverflow.com/questions/5231901/permission-problems-when-creating-a-dir-with-os-makedirs-python
+                stat_path = os.stat(path)
+                filemode = stat.S_IMODE(stat_path.st_mode)
+                os.chmod(self.full_roots[name], filemode)
+                # Set owner and group, too
+                os.chown(self.full_roots[name], stat_path.st_uid,
+                          stat_path.st_gid)
+
             except OSError as exception:
                 if exception.errno != errno.EEXIST:
                     raise errors.PluginError(
@@ -87,6 +99,15 @@ to serve all files under specified web root ({0})."""
         logger.debug("Attempting to save validation to %s", path)
         with open(path, "w") as validation_file:
             validation_file.write(validation.encode())
+
+        # Set permissions as parent directory (GH #1389)
+        parent_path = self.full_roots[achall.domain]
+        stat_parent_path = os.stat(parent_path)
+        filemode = stat.S_IMODE(stat_parent_path.st_mode)
+        # Remove execution bit (not needed for this file)
+        os.chmod(path, filemode & ~stat.S_IEXEC)
+        os.chown(path, stat_parent_path.st_uid, stat_parent_path.st_gid)
+
         return response
 
     def cleanup(self, achalls):  # pylint: disable=missing-docstring

letsencrypt/plugins/webroot_test.py

@@ -3,6 +3,7 @@ import os
 import shutil
 import tempfile
 import unittest
+import stat
 
 import mock
 
@@ -69,6 +70,23 @@ class AuthenticatorTest(unittest.TestCase):
         self.assertRaises(errors.PluginError, self.auth.prepare)
         os.chmod(self.path, 0o700)
 
+    def test_prepare_permissions(self):
+
+        # Remove exec bit from permission check, so that it
+        # matches the file
+        responses = self.auth.perform([self.achall])
+        parent_permissions = (stat.S_IMODE(os.stat(self.path).st_mode) &
+                              ~stat.S_IEXEC)
+
+        actual_permissions = stat.S_IMODE(os.stat(self.validation_path).st_mode)
+
+        self.assertEqual(parent_permissions, actual_permissions)
+        parent_gid = os.stat(self.path).st_gid
+        parent_uid = os.stat(self.path).st_uid
+
+        self.assertEqual(os.stat(self.validation_path).st_gid, parent_gid)
+        self.assertEqual(os.stat(self.validation_path).st_uid, parent_uid)
+
     def test_perform_cleanup(self):
         responses = self.auth.perform([self.achall])
         self.assertEqual(1, len(responses))