quiet and fast

.codecov.yml

@@ -6,13 +6,13 @@ coverage:
         flags: linux
         # Fixed target instead of auto set by #7173, can
         # be removed when flags in Codecov are added back.
-        target: 97.5
+        target: 97.6
         threshold: 0.1
         base: auto
       windows:
         flags: windows
         # Fixed target instead of auto set by #7173, can
         # be removed when flags in Codecov are added back.
-        target: 97.6
+        target: 97.0
         threshold: 0.1
         base: auto

.travis.yml

@@ -34,12 +34,15 @@ extended-test-suite: &extended-test-suite
 
 matrix:
   include:
-    # Main test suite
-    - python: "2.7"
-      env: ACME_SERVER=pebble TOXENV=integration
-      sudo: required
+    - sudo: required
+      env: TOXENV=docker_dev
       services: docker
-      <<: *not-on-master
+      addons:
+        apt:
+          packages:  # don't install nginx and apache
+            - libaugeas0
+      <<: *extended-test-suite
+
 # container-based infrastructure
 sudo: false
 

AUTHORS.md

@@ -15,7 +15,6 @@ Authors
 * [Alex Gaynor](https://github.com/alex)
 * [Alex Halderman](https://github.com/jhalderm)
 * [Alex Jordan](https://github.com/strugee)
-* [Alex Zorin](https://github.com/alexzorin)
 * [Amjad Mashaal](https://github.com/TheNavigat)
 * [Andrew Murray](https://github.com/radarhere)
 * [Anselm Levskaya](https://github.com/levskaya)

CHANGELOG.md

@@ -6,8 +6,7 @@ Certbot adheres to [Semantic Versioning](https://semver.org/).
 
 ### Added
 
-* Turn off session tickets for apache plugin by default
-* acme: Authz deactivation added to `acme` module.
+*
 
 ### Changed
 

Dockerfile

@@ -0,0 +1,35 @@
+FROM python:2-alpine3.9
+
+ENTRYPOINT [ "certbot" ]
+EXPOSE 80 443
+VOLUME /etc/letsencrypt /var/lib/letsencrypt
+WORKDIR /opt/certbot
+
+COPY CHANGELOG.md README.rst setup.py src/
+
+# Generate constraints file to pin dependency versions
+COPY letsencrypt-auto-source/pieces/dependency-requirements.txt .
+COPY tools /opt/certbot/tools
+RUN sh -c 'cat dependency-requirements.txt | /opt/certbot/tools/strip_hashes.py > unhashed_requirements.txt'
+RUN sh -c 'cat tools/dev_constraints.txt unhashed_requirements.txt | /opt/certbot/tools/merge_requirements.py > docker_constraints.txt'
+
+COPY acme src/acme
+COPY certbot src/certbot
+
+RUN apk add --no-cache --virtual .certbot-deps \
+        libffi \
+        libssl1.1 \
+        openssl \
+        ca-certificates \
+        binutils
+RUN apk add --no-cache --virtual .build-deps \
+        gcc \
+        linux-headers \
+        openssl-dev \
+        musl-dev \
+        libffi-dev \
+    && pip install -r /opt/certbot/dependency-requirements.txt \
+    && pip install --no-cache-dir --no-deps \
+        --editable /opt/certbot/src/acme \
+        --editable /opt/certbot/src \
+    && apk del .build-deps

acme/acme/client.py

@@ -123,21 +123,6 @@ class ClientBase(object):  # pylint: disable=too-many-instance-attributes
         """
         return self.update_registration(regr, update={'status': 'deactivated'})
 
-    def deactivate_authorization(self, authzr):
-        # type: (messages.AuthorizationResource) -> messages.AuthorizationResource
-        """Deactivate authorization.
-
-        :param messages.AuthorizationResource authzr: The Authorization resource
-            to be deactivated.
-
-        :returns: The Authorization resource that was deactivated.
-        :rtype: `.AuthorizationResource`
-
-        """
-        body = messages.UpdateAuthorization(status='deactivated')
-        response = self._post(authzr.uri, body)
-        return self._authzr_from_response(response)
-
     def _authzr_from_response(self, response, identifier=None, uri=None):
         authzr = messages.AuthorizationResource(
             body=messages.Authorization.from_json(response.json()),

acme/acme/client_test.py

@@ -637,14 +637,6 @@ class ClientTest(ClientTestBase):
             errors.PollError, self.client.poll_and_request_issuance,
             csr, authzrs, mintime=mintime, max_attempts=2)
 
-    def test_deactivate_authorization(self):
-        authzb = self.authzr.body.update(status=messages.STATUS_DEACTIVATED)
-        self.response.json.return_value = authzb.to_json()
-        authzr = self.client.deactivate_authorization(self.authzr)
-        self.assertEqual(authzb, authzr.body)
-        self.assertEqual(self.client.net.post.call_count, 1)
-        self.assertTrue(self.authzr.uri in self.net.post.call_args_list[0][0])
-
     def test_check_cert(self):
         self.response.headers['Location'] = self.certr.uri
         self.response.content = CERT_DER

acme/acme/messages.py

@@ -168,7 +168,6 @@ STATUS_VALID = Status('valid')
 STATUS_INVALID = Status('invalid')
 STATUS_REVOKED = Status('revoked')
 STATUS_READY = Status('ready')
-STATUS_DEACTIVATED = Status('deactivated')
 
 
 class IdentifierType(_Constant):
@@ -472,7 +471,7 @@ class Authorization(ResourceBody):
     :ivar datetime.datetime expires:
 
     """
-    identifier = jose.Field('identifier', decoder=Identifier.from_json, omitempty=True)
+    identifier = jose.Field('identifier', decoder=Identifier.from_json)
     challenges = jose.Field('challenges', omitempty=True)
     combinations = jose.Field('combinations', omitempty=True)
 
@@ -502,12 +501,6 @@ class NewAuthorization(Authorization):
     resource = fields.Resource(resource_type)
 
 
-class UpdateAuthorization(Authorization):
-    """Update authorization."""
-    resource_type = 'authz'
-    resource = fields.Resource(resource_type)
-
-
 class AuthorizationResource(ResourceWithURI):
     """Authorization Resource.
 

certbot-apache/MANIFEST.in

@@ -5,4 +5,3 @@ recursive-include certbot_apache/tests/testdata *
 include certbot_apache/centos-options-ssl-apache.conf
 include certbot_apache/options-ssl-apache.conf
 recursive-include certbot_apache/augeas_lens *.aug
-recursive-include certbot_apache/tls_configs *.conf

certbot-apache/certbot_apache/apache_util.py

@@ -1,8 +1,6 @@
 """ Utility functions for certbot-apache plugin """
 import binascii
 
-import pkg_resources
-
 from certbot import util
 from certbot.compat import os
 
@@ -107,15 +105,3 @@ def parse_define_file(filepath, varname):
 def unique_id():
     """ Returns an unique id to be used as a VirtualHost identifier"""
     return binascii.hexlify(os.urandom(16)).decode("utf-8")
-
-
-def find_ssl_apache_conf(prefix):
-    """
-    Find a TLS Apache config file in the dedicated storage.
-    :param str prefix: prefix of the TLS Apache config file to find
-    :return: the path the TLS Apache config file
-    :rtype: str
-    """
-    return pkg_resources.resource_filename(
-        "certbot_apache",
-        os.path.join("tls_configs", "{0}-options-ssl-apache.conf".format(prefix)))

certbot-apache/certbot_apache/centos-options-ssl-apache.conf

@@ -10,10 +10,16 @@ SSLEngine on
 SSLProtocol             all -SSLv2 -SSLv3
 SSLCipherSuite          ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA256:ECDHE-ECDSA-AES128-SHA:ECDHE-RSA-AES256-SHA384:ECDHE-RSA-AES128-SHA:ECDHE-ECDSA-AES256-SHA384:ECDHE-ECDSA-AES256-SHA:ECDHE-RSA-AES256-SHA:DHE-RSA-AES128-SHA256:DHE-RSA-AES128-SHA:DHE-RSA-AES256-SHA256:DHE-RSA-AES256-SHA:ECDHE-ECDSA-DES-CBC3-SHA:ECDHE-RSA-DES-CBC3-SHA:EDH-RSA-DES-CBC3-SHA:AES128-GCM-SHA256:AES256-GCM-SHA384:AES128-SHA256:AES256-SHA256:AES128-SHA:AES256-SHA:DES-CBC3-SHA:!DSS
 SSLHonorCipherOrder     on
-SSLSessionTickets       off
 
 SSLOptions +StrictRequire
 
 # Add vhost name to log entries:
 LogFormat "%h %l %u %t \"%r\" %>s %b \"%{Referer}i\" \"%{User-agent}i\"" vhost_combined
 LogFormat "%v %h %l %u %t \"%r\" %>s %b" vhost_common
+
+#CustomLog /var/log/apache2/access.log vhost_combined
+#LogLevel warn
+#ErrorLog /var/log/apache2/error.log
+
+# Always ensure Cookies have "Secure" set (JAH 2012/1)
+#Header edit Set-Cookie (?i)^(.*)(;\s*secure)??((\s*;)?(.*)) "$1; Secure$3$4"

certbot-apache/certbot_apache/configurator.py

@@ -9,6 +9,7 @@ import time
 
 from collections import defaultdict
 
+import pkg_resources
 import six
 
 import zope.component
@@ -22,7 +23,6 @@ from certbot import interfaces
 from certbot import util
 
 from certbot.achallenges import KeyAuthorizationAnnotatedChallenge  # pylint: disable=unused-import
-from certbot.compat import filesystem
 from certbot.compat import os
 from certbot.plugins import common
 from certbot.plugins.util import path_surgery
@@ -109,24 +109,14 @@ class ApacheConfigurator(common.Installer):
         handle_modules=False,
         handle_sites=False,
         challenge_location="/etc/apache2",
+        MOD_SSL_CONF_SRC=pkg_resources.resource_filename(
+            "certbot_apache", "options-ssl-apache.conf")
     )
 
     def option(self, key):
         """Get a value from options"""
         return self.options.get(key)
 
-    def pick_apache_config(self):
-        """
-        Pick the appropriate TLS Apache configuration file for current version of Apache and OS.
-        :return: the path to the TLS Apache configuration file to use
-        :rtype: str
-        """
-        # Disabling TLS session tickets is supported by Apache 2.4.11+.
-        # So for old versions of Apache we pick a configuration without this option.
-        if self.version < (2, 4, 11):
-            return apache_util.find_ssl_apache_conf("old")
-        return apache_util.find_ssl_apache_conf("current")
-
     def _prepare_options(self):
         """
         Set the values possibly changed by command line parameters to
@@ -905,7 +895,7 @@ class ApacheConfigurator(common.Installer):
                 if not new_vhost:
                     continue
                 internal_path = apache_util.get_internal_aug_path(new_vhost.path)
-                realpath = filesystem.realpath(new_vhost.filep)
+                realpath = os.path.realpath(new_vhost.filep)
                 if realpath not in file_paths:
                     file_paths[realpath] = new_vhost.filep
                     internal_paths[realpath].add(internal_path)
@@ -1231,11 +1221,11 @@ class ApacheConfigurator(common.Installer):
         """
 
         if self.conf("vhost-root") and os.path.exists(self.conf("vhost-root")):
-            fp = os.path.join(filesystem.realpath(self.option("vhost_root")),
+            fp = os.path.join(os.path.realpath(self.option("vhost_root")),
                               os.path.basename(non_ssl_vh_fp))
         else:
             # Use non-ssl filepath
-            fp = filesystem.realpath(non_ssl_vh_fp)
+            fp = os.path.realpath(non_ssl_vh_fp)
 
         if fp.endswith(".conf"):
             return fp[:-(len(".conf"))] + self.option("le_vhost_ext")
@@ -2348,9 +2338,8 @@ class ApacheConfigurator(common.Installer):
         # XXX if we ever try to enforce a local privilege boundary (eg, running
         # certbot for unprivileged users via setuid), this function will need
         # to be modified.
-        apache_config_path = self.pick_apache_config()
-        return common.install_version_controlled_file(
-            options_ssl, options_ssl_digest, apache_config_path, constants.ALL_SSL_OPTIONS_HASHES)
+        return common.install_version_controlled_file(options_ssl, options_ssl_digest,
+            self.option("MOD_SSL_CONF_SRC"), constants.ALL_SSL_OPTIONS_HASHES)
 
     def enable_autohsts(self, _unused_lineage, domains):
         """

certbot-apache/certbot_apache/constants.py

@@ -9,7 +9,6 @@ MOD_SSL_CONF_DEST = "options-ssl-apache.conf"
 UPDATED_MOD_SSL_CONF_DIGEST = ".updated-options-ssl-apache-conf-digest.txt"
 """Name of the hash of the updated or informed mod_ssl_conf as saved in `IConfig.config_dir`."""
 
-# NEVER REMOVE A SINGLE HASH FROM THIS LIST UNLESS YOU KNOW EXACTLY WHAT YOU ARE DOING!
 ALL_SSL_OPTIONS_HASHES = [
     '2086bca02db48daf93468332543c60ac6acdb6f0b58c7bfdf578a5d47092f82a',
     '4844d36c9a0f587172d9fa10f4f1c9518e3bcfa1947379f155e16a70a728c21a',
@@ -19,10 +18,6 @@ ALL_SSL_OPTIONS_HASHES = [
     'cfdd7c18d2025836ea3307399f509cfb1ebf2612c87dd600a65da2a8e2f2797b',
     '80720bd171ccdc2e6b917ded340defae66919e4624962396b992b7218a561791',
     'c0c022ea6b8a51ecc8f1003d0a04af6c3f2bc1c3ce506b3c2dfc1f11ef931082',
-    '717b0a89f5e4c39b09a42813ac6e747cfbdeb93439499e73f4f70a1fe1473f20',
-    '0fcdc81280cd179a07ec4d29d3595068b9326b455c488de4b09f585d5dafc137',
-    '86cc09ad5415cd6d5f09a947fe2501a9344328b1e8a8b458107ea903e80baa6c',
-    '06675349e457eae856120cdebb564efe546f0b87399f2264baeb41e442c724c7',
 ]
 """SHA256 hashes of the contents of previous versions of all versions of MOD_SSL_CONF_SRC"""
 

certbot-apache/certbot_apache/options-ssl-apache.conf

@@ -11,10 +11,16 @@ SSLProtocol             all -SSLv2 -SSLv3
 SSLCipherSuite          ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA256:ECDHE-ECDSA-AES128-SHA:ECDHE-RSA-AES256-SHA384:ECDHE-RSA-AES128-SHA:ECDHE-ECDSA-AES256-SHA384:ECDHE-ECDSA-AES256-SHA:ECDHE-RSA-AES256-SHA:DHE-RSA-AES128-SHA256:DHE-RSA-AES128-SHA:DHE-RSA-AES256-SHA256:DHE-RSA-AES256-SHA:ECDHE-ECDSA-DES-CBC3-SHA:ECDHE-RSA-DES-CBC3-SHA:EDH-RSA-DES-CBC3-SHA:AES128-GCM-SHA256:AES256-GCM-SHA384:AES128-SHA256:AES256-SHA256:AES128-SHA:AES256-SHA:DES-CBC3-SHA:!DSS
 SSLHonorCipherOrder     on
 SSLCompression          off
-SSLSessionTickets       off
 
 SSLOptions +StrictRequire
 
 # Add vhost name to log entries:
 LogFormat "%h %l %u %t \"%r\" %>s %b \"%{Referer}i\" \"%{User-agent}i\"" vhost_combined
 LogFormat "%v %h %l %u %t \"%r\" %>s %b" vhost_common
+
+#CustomLog /var/log/apache2/access.log vhost_combined
+#LogLevel warn
+#ErrorLog /var/log/apache2/error.log
+
+# Always ensure Cookies have "Secure" set (JAH 2012/1)
+#Header edit Set-Cookie (?i)^(.*)(;\s*secure)??((\s*;)?(.*)) "$1; Secure$3$4"

certbot-apache/certbot_apache/override_arch.py

@@ -1,4 +1,6 @@
 """ Distribution specific override class for Arch Linux """
+import pkg_resources
+
 import zope.interface
 
 from certbot import interfaces
@@ -24,4 +26,6 @@ class ArchConfigurator(configurator.ApacheConfigurator):
         handle_modules=False,
         handle_sites=False,
         challenge_location="/etc/httpd/conf",
+        MOD_SSL_CONF_SRC=pkg_resources.resource_filename(
+            "certbot_apache", "options-ssl-apache.conf")
     )

certbot-apache/certbot_apache/override_centos.py

@@ -1,6 +1,7 @@
 """ Distribution specific override class for CentOS family (RHEL, Fedora) """
 import logging
 
+import pkg_resources
 import zope.interface
 
 from certbot import errors
@@ -38,6 +39,8 @@ class CentOSConfigurator(configurator.ApacheConfigurator):
         handle_modules=False,
         handle_sites=False,
         challenge_location="/etc/httpd/conf.d",
+        MOD_SSL_CONF_SRC=pkg_resources.resource_filename(
+            "certbot_apache", "centos-options-ssl-apache.conf")
     )
 
     def config_test(self):
@@ -72,18 +75,6 @@ class CentOSConfigurator(configurator.ApacheConfigurator):
         # Finish with actual config check to see if systemctl restart helped
         super(CentOSConfigurator, self).config_test()
 
-    def pick_apache_config(self):
-        """
-        Pick the appropriate TLS Apache configuration file for current version of Apache and OS.
-        :return: the path to the TLS Apache configuration file to use
-        :rtype: str
-        """
-        # Disabling TLS session tickets is supported by Apache 2.4.11+.
-        # So for old versions of Apache we pick a configuration without this option.
-        if self.version < (2, 4, 11):
-            return apache_util.find_ssl_apache_conf("centos-old")
-        return apache_util.find_ssl_apache_conf("centos-current")
-
     def _prepare_options(self):
         """
         Override the options dictionary initialization in order to support

certbot-apache/certbot_apache/override_darwin.py

@@ -1,4 +1,6 @@
 """ Distribution specific override class for macOS """
+import pkg_resources
+
 import zope.interface
 
 from certbot import interfaces
@@ -24,4 +26,6 @@ class DarwinConfigurator(configurator.ApacheConfigurator):
         handle_modules=False,
         handle_sites=False,
         challenge_location="/etc/apache2/other",
+        MOD_SSL_CONF_SRC=pkg_resources.resource_filename(
+            "certbot_apache", "options-ssl-apache.conf")
     )

certbot-apache/certbot_apache/override_debian.py

@@ -1,12 +1,12 @@
 """ Distribution specific override class for Debian family (Ubuntu/Debian) """
 import logging
 
+import pkg_resources
 import zope.interface
 
 from certbot import errors
 from certbot import interfaces
 from certbot import util
-from certbot.compat import filesystem
 from certbot.compat import os
 
 from certbot_apache import apache_util
@@ -34,6 +34,8 @@ class DebianConfigurator(configurator.ApacheConfigurator):
         handle_modules=True,
         handle_sites=True,
         challenge_location="/etc/apache2",
+        MOD_SSL_CONF_SRC=pkg_resources.resource_filename(
+            "certbot_apache", "options-ssl-apache.conf")
     )
 
     def enable_site(self, vhost):
@@ -63,7 +65,7 @@ class DebianConfigurator(configurator.ApacheConfigurator):
         try:
             os.symlink(vhost.filep, enabled_path)
         except OSError as err:
-            if os.path.islink(enabled_path) and filesystem.realpath(
+            if os.path.islink(enabled_path) and os.path.realpath(
                enabled_path) == vhost.filep:
                 # Already in shape
                 vhost.enabled = True

certbot-apache/certbot_apache/override_fedora.py

@@ -1,4 +1,5 @@
 """ Distribution specific override class for Fedora 29+ """
+import pkg_resources
 import zope.interface
 
 from certbot import errors
@@ -30,6 +31,9 @@ class FedoraConfigurator(configurator.ApacheConfigurator):
         handle_modules=False,
         handle_sites=False,
         challenge_location="/etc/httpd/conf.d",
+        MOD_SSL_CONF_SRC=pkg_resources.resource_filename(
+            # TODO: eventually newest version of Fedora will need their own config
+            "certbot_apache", "centos-options-ssl-apache.conf")
     )
 
     def config_test(self):

certbot-apache/certbot_apache/override_gentoo.py

@@ -1,4 +1,6 @@
 """ Distribution specific override class for Gentoo Linux """
+import pkg_resources
+
 import zope.interface
 
 from certbot import interfaces
@@ -27,6 +29,8 @@ class GentooConfigurator(configurator.ApacheConfigurator):
         handle_modules=False,
         handle_sites=False,
         challenge_location="/etc/apache2/vhosts.d",
+        MOD_SSL_CONF_SRC=pkg_resources.resource_filename(
+            "certbot_apache", "options-ssl-apache.conf")
     )
 
     def _prepare_options(self):

certbot-apache/certbot_apache/override_suse.py

@@ -1,4 +1,6 @@
 """ Distribution specific override class for OpenSUSE """
+import pkg_resources
+
 import zope.interface
 
 from certbot import interfaces
@@ -24,4 +26,6 @@ class OpenSUSEConfigurator(configurator.ApacheConfigurator):
         handle_modules=False,
         handle_sites=False,
         challenge_location="/etc/apache2/vhosts.d",
+        MOD_SSL_CONF_SRC=pkg_resources.resource_filename(
+            "certbot_apache", "options-ssl-apache.conf")
     )

certbot-apache/certbot_apache/tests/centos_test.py

@@ -4,7 +4,6 @@ import unittest
 import mock
 
 from certbot import errors
-from certbot.compat import filesystem
 from certbot.compat import os
 
 from certbot_apache import obj
@@ -161,7 +160,7 @@ class MultipleVhostsTestCentOS(util.ApacheTest):
         """Make sure we read the sysconfig OPTIONS variable correctly"""
         # Return nothing for the process calls
         mock_cfg.return_value = ""
-        self.config.parser.sysconfig_filep = filesystem.realpath(
+        self.config.parser.sysconfig_filep = os.path.realpath(
             os.path.join(self.config.parser.root, "../sysconfig/httpd"))
         self.config.parser.variables = {}
 
@@ -190,13 +189,6 @@ class MultipleVhostsTestCentOS(util.ApacheTest):
                                        errors.SubprocessError]
         self.assertRaises(errors.MisconfigurationError, self.config.restart)
 
-    def test_pick_correct_tls_config(self):
-        self.config.version = (2, 4, 10)
-        self.assertTrue('centos-old' in self.config.pick_apache_config())
-
-        self.config.version = (2, 4, 11)
-        self.assertTrue('centos-current' in self.config.pick_apache_config())
-
 
 if __name__ == "__main__":
     unittest.main()  # pragma: no cover

certbot-apache/certbot_apache/tests/configurator_test.py

@@ -675,7 +675,8 @@ class MultipleVhostsTest(util.ApacheTest):
     def test_make_vhost_ssl_nonexistent_vhost_path(self):
         ssl_vhost = self.config.make_vhost_ssl(self.vh_truth[1])
         self.assertEqual(os.path.dirname(ssl_vhost.filep),
-                         os.path.dirname(filesystem.realpath(self.vh_truth[1].filep)))
+                            os.path.dirname(os.path.realpath(
+                                self.vh_truth[1].filep)))
 
     def test_make_vhost_ssl(self):
         ssl_vhost = self.config.make_vhost_ssl(self.vh_truth[0])
@@ -1335,7 +1336,7 @@ class MultipleVhostsTest(util.ApacheTest):
         self.config.parser.modules.add("ssl_module")
         self.config.parser.modules.add("mod_ssl.c")
         self.config.parser.modules.add("socache_shmcb_module")
-        tmp_path = filesystem.realpath(tempfile.mkdtemp("vhostroot"))
+        tmp_path = os.path.realpath(tempfile.mkdtemp("vhostroot"))
         filesystem.chmod(tmp_path, 0o755)
         mock_p = "certbot_apache.configurator.ApacheConfigurator._get_ssl_vhost_path"
         mock_a = "certbot_apache.parser.ApacheParser.add_include"
@@ -1706,7 +1707,7 @@ class InstallSslOptionsConfTest(util.ApacheTest):
                                              self.config.updated_mod_ssl_conf_digest)
 
     def _current_ssl_options_hash(self):
-        return crypto_util.sha256sum(self.config.pick_apache_config())
+        return crypto_util.sha256sum(self.config.option("MOD_SSL_CONF_SRC"))
 
     def _assert_current_file(self):
         self.assertTrue(os.path.isfile(self.config.mod_ssl_conf))
@@ -1742,7 +1743,7 @@ class InstallSslOptionsConfTest(util.ApacheTest):
             self.assertFalse(mock_logger.warning.called)
         self.assertTrue(os.path.isfile(self.config.mod_ssl_conf))
         self.assertEqual(crypto_util.sha256sum(
-            self.config.pick_apache_config()),
+            self.config.option("MOD_SSL_CONF_SRC")),
             self._current_ssl_options_hash())
         self.assertNotEqual(crypto_util.sha256sum(self.config.mod_ssl_conf),
             self._current_ssl_options_hash())
@@ -1758,31 +1759,18 @@ class InstallSslOptionsConfTest(util.ApacheTest):
                 "%s has been manually modified; updated file "
                 "saved to %s. We recommend updating %s for security purposes.")
         self.assertEqual(crypto_util.sha256sum(
-            self.config.pick_apache_config()),
+            self.config.option("MOD_SSL_CONF_SRC")),
             self._current_ssl_options_hash())
         # only print warning once
         with mock.patch("certbot.plugins.common.logger") as mock_logger:
             self._call()
             self.assertFalse(mock_logger.warning.called)
 
-    def test_ssl_config_files_hash_in_all_hashes(self):
-        """
-        It is really critical that all TLS Apache config files have their SHA256 hash registered in
-        constants.ALL_SSL_OPTIONS_HASHES. Otherwise Certbot will mistakenly assume that the config
-        file has been manually edited by the user, and will refuse to update it.
-        This test ensures that all necessary hashes are present.
-        """
+    def test_current_file_hash_in_all_hashes(self):
         from certbot_apache.constants import ALL_SSL_OPTIONS_HASHES
-        import pkg_resources
-        tls_configs_dir = pkg_resources.resource_filename("certbot_apache", "tls_configs")
-        all_files = [os.path.join(tls_configs_dir, name) for name in os.listdir(tls_configs_dir)
-                     if name.endswith('options-ssl-apache.conf')]
-        self.assertTrue(all_files)
-        for one_file in all_files:
-            file_hash = crypto_util.sha256sum(one_file)
-            self.assertTrue(file_hash in ALL_SSL_OPTIONS_HASHES,
-                            "Constants.ALL_SSL_OPTIONS_HASHES must be appended with the sha256 "
-                            "hash of {0} when it is updated.".format(one_file))
+        self.assertTrue(self._current_ssl_options_hash() in ALL_SSL_OPTIONS_HASHES,
+            "Constants.ALL_SSL_OPTIONS_HASHES must be appended"
+            " with the sha256 hash of self.config.mod_ssl_conf when it is updated.")
 
 
 if __name__ == "__main__":

certbot-apache/certbot_apache/tests/debian_test.py

@@ -79,9 +79,9 @@ class MultipleVhostsTestDebian(util.ApacheTest):
 
     def test_enable_site_failure(self):
         self.config.parser.root = "/tmp/nonexistent"
-        with mock.patch("certbot.compat.os.path.isdir") as mock_dir:
+        with mock.patch("os.path.isdir") as mock_dir:
             mock_dir.return_value = True
-            with mock.patch("certbot.compat.os.path.islink") as mock_link:
+            with mock.patch("os.path.islink") as mock_link:
                 mock_link.return_value = False
                 self.assertRaises(
                     errors.NotSupportedError,

certbot-apache/certbot_apache/tests/fedora_test.py

@@ -4,7 +4,6 @@ import unittest
 import mock
 
 from certbot import errors
-from certbot.compat import filesystem
 from certbot.compat import os
 
 from certbot_apache import obj
@@ -161,7 +160,7 @@ class MultipleVhostsTestFedora(util.ApacheTest):
         """Make sure we read the sysconfig OPTIONS variable correctly"""
         # Return nothing for the process calls
         mock_cfg.return_value = ""
-        self.config.parser.sysconfig_filep = filesystem.realpath(
+        self.config.parser.sysconfig_filep = os.path.realpath(
             os.path.join(self.config.parser.root, "../sysconfig/httpd"))
         self.config.parser.variables = {}
 

certbot-apache/certbot_apache/tests/gentoo_test.py

@@ -4,7 +4,6 @@ import unittest
 import mock
 
 from certbot import errors
-from certbot.compat import filesystem
 from certbot.compat import os
 
 from certbot_apache import obj
@@ -82,7 +81,7 @@ class MultipleVhostsTestGentoo(util.ApacheTest):
         """Make sure we read the Gentoo APACHE2_OPTS variable correctly"""
         defines = ['DEFAULT_VHOST', 'INFO',
                    'SSL', 'SSL_DEFAULT_VHOST', 'LANGUAGE']
-        self.config.parser.apacheconfig_filep = filesystem.realpath(
+        self.config.parser.apacheconfig_filep = os.path.realpath(
             os.path.join(self.config.parser.root, "../conf.d/apache2"))
         self.config.parser.variables = {}
         with mock.patch("certbot_apache.override_gentoo.GentooParser.update_modules"):

certbot-apache/certbot_apache/tls_configs/centos-old-options-ssl-apache.conf

@@ -1,18 +0,0 @@
-# This file contains important security parameters. If you modify this file
-# manually, Certbot will be unable to automatically provide future security
-# updates. Instead, Certbot will print and log an error message with a path to
-# the up-to-date file that you will need to refer to when manually updating
-# this file.
-
-SSLEngine on
-
-# Intermediate configuration, tweak to your needs
-SSLProtocol             all -SSLv2 -SSLv3
-SSLCipherSuite          ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA256:ECDHE-ECDSA-AES128-SHA:ECDHE-RSA-AES256-SHA384:ECDHE-RSA-AES128-SHA:ECDHE-ECDSA-AES256-SHA384:ECDHE-ECDSA-AES256-SHA:ECDHE-RSA-AES256-SHA:DHE-RSA-AES128-SHA256:DHE-RSA-AES128-SHA:DHE-RSA-AES256-SHA256:DHE-RSA-AES256-SHA:ECDHE-ECDSA-DES-CBC3-SHA:ECDHE-RSA-DES-CBC3-SHA:EDH-RSA-DES-CBC3-SHA:AES128-GCM-SHA256:AES256-GCM-SHA384:AES128-SHA256:AES256-SHA256:AES128-SHA:AES256-SHA:DES-CBC3-SHA:!DSS
-SSLHonorCipherOrder     on
-
-SSLOptions +StrictRequire
-
-# Add vhost name to log entries:
-LogFormat "%h %l %u %t \"%r\" %>s %b \"%{Referer}i\" \"%{User-agent}i\"" vhost_combined
-LogFormat "%v %h %l %u %t \"%r\" %>s %b" vhost_common

certbot-apache/certbot_apache/tls_configs/old-options-ssl-apache.conf

@@ -1,19 +0,0 @@
-# This file contains important security parameters. If you modify this file
-# manually, Certbot will be unable to automatically provide future security
-# updates. Instead, Certbot will print and log an error message with a path to
-# the up-to-date file that you will need to refer to when manually updating
-# this file.
-
-SSLEngine on
-
-# Intermediate configuration, tweak to your needs
-SSLProtocol             all -SSLv2 -SSLv3
-SSLCipherSuite          ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA256:ECDHE-ECDSA-AES128-SHA:ECDHE-RSA-AES256-SHA384:ECDHE-RSA-AES128-SHA:ECDHE-ECDSA-AES256-SHA384:ECDHE-ECDSA-AES256-SHA:ECDHE-RSA-AES256-SHA:DHE-RSA-AES128-SHA256:DHE-RSA-AES128-SHA:DHE-RSA-AES256-SHA256:DHE-RSA-AES256-SHA:ECDHE-ECDSA-DES-CBC3-SHA:ECDHE-RSA-DES-CBC3-SHA:EDH-RSA-DES-CBC3-SHA:AES128-GCM-SHA256:AES256-GCM-SHA384:AES128-SHA256:AES256-SHA256:AES128-SHA:AES256-SHA:DES-CBC3-SHA:!DSS
-SSLHonorCipherOrder     on
-SSLCompression          off
-
-SSLOptions +StrictRequire
-
-# Add vhost name to log entries:
-LogFormat "%h %l %u %t \"%r\" %>s %b \"%{Referer}i\" \"%{User-agent}i\"" vhost_combined
-LogFormat "%v %h %l %u %t \"%r\" %>s %b" vhost_common

certbot-apache/local-oldest-requirements.txt

@@ -1,3 +1,3 @@
 # Remember to update setup.py to match the package versions below.
 acme[dev]==0.29.0
--e .[dev]
+certbot[dev]==0.36.0

certbot-apache/setup.py

@@ -10,7 +10,7 @@ version = '0.37.0.dev0'
 # acme/certbot version.
 install_requires = [
     'acme>=0.29.0',
-    'certbot>=0.37.0.dev0',
+    'certbot>=0.36.0',
     'mock',
     'python-augeas',
     'setuptools',

certbot-ci/certbot_integration_tests/utils/acme_server.py

@@ -159,7 +159,7 @@ class ACMEServer(object):
 
         # Wait for the ACME CA server to be up.
         print('=> Waiting for boulder instance to respond...')
-        misc.check_until_timeout(self.acme_xdist['directory_url'], attempts=240)
+        misc.check_until_timeout(self.acme_xdist['directory_url'])
 
         # Configure challtestsrv to answer any A record request with ip of the docker host.
         response = requests.post('http://localhost:{0}/set-default-ipv4'.format(CHALLTESTSRV_PORT),

certbot-ci/certbot_integration_tests/utils/misc.py

@@ -28,13 +28,12 @@ RSA_KEY_TYPE = 'rsa'
 ECDSA_KEY_TYPE = 'ecdsa'
 
 
-def check_until_timeout(url, attempts=30):
+def check_until_timeout(url):
     """
     Wait and block until given url responds with status 200, or raise an exception
-    after the specified number of attempts.
+    after 150 attempts.
     :param str url: the URL to test
-    :param int attempts: the number of times to try to connect to the URL
-    :raise ValueError: exception raised if unable to reach the URL
+    :raise ValueError: exception raised after 150 unsuccessful attempts to reach the URL
     """
     try:
         import urllib3
@@ -44,7 +43,7 @@ def check_until_timeout(url, attempts=30):
         from requests.packages.urllib3.exceptions import InsecureRequestWarning
         requests.packages.urllib3.disable_warnings(InsecureRequestWarning)
 
-    for _ in range(attempts):
+    for _ in range(0, 150):
         time.sleep(1)
         try:
             if requests.get(url, verify=False).status_code == 200:
@@ -52,7 +51,7 @@ def check_until_timeout(url, attempts=30):
         except requests.exceptions.ConnectionError:
             pass
 
-    raise ValueError('Error, url did not respond after {0} attempts: {1}'.format(attempts, url))
+    raise ValueError('Error, url did not respond after 150 attempts: {0}'.format(url))
 
 
 class GracefulTCPServer(socketserver.TCPServer):

certbot-dns-cloudflare/Dockerfile

@@ -0,0 +1,5 @@
+FROM certbot/certbot
+
+COPY . src/certbot-dns-cloudflare
+
+RUN pip install --constraint docker_constraints.txt --no-cache-dir --editable src/certbot-dns-cloudflare

certbot-dns-cloudflare/certbot_dns_cloudflare/__init__.py

@@ -22,9 +22,7 @@ Credentials
 
 Use of this plugin requires a configuration file containing Cloudflare API
 credentials, obtained from your Cloudflare
-`account page <https://www.cloudflare.com/a/account/my-account>`_. This plugin
-does not currently support Cloudflare's "API Tokens", so please ensure you use
-the "Global API Key" for authentication.
+`account page <https://www.cloudflare.com/a/account/my-account>`_.
 
 .. code-block:: ini
    :name: credentials.ini

certbot-dns-cloudxns/Dockerfile

@@ -0,0 +1,5 @@
+FROM certbot/certbot
+
+COPY . src/certbot-dns-cloudxns
+
+RUN pip install --constraint docker_constraints.txt --no-cache-dir --editable src/certbot-dns-cloudxns

certbot-dns-digitalocean/Dockerfile

@@ -0,0 +1,5 @@
+FROM certbot/certbot
+
+COPY . src/certbot-dns-digitalocean
+
+RUN pip install --constraint docker_constraints.txt --no-cache-dir --editable src/certbot-dns-digitalocean

certbot-dns-dnsimple/Dockerfile

@@ -0,0 +1,5 @@
+FROM certbot/certbot
+
+COPY . src/certbot-dns-dnsimple
+
+RUN pip install --constraint docker_constraints.txt --no-cache-dir --editable src/certbot-dns-dnsimple

certbot-dns-dnsmadeeasy/Dockerfile

@@ -0,0 +1,5 @@
+FROM certbot/certbot
+
+COPY . src/certbot-dns-dnsmadeeasy
+
+RUN pip install --constraint docker_constraints.txt --no-cache-dir --editable src/certbot-dns-dnsmadeeasy

certbot-dns-gehirn/Dockerfile

@@ -0,0 +1,5 @@
+FROM certbot/certbot
+
+COPY . src/certbot-dns-gehirn
+
+RUN pip install --constraint docker_constraints.txt --no-cache-dir --editable src/certbot-dns-gehirn

certbot-dns-google/Dockerfile

@@ -0,0 +1,5 @@
+FROM certbot/certbot
+
+COPY . src/certbot-dns-google
+
+RUN pip install --constraint docker_constraints.txt --no-cache-dir --editable src/certbot-dns-google

certbot-dns-linode/Dockerfile

@@ -0,0 +1,5 @@
+FROM certbot/certbot
+
+COPY . src/certbot-dns-linode
+
+RUN pip install --constraint docker_constraints.txt --no-cache-dir --editable src/certbot-dns-linode

certbot-dns-luadns/Dockerfile

@@ -0,0 +1,5 @@
+FROM certbot/certbot
+
+COPY . src/certbot-dns-luadns
+
+RUN pip install --constraint docker_constraints.txt --no-cache-dir --editable src/certbot-dns-luadns

certbot-dns-nsone/Dockerfile

@@ -0,0 +1,5 @@
+FROM certbot/certbot
+
+COPY . src/certbot-dns-nsone
+
+RUN pip install --constraint docker_constraints.txt --no-cache-dir --editable src/certbot-dns-nsone

certbot-dns-ovh/Dockerfile

@@ -0,0 +1,5 @@
+FROM certbot/certbot
+
+COPY . src/certbot-dns-ovh
+
+RUN pip install --constraint docker_constraints.txt --no-cache-dir --editable src/certbot-dns-ovh

certbot-dns-rfc2136/Dockerfile

@@ -0,0 +1,5 @@
+FROM certbot/certbot
+
+COPY . src/certbot-dns-rfc2136
+
+RUN pip install --constraint docker_constraints.txt --no-cache-dir --editable src/certbot-dns-rfc2136

certbot-dns-route53/Dockerfile

@@ -0,0 +1,5 @@
+FROM certbot/certbot
+
+COPY . src/certbot-dns-route53
+
+RUN pip install --constraint docker_constraints.txt --no-cache-dir --editable src/certbot-dns-route53

certbot-dns-sakuracloud/Dockerfile

@@ -0,0 +1,5 @@
+FROM certbot/certbot
+
+COPY . src/certbot-dns-sakuracloud
+
+RUN pip install --constraint docker_constraints.txt --no-cache-dir --editable src/certbot-dns-sakuracloud

certbot-nginx/certbot_nginx/configurator.py

@@ -20,6 +20,7 @@ from certbot import crypto_util
 from certbot import errors
 from certbot import interfaces
 from certbot import util
+from certbot.compat import misc
 from certbot.compat import os
 from certbot.plugins import common
 
@@ -902,9 +903,13 @@ class NginxConfigurator(common.Installer):
         have permissions of root.
 
         """
-        util.make_or_verify_dir(self.config.work_dir, core_constants.CONFIG_DIRS_MODE)
-        util.make_or_verify_dir(self.config.backup_dir, core_constants.CONFIG_DIRS_MODE)
-        util.make_or_verify_dir(self.config.config_dir, core_constants.CONFIG_DIRS_MODE)
+        uid = misc.os_geteuid()
+        util.make_or_verify_dir(
+            self.config.work_dir, core_constants.CONFIG_DIRS_MODE, uid)
+        util.make_or_verify_dir(
+            self.config.backup_dir, core_constants.CONFIG_DIRS_MODE, uid)
+        util.make_or_verify_dir(
+            self.config.config_dir, core_constants.CONFIG_DIRS_MODE, uid)
 
     def get_version(self):
         """Return version of Nginx Server.

certbot/account.py

@@ -20,6 +20,7 @@ from certbot import constants
 from certbot import errors
 from certbot import interfaces
 from certbot import util
+from certbot.compat import misc
 from certbot.compat import os
 
 logger = logging.getLogger(__name__)
@@ -138,7 +139,8 @@ class AccountFileStorage(interfaces.AccountStorage):
     """
     def __init__(self, config):
         self.config = config
-        util.make_or_verify_dir(config.accounts_dir, 0o700, self.config.strict_permissions)
+        util.make_or_verify_dir(config.accounts_dir, 0o700, misc.os_geteuid(),
+                                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)
@@ -320,7 +322,8 @@ class AccountFileStorage(interfaces.AccountStorage):
 
     def _save(self, account, acme, regr_only):
         account_dir_path = self._account_dir_path(account.id)
-        util.make_or_verify_dir(account_dir_path, 0o700, self.config.strict_permissions)
+        util.make_or_verify_dir(account_dir_path, 0o700, misc.os_geteuid(),
+                                self.config.strict_permissions)
         try:
             with open(self._regr_path(account_dir_path), "w") as regr_file:
                 regr = account.regr

certbot/cert_manager.py

@@ -15,6 +15,7 @@ from certbot import interfaces
 from certbot import ocsp
 from certbot import storage
 from certbot import util
+from certbot.compat import misc
 from certbot.compat import os
 from certbot.display import util as display_util
 
@@ -105,7 +106,7 @@ def lineage_for_certname(cli_config, certname):
     """Find a lineage object with name certname."""
     configs_dir = cli_config.renewal_configs_dir
     # Verify the directory is there
-    util.make_or_verify_dir(configs_dir, mode=0o755)
+    util.make_or_verify_dir(configs_dir, mode=0o755, uid=misc.os_geteuid())
     try:
         renewal_file = storage.renewal_file_for_certname(cli_config, certname)
     except errors.CertStorageError:
@@ -374,7 +375,7 @@ def _search_lineages(cli_config, func, initial_rv, *args):
     """
     configs_dir = cli_config.renewal_configs_dir
     # Verify the directory is there
-    util.make_or_verify_dir(configs_dir, mode=0o755)
+    util.make_or_verify_dir(configs_dir, mode=0o755, uid=misc.os_geteuid())
 
     rv = initial_rv
     for renewal_file in storage.renewal_conf_files(cli_config):

certbot/client.py

@@ -30,6 +30,7 @@ from certbot import interfaces
 from certbot import reverter
 from certbot import storage
 from certbot import util
+from certbot.compat import misc
 from certbot.compat import os
 from certbot.display import enhancements
 from certbot.display import ops as display_ops
@@ -458,7 +459,9 @@ class Client(object):
 
         """
         for path in cert_path, chain_path, fullchain_path:
-            util.make_or_verify_dir(os.path.dirname(path), 0o755, self.config.strict_permissions)
+            util.make_or_verify_dir(
+                os.path.dirname(path), 0o755, misc.os_geteuid(),
+                self.config.strict_permissions)
 
 
         cert_file, abs_cert_path = _open_pem_file('cert_path', cert_path)

certbot/compat/_path.py

@@ -1,31 +0,0 @@
-"""This compat module wraps os.path to forbid some functions."""
-# pylint: disable=function-redefined
-from __future__ import absolute_import
-
-# First round of wrapping: we import statically all public attributes exposed by the os.path
-# module. This allows in particular to have pylint, mypy, IDEs be aware that most of os.path
-# members are available in certbot.compat.path.
-from os.path import *  # type: ignore  # pylint: disable=wildcard-import,unused-wildcard-import,redefined-builtin,os-module-forbidden
-
-# Second round of wrapping: we import dynamically all attributes from the os.path module that have
-# not yet been imported by the first round (static star import).
-import os.path as std_os_path  # pylint: disable=os-module-forbidden
-import sys as std_sys
-
-ourselves = std_sys.modules[__name__]
-for attribute in dir(std_os_path):
-    # Check if the attribute does not already exist in our module. It could be internal attributes
-    # of the module (__name__, __doc__), or attributes from standard os.path already imported with
-    # `from os.path import *`.
-    if not hasattr(ourselves, attribute):
-        setattr(ourselves, attribute, getattr(std_os_path, attribute))
-
-# Clean all remaining importables that are not from the core os.path module.
-del ourselves, std_os_path, std_sys
-
-
-# Function os.path.realpath is broken on some versions of Python for Windows.
-def realpath(*unused_args, **unused_kwargs):
-    """Method os.path.realpath() is forbidden"""
-    raise RuntimeError('Usage of os.path.realpath() is forbidden. '
-                       'Use certbot.compat.filesystem.realpath() instead.')

certbot/compat/filesystem.py

@@ -77,54 +77,6 @@ def copy_ownership_and_apply_mode(src, dst, mode, copy_user, copy_group):
     chmod(dst, mode)
 
 
-def check_mode(file_path, mode):
-    # type: (str, int) -> bool
-    """
-    Check if the given mode matches the permissions of the given file.
-    On Linux, will make a direct comparison, on Windows, mode will be compared against
-    the security model.
-    :param str file_path: Path of the file
-    :param int mode: POSIX mode to test
-    :rtype: bool
-    :return: True if the POSIX mode matches the file permissions
-    """
-    if POSIX_MODE:
-        return stat.S_IMODE(os.stat(file_path).st_mode) == mode
-
-    return _check_win_mode(file_path, mode)
-
-
-def check_owner(file_path):
-    # type: (str) -> bool
-    """
-    Check if given file is owned by current user.
-    :param str file_path: File path to check
-    :rtype: bool
-    :return: True if given file is owned by current user, False otherwise.
-    """
-    if POSIX_MODE:
-        return os.stat(file_path).st_uid == os.getuid()
-
-    # Get owner sid of the file
-    security = win32security.GetFileSecurity(file_path, win32security.OWNER_SECURITY_INFORMATION)
-    user = security.GetSecurityDescriptorOwner()
-
-    # Compare sids
-    return _get_current_user() == user
-
-
-def check_permissions(file_path, mode):
-    # type: (str, int) -> bool
-    """
-    Check if given file has the given mode and is owned by current user.
-    :param str file_path: File path to check
-    :param int mode: POSIX mode to check
-    :rtype: bool
-    :return: True if file has correct mode and owner, False otherwise.
-    """
-    return check_owner(file_path) and check_mode(file_path, mode)
-
-
 def open(file_path, flags, mode=0o777):  # pylint: disable=redefined-builtin
     # type: (str, int, int) -> int
     """
@@ -155,10 +107,6 @@ def open(file_path, flags, mode=0o777):  # pylint: disable=redefined-builtin
         security = attributes.SECURITY_DESCRIPTOR
         user = _get_current_user()
         dacl = _generate_dacl(user, mode)
-        # We set second parameter to 0 (`False`) to say that this security descriptor is
-        # NOT constructed from a default mechanism, but is explicitly set by the user.
-        # See https://docs.microsoft.com/en-us/windows/desktop/api/securitybaseapi/nf-securitybaseapi-setsecuritydescriptorowner  # pylint: disable=line-too-long
-        security.SetSecurityDescriptorOwner(user, 0)
         # We set first parameter to 1 (`True`) to say that this security descriptor contains
         # a DACL. Otherwise second and third parameters are ignored.
         # We set third parameter to 0 (`False`) to say that this security descriptor is
@@ -229,7 +177,6 @@ def mkdir(file_path, mode=0o777):
     security = attributes.SECURITY_DESCRIPTOR
     user = _get_current_user()
     dacl = _generate_dacl(user, mode)
-    security.SetSecurityDescriptorOwner(user, False)
     security.SetSecurityDescriptorDacl(1, dacl, 0)
 
     try:
@@ -260,22 +207,13 @@ def replace(src, dst):
         os.rename(src, dst)
 
 
-def realpath(file_path):
+def _apply_win_mode(file_path, mode):
     """
-    Find the real path for the given path. This method resolves symlinks, including
-    recursive symlinks, and is protected against symlinks that creates an infinite loop.
+    This function converts the given POSIX mode into a Windows ACL list, and applies it to the
+    file given its path. If the given path is a symbolic link, it will resolved to apply the
+    mode on the targeted file.
     """
     original_path = file_path
-
-    if POSIX_MODE:
-        path = os.path.realpath(file_path)
-        if os.path.islink(path):
-            # If path returned by realpath is still a link, it means that it failed to
-            # resolve the symlink because of a loop.
-            # See realpath code: https://github.com/python/cpython/blob/master/Lib/posixpath.py
-            raise RuntimeError('Error, link {0} is a loop!'.format(original_path))
-        return path
-
     inspected_paths = []  # type: List[str]
     while os.path.islink(file_path):
         link_path = file_path
@@ -285,53 +223,6 @@ def realpath(file_path):
         if file_path in inspected_paths:
             raise RuntimeError('Error, link {0} is a loop!'.format(original_path))
         inspected_paths.append(file_path)
-
-    return os.path.abspath(file_path)
-
-
-# On Windows is_executable run from an unprivileged shell may claim that a path is
-# executable when it is excutable only if run from a privileged shell. This result
-# is due to the fact that GetEffectiveRightsFromAcl calculate effective rights
-# without taking into consideration if the target user has currently required the
-# elevated privileges or not. However this is not a problem since certbot always
-# requires to be run under a privileged shell, so the user will always benefit
-# from the highest (privileged one) set of permissions on a given file.
-def is_executable(path):
-    """
-    Is path an executable file?
-    :param str path: path to test
-    :returns: True if path is an executable file
-    :rtype: bool
-    """
-    if POSIX_MODE:
-        return os.path.isfile(path) and os.access(path, os.X_OK)
-
-    return _win_is_executable(path)
-
-
-def _win_is_executable(path):
-    if not os.path.isfile(path):
-        return False
-
-    security = win32security.GetFileSecurity(path, win32security.DACL_SECURITY_INFORMATION)
-    dacl = security.GetSecurityDescriptorDacl()
-
-    mode = dacl.GetEffectiveRightsFromAcl({
-        'TrusteeForm': win32security.TRUSTEE_IS_SID,
-        'TrusteeType': win32security.TRUSTEE_IS_USER,
-        'Identifier': _get_current_user(),
-    })
-
-    return mode & ntsecuritycon.FILE_GENERIC_EXECUTE == ntsecuritycon.FILE_GENERIC_EXECUTE
-
-
-def _apply_win_mode(file_path, mode):
-    """
-    This function converts the given POSIX mode into a Windows ACL list, and applies it to the
-    file given its path. If the given path is a symbolic link, it will resolved to apply the
-    mode on the targeted file.
-    """
-    file_path = realpath(file_path)
     # Get owner sid of the file
     security = win32security.GetFileSecurity(file_path, win32security.OWNER_SECURITY_INFORMATION)
     user = security.GetSecurityDescriptorOwner()
@@ -442,28 +333,6 @@ def _generate_windows_flags(rights_desc):
     return flag
 
 
-def _check_win_mode(file_path, mode):
-    # Resolve symbolic links
-    file_path = realpath(file_path)
-    # Get current dacl file
-    security = win32security.GetFileSecurity(file_path, win32security.OWNER_SECURITY_INFORMATION
-                                             | win32security.DACL_SECURITY_INFORMATION)
-    dacl = security.GetSecurityDescriptorDacl()
-
-    # Get current file owner sid
-    user = security.GetSecurityDescriptorOwner()
-
-    if not dacl:
-        # No DACL means full control to everyone
-        # This is not a deterministic permissions set.
-        return False
-
-    # Calculate the target dacl
-    ref_dacl = _generate_dacl(user, mode)
-
-    return _compare_dacls(dacl, ref_dacl)
-
-
 def _compare_dacls(dacl1, dacl2):
     """
     This method compare the two given DACLs to check if they are identical.

certbot/compat/misc.py

@@ -30,10 +30,6 @@ else:
     MASK_FOR_PRIVATE_KEY_PERMISSIONS = 0
 
 
-# For Linux: define OS specific standard binary directories
-STANDARD_BINARY_DIRS = ["/usr/sbin", "/usr/local/bin", "/usr/local/sbin"] if POSIX_MODE else []
-
-
 def raise_for_non_administrative_windows_rights():
     # type: () -> None
     """
@@ -46,6 +42,22 @@ def raise_for_non_administrative_windows_rights():
         raise errors.Error('Error, certbot must be run on a shell with administrative rights.')
 
 
+def os_geteuid():
+    """
+    Get current user uid
+
+    :returns: The current user uid.
+    :rtype: int
+
+    """
+    try:
+        # Linux specific
+        return os.geteuid()
+    except AttributeError:
+        # Windows specific
+        return 0
+
+
 def readline_with_timeout(timeout, prompt):
     # type: (float, str) -> str
     """
@@ -76,6 +88,16 @@ def readline_with_timeout(timeout, prompt):
         return sys.stdin.readline()
 
 
+def compare_file_modes(mode1, mode2):
+    """Return true if the two modes can be considered as equals for this platform"""
+    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',

certbot/compat/os.py

@@ -26,9 +26,7 @@ for attribute in dir(std_os):
     if not hasattr(ourselves, attribute):
         setattr(ourselves, attribute, getattr(std_os, attribute))
 
-# Import our internal path module, then allow certbot.compat.os.path
-# to behave as a module (similarly to os.path).
-from certbot.compat import _path as path  # type: ignore  # pylint: disable=wrong-import-position
+# Similar to os.path, allow certbot.compat.os.path to behave as a module
 std_sys.modules[__name__ + '.path'] = path
 
 # Clean all remaining importables that are not from the core os module.
@@ -107,12 +105,3 @@ def replace(*unused_args, **unused_kwargs):
     """Method os.replace() is forbidden"""
     raise RuntimeError('Usage of os.replace() is forbidden. '
                        'Use certbot.compat.filesystem.replace() instead.')
-
-
-# Results given by os.access are inconsistent or partial on Windows, because this platform is not
-# following the POSIX approach.
-def access(*unused_args, **unused_kwargs):
-    """Method os.access() is forbidden"""
-    raise RuntimeError('Usage of os.access() is forbidden. '
-                       'Use certbot.compat.filesystem.check_mode() or '
-                       'certbot.compat.filesystem.is_executable() instead.')

certbot/constants.py

@@ -169,10 +169,9 @@ ACCOUNTS_DIR = "accounts"
 """Directory where all accounts are saved."""
 
 LE_REUSE_SERVERS = {
-    os.path.normpath('acme-v02.api.letsencrypt.org/directory'):
-        os.path.normpath('acme-v01.api.letsencrypt.org/directory'),
-    os.path.normpath('acme-staging-v02.api.letsencrypt.org/directory'):
-        os.path.normpath('acme-staging.api.letsencrypt.org/directory')
+    'acme-v02.api.letsencrypt.org/directory': 'acme-v01.api.letsencrypt.org/directory',
+    'acme-staging-v02.api.letsencrypt.org/directory':
+        'acme-staging.api.letsencrypt.org/directory'
 }
 """Servers that can reuse accounts from other servers."""
 

certbot/crypto_util.py

@@ -28,6 +28,7 @@ from acme.magic_typing import IO  # pylint: disable=unused-import, no-name-in-mo
 from certbot import errors
 from certbot import interfaces
 from certbot import util
+from certbot.compat import misc
 from certbot.compat import os
 
 logger = logging.getLogger(__name__)
@@ -60,7 +61,8 @@ def init_save_key(key_size, key_dir, keyname="key-certbot.pem"):
 
     config = zope.component.getUtility(interfaces.IConfig)
     # Save file
-    util.make_or_verify_dir(key_dir, 0o700, config.strict_permissions)
+    util.make_or_verify_dir(key_dir, 0o700, misc.os_geteuid(),
+                            config.strict_permissions)
     key_f, key_path = util.unique_file(
         os.path.join(key_dir, keyname), 0o600, "wb")
     with key_f:
@@ -90,7 +92,8 @@ def init_save_csr(privkey, names, path):
         privkey.pem, names, must_staple=config.must_staple)
 
     # Save CSR
-    util.make_or_verify_dir(path, 0o755, config.strict_permissions)
+    util.make_or_verify_dir(path, 0o755, misc.os_geteuid(),
+                            config.strict_permissions)
     csr_f, csr_filename = util.unique_file(
         os.path.join(path, "csr-certbot.pem"), 0o644, "wb")
     with csr_f:

certbot/hooks.py

@@ -8,7 +8,6 @@ from acme.magic_typing import Set, List  # pylint: disable=unused-import, no-nam
 
 from certbot import errors
 from certbot import util
-from certbot.compat import filesystem
 from certbot.compat import os
 from certbot.plugins import util as plug_util
 
@@ -255,7 +254,7 @@ def execute(cmd_name, shell_cmd):
                      cmd_name, shell_cmd, cmd.returncode)
     if err:
         logger.error('Error output from %s command %s:\n%s', cmd_name, base_cmd, err)
-    return err, out
+    return (err, out)
 
 
 def list_hooks(dir_path):
@@ -268,5 +267,5 @@ def list_hooks(dir_path):
 
     """
     allpaths = (os.path.join(dir_path, f) for f in os.listdir(dir_path))
-    hooks = [path for path in allpaths if filesystem.is_executable(path) and not path.endswith('~')]
+    hooks = [path for path in allpaths if util.is_exe(path) and not path.endswith('~')]
     return sorted(hooks)

certbot/log.py

@@ -17,7 +17,6 @@ from __future__ import print_function
 import functools
 import logging
 import logging.handlers
-import shutil
 import sys
 import tempfile
 import traceback
@@ -27,6 +26,7 @@ from acme import messages
 from certbot import constants
 from certbot import errors
 from certbot import util
+from certbot.compat import misc
 from certbot.compat import os
 
 # Logging format
@@ -134,7 +134,8 @@ def setup_log_file_handler(config, logfile, fmt):
     """
     # TODO: logs might contain sensitive data such as contents of the
     # private key! #525
-    util.set_up_core_dir(config.logs_dir, 0o700, config.strict_permissions)
+    util.set_up_core_dir(
+        config.logs_dir, 0o700, misc.os_geteuid(), config.strict_permissions)
     log_file_path = os.path.join(config.logs_dir, logfile)
     try:
         handler = logging.handlers.RotatingFileHandler(
@@ -239,10 +240,9 @@ class TempHandler(logging.StreamHandler):
 
     """
     def __init__(self):
-        self._workdir = tempfile.mkdtemp()
-        self.path = os.path.join(self._workdir, 'log')
-        stream = util.safe_open(self.path, mode='w', chmod=0o600)
+        stream = tempfile.NamedTemporaryFile('w', delete=False)
         super(TempHandler, self).__init__(stream)
+        self.path = stream.name
         self._delete = True
 
     def emit(self, record):
@@ -266,7 +266,7 @@ class TempHandler(logging.StreamHandler):
             # stream like stderr to be used
             self.stream.close()
             if self._delete:
-                shutil.rmtree(self._workdir)
+                os.remove(self.path)
             self._delete = False
             super(TempHandler, self).close()
         finally:

certbot/main.py

@@ -31,7 +31,6 @@ from certbot import reporter
 from certbot import storage
 from certbot import updater
 from certbot import util
-from certbot.compat import filesystem
 from certbot.compat import misc
 from certbot.compat import os
 from certbot.display import util as display_util, ops as display_ops
@@ -842,12 +841,12 @@ def _populate_from_certname(config):
     return config
 
 def _check_certificate_and_key(config):
-    if not os.path.isfile(filesystem.realpath(config.cert_path)):
+    if not os.path.isfile(os.path.realpath(config.cert_path)):
         raise errors.ConfigurationError("Error while reading certificate from path "
-                                        "{0}".format(config.cert_path))
-    if not os.path.isfile(filesystem.realpath(config.key_path)):
+                                       "{0}".format(config.cert_path))
+    if not os.path.isfile(os.path.realpath(config.key_path)):
         raise errors.ConfigurationError("Error while reading private key from path "
-                                        "{0}".format(config.key_path))
+                                       "{0}".format(config.key_path))
 def plugins_cmd(config, plugins):
     """List server software plugins.
 
@@ -1299,14 +1298,18 @@ def make_or_verify_needed_dirs(config):
     :rtype: None
 
     """
-    util.set_up_core_dir(config.config_dir, constants.CONFIG_DIRS_MODE, config.strict_permissions)
-    util.set_up_core_dir(config.work_dir, constants.CONFIG_DIRS_MODE, config.strict_permissions)
+    util.set_up_core_dir(config.config_dir, constants.CONFIG_DIRS_MODE,
+                         misc.os_geteuid(), config.strict_permissions)
+    util.set_up_core_dir(config.work_dir, constants.CONFIG_DIRS_MODE,
+                         misc.os_geteuid(), config.strict_permissions)
 
     hook_dirs = (config.renewal_pre_hooks_dir,
                  config.renewal_deploy_hooks_dir,
                  config.renewal_post_hooks_dir,)
     for hook_dir in hook_dirs:
-        util.make_or_verify_dir(hook_dir, strict=config.strict_permissions)
+        util.make_or_verify_dir(hook_dir,
+                                uid=misc.os_geteuid(),
+                                strict=config.strict_permissions)
 
 
 def set_displayer(config):

certbot/plugins/common.py

@@ -486,7 +486,7 @@ def dir_setup(test_dir, pkg):  # pragma: no cover
         link, (ex: OS X) such plugins will be confused. This function prevents
         such a case.
         """
-        return filesystem.realpath(tempfile.mkdtemp(prefix))
+        return os.path.realpath(tempfile.mkdtemp(prefix))
 
     temp_dir = expanded_tempdir("temp")
     config_dir = expanded_tempdir("config")

certbot/plugins/storage_test.py

@@ -31,7 +31,7 @@ class PluginStorageTest(test_util.ConfigTestCase):
         self.plugin.storage.storagepath = os.path.join(self.config.config_dir,
                                                        ".pluginstorage.json")
         with mock.patch("six.moves.builtins.open", mock_open):
-            with mock.patch('certbot.compat.os.path.isfile', return_value=True):
+            with mock.patch('os.path.isfile', return_value=True):
                 with mock.patch("certbot.reverter.util"):
                     self.assertRaises(errors.PluginStorageError,
                                       self.plugin.storage._load)  # pylint: disable=protected-access

certbot/plugins/util.py

@@ -3,11 +3,9 @@ import logging
 
 from certbot import util
 from certbot.compat import os
-from certbot.compat.misc import STANDARD_BINARY_DIRS
 
 logger = logging.getLogger(__name__)
 
-
 def get_prefixes(path):
     """Retrieves all possible path prefixes of a path, in descending order
     of length. For instance,
@@ -28,7 +26,6 @@ def get_prefixes(path):
             break
     return prefixes
 
-
 def path_surgery(cmd):
     """Attempt to perform PATH surgery to find cmd
 
@@ -38,9 +35,10 @@ def path_surgery(cmd):
 
     :returns: True if the operation succeeded, False otherwise
     """
+    dirs = ("/usr/sbin", "/usr/local/bin", "/usr/local/sbin")
     path = os.environ["PATH"]
     added = []
-    for d in STANDARD_BINARY_DIRS:
+    for d in dirs:
         if d not in path:
             path += os.pathsep + d
             added.append(d)

certbot/plugins/util_test.py

@@ -16,7 +16,6 @@ class GetPrefixTest(unittest.TestCase):
         self.assertEqual(get_prefixes('/'), [os.path.normpath('/')])
         self.assertEqual(get_prefixes('a'), ['a'])
 
-
 class PathSurgeryTest(unittest.TestCase):
     """Tests for certbot.plugins.path_surgery."""
 
@@ -30,15 +29,13 @@ class PathSurgeryTest(unittest.TestCase):
                 self.assertEqual(path_surgery("eg"), True)
                 self.assertEqual(mock_debug.call_count, 0)
                 self.assertEqual(os.environ["PATH"], all_path["PATH"])
-        if os.name != 'nt':
-            # This part is specific to Linux since on Windows no PATH surgery is ever done.
-            no_path = {"PATH": "/tmp/"}
-            with mock.patch.dict('os.environ', no_path):
-                path_surgery("thingy")
-                self.assertEqual(mock_debug.call_count, 2 if os.name != 'nt' else 1)
-                self.assertTrue("Failed to find" in mock_debug.call_args[0][0])
-                self.assertTrue("/usr/local/bin" in os.environ["PATH"])
-                self.assertTrue("/tmp" in os.environ["PATH"])
+        no_path = {"PATH": "/tmp/"}
+        with mock.patch.dict('os.environ', no_path):
+            path_surgery("thingy")
+            self.assertEqual(mock_debug.call_count, 2)
+            self.assertTrue("Failed to find" in mock_debug.call_args[0][0])
+            self.assertTrue("/usr/local/bin" in os.environ["PATH"])
+            self.assertTrue("/tmp" in os.environ["PATH"])
 
 
 if __name__ == "__main__":

certbot/plugins/webroot.py

@@ -24,7 +24,6 @@ from certbot.display import ops
 from certbot.display import util as display_util
 from certbot.plugins import common
 from certbot.plugins import util
-from certbot.util import safe_open
 
 logger = logging.getLogger(__name__)
 
@@ -208,7 +207,7 @@ to serve all files under specified web root ({0})."""
         old_umask = os.umask(0o022)
 
         try:
-            with safe_open(validation_path, mode="wb", chmod=0o644) as validation_file:
+            with open(validation_path, "wb") as validation_file:
                 validation_file.write(validation.encode())
         finally:
             os.umask(old_umask)

certbot/plugins/webroot_test.py

@@ -17,6 +17,7 @@ from acme import challenges
 
 from certbot import achallenges
 from certbot import errors
+from certbot.compat import misc
 from certbot.compat import os
 from certbot.compat import filesystem
 from certbot.display import util as display_util
@@ -167,14 +168,14 @@ class AuthenticatorTest(unittest.TestCase):
         # Remove exec bit from permission check, so that it
         # matches the file
         self.auth.perform([self.achall])
-        self.assertTrue(filesystem.check_mode(self.validation_path, 0o644))
+        self.assertTrue(misc.compare_file_modes(os.stat(self.validation_path).st_mode, 0o644))
 
         # Check permissions of the directories
 
         for dirpath, dirnames, _ in os.walk(self.path):
             for directory in dirnames:
                 full_path = os.path.join(dirpath, directory)
-                self.assertTrue(filesystem.check_mode(full_path, 0o755))
+                self.assertTrue(misc.compare_file_modes(os.stat(full_path).st_mode, 0o755))
 
         parent_gid = os.stat(self.path).st_gid
         parent_uid = os.stat(self.path).st_uid

certbot/reverter.py

@@ -15,6 +15,7 @@ from certbot import constants
 from certbot import errors
 from certbot import interfaces
 from certbot import util
+from certbot.compat import misc
 from certbot.compat import os
 from certbot.compat import filesystem
 
@@ -67,7 +68,8 @@ class Reverter(object):
         self.config = config
 
         util.make_or_verify_dir(
-            config.backup_dir, constants.CONFIG_DIRS_MODE, self.config.strict_permissions)
+            config.backup_dir, constants.CONFIG_DIRS_MODE, misc.os_geteuid(),
+            self.config.strict_permissions)
 
     def revert_temporary_config(self):
         """Reload users original configuration files after a temporary save.
@@ -223,7 +225,8 @@ class Reverter(object):
 
         """
         util.make_or_verify_dir(
-            cp_dir, constants.CONFIG_DIRS_MODE, self.config.strict_permissions)
+            cp_dir, constants.CONFIG_DIRS_MODE, misc.os_geteuid(),
+            self.config.strict_permissions)
 
         op_fd, existing_filepaths = self._read_and_append(
             os.path.join(cp_dir, "FILEPATHS"))
@@ -442,7 +445,8 @@ class Reverter(object):
             cp_dir = self.config.in_progress_dir
 
         util.make_or_verify_dir(
-            cp_dir, constants.CONFIG_DIRS_MODE, self.config.strict_permissions)
+            cp_dir, constants.CONFIG_DIRS_MODE, misc.os_geteuid(),
+            self.config.strict_permissions)
 
         return cp_dir
 

certbot/tests/account_test.py

@@ -2,6 +2,7 @@
 import datetime
 import json
 import shutil
+import stat
 import unittest
 
 import josepy as jose
@@ -12,7 +13,6 @@ from acme import messages
 
 import certbot.tests.util as test_util
 from certbot import errors
-from certbot.compat import filesystem
 from certbot.compat import misc
 from certbot.compat import os
 
@@ -116,6 +116,7 @@ class AccountFileStorageTest(test_util.ConfigTestCase):
         self.assertTrue(os.path.isdir(
             misc.underscores_for_unsupported_characters_in_path(self.config.accounts_dir)))
 
+    @test_util.broken_on_windows
     def test_save_and_restore(self):
         self.storage.save(self.acc, self.mock_client)
         account_path = os.path.join(self.config.accounts_dir, self.acc.id)
@@ -123,8 +124,8 @@ class AccountFileStorageTest(test_util.ConfigTestCase):
         for file_name in "regr.json", "meta.json", "private_key.json":
             self.assertTrue(os.path.exists(
                 os.path.join(account_path, file_name)))
-        self.assertTrue(
-            filesystem.check_mode(os.path.join(account_path, "private_key.json"), 0o400))
+        self.assertTrue(oct(os.stat(os.path.join(
+            account_path, "private_key.json"))[stat.ST_MODE] & 0o777) in ("0400", "0o400"))
 
         # restore
         loaded = self.storage.load(self.acc.id)
@@ -218,12 +219,14 @@ class AccountFileStorageTest(test_util.ConfigTestCase):
         self._set_server('https://acme-staging.api.letsencrypt.org/directory')
         self.assertEqual([], self.storage.find_all())
 
+    @test_util.broken_on_windows
     def test_upgrade_version_staging(self):
         self._set_server('https://acme-staging.api.letsencrypt.org/directory')
         self.storage.save(self.acc, self.mock_client)
         self._set_server('https://acme-staging-v02.api.letsencrypt.org/directory')
         self.assertEqual([self.acc], self.storage.find_all())
 
+    @test_util.broken_on_windows
     def test_upgrade_version_production(self):
         self._set_server('https://acme-v01.api.letsencrypt.org/directory')
         self.storage.save(self.acc, self.mock_client)
@@ -241,6 +244,7 @@ class AccountFileStorageTest(test_util.ConfigTestCase):
         self._set_server('https://acme-staging-v02.api.letsencrypt.org/directory')
         self.assertEqual([], self.storage.find_all())
 
+    @test_util.broken_on_windows
     def test_upgrade_load(self):
         self._set_server('https://acme-staging.api.letsencrypt.org/directory')
         self.storage.save(self.acc, self.mock_client)
@@ -249,6 +253,7 @@ class AccountFileStorageTest(test_util.ConfigTestCase):
         account = self.storage.load(self.acc.id)
         self.assertEqual(prev_account, account)
 
+    @test_util.broken_on_windows
     def test_upgrade_load_single_account(self):
         self._set_server('https://acme-staging.api.letsencrypt.org/directory')
         self.storage.save(self.acc, self.mock_client)
@@ -273,6 +278,7 @@ class AccountFileStorageTest(test_util.ConfigTestCase):
                 errors.AccountStorageError, self.storage.save,
                     self.acc, self.mock_client)
 
+    @test_util.broken_on_windows
     def test_delete(self):
         self.storage.save(self.acc, self.mock_client)
         self.storage.delete(self.acc.id)
@@ -307,10 +313,12 @@ class AccountFileStorageTest(test_util.ConfigTestCase):
         self._set_server('https://acme-staging-v02.api.letsencrypt.org/directory')
         self.assertRaises(errors.AccountNotFound, self.storage.load, self.acc.id)
 
+    @test_util.broken_on_windows
     def test_delete_folders_up(self):
         self._test_delete_folders('https://acme-staging.api.letsencrypt.org/directory')
         self._assert_symlinked_account_removed()
 
+    @test_util.broken_on_windows
     def test_delete_folders_down(self):
         self._test_delete_folders('https://acme-staging-v02.api.letsencrypt.org/directory')
         self._assert_symlinked_account_removed()
@@ -320,14 +328,15 @@ class AccountFileStorageTest(test_util.ConfigTestCase):
         with open(os.path.join(self.config.accounts_dir, 'foo'), 'w') as f:
             f.write('bar')
 
+    @test_util.broken_on_windows
     def test_delete_shared_account_up(self):
         self._set_server_and_stop_symlink('https://acme-staging-v02.api.letsencrypt.org/directory')
         self._test_delete_folders('https://acme-staging.api.letsencrypt.org/directory')
 
+    @test_util.broken_on_windows
     def test_delete_shared_account_down(self):
         self._set_server_and_stop_symlink('https://acme-staging-v02.api.letsencrypt.org/directory')
         self._test_delete_folders('https://acme-staging-v02.api.letsencrypt.org/directory')
 
-
 if __name__ == "__main__":
     unittest.main()  # pragma: no cover

certbot/tests/cert_manager_test.py

@@ -97,8 +97,8 @@ class UpdateLiveSymlinksTest(BaseCertManagerTest):
                 for kind in ALL_FOUR:
                     os.chdir(os.path.dirname(self.config_files[domain][kind]))
                     self.assertEqual(
-                        filesystem.realpath(os.readlink(self.config_files[domain][kind])),
-                        filesystem.realpath(archive_paths[domain][kind]))
+                        os.path.realpath(os.readlink(self.config_files[domain][kind])),
+                        os.path.realpath(archive_paths[domain][kind]))
         finally:
             os.chdir(prev_dir)
 
@@ -277,12 +277,13 @@ class SearchLineagesTest(BaseCertManagerTest):
     @mock.patch('certbot.storage.renewal_conf_files')
     @mock.patch('certbot.storage.RenewableCert')
     def test_cert_storage_error(self, mock_renewable_cert, mock_renewal_conf_files,
-                                mock_make_or_verify_dir):
+        mock_make_or_verify_dir):
         mock_renewal_conf_files.return_value = ["badfile"]
         mock_renewable_cert.side_effect = errors.CertStorageError
         from certbot import cert_manager
         # pylint: disable=protected-access
