Merge branch 'master' into certbot-ci-part2

certbot-apache/certbot_apache/configurator.py

@@ -32,7 +32,6 @@ from certbot_apache import display_ops
 from certbot_apache import http_01
 from certbot_apache import obj
 from certbot_apache import parser
-from certbot_apache import tls_sni_01
 
 from collections import defaultdict
 
@@ -215,15 +214,13 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
     @property
     def mod_ssl_conf(self):
         """Full absolute path to SSL configuration file."""
-        return os.path.join(self.config.config_dir,
+        return os.path.join(self.config.config_dir, constants.MOD_SSL_CONF_DEST)
-                            constants.MOD_SSL_CONF_DEST)
 
     @property
     def updated_mod_ssl_conf_digest(self):
         """Full absolute path to digest of updated SSL configuration file."""
         return os.path.join(self.config.config_dir, constants.UPDATED_MOD_SSL_CONF_DIGEST)
 
-
     def prepare(self):
         """Prepare the authenticator/installer.
 
@@ -396,7 +393,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         if len(name.split(".")) == len(domain.split(".")):
             return fnmatch.fnmatch(name, domain)
 
-
     def _choose_vhosts_wildcard(self, domain, create_ssl=True):
         """Prompts user to choose vhosts to install a wildcard certificate for"""
 
@@ -441,7 +437,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         self._wildcard_vhosts[domain] = return_vhosts
         return return_vhosts
 
-
     def _deploy_cert(self, vhost, cert_path, key_path, chain_path, fullchain_path):
         """
         Helper function for deploy_cert() that handles the actual deployment
@@ -449,8 +444,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         domain originally passed for deploy_cert(). This is especially true
         with wildcard certificates
         """
-
-
         # This is done first so that ssl module is enabled and cert_path,
         # cert_key... can all be parsed appropriately
         self.prepare_server_https("443")
@@ -1925,7 +1918,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
             self.parser.add_dir(vhost.path, "RewriteRule",
                     constants.REWRITE_HTTPS_ARGS)
 
-
     def _verify_no_certbot_redirect(self, vhost):
         """Checks to see if a redirect was already installed by certbot.
 
@@ -2193,7 +2185,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         :raises .errors.MisconfigurationError: If reload fails
 
         """
-        error = ""
         try:
             util.run_script(self.option("restart_cmd"))
         except errors.SubprocessError as err:
@@ -2267,7 +2258,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
     ###########################################################################
     def get_chall_pref(self, unused_domain):  # pylint: disable=no-self-use
         """Return list of challenge preferences."""
-        return [challenges.HTTP01, challenges.TLSSNI01]
+        return [challenges.HTTP01]
 
     def perform(self, achalls):
         """Perform the configuration related challenge.
@@ -2280,20 +2271,15 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         self._chall_out.update(achalls)
         responses = [None] * len(achalls)
         http_doer = http_01.ApacheHttp01(self)
-        sni_doer = tls_sni_01.ApacheTlsSni01(self)
 
         for i, achall in enumerate(achalls):
             # Currently also have chall_doer hold associated index of the
             # challenge. This helps to put all of the responses back together
             # when they are all complete.
-            if isinstance(achall.chall, challenges.HTTP01):
+            http_doer.add_chall(achall, i)
-                http_doer.add_chall(achall, i)
-            else:  # tls-sni-01
-                sni_doer.add_chall(achall, i)
 
         http_response = http_doer.perform()
-        sni_response = sni_doer.perform()
+        if http_response:
-        if http_response or sni_response:
             # Must reload in order to activate the challenges.
             # Handled here because we may be able to load up other challenge
             # types
@@ -2304,7 +2290,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
             time.sleep(3)
 
             self._update_responses(responses, http_response, http_doer)
-            self._update_responses(responses, sni_response, sni_doer)
 
         return responses
 

certbot-apache/certbot_apache/tests/configurator_test.py

@@ -812,32 +812,19 @@ class MultipleVhostsTest(util.ApacheTest):
         self.assertEqual(self.config.add_name_vhost.call_count, 2)
 
     @mock.patch("certbot_apache.configurator.http_01.ApacheHttp01.perform")
