Merge branch 'master' into pylint

# Conflicts:
#	acme/acme/client.py
#	acme/acme/crypto_util.py
#	acme/acme/standalone.py
#	certbot-apache/certbot_apache/configurator.py
#	certbot-apache/certbot_apache/parser.py
#	certbot-apache/certbot_apache/tests/tls_sni_01_test.py
#	certbot-apache/certbot_apache/tests/util.py
#	certbot-apache/certbot_apache/tls_sni_01.py
#	certbot-nginx/certbot_nginx/configurator.py
#	certbot-nginx/certbot_nginx/parser.py
#	certbot-nginx/certbot_nginx/tests/util.py
#	certbot/account.py
#	certbot/cert_manager.py
#	certbot/cli.py
#	certbot/configuration.py
#	certbot/main.py
#	certbot/ocsp.py
#	certbot/plugins/dns_common_lexicon.py
#	certbot/plugins/standalone.py
#	certbot/plugins/util.py
#	certbot/plugins/webroot.py
#	certbot/tests/auth_handler_test.py
#	certbot/tests/cert_manager_test.py
#	certbot/tests/display/util_test.py
#	certbot/tests/main_test.py
#	certbot/tests/util.py
#	certbot/util.py
#	tox.ini

certbot/plugins/common.py

@@ -11,6 +11,8 @@ import zope.interface
 
 from josepy import util as jose_util
 
+from acme.magic_typing import List  # pylint: disable=unused-import, no-name-in-module
+from certbot import achallenges  # pylint: disable=unused-import
 from certbot import constants
 from certbot import crypto_util
 from certbot import errors
@@ -330,8 +332,8 @@ class ChallengePerformer(object):
 
     def __init__(self, configurator):
         self.configurator = configurator
-        self.achalls = []
-        self.indices = []
+        self.achalls = []  # type: List[achallenges.KeyAuthorizationAnnotatedChallenge]
+        self.indices = []  # type: List[int]
 
     def add_chall(self, achall, idx=None):
         """Store challenge to be performed when perform() is called.
@@ -348,7 +350,7 @@ class ChallengePerformer(object):
     def perform(self):
         """Perform all added challenges.
 
-        :returns: challenge respones
+        :returns: challenge responses
         :rtype: `list` of `acme.challenges.KeyAuthorizationChallengeResponse`
 
 

certbot/plugins/disco.py

@@ -9,6 +9,7 @@ import six
 import zope.interface
 import zope.interface.verify
 
+from acme.magic_typing import Dict  # pylint: disable=unused-import, no-name-in-module
 from certbot import constants
 from certbot import errors
 from certbot import interfaces
@@ -28,12 +29,17 @@ class PluginEntryPoint(object):
         "certbot-dns-digitalocean",
         "certbot-dns-dnsimple",
         "certbot-dns-dnsmadeeasy",
+        "certbot-dns-gehirn",
         "certbot-dns-google",
+        "certbot-dns-linode",
         "certbot-dns-luadns",
         "certbot-dns-nsone",
+        "certbot-dns-ovh",
         "certbot-dns-rfc2136",
         "certbot-dns-route53",
+        "certbot-dns-sakuracloud",
         "certbot-nginx",
+        "certbot-postfix",
     ]
     """Distributions for which prefix will be omitted."""
 
@@ -192,7 +198,7 @@ class PluginsRegistry(collections.Mapping):
     @classmethod
     def find_all(cls):
         """Find plugins using setuptools entry points."""