-        self.assertEqual(cert_manager._search_lineages(self.config, lambda x: x, "check"), "check")
+        self.assertEqual(cert_manager._search_lineages(self.config, lambda x: x, "check"),
+            "check")
         self.assertTrue(mock_make_or_verify_dir.called)
 
 
@@ -293,28 +294,33 @@ class LineageForCertnameTest(BaseCertManagerTest):
     @mock.patch('certbot.storage.renewal_file_for_certname')
     @mock.patch('certbot.storage.RenewableCert')
     def test_found_match(self, mock_renewable_cert, mock_renewal_conf_file,
-                         mock_make_or_verify_dir):
+        mock_make_or_verify_dir):
         mock_renewal_conf_file.return_value = "somefile.conf"
         mock_match = mock.Mock(lineagename="example.com")
         mock_renewable_cert.return_value = mock_match
         from certbot import cert_manager
-        self.assertEqual(cert_manager.lineage_for_certname(self.config, "example.com"), mock_match)
+        self.assertEqual(cert_manager.lineage_for_certname(self.config, "example.com"),
+            mock_match)
         self.assertTrue(mock_make_or_verify_dir.called)
 
     @mock.patch('certbot.util.make_or_verify_dir')
     @mock.patch('certbot.storage.renewal_file_for_certname')