-    @mock.patch("certbot_apache.configurator.tls_sni_01.ApacheTlsSni01.perform")
     @mock.patch("certbot_apache.configurator.ApacheConfigurator.restart")
-    def test_perform(self, mock_restart, mock_tls_perform, mock_http_perform):
+    def test_perform(self, mock_restart, mock_http_perform):
         # Only tests functionality specific to configurator.perform
         # Note: As more challenges are offered this will have to be expanded
         account_key, achalls = self.get_key_and_achalls()
 
-        all_expected = []
+        expected = [achall.response(account_key) for achall in achalls]
-        http_expected = []
+        mock_http_perform.return_value = expected
-        tls_expected = []
-        for achall in achalls:
-            response = achall.response(account_key)
-            if isinstance(achall.chall, challenges.HTTP01):
-                http_expected.append(response)
-            else:
-                tls_expected.append(response)
-            all_expected.append(response)
-
-        mock_http_perform.return_value = http_expected
-        mock_tls_perform.return_value = tls_expected
 
         responses = self.config.perform(achalls)
 
         self.assertEqual(mock_http_perform.call_count, 1)
-        self.assertEqual(mock_tls_perform.call_count, 1)
+        self.assertEqual(responses, expected)
-        self.assertEqual(responses, all_expected)
 
         self.assertEqual(mock_restart.call_count, 1)
 

certbot-apache/certbot_apache/tests/tls_sni_01_test.py