-        plugins = {}
+        plugins = {}  # type: Dict[str, PluginEntryPoint]
         # pylint: disable=not-callable
         entry_points = itertools.chain(
             pkg_resources.iter_entry_points(

certbot/plugins/disco_test.py

@@ -8,6 +8,7 @@ import pkg_resources
 import six
 import zope.interface
 
+from acme.magic_typing import List  # pylint: disable=unused-import, no-name-in-module
 from certbot import errors
 from certbot import interfaces
 
@@ -250,7 +251,7 @@ class PluginsRegistryTest(unittest.TestCase):
         self.plugin_ep.prepare.assert_called_once_with()
 
     def test_prepare_order(self):
-        order = []
+        order = []  # type: List[str]
         plugins = dict(
             (c, mock.MagicMock(prepare=functools.partial(order.append, c)))
             for c in string.ascii_letters)

certbot/plugins/dns_common_lexicon.py

@@ -1,12 +1,22 @@
 """Common code for DNS Authenticator Plugins built on Lexicon."""
-
 import logging
 
 from requests.exceptions import HTTPError, RequestException
 
+from acme.magic_typing import Union, Dict, Any  # pylint: disable=unused-import,no-name-in-module
 from certbot import errors
 from certbot.plugins import dns_common
 
+# Lexicon is not declared as a dependency in Certbot itself,
+# but in the Certbot plugins backed by Lexicon.
+# So we catch import error here to allow this module to be
+# always importable, even if it does not make sense to use it
+# if Lexicon is not available, obviously.
+try:
+    from lexicon.config import ConfigResolver
+except ImportError:
+    ConfigResolver = None  # type: ignore
+
 logger = logging.getLogger(__name__)
 
 
@@ -68,7 +78,12 @@ class LexiconClient(object):
 
         for domain_name in domain_name_guesses:
             try:
-                self.provider.options['domain'] = domain_name
+                if hasattr(self.provider, 'options'):
+                    # For Lexicon 2.x
+                    self.provider.options['domain'] = domain_name
+                else:
+                    # For Lexicon 3.x
+                    self.provider.domain = domain_name
 
                 self.provider.authenticate()
 
@@ -95,4 +110,28 @@ class LexiconClient(object):
         if not str(e).startswith('No domain found'):
             return errors.PluginError('Unexpected error determining zone identifier for {0}: {1}'
                                       .format(domain_name, e))
-        return None
+
+
+def build_lexicon_config(lexicon_provider_name, lexicon_options, provider_options):
+    # type: (str, Dict, Dict) -> Union[ConfigResolver, Dict]
+    """
+    Convenient function to build a Lexicon 2.x/3.x config object.
+    :param str lexicon_provider_name: the name of the lexicon provider to use
+    :param dict lexicon_options: options specific to lexicon
+    :param dict provider_options: options specific to provider
+    :return: configuration to apply to the provider
+    :rtype: ConfigurationResolver or dict
+    """
+    config = {'provider_name': lexicon_provider_name}  # type: Dict[str, Any]
+    config.update(lexicon_options)
+    if not ConfigResolver:
+        # Lexicon 2.x
+        config.update(provider_options)
+    else:
+        # Lexicon 3.x
+        provider_config = {}
+        provider_config.update(provider_options)
+        config[lexicon_provider_name] = provider_config
+        config = ConfigResolver().with_dict(config).with_env()
+
+    return config

certbot/plugins/enhancements.py

@@ -0,0 +1,164 @@
+"""New interface style Certbot enhancements"""
+import abc
+import six
+
+from certbot import constants
+
+from acme.magic_typing import Dict, List, Any  # pylint: disable=unused-import, no-name-in-module
+
+def enabled_enhancements(config):
+    """
+    Generator to yield the enabled new style enhancements.
+
+    :param config: Configuration.
+    :type config: :class:`certbot.interfaces.IConfig`
+    """
+    for enh in _INDEX:
+        if getattr(config, enh["cli_dest"]):
+            yield enh
+
+def are_requested(config):
+    """
+    Checks if one or more of the requested enhancements are those of the new
+    enhancement interfaces.
+
+    :param config: Configuration.
+    :type config: :class:`certbot.interfaces.IConfig`
+    """
+    return any(enabled_enhancements(config))
+
+def are_supported(config, installer):
+    """
+    Checks that all of the requested enhancements are supported by the
+    installer.
+
+    :param config: Configuration.
+    :type config: :class:`certbot.interfaces.IConfig`
+
+    :param installer: Installer object
+    :type installer: interfaces.IInstaller
+
+    :returns: If all the requested enhancements are supported by the installer
+    :rtype: bool
+    """
+    for enh in enabled_enhancements(config):
+        if not isinstance(installer, enh["class"]):
+            return False
+    return True
+
+def enable(lineage, domains, installer, config):
+    """
+    Run enable method for each requested enhancement that is supported.
+
+    :param lineage: Certificate lineage object
+    :type lineage: certbot.storage.RenewableCert
+
+    :param domains: List of domains in certificate to enhance
+    :type domains: str
+
+    :param installer: Installer object
+    :type installer: interfaces.IInstaller
+
+    :param config: Configuration.
+    :type config: :class:`certbot.interfaces.IConfig`
+    """
+    for enh in enabled_enhancements(config):
+        getattr(installer, enh["enable_function"])(lineage, domains)
+
+def populate_cli(add):
+    """
+    Populates the command line flags for certbot.cli.HelpfulParser
+
+    :param add: Add function of certbot.cli.HelpfulParser
+    :type add: func
+    """
+    for enh in _INDEX:
+        add(enh["cli_groups"], enh["cli_flag"], action=enh["cli_action"],
+            dest=enh["cli_dest"], default=enh["cli_flag_default"],
+            help=enh["cli_help"])
+
+
+@six.add_metaclass(abc.ABCMeta)
+class AutoHSTSEnhancement(object):
+    """
+    Enhancement interface that installer plugins can implement in order to
+    provide functionality that configures the software to have a
+    'Strict-Transport-Security' with initially low max-age value that will
+    increase over time.
+
+    The plugins implementing new style enhancements are responsible of handling
+    the saving of configuration checkpoints as well as calling possible restarts
+    of managed software themselves. For update_autohsts method, the installer may
+    have to call prepare() to finalize the plugin initialization.
+
+    Methods:
+        enable_autohsts is called when the header is initially installed using a
+        low max-age value.
+
+        update_autohsts is called every time when Certbot is run using 'renew'
+        verb. The max-age value should be increased over time using this method.
+
+        deploy_autohsts is called for every lineage that has had its certificate
+        renewed. A long HSTS max-age value should be set here, as we should be
+        confident that the user is able to automatically renew their certificates.
+
+
+    """
+
+    @abc.abstractmethod
+    def update_autohsts(self, lineage, *args, **kwargs):
+        """
+        Gets called for each lineage every time Certbot is run with 'renew' verb.
+        Implementation of this method should increase the max-age value.
+
+        :param lineage: Certificate lineage object
+        :type lineage: certbot.storage.RenewableCert
+
+        .. note:: prepare() method inherited from `interfaces.IPlugin` might need
+            to be called manually within implementation of this interface method
+            to finalize the plugin initialization.
+        """
+
+    @abc.abstractmethod
+    def deploy_autohsts(self, lineage, *args, **kwargs):
+        """
+        Gets called for a lineage when its certificate is successfully renewed.
+        Long max-age value should be set in implementation of this method.
+
+        :param lineage: Certificate lineage object
+        :type lineage: certbot.storage.RenewableCert
+        """
+
+    @abc.abstractmethod
+    def enable_autohsts(self, lineage, domains, *args, **kwargs):
+        """
+        Enables the AutoHSTS enhancement, installing
+        Strict-Transport-Security header with a low initial value to be increased
+        over the subsequent runs of Certbot renew.
+
+        :param lineage: Certificate lineage object
+        :type lineage: certbot.storage.RenewableCert
+
+        :param domains: List of domains in certificate to enhance
+        :type domains: str
+        """
+
+# This is used to configure internal new style enhancements in Certbot. These
+# enhancement interfaces need to be defined in this file. Please do not modify
+# this list from plugin code.
+_INDEX = [
+    {
+        "name": "AutoHSTS",
+        "cli_help": "Gradually increasing max-age value for HTTP Strict Transport "+
+                    "Security security header",
+        "cli_flag": "--auto-hsts",
+        "cli_flag_default": constants.CLI_DEFAULTS["auto_hsts"],
+        "cli_groups": ["security", "enhance"],
+        "cli_dest": "auto_hsts",
+        "cli_action": "store_true",
+        "class": AutoHSTSEnhancement,
+        "updater_function": "update_autohsts",
+        "deployer_function": "deploy_autohsts",
+        "enable_function": "enable_autohsts"
+    }
+]  # type: List[Dict[str, Any]]

certbot/plugins/enhancements_test.py

@@ -0,0 +1,65 @@
+"""Tests for new style enhancements"""
+import unittest
+import mock
+
+from certbot.plugins import enhancements
+from certbot.plugins import null
+
+import certbot.tests.util as test_util
+
+
+class EnhancementTest(test_util.ConfigTestCase):
+    """Tests for new style enhancements in certbot.plugins.enhancements"""
+
+    def setUp(self):
+        super(EnhancementTest, self).setUp()
+        self.mockinstaller = mock.MagicMock(spec=enhancements.AutoHSTSEnhancement)
+
+
+    @test_util.patch_get_utility()
+    def test_enhancement_enabled_enhancements(self, _):
+        FAKEINDEX = [
+            {
+                "name": "autohsts",
+                "cli_dest": "auto_hsts",
+            },
+            {
+                "name": "somethingelse",
+                "cli_dest": "something",
+            }
+        ]
+        with mock.patch("certbot.plugins.enhancements._INDEX", FAKEINDEX):
+            self.config.auto_hsts = True
+            self.config.something = True
+            enabled = list(enhancements.enabled_enhancements(self.config))
+        self.assertEqual(len(enabled), 2)
+        self.assertTrue([i for i in enabled if i["name"] == "autohsts"])
+        self.assertTrue([i for i in enabled if i["name"] == "somethingelse"])
+
+    def test_are_requested(self):
+        self.assertEqual(
+            len([i for i in enhancements.enabled_enhancements(self.config)]), 0)
+        self.assertFalse(enhancements.are_requested(self.config))
+        self.config.auto_hsts = True
+        self.assertEqual(
+            len([i for i in enhancements.enabled_enhancements(self.config)]), 1)
+        self.assertTrue(enhancements.are_requested(self.config))
+
+    def test_are_supported(self):
+        self.config.auto_hsts = True
+        unsupported = null.Installer(self.config, "null")
+        self.assertTrue(enhancements.are_supported(self.config, self.mockinstaller))
+        self.assertFalse(enhancements.are_supported(self.config, unsupported))
+
+    def test_enable(self):
+        self.config.auto_hsts = True
+        domains = ["example.com", "www.example.com"]
+        lineage = "lineage"
+        enhancements.enable(lineage, domains, self.mockinstaller, self.config)
+        self.assertTrue(self.mockinstaller.enable_autohsts.called)
+        self.assertEqual(self.mockinstaller.enable_autohsts.call_args[0],
+                          (lineage, domains))
+
+
+if __name__ == '__main__':
+    unittest.main()  # pragma: no cover

certbot/plugins/manual.py

@@ -5,7 +5,9 @@ import zope.component
 import zope.interface
 
 from acme import challenges
+from acme.magic_typing import Dict  # pylint: disable=unused-import, no-name-in-module
 
+from certbot import achallenges  # pylint: disable=unused-import
 from certbot import interfaces
 from certbot import errors
 from certbot import hooks
@@ -13,34 +15,6 @@ from certbot import reverter
 from certbot.plugins import common
 
 
-class ManualTlsSni01(common.TLSSNI01):
-    """TLS-SNI-01 authenticator for the Manual plugin
-
-    :ivar configurator: Authenticator object
-    :type configurator: :class:`~certbot.plugins.manual.Authenticator`
-
-    :ivar list achalls: Annotated
-        class:`~certbot.achallenges.KeyAuthorizationAnnotatedChallenge`
-        challenges
-
-    :param list indices: Meant to hold indices of challenges in a
-        larger array. NginxTlsSni01 is capable of solving many challenges
-        at once which causes an indexing issue within NginxConfigurator
-        who must return all responses in order.  Imagine NginxConfigurator
-        maintaining state about where all of the http-01 Challenges,
-        TLS-SNI-01 Challenges belong in the response array.  This is an
-        optional utility.
-
-    :param str challenge_conf: location of the challenge config file
-    """
-
-    def perform(self):
-        """Create the SSL certificates and private keys"""
-
-        for achall in self.achalls:
-            self._setup_challenge_cert(achall)
-
-
 @zope.interface.implementer(interfaces.IAuthenticator)
 @zope.interface.provider(interfaces.IPluginFactory)
 class Authenticator(common.Plugin):
@@ -61,14 +35,9 @@ class Authenticator(common.Plugin):
         'type of challenge. $CERTBOT_DOMAIN will always contain the domain '
         'being authenticated. For HTTP-01 and DNS-01, $CERTBOT_VALIDATION '
         'is the validation string, and $CERTBOT_TOKEN is the filename of the '
-        'resource requested when performing an HTTP-01 challenge. When '
-        'performing a TLS-SNI-01 challenge, $CERTBOT_SNI_DOMAIN will contain '
-        'the SNI name for which the ACME server expects to be presented with '
-        'the self-signed certificate located at $CERTBOT_CERT_PATH. The '
-        'secret key needed to complete the TLS handshake is located at '
-        '$CERTBOT_KEY_PATH. An additional cleanup script can also be '
-        'provided and can use the additional variable $CERTBOT_AUTH_OUTPUT '
-        'which contains the stdout output from the auth script.')
+        'resource requested when performing an HTTP-01 challenge. An additional '
+        'cleanup script can also be provided and can use the additional variable '
+        '$CERTBOT_AUTH_OUTPUT which contains the stdout output from the auth script.')
     _DNS_INSTRUCTIONS = """\
 Please deploy a DNS TXT record under the name
 {domain} with the following value:
@@ -85,21 +54,25 @@ And make it available on your web server at this URL:
 
 {uri}
 """