-    def test_no_match(self, mock_renewal_conf_file, mock_make_or_verify_dir):
+    def test_no_match(self, mock_renewal_conf_file,
+        mock_make_or_verify_dir):
         mock_renewal_conf_file.return_value = "other.com.conf"
         from certbot import cert_manager
-        self.assertEqual(cert_manager.lineage_for_certname(self.config, "example.com"), None)
+        self.assertEqual(cert_manager.lineage_for_certname(self.config, "example.com"),
+            None)
         self.assertTrue(mock_make_or_verify_dir.called)
 
     @mock.patch('certbot.util.make_or_verify_dir')
     @mock.patch('certbot.storage.renewal_file_for_certname')
-    def test_no_renewal_file(self, mock_renewal_conf_file, mock_make_or_verify_dir):
+    def test_no_renewal_file(self, mock_renewal_conf_file,
+        mock_make_or_verify_dir):
         mock_renewal_conf_file.side_effect = errors.CertStorageError()
         from certbot import cert_manager
-        self.assertEqual(cert_manager.lineage_for_certname(self.config, "example.com"), None)
+        self.assertEqual(cert_manager.lineage_for_certname(self.config, "example.com"),
+            None)
         self.assertTrue(mock_make_or_verify_dir.called)
 
 
@@ -325,7 +331,7 @@ class DomainsForCertnameTest(BaseCertManagerTest):
     @mock.patch('certbot.storage.renewal_file_for_certname')
     @mock.patch('certbot.storage.RenewableCert')
     def test_found_match(self, mock_renewable_cert, mock_renewal_conf_file,
-                         mock_make_or_verify_dir):
+        mock_make_or_verify_dir):
         mock_renewal_conf_file.return_value = "somefile.conf"
         mock_match = mock.Mock(lineagename="example.com")
         domains = ["example.com", "example.org"]
