[Windows] Security model for files permissions - STEP 1 (#6893)

This PR is the first part of #6497 to ease the integration, following the new plan propose by @bmw here: #6497 (comment)

This step 1 refactor existing certbot.compat module into certbot.compat.misc, without any logic changed. Package certbot.compat will host the new modules that constitute the security model for Windows.

* Create the certbot.compat package. Move logic in certbot.compat.misc

* Add doc

* Fix lint

* Correct mypy

* Update client.py

certbot/tests/account_test.py

@@ -12,11 +12,9 @@ import pytz
 
 from acme import messages
 
-from certbot import compat
-from certbot import errors
-
 import certbot.tests.util as test_util
-
+from certbot import errors
+from certbot.compat import misc
 
 KEY = jose.JWKRSA.load(test_util.load_vector("rsa512_key.pem"))
 
@@ -116,7 +114,7 @@ class AccountFileStorageTest(test_util.ConfigTestCase):
 
     def test_init_creates_dir(self):
         self.assertTrue(os.path.isdir(
-            compat.underscores_for_unsupported_characters_in_path(self.config.accounts_dir)))
+            misc.underscores_for_unsupported_characters_in_path(self.config.accounts_dir)))
 
     @test_util.broken_on_windows
     def test_save_and_restore(self):

certbot/tests/compat_test.py

@@ -1,8 +1,9 @@
 """Tests for certbot.compat."""
 import os
 
-from certbot import compat
 import certbot.tests.util as test_util
+from certbot.compat import misc
+
 
 class OsReplaceTest(test_util.TempDirTestCase):
     """Test to ensure consistent behavior of os_rename method"""
@@ -15,7 +16,7 @@ class OsReplaceTest(test_util.TempDirTestCase):
         open(dst, 'w').close()
 
         # On Windows, a direct call to os.rename will fail because dst already exists.
-        compat.os_rename(src, dst)
+        misc.os_rename(src, dst)
 
         self.assertFalse(os.path.exists(src))
         self.assertTrue(os.path.exists(dst))

certbot/tests/configuration_test.py

@@ -4,12 +4,12 @@ import unittest
 
 import mock
 
-from certbot import compat
 from certbot import constants
 from certbot import errors
-
+from certbot.compat import misc
 from certbot.tests import util as test_util
 
+
 class NamespaceConfigTest(test_util.ConfigTestCase):
     """Tests for certbot.configuration.NamespaceConfig."""
 
@@ -48,7 +48,7 @@ class NamespaceConfigTest(test_util.ConfigTestCase):
         mock_constants.KEY_DIR = 'keys'
         mock_constants.TEMP_CHECKPOINT_DIR = 't'
 
