quiet and fast

Fix AppVeyor on the apache-parser-v2 branch

.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.6
+        target: 97.5
         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.0
+        target: 97.6
         threshold: 0.1
         base: auto

.travis.yml

@@ -40,220 +40,6 @@ matrix:
       sudo: required
       services: docker
       <<: *not-on-master
-
-    # This job is always executed, including on master
-    - python: "2.7"
-      env: TOXENV=py27-cover FYI="py27 tests + code coverage"
-
-    - python: "2.7"
-      env: TOXENV=lint
-      <<: *not-on-master
-    - python: "3.4"
-      env: TOXENV=mypy
-      <<: *not-on-master
-    - python: "3.5"
-      env: TOXENV=mypy
-      <<: *not-on-master
-    - python: "2.7"
-      env: TOXENV='py27-{acme,apache,certbot,dns,nginx}-oldest'
-      sudo: required
-      services: docker
-      <<: *not-on-master
-    - python: "3.4"
-      env: TOXENV=py34
-      sudo: required
-      services: docker
-      <<: *not-on-master
-    - python: "3.7"
-      dist: xenial
-      env: TOXENV=py37
-      sudo: required
-      services: docker
-      <<: *not-on-master
-    - sudo: required
-      env: TOXENV=apache_compat
-      services: docker
-      before_install:
-      addons:
-      <<: *not-on-master
-    - sudo: required
-      env: TOXENV=le_auto_xenial
-      services: docker
-      <<: *not-on-master
-    - python: "2.7"
-      env: TOXENV=apacheconftest-with-pebble
-      sudo: required
-      services: docker
-      <<: *not-on-master
-    - python: "2.7"
-      env: TOXENV=nginxroundtrip
-      <<: *not-on-master
-
-    # Extended test suite on cron jobs and pushes to tested branches other than master
-    - sudo: required
-      env: TOXENV=nginx_compat
-      services: docker
-      before_install:
-      addons:
-      <<: *extended-test-suite
-    - python: "2.7"
-      env:
-        - TOXENV=travis-test-farm-apache2
-        - secure: "f+j/Lj9s1lcuKo5sEFrlRd1kIAMnIJI4z0MTI7QF8jl9Fkmbx7KECGzw31TNgzrOSzxSapHbcueFYvNCLKST+kE/8ogMZBbwqXfEDuKpyF6BY3uYoJn+wPVE5pIb8Hhe08xPte8TTDSMIyHI3EyTfcAKrIreauoArePvh/cRvSw="
-      <<: *extended-test-suite
-    - python: "2.7"
-      env:
-        - TOXENV=travis-test-farm-leauto-upgrades
-        - secure: "f+j/Lj9s1lcuKo5sEFrlRd1kIAMnIJI4z0MTI7QF8jl9Fkmbx7KECGzw31TNgzrOSzxSapHbcueFYvNCLKST+kE/8ogMZBbwqXfEDuKpyF6BY3uYoJn+wPVE5pIb8Hhe08xPte8TTDSMIyHI3EyTfcAKrIreauoArePvh/cRvSw="
-      git:
-        depth: false  # This is needed to have the history to checkout old versions of certbot-auto.
-      <<: *extended-test-suite
-    - python: "2.7"
-      env:
-        - TOXENV=travis-test-farm-certonly-standalone
-        - secure: "f+j/Lj9s1lcuKo5sEFrlRd1kIAMnIJI4z0MTI7QF8jl9Fkmbx7KECGzw31TNgzrOSzxSapHbcueFYvNCLKST+kE/8ogMZBbwqXfEDuKpyF6BY3uYoJn+wPVE5pIb8Hhe08xPte8TTDSMIyHI3EyTfcAKrIreauoArePvh/cRvSw="
-      <<: *extended-test-suite
-    - python: "2.7"
-      env:
-        - TOXENV=travis-test-farm-sdists
-        - secure: "f+j/Lj9s1lcuKo5sEFrlRd1kIAMnIJI4z0MTI7QF8jl9Fkmbx7KECGzw31TNgzrOSzxSapHbcueFYvNCLKST+kE/8ogMZBbwqXfEDuKpyF6BY3uYoJn+wPVE5pIb8Hhe08xPte8TTDSMIyHI3EyTfcAKrIreauoArePvh/cRvSw="
-      <<: *extended-test-suite
-    - python: "3.7"
-      dist: xenial
-      env: TOXENV=py37 CERTBOT_NO_PIN=1
-      <<: *extended-test-suite
-    - python: "2.7"
-      env: ACME_SERVER=boulder-v1 TOXENV=integration
-      sudo: required
-      services: docker
-      <<: *extended-test-suite
-    - python: "2.7"
-      env: ACME_SERVER=boulder-v2 TOXENV=integration
-      sudo: required
-      services: docker
-      <<: *extended-test-suite
-    - python: "2.7"
-      env: TOXENV=py27-certbot-oldest
-      <<: *extended-test-suite
-    - python: "2.7"
-      env: TOXENV=py27-nginx-oldest
-      <<: *extended-test-suite
-    - python: "2.7"
-      env: ACME_SERVER=boulder-v1 TOXENV=integration-certbot-oldest
-      sudo: required
-      services: docker
-      <<: *extended-test-suite
-    - python: "2.7"
-      env: ACME_SERVER=boulder-v2 TOXENV=integration-certbot-oldest
-      sudo: required
-      services: docker
-      <<: *extended-test-suite
-    - python: "2.7"
-      env: ACME_SERVER=boulder-v1 TOXENV=integration-nginx-oldest
-      sudo: required
-      services: docker
-      <<: *extended-test-suite
-    - python: "2.7"
-      env: ACME_SERVER=boulder-v2 TOXENV=integration-nginx-oldest
-      sudo: required
-      services: docker
-      <<: *extended-test-suite
-    - python: "3.4"
-      env: TOXENV=py34
-      <<: *extended-test-suite
-    - python: "3.5"
-      env: TOXENV=py35
-      <<: *extended-test-suite
-    - python: "3.6"
-      env: TOXENV=py36
-      <<: *extended-test-suite
-    - python: "3.7"
-      dist: xenial
-      env: TOXENV=py37
-      <<: *extended-test-suite
-    - python: "3.4"
-      env: ACME_SERVER=boulder-v1 TOXENV=integration
-      sudo: required
-      services: docker
-      <<: *extended-test-suite
-    - python: "3.4"
-      env: ACME_SERVER=boulder-v2 TOXENV=integration
-      sudo: required
-      services: docker
-      <<: *extended-test-suite
-    - python: "3.5"
-      env: ACME_SERVER=boulder-v1 TOXENV=integration
-      sudo: required
-      services: docker
-      <<: *extended-test-suite
-    - python: "3.5"
-      env: ACME_SERVER=boulder-v2 TOXENV=integration
-      sudo: required
-      services: docker
-      <<: *extended-test-suite
-    - python: "3.6"
-      env: ACME_SERVER=boulder-v1 TOXENV=integration
-      sudo: required
-      services: docker
-      <<: *extended-test-suite
-    - python: "3.6"
-      env: ACME_SERVER=boulder-v2 TOXENV=integration
-      sudo: required
-      services: docker
-      <<: *extended-test-suite
-    - python: "3.7"
-      dist: xenial
-      env: ACME_SERVER=boulder-v1 TOXENV=integration
-      sudo: required
-      services: docker
-      <<: *extended-test-suite
-    - python: "3.7"
-      dist: xenial
-      env: ACME_SERVER=boulder-v2 TOXENV=integration
-      sudo: required
-      services: docker
-      <<: *extended-test-suite
-    - sudo: required
-      env: TOXENV=le_auto_jessie
-      services: docker
-      <<: *extended-test-suite
-    - sudo: required
-      env: TOXENV=le_auto_centos6
-      services: docker
-      <<: *extended-test-suite
-    - sudo: required
-      env: TOXENV=docker_dev
-      services: docker
-      addons:
-        apt:
-          packages:  # don't install nginx and apache
-            - libaugeas0
-      <<: *extended-test-suite
-    - language: generic
-      env: TOXENV=py27
-      os: osx
-      # Using this osx_image is a workaround for
-      # https://travis-ci.community/t/xcode-8-3-homebrew-outdated-error/3798.
-      osx_image: xcode10.2
-      addons:
-        homebrew:
-          packages:
-            - augeas
-            - python2
-      <<: *extended-test-suite
-    - language: generic
-      env: TOXENV=py3
-      os: osx
-      # Using this osx_image is a workaround for
-      # https://travis-ci.community/t/xcode-8-3-homebrew-outdated-error/3798.
-      osx_image: xcode10.2
-      addons:
-        homebrew:
-          packages:
-            - augeas
-            - python3
-      <<: *extended-test-suite
-
 # container-based infrastructure
 sudo: false
 
