Merge remote-tracking branch 'origin/master' into ocsp_apache_continued

.azure-pipelines/advanced.yml

@@ -1,5 +1,8 @@
 # Advanced pipeline for isolated checks and release purpose
 trigger:
+  # When changing these triggers, please ensure the documentation under
+  # "Running tests in CI" is still correct.
+  - azure-test-*
   - test-*
   - '*.x'
 pr:

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

@@ -33,22 +33,24 @@ jobs:
     pool:
       vmImage: $(imageName)
     steps:
+      - powershell: Invoke-WebRequest https://www.python.org/ftp/python/3.8.1/python-3.8.1-amd64-webinstall.exe -OutFile C:\py3-setup.exe
+        displayName: Get Python
+      - script: C:\py3-setup.exe /quiet PrependPath=1 InstallAllUsers=1 Include_launcher=1 InstallLauncherAllUsers=1 Include_test=0 Include_doc=0 Include_dev=1 Include_debug=0 Include_tcltk=0 TargetDir=C:\py3
+        displayName: Install Python
       - task: DownloadPipelineArtifact@2
         inputs:
           artifact: windows-installer
           path: $(Build.SourcesDirectory)/bin
         displayName: Retrieve Windows installer
-      - script: $(Build.SourcesDirectory)\bin\certbot-beta-installer-win32.exe /S
-        displayName: Install Certbot
-      - powershell: Invoke-WebRequest https://www.python.org/ftp/python/3.8.1/python-3.8.1-amd64-webinstall.exe -OutFile C:\py3-setup.exe
-        displayName: Get Python
-      - script: C:\py3-setup.exe /quiet PrependPath=1 InstallAllUsers=1 Include_launcher=1 InstallLauncherAllUsers=1 Include_test=0 Include_doc=0 Include_dev=1 Include_debug=0 Include_tcltk=0 TargetDir=C:\py3
-        displayName: Install Python
       - 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 integration tests
+        displayName: Run certbot integration tests

.travis.yml

@@ -14,17 +14,19 @@ before_script:
   - export TOX_TESTENV_PASSENV=TRAVIS
 
 # 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`. 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:
     # apache-parser-v2 is a temporary branch for doing work related to
     # rewriting the parser in the Apache plugin.
     - apache-parser-v2
     - master
     - /^\d+\.\d+\.x$/
-    - /^test-.*$/
+    - /^(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
@@ -57,7 +59,7 @@ matrix:
       # cryptography we support cannot be compiled against the version of
       # OpenSSL in Xenial or newer.
       dist: trusty
-      env: TOXENV='py27-{acme,apache,certbot,dns,nginx}-oldest'
+      env: TOXENV='py27-{acme,apache,apache-v2,certbot,dns,nginx}-oldest'
       <<: *not-on-master
     - python: "3.5"
       env: TOXENV=py35
@@ -261,8 +263,10 @@ addons:
 # except in tests where the environment variable CERTBOT_NO_PIN is set.
 # virtualenv is listed here explicitly to make sure it is upgraded when
 # CERTBOT_NO_PIN is set to work around failures we've seen when using an older
-# version of virtualenv.
-install: 'tools/pip_install.py -U codecov tox virtualenv'
+# 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 codecov tox virtualenv'
 # Most of the time TRAVIS_RETRY is an empty string, and has no effect on the
 # script command. It is set only to `travis_retry` during farm tests, in
 # order to trigger the Travis retry feature, and compensate the inherent
@@ -274,6 +278,7 @@ after_success: '[ "$TOXENV" == "py27-cover" ] && codecov -F linux'
 notifications:
   email: false
   irc:
+    if: NOT branch =~ ^(travis-)?test-.*$
     channels:
       # This is set to a secure variable to prevent forks from sending
       # notifications. This value was created by installing

AUTHORS.md

@@ -36,6 +36,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)
@@ -100,6 +101,7 @@ 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)
 * [Jaap Eldering](https://github.com/eldering)
 * [Jacob Hoffman-Andrews](https://github.com/jsha)
@@ -124,6 +126,7 @@ 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)

acme/acme/challenges.py

@@ -303,7 +303,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

acme/docs/conf.py

@@ -113,7 +113,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/setup.py

@@ -4,7 +4,7 @@ from setuptools import find_packages
 from setuptools import setup
 from setuptools.command.test import test as TestCommand
 
-version = '1.2.0.dev0'
+version = '1.3.0.dev0'
 
 # Please update tox.ini when modifying dependency version requirements
 install_requires = [
@@ -61,7 +61,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',
@@ -70,7 +70,6 @@ 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',

acme/tests/challenges_test.py

@@ -181,7 +181,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):
@@ -197,7 +197,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):

certbot-apache/MANIFEST.in

@@ -1,7 +1,6 @@
 include LICENSE.txt
 include README.rst
 recursive-include tests *
-include certbot_apache/_internal/centos-options-ssl-apache.conf
 include certbot_apache/_internal/options-ssl-apache.conf
 recursive-include certbot_apache/_internal/augeas_lens *.aug
 global-exclude __pycache__

certbot-apache/certbot_apache/_internal/apache_util.py

@@ -1,15 +1,22 @@
 """ Utility functions for certbot-apache plugin """
 import binascii
+import fnmatch
 import hashlib
-import struct
+import logging
+import re
 import shutil
+import struct
+import subprocess
 import time
 
 from certbot import crypto_util
 from certbot import errors
 from certbot import util
+
 from certbot.compat import os
 
+logger = logging.getLogger(__name__)
+
 
 def get_apache_ocsp_struct(ttl, ocsp_response):
     """Create Apache OCSP response structure to be used in response cache
@@ -199,3 +206,131 @@ def parse_define_file(filepath, varname):
 def unique_id():
     """ Returns an unique id to be used as a VirtualHost identifier"""
     return binascii.hexlify(os.urandom(16)).decode("utf-8")
+
+
+def 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 = dict()
+    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

certbot-apache/certbot_apache/_internal/apacheparser.py

@@ -0,0 +1,169 @@
+""" 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):
+        """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
+
+    def add_child_block(self, name, parameters=None, position=None):  # pylint: disable=unused-argument
+        """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
+
+    def add_child_directive(self, name, parameters=None, position=None):  # pylint: disable=unused-argument
+        """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)]
+
+    def find_comments(self, comment, exact=False): # pylint: disable=unused-argument
+        """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  # pylint: disable=unused-import, no-name-in-module
+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 = list()
+        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 = list()
+        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 = list()
+        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/centos-options-ssl-apache.conf

@@ -1,25 +0,0 @@
-# This file contains important security parameters. If you modify this file
-# manually, Certbot will be unable to automatically provide future security
-# updates. Instead, Certbot will print and log an error message with a path to
-# the up-to-date file that you will need to refer to when manually updating
-# this file.
-
-SSLEngine on
-
-# Intermediate configuration, tweak to your needs
-SSLProtocol             all -SSLv2 -SSLv3
-SSLCipherSuite          ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA256:ECDHE-ECDSA-AES128-SHA:ECDHE-RSA-AES256-SHA384:ECDHE-RSA-AES128-SHA:ECDHE-ECDSA-AES256-SHA384:ECDHE-ECDSA-AES256-SHA:ECDHE-RSA-AES256-SHA:DHE-RSA-AES128-SHA256:DHE-RSA-AES128-SHA:DHE-RSA-AES256-SHA256:DHE-RSA-AES256-SHA:ECDHE-ECDSA-DES-CBC3-SHA:ECDHE-RSA-DES-CBC3-SHA:EDH-RSA-DES-CBC3-SHA:AES128-GCM-SHA256:AES256-GCM-SHA384:AES128-SHA256:AES256-SHA256:AES128-SHA:AES256-SHA:DES-CBC3-SHA:!DSS
-SSLHonorCipherOrder     on
-
-SSLOptions +StrictRequire
-
-# Add vhost name to log entries:
-LogFormat "%h %l %u %t \"%r\" %>s %b \"%{Referer}i\" \"%{User-agent}i\"" vhost_combined
-LogFormat "%v %h %l %u %t \"%r\" %>s %b" vhost_common
-
-#CustomLog /var/log/apache2/access.log vhost_combined
-#LogLevel warn
-#ErrorLog /var/log/apache2/error.log
-
-# Always ensure Cookies have "Secure" set (JAH 2012/1)
-#Header edit Set-Cookie (?i)^(.*)(;\s*secure)??((\s*;)?(.*)) "$1; Secure$3$4"

certbot-apache/certbot_apache/_internal/configurator.py

@@ -30,8 +30,10 @@ from certbot.plugins import common
 from certbot.plugins.enhancements import AutoHSTSEnhancement
 from certbot.plugins.util import path_surgery
 from certbot_apache._internal import apache_util
+from certbot_apache._internal import assertions
 from certbot_apache._internal import constants
 from certbot_apache._internal import display_ops
+from certbot_apache._internal import dualparser
 from certbot_apache._internal import http_01
 from certbot_apache._internal import obj
 from certbot_apache._internal import parser
@@ -182,6 +184,7 @@ class ApacheConfigurator(common.Installer):
 
         """
         version = kwargs.pop("version", None)
+        use_parsernode = kwargs.pop("use_parsernode", False)
         super(ApacheConfigurator, self).__init__(*args, **kwargs)
 
         # Add name_server association dict
@@ -197,10 +200,15 @@ class ApacheConfigurator(common.Installer):
         self._autohsts = {}  # type: Dict[str, Dict[str, Union[int, float]]]
         # Reverter save notes
         self.save_notes = ""
-
+        # Should we use ParserNode implementation instead of the old behavior
+        self.USE_PARSERNODE = use_parsernode
+        # Saves the list of file paths that were parsed initially, and
+        # not added to parser tree by self.conf("vhost-root") for example.
+        self.parsed_paths = []  # type: List[str]
         # These will be set in the prepare function
         self._prepared = False
         self.parser = None
+        self.parser_root = None
         self.version = version
         self.vhosts = None
         self.options = copy.deepcopy(self.OS_DEFAULTS)
@@ -250,6 +258,14 @@ class ApacheConfigurator(common.Installer):
         # Perform the actual Augeas initialization to be able to react
         self.parser = self.get_parser()
 
+        # Set up ParserNode root
+        pn_meta = {"augeasparser": self.parser,
+                   "augeaspath": self.parser.get_root_augpath(),
+                   "ac_ast": None}
+        if self.USE_PARSERNODE:
+            self.parser_root = self.get_parsernode_root(pn_meta)
+            self.parsed_paths = self.parser_root.parsed_paths()
+
         # Check for errors in parsing files with Augeas
         self.parser.check_parsing_errors("httpd.aug")
 
@@ -345,6 +361,22 @@ class ApacheConfigurator(common.Installer):
             self.option("server_root"), self.conf("vhost-root"),
             self.version, configurator=self)
 
+    def get_parsernode_root(self, metadata):
+        """Initializes the ParserNode parser root instance."""
+
+        apache_vars = dict()
+        apache_vars["defines"] = apache_util.parse_defines(self.option("ctl"))
+        apache_vars["includes"] = apache_util.parse_includes(self.option("ctl"))
+        apache_vars["modules"] = apache_util.parse_modules(self.option("ctl"))
+        metadata["apache_vars"] = apache_vars
+
+        return dualparser.DualBlockNode(
+            name=assertions.PASS,
+            ancestor=None,
+            filepath=self.parser.loc["root"],
+            metadata=metadata
+        )
+
     def _wildcard_domain(self, domain):
         """
         Checks if domain is a wildcard domain
@@ -869,6 +901,29 @@ class ApacheConfigurator(common.Installer):
         return vhost
 
     def get_virtual_hosts(self):
+        """
+        Temporary wrapper for legacy and ParserNode version for
+        get_virtual_hosts. This should be replaced with the ParserNode
+        implementation when ready.
+        """
+
+        v1_vhosts = self.get_virtual_hosts_v1()
+        if self.USE_PARSERNODE:
+            v2_vhosts = self.get_virtual_hosts_v2()
+
+            for v1_vh in v1_vhosts:
+                found = False
+                for v2_vh in v2_vhosts:
+                    if assertions.isEqualVirtualHost(v1_vh, v2_vh):
+                        found = True
+                        break
+                if not found:
+                    raise AssertionError("Equivalent for {} was not found".format(v1_vh.path))
+
+            return v2_vhosts
+        return v1_vhosts
+
+    def get_virtual_hosts_v1(self):
         """Returns list of virtual hosts found in the Apache configuration.
 
         :returns: List of :class:`~certbot_apache._internal.obj.VirtualHost`
@@ -921,6 +976,80 @@ class ApacheConfigurator(common.Installer):
                     vhs.append(new_vhost)
         return vhs
 
+    def get_virtual_hosts_v2(self):
+        """Returns list of virtual hosts found in the Apache configuration using
+        ParserNode interface.
+        :returns: List of :class:`~certbot_apache.obj.VirtualHost`
+            objects found in configuration
+        :rtype: list
+        """
+
+        vhs = []
+        vhosts = self.parser_root.find_blocks("VirtualHost", exclude=False)
+        for vhblock in vhosts:
+            vhs.append(self._create_vhost_v2(vhblock))
+        return vhs
+
+    def _create_vhost_v2(self, node):
+        """Used by get_virtual_hosts_v2 to create vhost objects using ParserNode
+        interfaces.
+        :param interfaces.BlockNode node: The BlockNode object of VirtualHost block
+        :returns: newly created vhost
+        :rtype: :class:`~certbot_apache.obj.VirtualHost`
+        """
+        addrs = set()
+        for param in node.parameters:
+            addrs.add(obj.Addr.fromstring(param))
+
+        is_ssl = False
+        # Exclusion to match the behavior in get_virtual_hosts_v2
+        sslengine = node.find_directives("SSLEngine", exclude=False)
+        if sslengine:
+            for directive in sslengine:
+                if directive.parameters[0].lower() == "on":
+                    is_ssl = True
+                    break
+
+        # "SSLEngine on" might be set outside of <VirtualHost>
+        # Treat vhosts with port 443 as ssl vhosts
+        for addr in addrs:
+            if addr.get_port() == "443":
+                is_ssl = True
+
+        enabled = apache_util.included_in_paths(node.filepath, self.parsed_paths)
+
+        macro = False
+        # Check if the VirtualHost is contained in a mod_macro block
+        if node.find_ancestors("Macro"):
+            macro = True
+        vhost = obj.VirtualHost(
+            node.filepath, None, addrs, is_ssl, enabled, modmacro=macro, node=node
+        )
+        self._populate_vhost_names_v2(vhost)
+        return vhost
+
+    def _populate_vhost_names_v2(self, vhost):
+        """Helper function that populates the VirtualHost names.
+        :param host: In progress vhost whose names will be added
+        :type host: :class:`~certbot_apache.obj.VirtualHost`
+        """
+
+        servername_match = vhost.node.find_directives("ServerName",
+                                                      exclude=False)
+        serveralias_match = vhost.node.find_directives("ServerAlias",
+                                                       exclude=False)
+
+        servername = None
+        if servername_match:
+            servername = servername_match[-1].parameters[-1]
+
+        if not vhost.modmacro:
+            for alias in serveralias_match:
+                for serveralias in alias.parameters:
+                    vhost.aliases.add(serveralias)
+            vhost.name = servername
+
+
     def is_name_vhost(self, target_addr):
         """Returns if vhost is a name based vhost
 

certbot-apache/certbot_apache/_internal/constants.py

@@ -24,6 +24,8 @@ ALL_SSL_OPTIONS_HASHES = [
     '0fcdc81280cd179a07ec4d29d3595068b9326b455c488de4b09f585d5dafc137',
     '86cc09ad5415cd6d5f09a947fe2501a9344328b1e8a8b458107ea903e80baa6c',
     '06675349e457eae856120cdebb564efe546f0b87399f2264baeb41e442c724c7',
+    '5cc003edd93fb9cd03d40c7686495f8f058f485f75b5e764b789245a386e6daf',
+    '007cd497a56a3bb8b6a2c1aeb4997789e7e38992f74e44cc5d13a625a738ac73',
 ]
 """SHA256 hashes of the contents of previous versions of all versions of MOD_SSL_CONF_SRC"""
 

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 = list()
+
+        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 = list()
+        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/interfaces.py

@@ -0,0 +1,516 @@
+"""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
+
+from acme.magic_typing import Any, Dict, Optional, Tuple  # pylint: disable=unused-import, no-name-in-module
+
+
+@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

@@ -124,7 +124,7 @@ class VirtualHost(object):
     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):
 
         """Initialize a VH."""
         self.filep = filep
@@ -136,6 +136,7 @@ class VirtualHost(object):
         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/options-ssl-apache.conf

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

certbot-apache/certbot_apache/_internal/override_centos.py

@@ -38,7 +38,7 @@ class CentOSConfigurator(configurator.ApacheConfigurator):
         handle_sites=False,
         challenge_location="/etc/httpd/conf.d",
         MOD_SSL_CONF_SRC=pkg_resources.resource_filename(
-            "certbot_apache", os.path.join("_internal", "centos-options-ssl-apache.conf"))
+            "certbot_apache", os.path.join("_internal", "options-ssl-apache.conf"))
     )
 
     def config_test(self):

certbot-apache/certbot_apache/_internal/override_fedora.py

@@ -33,7 +33,7 @@ class FedoraConfigurator(configurator.ApacheConfigurator):
         challenge_location="/etc/httpd/conf.d",
         MOD_SSL_CONF_SRC=pkg_resources.resource_filename(
             # TODO: eventually newest version of Fedora will need their own config
-            "certbot_apache", os.path.join("_internal", "centos-options-ssl-apache.conf"))
+            "certbot_apache", os.path.join("_internal", "options-ssl-apache.conf"))
     )
 
     def config_test(self):

certbot-apache/certbot_apache/_internal/override_gentoo.py

@@ -70,6 +70,6 @@ class GentooParser(parser.ApacheParser):
     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/parser.py

@@ -3,7 +3,6 @@ import copy
 import fnmatch
 import logging
 import re
-import subprocess
 import sys
 
 import six
@@ -13,6 +12,7 @@ from acme.magic_typing import List  # pylint: disable=unused-import, no-name-in-
 from acme.magic_typing import Set  # pylint: disable=unused-import, no-name-in-module
 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__)
@@ -290,32 +290,15 @@ class ApacheParser(object):
 
     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"""