-        ref_path = compat.underscores_for_unsupported_characters_in_path(
+        ref_path = misc.underscores_for_unsupported_characters_in_path(
             'acc/acme-server.org:443/new')
         self.assertEqual(
             os.path.normpath(self.config.accounts_dir),

certbot/tests/display/util_test.py

@@ -4,14 +4,13 @@ import socket
 import tempfile
 import unittest
 
-import six
 import mock
+import six
 
 from certbot import errors
 from certbot import interfaces
 from certbot.display import util as display_util
 
-
 CHOICES = [("First", "Description1"), ("Second", "Description2")]
 TAGS = ["tag1", "tag2", "tag3"]
 TAGS_CHOICES = [("1", "tag1"), ("2", "tag2"), ("3", "tag3")]
@@ -32,7 +31,7 @@ class InputWithTimeoutTest(unittest.TestCase):
     def test_input(self, prompt=None):
         expected = "foo bar"
         stdin = six.StringIO(expected + "\n")
-        with mock.patch("certbot.compat.select.select") as mock_select:
+        with mock.patch("certbot.compat.misc.select.select") as mock_select:
             mock_select.return_value = ([stdin], [], [],)
             self.assertEqual(self._call(prompt), expected)
 

certbot/tests/log_test.py

@@ -8,14 +8,13 @@ import unittest
 
 import mock
 import six
-
 from acme import messages
 from acme.magic_typing import Optional  # pylint: disable=unused-import, no-name-in-module
 
-from certbot import compat
 from certbot import constants
 from certbot import errors
 from certbot import util
+from certbot.compat import misc
 from certbot.tests import util as test_util
 
 
@@ -261,7 +260,7 @@ class TempHandlerTest(unittest.TestCase):
 
     def test_permissions(self):
         self.assertTrue(
-            util.check_permissions(self.handler.path, 0o600, compat.os_geteuid()))
+            util.check_permissions(self.handler.path, 0o600, misc.os_geteuid()))
 
     def test_delete(self):
         self.handler.close()

certbot/tests/main_test.py

@@ -3,42 +3,41 @@
 # pylint: disable=too-many-lines
 from __future__ import print_function
 
+import datetime
 import itertools
 import json
-import mock
 import os
 import shutil
+import sys
+import tempfile
 import traceback
 import unittest
-import datetime
-import pytz
-import tempfile
-import sys
 
 import josepy as jose
+import mock
+import pytz
 import six
 from six.moves import reload_module  # pylint: disable=import-error
 
 from acme.magic_typing import List  # pylint: disable=unused-import, no-name-in-module
+
+import certbot.tests.util as test_util
 from certbot import account
 from certbot import cli
-from certbot import compat
-from certbot import constants
 from certbot import configuration
+from certbot import constants
 from certbot import crypto_util
 from certbot import errors
 from certbot import interfaces  # pylint: disable=unused-import
 from certbot import main
 from certbot import updater
 from certbot import util
-
+from certbot.compat import misc
 from certbot.plugins import disco
 from certbot.plugins import enhancements
 from certbot.plugins import manual
 from certbot.plugins import null
 
-import certbot.tests.util as test_util
-
 CERT_PATH = test_util.vector_path('cert_512.pem')
 CERT = test_util.vector_path('cert_512.pem')
 CSR = test_util.vector_path('csr_512.der')
@@ -1587,7 +1586,7 @@ class MakeOrVerifyNeededDirs(test_util.ConfigTestCase):
         for core_dir in (self.config.config_dir, self.config.work_dir,):
             mock_util.set_up_core_dir.assert_any_call(
                 core_dir, constants.CONFIG_DIRS_MODE,
-                compat.os_geteuid(), self.config.strict_permissions
+                misc.os_geteuid(), self.config.strict_permissions
             )
 
         hook_dirs = (self.config.renewal_pre_hooks_dir,
@@ -1596,7 +1595,7 @@ class MakeOrVerifyNeededDirs(test_util.ConfigTestCase):
         for hook_dir in hook_dirs:
             # default mode of 755 is used
             mock_util.make_or_verify_dir.assert_any_call(
-                hook_dir, uid=compat.os_geteuid(),
+                hook_dir, uid=misc.os_geteuid(),
                 strict=self.config.strict_permissions)
 
 

certbot/tests/reverter_test.py

@@ -10,7 +10,6 @@ import mock
 import six
 
 from certbot import errors
-
 from certbot.tests import util as test_util
 
 
@@ -356,7 +355,7 @@ class TestFullCheckpointsReverter(test_util.ConfigTestCase):
         self.assertRaises(
             errors.ReverterError, self.reverter.finalize_checkpoint, "Title")
 
-    @mock.patch("certbot.reverter.compat.os_rename")
+    @mock.patch("certbot.reverter.misc.os_rename")
     def test_finalize_checkpoint_no_rename_directory(self, mock_rename):
 
         self.reverter.add_to_checkpoint(self.sets[0], "perm save")

certbot/tests/storage_test.py

@@ -12,12 +12,10 @@ import pytz
 import six
 
 import certbot
-from certbot import compat
-from certbot import errors
-from certbot.storage import ALL_FOUR
-
 import certbot.tests.util as test_util
-
+from certbot import errors
+from certbot.compat import misc
+from certbot.storage import ALL_FOUR
 
 CERT = test_util.load_cert('cert_512.pem')
 
@@ -572,21 +570,21 @@ class RenewableCertTests(BaseRenewableCertTest):
         for kind in ALL_FOUR:
             self._write_out_kind(kind, 1)
         self.test_rc.update_all_links_to(1)
-        self.assertTrue(compat.compare_file_modes(
+        self.assertTrue(misc.compare_file_modes(
             os.stat(self.test_rc.version("privkey", 1)).st_mode, 0o600))
         os.chmod(self.test_rc.version("privkey", 1), 0o444)
         # If no new key, permissions should be the same (we didn't write any keys)
         self.test_rc.save_successor(1, b"newcert", None, b"new chain", self.config)
-        self.assertTrue(compat.compare_file_modes(
+        self.assertTrue(misc.compare_file_modes(
             os.stat(self.test_rc.version("privkey", 2)).st_mode, 0o444))
         # If new key, permissions should be kept as 644
         self.test_rc.save_successor(2, b"newcert", b"new_privkey", b"new chain", self.config)
-        self.assertTrue(compat.compare_file_modes(
+        self.assertTrue(misc.compare_file_modes(
             os.stat(self.test_rc.version("privkey", 3)).st_mode, 0o644))
         # If permissions reverted, next renewal will also revert permissions of new key
         os.chmod(self.test_rc.version("privkey", 3), 0o400)
         self.test_rc.save_successor(3, b"newcert", b"new_privkey", b"new chain", self.config)
-        self.assertTrue(compat.compare_file_modes(
+        self.assertTrue(misc.compare_file_modes(
             os.stat(self.test_rc.version("privkey", 4)).st_mode, 0o600))
 
     @test_util.broken_on_windows
@@ -624,7 +622,7 @@ class RenewableCertTests(BaseRenewableCertTest):
             self.config.live_dir, "README")))
         self.assertTrue(os.path.exists(os.path.join(
             self.config.live_dir, "the-lineage.com", "README")))
-        self.assertTrue(compat.compare_file_modes(os.stat(result.key_path).st_mode, 0o600))
+        self.assertTrue(misc.compare_file_modes(os.stat(result.key_path).st_mode, 0o600))
         with open(result.fullchain, "rb") as f:
             self.assertEqual(f.read(), b"cert" + b"chain")
         # Let's do it again and make sure it makes a different lineage

certbot/tests/util_test.py

@@ -8,9 +8,9 @@ import mock
 import six
 from six.moves import reload_module  # pylint: disable=import-error
 
-from certbot import compat
-from certbot import errors
 import certbot.tests.util as test_util
+from certbot import errors
+from certbot.compat import misc
 
 
 class RunScriptTest(unittest.TestCase):
@@ -119,7 +119,7 @@ class SetUpCoreDirTest(test_util.TempDirTestCase):
     @mock.patch('certbot.util.lock_dir_until_exit')
     def test_success(self, mock_lock):
         new_dir = os.path.join(self.tempdir, 'new')
-        self._call(new_dir, 0o700, compat.os_geteuid(), False)
+        self._call(new_dir, 0o700, misc.os_geteuid(), False)
         self.assertTrue(os.path.exists(new_dir))
         self.assertEqual(mock_lock.call_count, 1)
 
@@ -127,7 +127,7 @@ class SetUpCoreDirTest(test_util.TempDirTestCase):
     def test_failure(self, mock_make_or_verify):
         mock_make_or_verify.side_effect = OSError
         self.assertRaises(errors.Error, self._call,
-                          self.tempdir, 0o700, compat.os_geteuid(), False)
+                          self.tempdir, 0o700, misc.os_geteuid(), False)
 
 
 class MakeOrVerifyDirTest(test_util.TempDirTestCase):
@@ -144,7 +144,7 @@ class MakeOrVerifyDirTest(test_util.TempDirTestCase):
         self.path = os.path.join(self.tempdir, "foo")
         os.mkdir(self.path, 0o600)
 
-        self.uid = compat.os_geteuid()
+        self.uid = misc.os_geteuid()
 
     def _call(self, directory, mode):
         from certbot.util import make_or_verify_dir
@@ -154,11 +154,11 @@ class MakeOrVerifyDirTest(test_util.TempDirTestCase):
         path = os.path.join(self.tempdir, "bar")
         self._call(path, 0o650)
         self.assertTrue(os.path.isdir(path))
-        self.assertTrue(compat.compare_file_modes(os.stat(path).st_mode, 0o650))
+        self.assertTrue(misc.compare_file_modes(os.stat(path).st_mode, 0o650))
 
     def test_existing_correct_mode_does_not_fail(self):
         self._call(self.path, 0o600)
-        self.assertTrue(compat.compare_file_modes(os.stat(self.path).st_mode, 0o600))
+        self.assertTrue(misc.compare_file_modes(os.stat(self.path).st_mode, 0o600))
 
     @test_util.skip_on_windows('Umask modes are mostly ignored on Windows.')
     def test_existing_wrong_mode_fails(self):
@@ -181,7 +181,7 @@ class CheckPermissionsTest(test_util.TempDirTestCase):
     def setUp(self):
         super(CheckPermissionsTest, self).setUp()
 
-        self.uid = compat.os_geteuid()
+        self.uid = misc.os_geteuid()
 
     def _call(self, mode):
         from certbot.util import check_permissions
@@ -223,8 +223,8 @@ class UniqueFileTest(test_util.TempDirTestCase):
     def test_right_mode(self):
         fd1, name1 = self._call(0o700)
         fd2, name2 = self._call(0o600)
-        self.assertTrue(compat.compare_file_modes(0o700, os.stat(name1).st_mode))
-        self.assertTrue(compat.compare_file_modes(0o600, os.stat(name2).st_mode))
+        self.assertTrue(misc.compare_file_modes(0o700, os.stat(name1).st_mode))
+        self.assertTrue(misc.compare_file_modes(0o600, os.stat(name2).st_mode))
         fd1.close()
         fd2.close()