From a966bc0797b9a9fa3d6aaa0a5a81d61257b49dc0 Mon Sep 17 00:00:00 2001 From: Liam Marshall Date: Sat, 24 Oct 2015 21:27:49 -0500 Subject: [PATCH 01/13] letsencrypt.plugins.manual: Add disclaimer about IP logging --- letsencrypt/plugins/manual.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/letsencrypt/plugins/manual.py b/letsencrypt/plugins/manual.py index f7717064b..d722b8e36 100644 --- a/letsencrypt/plugins/manual.py +++ b/letsencrypt/plugins/manual.py @@ -46,6 +46,15 @@ If you don't have HTTP server configured, you can run the following command on the target server (as root): {command} +""" + + # a disclaimer about your current IP being transmitted to Let's Encrypt's servers. + IP_DISCLAIMER = """\ +NOTE: The IP of this machine will be publicly logged as having requested this certificate. \ +If you're running letsencrypt in manual mode on a machine that is not your server, \ +please ensure you're okay with that. + +Are you OK with your IP being logged? """ # "cd /tmp/letsencrypt" makes sure user doesn't serve /root, @@ -151,6 +160,10 @@ binary for temporary key/certificate generation.""".replace("\n", "") if self._httpd.poll() is not None: raise errors.Error("Couldn't execute manual command") else: + if not zope.component.getUtility(interfaces.IDisplay).yesno( + self.IP_DISCLAIMER, "Yes", "No"): + raise errors.Error("Must agree to proceed") + self._notify_and_wait(self.MESSAGE_TEMPLATE.format( validation=validation.json_dumps(), response=response, uri=response.uri(achall.domain, achall.challb.chall), From 4adc6d32692e6d2e2e70b81a8f7ec5865ed687a3 Mon Sep 17 00:00:00 2001 From: Liam Marshall Date: Sat, 24 Oct 2015 21:50:27 -0500 Subject: [PATCH 02/13] Make pep8 happy --- letsencrypt/plugins/manual.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/plugins/manual.py b/letsencrypt/plugins/manual.py index d722b8e36..cdbc97aa0 100644 --- a/letsencrypt/plugins/manual.py +++ b/letsencrypt/plugins/manual.py @@ -161,7 +161,7 @@ binary for temporary key/certificate generation.""".replace("\n", "") raise errors.Error("Couldn't execute manual command") else: if not zope.component.getUtility(interfaces.IDisplay).yesno( - self.IP_DISCLAIMER, "Yes", "No"): + self.IP_DISCLAIMER, "Yes", "No"): raise errors.Error("Must agree to proceed") self._notify_and_wait(self.MESSAGE_TEMPLATE.format( From aa0c7d993256fe063cbf1254c5d4ed215902f3ac Mon Sep 17 00:00:00 2001 From: Liam Marshall Date: Sat, 24 Oct 2015 22:03:30 -0500 Subject: [PATCH 03/13] manual_test: mock yesno interaction --- letsencrypt/plugins/manual_test.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/letsencrypt/plugins/manual_test.py b/letsencrypt/plugins/manual_test.py index 8cfff1cc5..d5266d711 100644 --- a/letsencrypt/plugins/manual_test.py +++ b/letsencrypt/plugins/manual_test.py @@ -43,11 +43,13 @@ class AuthenticatorTest(unittest.TestCase): def test_perform_empty(self): self.assertEqual([], self.auth.perform([])) + @mock.patch("letsencrypt.proof_of_possession.zope.component.getUtility") @mock.patch("letsencrypt.plugins.manual.sys.stdout") @mock.patch("acme.challenges.SimpleHTTPResponse.simple_verify") @mock.patch("__builtin__.raw_input") - def test_perform(self, mock_raw_input, mock_verify, mock_stdout): + def test_perform(self, mock_raw_input, mock_verify, mock_stdout, mock_input, mock_interaction): mock_verify.return_value = True + mock_interaction.yesno.return_value = True resp = challenges.SimpleHTTPResponse(tls=False) self.assertEqual([resp], self.auth.perform(self.achalls)) From 2d295bce9d6324c34e1034640399ca714a84ec06 Mon Sep 17 00:00:00 2001 From: Liam Marshall Date: Sat, 24 Oct 2015 22:17:16 -0500 Subject: [PATCH 04/13] Better error message --- letsencrypt/plugins/manual.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/plugins/manual.py b/letsencrypt/plugins/manual.py index cdbc97aa0..6798c9c92 100644 --- a/letsencrypt/plugins/manual.py +++ b/letsencrypt/plugins/manual.py @@ -162,7 +162,7 @@ binary for temporary key/certificate generation.""".replace("\n", "") else: if not zope.component.getUtility(interfaces.IDisplay).yesno( self.IP_DISCLAIMER, "Yes", "No"): - raise errors.Error("Must agree to proceed") + raise errors.Error("Must agree to IP logging to proceed") self._notify_and_wait(self.MESSAGE_TEMPLATE.format( validation=validation.json_dumps(), response=response, From a45c4d157aa9f120025b1d133eca612f7c83fee7 Mon Sep 17 00:00:00 2001 From: Liam Marshall Date: Sat, 24 Oct 2015 23:27:39 -0500 Subject: [PATCH 05/13] Oops, copy-pasted the patch --- letsencrypt/plugins/manual_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt/plugins/manual_test.py b/letsencrypt/plugins/manual_test.py index d5266d711..617bad958 100644 --- a/letsencrypt/plugins/manual_test.py +++ b/letsencrypt/plugins/manual_test.py @@ -43,11 +43,11 @@ class AuthenticatorTest(unittest.TestCase): def test_perform_empty(self): self.assertEqual([], self.auth.perform([])) - @mock.patch("letsencrypt.proof_of_possession.zope.component.getUtility") + @mock.patch("letsencrypt.plugins.manual.zope.component.getUtility") @mock.patch("letsencrypt.plugins.manual.sys.stdout") @mock.patch("acme.challenges.SimpleHTTPResponse.simple_verify") @mock.patch("__builtin__.raw_input") - def test_perform(self, mock_raw_input, mock_verify, mock_stdout, mock_input, mock_interaction): + def test_perform(self, mock_raw_input, mock_verify, mock_stdout, mock_interaction): mock_verify.return_value = True mock_interaction.yesno.return_value = True From a88f9cdc374783adb8f42e4fd09d99559f98f055 Mon Sep 17 00:00:00 2001 From: Liam Marshall Date: Sun, 25 Oct 2015 14:31:30 -0500 Subject: [PATCH 06/13] Switch from Error to PluginError --- letsencrypt/plugins/manual.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/plugins/manual.py b/letsencrypt/plugins/manual.py index 6798c9c92..61a5acdb3 100644 --- a/letsencrypt/plugins/manual.py +++ b/letsencrypt/plugins/manual.py @@ -162,7 +162,7 @@ binary for temporary key/certificate generation.""".replace("\n", "") else: if not zope.component.getUtility(interfaces.IDisplay).yesno( self.IP_DISCLAIMER, "Yes", "No"): - raise errors.Error("Must agree to IP logging to proceed") + raise errors.PluginError("Must agree to IP logging to proceed") self._notify_and_wait(self.MESSAGE_TEMPLATE.format( validation=validation.json_dumps(), response=response, From a21e149f74466f9cb56509071099b7b56e97a308 Mon Sep 17 00:00:00 2001 From: Liam Marshall Date: Sun, 25 Oct 2015 14:54:39 -0500 Subject: [PATCH 07/13] Need to *call* mock_interaction as a function --- letsencrypt/plugins/manual_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/plugins/manual_test.py b/letsencrypt/plugins/manual_test.py index 617bad958..7a75c02b5 100644 --- a/letsencrypt/plugins/manual_test.py +++ b/letsencrypt/plugins/manual_test.py @@ -49,7 +49,7 @@ class AuthenticatorTest(unittest.TestCase): @mock.patch("__builtin__.raw_input") def test_perform(self, mock_raw_input, mock_verify, mock_stdout, mock_interaction): mock_verify.return_value = True - mock_interaction.yesno.return_value = True + mock_interaction().yesno.return_value = True resp = challenges.SimpleHTTPResponse(tls=False) self.assertEqual([resp], self.auth.perform(self.achalls)) From 7e94876f73d069375b10c56701fdb24bf6ef7988 Mon Sep 17 00:00:00 2001 From: Liam Marshall Date: Sun, 25 Oct 2015 15:04:26 -0500 Subject: [PATCH 08/13] Add a test for disagreeing with IP logging --- letsencrypt/plugins/manual_test.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/letsencrypt/plugins/manual_test.py b/letsencrypt/plugins/manual_test.py index 7a75c02b5..9476e2316 100644 --- a/letsencrypt/plugins/manual_test.py +++ b/letsencrypt/plugins/manual_test.py @@ -106,6 +106,18 @@ class AuthenticatorTest(unittest.TestCase): self.auth_test_mode.cleanup(self.achalls) mock_killpg.assert_called_once_with(1234, signal.SIGTERM) + @mock.patch("letsencrypt.plugins.manual.zope.component.getUtility") + @mock.patch("letsencrypt.plugins.manual.sys.stdout") + @mock.patch("acme.challenges.SimpleHTTPResponse.simple_verify") + @mock.patch("__builtin__.raw_input") + def test_disagree_with_ip_logging(self, mock_raw_input, mock_verify, mock_stdout, mock_interaction): + mock_verify.return_value = True + mock_interaction().yesno.return_value = False + + resp = challenges.SimpleHTTPResponse(tls=False) + with self.assertRaises(errors.PluginError): + self.auth.perform(self.achalls) + if __name__ == "__main__": unittest.main() # pragma: no cover From 17bd3790177f0cd8fda2009f295be14b61c78cdd Mon Sep 17 00:00:00 2001 From: Liam Marshall Date: Sun, 25 Oct 2015 15:32:32 -0500 Subject: [PATCH 09/13] Clean up tests --- letsencrypt/plugins/manual_test.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/letsencrypt/plugins/manual_test.py b/letsencrypt/plugins/manual_test.py index 9476e2316..d3b880e47 100644 --- a/letsencrypt/plugins/manual_test.py +++ b/letsencrypt/plugins/manual_test.py @@ -110,11 +110,11 @@ class AuthenticatorTest(unittest.TestCase): @mock.patch("letsencrypt.plugins.manual.sys.stdout") @mock.patch("acme.challenges.SimpleHTTPResponse.simple_verify") @mock.patch("__builtin__.raw_input") - def test_disagree_with_ip_logging(self, mock_raw_input, mock_verify, mock_stdout, mock_interaction): - mock_verify.return_value = True + def test_disagree_with_ip_logging(self, mock_raw_input, mock_verify, mock_stdout, + mock_interaction): + # pylint: disable=unused-argument mock_interaction().yesno.return_value = False - resp = challenges.SimpleHTTPResponse(tls=False) with self.assertRaises(errors.PluginError): self.auth.perform(self.achalls) From 88cc70b5a526b2ce7666135f3dc0b5844bebbe94 Mon Sep 17 00:00:00 2001 From: Liam Marshall Date: Sun, 25 Oct 2015 15:46:21 -0500 Subject: [PATCH 10/13] Oops, can't use the form of assertRaises on 2.6 --- letsencrypt/plugins/manual_test.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/letsencrypt/plugins/manual_test.py b/letsencrypt/plugins/manual_test.py index d3b880e47..77871a51e 100644 --- a/letsencrypt/plugins/manual_test.py +++ b/letsencrypt/plugins/manual_test.py @@ -115,8 +115,7 @@ class AuthenticatorTest(unittest.TestCase): # pylint: disable=unused-argument mock_interaction().yesno.return_value = False - with self.assertRaises(errors.PluginError): - self.auth.perform(self.achalls) + self.assertRaises(errors.PluginError, self.auth.perform, self.achalls) if __name__ == "__main__": From 0a5303ccf0660d9984e6c1ebe457c123947164e0 Mon Sep 17 00:00:00 2001 From: Liam Marshall Date: Sun, 25 Oct 2015 16:15:10 -0500 Subject: [PATCH 11/13] test_disagree_with_ip_logging: move, remove spurious @patches --- letsencrypt/plugins/manual_test.py | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/letsencrypt/plugins/manual_test.py b/letsencrypt/plugins/manual_test.py index 77871a51e..673942799 100644 --- a/letsencrypt/plugins/manual_test.py +++ b/letsencrypt/plugins/manual_test.py @@ -63,6 +63,12 @@ class AuthenticatorTest(unittest.TestCase): mock_verify.return_value = False self.assertEqual([None], self.auth.perform(self.achalls)) + @mock.patch("letsencrypt.plugins.manual.zope.component.getUtility") + def test_disagree_with_ip_logging(self, mock_interaction): + mock_interaction().yesno.return_value = False + + self.assertRaises(errors.PluginError, self.auth.perform, self.achalls) + @mock.patch("letsencrypt.plugins.manual.subprocess.Popen", autospec=True) def test_perform_test_command_oserror(self, mock_popen): mock_popen.side_effect = OSError @@ -106,17 +112,6 @@ class AuthenticatorTest(unittest.TestCase): self.auth_test_mode.cleanup(self.achalls) mock_killpg.assert_called_once_with(1234, signal.SIGTERM) - @mock.patch("letsencrypt.plugins.manual.zope.component.getUtility") - @mock.patch("letsencrypt.plugins.manual.sys.stdout") - @mock.patch("acme.challenges.SimpleHTTPResponse.simple_verify") - @mock.patch("__builtin__.raw_input") - def test_disagree_with_ip_logging(self, mock_raw_input, mock_verify, mock_stdout, - mock_interaction): - # pylint: disable=unused-argument - mock_interaction().yesno.return_value = False - - self.assertRaises(errors.PluginError, self.auth.perform, self.achalls) - if __name__ == "__main__": unittest.main() # pragma: no cover From 3f22e0d6f2b89f6ee3d36f882b0de2b58d937880 Mon Sep 17 00:00:00 2001 From: Liam Marshall Date: Sun, 25 Oct 2015 16:32:56 -0500 Subject: [PATCH 12/13] test_disagree_with_ip_logging: make it fail nicely --- letsencrypt/plugins/manual_test.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/letsencrypt/plugins/manual_test.py b/letsencrypt/plugins/manual_test.py index 673942799..e04ff3594 100644 --- a/letsencrypt/plugins/manual_test.py +++ b/letsencrypt/plugins/manual_test.py @@ -64,8 +64,10 @@ class AuthenticatorTest(unittest.TestCase): self.assertEqual([None], self.auth.perform(self.achalls)) @mock.patch("letsencrypt.plugins.manual.zope.component.getUtility") - def test_disagree_with_ip_logging(self, mock_interaction): + @mock.patch("letsencrypt.plugins.manual.Authenticator._notify_and_wait") + def test_disagree_with_ip_logging(self, mock_notify, mock_interaction): mock_interaction().yesno.return_value = False + mock_notify.side_effect = errors.Error("Exception not raised, continued execution even after disagreeing with IP logging") self.assertRaises(errors.PluginError, self.auth.perform, self.achalls) From a90013bc2df6129771bc0dd1f75ed18be2c89049 Mon Sep 17 00:00:00 2001 From: Liam Marshall Date: Sun, 25 Oct 2015 16:43:03 -0500 Subject: [PATCH 13/13] Make linter happy --- letsencrypt/plugins/manual_test.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/letsencrypt/plugins/manual_test.py b/letsencrypt/plugins/manual_test.py index e04ff3594..a52129635 100644 --- a/letsencrypt/plugins/manual_test.py +++ b/letsencrypt/plugins/manual_test.py @@ -67,7 +67,8 @@ class AuthenticatorTest(unittest.TestCase): @mock.patch("letsencrypt.plugins.manual.Authenticator._notify_and_wait") def test_disagree_with_ip_logging(self, mock_notify, mock_interaction): mock_interaction().yesno.return_value = False - mock_notify.side_effect = errors.Error("Exception not raised, continued execution even after disagreeing with IP logging") + mock_notify.side_effect = errors.Error("Exception not raised, \ + continued execution even after disagreeing with IP logging") self.assertRaises(errors.PluginError, self.auth.perform, self.achalls)