From a0acf7c7030a7687f6b0bad1640f1f730d221071 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sun, 21 Jun 2015 08:18:51 +0000 Subject: [PATCH 1/7] acme.verify.simple_http_simple_verify --- acme/challenges.py | 2 ++ acme/verify.py | 33 +++++++++++++++++++++++ acme/verify_test.py | 43 ++++++++++++++++++++++++++++++ letsencrypt/plugins/manual.py | 29 +++----------------- letsencrypt/plugins/manual_test.py | 20 ++++++-------- setup.py | 1 - 6 files changed, 90 insertions(+), 38 deletions(-) create mode 100644 acme/verify.py create mode 100644 acme/verify_test.py diff --git a/acme/challenges.py b/acme/challenges.py index 9ea06645d..b856be888 100644 --- a/acme/challenges.py +++ b/acme/challenges.py @@ -72,6 +72,8 @@ class SimpleHTTPResponse(ChallengeResponse): [RFC4648]", base64.b64decode ignores those characters """ + # TODO: check that path combined with uri does not go above + # URI_ROOT_PATH! return len(self.path) <= 25 @property diff --git a/acme/verify.py b/acme/verify.py new file mode 100644 index 000000000..9945b75a7 --- /dev/null +++ b/acme/verify.py @@ -0,0 +1,33 @@ +"""Simple challenges verification utilities.""" +import logging + +import requests + + +logger = logging.getLogger(__name__) + + +def simple_http_simple_verify(response, chall, domain): + """Verify SimpleHTTP. + + According to the ACME specification, "the ACME server MUST ignore + the certificate provided by the HTTPS server", so ``requests.get`` + is called with ``verify=False``. + + """ + uri = response.uri(domain) + logger.debug("Verifying %s at %s...", chall.typ, uri) + try: + http_response = requests.get(uri, verify=False) + except requests.exceptions.RequestException as error: + logger.error("Unable to verify %s: %s", uri, error) + return False + logger.debug( + 'Received %s. Headers: %s', http_response, http_response.headers) + + good_token = http_response.text == chall.token + if not good_token: + logger.error( + "Unable to verify %s! Expected: %r, returned: %r.", + uri, chall.token, http_response.text) + return response.good_path and http_response and good_token diff --git a/acme/verify_test.py b/acme/verify_test.py new file mode 100644 index 000000000..a76ba959f --- /dev/null +++ b/acme/verify_test.py @@ -0,0 +1,43 @@ +"""Tests for acme.verify.""" +import unittest + +import mock +import requests + +from acme import challenges + + +class SimpleHTTPSimpleVerifyTest(unittest.TestCase): + """Tests for acme.verify.simple_http_simple_verify.""" + + def setUp(self): + self.chall = challenges.SimpleHTTP(token="foo") + self.resp_http = challenges.SimpleHTTPResponse(path="bar", tls=False) + self.resp_https = challenges.SimpleHTTPResponse(path="bar", tls=True) + + @classmethod + def _call(cls, *args, **kwargs): + from acme.verify import simple_http_simple_verify + return simple_http_simple_verify(*args, **kwargs) + + @mock.patch("acme.verify.requests.get") + def test_good_token(self, mock_get): + for resp in self.resp_http, self.resp_https: + mock_get.reset_mock() + mock_get.return_value = mock.MagicMock(text=self.chall.token) + self.assertTrue(self._call(resp, self.chall, "local")) + mock_get.assert_called_once_with(resp.uri("local"), verify=False) + + @mock.patch("acme.verify.requests.get") + def test_bad_token(self, mock_get): + mock_get().text = self.chall.token + "!" + self.assertFalse(self._call(self.resp_http, self.chall, "local")) + + @mock.patch("acme.verify.requests.get") + def test_connection_error(self, mock_get): + mock_get.side_effect = requests.exceptions.RequestException + self.assertFalse(self._call(self.resp_http, self.chall, "local")) + + +if __name__ == '__main__': + unittest.main() # pragma: no cover diff --git a/letsencrypt/plugins/manual.py b/letsencrypt/plugins/manual.py index b16665581..7626b8031 100644 --- a/letsencrypt/plugins/manual.py +++ b/letsencrypt/plugins/manual.py @@ -1,22 +1,18 @@ """Manual plugin.""" -import logging import os import sys -import requests import zope.component import zope.interface from acme import challenges from acme import jose +from acme import verify as acme_verify from letsencrypt import interfaces from letsencrypt.plugins import common -logger = logging.getLogger(__name__) - - class ManualAuthenticator(common.Plugin): """Manual Authenticator. @@ -61,9 +57,7 @@ s.serve_forever()" """ According to the ACME specification, "the ACME server MUST ignore the certificate provided by the HTTPS server", so the first command - generates temporary self-signed certificate. For the same reason - ``requests.get`` in `_verify` sets ``verify=False``. Python HTTPS - server command serves the ``token`` on all URIs. + generates temporary self-signed certificate. """ @@ -109,7 +103,8 @@ binary for temporary key/certificate generation.""".replace("\n", "") uri=response.uri(achall.domain), command=self.template.format(achall=achall, response=response))) - if self._verify(achall, response): + if acme_verify.simple_http_simple_verify( + response, achall.challb, achall.domain): return response else: return None @@ -121,21 +116,5 @@ binary for temporary key/certificate generation.""".replace("\n", "") sys.stdout.write(message) raw_input("Press ENTER to continue") - def _verify(self, achall, chall_response): # pylint: disable=no-self-use - uri = chall_response.uri(achall.domain) - logger.debug("Verifying %s...", uri) - try: - response = requests.get(uri, verify=False) - except requests.exceptions.ConnectionError as error: - logger.exception(error) - return False - - ret = response.text == achall.token - if not ret: - logger.error("Unable to verify %s! Expected: %r, returned: %r.", - uri, achall.token, response.text) - - return ret - def cleanup(self, achalls): # pylint: disable=missing-docstring,no-self-use pass # pragma: no cover diff --git a/letsencrypt/plugins/manual_test.py b/letsencrypt/plugins/manual_test.py index c95654dec..059aebaba 100644 --- a/letsencrypt/plugins/manual_test.py +++ b/letsencrypt/plugins/manual_test.py @@ -2,7 +2,6 @@ import unittest import mock -import requests from acme import challenges @@ -32,28 +31,25 @@ class ManualAuthenticatorTest(unittest.TestCase): @mock.patch("letsencrypt.plugins.manual.sys.stdout") @mock.patch("letsencrypt.plugins.manual.os.urandom") - @mock.patch("letsencrypt.plugins.manual.requests.get") + @mock.patch("acme.verify.simple_http_simple_verify") @mock.patch("__builtin__.raw_input") - def test_perform(self, mock_raw_input, mock_get, mock_urandom, mock_stdout): + def test_perform(self, mock_raw_input, mock_verify, mock_urandom, + mock_stdout): mock_urandom.return_value = "foo" - mock_get().text = self.achalls[0].token + mock_verify.return_value = True - self.assertEqual( - [challenges.SimpleHTTPResponse(tls=False, path='Zm9v')], - self.auth.perform(self.achalls)) + resp = challenges.SimpleHTTPResponse(tls=False, path='Zm9v') + self.assertEqual([resp], self.auth.perform(self.achalls)) mock_raw_input.assert_called_once() - mock_get.assert_called_with( - "http://foo.com/.well-known/acme-challenge/Zm9v", verify=False) + mock_verify.assert_called_with(resp, self.achalls[0].challb, "foo.com") message = mock_stdout.write.mock_calls[0][1][0] self.assertTrue(self.achalls[0].token in message) self.assertTrue('Zm9v' in message) - mock_get().text = self.achalls[0].token + '!' + mock_verify.return_value = False self.assertEqual([None], self.auth.perform(self.achalls)) - mock_get.side_effect = requests.exceptions.ConnectionError - self.assertEqual([None], self.auth.perform(self.achalls)) if __name__ == "__main__": unittest.main() # pragma: no cover diff --git a/setup.py b/setup.py index 520802b9f..ca2746113 100644 --- a/setup.py +++ b/setup.py @@ -61,7 +61,6 @@ letsencrypt_install_requires = [ 'pyrfc3339', 'python2-pythondialog>=3.2.2rc1', # Debian squeeze support, cf. #280 'pytz', - 'requests', 'zope.component', 'zope.interface', 'M2Crypto', From 36752a3dabc3a647e46feb19a751a26975d2ac42 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Wed, 24 Jun 2015 11:12:02 +0000 Subject: [PATCH 2/7] simpleHttp needs text/plain or absent. --- acme/challenges.py | 2 ++ acme/verify.py | 8 ++++++-- acme/verify_test.py | 13 +++++++++++-- letsencrypt/plugins/manual.py | 14 ++++++++++---- 4 files changed, 29 insertions(+), 8 deletions(-) diff --git a/acme/challenges.py b/acme/challenges.py index b856be888..39a927bc5 100644 --- a/acme/challenges.py +++ b/acme/challenges.py @@ -63,6 +63,8 @@ class SimpleHTTPResponse(ChallengeResponse): MAX_PATH_LEN = 25 """Maximum allowed `path` length.""" + CONTENT_TYPE = "text/plain" + @property def good_path(self): """Is `path` good? diff --git a/acme/verify.py b/acme/verify.py index 9945b75a7..c0092d9fb 100644 --- a/acme/verify.py +++ b/acme/verify.py @@ -20,7 +20,7 @@ def simple_http_simple_verify(response, chall, domain): try: http_response = requests.get(uri, verify=False) except requests.exceptions.RequestException as error: - logger.error("Unable to verify %s: %s", uri, error) + logger.error("Unable to reach %s: %s", uri, error) return False logger.debug( 'Received %s. Headers: %s', http_response, http_response.headers) @@ -30,4 +30,8 @@ def simple_http_simple_verify(response, chall, domain): logger.error( "Unable to verify %s! Expected: %r, returned: %r.", uri, chall.token, http_response.text) - return response.good_path and http_response and good_token + # TODO: spec contradicts itself, c.f. + # https://github.com/letsencrypt/acme-spec/pull/156/files#r33136438 + good_ct = response.CONTENT_TYPE == http_response.headers.get( + "Content-Type", response.CONTENT_TYPE) + return response.good_path and good_ct and good_token diff --git a/acme/verify_test.py b/acme/verify_test.py index a76ba959f..908c7ff1b 100644 --- a/acme/verify_test.py +++ b/acme/verify_test.py @@ -14,6 +14,8 @@ class SimpleHTTPSimpleVerifyTest(unittest.TestCase): self.chall = challenges.SimpleHTTP(token="foo") self.resp_http = challenges.SimpleHTTPResponse(path="bar", tls=False) self.resp_https = challenges.SimpleHTTPResponse(path="bar", tls=True) + self.good_headers = { + 'Content-Type': challenges.SimpleHTTPResponse.CONTENT_TYPE} @classmethod def _call(cls, *args, **kwargs): @@ -24,13 +26,20 @@ class SimpleHTTPSimpleVerifyTest(unittest.TestCase): def test_good_token(self, mock_get): for resp in self.resp_http, self.resp_https: mock_get.reset_mock() - mock_get.return_value = mock.MagicMock(text=self.chall.token) + mock_get.return_value = mock.MagicMock( + text=self.chall.token, headers=self.good_headers) self.assertTrue(self._call(resp, self.chall, "local")) mock_get.assert_called_once_with(resp.uri("local"), verify=False) @mock.patch("acme.verify.requests.get") def test_bad_token(self, mock_get): - mock_get().text = self.chall.token + "!" + mock_get.return_value = mock.MagicMock( + text=self.chall.token + "!", headers=self.good_headers) + self.assertFalse(self._call(self.resp_http, self.chall, "local")) + + @mock.patch("acme.verify.requests.get") + def test_bad_content_type(self, mock_get): + mock_get().text = self.chall.token self.assertFalse(self._call(self.resp_http, self.chall, "local")) @mock.patch("acme.verify.requests.get") diff --git a/letsencrypt/plugins/manual.py b/letsencrypt/plugins/manual.py index 7626b8031..5e964a17e 100644 --- a/letsencrypt/plugins/manual.py +++ b/letsencrypt/plugins/manual.py @@ -30,6 +30,8 @@ Make sure your web server displays the following content at {achall.token} +Content-Type header MUST be set to {ct}. + If you don't have HTTP server configured, you can run the following command on the target server (as root): @@ -40,7 +42,10 @@ command on the target server (as root): mkdir -p {response.URI_ROOT_PATH} echo -n {achall.token} > {response.URI_ROOT_PATH}/{response.path} # run only once per server: -python -m SimpleHTTPServer 80""" +python -c "import BaseHTTPServer, SimpleHTTPServer; \\ +SimpleHTTPServer.SimpleHTTPRequestHandler.extensions_map = {{'': '{ct}'}}; \\ +s = BaseHTTPServer.HTTPServer(('', 80), SimpleHTTPServer.SimpleHTTPRequestHandler); \\ +s.serve_forever()" """ """Non-TLS command template.""" # https://www.piware.de/2011/01/creating-an-https-server-in-python/ @@ -50,6 +55,7 @@ echo -n {achall.token} > {response.URI_ROOT_PATH}/{response.path} # run only once per server: openssl req -new -newkey rsa:4096 -subj "/" -days 1 -nodes -x509 -keyout key.pem -out cert.pem python -c "import BaseHTTPServer, SimpleHTTPServer, ssl; \\ +SimpleHTTPServer.SimpleHTTPRequestHandler.extensions_map = {{'': '{ct}'}}; \\ s = BaseHTTPServer.HTTPServer(('', 443), SimpleHTTPServer.SimpleHTTPRequestHandler); \\ s.socket = ssl.wrap_socket(s.socket, keyfile='key.pem', certfile='cert.pem'); \\ s.serve_forever()" """ @@ -99,9 +105,9 @@ binary for temporary key/certificate generation.""".replace("\n", "") assert response.good_path # is encoded os.urandom(18) good? self._notify_and_wait(self.MESSAGE_TEMPLATE.format( - achall=achall, response=response, - uri=response.uri(achall.domain), - command=self.template.format(achall=achall, response=response))) + achall=achall, response=response, uri=response.uri(achall.domain), + ct=response.CONTENT_TYPE, command=self.template.format( + achall=achall, response=response, ct=response.CONTENT_TYPE))) if acme_verify.simple_http_simple_verify( response, achall.challb, achall.domain): From ce32de023ddf8676cf2525589eda9864b35cb348 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Wed, 24 Jun 2015 20:32:42 +0000 Subject: [PATCH 3/7] Move simple_http_simple_verify to SimpleHTTPResponse.simple_verify. --- acme/challenges.py | 41 +++++++++++++++++++++++ acme/challenges_test.py | 34 +++++++++++++++++++ acme/verify.py | 37 --------------------- acme/verify_test.py | 52 ------------------------------ letsencrypt/plugins/manual.py | 4 +-- letsencrypt/plugins/manual_test.py | 4 +-- 6 files changed, 78 insertions(+), 94 deletions(-) delete mode 100644 acme/verify.py delete mode 100644 acme/verify_test.py diff --git a/acme/challenges.py b/acme/challenges.py index 39a927bc5..e15893006 100644 --- a/acme/challenges.py +++ b/acme/challenges.py @@ -2,13 +2,18 @@ import binascii import functools import hashlib +import logging import Crypto.Random +import requests from acme import jose from acme import other +logger = logging.getLogger(__name__) + + # pylint: disable=too-few-public-methods @@ -95,6 +100,42 @@ class SimpleHTTPResponse(ChallengeResponse): return self._URI_TEMPLATE.format( scheme=self.scheme, domain=domain, path=self.path) + def simple_verify(self, chall, domain): + """Simple verify. + + According to the ACME specification, "the ACME server MUST + ignore the certificate provided by the HTTPS server", so + ``requests.get`` is called with ``verify=False``. + + :param .SimpleHTTP chall: Corresponding challenge. + :param str domain: Domain name being verified. + + :returns: ``True`` iff validation is successful, ``False`` + otherwise. + :rtype: bool + + """ + uri = self.uri(domain) + logger.debug("Verifying %s at %s...", chall.typ, uri) + try: + http_response = requests.get(uri, verify=False) + except requests.exceptions.RequestException as error: + logger.error("Unable to reach %s: %s", uri, error) + return False + logger.debug( + "Received %s. Headers: %s", http_response, http_response.headers) + + good_token = http_response.text == chall.token + if not good_token: + logger.error( + "Unable to verify %s! Expected: %r, returned: %r.", + uri, chall.token, http_response.text) + # TODO: spec contradicts itself, c.f. + # https://github.com/letsencrypt/acme-spec/pull/156/files#r33136438 + good_ct = self.CONTENT_TYPE == http_response.headers.get( + "Content-Type", self.CONTENT_TYPE) + return self.good_path and good_ct and good_token + @Challenge.register class DVSNI(DVChallenge): diff --git a/acme/challenges_test.py b/acme/challenges_test.py index f0b025ad3..a786065da 100644 --- a/acme/challenges_test.py +++ b/acme/challenges_test.py @@ -5,6 +5,8 @@ import unittest import Crypto.PublicKey.RSA import M2Crypto +import mock +import requests from acme import jose from acme import other @@ -49,6 +51,7 @@ class SimpleHTTPTest(unittest.TestCase): class SimpleHTTPResponseTest(unittest.TestCase): + # pylint: disable=too-many-instance-attributes def setUp(self): from acme.challenges import SimpleHTTPResponse @@ -66,6 +69,12 @@ class SimpleHTTPResponseTest(unittest.TestCase): 'tls': True, } + from acme.challenges import SimpleHTTP + self.chall = SimpleHTTP(token="foo") + self.resp_http = SimpleHTTPResponse(path="bar", tls=False) + self.resp_https = SimpleHTTPResponse(path="bar", tls=True) + self.good_headers = {'Content-Type': SimpleHTTPResponse.CONTENT_TYPE} + def test_good_path(self): self.assertTrue(self.msg_http.good_path) self.assertTrue(self.msg_https.good_path) @@ -98,6 +107,31 @@ class SimpleHTTPResponseTest(unittest.TestCase): hash(SimpleHTTPResponse.from_json(self.jmsg_http)) hash(SimpleHTTPResponse.from_json(self.jmsg_https)) + @mock.patch("acme.challenges.requests.get") + def test_simple_verify_good_token(self, mock_get): + for resp in self.resp_http, self.resp_https: + mock_get.reset_mock() + mock_get.return_value = mock.MagicMock( + text=self.chall.token, headers=self.good_headers) + self.assertTrue(resp.simple_verify(self.chall, "local")) + mock_get.assert_called_once_with(resp.uri("local"), verify=False) + + @mock.patch("acme.challenges.requests.get") + def test_simple_verify_bad_token(self, mock_get): + mock_get.return_value = mock.MagicMock( + text=self.chall.token + "!", headers=self.good_headers) + self.assertFalse(self.resp_http.simple_verify(self.chall, "local")) + + @mock.patch("acme.challenges.requests.get") + def test_simple_verify_bad_content_type(self, mock_get): + mock_get().text = self.chall.token + self.assertFalse(self.resp_http.simple_verify(self.chall, "local")) + + @mock.patch("acme.challenges.requests.get") + def test_simple_verify_connection_error(self, mock_get): + mock_get.side_effect = requests.exceptions.RequestException + self.assertFalse(self.resp_http.simple_verify(self.chall, "local")) + class DVSNITest(unittest.TestCase): diff --git a/acme/verify.py b/acme/verify.py deleted file mode 100644 index c0092d9fb..000000000 --- a/acme/verify.py +++ /dev/null @@ -1,37 +0,0 @@ -"""Simple challenges verification utilities.""" -import logging - -import requests - - -logger = logging.getLogger(__name__) - - -def simple_http_simple_verify(response, chall, domain): - """Verify SimpleHTTP. - - According to the ACME specification, "the ACME server MUST ignore - the certificate provided by the HTTPS server", so ``requests.get`` - is called with ``verify=False``. - - """ - uri = response.uri(domain) - logger.debug("Verifying %s at %s...", chall.typ, uri) - try: - http_response = requests.get(uri, verify=False) - except requests.exceptions.RequestException as error: - logger.error("Unable to reach %s: %s", uri, error) - return False - logger.debug( - 'Received %s. Headers: %s', http_response, http_response.headers) - - good_token = http_response.text == chall.token - if not good_token: - logger.error( - "Unable to verify %s! Expected: %r, returned: %r.", - uri, chall.token, http_response.text) - # TODO: spec contradicts itself, c.f. - # https://github.com/letsencrypt/acme-spec/pull/156/files#r33136438 - good_ct = response.CONTENT_TYPE == http_response.headers.get( - "Content-Type", response.CONTENT_TYPE) - return response.good_path and good_ct and good_token diff --git a/acme/verify_test.py b/acme/verify_test.py deleted file mode 100644 index 908c7ff1b..000000000 --- a/acme/verify_test.py +++ /dev/null @@ -1,52 +0,0 @@ -"""Tests for acme.verify.""" -import unittest - -import mock -import requests - -from acme import challenges - - -class SimpleHTTPSimpleVerifyTest(unittest.TestCase): - """Tests for acme.verify.simple_http_simple_verify.""" - - def setUp(self): - self.chall = challenges.SimpleHTTP(token="foo") - self.resp_http = challenges.SimpleHTTPResponse(path="bar", tls=False) - self.resp_https = challenges.SimpleHTTPResponse(path="bar", tls=True) - self.good_headers = { - 'Content-Type': challenges.SimpleHTTPResponse.CONTENT_TYPE} - - @classmethod - def _call(cls, *args, **kwargs): - from acme.verify import simple_http_simple_verify - return simple_http_simple_verify(*args, **kwargs) - - @mock.patch("acme.verify.requests.get") - def test_good_token(self, mock_get): - for resp in self.resp_http, self.resp_https: - mock_get.reset_mock() - mock_get.return_value = mock.MagicMock( - text=self.chall.token, headers=self.good_headers) - self.assertTrue(self._call(resp, self.chall, "local")) - mock_get.assert_called_once_with(resp.uri("local"), verify=False) - - @mock.patch("acme.verify.requests.get") - def test_bad_token(self, mock_get): - mock_get.return_value = mock.MagicMock( - text=self.chall.token + "!", headers=self.good_headers) - self.assertFalse(self._call(self.resp_http, self.chall, "local")) - - @mock.patch("acme.verify.requests.get") - def test_bad_content_type(self, mock_get): - mock_get().text = self.chall.token - self.assertFalse(self._call(self.resp_http, self.chall, "local")) - - @mock.patch("acme.verify.requests.get") - def test_connection_error(self, mock_get): - mock_get.side_effect = requests.exceptions.RequestException - self.assertFalse(self._call(self.resp_http, self.chall, "local")) - - -if __name__ == '__main__': - unittest.main() # pragma: no cover diff --git a/letsencrypt/plugins/manual.py b/letsencrypt/plugins/manual.py index 5e964a17e..5afc87cf4 100644 --- a/letsencrypt/plugins/manual.py +++ b/letsencrypt/plugins/manual.py @@ -7,7 +7,6 @@ import zope.interface from acme import challenges from acme import jose -from acme import verify as acme_verify from letsencrypt import interfaces from letsencrypt.plugins import common @@ -109,8 +108,7 @@ binary for temporary key/certificate generation.""".replace("\n", "") ct=response.CONTENT_TYPE, command=self.template.format( achall=achall, response=response, ct=response.CONTENT_TYPE))) - if acme_verify.simple_http_simple_verify( - response, achall.challb, achall.domain): + if response.simple_verify(achall.challb, achall.domain): return response else: return None diff --git a/letsencrypt/plugins/manual_test.py b/letsencrypt/plugins/manual_test.py index 059aebaba..d412735bf 100644 --- a/letsencrypt/plugins/manual_test.py +++ b/letsencrypt/plugins/manual_test.py @@ -31,7 +31,7 @@ class ManualAuthenticatorTest(unittest.TestCase): @mock.patch("letsencrypt.plugins.manual.sys.stdout") @mock.patch("letsencrypt.plugins.manual.os.urandom") - @mock.patch("acme.verify.simple_http_simple_verify") + @mock.patch("acme.challenges.SimpleHTTPResponse.simple_verify") @mock.patch("__builtin__.raw_input") def test_perform(self, mock_raw_input, mock_verify, mock_urandom, mock_stdout): @@ -41,7 +41,7 @@ class ManualAuthenticatorTest(unittest.TestCase): resp = challenges.SimpleHTTPResponse(tls=False, path='Zm9v') self.assertEqual([resp], self.auth.perform(self.achalls)) mock_raw_input.assert_called_once() - mock_verify.assert_called_with(resp, self.achalls[0].challb, "foo.com") + mock_verify.assert_called_with(self.achalls[0].challb, "foo.com") message = mock_stdout.write.mock_calls[0][1][0] self.assertTrue(self.achalls[0].token in message) From 87f197afb2b5ae0c13253c8eca73d0e5d932d07f Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Wed, 24 Jun 2015 15:41:25 +0000 Subject: [PATCH 4/7] manual: make sure user doesn't serve /root, or cert.pem/key.pem --- letsencrypt/plugins/manual.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/letsencrypt/plugins/manual.py b/letsencrypt/plugins/manual.py index 5afc87cf4..eff5e7784 100644 --- a/letsencrypt/plugins/manual.py +++ b/letsencrypt/plugins/manual.py @@ -37,8 +37,14 @@ command on the target server (as root): {command} """ + # "cd /tmp/letsencrypt" makes sure user doesn't serve /root, + # separate "public_html" ensures that cert.pem/key.pem are not + # served and makes it more obvious that Python command will serve + # anything recursively under the cwd + HTTP_TEMPLATE = """\ -mkdir -p {response.URI_ROOT_PATH} +mkdir -p /tmp/letsencrypt/public_html/{response.URI_ROOT_PATH} +cd /tmp/letsencrypt/public_html echo -n {achall.token} > {response.URI_ROOT_PATH}/{response.path} # run only once per server: python -c "import BaseHTTPServer, SimpleHTTPServer; \\ @@ -49,14 +55,15 @@ s.serve_forever()" """ # https://www.piware.de/2011/01/creating-an-https-server-in-python/ HTTPS_TEMPLATE = """\ -mkdir -p {response.URI_ROOT_PATH} # run only once per server +mkdir -p /tmp/letsencrypt/public_html/{response.URI_ROOT_PATH} +cd /tmp/letsencrypt/public_html echo -n {achall.token} > {response.URI_ROOT_PATH}/{response.path} # run only once per server: -openssl req -new -newkey rsa:4096 -subj "/" -days 1 -nodes -x509 -keyout key.pem -out cert.pem +openssl req -new -newkey rsa:4096 -subj "/" -days 1 -nodes -x509 -keyout ../key.pem -out ../cert.pem python -c "import BaseHTTPServer, SimpleHTTPServer, ssl; \\ SimpleHTTPServer.SimpleHTTPRequestHandler.extensions_map = {{'': '{ct}'}}; \\ s = BaseHTTPServer.HTTPServer(('', 443), SimpleHTTPServer.SimpleHTTPRequestHandler); \\ -s.socket = ssl.wrap_socket(s.socket, keyfile='key.pem', certfile='cert.pem'); \\ +s.socket = ssl.wrap_socket(s.socket, keyfile='../key.pem', certfile='../cert.pem'); \\ s.serve_forever()" """ """TLS command template. From 29e56d442f95db9c2e4f7dce88a526a48dfb429f Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Wed, 24 Jun 2015 20:42:46 +0000 Subject: [PATCH 5/7] Fix line-too-long --- acme/challenges_test.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/acme/challenges_test.py b/acme/challenges_test.py index a786065da..4a3867c44 100644 --- a/acme/challenges_test.py +++ b/acme/challenges_test.py @@ -86,10 +86,12 @@ class SimpleHTTPResponseTest(unittest.TestCase): self.assertEqual('https', self.msg_https.scheme) def test_uri(self): - self.assertEqual('http://example.com/.well-known/acme-challenge/' - '6tbIMBC5Anhl5bOlWT5ZFA', self.msg_http.uri('example.com')) - self.assertEqual('https://example.com/.well-known/acme-challenge/' - '6tbIMBC5Anhl5bOlWT5ZFA', self.msg_https.uri('example.com')) + self.assertEqual( + 'http://example.com/.well-known/acme-challenge/' + '6tbIMBC5Anhl5bOlWT5ZFA', self.msg_http.uri('example.com')) + self.assertEqual( + 'https://example.com/.well-known/acme-challenge/' + '6tbIMBC5Anhl5bOlWT5ZFA', self.msg_https.uri('example.com')) def test_to_partial_json(self): self.assertEqual(self.jmsg_http, self.msg_http.to_partial_json()) From 2ec451d00baba8056c933ea5f3e87977bc230442 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Wed, 24 Jun 2015 15:48:27 +0000 Subject: [PATCH 6/7] IConfig.simple_http_port (fixes #542). --- acme/challenges.py | 16 +++++++++++++++- acme/challenges_test.py | 11 +++++++++++ letsencrypt/cli.py | 4 +++- letsencrypt/interfaces.py | 2 ++ letsencrypt/plugins/manual.py | 11 +++++++---- letsencrypt/plugins/manual_test.py | 5 +++-- 6 files changed, 41 insertions(+), 8 deletions(-) diff --git a/acme/challenges.py b/acme/challenges.py index e15893006..0a2a461f0 100644 --- a/acme/challenges.py +++ b/acme/challenges.py @@ -88,6 +88,11 @@ class SimpleHTTPResponse(ChallengeResponse): """URL scheme for the provisioned resource.""" return "https" if self.tls else "http" + @property + def port(self): + """Port that the ACME client should be listening for validation.""" + return 443 if self.tls else 80 + def uri(self, domain): """Create an URI to the provisioned resource. @@ -100,7 +105,7 @@ class SimpleHTTPResponse(ChallengeResponse): return self._URI_TEMPLATE.format( scheme=self.scheme, domain=domain, path=self.path) - def simple_verify(self, chall, domain): + def simple_verify(self, chall, domain, port=None): """Simple verify. According to the ACME specification, "the ACME server MUST @@ -109,12 +114,21 @@ class SimpleHTTPResponse(ChallengeResponse): :param .SimpleHTTP chall: Corresponding challenge. :param str domain: Domain name being verified. + :param int port: Port used in the validation. :returns: ``True`` iff validation is successful, ``False`` otherwise. :rtype: bool """ + # TODO: ACME specification defines URI template that doesn't + # allow to use a custom port... Make sure port is not in the + # request URI, if it's standard. + if port is not None and port != self.port: + logger.warn( + "Using non-standard port for SimpleHTTP verification: %s", port) + domain += ":{0}".format(port) + uri = self.uri(domain) logger.debug("Verifying %s at %s...", chall.typ, uri) try: diff --git a/acme/challenges_test.py b/acme/challenges_test.py index 4a3867c44..bd332d1d9 100644 --- a/acme/challenges_test.py +++ b/acme/challenges_test.py @@ -7,6 +7,7 @@ import Crypto.PublicKey.RSA import M2Crypto import mock import requests +import urlparse from acme import jose from acme import other @@ -85,6 +86,10 @@ class SimpleHTTPResponseTest(unittest.TestCase): self.assertEqual('http', self.msg_http.scheme) self.assertEqual('https', self.msg_https.scheme) + def test_port(self): + self.assertEqual(80, self.msg_http.port) + self.assertEqual(443, self.msg_https.port) + def test_uri(self): self.assertEqual( 'http://example.com/.well-known/acme-challenge/' @@ -134,6 +139,12 @@ class SimpleHTTPResponseTest(unittest.TestCase): mock_get.side_effect = requests.exceptions.RequestException self.assertFalse(self.resp_http.simple_verify(self.chall, "local")) + @mock.patch("acme.challenges.requests.get") + def test_simple_verify_port(self, mock_get): + self.resp_http.simple_verify(self.chall, "local", 4430) + self.assertEqual("local:4430", urlparse.urlparse( + mock_get.mock_calls[0][1][0]).netloc) + class DVSNITest(unittest.TestCase): diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index e64044077..5e789bfb3 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -480,9 +480,11 @@ def create_parser(plugins, args): "testing", "--no-verify-ssl", action="store_true", help=config_help("no_verify_ssl"), default=flag_default("no_verify_ssl")) - helpful.add( # TODO: apache and nginx plugins do NOT respect it (#479) + helpful.add( # TODO: apache and nginx plugins do NOT respect it (#479) "testing", "--dvsni-port", type=int, default=flag_default("dvsni_port"), help=config_help("dvsni_port")) + helpful.add("testing", "--simple-http-port", type=int, + help=config_help("simple_http_port")) helpful.add("testing", "--no-simple-http-tls", action="store_true", help=config_help("no_simple_http_tls")) diff --git a/letsencrypt/interfaces.py b/letsencrypt/interfaces.py index 38ec4ada0..4b93757c8 100644 --- a/letsencrypt/interfaces.py +++ b/letsencrypt/interfaces.py @@ -188,6 +188,8 @@ class IConfig(zope.interface.Interface): no_simple_http_tls = zope.interface.Attribute( "Do not use TLS when solving SimpleHTTP challenges.") + simple_http_port = zope.interface.Attribute( + "Port used in the SimpleHttp challenge.") class IInstaller(IPlugin): diff --git a/letsencrypt/plugins/manual.py b/letsencrypt/plugins/manual.py index eff5e7784..700759194 100644 --- a/letsencrypt/plugins/manual.py +++ b/letsencrypt/plugins/manual.py @@ -49,7 +49,7 @@ echo -n {achall.token} > {response.URI_ROOT_PATH}/{response.path} # run only once per server: python -c "import BaseHTTPServer, SimpleHTTPServer; \\ SimpleHTTPServer.SimpleHTTPRequestHandler.extensions_map = {{'': '{ct}'}}; \\ -s = BaseHTTPServer.HTTPServer(('', 80), SimpleHTTPServer.SimpleHTTPRequestHandler); \\ +s = BaseHTTPServer.HTTPServer(('', {port}), SimpleHTTPServer.SimpleHTTPRequestHandler); \\ s.serve_forever()" """ """Non-TLS command template.""" @@ -62,7 +62,7 @@ echo -n {achall.token} > {response.URI_ROOT_PATH}/{response.path} openssl req -new -newkey rsa:4096 -subj "/" -days 1 -nodes -x509 -keyout ../key.pem -out ../cert.pem python -c "import BaseHTTPServer, SimpleHTTPServer, ssl; \\ SimpleHTTPServer.SimpleHTTPRequestHandler.extensions_map = {{'': '{ct}'}}; \\ -s = BaseHTTPServer.HTTPServer(('', 443), SimpleHTTPServer.SimpleHTTPRequestHandler); \\ +s = BaseHTTPServer.HTTPServer(('', {port}), SimpleHTTPServer.SimpleHTTPRequestHandler); \\ s.socket = ssl.wrap_socket(s.socket, keyfile='../key.pem', certfile='../cert.pem'); \\ s.serve_forever()" """ """TLS command template. @@ -113,9 +113,12 @@ binary for temporary key/certificate generation.""".replace("\n", "") self._notify_and_wait(self.MESSAGE_TEMPLATE.format( achall=achall, response=response, uri=response.uri(achall.domain), ct=response.CONTENT_TYPE, command=self.template.format( - achall=achall, response=response, ct=response.CONTENT_TYPE))) + achall=achall, response=response, ct=response.CONTENT_TYPE, + port=(response.port if self.config.simple_http_port is None + else self.config.simple_http_port)))) - if response.simple_verify(achall.challb, achall.domain): + if response.simple_verify( + achall.challb, achall.domain, self.config.simple_http_port): return response else: return None diff --git a/letsencrypt/plugins/manual_test.py b/letsencrypt/plugins/manual_test.py index d412735bf..a533bcc75 100644 --- a/letsencrypt/plugins/manual_test.py +++ b/letsencrypt/plugins/manual_test.py @@ -14,7 +14,8 @@ class ManualAuthenticatorTest(unittest.TestCase): def setUp(self): from letsencrypt.plugins.manual import ManualAuthenticator - self.config = mock.MagicMock(no_simple_http_tls=True) + self.config = mock.MagicMock( + no_simple_http_tls=True, simple_http_port=4430) self.auth = ManualAuthenticator(config=self.config, name="manual") self.achalls = [achallenges.SimpleHTTP( challb=acme_util.SIMPLE_HTTP, domain="foo.com", key=None)] @@ -41,7 +42,7 @@ class ManualAuthenticatorTest(unittest.TestCase): resp = challenges.SimpleHTTPResponse(tls=False, path='Zm9v') self.assertEqual([resp], self.auth.perform(self.achalls)) mock_raw_input.assert_called_once() - mock_verify.assert_called_with(self.achalls[0].challb, "foo.com") + mock_verify.assert_called_with(self.achalls[0].challb, "foo.com", 4430) message = mock_stdout.write.mock_calls[0][1][0] self.assertTrue(self.achalls[0].token in message) From dc9ffdbb7fd1cf98cd9180f4cf470d7fbbb40e45 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Thu, 2 Jul 2015 04:51:41 +0000 Subject: [PATCH 7/7] Update old TODO comment. --- letsencrypt/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 5e789bfb3..587a94934 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -480,7 +480,7 @@ def create_parser(plugins, args): "testing", "--no-verify-ssl", action="store_true", help=config_help("no_verify_ssl"), default=flag_default("no_verify_ssl")) - helpful.add( # TODO: apache and nginx plugins do NOT respect it (#479) + helpful.add( # TODO: apache plugin does NOT respect it (#479) "testing", "--dvsni-port", type=int, default=flag_default("dvsni_port"), help=config_help("dvsni_port")) helpful.add("testing", "--simple-http-port", type=int,