-    _TLSSNI_INSTRUCTIONS = """\
-Configure the service listening on port {port} to present the certificate
-{cert}
-using the secret key
-{key}
-when it receives a TLS ClientHello with the SNI extension set to
-{sni_domain}
+    _SUBSEQUENT_CHALLENGE_INSTRUCTIONS = """
+(This must be set up in addition to the previous challenges; do not remove,
+replace, or undo the previous challenge tasks yet.)
+"""
+    _SUBSEQUENT_DNS_CHALLENGE_INSTRUCTIONS = """
+(This must be set up in addition to the previous challenges; do not remove,
+replace, or undo the previous challenge tasks yet. Note that you might be
+asked to create multiple distinct TXT records with the same name. This is
+permitted by DNS standards.)
 """
 
     def __init__(self, *args, **kwargs):
         super(Authenticator, self).__init__(*args, **kwargs)
         self.reverter = reverter.Reverter(self.config)
         self.reverter.recovery_routine()
-        self.env = dict()
-        self.tls_sni_01 = None
+        self.env = dict() \
+        # type: Dict[achallenges.KeyAuthorizationAnnotatedChallenge, Dict[str, str]]
+        self.subsequent_dns_challenge = False
+        self.subsequent_any_challenge = False
 
     @classmethod
     def add_parser_arguments(cls, add):
@@ -134,7 +107,7 @@ when it receives a TLS ClientHello with the SNI extension set to
 
     def get_chall_pref(self, domain):
         # pylint: disable=missing-docstring,no-self-use,unused-argument
-        return [challenges.HTTP01, challenges.DNS01, challenges.TLSSNI01]
+        return [challenges.HTTP01, challenges.DNS01]
 
     def perform(self, achalls):  # pylint: disable=missing-docstring
         self._verify_ip_logging_ok()
@@ -145,12 +118,6 @@ when it receives a TLS ClientHello with the SNI extension set to
 
         responses = []
         for achall in achalls:
-            if isinstance(achall.chall, challenges.TLSSNI01):
-                # Make a new ManualTlsSni01 instance for each challenge
-                # because the manual plugin deals with one challenge at a time.
-                self.tls_sni_01 = ManualTlsSni01(self)
-                self.tls_sni_01.add_chall(achall)
-                self.tls_sni_01.perform()
             perform_achall(achall)
             responses.append(achall.response(achall.account_key))
         return responses
@@ -176,18 +143,8 @@ when it receives a TLS ClientHello with the SNI extension set to
             env['CERTBOT_TOKEN'] = achall.chall.encode('token')
         else:
             os.environ.pop('CERTBOT_TOKEN', None)
-        if isinstance(achall.chall, challenges.TLSSNI01):
-            env['CERTBOT_CERT_PATH'] = self.tls_sni_01.get_cert_path(achall)
-            env['CERTBOT_KEY_PATH'] = self.tls_sni_01.get_key_path(achall)
-            env['CERTBOT_SNI_DOMAIN'] = self.tls_sni_01.get_z_domain(achall)
-            os.environ.pop('CERTBOT_VALIDATION', None)
-            env.pop('CERTBOT_VALIDATION')
-        else:
-            os.environ.pop('CERTBOT_CERT_PATH', None)
-            os.environ.pop('CERTBOT_KEY_PATH', None)
-            os.environ.pop('CERTBOT_SNI_DOMAIN', None)
         os.environ.update(env)
-        _, out = hooks.execute(self.conf('auth-hook'))
+        _, out = self._execute_hook('auth-hook')
         env['CERTBOT_AUTH_OUTPUT'] = out.strip()
         self.env[achall] = env
 
@@ -198,19 +155,22 @@ when it receives a TLS ClientHello with the SNI extension set to
                 achall=achall, encoded_token=achall.chall.encode('token'),
                 port=self.config.http01_port,
                 uri=achall.chall.uri(achall.domain), validation=validation)
-        elif isinstance(achall.chall, challenges.DNS01):
+        else:
+            assert isinstance(achall.chall, challenges.DNS01)
             msg = self._DNS_INSTRUCTIONS.format(
                 domain=achall.validation_domain_name(achall.domain),
                 validation=validation)
-        else:
-            assert isinstance(achall.chall, challenges.TLSSNI01)
-            msg = self._TLSSNI_INSTRUCTIONS.format(
-                cert=self.tls_sni_01.get_cert_path(achall),
-                key=self.tls_sni_01.get_key_path(achall),
-                port=self.config.tls_sni_01_port,
-                sni_domain=self.tls_sni_01.get_z_domain(achall))
+        if isinstance(achall.chall, challenges.DNS01):
+            if self.subsequent_dns_challenge:
+                # 2nd or later dns-01 challenge
+                msg += self._SUBSEQUENT_DNS_CHALLENGE_INSTRUCTIONS
+            self.subsequent_dns_challenge = True
+        elif self.subsequent_any_challenge:
+            # 2nd or later challenge of another type
+            msg += self._SUBSEQUENT_CHALLENGE_INSTRUCTIONS
         display = zope.component.getUtility(interfaces.IDisplay)
         display.notification(msg, wrap=False, force_interactive=True)
+        self.subsequent_any_challenge = True
 
     def cleanup(self, achalls):  # pylint: disable=missing-docstring
         if self.conf('cleanup-hook'):
@@ -219,5 +179,8 @@ when it receives a TLS ClientHello with the SNI extension set to
                 if 'CERTBOT_TOKEN' not in env:
                     os.environ.pop('CERTBOT_TOKEN', None)
                 os.environ.update(env)
-                hooks.execute(self.conf('cleanup-hook'))
+                self._execute_hook('cleanup-hook')
         self.reverter.recovery_routine()
+
+    def _execute_hook(self, hook_name):
+        return hooks.execute(self.option_name(hook_name), self.conf(hook_name))

certbot/plugins/manual_test.py

@@ -4,6 +4,7 @@ import unittest
 
 import six
 import mock
+import sys
 
 from acme import challenges
 
@@ -20,8 +21,8 @@ class AuthenticatorTest(test_util.TempDirTestCase):
         super(AuthenticatorTest, self).setUp()
         self.http_achall = acme_util.HTTP01_A
         self.dns_achall = acme_util.DNS01_A
-        self.tls_sni_achall = acme_util.TLSSNI01_A
-        self.achalls = [self.http_achall, self.dns_achall, self.tls_sni_achall]
+        self.dns_achall_2 = acme_util.DNS01_A_2
+        self.achalls = [self.http_achall, self.dns_achall, self.dns_achall_2]
         for d in ["config_dir", "work_dir", "in_progress"]:
             os.mkdir(os.path.join(self.tempdir, d))
             # "backup_dir" and "temp_checkpoint_dir" get created in
@@ -36,8 +37,7 @@ class AuthenticatorTest(test_util.TempDirTestCase):
             backup_dir=os.path.join(self.tempdir, "backup_dir"),
             temp_checkpoint_dir=os.path.join(
                                         self.tempdir, "temp_checkpoint_dir"),
-            in_progress_dir=os.path.join(self.tempdir, "in_progess"),
-            tls_sni_01_port=5001)
+            in_progress_dir=os.path.join(self.tempdir, "in_progess"))
 
         from certbot.plugins.manual import Authenticator
         self.auth = Authenticator(self.config, name='manual')
@@ -56,9 +56,7 @@ class AuthenticatorTest(test_util.TempDirTestCase):
 
     def test_get_chall_pref(self):
         self.assertEqual(self.auth.get_chall_pref('example.org'),
-                         [challenges.HTTP01,
-                          challenges.DNS01,
-                          challenges.TLSSNI01])
+                         [challenges.HTTP01, challenges.DNS01])
 
     @test_util.patch_get_utility()
     def test_ip_logging_not_ok(self, mock_get_utility):
@@ -74,19 +72,16 @@ class AuthenticatorTest(test_util.TempDirTestCase):
     def test_script_perform(self):
         self.config.manual_public_ip_logging_ok = True
         self.config.manual_auth_hook = (
-            'echo ${CERTBOT_DOMAIN}; '
-            'echo ${CERTBOT_TOKEN:-notoken}; '
-            'echo ${CERTBOT_CERT_PATH:-nocert}; '
-            'echo ${CERTBOT_KEY_PATH:-nokey}; '
-            'echo ${CERTBOT_SNI_DOMAIN:-nosnidomain}; '
-            'echo ${CERTBOT_VALIDATION:-novalidation};')
-        dns_expected = '{0}\n{1}\n{2}\n{3}\n{4}\n{5}'.format(
+            '{0} -c "from __future__ import print_function;'
+            'import os;  print(os.environ.get(\'CERTBOT_DOMAIN\'));'
+            'print(os.environ.get(\'CERTBOT_TOKEN\', \'notoken\'));'
+            'print(os.environ.get(\'CERTBOT_VALIDATION\', \'novalidation\'));"'
+            .format(sys.executable))
+        dns_expected = '{0}\n{1}\n{2}'.format(
             self.dns_achall.domain, 'notoken',
-            'nocert', 'nokey', 'nosnidomain',
             self.dns_achall.validation(self.dns_achall.account_key))
-        http_expected = '{0}\n{1}\n{2}\n{3}\n{4}\n{5}'.format(
+        http_expected = '{0}\n{1}\n{2}'.format(
             self.http_achall.domain, self.http_achall.chall.encode('token'),
-            'nocert', 'nokey', 'nosnidomain',
             self.http_achall.validation(self.http_achall.account_key))
 
         self.assertEqual(
@@ -98,17 +93,6 @@ class AuthenticatorTest(test_util.TempDirTestCase):
         self.assertEqual(
             self.auth.env[self.http_achall]['CERTBOT_AUTH_OUTPUT'],
             http_expected)
-        # tls_sni_01 challenge must be perform()ed above before we can
-        # get the cert_path and key_path.
-        tls_sni_expected = '{0}\n{1}\n{2}\n{3}\n{4}\n{5}'.format(
-            self.tls_sni_achall.domain, 'notoken',
-            self.auth.tls_sni_01.get_cert_path(self.tls_sni_achall),
-            self.auth.tls_sni_01.get_key_path(self.tls_sni_achall),
-            self.auth.tls_sni_01.get_z_domain(self.tls_sni_achall),
-            'novalidation')
-        self.assertEqual(
-            self.auth.env[self.tls_sni_achall]['CERTBOT_AUTH_OUTPUT'],
-            tls_sni_expected)
 
     @test_util.patch_get_utility()
     def test_manual_perform(self, mock_get_utility):
@@ -118,18 +102,14 @@ class AuthenticatorTest(test_util.TempDirTestCase):
             [achall.response(achall.account_key) for achall in self.achalls])
         for i, (args, kwargs) in enumerate(mock_get_utility().notification.call_args_list):
             achall = self.achalls[i]
-            if isinstance(achall.chall, challenges.TLSSNI01):
-                self.assertTrue(
-                        self.auth.tls_sni_01.get_cert_path(
-                            self.tls_sni_achall) in args[0])
-            else:
-                self.assertTrue(
-                        achall.validation(achall.account_key) in args[0])
+            self.assertTrue(
+                achall.validation(achall.account_key) in args[0])
             self.assertFalse(kwargs['wrap'])
 
     def test_cleanup(self):
         self.config.manual_public_ip_logging_ok = True
-        self.config.manual_auth_hook = 'echo foo;'
+        self.config.manual_auth_hook = ('{0} -c "import sys; sys.stdout.write(\'foo\')"'
+                                        .format(sys.executable))
         self.config.manual_cleanup_hook = '# cleanup'
         self.auth.perform(self.achalls)
 
@@ -147,18 +127,6 @@ class AuthenticatorTest(test_util.TempDirTestCase):
                     achall.chall.encode('token'))
             else:
                 self.assertFalse('CERTBOT_TOKEN' in os.environ)
-            if isinstance(achall.chall, challenges.TLSSNI01):
-                self.assertEqual(
-                    os.environ['CERTBOT_CERT_PATH'],
-                    self.auth.tls_sni_01.get_cert_path(achall))
-                self.assertEqual(
-                    os.environ['CERTBOT_KEY_PATH'],
-                    self.auth.tls_sni_01.get_key_path(achall))
-                self.assertFalse(
-                    os.path.exists(os.environ['CERTBOT_CERT_PATH']))
-                self.assertFalse(
-                    os.path.exists(os.environ['CERTBOT_KEY_PATH']))
-
 
 
 if __name__ == '__main__':

certbot/plugins/selection.py

@@ -39,6 +39,35 @@ def pick_authenticator(
     return pick_plugin(
         config, default, plugins, question, (interfaces.IAuthenticator,))
 
+def get_unprepared_installer(config, plugins):
+    """
+    Get an unprepared interfaces.IInstaller object.
+
+    :param certbot.interfaces.IConfig config: Configuration
+    :param certbot.plugins.disco.PluginsRegistry plugins:
+        All plugins registered as entry points.
+
+    :returns: Unprepared installer plugin or None
+    :rtype: IPlugin or None
+    """
+
+    _, req_inst = cli_plugin_requests(config)
+    if not req_inst:
+        return None
+    installers = plugins.filter(lambda p_ep: p_ep.name == req_inst)
+    installers.init(config)
+    installers = installers.verify((interfaces.IInstaller,))
+    if len(installers) > 1:
+        raise errors.PluginSelectionError(
+            "Found multiple installers with the name %s, Certbot is unable to "
+            "determine which one to use. Skipping." % req_inst)
+    if installers:
+        inst = list(installers.values())[0]
+        logger.debug("Selecting plugin: %s", inst)
+        return inst.init(config)
+    else:
+        raise errors.PluginSelectionError(
+            "Could not select or initialize the requested installer %s." % req_inst)
 
 def pick_plugin(config, default, plugins, question, ifaces):
     """Pick plugin.
