python3?

Fixes #7713.

As discussed in #7713, providing a Powershell script as hook for Certbot is not working currently. This is because hooks are run in a `cmd` environment, that recognizes only `.bat` files as valid scripts that can be run from their bare name on command line.

On the other hand, the Powershell both `.bat` and `.ps1` scripts as valid scripts.

This PR makes hooks command be executed by Powershell, instead of `cmd` as `Popen` does by default when `shell=true` is used. It also modifies the tests to handle this new environment, in particular in term of encoding (UTF-16-LE is the default one in Powershell).

* Run hooks in powershell on Windows

* Fix hook test

* Fallback to unittest.mock

* In fact, shell_cmd as a list of str could not work. Declare only str as acceptable input for shell_cmd.

* Added changelog

.azure-pipelines/INSTALL.md

@@ -0,0 +1,119 @@
+# Configuring Azure Pipelines with Certbot
+
+Let's begin. All pipelines are defined in `.azure-pipelines`. Currently there are two:
+* `.azure-pipelines/main.yml` is the main one, executed on PRs for master, and pushes to master,
+* `.azure-pipelines/advanced.yml` add installer testing on top of the main pipeline, and is executed for `test-*` branches, release branches, and nightly run for master.
+
+Several templates are defined in `.azure-pipelines/templates`. These YAML files aggregate common jobs configuration that can be reused in several pipelines.
+
+Unlike Travis, where CodeCov is working without any action required, CodeCov supports Azure Pipelines
+using the coverage-bash utility (not python-coverage for now) only if you provide the Codecov repo token
+using the `CODECOV_TOKEN` environment variable. So `CODECOV_TOKEN` needs to be set as a secured
+environment variable to allow the main pipeline to publish coverage reports to CodeCov.
+
+This INSTALL.md file explains how to configure Azure Pipelines with Certbot in order to execute the CI/CD logic defined in `.azure-pipelines` folder with it.
+During this installation step, warnings describing user access and legal comitments will be displayed like this:
+```
+!!! ACCESS REQUIRED !!!
+```
+
+This document suppose that the Azure DevOps organization is named _certbot_, and the Azure DevOps project is also _certbot_.
+
+## Useful links
+
+* https://docs.microsoft.com/en-us/azure/devops/pipelines/yaml-schema?view=azure-devops&tabs=schema
+* https://www.azuredevopslabs.com/labs/azuredevops/github-integration/
+* https://docs.microsoft.com/en-us/azure/devops/pipelines/ecosystems/python?view=azure-devops
+
+## Prerequisites
+
+### Having a GitHub account
+
+Use your GitHub user for a normal GitHub account, or a user that has administrative rights to the GitHub organization if relevant.
+
+### Having an Azure DevOps account
+- Go to https://dev.azure.com/, click "Start free with GitHub"
+- Login to GitHub
+
+```
+!!! ACCESS REQUIRED !!!
+Personal user data (email + profile info, in read-only)
+```
+
+- Microsoft will create a Live account using the email referenced for the GitHub account. This account is also linked to GitHub account (meaning you can log it using GitHub authentication)
+- Proceed with account registration (birth date, country), add details about name and email contact
+
+```
+!!! ACCESS REQUIRED !!!
+Microsoft proposes to send commercial links to this mail
+Azure DevOps terms of service need to be accepted
+```
+
+_Logged to Azure DevOps, account is ready._
+
+### Installing Azure Pipelines to GitHub
+
+- On GitHub, go to Marketplace
+- Select Azure Pipeline, and "Set up a plan"
+- Select Free, then "Install it for free"
+- Click "Complete order and begin installation"
+
+```
+!!! ACCESS !!!
+Azure Pipeline needs RW on code, RO on metadata, RW on checks, commit statuses, deployments, issues, pull requests.
+RW access here is required to allow update of the pipelines YAML files from Azure DevOps interface, and to
+update the status of builds and PRs on GitHub side when Azure Pipelines are triggered.
+Note however that no admin access is defined here: this means that Azure Pipelines cannot do anything with
+protected branches, like master, and cannot modify the security context around this on GitHub.
+Access can be defined for all or only selected repositories, which is nice.
+```
+
+- Redirected to Azure DevOps, select the account created in _Having an Azure DevOps account_ section.
+- Select the organization, and click "Create a new project" (let's name it the same than the targeted github repo)
+- The Visibility is public, to profit from 10 parallel jobs
+
+```
+!!! ACCESS !!!
+Azure Pipelines needs access to the GitHub account (in term of being able to check it is valid), and the Resources shared between the GitHub account and Azure Pipelines.
+```
+
+_Done. We can move to pipelines configuration._
+
+## Import an existing pipelines from `.azure-pipelines` folder
+
+- On Azure DevOps, go to your organization (eg. _certbot_) then your project (eg. _certbot_)
+- Click "Pipelines" tab
+- Click "New pipeline"
+- Where is your code?: select "__Use the classic editor__"
+
+__Warning: Do not choose the GitHub option in Where is your code? section. Indeed, this option will trigger an OAuth
+grant permissions from Azure Pipelines to GitHub in order to setup a GitHub OAuth Application. The permissions asked
+then are way too large (admin level on almost everything), while the classic approach does not add any more
+permissions, and works perfectly well.__
+
+- Select GitHub in "Select your repository section", choose certbot/certbot in Repository, master in default branch.
+- Click on YAML option for "Select a template"
+- Choose a name for the pipeline (eg. test-pipeline), and browse to the actual pipeline YAML definition in the
+  "YAML file path" input (eg. `.azure-pipelines/test-pipeline.yml`)
+- Click "Save & queue", choose the master branch to build the first pipeline, and click "Save and run" button.
+
+_Done. Pipeline is operational. Repeat to add more pipelines from existing YAML files in `.azure-pipelines`._
+
+## Add a secret variable to a pipeline (like `CODECOV_TOKEN`)
+
+__NB: Following steps suppose that you already setup the YAML pipeline file to
+consume the secret variable that these steps will create as an environment variable.
+For a variable named `CODECOV_TOKEN` consuming the variable `codecov_token`,
+in the YAML file this setup would take the form of the following:
+```
+steps:
+    - script: ./do_something_that_consumes_CODECOV_TOKEN  # Eg. `codecov -F windows`
+      env:
+        CODECOV_TOKEN: $(codecov_token)
+```
+
+To set up a variable that is shared between pipelines, follow the instructions
+at
+https://docs.microsoft.com/en-us/azure/devops/pipelines/library/variable-groups.
+When adding variables to a group, don't forget to tick "Keep this value secret"
+if it shouldn't be shared publcily.

.azure-pipelines/advanced-test.yml

@@ -0,0 +1,13 @@
+# Advanced pipeline for running our full test suite on demand.
+trigger:
+  # When changing these triggers, please ensure the documentation under
+  # "Running tests in CI" is still correct.
+  - azure-test-*
+  - test-*
+pr: none
+
+jobs:
+  # Any addition here should be reflected in the advanced and release pipelines.
+  # It is advised to declare all jobs here as templates to improve maintainability.
+  - template: templates/tests-suite.yml
+  - template: templates/installer-tests.yml

.azure-pipelines/advanced.yml

@@ -0,0 +1,18 @@
+# Advanced pipeline for running our full test suite on protected branches.
+trigger:
+  - '*.x'
+pr: none
+# This pipeline is also nightly run on master
+schedules:
+  - cron: "0 4 * * *"
+    displayName: Nightly build
+    branches:
+      include:
+      - master
+    always: true
+
+jobs:
+  # Any addition here should be reflected in the advanced-test and release pipelines.
+  # It is advised to declare all jobs here as templates to improve maintainability.
+  - template: templates/tests-suite.yml
+  - template: templates/installer-tests.yml

.azure-pipelines/main.yml

@@ -0,0 +1,8 @@
+trigger:
+  - master
+pr:
+  - master
+  - '*.x'
+
+jobs:
+  - template: templates/tests-suite.yml

.azure-pipelines/release.yml

@@ -0,0 +1,13 @@
+# Release pipeline to build and deploy Certbot for Windows for GitHub release tags
+trigger:
+  tags:
+    include:
+      - v*
+pr: none
+
+jobs:
+  # Any addition here should be reflected in the advanced and advanced-test pipelines.
+  # It is advised to declare all jobs here as templates to improve maintainability.
+  - template: templates/tests-suite.yml
+  - template: templates/installer-tests.yml
+  - template: templates/changelog.yml

.azure-pipelines/templates/changelog.yml

@@ -0,0 +1,14 @@
+jobs:
+  - job: changelog
+    pool:
+      vmImage: vs2017-win2016
+    steps:
+      - bash: |
+          CERTBOT_VERSION="$(cd certbot && python -c "import certbot; print(certbot.__version__)" && cd ~-)"
+          "${BUILD_REPOSITORY_LOCALPATH}\tools\extract_changelog.py" "${CERTBOT_VERSION}" >> "${BUILD_ARTIFACTSTAGINGDIRECTORY}/release_notes.md"
+        displayName: Prepare changelog
+      - task: PublishPipelineArtifact@1
+        inputs:
+          path: $(Build.ArtifactStagingDirectory)
+          artifact: changelog
+        displayName: Publish changelog

.azure-pipelines/templates/installer-tests.yml

@@ -0,0 +1,61 @@
+jobs:
+  - job: installer_build
+    pool:
+      vmImage: vs2017-win2016
+    steps:
+      - task: UsePythonVersion@0
+        inputs:
+          versionSpec: 3.7
+          architecture: x86
+          addToPath: true
+      - script: python windows-installer/construct.py
+        displayName: Build Certbot installer
+      - task: CopyFiles@2
+        inputs:
+          sourceFolder: $(System.DefaultWorkingDirectory)/windows-installer/build/nsis
+          contents: '*.exe'
+          targetFolder: $(Build.ArtifactStagingDirectory)
+      - task: PublishPipelineArtifact@1
+        inputs:
+          path: $(Build.ArtifactStagingDirectory)
+          artifact: windows-installer
+        displayName: Publish Windows installer
+  - job: installer_run
+    dependsOn: installer_build
+    strategy:
+      matrix:
+        win2019:
+          imageName: windows-2019
+        win2016:
+          imageName: vs2017-win2016
+    pool:
+      vmImage: $(imageName)
+    steps:
+      - powershell: |
+          $currentVersion = $PSVersionTable.PSVersion
+          if ($currentVersion.Major -ne 5) {
+              throw "Powershell version is not 5.x"
+          }
+        condition: eq(variables['imageName'], 'vs2017-win2016')
+        displayName: Check Powershell 5.x is used in vs2017-win2016
+      - task: UsePythonVersion@0
+        inputs:
+          versionSpec: 3.8
+          addToPath: true
+      - task: DownloadPipelineArtifact@2
+        inputs:
+          artifact: windows-installer
+          path: $(Build.SourcesDirectory)/bin
+        displayName: Retrieve Windows installer
+      - script: |
+          py -3 -m venv venv
+          venv\Scripts\python tools\pip_install.py -e certbot-ci
+        displayName: Prepare Certbot-CI
+      - script: |
+          set PATH=%ProgramFiles(x86)%\Certbot\bin;%PATH%
+          venv\Scripts\python -m pytest certbot-ci\windows_installer_integration_tests --allow-persistent-changes --installer-path $(Build.SourcesDirectory)\bin\certbot-beta-installer-win32.exe
+        displayName: Run windows installer integration tests
+      - script: |
+          set PATH=%ProgramFiles(x86)%\Certbot\bin;%PATH%
+          venv\Scripts\python -m pytest certbot-ci\certbot_integration_tests\certbot_tests -n 4
+        displayName: Run certbot integration tests

.azure-pipelines/templates/tests-suite.yml

@@ -0,0 +1,39 @@
+jobs:
+  - job: test
+    strategy:
+      matrix:
+        macos-py27:
+          IMAGE_NAME: macOS-10.14
+          PYTHON_VERSION: 2.7
+          TOXENV: py27
+        macos-py38:
+          IMAGE_NAME: macOS-10.14
+          PYTHON_VERSION: 3.8
+          TOXENV: py38
+        windows-py35:
+          IMAGE_NAME: vs2017-win2016
+          PYTHON_VERSION: 3.5
+          TOXENV: py35
+        windows-py37-cover:
+          IMAGE_NAME: vs2017-win2016
+          PYTHON_VERSION: 3.7
+          TOXENV: py37-cover
+        windows-integration-certbot:
+          IMAGE_NAME: vs2017-win2016
+          PYTHON_VERSION: 3.7
+          TOXENV: integration-certbot
+          PYTEST_ADDOPTS: --numprocesses 4
+    pool:
+      vmImage: $(IMAGE_NAME)
+    steps:
+    - bash: brew install augeas
+      condition: startswith(variables['IMAGE_NAME'], 'macOS')
+      displayName: Install Augeas
+    - task: UsePythonVersion@0
+      inputs:
+        versionSpec: $(PYTHON_VERSION)
+        addToPath: true
+    - script: python tools/pip_install.py -U tox coverage
+      displayName: Install dependencies
+    - script: python -m tox
+      displayName: Run tox

.coveragerc

@@ -1,2 +1,5 @@
+[run]
+omit = */setup.py
+
 [report]
 omit = */setup.py

.github/stale.yml

@@ -0,0 +1,35 @@
+# Configuration for https://github.com/marketplace/stale
+
+# Number of days of inactivity before an Issue or Pull Request becomes stale
+daysUntilStale: 365
+
+# Number of days of inactivity before an Issue or Pull Request with the stale label is closed.
+# Set to false to disable. If disabled, issues still need to be closed manually, but will remain marked as stale.
+# When changing this value, be sure to also update markComment below.
+daysUntilClose: 30
+
+# Ignore issues with an assignee (defaults to false)
+exemptAssignees: true
+
+# Label to use when marking as stale
+staleLabel: needs-update
+
+# Comment to post when marking as stale. Set to `false` to disable
+markComment: >
+  We've made a lot of changes to Certbot since this issue was opened. If you
+  still have this issue with an up-to-date version of Certbot, can you please
+  add a comment letting us know? This helps us to better see what issues are
+  still affecting our users. If there is no activity in the next 30 days, this
+  issue will be automatically closed.
+
+# Comment to post when closing a stale Issue or Pull Request.
+closeComment: >
+  This issue has been closed due to lack of activity, but if you think it
+  should be reopened, please open a new issue with a link to this one and we'll
+  take a look.
+
+# Limit the number of actions per hour, from 1-30. Default is 30
+limitPerRun: 1
+
+# Don't mark pull requests as stale.
+only: issues

.gitignore

@@ -26,6 +26,7 @@ tags
 \#*#
 .idea
 .ropeproject
+.vscode
 
 # auth --cert-path --chain-path
 /*.pem
@@ -34,6 +35,7 @@ tags
 tests/letstest/letest-*/
 tests/letstest/*.pem
 tests/letstest/venv/
+tests/letstest/venv3/
 
 .venv
 
@@ -44,3 +46,15 @@ tests/letstest/venv/
 
 # docker files
 .docker
+
+# certbot tests
+.certbot_test_workspace
+**/assets/pebble*
+**/assets/challtestsrv*
+
+# snap files
+.snapcraft
+parts
+prime
+stage
+*.snap

.isort.cfg

@@ -0,0 +1,7 @@
+[settings]
+skip_glob=venv*
+skip=letsencrypt-auto-source
+force_sort_within_sections=True
+force_single_line=True
+order_by_type=False
+line_length=400

.pylintrc

@@ -24,6 +24,11 @@ persistent=yes
 # usually to register additional checkers.
 load-plugins=linter_plugin
 
+# A comma-separated list of package or module names from where C extensions may
+# be loaded. Extensions are loading into the active Python interpreter and may
+# run arbitrary code.
+extension-pkg-whitelist=pywintypes,win32api,win32file,win32security
+
 
 [MESSAGES CONTROL]
 
@@ -41,10 +46,14 @@ load-plugins=linter_plugin
 # --enable=similarities". If you want to run only the classes checker, but have
 # no Warning level messages displayed, use"--disable=all --enable=classes
 # --disable=W"
-disable=fixme,locally-disabled,locally-enabled,abstract-class-not-used,abstract-class-little-used,bad-continuation,too-few-public-methods,no-self-use,invalid-name,too-many-instance-attributes,cyclic-import,duplicate-code
-# abstract-class-not-used cannot be disabled locally (at least in
-# pylint 1.4.1), same for abstract-class-little-used
-
+# CERTBOT COMMENT
+# 1) Once certbot codebase is claimed to be compatible exclusively with Python 3,
+#    the useless-object-inheritance check can be enabled again, and code fixed accordingly.
+# 2) Check unsubscriptable-object tends to create a lot of false positives. Let's disable it.
+#    See https://github.com/PyCQA/pylint/issues/1498.
+# 3) Same as point 2 for no-value-for-parameter.
+#    See https://github.com/PyCQA/pylint/issues/2820.
+disable=fixme,locally-disabled,locally-enabled,bad-continuation,no-self-use,invalid-name,cyclic-import,duplicate-code,design,import-outside-toplevel,useless-object-inheritance,unsubscriptable-object,no-value-for-parameter,no-else-return,no-else-raise,no-else-break,no-else-continue
 
 [REPORTS]
 
@@ -251,7 +260,7 @@ ignored-modules=pkg_resources,confargparse,argparse,six.moves,six.moves.urllib
 
 # List of classes names for which member attributes should not be checked
 # (useful for classes with attributes dynamically set).
-ignored-classes=SQLObject
+ignored-classes=Field,Header,JWS,closing
 
 # When zope mode is activated, add a predefined set of Zope acquired attributes
 # to generated-members.
@@ -297,40 +306,6 @@ valid-classmethod-first-arg=cls
 valid-metaclass-classmethod-first-arg=mcs
 
 
-[DESIGN]
-
-# Maximum number of arguments for function / method
-max-args=6
-
-# Argument names that match this expression will be ignored. Default to name
-# with leading underscore
-ignored-argument-names=_.*
-
-# Maximum number of locals for function / method body
-max-locals=15
-
-# Maximum number of return / yield for function / method body
-max-returns=6
-
-# Maximum number of branch for function / method body
-max-branches=12
-
-# Maximum number of statements in function / method body
-max-statements=50
-
-# Maximum number of parents for a class (see R0901).
-max-parents=12
-
-# Maximum number of attributes for a class (see R0902).
-max-attributes=7
-
-# Minimum number of public methods for a class (see R0903).
-min-public-methods=2
-
-# Maximum number of public methods for a class (see R0904).
-max-public-methods=20
-
-
 [EXCEPTIONS]
 
 # Exceptions that will emit a warning when being caught. Defaults to

.travis.yml

@@ -1,205 +1,90 @@
 language: python
+dist: xenial
 
 cache:
     directories:
         - $HOME/.cache/pip
 
 before_script:
-  - 'if [ $TRAVIS_OS_NAME = osx ] ; then ulimit -n 1024 ; fi'
+  # On Travis, the fastest parallelization for integration tests has proved to be 4.
+  - 'if [[ "$TOXENV" == *"integration"* ]]; then export PYTEST_ADDOPTS="--numprocesses 4"; fi'
+  # Use Travis retry feature for farm tests since they are flaky
+  - 'if [[ "$TOXENV" == "travis-test-farm"* ]]; then export TRAVIS_RETRY=travis_retry; fi'
   - export TOX_TESTENV_PASSENV=TRAVIS
+  - 'if [[ "$SNAP" == true ]]; then snap/local/build_and_install.sh; fi'
 
 # Only build pushes to the master branch, PRs, and branches beginning with
-# `test-` or of the form `digit(s).digit(s).x`. This reduces the number of
-# simultaneous Travis runs, which speeds turnaround time on review since there
-# is a cap of on the number of simultaneous runs.
+# `test-`, `travis-test-`, or of the form `digit(s).digit(s).x` or
+# `vdigit(s).digit(s).digit(s)`. As documented at
+# https://docs.travis-ci.com/user/customizing-the-build/#safelisting-or-blocklisting-branches,
+# this includes tags so pushing tags of the form `vdigit(s).digit(s).digit(s)`
+# will also trigger tests. This reduces the number of simultaneous Travis runs,
+# which speeds turnaround time on review since there is a cap of on the number
+# of simultaneous runs.
 branches:
+  # When changing these branches, please ensure the documentation under
+  # "Running tests in CI" is still correct.
   only:
     - master
-    - /^\d+\.\d+\.x$/
-    - /^test-.*$/
+    - /^\d+\.\d+\.x$/  # this matches our point release branches
+    - /^v\d+\.\d+\.\d+$/  # this matches our release tags
+    - /^(travis-)?test-.*$/
 
 # Jobs for the main test suite are always executed (including on PRs) except for pushes on master.
 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.
 extended-test-suite: &extended-test-suite
   if: type = cron OR (type = push AND branch != master)
 
 matrix:
   include:
-    # Main test suite
-    - python: "2.7"
-      env: BOULDER_INTEGRATION=v1 INTEGRATION_TEST=all TOXENV=py27_install
+    - stage: "Snap"
       sudo: required
-      services: docker
-      <<: *not-on-master
-    - python: "2.7"
-      env: BOULDER_INTEGRATION=v2 INTEGRATION_TEST=all TOXENV=py27_install
-      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"
-
-    - sudo: required
-      env: TOXENV=nginx_compat
-      services: docker
-      before_install:
-      addons:
-      <<: *not-on-master
-    - 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,postfix}-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
-    - python: "3.7"
-      dist: xenial
-      env: TOXENV=py37 CERTBOT_NO_PIN=1
-      <<: *extended-test-suite
-    - python: "2.7"
-      env: BOULDER_INTEGRATION=v1 INTEGRATION_TEST=certbot TOXENV=py27-certbot-oldest
-      sudo: required
-      services: docker
-      <<: *extended-test-suite
-    - python: "2.7"
-      env: BOULDER_INTEGRATION=v2 INTEGRATION_TEST=certbot TOXENV=py27-certbot-oldest
-      sudo: required
-      services: docker
-      <<: *extended-test-suite
-    - python: "2.7"
-      env: BOULDER_INTEGRATION=v1 INTEGRATION_TEST=nginx TOXENV=py27-nginx-oldest
-      sudo: required
-      services: docker
-      <<: *extended-test-suite
-    - python: "2.7"
-      env: BOULDER_INTEGRATION=v2 INTEGRATION_TEST=nginx TOXENV=py27-nginx-oldest
-      sudo: required
-      services: docker
-      <<: *extended-test-suite
-    - python: "3.4"
-      env: TOXENV=py34 BOULDER_INTEGRATION=v1
-      sudo: required
-      services: docker
-      <<: *extended-test-suite
-    - python: "3.4"
-      env: TOXENV=py34 BOULDER_INTEGRATION=v2
-      sudo: required
-      services: docker
-      <<: *extended-test-suite
-    - python: "3.5"
-      env: TOXENV=py35 BOULDER_INTEGRATION=v1
-      sudo: required
-      services: docker
-      <<: *extended-test-suite
-    - python: "3.5"
-      env: TOXENV=py35 BOULDER_INTEGRATION=v2
-      sudo: required
-      services: docker
-      <<: *extended-test-suite
-    - python: "3.6"
-      env: TOXENV=py36 BOULDER_INTEGRATION=v1
-      sudo: required
-      services: docker
-      <<: *extended-test-suite
-    - python: "3.6"
-      env: TOXENV=py36 BOULDER_INTEGRATION=v2
-      sudo: required
-      services: docker
-      <<: *extended-test-suite
-    - python: "3.7"
-      dist: xenial
-      env: TOXENV=py37 BOULDER_INTEGRATION=v1
-      sudo: required
-      services: docker
-      <<: *extended-test-suite
-    - python: "3.7"
-      dist: xenial
-      env: TOXENV=py37 BOULDER_INTEGRATION=v2
-      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
+      env: SNAP=true TOXENV=integration-external,apacheconftest-external-with-pebble
       addons:
         apt:
-          packages:  # don't install nginx and apache
-            - libaugeas0
-      <<: *extended-test-suite
-    - language: generic
-      env: TOXENV=py27
-      os: osx
-      addons:
-        homebrew:
           packages:
-            - augeas
-            - python2
-      <<: *extended-test-suite
-    - language: generic
-      env: TOXENV=py3
-      os: osx
-      addons:
-        homebrew:
-          packages:
-            - augeas
-            - python3
+          - nginx-light
+        snaps:
+          - name: snapcraft
+            channel: stable
+            confinement: classic
+          - name: lxd
+            channel: stable
+      git:
+        # By default, Travis clones the repo to a depth of 50 commits which can
+        # break the ability to use `git describe` to set the version of the
+        # snap. This setting removes the --depth flag from git commands solving
+        # this problem. See
+        # https://docs.travis-ci.com/user/customizing-the-build#git-clone-depth
+        # for more info.
+        depth: false
+      deploy:
+        # This section relies on credentials stored in a SNAP_TOKEN environment
+        # variable in Travis. See
+        # https://docs.travis-ci.com/user/deployment/snaps/ for more info.
+        # This credential has a maximum lifetime of 1 year and the current
+        # credential will expire on 4/22/2021. The value of SNAP_TOKEN will
+        # need to be updated to use a new credential before then to prevent
+        # automated deploys from breaking. Remembering to do this is also
+        # tracked by https://github.com/certbot/certbot/issues/7931.
+        'on':
+          # Deploy on release tags or nightly runs from any branch. We only try
+          # to deploy from the certbot/certbot repo to prevent errors if forks
+          # of this repo try to run tests.
+          all_branches: true
+          condition: -n $TRAVIS_TAG || $TRAVIS_EVENT_TYPE = cron
+          repo: certbot/certbot
+        provider: snap
+        snap: certbot_*.snap
+        channel: edge
+        # skip_cleanup is needed to prevent Travis from deleting the snaps we
+        # just built and tested. See
+        # https://docs.travis-ci.com/user/deployment#uploading-files-and-skip_cleanup.
+        skip_cleanup: true
       <<: *extended-test-suite
 
 # container-based infrastructure
@@ -209,7 +94,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
@@ -219,19 +103,30 @@ addons:
     - nginx-light
     - openssl
 
-install: "$(command -v pip || command -v pip3) install codecov tox"
-script:
-    - tox
-    - '[ -z "${BOULDER_INTEGRATION+x}" ] || (tests/boulder-fetch.sh && tests/tox-boulder-integration.sh)'
-
-after_success: '[ "$TOXENV" == "py27-cover" ] && codecov'
+# 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. The option "-I" is set so when CERTBOT_NO_PIN is also
+# set, pip updates dependencies it thinks are already satisfied to avoid some
+# problems with its lack of real dependency resolution.
+install: 'tools/pip_install.py -I tox virtualenv'
+# Most of the time TRAVIS_RETRY is an empty string, and has no effect on the
+# script command. It is set only to `travis_retry` during farm tests, in
+# order to trigger the Travis retry feature, and compensate the inherent
+# flakiness of these specific tests.
+script: '$TRAVIS_RETRY tox'
 
 notifications:
   email: false
   irc:
+    if: NOT branch =~ ^(travis-)?test-.*$
     channels:
-      - secure: "SGWZl3ownKx9xKVV2VnGt7DqkTmutJ89oJV9tjKhSs84kLijU6EYdPnllqISpfHMTxXflNZuxtGo0wTDYHXBuZL47w1O32W6nzuXdra5zC+i4sYQwYULUsyfOv9gJX8zWAULiK0Z3r0oho45U+FR5ZN6TPCidi8/eGU+EEPwaAw="
+      # 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
-    use_notice: true

AUTHORS.md