@@ -338,10 +344,12 @@ class DomainsForCertnameTest(BaseCertManagerTest):
 
     @mock.patch('certbot.util.make_or_verify_dir')
     @mock.patch('certbot.storage.renewal_file_for_certname')
-    def test_no_match(self, mock_renewal_conf_file, mock_make_or_verify_dir):
+    def test_no_match(self, mock_renewal_conf_file,
+        mock_make_or_verify_dir):
         mock_renewal_conf_file.return_value = "somefile.conf"
         from certbot import cert_manager
-        self.assertEqual(cert_manager.domains_for_certname(self.config, "other.com"), None)
+        self.assertEqual(cert_manager.domains_for_certname(self.config, "other.com"),
+            None)
         self.assertTrue(mock_make_or_verify_dir.called)
 
 

certbot/tests/compat/filesystem_test.py

@@ -1,5 +1,4 @@
 """Tests for certbot.compat.filesystem"""
-import contextlib
 import errno
 import unittest
 
@@ -17,7 +16,6 @@ except ImportError:
 
 import certbot.tests.util as test_util
 from certbot import lock
-from certbot import util
 from certbot.compat import os
 from certbot.compat import filesystem
 from certbot.tests.util import TempDirTestCase
@@ -50,6 +48,18 @@ class WindowsChmodTests(TempDirTestCase):
         self.assertFalse(filesystem._compare_dacls(ref_dacl_probe, cur_dacl_probe))  # pylint: disable=protected-access
         self.assertTrue(filesystem._compare_dacls(ref_dacl_link, cur_dacl_link))  # pylint: disable=protected-access
 
