From be5bcfe463d9649c350181fe1a84e9d513db1ff3 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 30 Jan 2017 16:55:54 -0800 Subject: [PATCH] Remove optional dependencies (#4088) * Stop using already_listening in standalone * remove already_listening * remove psutil entirely * fix #595 * Add basic perform test * make pep8 happy * Add test_perform_eacces * add _setup_perform_error * Add test_perform_unexpected_socket_error * add test_perform_eaddrinuse_no_retry * add test_perform_eaddrinuse_retry * cleanup tests * stop using dnspython * don't install dns extras in tox * remove dns extras from setup.py * Add simple_verify back to DNS response * remove dnspython from oldest tests --- acme/acme/challenges.py | 31 ++----- acme/acme/challenges_test.py | 47 ++-------- acme/acme/dns_resolver.py | 45 ---------- acme/acme/dns_resolver_test.py | 77 ---------------- acme/acme/test_util.py | 16 ---- acme/acme/util.py | 18 ---- acme/acme/util_test.py | 18 ---- acme/setup.py | 6 -- certbot/plugins/standalone.py | 113 ++++++++++------------- certbot/plugins/standalone_test.py | 127 ++++++++++---------------- certbot/plugins/util.py | 125 -------------------------- certbot/plugins/util_test.py | 139 ----------------------------- certbot/tests/util.py | 16 ---- setup.py | 1 - tox.ini | 18 ++-- 15 files changed, 122 insertions(+), 675 deletions(-) delete mode 100644 acme/acme/dns_resolver.py delete mode 100644 acme/acme/dns_resolver_test.py diff --git a/acme/acme/challenges.py b/acme/acme/challenges.py index 3bfc1e62f..83b9b9edd 100644 --- a/acme/acme/challenges.py +++ b/acme/acme/challenges.py @@ -9,7 +9,6 @@ from cryptography.hazmat.primitives import hashes import OpenSSL import requests -from acme import dns_resolver from acme import errors from acme import crypto_util from acme import fields @@ -214,36 +213,24 @@ class DNS01Response(KeyAuthorizationChallengeResponse): def simple_verify(self, chall, domain, account_public_key): """Simple verify. + This method no longer checks DNS records and is a simple wrapper + around `KeyAuthorizationChallengeResponse.verify`. + :param challenges.DNS01 chall: Corresponding challenge. :param unicode domain: Domain name being verified. :param JWK account_public_key: Public key for the key pair being authorized. - :returns: ``True`` iff validation with the TXT records resolved from a - DNS server is successful. + :return: ``True`` iff verification of the key authorization was + successful. :rtype: bool """ - if not self.verify(chall, account_public_key): + # pylint: disable=unused-argument + verified = self.verify(chall, account_public_key) + if not verified: logger.debug("Verification of key authorization in response failed") - return False - - validation_domain_name = chall.validation_domain_name(domain) - validation = chall.validation(account_public_key) - logger.debug("Verifying %s at %s...", chall.typ, validation_domain_name) - - try: - txt_records = dns_resolver.txt_records_for_name( - validation_domain_name) - except errors.DependencyError: - raise errors.DependencyError("Local validation for 'dns-01' " - "challenges requires 'dnspython'") - exists = validation in txt_records - if not exists: - logger.debug("Key authorization from response (%r) doesn't match " - "any DNS response in %r", self.key_authorization, - txt_records) - return exists + return verified @Challenge.register # pylint: disable=too-many-ancestors diff --git a/acme/acme/challenges_test.py b/acme/acme/challenges_test.py index 5ac07abdd..49e790102 100644 --- a/acme/acme/challenges_test.py +++ b/acme/acme/challenges_test.py @@ -10,7 +10,6 @@ from six.moves.urllib import parse as urllib_parse # pylint: disable=import-err from acme import errors from acme import jose from acme import test_util -from acme.dns_resolver import DNS_REQUIREMENT CERT = test_util.load_comparable_cert('cert.pem') KEY = jose.JWKRSA(key=test_util.load_rsa_private_key('rsa512_key.pem')) @@ -92,7 +91,6 @@ class DNS01ResponseTest(unittest.TestCase): from acme.challenges import DNS01 self.chall = DNS01(token=(b'x' * 16)) self.response = self.chall.response(KEY) - self.records_for_name_path = "acme.dns_resolver.txt_records_for_name" def test_to_partial_json(self): self.assertEqual(self.jmsg, self.msg.to_partial_json()) @@ -105,45 +103,16 @@ class DNS01ResponseTest(unittest.TestCase): from acme.challenges import DNS01Response hash(DNS01Response.from_json(self.jmsg)) - def test_simple_verify_bad_key_authorization(self): + def test_simple_verify_failure(self): key2 = jose.JWKRSA.load(test_util.load_vector('rsa256_key.pem')) - self.response.simple_verify(self.chall, "local", key2.public_key()) + public_key = key2.public_key() + verified = self.response.simple_verify(self.chall, "local", public_key) + self.assertFalse(verified) - @mock.patch('acme.dns_resolver.DNS_AVAILABLE', False) - def test_simple_verify_without_dns(self): - self.assertRaises( - errors.DependencyError, self.response.simple_verify, - self.chall, 'local', KEY.public_key()) - - @test_util.skip_unless(test_util.requirement_available(DNS_REQUIREMENT), - "optional dependency dnspython is not available") - def test_simple_verify_good_validation(self): # pragma: no cover - with mock.patch(self.records_for_name_path) as mock_resolver: - mock_resolver.return_value = [ - self.chall.validation(KEY.public_key())] - self.assertTrue(self.response.simple_verify( - self.chall, "local", KEY.public_key())) - mock_resolver.assert_called_once_with( - self.chall.validation_domain_name("local")) - - @test_util.skip_unless(test_util.requirement_available(DNS_REQUIREMENT), - "optional dependency dnspython is not available") - def test_simple_verify_good_validation_multitxts(self): # pragma: no cover - with mock.patch(self.records_for_name_path) as mock_resolver: - mock_resolver.return_value = [ - "!", self.chall.validation(KEY.public_key())] - self.assertTrue(self.response.simple_verify( - self.chall, "local", KEY.public_key())) - mock_resolver.assert_called_once_with( - self.chall.validation_domain_name("local")) - - @test_util.skip_unless(test_util.requirement_available(DNS_REQUIREMENT), - "optional dependency dnspython is not available") - def test_simple_verify_bad_validation(self): # pragma: no cover - with mock.patch(self.records_for_name_path) as mock_resolver: - mock_resolver.return_value = ["!"] - self.assertFalse(self.response.simple_verify( - self.chall, "local", KEY.public_key())) + def test_simple_verify_success(self): + public_key = KEY.public_key() + verified = self.response.simple_verify(self.chall, "local", public_key) + self.assertTrue(verified) class DNS01Test(unittest.TestCase): diff --git a/acme/acme/dns_resolver.py b/acme/acme/dns_resolver.py deleted file mode 100644 index 2677d92ad..000000000 --- a/acme/acme/dns_resolver.py +++ /dev/null @@ -1,45 +0,0 @@ -"""DNS Resolver for ACME client. -Required only for local validation of 'dns-01' challenges. -""" -import logging - -from acme import errors -from acme import util - -DNS_REQUIREMENT = 'dnspython>=1.12' - -try: - util.activate(DNS_REQUIREMENT) - # pragma: no cover - import dns.exception - import dns.resolver - DNS_AVAILABLE = True -except errors.DependencyError: # pragma: no cover - DNS_AVAILABLE = False - - -logger = logging.getLogger(__name__) - - -def txt_records_for_name(name): - """Resolve the name and return the TXT records. - - :param unicode name: Domain name being verified. - - :returns: A list of txt records, if empty the name could not be resolved - :rtype: list of unicode - - """ - if not DNS_AVAILABLE: - raise errors.DependencyError( - '{0} is required to use this function'.format(DNS_REQUIREMENT)) - try: - dns_response = dns.resolver.query(name, 'TXT') - except dns.resolver.NXDOMAIN as error: - return [] - except dns.exception.DNSException as error: - logger.error("Error resolving %s: %s", name, str(error)) - return [] - - return [txt_rec.decode("utf-8") for rdata in dns_response - for txt_rec in rdata.strings] diff --git a/acme/acme/dns_resolver_test.py b/acme/acme/dns_resolver_test.py deleted file mode 100644 index 2e2edd0e7..000000000 --- a/acme/acme/dns_resolver_test.py +++ /dev/null @@ -1,77 +0,0 @@ -"""Tests for acme.dns_resolver.""" -import unittest - -import mock -from six.moves import reload_module # pylint: disable=import-error - -from acme import errors -from acme import test_util -from acme.dns_resolver import DNS_REQUIREMENT - - -if test_util.requirement_available(DNS_REQUIREMENT): - import dns - - -def create_txt_response(name, txt_records): - """ - Returns an RRSet containing the 'txt_records' as the result of a DNS - query for 'name'. - - This takes advantage of the fact that an Answer object mostly behaves - like an RRset. - """ - return dns.rrset.from_text_list(name, 60, "IN", "TXT", txt_records) - - -class TxtRecordsForNameTest(unittest.TestCase): - """Tests for acme.dns_resolver.txt_records_for_name.""" - @classmethod - def _call(cls, *args, **kwargs): - from acme.dns_resolver import txt_records_for_name - return txt_records_for_name(*args, **kwargs) - - -@test_util.skip_unless(test_util.requirement_available(DNS_REQUIREMENT), - "optional dependency dnspython is not available") -class TxtRecordsForNameWithDnsTest(TxtRecordsForNameTest): - """Tests for acme.dns_resolver.txt_records_for_name with dns.""" - @mock.patch("acme.dns_resolver.dns.resolver.query") - def test_txt_records_for_name_with_single_response(self, mock_dns): - mock_dns.return_value = create_txt_response('name', ['response']) - self.assertEqual(['response'], self._call('name')) - - @mock.patch("acme.dns_resolver.dns.resolver.query") - def test_txt_records_for_name_with_multiple_responses(self, mock_dns): - mock_dns.return_value = create_txt_response( - 'name', ['response1', 'response2']) - self.assertEqual(['response1', 'response2'], self._call('name')) - - @mock.patch("acme.dns_resolver.dns.resolver.query") - def test_txt_records_for_name_domain_not_found(self, mock_dns): - mock_dns.side_effect = dns.resolver.NXDOMAIN - self.assertEquals([], self._call('name')) - - @mock.patch("acme.dns_resolver.dns.resolver.query") - def test_txt_records_for_name_domain_other_error(self, mock_dns): - mock_dns.side_effect = dns.exception.DNSException - self.assertEquals([], self._call('name')) - - -class TxtRecordsForNameWithoutDnsTest(TxtRecordsForNameTest): - """Tests for acme.dns_resolver.txt_records_for_name without dns.""" - def setUp(self): - from acme import dns_resolver - dns_resolver.DNS_AVAILABLE = False - - def tearDown(self): - from acme import dns_resolver - reload_module(dns_resolver) - - def test_exception_raised(self): - self.assertRaises( - errors.DependencyError, self._call, "example.org") - - -if __name__ == '__main__': - unittest.main() # pragma: no cover diff --git a/acme/acme/test_util.py b/acme/acme/test_util.py index ba968511f..0f5763682 100644 --- a/acme/acme/test_util.py +++ b/acme/acme/test_util.py @@ -11,9 +11,7 @@ from cryptography.hazmat.backends import default_backend from cryptography.hazmat.primitives import serialization import OpenSSL -from acme import errors from acme import jose -from acme import util def vector_path(*names): @@ -78,20 +76,6 @@ def load_pyopenssl_private_key(*names): return OpenSSL.crypto.load_privatekey(loader, load_vector(*names)) -def requirement_available(requirement): - """Checks if requirement can be imported. - - :rtype: bool - :returns: ``True`` iff requirement can be imported - - """ - try: - util.activate(requirement) - except errors.DependencyError: # pragma: no cover - return False - return True # pragma: no cover - - def skip_unless(condition, reason): # pragma: no cover """Skip tests unless a condition holds. diff --git a/acme/acme/util.py b/acme/acme/util.py index ac445b271..1fff89a9e 100644 --- a/acme/acme/util.py +++ b/acme/acme/util.py @@ -1,25 +1,7 @@ """ACME utilities.""" -import pkg_resources import six -from acme import errors - def map_keys(dikt, func): """Map dictionary keys.""" return dict((func(key), value) for key, value in six.iteritems(dikt)) - - -def activate(requirement): - """Make requirement importable. - - :param str requirement: the distribution and version to activate - - :raises acme.errors.DependencyError: if cannot activate requirement - - """ - try: - for distro in pkg_resources.require(requirement): # pylint: disable=not-callable - distro.activate() - except (pkg_resources.DistributionNotFound, pkg_resources.VersionConflict): - raise errors.DependencyError('{0} is unavailable'.format(requirement)) diff --git a/acme/acme/util_test.py b/acme/acme/util_test.py index ba6465409..00aa8b02d 100644 --- a/acme/acme/util_test.py +++ b/acme/acme/util_test.py @@ -1,8 +1,6 @@ """Tests for acme.util.""" import unittest -from acme import errors - class MapKeysTest(unittest.TestCase): """Tests for acme.util.map_keys.""" @@ -14,21 +12,5 @@ class MapKeysTest(unittest.TestCase): self.assertEqual({2: 2, 4: 4}, map_keys({1: 2, 3: 4}, lambda x: x + 1)) -class ActivateTest(unittest.TestCase): - """Tests for acme.util.activate.""" - - @classmethod - def _call(cls, *args, **kwargs): - from acme.util import activate - return activate(*args, **kwargs) - - def test_failure(self): - self.assertRaises(errors.DependencyError, self._call, 'acme>99.0.0') - - def test_success(self): - self._call('acme') - import acme as unused_acme - - if __name__ == '__main__': unittest.main() # pragma: no cover diff --git a/acme/setup.py b/acme/setup.py index 37b71368c..d05d814ad 100644 --- a/acme/setup.py +++ b/acme/setup.py @@ -33,11 +33,6 @@ if sys.version_info < (2, 7): else: install_requires.append('mock') -# dnspython 1.12 is required to support both Python 2 and Python 3. -dns_extras = [ - 'dnspython>=1.12', -] - dev_extras = [ 'nose', 'tox', @@ -77,7 +72,6 @@ setup( include_package_data=True, install_requires=install_requires, extras_require={ - 'dns': dns_extras, 'dev': dev_extras, 'docs': docs_extras, }, diff --git a/certbot/plugins/standalone.py b/certbot/plugins/standalone.py index 4fc52479f..0c15930a3 100644 --- a/certbot/plugins/standalone.py +++ b/certbot/plugins/standalone.py @@ -18,7 +18,6 @@ from certbot import errors from certbot import interfaces from certbot.plugins import common -from certbot.plugins import util logger = logging.getLogger(__name__) @@ -208,74 +207,38 @@ class Authenticator(common.Plugin): # pylint: disable=unused-argument,missing-docstring return self.supported_challenges - 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 required ports is already taken.") - def perform(self, achalls): # pylint: disable=missing-docstring - self._verify_ports_are_available(achalls) + return [self._try_perform_single(achall) for achall in achalls] - try: - return self.perform2(achalls) - except errors.StandaloneBindError as error: - display = zope.component.getUtility(interfaces.IDisplay) + def _try_perform_single(self, achall): + while True: + try: + return self._perform_single(achall) + except errors.StandaloneBindError as error: + _handle_perform_error(error) - if error.socket_error.errno == socket.errno.EACCES: - display.notification( - "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), force_interactive=True) - elif error.socket_error.errno == socket.errno.EADDRINUSE: - display.notification( - "Could not bind TCP port {0} because it is already in " - "use by another process on this system (such as a web " - "server). Please stop the program in question and then " - "try again.".format(error.port), force_interactive=True) - else: - raise # XXX: How to handle unknown errors in binding? + def _perform_single(self, achall): + if isinstance(achall.chall, challenges.HTTP01): + server, response = self._perform_http_01(achall) + else: # tls-sni-01 + server, response = self._perform_tls_sni_01(achall) + self.served[server].add(achall) + return response - def perform2(self, achalls): - """Perform achallenges without IDisplay interaction.""" - responses = [] + def _perform_http_01(self, achall): + server = self.servers.run(self.config.http01_port, challenges.HTTP01) + response, validation = achall.response_and_validation() + resource = acme_standalone.HTTP01RequestHandler.HTTP01Resource( + chall=achall.chall, response=response, validation=validation) + self.http_01_resources.add(resource) + return server, response - for achall in achalls: - if isinstance(achall.chall, challenges.HTTP01): - server = self.servers.run( - self.config.http01_port, challenges.HTTP01) - response, validation = achall.response_and_validation() - self.http_01_resources.add( - acme_standalone.HTTP01RequestHandler.HTTP01Resource( - chall=achall.chall, response=response, - validation=validation)) - else: # tls-sni-01 - server = self.servers.run( - self.config.tls_sni_01_port, challenges.TLSSNI01) - response, (cert, _) = achall.response_and_validation( - cert_key=self.key) - self.certs[response.z_domain] = (self.key, cert) - self.served[server].add(achall) - responses.append(response) - - return responses + def _perform_tls_sni_01(self, achall): + port = self.config.tls_sni_01_port + server = self.servers.run(port, challenges.TLSSNI01) + response, (cert, _) = achall.response_and_validation(cert_key=self.key) + self.certs[response.z_domain] = (self.key, cert) + return server, response def cleanup(self, achalls): # pylint: disable=missing-docstring # reduce self.served and close servers if none challenges are served @@ -286,3 +249,25 @@ class Authenticator(common.Plugin): for port, server in six.iteritems(self.servers.running()): if not self.served[server]: self.servers.stop(port) + + +def _handle_perform_error(error): + if error.socket_error.errno == socket.errno.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: + display = zope.component.getUtility(interfaces.IDisplay) + msg = ( + "Could not bind TCP port {0} because it is already in " + "use by another process on this system (such as a web " + "server). Please stop the program in question and " + "then try again.".format(error.port)) + should_retry = display.yesno(msg, "Retry", + "Cancel", default=False) + if not should_retry: + raise errors.PluginError(msg) + else: + raise diff --git a/certbot/plugins/standalone_test.py b/certbot/plugins/standalone_test.py index 08e59c929..83e0fcf7f 100644 --- a/certbot/plugins/standalone_test.py +++ b/certbot/plugins/standalone_test.py @@ -8,11 +8,9 @@ import six from acme import challenges from acme import jose -from acme import standalone as acme_standalone from certbot import achallenges from certbot import errors -from certbot import interfaces from certbot.tests import acme_util from certbot.tests import util as test_util @@ -114,6 +112,7 @@ def get_open_port(): open_socket.close() return port + class AuthenticatorTest(unittest.TestCase): """Tests for certbot.plugins.standalone.Authenticator.""" @@ -124,6 +123,7 @@ class AuthenticatorTest(unittest.TestCase): tls_sni_01_port=get_open_port(), http01_port=get_open_port(), standalone_supported_challenges="tls-sni-01,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, @@ -146,6 +146,52 @@ class AuthenticatorTest(unittest.TestCase): self.assertEqual(self.auth.get_chall_pref(domain=None), [challenges.TLSSNI01]) + def test_perform(self): + achalls = self._get_achalls() + response = self.auth.perform(achalls) + + expected = [achall.response(achall.account_key) for achall in achalls] + self.assertEqual(response, expected) + + @test_util.patch_get_utility() + def test_perform_eaddrinuse_retry(self, mock_get_utility): + errno = socket.errno.EADDRINUSE + error = errors.StandaloneBindError(mock.MagicMock(errno=errno), -1) + self.auth.servers.run.side_effect = [error] + 2 * [mock.MagicMock()] + mock_yesno = mock_get_utility.return_value.yesno + mock_yesno.return_value = True + + self.test_perform() + self._assert_correct_yesno_call(mock_yesno) + + @test_util.patch_get_utility() + def test_perform_eaddrinuse_no_retry(self, mock_get_utility): + mock_yesno = mock_get_utility.return_value.yesno + mock_yesno.return_value = False + + errno = socket.errno.EADDRINUSE + self.assertRaises(errors.PluginError, self._fail_perform, errno) + self._assert_correct_yesno_call(mock_yesno) + + def _assert_correct_yesno_call(self, mock_yesno): + yesno_args, yesno_kwargs = mock_yesno.call_args + self.assertTrue("in use" in yesno_args[0]) + self.assertFalse(yesno_kwargs.get("default", True)) + + def test_perform_eacces(self): + errno = socket.errno.EACCES + self.assertRaises(errors.PluginError, self._fail_perform, errno) + + def test_perform_unexpected_socket_error(self): + errno = socket.errno.ENOTCONN + self.assertRaises( + errors.StandaloneBindError, self._fail_perform, errno) + + def _fail_perform(self, errno): + error = errors.StandaloneBindError(mock.MagicMock(errno=errno), -1) + self.auth.servers.run.side_effect = error + self.auth.perform(self._get_achalls()) + @classmethod def _get_achalls(cls): domain = b'localhost' @@ -157,84 +203,7 @@ class AuthenticatorTest(unittest.TestCase): return [http_01, tls_sni_01] - @mock.patch("certbot.plugins.standalone.util") - def test_perform_already_listening(self, mock_util): - 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.assertRaises( - errors.MisconfigurationError, self.auth.perform, [achall]) - mock_util.already_listening.assert_called_once_with(port, False) - mock_util.already_listening.reset_mock() - - @test_util.patch_get_utility() - def test_perform(self, unused_mock_get_utility): - 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) - - @test_util.patch_get_utility() - def _test_perform_bind_errors(self, errno, achalls, mock_get_utility): - port = get_open_port() - def _perform2(unused_achalls): - raise errors.StandaloneBindError(mock.Mock(errno=errno), port) - - self.auth.perform2 = mock.MagicMock(side_effect=_perform2) - self.auth.perform(achalls) - mock_get_utility.assert_called_once_with(interfaces.IDisplay) - notification = mock_get_utility.return_value.notification - self.assertEqual(1, notification.call_count) - self.assertTrue(str(port) in notification.call_args[0][0]) - - def test_perform_eacces(self): - # pylint: disable=no-value-for-parameter - self._test_perform_bind_errors(socket.errno.EACCES, []) - - def test_perform_eaddrinuse(self): - # pylint: disable=no-value-for-parameter - self._test_perform_bind_errors(socket.errno.EADDRINUSE, []) - - def test_perfom_unknown_bind_error(self): - self.assertRaises( - errors.StandaloneBindError, self._test_perform_bind_errors, - socket.errno.ENOTCONN, []) - - def test_perform2(self): - http_01, tls_sni_01 = self._get_achalls() - - self.auth.servers = mock.MagicMock() - - def _run(port, tls): # pylint: disable=unused-argument - return "server{0}".format(port) - - self.auth.servers.run.side_effect = _run - responses = self.auth.perform2([http_01, tls_sni_01]) - - self.assertTrue(isinstance(responses, list)) - self.assertEqual(2, len(responses)) - self.assertTrue(isinstance(responses[0], challenges.HTTP01Response)) - self.assertTrue(isinstance(responses[1], challenges.TLSSNI01Response)) - - self.assertEqual(self.auth.servers.run.mock_calls, [ - mock.call(self.config.http01_port, challenges.HTTP01), - mock.call(self.config.tls_sni_01_port, challenges.TLSSNI01), - ]) - self.assertEqual(self.auth.served, { - "server" + str(self.config.tls_sni_01_port): set([tls_sni_01]), - "server" + str(self.config.http01_port): set([http_01]), - }) - self.assertEqual(1, len(self.auth.http_01_resources)) - self.assertEqual(1, len(self.auth.certs)) - self.assertEqual(list(self.auth.http_01_resources), [ - acme_standalone.HTTP01RequestHandler.HTTP01Resource( - acme_util.HTTP01, responses[0], mock.ANY)]) - def test_cleanup(self): - self.auth.servers = mock.Mock() self.auth.servers.running.return_value = { 1: "server1", 2: "server2", diff --git a/certbot/plugins/util.py b/certbot/plugins/util.py index 20b0fdce7..e45c26735 100644 --- a/certbot/plugins/util.py +++ b/certbot/plugins/util.py @@ -1,34 +1,11 @@ """Plugin utilities.""" import logging import os -import socket -import zope.component - -from acme import errors as acme_errors -from acme import util as acme_util - -from certbot import interfaces from certbot import util -PSUTIL_REQUIREMENT = "psutil>=2.2.1" - -try: - acme_util.activate(PSUTIL_REQUIREMENT) - import psutil # pragma: no cover - USE_PSUTIL = True -except acme_errors.DependencyError: # pragma: no cover - USE_PSUTIL = False - logger = logging.getLogger(__name__) -RENEWER_EXTRA_MSG = ( - " For automated renewal, you may want to use a script that stops" - " and starts your webserver. You can find an example at" - " https://certbot.eff.org/docs/using.html#renewal ." - " Alternatively you can use the webroot plugin to renew without" - " needing to stop and start your webserver.") - def path_surgery(cmd): """Attempt to perform PATH surgery to find cmd @@ -59,105 +36,3 @@ def path_surgery(cmd): logger.warning("Failed to find %s in%s PATH: %s", cmd, expanded, path) return False - - -def already_listening(port, renewer=False): - """Check if a process is already listening on the port. - - If so, also tell the user via a display notification. - - .. warning:: - On some operating systems, this function can only usefully be - run as root. - - :param int port: The TCP port in question. - :returns: True or False. - - """ - - if USE_PSUTIL: - return already_listening_psutil(port, renewer=renewer) - else: - logger.debug("Psutil not found, using simple socket check.") - return already_listening_socket(port, renewer=renewer) - - -def already_listening_socket(port, renewer=False): - """Simple socket based check to find out if port is already in use - - :param int port: The TCP port in question. - :returns: True or False - """ - - try: - testsocket = socket.socket(socket.AF_INET, socket.SOCK_STREAM, 0) - testsocket.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) - try: - testsocket.bind(("", port)) - except socket.error: - display = zope.component.getUtility(interfaces.IDisplay) - extra = "" - if renewer: - extra = RENEWER_EXTRA_MSG - display.notification( - "Port {0} is already in use by another process. This will " - "prevent us from binding to that port. Please stop the " - "process that is populating the port in question and try " - "again. {1}".format(port, extra), force_interactive=True) - return True - finally: - testsocket.close() - except socket.error: - pass - return False - - -def already_listening_psutil(port, renewer=False): - """Psutil variant of the open port check - - :param int port: The TCP port in question. - :returns: True or False. - - """ - try: - net_connections = psutil.net_connections() - except psutil.AccessDenied as error: - logger.info("Access denied when trying to list network " - "connections: %s. Are you root?", error) - # this function is just a pre-check that often causes false - # positives and problems in testing (c.f. #680 on Mac, #255 - # generally); we will fail later in bind() anyway - return False - - listeners = [conn.pid for conn in net_connections - if conn.status == 'LISTEN' and - conn.type == socket.SOCK_STREAM and - conn.laddr[1] == port] - try: - if listeners and listeners[0] is not None: - # conn.pid may be None if the current process doesn't have - # permission to identify the listening process! Additionally, - # listeners may have more than one element if separate - # sockets have bound the same port on separate interfaces. - # We currently only have UI to notify the user about one - # of them at a time. - pid = listeners[0] - name = psutil.Process(pid).name() - display = zope.component.getUtility(interfaces.IDisplay) - extra = "" - if renewer: - extra = RENEWER_EXTRA_MSG - display.notification( - "The program {0} (process ID {1}) is already listening " - "on TCP port {2}. This will prevent us from binding to " - "that port. Please stop the {0} program temporarily " - "and then try again.{3}".format(name, pid, port, extra), - force_interactive=True) - return True - except (psutil.NoSuchProcess, psutil.AccessDenied): - # Perhaps the result of a race where the process could have - # exited or relinquished the port (NoSuchProcess), or the result - # of an OS policy where we're not allowed to look up the process - # name (AccessDenied). - pass - return False diff --git a/certbot/plugins/util_test.py b/certbot/plugins/util_test.py index b5d188835..947f24697 100644 --- a/certbot/plugins/util_test.py +++ b/certbot/plugins/util_test.py @@ -1,13 +1,9 @@ """Tests for certbot.plugins.util.""" import os -import socket import unittest import mock -from certbot.plugins.util import PSUTIL_REQUIREMENT -from certbot.tests import util as test_util - class PathSurgeryTest(unittest.TestCase): """Tests for certbot.plugins.path_surgery.""" @@ -34,140 +30,5 @@ class PathSurgeryTest(unittest.TestCase): self.assertTrue("/tmp" in os.environ["PATH"]) -class AlreadyListeningTest(unittest.TestCase): - """Tests for certbot.plugins.already_listening.""" - @classmethod - def _call(cls, *args, **kwargs): - from certbot.plugins.util import already_listening - return already_listening(*args, **kwargs) - - -class AlreadyListeningTestNoPsutil(AlreadyListeningTest): - """Tests for certbot.plugins.already_listening when - psutil is not available""" - @classmethod - def _call(cls, *args, **kwargs): - with mock.patch("certbot.plugins.util.USE_PSUTIL", False): - return super( - AlreadyListeningTestNoPsutil, cls)._call(*args, **kwargs) - - @test_util.patch_get_utility() - def test_ports_available(self, mock_getutil): - # Ensure we don't get error - with mock.patch("socket.socket.bind"): - self.assertFalse(self._call(80)) - self.assertFalse(self._call(80, True)) - self.assertEqual(mock_getutil.call_count, 0) - - @test_util.patch_get_utility() - def test_ports_blocked(self, mock_getutil): - with mock.patch("certbot.plugins.util.socket.socket.bind") as mock_bind: - mock_bind.side_effect = socket.error - self.assertTrue(self._call(80)) - self.assertTrue(self._call(80, True)) - with mock.patch("certbot.plugins.util.socket.socket") as mock_socket: - mock_socket.side_effect = socket.error - self.assertFalse(self._call(80)) - self.assertEqual(mock_getutil.call_count, 2) - - -@test_util.skip_unless(test_util.requirement_available(PSUTIL_REQUIREMENT), - "optional dependency psutil is not available") -class AlreadyListeningTestPsutil(AlreadyListeningTest): - """Tests for certbot.plugins.already_listening.""" - @mock.patch("certbot.plugins.util.psutil.net_connections") - @mock.patch("certbot.plugins.util.psutil.Process") - @test_util.patch_get_utility() - def test_race_condition(self, mock_get_utility, mock_process, mock_net): - # This tests a race condition, or permission problem, or OS - # incompatibility in which, for some reason, no process name can be - # found to match the identified listening PID. - import psutil - from psutil._common import sconn - conns = [ - sconn(fd=-1, family=2, type=1, laddr=("0.0.0.0", 30), - raddr=(), status="LISTEN", pid=None), - sconn(fd=3, family=2, type=1, laddr=("192.168.5.10", 32783), - raddr=("20.40.60.80", 22), status="ESTABLISHED", pid=1234), - sconn(fd=-1, family=10, type=1, laddr=("::1", 54321), - raddr=("::1", 111), status="CLOSE_WAIT", pid=None), - sconn(fd=3, family=2, type=1, laddr=("0.0.0.0", 17), - raddr=(), status="LISTEN", pid=4416)] - mock_net.return_value = conns - mock_process.side_effect = psutil.NoSuchProcess("No such PID") - # We simulate being unable to find the process name of PID 4416, - # which results in returning False. - self.assertFalse(self._call(17)) - self.assertEqual(mock_get_utility.generic_notification.call_count, 0) - mock_process.assert_called_once_with(4416) - - @mock.patch("certbot.plugins.util.psutil.net_connections") - @mock.patch("certbot.plugins.util.psutil.Process") - @test_util.patch_get_utility() - def test_not_listening(self, mock_get_utility, mock_process, mock_net): - from psutil._common import sconn - conns = [ - sconn(fd=-1, family=2, type=1, laddr=("0.0.0.0", 30), - raddr=(), status="LISTEN", pid=None), - sconn(fd=3, family=2, type=1, laddr=("192.168.5.10", 32783), - raddr=("20.40.60.80", 22), status="ESTABLISHED", pid=1234), - sconn(fd=-1, family=10, type=1, laddr=("::1", 54321), - raddr=("::1", 111), status="CLOSE_WAIT", pid=None)] - mock_net.return_value = conns - mock_process.name.return_value = "inetd" - self.assertFalse(self._call(17)) - self.assertEqual(mock_get_utility.generic_notification.call_count, 0) - self.assertEqual(mock_process.call_count, 0) - - @mock.patch("certbot.plugins.util.psutil.net_connections") - @mock.patch("certbot.plugins.util.psutil.Process") - @test_util.patch_get_utility() - def test_listening_ipv4(self, mock_get_utility, mock_process, mock_net): - from psutil._common import sconn - conns = [ - sconn(fd=-1, family=2, type=1, laddr=("0.0.0.0", 30), - raddr=(), status="LISTEN", pid=None), - sconn(fd=3, family=2, type=1, laddr=("192.168.5.10", 32783), - raddr=("20.40.60.80", 22), status="ESTABLISHED", pid=1234), - sconn(fd=-1, family=10, type=1, laddr=("::1", 54321), - raddr=("::1", 111), status="CLOSE_WAIT", pid=None), - sconn(fd=3, family=2, type=1, laddr=("0.0.0.0", 17), - raddr=(), status="LISTEN", pid=4416)] - mock_net.return_value = conns - mock_process.name.return_value = "inetd" - result = self._call(17, True) - self.assertTrue(result) - self.assertEqual(mock_get_utility.call_count, 1) - mock_process.assert_called_once_with(4416) - - @mock.patch("certbot.plugins.util.psutil.net_connections") - @mock.patch("certbot.plugins.util.psutil.Process") - @test_util.patch_get_utility() - def test_listening_ipv6(self, mock_get_utility, mock_process, mock_net): - from psutil._common import sconn - conns = [ - sconn(fd=-1, family=2, type=1, laddr=("0.0.0.0", 30), - raddr=(), status="LISTEN", pid=None), - sconn(fd=3, family=2, type=1, laddr=("192.168.5.10", 32783), - raddr=("20.40.60.80", 22), status="ESTABLISHED", pid=1234), - sconn(fd=-1, family=10, type=1, laddr=("::1", 54321), - raddr=("::1", 111), status="CLOSE_WAIT", pid=None), - sconn(fd=3, family=10, type=1, laddr=("::", 12345), raddr=(), - status="LISTEN", pid=4420), - sconn(fd=3, family=2, type=1, laddr=("0.0.0.0", 17), - raddr=(), status="LISTEN", pid=4416)] - mock_net.return_value = conns - mock_process.name.return_value = "inetd" - result = self._call(12345) - self.assertTrue(result) - self.assertEqual(mock_get_utility.call_count, 1) - mock_process.assert_called_once_with(4420) - - @mock.patch("certbot.plugins.util.psutil.net_connections") - def test_access_denied_exception(self, mock_net): - import psutil - mock_net.side_effect = psutil.AccessDenied("") - self.assertFalse(self._call(12345)) - if __name__ == "__main__": unittest.main() # pragma: no cover diff --git a/certbot/tests/util.py b/certbot/tests/util.py index 7d674e171..092807b56 100644 --- a/certbot/tests/util.py +++ b/certbot/tests/util.py @@ -13,9 +13,7 @@ from cryptography.hazmat.primitives import serialization import mock import OpenSSL -from acme import errors from acme import jose -from acme import util from certbot import constants from certbot import interfaces @@ -86,20 +84,6 @@ def load_pyopenssl_private_key(*names): return OpenSSL.crypto.load_privatekey(loader, load_vector(*names)) -def requirement_available(requirement): - """Checks if requirement can be imported. - - :rtype: bool - :returns: ``True`` iff requirement can be imported - - """ - try: - util.activate(requirement) - except errors.DependencyError: # pragma: no cover - return False - return True # pragma: no cover - - def skip_unless(condition, reason): # pragma: no cover """Skip tests unless a condition holds. diff --git a/setup.py b/setup.py index bc208f693..a47aff11b 100644 --- a/setup.py +++ b/setup.py @@ -67,7 +67,6 @@ dev_extras = [ 'astroid==1.3.5', 'coverage', 'nose', - 'psutil>=2.2.1', # for tests, optional 'pylint==1.4.2', # upstream #248 'tox', 'twine', diff --git a/tox.ini b/tox.ini index 6f6903948..e6317e665 100644 --- a/tox.ini +++ b/tox.ini @@ -13,7 +13,7 @@ envlist = modification,py{26,33,34,35,36},cover,lint # packages installed separately to ensure that downstream deps problems # are detected, c.f. #1002 commands = - pip install -e acme[dns,dev] + pip install -e acme[dev] nosetests -v acme --processes=-1 pip install -e .[dev] nosetests -v certbot --processes=-1 --process-timeout=100 @@ -37,35 +37,33 @@ deps = py{26,27}-oldest: cffi<=1.7 py{26,27}-oldest: cryptography==0.8 py{26,27}-oldest: configargparse==0.10.0 - py{26,27}-oldest: dnspython>=1.12 - py{26,27}-oldest: psutil==2.1.0 py{26,27}-oldest: PyOpenSSL==0.13 py{26,27}-oldest: requests<=2.11.1 [testenv:py33] commands = - pip install -e acme[dns,dev] + pip install -e acme[dev] nosetests -v acme --processes=-1 pip install -e .[dev] nosetests -v certbot --processes=-1 --process-timeout=100 [testenv:py34] commands = - pip install -e acme[dns,dev] + pip install -e acme[dev] nosetests -v acme --processes=-1 pip install -e .[dev] nosetests -v certbot --processes=-1 --process-timeout=100 [testenv:py35] commands = - pip install -e acme[dns,dev] + pip install -e acme[dev] nosetests -v acme --processes=-1 pip install -e .[dev] nosetests -v certbot --processes=-1 --process-timeout=100 [testenv:py36] commands = - pip install -e acme[dns,dev] + pip install -e acme[dev] nosetests -v acme --processes=-1 pip install -e .[dev] nosetests -v certbot --processes=-1 --process-timeout=100 @@ -73,12 +71,12 @@ commands = [testenv:py27_install] basepython = python2.7 commands = - pip install -e acme[dns,dev] -e .[dev] -e certbot-apache -e certbot-nginx -e letshelp-certbot + pip install -e acme[dev] -e .[dev] -e certbot-apache -e certbot-nginx -e letshelp-certbot [testenv:cover] basepython = python2.7 commands = - pip install -e acme[dns,dev] -e .[dev] -e certbot-apache -e certbot-nginx -e letshelp-certbot + pip install -e acme[dev] -e .[dev] -e certbot-apache -e certbot-nginx -e letshelp-certbot ./tox.cover.sh [testenv:lint] @@ -88,7 +86,7 @@ basepython = python2.7 # duplicate code checking; if one of the commands fails, others will # continue, but tox return code will reflect previous error commands = - pip install -q -e acme[dns,dev] -e .[dev] -e certbot-apache -e certbot-nginx -e certbot-compatibility-test -e letshelp-certbot + pip install -q -e acme[dev] -e .[dev] -e certbot-apache -e certbot-nginx -e certbot-compatibility-test -e letshelp-certbot pylint --reports=n --rcfile=.pylintrc acme/acme certbot certbot-apache/certbot_apache certbot-nginx/certbot_nginx certbot-compatibility-test/certbot_compatibility_test letshelp-certbot/letshelp_certbot [testenv:apacheconftest]