@@ -1,151 +0,0 @@
-"""Test for certbot_apache.tls_sni_01."""
-import shutil
-import unittest
-
-import mock
-
-from certbot import errors
-from certbot.plugins import common_test
-
-from certbot_apache import obj
-from certbot_apache.tests import util
-
-from six.moves import xrange  # pylint: disable=redefined-builtin, import-error
-
-
-class TlsSniPerformTest(util.ApacheTest):
-    """Test the ApacheTlsSni01 challenge."""
-
-    auth_key = common_test.AUTH_KEY
-    achalls = common_test.ACHALLS
-
-    def setUp(self):  # pylint: disable=arguments-differ
-        super(TlsSniPerformTest, self).setUp()
-
-        config = util.get_apache_configurator(
-            self.config_path, self.vhost_path, self.config_dir,
-            self.work_dir)
-        config.config.tls_sni_01_port = 443
-
-        from certbot_apache import tls_sni_01
-        self.sni = tls_sni_01.ApacheTlsSni01(config)
-
-    def tearDown(self):
-        shutil.rmtree(self.temp_dir)
-        shutil.rmtree(self.config_dir)
-        shutil.rmtree(self.work_dir)
-
-    def test_perform0(self):
-        resp = self.sni.perform()
-        self.assertEqual(len(resp), 0)
-
-    @mock.patch("certbot.util.exe_exists")
-    @mock.patch("certbot.util.run_script")
-    def test_perform1(self, _, mock_exists):
-        self.sni.configurator.parser.modules.add("socache_shmcb_module")
-        self.sni.configurator.parser.modules.add("ssl_module")
-
-        mock_exists.return_value = True
-        self.sni.configurator.parser.update_runtime_variables = mock.Mock()
-
-        achall = self.achalls[0]
-        self.sni.add_chall(achall)
-        response = self.achalls[0].response(self.auth_key)
-        mock_setup_cert = mock.MagicMock(return_value=response)
-        # pylint: disable=protected-access
-        self.sni._setup_challenge_cert = mock_setup_cert
-
-        responses = self.sni.perform()
-        mock_setup_cert.assert_called_once_with(achall)
-
-        # Check to make sure challenge config path is included in apache config
-        self.assertEqual(
-            len(self.sni.configurator.parser.find_dir(
-                "Include", self.sni.challenge_conf)), 1)
-        self.assertEqual(len(responses), 1)
-        self.assertEqual(responses[0], response)
-
-    def test_perform2(self):
-        # Avoid load module
-        self.sni.configurator.parser.modules.add("ssl_module")
-        self.sni.configurator.parser.modules.add("socache_shmcb_module")
-        acme_responses = []
-        for achall in self.achalls:
-            self.sni.add_chall(achall)
-            acme_responses.append(achall.response(self.auth_key))
-
-        mock_setup_cert = mock.MagicMock(side_effect=acme_responses)
-        # pylint: disable=protected-access
-        self.sni._setup_challenge_cert = mock_setup_cert
-
-        with mock.patch(
-            "certbot_apache.override_debian.DebianConfigurator.enable_mod"):
-            sni_responses = self.sni.perform()
-
-        self.assertEqual(mock_setup_cert.call_count, 2)
-
-        # Make sure calls made to mocked function were correct
-        self.assertEqual(
-            mock_setup_cert.call_args_list[0], mock.call(self.achalls[0]))
-        self.assertEqual(
-            mock_setup_cert.call_args_list[1], mock.call(self.achalls[1]))
-
-        self.assertEqual(
-            len(self.sni.configurator.parser.find_dir(
-                "Include", self.sni.challenge_conf)),
-            1)
-        self.assertEqual(len(sni_responses), 2)
-        for i in xrange(2):
-            self.assertEqual(sni_responses[i], acme_responses[i])
-
-    def test_mod_config(self):
-        z_domains = []
-        for achall in self.achalls:
-            self.sni.add_chall(achall)
-            z_domain = achall.response(self.auth_key).z_domain
-            z_domains.append(set([z_domain.decode('ascii')]))
-
-        self.sni._mod_config()  # pylint: disable=protected-access
-        self.sni.configurator.save()
-
-        self.sni.configurator.parser.find_dir(
-            "Include", self.sni.challenge_conf)
-        vh_match = self.sni.configurator.aug.match(
-            "/files" + self.sni.challenge_conf + "//VirtualHost")
-
-        vhs = []
-        for match in vh_match:
-            # pylint: disable=protected-access
-            vhs.append(self.sni.configurator._create_vhost(match))
-        self.assertEqual(len(vhs), 2)
-        for vhost in vhs:
-            self.assertEqual(vhost.addrs, set([obj.Addr.fromstring("*:443")]))
-            names = vhost.get_names()
-            self.assertTrue(names in z_domains)
-
-    def test_get_addrs_default(self):
-        self.sni.configurator.choose_vhost = mock.Mock(
-            return_value=obj.VirtualHost(
-                "path", "aug_path",
-                set([obj.Addr.fromstring("_default_:443")]),
-                False, False)
-        )
-
-        # pylint: disable=protected-access
-        self.assertEqual(
-            set([obj.Addr.fromstring("*:443")]),
-            self.sni._get_addrs(self.achalls[0]))
-
-    def test_get_addrs_no_vhost_found(self):
-        self.sni.configurator.choose_vhost = mock.Mock(
-            side_effect=errors.MissingCommandlineFlag(
-                "Failed to run Apache plugin non-interactively"))
-
-        # pylint: disable=protected-access
-        self.assertEqual(
-            set([obj.Addr.fromstring("*:443")]),
-            self.sni._get_addrs(self.achalls[0]))
-
-
-if __name__ == "__main__":
-    unittest.main()  # pragma: no cover

certbot-apache/certbot_apache/tls_sni_01.py

@@ -1,174 +0,0 @@
-"""A class that performs TLS-SNI-01 challenges for Apache"""
-
-import os
-import logging
-
-from acme.magic_typing import Set  # pylint: disable=unused-import, no-name-in-module
-from certbot.plugins import common
-from certbot.errors import PluginError, MissingCommandlineFlag
-
-from certbot_apache import obj
-
-logger = logging.getLogger(__name__)
-
-
-class ApacheTlsSni01(common.TLSSNI01):
-    """Class that performs TLS-SNI-01 challenges within the Apache configurator
-
-    :ivar configurator: ApacheConfigurator object
-    :type configurator: :class:`~apache.configurator.ApacheConfigurator`
-
-    :ivar list achalls: Annotated TLS-SNI-01
-        (`.KeyAuthorizationAnnotatedChallenge`) challenges.
-
-    :param list indices: Meant to hold indices of challenges in a
-        larger array. ApacheTlsSni01 is capable of solving many challenges
-        at once which causes an indexing issue within ApacheConfigurator
-        who must return all responses in order.  Imagine ApacheConfigurator
-        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
-
-    """
-
-    VHOST_TEMPLATE = """\
-<VirtualHost {vhost}>
-    ServerName {server_name}
-    UseCanonicalName on
-    SSLStrictSNIVHostCheck on
-
-    LimitRequestBody 1048576
-
-    Include {ssl_options_conf_path}
-    SSLCertificateFile {cert_path}
-    SSLCertificateKeyFile {key_path}
-
-    DocumentRoot {document_root}
-</VirtualHost>
-
-"""
-
-    def __init__(self, *args, **kwargs):
-        super(ApacheTlsSni01, self).__init__(*args, **kwargs)
-
-        self.challenge_conf = os.path.join(
-            self.configurator.conf("challenge-location"),
-            "le_tls_sni_01_cert_challenge.conf")
-
-    def perform(self):
-        """Perform a TLS-SNI-01 challenge."""
-        if not self.achalls:
-            return []
-        # Save any changes to the configuration as a precaution
-        # About to make temporary changes to the config
-        self.configurator.save("Changes before challenge setup", True)
-
-        # Prepare the server for HTTPS
-        self.configurator.prepare_server_https(
-            str(self.configurator.config.tls_sni_01_port), True)
-
-        responses = []
-
-        # Create all of the challenge certs
-        for achall in self.achalls:
-            responses.append(self._setup_challenge_cert(achall))
-
-        # Setup the configuration
-        addrs = self._mod_config()
-        self.configurator.save("Don't lose mod_config changes", True)
-        self.configurator.make_addrs_sni_ready(addrs)
-
-        # Save reversible changes
-        self.configurator.save("SNI Challenge", True)
-
-        return responses
-
-    def _mod_config(self):
-        """Modifies Apache config files to include challenge vhosts.
-
-        Result: Apache config includes virtual servers for issued challs
-
-        :returns: All TLS-SNI-01 addresses used
-        :rtype: set
-
-        """
-        addrs = set()  # type: Set[obj.Addr]
-        config_text = "<IfModule mod_ssl.c>\n"
-
-        for achall in self.achalls:
-            achall_addrs = self._get_addrs(achall)
-            addrs.update(achall_addrs)
-
-            config_text += self._get_config_text(achall, achall_addrs)
-
-        config_text += "</IfModule>\n"
-
-        self.configurator.parser.add_include(
-            self.configurator.parser.loc["default"], self.challenge_conf)
-        self.configurator.reverter.register_file_creation(
-            True, self.challenge_conf)
-
-        logger.debug("writing a config file with text:\n %s", config_text)
-        with open(self.challenge_conf, "w") as new_conf:
-            new_conf.write(config_text)
-
-        return addrs
-
-    def _get_addrs(self, achall):
-        """Return the Apache addresses needed for TLS-SNI-01."""
-        # TODO: Checkout _default_ rules.
-        addrs = set()
-        default_addr = obj.Addr(("*", str(
-            self.configurator.config.tls_sni_01_port)))
-
-        try:
-            vhost = self.configurator.choose_vhost(achall.domain,
-                                                   create_if_no_ssl=False)
-        except (PluginError, MissingCommandlineFlag):
-            # We couldn't find the virtualhost for this domain, possibly
-            # because it's a new vhost that's not configured yet
-            # (GH #677). See also GH #2600.
-            logger.warning("Falling back to default vhost %s...", default_addr)
-            addrs.add(default_addr)
-            return addrs
-
-        for addr in vhost.addrs:
-            if "_default_" == addr.get_addr():
-                addrs.add(default_addr)
-            else:
-                addrs.add(
-                    addr.get_sni_addr(
-                        self.configurator.config.tls_sni_01_port))
-
-        return addrs
-
-    def _get_config_text(self, achall, ip_addrs):
-        """Chocolate virtual server configuration text
-
-        :param .KeyAuthorizationAnnotatedChallenge achall: Annotated
-            TLS-SNI-01 challenge.
-
-        :param list ip_addrs: addresses of challenged domain
-            :class:`list` of type `~.obj.Addr`
-
-        :returns: virtual host configuration text
-        :rtype: str
-
-        """
-        ips = " ".join(str(i) for i in ip_addrs)
-        document_root = os.path.join(
-            self.configurator.config.work_dir, "tls_sni_01_page/")
-        # TODO: Python docs is not clear how multiline string literal
-        # newlines are parsed on different platforms. At least on
-        # Linux (Debian sid), when source file uses CRLF, Python still
-        # parses it as "\n"... c.f.:
-        # https://docs.python.org/2.7/reference/lexical_analysis.html
-        return self.VHOST_TEMPLATE.format(
-            vhost=ips,
-            server_name=achall.response(achall.account_key).z_domain.decode('ascii'),
-            ssl_options_conf_path=self.configurator.mod_ssl_conf,
-            cert_path=self.get_cert_path(achall),
-            key_path=self.get_key_path(achall),
-            document_root=document_root).replace("\n", os.linesep)

certbot-compatibility-test/certbot_compatibility_test/configurators/common.py

@@ -23,6 +23,9 @@ class Proxy(object):
     def __init__(self, args):
         """Initializes the plugin with the given command line args"""
         self._temp_dir = tempfile.mkdtemp()
+        # tempfile.mkdtemp() creates folders with too restrictive permissions to be accessible
+        # to an Apache worker, leading to HTTP challenge failures. Let's fix that.
+        os.chmod(self._temp_dir, 0o755)
         self.le_config = util.create_le_config(self._temp_dir)
         config_dir = util.extract_configs(args.configs, self._temp_dir)
         self._configs = [

certbot-compatibility-test/certbot_compatibility_test/test_driver.py

@@ -1,5 +1,6 @@
 """Tests Certbot plugins against different server configurations."""
 import argparse
+import contextlib
 import filecmp
 import logging
 import os
@@ -7,6 +8,7 @@ import shutil
 import tempfile
 import time
 import sys
+from urllib3.util import connection
 
 import OpenSSL
 
@@ -64,18 +66,19 @@ def test_authenticator(plugin, config, temp_dir):
                 "Plugin failed to complete %s for %s in %s",
                 type(achalls[i]), achalls[i].domain, config)
             success = False
-        elif isinstance(responses[i], challenges.TLSSNI01Response):
+        elif isinstance(responses[i], challenges.HTTP01Response):
-            verified = responses[i].simple_verify(achalls[i].chall,
+            # We fake the DNS resolution to ensure that any domain is resolved
-                                                  achalls[i].domain,
+            # to the local HTTP server setup for the compatibility tests
-                                                  util.JWK.public_key(),
+            with _fake_dns_resolution("127.0.0.1"):
-                                                  host="127.0.0.1",
+                verified = responses[i].simple_verify(
-                                                  port=plugin.https_port)
+                    achalls[i].chall, achalls[i].domain,
+                    util.JWK.public_key(), port=plugin.http_port)
             if verified:
                 logger.info(
-                    "tls-sni-01 verification for %s succeeded", achalls[i].domain)
+                    "http-01 verification for %s succeeded", achalls[i].domain)
             else:
                 logger.error(
-                    "**** tls-sni-01 verification for %s in %s failed",
+                    "**** http-01 verification for %s in %s failed",
                     achalls[i].domain, config)
                 success = False
 
@@ -102,9 +105,9 @@ def _create_achalls(plugin):
     for domain in names:
         prefs = plugin.get_chall_pref(domain)
         for chall_type in prefs:
-            if chall_type == challenges.TLSSNI01:
+            if chall_type == challenges.HTTP01:
-                chall = challenges.TLSSNI01(
+                chall = challenges.HTTP01(
-                    token=os.urandom(challenges.TLSSNI01.TOKEN_SIZE))
+                    token=os.urandom(challenges.HTTP01.TOKEN_SIZE))
                 challb = acme_util.chall_to_challb(
                     chall, messages.STATUS_PENDING)
                 achall = achallenges.KeyAuthorizationAnnotatedChallenge(
@@ -369,5 +372,21 @@ def main():
         sys.exit(1)
 
 
+@contextlib.contextmanager
+def _fake_dns_resolution(resolved_ip):
+    """Monkey patch urllib3 to make any hostname be resolved to the provided IP"""
+    _original_create_connection = connection.create_connection
+
+    def _patched_create_connection(address, *args, **kwargs):
+        _, port = address
+        return _original_create_connection((resolved_ip, port), *args, **kwargs)
+
+    try:
+        connection.create_connection = _patched_create_connection
+        yield
+    finally:
+        connection.create_connection = _original_create_connection
+
+
 if __name__ == "__main__":
     main()

certbot-nginx/tests/boulder-integration.sh

@@ -43,8 +43,6 @@ nginx -v
 reload_nginx
 certbot_test_nginx --domains nginx.wtf run
 test_deployment_and_rollback nginx.wtf
-certbot_test_nginx --domains nginx-tls.wtf run --preferred-challenges tls-sni
-test_deployment_and_rollback nginx-tls.wtf
 certbot_test_nginx --domains nginx2.wtf --preferred-challenges http
 test_deployment_and_rollback nginx2.wtf
 # Overlapping location block and server-block-level return 301
@@ -70,4 +68,4 @@ test_deployment_and_rollback nginx6.wtf
 # top
 nginx -c $nginx_root/nginx.conf -s stop
 
-coverage report --fail-under 75 --include 'certbot-nginx/*' --show-missing
+coverage report --fail-under 72 --include 'certbot-nginx/*' --show-missing

certbot/plugins/manual.py

@@ -15,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):
@@ -63,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 '
+        'resource requested when performing an HTTP-01 challenge. An additional '
-        'performing a TLS-SNI-01 challenge, $CERTBOT_SNI_DOMAIN will contain '
+        'cleanup script can also be provided and can use the additional variable '
-        'the SNI name for which the ACME server expects to be presented with '
+        '$CERTBOT_AUTH_OUTPUT which contains the stdout output from the auth script.')
-        '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.')
     _DNS_INSTRUCTIONS = """\
 Please deploy a DNS TXT record under the name
 {domain} with the following value:
@@ -86,14 +53,6 @@ Create a file containing just this data:
 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,
@@ -112,7 +71,6 @@ permitted by DNS standards.)
         self.reverter.recovery_routine()
         self.env = dict() \
         # type: Dict[achallenges.KeyAuthorizationAnnotatedChallenge, Dict[str, str]]
-        self.tls_sni_01 = None
         self.subsequent_dns_challenge = False
         self.subsequent_any_challenge = False
 
@@ -149,7 +107,7 @@ permitted by DNS standards.)
 
     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()
@@ -160,12 +118,6 @@ permitted by DNS standards.)
 
         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
@@ -191,16 +143,6 @@ permitted by DNS standards.)
             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 = self._execute_hook('auth-hook')
         env['CERTBOT_AUTH_OUTPUT'] = out.strip()
@@ -213,17 +155,11 @@ permitted by DNS standards.)
                 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

certbot/plugins/manual_test.py

@@ -22,8 +22,7 @@ class AuthenticatorTest(test_util.TempDirTestCase):
         self.http_achall = acme_util.HTTP01_A
         self.dns_achall = acme_util.DNS01_A
         self.dns_achall_2 = acme_util.DNS01_A_2
-        self.tls_sni_achall = acme_util.TLSSNI01_A
+        self.achalls = [self.http_achall, self.dns_achall, self.dns_achall_2]
-        self.achalls = [self.http_achall, self.dns_achall, self.tls_sni_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
@@ -38,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"),
+            in_progress_dir=os.path.join(self.tempdir, "in_progess"))
-            tls_sni_01_port=5001)
 
         from certbot.plugins.manual import Authenticator
         self.auth = Authenticator(self.config, name='manual')
@@ -58,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.HTTP01, challenges.DNS01])
-                          challenges.DNS01,
-                          challenges.TLSSNI01])
 
     @test_util.patch_get_utility()
     def test_ip_logging_not_ok(self, mock_get_utility):
@@ -79,18 +75,13 @@ class AuthenticatorTest(test_util.TempDirTestCase):
             '{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_CERT_PATH\', \'nocert\'));'
-            'print(os.environ.get(\'CERTBOT_KEY_PATH\', \'nokey\'));'
-            'print(os.environ.get(\'CERTBOT_SNI_DOMAIN\', \'nosnidomain\'));'
             'print(os.environ.get(\'CERTBOT_VALIDATION\', \'novalidation\'));"'
             .format(sys.executable))
-        dns_expected = '{0}\n{1}\n{2}\n{3}\n{4}\n{5}'.format(
+        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(
@@ -102,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):
@@ -122,13 +102,8 @@ 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.assertTrue(
+                achall.validation(achall.account_key) in args[0])
-                        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.assertFalse(kwargs['wrap'])
 
     @test_util.broken_on_windows
@@ -153,18 +128,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/standalone.py

@@ -6,7 +6,7 @@ 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
 
@@ -55,25 +55,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.HTTP01DualNetworkedServers(
-                servers = acme_standalone.TLSSNI01DualNetworkedServers(
+                address, self.http_01_resources)
-                    address, self.certs)  # type: acme_standalone.BaseDualNetworkedServers
-            else:  # challenges.HTTP01
-                servers = acme_standalone.HTTP01DualNetworkedServers(
-                    address, self.http_01_resources)
         except socket.error as error:
             raise errors.StandaloneBindError(error, port)
 
@@ -96,8 +92,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]
 
@@ -114,7 +108,7 @@ class ServerManager(object):
         return self._instances.copy()
 
 
-SUPPORTED_CHALLENGES = [challenges.HTTP01, challenges.TLSSNI01] \
+SUPPORTED_CHALLENGES = [challenges.HTTP01] \
 # type: List[Type[challenges.KeyAuthorizationChallenge]]
 
 
@@ -132,8 +126,6 @@ class SupportedChallengesAction(argparse.Action):
     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
@@ -141,15 +133,6 @@ class SupportedChallengesAction(argparse.Action):
 
         """
         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]
 
@@ -177,7 +160,7 @@ 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.
     """
@@ -187,10 +170,6 @@ 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)  # type: ServedType
 
         # Stuff below is shared across threads (i.e. servers read
@@ -219,9 +198,8 @@ class Authenticator(common.Plugin):
     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 "
+               "http-01 challenges from the certificate authority. Therefore, "
-               "authority. Therefore, it does not rely on any existing "
+               "it does not rely on any existing server program.")
-               "server program.")
 
     def prepare(self):  # pylint: disable=missing-docstring
         pass
@@ -241,10 +219,7 @@ class Authenticator(common.Plugin):
                 _handle_perform_error(error)
 
     def _perform_single(self, achall):
-        if isinstance(achall.chall, challenges.HTTP01):
+        servers, response = self._perform_http_01(achall)
-            servers, response = self._perform_http_01(achall)
-        else:  # tls-sni-01
-            servers, response = self._perform_tls_sni_01(achall)
         self.served[servers].add(achall)
         return response
 
@@ -258,14 +233,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():

certbot/plugins/standalone_test.py

@@ -44,9 +44,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)
 
@@ -98,10 +95,7 @@ class SupportedChallengesActionTest(unittest.TestCase):
         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
@@ -110,11 +104,6 @@ class SupportedChallengesActionTest(unittest.TestCase):
     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"))
-
 
 def get_open_port():
     """Gets an open port number from the OS."""
@@ -132,31 +121,31 @@ class AuthenticatorTest(unittest.TestCase):
         from certbot.plugins.standalone import Authenticator
 
         self.config = mock.MagicMock(
-            tls_sni_01_port=get_open_port(), http01_port=get_open_port(),
+            http01_port=get_open_port(),
-            standalone_supported_challenges="tls-sni-01,http-01")
+            standalone_supported_challenges="http-01")
         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])
+                         [challenges.HTTP01])
 
     def test_supported_challenges_configured(self):
-        self.config.standalone_supported_challenges = "tls-sni-01"
+        self.config.standalone_supported_challenges = "http-01"
         self.assertEqual(self.auth.supported_challenges,
-                         [challenges.TLSSNI01])
+                         [challenges.HTTP01])
 
     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])
+                         [challenges.HTTP01])
 
     def test_get_chall_pref_configured(self):
-        self.config.standalone_supported_challenges = "tls-sni-01"
+        self.config.standalone_supported_challenges = "http-01"
         self.assertEqual(self.auth.get_chall_pref(domain=None),
-                         [challenges.TLSSNI01])
+                         [challenges.HTTP01])
 
     def test_perform(self):
         achalls = self._get_achalls()
@@ -212,10 +201,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 = {
@@ -243,5 +230,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

tests/boulder-fetch.sh

@@ -12,12 +12,6 @@ fi
 
 cd ${BOULDERPATH}
 
-# Since https://github.com/letsencrypt/boulder/commit/92e8e1708a725e9d08a5da2f4a7132320ed2158b,
-# Boulder support for tls-sni-01 challenges is disabled. We still need to support it until this
-# challenge is officially removed from ACME CA server on production, and also removed from Certbot.
-# This sed command reactivate tls-sni-01 challenges inplace temporarily.
-sed -i 's/tls-alpn-01/tls-sni-01/g' test/config/ra.json
-
 docker-compose up -d boulder
 
 set +x  # reduce verbosity while waiting for boulder

tests/certbot-boulder-integration.sh

@@ -223,20 +223,20 @@ common plugins --init --prepare | grep webroot
 
 # We start a server listening on the port for the
 # unrequested challenge to prevent regressions in #3601.
-python ./tests/run_http_server.py $http_01_port &
+python ./tests/run_http_server.py $tls_alpn_01_port &
 python_server_pid=$!
-
 certname="le1.wtf"
-common --domains le1.wtf --preferred-challenges tls-sni-01 auth \
+common --domains le1.wtf --preferred-challenges http-01 auth \
        --cert-name $certname \
        --pre-hook 'echo wtf.pre >> "$HOOK_TEST"' \
        --post-hook 'echo wtf.post >> "$HOOK_TEST"'\
        --deploy-hook 'echo deploy >> "$HOOK_TEST"'
-kill $python_server_pid
 CheckDeployHook $certname
 
-python ./tests/run_http_server.py $tls_sni_01_port &
+# Previous test used to be a tls-sni-01 challenge that is not supported anymore.
-python_server_pid=$!
+# Now it is a http-01 challenge and this makes it a duplicate of the following test.
+# But removing it would break many tests here, as they are strongly coupled.
+# See https://github.com/certbot/certbot/pull/6852
 certname="le2.wtf"
 common --domains le2.wtf --preferred-challenges http-01 run \
        --cert-name $certname \
@@ -256,7 +256,7 @@ common certonly -a manual -d le.wtf --rsa-key-size 4096 --cert-name $certname \
 CheckRenewHook $certname
 
 certname="dns.le.wtf"
-common -a manual -d dns.le.wtf --preferred-challenges dns,tls-sni run \
+common -a manual -d dns.le.wtf --preferred-challenges dns run \
     --cert-name $certname \
     --manual-auth-hook ./tests/manual-dns-auth.sh \
     --manual-cleanup-hook ./tests/manual-dns-cleanup.sh \
@@ -398,7 +398,7 @@ CheckDirHooks 1
 # with fail.
 common -a manual -d dns1.le.wtf,fail.dns1.le.wtf \
     --allow-subset-of-names \
-    --preferred-challenges dns,tls-sni \
+    --preferred-challenges dns \
     --manual-auth-hook ./tests/manual-dns-auth.sh \
     --manual-cleanup-hook ./tests/manual-dns-cleanup.sh
 

tests/integration/_common.sh

@@ -3,10 +3,10 @@
 root=${root:-$(mktemp -d -t leitXXXX)}
 echo "Root integration tests directory: $root"
 config_dir="$root/conf"
-tls_sni_01_port=5001
+tls_alpn_01_port=5001
 http_01_port=5002
 sources="acme/,$(ls -dm certbot*/ | tr -d ' \n')"
-export root config_dir tls_sni_01_port http_01_port sources
+export root config_dir tls_alpn_01_port http_01_port sources
 certbot_path="$(command -v certbot)"
 # Flags that are added here will be added to Certbot calls within
 # certbot_test_no_force_renew.
@@ -60,7 +60,7 @@ certbot_test_no_force_renew () {
         "$certbot_path" \
             --server "${SERVER:-http://localhost:4000/directory}" \
             --no-verify-ssl \
-            --tls-sni-01-port $tls_sni_01_port \
+            --tls-sni-01-port $tls_alpn_01_port \
             --http-01-port $http_01_port \
             --manual-public-ip-logging-ok \
             $other_flags \

tools/venv3.py

@@ -1,4 +1,4 @@
-#!/usr/bin/env python
+#!/usr/bin/env python3
 # Developer virtualenv setup for Certbot client
 import _venv_common