@@ -134,13 +163,14 @@ def choose_plugin(prepared, question):
             return None
 
 noninstaller_plugins = ["webroot", "manual", "standalone", "dns-cloudflare", "dns-cloudxns",
-                        "dns-digitalocean", "dns-dnsimple", "dns-dnsmadeeasy", "dns-google",
-                        "dns-luadns", "dns-nsone", "dns-rfc2136", "dns-route53"]
+                        "dns-digitalocean", "dns-dnsimple", "dns-dnsmadeeasy", "dns-gehirn",
+                        "dns-google", "dns-linode", "dns-luadns", "dns-nsone", "dns-ovh",
+                        "dns-rfc2136", "dns-route53", "dns-sakuracloud"]
 
 def record_chosen_plugins(config, plugins, auth, inst):
     "Update the config entries to reflect the plugins we actually selected."
-    config.authenticator = plugins.find_init(auth).name if auth else "None"
-    config.installer = plugins.find_init(inst).name if inst else "None"
+    config.authenticator = plugins.find_init(auth).name if auth else None
+    config.installer = plugins.find_init(inst).name if inst else None
     logger.info("Plugins selected: Authenticator %s, Installer %s",
          config.authenticator, config.installer)
 
@@ -258,16 +288,24 @@ def cli_plugin_requests(config):  # pylint: disable=too-many-branches
         req_auth = set_configurator(req_auth, "dns-dnsimple")
     if config.dns_dnsmadeeasy:
         req_auth = set_configurator(req_auth, "dns-dnsmadeeasy")