+    def test_symlink_loop_mitigation(self):
+        link1_path = os.path.join(self.tempdir, 'link1')
+        link2_path = os.path.join(self.tempdir, 'link2')
+        link3_path = os.path.join(self.tempdir, 'link3')
+        os.symlink(link1_path, link2_path)
+        os.symlink(link2_path, link3_path)
+        os.symlink(link3_path, link1_path)
+
+        with self.assertRaises(RuntimeError) as error:
+            filesystem.chmod(link1_path, 0o755)
+        self.assertTrue('link1 is a loop!' in str(error.exception))
+
     def test_world_permission(self):
         everybody = win32security.ConvertStringSidToSid(EVERYBODY_SID)
 
@@ -308,54 +318,9 @@ class CopyOwnershipTest(test_util.TempDirTestCase):
         mock_chmod.assert_called_once_with(self.probe_path, 0o700)
 
 
-class CheckPermissionsTest(test_util.TempDirTestCase):
-    def setUp(self):
-        super(CheckPermissionsTest, self).setUp()
-        self.probe_path = _create_probe(self.tempdir)
-
-    def test_check_mode(self):
-        self.assertTrue(filesystem.check_mode(self.probe_path, 0o744))
-
-        filesystem.chmod(self.probe_path, 0o700)
-        self.assertFalse(filesystem.check_mode(self.probe_path, 0o744))
-
-    @unittest.skipIf(POSIX_MODE, reason='Test specific to Windows security')
-    def test_check_owner_windows(self):
-        self.assertTrue(filesystem.check_owner(self.probe_path))
-
-        system = win32security.ConvertStringSidToSid(SYSTEM_SID)
-        security = win32security.SECURITY_ATTRIBUTES().SECURITY_DESCRIPTOR
-        security.SetSecurityDescriptorOwner(system, False)
-
-        with mock.patch('win32security.GetFileSecurity') as mock_get:
-            mock_get.return_value = security
-            self.assertFalse(filesystem.check_owner(self.probe_path))
-
-    @unittest.skipUnless(POSIX_MODE, reason='Test specific to Linux security')
-    def test_check_owner_linux(self):
-        self.assertTrue(filesystem.check_owner(self.probe_path))
-
-        import os as std_os  # pylint: disable=os-module-forbidden
-        uid = std_os.getuid()
-
-        with mock.patch('os.getuid') as mock_uid:
-            mock_uid.return_value = uid + 1
-            self.assertFalse(filesystem.check_owner(self.probe_path))
-
-    def test_check_permissions(self):
-        self.assertTrue(filesystem.check_permissions(self.probe_path, 0o744))
-
-        with mock.patch('certbot.compat.filesystem.check_mode') as mock_mode:
-            mock_mode.return_value = False
-            self.assertFalse(filesystem.check_permissions(self.probe_path, 0o744))
-
-        with mock.patch('certbot.compat.filesystem.check_owner') as mock_owner:
-            mock_owner.return_value = False
-            self.assertFalse(filesystem.check_permissions(self.probe_path, 0o744))
-
-
 class OsReplaceTest(test_util.TempDirTestCase):
     """Test to ensure consistent behavior of rename method"""
