quiet and fast

Fix AppVeyor on the apache-parser-v2 branch

.codecov.yml

@@ -6,13 +6,13 @@ coverage:
         flags: linux
         # Fixed target instead of auto set by #7173, can
         # be removed when flags in Codecov are added back.
-        target: 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

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

AUTHORS.md

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

CHANGELOG.md

@@ -2,7 +2,24 @@
 
 Certbot adheres to [Semantic Versioning](https://semver.org/).
 
-## 0.36.0 - master
+## 0.37.0 - master
+
+### Added
+
+* Turn off session tickets for apache plugin by default
+* acme: Authz deactivation added to `acme` module.
+
+### Changed
+
+*
+
+### Fixed
+
+*
+
+More details about these changes can be found on our GitHub repo.
+
+## 0.36.0 - 2019-07-11
 
 ### Added
 
@@ -11,13 +28,20 @@ Certbot adheres to [Semantic Versioning](https://semver.org/).
 
 ### Changed
 
+* Support for Ubuntu 14.04 Trusty has been removed.
 * 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
 
-*
+* Replace some unnecessary platform-specific line separation.
 
 More details about these changes can be found on our GitHub repo.
 

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

Dockerfile-old

@@ -1,75 +0,0 @@
-# https://github.com/letsencrypt/letsencrypt/pull/431#issuecomment-103659297
-# it is more likely developers will already have ubuntu:trusty rather
-# than e.g. debian:jessie and image size differences are negligible
-FROM ubuntu:trusty
-MAINTAINER Jakub Warmuz <jakub@warmuz.org>
-MAINTAINER William Budington <bill@eff.org>
-
-# Note: this only exposes the port to other docker containers. You
-# still have to bind to 443@host at runtime, as per the ACME spec.
-EXPOSE 443
-
-# TODO: make sure --config-dir and --work-dir cannot be changed
-# through the CLI (certbot-docker wrapper that uses standalone
-# authenticator and text mode only?)
-VOLUME /etc/letsencrypt /var/lib/letsencrypt
-
-WORKDIR /opt/certbot
-
-# no need to mkdir anything:
-# https://docs.docker.com/reference/builder/#copy
-# If <dest> doesn't exist, it is created along with all missing
-# directories in its path.
-
-ENV DEBIAN_FRONTEND=noninteractive
-
-COPY letsencrypt-auto-source/letsencrypt-auto /opt/certbot/src/letsencrypt-auto-source/letsencrypt-auto
-RUN /opt/certbot/src/letsencrypt-auto-source/letsencrypt-auto --os-packages-only && \
-    apt-get clean && \
-    rm -rf /var/lib/apt/lists/* \
-           /tmp/* \
-           /var/tmp/*
-
-# the above is not likely to change, so by putting it further up the
-# Dockerfile we make sure we cache as much as possible
-
-
-COPY setup.py README.rst CHANGELOG.md MANIFEST.in letsencrypt-auto-source/pieces/pipstrap.py /opt/certbot/src/
-
-# all above files are necessary for setup.py and venv setup, however,
-# package source code directory has to be copied separately to a
-# subdirectory...
-# https://docs.docker.com/reference/builder/#copy: "If <src> is a
-# directory, the entire contents of the directory are copied,
-# including filesystem metadata. Note: The directory itself is not
-# copied, just its contents." Order again matters, three files are far
-# more likely to be cached than the whole project directory
-
-COPY certbot /opt/certbot/src/certbot/
-COPY acme /opt/certbot/src/acme/
-COPY certbot-apache /opt/certbot/src/certbot-apache/
-COPY certbot-nginx /opt/certbot/src/certbot-nginx/
-
-
-RUN VIRTUALENV_NO_DOWNLOAD=1 virtualenv --no-site-packages -p python2 /opt/certbot/venv
-
-# PATH is set now so pipstrap upgrades the correct (v)env
-ENV PATH /opt/certbot/venv/bin:$PATH
-RUN /opt/certbot/venv/bin/python /opt/certbot/src/pipstrap.py && \
-    /opt/certbot/venv/bin/pip install \
-    -e /opt/certbot/src/acme \
-    -e /opt/certbot/src \
-    -e /opt/certbot/src/certbot-apache \
-    -e /opt/certbot/src/certbot-nginx
-
-# install in editable mode (-e) to save space: it's not possible to
-# "rm -rf /opt/certbot/src" (it's stays in the underlaying image);
-# this might also help in debugging: you can "docker run --entrypoint
-# bash" and investigate, apply patches, etc.
-
-# set up certbot/letsencrypt wrapper to warn people about Dockerfile changes
-COPY tools/docker-warning.sh /opt/certbot/bin/certbot
-RUN ln -s /opt/certbot/bin/certbot /opt/certbot/bin/letsencrypt
-ENV PATH /opt/certbot/bin:$PATH
-
-ENTRYPOINT [ "certbot" ]

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

appveyor.yml

@@ -7,6 +7,9 @@ environment:
 
 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-.*$/
@@ -27,7 +30,8 @@ install:
   # Upgrade pip to avoid warnings
   - "python -m pip install --upgrade pip"
   # Ready to install tox and coverage
-  - "pip install tox codecov"
+  # tools/pip_install.py is used to pin packages to a known working version.
+  - "python tools\\pip_install.py tox codecov"
 
 build: off
 

certbot-apache/MANIFEST.in

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

certbot-apache/certbot_apache/apache_util.py

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

certbot-apache/certbot_apache/augeas_configurator.py

@@ -1,207 +0,0 @@
-"""Class of Augeas Configurators."""
-import logging
-
-
-from certbot import errors
-from certbot.plugins import common
-
-from certbot_apache import constants
-
-logger = logging.getLogger(__name__)
-
-
-class AugeasConfigurator(common.Installer):
-    """Base Augeas Configurator class.
-
-    :ivar config: Configuration.
-    :type config: :class:`~certbot.interfaces.IConfig`
-
-    :ivar aug: Augeas object
-    :type aug: :class:`augeas.Augeas`
-
-    :ivar str save_notes: Human-readable configuration change notes
-    :ivar reverter: saves and reverts checkpoints
-    :type reverter: :class:`certbot.reverter.Reverter`
-
-    """
-    def __init__(self, *args, **kwargs):
-        super(AugeasConfigurator, self).__init__(*args, **kwargs)
-
-        # Placeholder for augeas
-        self.aug = None
-
-        self.save_notes = ""
-
-
-    def init_augeas(self):
-        """ Initialize the actual Augeas instance """
-        import augeas
-        self.aug = augeas.Augeas(
-            # specify a directory to load our preferred lens from
-            loadpath=constants.AUGEAS_LENS_DIR,
-            # Do not save backup (we do it ourselves), do not load
-            # anything by default
-            flags=(augeas.Augeas.NONE |
-                   augeas.Augeas.NO_MODL_AUTOLOAD |
-                   augeas.Augeas.ENABLE_SPAN))
-        # See if any temporary changes need to be recovered
-        # This needs to occur before VirtualHost objects are setup...
-        # because this will change the underlying configuration and potential
-        # vhosts
-        self.recovery_routine()
-
-    def check_parsing_errors(self, lens):
-        """Verify Augeas can parse all of the lens files.
-
-        :param str lens: lens to check for errors
-
-        :raises .errors.PluginError: If there has been an error in parsing with
-            the specified lens.
-
-        """
-        error_files = self.aug.match("/augeas//error")
-
-        for path in error_files:
-            # Check to see if it was an error resulting from the use of
-            # the httpd lens
-            lens_path = self.aug.get(path + "/lens")
-            # As aug.get may return null
-            if lens_path and lens in lens_path:
-                msg = (
-                    "There has been an error in parsing the file {0} on line {1}: "
-                    "{2}".format(
-                    # Strip off /augeas/files and /error
-                    path[13:len(path) - 6],
-                    self.aug.get(path + "/line"),
-                    self.aug.get(path + "/message")))
-                raise errors.PluginError(msg)
-
-    def ensure_augeas_state(self):
-        """Makes sure that all Augeas dom changes are written to files to avoid
-        loss of configuration directives when doing additional augeas parsing,
-        causing a possible augeas.load() resulting dom reset
-        """
-
-        if self.unsaved_files():
-            self.save_notes += "(autosave)"
-            self.save()
-
-    def unsaved_files(self):
-        """Lists files that have modified Augeas DOM but the changes have not
-        been written to the filesystem yet, used by `self.save()` and
-        ApacheConfigurator to check the file state.
-
-        :raises .errors.PluginError: If there was an error in Augeas, in
-            an attempt to save the configuration, or an error creating a
-            checkpoint
-
-        :returns: `set` of unsaved files
-        """
-        save_state = self.aug.get("/augeas/save")
-        self.aug.set("/augeas/save", "noop")
-        # Existing Errors
-        ex_errs = self.aug.match("/augeas//error")
-        try:
-            # This is a noop save
-            self.aug.save()
-        except (RuntimeError, IOError):
-            self._log_save_errors(ex_errs)
-            # Erase Save Notes
-            self.save_notes = ""
-            raise errors.PluginError(
-                "Error saving files, check logs for more info.")
-
-        # Return the original save method
-        self.aug.set("/augeas/save", save_state)
-
-        # Retrieve list of modified files
-        # Note: Noop saves can cause the file to be listed twice, I used a
-        # set to remove this possibility. This is a known augeas 0.10 error.
-        save_paths = self.aug.match("/augeas/events/saved")
-
-        save_files = set()
-        if save_paths:
-            for path in save_paths:
-                save_files.add(self.aug.get(path)[6:])
-        return save_files
-
-    def save(self, title=None, temporary=False):
-        """Saves all changes to the configuration files.
-
-        This function first checks for save errors, if none are found,
-        all configuration changes made will be saved. According to the
-        function parameters. If an exception is raised, a new checkpoint
-        was not created.
-
-        :param str title: The title of the save. If a title is given, the
-            configuration will be saved as a new checkpoint and put in a
-            timestamped directory.
-
-        :param bool temporary: Indicates whether the changes made will
-            be quickly reversed in the future (ie. challenges)
-
-        """
-        save_files = self.unsaved_files()
-        if save_files:
-            self.add_to_checkpoint(save_files,
-                                   self.save_notes, temporary=temporary)
-
-        self.save_notes = ""
-        self.aug.save()
-
-        # Force reload if files were modified
-        # This is needed to recalculate augeas directive span
-        if save_files:
-            for sf in save_files:
-                self.aug.remove("/files/"+sf)
-            self.aug.load()
-        if title and not temporary:
-            self.finalize_checkpoint(title)
-
-    def _log_save_errors(self, ex_errs):
-        """Log errors due to bad Augeas save.
-
-        :param list ex_errs: Existing errors before save
-
-        """
-        # Check for the root of save problems
-        new_errs = self.aug.match("/augeas//error")
-        # logger.error("During Save - %s", mod_conf)
-        logger.error("Unable to save files: %s. Attempted Save Notes: %s",
-                     ", ".join(err[13:len(err) - 6] for err in new_errs
-                               # Only new errors caused by recent save
-                               if err not in ex_errs), self.save_notes)
-
-    # Wrapper functions for Reverter class
-    def recovery_routine(self):
-        """Revert all previously modified files.
-
-        Reverts all modified files that have not been saved as a checkpoint
-
-        :raises .errors.PluginError: If unable to recover the configuration
-
-        """
-        super(AugeasConfigurator, self).recovery_routine()
-        # Need to reload configuration after these changes take effect
-        self.aug.load()
-
-    def revert_challenge_config(self):
-        """Used to cleanup challenge configurations.
-
-        :raises .errors.PluginError: If unable to revert the challenge config.
-
-        """
-        self.revert_temporary_config()
-        self.aug.load()
-
-    def rollback_checkpoints(self, rollback=1):
-        """Rollback saved checkpoints.
-
-        :param int rollback: Number of checkpoints to revert
-
-        :raises .errors.PluginError: If there is a problem with the input or
-            the function is unable to correctly revert the configuration
-
-        """
-        super(AugeasConfigurator, self).rollback_checkpoints(rollback)
-        self.aug.load()

certbot-apache/certbot_apache/configurator.py

@@ -1,4 +1,4 @@
-"""Apache Configuration based off of Augeas Configurator."""
+"""Apache Configurator."""
 # pylint: disable=too-many-lines
 import copy
 import fnmatch
@@ -9,7 +9,6 @@ import time
 
 from collections import defaultdict
 
-import pkg_resources
 import six
 
 import zope.component
@@ -23,13 +22,13 @@ 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
 from certbot.plugins.enhancements import AutoHSTSEnhancement
 
 from certbot_apache import apache_util
-from certbot_apache import augeas_configurator
 from certbot_apache import constants
 from certbot_apache import display_ops
 from certbot_apache import http_01
@@ -70,13 +69,10 @@ logger = logging.getLogger(__name__)
 
 @zope.interface.implementer(interfaces.IAuthenticator, interfaces.IInstaller)
 @zope.interface.provider(interfaces.IPluginFactory)
-class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
+class ApacheConfigurator(common.Installer):
     # pylint: disable=too-many-instance-attributes,too-many-public-methods
     """Apache configurator.
 
-    State of Configurator: This code has been been tested and built for Ubuntu
-    14.04 Apache 2.4 and it works for Ubuntu 12.04 Apache 2.2
-
     :ivar config: Configuration.
     :type config: :class:`~certbot.interfaces.IConfig`
 
@@ -113,14 +109,24 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         handle_modules=False,
         handle_sites=False,
         challenge_location="/etc/apache2",
-        MOD_SSL_CONF_SRC=pkg_resources.resource_filename(
-            "certbot_apache", "options-ssl-apache.conf")
     )
 
     def option(self, key):
         """Get a value from options"""
         return self.options.get(key)
 
+    def pick_apache_config(self):
+        """
+        Pick the appropriate TLS Apache configuration file for current version of Apache and OS.
+        :return: the path to the TLS Apache configuration file to use
+        :rtype: str
+        """
+        # Disabling TLS session tickets is supported by Apache 2.4.11+.
+        # So for old versions of Apache we pick a configuration without this option.
+        if self.version < (2, 4, 11):
+            return apache_util.find_ssl_apache_conf("old")
+        return apache_util.find_ssl_apache_conf("current")
+
     def _prepare_options(self):
         """
         Set the values possibly changed by command line parameters to
@@ -201,6 +207,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         self._enhanced_vhosts = defaultdict(set)  # type: DefaultDict[str, Set[obj.VirtualHost]]
         # Temporary state for AutoHSTS enhancement
         self._autohsts = {}  # type: Dict[str, Dict[str, Union[int, float]]]
+        # Reverter save notes
+        self.save_notes = ""
 
         # These will be set in the prepare function
         self._prepared = False
@@ -231,12 +239,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         :raises .errors.PluginError: If there is any other error
 
         """
-        # Perform the actual Augeas initialization to be able to react
-        try:
-            self.init_augeas()
-        except ImportError:
-            raise errors.NoInstallationError("Problem in Augeas installation")
-
         self._prepare_options()
 
         # Verify Apache is installed
@@ -254,16 +256,14 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
             raise errors.NotSupportedError(
                 "Apache Version {0} not supported.".format(str(self.version)))
 
-        if not self._check_aug_version():
-            raise errors.NotSupportedError(
-                "Apache plugin support requires libaugeas0 and augeas-lenses "
-                "version 1.2.0 or higher, please make sure you have you have "
-                "those installed.")
-
+        # Recover from previous crash before Augeas initialization to have the
+        # correct parse tree from the get go.
+        self.recovery_routine()
+        # Perform the actual Augeas initialization to be able to react
         self.parser = self.get_parser()
 
         # Check for errors in parsing files with Augeas
-        self.check_parsing_errors("httpd.aug")
+        self.parser.check_parsing_errors("httpd.aug")
 
         # Get all of the available vhosts
         self.vhosts = self.get_virtual_hosts()
@@ -282,6 +282,67 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
                 " Apache configuration?".format(self.option("server_root")))
         self._prepared = True
 
+    def save(self, title=None, temporary=False):
+        """Saves all changes to the configuration files.
+
+        This function first checks for save errors, if none are found,
+        all configuration changes made will be saved. According to the
+        function parameters. If an exception is raised, a new checkpoint
+        was not created.
+
+        :param str title: The title of the save. If a title is given, the
+            configuration will be saved as a new checkpoint and put in a
+            timestamped directory.
+
+        :param bool temporary: Indicates whether the changes made will
+            be quickly reversed in the future (ie. challenges)
+
+        """
+        save_files = self.parser.unsaved_files()
+        if save_files:
+            self.add_to_checkpoint(save_files,
+                                   self.save_notes, temporary=temporary)
+        # Handle the parser specific tasks
+        self.parser.save(save_files)
+        if title and not temporary:
+            self.finalize_checkpoint(title)
+
+    def recovery_routine(self):
+        """Revert all previously modified files.
+
+        Reverts all modified files that have not been saved as a checkpoint
+
+        :raises .errors.PluginError: If unable to recover the configuration
+
+        """
+        super(ApacheConfigurator, self).recovery_routine()
+        # Reload configuration after these changes take effect if needed
+        # ie. ApacheParser has been initialized.
+        if self.parser:
+            # TODO: wrap into non-implementation specific  parser interface
+            self.parser.aug.load()
+
+    def revert_challenge_config(self):
+        """Used to cleanup challenge configurations.
+
+        :raises .errors.PluginError: If unable to revert the challenge config.
+
+        """
+        self.revert_temporary_config()
+        self.parser.aug.load()
+
+    def rollback_checkpoints(self, rollback=1):
+        """Rollback saved checkpoints.
+
+        :param int rollback: Number of checkpoints to revert
+
+        :raises .errors.PluginError: If there is a problem with the input or
+            the function is unable to correctly revert the configuration
+
+        """
+        super(ApacheConfigurator, self).rollback_checkpoints(rollback)
+        self.parser.aug.load()
+
     def _verify_exe_availability(self, exe):
         """Checks availability of Apache executable"""
         if not util.exe_exists(exe):
@@ -289,26 +350,11 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
                 raise errors.NoInstallationError(
                     'Cannot find Apache executable {0}'.format(exe))
 
-    def _check_aug_version(self):
-        """ Checks that we have recent enough version of libaugeas.
-        If augeas version is recent enough, it will support case insensitive
-        regexp matching"""
-
-        self.aug.set("/test/path/testing/arg", "aRgUMeNT")
-        try:
-            matches = self.aug.match(
-                "/test//*[self::arg=~regexp('argument', 'i')]")
-        except RuntimeError:
-            self.aug.remove("/test/path")
-            return False
-        self.aug.remove("/test/path")
-        return matches
-
     def get_parser(self):
         """Initializes the ApacheParser"""
         # If user provided vhost_root value in command line, use it
         return parser.ApacheParser(
-            self.aug, self.option("server_root"), self.conf("vhost-root"),
+            self.option("server_root"), self.conf("vhost-root"),
             self.version, configurator=self)
 
     def _wildcard_domain(self, domain):
@@ -487,8 +533,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
             # install SSLCertificateFile, SSLCertificateKeyFile,
             # and SSLCertificateChainFile directives
             set_cert_path = cert_path
-            self.aug.set(path["cert_path"][-1], cert_path)
-            self.aug.set(path["cert_key"][-1], key_path)
+            self.parser.aug.set(path["cert_path"][-1], cert_path)
+            self.parser.aug.set(path["cert_key"][-1], key_path)
             if chain_path is not None:
                 self.parser.add_dir(vhost.path,
                                     "SSLCertificateChainFile", chain_path)
@@ -500,8 +546,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
                 raise errors.PluginError("Please provide the --fullchain-path "
                                          "option pointing to your full chain file")
             set_cert_path = fullchain_path
-            self.aug.set(path["cert_path"][-1], fullchain_path)
-            self.aug.set(path["cert_key"][-1], key_path)
+            self.parser.aug.set(path["cert_path"][-1], fullchain_path)
+            self.parser.aug.set(path["cert_key"][-1], key_path)
 
         # Enable the new vhost if needed
         if not vhost.enabled:
@@ -801,7 +847,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         """
         addrs = set()
         try:
-            args = self.aug.match(path + "/arg")
+            args = self.parser.aug.match(path + "/arg")
         except RuntimeError:
             logger.warning("Encountered a problem while parsing file: %s, skipping", path)
             return None
@@ -819,7 +865,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
                 is_ssl = True
 
         filename = apache_util.get_file_path(
-            self.aug.get("/augeas/files%s/path" % apache_util.get_file_path(path)))
+            self.parser.aug.get("/augeas/files%s/path" % apache_util.get_file_path(path)))
         if filename is None:
             return None
 
@@ -849,7 +895,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         # Make a list of parser paths because the parser_paths
         # dictionary may be modified during the loop.
         for vhost_path in list(self.parser.parser_paths):
-            paths = self.aug.match(
+            paths = self.parser.aug.match(
                 ("/files%s//*[label()=~regexp('%s')]" %
                     (vhost_path, parser.case_i("VirtualHost"))))
             paths = [path for path in paths if
@@ -859,7 +905,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
                 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)
@@ -1103,16 +1149,16 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         avail_fp = nonssl_vhost.filep
         ssl_fp = self._get_ssl_vhost_path(avail_fp)
 
-        orig_matches = self.aug.match("/files%s//* [label()=~regexp('%s')]" %
+        orig_matches = self.parser.aug.match("/files%s//* [label()=~regexp('%s')]" %
                                       (self._escape(ssl_fp),
                                        parser.case_i("VirtualHost")))
 
         self._copy_create_ssl_vhost_skeleton(nonssl_vhost, ssl_fp)
 
         # Reload augeas to take into account the new vhost
-        self.aug.load()
+        self.parser.aug.load()
         # Get Vhost augeas path for new vhost
-        new_matches = self.aug.match("/files%s//* [label()=~regexp('%s')]" %
+        new_matches = self.parser.aug.match("/files%s//* [label()=~regexp('%s')]" %
                                      (self._escape(ssl_fp),
                                       parser.case_i("VirtualHost")))
 
@@ -1123,7 +1169,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
             # Make Augeas aware of the new vhost
             self.parser.parse_file(ssl_fp)
             # Try to search again
-            new_matches = self.aug.match(
+            new_matches = self.parser.aug.match(
                 "/files%s//* [label()=~regexp('%s')]" %
                 (self._escape(ssl_fp),
                  parser.case_i("VirtualHost")))
@@ -1185,11 +1231,11 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         """
 
         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")
@@ -1273,8 +1319,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
                 "vhost for your HTTPS site located at {1} because they have "
                 "the potential to create redirection loops.".format(
                     vhost.filep, ssl_fp), reporter.MEDIUM_PRIORITY)
-        self.aug.set("/augeas/files%s/mtime" % (self._escape(ssl_fp)), "0")
-        self.aug.set("/augeas/files%s/mtime" % (self._escape(vhost.filep)), "0")
+        self.parser.aug.set("/augeas/files%s/mtime" % (self._escape(ssl_fp)), "0")
+        self.parser.aug.set("/augeas/files%s/mtime" % (self._escape(vhost.filep)), "0")
 
     def _sift_rewrite_rules(self, contents):
         """ Helper function for _copy_create_ssl_vhost_skeleton to prepare the
@@ -1349,7 +1395,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         """
 
         try:
-            span_val = self.aug.span(vhost.path)
+            span_val = self.parser.aug.span(vhost.path)
         except ValueError:
             logger.critical("Error while reading the VirtualHost %s from "
                          "file %s", vhost.name, vhost.filep, exc_info=True)
@@ -1384,13 +1430,13 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
 
     def _update_ssl_vhosts_addrs(self, vh_path):
         ssl_addrs = set()
-        ssl_addr_p = self.aug.match(vh_path + "/arg")
+        ssl_addr_p = self.parser.aug.match(vh_path + "/arg")
 
         for addr in ssl_addr_p:
             old_addr = obj.Addr.fromstring(
                 str(self.parser.get_arg(addr)))
             ssl_addr = old_addr.get_addr_obj("443")
-            self.aug.set(addr, str(ssl_addr))
+            self.parser.aug.set(addr, str(ssl_addr))
             ssl_addrs.add(ssl_addr)
 
         return ssl_addrs
@@ -1409,14 +1455,14 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
                                            vh_path, False)) > 1:
                 directive_path = self.parser.find_dir(directive, None,
                                                       vh_path, False)
-                self.aug.remove(re.sub(r"/\w*$", "", directive_path[0]))
+                self.parser.aug.remove(re.sub(r"/\w*$", "", directive_path[0]))
 
     def _remove_directives(self, vh_path, directives):
         for directive in directives:
             while self.parser.find_dir(directive, None, vh_path, False):
                 directive_path = self.parser.find_dir(directive, None,
                                                       vh_path, False)
-                self.aug.remove(re.sub(r"/\w*$", "", directive_path[0]))
+                self.parser.aug.remove(re.sub(r"/\w*$", "", directive_path[0]))
 
     def _add_dummy_ssl_directives(self, vh_path):
         self.parser.add_dir(vh_path, "SSLCertificateFile",
@@ -1455,7 +1501,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         """
         matches = self.parser.find_dir(
             "ServerAlias", start=vh_path, exclude=False)
-        aliases = (self.aug.get(match) for match in matches)
+        aliases = (self.parser.aug.get(match) for match in matches)
         return self.domain_in_names(aliases, target_name)
 
     def _add_name_vhost_if_necessary(self, vhost):
@@ -1638,7 +1684,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         if header_path:
             pat = '(?:[ "]|^)(strict-transport-security)(?:[ "]|$)'
             for match in header_path:
-                if re.search(pat, self.aug.get(match).lower()):
+                if re.search(pat, self.parser.aug.get(match).lower()):
                     hsts_dirpath = match
         if not hsts_dirpath:
             err_msg = ("Certbot was unable to find the existing HSTS header "
@@ -1652,7 +1698,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         # Our match statement was for string strict-transport-security, but
         # we need to update the value instead. The next index is for the value
         hsts_dirpath = hsts_dirpath.replace("arg[3]", "arg[4]")
-        self.aug.set(hsts_dirpath, hsts_maxage)
+        self.parser.aug.set(hsts_dirpath, hsts_maxage)
         note_msg = ("Increasing HSTS max-age value to {0} for VirtualHost "
                     "in {1}\n".format(nextstep_value, vhost.filep))
         logger.debug(note_msg)
@@ -1734,7 +1780,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         # We'll simply delete the directive, so that we'll have a
         # consistent OCSP cache path.
         if stapling_cache_aug_path:
-            self.aug.remove(
+            self.parser.aug.remove(
                     re.sub(r"/\w*$", "", stapling_cache_aug_path[0]))
 
         self.parser.add_dir_to_ifmodssl(ssl_vhost_aug_path,
@@ -1811,7 +1857,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
             # "Existing Header directive for virtualhost"
             pat = '(?:[ "]|^)(%s)(?:[ "]|$)' % (header_substring.lower())
             for match in header_path:
-                if re.search(pat, self.aug.get(match).lower()):
+                if re.search(pat, self.parser.aug.get(match).lower()):
                     raise errors.PluginEnhancementAlreadyPresent(
                         "Existing %s header" % (header_substring))
 
@@ -1938,11 +1984,11 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
                              constants.REWRITE_HTTPS_ARGS_WITH_END]
 
             for dir_path, args_paths in rewrite_args_dict.items():
-                arg_vals = [self.aug.get(x) for x in args_paths]
+                arg_vals = [self.parser.aug.get(x) for x in args_paths]
 
                 # Search for past redirection rule, delete it, set the new one
                 if arg_vals in constants.OLD_REWRITE_HTTPS_ARGS:
-                    self.aug.remove(dir_path)
+                    self.parser.aug.remove(dir_path)
                     self._set_https_redirection_rewrite_rule(vhost)
                     self.save()
                     raise errors.PluginEnhancementAlreadyPresent(
@@ -1998,7 +2044,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
 
         redirect_filepath = self._write_out_redirect(ssl_vhost, text)
 
-        self.aug.load()
+        self.parser.aug.load()
         # Make a new vhost data structure and add it to the lists
         new_vhost = self._create_vhost(parser.get_aug_path(self._escape(redirect_filepath)))
         self.vhosts.append(new_vhost)
@@ -2302,8 +2348,9 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         # XXX if we ever try to enforce a local privilege boundary (eg, running
         # certbot for unprivileged users via setuid), this function will need
         # to be modified.
-        return common.install_version_controlled_file(options_ssl, options_ssl_digest,
-            self.option("MOD_SSL_CONF_SRC"), constants.ALL_SSL_OPTIONS_HASHES)
+        apache_config_path = self.pick_apache_config()
+        return common.install_version_controlled_file(
+            options_ssl, options_ssl_digest, apache_config_path, constants.ALL_SSL_OPTIONS_HASHES)
 
     def enable_autohsts(self, _unused_lineage, domains):
         """

certbot-apache/certbot_apache/constants.py

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

certbot-apache/certbot_apache/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_arch.py

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

certbot-apache/certbot_apache/override_centos.py

@@ -1,7 +1,6 @@
 """ Distribution specific override class for CentOS family (RHEL, Fedora) """
 import logging
 
-import pkg_resources
 import zope.interface
 
 from certbot import errors
@@ -39,8 +38,6 @@ class CentOSConfigurator(configurator.ApacheConfigurator):
         handle_modules=False,
         handle_sites=False,
         challenge_location="/etc/httpd/conf.d",
-        MOD_SSL_CONF_SRC=pkg_resources.resource_filename(
-            "certbot_apache", "centos-options-ssl-apache.conf")
     )
 
     def config_test(self):
@@ -75,6 +72,18 @@ class CentOSConfigurator(configurator.ApacheConfigurator):
         # Finish with actual config check to see if systemctl restart helped
         super(CentOSConfigurator, self).config_test()
 
+    def pick_apache_config(self):
+        """
+        Pick the appropriate TLS Apache configuration file for current version of Apache and OS.
+        :return: the path to the TLS Apache configuration file to use
+        :rtype: str
+        """
+        # Disabling TLS session tickets is supported by Apache 2.4.11+.
+        # So for old versions of Apache we pick a configuration without this option.
+        if self.version < (2, 4, 11):
+            return apache_util.find_ssl_apache_conf("centos-old")
+        return apache_util.find_ssl_apache_conf("centos-current")
+
     def _prepare_options(self):
         """
         Override the options dictionary initialization in order to support
@@ -86,7 +95,7 @@ class CentOSConfigurator(configurator.ApacheConfigurator):
     def get_parser(self):
         """Initializes the ApacheParser"""
         return CentOSParser(
-            self.aug, self.option("server_root"), self.option("vhost_root"),
+            self.option("server_root"), self.option("vhost_root"),
             self.version, configurator=self)
 
     def _deploy_cert(self, *args, **kwargs):  # pylint: disable=arguments-differ
@@ -155,7 +164,7 @@ class CentOSConfigurator(configurator.ApacheConfigurator):
         for loadmod_path in loadmod_paths:
             nodir_path = loadmod_path.split("/directive")[0]
             # Remove the old LoadModule directive
-            self.aug.remove(loadmod_path)
+            self.parser.aug.remove(loadmod_path)
 
             # Create a new IfModule !mod_ssl.c if not already found on path
             ssl_ifmod = self.parser.get_ifmod(nodir_path, "!mod_ssl.c",

certbot-apache/certbot_apache/override_darwin.py

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

certbot-apache/certbot_apache/override_debian.py

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

certbot-apache/certbot_apache/override_fedora.py

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

certbot-apache/certbot_apache/override_gentoo.py

@@ -1,6 +1,4 @@
 """ Distribution specific override class for Gentoo Linux """
-import pkg_resources
-
 import zope.interface
 
 from certbot import interfaces
@@ -29,8 +27,6 @@ class GentooConfigurator(configurator.ApacheConfigurator):
         handle_modules=False,
         handle_sites=False,
         challenge_location="/etc/apache2/vhosts.d",
-        MOD_SSL_CONF_SRC=pkg_resources.resource_filename(
-            "certbot_apache", "options-ssl-apache.conf")
     )
 
     def _prepare_options(self):
@@ -44,7 +40,7 @@ class GentooConfigurator(configurator.ApacheConfigurator):
     def get_parser(self):
         """Initializes the ApacheParser"""
         return GentooParser(
-            self.aug, self.option("server_root"), self.option("vhost_root"),
+            self.option("server_root"), self.option("vhost_root"),
             self.version, configurator=self)
 
 

certbot-apache/certbot_apache/override_suse.py

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

certbot-apache/certbot_apache/parser.py

@@ -13,6 +13,8 @@ from acme.magic_typing import Dict, List, Set  # pylint: disable=unused-import,
 from certbot import errors
 from certbot.compat import os
 
+from certbot_apache import constants
+
 logger = logging.getLogger(__name__)
 
 
@@ -32,7 +34,7 @@ class ApacheParser(object):
     arg_var_interpreter = re.compile(r"\$\{[^ \}]*}")
     fnmatch_chars = set(["*", "?", "\\", "[", "]"])
 
-    def __init__(self, aug, root, vhostroot=None, version=(2, 4),
+    def __init__(self, root, vhostroot=None, version=(2, 4),
                  configurator=None):
         # Note: Order is important here.
 
@@ -41,11 +43,20 @@ class ApacheParser(object):
         # issues with aug.load() after adding new files / defines to parse tree
         self.configurator = configurator
 
+        # Initialize augeas
+        self.aug = None
+        self.init_augeas()
+
+        if not self.check_aug_version():
+            raise errors.NotSupportedError(
+                "Apache plugin support requires libaugeas0 and augeas-lenses "
+                "version 1.2.0 or higher, please make sure you have you have "
+                "those installed.")
+
         self.modules = set()  # type: Set[str]
         self.parser_paths = {}  # type: Dict[str, List[str]]
         self.variables = {}  # type: Dict[str, str]
 
-        self.aug = aug
         # Find configuration root and make sure augeas can parse it.
         self.root = os.path.abspath(root)
         self.loc = {"root": self._find_config_root()}
@@ -77,6 +88,146 @@ class ApacheParser(object):
             if self.find_dir("Define", exclude=False):
                 raise errors.PluginError("Error parsing runtime variables")
 
+    def init_augeas(self):
+        """ Initialize the actual Augeas instance """
+
+        try:
+            import augeas
+        except ImportError:  # pragma: no cover
+            raise errors.NoInstallationError("Problem in Augeas installation")
+
+        self.aug = augeas.Augeas(
+            # specify a directory to load our preferred lens from
+            loadpath=constants.AUGEAS_LENS_DIR,
+            # Do not save backup (we do it ourselves), do not load
+            # anything by default
+            flags=(augeas.Augeas.NONE |
+                   augeas.Augeas.NO_MODL_AUTOLOAD |
+                   augeas.Augeas.ENABLE_SPAN))
+
+    def check_parsing_errors(self, lens):
+        """Verify Augeas can parse all of the lens files.
+
+        :param str lens: lens to check for errors
+
+        :raises .errors.PluginError: If there has been an error in parsing with
+            the specified lens.
+
+        """
+        error_files = self.aug.match("/augeas//error")
+
+        for path in error_files:
+            # Check to see if it was an error resulting from the use of
+            # the httpd lens
+            lens_path = self.aug.get(path + "/lens")
+            # As aug.get may return null
+            if lens_path and lens in lens_path:
+                msg = (
+                    "There has been an error in parsing the file {0} on line {1}: "
+                    "{2}".format(
+                    # Strip off /augeas/files and /error
+                    path[13:len(path) - 6],
+                    self.aug.get(path + "/line"),
+                    self.aug.get(path + "/message")))
+                raise errors.PluginError(msg)
+
+    def check_aug_version(self):
+        """ Checks that we have recent enough version of libaugeas.
+        If augeas version is recent enough, it will support case insensitive
+        regexp matching"""
+
+        self.aug.set("/test/path/testing/arg", "aRgUMeNT")
+        try:
+            matches = self.aug.match(
+                "/test//*[self::arg=~regexp('argument', 'i')]")
+        except RuntimeError:
+            self.aug.remove("/test/path")
+            return False
+        self.aug.remove("/test/path")
+        return matches
+
+    def unsaved_files(self):
+        """Lists files that have modified Augeas DOM but the changes have not
+        been written to the filesystem yet, used by `self.save()` and
+        ApacheConfigurator to check the file state.
+
+        :raises .errors.PluginError: If there was an error in Augeas, in
+            an attempt to save the configuration, or an error creating a
+            checkpoint
+
+        :returns: `set` of unsaved files
+        """
+        save_state = self.aug.get("/augeas/save")
+        self.aug.set("/augeas/save", "noop")
+        # Existing Errors
+        ex_errs = self.aug.match("/augeas//error")
+        try:
+            # This is a noop save
+            self.aug.save()
+        except (RuntimeError, IOError):
+            self._log_save_errors(ex_errs)
+            # Erase Save Notes
+            self.configurator.save_notes = ""
+            raise errors.PluginError(
+                "Error saving files, check logs for more info.")
+
+        # Return the original save method
+        self.aug.set("/augeas/save", save_state)
+
+        # Retrieve list of modified files
+        # Note: Noop saves can cause the file to be listed twice, I used a
+        # set to remove this possibility. This is a known augeas 0.10 error.
+        save_paths = self.aug.match("/augeas/events/saved")
+
+        save_files = set()
+        if save_paths:
+            for path in save_paths:
+                save_files.add(self.aug.get(path)[6:])
+        return save_files
+
+    def ensure_augeas_state(self):
+        """Makes sure that all Augeas dom changes are written to files to avoid
+        loss of configuration directives when doing additional augeas parsing,
+        causing a possible augeas.load() resulting dom reset
+        """
+
+        if self.unsaved_files():
+            self.configurator.save_notes += "(autosave)"
+            self.configurator.save()
+
+    def save(self, save_files):
+        """Saves all changes to the configuration files.
+
+        save() is called from ApacheConfigurator to handle the parser specific
+        tasks of saving.
+
+        :param list save_files: list of strings of file paths that we need to save.
+
+        """
+        self.configurator.save_notes = ""
+        self.aug.save()
+
+        # Force reload if files were modified
+        # This is needed to recalculate augeas directive span
+        if save_files:
+            for sf in save_files:
+                self.aug.remove("/files/"+sf)
+            self.aug.load()
+
+    def _log_save_errors(self, ex_errs):
+        """Log errors due to bad Augeas save.
+
+        :param list ex_errs: Existing errors before save
+
+        """
+        # Check for the root of save problems
+        new_errs = self.aug.match("/augeas//error")
+        # logger.error("During Save - %s", mod_conf)
+        logger.error("Unable to save files: %s. Attempted Save Notes: %s",
+                     ", ".join(err[13:len(err) - 6] for err in new_errs
+                               # Only new errors caused by recent save
+                               if err not in ex_errs), self.configurator.save_notes)
+
     def add_include(self, main_config, inc_path):
         """Add Include for a new configuration file if one does not exist
 
@@ -658,8 +809,7 @@ class ApacheParser(object):
         use_new, remove_old = self._check_path_actions(filepath)
         # Ensure that we have the latest Augeas DOM state on disk before
         # calling aug.load() which reloads the state from disk
-        if self.configurator:
-            self.configurator.ensure_augeas_state()
+        self.ensure_augeas_state()
         # Test if augeas included file for Httpd.lens
         # Note: This works for augeas globs, ie. *.conf
         if use_new:

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

@@ -165,10 +165,6 @@ class CentOS6Tests(util.ApacheTest):
             "LoadModule", "ssl_module", start=self.vh_truth[1].path, exclude=False)
         self.assertEqual(len(post_loadmods), 1)
 
-
-
-
-
     def test_loadmod_non_duplicate(self):
         # the modules/mod_ssl.so exists in ssl.conf
         sslmod_args = ["ssl_module", "modules/mod_somethingelse.so"]
@@ -197,7 +193,7 @@ class CentOS6Tests(util.ApacheTest):
                                                     exclude=False)
         for mod in orig_loadmods:
             noarg_path = mod.rpartition("/")[0]
-            self.config.aug.remove(noarg_path)
+            self.config.parser.aug.remove(noarg_path)
         self.config.save()
         self.config.deploy_cert(
             "random.demo", "example/cert.pem", "example/key.pem",

certbot-apache/certbot_apache/tests/centos_test.py

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

certbot-apache/certbot_apache/tests/configurator_reverter_test.py

@@ -1,21 +1,20 @@
-"""Test for certbot_apache.augeas_configurator."""
+"""Test for certbot_apache.configurator implementations of reverter"""
 import shutil
 import unittest
 
 import mock
 
 from certbot import errors
-from certbot.compat import os
 
 from certbot_apache.tests import util
 
 
-class AugeasConfiguratorTest(util.ApacheTest):
-    """Test for Augeas Configurator base class."""
+class ConfiguratorReverterTest(util.ApacheTest):
+    """Test for ApacheConfigurator reverter methods"""
 
 
     def setUp(self):  # pylint: disable=arguments-differ
-        super(AugeasConfiguratorTest, self).setUp()
+        super(ConfiguratorReverterTest, self).setUp()
 
         self.config = util.get_apache_configurator(
             self.config_path, self.vhost_path, self.config_dir, self.work_dir)
@@ -28,20 +27,6 @@ class AugeasConfiguratorTest(util.ApacheTest):
         shutil.rmtree(self.work_dir)
         shutil.rmtree(self.temp_dir)
 
-    def test_bad_parse(self):
-        # pylint: disable=protected-access
-        self.config.parser.parse_file(os.path.join(
-            self.config.parser.root, "conf-available", "bad_conf_file.conf"))
-        self.assertRaises(
-            errors.PluginError, self.config.check_parsing_errors, "httpd.aug")
-
-    def test_bad_save(self):
-        mock_save = mock.Mock()
-        mock_save.side_effect = IOError
-        self.config.aug.save = mock_save
-
-        self.assertRaises(errors.PluginError, self.config.save)
-
     def test_bad_save_checkpoint(self):
         self.config.reverter.add_to_checkpoint = mock.Mock(
             side_effect=errors.ReverterError)
@@ -63,23 +48,9 @@ class AugeasConfiguratorTest(util.ApacheTest):
 
         self.assertTrue(mock_finalize.is_called)
 
-    def test_recovery_routine(self):
-        mock_load = mock.Mock()
-        self.config.aug.load = mock_load
-
-        self.config.recovery_routine()
-        self.assertEqual(mock_load.call_count, 1)
-
-    def test_recovery_routine_error(self):
-        self.config.reverter.recovery_routine = mock.Mock(
-            side_effect=errors.ReverterError)
-
-        self.assertRaises(
-            errors.PluginError, self.config.recovery_routine)
-
     def test_revert_challenge_config(self):
         mock_load = mock.Mock()
-        self.config.aug.load = mock_load
+        self.config.parser.aug.load = mock_load
 
         self.config.revert_challenge_config()
         self.assertEqual(mock_load.call_count, 1)
@@ -93,7 +64,7 @@ class AugeasConfiguratorTest(util.ApacheTest):
 
     def test_rollback_checkpoints(self):
         mock_load = mock.Mock()
-        self.config.aug.load = mock_load
+        self.config.parser.aug.load = mock_load
 
         self.config.rollback_checkpoints()
         self.assertEqual(mock_load.call_count, 1)
@@ -103,13 +74,11 @@ class AugeasConfiguratorTest(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
+        self.config.recovery_routine()
+        self.assertEqual(mock_load.call_count, 1)
 
 
 if __name__ == "__main__":

certbot-apache/certbot_apache/tests/configurator_test.py

@@ -51,25 +51,14 @@ class MultipleVhostsTest(util.ApacheTest):
         self.config.deploy_cert = mocked_deploy_cert
         return self.config
 
-    @mock.patch("certbot_apache.configurator.ApacheConfigurator.init_augeas")
     @mock.patch("certbot_apache.configurator.path_surgery")
-    def test_prepare_no_install(self, mock_surgery, _init_augeas):
+    def test_prepare_no_install(self, mock_surgery):
         silly_path = {"PATH": "/tmp/nothingness2342"}
         mock_surgery.return_value = False
         with mock.patch.dict('os.environ', silly_path):
             self.assertRaises(errors.NoInstallationError, self.config.prepare)
             self.assertEqual(mock_surgery.call_count, 1)
 
-    @mock.patch("certbot_apache.augeas_configurator.AugeasConfigurator.init_augeas")
-    def test_prepare_no_augeas(self, mock_init_augeas):
-        """ Test augeas initialization ImportError """
-        def side_effect_error():
-            """ Side effect error for the test """
-            raise ImportError
-        mock_init_augeas.side_effect = side_effect_error
-        self.assertRaises(
-            errors.NoInstallationError, self.config.prepare)
-
     @mock.patch("certbot_apache.parser.ApacheParser")
     @mock.patch("certbot_apache.configurator.util.exe_exists")
     def test_prepare_version(self, mock_exe_exists, _):
@@ -81,16 +70,6 @@ class MultipleVhostsTest(util.ApacheTest):
         self.assertRaises(
             errors.NotSupportedError, self.config.prepare)
 
-    @mock.patch("certbot_apache.parser.ApacheParser")
-    @mock.patch("certbot_apache.configurator.util.exe_exists")
-    def test_prepare_old_aug(self, mock_exe_exists, _):
-        mock_exe_exists.return_value = True
-        self.config.config_test = mock.Mock()
-        # pylint: disable=protected-access
-        self.config._check_aug_version = mock.Mock(return_value=False)
-        self.assertRaises(
-            errors.NotSupportedError, self.config.prepare)
-
     def test_prepare_locked(self):
         server_root = self.config.conf("server-root")
         self.config.config_test = mock.Mock()
@@ -675,7 +654,7 @@ class MultipleVhostsTest(util.ApacheTest):
         # span excludes the closing </VirtualHost> tag in older versions
         # of Augeas
         return_value = [self.vh_truth[0].filep, 1, 12, 0, 0, 0, 1142]
-        with mock.patch.object(self.config.aug, 'span') as mock_span:
+        with mock.patch.object(self.config.parser.aug, 'span') as mock_span:
             mock_span.return_value = return_value
             self.test_make_vhost_ssl()
 
@@ -683,7 +662,7 @@ class MultipleVhostsTest(util.ApacheTest):
         # span includes the closing </VirtualHost> tag in newer versions
         # of Augeas
         return_value = [self.vh_truth[0].filep, 1, 12, 0, 0, 0, 1157]
-        with mock.patch.object(self.config.aug, 'span') as mock_span:
+        with mock.patch.object(self.config.parser.aug, 'span') as mock_span:
             mock_span.return_value = return_value
             self.test_make_vhost_ssl()
 
@@ -696,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])
@@ -1232,7 +1210,7 @@ class MultipleVhostsTest(util.ApacheTest):
         except errors.PluginEnhancementAlreadyPresent:
             args_paths = self.config.parser.find_dir(
                 "RewriteRule", None, http_vhost.path, False)
-            arg_vals = [self.config.aug.get(x) for x in args_paths]
+            arg_vals = [self.config.parser.aug.get(x) for x in args_paths]
             self.assertEqual(arg_vals, constants.REWRITE_HTTPS_ARGS)
 
 
@@ -1335,15 +1313,6 @@ class MultipleVhostsTest(util.ApacheTest):
 
         return account_key, (achall1, achall2, achall3)
 
-    def test_aug_version(self):
-        mock_match = mock.Mock(return_value=["something"])
-        self.config.aug.match = mock_match
-        # pylint: disable=protected-access
-        self.assertEqual(self.config._check_aug_version(),
-                         ["something"])
-        self.config.aug.match.side_effect = RuntimeError
-        self.assertFalse(self.config._check_aug_version())
-
     def test_enable_site_nondebian(self):
         inc_path = "/path/to/wherever"
         vhost = self.vh_truth[0]
@@ -1366,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"
@@ -1512,7 +1481,7 @@ class MultipleVhostsTest(util.ApacheTest):
         self.assertEqual(first_id, second_id)
 
     def test_realpath_replaces_symlink(self):
-        orig_match = self.config.aug.match
+        orig_match = self.config.parser.aug.match
         mock_vhost = copy.deepcopy(self.vh_truth[0])
         mock_vhost.filep = mock_vhost.filep.replace('sites-enabled', u'sites-available')
         mock_vhost.path = mock_vhost.path.replace('sites-enabled', 'sites-available')
@@ -1526,7 +1495,7 @@ class MultipleVhostsTest(util.ApacheTest):
             return orig_match(aug_expr)
 
         self.config.parser.parser_paths = ["/mocked/path"]
-        self.config.aug.match = mock_match
+        self.config.parser.aug.match = mock_match
         vhs = self.config.get_virtual_hosts()
         self.assertEqual(len(vhs), 2)
         self.assertTrue(vhs[0] == self.vh_truth[1])
@@ -1552,8 +1521,8 @@ class AugeasVhostsTest(util.ApacheTest):
             self.work_dir)
 
     def test_choosevhost_with_illegal_name(self):
-        self.config.aug = mock.MagicMock()
-        self.config.aug.match.side_effect = RuntimeError
+        self.config.parser.aug = mock.MagicMock()
+        self.config.parser.aug.match.side_effect = RuntimeError
         path = "debian_apache_2_4/augeas_vhosts/apache2/sites-available/old-and-default.conf"
         chosen_vhost = self.config._create_vhost(path)
         self.assertEqual(None, chosen_vhost)
@@ -1737,7 +1706,7 @@ class InstallSslOptionsConfTest(util.ApacheTest):
                                              self.config.updated_mod_ssl_conf_digest)
 
     def _current_ssl_options_hash(self):
-        return crypto_util.sha256sum(self.config.option("MOD_SSL_CONF_SRC"))
+        return crypto_util.sha256sum(self.config.pick_apache_config())
 
     def _assert_current_file(self):
         self.assertTrue(os.path.isfile(self.config.mod_ssl_conf))
@@ -1773,7 +1742,7 @@ class InstallSslOptionsConfTest(util.ApacheTest):
             self.assertFalse(mock_logger.warning.called)
         self.assertTrue(os.path.isfile(self.config.mod_ssl_conf))
         self.assertEqual(crypto_util.sha256sum(
-            self.config.option("MOD_SSL_CONF_SRC")),
+            self.config.pick_apache_config()),
             self._current_ssl_options_hash())
         self.assertNotEqual(crypto_util.sha256sum(self.config.mod_ssl_conf),
             self._current_ssl_options_hash())
@@ -1789,18 +1758,31 @@ class InstallSslOptionsConfTest(util.ApacheTest):
                 "%s has been manually modified; updated file "
                 "saved to %s. We recommend updating %s for security purposes.")
         self.assertEqual(crypto_util.sha256sum(
-            self.config.option("MOD_SSL_CONF_SRC")),
+            self.config.pick_apache_config()),
             self._current_ssl_options_hash())
         # only print warning once
         with mock.patch("certbot.plugins.common.logger") as mock_logger:
             self._call()
             self.assertFalse(mock_logger.warning.called)
 
-    def test_current_file_hash_in_all_hashes(self):
+    def test_ssl_config_files_hash_in_all_hashes(self):
+        """
+        It is really critical that all TLS Apache config files have their SHA256 hash registered in
+        constants.ALL_SSL_OPTIONS_HASHES. Otherwise Certbot will mistakenly assume that the config
+        file has been manually edited by the user, and will refuse to update it.
+        This test ensures that all necessary hashes are present.
+        """
         from certbot_apache.constants import ALL_SSL_OPTIONS_HASHES
-        self.assertTrue(self._current_ssl_options_hash() in ALL_SSL_OPTIONS_HASHES,
-            "Constants.ALL_SSL_OPTIONS_HASHES must be appended"
-            " with the sha256 hash of self.config.mod_ssl_conf when it is updated.")
+        import pkg_resources
+        tls_configs_dir = pkg_resources.resource_filename("certbot_apache", "tls_configs")
+        all_files = [os.path.join(tls_configs_dir, name) for name in os.listdir(tls_configs_dir)
+                     if name.endswith('options-ssl-apache.conf')]
+        self.assertTrue(all_files)
+        for one_file in all_files:
+            file_hash = crypto_util.sha256sum(one_file)
+            self.assertTrue(file_hash in ALL_SSL_OPTIONS_HASHES,
+                            "Constants.ALL_SSL_OPTIONS_HASHES must be appended with the sha256 "
+                            "hash of {0} when it is updated.".format(one_file))
 
 
 if __name__ == "__main__":

certbot-apache/certbot_apache/tests/debian_test.py

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

certbot-apache/certbot_apache/tests/fedora_test.py

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

certbot-apache/certbot_apache/tests/gentoo_test.py

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

certbot-apache/certbot_apache/tests/parser_test.py

@@ -1,8 +1,8 @@
+# pylint: disable=too-many-public-methods
 """Tests for certbot_apache.parser."""
 import shutil
 import unittest
 
-import augeas
 import mock
 
 from certbot import errors
@@ -22,6 +22,27 @@ class BasicParserTest(util.ParserTest):
         shutil.rmtree(self.config_dir)
         shutil.rmtree(self.work_dir)
 
+    def test_bad_parse(self):
+        self.parser.parse_file(os.path.join(self.parser.root,
+                                            "conf-available", "bad_conf_file.conf"))
+        self.assertRaises(
+            errors.PluginError, self.parser.check_parsing_errors, "httpd.aug")
+
+    def test_bad_save(self):
+        mock_save = mock.Mock()
+        mock_save.side_effect = IOError
+        self.parser.aug.save = mock_save
+        self.assertRaises(errors.PluginError, self.parser.unsaved_files)
+
+    def test_aug_version(self):
+        mock_match = mock.Mock(return_value=["something"])
+        self.parser.aug.match = mock_match
+        # pylint: disable=protected-access
+        self.assertEqual(self.parser.check_aug_version(),
+                         ["something"])
+        self.parser.aug.match.side_effect = RuntimeError
+        self.assertFalse(self.parser.check_aug_version())
+
     def test_find_config_root_no_root(self):
         # pylint: disable=protected-access
         os.remove(self.parser.loc["root"])
@@ -311,21 +332,38 @@ class BasicParserTest(util.ParserTest):
 class ParserInitTest(util.ApacheTest):
     def setUp(self):  # pylint: disable=arguments-differ
         super(ParserInitTest, self).setUp()
-        self.aug = augeas.Augeas(
-            flags=augeas.Augeas.NONE | augeas.Augeas.NO_MODL_AUTOLOAD)
 
     def tearDown(self):
         shutil.rmtree(self.temp_dir)
         shutil.rmtree(self.config_dir)
         shutil.rmtree(self.work_dir)
 
+    @mock.patch("certbot_apache.parser.ApacheParser.init_augeas")
+    def test_prepare_no_augeas(self, mock_init_augeas):
+        from certbot_apache.parser import ApacheParser
+        mock_init_augeas.side_effect = errors.NoInstallationError
+        self.config.config_test = mock.Mock()
+        self.assertRaises(
+            errors.NoInstallationError, ApacheParser,
+            os.path.relpath(self.config_path), "/dummy/vhostpath",
+            version=(2, 4, 22), configurator=self.config)
+
+    def test_init_old_aug(self):
+        from certbot_apache.parser import ApacheParser
+        with mock.patch("certbot_apache.parser.ApacheParser.check_aug_version") as mock_c:
+            mock_c.return_value = False
+            self.assertRaises(
+                errors.NotSupportedError,
+                ApacheParser, os.path.relpath(self.config_path),
+                "/dummy/vhostpath", version=(2, 4, 22), configurator=self.config)
+
     @mock.patch("certbot_apache.parser.ApacheParser._get_runtime_cfg")
     def test_unparseable(self, mock_cfg):
         from certbot_apache.parser import ApacheParser
         mock_cfg.return_value = ('Define: TEST')
         self.assertRaises(
             errors.PluginError,
-            ApacheParser, self.aug, os.path.relpath(self.config_path),
+            ApacheParser, os.path.relpath(self.config_path),
             "/dummy/vhostpath", version=(2, 2, 22), configurator=self.config)
 
     def test_root_normalized(self):
@@ -337,8 +375,7 @@ class ParserInitTest(util.ApacheTest):
                 self.temp_dir,
                 "debian_apache_2_4/////multiple_vhosts/../multiple_vhosts/apache2")
 
-            parser = ApacheParser(self.aug, path,
-                                  "/dummy/vhostpath", configurator=self.config)
+            parser = ApacheParser(path, "/dummy/vhostpath", configurator=self.config)
 
         self.assertEqual(parser.root, self.config_path)
 
@@ -347,7 +384,7 @@ class ParserInitTest(util.ApacheTest):
         with mock.patch("certbot_apache.parser.ApacheParser."
                         "update_runtime_variables"):
             parser = ApacheParser(
-                self.aug, os.path.relpath(self.config_path),
+                os.path.relpath(self.config_path),
                 "/dummy/vhostpath", configurator=self.config)
 
         self.assertEqual(parser.root, self.config_path)
@@ -357,7 +394,7 @@ class ParserInitTest(util.ApacheTest):
         with mock.patch("certbot_apache.parser.ApacheParser."
                         "update_runtime_variables"):
             parser = ApacheParser(
-                self.aug, self.config_path + os.path.sep,
+                self.config_path + os.path.sep,
                 "/dummy/vhostpath", configurator=self.config)
         self.assertEqual(parser.root, self.config_path)
 

certbot-apache/certbot_apache/tests/util.py

@@ -78,8 +78,7 @@ class ParserTest(ApacheTest):
         with mock.patch("certbot_apache.parser.ApacheParser."
                         "update_runtime_variables"):
             self.parser = ApacheParser(
-                self.aug, self.config_path, self.vhost_path,
-                configurator=self.config)
+                self.config_path, self.vhost_path, configurator=self.config)
 
 
 def get_apache_configurator(  # pylint: disable=too-many-arguments, too-many-locals

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

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

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

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

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

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

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

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

certbot-apache/setup.py

@@ -4,13 +4,13 @@ from setuptools.command.test import test as TestCommand
 import sys
 
 
-version = '0.36.0.dev0'
+version = '0.37.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.
 install_requires = [
     'acme>=0.29.0',
-    'certbot>=0.36.0.dev0',
+    'certbot>=0.37.0.dev0',
     '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.36.0"
 BASENAME=$(basename $0)
 USAGE="Usage: $BASENAME [OPTIONS]
 A self-updating wrapper script for the Certbot ACME client. When run, updates
@@ -1314,18 +1314,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.36.0 \
+    --hash=sha256:486cee6c4861762fe4a94b4f44f7d227034d026d1a8d7ba2911ef4e86a737613 \
+    --hash=sha256:bf6745b823644cdca8461150455aeb67d417f87f80b9ec774c716e9587ef20a2
+acme==0.36.0 \
+    --hash=sha256:5570c8e87383fbc733224fd0f7d164313b67dd9c21deafe9ddc8e769441f0c86 \
+    --hash=sha256:0461ee3c882d865e98e624561843dc135fa1a1412b15603d7ebfbb392de6a668
+certbot-apache==0.36.0 \
+    --hash=sha256:2537f7fb67a38b6d1ed5ee79f6a799090ca609695ac3799bb840b2fb677ac98d \
+    --hash=sha256:458d20a3e9e8a88563d3deb0bbe38752bd2b80100f0e5854e4069390c1b4e5cd
+certbot-nginx==0.36.0 \
+    --hash=sha256:4303b54adf2030671c54bb3964c1f43aec0f677045e0cdb6d4fb931268d08310 \
+    --hash=sha256:4c34e6114dd8204b6667f101579dd9ab2b38fef0dd5a15702585edcb2aefb322
 
 UNLIKELY_EOF
     # -------------------------------------------------------------------------

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,178 @@ 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
-
-    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)
+        self._launch_process(
+            [challtestsrv_path, '-management', ':{0}'.format(CHALLTESTSRV_PORT), '-defaultIPv6', '""',
+             '-defaultIPv4', '127.0.0.1', '-http01', '""', '-tlsalpn01', '""', '-https01', '""'])
 
         # 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 +194,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 +202,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/misc.py

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

certbot-ci/certbot_integration_tests/utils/pebble_artifacts.py

@@ -0,0 +1,52 @@
+import json
+import platform
+import os
+import stat
+
+import pkg_resources
+import requests
+
+PEBBLE_VERSION = 'v2.1.0'
+ASSETS_PATH = pkg_resources.resource_filename('certbot_integration_tests', 'assets')
+
+
+def fetch(workspace):
+    suffix = '{0}-{1}{2}'.format(platform.system().lower(),
+                                 platform.machine().lower().replace('x86_64', 'amd64'),
+                                 '.exe' if platform.system() == 'Windows' else '')
+
+    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',
+                'certificate': os.path.join(ASSETS_PATH, 'cert.pem'),
+                'privateKey': os.path.join(ASSETS_PATH, 'key.pem'),
+                'httpPort': 5002,
+                'tlsPort': 5001,
+            },
+        }))
+
+    return config_path

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.37.0.dev0'
 
 install_requires = [
     'certbot',

certbot-dns-cloudflare/Dockerfile

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

certbot-dns-cloudflare/certbot_dns_cloudflare/__init__.py

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

certbot-dns-cloudflare/certbot_dns_cloudflare/dns_cloudflare.py

@@ -10,7 +10,7 @@ from certbot.plugins import dns_common
 
 logger = logging.getLogger(__name__)
 
-ACCOUNT_URL = 'https://www.cloudflare.com/a/account/my-account'
+ACCOUNT_URL = 'https://dash.cloudflare.com/profile/api-tokens'
 
 
 @zope.interface.implementer(interfaces.IAuthenticator)

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.37.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.

certbot-dns-cloudxns/Dockerfile

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

certbot-dns-cloudxns/setup.py

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

certbot-dns-digitalocean/Dockerfile

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

certbot-dns-digitalocean/setup.py

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

certbot-dns-dnsimple/Dockerfile

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

certbot-dns-dnsimple/setup.py

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

certbot-dns-dnsmadeeasy/Dockerfile

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

certbot-dns-dnsmadeeasy/setup.py

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

certbot-dns-gehirn/Dockerfile

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

certbot-dns-gehirn/setup.py

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

certbot-dns-google/Dockerfile

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

certbot-dns-google/setup.py

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

certbot-dns-linode/Dockerfile

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

certbot-dns-linode/setup.py

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

certbot-dns-luadns/Dockerfile

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

certbot-dns-luadns/setup.py

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

certbot-dns-nsone/Dockerfile

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

certbot-dns-nsone/setup.py

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

certbot-dns-ovh/Dockerfile

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

certbot-dns-ovh/setup.py

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

certbot-dns-rfc2136/Dockerfile

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

certbot-dns-rfc2136/setup.py

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

certbot-dns-route53/Dockerfile

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

certbot-dns-route53/MANIFEST.in

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

certbot-dns-route53/setup.py

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

certbot-dns-sakuracloud/Dockerfile

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

certbot-dns-sakuracloud/setup.py

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

certbot-nginx/certbot_nginx/configurator.py

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

certbot-nginx/certbot_nginx/constants.py

@@ -24,6 +24,7 @@ UPDATED_MOD_SSL_CONF_DIGEST = ".updated-options-ssl-nginx-conf-digest.txt"
 
 SSL_OPTIONS_HASHES_NEW = [
     '63e2bddebb174a05c9d8a7cf2adf72f7af04349ba59a1a925fe447f73b2f1abf',
+    '2901debc7ecbc10917edd9084c05464c9c5930b463677571eaf8c94bffd11ae2',
 ]
 """SHA256 hashes of the contents of versions of MOD_SSL_CONF_SRC for nginx >= 1.5.9"""
 
@@ -34,6 +35,7 @@ ALL_SSL_OPTIONS_HASHES = [
     '7f95624dd95cf5afc708b9f967ee83a24b8025dc7c8d9df2b556bbc64256b3ff',
     '394732f2bbe3e5e637c3fb5c6e980a1f1b90b01e2e8d6b7cff41dde16e2a756d',
     '4b16fec2bcbcd8a2f3296d886f17f9953ffdcc0af54582452ca1e52f5f776f16',
+    'c052ffff0ad683f43bffe105f7c606b339536163490930e2632a335c8d191cc4',
 ] + SSL_OPTIONS_HASHES_NEW
 """SHA256 hashes of the contents of all versions of MOD_SSL_CONF_SRC"""
 

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

@@ -4,7 +4,7 @@
 # the up-to-date file that you will need to refer to when manually updating
 # this file.
 
-ssl_session_cache shared:le_nginx_SSL:1m;
+ssl_session_cache shared:le_nginx_SSL:10m;
 ssl_session_timeout 1440m;
 
 ssl_protocols TLSv1 TLSv1.1 TLSv1.2;

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

@@ -4,7 +4,7 @@
 # the up-to-date file that you will need to refer to when manually updating
 # this file.
 
-ssl_session_cache shared:le_nginx_SSL:1m;
+ssl_session_cache shared:le_nginx_SSL:10m;
 ssl_session_timeout 1440m;
 ssl_session_tickets off;
 

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")

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.37.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.
 install_requires = [
     'acme>=0.29.0',
-    'certbot>=0.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.37.0.dev0'

certbot/account.py

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

certbot/cert_manager.py

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

certbot/cli.py

@@ -418,8 +418,8 @@ VERB_HELP = [
     }),
     ("config_changes", {
         "short": "Show changes that Certbot has made to server configurations",
-        "opts": "Options for controlling which changes are displayed",
-        "usage": "\n\n  certbot config_changes --num NUM [options]\n\n"
+        "opts": "Options for viewing configuration changes",
+        "usage": "\n\n  certbot config_changes [options]\n\n"
     }),
     ("rollback", {
         "short": "Roll back server conf changes made during certificate installation",
@@ -428,7 +428,7 @@ VERB_HELP = [
     }),
     ("plugins", {
         "short": "List plugins that are installed and available on your system",
-        "opts": 'Options for for the "plugins" subcommand',
+        "opts": 'Options for the "plugins" subcommand',
         "usage": "\n\n  certbot plugins [options]\n\n"
     }),
     ("update_symlinks", {
@@ -1289,9 +1289,6 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
 
 
 def _create_subparsers(helpful):
-    helpful.add("config_changes", "--num", type=int, default=flag_default("num"),
-                help="How many past revisions you want to be displayed")
-
     from certbot.client import sample_user_agent # avoid import loops
     helpful.add(
         None, "--user-agent", default=flag_default("user_agent"),
@@ -1421,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)
@@ -702,7 +699,7 @@ def rollback(default_installer, checkpoints, config, plugins):
         installer.restart()
 
 
-def view_config_changes(config, num=None):
+def view_config_changes(config):
     """View checkpoints and associated configuration changes.
 
     .. note:: This assumes that the installation is using a Reverter object.
@@ -713,7 +710,7 @@ def view_config_changes(config, num=None):
     """
     rev = reverter.Reverter(config)
     rev.recovery_routine()
-    rev.view_config_changes(num)
+    rev.view_config_changes()
 
 def _open_pem_file(cli_arg_path, pem_path):
     """Open a pem file.

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,206 @@ 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)
+
+        try:
+            handle = win32file.CreateFile(file_path, win32file.GENERIC_READ,
+                                          win32file.FILE_SHARE_READ & win32file.FILE_SHARE_WRITE,
+                                          attributes, disposition, 0, None)
+            handle.Close()
+        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
+
+        # 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 +260,22 @@ def replace(src, dst):
         os.rename(src, dst)
 
 
-def _apply_win_mode(file_path, mode):
+def realpath(file_path):
     """
-    This function converts the given POSIX mode into a Windows ACL list, and applies it to the
-    file given its path. If the given path is a symbolic link, it will resolved to apply the
-    mode on the targeted file.
+    Find the real path for the given path. This method resolves symlinks, including
+    recursive symlinks, and is protected against symlinks that creates an infinite loop.
     """
     original_path = file_path
+
+    if POSIX_MODE:
+        path = os.path.realpath(file_path)
+        if os.path.islink(path):
+            # If path returned by realpath is still a link, it means that it failed to
+            # resolve the symlink because of a loop.
+            # See realpath code: https://github.com/python/cpython/blob/master/Lib/posixpath.py
+            raise RuntimeError('Error, link {0} is a loop!'.format(original_path))
+        return path
+
     inspected_paths = []  # type: List[str]
     while os.path.islink(file_path):
         link_path = file_path
@@ -68,6 +285,53 @@ def _apply_win_mode(file_path, mode):
         if file_path in inspected_paths:
             raise RuntimeError('Error, link {0} is a loop!'.format(original_path))
         inspected_paths.append(file_path)
+
+    return os.path.abspath(file_path)
+
+
+# On Windows is_executable run from an unprivileged shell may claim that a path is
+# executable when it is excutable only if run from a privileged shell. This result
+# is due to the fact that GetEffectiveRightsFromAcl calculate effective rights
+# without taking into consideration if the target user has currently required the
+# elevated privileges or not. However this is not a problem since certbot always
+# requires to be run under a privileged shell, so the user will always benefit
+# from the highest (privileged one) set of permissions on a given file.
+def is_executable(path):
+    """
+    Is path an executable file?
+    :param str path: path to test
+    :returns: True if path is an executable file
+    :rtype: bool
+    """
+    if POSIX_MODE:
+        return os.path.isfile(path) and os.access(path, os.X_OK)
+
+    return _win_is_executable(path)
+
+
+def _win_is_executable(path):
+    if not os.path.isfile(path):
+        return False
+
+    security = win32security.GetFileSecurity(path, win32security.DACL_SECURITY_INFORMATION)
+    dacl = security.GetSecurityDescriptorDacl()
+
+    mode = dacl.GetEffectiveRightsFromAcl({
+        'TrusteeForm': win32security.TRUSTEE_IS_SID,
+        'TrusteeType': win32security.TRUSTEE_IS_USER,
+        'Identifier': _get_current_user(),
+    })
+
+    return mode & ntsecuritycon.FILE_GENERIC_EXECUTE == ntsecuritycon.FILE_GENERIC_EXECUTE
+
+
+def _apply_win_mode(file_path, mode):
+    """
+    This function converts the given POSIX mode into a Windows ACL list, and applies it to the
+    file given its path. If the given path is a symbolic link, it will resolved to apply the
+    mode on the targeted file.
+    """
+    file_path = realpath(file_path)
     # Get owner sid of the file
     security = win32security.GetFileSecurity(file_path, win32security.OWNER_SECURITY_INFORMATION)
     user = security.GetSecurityDescriptorOwner()
@@ -129,6 +393,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 +442,28 @@ def _generate_windows_flags(rights_desc):
     return flag
 
 
+def _check_win_mode(file_path, mode):
+    # Resolve symbolic links
+    file_path = realpath(file_path)
+    # Get current dacl file
+    security = win32security.GetFileSecurity(file_path, win32security.OWNER_SECURITY_INFORMATION
+                                             | win32security.DACL_SECURITY_INFORMATION)
+    dacl = security.GetSecurityDescriptorDacl()
+
+    # Get current file owner sid
+    user = security.GetSecurityDescriptorOwner()
+
+    if not dacl:
+        # No DACL means full control to everyone
+        # This is not a deterministic permissions set.
+        return False
+
+    # Calculate the target dacl
+    ref_dacl = _generate_dacl(user, mode)
+
+    return _compare_dacls(dacl, ref_dacl)
+
+
 def _compare_dacls(dacl1, dacl2):
     """
     This method compare the two given DACLs to check if they are identical.
@@ -173,3 +471,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.')

certbot/constants.py

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