+    if config.dns_gehirn:
+        req_auth = set_configurator(req_auth, "dns-gehirn")
     if config.dns_google:
         req_auth = set_configurator(req_auth, "dns-google")
+    if config.dns_linode:
+        req_auth = set_configurator(req_auth, "dns-linode")
     if config.dns_luadns:
         req_auth = set_configurator(req_auth, "dns-luadns")
     if config.dns_nsone:
         req_auth = set_configurator(req_auth, "dns-nsone")
+    if config.dns_ovh:
+        req_auth = set_configurator(req_auth, "dns-ovh")
     if config.dns_rfc2136:
         req_auth = set_configurator(req_auth, "dns-rfc2136")
     if config.dns_route53:
         req_auth = set_configurator(req_auth, "dns-route53")
+    if config.dns_sakuracloud:
+        req_auth = set_configurator(req_auth, "dns-sakuracloud")
     logger.debug("Requested authenticator %s and installer %s", req_auth, req_inst)
     return req_auth, req_inst
 

certbot/plugins/selection_test.py

@@ -6,10 +6,14 @@ import unittest
 import mock
 import zope.component
 
-from certbot.display import util as display_util
-from certbot.tests import util as test_util
+from certbot import errors
 from certbot import interfaces
 
+from acme.magic_typing import List  # pylint: disable=unused-import, no-name-in-module
+from certbot.display import util as display_util
+from certbot.plugins.disco import PluginsRegistry
+from certbot.tests import util as test_util
+
 
 class ConveniencePickPluginTest(unittest.TestCase):
     """Tests for certbot.plugins.selection.pick_*."""
@@ -47,7 +51,7 @@ class PickPluginTest(unittest.TestCase):
         self.default = None
         self.reg = mock.MagicMock()
         self.question = "Question?"
-        self.ifaces = []
+        self.ifaces = []  # type: List[interfaces.IPlugin]
 
     def _call(self):
         from certbot.plugins.selection import pick_plugin
@@ -169,5 +173,48 @@ class ChoosePluginTest(unittest.TestCase):
 
         self.assertTrue("default" in mock_util().menu.call_args[1])
 
+class GetUnpreparedInstallerTest(test_util.ConfigTestCase):
+    """Tests for certbot.plugins.selection.get_unprepared_installer."""
+
+    def setUp(self):
+        super(GetUnpreparedInstallerTest, self).setUp()
+        self.mock_apache_fail_ep = mock.Mock(
+            description_with_name="afail")
+        self.mock_apache_fail_ep.name = "afail"
+        self.mock_apache_ep = mock.Mock(
+            description_with_name="apache")
+        self.mock_apache_ep.name = "apache"
+        self.mock_apache_plugin = mock.MagicMock()
+        self.mock_apache_ep.init.return_value = self.mock_apache_plugin
+        self.plugins = PluginsRegistry({
+            "afail": self.mock_apache_fail_ep,
+            "apache": self.mock_apache_ep,
+        })
+
+    def _call(self):
+        from certbot.plugins.selection import get_unprepared_installer
+        return get_unprepared_installer(self.config, self.plugins)
+
+    def test_no_installer_defined(self):
+        self.config.configurator = None
+        self.assertEqual(self._call(), None)
+
+    def test_no_available_installers(self):
+        self.config.configurator = "apache"
+        self.plugins = PluginsRegistry({})
+        self.assertRaises(errors.PluginSelectionError, self._call)
+
+    def test_get_plugin(self):
+        self.config.configurator = "apache"
+        installer = self._call()
+        self.assertTrue(installer is self.mock_apache_plugin)
+
+    def test_multiple_installers_returned(self):
+        self.config.configurator = "apache"
+        # Two plugins with the same name
+        self.mock_apache_fail_ep.name = "apache"
+        self.assertRaises(errors.PluginSelectionError, self._call)
+
+
 if __name__ == "__main__":
     unittest.main()  # pragma: no cover

certbot/plugins/standalone.py

@@ -1,16 +1,20 @@
 """Standalone Authenticator."""
-import argparse
 import collections
 import logging
 import socket
+# https://github.com/python/typeshed/blob/master/stdlib/2and3/socket.pyi
+from socket import errno as socket_errors  # type: ignore
 
-import OpenSSL
+import OpenSSL  # pylint: disable=unused-import
 import six
 import zope.interface
 
 from acme import challenges
 from acme import standalone as acme_standalone
+# pylint: disable=unused-import, no-name-in-module
+from acme.magic_typing import DefaultDict, Dict, Set, Tuple, List, Type, TYPE_CHECKING
 
+from certbot import achallenges  # pylint: disable=unused-import
 from certbot import errors
 from certbot import interfaces
 
@@ -18,6 +22,11 @@ from certbot.plugins import common
 
 logger = logging.getLogger(__name__)
 
+if TYPE_CHECKING:
+    ServedType = DefaultDict[
+        acme_standalone.BaseDualNetworkedServers,
+        Set[achallenges.KeyAuthorizationAnnotatedChallenge]
+    ]
 
 class ServerManager(object):
     """Standalone servers manager.
@@ -33,7 +42,7 @@ class ServerManager(object):
 
     """
     def __init__(self, certs, http_01_resources):
-        self._instances = {}
+        self._instances = {}  # type: Dict[int, acme_standalone.BaseDualNetworkedServers]
         self.certs = certs
         self.http_01_resources = http_01_resources
 
@@ -45,24 +54,21 @@ class ServerManager(object):
 
         :param int port: Port to run the server on.
         :param challenge_type: Subclass of `acme.challenges.Challenge`,
-            either `acme.challenge.HTTP01` or `acme.challenges.TLSSNI01`.
+            currently only `acme.challenge.HTTP01`.
         :param str listenaddr: (optional) The address to listen on. Defaults to all addrs.
 
         :returns: DualNetworkedServers instance.
         :rtype: ACMEServerMixin
 
         """
-        assert challenge_type in (challenges.TLSSNI01, challenges.HTTP01)
+        assert challenge_type == challenges.HTTP01
         if port in self._instances:
             return self._instances[port]
 
         address = (listenaddr, port)
         try:
-            if challenge_type is challenges.TLSSNI01:
-                servers = acme_standalone.TLSSNI01DualNetworkedServers(address, self.certs)
-            else:  # challenges.HTTP01
-                servers = acme_standalone.HTTP01DualNetworkedServers(
-                    address, self.http_01_resources)
+            servers = acme_standalone.HTTP01DualNetworkedServers(
+                address, self.http_01_resources)
         except socket.error as error:
             raise errors.StandaloneBindError(error, port)
 
@@ -85,8 +91,6 @@ class ServerManager(object):
         for sockname in instance.getsocknames():
             logger.debug("Stopping server at %s:%d...",
                          *sockname[:2])
-        # Not calling server_close causes problems when renewing multiple
-        # certs with `certbot renew` using TLSSNI01 and PyOpenSSL 0.13
         instance.shutdown_and_server_close()
         del self._instances[port]
 
@@ -103,69 +107,13 @@ class ServerManager(object):
         return self._instances.copy()
 
 
-SUPPORTED_CHALLENGES = [challenges.TLSSNI01, challenges.HTTP01]
-
-
-class SupportedChallengesAction(argparse.Action):
-    """Action class for parsing standalone_supported_challenges."""
-
-    def __call__(self, parser, namespace, values, option_string=None):
-        logger.warning(
-            "The standalone specific supported challenges flag is "
-            "deprecated. Please use the --preferred-challenges flag "
-            "instead.")
-        converted_values = self._convert_and_validate(values)
-        namespace.standalone_supported_challenges = converted_values
-
-    def _convert_and_validate(self, data):
-        """Validate the value of supported challenges provided by the user.
-
-        References to "dvsni" are automatically converted to "tls-sni-01".
-
-        :param str data: comma delimited list of challenge types
-
-        :returns: validated and converted list of challenge types
-        :rtype: str
-
-        """
-        challs = data.split(",")
-
-        # tls-sni-01 was dvsni during private beta
-        if "dvsni" in challs:
-            logger.info(
-                "Updating legacy standalone_supported_challenges value")
-            challs = [challenges.TLSSNI01.typ if chall == "dvsni" else chall
-                      for chall in challs]
-            data = ",".join(challs)
-
-        unrecognized = [name for name in challs
-                        if name not in challenges.Challenge.TYPES]
-
-        # argparse.ArgumentErrors raised out of argparse.Action objects
-        # are caught by argparse which prints usage information and the
-        # error that occurred before calling sys.exit.
-        if unrecognized:
-            raise argparse.ArgumentError(
-                self,
-                "Unrecognized challenges: {0}".format(", ".join(unrecognized)))
-
-        choices = set(chall.typ for chall in SUPPORTED_CHALLENGES)
-        if not set(challs).issubset(choices):
-            raise argparse.ArgumentError(
-                self,
-                "Plugin does not support the following (valid) "
-                "challenges: {0}".format(", ".join(set(challs) - choices)))
-
-        return data
-
-
 @zope.interface.implementer(interfaces.IAuthenticator)
 @zope.interface.provider(interfaces.IPluginFactory)
 class Authenticator(common.Plugin):
     """Standalone Authenticator.
 
     This authenticator creates its own ephemeral TCP listener on the
