Disable notifications

# Conflicts:
#	certbot-ci/certbot_integration_tests/utils/certbot_call.py

.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: 98.0
+        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: 96.9
+        target: 97.6
         threshold: 0.1
         base: auto

.gitignore

@@ -47,3 +47,5 @@ tests/letstest/venv/
 
 # certbot tests
 .certbot_test_workspace
+**/assets/pebble*
+**/assets/challtestsrv*

.travis.yml

@@ -5,9 +5,21 @@ cache:
         - $HOME/.cache/pip
 
 before_script:
+  # Install required apt packages
+  - |
+    if [[ "$TRAVIS_OS_NAME" != "osx" ]]; then
+      ./certbot-auto --non-interactive --os-packages-only
+      sudo -E apt-get -yq --no-install-suggests --no-install-recommends install nginx-light
+      sudo -E /etc/init.d/nginx stop
+      sudo -E apt-get -yq --no-install-suggests --no-install-recommends install apache2
+      sudo -E /etc/init.d/apache2 stop
+      sudo -E chmod 777 -R /var/lib/apache2/module
+    fi
   - 'if [[ "$TRAVIS_OS_NAME" == "osx" ]]; then ulimit -n 1024 ; fi'
   # On Travis, the fastest parallelization for integration tests has proved to be 4.
   - 'if [[ "$TOXENV" == *"integration"* ]]; then export PYTEST_ADDOPTS="--numprocesses 4"; fi'
+  # Use Travis retry feature for farm tests since they are flaky
+  - 'if [[ "$TOXENV" == "travis-test-farm"* ]]; then export TRAVIS_RETRY=travis_retry; fi'
   - export TOX_TESTENV_PASSENV=TRAVIS
 
 # Only build pushes to the master branch, PRs, and branches beginning with
@@ -16,6 +28,9 @@ before_script:
 # is a cap of on the number of simultaneous runs.
 branches:
   only:
+    # apache-parser-v2 is a temporary branch for doing work related to
+    # rewriting the parser in the Apache plugin.
+    - apache-parser-v2
     - master
     - /^\d+\.\d+\.x$/
     - /^test-.*$/
@@ -24,17 +39,16 @@ branches:
 not-on-master: &not-on-master
   if: NOT (type = push AND branch = master)
 
-# Jobs for the extended test suite are executed for cron jobs and pushes on non-master branches.
+# Jobs for the extended test suite are executed for cron jobs and pushes to
+# non-development branches. See the explanation for apache-parser-v2 above.
 extended-test-suite: &extended-test-suite
-  if: type = cron OR (type = push AND branch != master)
+  if: type = cron OR (type = push AND branch NOT IN (apache-parser-v2, master))
 
 matrix:
   include:
     # Main test suite
     - python: "2.7"
       env: ACME_SERVER=pebble TOXENV=integration
-      sudo: required
-      services: docker
       <<: *not-on-master
 
     # This job is always executed, including on master
@@ -51,46 +65,31 @@ matrix:
       env: TOXENV=mypy
       <<: *not-on-master
     - python: "2.7"
+      # Ubuntu Trusty or older must be used because the oldest version of
+      # cryptography we support cannot be compiled against the version of
+      # OpenSSL in Xenial or newer.
+      dist: trusty
       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:
+    - env: TOXENV=apache_compat
       <<: *not-on-master
-    - sudo: required
-      env: TOXENV=le_auto_xenial
-      services: docker
+    - env: TOXENV=le_auto_xenial
       <<: *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:
+    - env: TOXENV=nginx_compat
       <<: *extended-test-suite
     - python: "2.7"
       env:
@@ -115,44 +114,29 @@ matrix:
         - 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
+      dist: trusty  # See py27-{acme,apache,certbot,dns,nginx}-oldest tests
       <<: *extended-test-suite
     - python: "2.7"
       env: ACME_SERVER=boulder-v2 TOXENV=integration-certbot-oldest
-      sudo: required
-      services: docker
+      dist: trusty  # See py27-{acme,apache,certbot,dns,nginx}-oldest tests
       <<: *extended-test-suite
     - python: "2.7"
       env: ACME_SERVER=boulder-v1 TOXENV=integration-nginx-oldest
-      sudo: required
-      services: docker
+      dist: trusty  # See py27-{acme,apache,certbot,dns,nginx}-oldest tests
       <<: *extended-test-suite
     - python: "2.7"
       env: ACME_SERVER=boulder-v2 TOXENV=integration-nginx-oldest
-      sudo: required
-      services: docker
+      dist: trusty  # See py27-{acme,apache,certbot,dns,nginx}-oldest tests
       <<: *extended-test-suite
     - python: "3.4"
       env: TOXENV=py34
@@ -164,66 +148,37 @@ matrix:
       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
+    - env: TOXENV=le_auto_jessie
       <<: *extended-test-suite
-    - sudo: required
-      env: TOXENV=le_auto_centos6
-      services: docker
+    - env: TOXENV=le_auto_centos6
       <<: *extended-test-suite
-    - sudo: required
-      env: TOXENV=docker_dev
-      services: docker
-      addons:
-        apt:
-          packages:  # don't install nginx and apache
-            - libaugeas0
+    - env: TOXENV=docker_dev
       <<: *extended-test-suite
     - language: generic
       env: TOXENV=py27
@@ -250,41 +205,29 @@ matrix:
             - python3
       <<: *extended-test-suite
 
-# container-based infrastructure
-sudo: false
-
-addons:
-  apt:
-    packages:  # Keep in sync with letsencrypt-auto-source/pieces/bootstrappers/deb_common.sh and Boulder.
-    - python-dev
-    - gcc
-    - libaugeas0
-    - libssl-dev
-    - libffi-dev
-    - ca-certificates
-    # For certbot-nginx integration testing
-    - nginx-light
-    - openssl
-
 # tools/pip_install.py is used to pin packages to a known working version
 # except in tests where the environment variable CERTBOT_NO_PIN is set.
 # virtualenv is listed here explicitly to make sure it is upgraded when
 # CERTBOT_NO_PIN is set to work around failures we've seen when using an older
 # version of virtualenv.
-install: "tools/pip_install.py -U codecov tox virtualenv"
-script: tox
+install: 'tools/pip_install.py -U codecov tox virtualenv'
+# Most of the time TRAVIS_RETRY is an empty string, and has no effect on the
+# script command. It is set only to `travis_retry` during farm tests, in
+# order to trigger the Travis retry feature, and compensate the inherent
+# flakiness of these specific tests.
+script: '$TRAVIS_RETRY tox'
 
 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