+
     def test_os_replace_to_existing_file(self):
         """Ensure that replace will effectively rename src into dst for all platforms."""
         src = os.path.join(self.tempdir, 'src')
@@ -370,112 +335,6 @@ class OsReplaceTest(test_util.TempDirTestCase):
         self.assertTrue(os.path.exists(dst))
 
 
-class RealpathTest(test_util.TempDirTestCase):
-    """Tests for realpath method"""
-    def setUp(self):
-        super(RealpathTest, self).setUp()
-        self.probe_path = _create_probe(self.tempdir)
-
-    def test_symlink_resolution(self):
-        # Remove any symlinks already in probe_path
-        self.probe_path = filesystem.realpath(self.probe_path)
-        # Absolute resolution
-        link_path = os.path.join(self.tempdir, 'link_abs')
-        os.symlink(self.probe_path, link_path)
-
-        self.assertEqual(self.probe_path, filesystem.realpath(self.probe_path))
-        self.assertEqual(self.probe_path, filesystem.realpath(link_path))
-
-        # Relative resolution
-        curdir = os.getcwd()
-        link_path = os.path.join(self.tempdir, 'link_rel')
-        probe_name = os.path.basename(self.probe_path)
-        try:
-            os.chdir(os.path.dirname(self.probe_path))
-            os.symlink(probe_name, link_path)
-
-            self.assertEqual(self.probe_path, filesystem.realpath(probe_name))
-            self.assertEqual(self.probe_path, filesystem.realpath(link_path))
-        finally:
-            os.chdir(curdir)
-
-    def test_symlink_loop_mitigation(self):
-        link1_path = os.path.join(self.tempdir, 'link1')
-        link2_path = os.path.join(self.tempdir, 'link2')
-        link3_path = os.path.join(self.tempdir, 'link3')
-        os.symlink(link1_path, link2_path)
-        os.symlink(link2_path, link3_path)
-        os.symlink(link3_path, link1_path)
-
-        with self.assertRaises(RuntimeError) as error:
-            filesystem.realpath(link1_path)
-        self.assertTrue('link1 is a loop!' in str(error.exception))
-
-
-class IsExecutableTest(test_util.TempDirTestCase):
-    """Tests for is_executable method"""
-    def test_not_executable(self):
-        file_path = os.path.join(self.tempdir, "foo")
-
-        # On Windows a file created within Certbot will always have all permissions to the
-        # Administrators group set. Since the unit tests are typically executed under elevated
-        # privileges, it means that current user will always have effective execute rights on the
-        # hook script, and so the test will fail. To prevent that and represent a file created
-        # outside Certbot as typically a hook file is, we mock the _generate_dacl function in
-        # certbot.compat.filesystem to give rights only to the current user. This implies removing
-        # all ACEs except the first one from the DACL created by original _generate_dacl function.
-
-        from certbot.compat.filesystem import _generate_dacl
-
-        def _execute_mock(user_sid, mode):
-            dacl = _generate_dacl(user_sid, mode)
-            for _ in range(1, dacl.GetAceCount()):
-                dacl.DeleteAce(1)  # DeleteAce dynamically updates the internal index mapping.
-            return dacl
-
-        # create a non-executable file
-        with mock.patch("certbot.compat.filesystem._generate_dacl", side_effect=_execute_mock):
-            os.close(filesystem.open(file_path, os.O_CREAT | os.O_WRONLY, 0o666))
-
-        self.assertFalse(filesystem.is_executable(file_path))
-
-    @mock.patch("certbot.compat.filesystem.os.path.isfile")
-    @mock.patch("certbot.compat.filesystem.os.access")
-    def test_full_path(self, mock_access, mock_isfile):
-        with _fix_windows_runtime():
-            mock_access.return_value = True
-            mock_isfile.return_value = True
-            self.assertTrue(filesystem.is_executable("/path/to/exe"))
-
-    @mock.patch("certbot.compat.filesystem.os.path.isfile")
-    @mock.patch("certbot.compat.filesystem.os.access")
-    def test_rel_path(self, mock_access, mock_isfile):
-        with _fix_windows_runtime():
-            mock_access.return_value = True
-            mock_isfile.return_value = True
-            self.assertTrue(filesystem.is_executable("exe"))
-
-    @mock.patch("certbot.compat.filesystem.os.path.isfile")
-    @mock.patch("certbot.compat.filesystem.os.access")
-    def test_not_found(self, mock_access, mock_isfile):
-        with _fix_windows_runtime():
-            mock_access.return_value = True
-            mock_isfile.return_value = False
-            self.assertFalse(filesystem.is_executable("exe"))
-
-
-@contextlib.contextmanager
-def _fix_windows_runtime():
-    if os.name != 'nt':
-        yield
-    else:
-        with mock.patch('win32security.GetFileSecurity') as mock_get:
-            dacl_mock = mock_get.return_value.GetSecurityDescriptorDacl
-            mode_mock = dacl_mock.return_value.GetEffectiveRightsFromAcl
-            mode_mock.return_value = ntsecuritycon.FILE_GENERIC_EXECUTE
-            yield
-
-
 def _get_security_dacl(target):
     return win32security.GetFileSecurity(target, win32security.DACL_SECURITY_INFORMATION)
 