-    necessary port in order to respond to incoming tls-sni-01 and http-01
+    necessary port in order to respond to incoming http-01
     challenges from the certificate authority. Therefore, it does not
     rely on any existing server program.
     """
@@ -175,47 +123,34 @@ class Authenticator(common.Plugin):
     def __init__(self, *args, **kwargs):
         super(Authenticator, self).__init__(*args, **kwargs)
 
-        # one self-signed key for all tls-sni-01 certificates
-        self.key = OpenSSL.crypto.PKey()
-        self.key.generate_key(OpenSSL.crypto.TYPE_RSA, 2048)
-
-        self.served = collections.defaultdict(set)
+        self.served = collections.defaultdict(set)  # type: ServedType
 
         # Stuff below is shared across threads (i.e. servers read
         # values, main thread writes). Due to the nature of CPython's
         # GIL, the operations are safe, c.f.
         # https://docs.python.org/2/faq/library.html#what-kinds-of-global-value-mutation-are-thread-safe
-        self.certs = {}
-        self.http_01_resources = set()
+        self.certs = {}  # type: Dict[bytes, Tuple[OpenSSL.crypto.PKey, OpenSSL.crypto.X509]]
+        self.http_01_resources = set() \
+        # type: Set[acme_standalone.HTTP01RequestHandler.HTTP01Resource]
 
         self.servers = ServerManager(self.certs, self.http_01_resources)
 
     @classmethod
     def add_parser_arguments(cls, add):
-        add("supported-challenges",
-            help=argparse.SUPPRESS,
-            action=SupportedChallengesAction,
-            default=",".join(chall.typ for chall in SUPPORTED_CHALLENGES))
-
-    @property
-    def supported_challenges(self):
-        """Challenges supported by this plugin."""
-        return [challenges.Challenge.TYPES[name] for name in
-                self.conf("supported-challenges").split(",")]
+        pass  # No additional argument for the standalone plugin parser
 
     def more_info(self):  # pylint: disable=missing-docstring
         return("This authenticator creates its own ephemeral TCP listener "
                "on the necessary port in order to respond to incoming "
-               "tls-sni-01 and http-01 challenges from the certificate "
-               "authority. Therefore, it does not rely on any existing "
-               "server program.")
+               "http-01 challenges from the certificate authority. Therefore, "
+               "it does not rely on any existing server program.")
 
     def prepare(self):  # pylint: disable=missing-docstring
         pass
 
     def get_chall_pref(self, domain):
         # pylint: disable=unused-argument,missing-docstring
-        return self.supported_challenges
+        return [challenges.HTTP01]
 
     def perform(self, achalls):  # pylint: disable=missing-docstring
         return [self._try_perform_single(achall) for achall in achalls]
@@ -225,14 +160,10 @@ class Authenticator(common.Plugin):
             try:
                 return self._perform_single(achall)
             except errors.StandaloneBindError as error:
-                if not _handle_perform_error(error):
-                    raise
+                _handle_perform_error(error)
 
     def _perform_single(self, achall):
-        if isinstance(achall.chall, challenges.HTTP01):
-            servers, response = self._perform_http_01(achall)
-        else:  # tls-sni-01
-            servers, response = self._perform_tls_sni_01(achall)
+        servers, response = self._perform_http_01(achall)
         self.served[servers].add(achall)
         return response
 
@@ -246,14 +177,6 @@ class Authenticator(common.Plugin):
         self.http_01_resources.add(resource)
         return servers, response
 
-    def _perform_tls_sni_01(self, achall):
-        port = self.config.tls_sni_01_port
-        addr = self.config.tls_sni_01_address
-        servers = self.servers.run(port, challenges.TLSSNI01, listenaddr=addr)
-        response, (cert, _) = achall.response_and_validation(cert_key=self.key)
-        self.certs[response.z_domain] = (self.key, cert)
-        return servers, response
-
     def cleanup(self, achalls):  # pylint: disable=missing-docstring
         # reduce self.served and close servers if no challenges are served
         for unused_servers, server_achalls in self.served.items():
@@ -266,13 +189,13 @@ class Authenticator(common.Plugin):
 
 
 def _handle_perform_error(error):
-    if error.socket_error.errno == socket.errno.EACCES:
+    if error.socket_error.errno == socket_errors.EACCES:
         raise errors.PluginError(
             "Could not bind TCP port {0} because you don't have "
             "the appropriate permissions (for example, you "
             "aren't running this program as "
             "root).".format(error.port))
-    elif error.socket_error.errno == socket.errno.EADDRINUSE:
+    elif error.socket_error.errno == socket_errors.EADDRINUSE:
         display = zope.component.getUtility(interfaces.IDisplay)
         msg = (
             "Could not bind TCP port {0} because it is already in "
@@ -283,5 +206,5 @@ def _handle_perform_error(error):
                                      "Cancel", default=False)
         if not should_retry:
             raise errors.PluginError(msg)
-        return True
-    return False
+    else:
+        raise error

certbot/plugins/standalone_test.py

@@ -1,13 +1,18 @@
 """Tests for certbot.plugins.standalone."""
-import argparse
 import socket
 import unittest
+# https://github.com/python/typeshed/blob/master/stdlib/2and3/socket.pyi
+from socket import errno as socket_errors  # type: ignore
 
 import josepy as jose
 import mock
 import six
 
+import OpenSSL.crypto  # pylint: disable=unused-import
+
 from acme import challenges
+from acme import standalone as acme_standalone  # pylint: disable=unused-import
+from acme.magic_typing import Dict, Tuple, Set  # pylint: disable=unused-import, no-name-in-module
 
 from certbot import achallenges
 from certbot import errors
@@ -21,8 +26,9 @@ class ServerManagerTest(unittest.TestCase):
 
     def setUp(self):
         from certbot.plugins.standalone import ServerManager
-        self.certs = {}
-        self.http_01_resources = {}
+        self.certs = {}  # type: Dict[bytes, Tuple[OpenSSL.crypto.PKey, OpenSSL.crypto.X509]]
+        self.http_01_resources = {} \
+        # type: Set[acme_standalone.HTTP01RequestHandler.HTTP01Resource]
         self.mgr = ServerManager(self.certs, self.http_01_resources)
 
     def test_init(self):
@@ -37,9 +43,6 @@ class ServerManagerTest(unittest.TestCase):
         self.mgr.stop(port=port)
         self.assertEqual(self.mgr.running(), {})
 
-    def test_run_stop_tls_sni_01(self):
-        self._test_run_stop(challenges.TLSSNI01)
-
     def test_run_stop_http_01(self):
         self._test_run_stop(challenges.HTTP01)
 
@@ -65,46 +68,8 @@ class ServerManagerTest(unittest.TestCase):
             errors.StandaloneBindError, self.mgr.run, port,
             challenge_type=challenges.HTTP01)
         self.assertEqual(self.mgr.running(), {})
-
-
-class SupportedChallengesActionTest(unittest.TestCase):
-    """Tests for plugins.standalone.SupportedChallengesAction."""
-
-    def _call(self, value):
-        with mock.patch("certbot.plugins.standalone.logger") as mock_logger:
-            # stderr is mocked to prevent potential argparse error
-            # output from cluttering test output
-            with mock.patch("sys.stderr"):
-                config = self.parser.parse_args([self.flag, value])
-
-        self.assertTrue(mock_logger.warning.called)
-        return getattr(config, self.dest)
-
-    def setUp(self):
-        self.flag = "--standalone-supported-challenges"
-        self.dest = self.flag[2:].replace("-", "_")
-        self.parser = argparse.ArgumentParser()
-
-        from certbot.plugins.standalone import SupportedChallengesAction
-        self.parser.add_argument(self.flag, action=SupportedChallengesAction)
-
-    def test_correct(self):
-        self.assertEqual("tls-sni-01", self._call("tls-sni-01"))
-        self.assertEqual("http-01", self._call("http-01"))
-        self.assertEqual("tls-sni-01,http-01", self._call("tls-sni-01,http-01"))
-        self.assertEqual("http-01,tls-sni-01", self._call("http-01,tls-sni-01"))
-
-    def test_unrecognized(self):
-        assert "foo" not in challenges.Challenge.TYPES
-        self.assertRaises(SystemExit, self._call, "foo")
-
-    def test_not_subset(self):
-        self.assertRaises(SystemExit, self._call, "dns")
-
-    def test_dvsni(self):
-        self.assertEqual("tls-sni-01", self._call("dvsni"))
-        self.assertEqual("http-01,tls-sni-01", self._call("http-01,dvsni"))
-        self.assertEqual("tls-sni-01,http-01", self._call("dvsni,http-01"))
+        some_server.close()
+        maybe_another_server.close()
 
 
 def get_open_port():
@@ -122,32 +87,16 @@ class AuthenticatorTest(unittest.TestCase):
     def setUp(self):
         from certbot.plugins.standalone import Authenticator
 
-        self.config = mock.MagicMock(
-            tls_sni_01_port=get_open_port(), http01_port=get_open_port(),
-            standalone_supported_challenges="tls-sni-01,http-01")
+        self.config = mock.MagicMock(http01_port=get_open_port())
         self.auth = Authenticator(self.config, name="standalone")
         self.auth.servers = mock.MagicMock()
 
-    def test_supported_challenges(self):
-        self.assertEqual(self.auth.supported_challenges,
-                         [challenges.TLSSNI01, challenges.HTTP01])
-
-    def test_supported_challenges_configured(self):
-        self.config.standalone_supported_challenges = "tls-sni-01"
-        self.assertEqual(self.auth.supported_challenges,
-                         [challenges.TLSSNI01])
-
     def test_more_info(self):
         self.assertTrue(isinstance(self.auth.more_info(), six.string_types))
 
     def test_get_chall_pref(self):
         self.assertEqual(self.auth.get_chall_pref(domain=None),
-                         [challenges.TLSSNI01, challenges.HTTP01])
-
-    def test_get_chall_pref_configured(self):
-        self.config.standalone_supported_challenges = "tls-sni-01"
-        self.assertEqual(self.auth.get_chall_pref(domain=None),
-                         [challenges.TLSSNI01])
+                         [challenges.HTTP01])
 
     def test_perform(self):
         achalls = self._get_achalls()
@@ -159,7 +108,7 @@ class AuthenticatorTest(unittest.TestCase):
     @test_util.patch_get_utility()
     def test_perform_eaddrinuse_retry(self, mock_get_utility):
         mock_utility = mock_get_utility()
-        errno = socket.errno.EADDRINUSE
+        errno = socket_errors.EADDRINUSE
         error = errors.StandaloneBindError(mock.MagicMock(errno=errno), -1)
         self.auth.servers.run.side_effect = [error] + 2 * [mock.MagicMock()]
         mock_yesno = mock_utility.yesno
@@ -174,7 +123,7 @@ class AuthenticatorTest(unittest.TestCase):
         mock_yesno = mock_utility.yesno
         mock_yesno.return_value = False
 
-        errno = socket.errno.EADDRINUSE
+        errno = socket_errors.EADDRINUSE
         self.assertRaises(errors.PluginError, self._fail_perform, errno)
         self._assert_correct_yesno_call(mock_yesno)
 
@@ -184,11 +133,11 @@ class AuthenticatorTest(unittest.TestCase):
         self.assertFalse(yesno_kwargs.get("default", True))
 
     def test_perform_eacces(self):
-        errno = socket.errno.EACCES
+        errno = socket_errors.EACCES
         self.assertRaises(errors.PluginError, self._fail_perform, errno)
 
     def test_perform_unexpected_socket_error(self):
-        errno = socket.errno.ENOTCONN
+        errno = socket_errors.ENOTCONN
         self.assertRaises(
             errors.StandaloneBindError, self._fail_perform, errno)
 
@@ -203,10 +152,8 @@ class AuthenticatorTest(unittest.TestCase):
         key = jose.JWK.load(test_util.load_vector('rsa512_key.pem'))
         http_01 = achallenges.KeyAuthorizationAnnotatedChallenge(
             challb=acme_util.HTTP01_P, domain=domain, account_key=key)
-        tls_sni_01 = achallenges.KeyAuthorizationAnnotatedChallenge(
-            challb=acme_util.TLSSNI01_P, domain=domain, account_key=key)
 
-        return [http_01, tls_sni_01]
+        return [http_01]
 
     def test_cleanup(self):
         self.auth.servers.running.return_value = {
@@ -234,5 +181,6 @@ class AuthenticatorTest(unittest.TestCase):
             "server1": set(), "server2": set([])})
         self.auth.servers.stop.assert_called_with(2)
 
+
 if __name__ == "__main__":
     unittest.main()  # pragma: no cover

certbot/plugins/storage.py

@@ -3,6 +3,7 @@ import json
 import logging
 import os
 
+from acme.magic_typing import Any, Dict  # pylint: disable=unused-import, no-name-in-module
 from certbot import errors
 
 logger = logging.getLogger(__name__)
@@ -38,7 +39,7 @@ class PluginStorage(object):
 
         :raises .errors.PluginStorageError: when unable to open or read the file
         """