+#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)
@@ -161,6 +162,7 @@ Authors
 * [Michael Schumacher](https://github.com/schumaml)
 * [Michael Strache](https://github.com/Jarodiv)
 * [Michael Sverdlin](https://github.com/sveder)
+* [Michael Watters](https://github.com/blackknight36)
 * [Michal Moravec](https://github.com/https://github.com/Majkl578)
 * [Michal Papis](https://github.com/mpapis)
 * [Minn Soe](https://github.com/MinnSoe)

CHANGELOG.md

@@ -2,7 +2,61 @@
 
 Certbot adheres to [Semantic Versioning](https://semver.org/).
 
-## 0.36.0 - master
+## 0.38.0 - master
+
+### Added
+
+*
+
+### Changed
+
+* If Certbot fails to rollback your server configuration, the error message
+  links to the Let's Encrypt forum. Change the link to the Help category now
+  that the Server category has been closed.
+* Replace platform.linux_distribution with distro.linux_distribution as a step
+  towards Python 3.8 support in Certbot.
+
+### Fixed
+
+* Fixed OS detection in the Apache plugin on Scientific Linux.
+
+More details about these changes can be found on our GitHub repo.
+
+## 0.37.2 - 2019-08-21
+
+* Stop disabling TLS session tickets in Nginx as it caused TLS failures on
+  some systems.
+
+More details about these changes can be found on our GitHub repo.
+
+## 0.37.1 - 2019-08-08
+
+### Fixed
+
+* Stop disabling TLS session tickets in Apache as it caused TLS failures on
+  some systems.
+
+More details about these changes can be found on our GitHub repo.
+
+## 0.37.0 - 2019-08-07
+
+### Added
+
+* Turn off session tickets for apache plugin by default
+* acme: Authz deactivation added to `acme` module.
+
+### Changed
+
+* Follow updated Mozilla recommendations for Nginx ssl_protocols, ssl_ciphers,
+  and ssl_prefer_server_ciphers
+
+### Fixed
+
+* Fix certbot-auto failures on RHEL 8.
+
+More details about these changes can be found on our GitHub repo.
+
+## 0.36.0 - 2019-07-11
 
 ### Added
 
@@ -15,7 +69,12 @@ Certbot adheres to [Semantic Versioning](https://semver.org/).
 * Update the 'manage your account' help to be more generic.
 * The error message when Certbot's Apache plugin is unable to modify your
   Apache configuration has been improved.
+* Certbot's config_changes subcommand has been deprecated and will be
+  removed in a future release.
 * `certbot config_changes` no longer accepts a --num parameter.
+* The functions `certbot.plugins.common.Installer.view_config_changes` and
+  `certbot.reverter.Reverter.view_config_changes` have been deprecated and will
+  be removed in a future release.
 
 ### Fixed
 

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.dev0'
+version = '0.38.0.dev0'
 
 # Please update tox.ini when modifying dependency version requirements
 install_requires = [

appveyor.yml

@@ -4,9 +4,13 @@ environment:
   matrix:
     - TOXENV: py35
     - TOXENV: py37-cover
+    - TOXENV: integration-certbot
 
 branches:
   only:
+    # apache-parser-v2 is a temporary branch for doing work related to
+    # rewriting the parser in the Apache plugin.
+    - apache-parser-v2
     - master
     - /^\d+\.\d+\.x$/ # Version branches like X.X.X
     - /^test-.*$/
@@ -21,14 +25,16 @@ init:
 
 install:
   # Use Python 3.7 by default
-  - "SET PATH=C:\\Python37;C:\\Python37\\Scripts;%PATH%"
+  - SET PATH=C:\\Python37;C:\\Python37\\Scripts;%PATH%
+  # Using 4 processes is proven to be the most efficient integration tests config for AppVeyor
+  - IF %TOXENV%==integration-certbot SET PYTEST_ADDOPTS=--numprocesses=4
   # Check env
-  - "python --version"
+  - python --version
   # Upgrade pip to avoid warnings
-  - "python -m pip install --upgrade pip"
+  - python -m pip install --upgrade pip
   # Ready to install tox and coverage
   # tools/pip_install.py is used to pin packages to a known working version.
-  - "python tools\\pip_install.py tox codecov"
+  - python tools\\pip_install.py tox codecov
 
 build: off
 

certbot-apache/certbot_apache/configurator.py

@@ -23,6 +23,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
@@ -895,7 +896,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 +1222,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")

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/entrypoint.py

@@ -31,6 +31,8 @@ OVERRIDE_CLASSES = {
     "gentoo base system": override_gentoo.GentooConfigurator,
     "opensuse": override_suse.OpenSUSEConfigurator,
     "suse": override_suse.OpenSUSEConfigurator,
+    "scientific": override_centos.CentOSConfigurator,
+    "scientific linux": override_centos.CentOSConfigurator,
 }
 
 

certbot-apache/certbot_apache/http_01.py

@@ -169,8 +169,7 @@ class ApacheHttp01(common.TLSSNI01):
 
     def _set_up_challenges(self):
         if not os.path.isdir(self.challenge_dir):
-            os.makedirs(self.challenge_dir)
-            filesystem.chmod(self.challenge_dir, 0o755)
+            filesystem.makedirs(self.challenge_dir, 0o755)
 
         responses = []
         for achall in self.achalls:

certbot-apache/certbot_apache/override_debian.py

@@ -7,6 +7,7 @@ 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
@@ -65,7 +66,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/tests/apache-conf-files/apache-conf-test-pebble.py

@@ -15,7 +15,7 @@ SCRIPT_DIRNAME = os.path.dirname(__file__)
 def main(args=None):
     if not args:
         args = sys.argv[1:]
-    with acme_server.setup_acme_server('pebble', [], False) as acme_xdist:
+    with acme_server.ACMEServer('pebble', [], False) as acme_xdist:
         environ = os.environ.copy()
         environ['SERVER'] = acme_xdist['directory_url']
         command = [os.path.join(SCRIPT_DIRNAME, 'apache-conf-test')]

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 = {}
 

certbot-apache/certbot_apache/tests/configurator_reverter_test.py

@@ -74,14 +74,6 @@ class ConfiguratorReverterTest(util.ApacheTest):
             side_effect=errors.ReverterError)
         self.assertRaises(errors.PluginError, self.config.rollback_checkpoints)
 
-    def test_view_config_changes(self):
-        self.config.view_config_changes()
-
-    def test_view_config_changes_error(self):
-        self.config.reverter.view_config_changes = mock.Mock(
-            side_effect=errors.ReverterError)
-        self.assertRaises(errors.PluginError, self.config.view_config_changes)
-
     def test_recovery_routine_reload(self):
         mock_load = mock.Mock()
         self.config.parser.aug.load = mock_load

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"

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/local-oldest-requirements.txt

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

certbot-apache/setup.py

@@ -4,13 +4,13 @@ from setuptools.command.test import test as TestCommand
 import sys
 
 
-version = '0.36.0.dev0'
+version = '0.38.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.dev0',
+    'certbot>=0.37.0',
     'mock',
     'python-augeas',
     'setuptools',

certbot-auto

@@ -31,7 +31,7 @@ if [ -z "$VENV_PATH" ]; then
 fi
 VENV_BIN="$VENV_PATH/bin"
 BOOTSTRAP_VERSION_PATH="$VENV_PATH/certbot-auto-bootstrap-version.txt"
-LE_AUTO_VERSION="0.35.1"
+LE_AUTO_VERSION="0.37.2"
 BASENAME=$(basename $0)
 USAGE="Usage: $BASENAME [OPTIONS]
 A self-updating wrapper script for the Certbot ACME client. When run, updates
@@ -755,13 +755,31 @@ elif [ -f /etc/redhat-release ]; then
   prev_le_python="$LE_PYTHON"
   unset LE_PYTHON
   DeterminePythonVersion "NOCRASH"
-  # Starting to Fedora 29, python2 is on a deprecation path. Let's move to python3 then.
+
   RPM_DIST_NAME=`(. /etc/os-release 2> /dev/null && echo $ID) || echo "unknown"`
-  RPM_DIST_VERSION=0
-  if [ "$RPM_DIST_NAME" = "fedora" ]; then
-    RPM_DIST_VERSION=`(. /etc/os-release 2> /dev/null && echo $VERSION_ID) || echo "0"`
+
+  # Set RPM_DIST_VERSION to VERSION_ID from /etc/os-release after splitting on
+  # '.' characters (e.g. "8.0" becomes "8"). If the command exits with an
+  # error, RPM_DIST_VERSION is set to "unknown".
+  RPM_DIST_VERSION=$( (. /etc/os-release 2> /dev/null && echo "$VERSION_ID") | cut -d '.' -f1 || echo "unknown")
+
+  # If RPM_DIST_VERSION is an empty string or it contains any nonnumeric
+  # characters, the value is unexpected so we set RPM_DIST_VERSION to 0.
+  if [ -z "$RPM_DIST_VERSION" ] || [ -n "$(echo "$RPM_DIST_VERSION" | tr -d '[0-9]')" ]; then
+     RPM_DIST_VERSION=0
   fi
+
+  # Starting to Fedora 29, python2 is on a deprecation path. Let's move to python3 then.
+  # RHEL 8 also uses python3 by default.
   if [ "$RPM_DIST_NAME" = "fedora" -a "$RPM_DIST_VERSION" -ge 29 -o "$PYVER" -eq 26 ]; then
+    RPM_USE_PYTHON_3=1
+  elif [ "$RPM_DIST_NAME" = "rhel" -a "$RPM_DIST_VERSION" -ge 8 ]; then
+    RPM_USE_PYTHON_3=1
+  else
+    RPM_USE_PYTHON_3=0
+  fi
+
+  if [ "$RPM_USE_PYTHON_3" = 1 ]; then
     Bootstrap() {
       BootstrapMessage "RedHat-based OSes that will use Python3"
       BootstrapRpmPython3
@@ -775,6 +793,7 @@ elif [ -f /etc/redhat-release ]; then
     }
     BOOTSTRAP_VERSION="BootstrapRpmCommon $BOOTSTRAP_RPM_COMMON_VERSION"
   fi
+
   LE_PYTHON="$prev_le_python"
 elif [ -f /etc/os-release ] && `grep -q openSUSE /etc/os-release` ; then
   Bootstrap() {
@@ -1314,18 +1333,18 @@ letsencrypt==0.7.0 \
     --hash=sha256:105a5fb107e45bcd0722eb89696986dcf5f08a86a321d6aef25a0c7c63375ade \
     --hash=sha256:c36e532c486a7e92155ee09da54b436a3c420813ec1c590b98f635d924720de9
 
-certbot==0.35.1 \
-    --hash=sha256:24821e10b05084a45c5bf29da704115f2637af613866589737cff502294dad2a \
-    --hash=sha256:d7e8ecc14e06ed1dc691c6069bc9ce42dce04e8db1684ddfab446fbd71290860
-acme==0.35.1 \
-    --hash=sha256:3ec62f638f2b3684bcb3d8476345c7ae37c8f3b28f2999622ff836aec6e73d64 \
-    --hash=sha256:a988b8b418cc74075e68b4acf3ff64c026bf52c377b0d01223233660a755c423
-certbot-apache==0.35.1 \
-    --hash=sha256:ee4fe10cbd18e0aa7fe36d43ad7792187f41a7298f383610b87049c3a6493bbb \
-    --hash=sha256:69962eafe0ec9be8eb2845e3622da6f37ecaeee7e517ea172d71d7b31f01be71
-certbot-nginx==0.35.1 \
-    --hash=sha256:22150f13b3c0bd1c3c58b11a64886dad9695796aac42f5809da7ec66de187760 \
-    --hash=sha256:85e9a48b4b549f6989304f66cb2fad822c3f8717d361bde0d6a43aabb792d461
+certbot==0.37.2 \
+    --hash=sha256:8f6f0097fb2aac64f13e5d6974781ac85a051d84a6cb3f4d79c6b75c5ea451b8 \
+    --hash=sha256:e454368aa8d62559c673091b511319c130c8e0ea1c4dfa314ed7bdc91dd96ef5
+acme==0.37.2 \
+    --hash=sha256:5666ba927a9e7bf3f9ed5a268bd5acf627b5838fb409e8401f05d2aaaee188ba \
+    --hash=sha256:88798fae3bc692397db79c66930bd02fcaba8a6b1fba9a62f111dda42cc47f5c
+certbot-apache==0.37.2 \
+    --hash=sha256:e3ae7057f727506ab3796095ed66ca083f4e295d06f209ab96d2a3f37dea51b9 \
+    --hash=sha256:4cb44d1a7c56176a84446a11412c561479ed0fed19848632e61f104dbf6a3031
+certbot-nginx==0.37.2 \
+    --hash=sha256:a92dffdf3daca97db5d7ae2287e505110c3fa01c035b9356abb2ef9fa32e8695 \
+    --hash=sha256:404f7b5b7611f0dce8773739170f306e94a59b69528cb74337e7f354936ac061
 
 UNLIKELY_EOF
     # -------------------------------------------------------------------------

certbot-ci/certbot_integration_tests/apache_tests/apache_config.py

@@ -0,0 +1,171 @@
+import shutil
+import subprocess
+import os
+
+import pkg_resources
+import getpass
+
+
+def construct_apache_config_dir(apache_root, http_port, https_port, key_path=None,
+                                cert_path=None, wtf_prefix='le'):
+    config_path = os.path.join(apache_root, 'config')
+    shutil.copytree('/etc/apache2', config_path, symlinks=True)
+
+    webroot_path = os.path.join(apache_root, 'www')
+    os.mkdir(webroot_path)
+    with open(os.path.join(webroot_path, 'index.html'), 'w') as file_h:
+        file_h.write('Hello World!')
+
+    main_config_path = os.path.join(config_path, 'apache2.conf')
+    with open(main_config_path, 'w') as file_h:
+        file_h.write('''\
+ServerRoot "{config}"
+DefaultRuntimeDir ${{APACHE_RUN_DIR}}
+PidFile ${{APACHE_PID_FILE}}
+Timeout 300
+KeepAlive On
+MaxKeepAliveRequests 100
+KeepAliveTimeout 5
+User ${{APACHE_RUN_USER}}
+Group ${{APACHE_RUN_GROUP}}
+HostnameLookups Off
+ErrorLog ${{APACHE_LOG_DIR}}/error.log
+LogLevel warn
+
+IncludeOptional mods-enabled/*.load
+IncludeOptional mods-enabled/*.conf
+
+Include ports.conf
+
+<Directory />
+    Options FollowSymLinks
+    AllowOverride None
+    Require all denied
+</Directory>
+
+<Directory /usr/share>
+    AllowOverride None
+    Require all granted
+</Directory>
+
+<Directory {webroot}/>
+    Options Indexes FollowSymLinks
+    AllowOverride None
+    Require all granted
+</Directory>
+
+AccessFileName .htaccess
+
+<FilesMatch "^\.ht">
+    Require all denied
+</FilesMatch>
+
+LogFormat "%v:%p %h %l %u %t \\"%r\\" %>s %O \\"%{{Referer}}i\\" \\"%{{User-Agent}}i\\"" vhost_combined
+LogFormat "%h %l %u %t \\"%r\\" %>s %O \\"%{{Referer}}i\\" \\"%{{User-Agent}}i\\"" combined
+LogFormat "%h %l %u %t \\"%r\\" %>s %O" common
+LogFormat "%{{Referer}}i -> %U" referer
+LogFormat "%{{User-agent}}i" agent
+
+IncludeOptional conf-enabled/*.conf
+IncludeOptional sites-enabled/*.conf
+'''.format(config=config_path, webroot=webroot_path))
+
+    with open(os.path.join(config_path, 'ports.conf'), 'w') as file_h:
+        file_h.write('''\
+Listen {http}
+<IfModule ssl_module>
+    Listen {https}
+</IfModule>
+<IfModule mod_gnutls.c>
+    Listen {https}
+</IfModule>
+'''.format(http=http_port, https=https_port))
+
+    new_environ = os.environ.copy()
+    new_environ['APACHE_CONFDIR'] = config_path
+
+    run_path = os.path.join(apache_root, 'run')
+    lock_path = os.path.join(apache_root, 'lock')
+    logs_path = os.path.join(apache_root, 'logs')
+    os.mkdir(run_path)
+    os.mkdir(lock_path)
+    os.mkdir(logs_path)
+
+    user = getpass.getuser()
+    user = user if user != 'root' else 'www-data'
+    group = user
+
+    pid_file = os.path.join(run_path, 'apache.pid')
+
+    with open(os.path.join(config_path, 'envvars'), 'w') as file_h:
+        file_h.write('''\
+unset HOME
+export APACHE_RUN_USER={user}
+export APACHE_RUN_GROUP={group}
+export APACHE_PID_FILE={pid_file}
+export APACHE_RUN_DIR={run_path}
+export APACHE_LOCK_DIR={lock_path}
+export APACHE_LOG_DIR={logs_path}
+export LANG=C
+'''.format(user=user, group=group, pid_file=pid_file,
+           run_path=run_path, lock_path=lock_path, logs_path=logs_path))
+
+    new_environ['APACHE_RUN_USER'] = user
+    new_environ['APACHE_RUN_GROUP'] = group
+    new_environ['APACHE_PID_FILE'] = pid_file
+    new_environ['APACHE_RUN_DIR'] = run_path
+    new_environ['APACHE_LOCK_DIR'] = lock_path
+    new_environ['APACHE_LOG_DIR'] = logs_path
+
+    le_host = 'apache.{0}.wtf'.format(wtf_prefix)
+
+    with open(os.path.join(config_path, 'sites-available', '000-default.conf'), 'w') as file_h:
+        file_h.write('''\
+<VirtualHost *:{http}>
+    ServerAdmin webmaster@localhost
+    ServerName {le_host}
+    DocumentRoot {webroot}
+    
+    ErrorLog ${{APACHE_LOG_DIR}}/error.log
+    CustomLog ${{APACHE_LOG_DIR}}/access.log combined
+</VirtualHost>
+'''.format(http=http_port, le_host=le_host, webroot=webroot_path))
+
+    key_path = key_path if key_path \
+        else pkg_resources.resource_filename('certbot_integration_tests', 'assets/key.pem')
+    cert_path = cert_path if cert_path \
+        else pkg_resources.resource_filename('certbot_integration_tests', 'assets/cert.pem')
+
+    with open(os.path.join(config_path, 'sites-available', 'default-ssl.conf'), 'w') as file_h:
+        file_h.write('''\
+<IfModule mod_ssl.c>
+    <VirtualHost _default_:{https}>
+        ServerAdmin webmaster@localhost
+        ServerName {le_host}
+        DocumentRoot {webroot}
+        
+        ErrorLog ${{APACHE_LOG_DIR}}/error.log
+        CustomLog ${{APACHE_LOG_DIR}}/access.log combined
+        
+        SSLEngine on
+        SSLCertificateFile {cert_path}
+        SSLCertificateKeyFile {key_path}
+        
+        <FilesMatch "\.(cgi|shtml|phtml|php)$">
+            SSLOptions +StdEnvVars
+        </FilesMatch>
+        
+        <Directory /usr/lib/cgi-bin>
+            SSLOptions +StdEnvVars
+        </Directory>
+    </VirtualHost>
+</IfModule>
+'''.format(https=https_port, le_host=le_host, webroot=webroot_path,
+           cert_path=cert_path, key_path=key_path))
+
+    return new_environ, config_path, pid_file
+
+
+def test():
+    env = construct_apache_config_dir('/tmp/test1', 5001, 5002)
+    subprocess.call(['apache2ctl', '-DFOREGROUND'], env=env)

certbot-ci/certbot_integration_tests/apache_tests/context.py

@@ -0,0 +1,51 @@
+import errno
+import os
+import signal
+import subprocess
+
+from certbot_integration_tests.certbot_tests import context as certbot_context
+from certbot_integration_tests.apache_tests import apache_config
+from certbot_integration_tests.utils import certbot_call
+
+
+class IntegrationTestsContext(certbot_context.IntegrationTestsContext):
+    def __init__(self, request):
+        super(IntegrationTestsContext, self).__init__(request)
+
+        subprocess.check_output(['chmod', '+x', self.workspace])
+
+        self.apache_root = os.path.join(self.workspace, 'apache')
+        os.mkdir(self.apache_root)
+
+        self.env, self.config_dir, self.pid_file = apache_config.construct_apache_config_dir(
+            self.apache_root, self.http_01_port, self.tls_alpn_01_port,
+            wtf_prefix=self.worker_id)
+
+    def cleanup(self):
+        self._stop_apache()
+        super(IntegrationTestsContext, self).cleanup()
+
+    def certbot_test_apache(self, args):
+        command = ['--authenticator', 'apache', '--installer', 'apache',
+                   '--apache-server-root', self.config_dir,
+                   '--apache-challenge-location', self.apache_root]
+        command.extend(args)
+
+        return certbot_call.certbot_test(
+            command, self.directory_url, self.http_01_port, self.tls_alpn_01_port,
+            self.config_dir, self.workspace, env=self.env, force_renew=True)
+
+    def _stop_apache(self):
+        try:
+            with open(self.pid_file) as file_h:
+                pid = int(file_h.read().strip())
+        except BaseException:
+            pid = None
+
+        if pid:
+            try:
+                os.kill(pid, signal.SIGTERM)
+            except OSError as err:
+                # Ignore "No such process" error, Apache may already be stopped.
+                if err.errno != errno.ESRCH:
+                    raise

certbot-ci/certbot_integration_tests/apache_tests/test_main.py

@@ -0,0 +1,18 @@
+import pytest
+
+from certbot_integration_tests.apache_tests import context as apache_context
+
+
+@pytest.fixture()
+def context(request):
+    # Fixture request is a built-in pytest fixture describing current test request.
+    integration_test_context = apache_context.IntegrationTestsContext(request)
+    try:
+        yield integration_test_context
+    finally:
+        integration_test_context.cleanup()
+
+
+def test_it(context):
+    command = ['-d', 'apache.{0}.wtf'.format(context.worker_id)]
+    context.certbot_test_apache(command)

certbot-ci/certbot_integration_tests/assets/hook.py

@@ -0,0 +1,11 @@
+#!/usr/bin/env python
+import sys
+import os
+
+hook_script_type = os.path.basename(os.path.dirname(sys.argv[1]))
+if hook_script_type == 'deploy' and ('RENEWED_DOMAINS' not in os.environ or 'RENEWED_LINEAGE' not in os.environ):
+    sys.stderr.write('Environment variables not properly set!\n')
+    sys.exit(1)
+
+with open(sys.argv[2], 'a') as file_h:
+    file_h.write(hook_script_type + '\n')

certbot-ci/certbot_integration_tests/certbot_tests/assertions.py

@@ -1,6 +1,17 @@
 """This module contains advanced assertions for the certbot integration tests."""
 import os
-import grp
+try:
+    import grp
+    POSIX_MODE = True
+except ImportError:
+    import win32api
+    import win32security
+    import ntsecuritycon
+    POSIX_MODE = False
+
+EVERYBODY_SID = 'S-1-1-0'
+SYSTEM_SID = 'S-1-5-18'
+ADMINS_SID = 'S-1-5-32-544'
 
 
 def assert_hook_execution(probe_path, probe_content):
@@ -10,9 +21,10 @@ def assert_hook_execution(probe_path, probe_content):
     :param probe_content: content expected when the hook is executed
     """
     with open(probe_path, 'r') as file:
-        lines = file.readlines()
+        data = file.read()
 
-    assert '{0}{1}'.format(probe_content, os.linesep) in lines
+    lines = [line.strip() for line in data.splitlines()]
+    assert probe_content in lines
 
 
 def assert_saved_renew_hook(config_dir, lineage):
@@ -38,16 +50,51 @@ def assert_cert_count_for_lineage(config_dir, lineage, count):
     assert len(certs) == count
 
 
-def assert_equals_permissions(file1, file2, mask):
+def assert_equals_group_permissions(file1, file2):
     """
-    Assert that permissions on two files are identical in respect to a given umask.
+    Assert that two files have the same permissions for group owner.
     :param file1: first file path to compare
     :param file2: second file path to compare
-    :param mask: 3-octal representation of a POSIX umask under which the two files mode
-                 should match (eg. 0o074 will test RWX on group and R on world)
     """
-    mode_file1 = os.stat(file1).st_mode & mask
-    mode_file2 = os.stat(file2).st_mode & mask
+    # On Windows there is no group, so this assertion does nothing on this platform
+    if POSIX_MODE:
+        mode_file1 = os.stat(file1).st_mode & 0o070
+        mode_file2 = os.stat(file2).st_mode & 0o070
+
+        assert mode_file1 == mode_file2
+
+
+def assert_equals_world_read_permissions(file1, file2):
+    """
+    Assert that two files have the same read permissions for everyone.
+    :param file1: first file path to compare
+    :param file2: second file path to compare
+    """
+    if POSIX_MODE:
+        mode_file1 = os.stat(file1).st_mode & 0o004
+        mode_file2 = os.stat(file2).st_mode & 0o004
+    else:
+        everybody = win32security.ConvertStringSidToSid(EVERYBODY_SID)
+
+        security1 = win32security.GetFileSecurity(file1, win32security.DACL_SECURITY_INFORMATION)
+        dacl1 = security1.GetSecurityDescriptorDacl()
+
+        mode_file1 = dacl1.GetEffectiveRightsFromAcl({
+            'TrusteeForm': win32security.TRUSTEE_IS_SID,
+            'TrusteeType': win32security.TRUSTEE_IS_USER,
+            'Identifier': everybody,
+        })
+        mode_file1 = mode_file1 & ntsecuritycon.FILE_GENERIC_READ
+
+        security2 = win32security.GetFileSecurity(file2, win32security.DACL_SECURITY_INFORMATION)
+        dacl2 = security2.GetSecurityDescriptorDacl()
+
+        mode_file2 = dacl2.GetEffectiveRightsFromAcl({
+            'TrusteeForm': win32security.TRUSTEE_IS_SID,
+            'TrusteeType': win32security.TRUSTEE_IS_USER,
+            'Identifier': everybody,
+        })
+        mode_file2 = mode_file2 & ntsecuritycon.FILE_GENERIC_READ
 
     assert mode_file1 == mode_file2
 
@@ -57,20 +104,57 @@ def assert_equals_group_owner(file1, file2):
     Assert that two files have the same group owner.
     :param file1: first file path to compare
     :param file2: second file path to compare
-    :return:
     """
-    group_owner_file1 = grp.getgrgid(os.stat(file1).st_gid)[0]
-    group_owner_file2 = grp.getgrgid(os.stat(file2).st_gid)[0]
+    # On Windows there is no group, so this assertion does nothing on this platform
+    if POSIX_MODE:
+        group_owner_file1 = grp.getgrgid(os.stat(file1).st_gid)[0]
+        group_owner_file2 = grp.getgrgid(os.stat(file2).st_gid)[0]
 
-    assert group_owner_file1 == group_owner_file2
+        assert group_owner_file1 == group_owner_file2
 
 
-def assert_world_permissions(file, mode):
+def assert_world_no_permissions(file):
     """
-    Assert that a file has the expected world permission.
-    :param file: file path to check
-    :param mode: world permissions mode expected
+    Assert that the given file is not world-readable.
+    :param file: path of the file to check
     """
-    mode_file_all = os.stat(file).st_mode & 0o007
+    if POSIX_MODE:
+        mode_file_all = os.stat(file).st_mode & 0o007
+        assert mode_file_all == 0
+    else:
+        security = win32security.GetFileSecurity(file, win32security.DACL_SECURITY_INFORMATION)
+        dacl = security.GetSecurityDescriptorDacl()
+        mode = dacl.GetEffectiveRightsFromAcl({
+            'TrusteeForm': win32security.TRUSTEE_IS_SID,
+            'TrusteeType': win32security.TRUSTEE_IS_USER,
+            'Identifier': win32security.ConvertStringSidToSid(EVERYBODY_SID),
+        })
 
-    assert mode_file_all == mode
+        assert not mode
+
+
+def assert_world_read_permissions(file):
+    """
+    Assert that the given file is world-readable, but not world-writable or world-executable.
+    :param file: path of the file to check
+    """
+    if POSIX_MODE:
+        mode_file_all = os.stat(file).st_mode & 0o007
+        assert mode_file_all == 4
+    else:
+        security = win32security.GetFileSecurity(file, win32security.DACL_SECURITY_INFORMATION)
+        dacl = security.GetSecurityDescriptorDacl()
+        mode = dacl.GetEffectiveRightsFromAcl({
+            'TrusteeForm': win32security.TRUSTEE_IS_SID,
+            'TrusteeType': win32security.TRUSTEE_IS_USER,
+            'Identifier': win32security.ConvertStringSidToSid(EVERYBODY_SID),
+        })
+
+        assert not mode & ntsecuritycon.FILE_GENERIC_WRITE
+        assert not mode & ntsecuritycon.FILE_GENERIC_EXECUTE
+        assert mode & ntsecuritycon.FILE_GENERIC_READ == ntsecuritycon.FILE_GENERIC_READ
+
+
+def _get_current_user():
+    account_name = win32api.GetUserNameEx(win32api.NameSamCompatible)
+    return win32security.LookupAccountName(None, account_name)[0]

certbot-ci/certbot_integration_tests/certbot_tests/context.py

@@ -1,10 +1,11 @@
 """Module to handle the context of integration tests."""
+import logging
 import os
 import shutil
 import sys
 import tempfile
 
-from certbot_integration_tests.utils import misc, certbot_call
+from certbot_integration_tests.utils import certbot_call
 
 
 class IntegrationTestsContext(object):
@@ -19,7 +20,7 @@ class IntegrationTestsContext(object):
             self.worker_id = 'primary'
             acme_xdist = request.config.acme_xdist
 
-        self.acme_server =acme_xdist['acme_server']
+        self.acme_server = acme_xdist['acme_server']
         self.directory_url = acme_xdist['directory_url']
         self.tls_alpn_01_port = acme_xdist['https_port'][self.worker_id]
         self.http_01_port = acme_xdist['http_port'][self.worker_id]
@@ -30,7 +31,10 @@ class IntegrationTestsContext(object):
 
         self.workspace = tempfile.mkdtemp()
         self.config_dir = os.path.join(self.workspace, 'conf')
-        self.hook_probe = tempfile.mkstemp(dir=self.workspace)[1]
+
+        probe = tempfile.mkstemp(dir=self.workspace)
+        os.close(probe[0])
+        self.hook_probe = probe[1]
 
         self.manual_dns_auth_hook = (
             '{0} -c "import os; import requests; import json; '

certbot-ci/certbot_integration_tests/certbot_tests/test_main.py

@@ -11,8 +11,11 @@ from os.path import join, exists
 import pytest
 from certbot_integration_tests.certbot_tests import context as certbot_context
 from certbot_integration_tests.certbot_tests.assertions import (
-    assert_hook_execution, assert_saved_renew_hook, assert_cert_count_for_lineage,
-    assert_world_permissions, assert_equals_group_owner, assert_equals_permissions,
+    assert_hook_execution, assert_saved_renew_hook,
+    assert_cert_count_for_lineage,
+    assert_world_no_permissions, assert_world_read_permissions,
+    assert_equals_group_owner, assert_equals_group_permissions, assert_equals_world_read_permissions,
+    EVERYBODY_SID
 )
 from certbot_integration_tests.utils import misc
 
@@ -84,9 +87,9 @@ def test_http_01(context):
         context.certbot([
             '--domains', certname, '--preferred-challenges', 'http-01', 'run',
             '--cert-name', certname,
-            '--pre-hook', 'echo wtf.pre >> "{0}"'.format(context.hook_probe),
-            '--post-hook', 'echo wtf.post >> "{0}"'.format(context.hook_probe),
-            '--deploy-hook', 'echo deploy >> "{0}"'.format(context.hook_probe)
+            '--pre-hook', misc.echo('wtf_pre', context.hook_probe),
+            '--post-hook', misc.echo('wtf_post', context.hook_probe),
+            '--deploy-hook', misc.echo('deploy', context.hook_probe),
         ])
 
     assert_hook_execution(context.hook_probe, 'deploy')
@@ -104,9 +107,9 @@ def test_manual_http_auth(context):
             '--cert-name', certname,
             '--manual-auth-hook', scripts[0],
             '--manual-cleanup-hook', scripts[1],
-            '--pre-hook', 'echo wtf.pre >> "{0}"'.format(context.hook_probe),
-            '--post-hook', 'echo wtf.post >> "{0}"'.format(context.hook_probe),
-            '--renew-hook', 'echo renew >> "{0}"'.format(context.hook_probe)
+            '--pre-hook', misc.echo('wtf_pre', context.hook_probe),
+            '--post-hook', misc.echo('wtf_post', context.hook_probe),
+            '--renew-hook', misc.echo('renew', context.hook_probe),
         ])
 
     with pytest.raises(AssertionError):
@@ -122,9 +125,9 @@ def test_manual_dns_auth(context):
         'run', '--cert-name', certname,
         '--manual-auth-hook', context.manual_dns_auth_hook,
         '--manual-cleanup-hook', context.manual_dns_cleanup_hook,
-        '--pre-hook', 'echo wtf.pre >> "{0}"'.format(context.hook_probe),
-        '--post-hook', 'echo wtf.post >> "{0}"'.format(context.hook_probe),
-        '--renew-hook', 'echo renew >> "{0}"'.format(context.hook_probe)
+        '--pre-hook', misc.echo('wtf_pre', context.hook_probe),
+        '--post-hook', misc.echo('wtf_post', context.hook_probe),
+        '--renew-hook', misc.echo('renew', context.hook_probe),
     ])
 
     with pytest.raises(AssertionError):
@@ -173,21 +176,19 @@ def test_renew_files_permissions(context):
     certname = context.get_domain('renew')
     context.certbot(['-d', certname])
 
+    privkey1 = join(context.config_dir, 'archive', certname, 'privkey1.pem')
+    privkey2 = join(context.config_dir, 'archive', certname, 'privkey2.pem')
+
     assert_cert_count_for_lineage(context.config_dir, certname, 1)
-    assert_world_permissions(
-        join(context.config_dir, 'archive', certname, 'privkey1.pem'), 0)
+    assert_world_no_permissions(privkey1)
 
     context.certbot(['renew'])
 
     assert_cert_count_for_lineage(context.config_dir, certname, 2)
-    assert_world_permissions(
-        join(context.config_dir, 'archive', certname, 'privkey2.pem'), 0)
-    assert_equals_group_owner(
-        join(context.config_dir, 'archive', certname, 'privkey1.pem'),
-        join(context.config_dir, 'archive', certname, 'privkey2.pem'))
-    assert_equals_permissions(
-        join(context.config_dir, 'archive', certname, 'privkey1.pem'),
-        join(context.config_dir, 'archive', certname, 'privkey2.pem'), 0o074)
+    assert_world_no_permissions(privkey2)
+    assert_equals_group_owner(privkey1, privkey2)
+    assert_equals_world_read_permissions(privkey1, privkey2)
+    assert_equals_group_permissions(privkey1, privkey2)
 
 
 def test_renew_with_hook_scripts(context):
@@ -211,15 +212,35 @@ def test_renew_files_propagate_permissions(context):
 
     assert_cert_count_for_lineage(context.config_dir, certname, 1)
 
-    os.chmod(join(context.config_dir, 'archive', certname, 'privkey1.pem'), 0o444)
+    privkey1 = join(context.config_dir, 'archive', certname, 'privkey1.pem')
+    privkey2 = join(context.config_dir, 'archive', certname, 'privkey2.pem')
+
+    if os.name != 'nt':
+        os.chmod(privkey1, 0o444)
+    else:
+        import win32security
+        import ntsecuritycon
+        # Get the current DACL of the private key
+        security = win32security.GetFileSecurity(privkey1, win32security.DACL_SECURITY_INFORMATION)
+        dacl = security.GetSecurityDescriptorDacl()
+        # Create a read permission for Everybody group
+        everybody = win32security.ConvertStringSidToSid(EVERYBODY_SID)
+        dacl.AddAccessAllowedAce(win32security.ACL_REVISION, ntsecuritycon.FILE_GENERIC_READ, everybody)
+        # Apply the updated DACL to the private key
+        security.SetSecurityDescriptorDacl(1, dacl, 0)
+        win32security.SetFileSecurity(privkey1, win32security.DACL_SECURITY_INFORMATION, security)
+
     context.certbot(['renew'])
 
     assert_cert_count_for_lineage(context.config_dir, certname, 2)
-    assert_world_permissions(
-        join(context.config_dir, 'archive', certname, 'privkey2.pem'), 4)
-    assert_equals_permissions(
-        join(context.config_dir, 'archive', certname, 'privkey1.pem'),
-        join(context.config_dir, 'archive', certname, 'privkey2.pem'), 0o074)
+    if os.name != 'nt':
+        # On Linux, read world permissions + all group permissions will be copied from the previous private key
+        assert_world_read_permissions(privkey2)
+        assert_equals_world_read_permissions(privkey1, privkey2)
+        assert_equals_group_permissions(privkey1, privkey2)
+    else:
+        # On Windows, world will never have any permissions, and group permission is irrelevant for this platform
+        assert_world_no_permissions(privkey2)
 
 
 def test_graceful_renew_it_is_not_time(context):
@@ -229,7 +250,7 @@ def test_graceful_renew_it_is_not_time(context):
 
     assert_cert_count_for_lineage(context.config_dir, certname, 1)
 
-    context.certbot(['renew', '--deploy-hook', 'echo deploy >> "{0}"'.format(context.hook_probe)],
+    context.certbot(['renew', '--deploy-hook', misc.echo('deploy', context.hook_probe)],
                     force_renew=False)
 
     assert_cert_count_for_lineage(context.config_dir, certname, 1)
@@ -250,7 +271,7 @@ def test_graceful_renew_it_is_time(context):
     with open(join(context.config_dir, 'renewal', '{0}.conf'.format(certname)), 'w') as file:
         file.writelines(lines)
 
-    context.certbot(['renew', '--deploy-hook', 'echo deploy >> "{0}"'.format(context.hook_probe)],
+    context.certbot(['renew', '--deploy-hook', misc.echo('deploy', context.hook_probe)],
                     force_renew=False)
 
     assert_cert_count_for_lineage(context.config_dir, certname, 2)
@@ -317,9 +338,9 @@ def test_renew_hook_override(context):
     context.certbot([
         'certonly', '-d', certname,
         '--preferred-challenges', 'http-01',
-        '--pre-hook', 'echo pre >> "{0}"'.format(context.hook_probe),
-        '--post-hook', 'echo post >> "{0}"'.format(context.hook_probe),
-        '--deploy-hook', 'echo deploy >> "{0}"'.format(context.hook_probe)
+        '--pre-hook', misc.echo('pre', context.hook_probe),
+        '--post-hook', misc.echo('post', context.hook_probe),
+        '--deploy-hook', misc.echo('deploy', context.hook_probe),
     ])
 
     assert_hook_execution(context.hook_probe, 'pre')
@@ -330,14 +351,14 @@ def test_renew_hook_override(context):
     open(context.hook_probe, 'w').close()
     context.certbot([
         'renew', '--cert-name', certname,
-        '--pre-hook', 'echo pre-override >> "{0}"'.format(context.hook_probe),
-        '--post-hook', 'echo post-override >> "{0}"'.format(context.hook_probe),
-        '--deploy-hook', 'echo deploy-override >> "{0}"'.format(context.hook_probe)
+        '--pre-hook', misc.echo('pre_override', context.hook_probe),
+        '--post-hook', misc.echo('post_override', context.hook_probe),
+        '--deploy-hook', misc.echo('deploy_override', context.hook_probe),
     ])
 
-    assert_hook_execution(context.hook_probe, 'pre-override')
-    assert_hook_execution(context.hook_probe, 'post-override')
-    assert_hook_execution(context.hook_probe, 'deploy-override')
+    assert_hook_execution(context.hook_probe, 'pre_override')
+    assert_hook_execution(context.hook_probe, 'post_override')
+    assert_hook_execution(context.hook_probe, 'deploy_override')
     with pytest.raises(AssertionError):
         assert_hook_execution(context.hook_probe, 'pre')
     with pytest.raises(AssertionError):
@@ -349,11 +370,11 @@ def test_renew_hook_override(context):
     open(context.hook_probe, 'w').close()
     context.certbot(['renew', '--cert-name', certname])
 
-    assert_hook_execution(context.hook_probe, 'pre-override')
-    assert_hook_execution(context.hook_probe, 'post-override')
-    assert_hook_execution(context.hook_probe, 'deploy-override')
+    assert_hook_execution(context.hook_probe, 'pre_override')
+    assert_hook_execution(context.hook_probe, 'post_override')
+    assert_hook_execution(context.hook_probe, 'deploy_override')
+
 
-    
 def test_invalid_domain_with_dns_challenge(context):
     """Test certificate issuance failure with DNS-01 challenge."""
     # Manual dns auth hooks from misc are designed to fail if the domain contains 'fail-*'.
@@ -512,7 +533,7 @@ def test_revoke_multiple_lineages(context):
         data = file.read()
 
     data = re.sub('archive_dir = .*\n',
-                  'archive_dir = {0}\n'.format(join(context.config_dir, 'archive', cert1)),
+                  'archive_dir = {0}\n'.format(join(context.config_dir, 'archive', cert1).replace('\\', '\\\\')),
                   data)
 
     with open(join(context.config_dir, 'renewal', '{0}.conf'.format(cert2)), 'w') as file:
@@ -555,11 +576,9 @@ def test_ocsp_status_stale(context):
 
 def test_ocsp_status_live(context):
     """Test retrieval of OCSP statuses for live config"""
-    if context.acme_server == 'pebble':
-        pytest.skip('Pebble does not support OCSP status requests.')
+    cert = context.get_domain('ocsp-check')
 
     # OSCP 1: Check live certificate OCSP status (VALID)
-    cert = context.get_domain('ocsp-check')
     context.certbot(['--domains', cert])
     output = context.certbot(['certificates'])
 

certbot-ci/certbot_integration_tests/conftest.py

@@ -68,17 +68,18 @@ def _setup_primary_node(config):
     :param config: Configuration of the pytest primary node
     """
     # Check for runtime compatibility: some tools are required to be available in PATH
-    try:
-        subprocess.check_output(['docker', '-v'], stderr=subprocess.STDOUT)
-    except (subprocess.CalledProcessError, OSError):
-        raise ValueError('Error: docker is required in PATH to launch the integration tests, '
-                         'but is not installed or not available for current user.')
+    if 'boulder' in config.option.acme_server:
+        try:
+            subprocess.check_output(['docker', '-v'], stderr=subprocess.STDOUT)
+        except (subprocess.CalledProcessError, OSError):
+            raise ValueError('Error: docker is required in PATH to launch the integration tests on'
+                             'boulder, but is not installed or not available for current user.')
 
-    try:
-        subprocess.check_output(['docker-compose', '-v'], stderr=subprocess.STDOUT)
-    except (subprocess.CalledProcessError, OSError):
-        raise ValueError('Error: docker-compose is required in PATH to launch the integration tests, '
-                         'but is not installed or not available for current user.')
+        try:
+            subprocess.check_output(['docker-compose', '-v'], stderr=subprocess.STDOUT)
+        except (subprocess.CalledProcessError, OSError):
+            raise ValueError('Error: docker-compose is required in PATH to launch the integration tests, '
+                             'but is not installed or not available for current user.')
 
     # Parameter numprocesses is added to option by pytest-xdist
     workers = ['primary'] if not config.option.numprocesses\
@@ -86,7 +87,7 @@ def _setup_primary_node(config):
 
     # By calling setup_acme_server we ensure that all necessary acme server instances will be
     # fully started. This runtime is reflected by the acme_xdist returned.
-    acme_server = acme_lib.setup_acme_server(config.option.acme_server, workers)
+    acme_server = acme_lib.ACMEServer(config.option.acme_server, workers)
     config.add_cleanup(acme_server.stop)
     print('ACME xdist config:\n{0}'.format(acme_server.acme_xdist))
     acme_server.start()

certbot-ci/certbot_integration_tests/nginx_tests/nginx_config.py

@@ -21,9 +21,9 @@ def construct_nginx_config(nginx_root, nginx_webroot, http_port, https_port, oth
     :rtype: str
     """
     key_path = key_path if key_path \
-        else pkg_resources.resource_filename('certbot_integration_tests', 'assets/nginx_key.pem')
+        else pkg_resources.resource_filename('certbot_integration_tests', 'assets/key.pem')
     cert_path = cert_path if cert_path \
-        else pkg_resources.resource_filename('certbot_integration_tests', 'assets/nginx_cert.pem')
+        else pkg_resources.resource_filename('certbot_integration_tests', 'assets/cert.pem')
     return '''\
 # This error log will be written regardless of server scope error_log
 # definitions, so we have to set this here in the main scope.

certbot-ci/certbot_integration_tests/utils/acme_server.py

@@ -1,6 +1,7 @@
 #!/usr/bin/env python
 """Module to setup an ACME CA server environment able to run multiple tests in parallel"""
 from __future__ import print_function
+import errno
 import json
 import tempfile
 import time
@@ -11,184 +12,185 @@ import sys
 from os.path import join
 
 import requests
-import yaml
 
-from certbot_integration_tests.utils import misc, proxy
+from certbot_integration_tests.utils import misc, proxy, pebble_artifacts
 from certbot_integration_tests.utils.constants import *
 
 
 class ACMEServer(object):
     """
-    Handler exposing methods to start and stop the ACME server, and get its configuration
-    (eg. challenges ports). ACMEServer is also a context manager, and so can be used to
-    ensure ACME server is started/stopped upon context enter/exit.
+    ACMEServer configures and handles the lifecycle of an ACME CA server and an HTTP reverse proxy
+    instance, to allow parallel execution of integration tests against the unique http-01 port
+    expected by the ACME CA server.
+    Typically all pytest integration tests will be executed in this context.
+    ACMEServer gives access the acme_xdist parameter, listing the ports and directory url to use
+    for each pytest node. It exposes also start and stop methods in order to start the stack, and
+    stop it with proper resources cleanup.
+    ACMEServer is also a context manager, and so can be used to ensure ACME server is started/stopped
+    upon context enter/exit.
     """
-    def __init__(self, acme_xdist, start, server_cleanup):
-        self._proxy_process = None
-        self._server_cleanup = server_cleanup
-        self.acme_xdist = acme_xdist
-        self.start = start
+    def __init__(self, acme_server, nodes, http_proxy=True, stdout=False):
+        """
+        Create an ACMEServer instance.
+        :param str acme_server: the type of acme server used (boulder-v1, boulder-v2 or pebble)
+        :param list nodes: list of node names that will be setup by pytest xdist
+        :param bool http_proxy: if False do not start the HTTP proxy
+        :param bool stdout: if True stream subprocesses stdout to standard stdout
+        """
+        self._construct_acme_xdist(acme_server, nodes)
+
+        self._acme_type = 'pebble' if acme_server == 'pebble' else 'boulder'
+        self._proxy = http_proxy
+        self._workspace = tempfile.mkdtemp()
+        self._processes = []
+        self._stdout = sys.stdout if stdout else open(os.devnull, 'w')
+
+    def start(self):
+        """Start the test stack"""
+        try:
+            if self._proxy:
+                self._prepare_http_proxy()
+            if self._acme_type == 'pebble':
+                self._prepare_pebble_server()
+            if self._acme_type == 'boulder':
+                self._prepare_boulder_server()
+        except BaseException as e:
+            self.stop()
+            raise e
 
     def stop(self):
-        if self._proxy_process:
-            self._proxy_process.terminate()
-            self._proxy_process.wait()
-        self._server_cleanup()
+        """Stop the test stack, and clean its resources"""
+        print('=> Tear down the test infrastructure...')
+        try:
+            for process in self._processes:
+                try:
+                    process.terminate()
+                except OSError as e:
+                    # Process may be not started yet, so no PID and terminate fails.
+                    # Then the process never started, and the situation is acceptable.
+                    if e.errno != errno.ESRCH:
+                        raise
+            for process in self._processes:
+                process.wait()
+
+            if os.path.exists(os.path.join(self._workspace, 'boulder')):
+                # Boulder docker generates build artifacts owned by root with 0o744 permissions.
+                # If we started the acme server from a normal user that has access to the Docker
+                # daemon, this user will not be able to delete these artifacts from the host.
+                # We need to do it through a docker.
+                process = self._launch_process(['docker', 'run', '--rm', '-v',
+                                                '{0}:/workspace'.format(self._workspace),
+                                                'alpine', 'rm', '-rf', '/workspace/boulder'])
+                process.wait()
+        finally:
+            shutil.rmtree(self._workspace)
+        if self._stdout != sys.stdout:
+            self._stdout.close()
+        print('=> Test infrastructure stopped and cleaned up.')
 
     def __enter__(self):
-        self._proxy_process = self.start()
+        self.start()
         return self.acme_xdist
 
     def __exit__(self, exc_type, exc_val, exc_tb):
         self.stop()
 
+    def _construct_acme_xdist(self, acme_server, nodes):
+        """Generate and return the acme_xdist dict"""
+        acme_xdist = {'acme_server': acme_server, 'challtestsrv_port': CHALLTESTSRV_PORT}
 
-def setup_acme_server(acme_server, nodes, proxy=True):
-    """
-    This method will setup an ACME CA server and an HTTP reverse proxy instance, to allow parallel
-    execution of integration tests against the unique http-01 port expected by the ACME CA server.
-    Typically all pytest integration tests will be executed in this context.
-    An ACMEServer instance will be returned, giving access to the ports and directory url to use
-    for each pytest node, and its start and stop methods are appropriately configured to
-    respectively start the server, and stop it with proper resources cleanup.
-    :param str acme_server: the type of acme server used (boulder-v1, boulder-v2 or pebble)
-    :param str[] nodes: list of node names that will be setup by pytest xdist
-    :param bool proxy: set to False to not start the HTTP proxy
-    :return: a properly configured ACMEServer instance
-    :rtype: ACMEServer
-    """
-    acme_type = 'pebble' if acme_server == 'pebble' else 'boulder'
-    acme_xdist = _construct_acme_xdist(acme_server, nodes)
-    workspace, server_cleanup = _construct_workspace(acme_type)
+        # Directory and ACME port are set implicitly in the docker-compose.yml files of Boulder/Pebble.
+        if acme_server == 'pebble':
+            acme_xdist['directory_url'] = PEBBLE_DIRECTORY_URL
+        else:  # boulder
+            acme_xdist['directory_url'] = BOULDER_V2_DIRECTORY_URL \
+                if acme_server == 'boulder-v2' else BOULDER_V1_DIRECTORY_URL
 
-    def start():
-        proxy_process = _prepare_http_proxy(acme_xdist) if proxy else None
-        _prepare_acme_server(workspace, acme_type, acme_xdist)
+        acme_xdist['http_port'] = {node: port for (node, port)
+                                   in zip(nodes, range(5200, 5200 + len(nodes)))}
+        acme_xdist['https_port'] = {node: port for (node, port)
+                                    in zip(nodes, range(5100, 5100 + len(nodes)))}
+        acme_xdist['other_port'] = {node: port for (node, port)
+                                    in zip(nodes, range(5300, 5300 + len(nodes)))}
 
-        return proxy_process
+        self.acme_xdist = acme_xdist
 
-    return ACMEServer(acme_xdist, start, server_cleanup)
+    def _prepare_pebble_server(self):
+        """Configure and launch the Pebble server"""
+        print('=> Starting pebble instance deployment...')
+        pebble_path, challtestsrv_path, pebble_config_path = pebble_artifacts.fetch(self._workspace)
 
+        # Configure Pebble at full speed (PEBBLE_VA_NOSLEEP=1) and not randomly refusing valid
+        # nonce (PEBBLE_WFE_NONCEREJECT=0) to have a stable test environment.
+        environ = os.environ.copy()
+        environ['PEBBLE_VA_NOSLEEP'] = '1'
+        environ['PEBBLE_WFE_NONCEREJECT'] = '0'
 
-def _construct_acme_xdist(acme_server, nodes):
-    """Generate and return the acme_xdist dict"""
-    acme_xdist = {'acme_server': acme_server, 'challtestsrv_port': CHALLTESTSRV_PORT}
+        self._launch_process(
+            [pebble_path, '-config', pebble_config_path, '-dnsserver', '127.0.0.1:8053'],
+            env=environ)
 
-    # Directory and ACME port are set implicitly in the docker-compose.yml files of Boulder/Pebble.
-    if acme_server == 'pebble':
-        acme_xdist['directory_url'] = PEBBLE_DIRECTORY_URL
-    else:  # boulder
-        acme_xdist['directory_url'] = BOULDER_V2_DIRECTORY_URL \
-            if acme_server == 'boulder-v2' else BOULDER_V1_DIRECTORY_URL
+        self._launch_process(
+            [challtestsrv_path, '-management', ':{0}'.format(CHALLTESTSRV_PORT), '-defaultIPv6', '""',
+             '-defaultIPv4', '127.0.0.1', '-http01', '""', '-tlsalpn01', '""', '-https01', '""'])
 
-    acme_xdist['http_port'] = {node: port for (node, port)
-                               in zip(nodes, range(5200, 5200 + len(nodes)))}
-    acme_xdist['https_port'] = {node: port for (node, port)
-                                in zip(nodes, range(5100, 5100 + len(nodes)))}
-    acme_xdist['other_port'] = {node: port for (node, port)
-                                in zip(nodes, range(5300, 5300 + len(nodes)))}
-
-    return acme_xdist
-
-
-def _construct_workspace(acme_type):
-    """Create a temporary workspace for integration tests stack"""
-    workspace = tempfile.mkdtemp()
-
-    def cleanup():
-        """Cleanup function to call that will teardown relevant dockers and their configuration."""
-        print('=> Tear down the {0} instance...'.format(acme_type))
-        instance_path = join(workspace, acme_type)
-        try:
-            if os.path.isfile(join(instance_path, 'docker-compose.yml')):
-                _launch_command(['docker-compose', 'down'], cwd=instance_path)
-        except subprocess.CalledProcessError:
-            pass
-        print('=> Finished tear down of {0} instance.'.format(acme_type))
-
-        if acme_type == 'boulder' and os.path.exists(os.path.join(workspace, 'boulder')):
-            # Boulder docker generates build artifacts owned by root user with 0o744 permissions.
-            # If we started the acme server from a normal user that has access to the Docker
-            # daemon, this user will not be able to delete these artifacts from the host.
-            # We need to do it through a docker.
-            _launch_command(['docker', 'run', '--rm', '-v', '{0}:/workspace'.format(workspace),
-                             'alpine', 'rm', '-rf', '/workspace/boulder'])
-
-        shutil.rmtree(workspace)
-
-    return workspace, cleanup
-
-
-def _prepare_acme_server(workspace, acme_type, acme_xdist):
-    """Configure and launch the ACME server, Boulder or Pebble"""
-    print('=> Starting {0} instance deployment...'.format(acme_type))
-    instance_path = join(workspace, acme_type)
-    try:
-        # Load Boulder/Pebble from git, that includes a docker-compose.yml ready for production.
-        _launch_command(['git', 'clone', 'https://github.com/letsencrypt/{0}'.format(acme_type),
-                         '--single-branch', '--depth=1', instance_path])
-        if acme_type == 'boulder':
-            # Allow Boulder to ignore usual limit rate policies, useful for tests.
-            os.rename(join(instance_path, 'test/rate-limit-policies-b.yml'),
-                      join(instance_path, 'test/rate-limit-policies.yml'))
-        if acme_type == 'pebble':
-            with open(os.path.join(instance_path, 'docker-compose.yml'), 'r') as file_handler:
-                config = yaml.load(file_handler.read())
-
-            # Configure Pebble at full speed (PEBBLE_VA_NOSLEEP=1) and not randomly refusing valid
-            # nonce (PEBBLE_WFE_NONCEREJECT=0) to have a stable test environment.
-            config['services']['pebble'].setdefault('environment', [])\
-                .extend(['PEBBLE_VA_NOSLEEP=1', 'PEBBLE_WFE_NONCEREJECT=0'])
-
-            # Also disable strict mode for now, since Pebble v2.1.0 added specs in
-            # strict mode for which Certbot is not compliant for now.
-            # See https://github.com/certbot/certbot/pull/7175
-            # TODO: Add back -strict mode once Certbot is compliant with Pebble v2.1.0+
-            config['services']['pebble']['command'] = config['services']['pebble']['command']\
-                .replace('-strict', '')
-
-            with open(os.path.join(instance_path, 'docker-compose.yml'), 'w') as file_handler:
-                file_handler.write(yaml.dump(config))
-
-        # Launch the ACME CA server.
-        _launch_command(['docker-compose', 'up', '--force-recreate', '-d'], cwd=instance_path)
+        # pebble_ocsp_server is imported here and not at the top of module in order to avoid a useless
+        # ImportError, in the case where cryptography dependency is too old to support ocsp, but
+        # Boulder is used instead of Pebble, so pebble_ocsp_server is not used. This is the typical
+        # situation of integration-certbot-oldest tox testenv.
+        from certbot_integration_tests.utils import pebble_ocsp_server
+        self._launch_process([sys.executable, pebble_ocsp_server.__file__])
 
         # Wait for the ACME CA server to be up.
-        print('=> Waiting for {0} instance to respond...'.format(acme_type))
-        misc.check_until_timeout(acme_xdist['directory_url'])
+        print('=> Waiting for pebble instance to respond...')
+        misc.check_until_timeout(self.acme_xdist['directory_url'])
+
+        print('=> Finished pebble instance deployment.')
+
+    def _prepare_boulder_server(self):
+        """Configure and launch the Boulder server"""
+        print('=> Starting boulder instance deployment...')
+        instance_path = join(self._workspace, 'boulder')
+
+        # Load Boulder from git, that includes a docker-compose.yml ready for production.
+        process = self._launch_process(['git', 'clone', 'https://github.com/letsencrypt/boulder',
+                                        '--single-branch', '--depth=1', instance_path])
+        process.wait()
+
+        # Allow Boulder to ignore usual limit rate policies, useful for tests.
+        os.rename(join(instance_path, 'test/rate-limit-policies-b.yml'),
+                  join(instance_path, 'test/rate-limit-policies.yml'))
+
+        # Launch the Boulder server
+        self._launch_process(['docker-compose', 'up', '--force-recreate'], cwd=instance_path)
+
+        # Wait for the ACME CA server to be up.
+        print('=> Waiting for boulder instance to respond...')
+        misc.check_until_timeout(self.acme_xdist['directory_url'], attempts=240)
 
         # Configure challtestsrv to answer any A record request with ip of the docker host.
-        acme_subnet = '10.77.77' if acme_type == 'boulder' else '10.30.50'
-        response = requests.post('http://localhost:{0}/set-default-ipv4'
-                                 .format(acme_xdist['challtestsrv_port']),
-                                 json={'ip': '{0}.1'.format(acme_subnet)})
+        response = requests.post('http://localhost:{0}/set-default-ipv4'.format(CHALLTESTSRV_PORT),
+                                 json={'ip': '10.77.77.1'})
         response.raise_for_status()
 
-        print('=> Finished {0} instance deployment.'.format(acme_type))
-    except BaseException:
-        print('Error while setting up {0} instance.'.format(acme_type))
-        raise
+        print('=> Finished boulder instance deployment.')
 
+    def _prepare_http_proxy(self):
+        """Configure and launch an HTTP proxy"""
+        print('=> Configuring the HTTP proxy...')
+        mapping = {r'.+\.{0}\.wtf'.format(node): 'http://127.0.0.1:{0}'.format(port)
+                   for node, port in self.acme_xdist['http_port'].items()}
+        command = [sys.executable, proxy.__file__, str(HTTP_01_PORT), json.dumps(mapping)]
+        self._launch_process(command)
+        print('=> Finished configuring the HTTP proxy.')
 
-def _prepare_http_proxy(acme_xdist):
-    """Configure and launch an HTTP proxy"""
-    print('=> Configuring the HTTP proxy...')
-    mapping = {r'.+\.{0}\.wtf'.format(node): 'http://127.0.0.1:{0}'.format(port)
-               for node, port in acme_xdist['http_port'].items()}
-    command = [sys.executable, proxy.__file__, str(HTTP_01_PORT), json.dumps(mapping)]
-    process = subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
-    print('=> Finished configuring the HTTP proxy.')
-
-    return process
-
-
-def _launch_command(command, cwd=os.getcwd()):
-    """Launch silently an OS command, output will be displayed in case of failure"""
-    try:
-        subprocess.check_output(command, stderr=subprocess.STDOUT, cwd=cwd, universal_newlines=True)
-    except subprocess.CalledProcessError as e:
-        sys.stderr.write(e.output)
-        raise
+    def _launch_process(self, command, cwd=os.getcwd(), env=None):
+        """Launch silently an subprocess OS command"""
+        if not env:
+            env = os.environ
+        process = subprocess.Popen(command, stdout=self._stdout, stderr=subprocess.STDOUT, cwd=cwd, env=env)
+        self._processes.append(process)
+        return process
 
 
 def main():
@@ -199,8 +201,7 @@ def main():
         raise ValueError('Invalid server value {0}, should be one of {1}'
                          .format(server_type, possible_values))
 
-    acme_server = setup_acme_server(server_type, [], False)
-    process = None
+    acme_server = ACMEServer(server_type, [], http_proxy=False, stdout=True)
 
     try:
         with acme_server as acme_xdist:
@@ -208,15 +209,10 @@ def main():
                   .format(acme_xdist['directory_url']))
             print('--> Press CTRL+C to stop the ACME server.')
 
-            docker_name = 'pebble_pebble_1' if 'pebble' in server_type else 'boulder_boulder_1'
-            process = subprocess.Popen(['docker', 'logs', '-f', docker_name])
-
             while True:
                 time.sleep(3600)
     except KeyboardInterrupt:
-        if process:
-            process.terminate()
-            process.wait()
+        pass
 
 
 if __name__ == '__main__':

certbot-ci/certbot_integration_tests/utils/certbot_call.py

@@ -6,12 +6,12 @@ import subprocess
 import sys
 import os
 
-from certbot_integration_tests.utils import misc
+import certbot_integration_tests
 from certbot_integration_tests.utils.constants import *
 
 
 def certbot_test(certbot_args, directory_url, http_01_port, tls_alpn_01_port,
-                 config_dir, workspace, force_renew=True):
+                 config_dir, workspace, env=None, force_renew=True):
     """
     Invoke the certbot executable available in PATH in a test context for the given args.
     The test context consists in running certbot in debug mode, with various flags suitable
@@ -23,28 +23,70 @@ def certbot_test(certbot_args, directory_url, http_01_port, tls_alpn_01_port,
     :param int tls_alpn_01_port: port for the TLS-ALPN-01 challenges
     :param str config_dir: certbot configuration directory to use
     :param str workspace: certbot current directory to use
+    :param obj env: the environment variables to use (default: None, then current env will be used)
     :param bool force_renew: set False to not force renew existing certificates (default: True)
     :return: stdout as string
     :rtype: str
     """
+    new_environ = env if env else os.environ.copy()
     command, env = _prepare_args_env(certbot_args, directory_url, http_01_port, tls_alpn_01_port,
-                                     config_dir, workspace, force_renew)
+                                     config_dir, workspace, force_renew, new_environ)
 
     return subprocess.check_output(command, universal_newlines=True, cwd=workspace, env=env)
 
 
-def _prepare_args_env(certbot_args, directory_url, http_01_port, tls_alpn_01_port,
-                      config_dir, workspace, force_renew):
-    new_environ = os.environ.copy()
+def _prepare_environ(workspace, new_environ):
+    new_environ = new_environ.copy()
     new_environ['TMPDIR'] = workspace
 
+    # So, pytest is nice, and a little too nice for our usage.
+    # In order to help user to call seamlessly any piece of python code without requiring to
+    # install it as a full-fledged setuptools distribution for instance, it may inject the path
+    # to the test files into the PYTHONPATH. This allows the python interpreter to import
+    # as modules any python file available at this path.
+    # See https://docs.pytest.org/en/3.2.5/pythonpath.html for the explanation and description.
+    # However this behavior is not good in integration tests, in particular the nginx oldest ones.
+    # Indeed during these kind of tests certbot is installed as a transitive dependency to
+    # certbot-nginx. Here is the trick: this certbot version is not necessarily the same as
+    # the certbot codebase lying in current working directory. For instance in oldest tests
+    # certbot==0.36.0 may be installed while the codebase corresponds to certbot==0.37.0.dev0.
+    # Then during a pytest run, PYTHONPATH contains the path to the Certbot codebase, so invoking
+    # certbot will import the modules from the codebase (0.37.0.dev0), not from the
+    # required/installed version (0.36.0).
+    # This will lead to funny and totally incomprehensible errors. To avoid that, we ensure that
+    # if PYTHONPATH is set, it does not contain the path to the root of the codebase.
+    if new_environ.get('PYTHONPATH'):
+        # certbot_integration_tests.__file__ is:
+        # '/path/to/certbot/certbot-ci/certbot_integration_tests/__init__.pyc'
+        # ... and we want '/path/to/certbot'
+        certbot_root = os.path.dirname(os.path.dirname(os.path.dirname(certbot_integration_tests.__file__)))
+        python_paths = [path for path in new_environ['PYTHONPATH'].split(':') if path != certbot_root]
+        new_environ['PYTHONPATH'] = ':'.join(python_paths)
+
+    return new_environ
+
+
+def _compute_additional_args(workspace, environ, force_renew):
     additional_args = []
-    if misc.get_certbot_version() >= LooseVersion('0.30.0'):
+    output = subprocess.check_output(['certbot', '--version'],
+                                     universal_newlines=True, stderr=subprocess.STDOUT,
+                                     cwd=workspace, env=environ)
+    version_str = output.split(' ')[1].strip()  # Typical response is: output = 'certbot 0.31.0.dev0'
+    if LooseVersion(version_str) >= LooseVersion('0.30.0'):
         additional_args.append('--no-random-sleep-on-renew')
 
     if force_renew:
         additional_args.append('--renew-by-default')
 
+    return additional_args
+
+
+def _prepare_args_env(certbot_args, directory_url, http_01_port, tls_alpn_01_port,
+                      config_dir, workspace, force_renew, new_environ):
+
+    new_environ = _prepare_environ(workspace, new_environ)
+    additional_args = _compute_additional_args(workspace, new_environ, force_renew)
+
     command = [
         'certbot',
         '--server', directory_url,

certbot-ci/certbot_integration_tests/utils/constants.py

@@ -5,3 +5,5 @@ CHALLTESTSRV_PORT = 8055
 BOULDER_V1_DIRECTORY_URL = 'http://localhost:4000/directory'
 BOULDER_V2_DIRECTORY_URL = 'http://localhost:4001/directory'
 PEBBLE_DIRECTORY_URL = 'https://localhost:14000/dir'
+PEBBLE_MANAGEMENT_URL = 'https://localhost:15000'
+MOCK_OCSP_SERVER_PORT = 4002

certbot-ci/certbot_integration_tests/utils/misc.py

@@ -3,9 +3,11 @@ Misc module contains stateless functions that could be used during pytest execut
 or outside during setup/teardown of the integration tests environment.
 """
 import contextlib
+import logging
 import errno
 import multiprocessing
 import os
+import re
 import shutil
 import stat
 import subprocess
@@ -23,17 +25,17 @@ from cryptography.hazmat.primitives.asymmetric import ec
 from cryptography.hazmat.primitives.serialization import Encoding, PrivateFormat, NoEncryption
 from six.moves import socketserver, SimpleHTTPServer
 
-
 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 +45,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 +53,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):
@@ -62,6 +64,10 @@ class GracefulTCPServer(socketserver.TCPServer):
     allow_reuse_address = True
 
 
+def _run_server(port):
+    GracefulTCPServer(('', port), SimpleHTTPServer.SimpleHTTPRequestHandler).serve_forever()
+
+
 @contextlib.contextmanager
 def create_http_server(port):
     """
@@ -74,10 +80,7 @@ def create_http_server(port):
     current_cwd = os.getcwd()
     webroot = tempfile.mkdtemp()
 
-    def run():
-        GracefulTCPServer(('', port), SimpleHTTPServer.SimpleHTTPRequestHandler).serve_forever()
-
-    process = multiprocessing.Process(target=run)
+    process = multiprocessing.Process(target=_run_server, args=(port,))
 
     try:
         # SimpleHTTPServer is designed to serve files from the current working directory at the
@@ -119,15 +122,9 @@ def generate_test_file_hooks(config_dir, hook_probe):
     :param str config_dir: current certbot config directory
     :param hook_probe: path to the hook probe to test hook scripts execution
     """
-    if sys.platform == 'win32':
-        extension = 'bat'
-    else:
-        extension = 'sh'
+    hook_path = pkg_resources.resource_filename('certbot_integration_tests', 'assets/hook.py')
 
-    renewal_hooks_dirs = list_renewal_hooks_dirs(config_dir)
-    renewal_deploy_hook_path = os.path.join(renewal_hooks_dirs[1], 'hook.sh')
-
-    for hook_dir in renewal_hooks_dirs:
+    for hook_dir in list_renewal_hooks_dirs(config_dir):
         # We want an equivalent of bash `chmod -p $HOOK_DIR, that does not fail if one folder of
         # the hierarchy already exists. It is not the case of os.makedirs. Python 3 has an
         # optional parameter `exists_ok` to not fail on existing dir, but Python 2.7 does not.
@@ -137,26 +134,25 @@ def generate_test_file_hooks(config_dir, hook_probe):
         except OSError as error:
             if error.errno != errno.EEXIST:
                 raise
-        hook_path = os.path.join(hook_dir, 'hook.{0}'.format(extension))
-        if extension == 'sh':
-            data = '''\
-#!/bin/bash -xe
-if [ "$0" = "{0}" ]; then
-    if [ -z "$RENEWED_DOMAINS" -o -z "$RENEWED_LINEAGE" ]; then
-        echo "Environment variables not properly set!" >&2
-        exit 1
-    fi
-fi
-echo $(basename $(dirname "$0")) >> "{1}"\
-'''.format(renewal_deploy_hook_path, hook_probe)
-        else:
-            # TODO: Write the equivalent bat file for Windows
-            data = '''\
 
-'''
-        with open(hook_path, 'w') as file:
-            file.write(data)
-        os.chmod(hook_path, os.stat(hook_path).st_mode | stat.S_IEXEC)
+        if os.name != 'nt':
+            entrypoint_script_path = os.path.join(hook_dir, 'entrypoint.sh')
+            entrypoint_script = '''\
+#!/usr/bin/env bash
+set -e
+"{0}" "{1}" "{2}" "{3}"
+'''.format(sys.executable, hook_path, entrypoint_script_path, hook_probe)
+        else:
+            entrypoint_script_path = os.path.join(hook_dir, 'entrypoint.bat')
+            entrypoint_script = '''\
+@echo off
+"{0}" "{1}" "{2}" "{3}"
+            '''.format(sys.executable, hook_path, entrypoint_script_path, hook_probe)
+
+        with open(entrypoint_script_path, 'w') as file_h:
+            file_h.write(entrypoint_script)
+
+        os.chmod(entrypoint_script_path, os.stat(entrypoint_script_path).st_mode | stat.S_IEXEC)
 
 
 @contextlib.contextmanager
@@ -193,7 +189,7 @@ for _ in range(0, 10):
     except requests.exceptions.ConnectionError:
         pass
 raise ValueError('Error, url did not respond after 10 attempts: {{0}}'.format(url))
-'''.format(http_server_root, http_port))
+'''.format(http_server_root.replace('\\', '\\\\'), http_port))
         os.chmod(auth_script_path, 0o755)
 
         cleanup_script_path = os.path.join(tempdir, 'cleanup.py')
@@ -204,7 +200,7 @@ import os
 import shutil
 well_known = os.path.join('{0}', '.well-known')
 shutil.rmtree(well_known)
-'''.format(http_server_root))
+'''.format(http_server_root.replace('\\', '\\\\')))
         os.chmod(cleanup_script_path, 0o755)
 
         yield ('{0} {1}'.format(sys.executable, auth_script_path),
@@ -213,18 +209,6 @@ shutil.rmtree(well_known)
         shutil.rmtree(tempdir)
 
 
-def get_certbot_version():
-    """
-    Find the version of the certbot available in PATH.
-    :return str: the certbot version
-    """
-    output = subprocess.check_output(['certbot', '--version'],
-                                     universal_newlines=True, stderr=subprocess.STDOUT)
-    # Typical response is: output = 'certbot 0.31.0.dev0'
-    version_str = output.split(' ')[1].strip()
-    return LooseVersion(version_str)
-
-
 def generate_csr(domains, key_path, csr_path, key_type=RSA_KEY_TYPE):
     """
     Generate a private key, and a CSR for the given domains using this key.
@@ -287,4 +271,32 @@ def load_sample_data_path(workspace):
     original = pkg_resources.resource_filename('certbot_integration_tests', 'assets/sample-config')
     copied = os.path.join(workspace, 'sample-config')
     shutil.copytree(original, copied, symlinks=True)
+
+    if os.name == 'nt':
+        # Fix the symlinks on Windows since GIT is not creating them upon checkout
+        for lineage in ['a.encryption-example.com', 'b.encryption-example.com']:
+            current_live = os.path.join(copied, 'live', lineage)
+            for name in os.listdir(current_live):
+                if name != 'README':
+                    current_file = os.path.join(current_live, name)
+                    with open(current_file) as file_h:
+                        src = file_h.read()
+                    os.unlink(current_file)
+                    os.symlink(os.path.join(current_live, src), current_file)
+
     return copied
+
+
+def echo(keyword, path=None):
+    """
+    Generate a platform independent executable command
+    that echoes the given keyword into the given file.
+    :param keyword: the keyword to echo (must be a single keyword)
+    :param path: path to the file were keyword is echoed
+    :return: the executable command
+    """
+    if not re.match(r'^\w+$', keyword):
+        raise ValueError('Error, keyword `{0}` is not a single keyword.'
+                         .format(keyword))
+    return '{0} -c "from __future__ import print_function; print(\'{1}\')"{2}'.format(
+        os.path.basename(sys.executable), keyword, ' >> "{0}"'.format(path) if path else '')

certbot-ci/certbot_integration_tests/utils/pebble_artifacts.py

@@ -0,0 +1,53 @@
+import json
+import os
+import stat
+
+import pkg_resources
+import requests
+
+from certbot_integration_tests.utils.constants import MOCK_OCSP_SERVER_PORT
+
+PEBBLE_VERSION = 'v2.2.1'
+ASSETS_PATH = pkg_resources.resource_filename('certbot_integration_tests', 'assets')
+
+
+def fetch(workspace):
+    suffix = 'linux-amd64' if os.name != 'nt' else 'windows-amd64.exe'
+
+    pebble_path = _fetch_asset('pebble', suffix)
+    challtestsrv_path = _fetch_asset('pebble-challtestsrv', suffix)
+    pebble_config_path = _build_pebble_config(workspace)
+
+    return pebble_path, challtestsrv_path, pebble_config_path
+
+
+def _fetch_asset(asset, suffix):
+    asset_path = os.path.join(ASSETS_PATH, '{0}_{1}_{2}'.format(asset, PEBBLE_VERSION, suffix))
+    if not os.path.exists(asset_path):
+        asset_url = ('https://github.com/letsencrypt/pebble/releases/download/{0}/{1}_{2}'
+                     .format(PEBBLE_VERSION, asset, suffix))
+        response = requests.get(asset_url)
+        response.raise_for_status()
+        with open(asset_path, 'wb') as file_h:
+            file_h.write(response.content)
+    os.chmod(asset_path, os.stat(asset_path).st_mode | stat.S_IEXEC)
+
+    return asset_path
+
+
+def _build_pebble_config(workspace):
+    config_path = os.path.join(workspace, 'pebble-config.json')
+    with open(config_path, 'w') as file_h:
+        file_h.write(json.dumps({
+            'pebble': {
+                'listenAddress': '0.0.0.0:14000',
+                'managementListenAddress': '0.0.0.0:15000',
+                'certificate': os.path.join(ASSETS_PATH, 'cert.pem'),
+                'privateKey': os.path.join(ASSETS_PATH, 'key.pem'),
+                'httpPort': 5002,
+                'tlsPort': 5001,
+                'ocspResponderURL': 'http://127.0.0.1:{0}'.format(MOCK_OCSP_SERVER_PORT),
+            },
+        }))
+
+    return config_path

certbot-ci/certbot_integration_tests/utils/pebble_ocsp_server.py

@@ -0,0 +1,71 @@
+#!/usr/bin/env python
+"""
+This runnable module interfaces itself with the Pebble management interface in order
+to serve a mock OCSP responder during integration tests against Pebble.
+"""
+import datetime
+import re
+
+import requests
+from dateutil import parser
+
+from cryptography.hazmat.backends import default_backend
+from cryptography.hazmat.primitives import serialization, hashes
+from cryptography import x509
+from cryptography.x509 import ocsp
+from six.moves import BaseHTTPServer
+
+from certbot_integration_tests.utils.misc import GracefulTCPServer
+from certbot_integration_tests.utils.constants import MOCK_OCSP_SERVER_PORT, PEBBLE_MANAGEMENT_URL
+
+
+class _ProxyHandler(BaseHTTPServer.BaseHTTPRequestHandler):
+    def do_POST(self):
+        request = requests.get(PEBBLE_MANAGEMENT_URL + '/intermediate-keys/0', verify=False)
+        issuer_key = serialization.load_pem_private_key(request.content, None, default_backend())
+
+        request = requests.get(PEBBLE_MANAGEMENT_URL + '/intermediates/0', verify=False)
+        issuer_cert = x509.load_pem_x509_certificate(request.content, default_backend())
+
+        try:
+            content_len = int(self.headers.getheader('content-length', 0))
+        except AttributeError:
+            content_len = int(self.headers.get('Content-Length'))
+
+        ocsp_request = ocsp.load_der_ocsp_request(self.rfile.read(content_len))
+        response = requests.get('{0}/cert-status-by-serial/{1}'.format(
+            PEBBLE_MANAGEMENT_URL, str(hex(ocsp_request.serial_number)).replace('0x', '')), verify=False)
+
+        if not response.ok:
+            ocsp_response = ocsp.OCSPResponseBuilder.build_unsuccessful(ocsp.OCSPResponseStatus.UNAUTHORIZED)
+        else:
+            data = response.json()
+
+            now = datetime.datetime.utcnow()
+            cert = x509.load_pem_x509_certificate(data['Certificate'].encode(), default_backend())
+            if data['Status'] != 'Revoked':
+                ocsp_status, revocation_time, revocation_reason = ocsp.OCSPCertStatus.GOOD, None, None
+            else:
+                ocsp_status, revocation_reason = ocsp.OCSPCertStatus.REVOKED, x509.ReasonFlags.unspecified
+                revoked_at = re.sub(r'( \+\d{4}).*$', r'\1', data['RevokedAt'])  # "... +0000 UTC" => "+0000"
+                revocation_time = parser.parse(revoked_at)
+
+            ocsp_response = ocsp.OCSPResponseBuilder().add_response(
+                cert=cert, issuer=issuer_cert, algorithm=hashes.SHA1(),
+                cert_status=ocsp_status,
+                this_update=now, next_update=now + datetime.timedelta(hours=1),
+                revocation_time=revocation_time, revocation_reason=revocation_reason
+            ).responder_id(
+                ocsp.OCSPResponderEncoding.NAME, issuer_cert
+            ).sign(issuer_key, hashes.SHA256())
+
+        self.send_response(200)
+        self.end_headers()
+        self.wfile.write(ocsp_response.public_bytes(serialization.Encoding.DER))
+
+
+if __name__ == '__main__':
+    try:
+        GracefulTCPServer(('', MOCK_OCSP_SERVER_PORT), _ProxyHandler).serve_forever()
+    except KeyboardInterrupt:
+        pass

certbot-ci/setup.py

@@ -1,5 +1,7 @@
-from setuptools import setup
-from setuptools import find_packages
+import sys
+
+from distutils.version import StrictVersion
+from setuptools import setup, find_packages, __version__ as setuptools_version
 
 
 version = '0.32.0.dev0'
@@ -11,11 +13,23 @@ install_requires = [
     'pytest',
     'pytest-cov',
     'pytest-xdist',
+    'python-dateutil',
     'pyyaml',
     'requests',
     'six',
 ]
 
+# Add pywin32 on Windows platforms to handle low-level system calls.
+# This dependency needs to be added using environment markers to avoid its installation on Linux.
+# However environment markers are supported only with setuptools >= 36.2.
+# So this dependency is not added for old Linux distributions with old setuptools,
+# in order to allow these systems to build certbot from sources.
+if StrictVersion(setuptools_version) >= StrictVersion('36.2'):
+    install_requires.append("pywin32>=224 ; sys_platform == 'win32'")
+elif 'bdist_wheel' in sys.argv[1:]:
+    raise RuntimeError('Error, you are trying to build certbot wheels using an old version '
+                       'of setuptools. Version 36.2+ of setuptools is required.')
+
 setup(
     name='certbot-ci',
     version=version,

certbot-compatibility-test/setup.py

@@ -4,7 +4,7 @@ from setuptools import setup
 from setuptools import find_packages
 
 
-version = '0.36.0.dev0'
+version = '0.38.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.dev0'
+version = '0.38.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.dev0'
+version = '0.38.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.dev0'
+version = '0.38.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.dev0'
+version = '0.38.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.dev0'
+version = '0.38.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.dev0'
+version = '0.38.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.dev0'
+version = '0.38.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.dev0'
+version = '0.38.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.dev0'
+version = '0.38.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.dev0'
+version = '0.38.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.dev0'
+version = '0.38.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.dev0'
+version = '0.38.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.dev0'
+version = '0.38.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.dev0'
+version = '0.38.0.dev0'
 
 # Please update tox.ini when modifying dependency version requirements
 install_requires = [

certbot-nginx/MANIFEST.in

@@ -2,5 +2,4 @@ include LICENSE.txt
 include README.rst
 recursive-include docs *
 recursive-include certbot_nginx/tests/testdata *
-include certbot_nginx/options-ssl-nginx.conf
-include certbot_nginx/options-ssl-nginx-old.conf
+recursive-include certbot_nginx/tls_configs *.conf

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
 
@@ -128,7 +127,10 @@ class NginxConfigurator(common.Installer):
         config_filename = "options-ssl-nginx.conf"
         if self.version < (1, 5, 9):
             config_filename = "options-ssl-nginx-old.conf"
-        return pkg_resources.resource_filename("certbot_nginx", config_filename)
+        elif self.version < (1, 13, 0):
+            config_filename = "options-ssl-nginx-tls12-only.conf"
+        return pkg_resources.resource_filename(
+            "certbot_nginx", os.path.join("tls_configs", config_filename))
 
     @property
     def mod_ssl_conf(self):
@@ -903,13 +905,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/certbot_nginx/constants.py

@@ -23,10 +23,19 @@ UPDATED_MOD_SSL_CONF_DIGEST = ".updated-options-ssl-nginx-conf-digest.txt"
 """Name of the hash of the updated or informed mod_ssl_conf as saved in `IConfig.config_dir`."""
 
 SSL_OPTIONS_HASHES_NEW = [
+    '108c4555058a087496a3893aea5d9e1cee0f20a3085d44a52dc1a66522299ac3',
+    'd5e021706ecdccc7090111b0ae9a29ef61523e927f020e410caf0a1fd7063981',
+]
+"""SHA256 hashes of the contents of versions of MOD_SSL_CONF_SRC for nginx >= 1.13.0"""
+
+SSL_OPTIONS_HASHES_MEDIUM = [
     '63e2bddebb174a05c9d8a7cf2adf72f7af04349ba59a1a925fe447f73b2f1abf',
     '2901debc7ecbc10917edd9084c05464c9c5930b463677571eaf8c94bffd11ae2',
+    '30baca73ed9a5b0e9a69ea40e30482241d8b1a7343aa79b49dc5d7db0bf53b6c',
+    '02329eb19930af73c54b3632b3165d84571383b8c8c73361df940cb3894dd426',
 ]
-"""SHA256 hashes of the contents of versions of MOD_SSL_CONF_SRC for nginx >= 1.5.9"""
+"""SHA256 hashes of the contents of versions of MOD_SSL_CONF_SRC for nginx >= 1.5.9
+   and nginx < 1.13.0"""
 
 ALL_SSL_OPTIONS_HASHES = [
     '0f81093a1465e3d4eaa8b0c14e77b2a2e93568b0fc1351c2b87893a95f0de87c',
@@ -36,7 +45,8 @@ ALL_SSL_OPTIONS_HASHES = [
     '394732f2bbe3e5e637c3fb5c6e980a1f1b90b01e2e8d6b7cff41dde16e2a756d',
     '4b16fec2bcbcd8a2f3296d886f17f9953ffdcc0af54582452ca1e52f5f776f16',
     'c052ffff0ad683f43bffe105f7c606b339536163490930e2632a335c8d191cc4',
-] + SSL_OPTIONS_HASHES_NEW
+    '02329eb19930af73c54b3632b3165d84571383b8c8c73361df940cb3894dd426',
+] + SSL_OPTIONS_HASHES_MEDIUM + SSL_OPTIONS_HASHES_NEW
 """SHA256 hashes of the contents of all versions of MOD_SSL_CONF_SRC"""
 
 def os_constant(key):

certbot-nginx/certbot_nginx/options-ssl-nginx-old.conf

@@ -1,13 +0,0 @@
-# This file contains important security parameters. If you modify this file
-# manually, Certbot will be unable to automatically provide future security
-# updates. Instead, Certbot will print and log an error message with a path to
-# the up-to-date file that you will need to refer to when manually updating
-# this file.
-
-ssl_session_cache shared:le_nginx_SSL:10m;
-ssl_session_timeout 1440m;
-
-ssl_protocols TLSv1 TLSv1.1 TLSv1.2;
-ssl_prefer_server_ciphers on;
-
-ssl_ciphers "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";

certbot-nginx/certbot_nginx/options-ssl-nginx.conf

@@ -1,14 +0,0 @@
-# This file contains important security parameters. If you modify this file
-# manually, Certbot will be unable to automatically provide future security
-# updates. Instead, Certbot will print and log an error message with a path to
-# the up-to-date file that you will need to refer to when manually updating
-# this file.
-
-ssl_session_cache shared:le_nginx_SSL:10m;
-ssl_session_timeout 1440m;
-ssl_session_tickets off;
-
-ssl_protocols TLSv1 TLSv1.1 TLSv1.2;
-ssl_prefer_server_ciphers on;
-
-ssl_ciphers "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";

certbot-nginx/certbot_nginx/tests/configurator_test.py

@@ -427,11 +427,6 @@ class NginxConfiguratorTest(util.NginxTest):
         mock_recovery_routine.side_effect = errors.ReverterError("foo")
         self.assertRaises(errors.PluginError, self.config.recovery_routine)
 
-    @mock.patch("certbot.reverter.Reverter.view_config_changes")
-    def test_view_config_changes_throws_error_from_reverter(self, mock_view_config_changes):
-        mock_view_config_changes.side_effect = errors.ReverterError("foo")
-        self.assertRaises(errors.PluginError, self.config.view_config_changes)
-
     @mock.patch("certbot.reverter.Reverter.rollback_checkpoints")
     def test_rollback_checkpoints_throws_error_from_reverter(self, mock_rollback_checkpoints):
         mock_rollback_checkpoints.side_effect = errors.ReverterError("foo")
@@ -968,13 +963,40 @@ class InstallSslOptionsConfTest(util.NginxTest):
             "Constants.ALL_SSL_OPTIONS_HASHES must be appended"
             " with the sha256 hash of self.config.mod_ssl_conf when it is updated.")
 
-    def test_old_nginx_version_uses_old_config(self):
+    def test_ssl_config_files_hash_in_all_hashes(self):
+        """
+        It is really critical that all TLS Nginx 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_nginx.constants import ALL_SSL_OPTIONS_HASHES
+        import pkg_resources
+        all_files = [
+            pkg_resources.resource_filename("certbot_nginx", os.path.join("tls_configs", x))
+            for x in ("options-ssl-nginx.conf",
+                      "options-ssl-nginx-old.conf",
+                      "options-ssl-nginx-tls12-only.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))
+
+    def test_nginx_version_uses_correct_config(self):
         self.config.version = (1, 5, 8)
         self.assertEqual(os.path.basename(self.config.mod_ssl_conf_src),
                          "options-ssl-nginx-old.conf")
         self._call()
         self._assert_current_file()
         self.config.version = (1, 5, 9)
+        self.assertEqual(os.path.basename(self.config.mod_ssl_conf_src),
+                         "options-ssl-nginx-tls12-only.conf")
+        self._call()
+        self._assert_current_file()
+        self.config.version = (1, 13, 0)
         self.assertEqual(os.path.basename(self.config.mod_ssl_conf_src),
                          "options-ssl-nginx.conf")
 

certbot-nginx/certbot_nginx/tests/parser_test.py

@@ -30,8 +30,16 @@ class NginxParserTest(util.NginxTest): #pylint: disable=too-many-public-methods
         self.assertEqual(nparser.root, self.config_path)
 
     def test_root_absolute(self):
-        nparser = parser.NginxParser(os.path.relpath(self.config_path))
-        self.assertEqual(nparser.root, self.config_path)
+        curr_dir = os.getcwd()
+        try:
+            # On Windows current directory may be on a different drive than self.tempdir.
+            # However a relative path between two different drives is invalid. So we move to
+            # self.tempdir to ensure that we stay on the same drive.
+            os.chdir(self.temp_dir)
+            nparser = parser.NginxParser(os.path.relpath(self.config_path))
+            self.assertEqual(nparser.root, self.config_path)
+        finally:
+            os.chdir(curr_dir)
 
     def test_root_no_trailing_slash(self):
         nparser = parser.NginxParser(self.config_path + os.path.sep)

certbot-nginx/certbot_nginx/tests/util.py

@@ -3,7 +3,6 @@ import copy
 import shutil
 import tempfile
 import unittest
-import warnings
 
 import josepy as jose
 import mock
@@ -11,6 +10,7 @@ import pkg_resources
 import zope.component
 
 from certbot import configuration
+from certbot import util
 from certbot.compat import os
 from certbot.plugins import common
 from certbot.tests import util as test_util
@@ -34,20 +34,16 @@ class NginxTest(unittest.TestCase):  # pylint: disable=too-few-public-methods
             "rsa512_key.pem"))
 
     def tearDown(self):
-        # On Windows we have various files which are not correctly closed at the time of tearDown.
-        # For know, we log them until a proper file close handling is written.
-        # Useful for development only, so no warning when we are on a CI process.
-        def onerror_handler(_, path, excinfo):
-            """On error handler"""
-            if not os.environ.get('APPVEYOR'):  # pragma: no cover
-                message = ('Following error occurred when deleting path {0}'
-                           'during tearDown process: {1}'.format(path, str(excinfo)))
-                warnings.warn(message)
+        # Cleanup opened resources after a test. This is usually done through atexit handlers in
+        # Certbot, but during tests, atexit will not run registered functions before tearDown is
+        # called and instead will run them right before the entire test process exits.
+        # It is a problem on Windows, that does not accept to clean resources before closing them.
+        util._release_locks()  # pylint: disable=protected-access
 
-        shutil.rmtree(self.temp_dir, onerror=onerror_handler)
-        shutil.rmtree(self.config_dir, onerror=onerror_handler)
-        shutil.rmtree(self.work_dir, onerror=onerror_handler)
-        shutil.rmtree(self.logs_dir, onerror=onerror_handler)
+        shutil.rmtree(self.temp_dir)
+        shutil.rmtree(self.config_dir)
+        shutil.rmtree(self.work_dir)
+        shutil.rmtree(self.logs_dir)
 
 
 def get_data_filename(filename):

certbot-nginx/certbot_nginx/tls_configs/options-ssl-nginx-old.conf

@@ -0,0 +1,13 @@
+# 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.
+
+ssl_session_cache shared:le_nginx_SSL:10m;
+ssl_session_timeout 1440m;
+
+ssl_protocols TLSv1.2;
+ssl_prefer_server_ciphers off;
+
+ssl_ciphers "ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384";

certbot-nginx/certbot_nginx/tls_configs/options-ssl-nginx-tls12-only.conf

@@ -0,0 +1,13 @@
+# 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.
+
+ssl_session_cache shared:le_nginx_SSL:10m;
+ssl_session_timeout 1440m;
+
+ssl_protocols TLSv1.2;
+ssl_prefer_server_ciphers off;
+
+ssl_ciphers "ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384";

certbot-nginx/certbot_nginx/tls_configs/options-ssl-nginx.conf

@@ -0,0 +1,13 @@
+# 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.
+
+ssl_session_cache shared:le_nginx_SSL:10m;
+ssl_session_timeout 1440m;
+
+ssl_protocols TLSv1.2 TLSv1.3;
+ssl_prefer_server_ciphers off;
+
+ssl_ciphers "ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384";

certbot-nginx/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.34.0
+certbot[dev]==0.36.0

certbot-nginx/setup.py

@@ -4,13 +4,13 @@ from setuptools.command.test import test as TestCommand
 import sys
 
 
-version = '0.36.0.dev0'
+version = '0.38.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.
 install_requires = [
     'acme>=0.29.0',
-    'certbot>=0.34.0',
+    'certbot>=0.35.0',
     'mock',
     'PyOpenSSL',
     'pyparsing>=1.5.5',  # Python3 support; perhaps unnecessary?

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.dev0'
+__version__ = '0.38.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/cli.py

@@ -1418,10 +1418,10 @@ def _plugins_parsing(helpful, plugins):
                 help="Authenticator plugin name.")
     helpful.add("plugins", "-i", "--installer", default=flag_default("installer"),
                 help="Installer plugin name (also used to find domains).")
-    helpful.add(["plugins", "certonly", "run", "install", "config_changes"],
+    helpful.add(["plugins", "certonly", "run", "install"],
                 "--apache", action="store_true", default=flag_default("apache"),
                 help="Obtain and install certificates using Apache")
-    helpful.add(["plugins", "certonly", "run", "install", "config_changes"],
+    helpful.add(["plugins", "certonly", "run", "install"],
                 "--nginx", action="store_true", default=flag_default("nginx"),
                 help="Obtain and install certificates using Nginx")
     helpful.add(["plugins", "certonly"], "--standalone", action="store_true",

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)
@@ -627,7 +624,7 @@ class Client(object):
             reporter.add_message(
                 "An error occurred and we failed to restore your config and "
                 "restart your server. Please post to "
-                "https://community.letsencrypt.org/c/server-config "
+                "https://community.letsencrypt.org/c/help "
                 "with details about your configuration and this error you received.",
                 reporter.HIGH_PRIORITY)
             raise

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

@@ -1,12 +1,20 @@
 """Compat module to handle files security on Windows and Linux"""
 from __future__ import absolute_import
 
+import errno
 import os  # pylint: disable=os-module-forbidden
 import stat
 
 try:
-    import ntsecuritycon  # pylint: disable=import-error
-    import win32security  # pylint: disable=import-error
+    # pylint: disable=import-error
+    import ntsecuritycon
+    import win32security
+    import win32con
+    import win32api
+    import win32file
+    import pywintypes
+    import winerror
+    # pylint: enable=import-error
 except ImportError:
     POSIX_MODE = True
 else:
@@ -36,6 +44,209 @@ def chmod(file_path, mode):
         _apply_win_mode(file_path, mode)
 
 
+# One could ask why there is no copy_ownership() function, or even a reimplementation
+# of os.chown() that would modify the ownership of file without touching the mode itself.
+# This is because on Windows, it would require recalculating the existing DACL against
+# the new owner, since the DACL is composed of ACEs that targets a specific user, not dynamically
+# the current owner of a file. This action would be necessary to keep consistency between
+# the POSIX mode applied to the file and the current owner of this file.
+# Since copying and editing arbitrary DACL is very difficult, and since we actually know
+# the mode to apply at the time the owner of a file should change, it is easier to just
+# change the owner, then reapply the known mode, as copy_ownership_and_apply_mode() does.
+def copy_ownership_and_apply_mode(src, dst, mode, copy_user, copy_group):
+    # type: (str, str, int, bool, bool) -> None
+    """
+    Copy ownership (user and optionally group on Linux) from the source to the
+    destination, then apply given mode in compatible way for Linux and Windows.
+    This replaces the os.chown command.
+    :param str src: Path of the source file
+    :param str dst: Path of the destination file
+    :param int mode: Permission mode to apply on the destination file
+    :param bool copy_user: Copy user if `True`
+    :param bool copy_group: Copy group if `True` on Linux (has no effect on Windows)
+    """
+    if POSIX_MODE:
+        stats = os.stat(src)
+        user_id = stats.st_uid if copy_user else -1
+        group_id = stats.st_gid if copy_group else -1
+        os.chown(dst, user_id, group_id)
+    elif copy_user:
+        # There is no group handling in Windows
+        _copy_win_ownership(src, dst)
+
+    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
+    """
+    Wrapper of original os.open function, that will ensure on Windows that given mode
+    is correctly applied.
+    :param str file_path: The file path to open
+    :param int flags: Flags to apply on file while opened
+    :param int mode: POSIX mode to apply on file when opened,
+        Python defaults will be applied if ``None``
+    :returns: the file descriptor to the opened file
+    :rtype: int
+    :raise: OSError(errno.EEXIST) if the file already exists and os.O_CREAT & os.O_EXCL are set,
+            OSError(errno.EACCES) on Windows if the file already exists and is a directory, and
+                os.O_CREAT is set.
+    """
+    if POSIX_MODE:
+        # On Linux, invoke os.open directly.
+        return os.open(file_path, flags, mode)
+
+    # Windows: handle creation of the file atomically with proper permissions.
+    if flags & os.O_CREAT:
+        # If os.O_EXCL is set, we will use the "CREATE_NEW", that will raise an exception if
+        # file exists, matching the API contract of this bit flag. Otherwise, we use
+        # "CREATE_ALWAYS" that will always create the file whether it exists or not.
+        disposition = win32con.CREATE_NEW if flags & os.O_EXCL else win32con.CREATE_ALWAYS
+
+        attributes = win32security.SECURITY_ATTRIBUTES()
+        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
+        # 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-setsecuritydescriptordacl  # pylint: disable=line-too-long
+        security.SetSecurityDescriptorDacl(1, dacl, 0)
+
+        handle = None
+        try:
+            handle = win32file.CreateFile(file_path, win32file.GENERIC_READ,
+                                          win32file.FILE_SHARE_READ & win32file.FILE_SHARE_WRITE,
+                                          attributes, disposition, 0, None)
+        except pywintypes.error as err:
+            # Handle native windows errors into python errors to be consistent with the API
+            # of os.open in the situation of a file already existing or locked.
+            if err.winerror == winerror.ERROR_FILE_EXISTS:
+                raise OSError(errno.EEXIST, err.strerror)
+            if err.winerror == winerror.ERROR_SHARING_VIOLATION:
+                raise OSError(errno.EACCES, err.strerror)
+            raise err
+        finally:
+            if handle:
+                handle.Close()
+
+        # At this point, the file that did not exist has been created with proper permissions,
+        # so os.O_CREAT and os.O_EXCL are not needed anymore. We remove them from the flags to
+        # avoid a FileExists exception before calling os.open.
+        return os.open(file_path, flags ^ os.O_CREAT ^ os.O_EXCL)
+
+    # Windows: general case, we call os.open, let exceptions be thrown, then chmod if all is fine.
+    handle = os.open(file_path, flags)
+    chmod(file_path, mode)
+    return handle
+
+
+def makedirs(file_path, mode=0o777):
+    # type: (str, int) -> None
+    """
+    Rewrite of original os.makedirs function, that will ensure on Windows that given mode
+    is correctly applied.
+    :param str file_path: The file path to open
+    :param int mode: POSIX mode to apply on leaf directory when created, Python defaults
+                     will be applied if ``None``
+    """
+    if POSIX_MODE:
+        return os.makedirs(file_path, mode)
+
+    orig_mkdir_fn = os.mkdir
+    try:
+        # As we know that os.mkdir is called internally by os.makedirs, we will swap the function in
+        # os module for the time of makedirs execution on Windows.
+        os.mkdir = mkdir  # type: ignore
+        return os.makedirs(file_path, mode)
+    finally:
+        os.mkdir = orig_mkdir_fn
+
+
+def mkdir(file_path, mode=0o777):
+    # type: (str, int) -> None
+    """
+    Rewrite of original os.mkdir function, that will ensure on Windows that given mode
+    is correctly applied.
+    :param str file_path: The file path to open
+    :param int mode: POSIX mode to apply on directory when created, Python defaults
+                     will be applied if ``None``
+    """
+    if POSIX_MODE:
+        return os.mkdir(file_path, mode)
+
+    attributes = win32security.SECURITY_ATTRIBUTES()
+    security = attributes.SECURITY_DESCRIPTOR
+    user = _get_current_user()
+    dacl = _generate_dacl(user, mode)
+    security.SetSecurityDescriptorOwner(user, False)
+    security.SetSecurityDescriptorDacl(1, dacl, 0)
+
+    try:
+        win32file.CreateDirectory(file_path, attributes)
+    except pywintypes.error as err:
+        # Handle native windows error into python error to be consistent with the API
+        # of os.mkdir in the situation of a directory already existing.
+        if err.winerror == winerror.ERROR_ALREADY_EXISTS:
+            raise OSError(errno.EEXIST, err.strerror, file_path, err.winerror)
+        raise err
+
+    return None
+
+
 def replace(src, dst):
     # type: (str, str) -> None
     """
@@ -52,13 +263,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
@@ -68,6 +288,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()
@@ -129,6 +396,18 @@ def _analyze_mode(mode):
     }
 
 
+def _copy_win_ownership(src, dst):
+    security_src = win32security.GetFileSecurity(src, win32security.OWNER_SECURITY_INFORMATION)
+    user_src = security_src.GetSecurityDescriptorOwner()
+
+    security_dst = win32security.GetFileSecurity(dst, win32security.OWNER_SECURITY_INFORMATION)
+    # Second parameter indicates, if `False`, that the owner of the file is not provided by some
+    # default mechanism, but is explicitly set instead. This is obviously what we are doing here.
+    security_dst.SetSecurityDescriptorOwner(user_src, False)
+
+    win32security.SetFileSecurity(dst, win32security.OWNER_SECURITY_INFORMATION, security_dst)
+
+
 def _generate_windows_flags(rights_desc):
     # Some notes about how each POSIX right is interpreted.
     #
@@ -166,6 +445,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.
@@ -173,3 +474,17 @@ def _compare_dacls(dacl1, dacl2):
     """
     return ([dacl1.GetAce(index) for index in range(0, dacl1.GetAceCount())] ==
             [dacl2.GetAce(index) for index in range(0, dacl2.GetAceCount())])
+
+
+def _get_current_user():
+    """
+    Return the pySID corresponding to the current user.
+    """
+    account_name = win32api.GetUserNameEx(win32api.NameSamCompatible)
+    # LookupAccountName() expects the system name as first parameter. By passing None to it,
+    # we instruct Windows to first search the matching account in the machine local accounts,
+    # then into the primary domain accounts, if the machine has joined a domain, then finally
+    # into the trusted domains accounts. This is the preferred lookup mechanism to use in Windows
+    # if there is no reason to use a specific lookup mechanism.
+    # See https://docs.microsoft.com/en-us/windows/desktop/api/winbase/nf-winbase-lookupaccountnamea
+    return win32security.LookupAccountName(None, account_name)[0]

certbot/compat/misc.py

@@ -17,6 +17,23 @@ except ImportError:  # pragma: no cover
 from certbot import errors
 from certbot.compat import os
 
+
+# MASK_FOR_PRIVATE_KEY_PERMISSIONS defines what are the permissions flags to keep
+# when transferring the permissions from an old private key to a new one.
+if POSIX_MODE:
+    # On Linux, we keep read/write/execute permissions
+    # for group and read permissions for everybody.
+    MASK_FOR_PRIVATE_KEY_PERMISSIONS = stat.S_IRGRP | stat.S_IWGRP | stat.S_IXGRP | stat.S_IROTH
+else:
+    # On Windows, the mode returned by os.stat is not reliable,
+    # so we do not keep any permission from the previous private key.
+    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
     """
@@ -29,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
     """
@@ -75,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

@@ -17,6 +17,7 @@ from os import *  # type: ignore  # pylint: disable=wildcard-import,unused-wildc
 # and so not in `from os import *`.
 import os as std_os  # pylint: disable=os-module-forbidden
 import sys as std_sys
+
 ourselves = std_sys.modules[__name__]
 for attribute in dir(std_os):
     # Check if the attribute does not already exist in our module. It could be internal attributes
@@ -25,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.
@@ -45,12 +48,51 @@ del ourselves, std_os, std_sys
 # Basically, it states that appropriate permissions will be set for the owner, nothing for the
 # group, appropriate permissions for the "Everyone" group, and all permissions to the
 # "Administrators" group + "System" user, as they can do everything anyway.
-def chmod(*unused_args, **unused_kwargs):  # pylint: disable=function-redefined
+def chmod(*unused_args, **unused_kwargs):
     """Method os.chmod() is forbidden"""
     raise RuntimeError('Usage of os.chmod() is forbidden. '
                        'Use certbot.compat.filesystem.chmod() instead.')
 
 
+# Because uid is not a concept on Windows, chown is useless. In fact, it is not even available
+# on Python for Windows. So to be consistent on both platforms for Certbot, this method is
+# always forbidden.
+def chown(*unused_args, **unused_kwargs):
+    """Method os.chown() is forbidden"""
+    raise RuntimeError('Usage of os.chown() is forbidden.'
+                       'Use certbot.compat.filesystem.copy_ownership_and_apply_mode() instead.')
+
+
+# The os.open function on Windows has the same effect as a call to os.chown concerning the file
+# modes: these modes lack the correct control over the permissions given to the file. Instead,
+# filesystem.open invokes the Windows native API `CreateFile` to ensure that permissions are
+# atomically set in case of file creation, or invokes filesystem.chmod to properly set the
+# permissions for the other cases.
+def open(*unused_args, **unused_kwargs):
+    """Method os.open() is forbidden"""
+    raise RuntimeError('Usage of os.open() is forbidden. '
+                       'Use certbot.compat.filesystem.open() instead.')
+
+
+# Very similarly to os.open, os.mkdir has the same effects on Windows and creates an unsecured
+# folder. So a similar mitigation to security.chmod is provided on this platform.
+def mkdir(*unused_args, **unused_kwargs):
+    """Method os.mkdir() is forbidden"""
+    raise RuntimeError('Usage of os.mkdir() is forbidden. '
+                       'Use certbot.compat.filesystem.mkdir() instead.')
+
+
+# As said above, os.makedirs would call the original os.mkdir function recursively on Windows,
+# creating the same flaws for every actual folder created. This method is modified to ensure
+# that our modified os.mkdir is called on Windows, by monkey patching temporarily the mkdir method
+# on the original os module, executing the modified logic to correctly protect newly created
+# folders, then restoring original mkdir method in the os module.
+def makedirs(*unused_args, **unused_kwargs):
+    """Method os.makedirs() is forbidden"""
+    raise RuntimeError('Usage of os.makedirs() is forbidden. '
+                       'Use certbot.compat.filesystem.makedirs() instead.')
+
+
 # Because of the blocking strategy on file handlers on Windows, rename does not behave as expected
 # with POSIX systems: an exception will be raised if dst already exists.
 def rename(*unused_args, **unused_kwargs):
@@ -65,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.')