@@ -5,6 +5,7 @@ Authors
 * Aaron Zuehlke
 * Ada Lovelace
 * [Adam Woodbeck](https://github.com/awoodbeck)
+* [Adrien Ferrand](https://github.com/adferrand)
 * [Aidin Gharibnavaz](https://github.com/aidin36)
 * [AJ ONeal](https://github.com/coolaj86)
 * [Alcaro](https://github.com/Alcaro)
@@ -14,10 +15,13 @@ 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)
+* [Andrzej Górski](https://github.com/andrzej3393)
 * [Anselm Levskaya](https://github.com/levskaya)
 * [Antoine Jacoutot](https://github.com/ajacoutot)
+* [April King](https://github.com/april)
 * [asaph](https://github.com/asaph)
 * [Axel Beckert](https://github.com/xtaran)
 * [Bas](https://github.com/Mechazawa)
@@ -33,6 +37,7 @@ Authors
 * [Brad Warren](https://github.com/bmw)
 * [Brandon Kraft](https://github.com/kraftbj)
 * [Brandon Kreisel](https://github.com/kraftbj)
+* [Cameron Steel](https://github.com/Tugzrida)
 * [Ceesjan Luiten](https://github.com/quinox)
 * [Chad Whitacre](https://github.com/whit537)
 * [Chhatoi Pritam Baral](https://github.com/pritambaral)
@@ -75,6 +80,7 @@ Authors
 * [Fabian](https://github.com/faerbit)
 * [Faidon Liambotis](https://github.com/paravoid)
 * [Fan Jiang](https://github.com/tcz001)
+* [Felix Lechner](https://github.com/lechner)
 * [Felix Schwarz](https://github.com/FelixSchwarz)
 * [Felix Yan](https://github.com/felixonmars)
 * [Filip Ochnik](https://github.com/filipochnik)
@@ -96,7 +102,9 @@ Authors
 * [Harlan Lieberman-Berg](https://github.com/hlieberman)
 * [Henri Salo](https://github.com/fgeek)
 * [Henry Chen](https://github.com/henrychen95)
+* [Hugo van Kemenade](https://github.com/hugovk)
 * [Ingolf Becker](https://github.com/watercrossing)
+* [Ivan Nejgebauer](https://github.com/inejge)
 * [Jaap Eldering](https://github.com/eldering)
 * [Jacob Hoffman-Andrews](https://github.com/jsha)
 * [Jacob Sachs](https://github.com/jsachs)
@@ -120,10 +128,12 @@ Authors
 * [Jonathan Herlin](https://github.com/Jonher937)
 * [Jon Walsh](https://github.com/code-tree)
 * [Joona Hoikkala](https://github.com/joohoi)
+* [Josh McCullough](https://github.com/JoshMcCullough)
 * [Josh Soref](https://github.com/jsoref)
 * [Joubin Jabbari](https://github.com/joubin)
 * [Juho Juopperi](https://github.com/jkjuopperi)
 * [Kane York](https://github.com/riking)
+* [Kenichi Maehashi](https://github.com/kmaehashi)
 * [Kenneth Skovhede](https://github.com/kenkendk)
 * [Kevin Burke](https://github.com/kevinburke)
 * [Kevin London](https://github.com/kevinlondon)
@@ -159,8 +169,10 @@ Authors
 * [Michael Schumacher](https://github.com/schumaml)
 * [Michael Strache](https://github.com/Jarodiv)
 * [Michael Sverdlin](https://github.com/sveder)
+* [Michael Watters](https://github.com/blackknight36)
 * [Michal Moravec](https://github.com/https://github.com/Majkl578)
 * [Michal Papis](https://github.com/mpapis)
+* [Mickaël Schoentgen](https://github.com/BoboTiG)
 * [Minn Soe](https://github.com/MinnSoe)
 * [Min RK](https://github.com/minrk)
 * [Miquel Ruiz](https://github.com/miquelruiz)
@@ -224,6 +236,7 @@ Authors
 * [Stavros Korokithakis](https://github.com/skorokithakis)
 * [Stefan Weil](https://github.com/stweil)
 * [Steve Desmond](https://github.com/stevedesmond-ca)
+* [sydneyli](https://github.com/sydneyli)
 * [Tan Jay Jun](https://github.com/jayjun)
 * [Tapple Gao](https://github.com/tapple)
 * [Telepenin Nikolay](https://github.com/telepenin)
@@ -255,5 +268,6 @@ Authors
 * [Yomna](https://github.com/ynasser)
 * [Yoni Jah](https://github.com/yonjah)
 * [YourDaddyIsHere](https://github.com/YourDaddyIsHere)
+* [Yuseong Cho](https://github.com/g6123)
 * [Zach Shepherd](https://github.com/zjs)
 * [陈三](https://github.com/chenxsan)

CHANGELOG.md

@@ -0,0 +1 @@
+certbot/CHANGELOG.md

CODE_OF_CONDUCT.md

@@ -0,0 +1 @@
+This project is governed by [EFF's Public Projects Code of Conduct](https://www.eff.org/pages/eppcode).

CONTRIBUTING.md

@@ -33,3 +33,5 @@ started. In particular, we recommend you read these sections
  - [Finding issues to work on](https://certbot.eff.org/docs/contributing.html#find-issues-to-work-on)
  - [Coding style](https://certbot.eff.org/docs/contributing.html#coding-style)
  - [Submitting a pull request](https://certbot.eff.org/docs/contributing.html#submitting-a-pull-request)
+ - [EFF's Public Projects Code of Conduct](https://www.eff.org/pages/eppcode)
+

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,21 +1,20 @@
 # 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
 
 WORKDIR /opt/certbot/src
 
-# TODO: Install Apache/Nginx for plugin development.
 COPY . .
 RUN apt-get update && \
-    apt-get install apache2 git nginx-light -y && \
-    letsencrypt-auto-source/letsencrypt-auto --os-packages-only && \
+    apt-get install apache2 git python3-dev python3-venv gcc libaugeas0 \
+                    libssl-dev libffi-dev ca-certificates openssl nginx-light -y && \
     apt-get clean && \
     rm -rf /var/lib/apt/lists/* \
            /tmp/* \
            /var/tmp/*
 
-RUN VENV_NAME="../venv" python tools/venv.py
+RUN VENV_NAME="../venv3" python3 tools/venv3.py
 
-ENV PATH /opt/certbot/venv/bin:$PATH
+ENV PATH /opt/certbot/venv3/bin:$PATH

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

README.rst

@@ -1,157 +0,0 @@
-.. This file contains a series of comments that are used to include sections of this README in other files. Do not modify these comments unless you know what you are doing. tag:intro-begin
-
-Certbot is part of EFF’s effort to encrypt the entire Internet. Secure communication over the Web relies on HTTPS, which requires the use of a digital certificate that lets browsers verify the identity of web servers (e.g., is that really google.com?). Web servers obtain their certificates from trusted third parties called certificate authorities (CAs). Certbot is an easy-to-use client that fetches a certificate from Let’s Encrypt—an open certificate authority launched by the EFF, Mozilla, and others—and deploys it to a web server.
-
-Anyone who has gone through the trouble of setting up a secure website knows what a hassle getting and maintaining a certificate is. Certbot and Let’s Encrypt can automate away the pain and let you turn on and manage HTTPS with simple commands. Using Certbot and Let's Encrypt is free, so there’s no need to arrange payment.
-
-How you use Certbot depends on the configuration of your web server. The best way to get started is to use our `interactive guide <https://certbot.eff.org>`_. It generates instructions based on your configuration settings. In most cases, you’ll need `root or administrator access <https://certbot.eff.org/faq/#does-certbot-require-root-administrator-privileges>`_ to your web server to run Certbot.
-
-Certbot is meant to be run directly on your web server, not on your personal computer. If you’re using a hosted service and don’t have direct access to your web server, you might not be able to use Certbot. Check with your hosting provider for documentation about uploading certificates or using certificates issued by Let’s Encrypt.
-
-Certbot is a fully-featured, extensible client for the Let's
-Encrypt CA (or any other CA that speaks the `ACME
-<https://github.com/ietf-wg-acme/acme/blob/master/draft-ietf-acme-acme.md>`_
-protocol) that can automate the tasks of obtaining certificates and
-configuring webservers to use them. This client runs on Unix-based operating
-systems.
-
-To see the changes made to Certbot between versions please refer to our
-`changelog <https://github.com/certbot/certbot/blob/master/CHANGELOG.md>`_.
-
-Until May 2016, Certbot was named simply ``letsencrypt`` or ``letsencrypt-auto``,
-depending on install method. Instructions on the Internet, and some pieces of the
-software, may still refer to this older name.
-
-Contributing
-------------
-
-If you'd like to contribute to this project please read `Developer Guide
-<https://certbot.eff.org/docs/contributing.html>`_.
-
-.. _installation:
-
-Installation
-------------
-
-The easiest way to install Certbot is by visiting `certbot.eff.org`_, where you can
-find the correct installation instructions for many web server and OS combinations.
-For more information, see `Get Certbot <https://certbot.eff.org/docs/install.html>`_.
-
-.. _certbot.eff.org: https://certbot.eff.org/
-
-How to run the client
----------------------
-
-In many cases, you can just run ``certbot-auto`` or ``certbot``, and the
-client will guide you through the process of obtaining and installing certs
-interactively.
-
-For full command line help, you can type::
-
-  ./certbot-auto --help all
-
-
-You can also tell it exactly what you want it to do from the command line.
-For instance, if you want to obtain a cert for ``example.com``,
-``www.example.com``, and ``other.example.net``, using the Apache plugin to both
-obtain and install the certs, you could do this::
-
-  ./certbot-auto --apache -d example.com -d www.example.com -d other.example.net
-
-(The first time you run the command, it will make an account, and ask for an
-email and agreement to the Let's Encrypt Subscriber Agreement; you can
-automate those with ``--email`` and ``--agree-tos``)
-
-If you want to use a webserver that doesn't have full plugin support yet, you
-can still use "standalone" or "webroot" plugins to obtain a certificate::
-
-  ./certbot-auto certonly --standalone --email admin@example.com -d example.com -d www.example.com -d other.example.net
-
-
-Understanding the client in more depth
---------------------------------------
-
-To understand what the client is doing in detail, it's important to
-understand the way it uses plugins.  Please see the `explanation of
-plugins <https://certbot.eff.org/docs/using.html#plugins>`_ in
-the User Guide.
-
-Links
-=====
-
-.. Do not modify this comment unless you know what you're doing. tag:links-begin
-
-Documentation: https://certbot.eff.org/docs
-
-Software project: https://github.com/certbot/certbot
-
-Notes for developers: https://certbot.eff.org/docs/contributing.html
-
-Main Website: https://certbot.eff.org
-
-Let's Encrypt Website: https://letsencrypt.org
-
-Community: https://community.letsencrypt.org
-
-ACME spec: http://ietf-wg-acme.github.io/acme/
-
-ACME working area in github: https://github.com/ietf-wg-acme/acme
-
-|build-status| |coverage| |docs| |container|
-
-.. |build-status| image:: https://travis-ci.com/certbot/certbot.svg?branch=master
-   :target: https://travis-ci.com/certbot/certbot
-   :alt: Travis CI status
-
-.. |coverage| image:: https://codecov.io/gh/certbot/certbot/branch/master/graph/badge.svg
-   :target: https://codecov.io/gh/certbot/certbot
-   :alt: Coverage status
-
-.. |docs| image:: https://readthedocs.org/projects/letsencrypt/badge/
-   :target: https://readthedocs.org/projects/letsencrypt/
-   :alt: Documentation status
-
-.. |container| image:: https://quay.io/repository/letsencrypt/letsencrypt/status
-   :target: https://quay.io/repository/letsencrypt/letsencrypt
-   :alt: Docker Repository on Quay.io
-
-.. Do not modify this comment unless you know what you're doing. tag:links-end
-
-System Requirements
-===================
-
-See https://certbot.eff.org/docs/install.html#system-requirements.
-
-.. Do not modify this comment unless you know what you're doing. tag:intro-end
-
-.. Do not modify this comment unless you know what you're doing. tag:features-begin
-
-Current Features
-=====================
-
-* Supports multiple web servers:
-
-  - apache/2.x
-  - nginx/0.8.48+
-  - webroot (adds files to webroot directories in order to prove control of
-    domains and obtain certs)
-  - standalone (runs its own simple webserver to prove you control a domain)
-  - other server software via `third party plugins <https://certbot.eff.org/docs/using.html#third-party-plugins>`_
-
-* The private key is generated locally on your system.
-* Can talk to the Let's Encrypt CA or optionally to other ACME
-  compliant services.
-* Can get domain-validated (DV) certificates.
-* Can revoke certificates.
-* Adjustable RSA key bit-length (2048 (default), 4096, ...).
-* Can optionally install a http -> https redirect, so your site effectively
-  runs https only (Apache only)
-* Fully automated.
-* Configuration changes are logged and can be reverted.
-* Supports an interactive text UI, or can be driven entirely from the
-  command line.
-* Free and Open Source Software, made with Python.
-
-.. Do not modify this comment unless you know what you're doing. tag:features-end
-
-For extensive documentation on using and contributing to Certbot, go to https://certbot.eff.org/docs. If you would like to contribute to the project or run the latest code from git, you should read our `developer guide <https://certbot.eff.org/docs/contributing.html>`_.

README.rst

@@ -0,0 +1 @@
+certbot/README.rst

acme/MANIFEST.in

@@ -3,4 +3,6 @@ include README.rst
 include pytest.ini
 recursive-include docs *
 recursive-include examples *
-recursive-include acme/testdata *
+recursive-include tests *
+global-exclude __pycache__
+global-exclude *.py[cod]

acme/acme/__init__.py

@@ -13,7 +13,6 @@ import warnings
 #
 # It is based on
 # https://github.com/requests/requests/blob/1278ecdf71a312dc2268f3bfc0aabfab3c006dcf/requests/packages.py
-
 import josepy as jose
 
 for mod in list(sys.modules):
@@ -21,30 +20,3 @@ for mod in list(sys.modules):
     # preserved (acme.jose.* is josepy.*)
     if mod == 'josepy' or mod.startswith('josepy.'):
         sys.modules['acme.' + mod.replace('josepy', 'jose', 1)] = sys.modules[mod]
-
-
-# This class takes a similar approach to the cryptography project to deprecate attributes
-# in public modules. See the _ModuleWithDeprecation class here:
-# https://github.com/pyca/cryptography/blob/91105952739442a74582d3e62b3d2111365b0dc7/src/cryptography/utils.py#L129
-class _TLSSNI01DeprecationModule(object):
-    """
-    Internal class delegating to a module, and displaying warnings when
-    attributes related to TLS-SNI-01 are accessed.
-    """
-    def __init__(self, module):
-        self.__dict__['_module'] = module
-
-    def __getattr__(self, attr):
-        if 'TLSSNI01' in attr:
-            warnings.warn('{0} attribute is deprecated, and will be removed soon.'.format(attr),
-                          DeprecationWarning, stacklevel=2)
-        return getattr(self._module, attr)
-
-    def __setattr__(self, attr, value):  # pragma: no cover
-        setattr(self._module, attr, value)
-
-    def __delattr__(self, attr):  # pragma: no cover
-        delattr(self._module, attr)
-
-    def __dir__(self):  # pragma: no cover
-        return ['_module'] + dir(self._module)

acme/acme/challenges.py

@@ -1,30 +1,28 @@
 """ACME Identifier Validation Challenges."""
 import abc
+import codecs
 import functools
 import hashlib
 import logging
 import socket
-import sys
 
 from cryptography.hazmat.primitives import hashes  # type: ignore
 import josepy as jose
-import OpenSSL
 import requests
 import six
+from OpenSSL import SSL  # type: ignore # https://github.com/python/typeshed/issues/2052
+from OpenSSL import crypto
 
-from acme import errors
 from acme import crypto_util
+from acme import errors
 from acme import fields
-from acme import _TLSSNI01DeprecationModule
+from acme.mixins import ResourceMixin, TypeMixin
 
 logger = logging.getLogger(__name__)
 
 
-# pylint: disable=too-few-public-methods
-
-
 class Challenge(jose.TypedJSONObjectWithFields):
-    # _fields_to_partial_json | pylint: disable=abstract-method
+    # _fields_to_partial_json
     """ACME challenge."""
     TYPES = {}  # type: dict
 
@@ -37,8 +35,8 @@ class Challenge(jose.TypedJSONObjectWithFields):
             return UnrecognizedChallenge.from_json(jobj)
 
 
-class ChallengeResponse(jose.TypedJSONObjectWithFields):
-    # _fields_to_partial_json | pylint: disable=abstract-method
+class ChallengeResponse(ResourceMixin, TypeMixin, jose.TypedJSONObjectWithFields):
+    # _fields_to_partial_json
     """ACME challenge response."""
     TYPES = {}  # type: dict
     resource_type = 'challenge'
@@ -63,8 +61,7 @@ class UnrecognizedChallenge(Challenge):
         object.__setattr__(self, "jobj", jobj)
 
     def to_partial_json(self):
-        # pylint: disable=no-member
-        return self.jobj
+        return self.jobj  # pylint: disable=no-member
 
     @classmethod
     def from_json(cls, jobj):
@@ -97,6 +94,7 @@ class _TokenChallenge(Challenge):
         """
         # TODO: check that path combined with uri does not go above
         # URI_ROOT_PATH!
+        # pylint: disable=unsupported-membership-test
         return b'..' not in self.token and b'/' not in self.token
 
 
@@ -109,10 +107,6 @@ class KeyAuthorizationChallengeResponse(ChallengeResponse):
     key_authorization = jose.Field("keyAuthorization")
     thumbprint_hash_function = hashes.SHA256
 
-    def __init__(self, *args, **kwargs):
-        super(KeyAuthorizationChallengeResponse, self).__init__(*args, **kwargs)
-        self._dump_authorization_key(False)
-
     def verify(self, chall, account_public_key):
         """Verify the key authorization.
 
@@ -125,7 +119,7 @@ class KeyAuthorizationChallengeResponse(ChallengeResponse):
         :rtype: bool
 
         """
-        parts = self.key_authorization.split('.')  # pylint: disable=no-member
+        parts = self.key_authorization.split('.')
         if len(parts) != 2:
             logger.debug("Key authorization (%r) is not well formed",
                          self.key_authorization)
@@ -145,26 +139,14 @@ class KeyAuthorizationChallengeResponse(ChallengeResponse):
 
         return True
 
-    def _dump_authorization_key(self, dump):
-        # type: (bool) -> None
-        """
-        Set if keyAuthorization is dumped in the JSON representation of this ChallengeResponse.
-        NB: This method is declared as private because it will eventually be removed.
-        :param bool dump: True to dump the keyAuthorization, False otherwise
-        """
-        object.__setattr__(self, '_dump_auth_key', dump)
-
     def to_partial_json(self):
         jobj = super(KeyAuthorizationChallengeResponse, self).to_partial_json()
-        if not self._dump_auth_key:  # pylint: disable=no-member
-            jobj.pop('keyAuthorization', None)
-
+        jobj.pop('keyAuthorization', None)
         return jobj
 
 
 @six.add_metaclass(abc.ABCMeta)
 class KeyAuthorizationChallenge(_TokenChallenge):
-    # pylint: disable=abstract-class-little-used,too-many-ancestors
     """Challenge based on Key Authorization.
 
     :param response_cls: Subclass of `KeyAuthorizationChallengeResponse`
@@ -196,7 +178,7 @@ class KeyAuthorizationChallenge(_TokenChallenge):
         :rtype: KeyAuthorizationChallengeResponse
 
         """
-        return self.response_cls(
+        return self.response_cls(  # pylint: disable=not-callable
             key_authorization=self.key_authorization(account_key))
 
     @abc.abstractmethod
@@ -233,7 +215,7 @@ class DNS01Response(KeyAuthorizationChallengeResponse):
     """ACME dns-01 challenge response."""
     typ = "dns-01"
 
-    def simple_verify(self, chall, domain, account_public_key):
+    def simple_verify(self, chall, domain, account_public_key):  # pylint: disable=unused-argument
         """Simple verify.
 
         This method no longer checks DNS records and is a simple wrapper
@@ -249,14 +231,13 @@ class DNS01Response(KeyAuthorizationChallengeResponse):
         :rtype: bool
 
         """
-        # pylint: disable=unused-argument
         verified = self.verify(chall, account_public_key)
         if not verified:
             logger.debug("Verification of key authorization in response failed")
         return verified
 
 
-@Challenge.register  # pylint: disable=too-many-ancestors
+@Challenge.register
 class DNS01(KeyAuthorizationChallenge):
     """ACME dns-01 challenge."""
     response_cls = DNS01Response
@@ -329,7 +310,7 @@ class HTTP01Response(KeyAuthorizationChallengeResponse):
         uri = chall.uri(domain)
         logger.debug("Verifying %s at %s...", chall.typ, uri)
         try:
-            http_response = requests.get(uri)
+            http_response = requests.get(uri, verify=False)
         except requests.exceptions.RequestException as error:
             logger.error("Unable to reach %s: %s", uri, error)
             return False
@@ -346,7 +327,7 @@ class HTTP01Response(KeyAuthorizationChallengeResponse):
         return True
 
 
-@Challenge.register  # pylint: disable=too-many-ancestors
+@Challenge.register
 class HTTP01(KeyAuthorizationChallenge):
     """ACME http-01 challenge."""
     response_cls = HTTP01Response
@@ -387,12 +368,9 @@ class HTTP01(KeyAuthorizationChallenge):
 
 
 @ChallengeResponse.register
-class TLSSNI01Response(KeyAuthorizationChallengeResponse):
-    """ACME tls-sni-01 challenge response."""
-    typ = "tls-sni-01"
-
-    DOMAIN_SUFFIX = b".acme.invalid"
-    """Domain name suffix."""
+class TLSALPN01Response(KeyAuthorizationChallengeResponse):
+    """ACME tls-alpn-01 challenge response."""
+    typ = "tls-alpn-01"
 
     PORT = 443
     """Verification port as defined by the protocol.
@@ -402,28 +380,18 @@ class TLSSNI01Response(KeyAuthorizationChallengeResponse):
 
     """
 
-    @property
-    def z(self):  # pylint: disable=invalid-name
-        """``z`` value used for verification.
-
-        :rtype bytes:
-
-        """
-        return hashlib.sha256(
-            self.key_authorization.encode("utf-8")).hexdigest().lower().encode()
+    ID_PE_ACME_IDENTIFIER_V1 = b"1.3.6.1.5.5.7.1.30.1"
+    ACME_TLS_1_PROTOCOL = "acme-tls/1"
 
     @property
-    def z_domain(self):
-        """Domain name used for verification, generated from `z`.
+    def h(self):
+        """Hash value stored in challenge certificate"""
+        return hashlib.sha256(self.key_authorization.encode('utf-8')).digest()
 
-        :rtype bytes:
-
-        """
-        return self.z[:32] + b'.' + self.z[32:] + self.DOMAIN_SUFFIX
-
-    def gen_cert(self, key=None, bits=2048):
-        """Generate tls-sni-01 certificate.
+    def gen_cert(self, domain, key=None, bits=2048):
+        """Generate tls-alpn-01 certificate.
 
+        :param unicode domain: Domain verified by the challenge.
         :param OpenSSL.crypto.PKey key: Optional private key used in
             certificate generation. If not provided (``None``), then
             fresh key will be generated.
@@ -433,33 +401,38 @@ class TLSSNI01Response(KeyAuthorizationChallengeResponse):
 
         """
         if key is None:
-            key = OpenSSL.crypto.PKey()
-            key.generate_key(OpenSSL.crypto.TYPE_RSA, bits)
-        return crypto_util.gen_ss_cert(key, [
-            # z_domain is too big to fit into CN, hence first dummy domain
-            'dummy', self.z_domain.decode()], force_san=True), key
+            key = crypto.PKey()
+            key.generate_key(crypto.TYPE_RSA, bits)
 
-    def probe_cert(self, domain, **kwargs):
-        """Probe tls-sni-01 challenge certificate.
 
-        :param unicode domain:
+        der_value = b"DER:" + codecs.encode(self.h, 'hex')
+        acme_extension = crypto.X509Extension(self.ID_PE_ACME_IDENTIFIER_V1,
+                critical=True, value=der_value)
+
+        return crypto_util.gen_ss_cert(key, [domain], force_san=True,
+                extensions=[acme_extension]), key
+
+    def probe_cert(self, domain, host=None, port=None):
+        """Probe tls-alpn-01 challenge certificate.
+
+        :param unicode domain: domain being validated, required.
+        :param string host: IP address used to probe the certificate.
+        :param int port: Port used to probe the certificate.
 
         """
-        # TODO: domain is not necessary if host is provided
-        if "host" not in kwargs:
+        if host is None:
             host = socket.gethostbyname(domain)
             logger.debug('%s resolved to %s', domain, host)
-            kwargs["host"] = host
+        if port is None:
+            port = self.PORT
 
-        kwargs.setdefault("port", self.PORT)
-        kwargs["name"] = self.z_domain
-        # TODO: try different methods?
-        # pylint: disable=protected-access
-        return crypto_util.probe_sni(**kwargs)
+        return crypto_util.probe_sni(host=host, port=port, name=domain,
+                alpn_protocols=[self.ACME_TLS_1_PROTOCOL])
 
-    def verify_cert(self, cert):
-        """Verify tls-sni-01 challenge certificate.
+    def verify_cert(self, domain, cert):
+        """Verify tls-alpn-01 challenge certificate.
 
+        :param unicode domain: Domain name being validated.
         :param OpensSSL.crypto.X509 cert: Challenge certificate.
 
         :returns: Whether the certificate was successfully verified.
@@ -467,28 +440,40 @@ class TLSSNI01Response(KeyAuthorizationChallengeResponse):
 
         """
         # pylint: disable=protected-access
-        sans = crypto_util._pyopenssl_cert_or_req_san(cert)
-        logger.debug('Certificate %s. SANs: %s', cert.digest('sha256'), sans)
-        return self.z_domain.decode() in sans
+        names = crypto_util._pyopenssl_cert_or_req_all_names(cert)
+        logger.debug('Certificate %s. SANs: %s', cert.digest('sha256'), names)
+        if len(names) != 1 or names[0].lower() != domain.lower():
+            return False
 
+        for i in range(cert.get_extension_count()):
+            ext = cert.get_extension(i)
+            # FIXME: assume this is the ACME extension. Currently there is no
+            # way to get full OID of an unknown extension from pyopenssl.
+            if ext.get_short_name() == b'UNDEF':
+                data = ext.get_data()
+                return data == self.h
+
+        return False
+
+    # pylint: disable=too-many-arguments
     def simple_verify(self, chall, domain, account_public_key,
-                      cert=None, **kwargs):
+                      cert=None, host=None, port=None):
         """Simple verify.
 
         Verify ``validation`` using ``account_public_key``, optionally
-        probe tls-sni-01 certificate and check using `verify_cert`.
+        probe tls-alpn-01 certificate and check using `verify_cert`.
 
-        :param .challenges.TLSSNI01 chall: Corresponding challenge.
+        :param .challenges.TLSALPN01 chall: Corresponding challenge.
         :param str domain: Domain name being validated.
         :param JWK account_public_key:
         :param OpenSSL.crypto.X509 cert: Optional certificate. If not
             provided (``None``) certificate will be retrieved using
             `probe_cert`.
+        :param string host: IP address used to probe the certificate.
         :param int port: Port used to probe the certificate.
 
 
-        :returns: ``True`` iff client's control of the domain has been
-            verified.
+        :returns: ``True`` if and only if client's control of the domain has been verified.
         :rtype: bool
 
         """
@@ -498,30 +483,25 @@ class TLSSNI01Response(KeyAuthorizationChallengeResponse):
 
         if cert is None:
             try:
-                cert = self.probe_cert(domain=domain, **kwargs)
+                cert = self.probe_cert(domain=domain, host=host, port=port)
             except errors.Error as error:
                 logger.debug(str(error), exc_info=True)
                 return False
 
-        return self.verify_cert(cert)
+        return self.verify_cert(domain, cert)
 
 
 @Challenge.register  # pylint: disable=too-many-ancestors
-class TLSSNI01(KeyAuthorizationChallenge):
-    """ACME tls-sni-01 challenge."""
-    response_cls = TLSSNI01Response
+class TLSALPN01(KeyAuthorizationChallenge):
+    """ACME tls-alpn-01 challenge."""
+    response_cls = TLSALPN01Response
     typ = response_cls.typ
 
-    # boulder#962, ietf-wg-acme#22
-    #n = jose.Field("n", encoder=int, decoder=int)
-
-    def __init__(self, *args, **kwargs):
-        super(TLSSNI01, self).__init__(*args, **kwargs)
-
     def validation(self, account_key, **kwargs):
         """Generate validation.
 
         :param JWK account_key:
+        :param unicode domain: Domain verified by the challenge.
         :param OpenSSL.crypto.PKey cert_key: Optional private key used
             in certificate generation. If not provided (``None``), then
             fresh key will be generated.
@@ -529,37 +509,26 @@ class TLSSNI01(KeyAuthorizationChallenge):
         :rtype: `tuple` of `OpenSSL.crypto.X509` and `OpenSSL.crypto.PKey`
 
         """
-        return self.response(account_key).gen_cert(key=kwargs.get('cert_key'))
+        return self.response(account_key).gen_cert(
+            key=kwargs.get('cert_key'),
+            domain=kwargs.get('domain'))
+
+    @staticmethod
+    def is_supported():
+        """
+        Check if TLS-ALPN-01 challenge is supported on this machine.
+        This implies that a recent version of OpenSSL is installed (>= 1.0.2),
+        or a recent cryptography version shipped with the OpenSSL library is installed.
+
+        :returns: ``True`` if TLS-ALPN-01 is supported on this machine, ``False`` otherwise.
+        :rtype: bool
+
+        """
+        return (hasattr(SSL.Connection, "set_alpn_protos")
+                and hasattr(SSL.Context, "set_alpn_select_callback"))
 
 
-@ChallengeResponse.register
-class TLSALPN01Response(KeyAuthorizationChallengeResponse):
-    """ACME TLS-ALPN-01 challenge response.
-
-    This class only allows initiating a TLS-ALPN-01 challenge returned from the
-    CA. Full support for responding to TLS-ALPN-01 challenges by generating and
-    serving the expected response certificate is not currently provided.
-    """
-    typ = "tls-alpn-01"
-
-
-@Challenge.register  # pylint: disable=too-many-ancestors
-class TLSALPN01(KeyAuthorizationChallenge):
-    """ACME tls-alpn-01 challenge.
-
-    This class simply allows parsing the TLS-ALPN-01 challenge returned from
-    the CA. Full TLS-ALPN-01 support is not currently provided.
-
-    """
-    typ = "tls-alpn-01"
-    response_cls = TLSALPN01Response
-
-    def validation(self, account_key, **kwargs):
-        """Generate validation for the challenge."""
-        raise NotImplementedError()
-
-
-@Challenge.register  # pylint: disable=too-many-ancestors
+@Challenge.register
 class DNS(_TokenChallenge):
     """ACME "dns" challenge."""
     typ = "dns"
@@ -640,7 +609,3 @@ class DNSResponse(ChallengeResponse):
 
         """
         return chall.check_validation(self.validation, account_public_key)
-
-
-# Patching ourselves to warn about TLS-SNI challenge deprecation and removal.
-sys.modules[__name__] = _TLSSNI01DeprecationModule(sys.modules[__name__])

acme/acme/client.py

@@ -5,26 +5,27 @@ import datetime
 from email.utils import parsedate_tz
 import heapq
 import logging
+import re
+import sys
 import time
 
-import six
-from six.moves import http_client  # pylint: disable=import-error
 import josepy as jose
 import OpenSSL
-import re
-from requests_toolbelt.adapters.source import SourceAddressAdapter
 import requests
 from requests.adapters import HTTPAdapter
-import sys
+from requests_toolbelt.adapters.source import SourceAddressAdapter
+import six
+from six.moves import http_client
 
-from acme import challenges
 from acme import crypto_util
 from acme import errors
 from acme import jws
 from acme import messages
-# pylint: disable=unused-import, no-name-in-module
-from acme.magic_typing import Dict, List, Set, Text
-
+from acme.magic_typing import Dict
+from acme.magic_typing import List
+from acme.magic_typing import Set
+from acme.magic_typing import Text
+from acme.mixins import VersionedLEACMEMixin
 
 logger = logging.getLogger(__name__)
 
@@ -34,10 +35,9 @@ logger = logging.getLogger(__name__)
 # https://urllib3.readthedocs.org/en/latest/security.html#insecureplatformwarning
 if sys.version_info < (2, 7, 9):  # pragma: no cover
     try:
-        # pylint: disable=no-member
         requests.packages.urllib3.contrib.pyopenssl.inject_into_urllib3()  # type: ignore
     except AttributeError:
-        import urllib3.contrib.pyopenssl  # pylint: disable=import-error
+        import urllib3.contrib.pyopenssl
         urllib3.contrib.pyopenssl.inject_into_urllib3()
 
 DEFAULT_NETWORK_TIMEOUT = 45
@@ -45,7 +45,7 @@ DEFAULT_NETWORK_TIMEOUT = 45
 DER_CONTENT_TYPE = 'application/pkix-cert'
 
 
-class ClientBase(object):  # pylint: disable=too-many-instance-attributes
+class ClientBase(object):
     """ACME client base object.
 
     :ivar messages.Directory directory:
@@ -124,14 +124,21 @@ class ClientBase(object):  # pylint: disable=too-many-instance-attributes
         """
         return self.update_registration(regr, update={'status': 'deactivated'})
 
-    def query_registration(self, regr):
-        """Query server about registration.
+    def deactivate_authorization(self, authzr):
+        # type: (messages.AuthorizationResource) -> messages.AuthorizationResource
+        """Deactivate authorization.
 
-        :param messages.RegistrationResource: Existing Registration
-            Resource.
+        :param messages.AuthorizationResource authzr: The Authorization resource
+            to be deactivated.
+
+        :returns: The Authorization resource that was deactivated.
+        :rtype: `.AuthorizationResource`
 
         """
-        return self._send_recv_regr(regr, messages.UpdateRegistration())
+        body = messages.UpdateAuthorization(status='deactivated')
+        response = self._post(authzr.uri, body)
+        return self._authzr_from_response(response,
+            authzr.body.identifier, authzr.uri)
 
     def _authzr_from_response(self, response, identifier=None, uri=None):
         authzr = messages.AuthorizationResource(
@@ -156,23 +163,7 @@ class ClientBase(object):  # pylint: disable=too-many-instance-attributes
         :raises .UnexpectedUpdate:
 
         """
-        # Because sending keyAuthorization in a response challenge has been removed from the ACME
-        # spec, it is not included in the KeyAuthorizationResponseChallenge JSON by default.
-        # However as a migration path, we temporarily expect a malformed error from the server,
-        # and fallback by resending the challenge response with the keyAuthorization field.
-        # TODO: Remove this fallback for Certbot 0.34.0
-        try:
-            response = self._post(challb.uri, response)
-        except messages.Error as error:
-            if (error.code == 'malformed'
-                    and isinstance(response, challenges.KeyAuthorizationChallengeResponse)):
-                logger.debug('Error while responding to a challenge without keyAuthorization '
-                             'in the JWS, your ACME CA server may not support it:\n%s', error)
-                logger.debug('Retrying request with keyAuthorization set.')
-                response._dump_authorization_key(True)  # pylint: disable=protected-access
-                response = self._post(challb.uri, response)
-            else:
-                raise
+        response = self._post(challb.uri, response)
         try:
             authzr_uri = response.links['up']['url']
         except KeyError:
@@ -264,7 +255,6 @@ class Client(ClientBase):
             URI from which the resource will be downloaded.
 
         """
-        # pylint: disable=too-many-arguments
         self.key = key
         if net is None:
             net = ClientNetwork(key, alg=alg, verify_ssl=verify_ssl)
@@ -290,9 +280,17 @@ class Client(ClientBase):
         assert response.status_code == http_client.CREATED
 
         # "Instance of 'Field' has no key/contact member" bug:
-        # pylint: disable=no-member
         return self._regr_from_response(response)
 
+    def query_registration(self, regr):
+        """Query server about registration.
+
+        :param messages.RegistrationResource: Existing Registration
+            Resource.
+
+        """
+        return self._send_recv_regr(regr, messages.UpdateRegistration())
+
     def agree_to_tos(self, regr):
         """Agree to the terms-of-service.
 
@@ -436,7 +434,6 @@ class Client(ClientBase):
             was marked by the CA as invalid
 
         """
-        # pylint: disable=too-many-locals
         assert max_attempts > 0
         attempts = collections.defaultdict(int) # type: Dict[messages.AuthorizationResource, int]
         exhausted = set()
@@ -467,7 +464,6 @@ class Client(ClientBase):
             updated[authzr] = updated_authzr
 
             attempts[authzr] += 1
-            # pylint: disable=no-member
             if updated_authzr.body.status not in (
                     messages.STATUS_VALID, messages.STATUS_INVALID):
                 if attempts[authzr] < max_attempts:
@@ -608,7 +604,6 @@ class ClientV2(ClientBase):
         if response.status_code == 200 and 'Location' in response.headers:
             raise errors.ConflictError(response.headers.get('Location'))
         # "Instance of 'Field' has no key/contact member" bug:
-        # pylint: disable=no-member
         regr = self._regr_from_response(response)
         self.net.account = regr
         return regr
@@ -620,10 +615,13 @@ class ClientV2(ClientBase):
             Resource.
 
         """
-        self.net.account = regr
-        updated_regr = super(ClientV2, self).query_registration(regr)
-        self.net.account = updated_regr
-        return updated_regr
+        self.net.account = regr  # See certbot/certbot#6258
+        # ACME v2 requires to use a POST-as-GET request (POST an empty JWS) here.
+        # This is done by passing None instead of an empty UpdateRegistration to _post().
+        response = self._post(regr.uri, None)
+        self.net.account = self._regr_from_response(response, uri=regr.uri,
+                                                    terms_of_service=regr.terms_of_service)
+        return self.net.account
 
     def update_registration(self, regr, update=None):
         """Update registration.
@@ -729,9 +727,9 @@ class ClientV2(ClientBase):
         for authzr in responses:
             if authzr.body.status != messages.STATUS_VALID:
                 for chall in authzr.body.challenges:
-                    if chall.error != None:
+                    if chall.error is not None:
                         failed.append(authzr)
-        if len(failed) > 0:
+        if failed:
             raise errors.ValidationError(failed)
         return orderr.update(authorizations=responses)
 
@@ -775,36 +773,17 @@ class ClientV2(ClientBase):
 
     def external_account_required(self):
         """Checks if ACME server requires External Account Binding authentication."""
-        if hasattr(self.directory, 'meta') and self.directory.meta.external_account_required:
-            return True
-        else:
-            return False
+        return hasattr(self.directory, 'meta') and self.directory.meta.external_account_required
 
     def _post_as_get(self, *args, **kwargs):
         """
-        Send GET request using the POST-as-GET protocol if needed.
-        The request will be first issued using POST-as-GET for ACME v2. If the ACME CA servers do
-        not support this yet and return an error, request will be retried using GET.
-        For ACME v1, only GET request will be tried, as POST-as-GET is not supported.
+        Send GET request using the POST-as-GET protocol.
         :param args:
         :param kwargs:
         :return:
         """
-        if self.acme_version >= 2:
-            # We add an empty payload for POST-as-GET requests
-            new_args = args[:1] + (None,) + args[1:]
-            try:
-                return self._post(*new_args, **kwargs)  # pylint: disable=star-args
-            except messages.Error as error:
-                if error.code == 'malformed':
-                    logger.debug('Error during a POST-as-GET request, '
-                                 'your ACME CA server may not support it:\n%s', error)
-                    logger.debug('Retrying request with GET.')
-                else:  # pragma: no cover
-                    raise
-
-        # If POST-as-GET is not supported yet, we use a GET instead.
-        return self.net.get(*args, **kwargs)
+        new_args = args[:1] + (None,) + args[1:]
+        return self._post(*new_args, **kwargs)
 
 
 class BackwardsCompatibleClientV2(object):
@@ -882,8 +861,7 @@ class BackwardsCompatibleClientV2(object):
             for domain in dnsNames:
                 authorizations.append(self.client.request_domain_challenges(domain))
             return messages.OrderResource(authorizations=authorizations, csr_pem=csr_pem)
-        else:
-            return self.client.new_order(csr_pem)
+        return self.client.new_order(csr_pem)
 
     def finalize_order(self, orderr, deadline):
         """Finalize an order and obtain a certificate.
@@ -920,8 +898,7 @@ class BackwardsCompatibleClientV2(object):
             chain = crypto_util.dump_pyopenssl_chain(chain).decode()
 
             return orderr.update(fullchain_pem=(cert + chain))
-        else:
-            return self.client.finalize_order(orderr, deadline)
+        return self.client.finalize_order(orderr, deadline)
 
     def revoke(self, cert, rsn):
         """Revoke certificate.
@@ -939,8 +916,7 @@ class BackwardsCompatibleClientV2(object):
     def _acme_version_from_directory(self, directory):
         if hasattr(directory, 'newNonce'):
             return 2
-        else:
-            return 1
+        return 1
 
     def external_account_required(self):
         """Checks if the server requires an external account for ACMEv2 servers.
@@ -948,11 +924,10 @@ class BackwardsCompatibleClientV2(object):
         Always return False for ACMEv1 servers, as it doesn't use External Account Binding."""
         if self.acme_version == 1:
             return False
-        else:
-            return self.client.external_account_required()
+        return self.client.external_account_required()
 
 
-class ClientNetwork(object):  # pylint: disable=too-many-instance-attributes
+class ClientNetwork(object):
     """Wrapper around requests that signs POSTs for authentication.
 
     Also adds user agent, and handles Content-Type.
@@ -968,7 +943,7 @@ class ClientNetwork(object):  # pylint: disable=too-many-instance-attributes
     :param messages.RegistrationResource account: Account object. Required if you are
             planning to use .post() with acme_version=2 for anything other than
             creating a new account; may be set later after registering.
-    :param josepy.JWASignature alg: Algoritm to use in signing JWS.
+    :param josepy.JWASignature alg: Algorithm to use in signing JWS.
     :param bool verify_ssl: Whether to verify certificates on SSL connections.
     :param str user_agent: String to send as User-Agent header.
     :param float timeout: Timeout for requests.
@@ -978,7 +953,6 @@ class ClientNetwork(object):  # pylint: disable=too-many-instance-attributes
     def __init__(self, key, account=None, alg=jose.RS256, verify_ssl=True,
                  user_agent='acme-python', timeout=DEFAULT_NETWORK_TIMEOUT,
                  source_address=None):
-        # pylint: disable=too-many-arguments
         self.key = key
         self.account = account
         self.alg = alg
@@ -1014,6 +988,8 @@ class ClientNetwork(object):  # pylint: disable=too-many-instance-attributes
         :rtype: `josepy.JWS`
 
         """
+        if isinstance(obj, VersionedLEACMEMixin):
+            obj.le_acme_version = acme_version
         jobj = obj.json_dumps(indent=2).encode() if obj else b''
         logger.debug('JWS payload:\n%s', jobj)
         kwargs = {
@@ -1027,7 +1003,6 @@ class ClientNetwork(object):  # pylint: disable=too-many-instance-attributes
             if self.account is not None:
                 kwargs["kid"] = self.account["uri"]
         kwargs["key"] = self.key
-        # pylint: disable=star-args
         return jws.JWS.sign(jobj, **kwargs).json_dumps(indent=2)
 
     @classmethod
@@ -1050,6 +1025,9 @@ class ClientNetwork(object):  # pylint: disable=too-many-instance-attributes
 
         """
         response_ct = response.headers.get('Content-Type')
+        # Strip parameters from the media-type (rfc2616#section-3.7)
+        if response_ct:
+            response_ct = response_ct.split(';')[0].strip()
         try:
             # TODO: response.json() is called twice, once here, and
             # once in _get and _post clients
@@ -1087,7 +1065,6 @@ class ClientNetwork(object):  # pylint: disable=too-many-instance-attributes
         return response
 
     def _send_request(self, method, url, *args, **kwargs):
-        # pylint: disable=too-many-locals
         """Send HTTP request.
 
         Makes sure that `verify_ssl` is respected. Logs request and
@@ -1134,10 +1111,9 @@ class ClientNetwork(object):  # pylint: disable=too-many-instance-attributes
             err_regex = r".*host='(\S*)'.*Max retries exceeded with url\: (\/\w*).*(\[Errno \d+\])([A-Za-z ]*)"
             m = re.match(err_regex, str(e))
             if m is None:
-                raise # pragma: no cover
-            else:
-                host, path, _err_no, err_msg = m.groups()
-                raise ValueError("Requesting {0}{1}:{2}".format(host, path, err_msg))
+                raise  # pragma: no cover
+            host, path, _err_no, err_msg = m.groups()
+            raise ValueError("Requesting {0}{1}:{2}".format(host, path, err_msg))
 
         # If content is DER, log the base64 of it instead of raw bytes, to keep
         # binary data out of the logs.
@@ -1147,8 +1123,8 @@ class ClientNetwork(object):  # pylint: disable=too-many-instance-attributes
             debug_content = response.content.decode("utf-8")
         logger.debug('Received response:\nHTTP %d\n%s\n\n%s',
                      response.status_code,
-                     "\n".join(["{0}: {1}".format(k, v)
-                                for k, v in response.headers.items()]),
+                     "\n".join("{0}: {1}".format(k, v)
+                                for k, v in response.headers.items()),
                      debug_content)
         return response
 
@@ -1203,8 +1179,7 @@ class ClientNetwork(object):  # pylint: disable=too-many-instance-attributes
             if error.code == 'badNonce':
                 logger.debug('Retrying request after error:\n%s', error)
                 return self._post_once(*args, **kwargs)
-            else:
-                raise
+            raise
 
     def _post_once(self, url, obj, content_type=JOSE_CONTENT_TYPE,
             acme_version=1, **kwargs):

acme/acme/crypto_util.py

@@ -6,15 +6,14 @@ import os
 import re
 import socket
 
-from OpenSSL import crypto
-from OpenSSL import SSL # type: ignore # https://github.com/python/typeshed/issues/2052
 import josepy as jose
+from OpenSSL import crypto
+from OpenSSL import SSL  # type: ignore # https://github.com/python/typeshed/issues/2052
 
 from acme import errors
-# pylint: disable=unused-import, no-name-in-module
-from acme.magic_typing import Callable, Union, Tuple, Optional
-# pylint: enable=unused-import, no-name-in-module
-
+from acme.magic_typing import Callable
+from acme.magic_typing import Tuple
+from acme.magic_typing import Union
 
 logger = logging.getLogger(__name__)
 
@@ -28,6 +27,15 @@ logger = logging.getLogger(__name__)
 _DEFAULT_SSL_METHOD = SSL.SSLv23_METHOD  # type: ignore
 
 
+class _DefaultCertSelection(object):
+    def __init__(self, certs):
+        self.certs = certs
+
+    def __call__(self, connection):
+        server_name = connection.get_servername()
+        return self.certs.get(server_name, None)
+
+
 class SSLSocket(object):  # pylint: disable=too-few-public-methods
     """SSL wrapper for sockets.
 
@@ -35,12 +43,25 @@ class SSLSocket(object):  # pylint: disable=too-few-public-methods
     :ivar dict certs: Mapping from domain names (`bytes`) to
         `OpenSSL.crypto.X509`.
     :ivar method: See `OpenSSL.SSL.Context` for allowed values.
+    :ivar alpn_selection: Hook to select negotiated ALPN protocol for
+        connection.
+    :ivar cert_selection: Hook to select certificate for connection. If given,
+        `certs` parameter would be ignored, and therefore must be empty.
 
     """
-    def __init__(self, sock, certs, method=_DEFAULT_SSL_METHOD):
+    def __init__(self, sock, certs=None,
+            method=_DEFAULT_SSL_METHOD, alpn_selection=None,
+            cert_selection=None):
         self.sock = sock
-        self.certs = certs
+        self.alpn_selection = alpn_selection
         self.method = method
+        if not cert_selection and not certs:
+            raise ValueError("Neither cert_selection or certs specified.")
+        if cert_selection and certs:
+            raise ValueError("Both cert_selection and certs specified.")
+        if cert_selection is None:
+            cert_selection = _DefaultCertSelection(certs)
+        self.cert_selection = cert_selection
 
     def __getattr__(self, name):
         return getattr(self.sock, name)
@@ -57,24 +78,25 @@ class SSLSocket(object):  # pylint: disable=too-few-public-methods
         :type connection: :class:`OpenSSL.Connection`
 
         """
-        server_name = connection.get_servername()
-        try:
-            key, cert = self.certs[server_name]
-        except KeyError:
-            logger.debug("Server name (%s) not recognized, dropping SSL",
-                         server_name)
+        pair = self.cert_selection(connection)
+        if pair is None:
+            logger.debug("Certificate selection for server name %s failed, dropping SSL",
+                         connection.get_servername())
             return
+        key, cert = pair
         new_context = SSL.Context(self.method)
         new_context.set_options(SSL.OP_NO_SSLv2)
         new_context.set_options(SSL.OP_NO_SSLv3)
         new_context.use_privatekey(key)
         new_context.use_certificate(cert)
+        if self.alpn_selection is not None:
+            new_context.set_alpn_select_callback(self.alpn_selection)
         connection.set_context(new_context)
 
     class FakeConnection(object):
         """Fake OpenSSL.SSL.Connection."""
 
-        # pylint: disable=too-few-public-methods,missing-docstring
+        # pylint: disable=missing-function-docstring
 
         def __init__(self, connection):
             self._wrapped = connection
@@ -86,13 +108,15 @@ class SSLSocket(object):  # pylint: disable=too-few-public-methods
             # OpenSSL.SSL.Connection.shutdown doesn't accept any args
             return self._wrapped.shutdown()
 
-    def accept(self):  # pylint: disable=missing-docstring
+    def accept(self):  # pylint: disable=missing-function-docstring
         sock, addr = self.sock.accept()
 
         context = SSL.Context(self.method)
         context.set_options(SSL.OP_NO_SSLv2)
         context.set_options(SSL.OP_NO_SSLv3)
         context.set_tlsext_servername_callback(self._pick_certificate_cb)
+        if self.alpn_selection is not None:
+            context.set_alpn_select_callback(self.alpn_selection)
 
         ssl_sock = self.FakeConnection(SSL.Connection(context, sock))
         ssl_sock.set_accept_state()
@@ -108,8 +132,9 @@ class SSLSocket(object):  # pylint: disable=too-few-public-methods
         return ssl_sock, addr
 
 
-def probe_sni(name, host, port=443, timeout=300,
-              method=_DEFAULT_SSL_METHOD, source_address=('', 0)):
+def probe_sni(name, host, port=443, timeout=300, # pylint: disable=too-many-arguments
+              method=_DEFAULT_SSL_METHOD, source_address=('', 0),
+              alpn_protocols=None):
     """Probe SNI server for SSL certificate.
 
     :param bytes name: Byte string to send as the server name in the
@@ -121,6 +146,8 @@ def probe_sni(name, host, port=443, timeout=300,
     :param tuple source_address: Enables multi-path probing (selection
         of source interface). See `socket.creation_connection` for more
         info. Available only in Python 2.7+.
+    :param alpn_protocols: Protocols to request using ALPN.
+    :type alpn_protocols: `list` of `bytes`
 
     :raises acme.errors.Error: In case of any problems.
 
@@ -134,7 +161,6 @@ def probe_sni(name, host, port=443, timeout=300,
     socket_kwargs = {'source_address': source_address}
 
     try:
-        # pylint: disable=star-args
         logger.debug(
             "Attempting to connect to %s:%d%s.", host, port,
             " from {0}:{1}".format(
@@ -151,6 +177,8 @@ def probe_sni(name, host, port=443, timeout=300,
         client_ssl = SSL.Connection(context, client)
         client_ssl.set_connect_state()
         client_ssl.set_tlsext_host_name(name)  # pyOpenSSL>=0.13
+        if alpn_protocols is not None:
+            client_ssl.set_alpn_protos(alpn_protocols)
         try:
             client_ssl.do_handshake()
             client_ssl.shutdown()
@@ -195,8 +223,7 @@ def _pyopenssl_cert_or_req_all_names(loaded_cert_or_req):
 
     if common_name is None:
         return sans
-    else:
-        return [common_name] + [d for d in sans if d != common_name]
+    return [common_name] + [d for d in sans if d != common_name]
 
 def _pyopenssl_cert_or_req_san(cert_or_req):
     """Get Subject Alternative Names from certificate or CSR using pyOpenSSL.
@@ -242,12 +269,14 @@ def _pyopenssl_cert_or_req_san(cert_or_req):
 
 
 def gen_ss_cert(key, domains, not_before=None,
-                validity=(7 * 24 * 60 * 60), force_san=True):
+                validity=(7 * 24 * 60 * 60), force_san=True, extensions=None):
     """Generate new self-signed certificate.
 
     :type domains: `list` of `unicode`
     :param OpenSSL.crypto.PKey key:
     :param bool force_san:
+    :param extensions: List of additional extensions to include in the cert.
+    :type extensions: `list` of `OpenSSL.crypto.X509Extension`
 
     If more than one domain is provided, all of the domains are put into
     ``subjectAltName`` X.509 extension and first domain is set as the
@@ -260,10 +289,13 @@ def gen_ss_cert(key, domains, not_before=None,
     cert.set_serial_number(int(binascii.hexlify(os.urandom(16)), 16))
     cert.set_version(2)
 
-    extensions = [
+    if extensions is None:
+        extensions = []
+
+    extensions.append(
         crypto.X509Extension(
             b"basicConstraints", True, b"CA:TRUE, pathlen:0"),
-    ]
+    )
 
     cert.get_subject().CN = domains[0]
     # TODO: what to put into cert.get_subject()?
@@ -300,7 +332,6 @@ def dump_pyopenssl_chain(chain, filetype=crypto.FILETYPE_PEM):
 
     def _dump_cert(cert):
         if isinstance(cert, jose.ComparableX509):
-            # pylint: disable=protected-access
             cert = cert.wrapped
         return crypto.dump_certificate(filetype, cert)
 

acme/acme/errors.py

@@ -29,7 +29,12 @@ class NonceError(ClientError):
 class BadNonce(NonceError):
     """Bad nonce error."""
     def __init__(self, nonce, error, *args, **kwargs):
-        super(BadNonce, self).__init__(*args, **kwargs)
+        # MyPy complains here that there is too many arguments for BaseException constructor.
+        # This is an error fixed in typeshed, see https://github.com/python/mypy/issues/4183
+        # The fix is included in MyPy>=0.740, but upgrading it would bring dozen of errors due to
+        #   new types definitions. So we ignore the error until the code base is fixed to match
+        #   with MyPy>=0.740 referential.
+        super(BadNonce, self).__init__(*args, **kwargs)  # type: ignore
         self.nonce = nonce
         self.error = error
 
@@ -48,7 +53,8 @@ class MissingNonce(NonceError):
 
     """
     def __init__(self, response, *args, **kwargs):
-        super(MissingNonce, self).__init__(*args, **kwargs)
+        # See comment in BadNonce constructor above for an explanation of type: ignore here.
+        super(MissingNonce, self).__init__(*args, **kwargs)  # type: ignore
         self.response = response
 
     def __str__(self):
@@ -83,6 +89,7 @@ class PollError(ClientError):
         return '{0}(exhausted={1!r}, updated={2!r})'.format(
             self.__class__.__name__, self.exhausted, self.updated)
 
+
 class ValidationError(Error):
     """Error for authorization failures. Contains a list of authorization
     resources, each of which is invalid and should have an error field.
@@ -91,9 +98,11 @@ class ValidationError(Error):
         self.failed_authzrs = failed_authzrs
         super(ValidationError, self).__init__()
 
-class TimeoutError(Error):
+
+class TimeoutError(Error):  # pylint: disable=redefined-builtin
     """Error for when polling an authorization or an order times out."""
 
+
 class IssuanceError(Error):
     """Error sent by the server after requesting issuance of a certificate."""
 
@@ -105,6 +114,7 @@ class IssuanceError(Error):
         self.error = error
         super(IssuanceError, self).__init__()
 
+
 class ConflictError(ClientError):
     """Error for when the server returns a 409 (Conflict) HTTP status.
 

acme/acme/fields.py

@@ -4,7 +4,6 @@ import logging
 import josepy as jose
 import pyrfc3339
 
-
 logger = logging.getLogger(__name__)
 
 

acme/acme/jws.py

@@ -15,7 +15,7 @@ class Header(jose.Header):
     url = jose.Field('url', omitempty=True)
 
     @nonce.decoder
-    def nonce(value):  # pylint: disable=missing-docstring,no-self-argument
+    def nonce(value):  # pylint: disable=no-self-argument,missing-function-docstring
         try:
             return jose.decode_b64jose(value)
         except jose.DeserializationError as error:
@@ -40,10 +40,10 @@ class Signature(jose.Signature):
 class JWS(jose.JWS):
     """ACME-specific JWS. Includes none, url, and kid in protected header."""
     signature_cls = Signature
-    __slots__ = jose.JWS._orig_slots  # pylint: disable=no-member
+    __slots__ = jose.JWS._orig_slots
 
     @classmethod
-    # pylint: disable=arguments-differ,too-many-arguments
+    # pylint: disable=arguments-differ
     def sign(cls, payload, key, alg, nonce, url=None, kid=None):
         # Per ACME spec, jwk and kid are mutually exclusive, so only include a
         # jwk field if kid is not provided.

acme/acme/magic_typing.py

@@ -1,6 +1,7 @@
 """Shim class to not have to depend on typing module in prod."""
 import sys
 
+
 class TypingClass(object):
     """Ignore import errors by getting anything"""
     def __getattr__(self, name):
@@ -9,8 +10,6 @@ class TypingClass(object):
 try:
     # mypy doesn't respect modifying sys.modules
     from typing import *  # pylint: disable=wildcard-import, unused-wildcard-import
-    # pylint: disable=unused-import
     from typing import Collection, IO  # type: ignore
-    # pylint: enable=unused-import
 except ImportError:
     sys.modules[__name__] = TypingClass()

acme/acme/messages.py

@@ -1,37 +1,56 @@
 """ACME protocol messages."""
-import six
 import json
-try:
-    from collections.abc import Hashable  # pylint: disable=no-name-in-module
-except ImportError:  # pragma: no cover
-    from collections import Hashable
 
 import josepy as jose
+import six
 
 from acme import challenges
 from acme import errors
 from acme import fields
-from acme import util
 from acme import jws
+from acme import util
+from acme.mixins import ResourceMixin
+
+try:
+    from collections.abc import Hashable
+except ImportError:  # pragma: no cover
+    from collections import Hashable
+
+
 
 OLD_ERROR_PREFIX = "urn:acme:error:"
 ERROR_PREFIX = "urn:ietf:params:acme:error:"
 
 ERROR_CODES = {
+    'accountDoesNotExist': 'The request specified an account that does not exist',
+    'alreadyRevoked': 'The request specified a certificate to be revoked that has' \
+    ' already been revoked',
     'badCSR': 'The CSR is unacceptable (e.g., due to a short key)',
     'badNonce': 'The client sent an unacceptable anti-replay nonce',
+    'badPublicKey': 'The JWS was signed by a public key the server does not support',
+    'badRevocationReason': 'The revocation reason provided is not allowed by the server',
+    'badSignatureAlgorithm': 'The JWS was signed with an algorithm the server does not support',
+    'caa': 'Certification Authority Authorization (CAA) records forbid the CA from issuing' \
+    ' a certificate',
+    'compound': 'Specific error conditions are indicated in the "subproblems" array',
     'connection': ('The server could not connect to the client to verify the'
                    ' domain'),
+    'dns': 'There was a problem with a DNS query during identifier validation',
     'dnssec': 'The server could not validate a DNSSEC signed domain',
+    'incorrectResponse': 'Response received didn\'t match the challenge\'s requirements',
     # deprecate invalidEmail
     'invalidEmail': 'The provided email for a registration was invalid',
     'invalidContact': 'The provided contact URI was invalid',
     'malformed': 'The request message was malformed',
+    'rejectedIdentifier': 'The server will not issue certificates for the identifier',
+    'orderNotReady': 'The request attempted to finalize an order that is not ready to be finalized',
     'rateLimited': 'There were too many requests of a given type',
     'serverInternal': 'The server experienced an internal error',
     'tls': 'The server experienced a TLS error during domain verification',
     'unauthorized': 'The client lacks sufficient authorization',
+    'unsupportedContact': 'A contact URL for an account used an unsupported protocol scheme',
     'unknownHost': 'The server could not resolve a domain name',
+    'unsupportedIdentifier': 'An identifier is of an unsupported type',
     'externalAccountRequired': 'The server requires external account binding',
 }
 
@@ -46,8 +65,7 @@ def is_acme_error(err):
     """Check if argument is an ACME error."""
     if isinstance(err, Error) and (err.typ is not None):
         return (ERROR_PREFIX in err.typ) or (OLD_ERROR_PREFIX in err.typ)
-    else:
-        return False
+    return False
 
 
 @six.python_2_unicode_compatible
@@ -102,6 +120,7 @@ class Error(jose.JSONObjectWithFields, errors.Error):
         code = str(self.typ).split(':')[-1]
         if code in ERROR_CODES:
             return code
+        return None
 
     def __str__(self):
         return b' :: '.join(
@@ -116,18 +135,19 @@ class _Constant(jose.JSONDeSerializable, Hashable):  # type: ignore
     POSSIBLE_NAMES = NotImplemented
 
     def __init__(self, name):
-        self.POSSIBLE_NAMES[name] = self
+        super(_Constant, self).__init__()
+        self.POSSIBLE_NAMES[name] = self  # pylint: disable=unsupported-assignment-operation
         self.name = name
 
     def to_partial_json(self):
         return self.name
 
     @classmethod
-    def from_json(cls, value):
-        if value not in cls.POSSIBLE_NAMES:
+    def from_json(cls, jobj):
+        if jobj not in cls.POSSIBLE_NAMES:  # pylint: disable=unsupported-membership-test
             raise jose.DeserializationError(
                 '{0} not recognized'.format(cls.__name__))
-        return cls.POSSIBLE_NAMES[value]
+        return cls.POSSIBLE_NAMES[jobj]
 
     def __repr__(self):
         return '{0}({1})'.format(self.__class__.__name__, self.name)
@@ -152,6 +172,7 @@ STATUS_VALID = Status('valid')
 STATUS_INVALID = Status('invalid')
 STATUS_REVOKED = Status('revoked')
 STATUS_READY = Status('ready')
+STATUS_DEACTIVATED = Status('deactivated')
 
 
 class IdentifierType(_Constant):
@@ -186,7 +207,6 @@ class Directory(jose.JSONDeSerializable):
 
         def __init__(self, **kwargs):
             kwargs = dict((self._internal_name(k), v) for k, v in kwargs.items())
-            # pylint: disable=star-args
             super(Directory.Meta, self).__init__(**kwargs)
 
         @property
@@ -226,13 +246,13 @@ class Directory(jose.JSONDeSerializable):
         try:
             return self[name.replace('_', '-')]
         except KeyError as error:
-            raise AttributeError(str(error) + ': ' + name)
+            raise AttributeError(str(error))
 
     def __getitem__(self, name):
         try:
             return self._jobj[self._canon_key(name)]
         except KeyError:
-            raise KeyError('Directory field not found')
+            raise KeyError('Directory field "' + self._canon_key(name) + '" not found')
 
     def to_partial_json(self):
         return self._jobj
@@ -322,7 +342,7 @@ class Registration(ResourceBody):
 
     def _filter_contact(self, prefix):
         return tuple(
-            detail[len(prefix):] for detail in self.contact
+            detail[len(prefix):] for detail in self.contact  # pylint: disable=not-an-iterable
             if detail.startswith(prefix))
 
     @property
@@ -337,13 +357,13 @@ class Registration(ResourceBody):
 
 
 @Directory.register
-class NewRegistration(Registration):
+class NewRegistration(ResourceMixin, Registration):
     """New registration."""
     resource_type = 'new-reg'
     resource = fields.Resource(resource_type)
 
 
-class UpdateRegistration(Registration):
+class UpdateRegistration(ResourceMixin, Registration):
     """Update registration."""
     resource_type = 'reg'
     resource = fields.Resource(resource_type)
@@ -394,7 +414,6 @@ class ChallengeBody(ResourceBody):
 
     def __init__(self, **kwargs):
         kwargs = dict((self._internal_name(k), v) for k, v in kwargs.items())
-        # pylint: disable=star-args
         super(ChallengeBody, self).__init__(**kwargs)
 
     def encode(self, name):
@@ -442,7 +461,6 @@ class ChallengeResource(Resource):
     @property
     def uri(self):
         """The URL of the challenge body."""
-        # pylint: disable=function-redefined,no-member
         return self.body.uri
 
 
@@ -457,7 +475,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)
 
@@ -470,23 +488,29 @@ class Authorization(ResourceBody):
     wildcard = jose.Field('wildcard', omitempty=True)
 
     @challenges.decoder
-    def challenges(value):  # pylint: disable=missing-docstring,no-self-argument
+    def challenges(value):  # pylint: disable=no-self-argument,missing-function-docstring
         return tuple(ChallengeBody.from_json(chall) for chall in value)
 
     @property
     def resolved_combinations(self):
         """Combinations with challenges instead of indices."""
         return tuple(tuple(self.challenges[idx] for idx in combo)
-                     for combo in self.combinations)
+                     for combo in self.combinations)  # pylint: disable=not-an-iterable
 
 
 @Directory.register
-class NewAuthorization(Authorization):
+class NewAuthorization(ResourceMixin, Authorization):
     """New authorization."""
     resource_type = 'new-authz'
     resource = fields.Resource(resource_type)
 
 
+class UpdateAuthorization(ResourceMixin, Authorization):
+    """Update authorization."""
+    resource_type = 'authz'
+    resource = fields.Resource(resource_type)
+
+
 class AuthorizationResource(ResourceWithURI):
     """Authorization Resource.
 
@@ -499,7 +523,7 @@ class AuthorizationResource(ResourceWithURI):
 
 
 @Directory.register
-class CertificateRequest(jose.JSONObjectWithFields):
+class CertificateRequest(ResourceMixin, jose.JSONObjectWithFields):
     """ACME new-cert request.
 
     :ivar josepy.util.ComparableX509 csr:
@@ -525,7 +549,7 @@ class CertificateResource(ResourceWithURI):
 
 
 @Directory.register
-class Revocation(jose.JSONObjectWithFields):
+class Revocation(ResourceMixin, jose.JSONObjectWithFields):
     """Revocation message.
 
     :ivar .ComparableX509 certificate: `OpenSSL.crypto.X509` wrapped in
@@ -561,7 +585,7 @@ class Order(ResourceBody):
     error = jose.Field('error', omitempty=True, decoder=Error.from_json)
 
     @identifiers.decoder
-    def identifiers(value):  # pylint: disable=missing-docstring,no-self-argument
+    def identifiers(value):  # pylint: disable=no-self-argument,missing-function-docstring
         return tuple(Identifier.from_json(identifier) for identifier in value)
 
 class OrderResource(ResourceWithURI):

acme/acme/mixins.py

@@ -0,0 +1,65 @@
+"""Useful mixins for Challenge and Resource objects"""
+
+
+class VersionedLEACMEMixin(object):
+    """This mixin stores the version of Let's Encrypt's endpoint being used."""
+    @property
+    def le_acme_version(self):
+        """Define the version of ACME protocol to use"""
+        return getattr(self, '_le_acme_version', 1)
+
+    @le_acme_version.setter
+    def le_acme_version(self, version):
+        # We need to use object.__setattr__ to not depend on the specific implementation of
+        # __setattr__  in current class (eg. jose.TypedJSONObjectWithFields raises AttributeError
+        # for any attempt to set an attribute to make objects immutable).
+        object.__setattr__(self, '_le_acme_version', version)
+
+    def __setattr__(self, key, value):
+        if key == 'le_acme_version':
+            # Required for @property to operate properly. See comment above.
+            object.__setattr__(self, key, value)
+        else:
+            super(VersionedLEACMEMixin, self).__setattr__(key, value)  # pragma: no cover
+
+
+class ResourceMixin(VersionedLEACMEMixin):
+    """
+    This mixin generates a RFC8555 compliant JWS payload
+    by removing the `resource` field if needed (eg. ACME v2 protocol).
+    """
+    def to_partial_json(self):
+        """See josepy.JSONDeserializable.to_partial_json()"""
+        return _safe_jobj_compliance(super(ResourceMixin, self),
+                                     'to_partial_json', 'resource')
+
+    def fields_to_partial_json(self):
+        """See josepy.JSONObjectWithFields.fields_to_partial_json()"""
+        return _safe_jobj_compliance(super(ResourceMixin, self),
+                                     'fields_to_partial_json', 'resource')
+
+
+class TypeMixin(VersionedLEACMEMixin):
+    """
+    This mixin allows generation of a RFC8555 compliant JWS payload
+    by removing the `type` field if needed (eg. ACME v2 protocol).
+    """
+    def to_partial_json(self):
+        """See josepy.JSONDeserializable.to_partial_json()"""
+        return _safe_jobj_compliance(super(TypeMixin, self),
+                                     'to_partial_json', 'type')
+
+    def fields_to_partial_json(self):
+        """See josepy.JSONObjectWithFields.fields_to_partial_json()"""
+        return _safe_jobj_compliance(super(TypeMixin, self),
+                                     'fields_to_partial_json', 'type')
+
+
+def _safe_jobj_compliance(instance, jobj_method, uncompliant_field):
+    if hasattr(instance, jobj_method):
+        jobj = getattr(instance, jobj_method)()
+        if instance.le_acme_version == 2:
+            jobj.pop(uncompliant_field, None)
+        return jobj
+
+    raise AttributeError('Method {0}() is not implemented.'.format(jobj_method))  # pragma: no cover

acme/acme/standalone.py

@@ -1,30 +1,20 @@
 """Support for standalone client challenge solvers. """
-import argparse
 import collections
 import functools
 import logging
-import os
 import socket
-import sys
 import threading
 
-from six.moves import BaseHTTPServer  # type: ignore  # pylint: disable=import-error
-from six.moves import http_client  # pylint: disable=import-error
-from six.moves import socketserver  # type: ignore  # pylint: disable=import-error
-
-import OpenSSL
+from six.moves import BaseHTTPServer  # type: ignore
+from six.moves import http_client
+from six.moves import socketserver  # type: ignore
 
 from acme import challenges
 from acme import crypto_util
-from acme.magic_typing import List # pylint: disable=unused-import, no-name-in-module
-from acme import _TLSSNI01DeprecationModule
-
+from acme.magic_typing import List
 
 logger = logging.getLogger(__name__)
 
-# six.moves.* | pylint: disable=no-member,attribute-defined-outside-init
-# pylint: disable=too-few-public-methods,no-init
-
 
 class TLSServer(socketserver.TCPServer):
     """Generic TLS Server."""
@@ -37,21 +27,27 @@ class TLSServer(socketserver.TCPServer):
             self.address_family = socket.AF_INET
         self.certs = kwargs.pop("certs", {})
         self.method = kwargs.pop(
-            # pylint: disable=protected-access
             "method", crypto_util._DEFAULT_SSL_METHOD)
         self.allow_reuse_address = kwargs.pop("allow_reuse_address", True)
         socketserver.TCPServer.__init__(self, *args, **kwargs)
 
     def _wrap_sock(self):
         self.socket = crypto_util.SSLSocket(
-            self.socket, certs=self.certs, method=self.method)
+            self.socket, cert_selection=self._cert_selection,
+            alpn_selection=getattr(self, '_alpn_selection', None),
+            method=self.method)
 
-    def server_bind(self):  # pylint: disable=missing-docstring
+    def _cert_selection(self, connection):  # pragma: no cover
+        """Callback selecting certificate for connection."""
+        server_name = connection.get_servername()
+        return self.certs.get(server_name, None)
+
+    def server_bind(self):
         self._wrap_sock()
         return socketserver.TCPServer.server_bind(self)
 
 
-class ACMEServerMixin:  # pylint: disable=old-style-class
+class ACMEServerMixin:
     """ACME server common settings mixin."""
     # TODO: c.f. #858
     server_version = "ACME client standalone challenge solver"
@@ -83,7 +79,7 @@ class BaseDualNetworkedServers(object):
                 kwargs["ipv6"] = ip_version
                 new_address = (server_address[0],) + (port,) + server_address[2:]
                 new_args = (new_address,) + remaining_args
-                server = ServerClass(*new_args, **kwargs) # pylint: disable=star-args
+                server = ServerClass(*new_args, **kwargs)
                 logger.debug(
                     "Successfully bound to %s:%s using %s", new_address[0],
                     new_address[1], "IPv6" if ip_version else "IPv4")
@@ -91,8 +87,8 @@ class BaseDualNetworkedServers(object):
                 if self.servers:
                     # Already bound using IPv6.
                     logger.debug(
-                        "Certbot wasn't able to bind to %s:%s using %s, this " +
-                        "is often expected due to the dual stack nature of " +
+                        "Certbot wasn't able to bind to %s:%s using %s, this "
+                        "is often expected due to the dual stack nature of "
                         "IPv6 socket implementations.",
                         new_address[0], new_address[1],
                         "IPv6" if ip_version else "IPv4")
@@ -105,14 +101,13 @@ class BaseDualNetworkedServers(object):
                 # If two servers are set up and port 0 was passed in, ensure we always
                 # bind to the same port for both servers.
                 port = server.socket.getsockname()[1]
-        if len(self.servers) == 0:
+        if not self.servers:
             raise socket.error("Could not bind to IPv4 or IPv6.")
 
     def serve_forever(self):
         """Wraps socketserver.TCPServer.serve_forever"""
         for server in self.servers:
             thread = threading.Thread(
-                # pylint: disable=no-member
                 target=server.serve_forever)
             thread.start()
             self.threads.append(thread)
@@ -132,33 +127,38 @@ class BaseDualNetworkedServers(object):
         self.threads = []
 
 
-class TLSSNI01Server(TLSServer, ACMEServerMixin):
-    """TLSSNI01 Server."""
+class TLSALPN01Server(TLSServer, ACMEServerMixin):
+    """TLSALPN01 Server."""
 
-    def __init__(self, server_address, certs, ipv6=False):
+    ACME_TLS_1_PROTOCOL = b"acme-tls/1"
+
+    def __init__(self, server_address, certs, challenge_certs, ipv6=False):
         TLSServer.__init__(
-            self, server_address, BaseRequestHandlerWithLogging, certs=certs, ipv6=ipv6)
+            self, server_address, _BaseRequestHandlerWithLogging, certs=certs,
+            ipv6=ipv6)
+        self.challenge_certs = challenge_certs
 
+    def _cert_selection(self, connection):
+        # TODO: We would like to serve challenge cert only if asked for it via
+        # ALPN. To do this, we need to retrieve the list of protos from client
+        # hello, but this is currently impossible with openssl [0], and ALPN
+        # negotiation is done after cert selection.
+        # Therefore, currently we always return challenge cert, and terminate
+        # handshake in alpn_selection() if ALPN protos are not what we expect.
+        # [0] https://github.com/openssl/openssl/issues/4952
+        server_name = connection.get_servername()
+        logger.debug("Serving challenge cert for server name %s", server_name)
+        return self.challenge_certs.get(server_name, None)
 
-class TLSSNI01DualNetworkedServers(BaseDualNetworkedServers):
-    """TLSSNI01Server Wrapper. Tries everything for both. Failures for one don't
-       affect the other."""
-
-    def __init__(self, *args, **kwargs):
-        BaseDualNetworkedServers.__init__(self, TLSSNI01Server, *args, **kwargs)
-
-
-class BaseRequestHandlerWithLogging(socketserver.BaseRequestHandler):
-    """BaseRequestHandler with logging."""
-
-    def log_message(self, format, *args):  # pylint: disable=redefined-builtin
-        """Log arbitrary message."""
-        logger.debug("%s - - %s", self.client_address[0], format % args)
-
-    def handle(self):
-        """Handle request."""
-        self.log_message("Incoming request")
-        socketserver.BaseRequestHandler.handle(self)
+    def _alpn_selection(self, _connection, alpn_protos):
+        """Callback to select alpn protocol."""
+        if len(alpn_protos) == 1 and alpn_protos[0] == self.ACME_TLS_1_PROTOCOL:
+            logger.debug("Agreed on %s ALPN", self.ACME_TLS_1_PROTOCOL)
+            return self.ACME_TLS_1_PROTOCOL
+        logger.debug("Cannot agree on ALPN proto. Got: %s", str(alpn_protos))
+        # Explicitly close the connection now, by returning an empty string.
+        # See https://www.pyopenssl.org/en/stable/api/ssl.html#OpenSSL.SSL.Context.set_alpn_select_callback  # pylint: disable=line-too-long
+        return b""
 
 
 class HTTPServer(BaseHTTPServer.HTTPServer):
@@ -176,10 +176,10 @@ class HTTPServer(BaseHTTPServer.HTTPServer):
 class HTTP01Server(HTTPServer, ACMEServerMixin):
     """HTTP01 Server."""
 
-    def __init__(self, server_address, resources, ipv6=False):
+    def __init__(self, server_address, resources, ipv6=False, timeout=30):
         HTTPServer.__init__(
             self, server_address, HTTP01RequestHandler.partial_init(
-                simple_http_resources=resources), ipv6=ipv6)
+                simple_http_resources=resources, timeout=timeout), ipv6=ipv6)
 
 
 class HTTP01DualNetworkedServers(BaseDualNetworkedServers):
@@ -204,6 +204,7 @@ class HTTP01RequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
 
     def __init__(self, *args, **kwargs):
         self.simple_http_resources = kwargs.pop("simple_http_resources", set())
+        self.timeout = kwargs.pop('timeout', 30)
         BaseHTTPServer.BaseHTTPRequestHandler.__init__(self, *args, **kwargs)
 
     def log_message(self, format, *args):  # pylint: disable=redefined-builtin
@@ -215,7 +216,7 @@ class HTTP01RequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
         self.log_message("Incoming request")
         BaseHTTPServer.BaseHTTPRequestHandler.handle(self)
 
-    def do_GET(self):  # pylint: disable=invalid-name,missing-docstring
+    def do_GET(self):  # pylint: disable=invalid-name,missing-function-docstring
         if self.path == "/":
             self.handle_index()
         elif self.path.startswith("/" + challenges.HTTP01.URI_ROOT_PATH):
@@ -253,7 +254,7 @@ class HTTP01RequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
                          self.path)
 
     @classmethod
-    def partial_init(cls, simple_http_resources):
+    def partial_init(cls, simple_http_resources, timeout):
         """Partially initialize this handler.
 
         This is useful because `socketserver.BaseServer` takes
@@ -262,44 +263,18 @@ class HTTP01RequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
 
         """
         return functools.partial(
-            cls, simple_http_resources=simple_http_resources)
+            cls, simple_http_resources=simple_http_resources,
+            timeout=timeout)
 
 
-def simple_tls_sni_01_server(cli_args, forever=True):
-    """Run simple standalone TLSSNI01 server."""
-    logging.basicConfig(level=logging.DEBUG)
+class _BaseRequestHandlerWithLogging(socketserver.BaseRequestHandler):
+    """BaseRequestHandler with logging."""
 
-    parser = argparse.ArgumentParser()
-    parser.add_argument(
-        "-p", "--port", default=0, help="Port to serve at. By default "
-        "picks random free port.")
-    args = parser.parse_args(cli_args[1:])
+    def log_message(self, format, *args):  # pylint: disable=redefined-builtin
+        """Log arbitrary message."""
+        logger.debug("%s - - %s", self.client_address[0], format % args)
 
-    certs = {}
-
-    _, hosts, _ = next(os.walk('.')) # type: ignore # https://github.com/python/mypy/issues/465
-    for host in hosts:
-        with open(os.path.join(host, "cert.pem")) as cert_file:
-            cert_contents = cert_file.read()
-        with open(os.path.join(host, "key.pem")) as key_file:
-            key_contents = key_file.read()
-        certs[host.encode()] = (
-            OpenSSL.crypto.load_privatekey(
-                OpenSSL.crypto.FILETYPE_PEM, key_contents),
-            OpenSSL.crypto.load_certificate(
-                OpenSSL.crypto.FILETYPE_PEM, cert_contents))
-
-    server = TLSSNI01Server(('', int(args.port)), certs=certs)
-    logger.info("Serving at https://%s:%s...", *server.socket.getsockname()[:2])
-    if forever:  # pragma: no cover
-        server.serve_forever()
-    else:
-        server.handle_request()
-
-
-# Patching ourselves to warn about TLS-SNI challenge deprecation and removal.
-sys.modules[__name__] = _TLSSNI01DeprecationModule(sys.modules[__name__])
-
-
-if __name__ == "__main__":
-    sys.exit(simple_tls_sni_01_server(sys.argv))  # pragma: no cover
+    def handle(self):
+        """Handle request."""
+        self.log_message("Incoming request")
+        socketserver.BaseRequestHandler.handle(self)

acme/docs/conf.py

@@ -12,10 +12,8 @@
 # All configuration values have a default; values that are commented out
 # serve to show the default.
 
-import sys
 import os
-import shlex
-
+import sys
 
 here = os.path.abspath(os.path.dirname(__file__))
 
@@ -42,7 +40,7 @@ extensions = [
 ]
 
 autodoc_member_order = 'bysource'
-autodoc_default_flags = ['show-inheritance', 'private-members']
+autodoc_default_flags = ['show-inheritance']
 
 # Add any paths that contain templates here, relative to this directory.
 templates_path = ['_templates']
@@ -114,7 +112,7 @@ pygments_style = 'sphinx'
 #keep_warnings = False
 
 # If true, `todo` and `todoList` produce output, else they produce nothing.
-todo_include_todos = True
+todo_include_todos = False
 
 
 # -- Options for HTML output ----------------------------------------------

acme/examples/http01_example.py

@@ -26,8 +26,10 @@ Workflow:
     - Deactivate Account
 """
 from contextlib import contextmanager
+
 from cryptography.hazmat.backends import default_backend
 from cryptography.hazmat.primitives.asymmetric import rsa
+import josepy as jose
 import OpenSSL
 
 from acme import challenges
@@ -36,7 +38,6 @@ from acme import crypto_util
 from acme import errors
 from acme import messages
 from acme import standalone
-import josepy as jose
 
 # Constants:
 

acme/readthedocs.org.requirements.txt

@@ -1,10 +1,10 @@
 # readthedocs.org gives no way to change the install command to "pip
-# install -e .[docs]" (that would in turn install documentation
+# install -e acme[docs]" (that would in turn install documentation
 # dependencies), but it allows to specify a requirements.txt file at
 # https://readthedocs.org/dashboard/letsencrypt/advanced/ (c.f. #259)
 
 # Although ReadTheDocs certainly doesn't need to install the project
-# in --editable mode (-e), just "pip install .[docs]" does not work as
-# expected and "pip install -e .[docs]" must be used instead
+# in --editable mode (-e), just "pip install acme[docs]" does not work as
+# expected and "pip install -e acme[docs]" must be used instead
 
 -e acme[docs]

acme/setup.py

@@ -1,9 +1,12 @@
-from setuptools import setup
-from setuptools import find_packages
-from setuptools.command.test import test as TestCommand
+from distutils.version import StrictVersion
 import sys
 
-version = '0.33.0'
+from setuptools import __version__ as setuptools_version
+from setuptools import find_packages
+from setuptools import setup
+from setuptools.command.test import test as TestCommand
+
+version = '1.4.0.dev0'
 
 # Please update tox.ini when modifying dependency version requirements
 install_requires = [
@@ -15,7 +18,6 @@ install_requires = [
     # https://github.com/certbot/josepy/issues/13.
     'josepy>=1.1.0',
     # Connection.set_tlsext_host_name (>=0.13)
-    'mock',
     'PyOpenSSL>=0.13.1',
     'pyrfc3339',
     'pytz',
@@ -25,6 +27,15 @@ install_requires = [
     'six>=1.9.0',  # needed for python_2_unicode_compatible
 ]
 
+setuptools_known_environment_markers = (StrictVersion(setuptools_version) >= StrictVersion('36.2'))
+if setuptools_known_environment_markers:
+    install_requires.append('mock ; python_version < "3.3"')
+elif 'bdist_wheel' in sys.argv[1:]:
+    raise RuntimeError('Error, you are trying to build certbot wheels using an old version '
+                       'of setuptools. Version 36.2+ of setuptools is required.')
+elif sys.version_info < (3,3):
+    install_requires.append('mock')
+
 dev_extras = [
     'pytest',
     'pytest-xdist',
@@ -60,7 +71,7 @@ setup(
     author="Certbot Project",
     author_email='client-dev@letsencrypt.org',
     license='Apache License 2.0',
-    python_requires='>=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*',
+    python_requires='>=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*, !=3.4.*',
     classifiers=[
         'Development Status :: 5 - Production/Stable',
         'Intended Audience :: Developers',
@@ -69,10 +80,10 @@ setup(
         'Programming Language :: Python :: 2',
         'Programming Language :: Python :: 2.7',
         'Programming Language :: Python :: 3',
-        'Programming Language :: Python :: 3.4',
         'Programming Language :: Python :: 3.5',
         'Programming Language :: Python :: 3.6',
         'Programming Language :: Python :: 3.7',
+        'Programming Language :: Python :: 3.8',
         'Topic :: Internet :: WWW/HTTP',
         'Topic :: Security',
     ],

acme/tests/challenges_test.py

@@ -2,14 +2,17 @@
 import unittest
 
 import josepy as jose
-import mock
 import OpenSSL
+try:
+    import mock
+except ImportError: # pragma: no cover
+    from unittest import mock # type: ignore
 import requests
-
-from six.moves.urllib import parse as urllib_parse  # pylint: disable=import-error
+from six.moves.urllib import parse as urllib_parse
 
 from acme import errors
-from acme import test_util
+
+import test_util
 
 CERT = test_util.load_comparable_cert('cert.pem')
 KEY = jose.JWKRSA(key=test_util.load_rsa_private_key('rsa512_key.pem'))
@@ -21,7 +24,6 @@ class ChallengeTest(unittest.TestCase):
         from acme.challenges import Challenge
         from acme.challenges import UnrecognizedChallenge
         chall = UnrecognizedChallenge({"type": "foo"})
-        # pylint: disable=no-member
         self.assertEqual(chall, Challenge.from_json(chall.jobj))
 
 
@@ -77,7 +79,6 @@ class KeyAuthorizationChallengeResponseTest(unittest.TestCase):
 
 
 class DNS01ResponseTest(unittest.TestCase):
-    # pylint: disable=too-many-instance-attributes
 
     def setUp(self):
         from acme.challenges import DNS01Response
@@ -95,8 +96,6 @@ class DNS01ResponseTest(unittest.TestCase):
     def test_to_partial_json(self):
         self.assertEqual({k: v for k, v in self.jmsg.items() if k != 'keyAuthorization'},
                          self.msg.to_partial_json())
-        self.msg._dump_authorization_key(True)  # pylint: disable=protected-access
-        self.assertEqual(self.jmsg, self.msg.to_partial_json())
 
     def test_from_json(self):
         from acme.challenges import DNS01Response
@@ -151,7 +150,6 @@ class DNS01Test(unittest.TestCase):
 
 
 class HTTP01ResponseTest(unittest.TestCase):
-    # pylint: disable=too-many-instance-attributes
 
     def setUp(self):
         from acme.challenges import HTTP01Response
@@ -169,8 +167,6 @@ class HTTP01ResponseTest(unittest.TestCase):
     def test_to_partial_json(self):
         self.assertEqual({k: v for k, v in self.jmsg.items() if k != 'keyAuthorization'},
                          self.msg.to_partial_json())
-        self.msg._dump_authorization_key(True)  # pylint: disable=protected-access
-        self.assertEqual(self.jmsg, self.msg.to_partial_json())
 
     def test_from_json(self):
         from acme.challenges import HTTP01Response
@@ -191,7 +187,7 @@ class HTTP01ResponseTest(unittest.TestCase):
         mock_get.return_value = mock.MagicMock(text=validation)
         self.assertTrue(self.response.simple_verify(
             self.chall, "local", KEY.public_key()))
-        mock_get.assert_called_once_with(self.chall.uri("local"))
+        mock_get.assert_called_once_with(self.chall.uri("local"), verify=False)
 
     @mock.patch("acme.challenges.requests.get")
     def test_simple_verify_bad_validation(self, mock_get):
@@ -207,7 +203,7 @@ class HTTP01ResponseTest(unittest.TestCase):
                   HTTP01Response.WHITESPACE_CUTSET))
         self.assertTrue(self.response.simple_verify(
             self.chall, "local", KEY.public_key()))
-        mock_get.assert_called_once_with(self.chall.uri("local"))
+        mock_get.assert_called_once_with(self.chall.uri("local"), verify=False)
 
     @mock.patch("acme.challenges.requests.get")
     def test_simple_verify_connection_error(self, mock_get):
@@ -263,45 +259,68 @@ class HTTP01Test(unittest.TestCase):
             self.msg.update(token=b'..').good_token)
 
 
-class TLSSNI01ResponseTest(unittest.TestCase):
-    # pylint: disable=too-many-instance-attributes
+class TLSALPN01ResponseTest(unittest.TestCase):
 
     def setUp(self):
-        from acme.challenges import TLSSNI01
-        self.chall = TLSSNI01(
+        from acme.challenges import TLSALPN01
+        self.chall = TLSALPN01(
             token=jose.b64decode(b'a82d5ff8ef740d12881f6d3c2277ab2e'))
+        self.domain = u'example.com'
+        self.domain2 = u'example2.com'
 
         self.response = self.chall.response(KEY)
         self.jmsg = {
             'resource': 'challenge',
-            'type': 'tls-sni-01',
+            'type': 'tls-alpn-01',
             'keyAuthorization': self.response.key_authorization,
         }
 
-        # pylint: disable=invalid-name
-        label1 = b'dc38d9c3fa1a4fdcc3a5501f2d38583f'
-        label2 = b'b7793728f084394f2a1afd459556bb5c'
-        self.z = label1 + label2
-        self.z_domain = label1 + b'.' + label2 + b'.acme.invalid'
-        self.domain = 'foo.com'
-
-    def test_z_and_domain(self):
-        self.assertEqual(self.z, self.response.z)
-        self.assertEqual(self.z_domain, self.response.z_domain)
-
     def test_to_partial_json(self):
         self.assertEqual({k: v for k, v in self.jmsg.items() if k != 'keyAuthorization'},
                          self.response.to_partial_json())
-        self.response._dump_authorization_key(True)  # pylint: disable=protected-access
-        self.assertEqual(self.jmsg, self.response.to_partial_json())
 
     def test_from_json(self):
-        from acme.challenges import TLSSNI01Response
-        self.assertEqual(self.response, TLSSNI01Response.from_json(self.jmsg))
+        from acme.challenges import TLSALPN01Response
+        self.assertEqual(self.response, TLSALPN01Response.from_json(self.jmsg))
 
     def test_from_json_hashable(self):
-        from acme.challenges import TLSSNI01Response
-        hash(TLSSNI01Response.from_json(self.jmsg))
+        from acme.challenges import TLSALPN01Response
+        hash(TLSALPN01Response.from_json(self.jmsg))
+
+    def test_gen_verify_cert(self):
+        key1 = test_util.load_pyopenssl_private_key('rsa512_key.pem')
+        cert, key2 = self.response.gen_cert(self.domain, key1)
+        self.assertEqual(key1, key2)
+        self.assertTrue(self.response.verify_cert(self.domain, cert))
+
+    def test_gen_verify_cert_gen_key(self):
+        cert, key = self.response.gen_cert(self.domain)
+        self.assertTrue(isinstance(key, OpenSSL.crypto.PKey))
+        self.assertTrue(self.response.verify_cert(self.domain, cert))
+
+    def test_verify_bad_cert(self):
+        self.assertFalse(self.response.verify_cert(self.domain,
+            test_util.load_cert('cert.pem')))
+
+    def test_verify_bad_domain(self):
+        key1 = test_util.load_pyopenssl_private_key('rsa512_key.pem')
+        cert, key2 = self.response.gen_cert(self.domain, key1)
+        self.assertEqual(key1, key2)
+        self.assertFalse(self.response.verify_cert(self.domain2, cert))
+
+    def test_simple_verify_bad_key_authorization(self):
+        key2 = jose.JWKRSA.load(test_util.load_vector('rsa256_key.pem'))
+        self.response.simple_verify(self.chall, "local", key2.public_key())
+
+    @mock.patch('acme.challenges.TLSALPN01Response.verify_cert', autospec=True)
+    def test_simple_verify(self, mock_verify_cert):
+        mock_verify_cert.return_value = mock.sentinel.verification
+        self.assertEqual(
+            mock.sentinel.verification, self.response.simple_verify(
+                self.chall, self.domain, KEY.public_key(),
+                cert=mock.sentinel.cert))
+        mock_verify_cert.assert_called_once_with(
+            self.response, self.domain, mock.sentinel.cert)
 
     @mock.patch('acme.challenges.socket.gethostbyname')
     @mock.patch('acme.challenges.crypto_util.probe_sni')
@@ -310,136 +329,21 @@ class TLSSNI01ResponseTest(unittest.TestCase):
         self.response.probe_cert('foo.com')
         mock_gethostbyname.assert_called_once_with('foo.com')
         mock_probe_sni.assert_called_once_with(
-            host='127.0.0.1', port=self.response.PORT,
-            name=self.z_domain)
+            host='127.0.0.1', port=self.response.PORT, name='foo.com',
+            alpn_protocols=['acme-tls/1'])
 
         self.response.probe_cert('foo.com', host='8.8.8.8')
         mock_probe_sni.assert_called_with(
-            host='8.8.8.8', port=mock.ANY, name=mock.ANY)
+            host='8.8.8.8', port=mock.ANY, name='foo.com',
+            alpn_protocols=['acme-tls/1'])
 
-        self.response.probe_cert('foo.com', port=1234)
-        mock_probe_sni.assert_called_with(
-            host=mock.ANY, port=1234, name=mock.ANY)
-
-        self.response.probe_cert('foo.com', bar='baz')
-        mock_probe_sni.assert_called_with(
-            host=mock.ANY, port=mock.ANY, name=mock.ANY, bar='baz')
-
-        self.response.probe_cert('foo.com', name=b'xxx')
-        mock_probe_sni.assert_called_with(
-            host=mock.ANY, port=mock.ANY,
-            name=self.z_domain)
-
-    def test_gen_verify_cert(self):
-        key1 = test_util.load_pyopenssl_private_key('rsa512_key.pem')
-        cert, key2 = self.response.gen_cert(key1)
-        self.assertEqual(key1, key2)
-        self.assertTrue(self.response.verify_cert(cert))
-
-    def test_gen_verify_cert_gen_key(self):
-        cert, key = self.response.gen_cert()
-        self.assertTrue(isinstance(key, OpenSSL.crypto.PKey))
-        self.assertTrue(self.response.verify_cert(cert))
-
-    def test_verify_bad_cert(self):
-        self.assertFalse(self.response.verify_cert(
-            test_util.load_cert('cert.pem')))
-
-    def test_simple_verify_bad_key_authorization(self):
-        key2 = jose.JWKRSA.load(test_util.load_vector('rsa256_key.pem'))
-        self.response.simple_verify(self.chall, "local", key2.public_key())
-
-    @mock.patch('acme.challenges.TLSSNI01Response.verify_cert', autospec=True)
-    def test_simple_verify(self, mock_verify_cert):
-        mock_verify_cert.return_value = mock.sentinel.verification
-        self.assertEqual(
-            mock.sentinel.verification, self.response.simple_verify(
-                self.chall, self.domain, KEY.public_key(),
-                cert=mock.sentinel.cert))
-        mock_verify_cert.assert_called_once_with(
-            self.response, mock.sentinel.cert)
-
-    @mock.patch('acme.challenges.TLSSNI01Response.probe_cert')
+    @mock.patch('acme.challenges.TLSALPN01Response.probe_cert')
     def test_simple_verify_false_on_probe_error(self, mock_probe_cert):
         mock_probe_cert.side_effect = errors.Error
         self.assertFalse(self.response.simple_verify(
             self.chall, self.domain, KEY.public_key()))
 
 
-class TLSSNI01Test(unittest.TestCase):
-
-    def setUp(self):
-        self.jmsg = {
-            'type': 'tls-sni-01',
-            'token': 'a82d5ff8ef740d12881f6d3c2277ab2e',
-        }
-        from acme.challenges import TLSSNI01
-        self.msg = TLSSNI01(
-            token=jose.b64decode('a82d5ff8ef740d12881f6d3c2277ab2e'))
-
-    def test_to_partial_json(self):
-        self.assertEqual(self.jmsg, self.msg.to_partial_json())
-
-    def test_from_json(self):
-        from acme.challenges import TLSSNI01
-        self.assertEqual(self.msg, TLSSNI01.from_json(self.jmsg))
-
-    def test_from_json_hashable(self):
-        from acme.challenges import TLSSNI01
-        hash(TLSSNI01.from_json(self.jmsg))
-
-    def test_from_json_invalid_token_length(self):
-        from acme.challenges import TLSSNI01
-        self.jmsg['token'] = jose.encode_b64jose(b'abcd')
-        self.assertRaises(
-            jose.DeserializationError, TLSSNI01.from_json, self.jmsg)
-
-    @mock.patch('acme.challenges.TLSSNI01Response.gen_cert')
-    def test_validation(self, mock_gen_cert):
-        mock_gen_cert.return_value = ('cert', 'key')
-        self.assertEqual(('cert', 'key'), self.msg.validation(
-            KEY, cert_key=mock.sentinel.cert_key))
-        mock_gen_cert.assert_called_once_with(key=mock.sentinel.cert_key)
-
-    def test_deprecation_message(self):
-        with mock.patch('acme.warnings.warn') as mock_warn:
-            from acme.challenges import TLSSNI01
-            assert TLSSNI01
-        self.assertEqual(mock_warn.call_count, 1)
-        self.assertTrue('deprecated' in mock_warn.call_args[0][0])
-
-
-class TLSALPN01ResponseTest(unittest.TestCase):
-    # pylint: disable=too-many-instance-attributes
-
-    def setUp(self):
-        from acme.challenges import TLSALPN01Response
-        self.msg = TLSALPN01Response(key_authorization=u'foo')
-        self.jmsg = {
-            'resource': 'challenge',
-            'type': 'tls-alpn-01',
-            'keyAuthorization': u'foo',
-        }
-
-        from acme.challenges import TLSALPN01
-        self.chall = TLSALPN01(token=(b'x' * 16))
-        self.response = self.chall.response(KEY)
-
-    def test_to_partial_json(self):
-        self.assertEqual({k: v for k, v in self.jmsg.items() if k != 'keyAuthorization'},
-                         self.msg.to_partial_json())
-        self.msg._dump_authorization_key(True)  # pylint: disable=protected-access
-        self.assertEqual(self.jmsg, self.msg.to_partial_json())
-
-    def test_from_json(self):
-        from acme.challenges import TLSALPN01Response
-        self.assertEqual(self.msg, TLSALPN01Response.from_json(self.jmsg))
-
-    def test_from_json_hashable(self):
-        from acme.challenges import TLSALPN01Response
-        hash(TLSALPN01Response.from_json(self.jmsg))
-
-
 class TLSALPN01Test(unittest.TestCase):
 
     def setUp(self):
@@ -468,8 +372,13 @@ class TLSALPN01Test(unittest.TestCase):
         self.assertRaises(
             jose.DeserializationError, TLSALPN01.from_json, self.jmsg)
 
-    def test_validation(self):
-        self.assertRaises(NotImplementedError, self.msg.validation, KEY)
+    @mock.patch('acme.challenges.TLSALPN01Response.gen_cert')
+    def test_validation(self, mock_gen_cert):
+        mock_gen_cert.return_value = ('cert', 'key')
+        self.assertEqual(('cert', 'key'), self.msg.validation(
+            KEY, cert_key=mock.sentinel.cert_key, domain=mock.sentinel.domain))
+        mock_gen_cert.assert_called_once_with(key=mock.sentinel.cert_key,
+                                              domain=mock.sentinel.domain)
 
 
 class DNSTest(unittest.TestCase):
@@ -572,5 +481,18 @@ class DNSResponseTest(unittest.TestCase):
             self.msg.check_validation(self.chall, KEY.public_key()))
 
 
+class JWSPayloadRFC8555Compliant(unittest.TestCase):
+    """Test for RFC8555 compliance of JWS generated from resources/challenges"""
+    def test_challenge_payload(self):
+        from acme.challenges import HTTP01Response
+
+        challenge_body = HTTP01Response()
+        challenge_body.le_acme_version = 2
+
+        jobj = challenge_body.json_dumps(indent=2).encode()
+        # RFC8555 states that challenge responses must have an empty payload.
+        self.assertEqual(jobj, b'{}')
+
+
 if __name__ == '__main__':
     unittest.main()  # pragma: no cover

acme/tests/client_test.py

@@ -5,21 +5,22 @@ import datetime
 import json
 import unittest
 
-from six.moves import http_client  # pylint: disable=import-error
-
 import josepy as jose
-import mock
+try:
+    import mock
+except ImportError: # pragma: no cover
+    from unittest import mock # type: ignore
 import OpenSSL
 import requests
+from six.moves import http_client  # pylint: disable=import-error
 
 from acme import challenges
 from acme import errors
 from acme import jws as acme_jws
 from acme import messages
-from acme import messages_test
-from acme import test_util
-from acme.magic_typing import Dict # pylint: disable=unused-import, no-name-in-module
-
+from acme.mixins import VersionedLEACMEMixin
+import messages_test
+import test_util
 
 CERT_DER = test_util.load_vector('cert.der')
 CERT_SAN_PEM = test_util.load_vector('cert-san.pem')
@@ -63,8 +64,8 @@ class ClientTestBase(unittest.TestCase):
         self.contact = ('mailto:cert-admin@example.com', 'tel:+12025551212')
         reg = messages.Registration(
             contact=self.contact, key=KEY.public_key())
-        the_arg = dict(reg) # type: Dict
-        self.new_reg = messages.NewRegistration(**the_arg) # pylint: disable=star-args
+        the_arg = dict(reg)  # type: Dict
+        self.new_reg = messages.NewRegistration(**the_arg)
         self.regr = messages.RegistrationResource(
             body=reg, uri='https://www.letsencrypt-demo.org/acme/reg/1')
 
@@ -318,7 +319,6 @@ class BackwardsCompatibleClientV2Test(ClientTestBase):
 
 class ClientTest(ClientTestBase):
     """Tests for acme.client.Client."""
-    # pylint: disable=too-many-instance-attributes,too-many-public-methods
 
     def setUp(self):
         super(ClientTest, self).setUp()
@@ -358,7 +358,6 @@ class ClientTest(ClientTestBase):
 
     def test_register(self):
         # "Instance of 'Field' has no to_json/update member" bug:
-        # pylint: disable=no-member
         self.response.status_code = http_client.CREATED
         self.response.json.return_value = self.regr.body.to_json()
         self.response.headers['Location'] = self.regr.uri
@@ -371,7 +370,6 @@ class ClientTest(ClientTestBase):
 
     def test_update_registration(self):
         # "Instance of 'Field' has no to_json/update member" bug:
-        # pylint: disable=no-member
         self.response.headers['Location'] = self.regr.uri
         self.response.json.return_value = self.regr.body.to_json()
         self.assertEqual(self.regr, self.client.update_registration(self.regr))
@@ -463,34 +461,6 @@ class ClientTest(ClientTestBase):
             errors.ClientError, self.client.answer_challenge,
             self.challr.body, challenges.DNSResponse(validation=None))
 
-    def test_answer_challenge_key_authorization_fallback(self):
-        self.response.links['up'] = {'url': self.challr.authzr_uri}
-        self.response.json.return_value = self.challr.body.to_json()
-
-        def _wrapper_post(url, obj, *args, **kwargs):  # pylint: disable=unused-argument
-            """
-            Simulate an old ACME CA server, that would respond a 'malformed'
-            error if keyAuthorization is missing.
-            """
-            jobj = obj.to_partial_json()
-            if 'keyAuthorization' not in jobj:
-                raise messages.Error.with_code('malformed')
-            return self.response
-        self.net.post.side_effect = _wrapper_post
-
-        # This challenge response is of type KeyAuthorizationChallengeResponse, so the fallback
-        # should be triggered, and avoid an exception.
-        http_chall_response = challenges.HTTP01Response(key_authorization='test',
-                                                        resource=mock.MagicMock())
-        self.client.answer_challenge(self.challr.body, http_chall_response)
-
-        # This challenge response is not of type KeyAuthorizationChallengeResponse, so the fallback
-        # should not be triggered, leading to an exception.
-        dns_chall_response = challenges.DNSResponse(validation=None)
-        self.assertRaises(
-            errors.Error, self.client.answer_challenge,
-            self.challr.body, dns_chall_response)
-
     def test_retry_after_date(self):
         self.response.headers['Retry-After'] = 'Fri, 31 Dec 1999 23:59:59 GMT'
         self.assertEqual(
@@ -667,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
@@ -872,7 +850,6 @@ class ClientV2Test(ClientTestBase):
 
     def test_update_registration(self):
         # "Instance of 'Field' has no to_json/update member" bug:
-        # pylint: disable=no-member
         self.response.headers['Location'] = self.regr.uri
         self.response.json.return_value = self.regr.body.to_json()
         self.assertEqual(self.regr, self.client.update_registration(self.regr))
@@ -911,21 +888,8 @@ class ClientV2Test(ClientTestBase):
                 new_nonce_url='https://www.letsencrypt-demo.org/acme/new-nonce')
             self.client.net.get.assert_not_called()
 
-            class FakeError(messages.Error):  # pylint: disable=too-many-ancestors
-                """Fake error to reproduce a malformed request ACME error"""
-                def __init__(self):  # pylint: disable=super-init-not-called
-                    pass
-                @property
-                def code(self):
-                    return 'malformed'
-            self.client.net.post.side_effect = FakeError()
 
-            self.client.poll(self.authzr2)  # pylint: disable=protected-access
-
-            self.client.net.get.assert_called_once_with(self.authzr2.uri)
-
-
-class MockJSONDeSerializable(jose.JSONDeSerializable):
+class MockJSONDeSerializable(VersionedLEACMEMixin, jose.JSONDeSerializable):
     # pylint: disable=missing-docstring
     def __init__(self, value):
         self.value = value
@@ -934,13 +898,12 @@ class MockJSONDeSerializable(jose.JSONDeSerializable):
         return {'foo': self.value}
 
     @classmethod
-    def from_json(cls, value):
+    def from_json(cls, jobj):
         pass  # pragma: no cover
 
 
 class ClientNetworkTest(unittest.TestCase):
     """Tests for acme.client.ClientNetwork."""
-    # pylint: disable=too-many-public-methods
 
     def setUp(self):
         self.verify_ssl = mock.MagicMock()
@@ -990,8 +953,8 @@ class ClientNetworkTest(unittest.TestCase):
 
     def test_check_response_not_ok_jobj_error(self):
         self.response.ok = False
-        self.response.json.return_value = messages.Error(
-            detail='foo', typ='serverInternal', title='some title').to_json()
+        self.response.json.return_value = messages.Error.with_code(
+            'serverInternal', detail='foo', title='some title').to_json()
         # pylint: disable=protected-access
         self.assertRaises(
             messages.Error, self.net._check_response, self.response)
@@ -1016,10 +979,39 @@ class ClientNetworkTest(unittest.TestCase):
         self.response.json.side_effect = ValueError
         for response_ct in [self.net.JSON_CONTENT_TYPE, 'foo']:
             self.response.headers['Content-Type'] = response_ct
-            # pylint: disable=protected-access,no-value-for-parameter
+            # pylint: disable=protected-access
             self.assertEqual(
                 self.response, self.net._check_response(self.response))
 
+    @mock.patch('acme.client.logger')
+    def test_check_response_ok_ct_with_charset(self, mock_logger):
+        self.response.json.return_value = {}
+        self.response.headers['Content-Type'] = 'application/json; charset=utf-8'
+        # pylint: disable=protected-access
+        self.assertEqual(self.response, self.net._check_response(
+            self.response, content_type='application/json'))
+        try:
+            mock_logger.debug.assert_called_with(
+                'Ignoring wrong Content-Type (%r) for JSON decodable response',
+                'application/json; charset=utf-8'
+            )
+        except AssertionError:
+            return
+        raise AssertionError('Expected Content-Type warning ' #pragma: no cover
+            'to not have been logged')
+
+    @mock.patch('acme.client.logger')
+    def test_check_response_ok_bad_ct(self, mock_logger):
+        self.response.json.return_value = {}
+        self.response.headers['Content-Type'] = 'text/plain'
+        # pylint: disable=protected-access
+        self.assertEqual(self.response, self.net._check_response(
+            self.response, content_type='application/json'))
+        mock_logger.debug.assert_called_with(
+            'Ignoring wrong Content-Type (%r) for JSON decodable response',
+            'text/plain'
+        )
+
     def test_check_response_conflict(self):
         self.response.ok = False
         self.response.status_code = 409
@@ -1030,7 +1022,7 @@ class ClientNetworkTest(unittest.TestCase):
         self.response.json.return_value = {}
         for response_ct in [self.net.JSON_CONTENT_TYPE, 'foo']:
             self.response.headers['Content-Type'] = response_ct
-            # pylint: disable=protected-access,no-value-for-parameter
+            # pylint: disable=protected-access
             self.assertEqual(
                 self.response, self.net._check_response(self.response))
 
@@ -1146,7 +1138,6 @@ class ClientNetworkTest(unittest.TestCase):
 
 class ClientNetworkWithMockedResponseTest(unittest.TestCase):
     """Tests for acme.client.ClientNetwork which mock out response."""
-    # pylint: disable=too-many-instance-attributes
 
     def setUp(self):
         from acme.client import ClientNetwork
@@ -1156,8 +1147,8 @@ class ClientNetworkWithMockedResponseTest(unittest.TestCase):
         self.response.headers = {}
         self.response.links = {}
         self.response.checked = False
-        self.acmev1_nonce_response = mock.MagicMock(ok=False,
-            status_code=http_client.METHOD_NOT_ALLOWED)
+        self.acmev1_nonce_response = mock.MagicMock(
+            ok=False, status_code=http_client.METHOD_NOT_ALLOWED)
         self.acmev1_nonce_response.headers = {}
         self.obj = mock.MagicMock()
         self.wrapped_obj = mock.MagicMock()

acme/tests/crypto_util_test.py

@@ -5,21 +5,18 @@ import threading
 import time
 import unittest
 
-import six
-from six.moves import socketserver  #type: ignore  # pylint: disable=import-error
-
 import josepy as jose
 import OpenSSL
+import six
+from six.moves import socketserver  # type: ignore  # pylint: disable=import-error
 
 from acme import errors
-from acme import test_util
-from acme.magic_typing import List # pylint: disable=unused-import, no-name-in-module
+import test_util
 
 
 class SSLSocketAndProbeSNITest(unittest.TestCase):
     """Tests for acme.crypto_util.SSLSocket/probe_sni."""
 
-
     def setUp(self):
         self.cert = test_util.load_comparable_cert('rsa2048_cert.pem')
         key = test_util.load_pyopenssl_private_key('rsa2048_key.pem')
@@ -30,17 +27,16 @@ class SSLSocketAndProbeSNITest(unittest.TestCase):
 
         class _TestServer(socketserver.TCPServer):
 
-            # pylint: disable=too-few-public-methods
             # six.moves.* | pylint: disable=attribute-defined-outside-init,no-init
 
             def server_bind(self):  # pylint: disable=missing-docstring
-                self.socket = SSLSocket(socket.socket(), certs=certs)
+                self.socket = SSLSocket(socket.socket(),
+                        certs)
                 socketserver.TCPServer.server_bind(self)
 
         self.server = _TestServer(('', 0), socketserver.BaseRequestHandler)
         self.port = self.server.socket.getsockname()[1]
         self.server_thread = threading.Thread(
-            # pylint: disable=no-member
             target=self.server.handle_request)
 
     def tearDown(self):
@@ -67,7 +63,7 @@ class SSLSocketAndProbeSNITest(unittest.TestCase):
 
     def test_probe_connection_error(self):
         # pylint has a hard time with six
-        self.server.server_close()  # pylint: disable=no-member
+        self.server.server_close()
         original_timeout = socket.getdefaulttimeout()
         try:
             socket.setdefaulttimeout(1)
@@ -76,6 +72,18 @@ class SSLSocketAndProbeSNITest(unittest.TestCase):
             socket.setdefaulttimeout(original_timeout)
 
 
+class SSLSocketTest(unittest.TestCase):
+    """Tests for acme.crypto_util.SSLSocket."""
+
+    def test_ssl_socket_invalid_arguments(self):
+        from acme.crypto_util import SSLSocket
+        with self.assertRaises(ValueError):
+            _ = SSLSocket(None, {'sni': ('key', 'cert')},
+                    cert_selection=lambda _: None)
+        with self.assertRaises(ValueError):
+            _ = SSLSocket(None)
+
+
 class PyOpenSSLCertOrReqAllNamesTest(unittest.TestCase):
     """Test for acme.crypto_util._pyopenssl_cert_or_req_all_names."""
 

acme/tests/errors_test.py

@@ -1,7 +1,10 @@
 """Tests for acme.errors."""
 import unittest
 
-import mock
+try:
+    import mock
+except ImportError: # pragma: no cover
+    from unittest import mock # type: ignore
 
 
 class BadNonceTest(unittest.TestCase):
@@ -35,7 +38,7 @@ class PollErrorTest(unittest.TestCase):
     def setUp(self):
         from acme.errors import PollError
         self.timeout = PollError(
-            exhausted=set([mock.sentinel.AR]),
+            exhausted={mock.sentinel.AR},
             updated={})
         self.invalid = PollError(exhausted=set(), updated={
             mock.sentinel.AR: mock.sentinel.AR2})

acme/tests/jose_test.py

@@ -2,6 +2,7 @@
 import importlib
 import unittest
 
+
 class JoseTest(unittest.TestCase):
     """Tests for acme.jose shim."""
 
@@ -20,11 +21,10 @@ class JoseTest(unittest.TestCase):
 
         # We use the imports below with eval, but pylint doesn't
         # understand that.
-        # pylint: disable=eval-used,unused-variable
-        import acme
-        import josepy
-        acme_jose_mod = eval(acme_jose_path)
-        josepy_mod = eval(josepy_path)
+        import acme  # pylint: disable=unused-import
+        import josepy  # pylint: disable=unused-import
+        acme_jose_mod = eval(acme_jose_path)  # pylint: disable=eval-used
+        josepy_mod = eval(josepy_path)  # pylint: disable=eval-used
         self.assertIs(acme_jose_mod, josepy_mod)
         self.assertIs(getattr(acme_jose_mod, attribute), getattr(josepy_mod, attribute))
 

acme/tests/jws_test.py

@@ -3,8 +3,7 @@ import unittest
 
 import josepy as jose
 
-from acme import test_util
-
+import test_util
 
 KEY = jose.JWKRSA.load(test_util.load_vector('rsa512_key.pem'))
 

acme/tests/magic_typing_test.py

@@ -2,7 +2,10 @@
 import sys
 import unittest
 
-import mock
+try:
+    import mock
+except ImportError: # pragma: no cover
+    from unittest import mock # type: ignore
 
 
 class MagicTypingTest(unittest.TestCase):
@@ -18,7 +21,7 @@ class MagicTypingTest(unittest.TestCase):
         sys.modules['typing'] = typing_class_mock
         if 'acme.magic_typing' in sys.modules:
             del sys.modules['acme.magic_typing'] # pragma: no cover
-        from acme.magic_typing import Text # pylint: disable=no-name-in-module
+        from acme.magic_typing import Text
         self.assertEqual(Text, text_mock)
         del sys.modules['acme.magic_typing']
         sys.modules['typing'] = temp_typing
@@ -31,7 +34,7 @@ class MagicTypingTest(unittest.TestCase):
         sys.modules['typing'] = None
         if 'acme.magic_typing' in sys.modules:
             del sys.modules['acme.magic_typing'] # pragma: no cover
-        from acme.magic_typing import Text # pylint: disable=no-name-in-module
+        from acme.magic_typing import Text
         self.assertTrue(Text is None)
         del sys.modules['acme.magic_typing']
         sys.modules['typing'] = temp_typing

acme/tests/messages_test.py

@@ -2,12 +2,13 @@
 import unittest
 
 import josepy as jose
-import mock
+try:
+    import mock
+except ImportError: # pragma: no cover
+    from unittest import mock # type: ignore
 
 from acme import challenges
-from acme import test_util
-from acme.magic_typing import Dict # pylint: disable=unused-import, no-name-in-module
-
+import test_util
 
 CERT = test_util.load_comparable_cert('cert.der')
 CSR = test_util.load_comparable_csr('csr.der')
@@ -19,8 +20,7 @@ class ErrorTest(unittest.TestCase):
 
     def setUp(self):
         from acme.messages import Error, ERROR_PREFIX
-        self.error = Error(
-            detail='foo', typ=ERROR_PREFIX + 'malformed', title='title')
+        self.error = Error.with_code('malformed', detail='foo', title='title')
         self.jobj = {
             'detail': 'foo',
             'title': 'some title',
@@ -28,7 +28,6 @@ class ErrorTest(unittest.TestCase):
         }
         self.error_custom = Error(typ='custom', detail='bar')
         self.empty_error = Error()
-        self.jobj_custom = {'type': 'custom', 'detail': 'bar'}
 
     def test_default_typ(self):
         from acme.messages import Error
@@ -43,8 +42,7 @@ class ErrorTest(unittest.TestCase):
         hash(Error.from_json(self.error.to_json()))
 
     def test_description(self):
-        self.assertEqual(
-            'The request message was malformed', self.error.description)
+        self.assertEqual('The request message was malformed', self.error.description)
         self.assertTrue(self.error_custom.description is None)
 
     def test_code(self):
@@ -54,17 +52,17 @@ class ErrorTest(unittest.TestCase):
         self.assertEqual(None, Error().code)
 
     def test_is_acme_error(self):
-        from acme.messages import is_acme_error
+        from acme.messages import is_acme_error, Error
         self.assertTrue(is_acme_error(self.error))
         self.assertFalse(is_acme_error(self.error_custom))
+        self.assertFalse(is_acme_error(Error()))
         self.assertFalse(is_acme_error(self.empty_error))
         self.assertFalse(is_acme_error("must pet all the {dogs|rabbits}"))
 
     def test_unicode_error(self):
-        from acme.messages import Error, ERROR_PREFIX, is_acme_error
-        arabic_error = Error(
-                detail=u'\u0639\u062f\u0627\u0644\u0629', typ=ERROR_PREFIX + 'malformed',
-            title='title')
+        from acme.messages import Error, is_acme_error
+        arabic_error = Error.with_code(
+            'malformed', detail=u'\u0639\u062f\u0627\u0644\u0629', title='title')
         self.assertTrue(is_acme_error(arabic_error))
 
     def test_with_code(self):
@@ -305,8 +303,7 @@ class ChallengeBodyTest(unittest.TestCase):
         from acme.messages import Error
         from acme.messages import STATUS_INVALID
         self.status = STATUS_INVALID
-        error = Error(typ='urn:ietf:params:acme:error:serverInternal',
-                      detail='Unable to communicate with DNS server')
+        error = Error.with_code('serverInternal', detail='Unable to communicate with DNS server')
         self.challb = ChallengeBody(
             uri='http://challb', chall=self.chall, status=self.status,
             error=error)
@@ -458,6 +455,7 @@ class OrderResourceTest(unittest.TestCase):
             'authorizations': None,
         })
 
+
 class NewOrderTest(unittest.TestCase):
     """Tests for acme.messages.NewOrder."""
 
@@ -472,5 +470,18 @@ class NewOrderTest(unittest.TestCase):
         })
 
 
+class JWSPayloadRFC8555Compliant(unittest.TestCase):
+    """Test for RFC8555 compliance of JWS generated from resources/challenges"""
+    def test_message_payload(self):
+        from acme.messages import NewAuthorization
+
+        new_order = NewAuthorization()
+        new_order.le_acme_version = 2
+
+        jobj = new_order.json_dumps(indent=2).encode()
+        # RFC8555 states that JWS bodies must not have a resource field.
+        self.assertEqual(jobj, b'{}')
+
+
 if __name__ == '__main__':
     unittest.main()  # pragma: no cover

acme/tests/standalone_test.py

@@ -1,26 +1,22 @@
 """Tests for acme.standalone."""
-import multiprocessing
-import os
-import shutil
 import socket
 import threading
-import tempfile
 import unittest
-import time
-from contextlib import closing
-
-from six.moves import http_client  # pylint: disable=import-error
-from six.moves import socketserver  # type: ignore  # pylint: disable=import-error
 
 import josepy as jose
-import mock
+try:
+    import mock
+except ImportError: # pragma: no cover
+    from unittest import mock # type: ignore
 import requests
+from six.moves import http_client  # pylint: disable=import-error
+from six.moves import socketserver  # type: ignore  # pylint: disable=import-error
 
 from acme import challenges
 from acme import crypto_util
 from acme import errors
-from acme import test_util
-from acme.magic_typing import Set # pylint: disable=unused-import, no-name-in-module
+
+import test_util
 
 
 class TLSServerTest(unittest.TestCase):
@@ -31,41 +27,14 @@ class TLSServerTest(unittest.TestCase):
         from acme.standalone import TLSServer
         server = TLSServer(
             ('', 0), socketserver.BaseRequestHandler, bind_and_activate=True)
-        server.server_close()  # pylint: disable=no-member
+        server.server_close()
 
     def test_ipv6(self):
         if socket.has_ipv6:
             from acme.standalone import TLSServer
             server = TLSServer(
                 ('', 0), socketserver.BaseRequestHandler, bind_and_activate=True, ipv6=True)
-            server.server_close()  # pylint: disable=no-member
-
-
-class TLSSNI01ServerTest(unittest.TestCase):
-    """Test for acme.standalone.TLSSNI01Server."""
-
-
-    def setUp(self):
-        self.certs = {b'localhost': (
-            test_util.load_pyopenssl_private_key('rsa2048_key.pem'),
-            test_util.load_cert('rsa2048_cert.pem'),
-        )}
-        from acme.standalone import TLSSNI01Server
-        self.server = TLSSNI01Server(('localhost', 0), certs=self.certs)
-        # pylint: disable=no-member
-        self.thread = threading.Thread(target=self.server.serve_forever)
-        self.thread.start()
-
-    def tearDown(self):
-        self.server.shutdown()  # pylint: disable=no-member
-        self.thread.join()
-
-    def test_it(self):
-        host, port = self.server.socket.getsockname()[:2]
-        cert = crypto_util.probe_sni(
-            b'localhost', host=host, port=port, timeout=1)
-        self.assertEqual(jose.ComparableX509(cert),
-                         jose.ComparableX509(self.certs[b'localhost'][1]))
+            server.server_close()
 
 
 class HTTP01ServerTest(unittest.TestCase):
@@ -80,13 +49,12 @@ class HTTP01ServerTest(unittest.TestCase):
         from acme.standalone import HTTP01Server
         self.server = HTTP01Server(('', 0), resources=self.resources)
 
-        # pylint: disable=no-member
         self.port = self.server.socket.getsockname()[1]
         self.thread = threading.Thread(target=self.server.serve_forever)
         self.thread.start()
 
     def tearDown(self):
-        self.server.shutdown()  # pylint: disable=no-member
+        self.server.shutdown()
         self.thread.join()
 
     def test_index(self):
@@ -120,6 +88,81 @@ class HTTP01ServerTest(unittest.TestCase):
     def test_http01_not_found(self):
         self.assertFalse(self._test_http01(add=False))
 
+    def test_timely_shutdown(self):
+        from acme.standalone import HTTP01Server
+        server = HTTP01Server(('', 0), resources=set(), timeout=0.05)
+        server_thread = threading.Thread(target=server.serve_forever)
+        server_thread.start()
+
+        client = socket.socket()
+        client.connect(('localhost', server.socket.getsockname()[1]))
+
+        stop_thread = threading.Thread(target=server.shutdown)
+        stop_thread.start()
+        server_thread.join(5.)
+
+        is_hung = server_thread.is_alive()
+        try:
+            client.shutdown(socket.SHUT_RDWR)
+        except: # pragma: no cover, pylint: disable=bare-except
+            # may raise error because socket could already be closed
+            pass
+
+        self.assertFalse(is_hung, msg='Server shutdown should not be hung')
+
+
+@unittest.skipIf(not challenges.TLSALPN01.is_supported(), "pyOpenSSL too old")
+class TLSALPN01ServerTest(unittest.TestCase):
+    """Test for acme.standalone.TLSALPN01Server."""
+
+    def setUp(self):
+        self.certs = {b'localhost': (
+            test_util.load_pyopenssl_private_key('rsa2048_key.pem'),
+            test_util.load_cert('rsa2048_cert.pem'),
+        )}
+        # Use different certificate for challenge.
+        self.challenge_certs = {b'localhost': (
+            test_util.load_pyopenssl_private_key('rsa1024_key.pem'),
+            test_util.load_cert('rsa1024_cert.pem'),
+        )}
+        from acme.standalone import TLSALPN01Server
+        self.server = TLSALPN01Server(("localhost", 0), certs=self.certs,
+                challenge_certs=self.challenge_certs)
+        # pylint: disable=no-member
+        self.thread = threading.Thread(target=self.server.serve_forever)
+        self.thread.start()
+
+    def tearDown(self):
+        self.server.shutdown()  # pylint: disable=no-member
+        self.thread.join()
+
+    # TODO: This is not implemented yet, see comments in standalone.py
+    # def test_certs(self):
+    #    host, port = self.server.socket.getsockname()[:2]
+    #    cert = crypto_util.probe_sni(
+    #        b'localhost', host=host, port=port, timeout=1)
+    #    # Expect normal cert when connecting without ALPN.
+    #    self.assertEqual(jose.ComparableX509(cert),
+    #                     jose.ComparableX509(self.certs[b'localhost'][1]))
+
+    def test_challenge_certs(self):
+        host, port = self.server.socket.getsockname()[:2]
+        cert = crypto_util.probe_sni(
+            b'localhost', host=host, port=port, timeout=1,
+            alpn_protocols=[b"acme-tls/1"])
+        #  Expect challenge cert when connecting with ALPN.
+        self.assertEqual(
+                jose.ComparableX509(cert),
+                jose.ComparableX509(self.challenge_certs[b'localhost'][1])
+        )
+
+    def test_bad_alpn(self):
+        host, port = self.server.socket.getsockname()[:2]
+        with self.assertRaises(errors.Error):
+            crypto_util.probe_sni(
+                b'localhost', host=host, port=port, timeout=1,
+                alpn_protocols=[b"bad-alpn"])
+
 
 class BaseDualNetworkedServersTest(unittest.TestCase):
     """Test for acme.standalone.BaseDualNetworkedServers."""
@@ -139,7 +182,6 @@ class BaseDualNetworkedServersTest(unittest.TestCase):
                 # NB: On Windows, socket.IPPROTO_IPV6 constant may be missing.
                 # We use the corresponding value (41) instead.
                 level = getattr(socket, "IPPROTO_IPV6", 41)
-                # pylint: disable=no-member
                 self.socket.setsockopt(level, socket.IPV6_V6ONLY, 1)
                 try:
                     self.server_bind()
@@ -173,37 +215,9 @@ class BaseDualNetworkedServersTest(unittest.TestCase):
             prev_port = port
 
 
-class TLSSNI01DualNetworkedServersTest(unittest.TestCase):
-    """Test for acme.standalone.TLSSNI01DualNetworkedServers."""
-
-
-    def setUp(self):
-        self.certs = {b'localhost': (
-            test_util.load_pyopenssl_private_key('rsa2048_key.pem'),
-            test_util.load_cert('rsa2048_cert.pem'),
-        )}
-        from acme.standalone import TLSSNI01DualNetworkedServers
-        self.servers = TLSSNI01DualNetworkedServers(('localhost', 0), certs=self.certs)
-        self.servers.serve_forever()
-
-    def tearDown(self):
-        self.servers.shutdown_and_server_close()
-
-    def test_connect(self):
-        socknames = self.servers.getsocknames()
-        # connect to all addresses
-        for sockname in socknames:
-            host, port = sockname[:2]
-            cert = crypto_util.probe_sni(
-                b'localhost', host=host, port=port, timeout=1)
-            self.assertEqual(jose.ComparableX509(cert),
-                             jose.ComparableX509(self.certs[b'localhost'][1]))
-
-
 class HTTP01DualNetworkedServersTest(unittest.TestCase):
     """Tests for acme.standalone.HTTP01DualNetworkedServers."""
 
-
     def setUp(self):
         self.account_key = jose.JWK.load(
             test_util.load_vector('rsa1024_key.pem'))
@@ -212,7 +226,6 @@ class HTTP01DualNetworkedServersTest(unittest.TestCase):
         from acme.standalone import HTTP01DualNetworkedServers
         self.servers = HTTP01DualNetworkedServers(('', 0), resources=self.resources)
 
-        # pylint: disable=no-member
         self.port = self.servers.getsocknames()[0][1]
         self.servers.serve_forever()
 
@@ -251,60 +264,5 @@ class HTTP01DualNetworkedServersTest(unittest.TestCase):
         self.assertFalse(self._test_http01(add=False))
 
 
-class TestSimpleTLSSNI01Server(unittest.TestCase):
-    """Tests for acme.standalone.simple_tls_sni_01_server."""
-
-
-    def setUp(self):
-        # mirror ../examples/standalone
-        self.test_cwd = tempfile.mkdtemp()
-        localhost_dir = os.path.join(self.test_cwd, 'localhost')
-        os.makedirs(localhost_dir)
-        shutil.copy(test_util.vector_path('rsa2048_cert.pem'),
-                    os.path.join(localhost_dir, 'cert.pem'))
-        shutil.copy(test_util.vector_path('rsa2048_key.pem'),
-                    os.path.join(localhost_dir, 'key.pem'))
-
-        with closing(socket.socket(socket.AF_INET, socket.SOCK_STREAM)) as sock:
-            sock.bind(('', 0))
-            sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
-            self.port = sock.getsockname()[1]
-
-        from acme.standalone import simple_tls_sni_01_server
-        self.process = multiprocessing.Process(target=simple_tls_sni_01_server,
-                                               args=(['path', '-p', str(self.port)],))
-        self.old_cwd = os.getcwd()
-        os.chdir(self.test_cwd)
-
-    def tearDown(self):
-        os.chdir(self.old_cwd)
-        if self.process.is_alive():
-            self.process.terminate()
-            self.process.join(timeout=5)
-            # Check that we didn't timeout waiting for the process to
-            # terminate.
-            self.assertNotEqual(self.process.exitcode, None)
-        shutil.rmtree(self.test_cwd)
-
-    @mock.patch('acme.standalone.TLSSNI01Server.handle_request')
-    def test_mock(self, handle):
-        from acme.standalone import simple_tls_sni_01_server
-        simple_tls_sni_01_server(cli_args=['path', '-p', str(self.port)], forever=False)
-        self.assertEqual(handle.call_count, 1)
-
-    def test_live(self):
-        self.process.start()
-        cert = None
-        for _ in range(50):
-            time.sleep(0.1)
-            try:
-                cert = crypto_util.probe_sni(b'localhost', b'127.0.0.1', self.port)
-                break
-            except errors.Error:  # pragma: no cover
-                pass
-        self.assertEqual(jose.ComparableX509(cert),
-                         test_util.load_comparable_cert('rsa2048_cert.pem'))
-
-
 if __name__ == "__main__":
     unittest.main()  # pragma: no cover

acme/tests/test_util.py

@@ -4,19 +4,12 @@
 
 """
 import os
-import pkg_resources
-import unittest
 
 from cryptography.hazmat.backends import default_backend
 from cryptography.hazmat.primitives import serialization
 import josepy as jose
 from OpenSSL import crypto
-
-
-def vector_path(*names):
-    """Path to a test vector."""
-    return pkg_resources.resource_filename(
-        __name__, os.path.join('testdata', *names))
+import pkg_resources
 
 
 def load_vector(*names):
@@ -32,8 +25,7 @@ def _guess_loader(filename, loader_pem, loader_der):
         return loader_pem
     elif ext.lower() == '.der':
         return loader_der
-    else:  # pragma: no cover
-        raise ValueError("Loader could not be recognized based on extension")
+    raise ValueError("Loader could not be recognized based on extension")  # pragma: no cover
 
 
 def load_cert(*names):
@@ -73,24 +65,3 @@ def load_pyopenssl_private_key(*names):
     loader = _guess_loader(
         names[-1], crypto.FILETYPE_PEM, crypto.FILETYPE_ASN1)
     return crypto.load_privatekey(loader, load_vector(*names))
-
-
-def skip_unless(condition, reason):  # pragma: no cover
-    """Skip tests unless a condition holds.
-
-    This implements the basic functionality of unittest.skipUnless
-    which is only available on Python 2.7+.
-
-    :param bool condition: If ``False``, the test will be skipped
-    :param str reason: the reason for skipping the test
-
-    :rtype: callable
-    :returns: decorator that hides tests unless condition is ``True``
-
-    """
-    if hasattr(unittest, "skipUnless"):
-        return unittest.skipUnless(condition, reason)
-    elif condition:
-        return lambda cls: cls
-    else:
-        return lambda cls: None

acme/tests/testdata/README

@@ -10,6 +10,8 @@ and for the CSR:
 
   openssl req -key rsa2048_key.pem -new -subj '/CN=example.com' -outform DER > csr.der
 
-and for the certificate:
+and for the certificates:
 
-  openssl req -key rsa2047_key.pem -new -subj '/CN=example.com' -x509 -outform DER > cert.der
+  openssl req -key rsa2048_key.pem -new -subj '/CN=example.com' -x509 -outform DER > cert.der
+  openssl req -key rsa2048_key.pem -new -subj '/CN=example.com' -x509 > rsa2048_cert.pem
+  openssl req -key rsa1024_key.pem -new -subj '/CN=example.com' -x509 > rsa1024_cert.pem

acme/tests/testdata/rsa1024_cert.pem

@@ -0,0 +1,13 @@
+-----BEGIN CERTIFICATE-----
+MIIB/TCCAWagAwIBAgIJAOyRIBs3QT8QMA0GCSqGSIb3DQEBCwUAMBYxFDASBgNV
+BAMMC2V4YW1wbGUuY29tMB4XDTE4MDQyMzEwMzE0NFoXDTE4MDUyMzEwMzE0NFow
+FjEUMBIGA1UEAwwLZXhhbXBsZS5jb20wgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJ
+AoGBAJqJ87R8aVwByONxgQA9hwgvQd/QqI1r1UInXhEF2VnEtZGtUWLi100IpIqr
+Mq4qusDwNZ3g8cUPtSkvJGs89djoajMDIJP7lQUEKUYnYrI0q755Tr/DgLWSk7iW
+l5ezym0VzWUD0/xXUz8yRbNMTjTac80rS5SZk2ja2wWkYlRJAgMBAAGjUzBRMB0G
+A1UdDgQWBBSsaX0IVZ4XXwdeffVAbG7gnxSYjTAfBgNVHSMEGDAWgBSsaX0IVZ4X
+XwdeffVAbG7gnxSYjTAPBgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3DQEBCwUAA4GB
+ADe7SVmvGH2nkwVfONk8TauRUDkePN1CJZKFb2zW1uO9ANJ2v5Arm/OQp0BG/xnI
+Djw/aLTNVESF89oe15dkrUErtcaF413MC1Ld5lTCaJLHLGqDKY69e02YwRuxW7jY
+qarpt7k7aR5FbcfO5r4V/FK/Gvp4Dmoky8uap7SJIW6x
+-----END CERTIFICATE-----

appveyor.yml

@@ -1,40 +0,0 @@
-image: Visual Studio 2015
-
-environment:
-  matrix:
-    - TOXENV: py35
-    - TOXENV: py37-cover
-
-branches:
-  only:
-    - master
-    - /^\d+\.\d+\.x$/ # Version branches like X.X.X
-    - /^test-.*$/
-
-init:
-  # Since master can receive only commits from PR that have already been tested, following
-  # condition avoid to launch all jobs except the coverage one for commits pushed to master.
-  - ps: |
-      if (-Not $Env:APPVEYOR_PULL_REQUEST_NUMBER -And $Env:APPVEYOR_REPO_BRANCH -Eq 'master' `
-          -And -Not ($Env:TOXENV -Like '*-cover'))
-      { $Env:APPVEYOR_SKIP_FINALIZE_ON_EXIT = 'true'; Exit-AppVeyorBuild }
-
-install:
-  # Use Python 3.7 by default
-  - "SET PATH=C:\\Python37;C:\\Python37\\Scripts;%PATH%"
-  # Check env
-  - "python --version"
-  # Upgrade pip to avoid warnings
-  - "python -m pip install --upgrade pip"
-  # Ready to install tox and coverage
-  - "pip install tox codecov"
-
-build: off
-
-test_script:
-  - set TOX_TESTENV_PASSENV=APPVEYOR
-  # Test env is set by TOXENV env variable
-  - tox
-
-on_success:
-  - if exist .coverage codecov

certbot-apache/MANIFEST.in

@@ -1,7 +1,7 @@
 include LICENSE.txt
 include README.rst
-recursive-include docs *
-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 tests *
+recursive-include certbot_apache/_internal/augeas_lens *.aug
+recursive-include certbot_apache/_internal/tls_configs *.conf
+global-exclude __pycache__
+global-exclude *.py[cod]

certbot-apache/certbot_apache/_internal/__init__.py

@@ -0,0 +1 @@
+"""Certbot Apache plugin."""

certbot-apache/certbot_apache/_internal/apache_util.py

@@ -0,0 +1,256 @@
+""" Utility functions for certbot-apache plugin """
+import binascii
+import fnmatch
+import logging
+import re
+import subprocess
+
+import pkg_resources
+
+from certbot import errors
+from certbot import util
+
+from certbot.compat import os
+
+logger = logging.getLogger(__name__)
+
+
+def get_mod_deps(mod_name):
+    """Get known module dependencies.
+
+    .. note:: This does not need to be accurate in order for the client to
+        run.  This simply keeps things clean if the user decides to revert
+        changes.
+    .. warning:: If all deps are not included, it may cause incorrect parsing
+        behavior, due to enable_mod's shortcut for updating the parser's
+        currently defined modules (`.ApacheParser.add_mod`)
+        This would only present a major problem in extremely atypical
+        configs that use ifmod for the missing deps.
+
+    """
+    deps = {
+        "ssl": ["setenvif", "mime"]
+    }
+    return deps.get(mod_name, [])
+
+
+def get_file_path(vhost_path):
+    """Get file path from augeas_vhost_path.
+
+    Takes in Augeas path and returns the file name
+
+    :param str vhost_path: Augeas virtual host path
+
+    :returns: filename of vhost
+    :rtype: str
+
+    """
+    if not vhost_path or not vhost_path.startswith("/files/"):
+        return None
+
+    return _split_aug_path(vhost_path)[0]
+
+
+def get_internal_aug_path(vhost_path):
+    """Get the Augeas path for a vhost with the file path removed.
+
+    :param str vhost_path: Augeas virtual host path
+
+    :returns: Augeas path to vhost relative to the containing file
+    :rtype: str
+
+    """
+    return _split_aug_path(vhost_path)[1]
+
+
+def _split_aug_path(vhost_path):
+    """Splits an Augeas path into a file path and an internal path.
+
+    After removing "/files", this function splits vhost_path into the
+    file path and the remaining Augeas path.
+
+    :param str vhost_path: Augeas virtual host path
+
+    :returns: file path and internal Augeas path
+    :rtype: `tuple` of `str`
+
+    """
+    # Strip off /files
+    file_path = vhost_path[6:]
+    internal_path = []
+
+    # Remove components from the end of file_path until it becomes valid
+    while not os.path.exists(file_path):
+        file_path, _, internal_path_part = file_path.rpartition("/")
+        internal_path.append(internal_path_part)
+
+    return file_path, "/".join(reversed(internal_path))
+
+
+def parse_define_file(filepath, varname):
+    """ Parses Defines from a variable in configuration file
+
+    :param str filepath: Path of file to parse
+    :param str varname: Name of the variable
+
+    :returns: Dict of Define:Value pairs
+    :rtype: `dict`
+
+    """
+    return_vars = {}
+    # Get list of words in the variable
+    a_opts = util.get_var_from_file(varname, filepath).split()
+    for i, v in enumerate(a_opts):
+        # Handle Define statements and make sure it has an argument
+        if v == "-D" and len(a_opts) >= i+2:
+            var_parts = a_opts[i+1].partition("=")
+            return_vars[var_parts[0]] = var_parts[2]
+        elif len(v) > 2 and v.startswith("-D"):
+            # Found var with no whitespace separator
+            var_parts = v[2:].partition("=")
+            return_vars[var_parts[0]] = var_parts[2]
+    return return_vars
+
+
+def unique_id():
+    """ Returns an unique id to be used as a VirtualHost identifier"""
+    return binascii.hexlify(os.urandom(16)).decode("utf-8")
+
+
+def included_in_paths(filepath, paths):
+    """
+    Returns true if the filepath is included in the list of paths
+    that may contain full paths or wildcard paths that need to be
+    expanded.
+
+    :param str filepath: Filepath to check
+    :params list paths: List of paths to check against
+
+    :returns: True if included
+    :rtype: bool
+    """
+
+    return any(fnmatch.fnmatch(filepath, path) for path in paths)
+
+
+def parse_defines(apachectl):
+    """
+    Gets Defines from httpd process and returns a dictionary of
+    the defined variables.
+
+    :param str apachectl: Path to apachectl executable
+
+    :returns: dictionary of defined variables
+    :rtype: dict
+    """
+
+    variables = {}
+    define_cmd = [apachectl, "-t", "-D",
+                  "DUMP_RUN_CFG"]
+    matches = parse_from_subprocess(define_cmd, r"Define: ([^ \n]*)")
+    try:
+        matches.remove("DUMP_RUN_CFG")
+    except ValueError:
+        return {}
+
+    for match in matches:
+        if match.count("=") > 1:
+            logger.error("Unexpected number of equal signs in "
+                         "runtime config dump.")
+            raise errors.PluginError(
+                "Error parsing Apache runtime variables")
+        parts = match.partition("=")
+        variables[parts[0]] = parts[2]
+
+    return variables
+
+
+def parse_includes(apachectl):
+    """
+    Gets Include directives from httpd process and returns a list of
+    their values.
+
+    :param str apachectl: Path to apachectl executable
+
+    :returns: list of found Include directive values
+    :rtype: list of str
+    """
+
+    inc_cmd = [apachectl, "-t", "-D",
+               "DUMP_INCLUDES"]
+    return parse_from_subprocess(inc_cmd, r"\(.*\) (.*)")
+
+
+def parse_modules(apachectl):
+    """
+    Get loaded modules from httpd process, and return the list
+    of loaded module names.
+
+    :param str apachectl: Path to apachectl executable
+
+    :returns: list of found LoadModule module names
+    :rtype: list of str
+    """
+
+    mod_cmd = [apachectl, "-t", "-D",
+               "DUMP_MODULES"]
+    return parse_from_subprocess(mod_cmd, r"(.*)_module")
+
+
+def parse_from_subprocess(command, regexp):
+    """Get values from stdout of subprocess command
+
+    :param list command: Command to run
+    :param str regexp: Regexp for parsing
+
+    :returns: list parsed from command output
+    :rtype: list
+
+    """
+    stdout = _get_runtime_cfg(command)
+    return re.compile(regexp).findall(stdout)
+
+
+def _get_runtime_cfg(command):
+    """
+    Get runtime configuration info.
+
+    :param command: Command to run
+
+    :returns: stdout from command
+
+    """
+    try:
+        proc = subprocess.Popen(
+            command,
+            stdout=subprocess.PIPE,
+            stderr=subprocess.PIPE,
+            universal_newlines=True)
+        stdout, stderr = proc.communicate()
+
+    except (OSError, ValueError):
+        logger.error(
+            "Error running command %s for runtime parameters!%s",
+            command, os.linesep)
+        raise errors.MisconfigurationError(
+            "Error accessing loaded Apache parameters: {0}".format(
+                command))
+    # Small errors that do not impede
+    if proc.returncode != 0:
+        logger.warning("Error in checking parameter list: %s", stderr)
+        raise errors.MisconfigurationError(
+            "Apache is unable to check whether or not the module is "
+            "loaded because Apache is misconfigured.")
+
+    return stdout
+
+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("_internal", "tls_configs", "{0}-options-ssl-apache.conf".format(prefix)))

certbot-apache/certbot_apache/_internal/apacheparser.py

@@ -0,0 +1,172 @@
+""" apacheconfig implementation of the ParserNode interfaces """
+
+from certbot_apache._internal import assertions
+from certbot_apache._internal import interfaces
+from certbot_apache._internal import parsernode_util as util
+
+
+class ApacheParserNode(interfaces.ParserNode):
+    """ apacheconfig implementation of ParserNode interface.
+
+        Expects metadata `ac_ast` to be passed in, where `ac_ast` is the AST provided
+        by parsing the equivalent configuration text using the apacheconfig library.
+    """
+
+    def __init__(self, **kwargs):
+        ancestor, dirty, filepath, metadata = util.parsernode_kwargs(kwargs)  # pylint: disable=unused-variable
+        super(ApacheParserNode, self).__init__(**kwargs)
+        self.ancestor = ancestor
+        self.filepath = filepath
+        self.dirty = dirty
+        self.metadata = metadata
+        self._raw = self.metadata["ac_ast"]
+
+    def save(self, msg): # pragma: no cover
+        pass
+
+    def find_ancestors(self, name):  # pylint: disable=unused-variable
+        """Find ancestor BlockNodes with a given name"""
+        return [ApacheBlockNode(name=assertions.PASS,
+                                parameters=assertions.PASS,
+                                ancestor=self,
+                                filepath=assertions.PASS,
+                                metadata=self.metadata)]
+
+
+class ApacheCommentNode(ApacheParserNode):
+    """ apacheconfig implementation of CommentNode interface """
+
+    def __init__(self, **kwargs):
+        comment, kwargs = util.commentnode_kwargs(kwargs)  # pylint: disable=unused-variable
+        super(ApacheCommentNode, self).__init__(**kwargs)
+        self.comment = comment
+
+    def __eq__(self, other):  # pragma: no cover
+        if isinstance(other, self.__class__):
+            return (self.comment == other.comment and
+                    self.dirty == other.dirty and
+                    self.ancestor == other.ancestor and
+                    self.metadata == other.metadata and
+                    self.filepath == other.filepath)
+        return False
+
+
+class ApacheDirectiveNode(ApacheParserNode):
+    """ apacheconfig implementation of DirectiveNode interface """
+
+    def __init__(self, **kwargs):
+        name, parameters, enabled, kwargs = util.directivenode_kwargs(kwargs)
+        super(ApacheDirectiveNode, self).__init__(**kwargs)
+        self.name = name
+        self.parameters = parameters
+        self.enabled = enabled
+        self.include = None
+
+    def __eq__(self, other):  # pragma: no cover
+        if isinstance(other, self.__class__):
+            return (self.name == other.name and
+                    self.filepath == other.filepath and
+                    self.parameters == other.parameters and
+                    self.enabled == other.enabled and
+                    self.dirty == other.dirty and
+                    self.ancestor == other.ancestor and
+                    self.metadata == other.metadata)
+        return False
+
+    def set_parameters(self, _parameters):  # pragma: no cover
+        """Sets the parameters for DirectiveNode"""
+        return
+
+
+class ApacheBlockNode(ApacheDirectiveNode):
+    """ apacheconfig implementation of BlockNode interface """
+
+    def __init__(self, **kwargs):
+        super(ApacheBlockNode, self).__init__(**kwargs)
+        self.children = ()
+
+    def __eq__(self, other):  # pragma: no cover
+        if isinstance(other, self.__class__):
+            return (self.name == other.name and
+                    self.filepath == other.filepath and
+                    self.parameters == other.parameters and
+                    self.children == other.children and
+                    self.enabled == other.enabled and
+                    self.dirty == other.dirty and
+                    self.ancestor == other.ancestor and
+                    self.metadata == other.metadata)
+        return False
+
+    # pylint: disable=unused-argument
+    def add_child_block(self, name, parameters=None, position=None):  # pragma: no cover
+        """Adds a new BlockNode to the sequence of children"""
+        new_block = ApacheBlockNode(name=assertions.PASS,
+                                    parameters=assertions.PASS,
+                                    ancestor=self,
+                                    filepath=assertions.PASS,
+                                    metadata=self.metadata)
+        self.children += (new_block,)
+        return new_block
+
+    # pylint: disable=unused-argument
+    def add_child_directive(self, name, parameters=None, position=None):  # pragma: no cover
+        """Adds a new DirectiveNode to the sequence of children"""
+        new_dir = ApacheDirectiveNode(name=assertions.PASS,
+                                      parameters=assertions.PASS,
+                                      ancestor=self,
+                                      filepath=assertions.PASS,
+                                      metadata=self.metadata)
+        self.children += (new_dir,)
+        return new_dir
+
+    # pylint: disable=unused-argument
+    def add_child_comment(self, comment="", position=None):  # pragma: no cover
+
+        """Adds a new CommentNode to the sequence of children"""
+        new_comment = ApacheCommentNode(comment=assertions.PASS,
+                                        ancestor=self,
+                                        filepath=assertions.PASS,
+                                        metadata=self.metadata)
+        self.children += (new_comment,)
+        return new_comment
+
+    def find_blocks(self, name, exclude=True): # pylint: disable=unused-argument
+        """Recursive search of BlockNodes from the sequence of children"""
+        return [ApacheBlockNode(name=assertions.PASS,
+                                parameters=assertions.PASS,
+                                ancestor=self,
+                                filepath=assertions.PASS,
+                                metadata=self.metadata)]
+
+    def find_directives(self, name, exclude=True): # pylint: disable=unused-argument
+        """Recursive search of DirectiveNodes from the sequence of children"""
+        return [ApacheDirectiveNode(name=assertions.PASS,
+                                    parameters=assertions.PASS,
+                                    ancestor=self,
+                                    filepath=assertions.PASS,
+                                    metadata=self.metadata)]
+
+    # pylint: disable=unused-argument
+    def find_comments(self, comment, exact=False):  # pragma: no cover
+        """Recursive search of DirectiveNodes from the sequence of children"""
+        return [ApacheCommentNode(comment=assertions.PASS,
+                                  ancestor=self,
+                                  filepath=assertions.PASS,
+                                  metadata=self.metadata)]
+
+    def delete_child(self, child):  # pragma: no cover
+        """Deletes a ParserNode from the sequence of children"""
+        return
+
+    def unsaved_files(self):  # pragma: no cover
+        """Returns a list of unsaved filepaths"""
+        return [assertions.PASS]
+
+    def parsed_paths(self):  # pragma: no cover
+        """Returns a list of parsed configuration file paths"""
+        return [assertions.PASS]
+
+
+interfaces.CommentNode.register(ApacheCommentNode)
+interfaces.DirectiveNode.register(ApacheDirectiveNode)
+interfaces.BlockNode.register(ApacheBlockNode)

certbot-apache/certbot_apache/_internal/assertions.py

@@ -0,0 +1,142 @@
+"""Dual parser node assertions"""
+import fnmatch
+
+from certbot_apache._internal import interfaces
+
+
+PASS = "CERTBOT_PASS_ASSERT"
+
+
+def assertEqual(first, second):
+    """ Equality assertion """
+
+    if isinstance(first, interfaces.CommentNode):
+        assertEqualComment(first, second)
+    elif isinstance(first, interfaces.DirectiveNode):
+        assertEqualDirective(first, second)
+
+    # Do an extra interface implementation assertion, as the contents were
+    # already checked for BlockNode in the assertEqualDirective
+    if isinstance(first, interfaces.BlockNode):
+        assert isinstance(second, interfaces.BlockNode)
+
+    # Skip tests if filepath includes the pass value. This is done
+    # because filepath is variable of the base ParserNode interface, and
+    # unless the implementation is actually done, we cannot assume getting
+    # correct results from boolean assertion for dirty
+    if not isPass(first.filepath) and not isPass(second.filepath):
+        assert first.dirty == second.dirty
+        # We might want to disable this later if testing with two separate
+        # (but identical) directory structures.
+        assert first.filepath == second.filepath
+
+def assertEqualComment(first, second): # pragma: no cover
+    """ Equality assertion for CommentNode """
+
+    assert isinstance(first, interfaces.CommentNode)
+    assert isinstance(second, interfaces.CommentNode)
+
+    if not isPass(first.comment) and not isPass(second.comment):  # type: ignore
+        assert first.comment == second.comment  # type: ignore
+
+def _assertEqualDirectiveComponents(first, second): # pragma: no cover
+    """ Handles assertion for instance variables for DirectiveNode and BlockNode"""
+
+    # Enabled value cannot be asserted, because Augeas implementation
+    # is unable to figure that out.
+    # assert first.enabled == second.enabled
+    if not isPass(first.name) and not isPass(second.name):
+        assert first.name == second.name
+
+    if not isPass(first.parameters) and not isPass(second.parameters):
+        assert first.parameters == second.parameters
+
+def assertEqualDirective(first, second):
+    """ Equality assertion for DirectiveNode """
+
+    assert isinstance(first, interfaces.DirectiveNode)
+    assert isinstance(second, interfaces.DirectiveNode)
+    _assertEqualDirectiveComponents(first, second)
+
+def isPass(value): # pragma: no cover
+    """Checks if the value is set to PASS"""
+    if isinstance(value, bool):
+        return True
+    return PASS in value
+
+def isPassDirective(block):
+    """ Checks if BlockNode or DirectiveNode should pass the assertion """
+
+    if isPass(block.name):
+        return True
+    if isPass(block.parameters): # pragma: no cover
+        return True
+    if isPass(block.filepath): # pragma: no cover
+        return True
+    return False
+
+def isPassComment(comment):
+    """ Checks if CommentNode should pass the assertion """
+
+    if isPass(comment.comment):
+        return True
+    if isPass(comment.filepath): # pragma: no cover
+        return True
+    return False
+
+def isPassNodeList(nodelist): # pragma: no cover
+    """ Checks if a ParserNode in the nodelist should pass the assertion,
+    this function is used for results of find_* methods. Unimplemented find_*
+    methods should return a sequence containing a single ParserNode instance
+    with assertion pass string."""
+
+    try:
+        node = nodelist[0]
+    except IndexError:
+        node = None
+
+    if not node: # pragma: no cover
+        return False
+
+    if isinstance(node, interfaces.DirectiveNode):
+        return isPassDirective(node)
+    return isPassComment(node)
+
+def assertEqualSimple(first, second):
+    """ Simple assertion """
+    if not isPass(first) and not isPass(second):
+        assert first == second
+
+def isEqualVirtualHost(first, second):
+    """
+    Checks that two VirtualHost objects are similar. There are some built
+    in differences with the implementations: VirtualHost created by ParserNode
+    implementation doesn't have "path" defined, as it was used for Augeas path
+    and that cannot obviously be used in the future. Similarly the legacy
+    version lacks "node" variable, that has a reference to the BlockNode for the
+    VirtualHost.
+    """
+    return (
+        first.name == second.name and
+        first.aliases == second.aliases and
+        first.filep == second.filep and
+        first.addrs == second.addrs and
+        first.ssl == second.ssl and
+        first.enabled == second.enabled and
+        first.modmacro == second.modmacro and
+        first.ancestor == second.ancestor
+    )
+
+def assertEqualPathsList(first, second):  # pragma: no cover
+    """
+    Checks that the two lists of file paths match. This assertion allows for wildcard
+    paths.
+    """
+    if any(isPass(path) for path in first):
+        return
+    if any(isPass(path) for path in second):
+        return
+    for fpath in first:
+        assert any([fnmatch.fnmatch(fpath, spath) for spath in second])
+    for spath in second:
+        assert any([fnmatch.fnmatch(fpath, spath) for fpath in first])

certbot-apache/certbot_apache/_internal/augeasparser.py

@@ -0,0 +1,538 @@
+"""
+Augeas implementation of the ParserNode interfaces.
+
+Augeas works internally by using XPATH notation. The following is a short example
+of how this all works internally, to better understand what's going on under the
+hood.
+
+A configuration file /etc/apache2/apache2.conf with the following content:
+
+    # First comment line
+    # Second comment line
+    WhateverDirective whatevervalue
+    <ABlock>
+        DirectiveInABlock dirvalue
+    </ABlock>
+    SomeDirective somedirectivevalue
+    <ABlock>
+        AnotherDirectiveInABlock dirvalue
+    </ABlock>
+    # Yet another comment
+
+
+Translates over to Augeas path notation (of immediate children), when calling
+for example: aug.match("/files/etc/apache2/apache2.conf/*")
+
+[
+    "/files/etc/apache2/apache2.conf/#comment[1]",
+    "/files/etc/apache2/apache2.conf/#comment[2]",
+    "/files/etc/apache2/apache2.conf/directive[1]",
+    "/files/etc/apache2/apache2.conf/ABlock[1]",
+    "/files/etc/apache2/apache2.conf/directive[2]",
+    "/files/etc/apache2/apache2.conf/ABlock[2]",
+    "/files/etc/apache2/apache2.conf/#comment[3]"
+]
+
+Regardless of directives name, its key in the Augeas tree is always "directive",
+with index where needed of course. Comments work similarly, while blocks
+have their own key in the Augeas XPATH notation.
+
+It's important to note that all of the unique keys have their own indices.
+
+Augeas paths are case sensitive, while Apache configuration is case insensitive.
+It looks like this:
+
+    <block>
+        directive value
+    </block>
+    <Block>
+        Directive Value
+    </Block>
+    <block>
+        directive value
+    </block>
+    <bLoCk>
+        DiReCtiVe VaLuE
+    </bLoCk>
+
+Translates over to:
+
+[
+    "/files/etc/apache2/apache2.conf/block[1]",
+    "/files/etc/apache2/apache2.conf/Block[1]",
+    "/files/etc/apache2/apache2.conf/block[2]",
+    "/files/etc/apache2/apache2.conf/bLoCk[1]",
+]
+"""
+from acme.magic_typing import Set
+from certbot import errors
+from certbot.compat import os
+
+from certbot_apache._internal import apache_util
+from certbot_apache._internal import assertions
+from certbot_apache._internal import interfaces
+from certbot_apache._internal import parser
+from certbot_apache._internal import parsernode_util as util
+
+
+class AugeasParserNode(interfaces.ParserNode):
+    """ Augeas implementation of ParserNode interface """
+
+    def __init__(self, **kwargs):
+        ancestor, dirty, filepath, metadata = util.parsernode_kwargs(kwargs)  # pylint: disable=unused-variable
+        super(AugeasParserNode, self).__init__(**kwargs)
+        self.ancestor = ancestor
+        self.filepath = filepath
+        self.dirty = dirty
+        self.metadata = metadata
+        self.parser = self.metadata.get("augeasparser")
+        try:
+            if self.metadata["augeaspath"].endswith("/"):
+                raise errors.PluginError(
+                    "Augeas path: {} has a trailing slash".format(
+                        self.metadata["augeaspath"]
+                    )
+                )
+        except KeyError:
+            raise errors.PluginError("Augeas path is required")
+
+    def save(self, msg):
+        self.parser.save(msg)
+
+    def find_ancestors(self, name):
+        """
+        Searches for ancestor BlockNodes with a given name.
+
+        :param str name: Name of the BlockNode parent to search for
+
+        :returns: List of matching ancestor nodes.
+        :rtype: list of AugeasBlockNode
+        """
+
+        ancestors = []
+
+        parent = self.metadata["augeaspath"]
+        while True:
+            # Get the path of ancestor node
+            parent = parent.rpartition("/")[0]
+            # Root of the tree
+            if not parent or parent == "/files":
+                break
+            anc = self._create_blocknode(parent)
+            if anc.name.lower() == name.lower():
+                ancestors.append(anc)
+
+        return ancestors
+
+    def _create_blocknode(self, path):
+        """
+        Helper function to create a BlockNode from Augeas path. This is used by
+        AugeasParserNode.find_ancestors and AugeasBlockNode.
+        and AugeasBlockNode.find_blocks
+
+        """
+
+        name = self._aug_get_name(path)
+        metadata = {"augeasparser": self.parser, "augeaspath": path}
+
+        # Check if the file was included from the root config or initial state
+        enabled = self.parser.parsed_in_original(
+            apache_util.get_file_path(path)
+        )
+
+        return AugeasBlockNode(name=name,
+                               enabled=enabled,
+                               ancestor=assertions.PASS,
+                               filepath=apache_util.get_file_path(path),
+                               metadata=metadata)
+
+    def _aug_get_name(self, path):
+        """
+        Helper function to get name of a configuration block or variable from path.
+        """
+
+        # Remove the ending slash if any
+        if path[-1] == "/":  # pragma: no cover
+            path = path[:-1]
+
+        # Get the block name
+        name = path.split("/")[-1]
+
+        # remove [...], it's not allowed in Apache configuration and is used
+        # for indexing within Augeas
+        name = name.split("[")[0]
+        return name
+
+
+class AugeasCommentNode(AugeasParserNode):
+    """ Augeas implementation of CommentNode interface """
+
+    def __init__(self, **kwargs):
+        comment, kwargs = util.commentnode_kwargs(kwargs)  # pylint: disable=unused-variable
+        super(AugeasCommentNode, self).__init__(**kwargs)
+        # self.comment = comment
+        self.comment = comment
+
+    def __eq__(self, other):
+        if isinstance(other, self.__class__):
+            return (self.comment == other.comment and
+                    self.filepath == other.filepath and
+                    self.dirty == other.dirty and
+                    self.ancestor == other.ancestor and
+                    self.metadata == other.metadata)
+        return False
+
+
+class AugeasDirectiveNode(AugeasParserNode):
+    """ Augeas implementation of DirectiveNode interface """
+
+    def __init__(self, **kwargs):
+        name, parameters, enabled, kwargs = util.directivenode_kwargs(kwargs)
+        super(AugeasDirectiveNode, self).__init__(**kwargs)
+        self.name = name
+        self.enabled = enabled
+        if parameters:
+            self.set_parameters(parameters)
+
+    def __eq__(self, other):
+        if isinstance(other, self.__class__):
+            return (self.name == other.name and
+                    self.filepath == other.filepath and
+                    self.parameters == other.parameters and
+                    self.enabled == other.enabled and
+                    self.dirty == other.dirty and
+                    self.ancestor == other.ancestor and
+                    self.metadata == other.metadata)
+        return False
+
+    def set_parameters(self, parameters):
+        """
+        Sets parameters of a DirectiveNode or BlockNode object.
+
+        :param list parameters: List of all parameters for the node to set.
+        """
+        orig_params = self._aug_get_params(self.metadata["augeaspath"])
+
+        # Clear out old parameters
+        for _ in orig_params:
+            # When the first parameter is removed, the indices get updated
+            param_path = "{}/arg[1]".format(self.metadata["augeaspath"])
+            self.parser.aug.remove(param_path)
+        # Insert new ones
+        for pi, param in enumerate(parameters):
+            param_path = "{}/arg[{}]".format(self.metadata["augeaspath"], pi+1)
+            self.parser.aug.set(param_path, param)
+
+    @property
+    def parameters(self):
+        """
+        Fetches the parameters from Augeas tree, ensuring that the sequence always
+        represents the current state
+
+        :returns: Tuple of parameters for this DirectiveNode
+        :rtype: tuple:
+        """
+        return tuple(self._aug_get_params(self.metadata["augeaspath"]))
+
+    def _aug_get_params(self, path):
+        """Helper function to get parameters for DirectiveNodes and BlockNodes"""
+
+        arg_paths = self.parser.aug.match(path + "/arg")
+        return [self.parser.get_arg(apath) for apath in arg_paths]
+
+
+class AugeasBlockNode(AugeasDirectiveNode):
+    """ Augeas implementation of BlockNode interface """
+
+    def __init__(self, **kwargs):
+        super(AugeasBlockNode, self).__init__(**kwargs)
+        self.children = ()
+
+    def __eq__(self, other):
+        if isinstance(other, self.__class__):
+            return (self.name == other.name and
+                    self.filepath == other.filepath and
+                    self.parameters == other.parameters and
+                    self.children == other.children and
+                    self.enabled == other.enabled and
+                    self.dirty == other.dirty and
+                    self.ancestor == other.ancestor and
+                    self.metadata == other.metadata)
+        return False
+
+    # pylint: disable=unused-argument
+    def add_child_block(self, name, parameters=None, position=None):  # pragma: no cover
+        """Adds a new BlockNode to the sequence of children"""
+
+        insertpath, realpath, before = self._aug_resolve_child_position(
+            name,
+            position
+        )
+        new_metadata = {"augeasparser": self.parser, "augeaspath": realpath}
+
+        # Create the new block
+        self.parser.aug.insert(insertpath, name, before)
+        # Check if the file was included from the root config or initial state
+        enabled = self.parser.parsed_in_original(
+            apache_util.get_file_path(realpath)
+        )
+
+        # Parameters will be set at the initialization of the new object
+        new_block = AugeasBlockNode(name=name,
+                                    parameters=parameters,
+                                    enabled=enabled,
+                                    ancestor=assertions.PASS,
+                                    filepath=apache_util.get_file_path(realpath),
+                                    metadata=new_metadata)
+        return new_block
+
+    # pylint: disable=unused-argument
+    def add_child_directive(self, name, parameters=None, position=None):  # pragma: no cover
+        """Adds a new DirectiveNode to the sequence of children"""
+
+        if not parameters:
+            raise errors.PluginError("Directive requires parameters and none were set.")
+
+        insertpath, realpath, before = self._aug_resolve_child_position(
+            "directive",
+            position
+        )
+        new_metadata = {"augeasparser": self.parser, "augeaspath": realpath}
+
+        # Create the new directive
+        self.parser.aug.insert(insertpath, "directive", before)
+        # Set the directive key
+        self.parser.aug.set(realpath, name)
+        # Check if the file was included from the root config or initial state
+        enabled = self.parser.parsed_in_original(
+            apache_util.get_file_path(realpath)
+        )
+
+        new_dir = AugeasDirectiveNode(name=name,
+                                      parameters=parameters,
+                                      enabled=enabled,
+                                      ancestor=assertions.PASS,
+                                      filepath=apache_util.get_file_path(realpath),
+                                      metadata=new_metadata)
+        return new_dir
+
+    def add_child_comment(self, comment="", position=None):
+        """Adds a new CommentNode to the sequence of children"""
+
+        insertpath, realpath, before = self._aug_resolve_child_position(
+            "#comment",
+            position
+        )
+        new_metadata = {"augeasparser": self.parser, "augeaspath": realpath}
+
+        # Create the new comment
+        self.parser.aug.insert(insertpath, "#comment", before)
+        # Set the comment content
+        self.parser.aug.set(realpath, comment)
+
+        new_comment = AugeasCommentNode(comment=comment,
+                                        ancestor=assertions.PASS,
+                                        filepath=apache_util.get_file_path(realpath),
+                                        metadata=new_metadata)
+        return new_comment
+
+    def find_blocks(self, name, exclude=True):
+        """Recursive search of BlockNodes from the sequence of children"""
+
+        nodes = []
+        paths = self._aug_find_blocks(name)
+        if exclude:
+            paths = self.parser.exclude_dirs(paths)
+        for path in paths:
+            nodes.append(self._create_blocknode(path))
+
+        return nodes
+
+    def find_directives(self, name, exclude=True):
+        """Recursive search of DirectiveNodes from the sequence of children"""
+
+        nodes = []
+        ownpath = self.metadata.get("augeaspath")
+
+        directives = self.parser.find_dir(name, start=ownpath, exclude=exclude)
+        already_parsed = set()  # type: Set[str]
+        for directive in directives:
+            # Remove the /arg part from the Augeas path
+            directive = directive.partition("/arg")[0]
+            # find_dir returns an object for each _parameter_ of a directive
+            # so we need to filter out duplicates.
+            if directive not in already_parsed:
+                nodes.append(self._create_directivenode(directive))
+                already_parsed.add(directive)
+
+        return nodes
+
+    def find_comments(self, comment):
+        """
+        Recursive search of DirectiveNodes from the sequence of children.
+
+        :param str comment: Comment content to search for.
+        """
+
+        nodes = []
+        ownpath = self.metadata.get("augeaspath")
+
+        comments = self.parser.find_comments(comment, start=ownpath)
+        for com in comments:
+            nodes.append(self._create_commentnode(com))
+
+        return nodes
+
+    def delete_child(self, child):
+        """
+        Deletes a ParserNode from the sequence of children, and raises an
+        exception if it's unable to do so.
+        :param AugeasParserNode: child: A node to delete.
+        """
+        if not self.parser.aug.remove(child.metadata["augeaspath"]):
+
+            raise errors.PluginError(
+                ("Could not delete child node, the Augeas path: {} doesn't " +
+                 "seem to exist.").format(child.metadata["augeaspath"])
+            )
+
+    def unsaved_files(self):
+        """Returns a list of unsaved filepaths"""
+        return self.parser.unsaved_files()
+
+    def parsed_paths(self):
+        """
+        Returns a list of file paths that have currently been parsed into the parser
+        tree. The returned list may include paths with wildcard characters, for
+        example: ['/etc/apache2/conf.d/*.load']
+
+        This is typically called on the root node of the ParserNode tree.
+
+        :returns: list of file paths of files that have been parsed
+        """
+
+        res_paths = []
+
+        paths = self.parser.existing_paths
+        for directory in paths:
+            for filename in paths[directory]:
+                res_paths.append(os.path.join(directory, filename))
+
+        return res_paths
+
+    def _create_commentnode(self, path):
+        """Helper function to create a CommentNode from Augeas path"""
+
+        comment = self.parser.aug.get(path)
+        metadata = {"augeasparser": self.parser, "augeaspath": path}
+
+        # Because of the dynamic nature of AugeasParser and the fact that we're
+        # not populating the complete node tree, the ancestor has a dummy value
+        return AugeasCommentNode(comment=comment,
+                                 ancestor=assertions.PASS,
+                                 filepath=apache_util.get_file_path(path),
+                                 metadata=metadata)
+
+    def _create_directivenode(self, path):
+        """Helper function to create a DirectiveNode from Augeas path"""
+
+        name = self.parser.get_arg(path)
+        metadata = {"augeasparser": self.parser, "augeaspath": path}
+
+        # Check if the file was included from the root config or initial state
+        enabled = self.parser.parsed_in_original(
+            apache_util.get_file_path(path)
+        )
+        return AugeasDirectiveNode(name=name,
+                                   ancestor=assertions.PASS,
+                                   enabled=enabled,
+                                   filepath=apache_util.get_file_path(path),
+                                   metadata=metadata)
+
+    def _aug_find_blocks(self, name):
+        """Helper function to perform a search to Augeas DOM tree to search
+        configuration blocks with a given name"""
+
+        # The code here is modified from configurator.get_virtual_hosts()
+        blk_paths = set()
+        for vhost_path in list(self.parser.parser_paths):
+            paths = self.parser.aug.match(
+                ("/files%s//*[label()=~regexp('%s')]" %
+                    (vhost_path, parser.case_i(name))))
+            blk_paths.update([path for path in paths if
+                              name.lower() in os.path.basename(path).lower()])
+        return blk_paths
+
+    def _aug_resolve_child_position(self, name, position):
+        """
+        Helper function that iterates through the immediate children and figures
+        out the insertion path for a new AugeasParserNode.
+
+        Augeas also generalizes indices for directives and comments, simply by
+        using "directive" or "comment" respectively as their names.
+
+        This function iterates over the existing children of the AugeasBlockNode,
+        returning their insertion path, resulting Augeas path and if the new node
+        should be inserted before or after the returned insertion path.
+
+        Note: while Apache is case insensitive, Augeas is not, and blocks like
+        Nameofablock and NameOfABlock have different indices.
+
+        :param str name: Name of the AugeasBlockNode to insert, "directive" for
+            AugeasDirectiveNode or "comment" for AugeasCommentNode
+        :param int position: The position to insert the child AugeasParserNode to
+
+        :returns: Tuple of insert path, resulting path and a boolean if the new
+            node should be inserted before it.
+        :rtype: tuple of str, str, bool
+        """
+
+        # Default to appending
+        before = False
+
+        all_children = self.parser.aug.match("{}/*".format(
+            self.metadata["augeaspath"])
+        )
+
+        # Calculate resulting_path
+        # Augeas indices start at 1. We use counter to calculate the index to
+        # be used in resulting_path.
+        counter = 1
+        for i, child in enumerate(all_children):
+            if position is not None and i >= position:
+                # We're not going to insert the new node to an index after this
+                break
+            childname = self._aug_get_name(child)
+            if name == childname:
+                counter += 1
+
+        resulting_path = "{}/{}[{}]".format(
+            self.metadata["augeaspath"],
+            name,
+            counter
+        )
+
+        # Form the correct insert_path
+        # Inserting the only child and appending as the last child work
+        # similarly in Augeas.
+        append = not all_children or position is None or position >= len(all_children)
+        if append:
+            insert_path = "{}/*[last()]".format(
+                self.metadata["augeaspath"]
+            )
+        elif position == 0:
+            # Insert as the first child, before the current first one.
+            insert_path = all_children[0]
+            before = True
+        else:
+            insert_path = "{}/*[{}]".format(
+                self.metadata["augeaspath"],
+                position
+            )
+
+        return (insert_path, resulting_path, before)
+
+
+interfaces.CommentNode.register(AugeasCommentNode)
+interfaces.DirectiveNode.register(AugeasDirectiveNode)
+interfaces.BlockNode.register(AugeasBlockNode)

certbot-apache/certbot_apache/_internal/constants.py

@@ -1,6 +1,7 @@
 """Apache plugin constants."""
 import pkg_resources
 
+from certbot.compat import os
 
 MOD_SSL_CONF_DEST = "options-ssl-apache.conf"
 """Name of the mod_ssl config file as saved in `IConfig.config_dir`."""
@@ -9,6 +10,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,11 +20,18 @@ ALL_SSL_OPTIONS_HASHES = [
     'cfdd7c18d2025836ea3307399f509cfb1ebf2612c87dd600a65da2a8e2f2797b',
     '80720bd171ccdc2e6b917ded340defae66919e4624962396b992b7218a561791',
     'c0c022ea6b8a51ecc8f1003d0a04af6c3f2bc1c3ce506b3c2dfc1f11ef931082',
+    '717b0a89f5e4c39b09a42813ac6e747cfbdeb93439499e73f4f70a1fe1473f20',
+    '0fcdc81280cd179a07ec4d29d3595068b9326b455c488de4b09f585d5dafc137',
+    '86cc09ad5415cd6d5f09a947fe2501a9344328b1e8a8b458107ea903e80baa6c',
+    '06675349e457eae856120cdebb564efe546f0b87399f2264baeb41e442c724c7',
+    '5cc003edd93fb9cd03d40c7686495f8f058f485f75b5e764b789245a386e6daf',
+    '007cd497a56a3bb8b6a2c1aeb4997789e7e38992f74e44cc5d13a625a738ac73',
+    '34783b9e2210f5c4a23bced2dfd7ec289834716673354ed7c7abf69fe30192a3',
 ]
 """SHA256 hashes of the contents of previous versions of all versions of MOD_SSL_CONF_SRC"""
 
 AUGEAS_LENS_DIR = pkg_resources.resource_filename(
-    "certbot_apache", "augeas_lens")
+    "certbot_apache", os.path.join("_internal", "augeas_lens"))
 """Path to the Augeas lens directory"""
 
 REWRITE_HTTPS_ARGS = [

certbot-apache/certbot_apache/_internal/display_ops.py

@@ -1,15 +1,13 @@
 """Contains UI methods for Apache operations."""
 import logging
-import os
 
 import zope.component
 
 from certbot import errors
 from certbot import interfaces
-
+from certbot.compat import os
 import certbot.display.util as display_util
 
-
 logger = logging.getLogger(__name__)
 
 
@@ -23,10 +21,10 @@ def select_vhost_multiple(vhosts):
     :rtype: :class:`list`of type `~obj.Vhost`
     """
     if not vhosts:
-        return list()
+        return []
     tags_list = [vhost.display_repr()+"\n" for vhost in vhosts]
     # Remove the extra newline from the last entry
-    if len(tags_list):
+    if tags_list:
         tags_list[-1] = tags_list[-1][:-1]
     code, names = zope.component.getUtility(interfaces.IDisplay).checklist(
         "Which VirtualHosts would you like to install the wildcard certificate for?",
@@ -39,7 +37,7 @@ def select_vhost_multiple(vhosts):
 def _reversemap_vhosts(names, vhosts):
     """Helper function for select_vhost_multiple for mapping string
     representations back to actual vhost objects"""
-    return_vhosts = list()
+    return_vhosts = []
 
     for selection in names:
         for vhost in vhosts:
@@ -62,8 +60,7 @@ def select_vhost(domain, vhosts):
     code, tag = _vhost_menu(domain, vhosts)
     if code == display_util.OK:
         return vhosts[tag]
-    else:
-        return None
+    return None
 
 def _vhost_menu(domain, vhosts):
     """Select an appropriate Apache Vhost.
@@ -80,7 +77,7 @@ def _vhost_menu(domain, vhosts):
 
     if free_chars < 2:
         logger.debug("Display size is too small for "
-                     "certbot_apache.display_ops._vhost_menu()")
+                     "certbot_apache._internal.display_ops._vhost_menu()")
         # This runs the edge off the screen, but it doesn't cause an "error"
         filename_size = 1
         disp_name_size = 1
@@ -93,7 +90,7 @@ def _vhost_menu(domain, vhosts):
     for vhost in vhosts:
         if len(vhost.get_names()) == 1:
             disp_name = next(iter(vhost.get_names()))
-        elif len(vhost.get_names()) == 0:
+        elif not vhost.get_names():
             disp_name = ""
         else:
             disp_name = "Multiple Names"

certbot-apache/certbot_apache/_internal/dualparser.py

@@ -0,0 +1,306 @@
+""" Dual ParserNode implementation """
+from certbot_apache._internal import assertions
+from certbot_apache._internal import augeasparser
+from certbot_apache._internal import apacheparser
+
+
+class DualNodeBase(object):
+    """ Dual parser interface for in development testing. This is used as the
+    base class for dual parser interface classes. This class handles runtime
+    attribute value assertions."""
+
+    def save(self, msg):  # pragma: no cover
+        """ Call save for both parsers """
+        self.primary.save(msg)
+        self.secondary.save(msg)
+
+    def __getattr__(self, aname):
+        """ Attribute value assertion """
+        firstval = getattr(self.primary, aname)
+        secondval = getattr(self.secondary, aname)
+        exclusions = [
+            # Metadata will inherently be different, as ApacheParserNode does
+            # not have Augeas paths and so on.
+            aname == "metadata",
+            callable(firstval)
+        ]
+        if not any(exclusions):
+            assertions.assertEqualSimple(firstval, secondval)
+        return firstval
+
+    def find_ancestors(self, name):
+        """ Traverses the ancestor tree and returns ancestors matching name """
+        return self._find_helper(DualBlockNode, "find_ancestors", name)
+
+    def _find_helper(self, nodeclass, findfunc, search, **kwargs):
+        """A helper for find_* functions. The function specific attributes should
+        be passed as keyword arguments.
+
+        :param interfaces.ParserNode nodeclass: The node class for results.
+        :param str findfunc: Name of the find function to call
+        :param str search: The search term
+        """
+
+        primary_res = getattr(self.primary, findfunc)(search, **kwargs)
+        secondary_res = getattr(self.secondary, findfunc)(search, **kwargs)
+
+        # The order of search results for Augeas implementation cannot be
+        # assured.
+
+        pass_primary = assertions.isPassNodeList(primary_res)
+        pass_secondary = assertions.isPassNodeList(secondary_res)
+        new_nodes = []
+
+        if pass_primary and pass_secondary:
+            # Both unimplemented
+            new_nodes.append(nodeclass(primary=primary_res[0],
+                                       secondary=secondary_res[0]))  # pragma: no cover
+        elif pass_primary:
+            for c in secondary_res:
+                new_nodes.append(nodeclass(primary=primary_res[0],
+                                           secondary=c))
+        elif pass_secondary:
+            for c in primary_res:
+                new_nodes.append(nodeclass(primary=c,
+                                           secondary=secondary_res[0]))
+        else:
+            assert len(primary_res) == len(secondary_res)
+            matches = self._create_matching_list(primary_res, secondary_res)
+            for p, s in matches:
+                new_nodes.append(nodeclass(primary=p, secondary=s))
+
+        return new_nodes
+
+
+class DualCommentNode(DualNodeBase):
+    """ Dual parser implementation of CommentNode interface """
+
+    def __init__(self, **kwargs):
+        """ This initialization implementation allows ordinary initialization
+        of CommentNode objects as well as creating a DualCommentNode object
+        using precreated or fetched CommentNode objects if provided as optional
+        arguments primary and secondary.
+
+        Parameters other than the following are from interfaces.CommentNode:
+
+        :param CommentNode primary: Primary pre-created CommentNode, mainly
+            used when creating new DualParser nodes using add_* methods.
+        :param CommentNode secondary: Secondary pre-created CommentNode
+        """
+
+        kwargs.setdefault("primary", None)
+        kwargs.setdefault("secondary", None)
+        primary = kwargs.pop("primary")
+        secondary = kwargs.pop("secondary")
+
+        if primary or secondary:
+            assert primary and secondary
+            self.primary = primary
+            self.secondary = secondary
+        else:
+            self.primary = augeasparser.AugeasCommentNode(**kwargs)
+            self.secondary = apacheparser.ApacheCommentNode(**kwargs)
+
+        assertions.assertEqual(self.primary, self.secondary)
+
+
+class DualDirectiveNode(DualNodeBase):
+    """ Dual parser implementation of DirectiveNode interface """
+
+    def __init__(self, **kwargs):
+        """ This initialization implementation allows ordinary initialization
+        of DirectiveNode objects as well as creating a DualDirectiveNode object
+        using precreated or fetched DirectiveNode objects if provided as optional
+        arguments primary and secondary.
+
+        Parameters other than the following are from interfaces.DirectiveNode:
+
+        :param DirectiveNode primary: Primary pre-created DirectiveNode, mainly
+            used when creating new DualParser nodes using add_* methods.
+        :param DirectiveNode secondary: Secondary pre-created DirectiveNode
+
+
+        """
+
+        kwargs.setdefault("primary", None)
+        kwargs.setdefault("secondary", None)
+        primary = kwargs.pop("primary")
+        secondary = kwargs.pop("secondary")
+
+        if primary or secondary:
+            assert primary and secondary
+            self.primary = primary
+            self.secondary = secondary
+        else:
+            self.primary = augeasparser.AugeasDirectiveNode(**kwargs)
+            self.secondary = apacheparser.ApacheDirectiveNode(**kwargs)
+
+        assertions.assertEqual(self.primary, self.secondary)
+
+    def set_parameters(self, parameters):
+        """ Sets parameters and asserts that both implementation successfully
+        set the parameter sequence """
+
+        self.primary.set_parameters(parameters)
+        self.secondary.set_parameters(parameters)
+        assertions.assertEqual(self.primary, self.secondary)
+
+
+class DualBlockNode(DualNodeBase):
+    """ Dual parser implementation of BlockNode interface """
+
+    def __init__(self, **kwargs):
+        """ This initialization implementation allows ordinary initialization
+        of BlockNode objects as well as creating a DualBlockNode object
+        using precreated or fetched BlockNode objects if provided as optional
+        arguments primary and secondary.
+
+        Parameters other than the following are from interfaces.BlockNode:
+
+        :param BlockNode primary: Primary pre-created BlockNode, mainly
+            used when creating new DualParser nodes using add_* methods.
+        :param BlockNode secondary: Secondary pre-created BlockNode
+        """
+
+        kwargs.setdefault("primary", None)
+        kwargs.setdefault("secondary", None)
+        primary = kwargs.pop("primary")
+        secondary = kwargs.pop("secondary")
+
+        if primary or secondary:
+            assert primary and secondary
+            self.primary = primary
+            self.secondary = secondary
+        else:
+            self.primary = augeasparser.AugeasBlockNode(**kwargs)
+            self.secondary = apacheparser.ApacheBlockNode(**kwargs)
+
+        assertions.assertEqual(self.primary, self.secondary)
+
+    def add_child_block(self, name, parameters=None, position=None):
+        """ Creates a new child BlockNode, asserts that both implementations
+        did it in a similar way, and returns a newly created DualBlockNode object
+        encapsulating both of the newly created objects """
+
+        primary_new = self.primary.add_child_block(name, parameters, position)
+        secondary_new = self.secondary.add_child_block(name, parameters, position)
+        assertions.assertEqual(primary_new, secondary_new)
+        new_block = DualBlockNode(primary=primary_new, secondary=secondary_new)
+        return new_block
+
+    def add_child_directive(self, name, parameters=None, position=None):
+        """ Creates a new child DirectiveNode, asserts that both implementations
+        did it in a similar way, and returns a newly created DualDirectiveNode
+        object encapsulating both of the newly created objects """
+
+        primary_new = self.primary.add_child_directive(name, parameters, position)
+        secondary_new = self.secondary.add_child_directive(name, parameters, position)
+        assertions.assertEqual(primary_new, secondary_new)
+        new_dir = DualDirectiveNode(primary=primary_new, secondary=secondary_new)
+        return new_dir
+
+    def add_child_comment(self, comment="", position=None):
+        """ Creates a new child CommentNode, asserts that both implementations
+        did it in a similar way, and returns a newly created DualCommentNode
+        object encapsulating both of the newly created objects """
+
+        primary_new = self.primary.add_child_comment(comment, position)
+        secondary_new = self.secondary.add_child_comment(comment, position)
+        assertions.assertEqual(primary_new, secondary_new)
+        new_comment = DualCommentNode(primary=primary_new, secondary=secondary_new)
+        return new_comment
+
+    def _create_matching_list(self, primary_list, secondary_list):
+        """ Matches the list of primary_list to a list of secondary_list and
+        returns a list of tuples. This is used to create results for find_
+        methods.
+
+        This helper function exists, because we cannot ensure that the list of
+        search results returned by primary.find_* and secondary.find_* are ordered
+        in a same way. The function pairs the same search results from both
+        implementations to a list of tuples.
+        """
+
+        matched = []
+        for p in primary_list:
+            match = None
+            for s in secondary_list:
+                try:
+                    assertions.assertEqual(p, s)
+                    match = s
+                    break
+                except AssertionError:
+                    continue
+            if match:
+                matched.append((p, match))
+            else:
+                raise AssertionError("Could not find a matching node.")
+        return matched
+
+    def find_blocks(self, name, exclude=True):
+        """
+        Performs a search for BlockNodes using both implementations and does simple
+        checks for results. This is built upon the assumption that unimplemented
+        find_* methods return a list with a single assertion passing object.
+        After the assertion, it creates a list of newly created DualBlockNode
+        instances that encapsulate the pairs of returned BlockNode objects.
+        """
+
+        return self._find_helper(DualBlockNode, "find_blocks", name,
+                                 exclude=exclude)
+
+    def find_directives(self, name, exclude=True):
+        """
+        Performs a search for DirectiveNodes using both implementations and
+        checks the results. This is built upon the assumption that unimplemented
+        find_* methods return a list with a single assertion passing object.
+        After the assertion, it creates a list of newly created DualDirectiveNode
+        instances that encapsulate the pairs of returned DirectiveNode objects.
+        """
+
+        return self._find_helper(DualDirectiveNode, "find_directives", name,
+                                 exclude=exclude)
+
+    def find_comments(self, comment):
+        """
+        Performs a search for CommentNodes using both implementations and
+        checks the results. This is built upon the assumption that unimplemented
+        find_* methods return a list with a single assertion passing object.
+        After the assertion, it creates a list of newly created DualCommentNode
+        instances that encapsulate the pairs of returned CommentNode objects.
+        """
+
+        return self._find_helper(DualCommentNode, "find_comments", comment)
+
+    def delete_child(self, child):
+        """Deletes a child from the ParserNode implementations. The actual
+        ParserNode implementations are used here directly in order to be able
+        to match a child to the list of children."""
+
+        self.primary.delete_child(child.primary)
+        self.secondary.delete_child(child.secondary)
+
+    def unsaved_files(self):
+        """ Fetches the list of unsaved file paths and asserts that the lists
+        match """
+        primary_files = self.primary.unsaved_files()
+        secondary_files = self.secondary.unsaved_files()
+        assertions.assertEqualSimple(primary_files, secondary_files)
+
+        return primary_files
+
+    def parsed_paths(self):
+        """
+        Returns a list of file paths that have currently been parsed into the parser
+        tree. The returned list may include paths with wildcard characters, for
+        example: ['/etc/apache2/conf.d/*.load']
+
+        This is typically called on the root node of the ParserNode tree.
+
+        :returns: list of file paths of files that have been parsed
+        """
+
+        primary_paths = self.primary.parsed_paths()
+        secondary_paths = self.secondary.parsed_paths()
+        assertions.assertEqualPathsList(primary_paths, secondary_paths)
+        return primary_paths

certbot-apache/certbot_apache/_internal/entrypoint.py

@@ -1,23 +1,30 @@
 """ Entry point for Apache Plugin """
-from certbot import util
+from distutils.version import LooseVersion
 
-from certbot_apache import configurator
-from certbot_apache import override_arch
-from certbot_apache import override_darwin
-from certbot_apache import override_debian
-from certbot_apache import override_centos
-from certbot_apache import override_gentoo
-from certbot_apache import override_suse
+from certbot import util
+from certbot_apache._internal import configurator
+from certbot_apache._internal import override_arch
+from certbot_apache._internal import override_centos
+from certbot_apache._internal import override_darwin
+from certbot_apache._internal import override_debian
+from certbot_apache._internal import override_fedora
+from certbot_apache._internal import override_gentoo
+from certbot_apache._internal import override_suse
 
 OVERRIDE_CLASSES = {
     "arch": override_arch.ArchConfigurator,
+    "cloudlinux": override_centos.CentOSConfigurator,
     "darwin": override_darwin.DarwinConfigurator,
     "debian": override_debian.DebianConfigurator,
     "ubuntu": override_debian.DebianConfigurator,
     "centos": override_centos.CentOSConfigurator,
     "centos linux": override_centos.CentOSConfigurator,
-    "fedora": override_centos.CentOSConfigurator,
+    "fedora_old": override_centos.CentOSConfigurator,
+    "fedora": override_fedora.FedoraConfigurator,
+    "linuxmint": override_debian.DebianConfigurator,
     "ol": override_centos.CentOSConfigurator,
+    "oracle": override_centos.CentOSConfigurator,
+    "redhatenterpriseserver": override_centos.CentOSConfigurator,
     "red hat enterprise linux server": override_centos.CentOSConfigurator,
     "rhel": override_centos.CentOSConfigurator,
     "amazon": override_centos.CentOSConfigurator,
@@ -25,14 +32,24 @@ OVERRIDE_CLASSES = {
     "gentoo base system": override_gentoo.GentooConfigurator,
     "opensuse": override_suse.OpenSUSEConfigurator,
     "suse": override_suse.OpenSUSEConfigurator,
+    "sles": override_suse.OpenSUSEConfigurator,
+    "scientific": override_centos.CentOSConfigurator,
+    "scientific linux": override_centos.CentOSConfigurator,
 }
 
+
 def get_configurator():
     """ Get correct configurator class based on the OS fingerprint """
-    os_info = util.get_os_info()
+    os_name, os_version = util.get_os_info()
+    os_name = os_name.lower()
     override_class = None
+
+    # Special case for older Fedora versions
+    if os_name == 'fedora' and LooseVersion(os_version) < LooseVersion('29'):
+        os_name = 'fedora_old'
+
     try:
-        override_class = OVERRIDE_CLASSES[os_info[0].lower()]
+        override_class = OVERRIDE_CLASSES[os_name]
     except KeyError:
         # OS not found in the list
         os_like = util.get_systemd_os_like()
@@ -45,4 +62,5 @@ def get_configurator():
             override_class = configurator.ApacheConfigurator
     return override_class
 
+
 ENTRYPOINT = get_configurator()

certbot-apache/certbot_apache/_internal/http_01.py

@@ -1,16 +1,20 @@
 """A class that performs HTTP-01 challenges for Apache"""
 import logging
-import os
+import errno
 
-from acme.magic_typing import List, Set  # pylint: disable=unused-import, no-name-in-module
+from acme.magic_typing import List
+from acme.magic_typing import Set
 from certbot import errors
+from certbot.compat import filesystem
+from certbot.compat import os
 from certbot.plugins import common
-from certbot_apache.obj import VirtualHost  # pylint: disable=unused-import
-from certbot_apache.parser import get_aug_path
+from certbot_apache._internal.obj import VirtualHost  # pylint: disable=unused-import
+from certbot_apache._internal.parser import get_aug_path
 
 logger = logging.getLogger(__name__)
 
-class ApacheHttp01(common.TLSSNI01):
+
+class ApacheHttp01(common.ChallengePerformer):
     """Class that performs HTTP-01 challenges within the Apache configurator."""
 
     CONFIG_TEMPLATE22_PRE = """\
@@ -165,8 +169,15 @@ class ApacheHttp01(common.TLSSNI01):
 
     def _set_up_challenges(self):
         if not os.path.isdir(self.challenge_dir):
-            os.makedirs(self.challenge_dir)
-            os.chmod(self.challenge_dir, 0o755)
+            old_umask = os.umask(0o022)
+            try:
+                filesystem.makedirs(self.challenge_dir, 0o755)
+            except OSError as exception:
+                if exception.errno not in (errno.EEXIST, errno.EISDIR):
+                    raise errors.PluginError(
+                        "Couldn't create root for http-01 challenge")
+            finally:
+                os.umask(old_umask)
 
         responses = []
         for achall in self.achalls:
@@ -182,7 +193,7 @@ class ApacheHttp01(common.TLSSNI01):
         self.configurator.reverter.register_file_creation(True, name)
         with open(name, 'wb') as f:
             f.write(validation.encode())
-        os.chmod(name, 0o644)
+        filesystem.chmod(name, 0o644)
 
         return response
 
@@ -192,8 +203,8 @@ class ApacheHttp01(common.TLSSNI01):
 
         if vhost not in self.moded_vhosts:
             logger.debug(
-                "Adding a temporary challenge validation Include for name: %s " +
-                "in: %s", vhost.name, vhost.filep)
+                "Adding a temporary challenge validation Include for name: %s in: %s",
+                vhost.name, vhost.filep)
             self.configurator.parser.add_dir_beginning(
                 vhost.path, "Include", self.challenge_conf_pre)
             self.configurator.parser.add_dir(

certbot-apache/certbot_apache/_internal/interfaces.py

@@ -0,0 +1,515 @@
+"""ParserNode interface for interacting with configuration tree.
+
+General description
+-------------------
+
+The ParserNode interfaces are designed to be able to contain all the parsing logic,
+while allowing their users to interact with the configuration tree in a Pythonic
+and well structured manner.
+
+The structure allows easy traversal of the tree of ParserNodes. Each ParserNode
+stores a reference to its ancestor and immediate children, allowing the user to
+traverse the tree using built in interface methods as well as accessing the interface
+properties directly.
+
+ParserNode interface implementation should stand between the actual underlying
+parser functionality and the business logic within Configurator code, interfacing
+with both. The ParserNode tree is a result of configuration parsing action.
+
+ParserNode tree will be in charge of maintaining the parser state and hence the
+abstract syntax tree (AST). Interactions between ParserNode tree and underlying
+parser should involve only parsing the configuration files to this structure, and
+writing it back to the filesystem - while preserving the format including whitespaces.
+
+For some implementations (Apache for example) it's important to keep track of and
+to use state information while parsing conditional blocks and directives. This
+allows the implementation to set a flag to parts of the parsed configuration
+structure as not being in effect in a case of unmatched conditional block. It's
+important to store these blocks in the tree as well in order to not to conduct
+destructive actions (failing to write back parts of the configuration) while writing
+the AST back to the filesystem.
+
+The ParserNode tree is in charge of maintaining the its own structure while every
+child node fetched with find - methods or by iterating its list of children can be
+changed in place. When making changes the affected nodes should be flagged as "dirty"
+in order for the parser implementation to figure out the parts of the configuration
+that need to be written back to disk during the save() operation.
+
+
+Metadata
+--------
+
+The metadata holds all the implementation specific attributes of the ParserNodes -
+things like the positional information related to the AST, file paths, whitespacing,
+and any other information relevant to the underlying parser engine.
+
+Access to the metadata should be handled by implementation specific methods, allowing
+the Configurator functionality to access the underlying information where needed.
+
+For some implementations the node can be initialized using the information carried
+in metadata alone. This is useful especially when populating the ParserNode tree
+while parsing the configuration.
+
+
+Apache implementation
+---------------------
+
+The Apache implementation of ParserNode interface requires some implementation
+specific functionalities that are not described by the interface itself.
+
+Initialization
+
+When the user of a ParserNode class is creating these objects, they must specify
+the parameters as described in the documentation for the __init__ methods below.
+When these objects are created internally, however, some parameters may not be
+needed because (possibly more detailed) information is included in the metadata
+parameter. In this case, implementations can deviate from the required parameters
+from __init__, however, they should still behave the same when metadata is not
+provided.
+
+For consistency internally, if an argument is provided directly in the ParserNode
+initialization parameters as well as within metadata it's recommended to establish
+clear behavior around this scenario within the implementation.
+
+Conditional blocks
+
+Apache configuration can have conditional blocks, for example: <IfModule ...>,
+resulting the directives and subblocks within it being either enabled or disabled.
+While find_* interface methods allow including the disabled parts of the configuration
+tree in searches a special care needs to be taken while parsing the structure in
+order to reflect the active state of configuration.
+
+Whitespaces
+
+Each ParserNode object is responsible of storing its prepending whitespace characters
+in order to be able to write the AST back to filesystem like it was, preserving the
+format, this applies for parameters of BlockNode and DirectiveNode as well.
+When parameters of ParserNode are changed, the pre-existing whitespaces in the
+parameter sequence are discarded, as the general reason for storing them is to
+maintain the ability to write the configuration back to filesystem exactly like
+it was. This loses its meaning when we have to change the directives or blocks
+parameters for other reasons.
+
+Searches and matching
+
+Apache configuration is largely case insensitive, so the Apache implementation of
+ParserNode interface needs to provide the user means to match block and directive
+names and parameters in case insensitive manner. This does not apply to everything
+however, for example the parameters of a conditional statement may be case sensitive.
+For this reason the internal representation of data should not ignore the case.
+"""
+
+import abc
+import six
+
+
+
+@six.add_metaclass(abc.ABCMeta)
+class ParserNode(object):
+    """
+    ParserNode is the basic building block of the tree of such nodes,
+    representing the structure of the configuration. It is largely meant to keep
+    the structure information intact and idiomatically accessible.
+
+    The root node as well as the child nodes of it should be instances of ParserNode.
+    Nodes keep track of their differences to on-disk representation of configuration
+    by marking modified ParserNodes as dirty to enable partial write-to-disk for
+    different files in the configuration structure.
+
+    While for the most parts the usage and the child types are obvious, "include"-
+    and similar directives are an exception to this rule. This is because of the
+    nature of include directives - which unroll the contents of another file or
+    configuration block to their place. While we could unroll the included nodes
+    to the parent tree, it remains important to keep the context of include nodes
+    separate in order to write back the original configuration as it was.
+
+    For parsers that require the implementation to keep track of the whitespacing,
+    it's responsibility of each ParserNode object itself to store its prepending
+    whitespaces in order to be able to reconstruct the complete configuration file
+    as it was when originally read from the disk.
+
+    ParserNode objects should have the following attributes:
+
+    # Reference to ancestor node, or None if the node is the root node of the
+    # configuration tree.
+    ancestor: Optional[ParserNode]
+
+    # True if this node has been modified since last save.
+    dirty: bool
+
+    # Filepath of the file where the configuration element for this ParserNode
+    # object resides. For root node, the value for filepath is the httpd root
+    # configuration file. Filepath can be None if a configuration directive is
+    # defined in for example the httpd command line.
+    filepath: Optional[str]
+
+    # Metadata dictionary holds all the implementation specific key-value pairs
+    # for the ParserNode instance.
+    metadata: Dict[str, Any]
+    """
+
+    @abc.abstractmethod
+    def __init__(self, **kwargs):
+        """
+        Initializes the ParserNode instance, and sets the ParserNode specific
+        instance variables. This is not meant to be used directly, but through
+        specific classes implementing ParserNode interface.
+
+        :param ancestor: BlockNode ancestor for this CommentNode. Required.
+        :type ancestor: BlockNode or None
+
+        :param filepath: Filesystem path for the file where this CommentNode
+            does or should exist in the filesystem. Required.
+        :type filepath: str or None
+
+        :param dirty: Boolean flag for denoting if this CommentNode has been
+            created or changed after the last save. Default: False.
+        :type dirty: bool
+
+        :param metadata: Dictionary of metadata values for this ParserNode object.
+            Metadata information should be used only internally in the implementation.
+            Default: {}
+        :type metadata: dict
+        """
+
+    @abc.abstractmethod
+    def save(self, msg):
+        """
+        Save traverses the children, and attempts to write the AST to disk for
+        all the objects that are marked dirty. The actual operation of course
+        depends on the underlying implementation. save() shouldn't be called
+        from the Configurator outside of its designated save() method in order
+        to ensure that the Reverter checkpoints are created properly.
+
+        Note: this approach of keeping internal structure of the configuration
+        within the ParserNode tree does not represent the file inclusion structure
+        of actual configuration files that reside in the filesystem. To handle
+        file writes properly, the file specific temporary trees should be extracted
+        from the full ParserNode tree where necessary when writing to disk.
+
+        :param str msg: Message describing the reason for the save.
+
+        """
+
+    @abc.abstractmethod
+    def find_ancestors(self, name):
+        """
+        Traverses the ancestor tree up, searching for BlockNodes with a specific
+        name.
+
+        :param str name: Name of the ancestor BlockNode to search for
+
+        :returns: A list of ancestor BlockNodes that match the name
+        :rtype: list of BlockNode
+        """
+
+
+# Linter rule exclusion done because of https://github.com/PyCQA/pylint/issues/179
+@six.add_metaclass(abc.ABCMeta)  # pylint: disable=abstract-method
+class CommentNode(ParserNode):
+    """
+    CommentNode class is used for representation of comments within the parsed
+    configuration structure. Because of the nature of comments, it is not able
+    to have child nodes and hence it is always treated as a leaf node.
+
+    CommentNode stores its contents in class variable 'comment' and does not
+    have a specific name.
+
+    CommentNode objects should have the following attributes in addition to
+    the ones described in ParserNode:
+
+    # Contains the contents of the comment without the directive notation
+    # (typically # or /* ... */).
+    comment: str
+
+    """
+
+    @abc.abstractmethod
+    def __init__(self, **kwargs):
+        """
+        Initializes the CommentNode instance and sets its instance variables.
+
+        :param comment: Contents of the comment. Required.
+        :type comment: str
+
+        :param ancestor: BlockNode ancestor for this CommentNode. Required.
+        :type ancestor: BlockNode or None
+
+        :param filepath: Filesystem path for the file where this CommentNode
+            does or should exist in the filesystem. Required.
+        :type filepath: str or None
+
+        :param dirty: Boolean flag for denoting if this CommentNode has been
+            created or changed after the last save. Default: False.
+        :type dirty: bool
+        """
+        super(CommentNode, self).__init__(ancestor=kwargs['ancestor'],
+                                          dirty=kwargs.get('dirty', False),
+                                          filepath=kwargs['filepath'],
+                                          metadata=kwargs.get('metadata', {}))  # pragma: no cover
+
+
+@six.add_metaclass(abc.ABCMeta)
+class DirectiveNode(ParserNode):
+    """
+    DirectiveNode class represents a configuration directive within the configuration.
+    It can have zero or more parameters attached to it. Because of the nature of
+    single directives, it is not able to have child nodes and hence it is always
+    treated as a leaf node.
+
+    If a this directive was defined on the httpd command line, the ancestor instance
+    variable for this DirectiveNode should be None, and it should be inserted to the
+    beginning of root BlockNode children sequence.
+
+    DirectiveNode objects should have the following attributes in addition to
+    the ones described in ParserNode:
+
+    # True if this DirectiveNode is enabled and False if it is inside of an
+    # inactive conditional block.
+    enabled: bool
+
+    # Name, or key of the configuration directive. If BlockNode subclass of
+    # DirectiveNode is the root configuration node, the name should be None.
+    name: Optional[str]
+
+    # Tuple of parameters of this ParserNode object, excluding whitespaces.
+    parameters: Tuple[str, ...]
+
+    """
+
+    @abc.abstractmethod
+    def __init__(self, **kwargs):
+        """
+        Initializes the DirectiveNode instance and sets its instance variables.
+
+        :param name: Name or key of the DirectiveNode object. Required.
+        :type name: str or None
+
+        :param tuple parameters: Tuple of str parameters for this DirectiveNode.
+            Default: ().
+        :type parameters: tuple
+
+        :param ancestor: BlockNode ancestor for this DirectiveNode, or None for
+            root configuration node. Required.
+        :type ancestor: BlockNode or None
+
+        :param filepath: Filesystem path for the file where this DirectiveNode
+            does or should exist in the filesystem, or None for directives introduced
+            in the httpd command line. Required.
+        :type filepath: str or None
+
+        :param dirty: Boolean flag for denoting if this DirectiveNode has been
+            created or changed after the last save. Default: False.
+        :type dirty: bool
+
+        :param enabled: True if this DirectiveNode object is parsed in the active
+            configuration of the httpd. False if the DirectiveNode exists within a
+            unmatched conditional configuration block. Default: True.
+        :type enabled: bool
+
+        """
+        super(DirectiveNode, self).__init__(ancestor=kwargs['ancestor'],
+                                            dirty=kwargs.get('dirty', False),
+                                            filepath=kwargs['filepath'],
+                                            metadata=kwargs.get('metadata', {}))  # pragma: no cover
+
+    @abc.abstractmethod
+    def set_parameters(self, parameters):
+        """
+        Sets the sequence of parameters for this ParserNode object without
+        whitespaces. While the whitespaces for parameters are discarded when using
+        this method, the whitespacing preceeding the ParserNode itself should be
+        kept intact.
+
+        :param list parameters: sequence of parameters
+        """
+
+
+@six.add_metaclass(abc.ABCMeta)
+class BlockNode(DirectiveNode):
+    """
+    BlockNode class represents a block of nested configuration directives, comments
+    and other blocks as its children. A BlockNode can have zero or more parameters
+    attached to it.
+
+    Configuration blocks typically consist of one or more child nodes of all possible
+    types. Because of this, the BlockNode class has various discovery and structure
+    management methods.
+
+    Lists of parameters used as an optional argument for some of the methods should
+    be lists of strings that are applicable parameters for each specific BlockNode
+    or DirectiveNode type. As an example, for a following configuration example:
+
+        <VirtualHost *:80>
+           ...
+        </VirtualHost>
+
+    The node type would be BlockNode, name would be 'VirtualHost' and its parameters
+    would be: ['*:80'].
+
+    While for the following example:
+
+        LoadModule alias_module /usr/lib/apache2/modules/mod_alias.so
+
+    The node type would be DirectiveNode, name would be 'LoadModule' and its
+    parameters would be: ['alias_module', '/usr/lib/apache2/modules/mod_alias.so']
+
+    The applicable parameters are dependent on the underlying configuration language
+    and its grammar.
+
+    BlockNode objects should have the following attributes in addition to
+    the ones described in DirectiveNode:
+
+    # Tuple of direct children of this BlockNode object. The order of children
+    # in this tuple retain the order of elements in the parsed configuration
+    # block.
+    children: Tuple[ParserNode, ...]
+
+    """
+
+    @abc.abstractmethod
+    def add_child_block(self, name, parameters=None, position=None):
+        """
+        Adds a new BlockNode child node with provided values and marks the callee
+        BlockNode dirty. This is used to add new children to the AST. The preceeding
+        whitespaces should not be added based on the ancestor or siblings for the
+        newly created object. This is to match the current behavior of the legacy
+        parser implementation.
+
+        :param str name: The name of the child node to add
+        :param list parameters: list of parameters for the node
+        :param int position: Position in the list of children to add the new child
+            node to. Defaults to None, which appends the newly created node to the list.
+            If an integer is given, the child is inserted before that index in the
+            list similar to list().insert.
+
+        :returns: BlockNode instance of the created child block
+
+        """
+
+    @abc.abstractmethod
+    def add_child_directive(self, name, parameters=None, position=None):
+        """
+        Adds a new DirectiveNode child node with provided values and marks the
+        callee BlockNode dirty. This is used to add new children to the AST. The
+        preceeding whitespaces should not be added based on the ancestor or siblings
+        for the newly created object. This is to match the current behavior of the
+        legacy parser implementation.
+
+
+        :param str name: The name of the child node to add
+        :param list parameters: list of parameters for the node
+        :param int position: Position in the list of children to add the new child
+            node to. Defaults to None, which appends the newly created node to the list.
+            If an integer is given, the child is inserted before that index in the
+            list similar to list().insert.
+
+        :returns: DirectiveNode instance of the created child directive
+
+        """
+
+    @abc.abstractmethod
+    def add_child_comment(self, comment="", position=None):
+        """
+        Adds a new CommentNode child node with provided value and marks the
+        callee BlockNode dirty. This is used to add new children to the AST. The
+        preceeding whitespaces should not be added based on the ancestor or siblings
+        for the newly created object. This is to match the current behavior of the
+        legacy parser implementation.
+
+
+        :param str comment: Comment contents
+        :param int position: Position in the list of children to add the new child
+            node to. Defaults to None, which appends the newly created node to the list.
+            If an integer is given, the child is inserted before that index in the
+            list similar to list().insert.
+
+        :returns: CommentNode instance of the created child comment
+
+        """
+
+    @abc.abstractmethod
+    def find_blocks(self, name, exclude=True):
+        """
+        Find a configuration block by name. This method walks the child tree of
+        ParserNodes under the instance it was called from. This way it is possible
+        to search for the whole configuration tree, when starting from root node or
+        to do a partial search when starting from a specified branch. The lookup
+        should be case insensitive.
+
+        :param str name: The name of the directive to search for
+        :param bool exclude: If the search results should exclude the contents of
+            ParserNode objects that reside within conditional blocks and because
+            of current state are not enabled.
+
+        :returns: A list of found BlockNode objects.
+        """
+
+    @abc.abstractmethod
+    def find_directives(self, name, exclude=True):
+        """
+        Find a directive by name. This method walks the child tree of ParserNodes
+        under the instance it was called from. This way it is possible to search
+        for the whole configuration tree, when starting from root node, or to do
+        a partial search when starting from a specified branch. The lookup should
+        be case insensitive.
+
+        :param str name: The name of the directive to search for
+        :param bool exclude: If the search results should exclude the contents of
+            ParserNode objects that reside within conditional blocks and because
+            of current state are not enabled.
+
+        :returns: A list of found DirectiveNode objects.
+
+        """
+
+    @abc.abstractmethod
+    def find_comments(self, comment):
+        """
+        Find comments with value containing the search term.
+
+        This method walks the child tree of ParserNodes under the instance it was
+        called from. This way it is possible to search for the whole configuration
+        tree, when starting from root node, or to do a partial search when starting
+        from a specified branch. The lookup should be case sensitive.
+
+        :param str comment: The content of comment to search for
+
+        :returns: A list of found CommentNode objects.
+
+        """
+
+    @abc.abstractmethod
+    def delete_child(self, child):
+        """
+        Remove a specified child node from the list of children of the called
+        BlockNode object.
+
+        :param ParserNode child: Child ParserNode object to remove from the list
+            of children of the callee.
+        """
+
+    @abc.abstractmethod
+    def unsaved_files(self):
+        """
+        Returns a list of file paths that have been changed since the last save
+        (or the initial configuration parse). The intended use for this method
+        is to tell the Reverter which files need to be included in a checkpoint.
+
+        This is typically called for the root of the ParserNode tree.
+
+        :returns: list of file paths of files that have been changed but not yet
+            saved to disk.
+        """
+
+    @abc.abstractmethod
+    def parsed_paths(self):
+        """
+        Returns a list of file paths that have currently been parsed into the parser
+        tree. The returned list may include paths with wildcard characters, for
+        example: ['/etc/apache2/conf.d/*.load']
+
+        This is typically called on the root node of the ParserNode tree.
+
+        :returns: list of file paths of files that have been parsed
+        """

certbot-apache/certbot_apache/_internal/obj.py

@@ -1,7 +1,7 @@
 """Module contains classes used by the Apache Configurator."""
 import re
 
-from acme.magic_typing import Set # pylint: disable=unused-import, no-name-in-module
+from acme.magic_typing import Set
 from certbot.plugins import common
 
 
@@ -24,9 +24,9 @@ class Addr(common.Addr):
         return not self.__eq__(other)
 
     def __repr__(self):
-        return "certbot_apache.obj.Addr(" + repr(self.tup) + ")"
+        return "certbot_apache._internal.obj.Addr(" + repr(self.tup) + ")"
 
-    def __hash__(self):
+    def __hash__(self):  # pylint: disable=useless-super-delegation
         # Python 3 requires explicit overridden for __hash__ if __eq__ or
         # __cmp__ is overridden. See https://bugs.python.org/issue2235
         return super(Addr, self).__hash__()
@@ -47,8 +47,7 @@ class Addr(common.Addr):
             return 0
         elif self.get_addr() == "*":
             return 1
-        else:
-            return 2
+        return 2
 
     def conflicts(self, addr):
         r"""Returns if address could conflict with correct function of self.
@@ -99,7 +98,7 @@ class Addr(common.Addr):
         return self.get_addr_obj(port)
 
 
-class VirtualHost(object):  # pylint: disable=too-few-public-methods
+class VirtualHost(object):
     """Represents an Apache Virtualhost.
 
     :ivar str filep: file path of VH
@@ -125,9 +124,8 @@ class VirtualHost(object):  # pylint: disable=too-few-public-methods
     strip_name = re.compile(r"^(?:.+://)?([^ :$]*)")
 
     def __init__(self, filep, path, addrs, ssl, enabled, name=None,
-                 aliases=None, modmacro=False, ancestor=None):
+                 aliases=None, modmacro=False, ancestor=None, node=None):
 
-        # pylint: disable=too-many-arguments
         """Initialize a VH."""
         self.filep = filep
         self.path = path
@@ -138,6 +136,7 @@ class VirtualHost(object):  # pylint: disable=too-few-public-methods
         self.enabled = enabled
         self.modmacro = modmacro
         self.ancestor = ancestor
+        self.node = node
 
     def get_names(self):
         """Return a set of all names."""

certbot-apache/certbot_apache/_internal/override_arch.py

@@ -1,11 +1,9 @@
 """ Distribution specific override class for Arch Linux """
-import pkg_resources
-
 import zope.interface
 
 from certbot import interfaces
+from certbot_apache._internal import configurator
 
-from certbot_apache import configurator
 
 @zope.interface.provider(interfaces.IPluginFactory)
 class ArchConfigurator(configurator.ApacheConfigurator):
@@ -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/_internal/override_centos.py

@@ -1,18 +1,16 @@
 """ Distribution specific override class for CentOS family (RHEL, Fedora) """
 import logging
-import pkg_resources
-
-from acme.magic_typing import List  # pylint: disable=unused-import, no-name-in-module
 
 import zope.interface
 
+from acme.magic_typing import List
+from certbot import errors
 from certbot import interfaces
-
-
-from certbot_apache import apache_util
-from certbot_apache import configurator
-from certbot_apache import parser
+from certbot import util
 from certbot.errors import MisconfigurationError
+from certbot_apache._internal import apache_util
+from certbot_apache._internal import configurator
+from certbot_apache._internal import parser
 
 logger = logging.getLogger(__name__)
 
@@ -37,10 +35,40 @@ 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):
+        """
+        Override config_test to mitigate configtest error in vanilla installation
+        of mod_ssl in Fedora. The error is caused by non-existent self-signed
+        certificates referenced by the configuration, that would be autogenerated
+        during the first (re)start of httpd.
+        """
+
+        os_info = util.get_os_info()
+        fedora = os_info[0].lower() == "fedora"
+
+        try:
+            super(CentOSConfigurator, self).config_test()
+        except errors.MisconfigurationError:
+            if fedora:
+                self._try_restart_fedora()
+            else:
+                raise
+
+    def _try_restart_fedora(self):
+        """
+        Tries to restart httpd using systemctl to generate the self signed keypair.
+        """
+
+        try:
+            util.run_script(['systemctl', 'restart', 'httpd'])
+        except errors.SubprocessError as err:
+            raise errors.MisconfigurationError(str(err))
+
+        # Finish with actual config check to see if systemctl restart helped
+        super(CentOSConfigurator, self).config_test()
+
     def _prepare_options(self):
         """
         Override the options dictionary initialization in order to support
@@ -52,10 +80,10 @@ 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):
+    def _deploy_cert(self, *args, **kwargs):  # pylint: disable=arguments-differ
         """
         Override _deploy_cert in order to ensure that the Apache configuration
         has "LoadModule ssl_module..." before parsing the VirtualHost configuration
@@ -65,7 +93,6 @@ class CentOSConfigurator(configurator.ApacheConfigurator):
         if self.version < (2, 4, 0):
             self._deploy_loadmodule_ssl_if_needed()
 
-
     def _deploy_loadmodule_ssl_if_needed(self):
         """
         Add "LoadModule ssl_module <pre-existing path>" to main httpd.conf if
@@ -122,7 +149,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",
@@ -150,7 +177,7 @@ class CentOSParser(parser.ApacheParser):
     def parse_sysconfig_var(self):
         """ Parses Apache CLI options from CentOS configuration file """
         defines = apache_util.parse_define_file(self.sysconfig_filep, "OPTIONS")
-        for k in defines.keys():
+        for k in defines:
             self.variables[k] = defines[k]
 
     def not_modssl_ifmodule(self, path):

certbot-apache/certbot_apache/_internal/override_darwin.py

@@ -1,11 +1,9 @@
 """ Distribution specific override class for macOS """
-import pkg_resources
-
 import zope.interface
 
 from certbot import interfaces
+from certbot_apache._internal import configurator
 
-from certbot_apache import configurator
 
 @zope.interface.provider(interfaces.IPluginFactory)
 class DarwinConfigurator(configurator.ApacheConfigurator):
@@ -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/_internal/override_debian.py

@@ -1,19 +1,19 @@
 """ Distribution specific override class for Debian family (Ubuntu/Debian) """
 import logging
-import os
-import pkg_resources
 
 import zope.interface
 
 from certbot import errors
 from certbot import interfaces
 from certbot import util
-
-from certbot_apache import apache_util
-from certbot_apache import configurator
+from certbot.compat import filesystem
+from certbot.compat import os
+from certbot_apache._internal import apache_util
+from certbot_apache._internal import configurator
 
 logger = logging.getLogger(__name__)
 
+
 @zope.interface.provider(interfaces.IPluginFactory)
 class DebianConfigurator(configurator.ApacheConfigurator):
     """Debian specific ApacheConfigurator override class"""
@@ -33,8 +33,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):
@@ -44,14 +42,14 @@ class DebianConfigurator(configurator.ApacheConfigurator):
                   modules are enabled appropriately.
 
         :param vhost: vhost to enable
-        :type vhost: :class:`~certbot_apache.obj.VirtualHost`
+        :type vhost: :class:`~certbot_apache._internal.obj.VirtualHost`
 
         :raises .errors.NotSupportedError: If filesystem layout is not
             supported.
 
         """
         if vhost.enabled:
-            return
+            return None
 
         enabled_path = ("%s/sites-enabled/%s" %
                         (self.parser.root,
@@ -64,26 +62,25 @@ 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
-                return
-            else:
-                logger.warning(
-                    "Could not symlink %s to %s, got error: %s", enabled_path,
-                    vhost.filep, err.strerror)
-                errstring = ("Encountered error while trying to enable a " +
-                             "newly created VirtualHost located at {0} by " +
-                             "linking to it from {1}")
-                raise errors.NotSupportedError(errstring.format(vhost.filep,
-                                                                enabled_path))
+                return None
+            logger.warning(
+                "Could not symlink %s to %s, got error: %s", enabled_path,
+                vhost.filep, err.strerror)
+            errstring = ("Encountered error while trying to enable a " +
+                         "newly created VirtualHost located at {0} by " +
+                         "linking to it from {1}")
+            raise errors.NotSupportedError(errstring.format(vhost.filep,
+                                                            enabled_path))
         vhost.enabled = True
         logger.info("Enabling available site: %s", vhost.filep)
         self.save_notes += "Enabled site %s\n" % vhost.filep
+        return None
 
     def enable_mod(self, mod_name, temp=False):
-        # pylint: disable=unused-argument
         """Enables module in Apache.
 
         Both enables and reloads Apache so module is active.

certbot-apache/certbot_apache/_internal/override_fedora.py

@@ -0,0 +1,93 @@
+""" Distribution specific override class for Fedora 29+ """
+import zope.interface
+
+from certbot import errors
+from certbot import interfaces
+from certbot import util
+from certbot_apache._internal import apache_util
+from certbot_apache._internal import configurator
+from certbot_apache._internal import parser
+
+
+@zope.interface.provider(interfaces.IPluginFactory)
+class FedoraConfigurator(configurator.ApacheConfigurator):
+    """Fedora 29+ specific ApacheConfigurator override class"""
+
+    OS_DEFAULTS = dict(
+        server_root="/etc/httpd",
+        vhost_root="/etc/httpd/conf.d",
+        vhost_files="*.conf",
+        logs_root="/var/log/httpd",
+        ctl="httpd",
+        version_cmd=['httpd', '-v'],
+        restart_cmd=['apachectl', 'graceful'],
+        restart_cmd_alt=['apachectl', 'restart'],
+        conftest_cmd=['apachectl', 'configtest'],
+        enmod=None,
+        dismod=None,
+        le_vhost_ext="-le-ssl.conf",
+        handle_modules=False,
+        handle_sites=False,
+        challenge_location="/etc/httpd/conf.d",
+    )
+
+    def config_test(self):
+        """
+        Override config_test to mitigate configtest error in vanilla installation
+        of mod_ssl in Fedora. The error is caused by non-existent self-signed
+        certificates referenced by the configuration, that would be autogenerated
+        during the first (re)start of httpd.
+        """
+        try:
+            super(FedoraConfigurator, self).config_test()
+        except errors.MisconfigurationError:
+            self._try_restart_fedora()
+
+    def get_parser(self):
+        """Initializes the ApacheParser"""
+        return FedoraParser(
+            self.option("server_root"), self.option("vhost_root"),
+            self.version, configurator=self)
+
+    def _try_restart_fedora(self):
+        """
+        Tries to restart httpd using systemctl to generate the self signed keypair.
+        """
+        try:
+            util.run_script(['systemctl', 'restart', 'httpd'])
+        except errors.SubprocessError as err:
+            raise errors.MisconfigurationError(str(err))
+
+        # Finish with actual config check to see if systemctl restart helped
+        super(FedoraConfigurator, self).config_test()
+
+    def _prepare_options(self):
+        """
+        Override the options dictionary initialization to keep using apachectl
+        instead of httpd and so take advantages of this new bash script in newer versions
+        of Fedora to restart httpd.
+        """
+        super(FedoraConfigurator, self)._prepare_options()
+        self.options["restart_cmd"][0] = 'apachectl'
+        self.options["restart_cmd_alt"][0] = 'apachectl'
+        self.options["conftest_cmd"][0] = 'apachectl'
+
+
+class FedoraParser(parser.ApacheParser):
+    """Fedora 29+ specific ApacheParser override class"""
+    def __init__(self, *args, **kwargs):
+        # Fedora 29+ specific configuration file for Apache
+        self.sysconfig_filep = "/etc/sysconfig/httpd"
+        super(FedoraParser, self).__init__(*args, **kwargs)
+
+    def update_runtime_variables(self):
+        """ Override for update_runtime_variables for custom parsing """
+        # Opportunistic, works if SELinux not enforced
+        super(FedoraParser, self).update_runtime_variables()
+        self._parse_sysconfig_var()
+
+    def _parse_sysconfig_var(self):
+        """ Parses Apache CLI options from Fedora configuration file """
+        defines = apache_util.parse_define_file(self.sysconfig_filep, "OPTIONS")
+        for k in defines:
+            self.variables[k] = defines[k]

certbot-apache/certbot_apache/_internal/override_gentoo.py

@@ -1,13 +1,11 @@
 """ Distribution specific override class for Gentoo Linux """
-import pkg_resources
-
 import zope.interface
 
 from certbot import interfaces
+from certbot_apache._internal import apache_util
+from certbot_apache._internal import configurator
+from certbot_apache._internal import parser
 
-from certbot_apache import apache_util
-from certbot_apache import configurator
-from certbot_apache import parser
 
 @zope.interface.provider(interfaces.IPluginFactory)
 class GentooConfigurator(configurator.ApacheConfigurator):
@@ -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)
 
 
@@ -64,12 +60,12 @@ class GentooParser(parser.ApacheParser):
         """ Parses Apache CLI options from Gentoo configuration file """
         defines = apache_util.parse_define_file(self.apacheconfig_filep,
                                                 "APACHE2_OPTS")
-        for k in defines.keys():
+        for k in defines:
             self.variables[k] = defines[k]
 
     def update_modules(self):
         """Get loaded modules from httpd process, and add them to DOM"""
         mod_cmd = [self.configurator.option("ctl"), "modules"]
-        matches = self.parse_from_subprocess(mod_cmd, r"(.*)_module")
+        matches = apache_util.parse_from_subprocess(mod_cmd, r"(.*)_module")
         for mod in matches:
             self.add_mod(mod.strip())

certbot-apache/certbot_apache/_internal/override_suse.py

@@ -1,11 +1,9 @@
 """ Distribution specific override class for OpenSUSE """
-import pkg_resources
-
 import zope.interface
 
 from certbot import interfaces
+from certbot_apache._internal import configurator
 
-from certbot_apache import configurator
 
 @zope.interface.provider(interfaces.IPluginFactory)
 class OpenSUSEConfigurator(configurator.ApacheConfigurator):
@@ -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/_internal/parser.py

@@ -2,21 +2,22 @@
 import copy
 import fnmatch
 import logging
-import os
 import re
-import subprocess
 import sys
 
 import six
 
-from acme.magic_typing import Dict, List, Set  # pylint: disable=unused-import, no-name-in-module
+from acme.magic_typing import Dict
+from acme.magic_typing import List
 from certbot import errors
+from certbot.compat import os
+from certbot_apache._internal import apache_util
+from certbot_apache._internal import constants
 
 logger = logging.getLogger(__name__)
 
 
 class ApacheParser(object):
-    # pylint: disable=too-many-public-methods
     """Class handles the fine details of parsing the Apache Configuration.
 
     .. todo:: Make parsing general... remove sites-available etc...
@@ -29,9 +30,9 @@ class ApacheParser(object):
 
     """
     arg_var_interpreter = re.compile(r"\$\{[^ \}]*}")
-    fnmatch_chars = set(["*", "?", "\\", "[", "]"])
+    fnmatch_chars = {"*", "?", "\\", "[", "]"}
 
-    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.
 
@@ -40,11 +41,20 @@ class ApacheParser(object):
         # issues with aug.load() after adding new files / defines to parse tree
         self.configurator = configurator
 
-        self.modules = set()  # type: Set[str]
+        # 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 = {}  # type: Dict[str, 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()}
@@ -76,6 +86,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
 
@@ -83,7 +233,7 @@ class ApacheParser(object):
         :param str inc_path: path of file to include
 
         """
-        if len(self.find_dir(case_i("Include"), inc_path)) == 0:
+        if not self.find_dir(case_i("Include"), inc_path):
             logger.debug("Adding Include %s to %s",
                          inc_path, get_aug_path(main_config))
             self.add_dir(
@@ -98,14 +248,14 @@ class ApacheParser(object):
     def add_mod(self, mod_name):
         """Shortcut for updating parser modules."""
         if mod_name + "_module" not in self.modules:
-            self.modules.add(mod_name + "_module")
+            self.modules[mod_name + "_module"] = None
         if "mod_" + mod_name + ".c" not in self.modules:
-            self.modules.add("mod_" + mod_name + ".c")
+            self.modules["mod_" + mod_name + ".c"] = None
 
     def reset_modules(self):
         """Reset the loaded modules list. This is called from cleanup to clear
         temporarily loaded modules."""
-        self.modules = set()
+        self.modules = {}
         self.update_modules()
         self.parse_modules()
 
@@ -116,7 +266,7 @@ class ApacheParser(object):
             the iteration issue.  Else... parse and enable mods at same time.
 
         """
-        mods = set()  # type: Set[str]
+        mods = {}  # type: Dict[str, str]
         matches = self.find_dir("LoadModule")
         iterator = iter(matches)
         # Make sure prev_size != cur_size for do: while: iteration
@@ -130,41 +280,24 @@ class ApacheParser(object):
                 mod_name = self.get_arg(match_name)
                 mod_filename = self.get_arg(match_filename)
                 if mod_name and mod_filename:
-                    mods.add(mod_name)
-                    mods.add(os.path.basename(mod_filename)[:-2] + "c")
+                    mods[mod_name] = mod_filename
+                    mods[os.path.basename(mod_filename)[:-2] + "c"] = mod_filename
                 else:
-                    logger.debug("Could not read LoadModule directive from " +
-                                 "Augeas path: {0}".format(match_name[6:]))
+                    logger.debug("Could not read LoadModule directive from Augeas path: %s",
+                                 match_name[6:])
         self.modules.update(mods)
 
     def update_runtime_variables(self):
         """Update Includes, Defines and Includes from httpd config dump data"""
+
         self.update_defines()
         self.update_includes()
         self.update_modules()
 
     def update_defines(self):
-        """Get Defines from httpd process"""
+        """Updates the dictionary of known variables in the configuration"""
 
-        variables = dict()
-        define_cmd = [self.configurator.option("ctl"), "-t", "-D",
-                      "DUMP_RUN_CFG"]
-        matches = self.parse_from_subprocess(define_cmd, r"Define: ([^ \n]*)")
-        try:
-            matches.remove("DUMP_RUN_CFG")
-        except ValueError:
-            return
-
-        for match in matches:
-            if match.count("=") > 1:
-                logger.error("Unexpected number of equal signs in "
-                             "runtime config dump.")
-                raise errors.PluginError(
-                    "Error parsing Apache runtime variables")
-            parts = match.partition("=")
-            variables[parts[0]] = parts[2]
-
-        self.variables = variables
+        self.variables = apache_util.parse_defines(self.configurator.option("ctl"))
 
     def update_includes(self):
         """Get includes from httpd process, and add them to DOM if needed"""
@@ -174,9 +307,7 @@ class ApacheParser(object):
         # configuration files
         _ = self.find_dir("Include")
 
-        inc_cmd = [self.configurator.option("ctl"), "-t", "-D",
-                   "DUMP_INCLUDES"]
-        matches = self.parse_from_subprocess(inc_cmd, r"\(.*\) (.*)")
+        matches = apache_util.parse_includes(self.configurator.option("ctl"))
         if matches:
             for i in matches:
                 if not self.parsed_in_current(i):
@@ -185,57 +316,11 @@ class ApacheParser(object):
     def update_modules(self):
         """Get loaded modules from httpd process, and add them to DOM"""
 
-        mod_cmd = [self.configurator.option("ctl"), "-t", "-D",
-                       "DUMP_MODULES"]
-        matches = self.parse_from_subprocess(mod_cmd, r"(.*)_module")
+        matches = apache_util.parse_modules(self.configurator.option("ctl"))
         for mod in matches:
             self.add_mod(mod.strip())
 
-    def parse_from_subprocess(self, command, regexp):
-        """Get values from stdout of subprocess command
-
-        :param list command: Command to run
-        :param str regexp: Regexp for parsing
-
-        :returns: list parsed from command output
-        :rtype: list
-
-        """
-        stdout = self._get_runtime_cfg(command)
-        return re.compile(regexp).findall(stdout)
-
-    def _get_runtime_cfg(self, command):  # pylint: disable=no-self-use
-        """Get runtime configuration info.
-        :param command: Command to run
-
-        :returns: stdout from command
-
-        """
-        try:
-            proc = subprocess.Popen(
-                command,
-                stdout=subprocess.PIPE,
-                stderr=subprocess.PIPE,
-                universal_newlines=True)
-            stdout, stderr = proc.communicate()
-
-        except (OSError, ValueError):
-            logger.error(
-                "Error running command %s for runtime parameters!%s",
-                command, os.linesep)
-            raise errors.MisconfigurationError(
-                "Error accessing loaded Apache parameters: %s",
-                command)
-        # Small errors that do not impede
-        if proc.returncode != 0:
-            logger.warning("Error in checking parameter list: %s", stderr)
-            raise errors.MisconfigurationError(
-                "Apache is unable to check whether or not the module is "
-                "loaded because Apache is misconfigured.")
-
-        return stdout
-
-    def filter_args_num(self, matches, args):  # pylint: disable=no-self-use
+    def filter_args_num(self, matches, args):
         """Filter out directives with specific number of arguments.
 
         This function makes the assumption that all related arguments are given
@@ -251,12 +336,12 @@ class ApacheParser(object):
         """
         filtered = []
         if args == 1:
-            for i in range(len(matches)):
-                if matches[i].endswith("/arg"):
+            for i, match in enumerate(matches):
+                if match.endswith("/arg"):
                     filtered.append(matches[i][:-4])
         else:
-            for i in range(len(matches)):
-                if matches[i].endswith("/arg[%d]" % args):
+            for i, match in enumerate(matches):
+                if match.endswith("/arg[%d]" % args):
                     # Make sure we don't cause an IndexError (end of list)
                     # Check to make sure arg + 1 doesn't exist
                     if (i == (len(matches) - 1) or
@@ -461,7 +546,7 @@ class ApacheParser(object):
             "%s//*[self::directive=~regexp('%s')]" % (start, regex))
 
         if exclude:
-            matches = self._exclude_dirs(matches)
+            matches = self.exclude_dirs(matches)
 
         if arg is None:
             arg_suffix = "/arg"
@@ -474,7 +559,7 @@ class ApacheParser(object):
         # https://httpd.apache.org/docs/2.4/mod/core.html#include
         for match in matches:
             dir_ = self.aug.get(match).lower()
-            if dir_ == "include" or dir_ == "includeoptional":
+            if dir_ in ("include", "includeoptional"):
                 ordered_matches.extend(self.find_dir(
                     directive, arg,
                     self._get_include_path(self.get_arg(match + "/arg")),
@@ -514,8 +599,7 @@ class ApacheParser(object):
         # e.g. strip now, not later
         if not value:
             return None
-        else:
-            value = value.strip("'\"")
+        value = value.strip("'\"")
 
         variables = ApacheParser.arg_var_interpreter.findall(value)
 
@@ -528,9 +612,15 @@ class ApacheParser(object):
 
         return value
 
-    def _exclude_dirs(self, matches):
+    def get_root_augpath(self):
+        """
+        Returns the Augeas path of root configuration.
+        """
+        return get_aug_path(self.loc["root"])
+
+    def exclude_dirs(self, matches):
         """Exclude directives that are not loaded into the configuration."""
-        filters = [("ifmodule", self.modules), ("ifdefine", self.variables)]
+        filters = [("ifmodule", self.modules.keys()), ("ifdefine", self.variables)]
 
         valid_matches = []
 
@@ -571,6 +661,25 @@ class ApacheParser(object):
 
         return True
 
+    def standard_path_from_server_root(self, arg):
+        """Ensure paths are consistent and absolute
+
+        :param str arg: Argument of directive
+
+        :returns: Standardized argument path
+        :rtype: str
+        """
+        # Remove beginning and ending quotes
+        arg = arg.strip("'\"")
+
+        # Standardize the include argument based on server root
+        if not arg.startswith("/"):
+            # Normpath will condense ../
+            arg = os.path.normpath(os.path.join(self.root, arg))
+        else:
+            arg = os.path.normpath(arg)
+        return arg
+
     def _get_include_path(self, arg):
         """Converts an Apache Include directive into Augeas path.
 
@@ -591,16 +700,7 @@ class ApacheParser(object):
         # if matchObj.group() != arg:
         #     logger.error("Error: Invalid regexp characters in %s", arg)
         #     return []
-
-        # Remove beginning and ending quotes
-        arg = arg.strip("'\"")
-
-        # Standardize the include argument based on server root
-        if not arg.startswith("/"):
-            # Normpath will condense ../
-            arg = os.path.normpath(os.path.join(self.root, arg))
-        else:
-            arg = os.path.normpath(arg)
+        arg = self.standard_path_from_server_root(arg)
 
         # Attempts to add a transform to the file if one does not already exist
         if os.path.isdir(arg):
@@ -614,7 +714,7 @@ class ApacheParser(object):
         split_arg = arg.split("/")
         for idx, split in enumerate(split_arg):
             if any(char in ApacheParser.fnmatch_chars for char in split):
-                # Turn it into a augeas regex
+                # Turn it into an augeas regex
                 # TODO: Can this instead be an augeas glob instead of regex
                 split_arg[idx] = ("* [label()=~regexp('%s')]" %
                                   self.fnmatch_to_re(split))
@@ -624,7 +724,7 @@ class ApacheParser(object):
 
         return get_aug_path(arg)
 
-    def fnmatch_to_re(self, clean_fn_match):  # pylint: disable=no-self-use
+    def fnmatch_to_re(self, clean_fn_match):
         """Method converts Apache's basic fnmatch to regular expression.
 
         Assumption - Configs are assumed to be well-formed and only writable by
@@ -641,10 +741,9 @@ class ApacheParser(object):
         """
         if sys.version_info < (3, 6):
             # This strips off final /Z(?ms)
-            return fnmatch.translate(clean_fn_match)[:-7]
-        else:  # pragma: no cover
-            # Since Python 3.6, it returns a different pattern like (?s:.*\.load)\Z
-            return fnmatch.translate(clean_fn_match)[4:-3]
+            return fnmatch.translate(clean_fn_match)[:-7]  # pragma: no cover
+        # Since Python 3.6, it returns a different pattern like (?s:.*\.load)\Z
+        return fnmatch.translate(clean_fn_match)[4:-3]  # pragma: no cover
 
     def parse_file(self, filepath):
         """Parse file with Augeas
@@ -658,8 +757,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:
@@ -726,10 +824,7 @@ class ApacheParser(object):
                 use_new = False
             else:
                 use_new = True
-            if new_file_match == "*":
-                remove_old = True
-            else:
-                remove_old = False
+            remove_old = new_file_match == "*"
         except KeyError:
             use_new = True
             remove_old = False
@@ -850,8 +945,8 @@ def case_i(string):
     :param str string: string to make case i regex
 
     """
-    return "".join(["[" + c.upper() + c.lower() + "]"
-                    if c.isalpha() else c for c in re.escape(string)])
+    return "".join("[" + c.upper() + c.lower() + "]"
+                    if c.isalpha() else c for c in re.escape(string))
 
 
 def get_aug_path(file_path):

certbot-apache/certbot_apache/_internal/parsernode_util.py

@@ -0,0 +1,129 @@
+"""ParserNode utils"""
+
+
+def validate_kwargs(kwargs, required_names):
+    """
+    Ensures that the kwargs dict has all the expected values. This function modifies
+    the kwargs dictionary, and hence the returned dictionary should be used instead
+    in the caller function instead of the original kwargs.
+
+    :param dict kwargs: Dictionary of keyword arguments to validate.
+    :param list required_names: List of required parameter names.
+    """
+
+    validated_kwargs = {}
+    for name in required_names:
+        try:
+            validated_kwargs[name] = kwargs.pop(name)
+        except KeyError:
+            raise TypeError("Required keyword argument: {} undefined.".format(name))
+
+    # Raise exception if unknown key word arguments are found.
+    if kwargs:
+        unknown = ", ".join(kwargs.keys())
+        raise TypeError("Unknown keyword argument(s): {}".format(unknown))
+    return validated_kwargs
+
+
+def parsernode_kwargs(kwargs):
+    """
+    Validates keyword arguments for ParserNode. This function modifies the kwargs
+    dictionary, and hence the returned dictionary should be used instead in the
+    caller function instead of the original kwargs.
+
+    If metadata is provided, the otherwise required argument "filepath" may be
+    omitted if the implementation is able to extract its value from the metadata.
+    This usecase is handled within this function. Filepath defaults to None.
+
+    :param dict kwargs: Keyword argument dictionary to validate.
+
+    :returns: Tuple of validated and prepared arguments.
+    """
+
+    # As many values of ParserNode instances can be derived from the metadata,
+    # (ancestor being a common exception here) make sure we permit it here as well.
+    if "metadata" in kwargs:
+        # Filepath can be derived from the metadata in Augeas implementation.
+        # Default is None, as in this case the responsibility of populating this
+        # variable lies on the implementation.
+        kwargs.setdefault("filepath", None)
+
+    kwargs.setdefault("dirty", False)
+    kwargs.setdefault("metadata", {})
+
+    kwargs = validate_kwargs(kwargs, ["ancestor", "dirty", "filepath", "metadata"])
+    return kwargs["ancestor"], kwargs["dirty"], kwargs["filepath"], kwargs["metadata"]
+
+
+def commentnode_kwargs(kwargs):
+    """
+    Validates keyword arguments for CommentNode and sets the default values for
+    optional kwargs. This function modifies the kwargs dictionary, and hence the
+    returned dictionary should be used instead in the caller function instead of
+    the original kwargs.
+
+    If metadata is provided, the otherwise required argument "comment" may be
+    omitted if the implementation is able to extract its value from the metadata.
+    This usecase is handled within this function.
+
+    :param dict kwargs: Keyword argument dictionary to validate.
+
+    :returns: Tuple of validated and prepared arguments and ParserNode kwargs.
+    """
+
+    # As many values of ParserNode instances can be derived from the metadata,
+    # (ancestor being a common exception here) make sure we permit it here as well.
+    if "metadata" in kwargs:
+        kwargs.setdefault("comment", None)
+        # Filepath can be derived from the metadata in Augeas implementation.
+        # Default is None, as in this case the responsibility of populating this
+        # variable lies on the implementation.
+        kwargs.setdefault("filepath", None)
+
+    kwargs.setdefault("dirty", False)
+    kwargs.setdefault("metadata", {})
+
+    kwargs = validate_kwargs(kwargs, ["ancestor", "dirty", "filepath", "comment",
+                                      "metadata"])
+
+    comment = kwargs.pop("comment")
+    return comment, kwargs
+
+
+def directivenode_kwargs(kwargs):
+    """
+    Validates keyword arguments for DirectiveNode and BlockNode and sets the
+    default values for optional kwargs. This function modifies the kwargs
+    dictionary, and hence the returned dictionary should be used instead in the
+    caller function instead of the original kwargs.
+
+    If metadata is provided, the otherwise required argument "name" may be
+    omitted if the implementation is able to extract its value from the metadata.
+    This usecase is handled within this function.
+
+    :param dict kwargs: Keyword argument dictionary to validate.
+
+    :returns: Tuple of validated and prepared arguments and ParserNode kwargs.
+    """
+
+    # As many values of ParserNode instances can be derived from the metadata,
+    # (ancestor being a common exception here) make sure we permit it here as well.
+    if "metadata" in kwargs:
+        kwargs.setdefault("name", None)
+        # Filepath can be derived from the metadata in Augeas implementation.
+        # Default is None, as in this case the responsibility of populating this
+        # variable lies on the implementation.
+        kwargs.setdefault("filepath", None)
+
+    kwargs.setdefault("dirty", False)
+    kwargs.setdefault("enabled", True)
+    kwargs.setdefault("parameters", ())
+    kwargs.setdefault("metadata", {})
+
+    kwargs = validate_kwargs(kwargs, ["ancestor", "dirty", "filepath", "name",
+                                      "parameters", "enabled", "metadata"])
+
+    name = kwargs.pop("name")
+    parameters = kwargs.pop("parameters")
+    enabled = kwargs.pop("enabled")
+    return name, parameters, enabled, kwargs