@@ -282,13 +68,3 @@ after_success: '[ "$TOXENV" == "py27-cover" ] && codecov -F linux'
 
 notifications:
   email: false
-  irc:
-    channels:
-      # This is set to a secure variable to prevent forks from sending
-      # notifications. This value was created by installing
-      # https://github.com/travis-ci/travis.rb and running
-      # `travis encrypt "chat.freenode.net#certbot-devel"`.
-      - secure: "EWW66E2+KVPZyIPR8ViENZwfcup4Gx3/dlimmAZE0WuLwxDCshBBOd3O8Rf6pBokEoZlXM5eDT6XdyJj8n0DLslgjO62pExdunXpbcMwdY7l1ELxX2/UbnDTE6UnPYa09qVBHNG7156Z6yE0x2lH4M9Ykvp0G0cubjPQHylAwo0="
-    on_cancel: never
-    on_success: never
-    on_failure: always

AUTHORS.md

@@ -15,6 +15,7 @@ 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

@@ -2,6 +2,23 @@
 
 Certbot adheres to [Semantic Versioning](https://semver.org/).
 
+## 0.37.0 - master
+
+### Added
+
+* Turn off session tickets for apache plugin by default
+* acme: Authz deactivation added to `acme` module.
+
+### Changed
+
+*
+
+### Fixed
+
+*
+
+More details about these changes can be found on our GitHub repo.
+
 ## 0.36.0 - 2019-07-11
 
 ### Added

Dockerfile

@@ -1,35 +0,0 @@
-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

Dockerfile-dev

@@ -1,5 +1,5 @@
 # This Dockerfile builds an image for development.
-FROM ubuntu:xenial
+FROM debian:buster
 
 # Note: this only exposes the port to other docker containers.
 EXPOSE 80 443

acme/acme/client.py

@@ -123,6 +123,21 @@ 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,6 +637,14 @@ 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,6 +168,7 @@ STATUS_VALID = Status('valid')
 STATUS_INVALID = Status('invalid')
 STATUS_REVOKED = Status('revoked')
 STATUS_READY = Status('ready')
+STATUS_DEACTIVATED = Status('deactivated')
 
 
 class IdentifierType(_Constant):
@@ -471,7 +472,7 @@ class Authorization(ResourceBody):
     :ivar datetime.datetime expires:
 
     """
-    identifier = jose.Field('identifier', decoder=Identifier.from_json)
+    identifier = jose.Field('identifier', decoder=Identifier.from_json, omitempty=True)
     challenges = jose.Field('challenges', omitempty=True)
     combinations = jose.Field('combinations', omitempty=True)
 
@@ -501,6 +502,12 @@ 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.
 

acme/setup.py

@@ -3,7 +3,7 @@ from setuptools import find_packages
 from setuptools.command.test import test as TestCommand
 import sys
 
-version = '0.36.0'
+version = '0.37.0.dev0'
 
 # Please update tox.ini when modifying dependency version requirements
 install_requires = [

certbot-apache/MANIFEST.in

@@ -5,3 +5,4 @@ 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,6 +1,8 @@
 """ Utility functions for certbot-apache plugin """
 import binascii
 
+import pkg_resources
+
 from certbot import util
 from certbot.compat import os
 
@@ -105,3 +107,15 @@ 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/configurator.py

@@ -9,7 +9,6 @@ import time
 
 from collections import defaultdict
 
-import pkg_resources
 import six
 
 import zope.component
@@ -23,6 +22,7 @@ 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,14 +109,24 @@ 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
@@ -895,7 +905,7 @@ class ApacheConfigurator(common.Installer):
                 if not new_vhost:
                     continue
                 internal_path = apache_util.get_internal_aug_path(new_vhost.path)
-                realpath = os.path.realpath(new_vhost.filep)
+                realpath = filesystem.realpath(new_vhost.filep)
                 if realpath not in file_paths:
                     file_paths[realpath] = new_vhost.filep
                     internal_paths[realpath].add(internal_path)
@@ -1221,11 +1231,11 @@ class ApacheConfigurator(common.Installer):
         """
 
         if self.conf("vhost-root") and os.path.exists(self.conf("vhost-root")):
-            fp = os.path.join(os.path.realpath(self.option("vhost_root")),
+            fp = os.path.join(filesystem.realpath(self.option("vhost_root")),
                               os.path.basename(non_ssl_vh_fp))
         else:
             # Use non-ssl filepath
-            fp = os.path.realpath(non_ssl_vh_fp)
+            fp = filesystem.realpath(non_ssl_vh_fp)
 
         if fp.endswith(".conf"):
             return fp[:-(len(".conf"))] + self.option("le_vhost_ext")
@@ -2338,8 +2348,9 @@ 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.
-        return common.install_version_controlled_file(options_ssl, options_ssl_digest,
-            self.option("MOD_SSL_CONF_SRC"), constants.ALL_SSL_OPTIONS_HASHES)
+        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)
 
     def enable_autohsts(self, _unused_lineage, domains):
         """

certbot-apache/certbot_apache/constants.py

@@ -9,6 +9,7 @@ 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',
@@ -18,6 +19,10 @@ 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/override_arch.py

@@ -1,6 +1,4 @@
 """ Distribution specific override class for Arch Linux """
-import pkg_resources
-
 import zope.interface
 
 from certbot import interfaces
@@ -26,6 +24,4 @@ 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,7 +1,6 @@
 """ Distribution specific override class for CentOS family (RHEL, Fedora) """
 import logging
 
-import pkg_resources
 import zope.interface
 
 from certbot import errors
@@ -39,8 +38,6 @@ 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):
@@ -75,6 +72,18 @@ 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,6 +1,4 @@
 """ Distribution specific override class for macOS """
-import pkg_resources
-
 import zope.interface
 
 from certbot import interfaces
@@ -26,6 +24,4 @@ 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,8 +34,6 @@ 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):
@@ -65,7 +63,7 @@ class DebianConfigurator(configurator.ApacheConfigurator):
         try:
             os.symlink(vhost.filep, enabled_path)
         except OSError as err:
-            if os.path.islink(enabled_path) and os.path.realpath(
+            if os.path.islink(enabled_path) and filesystem.realpath(
                enabled_path) == vhost.filep:
                 # Already in shape
                 vhost.enabled = True

certbot-apache/certbot_apache/override_fedora.py

@@ -1,5 +1,4 @@
 """ Distribution specific override class for Fedora 29+ """
-import pkg_resources
 import zope.interface
 
 from certbot import errors
@@ -31,9 +30,6 @@ 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,6 +1,4 @@
 """ Distribution specific override class for Gentoo Linux """
-import pkg_resources
-
 import zope.interface
 
 from certbot import interfaces
@@ -29,8 +27,6 @@ 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,6 +1,4 @@
 """ Distribution specific override class for OpenSUSE """
-import pkg_resources
-
 import zope.interface
 
 from certbot import interfaces
@@ -26,6 +24,4 @@ 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,6 +4,7 @@ import unittest
 import mock
 
 from certbot import errors
+from certbot.compat import filesystem
 from certbot.compat import os
 
 from certbot_apache import obj
@@ -160,7 +161,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 = os.path.realpath(
+        self.config.parser.sysconfig_filep = filesystem.realpath(
             os.path.join(self.config.parser.root, "../sysconfig/httpd"))
         self.config.parser.variables = {}
 
@@ -189,6 +190,13 @@ 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,8 +675,7 @@ 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(os.path.realpath(
-                                self.vh_truth[1].filep)))
+                         os.path.dirname(filesystem.realpath(self.vh_truth[1].filep)))
 
     def test_make_vhost_ssl(self):
         ssl_vhost = self.config.make_vhost_ssl(self.vh_truth[0])
@@ -1336,7 +1335,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 = os.path.realpath(tempfile.mkdtemp("vhostroot"))
+        tmp_path = filesystem.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"
@@ -1707,7 +1706,7 @@ class InstallSslOptionsConfTest(util.ApacheTest):
                                              self.config.updated_mod_ssl_conf_digest)
 
     def _current_ssl_options_hash(self):
-        return crypto_util.sha256sum(self.config.option("MOD_SSL_CONF_SRC"))
+        return crypto_util.sha256sum(self.config.pick_apache_config())
 
     def _assert_current_file(self):
         self.assertTrue(os.path.isfile(self.config.mod_ssl_conf))
@@ -1743,7 +1742,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.option("MOD_SSL_CONF_SRC")),
+            self.config.pick_apache_config()),
             self._current_ssl_options_hash())
         self.assertNotEqual(crypto_util.sha256sum(self.config.mod_ssl_conf),
             self._current_ssl_options_hash())
@@ -1759,18 +1758,31 @@ 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.option("MOD_SSL_CONF_SRC")),
+            self.config.pick_apache_config()),
             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_current_file_hash_in_all_hashes(self):
+    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.
+        """
         from certbot_apache.constants import ALL_SSL_OPTIONS_HASHES
-        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.")
+        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))
 
 
 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("os.path.isdir") as mock_dir:
+        with mock.patch("certbot.compat.os.path.isdir") as mock_dir:
             mock_dir.return_value = True
-            with mock.patch("os.path.islink") as mock_link:
+            with mock.patch("certbot.compat.os.path.islink") as mock_link:
                 mock_link.return_value = False
                 self.assertRaises(
                     errors.NotSupportedError,

certbot-apache/certbot_apache/tests/fedora_test.py

@@ -4,6 +4,7 @@ import unittest
 import mock
 
 from certbot import errors
+from certbot.compat import filesystem
 from certbot.compat import os
 
 from certbot_apache import obj
@@ -160,7 +161,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 = os.path.realpath(
+        self.config.parser.sysconfig_filep = filesystem.realpath(
             os.path.join(self.config.parser.root, "../sysconfig/httpd"))
         self.config.parser.variables = {}
 

certbot-apache/certbot_apache/tests/gentoo_test.py

@@ -4,6 +4,7 @@ import unittest
 import mock
 
 from certbot import errors
+from certbot.compat import filesystem
 from certbot.compat import os
 
 from certbot_apache import obj
@@ -81,7 +82,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 = os.path.realpath(
+        self.config.parser.apacheconfig_filep = filesystem.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-current-options-ssl-apache.conf

@@ -10,16 +10,10 @@ 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/tls_configs/centos-old-options-ssl-apache.conf

@@ -0,0 +1,18 @@
+# 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/current-options-ssl-apache.conf

@@ -11,16 +11,10 @@ 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/tls_configs/old-options-ssl-apache.conf

@@ -0,0 +1,19 @@
+# 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
-certbot[dev]==0.36.0
+-e .[dev]

certbot-apache/setup.py

@@ -4,13 +4,13 @@ from setuptools.command.test import test as TestCommand
 import sys
 
 
-version = '0.36.0'
+version = '0.37.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.
 install_requires = [
     'acme>=0.29.0',
-    'certbot>=0.36.0',
+    'certbot>=0.37.0.dev0',
     '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'])
+        misc.check_until_timeout(self.acme_xdist['directory_url'], attempts=240)
 
         # 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,12 +28,13 @@ RSA_KEY_TYPE = 'rsa'
 ECDSA_KEY_TYPE = 'ecdsa'
 
 
-def check_until_timeout(url):
+def check_until_timeout(url, attempts=30):
     """
     Wait and block until given url responds with status 200, or raise an exception
-    after 150 attempts.
+    after the specified number of attempts.
     :param str url: the URL to test
-    :raise ValueError: exception raised after 150 unsuccessful attempts to reach the URL
+    :param int attempts: the number of times to try to connect to the URL
+    :raise ValueError: exception raised if unable to reach the URL
     """
     try:
         import urllib3
@@ -43,7 +44,7 @@ def check_until_timeout(url):
         from requests.packages.urllib3.exceptions import InsecureRequestWarning
         requests.packages.urllib3.disable_warnings(InsecureRequestWarning)
 
-    for _ in range(0, 150):
+    for _ in range(attempts):
         time.sleep(1)
         try:
             if requests.get(url, verify=False).status_code == 200:
@@ -51,7 +52,7 @@ def check_until_timeout(url):
         except requests.exceptions.ConnectionError:
             pass
 
-    raise ValueError('Error, url did not respond after 150 attempts: {0}'.format(url))
+    raise ValueError('Error, url did not respond after {0} attempts: {1}'.format(attempts, url))
 
 
 class GracefulTCPServer(socketserver.TCPServer):

certbot-compatibility-test/setup.py

@@ -4,7 +4,7 @@ from setuptools import setup
 from setuptools import find_packages
 
 
-version = '0.36.0'
+version = '0.37.0.dev0'
 
 install_requires = [
     'certbot',

certbot-dns-cloudflare/Dockerfile

@@ -1,5 +0,0 @@
-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,7 +22,9 @@ 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>`_.
+`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.
 
 .. code-block:: ini
    :name: credentials.ini

certbot-dns-cloudflare/setup.py

@@ -2,7 +2,7 @@ from setuptools import setup
 from setuptools import find_packages
 
 
-version = '0.36.0'
+version = '0.37.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.

certbot-dns-cloudxns/Dockerfile

@@ -1,5 +0,0 @@
-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-cloudxns/setup.py

@@ -2,7 +2,7 @@ from setuptools import setup
 from setuptools import find_packages
 
 
-version = '0.36.0'
+version = '0.37.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.

certbot-dns-digitalocean/Dockerfile

@@ -1,5 +0,0 @@
-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-digitalocean/setup.py

@@ -2,7 +2,7 @@ from setuptools import setup
 from setuptools import find_packages
 
 
-version = '0.36.0'
+version = '0.37.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.

certbot-dns-dnsimple/Dockerfile

@@ -1,5 +0,0 @@
-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-dnsimple/setup.py

@@ -3,7 +3,7 @@ from setuptools import setup
 from setuptools import find_packages
 
 
-version = '0.36.0'
+version = '0.37.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.

certbot-dns-dnsmadeeasy/Dockerfile

@@ -1,5 +0,0 @@
-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-dnsmadeeasy/setup.py

@@ -2,7 +2,7 @@ from setuptools import setup
 from setuptools import find_packages
 
 
-version = '0.36.0'
+version = '0.37.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.

certbot-dns-gehirn/Dockerfile

@@ -1,5 +0,0 @@
-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-gehirn/setup.py

@@ -2,7 +2,7 @@ from setuptools import setup
 from setuptools import find_packages
 
 
-version = '0.36.0'
+version = '0.37.0.dev0'
 
 # Please update tox.ini when modifying dependency version requirements
 install_requires = [

certbot-dns-google/Dockerfile

@@ -1,5 +0,0 @@
-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-google/setup.py

@@ -2,7 +2,7 @@ from setuptools import setup
 from setuptools import find_packages
 
 
-version = '0.36.0'
+version = '0.37.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.

certbot-dns-linode/Dockerfile

@@ -1,5 +0,0 @@
-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-linode/setup.py

@@ -1,7 +1,7 @@
 from setuptools import setup
 from setuptools import find_packages
 
-version = '0.36.0'
+version = '0.37.0.dev0'
 
 # Please update tox.ini when modifying dependency version requirements
 install_requires = [

certbot-dns-luadns/Dockerfile

@@ -1,5 +0,0 @@
-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-luadns/setup.py

@@ -2,7 +2,7 @@ from setuptools import setup
 from setuptools import find_packages
 
 
-version = '0.36.0'
+version = '0.37.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.

certbot-dns-nsone/Dockerfile

@@ -1,5 +0,0 @@
-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-nsone/setup.py

@@ -2,7 +2,7 @@ from setuptools import setup
 from setuptools import find_packages
 
 
-version = '0.36.0'
+version = '0.37.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.

certbot-dns-ovh/Dockerfile

@@ -1,5 +0,0 @@
-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-ovh/setup.py

@@ -2,7 +2,7 @@ from setuptools import setup
 from setuptools import find_packages
 
 
-version = '0.36.0'
+version = '0.37.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.

certbot-dns-rfc2136/Dockerfile

@@ -1,5 +0,0 @@
-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-rfc2136/setup.py

@@ -2,7 +2,7 @@ from setuptools import setup
 from setuptools import find_packages
 
 
-version = '0.36.0'
+version = '0.37.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.

certbot-dns-route53/Dockerfile

@@ -1,5 +0,0 @@
-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-route53/MANIFEST.in

@@ -1,3 +1,3 @@
-include LICENSE
+include LICENSE.txt
 include README
 recursive-include docs *

certbot-dns-route53/setup.py

@@ -1,7 +1,7 @@
 from setuptools import setup
 from setuptools import find_packages
 
-version = '0.36.0'
+version = '0.37.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.

certbot-dns-sakuracloud/Dockerfile

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

certbot-dns-sakuracloud/setup.py

@@ -2,7 +2,7 @@ from setuptools import setup
 from setuptools import find_packages
 
 
-version = '0.36.0'
+version = '0.37.0.dev0'
 
 # Please update tox.ini when modifying dependency version requirements
 install_requires = [

certbot-nginx/certbot_nginx/configurator.py

@@ -20,7 +20,6 @@ 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
 
@@ -903,13 +902,9 @@ class NginxConfigurator(common.Installer):
         have permissions of root.
 
         """
-        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)
+        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)
 
     def get_version(self):
         """Return version of Nginx Server.

certbot-nginx/setup.py

@@ -4,7 +4,7 @@ from setuptools.command.test import test as TestCommand
 import sys
 
 
-version = '0.36.0'
+version = '0.37.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.

certbot/__init__.py

@@ -1,4 +1,4 @@
 """Certbot client."""
 
 # version number like 1.2.3a0, must have at least 2 parts, like 1.2
-__version__ = '0.36.0'
+__version__ = '0.37.0.dev0'

certbot/account.py

@@ -20,7 +20,6 @@ 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__)
@@ -139,8 +138,7 @@ class AccountFileStorage(interfaces.AccountStorage):
     """
     def __init__(self, config):
         self.config = config
-        util.make_or_verify_dir(config.accounts_dir, 0o700, misc.os_geteuid(),
-                                self.config.strict_permissions)
+        util.make_or_verify_dir(config.accounts_dir, 0o700, 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)
@@ -322,8 +320,7 @@ 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, misc.os_geteuid(),
-                                self.config.strict_permissions)
+        util.make_or_verify_dir(account_dir_path, 0o700, 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,7 +15,6 @@ 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
 
@@ -106,7 +105,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, uid=misc.os_geteuid())
+    util.make_or_verify_dir(configs_dir, mode=0o755)
     try:
         renewal_file = storage.renewal_file_for_certname(cli_config, certname)
     except errors.CertStorageError:
@@ -375,7 +374,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, uid=misc.os_geteuid())
+    util.make_or_verify_dir(configs_dir, mode=0o755)
 
     rv = initial_rv
     for renewal_file in storage.renewal_conf_files(cli_config):

certbot/client.py

@@ -30,7 +30,6 @@ 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
@@ -459,9 +458,7 @@ class Client(object):
 
         """
         for path in cert_path, chain_path, fullchain_path:
-            util.make_or_verify_dir(
-                os.path.dirname(path), 0o755, misc.os_geteuid(),
-                self.config.strict_permissions)
+            util.make_or_verify_dir(os.path.dirname(path), 0o755, self.config.strict_permissions)
 
 
         cert_file, abs_cert_path = _open_pem_file('cert_path', cert_path)

certbot/compat/_path.py

@@ -0,0 +1,31 @@
+"""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,6 +77,54 @@ 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
     """
@@ -107,6 +155,10 @@ 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
@@ -177,6 +229,7 @@ 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:
@@ -207,13 +260,22 @@ def replace(src, dst):
         os.rename(src, dst)
 
 
-def _apply_win_mode(file_path, mode):
+def realpath(file_path):
     """
-    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.
+    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.
     """
     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
@@ -223,6 +285,53 @@ def _apply_win_mode(file_path, mode):
         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()
@@ -333,6 +442,28 @@ 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,6 +30,10 @@ 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
     """
@@ -42,22 +46,6 @@ 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
     """
@@ -88,16 +76,6 @@ 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,7 +26,9 @@ for attribute in dir(std_os):
     if not hasattr(ourselves, attribute):
         setattr(ourselves, attribute, getattr(std_os, attribute))
 
-# Similar to os.path, allow certbot.compat.os.path to behave as a module
+# 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
 std_sys.modules[__name__ + '.path'] = path
 
 # Clean all remaining importables that are not from the core os module.
@@ -105,3 +107,12 @@ 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,9 +169,10 @@ ACCOUNTS_DIR = "accounts"
 """Directory where all accounts are saved."""
 
 LE_REUSE_SERVERS = {
-    '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'
+    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')
 }
 """Servers that can reuse accounts from other servers."""
 

certbot/crypto_util.py

@@ -28,7 +28,6 @@ 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__)
@@ -61,8 +60,7 @@ 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, misc.os_geteuid(),
-                            config.strict_permissions)
+    util.make_or_verify_dir(key_dir, 0o700, config.strict_permissions)
     key_f, key_path = util.unique_file(
         os.path.join(key_dir, keyname), 0o600, "wb")
     with key_f:
@@ -92,8 +90,7 @@ def init_save_csr(privkey, names, path):
         privkey.pem, names, must_staple=config.must_staple)
 
     # Save CSR
-    util.make_or_verify_dir(path, 0o755, misc.os_geteuid(),
-                            config.strict_permissions)
+    util.make_or_verify_dir(path, 0o755, 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,6 +8,7 @@ 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
 
@@ -254,7 +255,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):
@@ -267,5 +268,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 util.is_exe(path) and not path.endswith('~')]
+    hooks = [path for path in allpaths if filesystem.is_executable(path) and not path.endswith('~')]
     return sorted(hooks)

certbot/log.py

@@ -17,6 +17,7 @@ from __future__ import print_function
 import functools
 import logging
 import logging.handlers
+import shutil
 import sys
 import tempfile
 import traceback
@@ -26,7 +27,6 @@ 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,8 +134,7 @@ 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, misc.os_geteuid(), config.strict_permissions)
+    util.set_up_core_dir(config.logs_dir, 0o700, config.strict_permissions)
     log_file_path = os.path.join(config.logs_dir, logfile)
     try:
         handler = logging.handlers.RotatingFileHandler(
@@ -240,9 +239,10 @@ class TempHandler(logging.StreamHandler):
 
     """
     def __init__(self):
-        stream = tempfile.NamedTemporaryFile('w', delete=False)
+        self._workdir = tempfile.mkdtemp()
+        self.path = os.path.join(self._workdir, 'log')
+        stream = util.safe_open(self.path, mode='w', chmod=0o600)
         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:
-                os.remove(self.path)
+                shutil.rmtree(self._workdir)
             self._delete = False
             super(TempHandler, self).close()
         finally:

certbot/main.py

@@ -31,6 +31,7 @@ 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
@@ -841,12 +842,12 @@ def _populate_from_certname(config):
     return config
 
 def _check_certificate_and_key(config):
-    if not os.path.isfile(os.path.realpath(config.cert_path)):
+    if not os.path.isfile(filesystem.realpath(config.cert_path)):
         raise errors.ConfigurationError("Error while reading certificate from path "
-                                       "{0}".format(config.cert_path))
-    if not os.path.isfile(os.path.realpath(config.key_path)):
+                                        "{0}".format(config.cert_path))
+    if not os.path.isfile(filesystem.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.
 
@@ -1298,18 +1299,14 @@ def make_or_verify_needed_dirs(config):
     :rtype: None
 
     """
-    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)
+    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)
 
     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,