@@ -493,7 +352,7 @@ def _set_owner(target, security_owner, user):
 def _create_probe(tempdir):
     filesystem.chmod(tempdir, 0o744)
     probe_path = os.path.join(tempdir, 'probe')
-    util.safe_open(probe_path, 'w', chmod=0o744).close()
+    open(probe_path, 'w').close()
     return probe_path
 
 

certbot/tests/compat/os_test.py

@@ -7,13 +7,8 @@ from certbot.compat import os
 class OsTest(unittest.TestCase):
     """Unit tests for os module."""
     def test_forbidden_methods(self):
-        # Checks for os module
-        for method in ['chmod', 'chown', 'open', 'mkdir',
-                       'makedirs', 'rename', 'replace', 'access']:
+        for method in ['chmod', 'chown', 'open', 'mkdir', 'makedirs', 'rename', 'replace']:
             self.assertRaises(RuntimeError, getattr(os, method))
-        # Checks for os.path module
-        for method in ['realpath']:
-            self.assertRaises(RuntimeError, getattr(os.path, method))
 
 
 if __name__ == "__main__":

certbot/tests/crypto_util_test.py

@@ -11,7 +11,6 @@ from certbot import errors
 from certbot import interfaces
 from certbot import util
 from certbot.compat import os
-from certbot.compat import filesystem
 
 RSA256_KEY = test_util.load_vector('rsa256_key.pem')
 RSA256_KEY_PATH = test_util.vector_path('rsa256_key.pem')
@@ -30,9 +29,6 @@ class InitSaveKeyTest(test_util.TempDirTestCase):
     def setUp(self):
         super(InitSaveKeyTest, self).setUp()
 
-        self.workdir = os.path.join(self.tempdir, 'workdir')
-        filesystem.mkdir(self.workdir, mode=0o700)
-
         logging.disable(logging.CRITICAL)
         zope.component.provideUtility(
             mock.Mock(strict_permissions=True), interfaces.IConfig)
@@ -50,15 +46,15 @@ class InitSaveKeyTest(test_util.TempDirTestCase):
     @mock.patch('certbot.crypto_util.make_key')
     def test_success(self, mock_make):
         mock_make.return_value = b'key_pem'
-        key = self._call(1024, self.workdir)
+        key = self._call(1024, self.tempdir)
         self.assertEqual(key.pem, b'key_pem')
         self.assertTrue('key-certbot.pem' in key.file)
-        self.assertTrue(os.path.exists(os.path.join(self.workdir, key.file)))
+        self.assertTrue(os.path.exists(os.path.join(self.tempdir, key.file)))
 
     @mock.patch('certbot.crypto_util.make_key')
     def test_key_failure(self, mock_make):
         mock_make.side_effect = ValueError
-        self.assertRaises(ValueError, self._call, 431, self.workdir)
+        self.assertRaises(ValueError, self._call, 431, self.tempdir)
 
 
 class InitSaveCSRTest(test_util.TempDirTestCase):

certbot/tests/hook_test.py

@@ -1,14 +1,14 @@
 """Tests for certbot.hooks."""
+import stat
 import unittest
 
 import mock
 from acme.magic_typing import List  # pylint: disable=unused-import, no-name-in-module
 
 from certbot import errors
-from certbot import util
 from certbot.compat import os
 from certbot.compat import filesystem
-from certbot.tests import util as test_util
+from certbot.tests import util
 
 
 class ValidateHooksTest(unittest.TestCase):
@@ -30,7 +30,7 @@ class ValidateHooksTest(unittest.TestCase):
         self.assertEqual("renew", types[-1])
 
 
-class ValidateHookTest(test_util.TempDirTestCase):
+class ValidateHookTest(util.TempDirTestCase):
     """Tests for certbot.hooks.validate_hook."""
 
     @classmethod
@@ -38,20 +38,22 @@ class ValidateHookTest(test_util.TempDirTestCase):
         from certbot.hooks import validate_hook
         return validate_hook(*args, **kwargs)
 
-    def test_hook_not_executable(self):
+    @util.broken_on_windows
+    def test_not_executable(self):
+        file_path = os.path.join(self.tempdir, "foo")
+        # create a non-executable file
+        os.close(filesystem.open(file_path, os.O_CREAT | os.O_WRONLY, 0o666))
         # prevent unnecessary modifications to PATH
         with mock.patch("certbot.hooks.plug_util.path_surgery"):
-            # We just mock out filesystem.is_executable since on Windows, it is difficult
-            # to get a fully working test around executable permissions. See
-            # certbot.tests.compat.filesystem::NotExecutableTest for more in-depth tests.
-            with mock.patch("certbot.hooks.filesystem.is_executable", return_value=False):
-                self.assertRaises(errors.HookCommandNotFound, self._call, 'dummy', "foo")
+            self.assertRaises(errors.HookCommandNotFound,
+                              self._call, file_path, "foo")
 
     @mock.patch("certbot.hooks.util.exe_exists")
     def test_not_found(self, mock_exe_exists):
         mock_exe_exists.return_value = False
         with mock.patch("certbot.hooks.plug_util.path_surgery") as mock_ps:
-            self.assertRaises(errors.HookCommandNotFound, self._call, "foo", "bar")
+            self.assertRaises(errors.HookCommandNotFound,
+                              self._call, "foo", "bar")
         self.assertTrue(mock_ps.called)
 
     @mock.patch("certbot.hooks._prog")
@@ -60,7 +62,7 @@ class ValidateHookTest(test_util.TempDirTestCase):
         self.assertFalse(mock_prog.called)
 
 
-class HookTest(test_util.ConfigTestCase):
+class HookTest(util.ConfigTestCase):
     """Common base class for hook tests."""
 
     @classmethod
@@ -452,7 +454,7 @@ class ExecuteTest(unittest.TestCase):
             self.assertTrue(mock_logger.error.called)
 
 
-class ListHooksTest(test_util.TempDirTestCase):
+class ListHooksTest(util.TempDirTestCase):
     """Tests for certbot.hooks.list_hooks."""
 
     @classmethod
@@ -492,7 +494,8 @@ def create_hook(file_path):
     :param str file_path: path to create the file at
 
     """
-    util.safe_open(file_path, mode="w", chmod=0o744).close()
+    open(file_path, "w").close()
+    filesystem.chmod(file_path, os.stat(file_path).st_mode | stat.S_IXUSR)
 
 
 if __name__ == '__main__':

certbot/tests/log_test.py

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

certbot/tests/main_test.py

@@ -31,6 +31,7 @@ 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.compat import os
 from certbot.compat import filesystem
 from certbot.plugins import disco
@@ -541,7 +542,7 @@ class MainTest(test_util.ConfigTestCase):  # pylint: disable=too-many-public-met
                     return True
                 return orig_open(fn)
 
-            with mock.patch("certbot.compat.os.path.isfile") as mock_if:
+            with mock.patch("os.path.isfile") as mock_if:
                 mock_if.side_effect = mock_isfile
                 with mock.patch('certbot.main.client') as client:
                     ret, stdout, stderr = self._call_no_clientmock(args, stdout)
@@ -808,9 +809,9 @@ class MainTest(test_util.ConfigTestCase):  # pylint: disable=too-many-public-met
         ifaces = []  # type: List[interfaces.IPlugin]
         plugins = mock_disco.PluginsRegistry.find_all()
 
-        def throw_error(directory, mode, strict):
+        def throw_error(directory, mode, uid, strict):
             """Raises error.Error."""
-            _, _, _ = directory, mode, strict
+            _, _, _, _ = directory, mode, uid, strict
             raise errors.Error()
 
         stdout = six.StringIO()
@@ -1592,7 +1593,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,
-                self.config.strict_permissions
+                misc.os_geteuid(), self.config.strict_permissions
             )
 
         hook_dirs = (self.config.renewal_pre_hooks_dir,
@@ -1601,7 +1602,8 @@ 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, strict=self.config.strict_permissions)
+                hook_dir, uid=misc.os_geteuid(),
+                strict=self.config.strict_permissions)
 
 
 class EnhanceTest(test_util.ConfigTestCase):

certbot/tests/storage_test.py

@@ -13,6 +13,7 @@ import six
 import certbot
 import certbot.tests.util as test_util
 from certbot import errors
+from certbot.compat import misc
 from certbot.compat import os
 from certbot.compat import filesystem
 from certbot.storage import ALL_FOUR
@@ -20,6 +21,7 @@ from certbot.storage import ALL_FOUR
 CERT = test_util.load_cert('cert_512.pem')
 
 
+
 def unlink_all(rc_object):
     """Unlink all four items associated with this RenewableCert."""
     for kind in ALL_FOUR:
@@ -496,6 +498,7 @@ class RenewableCertTests(BaseRenewableCertTest):
         self.assertTrue(self.test_rc.should_autorenew())
         mock_ocsp.return_value = False
 
+    @test_util.broken_on_windows
     @mock.patch("certbot.storage.relevant_values")
     def test_save_successor(self, mock_rv):
         # Mock relevant_values() to claim that all values are relevant here
@@ -559,7 +562,7 @@ class RenewableCertTests(BaseRenewableCertTest):
         self.assertFalse(os.path.islink(self.test_rc.version("privkey", 10)))
         self.assertFalse(os.path.exists(temp_config_file))
 
-    @test_util.skip_on_windows('Group/everybody permissions are not maintained on Windows.')
+    @test_util.broken_on_windows
     @mock.patch("certbot.storage.relevant_values")
     def test_save_successor_maintains_group_mode(self, mock_rv):
         # Mock relevant_values() to claim that all values are relevant here
@@ -568,18 +571,22 @@ class RenewableCertTests(BaseRenewableCertTest):
         for kind in ALL_FOUR:
             self._write_out_kind(kind, 1)
         self.test_rc.update_all_links_to(1)
-        self.assertTrue(filesystem.check_mode(self.test_rc.version("privkey", 1), 0o600))
+        self.assertTrue(misc.compare_file_modes(
+            os.stat(self.test_rc.version("privkey", 1)).st_mode, 0o600))
         filesystem.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(filesystem.check_mode(self.test_rc.version("privkey", 2), 0o444))
+        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(filesystem.check_mode(self.test_rc.version("privkey", 3), 0o644))
+        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
         filesystem.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(filesystem.check_mode(self.test_rc.version("privkey", 4), 0o600))
+        self.assertTrue(misc.compare_file_modes(
+            os.stat(self.test_rc.version("privkey", 4)).st_mode, 0o600))
 
     @mock.patch("certbot.storage.relevant_values")
     @mock.patch("certbot.storage.filesystem.copy_ownership_and_apply_mode")
@@ -615,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(filesystem.check_mode(result.key_path, 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.py

@@ -421,6 +421,15 @@ def skip_on_windows(reason):
     return wrapper
 
 
+def broken_on_windows(function):
+    """Decorator to skip temporarily a broken test on Windows."""
+    reason = 'Test is broken and ignored on windows but should be fixed.'
+    return unittest.skipIf(
+        sys.platform == 'win32'
+        and os.environ.get('SKIP_BROKEN_TESTS_ON_WINDOWS', 'true') == 'true',
+        reason)(function)
+
+
 def temp_join(path):
     """
     Return the given path joined to the tempdir path for the current platform

certbot/tests/util_test.py

@@ -9,6 +9,7 @@ from six.moves import reload_module  # pylint: disable=import-error
 
 import certbot.tests.util as test_util
 from certbot import errors
+from certbot.compat import misc
 from certbot.compat import os
 from certbot.compat import filesystem
 
@@ -52,13 +53,26 @@ class ExeExistsTest(unittest.TestCase):
         from certbot.util import exe_exists
         return exe_exists(exe)
 
-    def test_exe_exists(self):
-        with mock.patch("certbot.util.filesystem.is_executable", return_value=True):
-            self.assertTrue(self._call("/path/to/exe"))
+    @mock.patch("certbot.util.os.path.isfile")
+    @mock.patch("certbot.util.os.access")
+    def test_full_path(self, mock_access, mock_isfile):
+        mock_access.return_value = True
+        mock_isfile.return_value = True
+        self.assertTrue(self._call("/path/to/exe"))
 
-    def test_exe_not_exists(self):
-        with mock.patch("certbot.util.filesystem.is_executable", return_value=False):
-            self.assertFalse(self._call("/path/to/exe"))
+    @mock.patch("certbot.util.os.path.isfile")
+    @mock.patch("certbot.util.os.access")
+    def test_on_path(self, mock_access, mock_isfile):
+        mock_access.return_value = True
+        mock_isfile.return_value = True
+        self.assertTrue(self._call("exe"))
+
+    @mock.patch("certbot.util.os.path.isfile")
+    @mock.patch("certbot.util.os.access")
+    def test_not_found(self, mock_access, mock_isfile):
+        mock_access.return_value = False
+        mock_isfile.return_value = True
+        self.assertFalse(self._call("exe"))
 
 
 class LockDirUntilExit(test_util.TempDirTestCase):
@@ -106,14 +120,15 @@ 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, False)
+        self._call(new_dir, 0o700, misc.os_geteuid(), False)
         self.assertTrue(os.path.exists(new_dir))
         self.assertEqual(mock_lock.call_count, 1)
 
     @mock.patch('certbot.util.make_or_verify_dir')
     def test_failure(self, mock_make_or_verify):
         mock_make_or_verify.side_effect = OSError
-        self.assertRaises(errors.Error, self._call, self.tempdir, 0o700, False)
+        self.assertRaises(errors.Error, self._call,
+                          self.tempdir, 0o700, misc.os_geteuid(), False)
 
 
 class MakeOrVerifyDirTest(test_util.TempDirTestCase):
@@ -130,20 +145,23 @@ class MakeOrVerifyDirTest(test_util.TempDirTestCase):
         self.path = os.path.join(self.tempdir, "foo")
         filesystem.mkdir(self.path, 0o600)
 
+        self.uid = misc.os_geteuid()
+
     def _call(self, directory, mode):
         from certbot.util import make_or_verify_dir