@@ -325,9 +308,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):
@@ -336,56 +317,10 @@ 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: {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 filter_args_num(self, matches, args):  # pylint: disable=no-self-use
         """Filter out directives with specific number of arguments.
 
@@ -612,7 +547,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"
@@ -678,7 +613,13 @@ 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)]
 

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 = dict()
+    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

certbot-apache/setup.py

@@ -4,7 +4,7 @@ from setuptools import find_packages
 from setuptools import setup
 from setuptools.command.test import test as TestCommand
 
-version = '1.2.0.dev0'
+version = '1.3.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.
@@ -18,6 +18,9 @@ install_requires = [
     'zope.interface',
 ]
 
+dev_extras = [
+    'apacheconfig>=0.3.1',
+]
 
 class PyTest(TestCommand):
     user_options = []
@@ -42,7 +45,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',
         'Environment :: Plugins',
@@ -53,7 +56,6 @@ 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',
@@ -69,6 +71,9 @@ setup(
     packages=find_packages(),
     include_package_data=True,
     install_requires=install_requires,
+    extras_require={
+        'dev': dev_extras,
+    },
     entry_points={
         'certbot.plugins': [
             'apache = certbot_apache._internal.entrypoint:ENTRYPOINT',

certbot-apache/tests/augeasnode_test.py

@@ -0,0 +1,319 @@
+"""Tests for AugeasParserNode classes"""
+import mock
+
+import util
+
+from acme.magic_typing import List  # pylint: disable=unused-import, no-name-in-module
+from certbot import errors
+
+from certbot_apache._internal import assertions
+
+
+
+class AugeasParserNodeTest(util.ApacheTest):  # pylint: disable=too-many-public-methods
+    """Test AugeasParserNode using available test configurations"""
+
+    def setUp(self):  # pylint: disable=arguments-differ
+        super(AugeasParserNodeTest, self).setUp()
+
+        self.config = util.get_apache_configurator(
+            self.config_path, self.vhost_path, self.config_dir, self.work_dir, use_parsernode=True)
+        self.vh_truth = util.get_vh_truth(
+            self.temp_dir, "debian_apache_2_4/multiple_vhosts")
+
+    def test_save(self):
+        with mock.patch('certbot_apache._internal.parser.ApacheParser.save') as mock_save:
+            self.config.parser_root.save("A save message")
+        self.assertTrue(mock_save.called)
+        self.assertEqual(mock_save.call_args[0][0], "A save message")
+
+    def test_unsaved_files(self):
+        with mock.patch('certbot_apache._internal.parser.ApacheParser.unsaved_files') as mock_uf:
+            mock_uf.return_value = ["first", "second"]
+            files = self.config.parser_root.unsaved_files()
+        self.assertEqual(files, ["first", "second"])
+
+    def test_get_block_node_name(self):
+        from certbot_apache._internal.augeasparser import AugeasBlockNode
+        block = AugeasBlockNode(
+            name=assertions.PASS,
+            ancestor=None,
+            filepath=assertions.PASS,
+            metadata={"augeasparser": mock.Mock(), "augeaspath": "/files/anything"}
+        )
+        testcases = {
+            "/some/path/FirstNode/SecondNode": "SecondNode",
+            "/some/path/FirstNode/SecondNode/": "SecondNode",
+            "OnlyPathItem": "OnlyPathItem",
+            "/files/etc/apache2/apache2.conf/VirtualHost": "VirtualHost",
+            "/Anything": "Anything",
+        }
+        for test in testcases:
+            self.assertEqual(block._aug_get_name(test), testcases[test])  # pylint: disable=protected-access
+
+    def test_find_blocks(self):
+        blocks = self.config.parser_root.find_blocks("VirtualHost", exclude=False)
+        self.assertEqual(len(blocks), 12)
+
+    def test_find_blocks_case_insensitive(self):
+        vhs = self.config.parser_root.find_blocks("VirtualHost")
+        vhs2 = self.config.parser_root.find_blocks("viRtuAlHoST")
+        self.assertEqual(len(vhs), len(vhs2))
+
+    def test_find_directive_found(self):
+        directives = self.config.parser_root.find_directives("Listen")
+        self.assertEqual(len(directives), 1)
+        self.assertTrue(directives[0].filepath.endswith("/apache2/ports.conf"))
+        self.assertEqual(directives[0].parameters, (u'80',))
+
+    def test_find_directive_notfound(self):
+        directives = self.config.parser_root.find_directives("Nonexistent")
+        self.assertEqual(len(directives), 0)
+
+    def test_find_directive_from_block(self):
+        blocks = self.config.parser_root.find_blocks("virtualhost")
+        found = False
+        for vh in blocks:
+            if vh.filepath.endswith("sites-enabled/certbot.conf"):
+                servername = vh.find_directives("servername")
+                self.assertEqual(servername[0].parameters[0], "certbot.demo")
+                found = True
+        self.assertTrue(found)
+
+    def test_find_comments(self):
+        rootcomment = self.config.parser_root.find_comments(
+            "This is the main Apache server configuration file. "
+        )
+        self.assertEqual(len(rootcomment), 1)
+        self.assertTrue(rootcomment[0].filepath.endswith(
+            "debian_apache_2_4/multiple_vhosts/apache2/apache2.conf"
+        ))
+
+    def test_set_parameters(self):
+        servernames = self.config.parser_root.find_directives("servername")
+        names = []  # type: List[str]
+        for servername in servernames:
+            names += servername.parameters
+        self.assertFalse("going_to_set_this" in names)
+        servernames[0].set_parameters(["something", "going_to_set_this"])
+        servernames = self.config.parser_root.find_directives("servername")
+        names = []
+        for servername in servernames:
+            names += servername.parameters
+        self.assertTrue("going_to_set_this" in names)
+
+    def test_set_parameters_atinit(self):
+        from certbot_apache._internal.augeasparser import AugeasDirectiveNode
+        servernames = self.config.parser_root.find_directives("servername")
+        setparam = "certbot_apache._internal.augeasparser.AugeasDirectiveNode.set_parameters"
+        with mock.patch(setparam) as mock_set:
+            AugeasDirectiveNode(
+                name=servernames[0].name,
+                parameters=["test", "setting", "these"],
+                ancestor=assertions.PASS,
+                metadata=servernames[0].primary.metadata
+            )
+            self.assertTrue(mock_set.called)
+            self.assertEqual(
+                mock_set.call_args_list[0][0][0],
+                ["test", "setting", "these"]
+            )
+
+    def test_set_parameters_delete(self):
+        # Set params
+        servername = self.config.parser_root.find_directives("servername")[0]
+        servername.set_parameters(["thisshouldnotexistpreviously", "another",
+                                   "third"])
+
+        # Delete params
+        servernames = self.config.parser_root.find_directives("servername")
+        found = False
+        for servername in servernames:
+            if "thisshouldnotexistpreviously" in servername.parameters:
+                self.assertEqual(len(servername.parameters), 3)
+                servername.set_parameters(["thisshouldnotexistpreviously"])
+                found = True
+        self.assertTrue(found)
+
+        # Verify params
+        servernames = self.config.parser_root.find_directives("servername")
+        found = False
+        for servername in servernames:
+            if "thisshouldnotexistpreviously" in servername.parameters:
+                self.assertEqual(len(servername.parameters), 1)
+                servername.set_parameters(["thisshouldnotexistpreviously"])
+                found = True
+        self.assertTrue(found)
+
+    def test_add_child_comment(self):
+        newc = self.config.parser_root.primary.add_child_comment("The content")
+        comments = self.config.parser_root.find_comments("The content")
+        self.assertEqual(len(comments), 1)
+        self.assertEqual(
+            newc.metadata["augeaspath"],
+            comments[0].primary.metadata["augeaspath"]
+        )
+        self.assertEqual(newc.comment, comments[0].comment)
+
+    def test_delete_child(self):
+        listens = self.config.parser_root.primary.find_directives("Listen")
+        self.assertEqual(len(listens), 1)
+        self.config.parser_root.primary.delete_child(listens[0])
+
+        listens = self.config.parser_root.primary.find_directives("Listen")
+        self.assertEqual(len(listens), 0)
+
+    def test_delete_child_not_found(self):
+        listen = self.config.parser_root.find_directives("Listen")[0]
+        listen.primary.metadata["augeaspath"] = "/files/something/nonexistent"
+
+        self.assertRaises(
+            errors.PluginError,
+            self.config.parser_root.delete_child,
+            listen
+        )
+
+    def test_add_child_block(self):
+        nb = self.config.parser_root.add_child_block(
+            "NewBlock",
+            ["first", "second"]
+        )
+        rpath, _, directive = nb.primary.metadata["augeaspath"].rpartition("/")
+        self.assertEqual(
+            rpath,
+            self.config.parser_root.primary.metadata["augeaspath"]
+        )
+        self.assertTrue(directive.startswith("NewBlock"))
+
+    def test_add_child_block_beginning(self):
+        self.config.parser_root.add_child_block(
+            "Beginning",
+            position=0
+        )
+        parser = self.config.parser_root.primary.parser
+        root_path = self.config.parser_root.primary.metadata["augeaspath"]
+        # Get first child
+        first = parser.aug.match("{}/*[1]".format(root_path))
+        self.assertTrue(first[0].endswith("Beginning"))
+
+    def test_add_child_block_append(self):
+        self.config.parser_root.add_child_block(
+            "VeryLast",
+        )
+        parser = self.config.parser_root.primary.parser
+        root_path = self.config.parser_root.primary.metadata["augeaspath"]
+        # Get last child
+        last = parser.aug.match("{}/*[last()]".format(root_path))
+        self.assertTrue(last[0].endswith("VeryLast"))
+
+    def test_add_child_block_append_alt(self):
+        self.config.parser_root.add_child_block(
+            "VeryLastAlt",
+            position=99999
+        )
+        parser = self.config.parser_root.primary.parser
+        root_path = self.config.parser_root.primary.metadata["augeaspath"]
+        # Get last child
+        last = parser.aug.match("{}/*[last()]".format(root_path))
+        self.assertTrue(last[0].endswith("VeryLastAlt"))
+
+    def test_add_child_block_middle(self):
+        self.config.parser_root.add_child_block(
+            "Middle",
+            position=5
+        )
+        parser = self.config.parser_root.primary.parser
+        root_path = self.config.parser_root.primary.metadata["augeaspath"]
+        # Augeas indices start at 1 :(
+        middle = parser.aug.match("{}/*[6]".format(root_path))
+        self.assertTrue(middle[0].endswith("Middle"))
+
+    def test_add_child_block_existing_name(self):
+        parser = self.config.parser_root.primary.parser
+        root_path = self.config.parser_root.primary.metadata["augeaspath"]
+        # There already exists a single VirtualHost in the base config
+        new_block = parser.aug.match("{}/VirtualHost[2]".format(root_path))
+        self.assertEqual(len(new_block), 0)
+        vh = self.config.parser_root.add_child_block(
+            "VirtualHost",
+        )
+        new_block = parser.aug.match("{}/VirtualHost[2]".format(root_path))
+        self.assertEqual(len(new_block), 1)
+        self.assertTrue(vh.primary.metadata["augeaspath"].endswith("VirtualHost[2]"))
+
+    def test_node_init_error_bad_augeaspath(self):
+        from certbot_apache._internal.augeasparser import AugeasBlockNode
+        parameters = {
+            "name": assertions.PASS,
+            "ancestor": None,
+            "filepath": assertions.PASS,
+            "metadata": {
+                "augeasparser": mock.Mock(),
+                "augeaspath": "/files/path/endswith/slash/"
+            }
+        }
+        self.assertRaises(
+            errors.PluginError,
+            AugeasBlockNode,
+            **parameters
+        )
+
+    def test_node_init_error_missing_augeaspath(self):
+        from certbot_apache._internal.augeasparser import AugeasBlockNode
+        parameters = {
+            "name": assertions.PASS,
+            "ancestor": None,
+            "filepath": assertions.PASS,
+            "metadata": {
+                "augeasparser": mock.Mock(),
+            }
+        }
+        self.assertRaises(
+            errors.PluginError,
+            AugeasBlockNode,
+            **parameters
+        )
+
+    def test_add_child_directive(self):
+        self.config.parser_root.add_child_directive(
+            "ThisWasAdded",
+            ["with", "parameters"],
+            position=0
+        )
+        dirs = self.config.parser_root.find_directives("ThisWasAdded")
+        self.assertEqual(len(dirs), 1)
+        self.assertEqual(dirs[0].parameters, ("with", "parameters"))
+        # The new directive was added to the very first line of the config
+        self.assertTrue(dirs[0].primary.metadata["augeaspath"].endswith("[1]"))
+
+    def test_add_child_directive_exception(self):
+        self.assertRaises(
+            errors.PluginError,
+            self.config.parser_root.add_child_directive,
+            "ThisRaisesErrorBecauseMissingParameters"
+        )
+
+    def test_parsed_paths(self):
+        paths = self.config.parser_root.parsed_paths()
+        self.assertEqual(len(paths), 6)
+
+    def test_find_ancestors(self):
+        vhsblocks = self.config.parser_root.find_blocks("VirtualHost")
+        macro_test = False
+        nonmacro_test = False
+        for vh in vhsblocks:
+            if "/macro/" in vh.metadata["augeaspath"].lower():
+                ancs = vh.find_ancestors("Macro")
+                self.assertEqual(len(ancs), 1)
+                macro_test = True
+            else:
+                ancs = vh.find_ancestors("Macro")
+                self.assertEqual(len(ancs), 0)
+                nonmacro_test = True
+        self.assertTrue(macro_test)
+        self.assertTrue(nonmacro_test)
+
+    def test_find_ancestors_bad_path(self):
+        self.config.parser_root.primary.metadata["augeaspath"] = ""
+        ancs = self.config.parser_root.primary.find_ancestors("Anything")
+        self.assertEqual(len(ancs), 0)

certbot-apache/tests/centos_test.py

@@ -106,7 +106,7 @@ class MultipleVhostsTestCentOS(util.ApacheTest):
     def test_get_parser(self):
         self.assertIsInstance(self.config.parser, override_centos.CentOSParser)
 
-    @mock.patch("certbot_apache._internal.parser.ApacheParser._get_runtime_cfg")
+    @mock.patch("certbot_apache._internal.apache_util._get_runtime_cfg")
     def test_opportunistic_httpd_runtime_parsing(self, mock_get):
         define_val = (
             'Define: TEST1\n'
@@ -155,7 +155,7 @@ class MultipleVhostsTestCentOS(util.ApacheTest):
                 raise Exception("Missed: %s" % vhost)  # pragma: no cover
         self.assertEqual(found, 2)
 
-    @mock.patch("certbot_apache._internal.parser.ApacheParser._get_runtime_cfg")
+    @mock.patch("certbot_apache._internal.apache_util._get_runtime_cfg")
     def test_get_sysconfig_vars(self, mock_cfg):
         """Make sure we read the sysconfig OPTIONS variable correctly"""
         # Return nothing for the process calls

certbot-apache/tests/configurator_test.py

@@ -75,7 +75,8 @@ class MultipleVhostsTest(util.ApacheTest):
 
     @mock.patch("certbot_apache._internal.parser.ApacheParser")
     @mock.patch("certbot_apache._internal.configurator.util.exe_exists")
-    def _test_prepare_locked(self, unused_parser, unused_exe_exists):
+    @mock.patch("certbot_apache._internal.configurator.ApacheConfigurator.get_parsernode_root")
+    def _test_prepare_locked(self, _node, _exists, _parser):
         try:
             self.config.prepare()
         except errors.PluginError as err:
@@ -799,7 +800,7 @@ class MultipleVhostsTest(util.ApacheTest):
         self.assertEqual(mock_restart.call_count, 1)
 
     @mock.patch("certbot_apache._internal.configurator.ApacheConfigurator.restart")
-    @mock.patch("certbot_apache._internal.parser.ApacheParser._get_runtime_cfg")
+    @mock.patch("certbot_apache._internal.apache_util._get_runtime_cfg")
     def test_cleanup(self, mock_cfg, mock_restart):
         mock_cfg.return_value = ""
         _, achalls = self.get_key_and_achalls()
@@ -815,7 +816,7 @@ class MultipleVhostsTest(util.ApacheTest):
                 self.assertFalse(mock_restart.called)
 
     @mock.patch("certbot_apache._internal.configurator.ApacheConfigurator.restart")
-    @mock.patch("certbot_apache._internal.parser.ApacheParser._get_runtime_cfg")
+    @mock.patch("certbot_apache._internal.apache_util._get_runtime_cfg")
     def test_cleanup_no_errors(self, mock_cfg, mock_restart):
         mock_cfg.return_value = ""
         _, achalls = self.get_key_and_achalls()

certbot-apache/tests/debian_test.py

@@ -46,7 +46,7 @@ class MultipleVhostsTestDebian(util.ApacheTest):
 
     @mock.patch("certbot.util.run_script")
     @mock.patch("certbot.util.exe_exists")
-    @mock.patch("certbot_apache._internal.parser.subprocess.Popen")
+    @mock.patch("certbot_apache._internal.apache_util.subprocess.Popen")
     def test_enable_mod(self, mock_popen, mock_exe_exists, mock_run_script):
         mock_popen().communicate.return_value = ("Define: DUMP_RUN_CFG", "")
         mock_popen().returncode = 0

certbot-apache/tests/dualnode_test.py

@@ -0,0 +1,442 @@
+"""Tests for DualParserNode implementation"""
+import unittest
+
+import mock
+
+from certbot_apache._internal import assertions
+from certbot_apache._internal import augeasparser
+from certbot_apache._internal import dualparser
+
+
+class DualParserNodeTest(unittest.TestCase):  # pylint: disable=too-many-public-methods
+    """DualParserNode tests"""
+
+    def setUp(self):  # pylint: disable=arguments-differ
+        parser_mock = mock.MagicMock()
+        parser_mock.aug.match.return_value = []
+        parser_mock.get_arg.return_value = []
+        self.metadata = {"augeasparser": parser_mock, "augeaspath": "/invalid", "ac_ast": None}
+        self.block = dualparser.DualBlockNode(name="block",
+                                              ancestor=None,
+                                              filepath="/tmp/something",
+                                              metadata=self.metadata)
+        self.block_two = dualparser.DualBlockNode(name="block",
+                                                  ancestor=self.block,
+                                                  filepath="/tmp/something",
+                                                  metadata=self.metadata)
+        self.directive = dualparser.DualDirectiveNode(name="directive",
+                                                      ancestor=self.block,
+                                                      filepath="/tmp/something",
+                                                      metadata=self.metadata)
+        self.comment = dualparser.DualCommentNode(comment="comment",
+                                                  ancestor=self.block,
+                                                  filepath="/tmp/something",
+                                                  metadata=self.metadata)
+
+    def test_create_with_precreated(self):
+        cnode = dualparser.DualCommentNode(comment="comment",
+                                           ancestor=self.block,
+                                           filepath="/tmp/something",
+                                           primary=self.comment.secondary,
+                                           secondary=self.comment.primary)
+        dnode = dualparser.DualDirectiveNode(name="directive",
+                                             ancestor=self.block,
+                                             filepath="/tmp/something",
+                                             primary=self.directive.secondary,
+                                             secondary=self.directive.primary)
+        bnode = dualparser.DualBlockNode(name="block",
+                                         ancestor=self.block,
+                                         filepath="/tmp/something",
+                                         primary=self.block.secondary,
+                                         secondary=self.block.primary)
+        # Switched around
+        self.assertTrue(cnode.primary is self.comment.secondary)
+        self.assertTrue(cnode.secondary is self.comment.primary)
+        self.assertTrue(dnode.primary is self.directive.secondary)
+        self.assertTrue(dnode.secondary is self.directive.primary)
+        self.assertTrue(bnode.primary is self.block.secondary)
+        self.assertTrue(bnode.secondary is self.block.primary)
+
+    def test_set_params(self):
+        params = ("first", "second")
+        self.directive.primary.set_parameters = mock.Mock()
+        self.directive.secondary.set_parameters = mock.Mock()
+        self.directive.set_parameters(params)
+        self.assertTrue(self.directive.primary.set_parameters.called)
+        self.assertTrue(self.directive.secondary.set_parameters.called)
+
+    def test_set_parameters(self):
+        pparams = mock.MagicMock()
+        sparams = mock.MagicMock()
+        pparams.parameters = ("a", "b")
+        sparams.parameters = ("a", "b")
+        self.directive.primary.set_parameters = pparams
+        self.directive.secondary.set_parameters = sparams
+        self.directive.set_parameters(("param", "seq"))
+        self.assertTrue(pparams.called)
+        self.assertTrue(sparams.called)
+
+    def test_delete_child(self):
+        pdel = mock.MagicMock()
+        sdel = mock.MagicMock()
+        self.block.primary.delete_child = pdel
+        self.block.secondary.delete_child = sdel
+        self.block.delete_child(self.comment)
+        self.assertTrue(pdel.called)
+        self.assertTrue(sdel.called)
+
+    def test_unsaved_files(self):
+        puns = mock.MagicMock()
+        suns = mock.MagicMock()
+        puns.return_value = assertions.PASS
+        suns.return_value = assertions.PASS
+        self.block.primary.unsaved_files = puns
+        self.block.secondary.unsaved_files = suns
+        self.block.unsaved_files()
+        self.assertTrue(puns.called)
+        self.assertTrue(suns.called)
+
+    def test_getattr_equality(self):
+        self.directive.primary.variableexception = "value"
+        self.directive.secondary.variableexception = "not_value"
+        with self.assertRaises(AssertionError):
+            _ = self.directive.variableexception
+
+        self.directive.primary.variable = "value"
+        self.directive.secondary.variable = "value"
+        try:
+            self.directive.variable
+        except AssertionError: # pragma: no cover
+            self.fail("getattr check raised an AssertionError where it shouldn't have")
+
+    def test_parsernode_dirty_assert(self):
+        # disable assertion pass
+        self.comment.primary.comment = "value"
+        self.comment.secondary.comment = "value"
+        self.comment.primary.filepath = "x"
+        self.comment.secondary.filepath = "x"
+
+        self.comment.primary.dirty = False
+        self.comment.secondary.dirty = True
+        with self.assertRaises(AssertionError):
+            assertions.assertEqual(self.comment.primary, self.comment.secondary)
+
+    def test_parsernode_filepath_assert(self):
+        # disable assertion pass
+        self.comment.primary.comment = "value"
+        self.comment.secondary.comment = "value"
+
+        self.comment.primary.filepath = "first"
+        self.comment.secondary.filepath = "second"
+        with self.assertRaises(AssertionError):
+            assertions.assertEqual(self.comment.primary, self.comment.secondary)
+
+    def test_add_child_block(self):
+        mock_first = mock.MagicMock(return_value=self.block.primary)
+        mock_second = mock.MagicMock(return_value=self.block.secondary)
+        self.block.primary.add_child_block = mock_first
+        self.block.secondary.add_child_block = mock_second
+        self.block.add_child_block("Block")
+        self.assertTrue(mock_first.called)
+        self.assertTrue(mock_second.called)
+
+    def test_add_child_directive(self):
+        mock_first = mock.MagicMock(return_value=self.directive.primary)
+        mock_second = mock.MagicMock(return_value=self.directive.secondary)
+        self.block.primary.add_child_directive = mock_first
+        self.block.secondary.add_child_directive = mock_second
+        self.block.add_child_directive("Directive")
+        self.assertTrue(mock_first.called)
+        self.assertTrue(mock_second.called)
+
+    def test_add_child_comment(self):
+        mock_first = mock.MagicMock(return_value=self.comment.primary)
+        mock_second = mock.MagicMock(return_value=self.comment.secondary)
+        self.block.primary.add_child_comment = mock_first
+        self.block.secondary.add_child_comment = mock_second
+        self.block.add_child_comment("Comment")
+        self.assertTrue(mock_first.called)
+        self.assertTrue(mock_second.called)
+
+    def test_find_comments(self):
+        pri_comments = [augeasparser.AugeasCommentNode(comment="some comment",
+                                                       ancestor=self.block,
+                                                       filepath="/path/to/whatever",
+                                                       metadata=self.metadata)]
+        sec_comments = [augeasparser.AugeasCommentNode(comment=assertions.PASS,
+                                                       ancestor=self.block,
+                                                       filepath=assertions.PASS,
+                                                       metadata=self.metadata)]
+        find_coms_primary = mock.MagicMock(return_value=pri_comments)
+        find_coms_secondary = mock.MagicMock(return_value=sec_comments)
+        self.block.primary.find_comments = find_coms_primary
+        self.block.secondary.find_comments = find_coms_secondary
+
+        dcoms = self.block.find_comments("comment")
+        p_dcoms = [d.primary for d in dcoms]
+        s_dcoms = [d.secondary for d in dcoms]
+        p_coms = self.block.primary.find_comments("comment")
+        s_coms = self.block.secondary.find_comments("comment")
+        # Check that every comment response is represented in the list of
+        # DualParserNode instances.
+        for p in p_dcoms:
+            self.assertTrue(p in p_coms)
+        for s in s_dcoms:
+            self.assertTrue(s in s_coms)
+
+    def test_find_blocks_first_passing(self):
+        youshallnotpass = [augeasparser.AugeasBlockNode(name="notpassing",
+                                                        ancestor=self.block,
+                                                        filepath="/path/to/whatever",
+                                                        metadata=self.metadata)]
+        youshallpass = [augeasparser.AugeasBlockNode(name=assertions.PASS,
+                                                     ancestor=self.block,
+                                                     filepath=assertions.PASS,
+                                                     metadata=self.metadata)]
+        find_blocks_primary = mock.MagicMock(return_value=youshallpass)
+        find_blocks_secondary = mock.MagicMock(return_value=youshallnotpass)
+        self.block.primary.find_blocks = find_blocks_primary
+        self.block.secondary.find_blocks = find_blocks_secondary
+
+        blocks = self.block.find_blocks("something")
+        for block in blocks:
+            try:
+                assertions.assertEqual(block.primary, block.secondary)
+            except AssertionError: # pragma: no cover
+                self.fail("Assertion should have passed")
+            self.assertTrue(assertions.isPassDirective(block.primary))
+            self.assertFalse(assertions.isPassDirective(block.secondary))
+
+    def test_find_blocks_second_passing(self):
+        youshallnotpass = [augeasparser.AugeasBlockNode(name="notpassing",
+                                                        ancestor=self.block,
+                                                        filepath="/path/to/whatever",
+                                                        metadata=self.metadata)]
+        youshallpass = [augeasparser.AugeasBlockNode(name=assertions.PASS,
+                                                     ancestor=self.block,
+                                                     filepath=assertions.PASS,
+                                                     metadata=self.metadata)]
+        find_blocks_primary = mock.MagicMock(return_value=youshallnotpass)
+        find_blocks_secondary = mock.MagicMock(return_value=youshallpass)
+        self.block.primary.find_blocks = find_blocks_primary
+        self.block.secondary.find_blocks = find_blocks_secondary
+
+        blocks = self.block.find_blocks("something")
+        for block in blocks:
+            try:
+                assertions.assertEqual(block.primary, block.secondary)
+            except AssertionError: # pragma: no cover
+                self.fail("Assertion should have passed")
+            self.assertFalse(assertions.isPassDirective(block.primary))
+            self.assertTrue(assertions.isPassDirective(block.secondary))
+
+    def test_find_dirs_first_passing(self):
+        notpassing = [augeasparser.AugeasDirectiveNode(name="notpassing",
+                                                       ancestor=self.block,
+                                                       filepath="/path/to/whatever",
+                                                       metadata=self.metadata)]
+        passing = [augeasparser.AugeasDirectiveNode(name=assertions.PASS,
+                                                    ancestor=self.block,
+                                                    filepath=assertions.PASS,
+                                                    metadata=self.metadata)]
+        find_dirs_primary = mock.MagicMock(return_value=passing)
+        find_dirs_secondary = mock.MagicMock(return_value=notpassing)
+        self.block.primary.find_directives = find_dirs_primary
+        self.block.secondary.find_directives = find_dirs_secondary
+
+        directives = self.block.find_directives("something")
+        for directive in directives:
+            try:
+                assertions.assertEqual(directive.primary, directive.secondary)
+            except AssertionError: # pragma: no cover
+                self.fail("Assertion should have passed")
+            self.assertTrue(assertions.isPassDirective(directive.primary))
+            self.assertFalse(assertions.isPassDirective(directive.secondary))
+
+    def test_find_dirs_second_passing(self):
+        notpassing = [augeasparser.AugeasDirectiveNode(name="notpassing",
+                                                       ancestor=self.block,
+                                                       filepath="/path/to/whatever",
+                                                       metadata=self.metadata)]
+        passing = [augeasparser.AugeasDirectiveNode(name=assertions.PASS,
+                                                    ancestor=self.block,
+                                                    filepath=assertions.PASS,
+                                                    metadata=self.metadata)]
+        find_dirs_primary = mock.MagicMock(return_value=notpassing)
+        find_dirs_secondary = mock.MagicMock(return_value=passing)
+        self.block.primary.find_directives = find_dirs_primary
+        self.block.secondary.find_directives = find_dirs_secondary
+
+        directives = self.block.find_directives("something")
+        for directive in directives:
+            try:
+                assertions.assertEqual(directive.primary, directive.secondary)
+            except AssertionError: # pragma: no cover
+                self.fail("Assertion should have passed")
+            self.assertFalse(assertions.isPassDirective(directive.primary))
+            self.assertTrue(assertions.isPassDirective(directive.secondary))
+
+    def test_find_coms_first_passing(self):
+        notpassing = [augeasparser.AugeasCommentNode(comment="notpassing",
+                                                     ancestor=self.block,
+                                                     filepath="/path/to/whatever",
+                                                     metadata=self.metadata)]
+        passing = [augeasparser.AugeasCommentNode(comment=assertions.PASS,
+                                                  ancestor=self.block,
+                                                  filepath=assertions.PASS,
+                                                  metadata=self.metadata)]
+        find_coms_primary = mock.MagicMock(return_value=passing)
+        find_coms_secondary = mock.MagicMock(return_value=notpassing)
+        self.block.primary.find_comments = find_coms_primary
+        self.block.secondary.find_comments = find_coms_secondary
+
+        comments = self.block.find_comments("something")
+        for comment in comments:
+            try:
+                assertions.assertEqual(comment.primary, comment.secondary)
+            except AssertionError: # pragma: no cover
+                self.fail("Assertion should have passed")
+            self.assertTrue(assertions.isPassComment(comment.primary))
+            self.assertFalse(assertions.isPassComment(comment.secondary))
+
+    def test_find_coms_second_passing(self):
+        notpassing = [augeasparser.AugeasCommentNode(comment="notpassing",
+                                                     ancestor=self.block,
+                                                     filepath="/path/to/whatever",
+                                                     metadata=self.metadata)]
+        passing = [augeasparser.AugeasCommentNode(comment=assertions.PASS,
+                                                  ancestor=self.block,
+                                                  filepath=assertions.PASS,
+                                                  metadata=self.metadata)]
+        find_coms_primary = mock.MagicMock(return_value=notpassing)
+        find_coms_secondary = mock.MagicMock(return_value=passing)
+        self.block.primary.find_comments = find_coms_primary
+        self.block.secondary.find_comments = find_coms_secondary
+
+        comments = self.block.find_comments("something")
+        for comment in comments:
+            try:
+                assertions.assertEqual(comment.primary, comment.secondary)
+            except AssertionError: # pragma: no cover
+                self.fail("Assertion should have passed")
+            self.assertFalse(assertions.isPassComment(comment.primary))
+            self.assertTrue(assertions.isPassComment(comment.secondary))
+
+    def test_find_blocks_no_pass_equal(self):
+        notpassing1 = [augeasparser.AugeasBlockNode(name="notpassing",
+                                                    ancestor=self.block,
+                                                    filepath="/path/to/whatever",
+                                                    metadata=self.metadata)]
+        notpassing2 = [augeasparser.AugeasBlockNode(name="notpassing",
+                                                    ancestor=self.block,
+                                                    filepath="/path/to/whatever",
+                                                    metadata=self.metadata)]
+        find_blocks_primary = mock.MagicMock(return_value=notpassing1)
+        find_blocks_secondary = mock.MagicMock(return_value=notpassing2)
+        self.block.primary.find_blocks = find_blocks_primary
+        self.block.secondary.find_blocks = find_blocks_secondary
+
+        blocks = self.block.find_blocks("anything")
+        for block in blocks:
+            self.assertEqual(block.primary, block.secondary)
+            self.assertTrue(block.primary is not block.secondary)
+
+    def test_find_dirs_no_pass_equal(self):
+        notpassing1 = [augeasparser.AugeasDirectiveNode(name="notpassing",
+                                                        ancestor=self.block,
+                                                        filepath="/path/to/whatever",
+                                                        metadata=self.metadata)]
+        notpassing2 = [augeasparser.AugeasDirectiveNode(name="notpassing",
+                                                        ancestor=self.block,
+                                                        filepath="/path/to/whatever",
+                                                        metadata=self.metadata)]
+        find_dirs_primary = mock.MagicMock(return_value=notpassing1)
+        find_dirs_secondary = mock.MagicMock(return_value=notpassing2)
+        self.block.primary.find_directives = find_dirs_primary
+        self.block.secondary.find_directives = find_dirs_secondary
+
+        directives = self.block.find_directives("anything")
+        for directive in directives:
+            self.assertEqual(directive.primary, directive.secondary)
+            self.assertTrue(directive.primary is not directive.secondary)
+
+    def test_find_comments_no_pass_equal(self):
+        notpassing1 = [augeasparser.AugeasCommentNode(comment="notpassing",
+                                                      ancestor=self.block,
+                                                      filepath="/path/to/whatever",
+                                                      metadata=self.metadata)]
+        notpassing2 = [augeasparser.AugeasCommentNode(comment="notpassing",
+                                                      ancestor=self.block,
+                                                      filepath="/path/to/whatever",
+                                                      metadata=self.metadata)]
+        find_coms_primary = mock.MagicMock(return_value=notpassing1)
+        find_coms_secondary = mock.MagicMock(return_value=notpassing2)
+        self.block.primary.find_comments = find_coms_primary
+        self.block.secondary.find_comments = find_coms_secondary
+
+        comments = self.block.find_comments("anything")
+        for comment in comments:
+            self.assertEqual(comment.primary, comment.secondary)
+            self.assertTrue(comment.primary is not comment.secondary)
+
+    def test_find_blocks_no_pass_notequal(self):
+        notpassing1 = [augeasparser.AugeasBlockNode(name="notpassing",
+                                                    ancestor=self.block,
+                                                    filepath="/path/to/whatever",
+                                                    metadata=self.metadata)]
+        notpassing2 = [augeasparser.AugeasBlockNode(name="different",
+                                                    ancestor=self.block,
+                                                    filepath="/path/to/whatever",
+                                                    metadata=self.metadata)]
+        find_blocks_primary = mock.MagicMock(return_value=notpassing1)
+        find_blocks_secondary = mock.MagicMock(return_value=notpassing2)
+        self.block.primary.find_blocks = find_blocks_primary
+        self.block.secondary.find_blocks = find_blocks_secondary
+
+        with self.assertRaises(AssertionError):
+            _ = self.block.find_blocks("anything")
+
+    def test_parsernode_notequal(self):
+        ne_block = augeasparser.AugeasBlockNode(name="different",
+                                                ancestor=self.block,
+                                                filepath="/path/to/whatever",
+                                                metadata=self.metadata)
+        ne_directive = augeasparser.AugeasDirectiveNode(name="different",
+                                                        ancestor=self.block,
+                                                        filepath="/path/to/whatever",
+                                                        metadata=self.metadata)
+        ne_comment = augeasparser.AugeasCommentNode(comment="different",
+                                                    ancestor=self.block,
+                                                    filepath="/path/to/whatever",
+                                                    metadata=self.metadata)
+        self.assertFalse(self.block == ne_block)
+        self.assertFalse(self.directive == ne_directive)
+        self.assertFalse(self.comment == ne_comment)
+
+    def test_parsed_paths(self):
+        mock_p = mock.MagicMock(return_value=['/path/file.conf',
+                                              '/another/path',
+                                              '/path/other.conf'])
+        mock_s = mock.MagicMock(return_value=['/path/*.conf', '/another/path'])
+        self.block.primary.parsed_paths = mock_p
+        self.block.secondary.parsed_paths = mock_s
+        self.block.parsed_paths()
+        self.assertTrue(mock_p.called)
+        self.assertTrue(mock_s.called)
+
+    def test_parsed_paths_error(self):
+        mock_p = mock.MagicMock(return_value=['/path/file.conf'])
+        mock_s = mock.MagicMock(return_value=['/path/*.conf', '/another/path'])
+        self.block.primary.parsed_paths = mock_p
+        self.block.secondary.parsed_paths = mock_s
+        with self.assertRaises(AssertionError):
+            self.block.parsed_paths()
+
+    def test_find_ancestors(self):
+        primarymock = mock.MagicMock(return_value=[])
+        secondarymock = mock.MagicMock(return_value=[])
+        self.block.primary.find_ancestors = primarymock
+        self.block.secondary.find_ancestors = secondarymock
+        self.block.find_ancestors("anything")
+        self.assertTrue(primarymock.called)
+        self.assertTrue(secondarymock.called)

certbot-apache/tests/fedora_test.py

@@ -100,7 +100,7 @@ class MultipleVhostsTestFedora(util.ApacheTest):
     def test_get_parser(self):
         self.assertIsInstance(self.config.parser, override_fedora.FedoraParser)
 
-    @mock.patch("certbot_apache._internal.parser.ApacheParser._get_runtime_cfg")
+    @mock.patch("certbot_apache._internal.apache_util._get_runtime_cfg")
     def test_opportunistic_httpd_runtime_parsing(self, mock_get):
         define_val = (
             'Define: TEST1\n'
@@ -155,7 +155,7 @@ class MultipleVhostsTestFedora(util.ApacheTest):
                 raise Exception("Missed: %s" % vhost)  # pragma: no cover
         self.assertEqual(found, 2)
 
-    @mock.patch("certbot_apache._internal.parser.ApacheParser._get_runtime_cfg")
+    @mock.patch("certbot_apache._internal.apache_util._get_runtime_cfg")
     def test_get_sysconfig_vars(self, mock_cfg):
         """Make sure we read the sysconfig OPTIONS variable correctly"""
         # Return nothing for the process calls

certbot-apache/tests/gentoo_test.py

@@ -90,7 +90,7 @@ class MultipleVhostsTestGentoo(util.ApacheTest):
         for define in defines:
             self.assertTrue(define in self.config.parser.variables.keys())
 
-    @mock.patch("certbot_apache._internal.parser.ApacheParser.parse_from_subprocess")
+    @mock.patch("certbot_apache._internal.apache_util.parse_from_subprocess")
     def test_no_binary_configdump(self, mock_subprocess):
         """Make sure we don't call binary dumps other than modules from Apache
         as this is not supported in Gentoo currently"""
@@ -104,7 +104,7 @@ class MultipleVhostsTestGentoo(util.ApacheTest):
         self.config.parser.reset_modules()
         self.assertTrue(mock_subprocess.called)
 
-    @mock.patch("certbot_apache._internal.parser.ApacheParser._get_runtime_cfg")
+    @mock.patch("certbot_apache._internal.apache_util._get_runtime_cfg")
     def test_opportunistic_httpd_runtime_parsing(self, mock_get):
         mod_val = (
             'Loaded Modules:\n'

certbot-apache/tests/parser_test.py

@@ -165,7 +165,7 @@ class BasicParserTest(util.ParserTest):
             self.assertTrue(mock_logger.debug.called)
 
     @mock.patch("certbot_apache._internal.parser.ApacheParser.find_dir")
-    @mock.patch("certbot_apache._internal.parser.ApacheParser._get_runtime_cfg")
+    @mock.patch("certbot_apache._internal.apache_util._get_runtime_cfg")
     def test_update_runtime_variables(self, mock_cfg, _):
         define_val = (
             'ServerRoot: "/etc/apache2"\n'
@@ -271,7 +271,7 @@ class BasicParserTest(util.ParserTest):
             self.assertEqual(mock_parse.call_count, 25)
 
     @mock.patch("certbot_apache._internal.parser.ApacheParser.find_dir")
-    @mock.patch("certbot_apache._internal.parser.ApacheParser._get_runtime_cfg")
+    @mock.patch("certbot_apache._internal.apache_util._get_runtime_cfg")
     def test_update_runtime_variables_alt_values(self, mock_cfg, _):
         inc_val = (
             'Included configuration files:\n'
@@ -293,7 +293,7 @@ class BasicParserTest(util.ParserTest):
             # path derived from root configuration Include statements
             self.assertEqual(mock_parse.call_count, 1)
 
-    @mock.patch("certbot_apache._internal.parser.ApacheParser._get_runtime_cfg")
+    @mock.patch("certbot_apache._internal.apache_util._get_runtime_cfg")
     def test_update_runtime_vars_bad_output(self, mock_cfg):
         mock_cfg.return_value = "Define: TLS=443=24"
         self.parser.update_runtime_variables()
@@ -303,7 +303,7 @@ class BasicParserTest(util.ParserTest):
             errors.PluginError, self.parser.update_runtime_variables)
 
     @mock.patch("certbot_apache._internal.configurator.ApacheConfigurator.option")
-    @mock.patch("certbot_apache._internal.parser.subprocess.Popen")
+    @mock.patch("certbot_apache._internal.apache_util.subprocess.Popen")
     def test_update_runtime_vars_bad_ctl(self, mock_popen, mock_opt):
         mock_popen.side_effect = OSError
         mock_opt.return_value = "nonexistent"
@@ -311,7 +311,7 @@ class BasicParserTest(util.ParserTest):
             errors.MisconfigurationError,
             self.parser.update_runtime_variables)
 
-    @mock.patch("certbot_apache._internal.parser.subprocess.Popen")
+    @mock.patch("certbot_apache._internal.apache_util.subprocess.Popen")
     def test_update_runtime_vars_bad_exit(self, mock_popen):
         mock_popen().communicate.return_value = ("", "")
         mock_popen.returncode = -1
@@ -355,7 +355,7 @@ class ParserInitTest(util.ApacheTest):
                 ApacheParser, os.path.relpath(self.config_path),
                 "/dummy/vhostpath", version=(2, 4, 22), configurator=self.config)
 
-    @mock.patch("certbot_apache._internal.parser.ApacheParser._get_runtime_cfg")
+    @mock.patch("certbot_apache._internal.apache_util._get_runtime_cfg")
     def test_unparseable(self, mock_cfg):
         from certbot_apache._internal.parser import ApacheParser
         mock_cfg.return_value = ('Define: TEST')

certbot-apache/tests/parsernode_configurator_test.py

@@ -0,0 +1,37 @@
+"""Tests for ApacheConfigurator for AugeasParserNode classes"""
+import unittest
+
+import mock
+
+import util
+
+
+class ConfiguratorParserNodeTest(util.ApacheTest):  # pylint: disable=too-many-public-methods
+    """Test AugeasParserNode using available test configurations"""
+
+    def setUp(self):  # pylint: disable=arguments-differ
+        super(ConfiguratorParserNodeTest, self).setUp()
+
+        self.config = util.get_apache_configurator(
+            self.config_path, self.vhost_path, self.config_dir,
+            self.work_dir, use_parsernode=True)
+        self.vh_truth = util.get_vh_truth(
+            self.temp_dir, "debian_apache_2_4/multiple_vhosts")
+
+    def test_parsernode_get_vhosts(self):
+        self.config.USE_PARSERNODE = True
+        vhosts = self.config.get_virtual_hosts()
+        # Legacy get_virtual_hosts() do not set the node
+        self.assertTrue(vhosts[0].node is not None)
+
+    def test_parsernode_get_vhosts_mismatch(self):
+        vhosts = self.config.get_virtual_hosts_v2()
+        # One of the returned VirtualHost objects differs
+        vhosts[0].name = "IdidntExpectThat"
+        self.config.get_virtual_hosts_v2 = mock.MagicMock(return_value=vhosts)
+        with self.assertRaises(AssertionError):
+            _ = self.config.get_virtual_hosts()
+
+
+if __name__ == "__main__":
+    unittest.main()  # pragma: no cover

certbot-apache/tests/parsernode_test.py

@@ -0,0 +1,128 @@
+""" Tests for ParserNode interface """
+
+import unittest
+
+from certbot_apache._internal import interfaces
+from certbot_apache._internal import parsernode_util as util
+
+
+class DummyParserNode(interfaces.ParserNode):
+    """ A dummy class implementing ParserNode interface """
+
+    def __init__(self, **kwargs):
+        """
+        Initializes the ParserNode instance.
+        """
+        ancestor, dirty, filepath, metadata = util.parsernode_kwargs(kwargs)
+        self.ancestor = ancestor
+        self.dirty = dirty
+        self.filepath = filepath
+        self.metadata = metadata
+        super(DummyParserNode, self).__init__(**kwargs)
+
+    def save(self, msg):  # pragma: no cover
+        """Save"""
+        pass
+
+    def find_ancestors(self, name):  # pragma: no cover
+        """ Find ancestors """
+        return []
+
+
+class DummyCommentNode(DummyParserNode):
+    """ A dummy class implementing CommentNode interface """
+
+    def __init__(self, **kwargs):
+        """
+        Initializes the CommentNode instance and sets its instance variables.
+        """
+        comment, kwargs = util.commentnode_kwargs(kwargs)
+        self.comment = comment
+        super(DummyCommentNode, self).__init__(**kwargs)
+
+
+class DummyDirectiveNode(DummyParserNode):
+    """ A dummy class implementing DirectiveNode interface """
+
+    # pylint: disable=too-many-arguments
+    def __init__(self, **kwargs):
+        """
+        Initializes the DirectiveNode instance and sets its instance variables.
+        """
+        name, parameters, enabled, kwargs = util.directivenode_kwargs(kwargs)
+        self.name = name
+        self.parameters = parameters
+        self.enabled = enabled
+
+        super(DummyDirectiveNode, self).__init__(**kwargs)
+
+    def set_parameters(self, parameters):  # pragma: no cover
+        """Set parameters"""
+        pass
+
+
+class DummyBlockNode(DummyDirectiveNode):
+    """ A dummy class implementing BlockNode interface """
+
+    def add_child_block(self, name, parameters=None, position=None):  # pragma: no cover
+        """Add child block"""
+        pass
+
+    def add_child_directive(self, name, parameters=None, position=None):  # pragma: no cover
+        """Add child directive"""
+        pass
+
+    def add_child_comment(self, comment="", position=None):  # pragma: no cover
+        """Add child comment"""
+        pass
+
+    def find_blocks(self, name, exclude=True):  # pragma: no cover
+        """Find blocks"""
+        pass
+
+    def find_directives(self, name, exclude=True):  # pragma: no cover
+        """Find directives"""
+        pass
+
+    def find_comments(self, comment, exact=False):  # pragma: no cover
+        """Find comments"""
+        pass
+
+    def delete_child(self, child):  # pragma: no cover
+        """Delete child"""
+        pass
+
+    def unsaved_files(self):  # pragma: no cover
+        """Unsaved files"""
+        pass
+
+
+interfaces.CommentNode.register(DummyCommentNode)
+interfaces.DirectiveNode.register(DummyDirectiveNode)
+interfaces.BlockNode.register(DummyBlockNode)
+
+class ParserNodeTest(unittest.TestCase):
+    """Dummy placeholder test case for ParserNode interfaces"""
+
+    def test_dummy(self):
+        dummyblock = DummyBlockNode(
+            name="None",
+            parameters=(),
+            ancestor=None,
+            dirty=False,
+            filepath="/some/random/path"
+        )
+        dummydirective = DummyDirectiveNode(
+            name="Name",
+            ancestor=None,
+            filepath="/another/path"
+        )
+        dummycomment = DummyCommentNode(
+            comment="Comment",
+            ancestor=dummyblock,
+            filepath="/some/file"
+        )
+
+
+if __name__ == "__main__":
+    unittest.main()  # pragma: no cover

certbot-apache/tests/parsernode_util_test.py

@@ -0,0 +1,115 @@
+""" Tests for ParserNode utils """
+import unittest
+
+from certbot_apache._internal import parsernode_util as util
+
+
+class ParserNodeUtilTest(unittest.TestCase):
+    """Tests for ParserNode utils"""
+
+    def _setup_parsernode(self):
+        """ Sets up kwargs dict for ParserNode """
+        return {
+            "ancestor": None,
+            "dirty": False,
+            "filepath": "/tmp",
+        }
+
+    def _setup_commentnode(self):
+        """ Sets up kwargs dict for CommentNode """
+
+        pn = self._setup_parsernode()
+        pn["comment"] = "x"
+        return pn
+
+    def _setup_directivenode(self):
+        """ Sets up kwargs dict for DirectiveNode """
+
+        pn = self._setup_parsernode()
+        pn["name"] = "Name"
+        pn["parameters"] = ("first",)
+        pn["enabled"] = True
+        return pn
+
+    def test_unknown_parameter(self):
+        params = self._setup_parsernode()
+        params["unknown"] = "unknown"
+        self.assertRaises(TypeError, util.parsernode_kwargs, params)
+
+        params = self._setup_commentnode()
+        params["unknown"] = "unknown"
+        self.assertRaises(TypeError, util.commentnode_kwargs, params)
+
+        params = self._setup_directivenode()
+        params["unknown"] = "unknown"
+        self.assertRaises(TypeError, util.directivenode_kwargs, params)
+
+    def test_parsernode(self):
+        params = self._setup_parsernode()
+        ctrl = self._setup_parsernode()
+
+        ancestor, dirty, filepath, metadata = util.parsernode_kwargs(params)
+        self.assertEqual(ancestor, ctrl["ancestor"])
+        self.assertEqual(dirty, ctrl["dirty"])
+        self.assertEqual(filepath, ctrl["filepath"])
+        self.assertEqual(metadata, {})
+
+    def test_parsernode_from_metadata(self):
+        params = self._setup_parsernode()
+        params.pop("filepath")
+        md = {"some": "value"}
+        params["metadata"] = md
+
+        # Just testing that error from missing required parameters is not raised
+        _, _, _, metadata = util.parsernode_kwargs(params)
+        self.assertEqual(metadata, md)
+
+    def test_commentnode(self):
+        params = self._setup_commentnode()
+        ctrl = self._setup_commentnode()
+
+        comment, _ = util.commentnode_kwargs(params)
+        self.assertEqual(comment, ctrl["comment"])
+
+    def test_commentnode_from_metadata(self):
+        params = self._setup_commentnode()
+        params.pop("comment")
+        params["metadata"] = {}
+
+        # Just testing that error from missing required parameters is not raised
+        util.commentnode_kwargs(params)
+
+    def test_directivenode(self):
+        params = self._setup_directivenode()
+        ctrl = self._setup_directivenode()
+
+        name, parameters, enabled, _ = util.directivenode_kwargs(params)
+        self.assertEqual(name, ctrl["name"])
+        self.assertEqual(parameters, ctrl["parameters"])
+        self.assertEqual(enabled, ctrl["enabled"])
+
+    def test_directivenode_from_metadata(self):
+        params = self._setup_directivenode()
+        params.pop("filepath")
+        params.pop("name")
+        params["metadata"] = {"irrelevant": "value"}
+
+        # Just testing that error from missing required parameters is not raised
+        util.directivenode_kwargs(params)
+
+    def test_missing_required(self):
+        c_params = self._setup_commentnode()
+        c_params.pop("comment")
+        self.assertRaises(TypeError, util.commentnode_kwargs, c_params)
+
+        d_params = self._setup_directivenode()
+        d_params.pop("ancestor")
+        self.assertRaises(TypeError, util.directivenode_kwargs, d_params)
+
+        p_params = self._setup_parsernode()
+        p_params.pop("filepath")
+        self.assertRaises(TypeError, util.parsernode_kwargs, p_params)
+
+
+if __name__ == "__main__":
+    unittest.main()  # pragma: no cover

certbot-apache/tests/util.py

@@ -84,7 +84,8 @@ def get_apache_configurator(
         config_path, vhost_path,
         config_dir, work_dir, version=(2, 4, 7),
         os_info="generic",
-        conf_vhost_path=None):
+        conf_vhost_path=None,
+        use_parsernode=False):
     """Create an Apache Configurator with the specified options.
 
     :param conf: Function that returns binary paths. self.conf in Configurator
@@ -110,19 +111,21 @@ def get_apache_configurator(
             mock_exe_exists.return_value = True
             with mock.patch("certbot_apache._internal.parser.ApacheParser."
                             "update_runtime_variables"):
-                try:
-                    config_class = entrypoint.OVERRIDE_CLASSES[os_info]
-                except KeyError:
-                    config_class = configurator.ApacheConfigurator
-                config = config_class(config=mock_le_config, name="apache",
-                    version=version)
-                if not conf_vhost_path:
-                    config_class.OS_DEFAULTS["vhost_root"] = vhost_path
-                else:
-                    # Custom virtualhost path was requested
-                    config.config.apache_vhost_root = conf_vhost_path
-                config.config.apache_ctl = config_class.OS_DEFAULTS["ctl"]
-                config.prepare()
+                with mock.patch("certbot_apache._internal.apache_util.parse_from_subprocess") as mock_sp:
+                    mock_sp.return_value = []
+                    try:
+                        config_class = entrypoint.OVERRIDE_CLASSES[os_info]
+                    except KeyError:
+                        config_class = configurator.ApacheConfigurator
+                    config = config_class(config=mock_le_config, name="apache",
+                                          version=version, use_parsernode=use_parsernode)
+                    if not conf_vhost_path:
+                        config_class.OS_DEFAULTS["vhost_root"] = vhost_path
+                    else:
+                        # Custom virtualhost path was requested
+                        config.config.apache_vhost_root = conf_vhost_path
+                    config.config.apache_ctl = config_class.OS_DEFAULTS["ctl"]
+                    config.prepare()
     return config
 
 

certbot-auto

@@ -31,7 +31,7 @@ if [ -z "$VENV_PATH" ]; then
 fi
 VENV_BIN="$VENV_PATH/bin"
 BOOTSTRAP_VERSION_PATH="$VENV_PATH/certbot-auto-bootstrap-version.txt"
-LE_AUTO_VERSION="1.1.0"
+LE_AUTO_VERSION="1.2.0"
 BASENAME=$(basename $0)
 USAGE="Usage: $BASENAME [OPTIONS]
 A self-updating wrapper script for the Certbot ACME client. When run, updates
@@ -1274,11 +1274,11 @@ if [ "$1" = "--le-auto-phase2" ]; then
 # pip install hashin
 # hashin -r dependency-requirements.txt cryptography==1.5.2
 # ```
-ConfigArgParse==0.14.0 \
-    --hash=sha256:2e2efe2be3f90577aca9415e32cb629aa2ecd92078adbe27b53a03e53ff12e91
-certifi==2019.9.11 \
-    --hash=sha256:e4f3620cfea4f83eedc95b24abd9cd56f3c4b146dd0177e83a21b4eb49e21e50 \
-    --hash=sha256:fd7c7c74727ddcf00e9acd26bba8da604ffec95bf1c2144e67aff7a8b50e6cef
+ConfigArgParse==1.0 \
+    --hash=sha256:bf378245bc9cdc403a527e5b7406b991680c2a530e7e81af747880b54eb57133
+certifi==2019.11.28 \
+    --hash=sha256:017c25db2a153ce562900032d5bc68e9f191e44e9a0f762f373977de9df1fbb3 \
+    --hash=sha256:25b64c7da4cd7479594d035c08c2d809eb4aab3a26e5a990ea98cc450c320f1f
 cffi==1.13.2 \
     --hash=sha256:0b49274afc941c626b605fb59b59c3485c17dc776dc3cc7cc14aca74cc19cc42 \
     --hash=sha256:0e3ea92942cb1168e38c05c1d56b0527ce31f1a370f6117f1d490b8dcd6b3a04 \
@@ -1351,8 +1351,6 @@ enum34==1.1.6 \
 funcsigs==1.0.2 \
     --hash=sha256:330cc27ccbf7f1e992e69fef78261dc7c6569012cf397db8d3de0234e6c937ca \
     --hash=sha256:a7bb0f2cf3a3fd1ab2732cb49eba4252c2af4240442415b4abce3b87022a8f50
-future==0.18.2 \
-    --hash=sha256:b1bead90b70cf6ec3f0710ae53a525360fa360d306a86583adc6bf83a4db537d
 idna==2.8 \
     --hash=sha256:c357b3f628cf53ae2c4c05627ecc484553142ca23264e593d327bcde5e9c3407 \
     --hash=sha256:ea8b7f6188e6fa117537c3df7da9fc686d485087abf6ac197f9c46432f7e4a3c
@@ -1365,40 +1363,40 @@ josepy==1.2.0 \
 mock==1.3.0 \
     --hash=sha256:1e247dbecc6ce057299eb7ee019ad68314bb93152e81d9a6110d35f4d5eca0f6 \
     --hash=sha256:3f573a18be94de886d1191f27c168427ef693e8dcfcecf95b170577b2eb69cbb
-parsedatetime==2.4 \
-    --hash=sha256:3d817c58fb9570d1eec1dd46fa9448cd644eeed4fb612684b02dfda3a79cb84b \
-    --hash=sha256:9ee3529454bf35c40a77115f5a596771e59e1aee8c53306f346c461b8e913094
-pbr==5.4.3 \
-    --hash=sha256:2c8e420cd4ed4cec4e7999ee47409e876af575d4c35a45840d59e8b5f3155ab8 \
-    --hash=sha256:b32c8ccaac7b1a20c0ce00ce317642e6cf231cf038f9875e0280e28af5bf7ac9
-pyOpenSSL==19.0.0 \
-    --hash=sha256:aeca66338f6de19d1aa46ed634c3b9ae519a64b458f8468aec688e7e3c20f200 \
-    --hash=sha256:c727930ad54b10fc157015014b666f2d8b41f70c0d03e83ab67624fd3dd5d1e6
+parsedatetime==2.5 \
+    --hash=sha256:3b835fc54e472c17ef447be37458b400e3fefdf14bb1ffdedb5d2c853acf4ba1 \
+    --hash=sha256:d2e9ddb1e463de871d32088a3f3cea3dc8282b1b2800e081bd0ef86900451667
+pbr==5.4.4 \
+    --hash=sha256:139d2625547dbfa5fb0b81daebb39601c478c21956dc57e2e07b74450a8c506b \
+    --hash=sha256:61aa52a0f18b71c5cc58232d2cf8f8d09cd67fcad60b742a60124cb8d6951488
+pyOpenSSL==19.1.0 \
+    --hash=sha256:621880965a720b8ece2f1b2f54ea2071966ab00e2970ad2ce11d596102063504 \
+    --hash=sha256:9a24494b2602aaf402be5c9e30a0b82d4a5c67528fe8fb475e3f3bc00dd69507
 pyRFC3339==1.1 \
     --hash=sha256:67196cb83b470709c580bb4738b83165e67c6cc60e1f2e4f286cfcb402a926f4 \
     --hash=sha256:81b8cbe1519cdb79bed04910dd6fa4e181faf8c88dff1e1b987b5f7ab23a5b1a
 pycparser==2.19 \
     --hash=sha256:a988718abfad80b6b157acce7bf130a30876d27603738ac39f140993246b25b3
-pyparsing==2.4.5 \
-    --hash=sha256:20f995ecd72f2a1f4bf6b072b63b22e2eb457836601e76d6e5dfcd75436acc1f \
-    --hash=sha256:4ca62001be367f01bd3e92ecbb79070272a9d4964dce6a48a82ff0b8bc7e683a
+pyparsing==2.4.6 \
+    --hash=sha256:4c830582a84fb022400b85429791bc551f1f4871c33f23e44f353119e92f969f \
+    --hash=sha256:c342dccb5250c08d45fd6f8b4a559613ca603b57498511740e65cd11a2e7dcec
 python-augeas==0.5.0 \
     --hash=sha256:67d59d66cdba8d624e0389b87b2a83a176f21f16a87553b50f5703b23f29bac2
 pytz==2019.3 \
     --hash=sha256:1c557d7d0e871de1f5ccd5833f60fb2550652da6be2693c1e02300743d21500d \
     --hash=sha256:b02c06db6cf09c12dd25137e563b31700d3b80fcc4ad23abb7a315f2789819be
-requests==2.21.0 \
-    --hash=sha256:502a824f31acdacb3a35b6690b5fbf0bc41d63a24a45c4004352b0242707598e \
-    --hash=sha256:7bf2a778576d825600030a110f3c0e3e8edc51dfaafe1c146e39a2027784957b
+requests==2.22.0 \
+    --hash=sha256:11e007a8a2aa0323f5a921e9e6a2d7e4e67d9877e85773fba9ba6419025cbeb4 \
+    --hash=sha256:9cf5292fcd0f598c671cfc1e0d7d1a7f13bb8085e9a590f48c010551dc6c4b31
 requests-toolbelt==0.9.1 \
     --hash=sha256:380606e1d10dc85c3bd47bf5a6095f815ec007be7a8b69c878507068df059e6f \
     --hash=sha256:968089d4584ad4ad7c171454f0a5c6dac23971e9472521ea3b6d49d610aa6fc0
-six==1.13.0 \
-    --hash=sha256:1f1b7d42e254082a9db6279deae68afb421ceba6158efa6131de7b3003ee93fd \
-    --hash=sha256:30f610279e8b2578cab6db20741130331735c781b56053c59c4076da27f06b66
-urllib3==1.24.3 \
-    --hash=sha256:2393a695cd12afedd0dcb26fe5d50d0cf248e5a66f75dbd89a3d4eb333a61af4 \
-    --hash=sha256:a637e5fae88995b256e3409dc4d52c2e2e0ba32c42a6365fee8bbd2238de3cfb
+six==1.14.0 \
+    --hash=sha256:236bdbdce46e6e6a3d61a337c0f8b763ca1e8717c03b369e87a7ec7ce1319c0a \
+    --hash=sha256:8f3cd2e254d8f793e7f3d6d9df77b92252b52637291d0f0da013c76ea2724b6c
+urllib3==1.25.8 \
+    --hash=sha256:2f3db8b19923a873b3e5256dc9c2dedfa883e33d87c690d9c7913e1f40673cdc \
+    --hash=sha256:87716c2d2a7121198ebcb7ce7cccf6ce5e9ba539041cfbaeecfb641dc0bf6acc
 zope.component==4.6 \
     --hash=sha256:ec2afc5bbe611dcace98bb39822c122d44743d635dafc7315b9aef25097db9e6
 zope.deferredimport==4.3.1 \
@@ -1410,47 +1408,86 @@ zope.deprecation==4.4.0 \
 zope.event==4.4 \
     --hash=sha256:69c27debad9bdacd9ce9b735dad382142281ac770c4a432b533d6d65c4614bcf \
     --hash=sha256:d8e97d165fd5a0997b45f5303ae11ea3338becfe68c401dd88ffd2113fe5cae7
-zope.hookable==4.2.0 \
-    --hash=sha256:22886e421234e7e8cedc21202e1d0ab59960e40a47dd7240e9659a2d82c51370 \
-    --hash=sha256:39912f446e45b4e1f1951b5ffa2d5c8b074d25727ec51855ae9eab5408f105ab \
-    --hash=sha256:3adb7ea0871dbc56b78f62c4f5c024851fc74299f4f2a95f913025b076cde220 \
-    --hash=sha256:3d7c4b96341c02553d8b8d71065a9366ef67e6c6feca714f269894646bb8268b \
-    --hash=sha256:4e826a11a529ed0464ffcecf34b0b7bd1b4928dd5848c5c61bedd7833e8f4801 \
-    --hash=sha256:700d68cc30728de1c4c62088a981c6daeaefdf20a0d81995d2c0b7f442c5f88c \
-    --hash=sha256:77c82a430cedfbf508d1aa406b2f437363c24fa90c73f577ead0fb5295749b83 \
-    --hash=sha256:c1df3929a3666fc5a0c80d60a0c1e6f6ef97c7f6ed2f1b7cf49f3e6f3d4dde15 \
-    --hash=sha256:dba8b2dd2cd41cb5f37bfa3f3d82721b8ae10e492944e48ddd90a439227f2893 \
-    --hash=sha256:f492540305b15b5591bd7195d61f28946bb071de071cee5d68b6b8414da90fd2
-zope.interface==4.6.0 \
-    --hash=sha256:086707e0f413ff8800d9c4bc26e174f7ee4c9c8b0302fbad68d083071822316c \
-    --hash=sha256:1157b1ec2a1f5bf45668421e3955c60c610e31913cc695b407a574efdbae1f7b \
-    --hash=sha256:11ebddf765bff3bbe8dbce10c86884d87f90ed66ee410a7e6c392086e2c63d02 \
-    --hash=sha256:14b242d53f6f35c2d07aa2c0e13ccb710392bcd203e1b82a1828d216f6f6b11f \
-    --hash=sha256:1b3d0dcabc7c90b470e59e38a9acaa361be43b3a6ea644c0063951964717f0e5 \
-    --hash=sha256:20a12ab46a7e72b89ce0671e7d7a6c3c1ca2c2766ac98112f78c5bddaa6e4375 \
-    --hash=sha256:298f82c0ab1b182bd1f34f347ea97dde0fffb9ecf850ecf7f8904b8442a07487 \
-    --hash=sha256:2f6175722da6f23dbfc76c26c241b67b020e1e83ec7fe93c9e5d3dd18667ada2 \
-    --hash=sha256:3b877de633a0f6d81b600624ff9137312d8b1d0f517064dfc39999352ab659f0 \
-    --hash=sha256:4265681e77f5ac5bac0905812b828c9fe1ce80c6f3e3f8574acfb5643aeabc5b \
-    --hash=sha256:550695c4e7313555549aa1cdb978dc9413d61307531f123558e438871a883d63 \
-    --hash=sha256:5f4d42baed3a14c290a078e2696c5f565501abde1b2f3f1a1c0a94fbf6fbcc39 \
-    --hash=sha256:62dd71dbed8cc6a18379700701d959307823b3b2451bdc018594c48956ace745 \
-    --hash=sha256:7040547e5b882349c0a2cc9b50674b1745db551f330746af434aad4f09fba2cc \
-    --hash=sha256:7e099fde2cce8b29434684f82977db4e24f0efa8b0508179fce1602d103296a2 \
-    --hash=sha256:7e5c9a5012b2b33e87980cee7d1c82412b2ebabcb5862d53413ba1a2cfde23aa \
-    --hash=sha256:81295629128f929e73be4ccfdd943a0906e5fe3cdb0d43ff1e5144d16fbb52b1 \
-    --hash=sha256:95cc574b0b83b85be9917d37cd2fad0ce5a0d21b024e1a5804d044aabea636fc \
-    --hash=sha256:968d5c5702da15c5bf8e4a6e4b67a4d92164e334e9c0b6acf080106678230b98 \
-    --hash=sha256:9e998ba87df77a85c7bed53240a7257afe51a07ee6bc3445a0bf841886da0b97 \
-    --hash=sha256:a0c39e2535a7e9c195af956610dba5a1073071d2d85e9d2e5d789463f63e52ab \
-    --hash=sha256:a15e75d284178afe529a536b0e8b28b7e107ef39626a7809b4ee64ff3abc9127 \
-    --hash=sha256:a6a6ff82f5f9b9702478035d8f6fb6903885653bff7ec3a1e011edc9b1a7168d \
-    --hash=sha256:b639f72b95389620c1f881d94739c614d385406ab1d6926a9ffe1c8abbea23fe \
-    --hash=sha256:bad44274b151d46619a7567010f7cde23a908c6faa84b97598fd2f474a0c6891 \
-    --hash=sha256:bbcef00d09a30948756c5968863316c949d9cedbc7aabac5e8f0ffbdb632e5f1 \
-    --hash=sha256:d788a3999014ddf416f2dc454efa4a5dbeda657c6aba031cf363741273804c6b \
-    --hash=sha256:eed88ae03e1ef3a75a0e96a55a99d7937ed03e53d0cffc2451c208db445a2966 \
-    --hash=sha256:f99451f3a579e73b5dd58b1b08d1179791d49084371d9a47baad3b22417f0317
+zope.hookable==5.0.0 \
+    --hash=sha256:0992a0dd692003c09fb958e1480cebd1a28f2ef32faa4857d864f3ca8e9d6952 \
+    --hash=sha256:0f325838dbac827a1e2ed5d482c1f2656b6844dc96aa098f7727e76395fcd694 \
+    --hash=sha256:22a317ba00f61bac99eac1a5e330be7cb8c316275a21269ec58aa396b602af0c \
+    --hash=sha256:25531cb5e7b35e8a6d1d6eddef624b9a22ce5dcf8f4448ef0f165acfa8c3fc21 \
+    --hash=sha256:30890892652766fc80d11f078aca9a5b8150bef6b88aba23799581a53515c404 \
+    --hash=sha256:342d682d93937e5b8c232baffb32a87d5eee605d44f74566657c64a239b7f342 \
+    --hash=sha256:46b2fddf1f5aeb526e02b91f7e62afbb9fff4ffd7aafc97cdb00a0d717641567 \
+    --hash=sha256:523318ff96df9b8d378d997c00c5d4cbfbff68dc48ff5ee5addabdb697d27528 \
+    --hash=sha256:53aa02eb8921d4e667c69d76adeed8fe426e43870c101cb08dcd2f3468aff742 \
+    --hash=sha256:62e79e8fdde087cb20822d7874758f5acbedbffaf3c0fbe06309eb8a41ee4e06 \
+    --hash=sha256:74bf2f757f7385b56dc3548adae508d8b3ef952d600b4b12b88f7d1706b05dcc \
+    --hash=sha256:751ee9d89eb96e00c1d7048da9725ce392a708ed43406416dc5ed61e4d199764 \
+    --hash=sha256:7b83bc341e682771fe810b360cd5d9c886a948976aea4b979ff214e10b8b523b \
+    --hash=sha256:81eeeb27dbb0ddaed8070daee529f0d1bfe4f74c7351cce2aaca3ea287c4cc32 \
+    --hash=sha256:856509191e16930335af4d773c0fc31a17bae8991eb6f167a09d5eddf25b56cc \
+    --hash=sha256:8853e81fd07b18fa9193b19e070dc0557848d9945b1d2dac3b7782543458c87d \
+    --hash=sha256:94506a732da2832029aecdfe6ea07eb1b70ee06d802fff34e1b3618fe7cdf026 \
+    --hash=sha256:95ad874a8cc94e786969215d660143817f745225579bfe318c4676e218d3147c \
+    --hash=sha256:9758ec9174966ffe5c499b6c3d149f80aa0a9238020006a2b87c6af5963fcf48 \
+    --hash=sha256:a169823e331da939aa7178fc152e65699aeb78957e46c6f80ccb50ee4c3616c2 \
+    --hash=sha256:a67878a798f6ca292729a28c2226592b3d000dc6ee7825d31887b553686c7ac7 \
+    --hash=sha256:a9a6d9eb2319a09905670810e2de971d6c49013843700b4975e2fc0afe96c8db \
+    --hash=sha256:b3e118b58a3d2301960e6f5f25736d92f6b9f861728d3b8c26d69f54d8a157d2 \
+    --hash=sha256:ca6705c2a1fb5059a4efbe9f5426be4cdf71b3c9564816916fc7aa7902f19ede \
+    --hash=sha256:cf711527c9d4ae72085f137caffb4be74fc007ffb17cd103628c7d5ba17e205f \
+    --hash=sha256:d087602a6845ebe9d5a1c5a949fedde2c45f372d77fbce4f7fe44b68b28a1d03 \
+    --hash=sha256:d1080e1074ddf75ad6662a9b34626650759c19a9093e1a32a503d37e48da135b \
+    --hash=sha256:db9c60368aff2b7e6c47115f3ad9bd6e96aa298b12ed5f8cb13f5673b30be565 \
+    --hash=sha256:dbeb127a04473f5a989169eb400b67beb921c749599b77650941c21fe39cb8d9 \
+    --hash=sha256:dca336ca3682d869d291d7cd18284f6ff6876e4244eb1821430323056b000e2c \
+    --hash=sha256:dd69a9be95346d10c853b6233fcafe3c0315b89424b378f2ad45170d8e161568 \
+    --hash=sha256:dd79f8fae5894f1ee0a0042214685f2d039341250c994b825c10a4cd075d80f6 \
+    --hash=sha256:e647d850aa1286d98910133cee12bd87c354f7b7bb3f3cd816a62ba7fa2f7007 \
+    --hash=sha256:f37a210b5c04b2d4e4bac494ab15b70196f219a1e1649ddca78560757d4278fb \
+    --hash=sha256:f67820b6d33a705dc3c1c457156e51686f7b350ff57f2112e1a9a4dad38ec268 \
+    --hash=sha256:f68969978ccf0e6123902f7365aae5b7a9e99169d4b9105c47cf28e788116894 \
+    --hash=sha256:f717a0b34460ae1ac0064e91b267c0588ac2c098ffd695992e72cd5462d97a67 \
+    --hash=sha256:f9d58ccec8684ca276d5a4e7b0dfacca028336300a8f715d616d9f0ce9ae8096 \
+    --hash=sha256:fcc3513a54e656067cbf7b98bab0d6b9534b9eabc666d1f78aad6acdf0962736
+zope.interface==4.7.1 \
+    --hash=sha256:048b16ac882a05bc7ef534e8b9f15c9d7a6c190e24e8938a19b7617af4ed854a \
+    --hash=sha256:05816cf8e7407cf62f2ec95c0a5d69ec4fa5741d9ccd10db9f21691916a9a098 \
+    --hash=sha256:065d6a1ac89d35445168813bed45048ed4e67a4cdfc5a68fdb626a770378869f \
+    --hash=sha256:14157421f4121a57625002cc4f48ac7521ea238d697c4a4459a884b62132b977 \
+    --hash=sha256:18dc895945694f397a0be86be760ff664b790f95d8e7752d5bab80284ff9105d \
+    --hash=sha256:1962c9f838bd6ae4075d0014f72697510daefc7e1c7e48b2607df0b6e157989c \
+    --hash=sha256:1a67408cacd198c7e6274a19920bb4568d56459e659e23c4915528686ac1763a \
+    --hash=sha256:21bf781076dd616bd07cf0223f79d61ab4f45176076f90bc2890e18c48195da4 \
+    --hash=sha256:21c0a5d98650aebb84efa16ce2c8df1a46bdc4fe8a9e33237d0ca0b23f416ead \
+    --hash=sha256:23cfeea25d1e42ff3bf4f9a0c31e9d5950aa9e7c4b12f0c4bd086f378f7b7a71 \
+    --hash=sha256:24b6fce1fb71abf9f4093e3259084efcc0ef479f89356757780685bd2b06ef37 \
+    --hash=sha256:24f84ce24eb6b5fcdcb38ad9761524f1ae96f7126abb5e597f8a3973d9921409 \
+    --hash=sha256:25e0ef4a824017809d6d8b0ce4ab3288594ba283e4d4f94d8cfb81d73ed65114 \
+    --hash=sha256:2e8fdd625e9aba31228e7ddbc36bad5c38dc3ee99a86aa420f89a290bd987ce9 \
+    --hash=sha256:2f3bc2f49b67b1bea82b942d25bc958d4f4ea6709b411cb2b6b9718adf7914ce \
+    --hash=sha256:35d24be9d04d50da3a6f4d61de028c1dd087045385a0ff374d93ef85af61b584 \
+    --hash=sha256:35dbe4e8c73003dff40dfaeb15902910a4360699375e7b47d3c909a83ff27cd0 \
+    --hash=sha256:3dfce831b824ab5cf446ed0c350b793ac6fa5fe33b984305cb4c966a86a8fb79 \
+    --hash=sha256:3f7866365df5a36a7b8de8056cd1c605648f56f9a226d918ed84c85d25e8d55f \
+    --hash=sha256:455cc8c01de3bac6f9c223967cea41f4449f58b4c2e724ec8177382ddd183ab4 \
+    --hash=sha256:4bb937e998be9d5e345f486693e477ba79e4344674484001a0b646be1d530487 \
+    --hash=sha256:52303a20902ca0888dfb83230ca3ee6fbe63c0ad1dd60aa0bba7958ccff454d8 \
+    --hash=sha256:6e0a897d4e09859cc80c6a16a29697406ead752292ace17f1805126a4f63c838 \
+    --hash=sha256:6e1816e7c10966330d77af45f77501f9a68818c065dec0ad11d22b50a0e212e7 \
+    --hash=sha256:73b5921c5c6ce3358c836461b5470bf675601c96d5e5d8f2a446951470614f67 \
+    --hash=sha256:8093cd45cdb5f6c8591cfd1af03d32b32965b0f79b94684cd0c9afdf841982bb \
+    --hash=sha256:864b4a94b60db301899cf373579fd9ef92edddbf0fb2cd5ae99f53ef423ccc56 \
+    --hash=sha256:8a27b4d3ea9c6d086ce8e7cdb3e8d319b6752e2a03238a388ccc83ccbe165f50 \
+    --hash=sha256:91b847969d4784abd855165a2d163f72ac1e58e6dce09a5e46c20e58f19cc96d \
+    --hash=sha256:b47b1028be4758c3167e474884ccc079b94835f058984b15c145966c4df64d27 \
+    --hash=sha256:b68814a322835d8ad671b7acc23a3b2acecba527bb14f4b53fc925f8a27e44d8 \
+    --hash=sha256:bcb50a032c3b6ec7fb281b3a83d2b31ab5246c5b119588725b1350d3a1d9f6a3 \
+    --hash=sha256:c56db7d10b25ce8918b6aec6b08ac401842b47e6c136773bfb3b590753f7fb67 \
+    --hash=sha256:c94b77a13d4f47883e4f97f9fa00f5feadd38af3e6b3c7be45cfdb0a14c7149b \
+    --hash=sha256:db381f6fdaef483ad435f778086ccc4890120aff8df2ba5cfeeac24d280b3145 \
+    --hash=sha256:e6487d01c8b7ed86af30ea141fcc4f93f8a7dde26f94177c1ad637c353bd5c07 \
+    --hash=sha256:e86923fa728dfba39c5bb6046a450bd4eec8ad949ac404eca728cfce320d1732 \
+    --hash=sha256:f6ca36dc1e9eeb46d779869c60001b3065fb670b5775c51421c099ea2a77c3c9 \
+    --hash=sha256:fb62f2cbe790a50d95593fb40e8cca261c31a2f5637455ea39440d6457c2ba25
 zope.proxy==4.3.3 \
     --hash=sha256:04646ac04ffa9c8e32fb2b5c3cd42995b2548ea14251f3c21ca704afae88e42c \
     --hash=sha256:07b6bceea232559d24358832f1cd2ed344bbf05ca83855a5b9698b5f23c5ed60 \
@@ -1503,18 +1540,18 @@ letsencrypt==0.7.0 \
     --hash=sha256:105a5fb107e45bcd0722eb89696986dcf5f08a86a321d6aef25a0c7c63375ade \
     --hash=sha256:c36e532c486a7e92155ee09da54b436a3c420813ec1c590b98f635d924720de9
 
-certbot==1.1.0 \
-    --hash=sha256:66a5cab9267349941604c2c98082bfef85877653c023fc324b1c3869fb16add6 \
-    --hash=sha256:46e93661a0db53f416c0f5476d8d2e62bc7259b7660dd983453b85df9ef6e8b8
-acme==1.1.0 \
-    --hash=sha256:11b9beba706fb8f652c8910d46dd1939d670cac8169f3c66c18c080ed3353e71 \
-    --hash=sha256:c305a20eeb9cb02240347703d497891c13d43a47c794fa100d4dbb479a5370d9
-certbot-apache==1.1.0 \
-    --hash=sha256:9c847ff223c2e465e241c78d22f97cee77d5e551df608bed06c55f8627f4cbd2 \
-    --hash=sha256:05e84dfe96b72582cde97c490977d8e2d33d440c927a320debb4cf287f6fadcc
-certbot-nginx==1.1.0 \
-    --hash=sha256:bf06fa2f5059f0fdb7d352c8739e1ed0830db4f0d89e812dab4f081bda6ec7d6 \
-    --hash=sha256:0a80ecbd2a30f3757c7652cabfff854ca07873b1cf02ebbe1892786c3b3a5874
+certbot==1.2.0 \
+    --hash=sha256:e25c17125c00b3398c8e9b9d54ef473c0e8f5aff53389f313a51b06cf472d335 \
+    --hash=sha256:95dcbae085f8e4eb18442fe7b12994b08964a9a6e8e352e556cdb4a8a625373c
+acme==1.2.0 \
+    --hash=sha256:284d22fde75687a8ea72d737cac6bcbdc91f3c796221aa25378b8732ba6f6875 \
+    --hash=sha256:0630c740d49bda945e97bd35fc8d6f02d082c8cb9e18f8fec0dbb3d395ac26ab
+certbot-apache==1.2.0 \
+    --hash=sha256:3f7493918353d3bd6067d446a2cf263e03831c4c10ec685b83d644b47767090d \
+    --hash=sha256:b46e9def272103a68108e48bf7e410ea46801529b1ea6954f6506b14dd9df9b3
+certbot-nginx==1.2.0 \
+    --hash=sha256:efd32a2b32f2439279da446b6bf67684f591f289323c5f494ebfd86a566a28fd \
+    --hash=sha256:6fd7cf4f2545ad66e57000343227df9ccccaf04420e835e05cb3250fac1fa6db
 
 UNLIKELY_EOF
     # -------------------------------------------------------------------------

certbot-ci/setup.py

@@ -40,7 +40,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 :: 3 - Alpha',
         'Intended Audience :: Developers',
@@ -49,7 +49,6 @@ 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',

certbot-ci/windows_installer_integration_tests/conftest.py

@@ -0,0 +1,38 @@
+"""
+General conftest for pytest execution of all integration tests lying
+in the window_installer_integration tests package.
+As stated by pytest documentation, conftest module is used to set on
+for a directory a specific configuration using built-in pytest hooks.
+
+See https://docs.pytest.org/en/latest/reference.html#hook-reference
+"""
+from __future__ import print_function
+import os
+
+import pytest
+
+ROOT_PATH = os.path.dirname(os.path.dirname(os.path.dirname(__file__)))
+
+
+def pytest_addoption(parser):
+    """
+    Standard pytest hook to add options to the pytest parser.
+    :param parser: current pytest parser that will be used on the CLI
+    """
+    parser.addoption('--installer-path',
+                     default=os.path.join(ROOT_PATH, 'windows-installer', 'build',
+                                          'nsis', 'certbot-beta-installer-win32.exe'),
+                     help='set the path of the windows installer to use, default to '
+                          'CERTBOT_ROOT_PATH\\windows-installer\\build\\nsis\\certbot-beta-installer-win32.exe')
+    parser.addoption('--allow-persistent-changes', action='store_true',
+                     help='needs to be set, and confirm that the test will make persistent changes on this machine')
+
+
+def pytest_configure(config):
+    """
+    Standard pytest hook used to add a configuration logic for each node of a pytest run.
+    :param config: the current pytest configuration
+    """
+    if not config.option.allow_persistent_changes:
+        raise RuntimeError('This integration test would install Certbot on your machine. '
+                           'Please run it again with the `--allow-persistent-changes` flag set to acknowledge.')

certbot-ci/windows_installer_integration_tests/test_main.py

@@ -0,0 +1,61 @@
+import os
+import time
+import unittest
+import subprocess
+import re
+
+
+@unittest.skipIf(os.name != 'nt', reason='Windows installer tests must be run on Windows.')
+def test_it(request):
+    try:
+        subprocess.check_call(['certbot', '--version'])
+    except (subprocess.CalledProcessError, OSError):
+        pass
+    else:
+        raise AssertionError('Expect certbot to not be available in the PATH.')
+
+    try:
+        # Install certbot
+        subprocess.check_call([request.config.option.installer_path, '/S'])
+
+        # Assert certbot is installed and runnable
+        output = subprocess.check_output(['certbot', '--version'], universal_newlines=True)
+        assert re.match(r'^certbot \d+\.\d+\.\d+.*$', output), 'Flag --version does not output a version.'
+
+        # Assert renew task is installed and ready
+        output = _ps('(Get-ScheduledTask -TaskName "Certbot Renew Task").State', capture_stdout=True)
+        assert output.strip() == 'Ready'
+
+        # Assert renew task is working
+        now = time.time()
+        _ps('Start-ScheduledTask -TaskName "Certbot Renew Task"')
+
+        status = 'Running'
+        while status != 'Ready':
+            status = _ps('(Get-ScheduledTask -TaskName "Certbot Renew Task").State', capture_stdout=True).strip()
+            time.sleep(1)
+
+        log_path = os.path.join('C:\\', 'Certbot', 'log', 'letsencrypt.log')
+
+        modification_time = os.path.getmtime(log_path)
+        assert now < modification_time, 'Certbot log file has not been modified by the renew task.'
+
+        with open(log_path) as file_h:
+            data = file_h.read()
+        assert 'no renewal failures' in data, 'Renew task did not execute properly.'
+
+    finally:
+        # Sadly this command cannot work in non interactive mode: uninstaller will ask explicitly permission in an UAC prompt
+        # print('Uninstalling Certbot ...')
+        # uninstall_path = _ps('(gci "HKLM:\\SOFTWARE\\Wow6432Node\\Microsoft\\Windows\\CurrentVersion\\Uninstall"'
+        #                      ' | foreach { gp $_.PSPath }'
+        #                      ' | ? { $_ -match "Certbot" }'
+        #                      ' | select UninstallString)'
+        #                      '.UninstallString', capture_stdout=True)
+        # subprocess.check_call([uninstall_path, '/S'])
+        pass
+
+
+def _ps(powershell_str, capture_stdout=False):
+    fn = subprocess.check_output if capture_stdout else subprocess.check_call
+    return fn(['powershell.exe', '-c', powershell_str], universal_newlines=True)

certbot-compatibility-test/setup.py

@@ -3,7 +3,7 @@ import sys
 from setuptools import find_packages
 from setuptools import setup
 
-version = '1.2.0.dev0'
+version = '1.3.0.dev0'
 
 install_requires = [
     'certbot',
@@ -28,7 +28,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 :: 3 - Alpha',
         'Intended Audience :: Developers',
@@ -37,7 +37,6 @@ 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',

certbot-dns-cloudflare/certbot_dns_cloudflare/__init__.py

@@ -22,17 +22,40 @@ Credentials
 
 Use of this plugin requires a configuration file containing Cloudflare API
 credentials, obtained from your Cloudflare
-`account page <https://www.cloudflare.com/a/account/my-account>`_. This plugin
-does not currently support Cloudflare's "API Tokens", so please ensure you use
-the "Global API Key" for authentication.
+`account page <https://dash.cloudflare.com/profile/api-tokens>`_.
+
+Previously, Cloudflare's "Global API Key" was used for authentication, however
+this key can access the entire Cloudflare API for all domains in your account,
+meaning it could cause a lot of damage if leaked.
+
+Cloudflare's newer API Tokens can be restricted to specific domains and
+operations, and are therefore now the recommended authentication option.
+
+However, due to some shortcomings in Cloudflare's implementation of Tokens,
+Tokens created for Certbot currently require ``Zone:Zone:Read`` and ``Zone:DNS:Edit``
+permissions for **all** zones in your account. While this is not ideal, your Token
+will still have fewer permission than the Global key, so it's still worth doing.
+Hopefully Cloudflare will improve this in the future.
+
+Using Cloudflare Tokens also requires at least version 2.3.1 of the ``cloudflare``
+python module. If the version that automatically installed with this plugin is
+older than that, and you can't upgrade it on your system, you'll have to stick to
+the Global key.
 
 .. code-block:: ini
-   :name: credentials.ini
-   :caption: Example credentials file:
+   :name: certbot_cloudflare_token.ini
+   :caption: Example credentials file using restricted API Token (recommended):
+
+   # Cloudflare API token used by Certbot
+   dns_cloudflare_api_token = 0123456789abcdef0123456789abcdef01234567
+
+.. code-block:: ini
+   :name: certbot_cloudflare_key.ini
+   :caption: Example credentials file using Global API Key (not recommended):
 
    # Cloudflare API credentials used by Certbot
    dns_cloudflare_email = cloudflare@example.com
-   dns_cloudflare_api_key = 0123456789abcdef0123456789abcdef01234567
+   dns_cloudflare_api_key = 0123456789abcdef0123456789abcdef01234
 
 The path to this file can be provided interactively or using the
 ``--dns-cloudflare-credentials`` command-line argument. Certbot records the path

certbot-dns-cloudflare/certbot_dns_cloudflare/_internal/dns_cloudflare.py

@@ -4,6 +4,10 @@ import logging
 import CloudFlare
 import zope.interface
 
+from acme.magic_typing import Any
+from acme.magic_typing import Dict
+from acme.magic_typing import List
+
 from certbot import errors
 from certbot import interfaces
 from certbot.plugins import dns_common
@@ -38,14 +42,35 @@ class Authenticator(dns_common.DNSAuthenticator):
         return 'This plugin configures a DNS TXT record to respond to a dns-01 challenge using ' + \
                'the Cloudflare API.'
 
+    def _validate_credentials(self, credentials):
+        token = credentials.conf('api-token')
+        email = credentials.conf('email')
+        key = credentials.conf('api-key')
+        if token:
+            if email or key:
+                raise errors.PluginError('{}: dns_cloudflare_email and dns_cloudflare_api_key are '
+                                         'not needed when using an API Token'
+                                         .format(credentials.confobj.filename))
+        elif email or key:
+            if not email:
+                raise errors.PluginError('{}: dns_cloudflare_email is required when using a Global '
+                                         'API Key. (should be email address associated with '
+                                         'Cloudflare account)'.format(credentials.confobj.filename))
+            if not key:
+                raise errors.PluginError('{}: dns_cloudflare_api_key is required when using a '
+                                         'Global API Key. (see {})'
+                                         .format(credentials.confobj.filename, ACCOUNT_URL))
+        else:
+            raise errors.PluginError('{}: Either dns_cloudflare_api_token (recommended), or '
+                                     'dns_cloudflare_email and dns_cloudflare_api_key are required.'
+                                     ' (see {})'.format(credentials.confobj.filename, ACCOUNT_URL))
+
     def _setup_credentials(self):
         self.credentials = self._configure_credentials(
             'credentials',
             'Cloudflare credentials INI file',
-            {
-                'email': 'email address associated with Cloudflare account',
-                'api-key': 'API key for Cloudflare account, obtained from {0}'.format(ACCOUNT_URL)
-            }
+            None,
+            self._validate_credentials
         )
 
     def _perform(self, domain, validation_name, validation):
@@ -55,6 +80,8 @@ class Authenticator(dns_common.DNSAuthenticator):
         self._get_cloudflare_client().del_txt_record(domain, validation_name, validation)
 
     def _get_cloudflare_client(self):
+        if self.credentials.conf('api-token'):
+            return _CloudflareClient(None, self.credentials.conf('api-token'))
         return _CloudflareClient(self.credentials.conf('email'), self.credentials.conf('api-key'))
 
 
@@ -88,8 +115,15 @@ class _CloudflareClient(object):
             logger.debug('Attempting to add record to zone %s: %s', zone_id, data)
             self.cf.zones.dns_records.post(zone_id, data=data)  # zones | pylint: disable=no-member
         except CloudFlare.exceptions.CloudFlareAPIError as e:
+            code = int(e)
+            hint = None
+
+            if code == 9109:
+                hint = 'Does your API token have "Zone:DNS:Edit" permissions?'
+
             logger.error('Encountered CloudFlareAPIError adding TXT record: %d %s', e, e)
-            raise errors.PluginError('Error communicating with the Cloudflare API: {0}'.format(e))
+            raise errors.PluginError('Error communicating with the Cloudflare API: {0}{1}'
+                                     .format(e, ' ({0})'.format(hint) if hint else ''))
 
         record_id = self._find_txt_record_id(zone_id, record_name, record_content)
         logger.debug('Successfully added TXT record with record_id: %s', record_id)
@@ -139,6 +173,8 @@ class _CloudflareClient(object):
         """
 
         zone_name_guesses = dns_common.base_domain_name_guesses(domain)
+        zones = []  # type: List[Dict[str, Any]]
+        code = msg = None
 
         for zone_name in zone_name_guesses:
             params = {'name': zone_name,
@@ -148,16 +184,26 @@ class _CloudflareClient(object):
                 zones = self.cf.zones.get(params=params)  # zones | pylint: disable=no-member
             except CloudFlare.exceptions.CloudFlareAPIError as e:
                 code = int(e)
+                msg = str(e)
                 hint = None
 
                 if code == 6003:
-                    hint = 'Did you copy your entire API key?'
+                    hint = ('Did you copy your entire API token/key? To use Cloudflare tokens, '
+                            'you\'ll need the python package cloudflare>=2.3.1.{}'
+                    .format(' This certbot is running cloudflare ' + str(CloudFlare.__version__)
+                    if hasattr(CloudFlare, '__version__') else ''))
                 elif code == 9103:
-                    hint = 'Did you enter the correct email address?'
+                    hint = 'Did you enter the correct email address and Global key?'
+                elif code == 9109:
+                    hint = 'Did you enter a valid Cloudflare Token?'
 
-                raise errors.PluginError('Error determining zone_id: {0} {1}. Please confirm that '
-                                         'you have supplied valid Cloudflare API credentials.{2}'
-                                         .format(code, e, ' ({0})'.format(hint) if hint else ''))
+                if hint:
+                    raise errors.PluginError('Error determining zone_id: {0} {1}. Please confirm '
+                                  'that you have supplied valid Cloudflare API credentials. ({2})'
+                                                                         .format(code, msg, hint))
+                else:
+                    logger.debug('Unrecognised CloudFlareAPIError while finding zone_id: %d %s. '
+                                 'Continuing with next zone guess...', e, e)
 
             if zones:
                 zone_id = zones[0]['id']
@@ -165,9 +211,10 @@ class _CloudflareClient(object):
                 return zone_id
 
         raise errors.PluginError('Unable to determine zone_id for {0} using zone names: {1}. '
-                                 'Please confirm that the domain name has been entered correctly '
-                                 'and is already associated with the supplied Cloudflare account.'
-                                 .format(domain, zone_name_guesses))
+                                'Please confirm that the domain name has been entered correctly '
+                                'and is already associated with the supplied Cloudflare account.{2}'
+                                .format(domain, zone_name_guesses, ' The error from Cloudflare was:'
+                                ' {0} {1}'.format(code, msg) if code is not None else ''))
 
     def _find_txt_record_id(self, zone_id, record_name, record_content):
         """

certbot-dns-cloudflare/docs/conf.py

@@ -84,7 +84,7 @@ default_role = 'py:obj'
 pygments_style = 'sphinx'
 
 # If true, `todo` and `todoList` produce output, else they produce nothing.
-todo_include_todos = True
+todo_include_todos = False
 
 
 # -- Options for HTML output ----------------------------------------------

certbot-dns-cloudflare/setup.py

@@ -4,7 +4,7 @@ from setuptools import find_packages
 from setuptools import setup
 from setuptools.command.test import test as TestCommand
 
-version = '1.2.0.dev0'
+version = '1.3.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.
@@ -44,7 +44,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',
         'Environment :: Plugins',
@@ -55,7 +55,6 @@ 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',

certbot-dns-cloudflare/tests/dns_cloudflare_test.py

@@ -12,6 +12,9 @@ from certbot.plugins.dns_test_common import DOMAIN
 from certbot.tests import util as test_util
 
 API_ERROR = CloudFlare.exceptions.CloudFlareAPIError(1000, '', '')
+
+API_TOKEN = 'an-api-token'
+
 API_KEY = 'an-api-key'
 EMAIL = 'example@example.com'
 
@@ -49,6 +52,50 @@ class AuthenticatorTest(test_util.TempDirTestCase, dns_test_common.BaseAuthentic
         expected = [mock.call.del_txt_record(DOMAIN, '_acme-challenge.'+DOMAIN, mock.ANY)]
         self.assertEqual(expected, self.mock_client.mock_calls)
 
+    def test_api_token(self):
+        dns_test_common.write({"cloudflare_api_token": API_TOKEN},
+                              self.config.cloudflare_credentials)
+        self.auth.perform([self.achall])
+
+        expected = [mock.call.add_txt_record(DOMAIN, '_acme-challenge.'+DOMAIN, mock.ANY, mock.ANY)]
+        self.assertEqual(expected, self.mock_client.mock_calls)
+
+    def test_no_creds(self):
+        dns_test_common.write({}, self.config.cloudflare_credentials)
+        self.assertRaises(errors.PluginError,
+                          self.auth.perform,
+                          [self.achall])
+
+    def test_missing_email_or_key(self):
+        dns_test_common.write({"cloudflare_api_key": API_KEY}, self.config.cloudflare_credentials)
+        self.assertRaises(errors.PluginError,
+                          self.auth.perform,
+                          [self.achall])
+
+        dns_test_common.write({"cloudflare_email": EMAIL}, self.config.cloudflare_credentials)
+        self.assertRaises(errors.PluginError,
+                          self.auth.perform,
+                          [self.achall])
+
+    def test_email_or_key_with_token(self):
+        dns_test_common.write({"cloudflare_api_token": API_TOKEN, "cloudflare_email": EMAIL},
+                              self.config.cloudflare_credentials)
+        self.assertRaises(errors.PluginError,
+                          self.auth.perform,
+                          [self.achall])
+
+        dns_test_common.write({"cloudflare_api_token": API_TOKEN, "cloudflare_api_key": API_KEY},
+                              self.config.cloudflare_credentials)
+        self.assertRaises(errors.PluginError,
+                          self.auth.perform,
+                          [self.achall])
+
+        dns_test_common.write({"cloudflare_api_token": API_TOKEN, "cloudflare_email": EMAIL,
+                               "cloudflare_api_key": API_KEY}, self.config.cloudflare_credentials)
+        self.assertRaises(errors.PluginError,
+                          self.auth.perform,
+                          [self.achall])
+
 
 class CloudflareClientTest(unittest.TestCase):
     record_name = "foo"
@@ -83,7 +130,7 @@ class CloudflareClientTest(unittest.TestCase):
     def test_add_txt_record_error(self):
         self.cf.zones.get.return_value = [{'id': self.zone_id}]
 
-        self.cf.zones.dns_records.post.side_effect = API_ERROR
+        self.cf.zones.dns_records.post.side_effect = CloudFlare.exceptions.CloudFlareAPIError(9109, '', '')
 
         self.assertRaises(
             errors.PluginError,
@@ -106,6 +153,25 @@ class CloudflareClientTest(unittest.TestCase):
             self.cloudflare_client.add_txt_record,
             DOMAIN, self.record_name, self.record_content, self.record_ttl)
 
+    def test_add_txt_record_bad_creds(self):
+        self.cf.zones.get.side_effect = CloudFlare.exceptions.CloudFlareAPIError(6003, '', '')
+        self.assertRaises(
+            errors.PluginError,
+            self.cloudflare_client.add_txt_record,
+            DOMAIN, self.record_name, self.record_content, self.record_ttl)
+
+        self.cf.zones.get.side_effect = CloudFlare.exceptions.CloudFlareAPIError(9103, '', '')
+        self.assertRaises(
+            errors.PluginError,
+            self.cloudflare_client.add_txt_record,
+            DOMAIN, self.record_name, self.record_content, self.record_ttl)
+
+        self.cf.zones.get.side_effect = CloudFlare.exceptions.CloudFlareAPIError(9109, '', '')
+        self.assertRaises(
+            errors.PluginError,
+            self.cloudflare_client.add_txt_record,
+            DOMAIN, self.record_name, self.record_content, self.record_ttl)
+
     def test_del_txt_record(self):
         self.cf.zones.get.return_value = [{'id': self.zone_id}]
         self.cf.zones.dns_records.get.return_value = [{'id': self.record_id}]

certbot-dns-cloudxns/docs/conf.py

@@ -84,7 +84,7 @@ default_role = 'py:obj'
 pygments_style = 'sphinx'
 
 # If true, `todo` and `todoList` produce output, else they produce nothing.
-todo_include_todos = True
+todo_include_todos = False
 
 
 # -- Options for HTML output ----------------------------------------------

certbot-dns-cloudxns/setup.py

@@ -4,7 +4,7 @@ from setuptools import find_packages
 from setuptools import setup
 from setuptools.command.test import test as TestCommand
 
-version = '1.2.0.dev0'
+version = '1.3.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.
@@ -44,7 +44,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',
         'Environment :: Plugins',
@@ -55,7 +55,6 @@ 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',

certbot-dns-digitalocean/docs/conf.py

@@ -84,7 +84,7 @@ default_role = 'py:obj'
 pygments_style = 'sphinx'
 
 # If true, `todo` and `todoList` produce output, else they produce nothing.
-todo_include_todos = True
+todo_include_todos = False
 
 
 # -- Options for HTML output ----------------------------------------------

certbot-dns-digitalocean/setup.py

@@ -4,7 +4,7 @@ from setuptools import find_packages
 from setuptools import setup
 from setuptools.command.test import test as TestCommand
 
-version = '1.2.0.dev0'
+version = '1.3.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.
@@ -45,7 +45,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',
         'Environment :: Plugins',
@@ -56,7 +56,6 @@ 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',

certbot-dns-dnsimple/docs/conf.py

@@ -84,7 +84,7 @@ default_role = 'py:obj'
 pygments_style = 'sphinx'
 
 # If true, `todo` and `todoList` produce output, else they produce nothing.
-todo_include_todos = True
+todo_include_todos = False
 
 
 # -- Options for HTML output ----------------------------------------------

certbot-dns-dnsimple/setup.py

@@ -5,7 +5,7 @@ from setuptools import find_packages
 from setuptools import setup
 from setuptools.command.test import test as TestCommand
 
-version = '1.2.0.dev0'
+version = '1.3.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.
@@ -56,7 +56,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',
         'Environment :: Plugins',
@@ -67,7 +67,6 @@ 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',

certbot-dns-dnsmadeeasy/docs/conf.py

@@ -84,7 +84,7 @@ default_role = 'py:obj'
 pygments_style = 'sphinx'
 
 # If true, `todo` and `todoList` produce output, else they produce nothing.
-todo_include_todos = True
+todo_include_todos = False
 
 
 # -- Options for HTML output ----------------------------------------------

certbot-dns-dnsmadeeasy/setup.py

@@ -4,7 +4,7 @@ from setuptools import find_packages
 from setuptools import setup
 from setuptools.command.test import test as TestCommand
 
-version = '1.2.0.dev0'
+version = '1.3.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.
@@ -44,7 +44,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',
         'Environment :: Plugins',
@@ -55,7 +55,6 @@ 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',

certbot-dns-gehirn/docs/conf.py

@@ -84,7 +84,7 @@ default_role = 'py:obj'
 pygments_style = 'sphinx'
 
 # If true, `todo` and `todoList` produce output, else they produce nothing.
-todo_include_todos = True
+todo_include_todos = False
 
 
 # -- Options for HTML output ----------------------------------------------

certbot-dns-gehirn/setup.py

@@ -4,7 +4,7 @@ from setuptools import find_packages
 from setuptools import setup
 from setuptools.command.test import test as TestCommand
 
-version = '1.2.0.dev0'
+version = '1.3.0.dev0'
 
 # Please update tox.ini when modifying dependency version requirements
 install_requires = [
@@ -43,7 +43,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',
         'Environment :: Plugins',
@@ -54,7 +54,6 @@ 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',

certbot-dns-google/docs/conf.py

@@ -85,7 +85,7 @@ default_role = 'py:obj'
 pygments_style = 'sphinx'
 
 # If true, `todo` and `todoList` produce output, else they produce nothing.
-todo_include_todos = True
+todo_include_todos = False
 
 
 # -- Options for HTML output ----------------------------------------------

certbot-dns-google/setup.py

@@ -4,7 +4,7 @@ from setuptools import find_packages
 from setuptools import setup
 from setuptools.command.test import test as TestCommand
 
-version = '1.2.0.dev0'
+version = '1.3.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.
@@ -47,7 +47,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',
         'Environment :: Plugins',
@@ -58,7 +58,6 @@ 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',

certbot-dns-linode/docs/conf.py

@@ -84,7 +84,7 @@ default_role = 'py:obj'
 pygments_style = 'sphinx'
 
 # If true, `todo` and `todoList` produce output, else they produce nothing.
-todo_include_todos = True
+todo_include_todos = False
 
 
 # -- Options for HTML output ----------------------------------------------

certbot-dns-linode/setup.py

@@ -4,7 +4,7 @@ from setuptools import find_packages
 from setuptools import setup
 from setuptools.command.test import test as TestCommand
 
-version = '1.2.0.dev0'
+version = '1.3.0.dev0'
 
 # Please update tox.ini when modifying dependency version requirements
 install_requires = [
@@ -43,7 +43,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',
         'Environment :: Plugins',
@@ -54,7 +54,6 @@ 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',

certbot-dns-luadns/docs/conf.py

@@ -84,7 +84,7 @@ default_role = 'py:obj'
 pygments_style = 'sphinx'
 
 # If true, `todo` and `todoList` produce output, else they produce nothing.
-todo_include_todos = True
+todo_include_todos = False
 
 
 # -- Options for HTML output ----------------------------------------------

certbot-dns-luadns/setup.py

@@ -4,7 +4,7 @@ from setuptools import find_packages
 from setuptools import setup
 from setuptools.command.test import test as TestCommand
 
-version = '1.2.0.dev0'
+version = '1.3.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.
@@ -44,7 +44,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',
         'Environment :: Plugins',
@@ -55,7 +55,6 @@ 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',

certbot-dns-nsone/docs/conf.py

@@ -84,7 +84,7 @@ default_role = 'py:obj'
 pygments_style = 'sphinx'
 
 # If true, `todo` and `todoList` produce output, else they produce nothing.
-todo_include_todos = True
+todo_include_todos = False
 
 
 # -- Options for HTML output ----------------------------------------------

certbot-dns-nsone/setup.py

@@ -4,7 +4,7 @@ from setuptools import find_packages
 from setuptools import setup
 from setuptools.command.test import test as TestCommand
 
-version = '1.2.0.dev0'
+version = '1.3.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.
@@ -44,7 +44,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',
         'Environment :: Plugins',
@@ -55,7 +55,6 @@ 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',

certbot-dns-ovh/docs/conf.py

@@ -84,7 +84,7 @@ default_role = 'py:obj'
 pygments_style = 'sphinx'
 
 # If true, `todo` and `todoList` produce output, else they produce nothing.
-todo_include_todos = True
+todo_include_todos = False
 
 
 # -- Options for HTML output ----------------------------------------------

certbot-dns-ovh/setup.py

@@ -4,7 +4,7 @@ from setuptools import find_packages
 from setuptools import setup
 from setuptools.command.test import test as TestCommand
 
-version = '1.2.0.dev0'
+version = '1.3.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.
@@ -44,7 +44,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',
         'Environment :: Plugins',
@@ -55,7 +55,6 @@ 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',

certbot-dns-rfc2136/docs/conf.py

@@ -84,7 +84,7 @@ default_role = 'py:obj'
 pygments_style = 'sphinx'
 
 # If true, `todo` and `todoList` produce output, else they produce nothing.
-todo_include_todos = True
+todo_include_todos = False
 
 
 # -- Options for HTML output ----------------------------------------------

certbot-dns-rfc2136/setup.py

@@ -4,7 +4,7 @@ from setuptools import find_packages
 from setuptools import setup
 from setuptools.command.test import test as TestCommand
 
-version = '1.2.0.dev0'
+version = '1.3.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.
@@ -44,7 +44,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',
         'Environment :: Plugins',
@@ -55,7 +55,6 @@ 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',

certbot-dns-route53/docs/conf.py

@@ -84,7 +84,7 @@ default_role = 'py:obj'
 pygments_style = 'sphinx'
 
 # If true, `todo` and `todoList` produce output, else they produce nothing.
-todo_include_todos = True
+todo_include_todos = False
 
 
 # -- Options for HTML output ----------------------------------------------

certbot-dns-route53/setup.py

@@ -4,7 +4,7 @@ from setuptools import find_packages
 from setuptools import setup
 from setuptools.command.test import test as TestCommand
 
-version = '1.2.0.dev0'
+version = '1.3.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.
@@ -39,7 +39,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',
         'Environment :: Plugins',
@@ -50,7 +50,6 @@ 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',

certbot-dns-sakuracloud/docs/conf.py

@@ -84,7 +84,7 @@ default_role = 'py:obj'
 pygments_style = 'sphinx'
 
 # If true, `todo` and `todoList` produce output, else they produce nothing.
-todo_include_todos = True
+todo_include_todos = False
 
 
 # -- Options for HTML output ----------------------------------------------

certbot-dns-sakuracloud/setup.py

@@ -4,7 +4,7 @@ from setuptools import find_packages
 from setuptools import setup
 from setuptools.command.test import test as TestCommand
 
-version = '1.2.0.dev0'
+version = '1.3.0.dev0'
 
 # Please update tox.ini when modifying dependency version requirements
 install_requires = [
@@ -43,7 +43,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',
         'Environment :: Plugins',
@@ -54,7 +54,6 @@ 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',

certbot-nginx/certbot_nginx/_internal/configurator.py

@@ -313,9 +313,6 @@ class NginxConfigurator(common.Installer):
         .. todo:: This should maybe return list if no obvious answer
             is presented.
 
-        .. todo:: The special name "$hostname" corresponds to the machine's
-            hostname. Currently we just ignore this.
-
         :param str target_name: domain name
         :param bool create_if_no_match: If we should create a new vhost from default
             when there is no match found. If we can't choose a default, raise a
@@ -598,6 +595,12 @@ class NginxConfigurator(common.Installer):
         all_names = set()  # type: Set[str]
 
         for vhost in self.parser.get_vhosts():
+            try:
+                vhost.names.remove("$hostname")
+                vhost.names.add(socket.gethostname())
+            except KeyError:
+                pass
+
             all_names.update(vhost.names)
 
             for addr in vhost.addrs:
@@ -1008,7 +1011,7 @@ class NginxConfigurator(common.Installer):
             matches = re.findall(r"built with OpenSSL ([^ ]+) ", text)
             if not matches:
                 logger.warning("NGINX configured with OpenSSL alternatives is not officially"
-                    "supported by Certbot.")
+                    " supported by Certbot.")
                 return ""
         return matches[0]
 

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

@@ -10,4 +10,4 @@ ssl_session_timeout 1440m;
 ssl_protocols TLSv1.2;
 ssl_prefer_server_ciphers off;
 
-ssl_ciphers "ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384:ECDHE-RSA-AES128-SHA";
+ssl_ciphers "ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384";

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

@@ -11,4 +11,4 @@ ssl_session_tickets off;
 ssl_protocols TLSv1.2;
 ssl_prefer_server_ciphers off;
 
-ssl_ciphers "ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384:ECDHE-RSA-AES128-SHA";
+ssl_ciphers "ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384";

certbot-nginx/certbot_nginx/_internal/tls_configs/options-ssl-nginx-tls13-session-tix-on.conf

@@ -10,4 +10,4 @@ ssl_session_timeout 1440m;
 ssl_protocols TLSv1.2 TLSv1.3;
 ssl_prefer_server_ciphers off;
 
-ssl_ciphers "ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384:ECDHE-RSA-AES128-SHA";
+ssl_ciphers "ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384";

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

@@ -11,4 +11,4 @@ ssl_session_tickets off;
 ssl_protocols TLSv1.2 TLSv1.3;
 ssl_prefer_server_ciphers off;
 
-ssl_ciphers "ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384:ECDHE-RSA-AES128-SHA";
+ssl_ciphers "ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384";

certbot-nginx/setup.py

@@ -4,7 +4,7 @@ from setuptools import find_packages
 from setuptools import setup
 from setuptools.command.test import test as TestCommand
 
-version = '1.2.0.dev0'
+version = '1.3.0.dev0'
 
 # Remember to update local-oldest-requirements.txt when changing the minimum
 # acme/certbot version.
@@ -42,7 +42,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',
         'Environment :: Plugins',
@@ -53,7 +53,6 @@ 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',

certbot-nginx/tests/configurator_test.py

@@ -36,7 +36,7 @@ class NginxConfiguratorTest(util.NginxTest):
 
     def test_prepare(self):
         self.assertEqual((1, 6, 2), self.config.version)
-        self.assertEqual(11, len(self.config.parser.parsed))
+        self.assertEqual(12, len(self.config.parser.parsed))
 
     @mock.patch("certbot_nginx._internal.configurator.util.exe_exists")
     @mock.patch("certbot_nginx._internal.configurator.subprocess.Popen")
@@ -76,15 +76,17 @@ class NginxConfiguratorTest(util.NginxTest):
         else:  # pragma: no cover
             self.fail("Exception wasn't raised!")
 
+    @mock.patch("certbot_nginx._internal.configurator.socket.gethostname")
     @mock.patch("certbot_nginx._internal.configurator.socket.gethostbyaddr")
-    def test_get_all_names(self, mock_gethostbyaddr):
+    def test_get_all_names(self, mock_gethostbyaddr, mock_gethostname):
         mock_gethostbyaddr.return_value = ('155.225.50.69.nephoscale.net', [], [])
+        mock_gethostname.return_value = ('example.net')
         names = self.config.get_all_names()
         self.assertEqual(names, {
             "155.225.50.69.nephoscale.net", "www.example.org", "another.alias",
              "migration.com", "summer.com", "geese.com", "sslon.com",
              "globalssl.com", "globalsslsetssl.com", "ipv6.com", "ipv6ssl.com",
-             "headers.com"})
+             "headers.com", "example.net"})
 
     def test_supported_enhancements(self):
         self.assertEqual(['redirect', 'ensure-http-header', 'staple-ocsp'],
@@ -924,7 +926,7 @@ class NginxConfiguratorTest(util.NginxTest):
                                                 prefer_ssl=False,
                                                 no_ssl_filter_port='80')
             # Check that the dialog was called with only port 80 vhosts
-            self.assertEqual(len(mock_select_vhs.call_args[0][0]), 5)
+            self.assertEqual(len(mock_select_vhs.call_args[0][0]), 6)
 
 
 class InstallSslOptionsConfTest(util.NginxTest):

certbot-nginx/tests/parser_test.py

@@ -58,7 +58,8 @@ class NginxParserTest(util.NginxTest):
                            'sites-enabled/sslon.com',
                            'sites-enabled/globalssl.com',
                            'sites-enabled/ipv6.com',
-                           'sites-enabled/ipv6ssl.com']},
+                           'sites-enabled/ipv6ssl.com',
+                           'sites-enabled/example.net']},
                          set(nparser.parsed.keys()))
         self.assertEqual([['server_name', 'somename', 'alias', 'another.alias']],
                          nparser.parsed[nparser.abs_path('server.conf')])
