[Windows] Fix account paths on Windows when colons are involved (#6711)

The account path used to store user credentials is calculated from the domain used to contact the relevant ACME CA server.

For instance, if the directory URL is https://my.domain.net/directory, then the account path will be $CONFIG_DIR/accounts/my.domain.net.

However, if non standard HTTP/HTTPS port need to be used, colons will be involved. For instance, https://my.domain.net:14000/directory will give $CONFIG_DIR/accounts/my.domain.net:14000.

Colons in paths are supported on POSIX systems, but not on Windows (it is reserved for the root drive letter).

This PR replaces colons by underscores for account paths on Windows, and leaves them untouched on Linux.

* Fix account path on Windows when colons are involved

* Protect colon in drive letter

* Refactor compat platform specific logic

certbot/account.py

@@ -142,7 +142,7 @@ class AccountFileStorage(interfaces.AccountStorage):
     def __init__(self, config):
         self.config = config
         util.make_or_verify_dir(config.accounts_dir, 0o700, compat.os_geteuid(),
-                                   self.config.strict_permissions)
+                                self.config.strict_permissions)
 
     def _account_dir_path(self, account_id):
         return self._account_dir_path_for_server_path(account_id, self.config.server_path)

certbot/compat.py

@@ -1,12 +1,8 @@
 """
 Compatibility layer to run certbot both on Linux and Windows.
 
-The approach used here is similar to Modernizr for Web browsers.
-We do not check the platform type to determine if a particular logic is supported.
-Instead, we apply a logic, and then fallback to another logic if first logic
-is not supported at runtime.
-
-Then logic chains are abstracted into single functions to be exposed to certbot.
+This module contains all required platform specific code,
+allowing the rest of Certbot codebase to be platform agnostic.
 """
 import os
 import select
@@ -27,6 +23,8 @@ except ImportError:
 UNPRIVILEGED_SUBCOMMANDS_ALLOWED = [
     'certificates', 'enhance', 'revoke', 'delete',
     'register', 'unregister', 'config_changes', 'plugins']
+
+
 def raise_for_non_administrative_windows_rights(subcommand):
     """
     On Windows, raise if current shell does not have the administrative rights.
@@ -50,6 +48,7 @@ def raise_for_non_administrative_windows_rights(subcommand):
                 'Error, "{0}" subcommand must be run on a shell with administrative rights.'
                 .format(subcommand))
 
+
 def os_geteuid():
     """
     Get current user uid
@@ -65,6 +64,7 @@ def os_geteuid():
         # Windows specific
         return 0
 
+
 def os_rename(src, dst):
     """
     Rename a file to a destination path and handles situations where the destination exists.
@@ -117,6 +117,7 @@ def readline_with_timeout(timeout, prompt):
         # So no timeout on Windows for now.
         return sys.stdin.readline()
 
+
 def lock_file(fd):
     """
     Lock the file linked to the specified file descriptor.
@@ -131,6 +132,7 @@ def lock_file(fd):
         # Windows specific
         msvcrt.locking(fd, msvcrt.LK_NBLCK, 1)
 
+
 def release_locked_file(fd, path):
     """
     Remove, close, and release a lock file specified by its file descriptor and its path.
@@ -164,15 +166,17 @@ def release_locked_file(fd, path):
     finally:
         os.close(fd)
 
+
 def compare_file_modes(mode1, mode2):
     """Return true if the two modes can be considered as equals for this platform"""
-    if 'fcntl' in sys.modules:
+    if os.name != 'nt':
         # Linux specific: standard compare
         return oct(stat.S_IMODE(mode1)) == oct(stat.S_IMODE(mode2))
     # Windows specific: most of mode bits are ignored on Windows. Only check user R/W rights.
     return (stat.S_IMODE(mode1) & stat.S_IREAD == stat.S_IMODE(mode2) & stat.S_IREAD
             and stat.S_IMODE(mode1) & stat.S_IWRITE == stat.S_IMODE(mode2) & stat.S_IWRITE)
 
+
 WINDOWS_DEFAULT_FOLDERS = {
     'config': 'C:\\Certbot',
     'work': 'C:\\Certbot\\lib',
@@ -184,6 +188,7 @@ LINUX_DEFAULT_FOLDERS = {
     'logs': '/var/log/letsencrypt',
 }
 
+
 def get_default_folder(folder_type):
     """
     Return the relevant default folder for the current OS
@@ -194,8 +199,25 @@ def get_default_folder(folder_type):
     :rtype: str
 
     """
-    if 'fcntl' in sys.modules:
+    if os.name != 'nt':
         # Linux specific
         return LINUX_DEFAULT_FOLDERS[folder_type]
     # Windows specific
     return WINDOWS_DEFAULT_FOLDERS[folder_type]
+
+
+def underscores_for_unsupported_characters_in_path(path):
+    # type: (str) -> str
+    """
+    Replace unsupported characters in path for current OS by underscores.
+    :param str path: the path to normalize
+    :return: the normalized path
+    :rtype: str
+    """
+    if os.name != 'nt':
+        # Linux specific
+        return path
+
+    # Windows specific
+    drive, tail = os.path.splitdrive(path)
+    return drive + tail.replace(':', '_')

certbot/configuration.py

@@ -5,6 +5,7 @@ import os
 from six.moves.urllib import parse  # pylint: disable=import-error
 import zope.interface
 
+from certbot import compat
 from certbot import constants
 from certbot import errors
 from certbot import interfaces
@@ -69,6 +70,7 @@ class NamespaceConfig(object):
 
     def accounts_dir_for_server_path(self, server_path):
         """Path to accounts directory based on server_path"""
+        server_path = compat.underscores_for_unsupported_characters_in_path(server_path)
         return os.path.join(
             self.namespace.config_dir, constants.ACCOUNTS_DIR, server_path)
 

certbot/tests/account_test.py

@@ -12,6 +12,7 @@ import pytz
 
 from acme import messages
 
+from certbot import compat
 from certbot import errors
 
 import certbot.tests.util as test_util
@@ -114,7 +115,8 @@ class AccountFileStorageTest(test_util.ConfigTestCase):
         self.mock_client.directory.new_authz = new_authzr_uri
 
     def test_init_creates_dir(self):
-        self.assertTrue(os.path.isdir(self.config.accounts_dir))
+        self.assertTrue(os.path.isdir(
+            compat.underscores_for_unsupported_characters_in_path(self.config.accounts_dir)))
 
     @test_util.broken_on_windows
     def test_save_and_restore(self):

certbot/tests/configuration_test.py

@@ -4,6 +4,7 @@ import unittest
 
 import mock
 
+from certbot import compat
 from certbot import constants
 from certbot import errors
 
@@ -47,9 +48,11 @@ 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(
+            'acc/acme-server.org:443/new')
         self.assertEqual(
             os.path.normpath(self.config.accounts_dir),
-            os.path.normpath(os.path.join(self.config.config_dir, 'acc/acme-server.org:443/new')))
+            os.path.normpath(os.path.join(self.config.config_dir, ref_path)))
         self.assertEqual(
             os.path.normpath(self.config.backup_dir),
             os.path.normpath(os.path.join(self.config.work_dir, 'backups')))