From d9880721b31ab4e9e9d2fdc38d83ff2bd9078378 Mon Sep 17 00:00:00 2001 From: Adrien Ferrand Date: Mon, 18 Mar 2019 18:22:19 +0100 Subject: [PATCH] Remove tls sni in nginx plugin (#6857) * Remove tls-sni from nginx config * Add a dedicated configuration to define what is the HTTPS port for this certbot instance. * Correct some tests * Reestablish default vhost creation * Clean tls references for nginx integration tests * Associate https_port only to tests and nginx --- certbot-nginx/certbot_nginx/configurator.py | 26 +-- .../certbot_nginx/tests/configurator_test.py | 23 +-- .../certbot_nginx/tests/tls_sni_01_test.py | 158 ---------------- certbot-nginx/certbot_nginx/tests/util.py | 2 +- certbot-nginx/certbot_nginx/tls_sni_01.py | 177 ------------------ .../tests/boulder-integration.conf.sh | 2 +- tests/certbot-boulder-integration.sh | 2 +- tests/integration/_common.sh | 6 +- 8 files changed, 21 insertions(+), 375 deletions(-) delete mode 100644 certbot-nginx/certbot_nginx/tests/tls_sni_01_test.py delete mode 100644 certbot-nginx/certbot_nginx/tls_sni_01.py diff --git a/certbot-nginx/certbot_nginx/configurator.py b/certbot-nginx/certbot_nginx/configurator.py index ffe1ddac7..2357735f9 100644 --- a/certbot-nginx/certbot_nginx/configurator.py +++ b/certbot-nginx/certbot_nginx/configurator.py @@ -26,7 +26,6 @@ from certbot_nginx import constants from certbot_nginx import display_ops from certbot_nginx import nginxparser from certbot_nginx import parser -from certbot_nginx import tls_sni_01 from certbot_nginx import http_01 from certbot_nginx import obj # pylint: disable=unused-import from acme.magic_typing import List, Dict, Set # pylint: disable=unused-import, no-name-in-module @@ -149,7 +148,6 @@ class NginxConfigurator(common.Installer): # Make sure configuration is valid self.config_test() - self.parser = parser.NginxParser(self.conf('server-root')) install_ssl_options_conf(self.mod_ssl_conf, self.updated_mod_ssl_conf_digest) @@ -561,7 +559,7 @@ class NginxConfigurator(common.Installer): :rtype: set """ - all_names = set() # type: Set[str] + all_names = set() # type: Set[str] for vhost in self.parser.get_vhosts(): all_names.update(vhost.names) @@ -611,7 +609,8 @@ class NginxConfigurator(common.Installer): :type vhost: :class:`~certbot_nginx.obj.VirtualHost` """ - ipv6info = self.ipv6_info(self.config.tls_sni_01_port) + https_port = self.config.tls_sni_01_port + ipv6info = self.ipv6_info(https_port) ipv6_block = [''] ipv4_block = [''] @@ -625,7 +624,7 @@ class NginxConfigurator(common.Installer): ipv6_block = ['\n ', 'listen', ' ', - '[::]:{0}'.format(self.config.tls_sni_01_port), + '[::]:{0}'.format(https_port), ' ', 'ssl'] if not ipv6info[1]: @@ -637,7 +636,7 @@ class NginxConfigurator(common.Installer): ipv4_block = ['\n ', 'listen', ' ', - '{0}'.format(self.config.tls_sni_01_port), + '{0}'.format(https_port), ' ', 'ssl'] @@ -799,8 +798,6 @@ class NginxConfigurator(common.Installer): :param str domain: domain to enable redirect for :param `~obj.Vhost` vhost: vhost to enable redirect for """ - - http_vhost = None if vhost.ssl: http_vhost, _ = self._split_block(vhost, ['listen', 'server_name']) @@ -1051,19 +1048,14 @@ class NginxConfigurator(common.Installer): """ self._chall_out += len(achalls) responses = [None] * len(achalls) - sni_doer = tls_sni_01.NginxTlsSni01(self) http_doer = http_01.NginxHttp01(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) - else: # tls-sni-01 - sni_doer.add_chall(achall, i) + http_doer.add_chall(achall, i) - sni_response = sni_doer.perform() http_response = http_doer.perform() # Must restart in order to activate the challenges. # Handled here because we may be able to load up other challenge types @@ -1072,9 +1064,8 @@ class NginxConfigurator(common.Installer): # Go through all of the challenges and assign them to the proper place # in the responses return value. All responses must be in the same order # as the original challenges. - for chall_response, chall_doer in ((sni_response, sni_doer), (http_response, http_doer)): - for i, resp in enumerate(chall_response): - responses[chall_doer.indices[i]] = resp + for i, resp in enumerate(http_response): + responses[http_doer.indices[i]] = resp return responses @@ -1152,6 +1143,7 @@ def install_ssl_options_conf(options_ssl, options_ssl_digest): return common.install_version_controlled_file(options_ssl, options_ssl_digest, constants.MOD_SSL_CONF_SRC, constants.ALL_SSL_OPTIONS_HASHES) + def _determine_default_server_root(): if os.environ.get("CERTBOT_DOCS") == "1": default_server_root = "%s or %s" % (constants.LINUX_SERVER_ROOT, diff --git a/certbot-nginx/certbot_nginx/tests/configurator_test.py b/certbot-nginx/certbot_nginx/tests/configurator_test.py index 706e2637a..08e4a56ae 100644 --- a/certbot-nginx/certbot_nginx/tests/configurator_test.py +++ b/certbot-nginx/certbot_nginx/tests/configurator_test.py @@ -318,21 +318,13 @@ class NginxConfiguratorTest(util.NginxTest): ]], parsed_migration_conf[0]) - @mock.patch("certbot_nginx.configurator.tls_sni_01.NginxTlsSni01.perform") @mock.patch("certbot_nginx.configurator.http_01.NginxHttp01.perform") @mock.patch("certbot_nginx.configurator.NginxConfigurator.restart") @mock.patch("certbot_nginx.configurator.NginxConfigurator.revert_challenge_config") - def test_perform_and_cleanup(self, mock_revert, mock_restart, mock_http_perform, - mock_tls_perform): + def test_perform_and_cleanup(self, mock_revert, mock_restart, mock_http_perform): # Only tests functionality specific to configurator.perform # Note: As more challenges are offered this will have to be expanded - achall1 = achallenges.KeyAuthorizationAnnotatedChallenge( - challb=messages.ChallengeBody( - chall=challenges.TLSSNI01(token=b"kNdwjwOeX0I_A8DXt9Msmg"), - uri="https://ca.org/chall0_uri", - status=messages.Status("pending"), - ), domain="localhost", account_key=self.rsa512jwk) - achall2 = achallenges.KeyAuthorizationAnnotatedChallenge( + achall = achallenges.KeyAuthorizationAnnotatedChallenge( challb=messages.ChallengeBody( chall=challenges.HTTP01(token=b"m8TdO1qik4JVFtgPPurJmg"), uri="https://ca.org/chall1_uri", @@ -340,19 +332,16 @@ class NginxConfiguratorTest(util.NginxTest): ), domain="example.com", account_key=self.rsa512jwk) expected = [ - achall1.response(self.rsa512jwk), - achall2.response(self.rsa512jwk), + achall.response(self.rsa512jwk), ] - mock_tls_perform.return_value = expected[:1] - mock_http_perform.return_value = expected[1:] - responses = self.config.perform([achall1, achall2]) + mock_http_perform.return_value = expected[:] + responses = self.config.perform([achall]) - self.assertEqual(mock_tls_perform.call_count, 1) self.assertEqual(mock_http_perform.call_count, 1) self.assertEqual(responses, expected) - self.config.cleanup([achall1, achall2]) + self.config.cleanup([achall]) self.assertEqual(0, self.config._chall_out) # pylint: disable=protected-access self.assertEqual(mock_revert.call_count, 1) self.assertEqual(mock_restart.call_count, 2) diff --git a/certbot-nginx/certbot_nginx/tests/tls_sni_01_test.py b/certbot-nginx/certbot_nginx/tests/tls_sni_01_test.py deleted file mode 100644 index 62ca085ef..000000000 --- a/certbot-nginx/certbot_nginx/tests/tls_sni_01_test.py +++ /dev/null @@ -1,158 +0,0 @@ -"""Tests for certbot_nginx.tls_sni_01""" -import unittest - -import mock -import six - -from acme import challenges - -from certbot import achallenges -from certbot import errors - -from certbot.plugins import common_test -from certbot.tests import acme_util - -from certbot_nginx import obj -from certbot_nginx.tests import util - - -class TlsSniPerformTest(util.NginxTest): - """Test the NginxTlsSni01 challenge.""" - - account_key = common_test.AUTH_KEY - achalls = [ - achallenges.KeyAuthorizationAnnotatedChallenge( - challb=acme_util.chall_to_challb( - challenges.TLSSNI01(token=b"kNdwjwOeX0I_A8DXt9Msmg"), "pending"), - domain="www.example.com", account_key=account_key), - achallenges.KeyAuthorizationAnnotatedChallenge( - challb=acme_util.chall_to_challb( - challenges.TLSSNI01( - token=b"\xba\xa9\xda?