-                                uid=misc.os_geteuid(),
-                                strict=config.strict_permissions)
+        util.make_or_verify_dir(hook_dir, 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 os.path.realpath(tempfile.mkdtemp(prefix))
+        return filesystem.realpath(tempfile.mkdtemp(prefix))
 
     temp_dir = expanded_tempdir("temp")
     config_dir = expanded_tempdir("config")

certbot/plugins/dns_common.py

@@ -303,8 +303,8 @@ def validate_file(filename):
     if not os.path.exists(filename):
         raise errors.PluginError('File not found: {0}'.format(filename))
 
-    if not os.path.isfile(filename):
-        raise errors.PluginError('Path is not a file: {0}'.format(filename))
+    if os.path.isdir(filename):
+        raise errors.PluginError('Path is a directory: {0}'.format(filename))
 
 
 def validate_file_permissions(filename):

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('os.path.isfile', return_value=True):
+            with mock.patch('certbot.compat.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,9 +3,11 @@ 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,
@@ -26,6 +28,7 @@ def get_prefixes(path):
             break
     return prefixes
 
+
 def path_surgery(cmd):
     """Attempt to perform PATH surgery to find cmd
 
@@ -35,10 +38,9 @@ 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 dirs:
+    for d in STANDARD_BINARY_DIRS:
         if d not in path:
             path += os.pathsep + d
             added.append(d)

certbot/plugins/util_test.py

@@ -16,6 +16,7 @@ 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."""
 
@@ -29,13 +30,15 @@ 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"])
-        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 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"])
 
 
 if __name__ == "__main__":

certbot/plugins/webroot.py

@@ -24,6 +24,7 @@ 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__)
 
@@ -207,7 +208,7 @@ to serve all files under specified web root ({0})."""
         old_umask = os.umask(0o022)
 
         try:
-            with open(validation_path, "wb") as validation_file:
+            with safe_open(validation_path, mode="wb", chmod=0o644) as validation_file:
                 validation_file.write(validation.encode())
         finally:
             os.umask(old_umask)

certbot/plugins/webroot_test.py

@@ -17,7 +17,6 @@ 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
@@ -168,14 +167,14 @@ class AuthenticatorTest(unittest.TestCase):
         # Remove exec bit from permission check, so that it
         # matches the file
         self.auth.perform([self.achall])
-        self.assertTrue(misc.compare_file_modes(os.stat(self.validation_path).st_mode, 0o644))
+        self.assertTrue(filesystem.check_mode(self.validation_path, 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(misc.compare_file_modes(os.stat(full_path).st_mode, 0o755))
+                self.assertTrue(filesystem.check_mode(full_path, 0o755))
 
         parent_gid = os.stat(self.path).st_gid
         parent_uid = os.stat(self.path).st_uid

certbot/reverter.py

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

certbot/tests/account_test.py

@@ -2,7 +2,6 @@
 import datetime
 import json
 import shutil
-import stat
 import unittest
 
 import josepy as jose
@@ -13,6 +12,7 @@ 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,7 +116,6 @@ 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)
@@ -124,8 +123,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(oct(os.stat(os.path.join(
-            account_path, "private_key.json"))[stat.ST_MODE] & 0o777) in ("0400", "0o400"))
+        self.assertTrue(
+            filesystem.check_mode(os.path.join(account_path, "private_key.json"), 0o400))
 
         # restore
         loaded = self.storage.load(self.acc.id)
@@ -219,14 +218,12 @@ 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)
@@ -244,7 +241,6 @@ 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)
@@ -253,7 +249,6 @@ 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)
@@ -278,7 +273,6 @@ 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)
@@ -313,12 +307,10 @@ 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()
@@ -328,15 +320,14 @@ 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(
-                        os.path.realpath(os.readlink(self.config_files[domain][kind])),
-                        os.path.realpath(archive_paths[domain][kind]))
+                        filesystem.realpath(os.readlink(self.config_files[domain][kind])),
+                        filesystem.realpath(archive_paths[domain][kind]))
         finally:
             os.chdir(prev_dir)
 
@@ -277,13 +277,12 @@ 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)
 
 
@@ -294,33 +293,28 @@ 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)
 
 
@@ -331,7 +325,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"]
@@ -344,12 +338,10 @@ 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/cli_test.py

@@ -1,7 +1,6 @@
 """Tests for certbot.cli."""
 import argparse
 import copy
-import sys
 import tempfile
 import unittest
 
@@ -44,11 +43,15 @@ class TestReadFile(TempDirTestCase):
 class FlagDefaultTest(unittest.TestCase):
     """Tests cli.flag_default"""
 
-    def test_linux_directories(self):
-        if 'fcntl' in sys.modules:
+    def test_default_directories(self):
+        if os.name != 'nt':
             self.assertEqual(cli.flag_default('config_dir'), '/etc/letsencrypt')
             self.assertEqual(cli.flag_default('work_dir'), '/var/lib/letsencrypt')
             self.assertEqual(cli.flag_default('logs_dir'), '/var/log/letsencrypt')
+        else:
+            self.assertEqual(cli.flag_default('config_dir'), 'C:\\Certbot')
+            self.assertEqual(cli.flag_default('work_dir'), 'C:\\Certbot\\lib')
+            self.assertEqual(cli.flag_default('logs_dir'), 'C:\\Certbot\\log')
 
 
 class ParseTest(unittest.TestCase):  # pylint: disable=too-many-public-methods

certbot/tests/compat/filesystem_test.py

@@ -1,4 +1,5 @@
 """Tests for certbot.compat.filesystem"""
+import contextlib
 import errno
 import unittest
 
@@ -16,6 +17,7 @@ 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
@@ -48,18 +50,6 @@ 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)
 
@@ -318,9 +308,54 @@ 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')
@@ -335,6 +370,112 @@ 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)
 
@@ -352,7 +493,7 @@ def _set_owner(target, security_owner, user):
 def _create_probe(tempdir):
     filesystem.chmod(tempdir, 0o744)
     probe_path = os.path.join(tempdir, 'probe')
-    open(probe_path, 'w').close()
+    util.safe_open(probe_path, 'w', chmod=0o744).close()
     return probe_path
 
 

certbot/tests/compat/os_test.py

@@ -7,8 +7,13 @@ from certbot.compat import os
 class OsTest(unittest.TestCase):
     """Unit tests for os module."""
     def test_forbidden_methods(self):
-        for method in ['chmod', 'chown', 'open', 'mkdir', 'makedirs', 'rename', 'replace']:
+        # Checks for os module
+        for method in ['chmod', 'chown', 'open', 'mkdir',
+                       'makedirs', 'rename', 'replace', 'access']:
             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,6 +11,7 @@ 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')
@@ -29,6 +30,9 @@ 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)
@@ -46,15 +50,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.tempdir)
+        key = self._call(1024, self.workdir)
         self.assertEqual(key.pem, b'key_pem')
         self.assertTrue('key-certbot.pem' in key.file)
-        self.assertTrue(os.path.exists(os.path.join(self.tempdir, key.file)))
+        self.assertTrue(os.path.exists(os.path.join(self.workdir, 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.tempdir)
+        self.assertRaises(ValueError, self._call, 431, self.workdir)
 
 
 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
+from certbot.tests import util as test_util
 
 
 class ValidateHooksTest(unittest.TestCase):
@@ -30,7 +30,7 @@ class ValidateHooksTest(unittest.TestCase):
         self.assertEqual("renew", types[-1])
 
 
-class ValidateHookTest(util.TempDirTestCase):
+class ValidateHookTest(test_util.TempDirTestCase):
     """Tests for certbot.hooks.validate_hook."""
 
     @classmethod
@@ -38,22 +38,20 @@ class ValidateHookTest(util.TempDirTestCase):
         from certbot.hooks import validate_hook
         return validate_hook(*args, **kwargs)
 
-    @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))
+    def test_hook_not_executable(self):
         # prevent unnecessary modifications to PATH
         with mock.patch("certbot.hooks.plug_util.path_surgery"):
-            self.assertRaises(errors.HookCommandNotFound,
-                              self._call, file_path, "foo")
+            # 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")
 
     @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")
@@ -62,7 +60,7 @@ class ValidateHookTest(util.TempDirTestCase):
         self.assertFalse(mock_prog.called)
 
 
-class HookTest(util.ConfigTestCase):
+class HookTest(test_util.ConfigTestCase):
     """Common base class for hook tests."""
 
     @classmethod
@@ -454,7 +452,7 @@ class ExecuteTest(unittest.TestCase):
             self.assertTrue(mock_logger.error.called)
 
 
-class ListHooksTest(util.TempDirTestCase):
+class ListHooksTest(test_util.TempDirTestCase):
     """Tests for certbot.hooks.list_hooks."""
 
     @classmethod
@@ -494,8 +492,7 @@ def create_hook(file_path):
     :param str file_path: path to create the file at
 
     """
-    open(file_path, "w").close()
-    filesystem.chmod(file_path, os.stat(file_path).st_mode | stat.S_IXUSR)
+    util.safe_open(file_path, mode="w", chmod=0o744).close()
 
 
 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 misc
+from certbot.compat import filesystem
 from certbot.compat import os
 from certbot.tests import util as test_util
 
@@ -260,8 +260,7 @@ class TempHandlerTest(unittest.TestCase):
         self.handler.close()
 
     def test_permissions(self):
-        self.assertTrue(
-            util.check_permissions(self.handler.path, 0o600, misc.os_geteuid()))
+        self.assertTrue(filesystem.check_permissions(self.handler.path, 0o600))
 
     def test_delete(self):
         self.handler.close()

certbot/tests/main_test.py

@@ -31,7 +31,6 @@ 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
@@ -542,7 +541,7 @@ class MainTest(test_util.ConfigTestCase):  # pylint: disable=too-many-public-met
                     return True
                 return orig_open(fn)
 
-            with mock.patch("os.path.isfile") as mock_if:
+            with mock.patch("certbot.compat.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)
@@ -809,9 +808,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, uid, strict):
+        def throw_error(directory, mode, strict):
             """Raises error.Error."""
-            _, _, _, _ = directory, mode, uid, strict
+            _, _, _ = directory, mode, strict
             raise errors.Error()
 
         stdout = six.StringIO()
@@ -1593,7 +1592,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,
-                misc.os_geteuid(), self.config.strict_permissions
+                self.config.strict_permissions
             )
 
         hook_dirs = (self.config.renewal_pre_hooks_dir,
@@ -1602,8 +1601,7 @@ class MakeOrVerifyNeededDirs(test_util.ConfigTestCase):
         for hook_dir in hook_dirs:
             # default mode of 755 is used
             mock_util.make_or_verify_dir.assert_any_call(
-                hook_dir, uid=misc.os_geteuid(),
-                strict=self.config.strict_permissions)
+                hook_dir, strict=self.config.strict_permissions)
 
 
 class EnhanceTest(test_util.ConfigTestCase):