-        return make_or_verify_dir(directory, mode, strict=True)
+        return make_or_verify_dir(directory, mode, self.uid, strict=True)
 
     def test_creates_dir_when_missing(self):
         path = os.path.join(self.tempdir, "bar")
         self._call(path, 0o650)
         self.assertTrue(os.path.isdir(path))
-        self.assertTrue(filesystem.check_mode(path, 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(filesystem.check_mode(self.path, 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):
         self.assertRaises(errors.Error, self._call, self.path, 0o400)
 
@@ -153,6 +171,39 @@ class MakeOrVerifyDirTest(test_util.TempDirTestCase):
             self.assertRaises(OSError, self._call, "bar", 12312312)
 
 
+class CheckPermissionsTest(test_util.TempDirTestCase):
+    """Tests for certbot.util.check_permissions.
+
+    Note that it is not possible to test for a wrong file owner,
+    as this testing script would have to be run as root.
+
+    """
+
+    def setUp(self):
+        super(CheckPermissionsTest, self).setUp()
+
+        self.uid = misc.os_geteuid()
+
+    def _call(self, mode):
+        from certbot.util import check_permissions
+        return check_permissions(self.tempdir, mode, self.uid)
+
+    def test_ok_mode(self):
+        filesystem.chmod(self.tempdir, 0o600)
+        self.assertTrue(self._call(0o600))
+
+    # TODO: reactivate the test when all logic from windows file permissions is merged.
+    @test_util.broken_on_windows
+    def test_wrong_mode(self):
+        filesystem.chmod(self.tempdir, 0o400)
+        try:
+            self.assertFalse(self._call(0o600))
+        finally:
+            # Without proper write permissions, Windows is unable to delete a folder,
+            # even with admin permissions. Write access must be explicitly set first.
+            filesystem.chmod(self.tempdir, 0o700)
+
+
 class UniqueFileTest(test_util.TempDirTestCase):
     """Tests for certbot.util.unique_file."""
 
@@ -175,8 +226,8 @@ class UniqueFileTest(test_util.TempDirTestCase):
     def test_right_mode(self):
         fd1, name1 = self._call(0o700)
         fd2, name2 = self._call(0o600)
-        self.assertTrue(filesystem.check_mode(name1, 0o700))
-        self.assertTrue(filesystem.check_mode(name2, 0o600))
+        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()
 
@@ -477,7 +528,7 @@ class OsInfoTest(unittest.TestCase):
         from certbot.util import (get_os_info, get_systemd_os_info,
                                   get_os_info_ua)
 
-        with mock.patch('certbot.compat.os.path.isfile', return_value=True):
+        with mock.patch('os.path.isfile', return_value=True):
             self.assertEqual(get_os_info(
                 test_util.vector_path("os-release"))[0], 'systemdos')
             self.assertEqual(get_os_info(
@@ -485,13 +536,13 @@ class OsInfoTest(unittest.TestCase):
             self.assertEqual(get_systemd_os_info(os.devnull), ("", ""))
             self.assertEqual(get_os_info_ua(
                 test_util.vector_path("os-release")), "SystemdOS")
-        with mock.patch('certbot.compat.os.path.isfile', return_value=False):
+        with mock.patch('os.path.isfile', return_value=False):
             self.assertEqual(get_systemd_os_info(), ("", ""))
 
     def test_systemd_os_release_like(self):
         from certbot.util import get_systemd_os_like
 
-        with mock.patch('certbot.compat.os.path.isfile', return_value=True):
+        with mock.patch('os.path.isfile', return_value=True):
             id_likes = get_systemd_os_like(test_util.vector_path(
                 "os-release"))
             self.assertEqual(len(id_likes), 3)
@@ -501,7 +552,7 @@ class OsInfoTest(unittest.TestCase):
     def test_non_systemd_os_info(self, popen_mock):
         from certbot.util import (get_os_info, get_python_os_info,
                                      get_os_info_ua)
-        with mock.patch('certbot.compat.os.path.isfile', return_value=False):
+        with mock.patch('os.path.isfile', return_value=False):
             with mock.patch('platform.system_alias',
                             return_value=('NonSystemD', '42', '42')):
                 self.assertEqual(get_os_info()[0], 'nonsystemd')

certbot/util.py

@@ -21,6 +21,7 @@ from acme.magic_typing import Tuple, Union  # pylint: disable=unused-import, no-
 from certbot import constants
 from certbot import errors
 from certbot import lock
+from certbot.compat import misc
 from certbot.compat import os
 from certbot.compat import filesystem
 
@@ -86,6 +87,18 @@ def run_script(params, log=logger.error):
     return stdout, stderr
 
 
+def is_exe(path):
+    """Is path an executable file?
+
+    :param str path: path to test
+
+    :returns: True iff path is an executable file
+    :rtype: bool
+
+    """
+    return os.path.isfile(path) and os.access(path, os.X_OK)
+
+
 def exe_exists(exe):
     """Determine whether path/name refers to an executable.
 
@@ -97,10 +110,10 @@ def exe_exists(exe):
     """
     path, _ = os.path.split(exe)
     if path:
-        return filesystem.is_executable(exe)
+        return is_exe(exe)
     else:
         for path in os.environ["PATH"].split(os.pathsep):
-            if filesystem.is_executable(os.path.join(path, exe)):
+            if is_exe(os.path.join(path, exe)):
                 return True
 
     return False
@@ -131,11 +144,12 @@ def _release_locks():
     _LOCKS.clear()
 
 
-def set_up_core_dir(directory, mode, strict):
+def set_up_core_dir(directory, mode, uid, strict):
     """Ensure directory exists with proper permissions and is locked.
 
     :param str directory: Path to a directory.
     :param int mode: Directory mode.
+    :param int uid: Directory owner.
     :param bool strict: require directory to be owned by current user
 
     :raises .errors.LockError: if the directory cannot be locked
@@ -143,18 +157,19 @@ def set_up_core_dir(directory, mode, strict):
 
     """
     try:
-        make_or_verify_dir(directory, mode, strict)
+        make_or_verify_dir(directory, mode, uid, strict)
         lock_dir_until_exit(directory)
     except OSError as error:
         logger.debug("Exception was:", exc_info=True)
         raise errors.Error(PERM_ERR_FMT.format(error))
 
 
-def make_or_verify_dir(directory, mode=0o755, strict=False):
+def make_or_verify_dir(directory, mode=0o755, uid=0, strict=False):
     """Make sure directory exists with proper permissions.
 
     :param str directory: Path to a directory.
     :param int mode: Directory mode.
+    :param int uid: Directory owner.
     :param bool strict: require directory to be owned by current user
 
     :raises .errors.Error: if a directory already exists,
@@ -169,14 +184,29 @@ def make_or_verify_dir(directory, mode=0o755, strict=False):
         filesystem.makedirs(directory, mode)
     except OSError as exception:
         if exception.errno == errno.EEXIST:
-            if strict and not filesystem.check_permissions(directory, mode):
+            if strict and not check_permissions(directory, mode, uid):
                 raise errors.Error(
-                    "%s exists, but it should be owned by current user with"
-                    " permissions %s" % (directory, oct(mode)))
+                    "%s exists, but it should be owned by user %d with"
+                    "permissions %s" % (directory, uid, oct(mode)))
         else:
             raise
 
 
+def check_permissions(filepath, mode, uid=0):
+    """Check file or directory permissions.
+
+    :param str filepath: Path to the tested file (or directory).
+    :param int mode: Expected file mode.
+    :param int uid: Expected file owner.
+
+    :returns: True if `mode` and `uid` match, False otherwise.
+    :rtype: bool
+
+    """
+    file_stat = os.stat(filepath)
+    return misc.compare_file_modes(file_stat.st_mode, mode) and file_stat.st_uid == uid
+
+
 def safe_open(path, mode="w", chmod=None):
     """Safely open a file.
 

docs/contributing.rst

@@ -176,7 +176,7 @@ that can be used once the virtual environment is activated:
 
 .. code-block:: shell
 
-    certbot_test [ARGS...]
+    certbot_tests [ARGS...]
 
 - Execute certbot with the provided arguments and other arguments useful for testing purposes,
   such as: verbose output, full tracebacks in case Certbot crashes, *etc.*

docs/install.rst

@@ -42,7 +42,7 @@ client as root, either `letsencrypt-nosudo
 The Apache plugin currently requires an OS with augeas version 1.0; currently `it
 supports
 <https://github.com/certbot/certbot/blob/master/certbot-apache/certbot_apache/constants.py>`_
-modern OSes based on Debian, Ubuntu, Fedora, SUSE, Gentoo and Darwin.
+modern OSes based on Debian, Fedora, SUSE, Gentoo and Darwin.
 
 
 Additional integrity verification of certbot-auto script can be done by verifying its digital signature.
@@ -218,31 +218,6 @@ repo, if you have not already done so. Then run:
 
    sudo apt-get install certbot python-certbot-apache -t jessie-backports
 
-**Ubuntu**
-
-If you run Ubuntu Trusty, Xenial, or Bionic, certbot is available through the official PPA,
-that can be installed as followed:
-
-.. code-block:: shell
-
-   sudo apt-get update
-   sudo apt-get install software-properties-common
-   sudo add-apt-repository universe
-   sudo add-apt-repository ppa:certbot/certbot
-   sudo apt-get update
-
-Then, certbot can be installed using:
-
-.. code-block:: shell
-
-   sudo apt-get install certbot
-
-Optionally to install the Certbot Apache plugin, you can use:
-
-.. code-block:: shell
-
-   sudo apt-get install python-certbot-apache
-
 **Fedora**
 
 .. code-block:: shell

docs/packaging.rst

@@ -47,3 +47,82 @@ Notes for package maintainers
 4. ``jws`` is an internal script for ``acme`` module and it doesn't have to be packaged - it's mostly for debugging: you can use it as ``echo foo | jws sign | jws verify``.
 
 5. Do get in touch with us. We are happy to make any changes that will make packaging easier. If you need to apply some patches don't do it downstream - make a PR here.
+
+Already ongoing efforts
+=======================
+
+
+Arch
+----
+
+From our official releases:
+
+- https://www.archlinux.org/packages/community/any/python-acme
+- https://www.archlinux.org/packages/community/any/certbot
+- https://www.archlinux.org/packages/community/any/certbot-apache
+- https://www.archlinux.org/packages/community/any/certbot-nginx
+- https://www.archlinux.org/packages/community/any/certbot-dns-cloudflare
+- https://www.archlinux.org/packages/community/any/certbot-dns-cloudxns
+- https://www.archlinux.org/packages/community/any/certbot-dns-digitalocean
+- https://www.archlinux.org/packages/community/any/certbot-dns-dnsimple
+- https://www.archlinux.org/packages/community/any/certbot-dns-dnsmadeeasy
+- https://www.archlinux.org/packages/community/any/certbot-dns-google
+- https://www.archlinux.org/packages/community/any/certbot-dns-luadns
+- https://www.archlinux.org/packages/community/any/certbot-dns-nsone
+- https://www.archlinux.org/packages/community/any/certbot-dns-rfc2136
+- https://www.archlinux.org/packages/community/any/certbot-dns-route53
+
+From ``master``: https://aur.archlinux.org/packages/certbot-git
+
+Debian (and its derivatives, including Ubuntu)
+----------------------------------------------
+
+- https://packages.debian.org/sid/certbot
+- https://packages.debian.org/sid/python-certbot
+- https://packages.debian.org/sid/python-certbot-apache
+
+Fedora
+------
+
+In Fedora 23+.
+
+- https://apps.fedoraproject.org/packages/python-acme
+- https://apps.fedoraproject.org/packages/certbot
+- https://apps.fedoraproject.org/packages/python-certbot-apache
+- https://apps.fedoraproject.org/packages/python-certbot-dns-cloudflare
+- https://apps.fedoraproject.org/packages/python-certbot-dns-cloudxns
+- https://apps.fedoraproject.org/packages/python-certbot-dns-digitalocean
+- https://apps.fedoraproject.org/packages/python-certbot-dns-dnsimple
+- https://apps.fedoraproject.org/packages/python-certbot-dns-dnsmadeeasy
+- https://apps.fedoraproject.org/packages/python-certbot-dns-google
+- https://apps.fedoraproject.org/packages/python-certbot-dns-luadns
+- https://apps.fedoraproject.org/packages/python-certbot-dns-nsone
+- https://apps.fedoraproject.org/packages/python-certbot-dns-rfc2136
+- https://apps.fedoraproject.org/packages/python-certbot-dns-route53
+- https://apps.fedoraproject.org/packages/python-certbot-nginx
+
+FreeBSD
+-------
+
+- https://www.freshports.org/security/py-acme/
+- https://www.freshports.org/security/py-certbot/
+
+Gentoo
+------
+
+Currently, all ``certbot`` related packages are in the testing branch:
+
+- https://packages.gentoo.org/packages/app-crypt/certbot
+- https://packages.gentoo.org/packages/app-crypt/certbot-apache
+- https://packages.gentoo.org/packages/app-crypt/certbot-nginx
+- https://packages.gentoo.org/packages/app-crypt/acme
+
+GNU Guix
+--------
+
+- https://www.gnu.org/software/guix/package-list.html#certbot
+
+OpenBSD
+-------
+
+- http://cvsweb.openbsd.org/cgi-bin/cvsweb/ports/security/letsencrypt/client/

tests/letstest/multitester.py

@@ -375,7 +375,7 @@ def cleanup(cl_args, instances, targetlist):
     # If lengths of instances and targetlist aren't equal, instances failed to
     # start before running tests so leaving instances running for debugging
     # isn't very useful. Let's cleanup after ourselves instead.
-    if len(instances) != len(targetlist) or not cl_args.saveinstances:
+    if len(instances) == len(targetlist) or not cl_args.saveinstances:
         print('Terminating EC2 Instances')
         if cl_args.killboulder:
             boulder_server.terminate()

tools/dev_constraints.txt

@@ -42,12 +42,12 @@ mypy==0.600
 ndg-httpsclient==0.3.2
 oauth2client==2.0.0
 pathlib2==2.3.0
-pexpect==4.7.0
+pexpect==4.2.1
 pickleshare==0.7.4
 pkginfo==1.4.2
 pluggy==0.5.2
 prompt-toolkit==1.0.15
-ptyprocess==0.6.0
+ptyprocess==0.5.2
 py==1.8.0
 pyasn1==0.1.9
 pyasn1-modules==0.0.10
@@ -82,6 +82,6 @@ twine==1.11.0
 typed-ast==1.1.0
 typing==3.6.4
 uritemplate==0.6
-virtualenv==16.6.2
+virtualenv==16.6.1
 wcwidth==0.1.7
 wrapt==1.11.1