From 88507fea873c502eb2da038f0f04512e7239f66e Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 26 Jun 2015 18:07:40 -0700 Subject: [PATCH 01/50] Stopped catching silly things --- letsencrypt/cli.py | 6 +----- letsencrypt/tests/cli_test.py | 5 ----- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index c6df1052c..36cecafa4 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -676,11 +676,7 @@ def main(cli_args=sys.argv[1:]): except errors.Error as error: handle_exception_common() return error - except KeyboardInterrupt: - handle_exception_common() - # Ensures a new line is printed - return "" - except: # pylint: disable=bare-except + except Exception: # pylint: disable=broad-except handle_exception_common() return ("An unexpected error occured. Please see the logfiles in {0} " "for more details.".format(args.logs_dir)) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 6b8f77c62..6e826386b 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -67,11 +67,6 @@ class CLITest(unittest.TestCase): errors.Error, self._call, ['--debug'] + cmd_arg, attrs) self._call(cmd_arg, attrs) - attrs['view_config_changes.side_effect'] = [KeyboardInterrupt] - self.assertRaises( - KeyboardInterrupt, self._call, ['--debug'] + cmd_arg, attrs) - self._call(cmd_arg, attrs) - attrs['view_config_changes.side_effect'] = [ValueError] self.assertRaises( ValueError, self._call, ['--debug'] + cmd_arg, attrs) From b7a19486ed216db097fcf7829d4f4be74f178730 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sat, 27 Jun 2015 08:14:00 +0000 Subject: [PATCH 02/50] Plugins disco: catch PluginError in prepare(), debug log exceptions. --- letsencrypt/plugins/disco.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/letsencrypt/plugins/disco.py b/letsencrypt/plugins/disco.py index 059913e3b..e2e2d4c54 100644 --- a/letsencrypt/plugins/disco.py +++ b/letsencrypt/plugins/disco.py @@ -75,7 +75,7 @@ class PluginEntryPoint(object): if iface.implementedBy(self.plugin_cls): logger.debug( "%s implements %s but object does not verify: %s", - self.plugin_cls, iface.__name__, error) + self.plugin_cls, iface.__name__, error, exc_info=True) return False return True @@ -93,10 +93,14 @@ class PluginEntryPoint(object): try: self._initialized.prepare() except errors.MisconfigurationError as error: - logger.debug("Misconfigured %r: %s", self, error) + logger.debug("Misconfigured %r: %s", self, error, exc_info=True) self._prepared = error except errors.NoInstallationError as error: - logger.debug("No installation (%r): %s", self, error) + logger.debug( + "No installation (%r): %s", self, error, exc_info=True) + self._prepared = error + except errors.PluginError as error: + logger.debug("Other error:(%r): %s", self, error, exc_info=True) self._prepared = error else: self._prepared = True From be889d379437be18ce1c0518b2cfec15f6b8dd6a Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sat, 27 Jun 2015 08:19:16 +0000 Subject: [PATCH 03/50] letsencrypt_nginx: generate snakeoil cert/key (fixes #481). --- letsencrypt_nginx/configurator.py | 20 ++++++++++++++++---- letsencrypt_nginx/tests/configurator_test.py | 14 ++++++++++++++ 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/letsencrypt_nginx/configurator.py b/letsencrypt_nginx/configurator.py index b1dfdca31..852ff82ac 100644 --- a/letsencrypt_nginx/configurator.py +++ b/letsencrypt_nginx/configurator.py @@ -13,6 +13,7 @@ from acme import challenges from letsencrypt import achallenges from letsencrypt import constants as core_constants +from letsencrypt import crypto_util from letsencrypt import errors from letsencrypt import interfaces from letsencrypt import le_util @@ -263,6 +264,18 @@ class NginxConfigurator(common.Plugin): return all_names + def _get_snakeoil_paths(self): + # TODO: generate only once + tmp_dir = os.path.join(self.config.work_dir, "snakeoil") + key = crypto_util.init_save_key( + key_size=1024, key_dir=tmp_dir, keyname="key.pem") + cert_pem = crypto_util.make_ss_cert( + key.pem, domains=[socket.gethostname()]) + cert = os.path.join(tmp_dir, "cert.pem") + with open(cert, 'w') as cert_file: + cert_file.write(cert_pem) + return cert, key.file + def _make_server_ssl(self, vhost): """Makes a server SSL based on server_name and filename by adding a 'listen 443 ssl' directive to the server block. @@ -274,11 +287,10 @@ class NginxConfigurator(common.Plugin): :type vhost: :class:`~letsencrypt_nginx.obj.VirtualHost` """ + snakeoil_cert, snakeoil_key = self._get_snakeoil_paths() ssl_block = [['listen', '443 ssl'], - ['ssl_certificate', - '/etc/ssl/certs/ssl-cert-snakeoil.pem'], - ['ssl_certificate_key', - '/etc/ssl/private/ssl-cert-snakeoil.key'], + ['ssl_certificate', snakeoil_cert], + ['ssl_certificate_key', snakeoil_key], ['include', self.parser.loc["ssl_options"]]] self.parser.add_server_directives( vhost.filep, vhost.names, ssl_block) diff --git a/letsencrypt_nginx/tests/configurator_test.py b/letsencrypt_nginx/tests/configurator_test.py index 83085cc9f..48d71e27d 100644 --- a/letsencrypt_nginx/tests/configurator_test.py +++ b/letsencrypt_nginx/tests/configurator_test.py @@ -1,8 +1,10 @@ """Test for letsencrypt_nginx.configurator.""" +import os import shutil import unittest import mock +import OpenSSL from acme import challenges from acme import messages @@ -266,6 +268,18 @@ class NginxConfiguratorTest(util.NginxTest): mocked.returncode = 0 self.assertTrue(self.config.config_test()) + def test_get_snakeoil_paths(self): + # pylint: disable=protected-access + cert, key = self.config._get_snakeoil_paths() + self.assertTrue(os.path.exists(cert)) + self.assertTrue(os.path.exists(key)) + with open(cert) as cert_file: + OpenSSL.crypto.load_certificate( + OpenSSL.crypto.FILETYPE_PEM, cert_file.read()) + with open(key) as key_file: + OpenSSL.crypto.load_privatekey( + OpenSSL.crypto.FILETYPE_PEM, key_file.read()) + if __name__ == "__main__": unittest.main() # pragma: no cover From 30dfb6a1a97fa8f3c4dceefa76b6bb01daab2293 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sat, 27 Jun 2015 08:37:29 +0000 Subject: [PATCH 04/50] letsencrypt_nginx: respect IConfig.dvsni_port (partially fixes #479). --- letsencrypt_nginx/configurator.py | 4 ++-- letsencrypt_nginx/dvsni.py | 3 ++- letsencrypt_nginx/tests/configurator_test.py | 12 ++++++------ letsencrypt_nginx/tests/util.py | 1 + 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/letsencrypt_nginx/configurator.py b/letsencrypt_nginx/configurator.py index 852ff82ac..3e7de7322 100644 --- a/letsencrypt_nginx/configurator.py +++ b/letsencrypt_nginx/configurator.py @@ -288,7 +288,7 @@ class NginxConfigurator(common.Plugin): """ snakeoil_cert, snakeoil_key = self._get_snakeoil_paths() - ssl_block = [['listen', '443 ssl'], + ssl_block = [['listen', '{0} ssl'.format(self.config.dvsni_port)], ['ssl_certificate', snakeoil_cert], ['ssl_certificate_key', snakeoil_key], ['include', self.parser.loc["ssl_options"]]] @@ -296,7 +296,7 @@ class NginxConfigurator(common.Plugin): vhost.filep, vhost.names, ssl_block) vhost.ssl = True vhost.raw.extend(ssl_block) - vhost.addrs.add(obj.Addr('', '443', True, False)) + vhost.addrs.add(obj.Addr('', str(self.config.dvsni_port), True, False)) def get_all_certs_keys(self): """Find all existing keys, certs from configuration. diff --git a/letsencrypt_nginx/dvsni.py b/letsencrypt_nginx/dvsni.py index 53221614f..756783217 100644 --- a/letsencrypt_nginx/dvsni.py +++ b/letsencrypt_nginx/dvsni.py @@ -47,7 +47,8 @@ class NginxDvsni(common.Dvsni): self.configurator.save() addresses = [] - default_addr = "443 default_server ssl" + default_addr = "{0} default_server ssl".format( + self.configurator.config.dvsni_port) for achall in self.achalls: vhost = self.configurator.choose_vhost(achall.domain) diff --git a/letsencrypt_nginx/tests/configurator_test.py b/letsencrypt_nginx/tests/configurator_test.py index 48d71e27d..b2c1fc7f6 100644 --- a/letsencrypt_nginx/tests/configurator_test.py +++ b/letsencrypt_nginx/tests/configurator_test.py @@ -57,7 +57,7 @@ class NginxConfiguratorTest(util.NginxTest): filep = self.config.parser.abs_path('sites-enabled/example.com') self.config.parser.add_server_directives( filep, set(['.example.com', 'example.*']), - [['listen', '443 ssl']]) + [['listen', '5001 ssl']]) self.config.save() # pylint: disable=protected-access @@ -66,7 +66,7 @@ class NginxConfiguratorTest(util.NginxTest): ['listen', '127.0.0.1'], ['server_name', '.example.com'], ['server_name', 'example.*'], - ['listen', '443 ssl']]]], + ['listen', '5001 ssl']]]], parsed[0]) def test_choose_vhost(self): @@ -100,7 +100,7 @@ class NginxConfiguratorTest(util.NginxTest): nginx_conf = self.config.parser.abs_path('nginx.conf') example_conf = self.config.parser.abs_path('sites-enabled/example.com') - # Get the default 443 vhost + # Get the default SSL vhost self.config.deploy_cert( "www.example.com", "example/cert.pem", "example/key.pem") @@ -116,7 +116,7 @@ class NginxConfiguratorTest(util.NginxTest): ['listen', '127.0.0.1'], ['server_name', '.example.com'], ['server_name', 'example.*'], - ['listen', '443 ssl'], + ['listen', '5001 ssl'], ['ssl_certificate', 'example/cert.pem'], ['ssl_certificate_key', 'example/key.pem'], ['include', @@ -131,7 +131,7 @@ class NginxConfiguratorTest(util.NginxTest): [['location', '/'], [['root', 'html'], ['index', 'index.html index.htm']]], - ['listen', '443 ssl'], + ['listen', '5001 ssl'], ['ssl_certificate', '/etc/nginx/cert.pem'], ['ssl_certificate_key', '/etc/nginx/key.pem'], ['include', @@ -142,7 +142,7 @@ class NginxConfiguratorTest(util.NginxTest): nginx_conf = self.config.parser.abs_path('nginx.conf') example_conf = self.config.parser.abs_path('sites-enabled/example.com') - # Get the default 443 vhost + # Get the default SSL vhost self.config.deploy_cert( "www.example.com", "example/cert.pem", "example/key.pem") diff --git a/letsencrypt_nginx/tests/util.py b/letsencrypt_nginx/tests/util.py index 77c2ea198..414a2f315 100644 --- a/letsencrypt_nginx/tests/util.py +++ b/letsencrypt_nginx/tests/util.py @@ -53,6 +53,7 @@ def get_nginx_configurator( backup_dir=backups, temp_checkpoint_dir=os.path.join(work_dir, "temp_checkpoints"), in_progress_dir=os.path.join(backups, "IN_PROGRESS"), + dvsni_port=5001, ), name="nginx", version=version) From c459102a047792eb001332c2f6911a54c3cb2255 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sat, 27 Jun 2015 08:39:59 +0000 Subject: [PATCH 05/50] letsencrypt_nginx: call "nginx -c server_root/nginx.conf ..." --- letsencrypt_nginx/configurator.py | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/letsencrypt_nginx/configurator.py b/letsencrypt_nginx/configurator.py index 3e7de7322..ce101c04e 100644 --- a/letsencrypt_nginx/configurator.py +++ b/letsencrypt_nginx/configurator.py @@ -64,6 +64,11 @@ class NginxConfigurator(common.Plugin): "'nginx' binary, used for 'configtest' and retrieving nginx " "version number.") + @property + def nginx_conf(self): + """Nginx config file path.""" + return os.path.join(self.conf("server_root"), "nginx.conf") + def __init__(self, *args, **kwargs): """Initialize an Nginx Configurator. @@ -347,7 +352,7 @@ class NginxConfigurator(common.Plugin): :rtype: bool """ - return nginx_restart(self.conf('ctl')) + return nginx_restart(self.conf('ctl'), self.nginx_conf) def config_test(self): # pylint: disable=no-self-use """Check the configuration of Nginx for errors. @@ -358,7 +363,7 @@ class NginxConfigurator(common.Plugin): """ try: proc = subprocess.Popen( - [self.conf('ctl'), "-t"], + [self.conf('ctl'), "-c", self.nginx_conf, "-t"], stdout=subprocess.PIPE, stderr=subprocess.PIPE) stdout, stderr = proc.communicate() @@ -403,11 +408,12 @@ class NginxConfigurator(common.Plugin): """ try: proc = subprocess.Popen( - [self.conf('ctl'), "-V"], + [self.conf('ctl'), "-c", self.nginx_conf, "-V"], stdout=subprocess.PIPE, stderr=subprocess.PIPE) text = proc.communicate()[1] # nginx prints output to stderr - except (OSError, ValueError): + except (OSError, ValueError) as error: + logging.debug(error, exc_info=True) raise errors.PluginError( "Unable to run %s -V" % self.conf('ctl')) @@ -556,7 +562,7 @@ class NginxConfigurator(common.Plugin): self.restart() -def nginx_restart(nginx_ctl): +def nginx_restart(nginx_ctl, nginx_conf="/etc/nginx.conf"): """Restarts the Nginx Server. .. todo:: Nginx restart is fatal if the configuration references @@ -567,14 +573,14 @@ def nginx_restart(nginx_ctl): """ try: - proc = subprocess.Popen([nginx_ctl, "-s", "reload"], + proc = subprocess.Popen([nginx_ctl, "-c", nginx_conf, "-s", "reload"], stdout=subprocess.PIPE, stderr=subprocess.PIPE) stdout, stderr = proc.communicate() if proc.returncode != 0: # Maybe Nginx isn't running - nginx_proc = subprocess.Popen([nginx_ctl], + nginx_proc = subprocess.Popen([nginx_ctl, "-c", nginx_conf], stdout=subprocess.PIPE, stderr=subprocess.PIPE) stdout, stderr = nginx_proc.communicate() From 9652656e14647be83dba450cc91887e09788f0c1 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sat, 27 Jun 2015 08:41:19 +0000 Subject: [PATCH 06/50] Integration tests for nginx plugin (without root). --- letsencrypt_nginx/configurator.py | 9 ++- letsencrypt_nginx/dvsni.py | 6 ++ letsencrypt_nginx/tests/configurator_test.py | 6 ++ tests/boulder-integration-nginx.sh | 50 +++++++++++++++ tests/nginx.conf.sh | 67 ++++++++++++++++++++ 5 files changed, 136 insertions(+), 2 deletions(-) create mode 100755 tests/boulder-integration-nginx.sh create mode 100755 tests/nginx.conf.sh diff --git a/letsencrypt_nginx/configurator.py b/letsencrypt_nginx/configurator.py index ce101c04e..6613962c6 100644 --- a/letsencrypt_nginx/configurator.py +++ b/letsencrypt_nginx/configurator.py @@ -80,8 +80,7 @@ class NginxConfigurator(common.Plugin): super(NginxConfigurator, self).__init__(*args, **kwargs) # Verify that all directories and files exist with proper permissions - if os.geteuid() == 0: - self._verify_setup() + self._verify_setup() # Files to save self.save_notes = "" @@ -294,6 +293,12 @@ class NginxConfigurator(common.Plugin): """ snakeoil_cert, snakeoil_key = self._get_snakeoil_paths() ssl_block = [['listen', '{0} ssl'.format(self.config.dvsni_port)], + # access and error logs necessary for integration + # testing (non-root) + ['access_log', os.path.join( + self.config.work_dir, 'access.log')], + ['error_log', os.path.join( + self.config.work_dir, 'error.log')], ['ssl_certificate', snakeoil_cert], ['ssl_certificate_key', snakeoil_key], ['include', self.parser.loc["ssl_options"]]] diff --git a/letsencrypt_nginx/dvsni.py b/letsencrypt_nginx/dvsni.py index 756783217..9e79a90c2 100644 --- a/letsencrypt_nginx/dvsni.py +++ b/letsencrypt_nginx/dvsni.py @@ -134,6 +134,12 @@ class NginxDvsni(common.Dvsni): block.extend([['server_name', achall.nonce_domain], ['include', self.configurator.parser.loc["ssl_options"]], + # access and error logs necessary for + # integration testing (non-root) + ['access_log', os.path.join( + self.configurator.config.work_dir, 'access.log')], + ['error_log', os.path.join( + self.configurator.config.work_dir, 'error.log')], ['ssl_certificate', self.get_cert_file(achall)], ['ssl_certificate_key', achall.key.file], [['location', '/'], [['root', document_root]]]]) diff --git a/letsencrypt_nginx/tests/configurator_test.py b/letsencrypt_nginx/tests/configurator_test.py index b2c1fc7f6..bd700f144 100644 --- a/letsencrypt_nginx/tests/configurator_test.py +++ b/letsencrypt_nginx/tests/configurator_test.py @@ -111,12 +111,16 @@ class NginxConfiguratorTest(util.NginxTest): self.config.parser.load() + access_log = os.path.join(self.work_dir, "access.log") + error_log = os.path.join(self.work_dir, "error.log") self.assertEqual([[['server'], [['listen', '69.50.225.155:9000'], ['listen', '127.0.0.1'], ['server_name', '.example.com'], ['server_name', 'example.*'], ['listen', '5001 ssl'], + ['access_log', access_log], + ['error_log', error_log], ['ssl_certificate', 'example/cert.pem'], ['ssl_certificate_key', 'example/key.pem'], ['include', @@ -132,6 +136,8 @@ class NginxConfiguratorTest(util.NginxTest): [['root', 'html'], ['index', 'index.html index.htm']]], ['listen', '5001 ssl'], + ['access_log', access_log], + ['error_log', error_log], ['ssl_certificate', '/etc/nginx/cert.pem'], ['ssl_certificate_key', '/etc/nginx/key.pem'], ['include', diff --git a/tests/boulder-integration-nginx.sh b/tests/boulder-integration-nginx.sh new file mode 100755 index 000000000..da0dab21e --- /dev/null +++ b/tests/boulder-integration-nginx.sh @@ -0,0 +1,50 @@ +#!/bin/sh -xe + +# prerequisite: apt-get install --no-install-recommends nginx-light openssl + +if [ "xxx$root" = "xxx" ]; +then + root="$(mktemp -d)" +fi +export root + +export PATH="/usr/sbin:$PATH" # /usr/sbin/nginx + +echo "\nRoot integration tests directory: $root" +store_flags="--config-dir $root/conf --work-dir $root/work" +store_flags="$store_flags --logs-dir $root/logs" + +nginx_root="$root/nginx" +mkdir $nginx_root +root="$nginx_root" ./tests/nginx.conf.sh > $nginx_root/nginx.conf + +killall nginx || true +nginx -c $nginx_root/nginx.conf + +common() { + # first three flags required, rest is handy defaults + letsencrypt \ + --server "${SERVER:-http://localhost:4000/acme/new-reg}" \ + --no-verify-ssl \ + --dvsni-port 5001 \ + $store_flags \ + --text \ + --agree-eula \ + --email "" \ + -vvvvvvv "$@" +} + +test_nginx() { + common --configurator nginx \ + --nginx-server-root $nginx_root \ + "$@" +} + +test_nginx --domains nginx.wtf run +echo | openssl s_client -connect localhost:5001 \ + | openssl x509 -out $root/nginx.pem +diff -q $root/nginx.pem $root/conf/live/nginx.wtf/cert.pem + +# note: not reached if anything above fails, hence "killall" at the +# top +nginx -c $nginx_root/nginx.conf -s stop diff --git a/tests/nginx.conf.sh b/tests/nginx.conf.sh new file mode 100755 index 000000000..15fba922e --- /dev/null +++ b/tests/nginx.conf.sh @@ -0,0 +1,67 @@ +# Based on +# https://www.exratione.com/2014/03/running-nginx-as-a-non-root-user/ +# https://github.com/exratione/non-root-nginx/blob/9a77f62e5d5cb9c9026fd62eece76b9514011019/nginx.conf + +cat < Date: Sat, 27 Jun 2015 08:41:45 +0000 Subject: [PATCH 07/50] Travis CI: test nginx plugin. --- .travis.yml | 4 +++- tests/boulder-integration.sh | 9 +++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 6e29702ef..59a0ac8ad 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,7 +1,9 @@ language: python # http://docs.travis-ci.com/user/ci-environment/#CI-environment-OS -before_install: travis_retry sudo ./bootstrap/ubuntu.sh +before_install: + - travis_retry sudo ./bootstrap/ubuntu.sh + - apt-get install --no-install-recommends nginx-light openssl # using separate envs with different TOXENVs creates 4x1 Travis build # matrix, which allows us to clearly distinguish which component under diff --git a/tests/boulder-integration.sh b/tests/boulder-integration.sh index cbd3e9690..ba2c160e0 100755 --- a/tests/boulder-integration.sh +++ b/tests/boulder-integration.sh @@ -15,6 +15,9 @@ echo "\nRoot integration tests directory: $root" store_flags="--config-dir $root/conf --work-dir $root/work" store_flags="$store_flags --logs-dir $root/logs" +export PATH="/usr/sbin:$PATH" # /usr/sbin/nginx + + common() { # first three flags required, rest is handy defaults letsencrypt \ @@ -60,3 +63,9 @@ do live="$(readlink -f "$root/conf/live/le1.wtf/${x}.pem")" [ "${dir}/${latest}" = "$live" ] # renewer fails this test done + + +if type nginx; +then + . ./tests/boulder-integration-nginx.sh +fi From c96fc7e33af2314fb4815cced9bdda64c81fa0fe Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sat, 27 Jun 2015 09:43:33 +0000 Subject: [PATCH 08/50] travis_retry sudo apt-get ... --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 59a0ac8ad..a7b03d20a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,7 +3,7 @@ language: python # http://docs.travis-ci.com/user/ci-environment/#CI-environment-OS before_install: - travis_retry sudo ./bootstrap/ubuntu.sh - - apt-get install --no-install-recommends nginx-light openssl + - travis_retry sudo apt-get install --no-install-recommends nginx-light openssl # using separate envs with different TOXENVs creates 4x1 Travis build # matrix, which allows us to clearly distinguish which component under From 98844a196c1a92e64b717e55d40ca45941cd0a10 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sat, 27 Jun 2015 10:02:02 +0000 Subject: [PATCH 09/50] Add test for PluginError catching in disco --- letsencrypt/plugins/disco_test.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/letsencrypt/plugins/disco_test.py b/letsencrypt/plugins/disco_test.py index 1cd74385e..56808c7da 100644 --- a/letsencrypt/plugins/disco_test.py +++ b/letsencrypt/plugins/disco_test.py @@ -144,6 +144,16 @@ class PluginEntryPointTest(unittest.TestCase): self.assertFalse(self.plugin_ep.misconfigured) self.assertFalse(self.plugin_ep.available) + def test_prepare_generic_plugin_error(self): + plugin = mock.MagicMock() + plugin.prepare.side_effect = errors.PluginError + # pylint: disable=protected-access + self.plugin_ep._initialized = plugin + self.assertTrue(isinstance(self.plugin_ep.prepare(), errors.PluginError)) + self.assertTrue(self.plugin_ep.prepared) + self.assertFalse(self.plugin_ep.misconfigured) + self.assertFalse(self.plugin_ep.available) + def test_repr(self): self.assertEqual("PluginEntryPoint#sa", repr(self.plugin_ep)) From 096920b8b36e1d0cc5b33836f7c7a79808fcccf4 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sat, 27 Jun 2015 13:34:23 +0000 Subject: [PATCH 10/50] Refactor integration scripts, use --debug. --- tests/boulder-integration-nginx.sh | 50 --------------------------- tests/boulder-integration.sh | 20 +++-------- tests/integration/_common.sh | 25 ++++++++++++++ tests/{ => integration}/nginx.conf.sh | 0 tests/integration/nginx.sh | 28 +++++++++++++++ 5 files changed, 57 insertions(+), 66 deletions(-) delete mode 100755 tests/boulder-integration-nginx.sh create mode 100755 tests/integration/_common.sh rename tests/{ => integration}/nginx.conf.sh (100%) create mode 100755 tests/integration/nginx.sh diff --git a/tests/boulder-integration-nginx.sh b/tests/boulder-integration-nginx.sh deleted file mode 100755 index da0dab21e..000000000 --- a/tests/boulder-integration-nginx.sh +++ /dev/null @@ -1,50 +0,0 @@ -#!/bin/sh -xe - -# prerequisite: apt-get install --no-install-recommends nginx-light openssl - -if [ "xxx$root" = "xxx" ]; -then - root="$(mktemp -d)" -fi -export root - -export PATH="/usr/sbin:$PATH" # /usr/sbin/nginx - -echo "\nRoot integration tests directory: $root" -store_flags="--config-dir $root/conf --work-dir $root/work" -store_flags="$store_flags --logs-dir $root/logs" - -nginx_root="$root/nginx" -mkdir $nginx_root -root="$nginx_root" ./tests/nginx.conf.sh > $nginx_root/nginx.conf - -killall nginx || true -nginx -c $nginx_root/nginx.conf - -common() { - # first three flags required, rest is handy defaults - letsencrypt \ - --server "${SERVER:-http://localhost:4000/acme/new-reg}" \ - --no-verify-ssl \ - --dvsni-port 5001 \ - $store_flags \ - --text \ - --agree-eula \ - --email "" \ - -vvvvvvv "$@" -} - -test_nginx() { - common --configurator nginx \ - --nginx-server-root $nginx_root \ - "$@" -} - -test_nginx --domains nginx.wtf run -echo | openssl s_client -connect localhost:5001 \ - | openssl x509 -out $root/nginx.pem -diff -q $root/nginx.pem $root/conf/live/nginx.wtf/cert.pem - -# note: not reached if anything above fails, hence "killall" at the -# top -nginx -c $nginx_root/nginx.conf -s stop diff --git a/tests/boulder-integration.sh b/tests/boulder-integration.sh index ba2c160e0..06a1d8aa9 100755 --- a/tests/boulder-integration.sh +++ b/tests/boulder-integration.sh @@ -10,27 +10,15 @@ # # Note: this script is called by Boulder integration test suite! -root="$(mktemp -d)" -echo "\nRoot integration tests directory: $root" -store_flags="--config-dir $root/conf --work-dir $root/work" -store_flags="$store_flags --logs-dir $root/logs" - +. ./tests/integration/_common.sh export PATH="/usr/sbin:$PATH" # /usr/sbin/nginx common() { - # first three flags required, rest is handy defaults - letsencrypt \ - --server "${SERVER:-http://localhost:4000/acme/new-reg}" \ - --no-verify-ssl \ - --dvsni-port 5001 \ - $store_flags \ - --text \ - --agree-eula \ - --email "" \ + letsencrypt_test \ --authenticator standalone \ --installer null \ - -vvvvvvv "$@" + "$@" } common --domains le1.wtf auth @@ -67,5 +55,5 @@ done if type nginx; then - . ./tests/boulder-integration-nginx.sh + . ./tests/integration/nginx.sh fi diff --git a/tests/integration/_common.sh b/tests/integration/_common.sh new file mode 100755 index 000000000..0f26d3815 --- /dev/null +++ b/tests/integration/_common.sh @@ -0,0 +1,25 @@ +#!/bin/sh + +if [ "xxx$root" = "xxx" ]; +then + root="$(mktemp -d)" + echo "Root integration tests directory: $root" +fi +store_flags="--config-dir $root/conf --work-dir $root/work" +store_flags="$store_flags --logs-dir $root/logs" +export root store_flags + +letsencrypt_test () { + # first three flags required, rest is handy defaults + letsencrypt \ + --server "${SERVER:-http://localhost:4000/acme/new-reg}" \ + --no-verify-ssl \ + --dvsni-port 5001 \ + $store_flags \ + --text \ + --agree-eula \ + --email "" \ + --debug \ + -vvvvvvv \ + "$@" +} diff --git a/tests/nginx.conf.sh b/tests/integration/nginx.conf.sh similarity index 100% rename from tests/nginx.conf.sh rename to tests/integration/nginx.conf.sh diff --git a/tests/integration/nginx.sh b/tests/integration/nginx.sh new file mode 100755 index 000000000..856615ad5 --- /dev/null +++ b/tests/integration/nginx.sh @@ -0,0 +1,28 @@ +#!/bin/sh -xe +# prerequisite: apt-get install --no-install-recommends nginx-light openssl + +. ./tests/integration/_common.sh + +export PATH="/usr/sbin:$PATH" # /usr/sbin/nginx +nginx_root="$root/nginx" +mkdir $nginx_root +root="$nginx_root" ./tests/integration/nginx.conf.sh > $nginx_root/nginx.conf + +killall nginx || true +nginx -c $nginx_root/nginx.conf + +letsencrypt_test_nginx () { + letsencrypt_test \ + --configurator nginx \ + --nginx-server-root $nginx_root \ + "$@" +} + +letsencrypt_test_nginx --domains nginx.wtf run +echo | openssl s_client -connect localhost:5001 \ + | openssl x509 -out $root/nginx.pem +diff -q $root/nginx.pem $root/conf/live/nginx.wtf/cert.pem + +# note: not reached if anything above fails, hence "killall" at the +# top +nginx -c $nginx_root/nginx.conf -s stop From 4fbb5cb80f782ca30785f7171b3b95d6e95eaec6 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sat, 27 Jun 2015 16:51:58 +0000 Subject: [PATCH 11/50] Address review comments --- letsencrypt_nginx/configurator.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/letsencrypt_nginx/configurator.py b/letsencrypt_nginx/configurator.py index 6613962c6..6a492428d 100644 --- a/letsencrypt_nginx/configurator.py +++ b/letsencrypt_nginx/configurator.py @@ -281,8 +281,10 @@ class NginxConfigurator(common.Plugin): return cert, key.file def _make_server_ssl(self, vhost): - """Makes a server SSL based on server_name and filename by adding - a 'listen 443 ssl' directive to the server block. + """Make a server SSL. + + Make a server SSL based on server_name and filename by adding a + ``listen IConfig.dvsni_port ssl`` directive to the server block. .. todo:: Maybe this should create a new block instead of modifying the existing one? From cfbd33809e2ec54221ee96d943aa5ef43c959524 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sun, 28 Jun 2015 09:27:17 +0000 Subject: [PATCH 12/50] Remove acme.util --- acme/util.py | 9 --------- 1 file changed, 9 deletions(-) delete mode 100644 acme/util.py diff --git a/acme/util.py b/acme/util.py deleted file mode 100644 index 8bea9091a..000000000 --- a/acme/util.py +++ /dev/null @@ -1,9 +0,0 @@ -"""ACME utilities.""" -import json -import pkg_resources - - -def load_schema(name): - """Load JSON schema from distribution.""" - return json.load(open(pkg_resources.resource_filename( - __name__, "schemata/%s.json" % name))) From 7abff038dc8b276075382377bb50bc0283407001 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sun, 28 Jun 2015 09:31:42 +0000 Subject: [PATCH 13/50] Display tests: move test_visual to tests/display.py script. --- letsencrypt/tests/display/util_test.py | 82 ++++++++++---------------- tests/display.py | 22 +++++++ 2 files changed, 52 insertions(+), 52 deletions(-) create mode 100644 tests/display.py diff --git a/letsencrypt/tests/display/util_test.py b/letsencrypt/tests/display/util_test.py index 64ae3b713..41075c9ce 100644 --- a/letsencrypt/tests/display/util_test.py +++ b/letsencrypt/tests/display/util_test.py @@ -7,35 +7,20 @@ import mock from letsencrypt.display import util as display_util -class DisplayT(unittest.TestCase): - """Base class for both utility classes.""" - # pylint: disable=too-few-public-methods - def setUp(self): - self.choices = [("First", "Description1"), ("Second", "Description2")] - self.tags = ["tag1", "tag2", "tag3"] - self.tags_choices = [("1", "tag1"), ("2", "tag2"), ("3", "tag3")] +CHOICES = [("First", "Description1"), ("Second", "Description2")] +TAGS = ["tag1", "tag2", "tag3"] +TAGS_CHOICES = [("1", "tag1"), ("2", "tag2"), ("3", "tag3")] -def visual(displayer, choices): - """Visually test all of the display functions.""" - displayer.notification("Random notification!") - displayer.menu("Question?", choices, - ok_label="O", cancel_label="Can", help_label="??") - displayer.menu("Question?", [choice[1] for choice in choices], - ok_label="O", cancel_label="Can", help_label="??") - displayer.input("Input Message") - displayer.yesno("YesNo Message", yes_label="Yessir", no_label="Nosir") - displayer.checklist("Checklist Message", [choice[0] for choice in choices]) - - -class NcursesDisplayTest(DisplayT): +class NcursesDisplayTest(unittest.TestCase): """Test ncurses display. - Since this is mostly a wrapper, it might be more helpful to test the actual - dialog boxes. The test_visual function will actually display the various - boxes but requires the user to do the verification. If something seems amiss - please use the test_visual function to debug it, the automatic tests rely - on too much mocking. + Since this is mostly a wrapper, it might be more helpful to test the + actual dialog boxes. The test file located in ./tests/display.py + (relative to the root of the repository) will actually display the + various boxes but requires the user to do the verification. If + something seems amiss please use that test script to debug it, the + automatic tests rely on too much mocking. """ def setUp(self): @@ -43,7 +28,7 @@ class NcursesDisplayTest(DisplayT): self.displayer = display_util.NcursesDisplay() self.default_menu_options = { - "choices": self.choices, + "choices": CHOICES, "ok_label": "OK", "cancel_label": "Cancel", "help_button": False, @@ -63,7 +48,7 @@ class NcursesDisplayTest(DisplayT): def test_menu_tag_and_desc(self, mock_menu): mock_menu.return_value = (display_util.OK, "First") - ret = self.displayer.menu("Message", self.choices) + ret = self.displayer.menu("Message", CHOICES) mock_menu.assert_called_with("Message", **self.default_menu_options) self.assertEqual(ret, (display_util.OK, 0)) @@ -72,7 +57,7 @@ class NcursesDisplayTest(DisplayT): def test_menu_tag_and_desc_cancel(self, mock_menu): mock_menu.return_value = (display_util.CANCEL, "") - ret = self.displayer.menu("Message", self.choices) + ret = self.displayer.menu("Message", CHOICES) mock_menu.assert_called_with("Message", **self.default_menu_options) @@ -82,10 +67,10 @@ class NcursesDisplayTest(DisplayT): def test_menu_desc_only(self, mock_menu): mock_menu.return_value = (display_util.OK, "1") - ret = self.displayer.menu("Message", self.tags, help_label="More Info") + ret = self.displayer.menu("Message", TAGS, help_label="More Info") self.default_menu_options.update( - choices=self.tags_choices, help_button=True, help_label="More Info") + choices=TAGS_CHOICES, help_button=True, help_label="More Info") mock_menu.assert_called_with("Message", **self.default_menu_options) self.assertEqual(ret, (display_util.OK, 0)) @@ -94,7 +79,7 @@ class NcursesDisplayTest(DisplayT): def test_menu_desc_only_help(self, mock_menu): mock_menu.return_value = (display_util.HELP, "2") - ret = self.displayer.menu("Message", self.tags, help_label="More Info") + ret = self.displayer.menu("Message", TAGS, help_label="More Info") self.assertEqual(ret, (display_util.HELP, 1)) @@ -102,7 +87,7 @@ class NcursesDisplayTest(DisplayT): def test_menu_desc_only_cancel(self, mock_menu): mock_menu.return_value = (display_util.CANCEL, "") - ret = self.displayer.menu("Message", self.tags, help_label="More Info") + ret = self.displayer.menu("Message", TAGS, help_label="More Info") self.assertEqual(ret, (display_util.CANCEL, -1)) @@ -125,22 +110,19 @@ class NcursesDisplayTest(DisplayT): @mock.patch("letsencrypt.display.util." "dialog.Dialog.checklist") def test_checklist(self, mock_checklist): - self.displayer.checklist("message", self.tags) + self.displayer.checklist("message", TAGS) choices = [ - (self.tags[0], "", True), - (self.tags[1], "", True), - (self.tags[2], "", True), + (TAGS[0], "", True), + (TAGS[1], "", True), + (TAGS[2], "", True), ] mock_checklist.assert_called_with( "message", width=display_util.WIDTH, height=display_util.HEIGHT, choices=choices) - # def test_visual(self): - # visual(self.displayer, self.choices) - -class FileOutputDisplayTest(DisplayT): +class FileOutputDisplayTest(unittest.TestCase): """Test stdout display. Most of this class has to deal with visual output. In order to test how the @@ -168,7 +150,7 @@ class FileOutputDisplayTest(DisplayT): "FileDisplay._get_valid_int_ans") def test_menu(self, mock_ans): mock_ans.return_value = (display_util.OK, 1) - ret = self.displayer.menu("message", self.choices) + ret = self.displayer.menu("message", CHOICES) self.assertEqual(ret, (display_util.OK, 0)) def test_input_cancel(self): @@ -202,7 +184,7 @@ class FileOutputDisplayTest(DisplayT): @mock.patch("letsencrypt.display.util.FileDisplay.input") def test_checklist_valid(self, mock_input): mock_input.return_value = (display_util.OK, "2 1") - code, tag_list = self.displayer.checklist("msg", self.tags) + code, tag_list = self.displayer.checklist("msg", TAGS) self.assertEqual( (code, set(tag_list)), (display_util.OK, set(["tag1", "tag2"]))) @@ -214,7 +196,7 @@ class FileOutputDisplayTest(DisplayT): (display_util.OK, "1") ] - ret = self.displayer.checklist("msg", self.tags) + ret = self.displayer.checklist("msg", TAGS) self.assertEqual(ret, (display_util.OK, ["tag1"])) @mock.patch("letsencrypt.display.util.FileDisplay.input") @@ -223,7 +205,7 @@ class FileOutputDisplayTest(DisplayT): (display_util.OK, "10"), (display_util.CANCEL, "1") ] - ret = self.displayer.checklist("msg", self.tags) + ret = self.displayer.checklist("msg", TAGS) self.assertEqual(ret, (display_util.CANCEL, [])) def test_scrub_checklist_input_valid(self): @@ -240,7 +222,7 @@ class FileOutputDisplayTest(DisplayT): ] for i, list_ in enumerate(indices): set_tags = set( - self.displayer._scrub_checklist_input(list_, self.tags)) + self.displayer._scrub_checklist_input(list_, TAGS)) self.assertEqual(set_tags, exp[i]) def test_scrub_checklist_input_invalid(self): @@ -254,13 +236,13 @@ class FileOutputDisplayTest(DisplayT): ] for list_ in indices: self.assertEqual( - self.displayer._scrub_checklist_input(list_, self.tags), []) + self.displayer._scrub_checklist_input(list_, TAGS), []) def test_print_menu(self): # pylint: disable=protected-access # This is purely cosmetic... just make sure there aren't any exceptions - self.displayer._print_menu("msg", self.choices) - self.displayer._print_menu("msg", self.tags) + self.displayer._print_menu("msg", CHOICES) + self.displayer._print_menu("msg", TAGS) def test_wrap_lines(self): # pylint: disable=protected-access @@ -296,10 +278,6 @@ class FileOutputDisplayTest(DisplayT): self.displayer._get_valid_int_ans(3), (display_util.CANCEL, -1)) - # def test_visual(self): - # self.displayer = display_util.FileDisplay(sys.stdout) - # visual(self.displayer, self.choices) - class SeparateListInputTest(unittest.TestCase): """Test Module functions.""" diff --git a/tests/display.py b/tests/display.py new file mode 100644 index 000000000..dff56e42e --- /dev/null +++ b/tests/display.py @@ -0,0 +1,22 @@ +"""Manual test of display functions.""" +import sys + +from letsencrypt.display import util +from letsencrypt.tests.display import util_test + + +def test_visual(displayer, choices): + """Visually test all of the display functions.""" + displayer.notification("Random notification!") + displayer.menu("Question?", choices, + ok_label="O", cancel_label="Can", help_label="??") + displayer.menu("Question?", [choice[1] for choice in choices], + ok_label="O", cancel_label="Can", help_label="??") + displayer.input("Input Message") + displayer.yesno("YesNo Message", yes_label="Yessir", no_label="Nosir") + displayer.checklist("Checklist Message", [choice[0] for choice in choices]) + + +if __name__ == "__main__": + for displayer in util.NcursesDisplay(), util.FileDisplay(sys.stdout): + test_visual(displayer, util_test.CHOICES) From 46707406b5c788a650337040e5792cc5f938df91 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sun, 28 Jun 2015 09:38:03 +0000 Subject: [PATCH 14/50] Tests: don't cover plugins.common test functions. --- letsencrypt/plugins/common.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/letsencrypt/plugins/common.py b/letsencrypt/plugins/common.py index 03ddf7df7..1c5ccbefd 100644 --- a/letsencrypt/plugins/common.py +++ b/letsencrypt/plugins/common.py @@ -173,16 +173,18 @@ class Dvsni(object): return response -# test utils +# test utils used by letsencrypt_apache/letsencrypt_nginx (hence +# "pragma: no cover") TODO: this might quickly lead to dead code (also +# c.f. #383) -def setup_ssl_options(config_dir, src, dest): +def setup_ssl_options(config_dir, src, dest): # pragma: no cover """Move the ssl_options into position and return the path.""" option_path = os.path.join(config_dir, dest) shutil.copyfile(src, option_path) return option_path -def dir_setup(test_dir, pkg): +def dir_setup(test_dir, pkg): # pragma: no cover """Setup the directories necessary for the configurator.""" temp_dir = tempfile.mkdtemp("temp") config_dir = tempfile.mkdtemp("config") From 051a351a437f3f6b89baa08aa80277fcce5f91be Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sun, 28 Jun 2015 09:39:21 +0000 Subject: [PATCH 15/50] Move test_add_chal from letsencrypt_nginx (plugins.common 100% coverage). --- letsencrypt/plugins/common_test.py | 5 +++++ letsencrypt_nginx/tests/dvsni_test.py | 5 ----- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/letsencrypt/plugins/common_test.py b/letsencrypt/plugins/common_test.py index 6de86f2b8..c81d3939f 100644 --- a/letsencrypt/plugins/common_test.py +++ b/letsencrypt/plugins/common_test.py @@ -140,6 +140,11 @@ class DvsniTest(unittest.TestCase): from letsencrypt.plugins.common import Dvsni self.sni = Dvsni(configurator=mock.MagicMock()) + def test_add_chall(self): + self.sni.add_chall(self.achalls[0], 0) + self.assertEqual(1, len(self.sni.achalls)) + self.assertEqual([0], self.sni.indices) + def test_setup_challenge_cert(self): # This is a helper function that can be used for handling # open context managers more elegantly. It avoids dealing with diff --git a/letsencrypt_nginx/tests/dvsni_test.py b/letsencrypt_nginx/tests/dvsni_test.py index 5b46e8da6..4680c5c1e 100644 --- a/letsencrypt_nginx/tests/dvsni_test.py +++ b/letsencrypt_nginx/tests/dvsni_test.py @@ -61,11 +61,6 @@ class DvsniPerformTest(util.NginxTest): shutil.rmtree(self.config_dir) shutil.rmtree(self.work_dir) - def test_add_chall(self): - self.sni.add_chall(self.achalls[0], 0) - self.assertEqual(1, len(self.sni.achalls)) - self.assertEqual([0], self.sni.indices) - @mock.patch("letsencrypt_nginx.configurator" ".NginxConfigurator.choose_vhost") def test_perform(self, mock_choose): From bfba30701ea3cc93be2f4454bf96688da5c1d564 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sun, 28 Jun 2015 09:41:33 +0000 Subject: [PATCH 16/50] Bump core coverage --- tox.cover.sh | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tox.cover.sh b/tox.cover.sh index 172d2e05b..554f9bfc5 100755 --- a/tox.cover.sh +++ b/tox.cover.sh @@ -20,5 +20,7 @@ rm -f .coverage # --cover-erase is off, make sure stats are correct # don't use sequential composition (;), if letsencrypt_nginx returns # 0, coveralls submit will be triggered (c.f. .travis.yml, # after_success) -cover letsencrypt 95 && cover acme 100 && \ - cover letsencrypt_apache 76 && cover letsencrypt_nginx 96 +cover letsencrypt 97 && \ + cover acme 100 && \ + cover letsencrypt_apache 76 && \ + cover letsencrypt_nginx 96 From 60478e213bb03781135735b8f6191f40339b35cd Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sun, 28 Jun 2015 09:42:43 +0000 Subject: [PATCH 17/50] Bump apache coverage. --- tox.cover.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.cover.sh b/tox.cover.sh index 554f9bfc5..f1d882cee 100755 --- a/tox.cover.sh +++ b/tox.cover.sh @@ -22,5 +22,5 @@ rm -f .coverage # --cover-erase is off, make sure stats are correct # after_success) cover letsencrypt 97 && \ cover acme 100 && \ - cover letsencrypt_apache 76 && \ + cover letsencrypt_apache 78 && \ cover letsencrypt_nginx 96 From a0acf7c7030a7687f6b0bad1640f1f730d221071 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sun, 21 Jun 2015 08:18:51 +0000 Subject: [PATCH 18/50] 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 19/50] 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 20/50] 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 21/50] 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 22/50] 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 23/50] 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 a248980952e02cb23c5411d51cbb448175c8ff70 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 29 Jun 2015 11:53:03 -0700 Subject: [PATCH 24/50] Fixed traceback when not run as root --- letsencrypt/cli.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index e719706dd..9bb4438a5 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -652,6 +652,16 @@ def main2(cli_args, args, config, plugins): displayer = display_util.NcursesDisplay() zope.component.provideUtility(displayer) + # Setup logging ASAP, otherwise "No handlers could be found for + # logger ..." TODO: this should be done before plugins discovery + for directory in config.config_dir, config.work_dir: + le_util.make_or_verify_dir( + directory, constants.CONFIG_DIRS_MODE, os.geteuid()) + # TODO: logs might contain sensitive data such as contents of the + # private key! #525 + le_util.make_or_verify_dir(args.logs_dir, 0o700, os.geteuid()) + _setup_logging(args) + # do not log `args`, as it contains sensitive data (e.g. revoke --key)! logger.debug("Arguments: %r", cli_args) logger.debug("Discovered plugins: %r", plugins) @@ -682,16 +692,6 @@ def main(cli_args=sys.argv[1:]): args = create_parser(plugins, cli_args).parse_args(cli_args) config = configuration.NamespaceConfig(args) - # Setup logging ASAP, otherwise "No handlers could be found for - # logger ..." TODO: this should be done before plugins discovery - for directory in config.config_dir, config.work_dir: - le_util.make_or_verify_dir( - directory, constants.CONFIG_DIRS_MODE, os.geteuid()) - # TODO: logs might contain sensitive data such as contents of the - # private key! #525 - le_util.make_or_verify_dir(args.logs_dir, 0o700, os.geteuid()) - _setup_logging(args) - def handle_exception_common(): """Logs the exception and reraises it if in debug mode.""" logger.debug("Exiting abnormally", exc_info=True) From 85b5bc0cb2cff93ee129f64e2a3a9ed1f279d16d Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 29 Jun 2015 17:31:48 -0700 Subject: [PATCH 25/50] Reimplemented exception handling --- letsencrypt/cli.py | 63 ++++++++++++++++++----------------- letsencrypt/tests/cli_test.py | 22 ++++++++++-- 2 files changed, 52 insertions(+), 33 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 9bb4438a5..8f833522d 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -2,11 +2,13 @@ # TODO: Sanity check all input. Be sure to avoid shell code etc... import argparse import atexit +import functools import logging import logging.handlers import os import sys import time +import traceback import configargparse import zope.component @@ -642,15 +644,29 @@ def _setup_logging(args): logger.info("Saving debug log to %s", log_file_name) -def main2(cli_args, args, config, plugins): - """Continued main script execution.""" +def _handle_exception(exc_type, exc_value, trace, args): + logger.debug( + "Exiting abnormally:\n%s", + "".join(traceback.format_exception(exc_type, exc_value, trace))) - # Displayer - if args.text_mode: - displayer = display_util.FileDisplay(sys.stdout) + if issubclass(exc_type, errors.Error) and (not args or not args.debug): + sys.exit(exc_value) + elif issubclass(exc_type, Exception) and args and not args.debug: + sys.exit( + "An unexpected error occurred. Please see the logfiles in {0} for " + "more details.".format(args.logs_dir)) else: - displayer = display_util.NcursesDisplay() - zope.component.provideUtility(displayer) + traceback.print_exception(exc_type, exc_value, trace, file=sys.stderr) + + +def main(cli_args=sys.argv[1:]): + """Command line argument parsing and main script execution.""" + sys.excepthook = functools.partial(_handle_exception, args=None) + + # note: arg parser internally handles --help (and exits afterwards) + plugins = plugins_disco.PluginsRegistry.find_all() + args = create_parser(plugins, cli_args).parse_args(cli_args) + config = configuration.NamespaceConfig(args) # Setup logging ASAP, otherwise "No handlers could be found for # logger ..." TODO: this should be done before plugins discovery @@ -666,6 +682,15 @@ def main2(cli_args, args, config, plugins): logger.debug("Arguments: %r", cli_args) logger.debug("Discovered plugins: %r", plugins) + sys.excepthook = functools.partial(_handle_exception, args=args) + + # Displayer + if args.text_mode: + displayer = display_util.FileDisplay(sys.stdout) + else: + displayer = display_util.NcursesDisplay() + zope.component.provideUtility(displayer) + # Reporter report = reporter.Reporter() zope.component.provideUtility(report) @@ -685,29 +710,5 @@ def main2(cli_args, args, config, plugins): return args.func(args, config, plugins) -def main(cli_args=sys.argv[1:]): - """Command line argument parsing and main script execution.""" - # note: arg parser internally handles --help (and exits afterwards) - plugins = plugins_disco.PluginsRegistry.find_all() - args = create_parser(plugins, cli_args).parse_args(cli_args) - config = configuration.NamespaceConfig(args) - - def handle_exception_common(): - """Logs the exception and reraises it if in debug mode.""" - logger.debug("Exiting abnormally", exc_info=True) - if args.debug: - raise - - try: - return main2(cli_args, args, config, plugins) - except errors.Error as error: - handle_exception_common() - return error - except Exception: # pylint: disable=broad-except - handle_exception_common() - return ("An unexpected error occured. Please see the logfiles in {0} " - "for more details.".format(args.logs_dir)) - - if __name__ == "__main__": sys.exit(main()) # pragma: no cover diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 798ec227a..8da009864 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -66,12 +66,30 @@ class CLITest(unittest.TestCase): attrs = {'view_config_changes.side_effect' : error} self.assertRaises( errors.Error, self._call, ['--debug'] + cmd_arg, attrs) - self._call(cmd_arg, attrs) attrs['view_config_changes.side_effect'] = [ValueError] self.assertRaises( ValueError, self._call, ['--debug'] + cmd_arg, attrs) - self._call(cmd_arg, attrs) + + @mock.patch("letsencrypt.cli.sys") + def test_handle_exception(self, mock_sys): + # pylint: disable=protected-access + import StringIO + from letsencrypt import cli + from letsencrypt import errors + + cli._handle_exception(errors.Error, "detail", None, None) + mock_sys.exit.assert_called_once_with("detail") + + args = mock.MagicMock(debug=False) + cli._handle_exception(ValueError, "detail", None, args) + self.assertTrue("logfile" in mock_sys.exit.call_args_list[1][0][0]) + + mock_sys.stderr = StringIO.StringIO() + exc_value = "A very specific string" + cli._handle_exception(KeyboardInterrupt, exc_value, None, None) + self.assertTrue(exc_value in mock_sys.stderr.getvalue()) + if __name__ == '__main__': unittest.main() # pragma: no cover From a7a863e1f263d22d53713caa4ddb8fc8334cfb31 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Tue, 30 Jun 2015 14:52:05 +0000 Subject: [PATCH 26/50] Do not include /etc/nginx/mime.types in nginx integration testing. This file (or /etc/nginx in whole) might not exist on the target system. --- tests/integration/nginx.conf.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/nginx.conf.sh b/tests/integration/nginx.conf.sh index 15fba922e..7a05f0519 100755 --- a/tests/integration/nginx.conf.sh +++ b/tests/integration/nginx.conf.sh @@ -37,7 +37,7 @@ http { keepalive_timeout 65; types_hash_max_size 2048; - include /etc/nginx/mime.types; + #include /etc/nginx/mime.types; index index.html index.htm index.php; log_format main '\$remote_addr - \$remote_user [\$time_local] \$status ' From 13913fd8e0650c289ae7369a6afe0f0c10ee4228 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 30 Jun 2015 12:57:51 -0700 Subject: [PATCH 27/50] Added traceback dump --- .gitignore | 1 + letsencrypt/cli.py | 39 +++++++++++++++++++----- letsencrypt/tests/cli_test.py | 57 +++++++++++++++++++---------------- 3 files changed, 64 insertions(+), 33 deletions(-) diff --git a/.gitignore b/.gitignore index 9564cfbc7..54670e6da 100644 --- a/.gitignore +++ b/.gitignore @@ -5,6 +5,7 @@ build/ dist/ /venv/ /.tox/ +letsencrypt.log # coverage .coverage diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 8f833522d..f2d2c4ef0 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -645,18 +645,43 @@ def _setup_logging(args): def _handle_exception(exc_type, exc_value, trace, args): + """Logs exceptions and reports them to the user. + + Args is used to determine how to display exceptions to the user. In + general, if args.debug is True, then the full exception and traceback is + shown to the user, otherwise it is suppressed. If args itself is None, + then the traceback and exception is attempted to be written to a logfile. + If this is successful, the traceback is suppressed, otherwise it is shown + to the user. sys.exit is always called with a nonzero status. + + """ logger.debug( "Exiting abnormally:\n%s", "".join(traceback.format_exception(exc_type, exc_value, trace))) - if issubclass(exc_type, errors.Error) and (not args or not args.debug): - sys.exit(exc_value) - elif issubclass(exc_type, Exception) and args and not args.debug: - sys.exit( - "An unexpected error occurred. Please see the logfiles in {0} for " - "more details.".format(args.logs_dir)) + if issubclass(exc_type, Exception) and (args is None or not args.debug): + if args is None: + try: + with open("letsencrypt.log", "w") as logfile: + traceback.print_exception( + exc_type, exc_value, trace, file=logfile) + except: # pylint: disable=bare-except + sys.exit("".join( + traceback.format_exception(exc_type, exc_value, trace))) + + if issubclass(exc_type, errors.Error): + sys.exit(exc_value) + elif args is None: + sys.exit( + "An unexpected error occurred. Please see the logfile '{0}' " + "for more details.".format(os.path.abspath("letsencrypt.log"))) + else: + sys.exit( + "An unexpected error occurred. Please see the logfiles in {0} " + "for more details.".format(args.logs_dir)) else: - traceback.print_exception(exc_type, exc_value, trace, file=sys.stderr) + sys.exit("".join( + traceback.format_exception(exc_type, exc_value, trace))) def main(cli_args=sys.argv[1:]): diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 8da009864..cdc5826df 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -2,11 +2,14 @@ import itertools import os import shutil +import traceback import tempfile import unittest import mock +from letsencrypt import errors + class CLITest(unittest.TestCase): """Tests for different commands.""" @@ -20,16 +23,13 @@ class CLITest(unittest.TestCase): def tearDown(self): shutil.rmtree(self.tmp_dir) - def _call(self, args, client_mock_attrs=None): + def _call(self, args): from letsencrypt import cli args = ['--text', '--config-dir', self.config_dir, '--work-dir', self.work_dir, '--logs-dir', self.logs_dir] + args with mock.patch('letsencrypt.cli.sys.stdout') as stdout: with mock.patch('letsencrypt.cli.sys.stderr') as stderr: with mock.patch('letsencrypt.cli.client') as client: - if client_mock_attrs: - # pylint: disable=star-args - client.configure_mock(**client_mock_attrs) ret = cli.main(args) return ret, stdout, stderr, client @@ -59,36 +59,41 @@ class CLITest(unittest.TestCase): for r in xrange(len(flags)))): self._call(['plugins',] + list(args)) - def test_exceptions(self): - from letsencrypt import errors - cmd_arg = ['config_changes'] - error = [errors.Error('problem')] - attrs = {'view_config_changes.side_effect' : error} - self.assertRaises( - errors.Error, self._call, ['--debug'] + cmd_arg, attrs) - - attrs['view_config_changes.side_effect'] = [ValueError] - self.assertRaises( - ValueError, self._call, ['--debug'] + cmd_arg, attrs) - @mock.patch("letsencrypt.cli.sys") def test_handle_exception(self, mock_sys): # pylint: disable=protected-access - import StringIO from letsencrypt import cli - from letsencrypt import errors - cli._handle_exception(errors.Error, "detail", None, None) - mock_sys.exit.assert_called_once_with("detail") + mock_open = mock.mock_open() + with mock.patch("letsencrypt.cli.open", mock_open, create=True): + exception = Exception("detail") + cli._handle_exception( + Exception, exc_value=exception, trace=None, args=None) + mock_open().write.assert_called_once_with("".join( + traceback.format_exception_only(Exception, exception))) + error_msg = mock_sys.exit.call_args_list[0][0][0] + self.assertTrue("unexpected error" in error_msg) + + with mock.patch("letsencrypt.cli.open", mock_open, create=True): + mock_open.side_effect = [KeyboardInterrupt] + error = errors.Error("detail") + cli._handle_exception( + errors.Error, exc_value=error, trace=None, args=None) + # assert_any_call used because sys.exit doesn't exit in cli.py + mock_sys.exit.assert_any_call("".join( + traceback.format_exception_only(errors.Error, error))) args = mock.MagicMock(debug=False) - cli._handle_exception(ValueError, "detail", None, args) - self.assertTrue("logfile" in mock_sys.exit.call_args_list[1][0][0]) + cli._handle_exception( + Exception, exc_value=Exception("detail"), trace=None, args=args) + error_msg = mock_sys.exit.call_args_list[-1][0][0] + self.assertTrue("unexpected error" in error_msg) - mock_sys.stderr = StringIO.StringIO() - exc_value = "A very specific string" - cli._handle_exception(KeyboardInterrupt, exc_value, None, None) - self.assertTrue(exc_value in mock_sys.stderr.getvalue()) + interrupt = KeyboardInterrupt("detail") + cli._handle_exception( + KeyboardInterrupt, exc_value=interrupt, trace=None, args=None) + mock_sys.exit.assert_called_with("".join( + traceback.format_exception_only(KeyboardInterrupt, interrupt))) if __name__ == '__main__': From e682ae2503d22b7e1be3bd9da74952cae7958141 Mon Sep 17 00:00:00 2001 From: PatrickHeppler Date: Wed, 1 Jul 2015 16:41:18 +0200 Subject: [PATCH 28/50] Update README.rst Added example for SAN certificates --- README.rst | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/README.rst b/README.rst index db32889db..feeca1cab 100644 --- a/README.rst +++ b/README.rst @@ -17,10 +17,13 @@ It's all automated: * If domain control has been proven, a certificate will get issued and the tool will automatically install it. -All you need to do is:: +All you need to do to sign a single domain is:: user@www:~$ sudo letsencrypt -d www.example.org auth +For multiple domains (SAN) use:: + user@www:~$ sudo letsencrypt -d www.example.org -d example.org auth + and if you have a compatible web server (Apache or Nginx), Let's Encrypt can not only get a new certificate, but also deploy it and configure your server automatically!:: From 8b3a766dc1d54e76236c2706f3777a358e78eecd Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 1 Jul 2015 14:49:32 -0700 Subject: [PATCH 29/50] Made logfile location more clear --- letsencrypt/cli.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index f2d2c4ef0..741d1a30c 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -661,10 +661,11 @@ def _handle_exception(exc_type, exc_value, trace, args): if issubclass(exc_type, Exception) and (args is None or not args.debug): if args is None: + logfile = "letsencrypt.log" try: - with open("letsencrypt.log", "w") as logfile: + with open(logfile, "w") as logfd: traceback.print_exception( - exc_type, exc_value, trace, file=logfile) + exc_type, exc_value, trace, file=logfd) except: # pylint: disable=bare-except sys.exit("".join( traceback.format_exception(exc_type, exc_value, trace))) @@ -674,7 +675,7 @@ def _handle_exception(exc_type, exc_value, trace, args): elif args is None: sys.exit( "An unexpected error occurred. Please see the logfile '{0}' " - "for more details.".format(os.path.abspath("letsencrypt.log"))) + "for more details.".format(logfile)) else: sys.exit( "An unexpected error occurred. Please see the logfiles in {0} " From dc9ffdbb7fd1cf98cd9180f4cf470d7fbbb40e45 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Thu, 2 Jul 2015 04:51:41 +0000 Subject: [PATCH 30/50] 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, From 5d575e78b21a4a95ec85c9c790a5622d9155c2f3 Mon Sep 17 00:00:00 2001 From: PatrickHeppler Date: Thu, 2 Jul 2015 10:37:35 +0200 Subject: [PATCH 31/50] Update README.rst Fixed missing newline --- README.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/README.rst b/README.rst index feeca1cab..d60af4cbc 100644 --- a/README.rst +++ b/README.rst @@ -22,6 +22,7 @@ All you need to do to sign a single domain is:: user@www:~$ sudo letsencrypt -d www.example.org auth For multiple domains (SAN) use:: + user@www:~$ sudo letsencrypt -d www.example.org -d example.org auth and if you have a compatible web server (Apache or Nginx), Let's Encrypt can From 2b32b94c0bb4f2133666a87c6e909fd3e84f38a2 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Tue, 30 Jun 2015 10:50:14 +0000 Subject: [PATCH 32/50] acme.client.ClientNetwork --- acme/client.py | 308 +++++++++++----------- acme/client_test.py | 624 ++++++++++++++++++++++---------------------- 2 files changed, 470 insertions(+), 462 deletions(-) diff --git a/acme/client.py b/acme/client.py index d1bf84a21..aac539974 100644 --- a/acme/client.py +++ b/acme/client.py @@ -32,150 +32,18 @@ class Client(object): # pylint: disable=too-many-instance-attributes :ivar key: `.JWK` (private) :ivar alg: `.JWASignature` :ivar bool verify_ssl: Verify SSL certificates? + :ivar .ClientNetwork net: Client network. Useful for testing. If not + supplied, it will be initialized using `key`, `alg` and + `verify_ssl`. """ DER_CONTENT_TYPE = 'application/pkix-cert' - JSON_CONTENT_TYPE = 'application/json' - JSON_ERROR_CONTENT_TYPE = 'application/problem+json' - REPLAY_NONCE_HEADER = 'Replay-Nonce' - def __init__(self, new_reg_uri, key, alg=jose.RS256, verify_ssl=True): + def __init__(self, new_reg_uri, key, alg=jose.RS256, + verify_ssl=True, net=None): self.new_reg_uri = new_reg_uri self.key = key - self.alg = alg - self.verify_ssl = verify_ssl - self._nonces = set() - - def _wrap_in_jws(self, obj, nonce): - """Wrap `JSONDeSerializable` object in JWS. - - .. todo:: Implement ``acmePath``. - - :param JSONDeSerializable obj: - :rtype: `.JWS` - - """ - dumps = obj.json_dumps() - logger.debug('Serialized JSON: %s', dumps) - return jws.JWS.sign( - payload=dumps, key=self.key, alg=self.alg, nonce=nonce).json_dumps() - - @classmethod - def _check_response(cls, response, content_type=None): - """Check response content and its type. - - .. note:: - Checking is not strict: wrong server response ``Content-Type`` - HTTP header is ignored if response is an expected JSON object - (c.f. Boulder #56). - - :param str content_type: Expected Content-Type response header. - If JSON is expected and not present in server response, this - function will raise an error. Otherwise, wrong Content-Type - is ignored, but logged. - - :raises .messages.Error: If server response body - carries HTTP Problem (draft-ietf-appsawg-http-problem-00). - :raises .ClientError: In case of other networking errors. - - """ - logger.debug('Received response %s (headers: %s): %r', - response, response.headers, response.content) - - response_ct = response.headers.get('Content-Type') - try: - # TODO: response.json() is called twice, once here, and - # once in _get and _post clients - jobj = response.json() - except ValueError as error: - jobj = None - - if not response.ok: - if jobj is not None: - if response_ct != cls.JSON_ERROR_CONTENT_TYPE: - logger.debug( - 'Ignoring wrong Content-Type (%r) for JSON Error', - response_ct) - try: - raise messages.Error.from_json(jobj) - except jose.DeserializationError as error: - # Couldn't deserialize JSON object - raise errors.ClientError((response, error)) - else: - # response is not JSON object - raise errors.ClientError(response) - else: - if jobj is not None and response_ct != cls.JSON_CONTENT_TYPE: - logger.debug( - 'Ignoring wrong Content-Type (%r) for JSON decodable ' - 'response', response_ct) - - if content_type == cls.JSON_CONTENT_TYPE and jobj is None: - raise errors.ClientError( - 'Unexpected response Content-Type: {0}'.format(response_ct)) - - def _get(self, uri, content_type=JSON_CONTENT_TYPE, **kwargs): - """Send GET request. - - :raises .ClientError: - - :returns: HTTP Response - :rtype: `requests.Response` - - """ - logger.debug('Sending GET request to %s', uri) - kwargs.setdefault('verify', self.verify_ssl) - try: - response = requests.get(uri, **kwargs) - except requests.exceptions.RequestException as error: - raise errors.ClientError(error) - self._check_response(response, content_type=content_type) - return response - - def _add_nonce(self, response): - if self.REPLAY_NONCE_HEADER in response.headers: - nonce = response.headers[self.REPLAY_NONCE_HEADER] - error = jws.Header.validate_nonce(nonce) - if error is None: - logger.debug('Storing nonce: %r', nonce) - self._nonces.add(nonce) - else: - raise errors.ClientError('Invalid nonce ({0}): {1}'.format( - nonce, error)) - else: - raise errors.ClientError( - 'Server {0} response did not include a replay nonce'.format( - response.request.method)) - - def _get_nonce(self, uri): - if not self._nonces: - logger.debug('Requesting fresh nonce by sending HEAD to %s', uri) - self._add_nonce(requests.head(uri)) - return self._nonces.pop() - - def _post(self, uri, obj, content_type=JSON_CONTENT_TYPE, **kwargs): - """Send POST data. - - :param JSONDeSerializable obj: Will be wrapped in JWS. - :param str content_type: Expected ``Content-Type``, fails if not set. - - :raises acme.messages.ClientError: - - :returns: HTTP Response - :rtype: `requests.Response` - - """ - data = self._wrap_in_jws(obj, self._get_nonce(uri)) - logger.debug('Sending POST data to %s: %s', uri, data) - kwargs.setdefault('verify', self.verify_ssl) - try: - response = requests.post(uri, data=data, **kwargs) - except requests.exceptions.RequestException as error: - raise errors.ClientError(error) - - self._add_nonce(response) - self._check_response(response, content_type=content_type) - return response + self.net = ClientNetwork(key, alg, verify_ssl) if net is None else net @classmethod def _regr_from_response(cls, response, uri=None, new_authzr_uri=None, @@ -211,7 +79,7 @@ class Client(object): # pylint: disable=too-many-instance-attributes """ new_reg = messages.Registration(contact=contact) - response = self._post(self.new_reg_uri, new_reg) + response = self.net.post(self.new_reg_uri, new_reg) assert response.status_code == httplib.CREATED # TODO: handle errors regr = self._regr_from_response(response) @@ -230,7 +98,7 @@ class Client(object): # pylint: disable=too-many-instance-attributes :rtype: `.RegistrationResource` """ - response = self._post(regr.uri, regr.body) + response = self.net.post(regr.uri, regr.body) # TODO: Boulder returns httplib.ACCEPTED #assert response.status_code == httplib.OK @@ -290,7 +158,7 @@ class Client(object): # pylint: disable=too-many-instance-attributes """ new_authz = messages.Authorization(identifier=identifier) - response = self._post(new_authzr_uri, new_authz) + response = self.net.post(new_authzr_uri, new_authz) assert response.status_code == httplib.CREATED # TODO: handle errors return self._authzr_from_response(response, identifier) @@ -326,7 +194,7 @@ class Client(object): # pylint: disable=too-many-instance-attributes :raises .UnexpectedUpdate: """ - response = self._post(challb.uri, response) + response = self.net.post(challb.uri, response) try: authzr_uri = response.links['up']['url'] except KeyError: @@ -377,7 +245,7 @@ class Client(object): # pylint: disable=too-many-instance-attributes :rtype: (`.AuthorizationResource`, `requests.Response`) """ - response = self._get(authzr.uri) + response = self.net.get(authzr.uri) updated_authzr = self._authzr_from_response( response, authzr.body.identifier, authzr.uri, authzr.new_cert_uri) # TODO: check and raise UnexpectedUpdate @@ -403,7 +271,7 @@ class Client(object): # pylint: disable=too-many-instance-attributes csr=csr, authorizations=tuple(authzr.uri for authzr in authzrs)) content_type = self.DER_CONTENT_TYPE # TODO: add 'cert_type 'argument - response = self._post( + response = self.net.post( authzrs[0].new_cert_uri, # TODO: acme-spec #90 req, content_type=content_type, @@ -488,8 +356,8 @@ class Client(object): # pylint: disable=too-many-instance-attributes """ content_type = self.DER_CONTENT_TYPE # TODO: make it a param - response = self._get(uri, headers={'Accept': content_type}, - content_type=content_type) + response = self.net.get(uri, headers={'Accept': content_type}, + content_type=content_type) return response, jose.ComparableX509( M2Crypto.X509.load_cert_der_string(response.content)) @@ -551,8 +419,152 @@ class Client(object): # pylint: disable=too-many-instance-attributes :raises .ClientError: If revocation is unsuccessful. """ - response = self._post(messages.Revocation.url(self.new_reg_uri), - messages.Revocation(certificate=cert)) + response = self.net.post(messages.Revocation.url(self.new_reg_uri), + messages.Revocation(certificate=cert)) if response.status_code != httplib.OK: raise errors.ClientError( 'Successful revocation must return HTTP OK status') + + +class ClientNetwork(object): + """Client network.""" + JSON_CONTENT_TYPE = 'application/json' + JSON_ERROR_CONTENT_TYPE = 'application/problem+json' + REPLAY_NONCE_HEADER = 'Replay-Nonce' + + def __init__(self, key, alg=jose.RS256, verify_ssl=True): + self.key = key + self.alg = alg + self.verify_ssl = verify_ssl + self._nonces = set() + + def _wrap_in_jws(self, obj, nonce): + """Wrap `JSONDeSerializable` object in JWS. + + .. todo:: Implement ``acmePath``. + + :param JSONDeSerializable obj: + :rtype: `.JWS` + + """ + dumps = obj.json_dumps() + logger.debug('Serialized JSON: %s', dumps) + return jws.JWS.sign( + payload=dumps, key=self.key, alg=self.alg, nonce=nonce).json_dumps() + + @classmethod + def _check_response(cls, response, content_type=None): + """Check response content and its type. + + .. note:: + Checking is not strict: wrong server response ``Content-Type`` + HTTP header is ignored if response is an expected JSON object + (c.f. Boulder #56). + + :param str content_type: Expected Content-Type response header. + If JSON is expected and not present in server response, this + function will raise an error. Otherwise, wrong Content-Type + is ignored, but logged. + + :raises .messages.Error: If server response body + carries HTTP Problem (draft-ietf-appsawg-http-problem-00). + :raises .ClientError: In case of other networking errors. + + """ + logger.debug('Received response %s (headers: %s): %r', + response, response.headers, response.content) + + response_ct = response.headers.get('Content-Type') + try: + # TODO: response.json() is called twice, once here, and + # once in _get and _post clients + jobj = response.json() + except ValueError as error: + jobj = None + + if not response.ok: + if jobj is not None: + if response_ct != cls.JSON_ERROR_CONTENT_TYPE: + logger.debug( + 'Ignoring wrong Content-Type (%r) for JSON Error', + response_ct) + try: + raise messages.Error.from_json(jobj) + except jose.DeserializationError as error: + # Couldn't deserialize JSON object + raise errors.ClientError((response, error)) + else: + # response is not JSON object + raise errors.ClientError(response) + else: + if jobj is not None and response_ct != cls.JSON_CONTENT_TYPE: + logger.debug( + 'Ignoring wrong Content-Type (%r) for JSON decodable ' + 'response', response_ct) + + if content_type == cls.JSON_CONTENT_TYPE and jobj is None: + raise errors.ClientError( + 'Unexpected response Content-Type: {0}'.format(response_ct)) + + def get(self, uri, content_type=JSON_CONTENT_TYPE, **kwargs): + """Send GET request. + + :raises .ClientError: + + :returns: HTTP Response + :rtype: `requests.Response` + + """ + logger.debug('Sending GET request to %s', uri) + kwargs.setdefault('verify', self.verify_ssl) + try: + response = requests.get(uri, **kwargs) + except requests.exceptions.RequestException as error: + raise errors.ClientError(error) + self._check_response(response, content_type=content_type) + return response + + def _add_nonce(self, response): + if self.REPLAY_NONCE_HEADER in response.headers: + nonce = response.headers[self.REPLAY_NONCE_HEADER] + error = jws.Header.validate_nonce(nonce) + if error is None: + logger.debug('Storing nonce: %r', nonce) + self._nonces.add(nonce) + else: + raise errors.ClientError('Invalid nonce ({0}): {1}'.format( + nonce, error)) + else: + raise errors.ClientError( + 'Server {0} response did not include a replay nonce'.format( + response.request.method)) + + def _get_nonce(self, uri): + if not self._nonces: + logger.debug('Requesting fresh nonce by sending HEAD to %s', uri) + self._add_nonce(requests.head(uri)) + return self._nonces.pop() + + def post(self, uri, obj, content_type=JSON_CONTENT_TYPE, **kwargs): + """Send POST data. + + :param JSONDeSerializable obj: Will be wrapped in JWS. + :param str content_type: Expected ``Content-Type``, fails if not set. + + :raises acme.messages.ClientError: + + :returns: HTTP Response + :rtype: `requests.Response` + + """ + data = self._wrap_in_jws(obj, self._get_nonce(uri)) + logger.debug('Sending POST data to %s: %s', uri, data) + kwargs.setdefault('verify', self.verify_ssl) + try: + response = requests.post(uri, data=data, **kwargs) + except requests.exceptions.RequestException as error: + raise errors.ClientError(error) + + self._add_nonce(response) + self._check_response(response, content_type=content_type) + return response diff --git a/acme/client_test.py b/acme/client_test.py index d408f0564..e935d9563 100644 --- a/acme/client_test.py +++ b/acme/client_test.py @@ -23,27 +23,20 @@ KEY2 = jose.JWKRSA.load(pkg_resources.resource_string( class ClientTest(unittest.TestCase): - """Tests for acme.client.Client.""" - + """Tests for acme.client.Client.""" # pylint: disable=too-many-instance-attributes,too-many-public-methods def setUp(self): - self.verify_ssl = mock.MagicMock() - self.wrap_in_jws = mock.MagicMock(return_value=mock.sentinel.wrapped) + self.response = mock.MagicMock( + ok=True, status_code=httplib.OK, headers={}, links={}) + self.net = mock.MagicMock() + self.net.post.return_value = self.response + self.net.get.return_value = self.response from acme.client import Client - self.net = Client( + self.client = Client( new_reg_uri='https://www.letsencrypt-demo.org/acme/new-reg', - key=KEY, alg=jose.RS256, verify_ssl=self.verify_ssl) - self.nonce = jose.b64encode('Nonce') - self.net._nonces.add(self.nonce) # pylint: disable=protected-access - - self.response = mock.MagicMock(ok=True, status_code=httplib.OK) - self.response.headers = {} - self.response.links = {} - - self.post = mock.MagicMock(return_value=self.response) - self.get = mock.MagicMock(return_value=self.response) + key=KEY, alg=jose.RS256, net=self.net) self.identifier = messages.Identifier( typ=messages.IDENTIFIER_FQDN, value='example.com') @@ -78,10 +71,300 @@ class ClientTest(unittest.TestCase): uri='https://www.letsencrypt-demo.org/acme/cert/1', cert_chain_uri='https://www.letsencrypt-demo.org/ca') - def _mock_post_get(self): + def test_register(self): + self.response.status_code = httplib.CREATED + self.response.json.return_value = self.regr.body.to_json() + self.response.headers['Location'] = self.regr.uri + self.response.links.update({ + 'next': {'url': self.regr.new_authzr_uri}, + 'terms-of-service': {'url': self.regr.terms_of_service}, + }) + + self.assertEqual(self.regr, self.client.register(self.contact)) + # TODO: test POST call arguments + + # TODO: split here and separate test + reg_wrong_key = self.regr.body.update(key=KEY2.public()) + self.response.json.return_value = reg_wrong_key.to_json() + self.assertRaises( + errors.UnexpectedUpdate, self.client.register, self.contact) + + def test_register_missing_next(self): + self.response.status_code = httplib.CREATED + self.assertRaises( + errors.ClientError, self.client.register, self.regr.body) + + def test_update_registration(self): + self.response.headers['Location'] = self.regr.uri + self.response.json.return_value = self.regr.body.to_json() + self.assertEqual(self.regr, self.client.update_registration(self.regr)) + + # TODO: split here and separate test + self.response.json.return_value = self.regr.body.update( + contact=()).to_json() + self.assertRaises( + errors.UnexpectedUpdate, self.client.update_registration, self.regr) + + def test_agree_to_tos(self): + self.client.update_registration = mock.Mock() + self.client.agree_to_tos(self.regr) + regr = self.client.update_registration.call_args[0][0] + self.assertEqual(self.regr.terms_of_service, regr.body.agreement) + + def test_request_challenges(self): + self.response.status_code = httplib.CREATED + self.response.headers['Location'] = self.authzr.uri + self.response.json.return_value = self.authz.to_json() + self.response.links = { + 'next': {'url': self.authzr.new_cert_uri}, + } + + self.client.request_challenges(self.identifier, self.authzr.uri) + # TODO: test POST call arguments + + # TODO: split here and separate test + self.response.json.return_value = self.authz.update( + identifier=self.identifier.update(value='foo')).to_json() + self.assertRaises( + errors.UnexpectedUpdate, self.client.request_challenges, + self.identifier, self.authzr.uri) + + def test_request_challenges_missing_next(self): + self.response.status_code = httplib.CREATED + self.assertRaises( + errors.ClientError, self.client.request_challenges, + self.identifier, self.regr) + + def test_request_domain_challenges(self): + self.client.request_challenges = mock.MagicMock() + self.assertEqual( + self.client.request_challenges(self.identifier), + self.client.request_domain_challenges('example.com', self.regr)) + + def test_answer_challenge(self): + self.response.links['up'] = {'url': self.challr.authzr_uri} + self.response.json.return_value = self.challr.body.to_json() + + chall_response = challenges.DNSResponse() + + self.client.answer_challenge(self.challr.body, chall_response) + + # TODO: split here and separate test + self.assertRaises(errors.UnexpectedUpdate, self.client.answer_challenge, + self.challr.body.update(uri='foo'), chall_response) + + def test_answer_challenge_missing_next(self): + self.assertRaises(errors.ClientError, self.client.answer_challenge, + self.challr.body, challenges.DNSResponse()) + + def test_retry_after_date(self): + self.response.headers['Retry-After'] = 'Fri, 31 Dec 1999 23:59:59 GMT' + self.assertEqual( + datetime.datetime(1999, 12, 31, 23, 59, 59), + self.client.retry_after(response=self.response, default=10)) + + @mock.patch('acme.client.datetime') + def test_retry_after_invalid(self, dt_mock): + dt_mock.datetime.now.return_value = datetime.datetime(2015, 3, 27) + dt_mock.timedelta = datetime.timedelta + + self.response.headers['Retry-After'] = 'foooo' + self.assertEqual( + datetime.datetime(2015, 3, 27, 0, 0, 10), + self.client.retry_after(response=self.response, default=10)) + + @mock.patch('acme.client.datetime') + def test_retry_after_seconds(self, dt_mock): + dt_mock.datetime.now.return_value = datetime.datetime(2015, 3, 27) + dt_mock.timedelta = datetime.timedelta + + self.response.headers['Retry-After'] = '50' + self.assertEqual( + datetime.datetime(2015, 3, 27, 0, 0, 50), + self.client.retry_after(response=self.response, default=10)) + + @mock.patch('acme.client.datetime') + def test_retry_after_missing(self, dt_mock): + dt_mock.datetime.now.return_value = datetime.datetime(2015, 3, 27) + dt_mock.timedelta = datetime.timedelta + + self.assertEqual( + datetime.datetime(2015, 3, 27, 0, 0, 10), + self.client.retry_after(response=self.response, default=10)) + + def test_poll(self): + self.response.json.return_value = self.authzr.body.to_json() + self.assertEqual((self.authzr, self.response), + self.client.poll(self.authzr)) + + # TODO: split here and separate test + self.response.json.return_value = self.authz.update( + identifier=self.identifier.update(value='foo')).to_json() + self.assertRaises( + errors.UnexpectedUpdate, self.client.poll, self.authzr) + + def test_request_issuance(self): + self.response.content = messages_test.CERT.as_der() + self.response.headers['Location'] = self.certr.uri + self.response.links['up'] = {'url': self.certr.cert_chain_uri} + self.assertEqual(self.certr, self.client.request_issuance( + messages_test.CSR, (self.authzr,))) + # TODO: check POST args + + def test_request_issuance_missing_up(self): + self.response.content = messages_test.CERT.as_der() + self.response.headers['Location'] = self.certr.uri + self.assertEqual( + self.certr.update(cert_chain_uri=None), + self.client.request_issuance(messages_test.CSR, (self.authzr,))) + + def test_request_issuance_missing_location(self): + self.assertRaises( + errors.ClientError, self.client.request_issuance, + messages_test.CSR, (self.authzr,)) + + @mock.patch('acme.client.datetime') + @mock.patch('acme.client.time') + def test_poll_and_request_issuance(self, time_mock, dt_mock): + # clock.dt | pylint: disable=no-member + clock = mock.MagicMock(dt=datetime.datetime(2015, 3, 27)) + + def sleep(seconds): + """increment clock""" + clock.dt += datetime.timedelta(seconds=seconds) + time_mock.sleep.side_effect = sleep + + def now(): + """return current clock value""" + return clock.dt + dt_mock.datetime.now.side_effect = now + dt_mock.timedelta = datetime.timedelta + + def poll(authzr): # pylint: disable=missing-docstring + # record poll start time based on the current clock value + authzr.times.append(clock.dt) + + # suppose it takes 2 seconds for server to produce the + # result, increment clock + clock.dt += datetime.timedelta(seconds=2) + + if not authzr.retries: # no more retries + done = mock.MagicMock(uri=authzr.uri, times=authzr.times) + done.body.status = messages.STATUS_VALID + return done, [] + + # response (2nd result tuple element) is reduced to only + # Retry-After header contents represented as integer + # seconds; authzr.retries is a list of Retry-After + # headers, head(retries) is peeled of as a current + # Retry-After header, and tail(retries) is persisted for + # later poll() calls + return (mock.MagicMock(retries=authzr.retries[1:], + uri=authzr.uri + '.', times=authzr.times), + authzr.retries[0]) + self.client.poll = mock.MagicMock(side_effect=poll) + + mintime = 7 + + def retry_after(response, default): # pylint: disable=missing-docstring + # check that poll_and_request_issuance correctly passes mintime + self.assertEqual(default, mintime) + return clock.dt + datetime.timedelta(seconds=response) + self.client.retry_after = mock.MagicMock(side_effect=retry_after) + + def request_issuance(csr, authzrs): # pylint: disable=missing-docstring + return csr, authzrs + self.client.request_issuance = mock.MagicMock( + side_effect=request_issuance) + + csr = mock.MagicMock() + authzrs = ( + mock.MagicMock(uri='a', times=[], retries=(8, 20, 30)), + mock.MagicMock(uri='b', times=[], retries=(5,)), + ) + + cert, updated_authzrs = self.client.poll_and_request_issuance( + csr, authzrs, mintime=mintime) + self.assertTrue(cert[0] is csr) + self.assertTrue(cert[1] is updated_authzrs) + self.assertEqual(updated_authzrs[0].uri, 'a...') + self.assertEqual(updated_authzrs[1].uri, 'b.') + self.assertEqual(updated_authzrs[0].times, [ + datetime.datetime(2015, 3, 27), + # a is scheduled for 10, but b is polling [9..11), so it + # will be picked up as soon as b is finished, without + # additional sleeping + datetime.datetime(2015, 3, 27, 0, 0, 11), + datetime.datetime(2015, 3, 27, 0, 0, 33), + datetime.datetime(2015, 3, 27, 0, 1, 5), + ]) + self.assertEqual(updated_authzrs[1].times, [ + datetime.datetime(2015, 3, 27, 0, 0, 2), + datetime.datetime(2015, 3, 27, 0, 0, 9), + ]) + self.assertEqual(clock.dt, datetime.datetime(2015, 3, 27, 0, 1, 7)) + + def test_check_cert(self): + self.response.headers['Location'] = self.certr.uri + self.response.content = messages_test.CERT.as_der() + self.assertEqual(self.certr.update(body=messages_test.CERT), + self.client.check_cert(self.certr)) + + # TODO: split here and separate test + self.response.headers['Location'] = 'foo' + self.assertRaises( + errors.UnexpectedUpdate, self.client.check_cert, self.certr) + + def test_check_cert_missing_location(self): + self.response.content = messages_test.CERT.as_der() + self.assertRaises( + errors.ClientError, self.client.check_cert, self.certr) + + def test_refresh(self): + self.client.check_cert = mock.MagicMock() + self.assertEqual( + self.client.check_cert(self.certr), self.client.refresh(self.certr)) + + def test_fetch_chain(self): # pylint: disable=protected-access - self.net._post = self.post - self.net._get = self.get + self.client._get_cert = mock.MagicMock() + self.client._get_cert.return_value = ("response", "certificate") + self.assertEqual(self.client._get_cert(self.certr.cert_chain_uri)[1], + self.client.fetch_chain(self.certr)) + + def test_fetch_chain_no_up_link(self): + self.assertTrue(self.client.fetch_chain(self.certr.update( + cert_chain_uri=None)) is None) + + def test_revoke(self): + self.client.revoke(self.certr.body) + self.net.post.assert_called_once_with(messages.Revocation.url( + self.client.new_reg_uri), mock.ANY) + + def test_revoke_bad_status_raises_error(self): + self.response.status_code = httplib.METHOD_NOT_ALLOWED + self.assertRaises(errors.ClientError, self.client.revoke, self.certr) + + +class ClientNetworkTest(unittest.TestCase): + """Tests for acme.client.ClientNetwork.""" + + def setUp(self): + self.verify_ssl = mock.MagicMock() + self.wrap_in_jws = mock.MagicMock(return_value=mock.sentinel.wrapped) + + from acme.client import ClientNetwork + self.net = ClientNetwork( + key=KEY, alg=jose.RS256, verify_ssl=self.verify_ssl) + + self.nonce = jose.b64encode('Nonce') + # pylint: disable=protected-access + self.assertEqual(self.net._nonces, set()) + self.net._nonces.add(self.nonce) + + self.response = mock.MagicMock(ok=True, status_code=httplib.OK) + self.response.headers = {} + self.response.links = {} def test_init(self): self.assertTrue(self.net.verify_ssl is self.verify_ssl) @@ -153,14 +436,13 @@ class ClientTest(unittest.TestCase): def test_get_requests_error_passthrough(self, requests_mock): requests_mock.exceptions = requests.exceptions requests_mock.get.side_effect = requests.exceptions.RequestException - # pylint: disable=protected-access - self.assertRaises(errors.ClientError, self.net._get, 'uri') + self.assertRaises(errors.ClientError, self.net.get, 'uri') @mock.patch('acme.client.requests') def test_get(self, requests_mock): # pylint: disable=protected-access self.net._check_response = mock.MagicMock() - self.net._get('uri', content_type='ct') + self.net.get('uri', content_type='ct') self.net._check_response.assert_called_once_with( requests_mock.get('uri'), content_type='ct') @@ -172,10 +454,9 @@ class ClientTest(unittest.TestCase): def test_post_requests_error_passthrough(self, requests_mock): requests_mock.exceptions = requests.exceptions requests_mock.post.side_effect = requests.exceptions.RequestException - # pylint: disable=protected-access self._mock_wrap_in_jws() self.assertRaises( - errors.ClientError, self.net._post, 'uri', mock.sentinel.obj) + errors.ClientError, self.net.post, 'uri', mock.sentinel.obj) @mock.patch('acme.client.requests') def test_post(self, requests_mock): @@ -184,7 +465,7 @@ class ClientTest(unittest.TestCase): self._mock_wrap_in_jws() requests_mock.post().headers = { self.net.REPLAY_NONCE_HEADER: self.nonce} - self.net._post('uri', mock.sentinel.obj, content_type='ct') + self.net.post('uri', mock.sentinel.obj, content_type='ct') self.net._check_response.assert_called_once_with( requests_mock.post('uri', mock.sentinel.wrapped), content_type='ct') @@ -196,7 +477,7 @@ class ClientTest(unittest.TestCase): self.net._nonces.clear() self.assertRaises( - errors.ClientError, self.net._post, 'uri', mock.sentinel.obj) + errors.ClientError, self.net.post, 'uri', mock.sentinel.obj) nonce2 = jose.b64encode('Nonce2') requests_mock.head('uri').headers = { @@ -204,7 +485,7 @@ class ClientTest(unittest.TestCase): requests_mock.post('uri').headers = { self.net.REPLAY_NONCE_HEADER: self.nonce} - self.net._post('uri', mock.sentinel.obj) + self.net.post('uri', mock.sentinel.obj) requests_mock.head.assert_called_with('uri') self.wrap_in_jws.assert_called_once_with(mock.sentinel.obj, nonce2) @@ -213,7 +494,7 @@ class ClientTest(unittest.TestCase): # wrong nonce requests_mock.post('uri').headers = {self.net.REPLAY_NONCE_HEADER: 'F'} self.assertRaises( - errors.ClientError, self.net._post, 'uri', mock.sentinel.obj) + errors.ClientError, self.net.post, 'uri', mock.sentinel.obj) @mock.patch('acme.client.requests') def test_get_post_verify_ssl(self, requests_mock): @@ -223,301 +504,16 @@ class ClientTest(unittest.TestCase): for verify_ssl in [True, False]: self.net.verify_ssl = verify_ssl - self.net._get('uri') + self.net.get('uri') self.net._nonces.add('N') requests_mock.post().headers = { self.net.REPLAY_NONCE_HEADER: self.nonce} - self.net._post('uri', mock.sentinel.obj) + self.net.post('uri', mock.sentinel.obj) requests_mock.get.assert_called_once_with('uri', verify=verify_ssl) requests_mock.post.assert_called_with( 'uri', data=mock.sentinel.wrapped, verify=verify_ssl) requests_mock.reset_mock() - def test_register(self): - self.response.status_code = httplib.CREATED - self.response.json.return_value = self.regr.body.to_json() - self.response.headers['Location'] = self.regr.uri - self.response.links.update({ - 'next': {'url': self.regr.new_authzr_uri}, - 'terms-of-service': {'url': self.regr.terms_of_service}, - }) - - self._mock_post_get() - self.assertEqual(self.regr, self.net.register(self.contact)) - # TODO: test POST call arguments - - # TODO: split here and separate test - reg_wrong_key = self.regr.body.update(key=KEY2.public()) - self.response.json.return_value = reg_wrong_key.to_json() - self.assertRaises( - errors.UnexpectedUpdate, self.net.register, self.contact) - - def test_register_missing_next(self): - self.response.status_code = httplib.CREATED - self._mock_post_get() - self.assertRaises( - errors.ClientError, self.net.register, self.regr.body) - - def test_update_registration(self): - self.response.headers['Location'] = self.regr.uri - self.response.json.return_value = self.regr.body.to_json() - self._mock_post_get() - self.assertEqual(self.regr, self.net.update_registration(self.regr)) - - # TODO: split here and separate test - self.response.json.return_value = self.regr.body.update( - contact=()).to_json() - self.assertRaises( - errors.UnexpectedUpdate, self.net.update_registration, self.regr) - - def test_agree_to_tos(self): - self.net.update_registration = mock.Mock() - self.net.agree_to_tos(self.regr) - regr = self.net.update_registration.call_args[0][0] - self.assertEqual(self.regr.terms_of_service, regr.body.agreement) - - def test_request_challenges(self): - self.response.status_code = httplib.CREATED - self.response.headers['Location'] = self.authzr.uri - self.response.json.return_value = self.authz.to_json() - self.response.links = { - 'next': {'url': self.authzr.new_cert_uri}, - } - - self._mock_post_get() - self.net.request_challenges(self.identifier, self.authzr.uri) - # TODO: test POST call arguments - - # TODO: split here and separate test - self.response.json.return_value = self.authz.update( - identifier=self.identifier.update(value='foo')).to_json() - self.assertRaises(errors.UnexpectedUpdate, self.net.request_challenges, - self.identifier, self.authzr.uri) - - def test_request_challenges_missing_next(self): - self.response.status_code = httplib.CREATED - self._mock_post_get() - self.assertRaises( - errors.ClientError, self.net.request_challenges, - self.identifier, self.regr) - - def test_request_domain_challenges(self): - self.net.request_challenges = mock.MagicMock() - self.assertEqual( - self.net.request_challenges(self.identifier), - self.net.request_domain_challenges('example.com', self.regr)) - - def test_answer_challenge(self): - self.response.links['up'] = {'url': self.challr.authzr_uri} - self.response.json.return_value = self.challr.body.to_json() - - chall_response = challenges.DNSResponse() - - self._mock_post_get() - self.net.answer_challenge(self.challr.body, chall_response) - - # TODO: split here and separate test - self.assertRaises(errors.UnexpectedUpdate, self.net.answer_challenge, - self.challr.body.update(uri='foo'), chall_response) - - def test_answer_challenge_missing_next(self): - self._mock_post_get() - self.assertRaises(errors.ClientError, self.net.answer_challenge, - self.challr.body, challenges.DNSResponse()) - - def test_retry_after_date(self): - self.response.headers['Retry-After'] = 'Fri, 31 Dec 1999 23:59:59 GMT' - self.assertEqual( - datetime.datetime(1999, 12, 31, 23, 59, 59), - self.net.retry_after(response=self.response, default=10)) - - @mock.patch('acme.client.datetime') - def test_retry_after_invalid(self, dt_mock): - dt_mock.datetime.now.return_value = datetime.datetime(2015, 3, 27) - dt_mock.timedelta = datetime.timedelta - - self.response.headers['Retry-After'] = 'foooo' - self.assertEqual( - datetime.datetime(2015, 3, 27, 0, 0, 10), - self.net.retry_after(response=self.response, default=10)) - - @mock.patch('acme.client.datetime') - def test_retry_after_seconds(self, dt_mock): - dt_mock.datetime.now.return_value = datetime.datetime(2015, 3, 27) - dt_mock.timedelta = datetime.timedelta - - self.response.headers['Retry-After'] = '50' - self.assertEqual( - datetime.datetime(2015, 3, 27, 0, 0, 50), - self.net.retry_after(response=self.response, default=10)) - - @mock.patch('acme.client.datetime') - def test_retry_after_missing(self, dt_mock): - dt_mock.datetime.now.return_value = datetime.datetime(2015, 3, 27) - dt_mock.timedelta = datetime.timedelta - - self.assertEqual( - datetime.datetime(2015, 3, 27, 0, 0, 10), - self.net.retry_after(response=self.response, default=10)) - - def test_poll(self): - self.response.json.return_value = self.authzr.body.to_json() - self._mock_post_get() - self.assertEqual((self.authzr, self.response), - self.net.poll(self.authzr)) - - # TODO: split here and separate test - self.response.json.return_value = self.authz.update( - identifier=self.identifier.update(value='foo')).to_json() - self.assertRaises(errors.UnexpectedUpdate, self.net.poll, self.authzr) - - def test_request_issuance(self): - self.response.content = messages_test.CERT.as_der() - self.response.headers['Location'] = self.certr.uri - self.response.links['up'] = {'url': self.certr.cert_chain_uri} - self._mock_post_get() - self.assertEqual(self.certr, self.net.request_issuance( - messages_test.CSR, (self.authzr,))) - # TODO: check POST args - - def test_request_issuance_missing_up(self): - self.response.content = messages_test.CERT.as_der() - self.response.headers['Location'] = self.certr.uri - self._mock_post_get() - self.assertEqual( - self.certr.update(cert_chain_uri=None), - self.net.request_issuance(messages_test.CSR, (self.authzr,))) - - def test_request_issuance_missing_location(self): - self._mock_post_get() - self.assertRaises( - errors.ClientError, self.net.request_issuance, - messages_test.CSR, (self.authzr,)) - - @mock.patch('acme.client.datetime') - @mock.patch('acme.client.time') - def test_poll_and_request_issuance(self, time_mock, dt_mock): - # clock.dt | pylint: disable=no-member - clock = mock.MagicMock(dt=datetime.datetime(2015, 3, 27)) - - def sleep(seconds): - """increment clock""" - clock.dt += datetime.timedelta(seconds=seconds) - time_mock.sleep.side_effect = sleep - - def now(): - """return current clock value""" - return clock.dt - dt_mock.datetime.now.side_effect = now - dt_mock.timedelta = datetime.timedelta - - def poll(authzr): # pylint: disable=missing-docstring - # record poll start time based on the current clock value - authzr.times.append(clock.dt) - - # suppose it takes 2 seconds for server to produce the - # result, increment clock - clock.dt += datetime.timedelta(seconds=2) - - if not authzr.retries: # no more retries - done = mock.MagicMock(uri=authzr.uri, times=authzr.times) - done.body.status = messages.STATUS_VALID - return done, [] - - # response (2nd result tuple element) is reduced to only - # Retry-After header contents represented as integer - # seconds; authzr.retries is a list of Retry-After - # headers, head(retries) is peeled of as a current - # Retry-After header, and tail(retries) is persisted for - # later poll() calls - return (mock.MagicMock(retries=authzr.retries[1:], - uri=authzr.uri + '.', times=authzr.times), - authzr.retries[0]) - self.net.poll = mock.MagicMock(side_effect=poll) - - mintime = 7 - - def retry_after(response, default): # pylint: disable=missing-docstring - # check that poll_and_request_issuance correctly passes mintime - self.assertEqual(default, mintime) - return clock.dt + datetime.timedelta(seconds=response) - self.net.retry_after = mock.MagicMock(side_effect=retry_after) - - def request_issuance(csr, authzrs): # pylint: disable=missing-docstring - return csr, authzrs - self.net.request_issuance = mock.MagicMock(side_effect=request_issuance) - - csr = mock.MagicMock() - authzrs = ( - mock.MagicMock(uri='a', times=[], retries=(8, 20, 30)), - mock.MagicMock(uri='b', times=[], retries=(5,)), - ) - - cert, updated_authzrs = self.net.poll_and_request_issuance( - csr, authzrs, mintime=mintime) - self.assertTrue(cert[0] is csr) - self.assertTrue(cert[1] is updated_authzrs) - self.assertEqual(updated_authzrs[0].uri, 'a...') - self.assertEqual(updated_authzrs[1].uri, 'b.') - self.assertEqual(updated_authzrs[0].times, [ - datetime.datetime(2015, 3, 27), - # a is scheduled for 10, but b is polling [9..11), so it - # will be picked up as soon as b is finished, without - # additional sleeping - datetime.datetime(2015, 3, 27, 0, 0, 11), - datetime.datetime(2015, 3, 27, 0, 0, 33), - datetime.datetime(2015, 3, 27, 0, 1, 5), - ]) - self.assertEqual(updated_authzrs[1].times, [ - datetime.datetime(2015, 3, 27, 0, 0, 2), - datetime.datetime(2015, 3, 27, 0, 0, 9), - ]) - self.assertEqual(clock.dt, datetime.datetime(2015, 3, 27, 0, 1, 7)) - - def test_check_cert(self): - self.response.headers['Location'] = self.certr.uri - self.response.content = messages_test.CERT.as_der() - self._mock_post_get() - self.assertEqual(self.certr.update(body=messages_test.CERT), - self.net.check_cert(self.certr)) - - # TODO: split here and separate test - self.response.headers['Location'] = 'foo' - self.assertRaises( - errors.UnexpectedUpdate, self.net.check_cert, self.certr) - - def test_check_cert_missing_location(self): - self.response.content = messages_test.CERT.as_der() - self._mock_post_get() - self.assertRaises(errors.ClientError, self.net.check_cert, self.certr) - - def test_refresh(self): - self.net.check_cert = mock.MagicMock() - self.assertEqual( - self.net.check_cert(self.certr), self.net.refresh(self.certr)) - - def test_fetch_chain(self): - # pylint: disable=protected-access - self.net._get_cert = mock.MagicMock() - self.net._get_cert.return_value = ("response", "certificate") - self.assertEqual(self.net._get_cert(self.certr.cert_chain_uri)[1], - self.net.fetch_chain(self.certr)) - - def test_fetch_chain_no_up_link(self): - self.assertTrue(self.net.fetch_chain(self.certr.update( - cert_chain_uri=None)) is None) - - def test_revoke(self): - self._mock_post_get() - self.net.revoke(self.certr.body) - self.post.assert_called_once_with(messages.Revocation.url( - self.net.new_reg_uri), mock.ANY) - - def test_revoke_bad_status_raises_error(self): - self.response.status_code = httplib.METHOD_NOT_ALLOWED - self._mock_post_get() - self.assertRaises(errors.ClientError, self.net.revoke, self.certr) - if __name__ == '__main__': unittest.main() # pragma: no cover From 4407210e0114bcacb75567abb71dce27f908c96f Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Tue, 30 Jun 2015 13:15:11 +0000 Subject: [PATCH 33/50] Fix --no-verify-ssl in HEAD, refactor acme.client_tests. Fix #521 by introducing MissingNonceError, which by shows response headers when printed to STDOUT. More sensible solution (a'la #523) is blocked by boulder#417 (HTTP 405 response for HEAD). Split out ClientNetworkWithMockedResponseTest from ClientNetworkTest, which improves readability and makes it easier to test (less mocks). --- acme/client.py | 83 ++++++++++---------- acme/client_test.py | 181 +++++++++++++++++++++++++------------------- acme/errors.py | 40 +++++++++- acme/errors_test.py | 33 ++++++++ 4 files changed, 218 insertions(+), 119 deletions(-) create mode 100644 acme/errors_test.py diff --git a/acme/client.py b/acme/client.py index aac539974..4a4192528 100644 --- a/acme/client.py +++ b/acme/client.py @@ -506,24 +506,47 @@ class ClientNetwork(object): raise errors.ClientError( 'Unexpected response Content-Type: {0}'.format(response_ct)) - def get(self, uri, content_type=JSON_CONTENT_TYPE, **kwargs): - """Send GET request. + return response - :raises .ClientError: + def _send_request(self, method, url, *args, **kwargs): + """Send HTTP request. + + Makes sure that `verify_ssl` is respected. Logs request and + response (with headers). For allowed parameters please see + `requests.request`. + + :param str method: method for the new `requests.Request` object + :param str url: URL for the new `requests.Request` object + + :raises requests.exceptions.RequestException: in case of any problems :returns: HTTP Response :rtype: `requests.Response` + """ - logger.debug('Sending GET request to %s', uri) - kwargs.setdefault('verify', self.verify_ssl) - try: - response = requests.get(uri, **kwargs) - except requests.exceptions.RequestException as error: - raise errors.ClientError(error) - self._check_response(response, content_type=content_type) + logging.debug('Sending %s request to %s', method, url) + kwargs['verify'] = self.verify_ssl + response = requests.request(method, url, *args, **kwargs) + logging.debug('Received %s. Headers: %s. Content: %r', + response, response.headers, response.content) return response + def head(self, *args, **kwargs): + """Send HEAD request without checking the response. + + Note, that `_check_response` is not called, as it is expected + that status code other than successfuly 2xx will be returned, or + messages2.Error will be raised by the server. + + """ + return self._send_request('HEAD', *args, **kwargs) + + def get(self, url, content_type=JSON_CONTENT_TYPE, **kwargs): + """Send GET request and check response.""" + return self._check_response( + self._send_request('GET', url, **kwargs), content_type=content_type) + def _add_nonce(self, response): if self.REPLAY_NONCE_HEADER in response.headers: nonce = response.headers[self.REPLAY_NONCE_HEADER] @@ -532,39 +555,19 @@ class ClientNetwork(object): logger.debug('Storing nonce: %r', nonce) self._nonces.add(nonce) else: - raise errors.ClientError('Invalid nonce ({0}): {1}'.format( - nonce, error)) + raise errors.BadNonce(nonce, error) else: - raise errors.ClientError( - 'Server {0} response did not include a replay nonce'.format( - response.request.method)) + raise errors.MissingNonce(response) - def _get_nonce(self, uri): + def _get_nonce(self, url): if not self._nonces: - logger.debug('Requesting fresh nonce by sending HEAD to %s', uri) - self._add_nonce(requests.head(uri)) + logging.debug('Requesting fresh nonce') + self._add_nonce(self.head(url)) return self._nonces.pop() - def post(self, uri, obj, content_type=JSON_CONTENT_TYPE, **kwargs): - """Send POST data. - - :param JSONDeSerializable obj: Will be wrapped in JWS. - :param str content_type: Expected ``Content-Type``, fails if not set. - - :raises acme.messages.ClientError: - - :returns: HTTP Response - :rtype: `requests.Response` - - """ - data = self._wrap_in_jws(obj, self._get_nonce(uri)) - logger.debug('Sending POST data to %s: %s', uri, data) - kwargs.setdefault('verify', self.verify_ssl) - try: - response = requests.post(uri, data=data, **kwargs) - except requests.exceptions.RequestException as error: - raise errors.ClientError(error) - + def post(self, url, obj, content_type=JSON_CONTENT_TYPE, **kwargs): + """POST object wrapped in `.JWS` and check response.""" + data = self._wrap_in_jws(obj, self._get_nonce(url)) + response = self._send_request('POST', url, data=data, **kwargs) self._add_nonce(response) - self._check_response(response, content_type=content_type) - return response + return self._check_response(response, content_type=content_type) diff --git a/acme/client_test.py b/acme/client_test.py index e935d9563..b934e1efd 100644 --- a/acme/client_test.py +++ b/acme/client_test.py @@ -357,11 +357,6 @@ class ClientNetworkTest(unittest.TestCase): self.net = ClientNetwork( key=KEY, alg=jose.RS256, verify_ssl=self.verify_ssl) - self.nonce = jose.b64encode('Nonce') - # pylint: disable=protected-access - self.assertEqual(self.net._nonces, set()) - self.net._nonces.add(self.nonce) - self.response = mock.MagicMock(ok=True, status_code=httplib.OK) self.response.headers = {} self.response.links = {} @@ -422,97 +417,127 @@ class ClientNetworkTest(unittest.TestCase): self.response.json.side_effect = ValueError for response_ct in [self.net.JSON_CONTENT_TYPE, 'foo']: self.response.headers['Content-Type'] = response_ct - # pylint: disable=protected-access - self.net._check_response(self.response) + # pylint: disable=protected-access,no-value-for-parameter + self.assertEqual( + self.response, self.net._check_response(self.response)) def test_check_response_jobj(self): self.response.json.return_value = {} for response_ct in [self.net.JSON_CONTENT_TYPE, 'foo']: self.response.headers['Content-Type'] = response_ct + # pylint: disable=protected-access,no-value-for-parameter + self.assertEqual( + self.response, self.net._check_response(self.response)) + + @mock.patch('acme.client.requests') + def test_send_request(self, mock_requests): + mock_requests.request.return_value = self.response + # pylint: disable=protected-access + self.assertEqual(self.response, self.net._send_request( + 'HEAD', 'url', 'foo', bar='baz')) + mock_requests.request.assert_called_once_with( + 'HEAD', 'url', 'foo', verify=mock.ANY, bar='baz') + + @mock.patch('acme.client.requests') + def test_send_request_verify_ssl(self, mock_requests): + # pylint: disable=protected-access + for verify in True, False: + mock_requests.request.reset_mock() + mock_requests.request.return_value = self.response + self.net.verify_ssl = verify # pylint: disable=protected-access - self.net._check_response(self.response) + self.assertEqual( + self.response, self.net._send_request('GET', 'url')) + mock_requests.request.assert_called_once_with( + 'GET', 'url', verify=verify) @mock.patch('acme.client.requests') - def test_get_requests_error_passthrough(self, requests_mock): - requests_mock.exceptions = requests.exceptions - requests_mock.get.side_effect = requests.exceptions.RequestException - self.assertRaises(errors.ClientError, self.net.get, 'uri') - - @mock.patch('acme.client.requests') - def test_get(self, requests_mock): + def test_requests_error_passthrough(self, mock_requests): + mock_requests.exceptions = requests.exceptions + mock_requests.request.side_effect = requests.exceptions.RequestException # pylint: disable=protected-access - self.net._check_response = mock.MagicMock() - self.net.get('uri', content_type='ct') - self.net._check_response.assert_called_once_with( - requests_mock.get('uri'), content_type='ct') + self.assertRaises(requests.exceptions.RequestException, + self.net._send_request, 'GET', 'uri') + + +class ClientNetworkWithMockedResponseTest(unittest.TestCase): + """Tests for acme.client.ClientNetwork which mock out response.""" + # pylint: disable=too-many-instance-attributes + + def setUp(self): + from acme.client import ClientNetwork + self.net = ClientNetwork(key=None, alg=None) + + self.response = mock.MagicMock(ok=True, status_code=httplib.OK) + self.response.headers = {} + self.response.links = {} + self.checked_response = mock.MagicMock() + self.obj = mock.MagicMock() + self.wrapped_obj = mock.MagicMock() + self.content_type = mock.sentinel.content_type + + self.all_nonces = [jose.b64encode('Nonce'), jose.b64encode('Nonce2')] + self.available_nonces = self.all_nonces[:] + def send_request(*args, **kwargs): + # pylint: disable=unused-argument,missing-docstring + if self.available_nonces: + self.response.headers = { + self.net.REPLAY_NONCE_HEADER: self.available_nonces.pop()} + else: + self.response.headers = {} + return self.response - def _mock_wrap_in_jws(self): # pylint: disable=protected-access - self.net._wrap_in_jws = self.wrap_in_jws + self.net._send_request = self.send_request = mock.MagicMock( + side_effect=send_request) + self.net._check_response = self.check_response + self.net._wrap_in_jws = mock.MagicMock(return_value=self.wrapped_obj) - @mock.patch('acme.client.requests') - def test_post_requests_error_passthrough(self, requests_mock): - requests_mock.exceptions = requests.exceptions - requests_mock.post.side_effect = requests.exceptions.RequestException - self._mock_wrap_in_jws() - self.assertRaises( - errors.ClientError, self.net.post, 'uri', mock.sentinel.obj) + def check_response(self, response, content_type): + # pylint: disable=missing-docstring + self.assertEqual(self.response, response) + self.assertEqual(self.content_type, content_type) + return self.checked_response - @mock.patch('acme.client.requests') - def test_post(self, requests_mock): + def test_head(self): + self.assertEqual(self.response, self.net.head('url', 'foo', bar='baz')) + self.send_request.assert_called_once('HEAD', 'url', 'foo', bar='baz') + + def test_get(self): + self.assertEqual(self.checked_response, self.net.get( + 'url', content_type=self.content_type, bar='baz')) + self.send_request.assert_called_once_with('GET', 'url', bar='baz') + + def test_post(self): # pylint: disable=protected-access - self.net._check_response = mock.MagicMock() - self._mock_wrap_in_jws() - requests_mock.post().headers = { - self.net.REPLAY_NONCE_HEADER: self.nonce} - self.net.post('uri', mock.sentinel.obj, content_type='ct') - self.net._check_response.assert_called_once_with( - requests_mock.post('uri', mock.sentinel.wrapped), content_type='ct') + self.assertEqual(self.checked_response, self.net.post( + 'uri', self.obj, content_type=self.content_type)) + self.net._wrap_in_jws.assert_called_once_with( + self.obj, self.all_nonces.pop()) - @mock.patch('acme.client.requests') - def test_post_replay_nonce_handling(self, requests_mock): - # pylint: disable=protected-access - self.net._check_response = mock.MagicMock() - self._mock_wrap_in_jws() + assert not self.available_nonces + self.assertRaises(errors.MissingNonce, self.net.post, + 'uri', self.obj, content_type=self.content_type) + self.net._wrap_in_jws.assert_called_with( + self.obj, self.all_nonces.pop()) - self.net._nonces.clear() - self.assertRaises( - errors.ClientError, self.net.post, 'uri', mock.sentinel.obj) + def test_post_wrong_initial_nonce(self): # HEAD + self.available_nonces = ['f', jose.b64encode('good')] + self.assertRaises(errors.BadNonce, self.net.post, 'uri', + self.obj, content_type=self.content_type) - nonce2 = jose.b64encode('Nonce2') - requests_mock.head('uri').headers = { - self.net.REPLAY_NONCE_HEADER: nonce2} - requests_mock.post('uri').headers = { - self.net.REPLAY_NONCE_HEADER: self.nonce} + def test_post_wrong_post_response_nonce(self): + self.available_nonces = [jose.b64encode('good'), 'f'] + self.assertRaises(errors.BadNonce, self.net.post, 'uri', + self.obj, content_type=self.content_type) - self.net.post('uri', mock.sentinel.obj) - - requests_mock.head.assert_called_with('uri') - self.wrap_in_jws.assert_called_once_with(mock.sentinel.obj, nonce2) - self.assertEqual(self.net._nonces, set([self.nonce])) - - # wrong nonce - requests_mock.post('uri').headers = {self.net.REPLAY_NONCE_HEADER: 'F'} - self.assertRaises( - errors.ClientError, self.net.post, 'uri', mock.sentinel.obj) - - @mock.patch('acme.client.requests') - def test_get_post_verify_ssl(self, requests_mock): - # pylint: disable=protected-access - self._mock_wrap_in_jws() - self.net._check_response = mock.MagicMock() - - for verify_ssl in [True, False]: - self.net.verify_ssl = verify_ssl - self.net.get('uri') - self.net._nonces.add('N') - requests_mock.post().headers = { - self.net.REPLAY_NONCE_HEADER: self.nonce} - self.net.post('uri', mock.sentinel.obj) - requests_mock.get.assert_called_once_with('uri', verify=verify_ssl) - requests_mock.post.assert_called_with( - 'uri', data=mock.sentinel.wrapped, verify=verify_ssl) - requests_mock.reset_mock() + def test_head_get_post_error_passthrough(self): + self.send_request.side_effect = requests.exceptions.RequestException + for method in self.net.head, self.net.get: + self.assertRaises( + requests.exceptions.RequestException, method, 'GET', 'uri') + self.assertRaises(requests.exceptions.RequestException, + self.net.post, 'uri', obj=self.obj) if __name__ == '__main__': diff --git a/acme/errors.py b/acme/errors.py index 5046d7aee..9a96ec43a 100644 --- a/acme/errors.py +++ b/acme/errors.py @@ -5,11 +5,49 @@ from acme.jose import errors as jose_errors class Error(Exception): """Generic ACME error.""" + class SchemaValidationError(jose_errors.DeserializationError): """JSON schema ACME object validation error.""" + class ClientError(Error): """Network error.""" + class UnexpectedUpdate(ClientError): - """Unexpected update.""" + """Unexpected update error.""" + + +class NonceError(ClientError): + """Server response nonce error.""" + + +class BadNonce(NonceError): + """Bad nonce error.""" + def __init__(self, nonce, error, *args, **kwargs): + super(BadNonce, self).__init__(*args, **kwargs) + self.nonce = nonce + self.error = error + + def __str__(self): + return 'Invalid nonce ({0!r}): {1}'.format(self.nonce, self.error) + + +class MissingNonce(NonceError): + """Missing nonce error. + + According to the specification an "ACME server MUST include an + Replay-Nonce header field in each successful response to a POST it + provides to a client (...)". + + :ivar requests.Response response: HTTP Response + + """ + def __init__(self, response, *args, **kwargs): + super(MissingNonce, self).__init__(*args, **kwargs) + self.response = response + + def __str__(self): + return ('Server {0} response did not include a replay ' + 'nonce, headers: {1}'.format( + self.response.request.method, self.response.headers)) diff --git a/acme/errors_test.py b/acme/errors_test.py new file mode 100644 index 000000000..3790d91ed --- /dev/null +++ b/acme/errors_test.py @@ -0,0 +1,33 @@ +"""Tests for acme.errors.""" +import unittest + +import mock + + +class BadNonceTest(unittest.TestCase): + """Tests for acme.errors.BadNonce.""" + + def setUp(self): + from acme.errors import BadNonce + self.error = BadNonce(nonce="xxx", error="error") + + def test_str(self): + self.assertEqual("Invalid nonce ('xxx'): error", str(self.error)) + + +class MissingNonceTest(unittest.TestCase): + """Tests for acme.errors.MissingNonce.""" + + def setUp(self): + from acme.errors import MissingNonce + self.response = mock.MagicMock(headers={}) + self.response.request.method = 'FOO' + self.error = MissingNonce(self.response) + + def test_str(self): + self.assertTrue("FOO" in str(self.error)) + self.assertTrue("{}" in str(self.error)) + + +if __name__ == "__main__": + unittest.main() # pragma: no cover From c639673de5bce49212e49dcda103cde17b4860ab Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Fri, 3 Jul 2015 14:38:09 +0000 Subject: [PATCH 34/50] Read config from $XDG_CONFIG_HOME/letsencrypt/cli.ini. --- letsencrypt/constants.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/letsencrypt/constants.py b/letsencrypt/constants.py index 1c3280006..4b0a215e1 100644 --- a/letsencrypt/constants.py +++ b/letsencrypt/constants.py @@ -1,4 +1,5 @@ """Let's Encrypt constants.""" +import os import logging from acme import challenges @@ -8,7 +9,12 @@ SETUPTOOLS_PLUGINS_ENTRY_POINT = "letsencrypt.plugins" """Setuptools entry point group name for plugins.""" CLI_DEFAULTS = dict( - config_files=["/etc/letsencrypt/cli.ini"], + config_files=[ + "/etc/letsencrypt/cli.ini", + # http://freedesktop.org/wiki/Software/xdg-user-dirs/ + os.path.join(os.environ.get("XDG_CONFIG_HOME", "~/.config"), + "letsencrypt", "cli.ini"), + ], verbose_count=-(logging.WARNING / 10), server="https://www.letsencrypt-demo.org/acme/new-reg", rsa_key_size=2048, From 7c3c52c2b1e3ba855551c4181bd4af82f1adb553 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Fri, 3 Jul 2015 15:02:01 +0000 Subject: [PATCH 35/50] Add example dev config file, config file docs. --- docs/using.rst | 21 +++++++++++++++++++++ examples/dev-cli.ini | 17 +++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 examples/dev-cli.ini diff --git a/docs/using.rst b/docs/using.rst index 951c991d2..a9a4aa958 100644 --- a/docs/using.rst +++ b/docs/using.rst @@ -151,6 +151,27 @@ The ``letsencrypt`` commandline tool has a builtin help: ./venv/bin/letsencrypt --help +Configuration file +------------------ + +It is possible to specify configuration file with +``letsencrypt --config cli.ini`` (or shorter ``-c cli.ini``). For +instance, if you are a contributor, you might find the following +handy: + +.. include:: ../examples/dev-cli.ini + :code: ini + +By default, the following locations are searched: + +- ``/etc/letsencrypt/cli.ini`` +- ``$XDG_CONFIG_HOME/letsencrypt/cli.ini`` (or + ``~/.config/letsencrypt/cli.ini`` if ``$XDG_CONFIG_HOME`` is not + set). + +.. keep it up to date with constants.py + + .. _Augeas: http://augeas.net/ .. _M2Crypto: https://github.com/M2Crypto/M2Crypto .. _SWIG: http://www.swig.org/ diff --git a/examples/dev-cli.ini b/examples/dev-cli.ini new file mode 100644 index 000000000..761bc58c9 --- /dev/null +++ b/examples/dev-cli.ini @@ -0,0 +1,17 @@ +# This is an example configuration file for developers +config-dir = /tmp/le/conf +work-dir = /tmp/le/conf +logs-dir = /tmp/le/logs + +# make sure to use a valid email and domains! +email = foo@example.com +domains = example.com + +text = True +agree-eula = True +debug = True +# Unfortunately, it's not possible to specify "verbose" multiple times +# (correspondingly to -vvvvvv) +verbose = True + +authenticator = standalone From e0293d81f3263ab958d041374c19634b335b35eb Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sun, 5 Jul 2015 13:11:33 +0000 Subject: [PATCH 36/50] acme: drop PyCrypto and use cryptography instead. - Use cryptography in acme.jose.jwa/jwk. - Change Crypto.Random to os.urandom, c.f. https://cryptography.io/en/latest/random-numbers/?highlight=urandom --- acme/challenges.py | 4 +- acme/challenges_test.py | 14 ++- acme/client.py | 3 +- acme/client_test.py | 4 +- acme/jose/__init__.py | 2 +- acme/jose/jwa.py | 116 ++++++++++++------ acme/jose/jwa_test.py | 34 ++--- acme/jose/jwk.py | 92 +++++++++++--- acme/jose/jwk_test.py | 61 ++++++--- acme/jose/jws.py | 6 +- acme/jose/jws_test.py | 11 +- acme/jose/util.py | 41 ++++++- acme/jose/util_test.py | 50 +++++--- acme/jws_test.py | 11 +- acme/messages_test.py | 11 +- acme/other.py | 12 +- acme/other_test.py | 10 +- examples/acme_client.py | 10 +- letsencrypt/proof_of_possession.py | 17 ++- letsencrypt/tests/acme_util.py | 10 +- letsencrypt/tests/proof_of_possession_test.py | 11 +- setup.py | 5 +- 22 files changed, 370 insertions(+), 165 deletions(-) diff --git a/acme/challenges.py b/acme/challenges.py index 0a2a461f0..45db23e72 100644 --- a/acme/challenges.py +++ b/acme/challenges.py @@ -3,8 +3,8 @@ import binascii import functools import hashlib import logging +import os -import Crypto.Random import requests from acme import jose @@ -204,7 +204,7 @@ class DVSNIResponse(ChallengeResponse): decoder=functools.partial(jose.decode_b64jose, size=S_SIZE)) def __init__(self, s=None, *args, **kwargs): - s = Crypto.Random.get_random_bytes(self.S_SIZE) if s is None else s + s = os.urandom(self.S_SIZE) if s is None else s super(DVSNIResponse, self).__init__(s=s, *args, **kwargs) def z(self, chall): # pylint: disable=invalid-name diff --git a/acme/challenges_test.py b/acme/challenges_test.py index bd332d1d9..b39e40280 100644 --- a/acme/challenges_test.py +++ b/acme/challenges_test.py @@ -3,7 +3,8 @@ import os import pkg_resources import unittest -import Crypto.PublicKey.RSA +from cryptography.hazmat.backends import default_backend +from cryptography.hazmat.primitives import serialization import M2Crypto import mock import requests @@ -16,9 +17,10 @@ from acme import other CERT = jose.ComparableX509(M2Crypto.X509.load_cert( pkg_resources.resource_filename( 'letsencrypt.tests', os.path.join('testdata', 'cert.pem')))) -KEY = jose.HashableRSAKey(Crypto.PublicKey.RSA.importKey( +KEY = jose.ComparableRSAKey(serialization.load_pem_private_key( pkg_resources.resource_string( - 'acme.jose', os.path.join('testdata', 'rsa512_key.pem')))) + 'acme.jose', os.path.join('testdata', 'rsa512_key.pem')), + password=None, backend=default_backend())) class ChallengeResponseTest(unittest.TestCase): @@ -345,7 +347,7 @@ class RecoveryTokenResponseTest(unittest.TestCase): class ProofOfPossessionHintsTest(unittest.TestCase): def setUp(self): - jwk = jose.JWKRSA(key=KEY.publickey()) + jwk = jose.JWKRSA(key=KEY.public_key()) issuers = ( 'C=US, O=SuperT LLC, CN=SuperTrustworthy Public CA', 'O=LessTrustworthy CA Inc, CN=LessTrustworthy But StillSecure', @@ -413,7 +415,7 @@ class ProofOfPossessionTest(unittest.TestCase): def setUp(self): from acme.challenges import ProofOfPossession hints = ProofOfPossession.Hints( - jwk=jose.JWKRSA(key=KEY.publickey()), cert_fingerprints=(), + jwk=jose.JWKRSA(key=KEY.public_key()), cert_fingerprints=(), certs=(), serial_numbers=(), subject_key_identifiers=(), issuers=(), authorized_for=()) self.msg = ProofOfPossession( @@ -453,7 +455,7 @@ class ProofOfPossessionResponseTest(unittest.TestCase): # nonce and challenge nonce are the same, don't make the same # mistake here... signature = other.Signature( - alg=jose.RS256, jwk=jose.JWKRSA(key=KEY.publickey()), + alg=jose.RS256, jwk=jose.JWKRSA(key=KEY.public_key()), sig='\xa7\xc1\xe7\xe82o\xbc\xcd\xd0\x1e\x010#Z|\xaf\x15\x83' '\x94\x8f#\x9b\nQo(\x80\x15,\x08\xfcz\x1d\xfd\xfd.\xaap' '\xfa\x06\xd1\xa2f\x8d8X2>%d\xbd%\xe1T\xdd\xaa0\x18\xde' diff --git a/acme/client.py b/acme/client.py index 4a4192528..fbdb4543d 100644 --- a/acme/client.py +++ b/acme/client.py @@ -83,7 +83,8 @@ class Client(object): # pylint: disable=too-many-instance-attributes assert response.status_code == httplib.CREATED # TODO: handle errors regr = self._regr_from_response(response) - if regr.body.key != self.key.public() or regr.body.contact != contact: + if (regr.body.key != self.key.public_key() + or regr.body.contact != contact): raise errors.UnexpectedUpdate(regr) return regr diff --git a/acme/client_test.py b/acme/client_test.py index b934e1efd..7ae14d0be 100644 --- a/acme/client_test.py +++ b/acme/client_test.py @@ -44,7 +44,7 @@ class ClientTest(unittest.TestCase): # Registration self.contact = ('mailto:cert-admin@example.com', 'tel:+12025551212') reg = messages.Registration( - contact=self.contact, key=KEY.public(), recovery_token='t') + contact=self.contact, key=KEY.public_key(), recovery_token='t') self.regr = messages.RegistrationResource( body=reg, uri='https://www.letsencrypt-demo.org/acme/reg/1', new_authzr_uri='https://www.letsencrypt-demo.org/acme/new-reg', @@ -84,7 +84,7 @@ class ClientTest(unittest.TestCase): # TODO: test POST call arguments # TODO: split here and separate test - reg_wrong_key = self.regr.body.update(key=KEY2.public()) + reg_wrong_key = self.regr.body.update(key=KEY2.public_key()) self.response.json.return_value = reg_wrong_key.to_json() self.assertRaises( errors.UnexpectedUpdate, self.client.register, self.contact) diff --git a/acme/jose/__init__.py b/acme/jose/__init__.py index a4fe7008b..793b342b0 100644 --- a/acme/jose/__init__.py +++ b/acme/jose/__init__.py @@ -74,6 +74,6 @@ from acme.jose.jws import ( from acme.jose.util import ( ComparableX509, - HashableRSAKey, + ComparableRSAKey, ImmutableMap, ) diff --git a/acme/jose/jwa.py b/acme/jose/jwa.py index 97c770b78..c3d79ff20 100644 --- a/acme/jose/jwa.py +++ b/acme/jose/jwa.py @@ -4,20 +4,22 @@ https://tools.ietf.org/html/draft-ietf-jose-json-web-algorithms-40 """ import abc +import logging -from Crypto.Hash import HMAC -from Crypto.Hash import SHA256 -from Crypto.Hash import SHA384 -from Crypto.Hash import SHA512 - -from Crypto.Signature import PKCS1_PSS -from Crypto.Signature import PKCS1_v1_5 +import cryptography.exceptions +from cryptography.hazmat.backends import default_backend +from cryptography.hazmat.primitives import hashes +from cryptography.hazmat.primitives import hmac +from cryptography.hazmat.primitives.asymmetric import padding from acme.jose import errors from acme.jose import interfaces from acme.jose import jwk +logger = logging.getLogger(__name__) + + class JWA(interfaces.JSONDeSerializable): # pylint: disable=abstract-method # pylint: disable=too-few-public-methods # for some reason disable=abstract-method has to be on the line @@ -66,43 +68,79 @@ class _JWAHS(JWASignature): kty = jwk.JWKOct - def __init__(self, name, digestmod): + def __init__(self, name, hash_): super(_JWAHS, self).__init__(name) - self.digestmod = digestmod + self.hash = hash_() def sign(self, key, msg): - return HMAC.new(key, msg, self.digestmod).digest() + signer = hmac.HMAC(key, self.hash, backend=default_backend()) + signer.update(msg) + return signer.finalize() def verify(self, key, msg, sig): - """Verify the signature. - - .. warning:: - Does not protect against timing attack (no constant compare). - - """ - return self.sign(key, msg) == sig + verifier = hmac.HMAC(key, self.hash, backend=default_backend()) + verifier.update(msg) + try: + verifier.verify(sig) + except cryptography.exceptions.InvalidSignature as error: + logger.debug(error, exc_info=True) + return False + else: + return True -class _JWARS(JWASignature): +class _JWARSA(object): kty = jwk.JWKRSA - - def __init__(self, name, padding, digestmod): - super(_JWARS, self).__init__(name) - self.padding = padding - self.digestmod = digestmod + padding = NotImplemented + hash = NotImplemented def sign(self, key, msg): + """Sign the ``msg`` using ``key``.""" try: - return self.padding.new(key).sign(self.digestmod.new(msg)) - except TypeError: - raise errors.Error('Key has no private part necessary for signing') - except (AttributeError, ValueError): - # ValueError for PS, AttributeError for RS - raise errors.Error('Key too small ({0})'.format(key.size())) + signer = key.signer(self.padding, self.hash) + except AttributeError as error: + logger.debug(error, exc_info=True) + raise errors.Error("Public key cannot be used for signing") + except ValueError as error: # digest too large + logger.debug(error, exc_info=True) + raise errors.Error(str(error)) + signer.update(msg) + try: + return signer.finalize() + except ValueError as error: + logger.debug(error, exc_info=True) + raise errors.Error(str(error)) def verify(self, key, msg, sig): - return self.padding.new(key).verify(self.digestmod.new(msg), sig) + """Verify the ``msg` and ``sig`` using ``key``.""" + verifier = key.verifier(sig, self.padding, self.hash) + verifier.update(msg) + try: + verifier.verify() + except cryptography.exceptions.InvalidSignature as error: + logger.debug(error, exc_info=True) + return False + else: + return True + + +class _JWARS(_JWARSA, JWASignature): + + def __init__(self, name, hash_): + super(_JWARS, self).__init__(name) + self.padding = padding.PKCS1v15() + self.hash = hash_() + + +class _JWAPS(_JWARSA, JWASignature): + + def __init__(self, name, hash_): + super(_JWAPS, self).__init__(name) + self.padding = padding.PSS( + mgf=padding.MGF1(hash_()), + salt_length=padding.PSS.MAX_LENGTH) + self.hash = hash_() class _JWAES(JWASignature): # pylint: disable=abstract-class-not-used @@ -116,17 +154,17 @@ class _JWAES(JWASignature): # pylint: disable=abstract-class-not-used raise NotImplementedError() -HS256 = JWASignature.register(_JWAHS('HS256', SHA256)) -HS384 = JWASignature.register(_JWAHS('HS384', SHA384)) -HS512 = JWASignature.register(_JWAHS('HS512', SHA512)) +HS256 = JWASignature.register(_JWAHS('HS256', hashes.SHA256)) +HS384 = JWASignature.register(_JWAHS('HS384', hashes.SHA384)) +HS512 = JWASignature.register(_JWAHS('HS512', hashes.SHA512)) -RS256 = JWASignature.register(_JWARS('RS256', PKCS1_v1_5, SHA256)) -RS384 = JWASignature.register(_JWARS('RS384', PKCS1_v1_5, SHA384)) -RS512 = JWASignature.register(_JWARS('RS512', PKCS1_v1_5, SHA512)) +RS256 = JWASignature.register(_JWARS('RS256', hashes.SHA256)) +RS384 = JWASignature.register(_JWARS('RS384', hashes.SHA384)) +RS512 = JWASignature.register(_JWARS('RS512', hashes.SHA512)) -PS256 = JWASignature.register(_JWARS('PS256', PKCS1_PSS, SHA256)) -PS384 = JWASignature.register(_JWARS('PS384', PKCS1_PSS, SHA384)) -PS512 = JWASignature.register(_JWARS('PS512', PKCS1_PSS, SHA512)) +PS256 = JWASignature.register(_JWAPS('PS256', hashes.SHA256)) +PS384 = JWASignature.register(_JWAPS('PS384', hashes.SHA384)) +PS512 = JWASignature.register(_JWAPS('PS512', hashes.SHA512)) ES256 = JWASignature.register(_JWAES('ES256')) ES256 = JWASignature.register(_JWAES('ES384')) diff --git a/acme/jose/jwa_test.py b/acme/jose/jwa_test.py index f66b2a250..c8347ff69 100644 --- a/acme/jose/jwa_test.py +++ b/acme/jose/jwa_test.py @@ -3,17 +3,24 @@ import os import pkg_resources import unittest -from Crypto.PublicKey import RSA +from cryptography.hazmat.backends import default_backend +from cryptography.hazmat.primitives import serialization from acme.jose import errors -RSA256_KEY = RSA.importKey(pkg_resources.resource_string( - __name__, os.path.join('testdata', 'rsa256_key.pem'))) -RSA512_KEY = RSA.importKey(pkg_resources.resource_string( - __name__, os.path.join('testdata', 'rsa512_key.pem'))) -RSA1024_KEY = RSA.importKey(pkg_resources.resource_string( - __name__, os.path.join('testdata', 'rsa1024_key.pem'))) +RSA256_KEY = serialization.load_pem_private_key( + pkg_resources.resource_string( + __name__, os.path.join('testdata', 'rsa256_key.pem')), + password=None, backend=default_backend()) +RSA512_KEY = serialization.load_pem_private_key( + pkg_resources.resource_string( + __name__, os.path.join('testdata', 'rsa512_key.pem')), + password=None, backend=default_backend()) +RSA1024_KEY = serialization.load_pem_private_key( + pkg_resources.resource_string( + __name__, os.path.join('testdata', 'rsa1024_key.pem')), + password=None, backend=default_backend()) class JWASignatureTest(unittest.TestCase): @@ -71,14 +78,13 @@ class JWARSTest(unittest.TestCase): def test_sign_no_private_part(self): from acme.jose.jwa import RS256 self.assertRaises( - errors.Error, RS256.sign, RSA512_KEY.publickey(), 'foo') + errors.Error, RS256.sign, RSA512_KEY.public_key(), 'foo') def test_sign_key_too_small(self): from acme.jose.jwa import RS256 from acme.jose.jwa import PS256 self.assertRaises(errors.Error, RS256.sign, RSA256_KEY, 'foo') self.assertRaises(errors.Error, PS256.sign, RSA256_KEY, 'foo') - self.assertRaises(errors.Error, PS256.sign, RSA512_KEY, 'foo') def test_rs(self): from acme.jose.jwa import RS256 @@ -89,16 +95,14 @@ class JWARSTest(unittest.TestCase): '\xd2\xb9.>}\xfd' ) self.assertEqual(RS256.sign(RSA512_KEY, 'foo'), sig) - # next tests guard that only True/False are return as oppossed - # to e.g. 1/0 - self.assertTrue(RS256.verify(RSA512_KEY, 'foo', sig) is True) - self.assertFalse(RS256.verify(RSA512_KEY, 'foo', sig + '!') is False) + self.assertTrue(RS256.verify(RSA512_KEY.public_key(), 'foo', sig)) + self.assertFalse(RS256.verify(RSA512_KEY.public_key(), 'foo', sig + '!')) def test_ps(self): from acme.jose.jwa import PS256 sig = PS256.sign(RSA1024_KEY, 'foo') - self.assertTrue(PS256.verify(RSA1024_KEY, 'foo', sig) is True) - self.assertTrue(PS256.verify(RSA1024_KEY, 'foo', sig + '!') is False) + self.assertTrue(PS256.verify(RSA1024_KEY.public_key(), 'foo', sig)) + self.assertFalse(PS256.verify(RSA1024_KEY.public_key(), 'foo', sig + '!')) if __name__ == '__main__': diff --git a/acme/jose/jwk.py b/acme/jose/jwk.py index 7c55e99a8..376988ae2 100644 --- a/acme/jose/jwk.py +++ b/acme/jose/jwk.py @@ -2,7 +2,9 @@ import abc import binascii -import Crypto.PublicKey.RSA +from cryptography.hazmat.backends import default_backend +from cryptography.hazmat.primitives import serialization +from cryptography.hazmat.primitives.asymmetric import rsa from acme.jose import b64 from acme.jose import errors @@ -22,14 +24,12 @@ class JWK(json_util.TypedJSONObjectWithFields): raise NotImplementedError() @abc.abstractmethod - def public(self): # pragma: no cover + def public_key(self): # pragma: no cover """Generate JWK with public key. For symmetric cryptosystems, this would return ``self``. """ - # TODO: rename publickey to stay consistent with - # HashableRSAKey.publickey raise NotImplementedError() @@ -54,7 +54,7 @@ class JWKES(JWK): # pragma: no cover def load(cls, string): raise NotImplementedError() - def public(self): + def public_key(self): raise NotImplementedError() @@ -79,7 +79,7 @@ class JWKOct(JWK): def load(cls, string): return cls(key=string) - def public(self): + def public_key(self): return self @@ -87,7 +87,9 @@ class JWKOct(JWK): class JWKRSA(JWK): """RSA JWK. - :ivar key: `Crypto.PublicKey.RSA` wrapped in `.HashableRSAKey` + :ivar key: `cryptography.hazmat.primitives.rsa.RSAPrivateKey` + or `cryptography.hazmat.primitives.rsa.RSAPublicKey` wrapped + in `.ComparableRSAKey` """ typ = 'RSA' @@ -120,21 +122,73 @@ class JWKRSA(JWK): :rtype: :class:`JWKRSA` """ - return cls(key=util.HashableRSAKey( - Crypto.PublicKey.RSA.importKey(string))) + try: + key = serialization.load_pem_public_key( + string, backend=default_backend()) + except ValueError: # ValueError: Could not unserialize key data. + key = serialization.load_pem_private_key( + string, password=None, backend=default_backend()) + return cls(key=util.ComparableRSAKey(key)) - def public(self): - return type(self)(key=self.key.publickey()) + def public_key(self): + return type(self)(key=self.key.public_key()) @classmethod def fields_from_json(cls, jobj): - return cls(key=util.HashableRSAKey( - Crypto.PublicKey.RSA.construct( - (cls._decode_param(jobj['n']), - cls._decode_param(jobj['e']))))) + # pylint: disable=invalid-name + n, e = (cls._decode_param(jobj[x]) for x in ('n', 'e')) + public_numbers = rsa.RSAPublicNumbers(e=e, n=n) + if 'd' not in jobj: # public key + key = public_numbers.public_key(default_backend()) + else: # private key + d = cls._decode_param(jobj['d']) + if ('p' in jobj or 'q' in jobj or 'dp' in jobj or + 'dq' in jobj or 'qi' in jobj or 'oth' in jobj): + # "If the producer includes any of the other private + # key parameters, then all of the others MUST be + # present, with the exception of "oth", which MUST + # only be present when more than two prime factors + # were used." + p, q, dp, dq, qi, = all_params = tuple( + jobj.get(x) for x in ('p', 'q', 'dp', 'dq', 'qi')) + if tuple(param for param in all_params if param is None): + raise errors.Error( + "Some private parameters are missing: {0}".format( + all_params)) + p, q, dp, dq, qi = tuple(cls._decode_param(x) for x in all_params) + + # TODO: check for oth + else: + p, q = rsa.rsa_recover_prime_factors(n, e, d) # cryptography>=0.8 + dp = rsa.rsa_crt_dmp1(d, p) + dq = rsa.rsa_crt_dmq1(d, q) + qi = rsa.rsa_crt_iqmp(p, q) + + key = rsa.RSAPrivateNumbers( + p, q, d, dp, dq, qi, public_numbers).private_key(default_backend()) + + return cls(key=util.ComparableRSAKey(key)) def fields_to_partial_json(self): - return { - 'n': self._encode_param(self.key.n), - 'e': self._encode_param(self.key.e), - } + # pylint: disable=protected-access + if isinstance(self.key._wrapped, rsa.RSAPublicKey): + numbers = self.key.public_numbers() + params = { + 'n': numbers.n, + 'e': numbers.e, + } + else: # rsa.RSAPrivateKey + private = self.key.private_numbers() + public = self.key.public_key().public_numbers() + params = { + 'n': public.n, + 'e': public.e, + 'd': private.d, + 'p': private.p, + 'q': private.q, + 'dp': private.dmp1, + 'dq': private.dmq1, + 'qi': private.iqmp, + } + return dict((key, self._encode_param(value)) + for key, value in params.iteritems()) diff --git a/acme/jose/jwk_test.py b/acme/jose/jwk_test.py index 39d595f94..f3745b251 100644 --- a/acme/jose/jwk_test.py +++ b/acme/jose/jwk_test.py @@ -3,16 +3,21 @@ import os import pkg_resources import unittest -from Crypto.PublicKey import RSA +from cryptography.hazmat.backends import default_backend +from cryptography.hazmat.primitives import serialization from acme.jose import errors from acme.jose import util -RSA256_KEY = util.HashableRSAKey(RSA.importKey(pkg_resources.resource_string( - __name__, os.path.join('testdata', 'rsa256_key.pem')))) -RSA512_KEY = util.HashableRSAKey(RSA.importKey(pkg_resources.resource_string( - __name__, os.path.join('testdata', 'rsa512_key.pem')))) +RSA256_KEY = util.ComparableRSAKey(serialization.load_pem_private_key( + pkg_resources.resource_string( + __name__, os.path.join('testdata', 'rsa256_key.pem')), + password=None, backend=default_backend())) +RSA512_KEY = util.ComparableRSAKey(serialization.load_pem_private_key( + pkg_resources.resource_string( + __name__, os.path.join('testdata', 'rsa512_key.pem')), + password=None, backend=default_backend())) class JWKOctTest(unittest.TestCase): @@ -38,8 +43,8 @@ class JWKOctTest(unittest.TestCase): from acme.jose.jwk import JWKOct self.assertEqual(self.jwk, JWKOct.load('foo')) - def test_public(self): - self.assertTrue(self.jwk.public() is self.jwk) + def test_public_key(self): + self.assertTrue(self.jwk.public_key() is self.jwk) class JWKRSATest(unittest.TestCase): @@ -47,20 +52,32 @@ class JWKRSATest(unittest.TestCase): def setUp(self): from acme.jose.jwk import JWKRSA - self.jwk256 = JWKRSA(key=RSA256_KEY.publickey()) - self.jwk256_private = JWKRSA(key=RSA256_KEY) + self.jwk256 = JWKRSA(key=RSA256_KEY.public_key()) self.jwk256json = { 'kty': 'RSA', 'e': 'AQAB', 'n': 'm2Fylv-Uz7trgTW8EBHP3FQSMeZs2GNQ6VRo1sIVJEk', } - self.jwk512 = JWKRSA(key=RSA512_KEY.publickey()) + self.jwk512 = JWKRSA(key=RSA512_KEY.public_key()) self.jwk512json = { 'kty': 'RSA', 'e': 'AQAB', 'n': 'rHVztFHtH92ucFJD_N_HW9AsdRsUuHUBBBDlHwNlRd3fp5' '80rv2-6QWE30cWgdmJS86ObRz6lUTor4R0T-3C5Q', } + self.private = JWKRSA(key=RSA256_KEY) + self.private_json_small = self.jwk256json.copy() + self.private_json_small['d'] = ( + 'lPQED_EPTV0UIBfNI3KP2d9Jlrc2mrMllmf946bu-CE') + self.private_json = self.jwk256json.copy() + self.private_json.update({ + 'd': 'lPQED_EPTV0UIBfNI3KP2d9Jlrc2mrMllmf946bu-CE', + 'p': 'zUVNZn4lLLBD1R6NE8TKNQ', + 'q': 'wcfKfc7kl5jfqXArCRSURQ', + 'dp': 'CWJFq43QvT5Bm5iN8n1okQ', + 'dq': 'bHh2u7etM8LKKCF2pY2UdQ', + 'qi': 'oi45cEkbVoJjAbnQpFY87Q', + }) def test_equals(self): self.assertEqual(self.jwk256, self.jwk256) @@ -73,22 +90,34 @@ class JWKRSATest(unittest.TestCase): def test_load(self): from acme.jose.jwk import JWKRSA self.assertEqual( - JWKRSA(key=util.HashableRSAKey(RSA256_KEY)), JWKRSA.load( + JWKRSA(key=RSA256_KEY), JWKRSA.load( pkg_resources.resource_string( __name__, os.path.join('testdata', 'rsa256_key.pem')))) - def test_public(self): - self.assertEqual(self.jwk256, self.jwk256_private.public()) + def test_public_key(self): + self.assertEqual(self.jwk256, self.private.public_key()) def test_to_partial_json(self): self.assertEqual(self.jwk256.to_partial_json(), self.jwk256json) self.assertEqual(self.jwk512.to_partial_json(), self.jwk512json) + self.assertEqual(self.private.to_partial_json(), self.private_json) def test_from_json(self): from acme.jose.jwk import JWK - self.assertEqual(self.jwk256, JWK.from_json(self.jwk256json)) - # TODO: fix schemata to allow RSA512 - #self.assertEqual(self.jwk512, JWK.from_json(self.jwk512json)) + self.assertEqual( + self.jwk256, JWK.from_json(self.jwk256json)) + self.assertEqual( + self.jwk512, JWK.from_json(self.jwk512json)) + self.assertEqual(self.private, JWK.from_json(self.private_json)) + + def test_from_json_private_small(self): + from acme.jose.jwk import JWK + self.assertEqual(self.private, JWK.from_json(self.private_json_small)) + + def test_from_json_missing_one_additional(self): + from acme.jose.jwk import JWK + del self.private_json['q'] + self.assertRaises(errors.Error, JWK.from_json, self.private_json) def test_from_json_hashable(self): from acme.jose.jwk import JWK diff --git a/acme/jose/jws.py b/acme/jose/jws.py index 3ba60d40c..a9e38ead7 100644 --- a/acme/jose/jws.py +++ b/acme/jose/jws.py @@ -203,7 +203,7 @@ class Signature(json_util.JSONObjectWithFields): header_params = kwargs header_params['alg'] = alg if include_jwk: - header_params['jwk'] = key.public() + header_params['jwk'] = key.public_key() assert set(header_params).issubset(cls.header_cls._fields) assert protect.issubset(cls.header_cls._fields) @@ -354,12 +354,12 @@ class CLI(object): if args.key is not None: assert args.kty is not None - key = args.kty.load(args.key.read()) + key = args.kty.load(args.key.read()).public_key() else: key = None sys.stdout.write(sig.payload) - return int(not sig.verify(key=key)) + return not sig.verify(key=key) @classmethod def _alg_type(cls, arg): diff --git a/acme/jose/jws_test.py b/acme/jose/jws_test.py index 6ecce63d2..19f45ae94 100644 --- a/acme/jose/jws_test.py +++ b/acme/jose/jws_test.py @@ -4,7 +4,8 @@ import os import pkg_resources import unittest -import Crypto.PublicKey.RSA +from cryptography.hazmat.backends import default_backend +from cryptography.hazmat.primitives import serialization import M2Crypto import mock @@ -18,8 +19,10 @@ from acme.jose import util CERT = util.ComparableX509(M2Crypto.X509.load_cert( pkg_resources.resource_filename( 'letsencrypt.tests', 'testdata/cert.pem'))) -RSA512_KEY = Crypto.PublicKey.RSA.importKey(pkg_resources.resource_string( - __name__, os.path.join('testdata', 'rsa512_key.pem'))) +RSA512_KEY = util.ComparableRSAKey(serialization.load_pem_private_key( + pkg_resources.resource_string( + __name__, os.path.join('testdata', 'rsa512_key.pem')), + password=None, backend=default_backend())) class MediaTypeTest(unittest.TestCase): @@ -107,7 +110,7 @@ class JWSTest(unittest.TestCase): def setUp(self): self.privkey = jwk.JWKRSA(key=RSA512_KEY) - self.pubkey = self.privkey.public() + self.pubkey = self.privkey.public_key() from acme.jose.jws import JWS self.unprotected = JWS.sign( diff --git a/acme/jose/util.py b/acme/jose/util.py index 2312055f7..8fa341fa2 100644 --- a/acme/jose/util.py +++ b/acme/jose/util.py @@ -1,6 +1,8 @@ """JOSE utilities.""" import collections +from cryptography.hazmat.primitives.asymmetric import rsa + class abstractclassmethod(classmethod): # pylint: disable=invalid-name,too-few-public-methods @@ -41,9 +43,14 @@ class ComparableX509(object): # pylint: disable=too-few-public-methods return self.as_der() == other.as_der() -class HashableRSAKey(object): # pylint: disable=too-few-public-methods - """Wrapper for `Crypto.PublicKey.RSA` objects that supports hashing.""" +class ComparableRSAKey(object): # pylint: disable=too-few-public-methods + """Wrapper for `cryptography` RSA keys. + Wraps around: + - `cryptography.hazmat.primitives.assymetric.RSAPrivateKey` + - `cryptography.hazmat.primitives.assymetric.RSAPublicKey` + + """ def __init__(self, wrapped): self._wrapped = wrapped @@ -51,14 +58,36 @@ class HashableRSAKey(object): # pylint: disable=too-few-public-methods return getattr(self._wrapped, name) def __eq__(self, other): - return self._wrapped == other + # pylint: disable=protected-access + if (not isinstance(other, self.__class__) or + self._wrapped.__class__ is not other._wrapped.__class__): + return False + # RSA*KeyWithSerialization requires cryptography>=0.8 + if isinstance(self._wrapped, rsa.RSAPrivateKeyWithSerialization): + return self.private_numbers() == other.private_numbers() + elif isinstance(self._wrapped, rsa.RSAPublicKeyWithSerialization): + return self.public_numbers() == other.public_numbers() + else: + return False # we shouldn't reach here... + def __hash__(self): - return hash((type(self), self.exportKey(format='DER'))) + # public_numbers() hasn't got stable hash! + if isinstance(self._wrapped, rsa.RSAPrivateKeyWithSerialization): + priv = self.private_numbers() + pub = priv.public_numbers + return hash((type(self), priv.p, priv.q, priv.dmp1, + priv.dmq1, priv.iqmp, pub.n, pub.e)) + elif isinstance(self._wrapped, rsa.RSAPublicKeyWithSerialization): + pub = self.public_numbers() + return hash((type(self), pub.n, pub.e)) - def publickey(self): + def __repr__(self): + return '<{0}({1!r})>'.format(self.__class__.__name__, self._wrapped) + + def public_key(self): """Get wrapped public key.""" - return type(self)(self._wrapped.publickey()) + return type(self)(self._wrapped.public_key()) class ImmutableMap(collections.Mapping, collections.Hashable): diff --git a/acme/jose/util_test.py b/acme/jose/util_test.py index b5592c57e..80d8b8999 100644 --- a/acme/jose/util_test.py +++ b/acme/jose/util_test.py @@ -4,32 +4,52 @@ import os import pkg_resources import unittest -import Crypto.PublicKey.RSA +from cryptography.hazmat.backends import default_backend +from cryptography.hazmat.primitives import serialization -class HashableRSAKeyTest(unittest.TestCase): - """Tests for acme.jose.util.HashableRSAKey.""" +class ComparableRSAKeyTest(unittest.TestCase): + """Tests for acme.jose.util.ComparableRSAKey.""" def setUp(self): - from acme.jose.util import HashableRSAKey - self.key = HashableRSAKey(Crypto.PublicKey.RSA.importKey( - pkg_resources.resource_string( - __name__, os.path.join('testdata', 'rsa256_key.pem')))) - self.key_same = HashableRSAKey(Crypto.PublicKey.RSA.importKey( - pkg_resources.resource_string( - __name__, os.path.join('testdata', 'rsa256_key.pem')))) + from acme.jose.util import ComparableRSAKey + backend = default_backend() + def load_key(): # pylint: disable=missing-docstring + return ComparableRSAKey(serialization.load_pem_private_key( + pkg_resources.resource_string( + __name__, os.path.join('testdata', 'rsa256_key.pem')), + password=None, backend=backend)) + self.key = load_key() + self.key_same = load_key() + + def test_getattr_proxy(self): + self.assertEqual(256, self.key.key_size) def test_eq(self): - # if __eq__ is not defined, then two HashableRSAKeys with same - # _wrapped do not equate self.assertEqual(self.key, self.key_same) + def test_not_eq_different_types(self): + self.assertFalse(self.key.__eq__(5)) + + def test_not_eq_not_wrapped(self): + # pylint: disable=protected-access + self.assertFalse(self.key.__eq__(self.key_same._wrapped)) + + def test_not_eq_no_serialization(self): + from acme.jose.util import ComparableRSAKey + self.assertFalse(ComparableRSAKey(5).__eq__(ComparableRSAKey(5))) + def test_hash(self): self.assertTrue(isinstance(hash(self.key), int)) + self.assertEqual(hash(self.key), hash(self.key_same)) - def test_publickey(self): - from acme.jose.util import HashableRSAKey - self.assertTrue(isinstance(self.key.publickey(), HashableRSAKey)) + def test_repr(self): + self.assertTrue(repr(self.key).startswith( + '=0.5 +key = jose.JWKRSA(key=jose.ComparableRSAKey(rsa.generate_private_key( + public_exponent=65537, + key_size=2048, + backend=default_backend()))) acme = client.Client(NEW_REG_URL, key) regr = acme.register(contact=()) diff --git a/letsencrypt/proof_of_possession.py b/letsencrypt/proof_of_possession.py index 9cec341de..a70fff697 100644 --- a/letsencrypt/proof_of_possession.py +++ b/letsencrypt/proof_of_possession.py @@ -1,5 +1,6 @@ """Proof of Possession Identifier Validation Challenge.""" import M2Crypto +import logging import os import zope.component @@ -11,6 +12,9 @@ from letsencrypt import interfaces from letsencrypt.display import util as display_util +logger = logging.getLogger(__name__) + + class ProofOfPossession(object): # pylint: disable=too-few-public-methods """Proof of Possession Identifier Validation Challenge. @@ -39,19 +43,22 @@ class ProofOfPossession(object): # pylint: disable=too-few-public-methods return None for cert, key, _ in self.installer.get_all_certs_keys(): - der_cert_key = M2Crypto.X509.load_cert(cert).get_pubkey().as_der() + pkey = M2Crypto.X509.load_cert(cert).get_pubkey() try: - cert_key = achall.alg.kty.load(der_cert_key) - # If JWKES.load raises other exceptions, they should be caught here - except (IndexError, ValueError, TypeError): + rsa_pkey = pkey.get_rsa() + except ValueError: + logger.warn("Only RSA supported at this time") continue + pem_cert_key = rsa_pkey.as_pem() + cert_key = achall.alg.kty.load(pem_cert_key) + # TODO: If JWKES.load raises other exceptions, they should be caught here if cert_key == achall.hints.jwk: return self._gen_response(achall, key) # Is there are different prompt we should give the user? code, key = zope.component.getUtility( interfaces.IDisplay).input( - "Path to private key for identifier: %s " % achall.domain) + "QPath to private key for identifier: %s " % achall.domain) if code != display_util.CANCEL: return self._gen_response(achall, key) diff --git a/letsencrypt/tests/acme_util.py b/letsencrypt/tests/acme_util.py index 7ac05c1fa..b0939dc68 100644 --- a/letsencrypt/tests/acme_util.py +++ b/letsencrypt/tests/acme_util.py @@ -4,16 +4,18 @@ import itertools import os import pkg_resources -import Crypto.PublicKey.RSA +from cryptography.hazmat.backends import default_backend +from cryptography.hazmat.primitives import serialization from acme import challenges from acme import jose from acme import messages -KEY = jose.HashableRSAKey(Crypto.PublicKey.RSA.importKey( +KEY = jose.ComparableRSAKey(serialization.load_pem_private_key( pkg_resources.resource_string( - "acme.jose", os.path.join("testdata", "rsa512_key.pem")))) + __name__, os.path.join('testdata', 'rsa512_key.pem')), + password=None, backend=default_backend())) # Challenges SIMPLE_HTTP = challenges.SimpleHTTP( @@ -30,7 +32,7 @@ RECOVERY_TOKEN = challenges.RecoveryToken() POP = challenges.ProofOfPossession( alg="RS256", nonce="xD\xf9\xb9\xdbU\xed\xaa\x17\xf1y|\x81\x88\x99 ", hints=challenges.ProofOfPossession.Hints( - jwk=jose.JWKRSA(key=KEY.publickey()), + jwk=jose.JWKRSA(key=KEY.public_key()), cert_fingerprints=( "93416768eb85e33adc4277f4c9acd63e7418fcfe", "16d95b7b63f1972b980b14c20291f3c0d1855d95", diff --git a/letsencrypt/tests/proof_of_possession_test.py b/letsencrypt/tests/proof_of_possession_test.py index 415e4caed..68f740ebd 100644 --- a/letsencrypt/tests/proof_of_possession_test.py +++ b/letsencrypt/tests/proof_of_possession_test.py @@ -1,9 +1,10 @@ """Tests for letsencrypt.proof_of_possession.""" -import Crypto.PublicKey.RSA import os import pkg_resources import unittest +from cryptography.hazmat.backends import default_backend +from cryptography.hazmat.primitives import serialization import mock from acme import challenges @@ -28,8 +29,10 @@ CERT3_PATH = pkg_resources.resource_filename( BASE_PACKAGE, os.path.join("testdata", "matching_cert.pem")) CERT3_KEY_PATH = pkg_resources.resource_filename( BASE_PACKAGE, os.path.join("testdata", "rsa512_key.pem")) -CERT3_KEY = Crypto.PublicKey.RSA.importKey(pkg_resources.resource_string( - BASE_PACKAGE, os.path.join('testdata', 'rsa512_key.pem'))).publickey() +with open(CERT3_KEY_PATH) as cert3_file: + CERT3_KEY = jose.ComparableRSAKey(serialization.load_pem_private_key( + cert3_file.read(), password=None, + backend=default_backend())).public_key() class ProofOfPossessionTest(unittest.TestCase): @@ -55,7 +58,7 @@ class ProofOfPossessionTest(unittest.TestCase): def test_perform_bad_challenge(self): hints = challenges.ProofOfPossession.Hints( - jwk=jose.jwk.JWKOct(key=CERT3_KEY), cert_fingerprints=(), + jwk=jose.jwk.JWKOct(key="foo"), cert_fingerprints=(), certs=(), serial_numbers=(), subject_key_identifiers=(), issuers=(), authorized_for=()) chall = challenges.ProofOfPossession( diff --git a/setup.py b/setup.py index ca2746113..8b58a3e80 100644 --- a/setup.py +++ b/setup.py @@ -35,9 +35,11 @@ changes = read_file(os.path.join(here, 'CHANGES.rst')) # maintainers. and will make the future migration a lot easier. acme_install_requires = [ 'argparse', + # load_pem_private/public_key (>=0.6) + # rsa_recover_prime_factors (>=0.8) + 'cryptography>=0.8', #'letsencrypt' # TODO: uses testdata vectors 'mock', - 'pycrypto', 'pyrfc3339', 'ndg-httpsclient', # urllib3 InsecurePlatformWarning (#304) 'pyasn1', # urllib3 InsecurePlatformWarning (#304) @@ -83,6 +85,7 @@ letsencrypt_nginx_install_requires = [ install_requires = [ 'argparse', + 'cryptography>=0.8', 'ConfigArgParse', 'configobj', 'mock', From 25f1e45d947d895b4d629549b513b37e033c5ff9 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Mon, 6 Jul 2015 07:55:29 +0000 Subject: [PATCH 37/50] Remove acme.util docs --- docs/pkgs/acme/index.rst | 7 ------- 1 file changed, 7 deletions(-) diff --git a/docs/pkgs/acme/index.rst b/docs/pkgs/acme/index.rst index 2df2615a5..04194c353 100644 --- a/docs/pkgs/acme/index.rst +++ b/docs/pkgs/acme/index.rst @@ -47,10 +47,3 @@ Errors .. automodule:: acme.errors :members: - - -Utilities ---------- - -.. automodule:: acme.util - :members: From 2c6ef0feef60baebaf90901d2e7280f3d7101c0c Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Mon, 6 Jul 2015 09:19:00 +0000 Subject: [PATCH 38/50] Update hacking docs (venv/bin/activate, ./tox-cover.sh, integration, ipdb). --- docs/contributing.rst | 86 +++++++++++++++++++++++++++++++++---------- 1 file changed, 67 insertions(+), 19 deletions(-) diff --git a/docs/contributing.rst b/docs/contributing.rst index 05a6875fe..b415390cf 100644 --- a/docs/contributing.rst +++ b/docs/contributing.rst @@ -7,15 +7,26 @@ Contributing Hacking ======= -In order to start hacking, you will first have to create a development -environment. Start by :doc:`installing dependencies and setting up -Let's Encrypt `. +Start by :doc:`installing dependencies and setting up Let's Encrypt +`. -Now you can install the development packages: +When you're done activate the virtualenv: .. code-block:: shell - ./venv/bin/pip install -r requirements.txt -e .[dev,docs,testing] + source ./venv/bin/activate + +This step should prepend you prompt with ``(venv)`` and save you from +typing ``./venv/bin/...``. It is also required to run some of the +`testing`_ tools. Virtualenv can be disabled at any time by typing +``deactivate``. More information can be found in `virtualenv +documentation`_. + +Install the development packages: + +.. code-block:: shell + + pip install -r requirements.txt -e .[dev,docs,testing] .. note:: `-e` (short for `--editable`) turns on *editable mode* in which any source code changes in the current working @@ -25,23 +36,61 @@ Now you can install the development packages: This is roughly equivalent to `python setup.py develop`. For more info see `man pip`. -The code base, including your pull requests, **must** have 100% test -statement coverage **and** be compliant with the :ref:`coding style -`. +The code base, including your pull requests, **must** have 100% unit +test coverage, pass our `integration`_ tests **and** be compliant with +the :ref:`coding style `. + +.. _`virtualenv documentation`: https://virtualenv.pypa.io + + +Testing +------- The following tools are there to help you: -- ``./venv/bin/tox`` starts a full set of tests. Please make sure you - run it before submitting a new pull request. +- ``tox`` starts a full set of tests. Please make sure you run it + before submitting a new pull request. -- ``./venv/bin/tox -e cover`` checks the test coverage only. +- ``tox -e cover`` checks the test coverage only. Calling the + ``./tox-cover.sh`` script directly might be a bit quicker, though. -- ``./venv/bin/tox -e lint`` checks the style of the whole project, - while ``./venv/bin/pylint --rcfile=.pylintrc file`` will check a - single ``file`` only. +- ``tox -e lint`` checks the style of the whole project, while + ``pylint --rcfile=.pylintrc path`` will check a single file or + specific directory only. -.. _installing dependencies and setting up Let's Encrypt: - https://letsencrypt.readthedocs.org/en/latest/using.html +- For debugging, we recommend ``pip install ipdb`` and putting + ``import ipdb; ipdb.set_trace()`` statement inside the source + code. Alternatively, you can use Python'd standard library `pdb`, + but you won't get TAB completion... + + +Integration +~~~~~~~~~~~ + +First, install `Go`_ 1.4 and start Boulder_, an ACME CA server:: + + ./tests/boulder-start.sh + +The script will download, compile and run the executable; please be +patient - it will take some time... Once its ready, you will see +``Server running, listening on 127.0.0.1:4000...``. You may now run +(in a separate terminal):: + + ./tests/boulder-integration.sh && echo OK || echo FAIL + +If you would like to test `lesencrypt_nginx` plugin (highly +encouraged) make sure to install prerequisites as listed in +``tests/integration/nginx.sh``: + +.. include:: ../tests/integration/nginx.sh + :start-line: 1 + :end-line: 2 + :code: shell + +and rerun the integration tests suite. + +.. _Boulder: https://github.com/letsencrypt/boulder +.. _Go: https://golang.org Vagrant @@ -185,7 +234,7 @@ Please: """ return arg -4. Remember to use ``./venv/bin/pylint``. +4. Remember to use ``pylint``. .. _Google Python Style Guide: https://google-styleguide.googlecode.com/svn/trunk/pyguide.html @@ -202,8 +251,7 @@ commands: .. code-block:: shell - cd docs - make clean html SPHINXBUILD=../venv/bin/sphinx-build + make -C docs clean html This should generate documentation in the ``docs/_build/html`` directory. From 9197fa6b5c2114e0f3e141843b197626be10a00d Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Wed, 13 May 2015 07:59:13 +0000 Subject: [PATCH 39/50] acme: M2Crypto -> pyOpenSSL --- acme/challenges_test.py | 9 +++++---- acme/client.py | 18 ++++++++--------- acme/client_test.py | 10 ++++++---- acme/jose/json_util.py | 20 ++++++++++--------- acme/jose/json_util_test.py | 14 ++++++++------ acme/jose/jws.py | 10 ++++++---- acme/jose/jws_test.py | 13 ++++++++----- acme/jose/util.py | 25 ++++++++++++++++++++---- acme/jose/util_test.py | 32 +++++++++++++++++++++++++++++++ acme/messages.py | 6 +++--- acme/messages_test.py | 20 +++++++++---------- examples/acme_client.py | 8 ++++---- letsencrypt/client.py | 18 ++++++++++------- letsencrypt/renewer.py | 8 ++++++-- letsencrypt/tests/client_test.py | 6 +++--- letsencrypt/tests/renewer_test.py | 12 ++++++++---- setup.py | 2 +- 17 files changed, 151 insertions(+), 80 deletions(-) diff --git a/acme/challenges_test.py b/acme/challenges_test.py index b39e40280..9d03e01cd 100644 --- a/acme/challenges_test.py +++ b/acme/challenges_test.py @@ -5,8 +5,8 @@ import unittest from cryptography.hazmat.backends import default_backend from cryptography.hazmat.primitives import serialization -import M2Crypto import mock +import OpenSSL import requests import urlparse @@ -14,8 +14,8 @@ from acme import jose from acme import other -CERT = jose.ComparableX509(M2Crypto.X509.load_cert( - pkg_resources.resource_filename( +CERT = jose.ComparableX509(OpenSSL.crypto.load_certificate( + OpenSSL.crypto.FILETYPE_PEM, pkg_resources.resource_string( 'letsencrypt.tests', os.path.join('testdata', 'cert.pem')))) KEY = jose.ComparableRSAKey(serialization.load_pem_private_key( pkg_resources.resource_string( @@ -370,7 +370,8 @@ class ProofOfPossessionHintsTest(unittest.TestCase): self.jmsg_to = { 'jwk': jwk, 'certFingerprints': cert_fingerprints, - 'certs': (jose.b64encode(CERT.as_der()),), + 'certs': (jose.b64encode(OpenSSL.crypto.dump_certificate( + OpenSSL.crypto.FILETYPE_ASN1, CERT)),), 'subjectKeyIdentifiers': subject_key_identifiers, 'serialNumbers': serial_numbers, 'issuers': issuers, diff --git a/acme/client.py b/acme/client.py index fbdb4543d..67d4f1a69 100644 --- a/acme/client.py +++ b/acme/client.py @@ -5,7 +5,7 @@ import httplib import logging import time -import M2Crypto +import OpenSSL import requests import werkzeug @@ -256,7 +256,7 @@ class Client(object): # pylint: disable=too-many-instance-attributes """Request issuance. :param csr: CSR - :type csr: `M2Crypto.X509.Request` wrapped in `.ComparableX509` + :type csr: `OpenSSL.crypto.X509Req` wrapped in `.ComparableX509` :param authzrs: `list` of `.AuthorizationResource` @@ -287,8 +287,8 @@ class Client(object): # pylint: disable=too-many-instance-attributes return messages.CertificateResource( uri=uri, authzrs=authzrs, cert_chain_uri=cert_chain_uri, - body=jose.ComparableX509( - M2Crypto.X509.load_cert_der_string(response.content))) + body=jose.ComparableX509(OpenSSL.crypto.load_certificate( + OpenSSL.crypto.FILETYPE_ASN1, response.content))) def poll_and_request_issuance(self, csr, authzrs, mintime=5): """Poll and request issuance. @@ -300,7 +300,7 @@ class Client(object): # pylint: disable=too-many-instance-attributes .. todo:: add `max_attempts` or `timeout` :param csr: CSR. - :type csr: `M2Crypto.X509.Request` wrapped in `.ComparableX509` + :type csr: `OpenSSL.crypto.X509Req` wrapped in `.ComparableX509` :param authzrs: `list` of `.AuthorizationResource` @@ -359,8 +359,8 @@ class Client(object): # pylint: disable=too-many-instance-attributes content_type = self.DER_CONTENT_TYPE # TODO: make it a param response = self.net.get(uri, headers={'Accept': content_type}, content_type=content_type) - return response, jose.ComparableX509( - M2Crypto.X509.load_cert_der_string(response.content)) + return response, jose.ComparableX509(OpenSSL.crypto.load_certificate( + OpenSSL.crypto.FILETYPE_ASN1, response.content)) def check_cert(self, certr): """Check for new cert. @@ -403,7 +403,7 @@ class Client(object): # pylint: disable=too-many-instance-attributes :type certr: `.CertificateResource` :returns: Certificate chain, or `None` if no "up" Link was provided. - :rtype: `M2Crypto.X509.X509` wrapped in `.ComparableX509` + :rtype: `OpenSSL.crypto.X509` wrapped in `.ComparableX509` """ if certr.cert_chain_uri is not None: @@ -414,7 +414,7 @@ class Client(object): # pylint: disable=too-many-instance-attributes def revoke(self, cert): """Revoke certificate. - :param .ComparableX509 cert: `M2Crypto.X509.X509` wrapped in + :param .ComparableX509 cert: `OpenSSL.crypto.X509` wrapped in `.ComparableX509` :raises .ClientError: If revocation is unsuccessful. diff --git a/acme/client_test.py b/acme/client_test.py index 7ae14d0be..b25a1866c 100644 --- a/acme/client_test.py +++ b/acme/client_test.py @@ -16,6 +16,8 @@ from acme import messages from acme import messages_test +CERT_DER = pkg_resources.resource_string( + 'acme.jose', os.path.join('testdata', 'cert.der')) KEY = jose.JWKRSA.load(pkg_resources.resource_string( 'acme.jose', os.path.join('testdata', 'rsa512_key.pem'))) KEY2 = jose.JWKRSA.load(pkg_resources.resource_string( @@ -204,7 +206,7 @@ class ClientTest(unittest.TestCase): errors.UnexpectedUpdate, self.client.poll, self.authzr) def test_request_issuance(self): - self.response.content = messages_test.CERT.as_der() + self.response.content = CERT_DER self.response.headers['Location'] = self.certr.uri self.response.links['up'] = {'url': self.certr.cert_chain_uri} self.assertEqual(self.certr, self.client.request_issuance( @@ -212,7 +214,7 @@ class ClientTest(unittest.TestCase): # TODO: check POST args def test_request_issuance_missing_up(self): - self.response.content = messages_test.CERT.as_der() + self.response.content = CERT_DER self.response.headers['Location'] = self.certr.uri self.assertEqual( self.certr.update(cert_chain_uri=None), @@ -306,7 +308,7 @@ class ClientTest(unittest.TestCase): def test_check_cert(self): self.response.headers['Location'] = self.certr.uri - self.response.content = messages_test.CERT.as_der() + self.response.content = CERT_DER self.assertEqual(self.certr.update(body=messages_test.CERT), self.client.check_cert(self.certr)) @@ -316,7 +318,7 @@ class ClientTest(unittest.TestCase): errors.UnexpectedUpdate, self.client.check_cert, self.certr) def test_check_cert_missing_location(self): - self.response.content = messages_test.CERT.as_der() + self.response.content = CERT_DER self.assertRaises( errors.ClientError, self.client.check_cert, self.certr) diff --git a/acme/jose/json_util.py b/acme/jose/json_util.py index 70ac5a549..fe3831296 100644 --- a/acme/jose/json_util.py +++ b/acme/jose/json_util.py @@ -10,7 +10,7 @@ import abc import binascii import logging -import M2Crypto +import OpenSSL from acme.jose import b64 from acme.jose import errors @@ -321,26 +321,28 @@ def encode_cert(cert): :type cert: :class:`acme.jose.util.ComparableX509` """ - return b64.b64encode(cert.as_der()) + return b64.b64encode(OpenSSL.crypto.dump_certificate( + OpenSSL.crypto.FILETYPE_ASN1, cert)) def decode_cert(b64der): """Decode JOSE Base-64 DER-encoded certificate.""" try: - return util.ComparableX509(M2Crypto.X509.load_cert_der_string( - decode_b64jose(b64der))) - except M2Crypto.X509.X509Error as error: + return util.ComparableX509(OpenSSL.crypto.load_certificate( + OpenSSL.crypto.FILETYPE_ASN1, decode_b64jose(b64der))) + except OpenSSL.crypto.Error as error: raise errors.DeserializationError(error) def encode_csr(csr): """Encode CSR as JOSE Base-64 DER.""" - return encode_cert(csr) + return b64.b64encode(OpenSSL.crypto.dump_certificate_request( + OpenSSL.crypto.FILETYPE_ASN1, csr)) def decode_csr(b64der): """Decode JOSE Base-64 DER-encoded CSR.""" try: - return util.ComparableX509(M2Crypto.X509.load_request_der_string( - decode_b64jose(b64der))) - except M2Crypto.X509.X509Error as error: + return util.ComparableX509(OpenSSL.crypto.load_certificate_request( + OpenSSL.crypto.FILETYPE_ASN1, decode_b64jose(b64der))) + except OpenSSL.crypto.Error as error: raise errors.DeserializationError(error) diff --git a/acme/jose/json_util_test.py b/acme/jose/json_util_test.py index 242e37589..9e622bea6 100644 --- a/acme/jose/json_util_test.py +++ b/acme/jose/json_util_test.py @@ -4,18 +4,20 @@ import os import pkg_resources import unittest -import M2Crypto import mock +import OpenSSL from acme.jose import errors from acme.jose import interfaces from acme.jose import util -CERT = M2Crypto.X509.load_cert(pkg_resources.resource_filename( - 'letsencrypt.tests', os.path.join('testdata', 'cert.pem'))) -CSR = M2Crypto.X509.load_request(pkg_resources.resource_filename( - 'letsencrypt.tests', os.path.join('testdata', 'csr.pem'))) +CERT = OpenSSL.crypto.load_certificate( + OpenSSL.crypto.FILETYPE_PEM, pkg_resources.resource_string( + 'letsencrypt.tests', os.path.join('testdata', 'cert.pem'))) +CSR = OpenSSL.crypto.load_certificate_request( + OpenSSL.crypto.FILETYPE_PEM, pkg_resources.resource_string( + 'letsencrypt.tests', os.path.join('testdata', 'csr.pem'))) class FieldTest(unittest.TestCase): @@ -280,7 +282,7 @@ class DeEncodersTest(unittest.TestCase): def test_encode_csr(self): from acme.jose.json_util import encode_csr - self.assertEqual(self.b64_cert, encode_csr(CERT)) + self.assertEqual(self.b64_csr, encode_csr(CSR)) def test_decode_csr(self): from acme.jose.json_util import decode_csr diff --git a/acme/jose/jws.py b/acme/jose/jws.py index a9e38ead7..6d1a5db2b 100644 --- a/acme/jose/jws.py +++ b/acme/jose/jws.py @@ -3,7 +3,7 @@ import argparse import base64 import sys -import M2Crypto +import OpenSSL from acme.jose import b64 from acme.jose import errors @@ -122,14 +122,16 @@ class Header(json_util.JSONObjectWithFields): @x5c.encoder def x5c(value): # pylint: disable=missing-docstring,no-self-argument - return [base64.b64encode(cert.as_der()) for cert in value] + return [base64.b64encode(OpenSSL.crypto.dump_certificate( + OpenSSL.crypto.FILETYPE_ASN1, cert)) for cert in value] @x5c.decoder def x5c(value): # pylint: disable=missing-docstring,no-self-argument try: - return tuple(util.ComparableX509(M2Crypto.X509.load_cert_der_string( + return tuple(util.ComparableX509(OpenSSL.crypto.load_certificate( + OpenSSL.crypto.FILETYPE_ASN1, base64.b64decode(cert))) for cert in value) - except M2Crypto.X509.X509Error as error: + except OpenSSL.crypto.Error as error: raise errors.DeserializationError(error) diff --git a/acme/jose/jws_test.py b/acme/jose/jws_test.py index 19f45ae94..1520f149e 100644 --- a/acme/jose/jws_test.py +++ b/acme/jose/jws_test.py @@ -6,8 +6,8 @@ import unittest from cryptography.hazmat.backends import default_backend from cryptography.hazmat.primitives import serialization -import M2Crypto import mock +import OpenSSL from acme.jose import b64 from acme.jose import errors @@ -16,8 +16,8 @@ from acme.jose import jwk from acme.jose import util -CERT = util.ComparableX509(M2Crypto.X509.load_cert( - pkg_resources.resource_filename( +CERT = util.ComparableX509(OpenSSL.crypto.load_certificate( + OpenSSL.crypto.FILETYPE_PEM, pkg_resources.resource_string( 'letsencrypt.tests', 'testdata/cert.pem'))) RSA512_KEY = util.ComparableRSAKey(serialization.load_pem_private_key( pkg_resources.resource_string( @@ -76,10 +76,13 @@ class HeaderTest(unittest.TestCase): from acme.jose.jws import Header header = Header(x5c=(CERT, CERT)) jobj = header.to_partial_json() - cert_b64 = base64.b64encode(CERT.as_der()) + cert_b64 = base64.b64encode(OpenSSL.crypto.dump_certificate( + OpenSSL.crypto.FILETYPE_ASN1, CERT)) self.assertEqual(jobj, {'x5c': [cert_b64, cert_b64]}) self.assertEqual(header, Header.from_json(jobj)) - jobj['x5c'][0] = base64.b64encode('xxx' + CERT.as_der()) + jobj['x5c'][0] = base64.b64encode( + 'xxx' + OpenSSL.crypto.dump_certificate( + OpenSSL.crypto.FILETYPE_ASN1, CERT)) self.assertRaises(errors.DeserializationError, Header.from_json, jobj) def test_find_key(self): diff --git a/acme/jose/util.py b/acme/jose/util.py index 8fa341fa2..a9b1382be 100644 --- a/acme/jose/util.py +++ b/acme/jose/util.py @@ -2,6 +2,7 @@ import collections from cryptography.hazmat.primitives.asymmetric import rsa +import OpenSSL class abstractclassmethod(classmethod): @@ -25,12 +26,12 @@ class abstractclassmethod(classmethod): class ComparableX509(object): # pylint: disable=too-few-public-methods - """Wrapper for M2Crypto.X509.* objects that supports __eq__. + """Wrapper for OpenSSL.crypto.X509** objects that supports __eq__. Wraps around: - - :class:`M2Crypto.X509.X509` - - :class:`M2Crypto.X509.Request` + - :class:`OpenSSL.crypto.X509` + - :class:`OpenSSL.crypto.X509Req` """ def __init__(self, wrapped): @@ -40,7 +41,23 @@ class ComparableX509(object): # pylint: disable=too-few-public-methods return getattr(self._wrapped, name) def __eq__(self, other): - return self.as_der() == other.as_der() + filetype = OpenSSL.crypto.FILETYPE_ASN1 + def as_der(obj): + # pylint: disable=missing-docstring,protected-access + if isinstance(obj, type(self)): + obj = obj._wrapped + if isinstance(obj, OpenSSL.crypto.X509): + func = OpenSSL.crypto.dump_certificate + elif isinstance(obj, OpenSSL.crypto.X509Req): + func = OpenSSL.crypto.dump_certificate_request + else: + raise TypeError( + "Equality for {0} not provided".format(obj.__class__)) + return func(filetype, obj) + return as_der(self) == as_der(other) + + def __repr__(self): + return '<{0}({1!r})>'.format(self.__class__.__name__, self._wrapped) class ComparableRSAKey(object): # pylint: disable=too-few-public-methods diff --git a/acme/jose/util_test.py b/acme/jose/util_test.py index 80d8b8999..95b5c6717 100644 --- a/acme/jose/util_test.py +++ b/acme/jose/util_test.py @@ -6,6 +6,38 @@ import unittest from cryptography.hazmat.backends import default_backend from cryptography.hazmat.primitives import serialization +import OpenSSL + + +class ComparableX509Test(unittest.TestCase): + """Tests for acme.jose.util.ComparableX509.""" + + def setUp(self): + from acme.jose.util import ComparableX509 + def load_cert(): # pylint: disable=missing-docstring + return ComparableX509(OpenSSL.crypto.load_certificate_request( + OpenSSL.crypto.FILETYPE_ASN1, pkg_resources.resource_string( + __name__, os.path.join('testdata', 'csr.der')))) + self.cert = load_cert() + self.cert_same = load_cert() + self.cert2 = ComparableX509(OpenSSL.crypto.load_certificate_request( + OpenSSL.crypto.FILETYPE_PEM, pkg_resources.resource_string( + 'letsencrypt.tests', os.path.join('testdata', 'csr-san.pem')))) + + def test_eq(self): + self.assertEqual(self.cert, self.cert_same) + + def test_not_eq(self): + self.assertNotEqual(self.cert, self.cert2) + + def test_eq_wrong_types(self): + from acme.jose.util import ComparableX509 + self.assertRaises( + TypeError, ComparableX509(5).__eq__, ComparableX509(5)) + + def test_repr(self): + self.assertTrue(repr(self.cert).startswith( + ' Date: Sun, 5 Jul 2015 20:34:45 +0000 Subject: [PATCH 40/50] Drop PyCrypto. --- letsencrypt/crypto_util.py | 9 +++---- .../plugins/standalone/authenticator.py | 2 -- .../standalone/tests/authenticator_test.py | 11 +++----- letsencrypt/revoker.py | 26 +++++++++++-------- letsencrypt/tests/revoker_test.py | 20 +++++++++----- setup.py | 2 -- 6 files changed, 35 insertions(+), 35 deletions(-) diff --git a/letsencrypt/crypto_util.py b/letsencrypt/crypto_util.py index 82b1b4867..3abe2d130 100644 --- a/letsencrypt/crypto_util.py +++ b/letsencrypt/crypto_util.py @@ -8,10 +8,6 @@ import logging import os import time -import Crypto.Hash.SHA256 -import Crypto.PublicKey.RSA -import Crypto.Signature.PKCS1_v1_5 - import M2Crypto import OpenSSL @@ -169,7 +165,10 @@ def make_key(bits): :rtype: str """ - return Crypto.PublicKey.RSA.generate(bits).exportKey(format="PEM") + assert bits >= 1024 # XXX + key = OpenSSL.crypto.PKey() + key.generate_key(OpenSSL.crypto.TYPE_RSA, bits) + return OpenSSL.crypto.dump_privatekey(OpenSSL.crypto.FILETYPE_PEM, key) def valid_privkey(privkey): diff --git a/letsencrypt/plugins/standalone/authenticator.py b/letsencrypt/plugins/standalone/authenticator.py index 971f90266..a90c02281 100644 --- a/letsencrypt/plugins/standalone/authenticator.py +++ b/letsencrypt/plugins/standalone/authenticator.py @@ -6,7 +6,6 @@ import socket import sys import time -import Crypto.Random import OpenSSL.crypto import OpenSSL.SSL import zope.component @@ -267,7 +266,6 @@ class StandaloneAuthenticator(common.Plugin): sys.stdout.flush() fork_result = os.fork() - Crypto.Random.atfork() if fork_result: # PARENT process (still the Let's Encrypt client process) self.child_pid = fork_result diff --git a/letsencrypt/plugins/standalone/tests/authenticator_test.py b/letsencrypt/plugins/standalone/tests/authenticator_test.py index 1794ff65c..b61482990 100644 --- a/letsencrypt/plugins/standalone/tests/authenticator_test.py +++ b/letsencrypt/plugins/standalone/tests/authenticator_test.py @@ -374,10 +374,8 @@ class StartListenerTest(unittest.TestCase): StandaloneAuthenticator self.authenticator = StandaloneAuthenticator(config=CONFIG, name=None) - @mock.patch("letsencrypt.plugins.standalone.authenticator." - "Crypto.Random.atfork") @mock.patch("letsencrypt.plugins.standalone.authenticator.os.fork") - def test_start_listener_fork_parent(self, mock_fork, mock_atfork): + def test_start_listener_fork_parent(self, mock_fork): self.authenticator.do_parent_process = mock.Mock() self.authenticator.do_parent_process.return_value = True mock_fork.return_value = 22222 @@ -387,12 +385,9 @@ class StartListenerTest(unittest.TestCase): self.assertTrue(result) self.assertEqual(self.authenticator.child_pid, 22222) self.authenticator.do_parent_process.assert_called_once_with(1717) - mock_atfork.assert_called_once_with() - @mock.patch("letsencrypt.plugins.standalone.authenticator." - "Crypto.Random.atfork") @mock.patch("letsencrypt.plugins.standalone.authenticator.os.fork") - def test_start_listener_fork_child(self, mock_fork, mock_atfork): + def test_start_listener_fork_child(self, mock_fork): self.authenticator.do_parent_process = mock.Mock() self.authenticator.do_child_process = mock.Mock() mock_fork.return_value = 0 @@ -400,7 +395,7 @@ class StartListenerTest(unittest.TestCase): self.assertEqual(self.authenticator.child_pid, os.getpid()) self.authenticator.do_child_process.assert_called_once_with( 1717, "key") - mock_atfork.assert_called_once_with() + class DoParentProcessTest(unittest.TestCase): """Tests for do_parent_process() method.""" diff --git a/letsencrypt/revoker.py b/letsencrypt/revoker.py index 9faf9339c..d8bec5fff 100644 --- a/letsencrypt/revoker.py +++ b/letsencrypt/revoker.py @@ -13,8 +13,8 @@ import os import shutil import tempfile -import Crypto.PublicKey.RSA import M2Crypto +import OpenSSL from acme.jose import util as jose_util @@ -70,10 +70,11 @@ class Revoker(object): """ certs = [] try: - clean_pem = Crypto.PublicKey.RSA.importKey( - authkey.pem).exportKey("PEM") - # https://www.dlitz.net/software/pycrypto/api/current/Crypto.PublicKey.RSA-module.html - except (IndexError, ValueError, TypeError): + clean_pem = OpenSSL.crypto.dump_privatekey( + OpenSSL.crypto.FILETYPE_PEM, OpenSSL.crypto.load_privatekey( + OpenSSL.crypto.FILETYPE_PEM, authkey.pem)) + except OpenSSL.crypto.Error as error: + logger.debug(error, exc_info=True) raise errors.RevokerError( "Invalid key file specified to revoke_from_key") @@ -86,9 +87,11 @@ class Revoker(object): # certificate. _, b_k = self._row_to_backup(row) try: - test_pem = Crypto.PublicKey.RSA.importKey( - open(b_k).read()).exportKey("PEM") - except (IndexError, ValueError, TypeError): + test_pem = OpenSSL.crypto.dump_privatekey( + OpenSSL.crypto.FILETYPE_PEM, OpenSSL.crypto.load_privatekey( + OpenSSL.crypto.FILETYPE_PEM, open(b_k).read())) + except OpenSSL.crypto.Error as error: + logger.debug(error, exc_info=True) # This should never happen given the assumptions of the # module. If it does, it is probably best to delete the # the offending key/cert. For now... just raise an exception @@ -248,10 +251,11 @@ class Revoker(object): certificate = jose_util.ComparableX509(cert._cert) try: with open(cert.backup_key_path, "rU") as backup_key_file: - key = Crypto.PublicKey.RSA.importKey(backup_key_file.read()) - + key = OpenSSL.crypto.load_privatekey( + OpenSSL.crypto.FILETYPE_PEM, backup_key_file.read()) # If the key file doesn't exist... or is corrupted - except (IndexError, ValueError, TypeError): + except OpenSSL.crypto.Error as error: + logger.debug(error, exc_info=True) raise errors.RevokerError( "Corrupted backup key file: %s" % cert.backup_key_path) diff --git a/letsencrypt/tests/revoker_test.py b/letsencrypt/tests/revoker_test.py index 893865ce9..ae4eaa900 100644 --- a/letsencrypt/tests/revoker_test.py +++ b/letsencrypt/tests/revoker_test.py @@ -7,12 +7,18 @@ import tempfile import unittest import mock +import OpenSSL from letsencrypt import errors from letsencrypt import le_util from letsencrypt.display import util as display_util +KEY = OpenSSL.crypto.load_privatekey( + OpenSSL.crypto.FILETYPE_PEM, pkg_resources.resource_string( + __name__, os.path.join("testdata", "rsa512_key.pem"))) + + class RevokerBase(unittest.TestCase): # pylint: disable=too-few-public-methods """Base Class for Revoker Tests.""" def setUp(self): @@ -77,13 +83,13 @@ class RevokerTest(RevokerBase): self.assertEqual(mock_net.call_count, 2) - @mock.patch("letsencrypt.revoker.Crypto.PublicKey.RSA.importKey") - def test_revoke_by_invalid_keys(self, mock_import): - mock_import.side_effect = ValueError + @mock.patch("letsencrypt.revoker.OpenSSL.crypto.load_privatekey") + def test_revoke_by_invalid_keys(self, mock_load_privatekey): + mock_load_privatekey.side_effect = OpenSSL.crypto.Error self.assertRaises( errors.RevokerError, self.revoker.revoke_from_key, self.key) - mock_import.side_effect = [mock.Mock(), IndexError] + mock_load_privatekey.side_effect = [KEY, OpenSSL.crypto.Error] self.assertRaises( errors.RevokerError, self.revoker.revoke_from_key, self.key) @@ -192,10 +198,10 @@ class RevokerTest(RevokerBase): self.revoker._safe_revoke(self.certs) self.assertTrue(mock_log.error.called) - @mock.patch("letsencrypt.revoker.Crypto.PublicKey.RSA.importKey") - def test_acme_revoke_failure(self, mock_crypto): + @mock.patch("letsencrypt.revoker.OpenSSL.crypto.load_privatekey") + def test_acme_revoke_failure(self, mock_load_privatekey): # pylint: disable=protected-access - mock_crypto.side_effect = ValueError + mock_load_privatekey.side_effect = OpenSSL.crypto.Error self.assertRaises( errors.Error, self.revoker._acme_revoke, self.certs[0]) diff --git a/setup.py b/setup.py index 7b4cc5068..b80983f24 100644 --- a/setup.py +++ b/setup.py @@ -57,7 +57,6 @@ letsencrypt_install_requires = [ 'mock', 'parsedatetime', 'psutil>=2.1.0', # net_connections introduced in 2.1.0 - 'pycrypto', # https://pyopenssl.readthedocs.org/en/latest/api/crypto.html#OpenSSL.crypto.X509Req.get_extensions 'PyOpenSSL>=0.15', 'pyrfc3339', @@ -93,7 +92,6 @@ install_requires = [ 'parsedatetime', 'psutil>=2.1.0', # net_connections introduced in 2.1.0 'pyasn1', # urllib3 InsecurePlatformWarning (#304) - 'pycrypto', # https://pyopenssl.readthedocs.org/en/latest/api/crypto.html#OpenSSL.crypto.X509Req.get_extensions 'PyOpenSSL>=0.15', 'pyparsing>=1.5.5', # Python3 support; perhaps unnecessary? From 02e7154c0d51805f5557f469f68096216642e8d4 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sun, 5 Jul 2015 22:43:47 +0000 Subject: [PATCH 41/50] Drop M2Crypto --- bootstrap/_deb_common.sh | 6 - bootstrap/_rpm_common.sh | 1 - bootstrap/mac.sh | 2 +- docs/conf.py | 2 +- docs/using.rst | 18 -- letsencrypt/achallenges.py | 5 +- letsencrypt/client.py | 7 +- letsencrypt/crypto_util.py | 158 +++++++++++------- letsencrypt/display/util.py | 1 + letsencrypt/proof_of_possession.py | 24 ++- letsencrypt/revoker.py | 65 ++++--- letsencrypt/tests/achallenges_test.py | 21 ++- letsencrypt/tests/crypto_util_test.py | 4 +- letsencrypt/tests/proof_of_possession_test.py | 11 +- letsencrypt/tests/revoker_test.py | 23 ++- requirements-swig-3.0.5.txt | 67 -------- setup.py | 6 +- 17 files changed, 204 insertions(+), 217 deletions(-) delete mode 100644 requirements-swig-3.0.5.txt diff --git a/bootstrap/_deb_common.sh b/bootstrap/_deb_common.sh index a4dff6b1c..687f30d09 100755 --- a/bootstrap/_deb_common.sh +++ b/bootstrap/_deb_common.sh @@ -44,20 +44,14 @@ else fi -# dpkg-dev: dpkg-architecture binary necessary to compile M2Crypto, c.f. -# #276, https://github.com/martinpaljak/M2Crypto/issues/62, -# M2Crypto setup.py:add_multiarch_paths - apt-get install -y --no-install-recommends \ git-core \ python \ python-dev \ "$virtualenv" \ gcc \ - swig \ dialog \ libaugeas0 \ libssl-dev \ libffi-dev \ ca-certificates \ - dpkg-dev \ diff --git a/bootstrap/_rpm_common.sh b/bootstrap/_rpm_common.sh index 1209cd44a..398cfe315 100755 --- a/bootstrap/_rpm_common.sh +++ b/bootstrap/_rpm_common.sh @@ -12,7 +12,6 @@ yum install -y \ python-virtualenv \ python-devel \ gcc \ - swig \ dialog \ augeas-libs \ openssl-devel \ diff --git a/bootstrap/mac.sh b/bootstrap/mac.sh index 9f0f22a17..a48afe11e 100755 --- a/bootstrap/mac.sh +++ b/bootstrap/mac.sh @@ -1,2 +1,2 @@ #!/bin/sh -brew install augeas swig +brew install augeas diff --git a/docs/conf.py b/docs/conf.py index a6e5da4ff..c285c19e5 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -23,7 +23,7 @@ import mock # http://docs.readthedocs.org/en/latest/faq.html#i-get-import-errors-on-libraries-that-depend-on-c-modules # c.f. #262 sys.modules.update( - (mod_name, mock.MagicMock()) for mod_name in ['augeas', 'M2Crypto']) + (mod_name, mock.MagicMock()) for mod_name in ['augeas']) here = os.path.abspath(os.path.dirname(__file__)) diff --git a/docs/using.rst b/docs/using.rst index 951c991d2..1fe3702f9 100644 --- a/docs/using.rst +++ b/docs/using.rst @@ -52,7 +52,6 @@ are provided mainly for the :ref:`developers ` reference. In general: * ``sudo`` is required as a suggested way of running privileged process -* `SWIG`_ is required for compiling `M2Crypto`_ * `Augeas`_ is required for the Python bindings @@ -102,14 +101,6 @@ Centos 7 sudo ./bootstrap/centos.sh -For installation run this modified command (note the trailing -backslash): - -.. code-block:: shell - - SWIG_FEATURES="-includeall -D__`uname -m`__-I/usr/include/openssl" \ - ./venv/bin/pip install -r requirements.txt . - Installation ============ @@ -127,13 +118,6 @@ Installation your operating system and are **not supported** by the Let's Encrypt team! -.. note:: If your operating system uses SWIG 3.0.5+, you will need to - run ``pip install -r requirements-swig-3.0.5.txt -r - requirements.txt .`` instead. Known affected systems: - - * Fedora 22 - * some versions of Mac OS X - Usage ===== @@ -152,6 +136,4 @@ The ``letsencrypt`` commandline tool has a builtin help: .. _Augeas: http://augeas.net/ -.. _M2Crypto: https://github.com/M2Crypto/M2Crypto -.. _SWIG: http://www.swig.org/ .. _Virtualenv: https://virtualenv.pypa.io diff --git a/letsencrypt/achallenges.py b/letsencrypt/achallenges.py index 88dcdbe11..80535026b 100644 --- a/letsencrypt/achallenges.py +++ b/letsencrypt/achallenges.py @@ -58,7 +58,10 @@ class DVSNI(AnnotatedChallenge): """ response = challenges.DVSNIResponse(s=s) cert_pem = crypto_util.make_ss_cert(self.key.pem, [ - self.nonce_domain, self.domain, response.z_domain(self.challb)]) + # TODO: setting nonce_domain first will cause "Error: + # [('asn1 encoding routines', 'ASN1_mbstring_ncopy', + # 'string too long')]" (get_subject().CN = domains[0]) + self.domain, self.nonce_domain, response.z_domain(self.challb)]) return cert_pem, response diff --git a/letsencrypt/client.py b/letsencrypt/client.py index 71fe05e29..2db07081c 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -3,7 +3,6 @@ import logging import os import pkg_resources -import M2Crypto import OpenSSL.crypto import zope.component @@ -435,8 +434,10 @@ def validate_key_csr(privkey, csr=None): if csr: if csr.form == "der": - csr_obj = M2Crypto.X509.load_request_der_string(csr.data) - csr = le_util.CSR(csr.file, csr_obj.as_pem(), "der") + csr_obj = OpenSSL.crypto.load_certificate_request( + OpenSSL.crypto.FILETYPE_ASN1, csr.data) + csr = le_util.CSR(csr.file, OpenSSL.crypto.dump_certificate( + OpenSSL.crypto.FILETYPE_PEM, csr_obj), "pem") # If CSR is provided, it must be readable and valid. if csr.data and not crypto_util.valid_csr(csr.data): diff --git a/letsencrypt/crypto_util.py b/letsencrypt/crypto_util.py index 3abe2d130..a57f0634d 100644 --- a/letsencrypt/crypto_util.py +++ b/letsencrypt/crypto_util.py @@ -4,13 +4,13 @@ is capable of handling the signatures. """ +import datetime import logging import os -import time -import M2Crypto import OpenSSL +from letsencrypt import errors from letsencrypt import le_util @@ -86,7 +86,7 @@ def init_save_csr(privkey, names, path, csrname="csr-letsencrypt.pem"): def make_csr(key_str, domains): """Generate a CSR. - :param str key_str: RSA key. + :param str key_str: PEM-encoded RSA key. :param list domains: Domains included in the certificate. .. todo:: Detect duplicates in `domains`? Using a set doesn't @@ -97,25 +97,23 @@ def make_csr(key_str, domains): """ assert domains, "Must provide one or more hostnames for the CSR." - rsa_key = M2Crypto.RSA.load_key_string(key_str) - pubkey = M2Crypto.EVP.PKey() - pubkey.assign_rsa(rsa_key) - - csr = M2Crypto.X509.Request() - csr.set_pubkey(pubkey) - # TODO: what to put into csr.get_subject()? - - extstack = M2Crypto.X509.X509_Extension_Stack() - ext = M2Crypto.X509.new_extension( - "subjectAltName", ", ".join("DNS:%s" % d for d in domains)) - - extstack.push(ext) - csr.add_extensions(extstack) - csr.sign(pubkey, "sha256") - assert csr.verify(pubkey) - pubkey2 = csr.get_pubkey() - assert csr.verify(pubkey2) - return csr.as_pem(), csr.as_der() + pkey = OpenSSL.crypto.load_privatekey(OpenSSL.crypto.FILETYPE_PEM, key_str) + req = OpenSSL.crypto.X509Req() + req.get_subject().CN = domains[0] + # TODO: what to put into req.get_subject()? + # TODO: put SAN if len(domains) > 1 + req.add_extensions([ + OpenSSL.crypto.X509Extension( + "subjectAltName", + critical=False, + value=", ".join("DNS:%s" % d for d in domains) + ), + ]) + req.set_pubkey(pkey) + req.sign(pkey, "sha256") + return tuple(OpenSSL.crypto.dump_certificate_request(method, req) + for method in (OpenSSL.crypto.FILETYPE_PEM, + OpenSSL.crypto.FILETYPE_ASN1)) # WARNING: the csr and private key file are possible attack vectors for TOCTOU @@ -135,9 +133,11 @@ def valid_csr(csr): """ try: - csr_obj = M2Crypto.X509.load_request_string(csr) - return bool(csr_obj.verify(csr_obj.get_pubkey())) - except M2Crypto.X509.X509Error: + req = OpenSSL.crypto.load_certificate_request( + OpenSSL.crypto.FILETYPE_PEM, csr) + return req.verify(req.get_pubkey()) + except OpenSSL.crypto.Error as error: + logger.debug(error, exc_info=True) return False @@ -145,15 +145,20 @@ def csr_matches_pubkey(csr, privkey): """Does private key correspond to the subject public key in the CSR? :param str csr: CSR in PEM. - :param str privkey: Private key file contents + :param str privkey: Private key file contents (PEM) :returns: Correspondence of private key to CSR subject public key. :rtype: bool """ - csr_obj = M2Crypto.X509.load_request_string(csr) - privkey_obj = M2Crypto.RSA.load_key_string(privkey) - return csr_obj.get_pubkey().get_rsa().pub() == privkey_obj.pub() + req = OpenSSL.crypto.load_certificate_request( + OpenSSL.crypto.FILETYPE_PEM, csr) + pkey = OpenSSL.crypto.load_privatekey(OpenSSL.crypto.FILETYPE_PEM, privkey) + try: + return req.verify(pkey) + except OpenSSL.crypto.Error as error: + logger.debug(error, exc_info=True) + return False def make_key(bits): @@ -174,18 +179,39 @@ def make_key(bits): def valid_privkey(privkey): """Is valid RSA private key? - :param str privkey: Private key file contents + :param str privkey: Private key file contents in PEM :returns: Validity of private key. :rtype: bool """ try: - return bool(M2Crypto.RSA.load_key_string(privkey).check_key()) - except M2Crypto.RSA.RSAError: + return OpenSSL.crypto.load_privatekey( + OpenSSL.crypto.FILETYPE_PEM, privkey).check() + except (TypeError, OpenSSL.crypto.Error): return False +def _pyopenssl_load(data, method, types=( + OpenSSL.crypto.FILETYPE_PEM, OpenSSL.crypto.FILETYPE_ASN1)): + openssl_errors = [] + for filetype in types: + try: + return method(filetype, data), filetype + except OpenSSL.crypto.Error as error: # TODO: anything else? + openssl_errors.append(error) + raise errors.Error("Unable to load: {0}".format(",".join( + str(error) for error in openssl_errors))) + +def pyopenssl_load_certificate(data): + """Load PEM/DER certificate. + + :raises errors.Error: + + """ + return _pyopenssl_load(data, OpenSSL.crypto.load_certificate) + + def make_ss_cert(key_str, domains, not_before=None, validity=(7 * 24 * 60 * 60)): """Returns new self-signed cert in PEM form. @@ -193,44 +219,36 @@ def make_ss_cert(key_str, domains, not_before=None, Uses key_str and contains all domains. """ - assert domains, "Must provide one or more hostnames for the CSR." - - rsa_key = M2Crypto.RSA.load_key_string(key_str) - pubkey = M2Crypto.EVP.PKey() - pubkey.assign_rsa(rsa_key) - - cert = M2Crypto.X509.X509() - cert.set_pubkey(pubkey) + assert domains, "Must provide one or more hostnames for the cert." + pkey = OpenSSL.crypto.load_privatekey(OpenSSL.crypto.FILETYPE_PEM, key_str) + cert = OpenSSL.crypto.X509() cert.set_serial_number(1337) cert.set_version(2) - current_ts = long(time.time() if not_before is None else not_before) - current = M2Crypto.ASN1.ASN1_UTCTIME() - current.set_time(current_ts) - expire = M2Crypto.ASN1.ASN1_UTCTIME() - expire.set_time(current_ts + validity) - cert.set_not_before(current) - cert.set_not_after(expire) + extensions = [ + OpenSSL.crypto.X509Extension( + "basicConstraints", True, 'CA:TRUE, pathlen:0'), + ] - subject = cert.get_subject() - subject.C = "US" - subject.ST = "Michigan" - subject.L = "Ann Arbor" - subject.O = "University of Michigan and the EFF" - subject.CN = domains[0] + cert.get_subject().CN = domains[0] + # TODO: what to put into cert.get_subject()? cert.set_issuer(cert.get_subject()) if len(domains) > 1: - cert.add_ext(M2Crypto.X509.new_extension( - "basicConstraints", "CA:FALSE")) - cert.add_ext(M2Crypto.X509.new_extension( - "subjectAltName", ", ".join(["DNS:%s" % d for d in domains]))) + extensions.append(OpenSSL.crypto.X509Extension( + "subjectAltName", + critical=False, + value=", ".join("DNS:%s" % d for d in domains) + )) - cert.sign(pubkey, "sha256") - assert cert.verify(pubkey) - assert cert.verify() - # print check_purpose(,0 - return cert.as_pem() + cert.add_extensions(extensions) + + cert.gmtime_adj_notBefore(0 if not_before is None else not_before) + cert.gmtime_adj_notAfter(validity) + + cert.set_pubkey(pkey) + cert.sign(pkey, "sha256") + return OpenSSL.crypto.dump_certificate(OpenSSL.crypto.FILETYPE_PEM, cert) def _pyopenssl_cert_or_req_san(cert_or_req): @@ -308,3 +326,19 @@ def get_sans_from_csr(csr, typ=OpenSSL.crypto.FILETYPE_PEM): """ return _get_sans_from_cert_or_req( csr, OpenSSL.crypto.load_certificate_request, typ) + + +def asn1_generalizedtime_to_dt(timestamp): + """Convert ASN.1 GENERALIZEDTIME to datetime. + + Useful for deserialization of `OpenSSL.crypto.X509.get_notAfter` and + `OpenSSL.crypto.X509.get_notAfter` outputs. + + """ + # TODO: there are two other ts formats + return datetime.datetime.strptime(timestamp, '%Y%m%d%H%M%SZ') + + +def pyopenssl_x509_name_as_text(x509name): + """Convert `OpenSSL.crypto.X509Name to text.""" + return "/".join("{0}={1}" for key, value in x509name.get_components()) diff --git a/letsencrypt/display/util.py b/letsencrypt/display/util.py index bd509dd28..de3e829fe 100644 --- a/letsencrypt/display/util.py +++ b/letsencrypt/display/util.py @@ -409,6 +409,7 @@ def separate_list_input(input_): """ no_commas = input_.replace(",", " ") # Each string is naturally unicode, this causes problems with M2Crypto SANs + # TODO: check if above is still true when M2Crypto is gone ^ return [str(string) for string in no_commas.split()] diff --git a/letsencrypt/proof_of_possession.py b/letsencrypt/proof_of_possession.py index a70fff697..3c625ba90 100644 --- a/letsencrypt/proof_of_possession.py +++ b/letsencrypt/proof_of_possession.py @@ -1,7 +1,9 @@ """Proof of Possession Identifier Validation Challenge.""" -import M2Crypto import logging import os + +from cryptography import x509 +from cryptography.hazmat.backends import default_backend import zope.component from acme import challenges @@ -43,15 +45,21 @@ class ProofOfPossession(object): # pylint: disable=too-few-public-methods return None for cert, key, _ in self.installer.get_all_certs_keys(): - pkey = M2Crypto.X509.load_cert(cert).get_pubkey() + with open(cert) as cert_file: + cert_data = cert_file.read() try: - rsa_pkey = pkey.get_rsa() + cert_obj = x509.load_pem_x509_certificate( + cert_data, default_backend()) except ValueError: - logger.warn("Only RSA supported at this time") - continue - pem_cert_key = rsa_pkey.as_pem() - cert_key = achall.alg.kty.load(pem_cert_key) - # TODO: If JWKES.load raises other exceptions, they should be caught here + try: + cert_obj = x509.load_der_x509_certificate( + cert_data, default_backend()) + except ValueError: + logger.warn("Certificate is neither PER nor DER: %s", cert) + + # TODO: only RSA is supported + cert_key = achall.alg.kty(key=jose.ComparableRSAKey( + cert_obj.public_key())) if cert_key == achall.hints.jwk: return self._gen_response(achall, key) diff --git a/letsencrypt/revoker.py b/letsencrypt/revoker.py index d8bec5fff..7597ff2ce 100644 --- a/letsencrypt/revoker.py +++ b/letsencrypt/revoker.py @@ -13,11 +13,11 @@ import os import shutil import tempfile -import M2Crypto import OpenSSL from acme.jose import util as jose_util +from letsencrypt import crypto_util from letsencrypt import errors from letsencrypt import le_util from letsencrypt import network @@ -196,10 +196,15 @@ class Revoker(object): for (cert_path, _, path) in self.installer.get_all_certs_keys(): try: - cert_sha1 = M2Crypto.X509.load_cert( - cert_path).get_fingerprint(md="sha1") - except (IOError, M2Crypto.X509.X509Error): + with open(cert_path) as cert_file: + cert_data = cert_file.read() + except IOError: continue + try: + cert_obj, _ = crypto_util.pyopenssl_load_certificate(cert_data) + except errors.Error: + continue + cert_sha1 = cert_obj.digest("sha1") if cert_sha1 in csha1_vhlist: csha1_vhlist[cert_sha1].append(path) else: @@ -246,7 +251,6 @@ class Revoker(object): """ # XXX | pylint: disable=unused-variable - # These will both have to change in the future away from M2Crypto # pylint: disable=protected-access certificate = jose_util.ComparableX509(cert._cert) try: @@ -373,8 +377,8 @@ class Revoker(object): class Cert(object): """Cert object used for Revocation convenience. - :ivar _cert: M2Crypto X509 cert - :type _cert: :class:`M2Crypto.X509` + :ivar _cert: Certificate + :type _cert: :class:`OpenSSL.crypto.X509` :ivar int idx: convenience index used for listing :ivar orig: (`str` path - original certificate, `str` status) @@ -402,8 +406,16 @@ class Cert(object): """ try: - self._cert = M2Crypto.X509.load_cert(cert_path) - except (IOError, M2Crypto.X509.X509Error): + with open(cert_path) as cert_file: + cert_data = cert_file.read() + except IOError: + raise errors.RevokerError( + "Error loading certificate: %s" % cert_path) + + try: + self._cert = OpenSSL.crypto.load_certificate( + OpenSSL.crypto.FILETYPE_PEM, cert_data) + except OpenSSL.crypto.Error: raise errors.RevokerError( "Error loading certificate: %s" % cert_path) @@ -451,8 +463,11 @@ class Cert(object): if not os.path.isfile(orig): status = Cert.DELETED_MSG else: - o_cert = M2Crypto.X509.load_cert(orig) - if self.get_fingerprint() != o_cert.get_fingerprint(md="sha1"): + with open(orig) as orig_file: + orig_data = orig_file.read() + o_cert = OpenSSL.crypto.load_certificate( + OpenSSL.crypto.FILETYPE_PEM, orig_data) + if self.get_fingerprint() != o_cert.digest("sha1"): status = Cert.CHANGED_MSG # Verify original key path @@ -472,47 +487,49 @@ class Cert(object): self.backup_path = backup self.backup_key_path = backup_key - # M2Crypto is eventually going to be replaced, hence the reason for _cert def get_cn(self): """Get common name.""" return self._cert.get_subject().CN def get_fingerprint(self): """Get SHA1 fingerprint.""" - return self._cert.get_fingerprint(md="sha1") + return self._cert.digest("sha1") def get_not_before(self): """Get not_valid_before field.""" - return self._cert.get_not_before().get_datetime() + return crypto_util.asn1_generalizedtime_to_dt( + self._cert.get_notBefore()) def get_not_after(self): """Get not_valid_after field.""" - return self._cert.get_not_after().get_datetime() + return crypto_util.asn1_generalizedtime_to_dt( + self._cert.get_notAfter()) def get_der(self): """Get certificate in der format.""" - return self._cert.as_der() + return OpenSSL.crypto.dump_certificate( + OpenSSL.crypto.FILETYPE_ASN1, self._cert) def get_pub_key(self): """Get public key size. - .. todo:: M2Crypto doesn't support ECC, this will have to be updated + .. todo:: Support for ECC """ - return "RSA " + str(self._cert.get_pubkey().size() * 8) + return "RSA {0}".format(self._cert.get_pubkey().bits) def get_san(self): """Get subject alternative name if available.""" - try: - return self._cert.get_ext("subjectAltName").get_value() - except LookupError: - return "" + # pylint: disable=protected-access + return ", ".join(crypto_util._pyopenssl_cert_or_req_san(self._cert)) def __str__(self): text = [ - "Subject: %s" % self._cert.get_subject().as_text(), + "Subject: %s" % crypto_util.pyopenssl_x509_name_as_text( + self._cert.get_subject()), "SAN: %s" % self.get_san(), - "Issuer: %s" % self._cert.get_issuer().as_text(), + "Issuer: %s" % crypto_util.pyopenssl_x509_name_as_text( + self._cert.get_issuer()), "Public Key: %s" % self.get_pub_key(), "Not Before: %s" % str(self.get_not_before()), "Not After: %s" % str(self.get_not_after()), diff --git a/letsencrypt/tests/achallenges_test.py b/letsencrypt/tests/achallenges_test.py index b4ed6265b..b06398fe2 100644 --- a/letsencrypt/tests/achallenges_test.py +++ b/letsencrypt/tests/achallenges_test.py @@ -1,12 +1,13 @@ """Tests for letsencrypt.achallenges.""" import os import pkg_resources -import re import unittest -import M2Crypto +import OpenSSL from acme import challenges + +from letsencrypt import crypto_util from letsencrypt import le_util from letsencrypt.tests import acme_util @@ -31,15 +32,13 @@ class DVSNITest(unittest.TestCase): def test_gen_cert_and_response(self): cert_pem, _ = self.achall.gen_cert_and_response(s=self.response.s) - cert = M2Crypto.X509.load_cert_string(cert_pem) - self.assertEqual(cert.get_subject().CN, self.chall.nonce_domain) - - sans = cert.get_ext("subjectAltName").get_value() - self.assertEqual( - set([self.chall.nonce_domain, "example.com", - self.response.z_domain(self.chall)]), - set(re.findall(r"DNS:([^, $]*)", sans)), - ) + cert = OpenSSL.crypto.load_certificate( + OpenSSL.crypto.FILETYPE_PEM, cert_pem) + self.assertEqual(cert.get_subject().CN, "example.com") + # pylint: disable=protected-access + self.assertEqual(crypto_util._pyopenssl_cert_or_req_san(cert), [ + "example.com", self.chall.nonce_domain, + self.response.z_domain(self.chall)]) if __name__ == "__main__": diff --git a/letsencrypt/tests/crypto_util_test.py b/letsencrypt/tests/crypto_util_test.py index e2d996640..06bdc4cd8 100644 --- a/letsencrypt/tests/crypto_util_test.py +++ b/letsencrypt/tests/crypto_util_test.py @@ -6,7 +6,6 @@ import shutil import tempfile import unittest -import M2Crypto import OpenSSL import mock @@ -146,7 +145,8 @@ class MakeKeyTest(unittest.TestCase): # pylint: disable=too-few-public-methods def test_it(self): # pylint: disable=no-self-use from letsencrypt.crypto_util import make_key # Do not test larger keys as it takes too long. - M2Crypto.RSA.load_key_string(make_key(1024)) + OpenSSL.crypto.load_privatekey( + OpenSSL.crypto.FILETYPE_PEM, make_key(1024)) class ValidPrivkeyTest(unittest.TestCase): diff --git a/letsencrypt/tests/proof_of_possession_test.py b/letsencrypt/tests/proof_of_possession_test.py index 68f740ebd..3013e2e12 100644 --- a/letsencrypt/tests/proof_of_possession_test.py +++ b/letsencrypt/tests/proof_of_possession_test.py @@ -1,6 +1,7 @@ """Tests for letsencrypt.proof_of_possession.""" import os import pkg_resources +import tempfile import unittest from cryptography.hazmat.backends import default_backend @@ -18,9 +19,7 @@ from letsencrypt.display import util as display_util BASE_PACKAGE = "letsencrypt.tests" CERT0_PATH = pkg_resources.resource_filename( - BASE_PACKAGE, os.path.join("testdata", "cert.pem")) -CERT1_PATH = pkg_resources.resource_filename( - BASE_PACKAGE, os.path.join("testdata", "cert-san.pem")) + "acme.jose", os.path.join("testdata", "cert.der")) CERT2_PATH = pkg_resources.resource_filename( BASE_PACKAGE, os.path.join("testdata", "dsa_cert.pem")) CERT2_KEY_PATH = pkg_resources.resource_filename( @@ -38,7 +37,8 @@ with open(CERT3_KEY_PATH) as cert3_file: class ProofOfPossessionTest(unittest.TestCase): def setUp(self): self.installer = mock.MagicMock() - certs = [CERT0_PATH, CERT1_PATH, CERT2_PATH, CERT3_PATH] + self.cert1_path = tempfile.mkstemp()[1] + certs = [CERT0_PATH, self.cert1_path, CERT2_PATH, CERT3_PATH] keys = [None, None, CERT2_KEY_PATH, CERT3_KEY_PATH] self.installer.get_all_certs_keys.return_value = zip( certs, keys, 4 * [None]) @@ -56,6 +56,9 @@ class ProofOfPossessionTest(unittest.TestCase): self.achall = achallenges.ProofOfPossession( challb=challb, domain="example.com") + def tearDown(self): + os.remove(self.cert1_path) + def test_perform_bad_challenge(self): hints = challenges.ProofOfPossession.Hints( jwk=jose.jwk.JWKOct(key="foo"), cert_fingerprints=(), diff --git a/letsencrypt/tests/revoker_test.py b/letsencrypt/tests/revoker_test.py index ae4eaa900..24336d023 100644 --- a/letsencrypt/tests/revoker_test.py +++ b/letsencrypt/tests/revoker_test.py @@ -267,18 +267,28 @@ class RevokerInstallerTest(RevokerBase): self.assertEqual( sha_vh[cert.get_fingerprint()], self.installs[i]) - @mock.patch("letsencrypt.revoker.M2Crypto.X509.load_cert") - def test_get_installed_load_failure(self, mock_m2): + @mock.patch("letsencrypt.revoker.OpenSSL.crypto.load_certificate") + def test_get_installed_load_failure(self, mock_load_certificate): mock_installer = mock.MagicMock() mock_installer.get_all_certs_keys.return_value = self.certs_keys - mock_m2.side_effect = IOError + mock_load_certificate.side_effect = OpenSSL.crypto.Error revoker = self._get_revoker(mock_installer) # pylint: disable=protected-access self.assertEqual(revoker._get_installed_locations(), {}) + def test_get_installed_load_failure_open(self): + tmp = tempfile.mkdtemp() + mock_installer = mock.MagicMock() + mock_installer.get_all_certs_keys.return_value = [( + os.path.join(tmp, 'missing'), None, None)] + revoker = self._get_revoker(mock_installer) + # pylint: disable=protected-access + self.assertEqual(revoker._get_installed_locations(), {}) + os.rmdir(tmp) + class RevokerClassMethodsTest(RevokerBase): def setUp(self): @@ -334,6 +344,13 @@ class CertTest(unittest.TestCase): from letsencrypt.revoker import Cert self.assertRaises(errors.RevokerError, Cert, self.key_path) + def test_failed_load_open(self): + tmp = tempfile.mkdtemp() + from letsencrypt.revoker import Cert + self.assertRaises( + errors.RevokerError, Cert, os.path.join(tmp, 'missing')) + os.rmdir(tmp) + def test_no_row(self): self.assertEqual(self.certs[0].get_row(), None) diff --git a/requirements-swig-3.0.5.txt b/requirements-swig-3.0.5.txt deleted file mode 100644 index 9ef45d950..000000000 --- a/requirements-swig-3.0.5.txt +++ /dev/null @@ -1,67 +0,0 @@ -# Support swig 3.0.5+ -# https://github.com/M2Crypto/M2Crypto/issues/24 -# https://github.com/M2Crypto/M2Crypto/pull/30 -git+https://github.com/M2Crypto/M2Crypto.git@d13a3a46c8934c5f50b31d5f95b23e6e06f845c3#egg=M2Crypto - -# This requirements file will fail on Travis CI 12.04 LTS Ubuntu build -# machine under TOX_ENV=py26 with very confusing error (full tracback -# at https://api.travis-ci.org/jobs/66529698/log.txt?deansi=true): - -#Traceback (most recent call last): -# File "setup.py", line 133, in -# include_package_data=True, -# File "/opt/python/2.6.9/lib/python2.6/distutils/core.py", line 152, in setup -# dist.run_commands() -# File "/opt/python/2.6.9/lib/python2.6/distutils/dist.py", line 975, in run_commands -# self.run_command(cmd) -# File "/opt/python/2.6.9/lib/python2.6/distutils/dist.py", line 995, in run_command -# cmd_obj.run() -# File "/home/travis/build/letsencrypt/lets-encrypt-preview/.tox/py26/lib/python2.6/site-packages/setuptools/command/test.py", line 142, in run -# self.with_project_on_sys_path(self.run_tests) -# File "/home/travis/build/letsencrypt/lets-encrypt-preview/.tox/py26/lib/python2.6/site-packages/setuptools/command/test.py", line 122, in with_project_on_sys_path -# func() -# File "/home/travis/build/letsencrypt/lets-encrypt-preview/.tox/py26/lib/python2.6/site-packages/setuptools/command/test.py", line 163, in run_tests -# testRunner=self._resolve_as_ep(self.test_runner), -# File "/opt/python/2.6.9/lib/python2.6/unittest.py", line 816, in __init__ -# self.parseArgs(argv) -# File "/opt/python/2.6.9/lib/python2.6/unittest.py", line 843, in parseArgs -# self.createTests() -# File "/opt/python/2.6.9/lib/python2.6/unittest.py", line 849, in createTests -# self.module) -# File "/opt/python/2.6.9/lib/python2.6/unittest.py", line 613, in loadTestsFromNames -# suites = [self.loadTestsFromName(name, module) for name in names] -# File "/opt/python/2.6.9/lib/python2.6/unittest.py", line 587, in loadTestsFromName -# return self.loadTestsFromModule(obj) -# File "/home/travis/build/letsencrypt/lets-encrypt-preview/.tox/py26/lib/python2.6/site-packages/setuptools/command/test.py", line 37, in loadTestsFromModule -# tests.append(self.loadTestsFromName(submodule)) -# File "/opt/python/2.6.9/lib/python2.6/unittest.py", line 584, in loadTestsFromName -# parent, obj = obj, getattr(obj, part) -#AttributeError: 'module' object has no attribute 'continuity_auth' - -# the above error happens because letsencrypt.continuity_auth cannot import M2Crypto: - -#>>> import M2Crypto -#Traceback (most recent call last): -# File "", line 1, in -# File "/root/lets-encrypt-preview/venv/lib/python2.6/site-packages/M2Crypto-0.21.1-py2.6-linux-x86_64.egg/M2Crypto/__init__.py", line 22, in -# import m2crypto -# File "/root/lets-encrypt-preview/venv/lib/python2.6/site-packages/M2Crypto-0.21.1-py2.6-linux-x86_64.egg/M2Crypto/m2crypto.py", line 26, in -# _m2crypto = swig_import_helper() -# File "/root/lets-encrypt-preview/venv/lib/python2.6/site-packages/M2Crypto-0.21.1-py2.6-linux-x86_64.egg/M2Crypto/m2crypto.py", line 22, in swig_import_helper -# _mod = imp.load_module('_m2crypto', fp, pathname, description) -#ImportError: /root/lets-encrypt-preview/venv/lib/python2.6/site-packages/M2Crypto-0.21.1-py2.6-linux-x86_64.egg/M2Crypto/_m2crypto.so: undefined symbol: SSLv2_method - -# For more info see: - -# - https://github.com/martinpaljak/M2Crypto/commit/84977c532c2444c5487db57146d81bb68dd5431d -# - http://stackoverflow.com/questions/10547332/install-m2crypto-on-a-virtualenv-without-system-packages -# - http://stackoverflow.com/questions/8206546/undefined-symbol-sslv2-method - -# In short: Python has been built without SSLv2 support, and -# github.com/M2Crypto/M2Crypto version doesn't contain necessary -# patch, but it's the only one that has a patch for newer versions of -# swig... - -# Problem seems not exists on Python 2.7. It's unlikely that the -# target distribution has swig 3.0.5+ and doesn't have Python 2.7, so -# this file should only be used in conjuction with Python 2.6. diff --git a/setup.py b/setup.py index b80983f24..042c6e159 100644 --- a/setup.py +++ b/setup.py @@ -53,7 +53,7 @@ letsencrypt_install_requires = [ 'argparse', 'ConfigArgParse', 'configobj', - 'M2Crypto', + #'cryptography>=0.7', # load_pem_x509_certificate, version pin mismatch 'mock', 'parsedatetime', 'psutil>=2.1.0', # net_connections introduced in 2.1.0 @@ -64,7 +64,6 @@ letsencrypt_install_requires = [ 'pytz', 'zope.component', 'zope.interface', - 'M2Crypto', ] letsencrypt_apache_install_requires = [ #'acme', @@ -103,9 +102,6 @@ install_requires = [ 'werkzeug', 'zope.component', 'zope.interface', - # order of items in install_requires DOES matter and M2Crypto has - # to go last, see #152 - 'M2Crypto', ] assert set(install_requires) == set.union(*(set(ireq) for ireq in ( From e276f2aa6b19e0d7250c815ed67070b2b12a4fd0 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Mon, 6 Jul 2015 11:48:09 +0000 Subject: [PATCH 42/50] crypto imports cleanup --- letsencrypt/client.py | 2 +- letsencrypt/plugins/standalone/authenticator.py | 3 +-- letsencrypt/plugins/standalone/tests/authenticator_test.py | 3 +-- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/letsencrypt/client.py b/letsencrypt/client.py index 2db07081c..65659effa 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -3,7 +3,7 @@ import logging import os import pkg_resources -import OpenSSL.crypto +import OpenSSL import zope.component from acme import jose diff --git a/letsencrypt/plugins/standalone/authenticator.py b/letsencrypt/plugins/standalone/authenticator.py index a90c02281..35b579eea 100644 --- a/letsencrypt/plugins/standalone/authenticator.py +++ b/letsencrypt/plugins/standalone/authenticator.py @@ -6,8 +6,7 @@ import socket import sys import time -import OpenSSL.crypto -import OpenSSL.SSL +import OpenSSL import zope.component import zope.interface diff --git a/letsencrypt/plugins/standalone/tests/authenticator_test.py b/letsencrypt/plugins/standalone/tests/authenticator_test.py index b61482990..e98616f84 100644 --- a/letsencrypt/plugins/standalone/tests/authenticator_test.py +++ b/letsencrypt/plugins/standalone/tests/authenticator_test.py @@ -7,8 +7,7 @@ import socket import unittest import mock -import OpenSSL.crypto -import OpenSSL.SSL +import OpenSSL from acme import challenges From e05b10974c5a283e4b9b0a89a7cb3c4b5ca0b1b6 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Tue, 7 Jul 2015 07:20:48 +0000 Subject: [PATCH 43/50] test/acme_util.py: fix nonce lengths --- letsencrypt/achallenges.py | 3 --- letsencrypt/tests/acme_util.py | 6 +++--- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/letsencrypt/achallenges.py b/letsencrypt/achallenges.py index 80535026b..0be626b35 100644 --- a/letsencrypt/achallenges.py +++ b/letsencrypt/achallenges.py @@ -58,9 +58,6 @@ class DVSNI(AnnotatedChallenge): """ response = challenges.DVSNIResponse(s=s) cert_pem = crypto_util.make_ss_cert(self.key.pem, [ - # TODO: setting nonce_domain first will cause "Error: - # [('asn1 encoding routines', 'ASN1_mbstring_ncopy', - # 'string too long')]" (get_subject().CN = domains[0]) self.domain, self.nonce_domain, response.z_domain(self.challb)]) return cert_pem, response diff --git a/letsencrypt/tests/acme_util.py b/letsencrypt/tests/acme_util.py index b0939dc68..4b660c648 100644 --- a/letsencrypt/tests/acme_util.py +++ b/letsencrypt/tests/acme_util.py @@ -21,8 +21,8 @@ KEY = jose.ComparableRSAKey(serialization.load_pem_private_key( SIMPLE_HTTP = challenges.SimpleHTTP( token="evaGxfADs6pSRb2LAv9IZf17Dt3juxGJ+PCt92wr+oA") DVSNI = challenges.DVSNI( - r="O*\xb4-\xad\xec\x95>\xed\xa9\r0\x94\xe8\x97\x9c&6\xbf'\xb3" - "\xed\x9a9nX\x0f'\\m\xe7\x12", nonce="a82d5ff8ef740d12881f6d3c2277ab2e") + r=jose.b64decode("Tyq0La3slT7tqQ0wlOiXnCY2vyez7Zo5blgPJ1xt5xI"), + nonce=jose.b64decode("a82d5ff8ef740d12881f6d3c2277ab2e")) DNS = challenges.DNS(token="17817c66b60ce2e4012dfad92657527a") RECOVERY_CONTACT = challenges.RecoveryContact( activation_url="https://example.ca/sendrecovery/a5bd99383fb0", @@ -30,7 +30,7 @@ RECOVERY_CONTACT = challenges.RecoveryContact( contact="c********n@example.com") RECOVERY_TOKEN = challenges.RecoveryToken() POP = challenges.ProofOfPossession( - alg="RS256", nonce="xD\xf9\xb9\xdbU\xed\xaa\x17\xf1y|\x81\x88\x99 ", + alg="RS256", nonce=jose.b64decode("eET5udtV7aoX8Xl8gYiZIA"), hints=challenges.ProofOfPossession.Hints( jwk=jose.JWKRSA(key=KEY.public_key()), cert_fingerprints=( From 9a9f91b4ee9ff32a4d7acba3d66f95e4bd2cb7aa Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Tue, 7 Jul 2015 07:21:48 +0000 Subject: [PATCH 44/50] Fix typo --- letsencrypt/proof_of_possession.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/proof_of_possession.py b/letsencrypt/proof_of_possession.py index 3c625ba90..09723dd96 100644 --- a/letsencrypt/proof_of_possession.py +++ b/letsencrypt/proof_of_possession.py @@ -66,7 +66,7 @@ class ProofOfPossession(object): # pylint: disable=too-few-public-methods # Is there are different prompt we should give the user? code, key = zope.component.getUtility( interfaces.IDisplay).input( - "QPath to private key for identifier: %s " % achall.domain) + "Path to private key for identifier: %s " % achall.domain) if code != display_util.CANCEL: return self._gen_response(achall, key) From 20a08b50f2a63709a8919d7b9b1937ed842d9484 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Tue, 7 Jul 2015 08:03:38 +0000 Subject: [PATCH 45/50] ComparableX509 and ComparableX509Req: __eq__, __ne__, __hash__ data model fixes. --- acme/jose/json_util_test.py | 8 +++---- acme/jose/util.py | 43 +++++++++++++++++++++---------------- acme/jose/util_test.py | 32 +++++++++++++++++---------- 3 files changed, 50 insertions(+), 33 deletions(-) diff --git a/acme/jose/json_util_test.py b/acme/jose/json_util_test.py index 9e622bea6..9e493e80c 100644 --- a/acme/jose/json_util_test.py +++ b/acme/jose/json_util_test.py @@ -12,12 +12,12 @@ from acme.jose import interfaces from acme.jose import util -CERT = OpenSSL.crypto.load_certificate( +CERT = util.ComparableX509(OpenSSL.crypto.load_certificate( OpenSSL.crypto.FILETYPE_PEM, pkg_resources.resource_string( - 'letsencrypt.tests', os.path.join('testdata', 'cert.pem'))) -CSR = OpenSSL.crypto.load_certificate_request( + 'letsencrypt.tests', os.path.join('testdata', 'cert.pem')))) +CSR = util.ComparableX509(OpenSSL.crypto.load_certificate_request( OpenSSL.crypto.FILETYPE_PEM, pkg_resources.resource_string( - 'letsencrypt.tests', os.path.join('testdata', 'csr.pem'))) + 'letsencrypt.tests', os.path.join('testdata', 'csr.pem')))) class FieldTest(unittest.TestCase): diff --git a/acme/jose/util.py b/acme/jose/util.py index a9b1382be..eebbe7468 100644 --- a/acme/jose/util.py +++ b/acme/jose/util.py @@ -35,26 +35,31 @@ class ComparableX509(object): # pylint: disable=too-few-public-methods """ def __init__(self, wrapped): + assert isinstance(wrapped, OpenSSL.crypto.X509) or isinstance( + wrapped, OpenSSL.crypto.X509Req) self._wrapped = wrapped def __getattr__(self, name): return getattr(self._wrapped, name) + def _dump(self, filetype=OpenSSL.crypto.FILETYPE_ASN1): + # pylint: disable=missing-docstring,protected-access + if isinstance(self._wrapped, OpenSSL.crypto.X509): + func = OpenSSL.crypto.dump_certificate + else: # assert in __init__ makes sure this is X509Req + func = OpenSSL.crypto.dump_certificate_request + return func(filetype, self._wrapped) + def __eq__(self, other): - filetype = OpenSSL.crypto.FILETYPE_ASN1 - def as_der(obj): - # pylint: disable=missing-docstring,protected-access - if isinstance(obj, type(self)): - obj = obj._wrapped - if isinstance(obj, OpenSSL.crypto.X509): - func = OpenSSL.crypto.dump_certificate - elif isinstance(obj, OpenSSL.crypto.X509Req): - func = OpenSSL.crypto.dump_certificate_request - else: - raise TypeError( - "Equality for {0} not provided".format(obj.__class__)) - return func(filetype, obj) - return as_der(self) == as_der(other) + if not isinstance(other, self.__class__): + return NotImplemented + return self._dump() == other._dump() # pylint: disable=protected-access + + def __hash__(self): + return hash((self.__class__, self._dump())) + + def __ne__(self, other): + return not self == other def __repr__(self): return '<{0}({1!r})>'.format(self.__class__.__name__, self._wrapped) @@ -78,7 +83,7 @@ class ComparableRSAKey(object): # pylint: disable=too-few-public-methods # pylint: disable=protected-access if (not isinstance(other, self.__class__) or self._wrapped.__class__ is not other._wrapped.__class__): - return False + return NotImplemented # RSA*KeyWithSerialization requires cryptography>=0.8 if isinstance(self._wrapped, rsa.RSAPrivateKeyWithSerialization): return self.private_numbers() == other.private_numbers() @@ -87,24 +92,26 @@ class ComparableRSAKey(object): # pylint: disable=too-few-public-methods else: return False # we shouldn't reach here... + def __ne__(self, other): + return not self == other def __hash__(self): # public_numbers() hasn't got stable hash! if isinstance(self._wrapped, rsa.RSAPrivateKeyWithSerialization): priv = self.private_numbers() pub = priv.public_numbers - return hash((type(self), priv.p, priv.q, priv.dmp1, + return hash((self.__class__, priv.p, priv.q, priv.dmp1, priv.dmq1, priv.iqmp, pub.n, pub.e)) elif isinstance(self._wrapped, rsa.RSAPublicKeyWithSerialization): pub = self.public_numbers() - return hash((type(self), pub.n, pub.e)) + return hash((self.__class__, pub.n, pub.e)) def __repr__(self): return '<{0}({1!r})>'.format(self.__class__.__name__, self._wrapped) def public_key(self): """Get wrapped public key.""" - return type(self)(self._wrapped.public_key()) + return self.__class__(self._wrapped.public_key()) class ImmutableMap(collections.Mapping, collections.Hashable): diff --git a/acme/jose/util_test.py b/acme/jose/util_test.py index 95b5c6717..8cb10e62a 100644 --- a/acme/jose/util_test.py +++ b/acme/jose/util_test.py @@ -27,13 +27,15 @@ class ComparableX509Test(unittest.TestCase): def test_eq(self): self.assertEqual(self.cert, self.cert_same) - def test_not_eq(self): + def test_eq_wrong_types(self): + self.assertNotEqual(self.cert, 5) + + def test_ne(self): self.assertNotEqual(self.cert, self.cert2) - def test_eq_wrong_types(self): - from acme.jose.util import ComparableX509 - self.assertRaises( - TypeError, ComparableX509(5).__eq__, ComparableX509(5)) + def test_hash(self): + self.assertEqual(hash(self.cert), hash(self.cert_same)) + self.assertNotEqual(hash(self.cert), hash(self.cert2)) def test_repr(self): self.assertTrue(repr(self.cert).startswith( @@ -53,6 +55,10 @@ class ComparableRSAKeyTest(unittest.TestCase): password=None, backend=backend)) self.key = load_key() self.key_same = load_key() + self.key2 = ComparableRSAKey(serialization.load_pem_private_key( + pkg_resources.resource_string( + __name__, os.path.join('testdata', 'rsa512_key.pem')), + password=None, backend=backend)) def test_getattr_proxy(self): self.assertEqual(256, self.key.key_size) @@ -60,20 +66,24 @@ class ComparableRSAKeyTest(unittest.TestCase): def test_eq(self): self.assertEqual(self.key, self.key_same) - def test_not_eq_different_types(self): - self.assertFalse(self.key.__eq__(5)) + def test_ne(self): + self.assertNotEqual(self.key, self.key2) - def test_not_eq_not_wrapped(self): + def test_ne_different_types(self): + self.assertNotEqual(self.key, 5) + + def test_ne_not_wrapped(self): # pylint: disable=protected-access - self.assertFalse(self.key.__eq__(self.key_same._wrapped)) + self.assertNotEqual(self.key, self.key_same._wrapped) - def test_not_eq_no_serialization(self): + def test_ne_no_serialization(self): from acme.jose.util import ComparableRSAKey - self.assertFalse(ComparableRSAKey(5).__eq__(ComparableRSAKey(5))) + self.assertNotEqual(ComparableRSAKey(5), ComparableRSAKey(5)) def test_hash(self): self.assertTrue(isinstance(hash(self.key), int)) self.assertEqual(hash(self.key), hash(self.key_same)) + self.assertNotEqual(hash(self.key), hash(self.key2)) def test_repr(self): self.assertTrue(repr(self.key).startswith( From 9ab40444b66d59b8a6f623db2ad3125058a2fba5 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Tue, 7 Jul 2015 08:15:33 +0000 Subject: [PATCH 46/50] More Python data model fixes for acme. --- acme/jose/jwa.py | 7 ++++++- acme/jose/jwa_test.py | 5 +++++ acme/messages.py | 2 +- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/acme/jose/jwa.py b/acme/jose/jwa.py index c3d79ff20..f081aa169 100644 --- a/acme/jose/jwa.py +++ b/acme/jose/jwa.py @@ -35,7 +35,12 @@ class JWASignature(JWA): self.name = name def __eq__(self, other): - return isinstance(other, JWASignature) and self.name == other.name + if not isinstance(other, JWASignature): + return NotImplemented + return self.name == other.name + + def __ne__(self, other): + return not self == other @classmethod def register(cls, signature_cls): diff --git a/acme/jose/jwa_test.py b/acme/jose/jwa_test.py index c8347ff69..91b864cf5 100644 --- a/acme/jose/jwa_test.py +++ b/acme/jose/jwa_test.py @@ -44,8 +44,13 @@ class JWASignatureTest(unittest.TestCase): def test_eq(self): self.assertEqual(self.Sig1, self.Sig1) + + def test_ne(self): self.assertNotEqual(self.Sig1, self.Sig2) + def test_ne_other_type(self): + self.assertNotEqual(self.Sig1, 5) + def test_repr(self): self.assertEqual('Sig1', repr(self.Sig1)) self.assertEqual('Sig2', repr(self.Sig2)) diff --git a/acme/messages.py b/acme/messages.py index 35c325e4e..c224e9bde 100644 --- a/acme/messages.py +++ b/acme/messages.py @@ -84,7 +84,7 @@ class _Constant(jose.JSONDeSerializable): return isinstance(other, type(self)) and other.name == self.name def __ne__(self, other): - return not self.__eq__(other) + return not self == other class Status(_Constant): From 90b27ff9cf2b9fe01a2585a8a8c2d20c52e8c4ea Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Tue, 7 Jul 2015 17:00:08 +0000 Subject: [PATCH 47/50] ComparableX509Test for cert and CSR --- acme/jose/util_test.py | 46 +++++++++++++++++++++++++----------------- 1 file changed, 28 insertions(+), 18 deletions(-) diff --git a/acme/jose/util_test.py b/acme/jose/util_test.py index 8cb10e62a..f29b0792f 100644 --- a/acme/jose/util_test.py +++ b/acme/jose/util_test.py @@ -14,32 +14,42 @@ class ComparableX509Test(unittest.TestCase): def setUp(self): from acme.jose.util import ComparableX509 - def load_cert(): # pylint: disable=missing-docstring - return ComparableX509(OpenSSL.crypto.load_certificate_request( - OpenSSL.crypto.FILETYPE_ASN1, pkg_resources.resource_string( - __name__, os.path.join('testdata', 'csr.der')))) - self.cert = load_cert() - self.cert_same = load_cert() - self.cert2 = ComparableX509(OpenSSL.crypto.load_certificate_request( - OpenSSL.crypto.FILETYPE_PEM, pkg_resources.resource_string( - 'letsencrypt.tests', os.path.join('testdata', 'csr-san.pem')))) + def _load(method, filename): # pylint: disable=missing-docstring + return ComparableX509(method( + OpenSSL.crypto.FILETYPE_PEM, pkg_resources.resource_string( + 'letsencrypt.tests', os.path.join('testdata', filename)))) + + self.req1 = _load(OpenSSL.crypto.load_certificate_request, 'csr.pem') + self.req2 = _load(OpenSSL.crypto.load_certificate_request, 'csr.pem') + self.req_other = _load(OpenSSL.crypto.load_certificate_request, 'csr-san.pem') + + self.cert1 = _load(OpenSSL.crypto.load_certificate, 'cert.pem') + self.cert2 = _load(OpenSSL.crypto.load_certificate, 'cert.pem') + self.cert_other = _load(OpenSSL.crypto.load_certificate, 'cert-san.pem') def test_eq(self): - self.assertEqual(self.cert, self.cert_same) - - def test_eq_wrong_types(self): - self.assertNotEqual(self.cert, 5) + self.assertEqual(self.req1, self.req2) + self.assertEqual(self.cert1, self.cert2) def test_ne(self): - self.assertNotEqual(self.cert, self.cert2) + self.assertNotEqual(self.req1, self.req_other) + self.assertNotEqual(self.cert1, self.cert_other) + + def test_ne_wrong_types(self): + self.assertNotEqual(self.req1, 5) + self.assertNotEqual(self.cert1, 5) def test_hash(self): - self.assertEqual(hash(self.cert), hash(self.cert_same)) - self.assertNotEqual(hash(self.cert), hash(self.cert2)) + self.assertEqual(hash(self.req1), hash(self.req2)) + self.assertNotEqual(hash(self.req1), hash(self.req_other)) + + self.assertEqual(hash(self.cert1), hash(self.cert2)) + self.assertNotEqual(hash(self.cert1), hash(self.cert_other)) def test_repr(self): - self.assertTrue(repr(self.cert).startswith( - ' Date: Wed, 8 Jul 2015 08:41:13 +0000 Subject: [PATCH 48/50] Move asn1_generalizedtime_to_dt todo comment to docstring. --- letsencrypt/crypto_util.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/letsencrypt/crypto_util.py b/letsencrypt/crypto_util.py index a57f0634d..933f97362 100644 --- a/letsencrypt/crypto_util.py +++ b/letsencrypt/crypto_util.py @@ -334,8 +334,10 @@ def asn1_generalizedtime_to_dt(timestamp): Useful for deserialization of `OpenSSL.crypto.X509.get_notAfter` and `OpenSSL.crypto.X509.get_notAfter` outputs. + .. todo:: This function support only one format: `%Y%m%d%H%M%SZ`. + Implement remaining two. + """ - # TODO: there are two other ts formats return datetime.datetime.strptime(timestamp, '%Y%m%d%H%M%SZ') From a7817de4ab3b3a7f52088ea8b8f2e243d74ae9b8 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Wed, 8 Jul 2015 11:30:09 +0000 Subject: [PATCH 49/50] Rewrite JWK.load, JWKRSA autowraps ComparableRSAKey. --- acme/jose/jwa_test.py | 23 ++++----- acme/jose/jwk.py | 105 +++++++++++++++++++++++++++++------------- acme/jose/jwk_test.py | 29 ++++++++++-- acme/other.py | 2 +- 4 files changed, 109 insertions(+), 50 deletions(-) diff --git a/acme/jose/jwa_test.py b/acme/jose/jwa_test.py index 91b864cf5..147038788 100644 --- a/acme/jose/jwa_test.py +++ b/acme/jose/jwa_test.py @@ -7,16 +7,9 @@ from cryptography.hazmat.backends import default_backend from cryptography.hazmat.primitives import serialization from acme.jose import errors +from acme.jose import jwk_test -RSA256_KEY = serialization.load_pem_private_key( - pkg_resources.resource_string( - __name__, os.path.join('testdata', 'rsa256_key.pem')), - password=None, backend=default_backend()) -RSA512_KEY = serialization.load_pem_private_key( - pkg_resources.resource_string( - __name__, os.path.join('testdata', 'rsa512_key.pem')), - password=None, backend=default_backend()) RSA1024_KEY = serialization.load_pem_private_key( pkg_resources.resource_string( __name__, os.path.join('testdata', 'rsa1024_key.pem')), @@ -83,13 +76,13 @@ class JWARSTest(unittest.TestCase): def test_sign_no_private_part(self): from acme.jose.jwa import RS256 self.assertRaises( - errors.Error, RS256.sign, RSA512_KEY.public_key(), 'foo') + errors.Error, RS256.sign, jwk_test.RSA512_KEY.public_key(), 'foo') def test_sign_key_too_small(self): from acme.jose.jwa import RS256 from acme.jose.jwa import PS256 - self.assertRaises(errors.Error, RS256.sign, RSA256_KEY, 'foo') - self.assertRaises(errors.Error, PS256.sign, RSA256_KEY, 'foo') + self.assertRaises(errors.Error, RS256.sign, jwk_test.RSA256_KEY, 'foo') + self.assertRaises(errors.Error, PS256.sign, jwk_test.RSA256_KEY, 'foo') def test_rs(self): from acme.jose.jwa import RS256 @@ -99,9 +92,11 @@ class JWARSTest(unittest.TestCase): '\xa4\x99\x1e\x19&\xd8\xc7\x99S\x97\xfc\x85\x0cOV\xe6\x07\x99' '\xd2\xb9.>}\xfd' ) - self.assertEqual(RS256.sign(RSA512_KEY, 'foo'), sig) - self.assertTrue(RS256.verify(RSA512_KEY.public_key(), 'foo', sig)) - self.assertFalse(RS256.verify(RSA512_KEY.public_key(), 'foo', sig + '!')) + self.assertEqual(RS256.sign(jwk_test.RSA512_KEY, 'foo'), sig) + self.assertTrue(RS256.verify( + jwk_test.RSA512_KEY.public_key(), 'foo', sig)) + self.assertFalse(RS256.verify( + jwk_test.RSA512_KEY.public_key(), 'foo', sig + '!')) def test_ps(self): from acme.jose.jwa import PS256 diff --git a/acme/jose/jwk.py b/acme/jose/jwk.py index 376988ae2..9454c6f9e 100644 --- a/acme/jose/jwk.py +++ b/acme/jose/jwk.py @@ -1,9 +1,12 @@ """JSON Web Key.""" import abc import binascii +import logging +import cryptography.exceptions from cryptography.hazmat.backends import default_backend from cryptography.hazmat.primitives import serialization +from cryptography.hazmat.primitives.asymmetric import ec from cryptography.hazmat.primitives.asymmetric import rsa from acme.jose import b64 @@ -12,16 +15,16 @@ from acme.jose import json_util from acme.jose import util +logger = logging.getLogger(__name__) + + class JWK(json_util.TypedJSONObjectWithFields): # pylint: disable=too-few-public-methods """JSON Web Key.""" type_field_name = 'kty' TYPES = {} - - @util.abstractclassmethod - def load(cls, string): # pragma: no cover - """Load key from normalized string form.""" - raise NotImplementedError() + cryptography_key_types = () + """Subclasses should override.""" @abc.abstractmethod def public_key(self): # pragma: no cover @@ -32,6 +35,63 @@ class JWK(json_util.TypedJSONObjectWithFields): """ raise NotImplementedError() + @classmethod + def _load_cryptography_key(cls, data, password=None, backend=None): + backend = default_backend() if backend is None else backend + exceptions = {} + + # private key? + for loader in (serialization.load_pem_private_key, + serialization.load_der_private_key): + try: + return loader(data, password, backend) + except (ValueError, TypeError, + cryptography.exceptions.UnsupportedAlgorithm) as error: + exceptions[loader] = error + + # public key? + for loader in (serialization.load_pem_public_key, + serialization.load_der_public_key): + try: + return loader(data, backend) + except (ValueError, + cryptography.exceptions.UnsupportedAlgorithm) as error: + exceptions[loader] = error + + # no luck + raise errors.Error("Unable to deserialize key: {0}".format(exceptions)) + + @classmethod + def load(cls, data, password=None, backend=None): + """Load serialized key as JWK. + + :param str data: Public or private key serialized as PEM or DER. + :param str password: Optional password. + :param backend: A `.PEMSerializationBackend` and + `.DERSerializationBackend` provider. + + :raises errors.Error: if unable to deserialize, or unsupported + JWK algorithm + + :returns: JWK of an appropriate type. + :rtype: `JWK` + + """ + try: + key = cls._load_cryptography_key(data, password, backend) + except errors.Error as error: + logger.debug("Loading symmetric key, assymentric failed: %s", error) + return JWKOct(key=data) + + if cls.typ is not NotImplemented and not isinstance( + key, cls.cryptography_key_types): + raise errors.Error("Unable to deserialize {0} into {1}".format( + key.__class__, cls.__class__)) + for jwk_cls in cls.TYPES.itervalues(): + if isinstance(key, jwk_cls.cryptography_key_types): + return jwk_cls(key=key) + raise errors.Error("Unsupported algorithm: {0}".format(key.__class__)) + @JWK.register class JWKES(JWK): # pragma: no cover @@ -42,6 +102,8 @@ class JWKES(JWK): # pragma: no cover """ typ = 'ES' + cryptography_key_types = ( + ec.EllipticCurvePublicKey, ec.EllipticCurvePrivateKey) def fields_to_partial_json(self): raise NotImplementedError() @@ -50,10 +112,6 @@ class JWKES(JWK): # pragma: no cover def fields_from_json(cls, jobj): raise NotImplementedError() - @classmethod - def load(cls, string): - raise NotImplementedError() - def public_key(self): raise NotImplementedError() @@ -75,10 +133,6 @@ class JWKOct(JWK): def fields_from_json(cls, jobj): return cls(key=jobj['k']) - @classmethod - def load(cls, string): - return cls(key=string) - def public_key(self): return self @@ -93,8 +147,15 @@ class JWKRSA(JWK): """ typ = 'RSA' + cryptography_key_types = (rsa.RSAPublicKey, rsa.RSAPrivateKey) __slots__ = ('key',) + def __init__(self, *args, **kwargs): + if 'key' in kwargs and not isinstance( + kwargs['key'], util.ComparableRSAKey): + kwargs['key'] = util.ComparableRSAKey(kwargs['key']) + super(JWKRSA, self).__init__(*args, **kwargs) + @classmethod def _encode_param(cls, data): def _leading_zeros(arg): @@ -112,24 +173,6 @@ class JWKRSA(JWK): except ValueError: # invalid literal for long() with base 16 raise errors.DeserializationError() - @classmethod - def load(cls, string): - """Load RSA key from string. - - :param str string: RSA key in string form. - - :returns: - :rtype: :class:`JWKRSA` - - """ - try: - key = serialization.load_pem_public_key( - string, backend=default_backend()) - except ValueError: # ValueError: Could not unserialize key data. - key = serialization.load_pem_private_key( - string, password=None, backend=default_backend()) - return cls(key=util.ComparableRSAKey(key)) - def public_key(self): return type(self)(key=self.key.public_key()) diff --git a/acme/jose/jwk_test.py b/acme/jose/jwk_test.py index f3745b251..06032f45b 100644 --- a/acme/jose/jwk_test.py +++ b/acme/jose/jwk_test.py @@ -10,14 +10,28 @@ from acme.jose import errors from acme.jose import util -RSA256_KEY = util.ComparableRSAKey(serialization.load_pem_private_key( +DSA_PEM = pkg_resources.resource_string( + 'letsencrypt.tests', os.path.join('testdata', 'dsa512_key.pem')) +RSA256_KEY = serialization.load_pem_private_key( pkg_resources.resource_string( __name__, os.path.join('testdata', 'rsa256_key.pem')), - password=None, backend=default_backend())) -RSA512_KEY = util.ComparableRSAKey(serialization.load_pem_private_key( + password=None, backend=default_backend()) +RSA512_KEY = serialization.load_pem_private_key( pkg_resources.resource_string( __name__, os.path.join('testdata', 'rsa512_key.pem')), - password=None, backend=default_backend())) + password=None, backend=default_backend()) + + +class JWKTest(unittest.TestCase): + """Tests for acme.jose.jwk.JWK.""" + + def test_load(self): + from acme.jose.jwk import JWK + self.assertRaises(errors.Error, JWK.load, DSA_PEM) + + def test_load_subclass_wrong_type(self): + from acme.jose.jwk import JWKRSA + self.assertRaises(errors.Error, JWKRSA.load, DSA_PEM) class JWKOctTest(unittest.TestCase): @@ -49,6 +63,7 @@ class JWKOctTest(unittest.TestCase): class JWKRSATest(unittest.TestCase): """Tests for acme.jose.jwk.JWKRSA.""" + # pylint: disable=too-many-instance-attributes def setUp(self): from acme.jose.jwk import JWKRSA @@ -58,6 +73,8 @@ class JWKRSATest(unittest.TestCase): 'e': 'AQAB', 'n': 'm2Fylv-Uz7trgTW8EBHP3FQSMeZs2GNQ6VRo1sIVJEk', } + self.jwk256_comparable = JWKRSA(key=util.ComparableRSAKey( + RSA256_KEY.public_key())) self.jwk512 = JWKRSA(key=RSA512_KEY.public_key()) self.jwk512json = { 'kty': 'RSA', @@ -79,6 +96,10 @@ class JWKRSATest(unittest.TestCase): 'qi': 'oi45cEkbVoJjAbnQpFY87Q', }) + def test_init_comparable(self): + self.assertTrue(isinstance(self.jwk256.key, util.ComparableRSAKey)) + self.assertEqual(self.jwk256, self.jwk256_comparable) + def test_equals(self): self.assertEqual(self.jwk256, self.jwk256) self.assertEqual(self.jwk512, self.jwk512) diff --git a/acme/other.py b/acme/other.py index 5170d49cc..cf6425b7f 100644 --- a/acme/other.py +++ b/acme/other.py @@ -42,7 +42,7 @@ class Signature(jose.JSONObjectWithFields): :param key: Key used for signing. :type key: `cryptography.hazmat.primitives.assymetric.rsa.RSAPrivateKey` - wrapped in `.ComparableRSAKey`. + (optionally wrapped in `.ComparableRSAKey`). :param str nonce: Nonce to be used. If None, nonce of ``nonce_size`` will be randomly generated. From 36eafde21319411cff72fdc876113b69f89ccc90 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Wed, 8 Jul 2015 12:07:05 +0000 Subject: [PATCH 50/50] Use ComparableRSAKey autowrap throughout the code base. --- acme/challenges_test.py | 4 ++-- acme/jose/jwk.py | 2 +- acme/jose/jwk_test.py | 5 ++--- acme/jose/jws_test.py | 4 ++-- acme/jws_test.py | 4 ++-- acme/messages_test.py | 4 ++-- acme/other_test.py | 4 ++-- examples/acme_client.py | 4 ++-- letsencrypt/proof_of_possession.py | 4 +--- letsencrypt/tests/acme_util.py | 4 ++-- letsencrypt/tests/proof_of_possession_test.py | 4 ++-- 11 files changed, 20 insertions(+), 23 deletions(-) diff --git a/acme/challenges_test.py b/acme/challenges_test.py index 9d03e01cd..94c04388d 100644 --- a/acme/challenges_test.py +++ b/acme/challenges_test.py @@ -17,10 +17,10 @@ from acme import other CERT = jose.ComparableX509(OpenSSL.crypto.load_certificate( OpenSSL.crypto.FILETYPE_PEM, pkg_resources.resource_string( 'letsencrypt.tests', os.path.join('testdata', 'cert.pem')))) -KEY = jose.ComparableRSAKey(serialization.load_pem_private_key( +KEY = serialization.load_pem_private_key( pkg_resources.resource_string( 'acme.jose', os.path.join('testdata', 'rsa512_key.pem')), - password=None, backend=default_backend())) + password=None, backend=default_backend()) class ChallengeResponseTest(unittest.TestCase): diff --git a/acme/jose/jwk.py b/acme/jose/jwk.py index 9454c6f9e..2b48d56e6 100644 --- a/acme/jose/jwk.py +++ b/acme/jose/jwk.py @@ -210,7 +210,7 @@ class JWKRSA(JWK): key = rsa.RSAPrivateNumbers( p, q, d, dp, dq, qi, public_numbers).private_key(default_backend()) - return cls(key=util.ComparableRSAKey(key)) + return cls(key=key) def fields_to_partial_json(self): # pylint: disable=protected-access diff --git a/acme/jose/jwk_test.py b/acme/jose/jwk_test.py index 06032f45b..5be28ba17 100644 --- a/acme/jose/jwk_test.py +++ b/acme/jose/jwk_test.py @@ -111,9 +111,8 @@ class JWKRSATest(unittest.TestCase): def test_load(self): from acme.jose.jwk import JWKRSA self.assertEqual( - JWKRSA(key=RSA256_KEY), JWKRSA.load( - pkg_resources.resource_string( - __name__, os.path.join('testdata', 'rsa256_key.pem')))) + self.private, JWKRSA.load(pkg_resources.resource_string( + __name__, os.path.join('testdata', 'rsa256_key.pem')))) def test_public_key(self): self.assertEqual(self.jwk256, self.private.public_key()) diff --git a/acme/jose/jws_test.py b/acme/jose/jws_test.py index 1520f149e..72b8b7b22 100644 --- a/acme/jose/jws_test.py +++ b/acme/jose/jws_test.py @@ -19,10 +19,10 @@ from acme.jose import util CERT = util.ComparableX509(OpenSSL.crypto.load_certificate( OpenSSL.crypto.FILETYPE_PEM, pkg_resources.resource_string( 'letsencrypt.tests', 'testdata/cert.pem'))) -RSA512_KEY = util.ComparableRSAKey(serialization.load_pem_private_key( +RSA512_KEY = serialization.load_pem_private_key( pkg_resources.resource_string( __name__, os.path.join('testdata', 'rsa512_key.pem')), - password=None, backend=default_backend())) + password=None, backend=default_backend()) class MediaTypeTest(unittest.TestCase): diff --git a/acme/jws_test.py b/acme/jws_test.py index 77e52ddc6..e65a3bd46 100644 --- a/acme/jws_test.py +++ b/acme/jws_test.py @@ -10,10 +10,10 @@ from acme import errors from acme import jose -RSA512_KEY = jose.ComparableRSAKey(serialization.load_pem_private_key( +RSA512_KEY = serialization.load_pem_private_key( pkg_resources.resource_string( 'acme.jose', os.path.join('testdata', 'rsa512_key.pem')), - password=None, backend=default_backend())) + password=None, backend=default_backend()) class HeaderTest(unittest.TestCase): diff --git a/acme/messages_test.py b/acme/messages_test.py index 1380fe2f2..59b1685dc 100644 --- a/acme/messages_test.py +++ b/acme/messages_test.py @@ -18,10 +18,10 @@ CERT = jose.ComparableX509(OpenSSL.crypto.load_certificate( CSR = jose.ComparableX509(OpenSSL.crypto.load_certificate_request( OpenSSL.crypto.FILETYPE_ASN1, pkg_resources.resource_string( 'acme.jose', os.path.join('testdata', 'csr.der')))) -KEY = jose.util.ComparableRSAKey(serialization.load_pem_private_key( +KEY = serialization.load_pem_private_key( pkg_resources.resource_string( 'acme.jose', os.path.join('testdata', 'rsa512_key.pem')), - password=None, backend=default_backend())) + password=None, backend=default_backend()) CERT = jose.ComparableX509(OpenSSL.crypto.load_certificate( OpenSSL.crypto.FILETYPE_ASN1, pkg_resources.resource_string( 'acme.jose', os.path.join('testdata', 'cert.der')))) diff --git a/acme/other_test.py b/acme/other_test.py index 22ef70a4c..64699038e 100644 --- a/acme/other_test.py +++ b/acme/other_test.py @@ -9,10 +9,10 @@ from cryptography.hazmat.primitives import serialization from acme import jose -KEY = jose.ComparableRSAKey(serialization.load_pem_private_key( +KEY = serialization.load_pem_private_key( pkg_resources.resource_string( 'acme.jose', os.path.join('testdata', 'rsa512_key.pem')), - password=None, backend=default_backend())) + password=None, backend=default_backend()) class SignatureTest(unittest.TestCase): diff --git a/examples/acme_client.py b/examples/acme_client.py index f7e3916d8..e07031fbe 100644 --- a/examples/acme_client.py +++ b/examples/acme_client.py @@ -20,10 +20,10 @@ BITS = 2048 # minimum for Boulder DOMAIN = 'example1.com' # example.com is ignored by Boulder # generate_private_key requires cryptography>=0.5 -key = jose.JWKRSA(key=jose.ComparableRSAKey(rsa.generate_private_key( +key = jose.JWKRSA(key=rsa.generate_private_key( public_exponent=65537, key_size=2048, - backend=default_backend()))) + backend=default_backend())) acme = client.Client(NEW_REG_URL, key) regr = acme.register(contact=()) diff --git a/letsencrypt/proof_of_possession.py b/letsencrypt/proof_of_possession.py index 09723dd96..f13238c85 100644 --- a/letsencrypt/proof_of_possession.py +++ b/letsencrypt/proof_of_possession.py @@ -57,9 +57,7 @@ class ProofOfPossession(object): # pylint: disable=too-few-public-methods except ValueError: logger.warn("Certificate is neither PER nor DER: %s", cert) - # TODO: only RSA is supported - cert_key = achall.alg.kty(key=jose.ComparableRSAKey( - cert_obj.public_key())) + cert_key = achall.alg.kty(key=cert_obj.public_key()) if cert_key == achall.hints.jwk: return self._gen_response(achall, key) diff --git a/letsencrypt/tests/acme_util.py b/letsencrypt/tests/acme_util.py index 4b660c648..8e19a9ca8 100644 --- a/letsencrypt/tests/acme_util.py +++ b/letsencrypt/tests/acme_util.py @@ -12,10 +12,10 @@ from acme import jose from acme import messages -KEY = jose.ComparableRSAKey(serialization.load_pem_private_key( +KEY = serialization.load_pem_private_key( pkg_resources.resource_string( __name__, os.path.join('testdata', 'rsa512_key.pem')), - password=None, backend=default_backend())) + password=None, backend=default_backend()) # Challenges SIMPLE_HTTP = challenges.SimpleHTTP( diff --git a/letsencrypt/tests/proof_of_possession_test.py b/letsencrypt/tests/proof_of_possession_test.py index 3013e2e12..d91b8bdb6 100644 --- a/letsencrypt/tests/proof_of_possession_test.py +++ b/letsencrypt/tests/proof_of_possession_test.py @@ -29,9 +29,9 @@ CERT3_PATH = pkg_resources.resource_filename( CERT3_KEY_PATH = pkg_resources.resource_filename( BASE_PACKAGE, os.path.join("testdata", "rsa512_key.pem")) with open(CERT3_KEY_PATH) as cert3_file: - CERT3_KEY = jose.ComparableRSAKey(serialization.load_pem_private_key( + CERT3_KEY = serialization.load_pem_private_key( cert3_file.read(), password=None, - backend=default_backend())).public_key() + backend=default_backend()).public_key() class ProofOfPossessionTest(unittest.TestCase):