From c28a94529571adcaef6ed3cc01b4e17f46cd0c7d Mon Sep 17 00:00:00 2001 From: James Kasten Date: Thu, 12 Feb 2015 01:06:30 -0800 Subject: [PATCH 01/10] Clarify authenticator interface --- letsencrypt/client/auth_handler.py | 16 +++++++++++----- letsencrypt/client/interfaces.py | 21 ++++++++++----------- 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/letsencrypt/client/auth_handler.py b/letsencrypt/client/auth_handler.py index 6f0ece535..ee69898c6 100644 --- a/letsencrypt/client/auth_handler.py +++ b/letsencrypt/client/auth_handler.py @@ -151,8 +151,10 @@ class AuthHandler(object): # pylint: disable=too-many-instance-attributes flat_auth.extend(ichall.chall for ichall in self.dv_c[dom]) try: - client_resp = self.client_auth.perform(flat_client) - dv_resp = self.dv_auth.perform(flat_auth) + if flat_client: + client_resp = self.client_auth.perform(flat_client) + if flat_auth: + dv_resp = self.dv_auth.perform(flat_auth) # This will catch both specific types of errors. except errors.LetsEncryptAuthHandlerError as err: logging.critical("Failure in setting up challenges:") @@ -212,9 +214,13 @@ class AuthHandler(object): # pylint: disable=too-many-instance-attributes # These are indexed challenges... give just the challenges to the auth # Chose to make these lists instead of a generator to make it easier to # work with... - self.dv_auth.cleanup([ichall.chall for ichall in self.dv_c[domain]]) - self.client_auth.cleanup( - [ichall.chall for ichall in self.client_c[domain]]) + dv_list = [ichall.chall for ichall in self.dv_c[domain]] + client_list = [ichall.chall for ichall in self.client_c[domain]] + if dv_list: + self.dv_auth.cleanup(dv_list) + if client_list: + self.client_auth.cleanup(client_list) + def _cleanup_state(self, delete_list): """Cleanup state after an authorization is received. diff --git a/letsencrypt/client/interfaces.py b/letsencrypt/client/interfaces.py index 9fcd95c6a..04c7d35e7 100644 --- a/letsencrypt/client/interfaces.py +++ b/letsencrypt/client/interfaces.py @@ -30,6 +30,9 @@ class IAuthenticator(zope.interface.Interface): :param list chall_list: List of namedtuple types defined in :mod:`letsencrypt.client.challenge_util` (``DvsniChall``, etc.). + - chall_list will never be empty + - chall_list will only contain types found within + :func:`get_chall_pref` :returns: ACME Challenge responses or if it cannot be completed then: @@ -43,20 +46,16 @@ class IAuthenticator(zope.interface.Interface): """ def cleanup(chall_list): - """Revert changes and shutdown after challenges complete.""" + """Revert changes and shutdown after challenges complete. + :param list chall_list: List of namedtuple types defined in + :mod:`letsencrypt.client.challenge_util` (``DvsniChall``, etc.) -class IChallenge(zope.interface.Interface): - """Let's Encrypt challenge.""" + - Only challenges given previously in the perform function will be + found in chall_list. + - chall_list will never be empty - def perform(): - """Perform the challenge.""" - - def generate_response(): - """Generate response.""" - - def cleanup(): - """Cleanup.""" + """ class IConfig(zope.interface.Interface): From d976de4104558ba546c28ccb7a1280f90446c6be Mon Sep 17 00:00:00 2001 From: James Kasten Date: Thu, 12 Feb 2015 01:32:26 -0800 Subject: [PATCH 02/10] fixed use before assignment bug --- letsencrypt/client/auth_handler.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/letsencrypt/client/auth_handler.py b/letsencrypt/client/auth_handler.py index ee69898c6..2315eb24b 100644 --- a/letsencrypt/client/auth_handler.py +++ b/letsencrypt/client/auth_handler.py @@ -145,16 +145,19 @@ class AuthHandler(object): # pylint: disable=too-many-instance-attributes # Order is important here as we will not expose the outside # Authenticator to our own indices. flat_client = [] - flat_auth = [] + flat_dv = [] + for dom in self.domains: flat_client.extend(ichall.chall for ichall in self.client_c[dom]) - flat_auth.extend(ichall.chall for ichall in self.dv_c[dom]) + flat_dv.extend(ichall.chall for ichall in self.dv_c[dom]) + client_resp = [] + dv_resp = [] try: if flat_client: client_resp = self.client_auth.perform(flat_client) - if flat_auth: - dv_resp = self.dv_auth.perform(flat_auth) + if flat_dv: + dv_resp = self.dv_auth.perform(flat_dv) # This will catch both specific types of errors. except errors.LetsEncryptAuthHandlerError as err: logging.critical("Failure in setting up challenges:") @@ -169,8 +172,10 @@ class AuthHandler(object): # pylint: disable=too-many-instance-attributes logging.info("Ready for verification...") # Assemble Responses - self._assign_responses(client_resp, self.client_c) - self._assign_responses(dv_resp, self.dv_c) + if client_resp: + self._assign_responses(client_resp, self.client_c) + if dv_resp: + self._assign_responses(dv_resp, self.dv_c) def _assign_responses(self, flat_list, ichall_dict): """Assign responses from flat_list back to the IndexedChall dicts. From 23ba0166fe5a6e80db372f701ec5c54cf611181d Mon Sep 17 00:00:00 2001 From: James Kasten Date: Thu, 12 Feb 2015 01:40:40 -0800 Subject: [PATCH 03/10] Fix quotes --- letsencrypt/client/tests/auth_handler_test.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/letsencrypt/client/tests/auth_handler_test.py b/letsencrypt/client/tests/auth_handler_test.py index 137b1627e..2125b4882 100644 --- a/letsencrypt/client/tests/auth_handler_test.py +++ b/letsencrypt/client/tests/auth_handler_test.py @@ -25,8 +25,8 @@ class SatisfyChallengesTest(unittest.TestCase): def setUp(self): from letsencrypt.client.auth_handler import AuthHandler - self.mock_dv_auth = mock.MagicMock(name='ApacheConfigurator') - self.mock_client_auth = mock.MagicMock(name='ClientAuthenticator') + self.mock_dv_auth = mock.MagicMock(name="ApacheConfigurator") + self.mock_client_auth = mock.MagicMock(name="ClientAuthenticator") self.mock_dv_auth.get_chall_pref.return_value = ["dvsni"] self.mock_client_auth.get_chall_pref.return_value = ["recoveryToken"] @@ -293,8 +293,8 @@ class GetAuthorizationsTest(unittest.TestCase): def setUp(self): from letsencrypt.client.auth_handler import AuthHandler - self.mock_dv_auth = mock.MagicMock(name='ApacheConfigurator') - self.mock_client_auth = mock.MagicMock(name='ClientAuthenticator') + self.mock_dv_auth = mock.MagicMock(name="ApacheConfigurator") + self.mock_client_auth = mock.MagicMock(name="ClientAuthenticator") self.mock_sat_chall = mock.MagicMock(name="_satisfy_challenges") self.mock_acme_auth = mock.MagicMock(name="acme_authorization") @@ -484,5 +484,5 @@ def gen_path(str_list, challenges): return path -if __name__ == '__main__': +if __name__ == "__main__": unittest.main() From 3a7960710306d3e601759a6fc36361ff6f04ed67 Mon Sep 17 00:00:00 2001 From: James Kasten Date: Thu, 12 Feb 2015 02:42:49 -0800 Subject: [PATCH 04/10] Add tests to satisfy empty list assumption of authenticator interface --- letsencrypt/client/auth_handler.py | 1 + letsencrypt/client/tests/acme_util.py | 2 +- letsencrypt/client/tests/auth_handler_test.py | 70 ++++++++++++++----- 3 files changed, 55 insertions(+), 18 deletions(-) diff --git a/letsencrypt/client/auth_handler.py b/letsencrypt/client/auth_handler.py index 2315eb24b..984862f46 100644 --- a/letsencrypt/client/auth_handler.py +++ b/letsencrypt/client/auth_handler.py @@ -129,6 +129,7 @@ class AuthHandler(object): # pylint: disable=too-many-instance-attributes .. todo:: It might be worth it to try different challenges to find one that doesn't throw an exception + .. todo:: separate into more functions """ logging.info("Performing the following challenges:") diff --git a/letsencrypt/client/tests/acme_util.py b/letsencrypt/client/tests/acme_util.py index aa142af8e..08a7e44bd 100644 --- a/letsencrypt/client/tests/acme_util.py +++ b/letsencrypt/client/tests/acme_util.py @@ -26,7 +26,7 @@ CHALLENGES = { "successURL": "https://example.ca/confirmrecovery/bb1b9928932", "contact": "c********n@example.com" }, - "recoveryTokent": + "recoveryToken": { "type": "recoveryToken" }, diff --git a/letsencrypt/client/tests/auth_handler_test.py b/letsencrypt/client/tests/auth_handler_test.py index 2125b4882..945141f4e 100644 --- a/letsencrypt/client/tests/auth_handler_test.py +++ b/letsencrypt/client/tests/auth_handler_test.py @@ -59,6 +59,29 @@ class SatisfyChallengesTest(unittest.TestCase): self.assertEqual(len(self.handler.dv_c[dom]), 1) self.assertEqual(len(self.handler.client_c[dom]), 0) + def test_name1_rectok1(self): + dom = "0" + challenge = [acme_util.CHALLENGES["recoveryToken"]] + msg = acme_util.get_chall_msg(dom, "nonce0", challenge) + self.handler.add_chall_msg(dom, msg, "dummy_key") + + self.handler._satisfy_challenges() # pylint: disable=protected-access + + self.assertEqual(len(self.handler.responses), 1) + self.assertEqual(len(self.handler.responses[dom]), 1) + + # Test if statement for dv_auth perform + self.assertEqual(self.mock_client_auth.perform.call_count, 1) + self.assertEqual(self.mock_dv_auth.perform.call_count, 0) + + self.assertEqual("RecTokenChall0", self.handler.responses[dom][0]) + # Assert 1 domain + self.assertEqual(len(self.handler.dv_c), 1) + self.assertEqual(len(self.handler.client_c), 1) + # Assert 1 auth challenge, 0 dv + self.assertEqual(len(self.handler.dv_c[dom]), 0) + self.assertEqual(len(self.handler.client_c[dom]), 1) + def test_name5_dvsni5(self): challenge = [acme_util.CHALLENGES["dvsni"]] for i in xrange(5): @@ -74,6 +97,10 @@ class SatisfyChallengesTest(unittest.TestCase): self.assertEqual(len(self.handler.client_c), 5) # Each message contains 1 auth, 0 client + # Test proper call count for methods + self.assertEqual(self.mock_client_auth.perform.call_count, 0) + self.assertEqual(self.mock_dv_auth.perform.call_count, 1) + for i in xrange(5): dom = str(i) self.assertEqual(len(self.handler.responses[dom]), 1) @@ -103,6 +130,10 @@ class SatisfyChallengesTest(unittest.TestCase): self.assertEqual(len(self.handler.dv_c), 1) self.assertEqual(len(self.handler.client_c), 1) + # Test if statement for client_auth perform + self.assertEqual(self.mock_client_auth.perform.call_count, 0) + self.assertEqual(self.mock_dv_auth.perform.call_count, 1) + self.assertEqual( self.handler.responses[dom], self._get_exp_response(dom, path, challenges)) @@ -251,33 +282,38 @@ class SatisfyChallengesTest(unittest.TestCase): str(i), "nonce%d" % i, challenges, combos), "dummy_key") - mock_chall_path.return_value = gen_path( - ["dvsni", "proofOfPossession"], challenges) + mock_chall_path.side_effect = [ + gen_path(["dvsni", "proofOfPossession"], challenges), + gen_path(["proofOfPossession"], challenges), + gen_path(["dvsni"], challenges), + ] # This may change in the future... but for now catch the error self.assertRaises(errors.LetsEncryptAuthHandlerError, self.handler._satisfy_challenges) # Verify cleanup is actually run correctly - self.assertEqual(self.mock_dv_auth.cleanup.call_count, 3) - self.assertEqual(self.mock_client_auth.cleanup.call_count, 3) + self.assertEqual(self.mock_dv_auth.cleanup.call_count, 2) + self.assertEqual(self.mock_client_auth.cleanup.call_count, 2) + + + dv_cleanup_args = self.mock_dv_auth.cleanup.call_args_list + client_cleanup_args = self.mock_client_auth.cleanup.call_args_list # Check DV cleanup - mock_cleanup_args = self.mock_dv_auth.cleanup.call_args_list - for i in xrange(3): - # Assert length of arg list was 1 - arg_chall_list = mock_cleanup_args[i][0][0] - self.assertEqual(len(arg_chall_list), 1) - self.assertTrue(isinstance(arg_chall_list[0], - challenge_util.DvsniChall)) + for i in xrange(2): + dv_chall_list = dv_cleanup_args[i][0][0] + self.assertEqual(len(dv_chall_list), 1) + self.assertTrue( + isinstance(dv_chall_list[0], challenge_util.DvsniChall)) + # Check Auth cleanup - mock_cleanup_args = self.mock_client_auth.cleanup.call_args_list - for i in xrange(3): - arg_chall_list = mock_cleanup_args[i][0][0] - self.assertEqual(len(arg_chall_list), 1) - self.assertTrue(isinstance(arg_chall_list[0], - challenge_util.PopChall)) + for i in xrange(2): + client_chall_list = client_cleanup_args[i][0][0] + self.assertEqual(len(client_chall_list), 1) + self.assertTrue( + isinstance(client_chall_list[0], challenge_util.PopChall)) def _get_exp_response(self, domain, path, challenges): # pylint: disable=no-self-use From e1cdc79bdcb712e14a22658f153a2f4839828fb1 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Thu, 12 Feb 2015 15:21:35 -0800 Subject: [PATCH 05/10] 100% coverage for obj.py --- letsencrypt/client/tests/apache/obj_test.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/letsencrypt/client/tests/apache/obj_test.py b/letsencrypt/client/tests/apache/obj_test.py index 0dccd3afb..070fa7b11 100644 --- a/letsencrypt/client/tests/apache/obj_test.py +++ b/letsencrypt/client/tests/apache/obj_test.py @@ -31,9 +31,7 @@ class AddrTest(unittest.TestCase): def test_eq(self): self.assertEqual(self.addr1, self.addr2.get_addr_obj("")) self.assertNotEqual(self.addr1, self.addr2) - # This is specifically designed to hit line 28 but coverage denies me - # the satisfaction :( - self.assertNotEqual(self.addr1, 3333) + self.assertFalse(self.addr1 == 3333) def test_set_inclusion(self): from letsencrypt.client.apache.obj import Addr @@ -63,7 +61,7 @@ class VirtualHostTest(unittest.TestCase): self.assertEqual(vhost1b, self.vhost1) self.assertEqual(str(vhost1b), str(self.vhost1)) - self.assertNotEqual(vhost1b, 1234) + self.assertFalse(vhost1b == 1234) if __name__ == "__main__": From 42865fbc92cb058e641e2ef4879c6f6413a6f33f Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Thu, 12 Feb 2015 19:27:33 -0800 Subject: [PATCH 06/10] Add (Linux-specific) already_listening method --- .../client/standalone_authenticator.py | 53 ++++++++++++++- .../tests/standalone_authenticator_test.py | 66 +++++++++++++++++++ 2 files changed, 117 insertions(+), 2 deletions(-) diff --git a/letsencrypt/client/standalone_authenticator.py b/letsencrypt/client/standalone_authenticator.py index 81c3e381f..206e28c78 100644 --- a/letsencrypt/client/standalone_authenticator.py +++ b/letsencrypt/client/standalone_authenticator.py @@ -2,6 +2,7 @@ import os import signal import socket +import subprocess import sys import time @@ -150,8 +151,9 @@ class StandaloneAuthenticator(object): elif self.subproc_state == "inuse": display.generic_notification( "Could not bind TCP port {0} because it is already in " - "use it is already in use by another process on this " - "system (such as a web server).".format(port)) + "use by another process on this system (such as a web " + "server). Please stop the program in question and then " + "try again.".format(port)) return False elif self.subproc_state == "cantbind": display.generic_notification( @@ -258,6 +260,47 @@ class StandaloneAuthenticator(object): # should terminate via sys.exit(). return self.do_child_process(port, key) + def already_listening(self, port): # pylint: disable=no-self-use + """Check if a process is already listening on the port. + + If so, also tell the user via a display notification. + + .. warning:: + The current implementation is Linux-specific. (On other + operating systems, it will simply not detect bound ports.) + This function can only usefully be run as root. + + :param int port: The TCP port in question. + :returns: True or False.""" + + try: + proc = subprocess.Popen( + ["/bin/netstat", "-nta", "--program"], + stdout=subprocess.PIPE, stderr=subprocess.PIPE) + stdout, _ = proc.communicate() + if proc.wait() != 0: + raise OSError("netstat subprocess failed") + lines = [x.split() for x in stdout.split("\n")[2:] if x] + listeners = [L[6] for L in lines if L[0] == 'tcp' \ + and L[5] == 'LISTEN' \ + and L[3] == '0.0.0.0:{0}'.format(port)] + if listeners: + pid, name = listeners[0].split("/") + display = zope.component.getUtility(interfaces.IDisplay) + display.generic_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.".format(name, pid, port)) + return True + except (OSError, ValueError, IndexError): + # A sign that this command isn't available or usable this + # way on this operating system, or there was something + # unexpected about the format of the netstat output; we will + # not be able to recover from this condition. + pass + return False + # IAuthenticator method implementations follow def get_chall_pref(self, unused_domain): # pylint: disable=no-self-use @@ -317,6 +360,12 @@ class StandaloneAuthenticator(object): results_if_failure.append(False) if not self.tasks: raise ValueError("nothing for .perform() to do") + if self.already_listening(constants.DVSNI_CHALLENGE_PORT): + # If we know a process is already listening on this port, + # tell the user, and don't even attempt to bind it. (This + # test is Linux-specific and won't indicate that the port + # if invoked on a different operating system.) + return results_if_failure # Try to do the authentication; note that this creates # the listener subprocess via os.fork() if self.start_listener(constants.DVSNI_CHALLENGE_PORT, key): diff --git a/letsencrypt/client/tests/standalone_authenticator_test.py b/letsencrypt/client/tests/standalone_authenticator_test.py index 8b0336a59..940ead6d0 100644 --- a/letsencrypt/client/tests/standalone_authenticator_test.py +++ b/letsencrypt/client/tests/standalone_authenticator_test.py @@ -96,6 +96,7 @@ class SNICallbackTest(unittest.TestCase): called_ctx = connection.set_context.call_args[0][0] self.assertTrue(isinstance(called_ctx, OpenSSL.SSL.Context)) + class ClientSignalHandlerTest(unittest.TestCase): """Tests for client_signal_handler() method.""" def setUp(self): @@ -177,6 +178,60 @@ class SubprocSignalHandlerTest(unittest.TestCase): mock_exit.assert_called_once_with(0) +class AlreadyListeningTest(unittest.TestCase): + """Tests for already_listening() method.""" + def setUp(self): + from letsencrypt.client.standalone_authenticator import \ + StandaloneAuthenticator + self.authenticator = StandaloneAuthenticator() + + @mock.patch("letsencrypt.client.standalone_authenticator.subprocess.Popen") + def test_subprocess_fails(self, mock_popen): + subprocess_object = mock.MagicMock() + subprocess_object.communicate.return_value = ("foo", "bar") + subprocess_object.wait.return_value = 1 + mock_popen.return_value = subprocess_object + result = self.authenticator.already_listening(17) + self.assertFalse(result) + subprocess_object.wait.assert_called_once_with() + + @mock.patch("letsencrypt.client.standalone_authenticator.subprocess.Popen") + def test_no_relevant_line(self, mock_popen): + # pylint: disable=line-too-long,trailing-whitespace + subprocess_object = mock.MagicMock() + subprocess_object.communicate.return_value = ( + """Active Internet connections (servers and established) +Proto Recv-Q Send-Q Local Address Foreign Address State PID/Program name +tcp 0 0 127.0.1.1:53 0.0.0.0:* LISTEN 1234/foo +tcp 0 0 127.0.0.1:631 0.0.0.0:* LISTEN 2345/bar +tcp 0 0 0.0.0.0:180 0.0.0.0:* LISTEN 11111/hello """, + "I am the standard error") + subprocess_object.wait.return_value = 0 + mock_popen.return_value = subprocess_object + result = self.authenticator.already_listening(17) + self.assertFalse(result) + + @mock.patch("letsencrypt.client.standalone_authenticator.subprocess.Popen") + @mock.patch("letsencrypt.client.standalone_authenticator." + "zope.component.getUtility") + def test_has_relevant_line(self, mock_get_utility, mock_popen): + # pylint: disable=line-too-long,trailing-whitespace + subprocess_object = mock.MagicMock() + subprocess_object.communicate.return_value = ( + """Active Internet connections (servers and established) +Proto Recv-Q Send-Q Local Address Foreign Address State PID/Program name +tcp 0 0 127.0.1.1:53 0.0.0.0:* LISTEN 1234/foo +tcp 0 0 127.0.0.1:631 0.0.0.0:* LISTEN 2345/bar +tcp 0 0 0.0.0.0:17 0.0.0.0:* LISTEN 11111/hello +tcp 0 0 0.0.0.0:1728 0.0.0.0:* LISTEN 2345/bar """, + "I am the standard error") + subprocess_object.wait.return_value = 0 + mock_popen.return_value = subprocess_object + result = self.authenticator.already_listening(17) + self.assertTrue(result) + self.assertEqual(mock_get_utility.call_count, 1) + + class PerformTest(unittest.TestCase): """Tests for perform() method.""" def setUp(self): @@ -184,6 +239,17 @@ class PerformTest(unittest.TestCase): StandaloneAuthenticator self.authenticator = StandaloneAuthenticator() + def test_perform_when_already_listening(self): + test_key = pkg_resources.resource_string( + __name__, "testdata/rsa256_key.pem") + key = le_util.Key("something", test_key) + chall1 = challenge_util.DvsniChall( + "foo.example.com", "whee", "foononce", key) + self.authenticator.already_listening = mock.Mock() + self.authenticator.already_listening.return_value = True + result = self.authenticator.perform([chall1]) + self.assertEqual(result, [None]) + def test_can_perform(self): """What happens if start_listener() returns True.""" test_key = pkg_resources.resource_string( From 2001d180af81c90812f4c91809c92500da5898ba Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Thu, 12 Feb 2015 22:55:25 -0800 Subject: [PATCH 07/10] Fix typo in comment --- letsencrypt/client/standalone_authenticator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/client/standalone_authenticator.py b/letsencrypt/client/standalone_authenticator.py index 206e28c78..3e9a4381c 100644 --- a/letsencrypt/client/standalone_authenticator.py +++ b/letsencrypt/client/standalone_authenticator.py @@ -364,7 +364,7 @@ class StandaloneAuthenticator(object): # If we know a process is already listening on this port, # tell the user, and don't even attempt to bind it. (This # test is Linux-specific and won't indicate that the port - # if invoked on a different operating system.) + # is bound if invoked on a different operating system.) return results_if_failure # Try to do the authentication; note that this creates # the listener subprocess via os.fork() From 5c5313ba73777748ad30811101e3bf92a4ec7c5f Mon Sep 17 00:00:00 2001 From: James Kasten Date: Fri, 13 Feb 2015 01:56:11 -0800 Subject: [PATCH 08/10] fix domains bug introduced in 220 --- letsencrypt/scripts/main.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/letsencrypt/scripts/main.py b/letsencrypt/scripts/main.py index d73f6b668..e61a93fee 100755 --- a/letsencrypt/scripts/main.py +++ b/letsencrypt/scripts/main.py @@ -138,8 +138,7 @@ def main(): # pylint: disable=too-many-branches else: auth = client.determine_authenticator(config) - if args.domains is None: - domains = choose_names(installer) + doms = choose_names(installer) if args.domains is None else args.domains # Prepare for init of Client if args.privkey is None: @@ -157,11 +156,11 @@ def main(): # pylint: disable=too-many-branches # I am not sure the best way to handle all of the unimplemented abilities, # but this code should be safe on all environments. if auth is not None: - cert_file, chain_file = acme.obtain_certificate(domains) + cert_file, chain_file = acme.obtain_certificate(doms) if installer is not None and cert_file is not None: - acme.deploy_certificate(domains, privkey, cert_file, chain_file) + acme.deploy_certificate(doms, privkey, cert_file, chain_file) if installer is not None: - acme.enhance_config(domains, args.redirect) + acme.enhance_config(doms, args.redirect) def display_eula(): From 67627c19d791507dffc352b7308010dc8b1ef64a Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Fri, 13 Feb 2015 14:59:59 -0800 Subject: [PATCH 09/10] netstat output can differ on an IPv6 system --- .../client/standalone_authenticator.py | 10 +++++++--- .../tests/standalone_authenticator_test.py | 19 +++++++++++++++++++ 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/letsencrypt/client/standalone_authenticator.py b/letsencrypt/client/standalone_authenticator.py index 3e9a4381c..c4a1c63b1 100644 --- a/letsencrypt/client/standalone_authenticator.py +++ b/letsencrypt/client/standalone_authenticator.py @@ -281,9 +281,13 @@ class StandaloneAuthenticator(object): if proc.wait() != 0: raise OSError("netstat subprocess failed") lines = [x.split() for x in stdout.split("\n")[2:] if x] - listeners = [L[6] for L in lines if L[0] == 'tcp' \ - and L[5] == 'LISTEN' \ - and L[3] == '0.0.0.0:{0}'.format(port)] + listeners = [L[6] for L in lines if + # IPv4 socket case + (L[0] == 'tcp' and L[5] == 'LISTEN' \ + and L[3] == '0.0.0.0:{0}'.format(port)) or \ + # IPv6 socket case + (L[0] == 'tcp6' and L[5] == 'LISTEN' \ + and L[3] == ':::{0}'.format(port))] if listeners: pid, name = listeners[0].split("/") display = zope.component.getUtility(interfaces.IDisplay) diff --git a/letsencrypt/client/tests/standalone_authenticator_test.py b/letsencrypt/client/tests/standalone_authenticator_test.py index 940ead6d0..9787073b1 100644 --- a/letsencrypt/client/tests/standalone_authenticator_test.py +++ b/letsencrypt/client/tests/standalone_authenticator_test.py @@ -231,6 +231,25 @@ tcp 0 0 0.0.0.0:1728 0.0.0.0:* LISTEN self.assertTrue(result) self.assertEqual(mock_get_utility.call_count, 1) + @mock.patch("letsencrypt.client.standalone_authenticator.subprocess.Popen") + @mock.patch("letsencrypt.client.standalone_authenticator." + "zope.component.getUtility") + def test_has_relevant_ipv6_line(self, mock_get_utility, mock_popen): + # pylint: disable=line-too-long,trailing-whitespace + subprocess_object = mock.MagicMock() + subprocess_object.communicate.return_value = ( + """Active Internet connections (servers and established) +Proto Recv-Q Send-Q Local Address Foreign Address State PID/Program name +tcp 0 0 127.0.1.1:53 0.0.0.0:* LISTEN 1234/foo +tcp 0 0 127.0.0.1:631 0.0.0.0:* LISTEN 2345/bar +tcp6 0 0 :::17 :::* LISTEN 11111/hello +tcp 0 0 0.0.0.0:1728 0.0.0.0:* LISTEN 2345/bar """, + "I am the standard error") + subprocess_object.wait.return_value = 0 + mock_popen.return_value = subprocess_object + result = self.authenticator.already_listening(17) + self.assertTrue(result) + self.assertEqual(mock_get_utility.call_count, 1) class PerformTest(unittest.TestCase): """Tests for perform() method.""" From fbd9e6f0db05638e0e483cde9ccec84d0863ceca Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Fri, 13 Feb 2015 15:14:01 -0800 Subject: [PATCH 10/10] Specify location of netstat binary as a constant --- letsencrypt/client/constants.py | 4 ++++ letsencrypt/client/standalone_authenticator.py | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/letsencrypt/client/constants.py b/letsencrypt/client/constants.py index 291506940..5a1715788 100644 --- a/letsencrypt/client/constants.py +++ b/letsencrypt/client/constants.py @@ -66,3 +66,7 @@ IConfig.work_dir. Used for easy revocation.""" REC_TOKEN_DIR = "recovery_tokens" """Directory where all recovery tokens are saved (relative to IConfig.work_dir).""" + +NETSTAT = "/bin/netstat" +"""Location of netstat binary for checking whether a listener is already +running on the specified port (Linux-specific).""" diff --git a/letsencrypt/client/standalone_authenticator.py b/letsencrypt/client/standalone_authenticator.py index c4a1c63b1..e2b1d7872 100644 --- a/letsencrypt/client/standalone_authenticator.py +++ b/letsencrypt/client/standalone_authenticator.py @@ -275,7 +275,7 @@ class StandaloneAuthenticator(object): try: proc = subprocess.Popen( - ["/bin/netstat", "-nta", "--program"], + [constants.NETSTAT, "-nta", "--program"], stdout=subprocess.PIPE, stderr=subprocess.PIPE) stdout, _ = proc.communicate() if proc.wait() != 0: