diff --git a/certbot/plugins/standalone.py b/certbot/plugins/standalone.py index 0195b2726..e8c11a416 100644 --- a/certbot/plugins/standalone.py +++ b/certbot/plugins/standalone.py @@ -194,15 +194,6 @@ class Authenticator(common.Plugin): return [challenges.Challenge.TYPES[name] for name in self.conf("supported-challenges").split(",")] - @property - def _necessary_ports(self): - necessary_ports = set() - if challenges.HTTP01 in self.supported_challenges: - necessary_ports.add(self.config.http01_port) - if challenges.TLSSNI01 in self.supported_challenges: - necessary_ports.add(self.config.tls_sni_01_port) - return necessary_ports - 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 " @@ -217,12 +208,30 @@ class Authenticator(common.Plugin): # pylint: disable=unused-argument,missing-docstring return self.supported_challenges - def perform(self, achalls): # pylint: disable=missing-docstring - renewer = self.config.verb == "renew" - if any(util.already_listening(port, renewer) for port in self._necessary_ports): + def _verify_ports_are_available(self, achalls): + """Confirm the ports are available to solve all achalls. + + :param list achalls: list of + :class:`~certbot.achallenges.AnnotatedChallenge` + + :raises .errors.MisconfigurationError: if required port is + unavailable + + """ + ports = [] + if any(isinstance(ac.chall, challenges.HTTP01) for ac in achalls): + ports.append(self.config.http01_port) + if any(isinstance(ac.chall, challenges.TLSSNI01) for ac in achalls): + ports.append(self.config.tls_sni_01_port) + + renewer = (self.config.verb == "renew") + + if any(util.already_listening(port, renewer) for port in ports): raise errors.MisconfigurationError( - "At least one of the (possibly) required ports is " - "already taken.") + "At least one of the required ports is already taken.") + + def perform(self, achalls): # pylint: disable=missing-docstring + self._verify_ports_are_available(achalls) try: return self.perform2(achalls) diff --git a/certbot/plugins/standalone_test.py b/certbot/plugins/standalone_test.py index 1dfa3950a..56f842f68 100644 --- a/certbot/plugins/standalone_test.py +++ b/certbot/plugins/standalone_test.py @@ -137,20 +137,33 @@ class AuthenticatorTest(unittest.TestCase): self.assertEqual(self.auth.get_chall_pref(domain=None), [challenges.TLSSNI01]) + @classmethod + def _get_achalls(cls): + domain = b'localhost' + 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] + @mock.patch("certbot.plugins.standalone.util") def test_perform_already_listening(self, mock_util): - for chall, port in ((challenges.TLSSNI01.typ, 1234), - (challenges.HTTP01.typ, 4321)): + http_01, tls_sni_01 = self._get_achalls() + + for achall, port in ((http_01, self.config.http01_port,), + (tls_sni_01, self.config.tls_sni_01_port)): mock_util.already_listening.return_value = True - self.config.standalone_supported_challenges = chall self.assertRaises( - errors.MisconfigurationError, self.auth.perform, []) + errors.MisconfigurationError, self.auth.perform, [achall]) mock_util.already_listening.assert_called_once_with(port, False) mock_util.already_listening.reset_mock() @mock.patch("certbot.plugins.standalone.zope.component.getUtility") def test_perform(self, unused_mock_get_utility): - achalls = [1, 2, 3] + achalls = self._get_achalls() + self.auth.perform2 = mock.Mock(return_value=mock.sentinel.responses) self.assertEqual(mock.sentinel.responses, self.auth.perform(achalls)) self.auth.perform2.assert_called_once_with(achalls) @@ -181,12 +194,7 @@ class AuthenticatorTest(unittest.TestCase): socket.errno.ENOTCONN, []) def test_perform2(self): - domain = b'localhost' - 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) + http_01, tls_sni_01 = self._get_achalls() self.auth.servers = mock.MagicMock() diff --git a/tests/boulder-integration.sh b/tests/boulder-integration.sh index ab8fde5f6..a70f13f8e 100755 --- a/tests/boulder-integration.sh +++ b/tests/boulder-integration.sh @@ -33,8 +33,17 @@ common() { "$@" } -common --domains le1.wtf --standalone-supported-challenges tls-sni-01 auth -common --domains le2.wtf --standalone-supported-challenges http-01 run +# We start a server listening on the port for the +# unrequested challenge to prevent regressions in #3601. +python -m SimpleHTTPServer $http_01_port & +python_server_pid=$! +common --domains le1.wtf --preferred-challenges tls-sni-01 auth +kill $python_server_pid +python -m SimpleHTTPServer $tls_sni_01_port & +python_server_pid=$! +common --domains le2.wtf --preferred-challenges http-01 run +kill $python_server_pid + common -a manual -d le.wtf auth --rsa-key-size 4096 export CSR_PATH="${root}/csr.der" KEY_PATH="${root}/key.pem" \ diff --git a/tests/integration/_common.sh b/tests/integration/_common.sh index 8992a18c0..935d44994 100755 --- a/tests/integration/_common.sh +++ b/tests/integration/_common.sh @@ -9,7 +9,9 @@ then fi store_flags="--config-dir $root/conf --work-dir $root/work" store_flags="$store_flags --logs-dir $root/logs" -export root store_flags +tls_sni_01_port=5001 +http_01_port=5002 +export root store_flags tls_sni_01_port http_01_port certbot_test () { certbot_test_no_force_renew \ @@ -21,8 +23,8 @@ certbot_test_no_force_renew () { certbot \ --server "${SERVER:-http://localhost:4000/directory}" \ --no-verify-ssl \ - --tls-sni-01-port 5001 \ - --http-01-port 5002 \ + --tls-sni-01-port $tls_sni_01_port \ + --http-01-port $http_01_port \ --manual-test-mode \ $store_flags \ --non-interactive \