@@ -88,7 +89,7 @@ class NginxParserTest(util.NginxTest):
         parsed = nparser._parse_files(nparser.abs_path(
             'sites-enabled/example.com.test'))
         self.assertEqual(3, len(glob.glob(nparser.abs_path('*.test'))))
-        self.assertEqual(8, len(
+        self.assertEqual(9, len(
             glob.glob(nparser.abs_path('sites-enabled/*.test'))))
         self.assertEqual([[['server'], [['listen', '69.50.225.155:9000'],
                                         ['listen', '127.0.0.1'],
@@ -171,7 +172,7 @@ class NginxParserTest(util.NginxTest):
                                                   '*.www.example.com']),
                                  [], [2, 1, 0])
 
-        self.assertEqual(13, len(vhosts))
+        self.assertEqual(14, len(vhosts))
         example_com = [x for x in vhosts if 'example.com' in x.filep][0]
         self.assertEqual(vhost3, example_com)
         default = [x for x in vhosts if 'default' in x.filep][0]

certbot-nginx/tests/testdata/etc_nginx/sites-enabled/example.net

@@ -0,0 +1,5 @@
+server {
+    listen 80;
+    listen [::]:80;
+    server_name $hostname;
+}

certbot/CHANGELOG.md

@@ -2,20 +2,46 @@
 
 Certbot adheres to [Semantic Versioning](https://semver.org/).
 
-## 1.2.0 - master
+## 1.3.0 - master
 
 ### Added
 
 * OCSP prefetching functionality for Apache plugin that attempts to refresh the OCSP
   response cache for managed certificates when scheduled Certbot renew is being run. 
+* Added certbot.ocsp Certbot's API. The certbot.ocsp module can be used to 
+  determine the OCSP status of certificates.
+* Don't verify the existing certificate in HTTP01Response.simple_verify, for 
+  compatibility with the real-world ACME challenge checks.
 
 ### Changed
 
-* Add directory field to error message when field is missing.
+*
 
 ### Fixed
 
 *
+>>>>>>> origin/master
+
+More details about these changes can be found on our GitHub repo.
+
+## 1.2.0 - 2020-02-04
+
+### Added
+
+* Added support for Cloudflare's limited-scope API Tokens
+* Added support for `$hostname` in nginx `server_name` directive
+
+### Changed
+
+* Add directory field to error message when field is missing.
+* If MD5 hasher is not available, try it in non-security mode (fix for FIPS systems) -- [#1948](https://github.com/certbot/certbot/issues/1948)
+* Disable old SSL versions and ciphersuites and remove `SSLCompression off` setting to follow Mozilla recommendations in Apache.
+* Remove ECDHE-RSA-AES128-SHA from NGINX ciphers list now that Windows 2008 R2 and Windows 7 are EOLed
+* Support for Python 3.4 has been removed.
+
+### Fixed
+
+* Fix collections.abc imports for Python 3.9.
 
 More details about these changes can be found on our GitHub repo.
 

certbot/README.rst

@@ -71,7 +71,7 @@ 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| |coverage| |container|
 
 .. |build-status| image:: https://travis-ci.com/certbot/certbot.svg?branch=master
    :target: https://travis-ci.com/certbot/certbot
@@ -81,10 +81,6 @@ ACME working area in github: https://github.com/ietf-wg-acme/acme
    :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

certbot/certbot/__init__.py

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

certbot/certbot/_internal/account.py

@@ -56,11 +56,18 @@ class Account(object):
                 tz=pytz.UTC).replace(microsecond=0),
             creation_host=socket.getfqdn()) if meta is None else meta
 
-        self.id = hashlib.md5(
-            self.key.key.public_key().public_bytes(
-                encoding=serialization.Encoding.PEM,
-                format=serialization.PublicFormat.SubjectPublicKeyInfo)
-        ).hexdigest()
+        # try MD5, else use MD5 in non-security mode (e.g. for FIPS systems / RHEL)
+        try:
+            hasher = hashlib.md5()
+        except ValueError:
+            hasher = hashlib.new('md5', usedforsecurity=False) # type: ignore
+
+        hasher.update(self.key.key.public_key().public_bytes(
+            encoding=serialization.Encoding.PEM,
+            format=serialization.PublicFormat.SubjectPublicKeyInfo)
+        )
+
+        self.id = hasher.hexdigest()
         # Implementation note: Email? Multiple accounts can have the
         # same email address. Registration URI? Assigned by the
         # server, not guaranteed to be stable over time, nor

certbot/certbot/_internal/cert_manager.py

@@ -11,8 +11,8 @@ from acme.magic_typing import List  # pylint: disable=unused-import, no-name-in-
 from certbot import crypto_util
 from certbot import errors
 from certbot import interfaces
+from certbot import ocsp
 from certbot import util
-from certbot._internal import ocsp
 from certbot._internal import storage
 from certbot.compat import os
 from certbot.display import util as display_util

certbot/certbot/_internal/main.py

@@ -1350,10 +1350,6 @@ def main(cli_args=None):
         if config.func != plugins_cmd:  # pylint: disable=comparison-with-callable
             raise
 
-    if sys.version_info[:2] == (3, 4):
-        logger.warning("Python 3.4 support will be dropped in the next release "
-                       "of Certbot - please upgrade your Python version to 3.5+.")
-
     set_displayer(config)
 
     # Reporter

certbot/certbot/_internal/plugins/disco.py

@@ -13,6 +13,12 @@ from certbot import errors
 from certbot import interfaces
 from certbot._internal import constants
 
+try:
+    # Python 3.3+
+    from collections.abc import Mapping
+except ImportError:  # pragma: no cover
+    from collections import Mapping
+
 logger = logging.getLogger(__name__)
 
 
@@ -178,7 +184,7 @@ class PluginEntryPoint(object):
         return "\n".join(lines)
 
 
-class PluginsRegistry(collections.Mapping):
+class PluginsRegistry(Mapping):
     """Plugins registry."""
 
     def __init__(self, plugins):

certbot/certbot/_internal/renewal.py

@@ -471,4 +471,7 @@ def handle_renewal_request(config):
     if renew_failures or parse_failures:
         raise errors.Error("{0} renew failure(s), {1} parse failure(s)".format(
             len(renew_failures), len(parse_failures)))
+
+    # Windows installer integration tests rely on handle_renewal_request behavior here.
+    # If the text below changes, these tests will need to be updated accordingly.
     logger.debug("no renewal failures")

certbot/certbot/compat/filesystem.py

@@ -263,7 +263,7 @@ def replace(src, dst):
     :param str dst: The new file path.
     """
     if hasattr(os, 'replace'):
-        # Use replace if possible. On Windows, only Python >= 3.4 is supported
+        # Use replace if possible. On Windows, only Python >= 3.5 is supported
         # so we can assume that os.replace() is always available for this platform.
         getattr(os, 'replace')(src, dst)
     else:

certbot/certbot/ocsp.py

@@ -21,7 +21,7 @@ from acme.magic_typing import Tuple  # pylint: disable=unused-import, no-name-in
 from certbot import crypto_util
 from certbot import errors
 from certbot import util
-from certbot._internal.storage import RenewableCert  # pylint: disable=unused-import
+from certbot.interfaces import RenewableCert  # pylint: disable=unused-import
 
 try:
     # Only cryptography>=2.5 has ocsp module
@@ -63,13 +63,14 @@ class RevocationChecker(object):
 
         .. todo:: Make this a non-blocking call
 
-        :param `.storage.RenewableCert` cert: Certificate object
+        :param `.interfaces.RenewableCert` cert: Certificate object
 
         :returns: True if revoked; False if valid or the check failed or cert is expired.
         :rtype: bool
 
         """
-        return self.ocsp_revoked_by_paths(cert.cert, cert.chain)
+
+        return self.ocsp_revoked_by_paths(cert.cert_path, cert.chain_path)
 
     def ocsp_revoked_by_paths(self, cert_path, chain_path, response_file=None):
         # type: (str, str, Optional[str]) -> bool

certbot/certbot/tests/util.py

@@ -1,8 +1,4 @@
-"""Test utilities.
-
-.. warning:: This module is not part of the public API.
-
-"""
+"""Test utilities."""
 import logging
 from multiprocessing import Event
 from multiprocessing import Process

certbot/docs/api/certbot.ocsp.rst

@@ -0,0 +1,7 @@
+certbot.ocsp package
+======================
+
+.. automodule:: certbot.ocsp
+    :members:
+    :undoc-members:
+    :show-inheritance:

certbot/docs/api/certbot.rst

@@ -26,6 +26,7 @@ Submodules
    certbot.errors
    certbot.interfaces
    certbot.main
+   certbot.ocsp
    certbot.reverter
    certbot.util
 

certbot/docs/cli-help.txt

@@ -113,7 +113,7 @@ optional arguments:
                         case, and to know when to deprecate support for past
                         Python versions and flags. If you wish to hide this
                         information from the Let's Encrypt server, set this to
-                        "". (default: CertbotACMEClient/1.1.0 (certbot(-auto);
+                        "". (default: CertbotACMEClient/1.2.0 (certbot(-auto);
                         OS_NAME OS_VERSION) Authenticator/XXX Installer/YYY
                         (SUBCOMMAND; flags: FLAGS) Py/major.minor.patchlevel).
                         The flags encoded in the user agent are: --duplicate,

certbot/docs/compatibility.rst

@@ -0,0 +1,39 @@
+=======================
+Backwards Compatibility
+=======================
+
+All Certbot components including `acme <https://acme-python.readthedocs.io/>`_,
+Certbot, and :ref:`non-third party plugins <plugins>` follow `Semantic
+Versioning <https://semver.org/>`_ both for its Python :doc:`API <api>` and for the
+application itself. This means that we will not change behavior in a backwards
+incompatible way except in a new major version of the project.
+
+.. note:: None of this applies to the behavior of Certbot distribution
+    mechanisms such as :ref:`certbot-auto <certbot-auto>` or OS packages whose
+    behavior may change at any time. Semantic versioning only applies to the
+    common Certbot components that are installed by various distribution
+    methods.
+
+For Certbot as an application, the command line interface and non-interactive
+behavior can be considered stable with two exceptions. The first is that no
+aspects of Certbot's console or log output should be considered stable and it
+may change at any time. The second is that Certbot's behavior should only be
+considered stable with certain files but not all. Files with which users should
+expect Certbot to maintain its current behavior with are:
+
+* ``/etc/letsencrypt/live/<domain>/{cert,chain,fullchain,privkey}.pem`` where
+  ``<domain>`` is the name given to ``--cert-name``. If ``--cert-name`` is not
+  set by the user, it is the first domain given to ``--domains``.
+* :ref:`CLI configuration files <config-file>`
+* Hook directories in ``/etc/letsencrypt/renewal-hooks``
+
+Certbot's behavior with other files may change at any point.
+
+Another area where Certbot should not be considered stable is its behavior when
+not run in non-interactive mode which also may change at any point.
+
+In general, if we're making a change that we expect will break some users, we
+will bump the major version and will have warned about it in a prior release
+when possible. For our Python API, we will issue warnings using Python's
+warning module. For application level changes, we will print and log warning
+messages.

certbot/docs/conf.py

@@ -122,7 +122,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
 
 suppress_warnings = ['image.nonlocal_uri']