certbot/tests/storage_test.py

@@ -13,7 +13,6 @@ 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
@@ -21,7 +20,6 @@ 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:
@@ -498,7 +496,6 @@ 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
@@ -562,7 +559,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.broken_on_windows
+    @test_util.skip_on_windows('Group/everybody permissions are not maintained 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
@@ -571,22 +568,18 @@ class RenewableCertTests(BaseRenewableCertTest):
         for kind in ALL_FOUR:
             self._write_out_kind(kind, 1)
         self.test_rc.update_all_links_to(1)
-        self.assertTrue(misc.compare_file_modes(
-            os.stat(self.test_rc.version("privkey", 1)).st_mode, 0o600))
+        self.assertTrue(filesystem.check_mode(self.test_rc.version("privkey", 1), 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(misc.compare_file_modes(
-            os.stat(self.test_rc.version("privkey", 2)).st_mode, 0o444))
+        self.assertTrue(filesystem.check_mode(self.test_rc.version("privkey", 2), 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(misc.compare_file_modes(
-            os.stat(self.test_rc.version("privkey", 3)).st_mode, 0o644))
+        self.assertTrue(filesystem.check_mode(self.test_rc.version("privkey", 3), 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(misc.compare_file_modes(
-            os.stat(self.test_rc.version("privkey", 4)).st_mode, 0o600))
+        self.assertTrue(filesystem.check_mode(self.test_rc.version("privkey", 4), 0o600))
 
     @mock.patch("certbot.storage.relevant_values")
     @mock.patch("certbot.storage.filesystem.copy_ownership_and_apply_mode")
@@ -622,7 +615,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(misc.compare_file_modes(os.stat(result.key_path).st_mode, 0o600))
+        self.assertTrue(filesystem.check_mode(result.key_path, 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,15 +421,6 @@ 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