-        data = dict()
+        data = dict()  # type: Dict[str, Any]
         filedata = ""
         try:
             with open(self._storagepath, 'r') as fh:
@@ -83,7 +84,8 @@ class PluginStorage(object):
             raise errors.PluginStorageError(errmsg)
         try:
             with os.fdopen(os.open(self._storagepath,
-                                   os.O_WRONLY | os.O_CREAT, 0o600), 'w') as fh:
+                                   os.O_WRONLY | os.O_CREAT | os.O_TRUNC,
+                                   0o600), 'w') as fh:
                 fh.write(serialized)
         except IOError as e:
             errmsg = "Could not write PluginStorage data to file {0} : {1}".format(

certbot/plugins/util.py

@@ -9,18 +9,19 @@ logger = logging.getLogger(__name__)
 def get_prefixes(path):
     """Retrieves all possible path prefixes of a path, in descending order
     of length. For instance,
-        /a/b/c/ => ['/a/b/c/', '/a/b/c', '/a/b', '/a', '/']
+        (linux) /a/b/c returns ['/a/b/c', '/a/b', '/a', '/']
+        (windows) C:\\a\\b\\c returns ['C:\\a\\b\\c', 'C:\\a\\b', 'C:\\a', 'C:']
     :param str path: the path to break into prefixes
 
     :returns: all possible path prefixes of given path in descending order
     :rtype: `list` of `str`
     """
-    prefix = path
+    prefix = os.path.normpath(path)
     prefixes = []
     while prefix:
         prefixes.append(prefix)
         prefix, _ = os.path.split(prefix)
-        # break once we hit '/'
+        # break once we hit the root path
         if prefix == prefixes[-1]:
             break
     return prefixes
@@ -49,7 +50,8 @@ def path_surgery(cmd):
 
     if util.exe_exists(cmd):
         return True
-    expanded = " expanded" if any(added) else ""
-    logger.warning("Failed to find executable %s in%s PATH: %s", cmd,
-                   expanded, path)
-    return False
+    else:
+        expanded = " expanded" if any(added) else ""
+        logger.debug("Failed to find executable %s in%s PATH: %s", cmd,
+                     expanded, path)
+        return False

certbot/plugins/util_test.py

@@ -4,21 +4,21 @@ import unittest
 
 import mock
 
-
 class GetPrefixTest(unittest.TestCase):
     """Tests for certbot.plugins.get_prefixes."""
     def test_get_prefix(self):
         from certbot.plugins.util import get_prefixes
-        self.assertEqual(get_prefixes("/a/b/c/"), ['/a/b/c/', '/a/b/c', '/a/b', '/a', '/'])
-        self.assertEqual(get_prefixes("/"), ["/"])
-        self.assertEqual(get_prefixes("a"), ["a"])
+        self.assertEqual(
+            get_prefixes('/a/b/c'),
+            [os.path.normpath(path) for path in ['/a/b/c', '/a/b', '/a', '/']])
+        self.assertEqual(get_prefixes('/'), [os.path.normpath('/')])
+        self.assertEqual(get_prefixes('a'), ['a'])
 
 class PathSurgeryTest(unittest.TestCase):
     """Tests for certbot.plugins.path_surgery."""
 
-    @mock.patch("certbot.plugins.util.logger.warning")
     @mock.patch("certbot.plugins.util.logger.debug")
-    def test_path_surgery(self, mock_debug, mock_warn):
+    def test_path_surgery(self, mock_debug):
         from certbot.plugins.util import path_surgery
         all_path = {"PATH": "/usr/local/bin:/bin/:/usr/sbin/:/usr/local/sbin/"}
         with mock.patch.dict('os.environ', all_path):
@@ -26,14 +26,12 @@ class PathSurgeryTest(unittest.TestCase):
                 mock_exists.return_value = True
                 self.assertEqual(path_surgery("eg"), True)
                 self.assertEqual(mock_debug.call_count, 0)
-                self.assertEqual(mock_warn.call_count, 0)
                 self.assertEqual(os.environ["PATH"], all_path["PATH"])
         no_path = {"PATH": "/tmp/"}
         with mock.patch.dict('os.environ', no_path):
             path_surgery("thingy")
-            self.assertEqual(mock_debug.call_count, 1)
-            self.assertEqual(mock_warn.call_count, 1)
-            self.assertTrue("Failed to find" in mock_warn.call_args[0][0])
+            self.assertEqual(mock_debug.call_count, 2)
+            self.assertTrue("Failed to find" in mock_debug.call_args[0][0])
             self.assertTrue("/usr/local/bin" in os.environ["PATH"])
             self.assertTrue("/tmp" in os.environ["PATH"])
 

certbot/plugins/webroot.py

@@ -10,8 +10,12 @@ import six
 import zope.component
 import zope.interface
 
-from acme import challenges
+from acme import challenges  # pylint: disable=unused-import
+# pylint: disable=unused-import, no-name-in-module
+from acme.magic_typing import Dict, Set, DefaultDict, List
+# pylint: enable=unused-import, no-name-in-module
 
+from certbot import achallenges  # pylint: disable=unused-import
 from certbot import cli
 from certbot import errors
 from certbot import interfaces
@@ -64,10 +68,11 @@ to serve all files under specified web root ({0})."""
 
     def __init__(self, *args, **kwargs):
         super(Authenticator, self).__init__(*args, **kwargs)
-        self.full_roots = {}
-        self.performed = collections.defaultdict(set)
+        self.full_roots = {}  # type: Dict[str, str]
+        self.performed = collections.defaultdict(set) \
+        # type: DefaultDict[str, Set[achallenges.KeyAuthorizationAnnotatedChallenge]]
         # stack of dirs successfully created by this authenticator
-        self._created_dirs = []
+        self._created_dirs = []  # type: List[str]
 
     def prepare(self):  # pylint: disable=missing-docstring
         pass
@@ -156,7 +161,6 @@ to serve all files under specified web root ({0})."""
                 " --help webroot for examples.")
         for name, path in path_map.items():
             self.full_roots[name] = os.path.join(path, challenges.HTTP01.URI_ROOT_PATH)
-
             logger.debug("Creating root challenges validation dir at %s",
                          self.full_roots[name])
 
@@ -166,7 +170,9 @@ to serve all files under specified web root ({0})."""
             old_umask = os.umask(0o022)
             try:
                 stat_path = os.stat(path)
-                for prefix in sorted(util.get_prefixes(self.full_roots[name]), key=len):
+                # We ignore the last prefix in the next iteration,
+                # as it does not correspond to a folder path ('/' or 'C:')
+                for prefix in sorted(util.get_prefixes(self.full_roots[name])[:-1], key=len):
                     try:
                         # This is coupled with the "umask" call above because
                         # os.mkdir's "mode" parameter may not always work:
@@ -176,7 +182,7 @@ to serve all files under specified web root ({0})."""
                         # Set owner as parent directory if possible
                         try:
                             os.chown(prefix, stat_path.st_uid, stat_path.st_gid)
-                        except OSError as exception:
+                        except (OSError, AttributeError) as exception:
                             logger.info("Unable to change owner and uid of webroot directory")
                             logger.debug("Error was: %s", exception)
                     except OSError as exception:
@@ -207,7 +213,6 @@ to serve all files under specified web root ({0})."""
             os.umask(old_umask)
 
         self.performed[root_path].add(achall)
-
         return response
 
     def cleanup(self, achalls):  # pylint: disable=missing-docstring
@@ -219,8 +224,8 @@ to serve all files under specified web root ({0})."""
                 os.remove(validation_path)
                 self.performed[root_path].remove(achall)
 
-        not_removed = []
-        while self._created_dirs:
+        not_removed = []  # type: List[str]
+        while len(self._created_dirs) > 0:
             path = self._created_dirs.pop()
             try:
                 os.rmdir(path)

certbot/plugins/webroot_test.py

@@ -4,9 +4,9 @@ from __future__ import print_function
 
 import argparse
 import errno
+import json
 import os
 import shutil
-import stat
 import tempfile
 import unittest
 
@@ -18,12 +18,11 @@ from acme import challenges
 
 from certbot import achallenges
 from certbot import errors
+from certbot.compat import misc
 from certbot.display import util as display_util
-
 from certbot.tests import acme_util
 from certbot.tests import util as test_util
 
-
 KEY = jose.JWKRSA.load(test_util.load_vector("rsa512_key.pem"))
 
 
@@ -142,6 +141,7 @@ class AuthenticatorTest(unittest.TestCase):
             self.assertRaises(errors.PluginError, self.auth.perform, [])
         os.chmod(self.path, 0o700)
 
+    @test_util.skip_on_windows('On Windows, there is no chown.')
     @mock.patch("certbot.plugins.webroot.os.chown")
     def test_failed_chown(self, mock_chown):
         mock_chown.side_effect = OSError(errno.EACCES, "msg")
@@ -169,16 +169,14 @@ class AuthenticatorTest(unittest.TestCase):
         # Remove exec bit from permission check, so that it
         # matches the file
         self.auth.perform([self.achall])
-        path_permissions = stat.S_IMODE(os.stat(self.validation_path).st_mode)
-        self.assertEqual(path_permissions, 0o644)
+        self.assertTrue(misc.compare_file_modes(os.stat(self.validation_path).st_mode, 0o644))
 
         # Check permissions of the directories
 
         for dirpath, dirnames, _ in os.walk(self.path):
             for directory in dirnames:
                 full_path = os.path.join(dirpath, directory)
-                dir_permissions = stat.S_IMODE(os.stat(full_path).st_mode)
-                self.assertEqual(dir_permissions, 0o755)
+                self.assertTrue(misc.compare_file_modes(os.stat(full_path).st_mode, 0o755))
 
         parent_gid = os.stat(self.path).st_gid
         parent_uid = os.stat(self.path).st_uid
@@ -274,7 +272,7 @@ class WebrootActionTest(unittest.TestCase):
 
     def test_webroot_map_action(self):
         args = self.parser.parse_args(
-            ["--webroot-map", '{{"thing.com":"{0}"}}'.format(self.path)])
+            ["--webroot-map", json.dumps({'thing.com': self.path})])
         self.assertEqual(args.webroot_map["thing.com"], self.path)
 
     def test_domain_before_webroot(self):