From 2bc64183a8fe963959e91bfd87fb8f0e64b17650 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 11 Nov 2019 17:11:47 -0800 Subject: [PATCH 01/32] fix docstring --- certbot/plugins/common_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/certbot/plugins/common_test.py b/certbot/plugins/common_test.py index 977500f86..5d529e993 100644 --- a/certbot/plugins/common_test.py +++ b/certbot/plugins/common_test.py @@ -178,7 +178,7 @@ class InstallerTest(test_util.ConfigTestCase): class AddrTest(unittest.TestCase): - """Tests for certbot._internal.client.plugins.common.Addr.""" + """Tests for certbot._internal.plugins.common.Addr.""" def setUp(self): from certbot.plugins.common import Addr From 86926dff9293ab24d4f025ccf70c47619750d897 Mon Sep 17 00:00:00 2001 From: OsirisInferi Date: Tue, 4 Feb 2020 19:27:27 +0100 Subject: [PATCH 02/32] Use unrestrictive umask for challenge directory --- certbot-apache/certbot_apache/_internal/http_01.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/certbot-apache/certbot_apache/_internal/http_01.py b/certbot-apache/certbot_apache/_internal/http_01.py index c34abc2b4..53ccd2bc7 100644 --- a/certbot-apache/certbot_apache/_internal/http_01.py +++ b/certbot-apache/certbot_apache/_internal/http_01.py @@ -168,7 +168,9 @@ class ApacheHttp01(common.ChallengePerformer): def _set_up_challenges(self): if not os.path.isdir(self.challenge_dir): + old_umask = os.umask(0o022) filesystem.makedirs(self.challenge_dir, 0o755) + os.umask(old_umask) responses = [] for achall in self.achalls: From 601a114d1ba6030f3f765ff86bb39658172e0a75 Mon Sep 17 00:00:00 2001 From: OsirisInferi Date: Tue, 4 Feb 2020 19:47:27 +0100 Subject: [PATCH 03/32] Update changelog --- certbot/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md index 86d27143c..01cd3d402 100644 --- a/certbot/CHANGELOG.md +++ b/certbot/CHANGELOG.md @@ -19,6 +19,7 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). ### Fixed * Fix collections.abc imports for Python 3.9. +* Fix Apache plugin to use less restrictive umask for making the challenge directory when a restrictive umask was set when certbot was started. More details about these changes can be found on our GitHub repo. From f3ed13374456f3b53fc87dc0fa1ed71b1efa37e7 Mon Sep 17 00:00:00 2001 From: OsirisInferi Date: Wed, 5 Feb 2020 22:17:29 +0100 Subject: [PATCH 04/32] Wrap makedirs() within exception handelrs --- certbot-apache/certbot_apache/_internal/http_01.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/certbot-apache/certbot_apache/_internal/http_01.py b/certbot-apache/certbot_apache/_internal/http_01.py index 53ccd2bc7..ad62a77bb 100644 --- a/certbot-apache/certbot_apache/_internal/http_01.py +++ b/certbot-apache/certbot_apache/_internal/http_01.py @@ -169,8 +169,14 @@ class ApacheHttp01(common.ChallengePerformer): def _set_up_challenges(self): if not os.path.isdir(self.challenge_dir): old_umask = os.umask(0o022) - filesystem.makedirs(self.challenge_dir, 0o755) - os.umask(old_umask) + try: + filesystem.makedirs(self.challenge_dir, 0o755) + except OSError as exception: + if exception.errno not in (errno.EEXIST, errno.EISDIR): + raise errors.PluginError( + "Couldn't create root for http-01 challenge") + finally: + os.umask(old_umask) responses = [] for achall in self.achalls: From d3a4b8fd8c068624b40179f567e191b6979bf6cf Mon Sep 17 00:00:00 2001 From: OsirisInferi Date: Wed, 5 Feb 2020 22:27:12 +0100 Subject: [PATCH 05/32] Missing import --- certbot-apache/certbot_apache/_internal/http_01.py | 1 + 1 file changed, 1 insertion(+) diff --git a/certbot-apache/certbot_apache/_internal/http_01.py b/certbot-apache/certbot_apache/_internal/http_01.py index ad62a77bb..6c822cc38 100644 --- a/certbot-apache/certbot_apache/_internal/http_01.py +++ b/certbot-apache/certbot_apache/_internal/http_01.py @@ -1,5 +1,6 @@ """A class that performs HTTP-01 challenges for Apache""" import logging +import errno from acme.magic_typing import List # pylint: disable=unused-import, no-name-in-module from acme.magic_typing import Set # pylint: disable=unused-import, no-name-in-module From df584a3b90f9c18efaac07a5d7fb0bf9be5a81a9 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 12 Feb 2020 13:12:03 -0800 Subject: [PATCH 06/32] Remove _internal from docstring. --- certbot/tests/plugins/common_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/certbot/tests/plugins/common_test.py b/certbot/tests/plugins/common_test.py index fc05bf894..7543f28f3 100644 --- a/certbot/tests/plugins/common_test.py +++ b/certbot/tests/plugins/common_test.py @@ -177,7 +177,7 @@ class InstallerTest(test_util.ConfigTestCase): class AddrTest(unittest.TestCase): - """Tests for certbot._internal.plugins.common.Addr.""" + """Tests for certbot.plugins.common.Addr.""" def setUp(self): from certbot.plugins.common import Addr From 9819443440382695b74b77379d76e4886c0bdf70 Mon Sep 17 00:00:00 2001 From: osirisinferi Date: Sat, 22 Feb 2020 15:22:27 +0100 Subject: [PATCH 07/32] Add test --- certbot-apache/tests/http_01_test.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/certbot-apache/tests/http_01_test.py b/certbot-apache/tests/http_01_test.py index 643a6bdd5..422a76443 100644 --- a/certbot-apache/tests/http_01_test.py +++ b/certbot-apache/tests/http_01_test.py @@ -1,5 +1,6 @@ """Test for certbot_apache._internal.http_01.""" import unittest +import errno import mock @@ -197,6 +198,12 @@ class ApacheHttp01Test(util.ApacheTest): self.assertTrue(os.path.exists(challenge_dir)) + @mock.patch("certbot_apache._internal.http_01.filesystem.makedirs") + def test_failed_makedirs(self, mock_makedirs): + mock_makedirs.side_effect = OSError(errno.EACCES, "msg") + self.http.add_chall(self.achalls[0]) + self.assertRaises(errors.PluginError, self.http.perform) + def _test_challenge_conf(self): with open(self.http.challenge_conf_pre) as f: pre_conf_contents = f.read() From 2633c3ffb6a4f66933daef238b6a140ffc059818 Mon Sep 17 00:00:00 2001 From: alexzorin Date: Mon, 24 Feb 2020 07:49:42 +1100 Subject: [PATCH 08/32] acme: ignore params in content-type check (#7342) * acme: ignore params in content-type check Fixes the warning in #7339 * Suppress coverage complaint in test * Update CHANGELOG * Repair symlink Co-authored-by: Adrien Ferrand --- acme/acme/client.py | 3 +++ acme/tests/client_test.py | 29 +++++++++++++++++++++++++++++ certbot/CHANGELOG.md | 1 + 3 files changed, 33 insertions(+) diff --git a/acme/acme/client.py b/acme/acme/client.py index 3e03748b5..cecb727c7 100644 --- a/acme/acme/client.py +++ b/acme/acme/client.py @@ -1022,6 +1022,9 @@ class ClientNetwork(object): """ response_ct = response.headers.get('Content-Type') + # Strip parameters from the media-type (rfc2616#section-3.7) + if response_ct: + response_ct = response_ct.split(';')[0].strip() try: # TODO: response.json() is called twice, once here, and # once in _get and _post clients diff --git a/acme/tests/client_test.py b/acme/tests/client_test.py index a38fedbd6..a4966140f 100644 --- a/acme/tests/client_test.py +++ b/acme/tests/client_test.py @@ -980,6 +980,35 @@ class ClientNetworkTest(unittest.TestCase): self.assertEqual( self.response, self.net._check_response(self.response)) + @mock.patch('acme.client.logger') + def test_check_response_ok_ct_with_charset(self, mock_logger): + self.response.json.return_value = {} + self.response.headers['Content-Type'] = 'application/json; charset=utf-8' + # pylint: disable=protected-access + self.assertEqual(self.response, self.net._check_response( + self.response, content_type='application/json')) + try: + mock_logger.debug.assert_called_with( + 'Ignoring wrong Content-Type (%r) for JSON decodable response', + 'application/json; charset=utf-8' + ) + except AssertionError: + return + raise AssertionError('Expected Content-Type warning ' #pragma: no cover + 'to not have been logged') + + @mock.patch('acme.client.logger') + def test_check_response_ok_bad_ct(self, mock_logger): + self.response.json.return_value = {} + self.response.headers['Content-Type'] = 'text/plain' + # pylint: disable=protected-access + self.assertEqual(self.response, self.net._check_response( + self.response, content_type='application/json')) + mock_logger.debug.assert_called_with( + 'Ignoring wrong Content-Type (%r) for JSON decodable response', + 'text/plain' + ) + def test_check_response_conflict(self): self.response.ok = False self.response.status_code = 409 diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md index 126b07eec..bc5ad90d6 100644 --- a/certbot/CHANGELOG.md +++ b/certbot/CHANGELOG.md @@ -14,6 +14,7 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). ### Changed * certbot._internal.cli is now a package split in submodules instead of a whole module. +* Fix acme module warnings when response Content-Type includes params (e.g. charset). ### Fixed From 4fd04366aad02e2fa51057d4912346bde9e39d02 Mon Sep 17 00:00:00 2001 From: martin-c Date: Sun, 23 Feb 2020 22:14:51 +0100 Subject: [PATCH 09/32] Fix issue #7165 in _create_challenge_dirs(), attempt to fix pylint errors (#7568) * fix issue #7165 by checking if directory exists before trying to create it, fix possible pylint issues in webroot.py * fix get_chall_pref definition * Update CHANGELOG.md * Update CHANGELOG.md Co-authored-by: Adrien Ferrand --- certbot/CHANGELOG.md | 2 ++ certbot/certbot/_internal/plugins/webroot.py | 18 ++++++++++-------- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md index bc5ad90d6..30479f25b 100644 --- a/certbot/CHANGELOG.md +++ b/certbot/CHANGELOG.md @@ -15,6 +15,8 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). * certbot._internal.cli is now a package split in submodules instead of a whole module. * Fix acme module warnings when response Content-Type includes params (e.g. charset). +* Fixed issue where webroot plugin would incorrectly raise `Read-only file system` + error when creating challenge directories (issue #7165). ### Fixed diff --git a/certbot/certbot/_internal/plugins/webroot.py b/certbot/certbot/_internal/plugins/webroot.py index 042b60656..9383ce66d 100644 --- a/certbot/certbot/_internal/plugins/webroot.py +++ b/certbot/certbot/_internal/plugins/webroot.py @@ -1,7 +1,6 @@ """Webroot plugin.""" import argparse import collections -import errno import json import logging @@ -71,7 +70,7 @@ to serve all files under specified web root ({0}).""" super(Authenticator, self).__init__(*args, **kwargs) self.full_roots = {} # type: Dict[str, str] self.performed = collections.defaultdict(set) \ - # type: DefaultDict[str, Set[achallenges.KeyAuthorizationAnnotatedChallenge]] + # type: DefaultDict[str, Set[achallenges.KeyAuthorizationAnnotatedChallenge]] # stack of dirs successfully created by this authenticator self._created_dirs = [] # type: List[str] @@ -137,7 +136,7 @@ to serve all files under specified web root ({0}).""" "webroot when using the webroot plugin.") return None if index == 0 else known_webroots[index - 1] # code == display_util.OK - def _prompt_for_new_webroot(self, domain, allowraise=False): + def _prompt_for_new_webroot(self, domain, allowraise=False): # pylint: no-self-use code, webroot = ops.validated_directory( _validate_webroot, "Input the webroot for {0}:".format(domain), @@ -170,6 +169,10 @@ to serve all files under specified web root ({0}).""" # We ignore the last prefix in the next iteration, # as it does not correspond to a folder path ('/' or 'C:') for prefix in sorted(util.get_prefixes(self.full_roots[name])[:-1], key=len): + if os.path.isdir(prefix): + # Don't try to create directory if it already exists, as some filesystems + # won't reliably raise EEXIST or EISDIR if directory exists. + continue try: # Set owner as parent directory if possible, apply mode for Linux/Windows. # For Linux, this is coupled with the "umask" call above because @@ -184,14 +187,13 @@ to serve all files under specified web root ({0}).""" logger.info("Unable to change owner and uid of webroot directory") logger.debug("Error was: %s", exception) except OSError as exception: - if exception.errno not in (errno.EEXIST, errno.EISDIR): - raise errors.PluginError( - "Couldn't create root for {0} http-01 " - "challenge responses: {1}".format(name, exception)) + raise errors.PluginError( + "Couldn't create root for {0} http-01 " + "challenge responses: {1}".format(name, exception)) finally: os.umask(old_umask) - def _get_validation_path(self, root_path, achall): + def _get_validation_path(self, root_path, achall): # pylint: no-self-use return os.path.join(root_path, achall.chall.encode("token")) def _perform_single(self, achall): From 4ea98d830bcc3d1b980a4055243c6a6a25d8dc54 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 24 Feb 2020 12:31:16 -0800 Subject: [PATCH 10/32] remove _internal docs (#7801) --- certbot/CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md index 30479f25b..57fbc820b 100644 --- a/certbot/CHANGELOG.md +++ b/certbot/CHANGELOG.md @@ -13,7 +13,6 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). ### Changed -* certbot._internal.cli is now a package split in submodules instead of a whole module. * Fix acme module warnings when response Content-Type includes params (e.g. charset). * Fixed issue where webroot plugin would incorrectly raise `Read-only file system` error when creating challenge directories (issue #7165). From f4c0a9fd63c9be4cd4e745dd5f701040bcd14682 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 27 Feb 2020 10:43:41 -0800 Subject: [PATCH 11/32] Split advanced pipeline (#7813) I want to do what I did in https://github.com/certbot/certbot/pull/7733 to our Azure Pipelines setup, but unfortunately this isn't currently possible. The only filters available for service hooks for the "build completed" trigger are the pipeline and build status. See ![Screen Shot 2020-02-26 at 3 04 56 PM](https://user-images.githubusercontent.com/6504915/75396464-64ad0780-58a9-11ea-97a1-3454a9754675.png) To accomplish this, I propose splitting the "advanced" pipeline into two cases. One is for builds on protected branches where we want to be notified if they fail while the other is just used to manually run tests on certain branches. --- .azure-pipelines/advanced-test.yml | 12 ++++++++++++ .azure-pipelines/advanced.yml | 10 ++-------- .azure-pipelines/release.yml | 2 +- 3 files changed, 15 insertions(+), 9 deletions(-) create mode 100644 .azure-pipelines/advanced-test.yml diff --git a/.azure-pipelines/advanced-test.yml b/.azure-pipelines/advanced-test.yml new file mode 100644 index 000000000..b9ac9c38a --- /dev/null +++ b/.azure-pipelines/advanced-test.yml @@ -0,0 +1,12 @@ +# Advanced pipeline for running our full test suite on demand. +trigger: + # When changing these triggers, please ensure the documentation under + # "Running tests in CI" is still correct. + - azure-test-* + - test-* + +jobs: + # Any addition here should be reflected in the advanced and release pipelines. + # It is advised to declare all jobs here as templates to improve maintainability. + - template: templates/tests-suite.yml + - template: templates/installer-tests.yml diff --git a/.azure-pipelines/advanced.yml b/.azure-pipelines/advanced.yml index dda7f9bfd..7f0f5de50 100644 --- a/.azure-pipelines/advanced.yml +++ b/.azure-pipelines/advanced.yml @@ -1,12 +1,6 @@ -# Advanced pipeline for isolated checks and release purpose +# Advanced pipeline for running our full test suite on protected branches. trigger: - # When changing these triggers, please ensure the documentation under - # "Running tests in CI" is still correct. - - azure-test-* - - test-* - '*.x' -pr: - - test-* # This pipeline is also nightly run on master schedules: - cron: "0 4 * * *" @@ -17,7 +11,7 @@ schedules: always: true jobs: - # Any addition here should be reflected in the release pipeline. + # Any addition here should be reflected in the advanced-test and release pipelines. # It is advised to declare all jobs here as templates to improve maintainability. - template: templates/tests-suite.yml - template: templates/installer-tests.yml diff --git a/.azure-pipelines/release.yml b/.azure-pipelines/release.yml index aeb5ee327..e9acbc69a 100644 --- a/.azure-pipelines/release.yml +++ b/.azure-pipelines/release.yml @@ -6,7 +6,7 @@ trigger: pr: none jobs: - # Any addition here should be reflected in the advanced pipeline. + # Any addition here should be reflected in the advanced and advanced-test pipelines. # It is advised to declare all jobs here as templates to improve maintainability. - template: templates/tests-suite.yml - template: templates/installer-tests.yml From 24aa1e9127802f9c6ac459bbf91e6ff9b4595483 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 27 Feb 2020 10:47:43 -0800 Subject: [PATCH 12/32] update letstest reqs (#7809) I don't fully understand why, but since I updated my macbook to macOS Catalina, the test script currently fails to run for me with the versions of our dependencies we have pinned. Updating the dependencies solves the problem though and you can see Travis also successfully running tests with these new dependencies at https://travis-ci.com/certbot/certbot/builds/150573696. --- tests/letstest/requirements.txt | 38 ++++++++++++++------------------- 1 file changed, 16 insertions(+), 22 deletions(-) diff --git a/tests/letstest/requirements.txt b/tests/letstest/requirements.txt index 64e1f6a0c..24bd77331 100644 --- a/tests/letstest/requirements.txt +++ b/tests/letstest/requirements.txt @@ -1,25 +1,19 @@ -asn1crypto==0.24.0 -awscli==1.16.157 -bcrypt==3.1.6 -boto3==1.9.146 -botocore==1.12.147 -cffi==1.12.3 -colorama==0.3.9 -cryptography==2.4.2 -docutils==0.14 -enum34==1.1.6 +bcrypt==3.1.7 +boto3==1.12.7 +botocore==1.15.7 +cffi==1.14.0 +cryptography==2.8 +docutils==0.15.2 +enum34==1.1.9 Fabric==1.14.1 -futures==3.2.0 -idna==2.8 -ipaddress==1.0.22 -jmespath==0.9.4 -paramiko==2.4.2 -pyasn1==0.4.5 +futures==3.3.0 +ipaddress==1.0.23 +jmespath==0.9.5 +paramiko==2.7.1 pycparser==2.19 PyNaCl==1.3.0 -python-dateutil==2.8.0 -PyYAML==3.10 -rsa==3.4.2 -s3transfer==0.2.0 -six==1.12.0 -urllib3==1.24.3 +python-dateutil==2.8.1 +PyYAML==5.3 +s3transfer==0.3.3 +six==1.14.0 +urllib3==1.25.8 From 8c75a9de9fe28ca0e4baf8686620a9c6b5733515 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 27 Feb 2020 10:47:56 -0800 Subject: [PATCH 13/32] Remove unused notify code. (#7805) This code is unused and hasn't been modified since 2015 except for various times our files have been renamed. Let's remove it. --- certbot/certbot/_internal/notify.py | 34 ------------------- certbot/tests/notify_test.py | 52 ----------------------------- 2 files changed, 86 deletions(-) delete mode 100644 certbot/certbot/_internal/notify.py delete mode 100644 certbot/tests/notify_test.py diff --git a/certbot/certbot/_internal/notify.py b/certbot/certbot/_internal/notify.py deleted file mode 100644 index dda0a85af..000000000 --- a/certbot/certbot/_internal/notify.py +++ /dev/null @@ -1,34 +0,0 @@ -"""Send e-mail notification to system administrators.""" - -import email -import smtplib -import socket -import subprocess - - -def notify(subject, whom, what): - """Send email notification. - - Try to notify the addressee (``whom``) by e-mail, with Subject: - defined by ``subject`` and message body by ``what``. - - """ - msg = email.message_from_string(what) - msg.add_header("From", "Certbot renewal agent ") - msg.add_header("To", whom) - msg.add_header("Subject", subject) - msg = msg.as_string() - try: - lmtp = smtplib.LMTP() - lmtp.connect() - lmtp.sendmail("root", [whom], msg) - except (smtplib.SMTPHeloError, smtplib.SMTPRecipientsRefused, - smtplib.SMTPSenderRefused, smtplib.SMTPDataError, socket.error): - # We should try using /usr/sbin/sendmail in this case - try: - proc = subprocess.Popen(["/usr/sbin/sendmail", "-t"], - stdin=subprocess.PIPE) - proc.communicate(msg) - except OSError: - return False - return True diff --git a/certbot/tests/notify_test.py b/certbot/tests/notify_test.py deleted file mode 100644 index d6f7d2239..000000000 --- a/certbot/tests/notify_test.py +++ /dev/null @@ -1,52 +0,0 @@ -"""Tests for certbot._internal.notify.""" -import socket -import unittest - -import mock - - -class NotifyTests(unittest.TestCase): - """Tests for the notifier.""" - - @mock.patch("certbot._internal.notify.smtplib.LMTP") - def test_smtp_success(self, mock_lmtp): - from certbot._internal.notify import notify - lmtp_obj = mock.MagicMock() - mock_lmtp.return_value = lmtp_obj - self.assertTrue(notify("Goose", "auntrhody@example.com", - "The old grey goose is dead.")) - self.assertEqual(lmtp_obj.connect.call_count, 1) - self.assertEqual(lmtp_obj.sendmail.call_count, 1) - - @mock.patch("certbot._internal.notify.smtplib.LMTP") - @mock.patch("certbot._internal.notify.subprocess.Popen") - def test_smtp_failure(self, mock_popen, mock_lmtp): - from certbot._internal.notify import notify - lmtp_obj = mock.MagicMock() - mock_lmtp.return_value = lmtp_obj - lmtp_obj.sendmail.side_effect = socket.error(17) - proc = mock.MagicMock() - mock_popen.return_value = proc - self.assertTrue(notify("Goose", "auntrhody@example.com", - "The old grey goose is dead.")) - self.assertEqual(lmtp_obj.sendmail.call_count, 1) - self.assertEqual(proc.communicate.call_count, 1) - - @mock.patch("certbot._internal.notify.smtplib.LMTP") - @mock.patch("certbot._internal.notify.subprocess.Popen") - def test_everything_fails(self, mock_popen, mock_lmtp): - from certbot._internal.notify import notify - lmtp_obj = mock.MagicMock() - mock_lmtp.return_value = lmtp_obj - lmtp_obj.sendmail.side_effect = socket.error(17) - proc = mock.MagicMock() - mock_popen.return_value = proc - proc.communicate.side_effect = OSError("What we have here is a " - "failure to communicate.") - self.assertFalse(notify("Goose", "auntrhody@example.com", - "The old grey goose is dead.")) - self.assertEqual(lmtp_obj.sendmail.call_count, 1) - self.assertEqual(proc.communicate.call_count, 1) - -if __name__ == "__main__": - unittest.main() # pragma: no cover From 2f737ee292680e2f8043e0dfe3affcccc03914e8 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 27 Feb 2020 10:49:50 -0800 Subject: [PATCH 14/32] Change how _USE_DISTRO is set for mypy (#7804) If you run `mypy --platform darwin certbot/certbot/util.py` you'll get: ``` certbot/certbot/util.py:303: error: Name 'distro' is not defined certbot/certbot/util.py:319: error: Name 'distro' is not defined certbot/certbot/util.py:369: error: Name 'distro' is not defined ``` This is because mypy's logic for handling platform specific code is pretty simple and can't figure out what we're doing with `_USE_DISTRO` here. See https://mypy.readthedocs.io/en/stable/common_issues.html#python-version-and-system-platform-checks for more info. Setting `_USE_DISTRO` to the result of `sys.platform.startswith('linux')` solves the problem without changing the overall behavior of our code here though. This fixes part of https://github.com/certbot/certbot/issues/7803, but there's more work to be done on Windows. --- certbot/certbot/util.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/certbot/certbot/util.py b/certbot/certbot/util.py index aff2952f7..e69b11543 100644 --- a/certbot/certbot/util.py +++ b/certbot/certbot/util.py @@ -25,11 +25,9 @@ from certbot._internal import lock from certbot.compat import filesystem from certbot.compat import os -if sys.platform.startswith('linux'): +_USE_DISTRO = sys.platform.startswith('linux') +if _USE_DISTRO: import distro - _USE_DISTRO = True -else: - _USE_DISTRO = False logger = logging.getLogger(__name__) From a2be8e1956c79662fd28d8b8af4802ea89cf29bf Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 27 Feb 2020 10:50:20 -0800 Subject: [PATCH 15/32] Fix tests on macOS Catalina (#7794) This PR fixes the failures that can be seen at https://dev.azure.com/certbot/certbot/_build/results?buildId=1184&view=results. You can see this code running on macOS Catalina at https://dev.azure.com/certbot/certbot/_build/results?buildId=1192&view=results. --- certbot/tests/cli_test.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/certbot/tests/cli_test.py b/certbot/tests/cli_test.py index 3a7fb57f8..be2c8f29e 100644 --- a/certbot/tests/cli_test.py +++ b/certbot/tests/cli_test.py @@ -30,15 +30,23 @@ class TestReadFile(TempDirTestCase): # However a relative path between two different drives is invalid. So we move to # self.tempdir to ensure that we stay on the same drive. os.chdir(self.tempdir) - rel_test_path = os.path.relpath(os.path.join(self.tempdir, 'foo')) + # The read-only filesystem introduced with macOS Catalina can break + # code using relative paths below. See + # https://bugs.python.org/issue38295 for another example of this. + # Eliminating any possible symlinks in self.tempdir before passing + # it to os.path.relpath solves the problem. This is done by calling + # filesystem.realpath which removes any symlinks in the path on + # POSIX systems. + real_path = filesystem.realpath(os.path.join(self.tempdir, 'foo')) + relative_path = os.path.relpath(real_path) self.assertRaises( - argparse.ArgumentTypeError, cli.read_file, rel_test_path) + argparse.ArgumentTypeError, cli.read_file, relative_path) test_contents = b'bar\n' - with open(rel_test_path, 'wb') as f: + with open(relative_path, 'wb') as f: f.write(test_contents) - path, contents = cli.read_file(rel_test_path) + path, contents = cli.read_file(relative_path) self.assertEqual(path, os.path.abspath(path)) self.assertEqual(contents, test_contents) finally: From 6309ded92f03104f2baa9b881db9827f5fe11e4c Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 27 Feb 2020 14:43:28 -0800 Subject: [PATCH 16/32] Remove references to deprecated flags in Certbot. (#7509) Related to https://github.com/certbot/certbot/pull/7482, this removes some references to deprecated options in Certbot. The only references I didn't remove were: * In `certbot/tests/testdata/sample-renewal*` which contains a lot of old values and I think there's even some value in keeping them so we know if we make a change that suddenly causes old renewal configuration files to error. * In the Apache and Nginx plugins and I created https://github.com/certbot/certbot/issues/7508 to resolve that issue. --- certbot/certbot/_internal/main.py | 2 +- certbot/certbot/display/ops.py | 2 +- certbot/tests/cli_test.py | 2 -- 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/certbot/certbot/_internal/main.py b/certbot/certbot/_internal/main.py index 8674cd151..4a57dd78d 100644 --- a/certbot/certbot/_internal/main.py +++ b/certbot/certbot/_internal/main.py @@ -394,7 +394,7 @@ def _find_domains_or_certname(config, installer, question=None): :param installer: Installer object :type installer: interfaces.IInstaller - :param `str` question: Overriding dialog question to ask the user if asked + :param `str` question: Overriding default question to ask the user if asked to choose from domain names. :returns: Two-part tuple of domains and certname diff --git a/certbot/certbot/display/ops.py b/certbot/certbot/display/ops.py index eab9d251d..f24f6ed99 100644 --- a/certbot/certbot/display/ops.py +++ b/certbot/certbot/display/ops.py @@ -107,7 +107,7 @@ def choose_names(installer, question=None): :param installer: An installer object :type installer: :class:`certbot.interfaces.IInstaller` - :param `str` question: Overriding dialog question to ask the user if asked + :param `str` question: Overriding default question to ask the user if asked to choose from domain names. :returns: List of selected names diff --git a/certbot/tests/cli_test.py b/certbot/tests/cli_test.py index be2c8f29e..7d21f8bb8 100644 --- a/certbot/tests/cli_test.py +++ b/certbot/tests/cli_test.py @@ -150,7 +150,6 @@ class ParseTest(unittest.TestCase): self.assertTrue("how a certificate is deployed" in out) self.assertTrue("--webroot-path" in out) self.assertTrue("--text" not in out) - self.assertTrue("--dialog" not in out) self.assertTrue("%s" not in out) self.assertTrue("{0}" not in out) self.assertTrue("--renew-hook" not in out) @@ -211,7 +210,6 @@ class ParseTest(unittest.TestCase): self.assertTrue("how a certificate is deployed" in out) self.assertTrue("--webroot-path" in out) self.assertTrue("--text" not in out) - self.assertTrue("--dialog" not in out) self.assertTrue("%s" not in out) self.assertTrue("{0}" not in out) From fa67b7ba0fb03453fc8d03e3631d6782a54a233b Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 27 Feb 2020 14:44:39 -0800 Subject: [PATCH 17/32] Remove codecov (#7811) After getting a +1 from everyone on the team, this PR removes the use of `codecov` from the Certbot repo because we keep having problems with it. Two noteworthy things about this PR are: 1. I left the text at https://github.com/certbot/certbot/blob/4ea98d830bcc3d1b980a4055243c6a6a25d8dc54/.azure-pipelines/INSTALL.md#add-a-secret-variable-to-a-pipeline-like-codecov_token because I think it's useful to document how to set up a secret variable in general. 2. I'm not sure what the text "Option -e makes sure we fail fast and don't submit to codecov." in `tox.cover.py` refers to but it seems incorrect since `-e` isn't accepted or used by the script so I just deleted the line. As part of this, I said I'd open an issue to track setting up coveralls (which seems to be the only real alternative to codecov) which is at https://github.com/certbot/certbot/issues/7810. With my change, failure output looks something like: ``` $ tox -e py27-cover ... Name Stmts Miss Cover Missing ------------------------------------------------------------------------------------------ certbot/certbot/__init__.py 1 0 100% certbot/certbot/_internal/__init__.py 0 0 100% certbot/certbot/_internal/account.py 191 4 98% 62-63, 206, 337 ... certbot/tests/storage_test.py 530 0 100% certbot/tests/util_test.py 374 29 92% 211-213, 480-484, 489-499, 504-511, 545-547, 552-554 ------------------------------------------------------------------------------------------ TOTAL 14451 647 96% Command '['/path/to/certbot/dir/.tox/py27-cover/bin/python', '-m', 'coverage', 'report', '--fail-under', '100', '--include', 'certbot/*', '--show-missing']' returned non-zero exit status 2 Test coverage on certbot did not meet threshold of 100%. ERROR: InvocationError for command /Users/bmw/Development/certbot/certbot/.tox/py27-cover/bin/python tox.cover.py (exited with code 1) _________________________________________________________________________________________________________________________________________________________ summary _________________________________________________________________________________________________________________________________________________________ ERROR: py27-cover: commands failed ``` I printed the exception just so we're not throwing away information. I think it's also possible we fail for a reason other than the threshold not meeting the percentage, but I've personally never seen this, `coverage report` output is not being captured so hopefully that would inform devs if something else is going on, and saying something like "Test coverage probably did not..." seems like overkill to me personally. * remove codecov * remove unused variable group * remove codecov.yml * Improve tox.cover.py failure output. --- .azure-pipelines/templates/tests-suite.yml | 13 ------------- .codecov.yml | 18 ------------------ .travis.yml | 4 +--- certbot/README.rst | 6 +----- tools/dev_constraints.txt | 1 - tox.cover.py | 19 +++++++++++++------ 6 files changed, 15 insertions(+), 46 deletions(-) delete mode 100644 .codecov.yml diff --git a/.azure-pipelines/templates/tests-suite.yml b/.azure-pipelines/templates/tests-suite.yml index 069ea94d6..d330b7954 100644 --- a/.azure-pipelines/templates/tests-suite.yml +++ b/.azure-pipelines/templates/tests-suite.yml @@ -25,8 +25,6 @@ jobs: PYTEST_ADDOPTS: --numprocesses 4 pool: vmImage: $(IMAGE_NAME) - variables: - - group: certbot-common steps: - bash: brew install augeas condition: startswith(variables['IMAGE_NAME'], 'macOS') @@ -39,14 +37,3 @@ jobs: displayName: Install dependencies - script: python -m tox displayName: Run tox - # We do not require codecov report upload to succeed. So to avoid to break the pipeline if - # something goes wrong, each command is suffixed with a command that hides any non zero exit - # codes and echoes an informative message instead. - - bash: | - curl -s https://codecov.io/bash -o codecov-bash || echo "Failed to download codecov-bash" - chmod +x codecov-bash || echo "Failed to apply execute permissions on codecov-bash" - ./codecov-bash -F windows || echo "Codecov did not collect coverage reports" - condition: in(variables['TOXENV'], 'py37-cover', 'integration-certbot') - env: - CODECOV_TOKEN: $(codecov_token) - displayName: Publish coverage diff --git a/.codecov.yml b/.codecov.yml deleted file mode 100644 index 0a97fffe3..000000000 --- a/.codecov.yml +++ /dev/null @@ -1,18 +0,0 @@ -coverage: - status: - project: - default: off - linux: - flags: linux - # Fixed target instead of auto set by #7173, can - # be removed when flags in Codecov are added back. - target: 97.4 - threshold: 0.1 - base: auto - windows: - flags: windows - # Fixed target instead of auto set by #7173, can - # be removed when flags in Codecov are added back. - target: 97.4 - threshold: 0.1 - base: auto diff --git a/.travis.yml b/.travis.yml index e5354898d..d498d0305 100644 --- a/.travis.yml +++ b/.travis.yml @@ -247,15 +247,13 @@ addons: # version of virtualenv. The option "-I" is set so when CERTBOT_NO_PIN is also # set, pip updates dependencies it thinks are already satisfied to avoid some # problems with its lack of real dependency resolution. -install: 'tools/pip_install.py -I codecov tox virtualenv' +install: 'tools/pip_install.py -I tox virtualenv' # Most of the time TRAVIS_RETRY is an empty string, and has no effect on the # script command. It is set only to `travis_retry` during farm tests, in # order to trigger the Travis retry feature, and compensate the inherent # flakiness of these specific tests. script: '$TRAVIS_RETRY tox' -after_success: '[ "$TOXENV" == "py27-cover" ] && codecov -F linux' - notifications: email: false irc: diff --git a/certbot/README.rst b/certbot/README.rst index d1b1e4fe2..5ed74f247 100644 --- a/certbot/README.rst +++ b/certbot/README.rst @@ -71,16 +71,12 @@ ACME spec: http://ietf-wg-acme.github.io/acme/ ACME working area in github: https://github.com/ietf-wg-acme/acme -|build-status| |coverage| |container| +|build-status| |container| .. |build-status| image:: https://travis-ci.com/certbot/certbot.svg?branch=master :target: https://travis-ci.com/certbot/certbot :alt: Travis CI status -.. |coverage| image:: https://codecov.io/gh/certbot/certbot/branch/master/graph/badge.svg - :target: https://codecov.io/gh/certbot/certbot - :alt: Coverage status - .. |container| image:: https://quay.io/repository/letsencrypt/letsencrypt/status :target: https://quay.io/repository/letsencrypt/letsencrypt :alt: Docker Repository on Quay.io diff --git a/tools/dev_constraints.txt b/tools/dev_constraints.txt index 7d2013c7a..cfa036435 100644 --- a/tools/dev_constraints.txt +++ b/tools/dev_constraints.txt @@ -18,7 +18,6 @@ boto3==1.11.7 botocore==1.14.7 cached-property==1.5.1 cloudflare==2.3.1 -codecov==2.0.15 configparser==3.7.4 contextlib2==0.6.0.post1 coverage==4.5.4 diff --git a/tox.cover.py b/tox.cover.py index 3e69a14d6..4848b2740 100755 --- a/tox.cover.py +++ b/tox.cover.py @@ -1,4 +1,6 @@ #!/usr/bin/env python +from __future__ import print_function + import argparse import os import subprocess @@ -48,18 +50,23 @@ def cover(package): subprocess.check_call([sys.executable, '-m', 'pytest', '--cov', pkg_dir, '--cov-append', '--cov-report=', pkg_dir]) - subprocess.check_call([ - sys.executable, '-m', 'coverage', 'report', '--fail-under', str(threshold), '--include', - '{0}/*'.format(pkg_dir), '--show-missing']) + try: + subprocess.check_call([ + sys.executable, '-m', 'coverage', 'report', '--fail-under', + str(threshold), '--include', '{0}/*'.format(pkg_dir), + '--show-missing']) + except subprocess.CalledProcessError as err: + print(err) + print('Test coverage on', pkg_dir, + 'did not meet threshold of {0}%.'.format(threshold)) + sys.exit(1) def main(): description = """ This script is used by tox.ini (and thus by Travis CI and Azure Pipelines) in order to generate separate stats for each package. It should be removed once -those packages are moved to a separate repo. - -Option -e makes sure we fail fast and don't submit to codecov.""" +those packages are moved to a separate repo.""" parser = argparse.ArgumentParser(description=description) parser.add_argument('--packages', nargs='+') From 50ea6085537dfec3bceaa4f9f4e4065de84d1407 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 27 Feb 2020 15:07:33 -0800 Subject: [PATCH 18/32] Don't run advanced tests on PRs. (#7820) When I wrote https://github.com/certbot/certbot/pull/7813, I didn't understand the default behavior for pull requests if you don't specify `pr` in the yaml file. According to https://docs.microsoft.com/en-us/azure/devops/pipelines/build/triggers?view=azure-devops&tabs=yaml#pr-triggers: > If no pr triggers appear in your YAML file, pull request builds are automatically enabled for all branches... This is not the behavior we want. This PR fixes the problem by disabling builds on PRs. You should be able to see this working because the advanced tests should not run on this PR but they did run on https://github.com/certbot/certbot/pull/7811. --- .azure-pipelines/advanced-test.yml | 1 + .azure-pipelines/advanced.yml | 1 + 2 files changed, 2 insertions(+) diff --git a/.azure-pipelines/advanced-test.yml b/.azure-pipelines/advanced-test.yml index b9ac9c38a..5be29ba79 100644 --- a/.azure-pipelines/advanced-test.yml +++ b/.azure-pipelines/advanced-test.yml @@ -4,6 +4,7 @@ trigger: # "Running tests in CI" is still correct. - azure-test-* - test-* +pr: none jobs: # Any addition here should be reflected in the advanced and release pipelines. diff --git a/.azure-pipelines/advanced.yml b/.azure-pipelines/advanced.yml index 7f0f5de50..d950e6524 100644 --- a/.azure-pipelines/advanced.yml +++ b/.azure-pipelines/advanced.yml @@ -1,6 +1,7 @@ # Advanced pipeline for running our full test suite on protected branches. trigger: - '*.x' +pr: none # This pipeline is also nightly run on master schedules: - cron: "0 4 * * *" From 9f8e4507ad0cb3dbedb726dda4c46affb1eb7ad3 Mon Sep 17 00:00:00 2001 From: Michael Brown Date: Fri, 28 Feb 2020 00:44:23 +0000 Subject: [PATCH 19/32] Document safe and simple usage by services without root privileges (#7821) Certificates are public information by design: they are provided by web servers without any prior authentication required. In a public key cryptographic system, only the private key is secret information. The private key file is already created as accessible only to the root user with mode 0600, and these file permissions are set before any key content is written to the file. There is no window within which an attacker with access to the containing directory would be able to read the private key content. Older versions of Certbot (prior to 0.29.0) would create private key files with mode 0644 and rely solely on the containing directory permissions to restrict access. We therefore cannot (yet) set the relevant default directory permissions to 0755, since it is possible that a user could install Certbot, obtain a certificate, then downgrade to a pre-0.29.0 version of Certbot, then obtain another certificate. This chain of events would leave the second certificate's private key file exposed. As a compromise solution, document the fact that it is safe for the common case of non-downgrading users to change the permissions of /etc/letsencrypt/{live,archive} to 0755, and explain how to use chgrp and chmod to make the private key file readable by a non-root service user. This provides guidance on the simplest way to solve the common problem of making keys and certificates usable by services that run without root privileges, with no requirement to create a custom (and hence error-prone) executable hook. Remove the existing custom executable hook example, so that the documentation contains only the simplest and safest way to solve this very common problem. Signed-off-by: Michael Brown --- certbot/docs/using.rst | 48 ++++++++++-------------------------------- 1 file changed, 11 insertions(+), 37 deletions(-) diff --git a/certbot/docs/using.rst b/certbot/docs/using.rst index 27ae826bd..8ec172c24 100644 --- a/certbot/docs/using.rst +++ b/certbot/docs/using.rst @@ -485,43 +485,6 @@ If you want your hook to run only after a successful renewal, use ``certbot renew --deploy-hook /path/to/deploy-hook-script`` -For example, if you have a daemon that does not read its certificates as the -root user, a deploy hook like this can copy them to the correct location and -apply appropriate file permissions. - -/path/to/deploy-hook-script - -.. code-block:: none - - #!/bin/sh - - set -e - - for domain in $RENEWED_DOMAINS; do - case $domain in - example.com) - daemon_cert_root=/etc/some-daemon/certs - - # Make sure the certificate and private key files are - # never world readable, even just for an instant while - # we're copying them into daemon_cert_root. - umask 077 - - cp "$RENEWED_LINEAGE/fullchain.pem" "$daemon_cert_root/$domain.cert" - cp "$RENEWED_LINEAGE/privkey.pem" "$daemon_cert_root/$domain.key" - - # Apply the proper file ownership and permissions for - # the daemon to read its certificate and key. - chown some-daemon "$daemon_cert_root/$domain.cert" \ - "$daemon_cert_root/$domain.key" - chmod 400 "$daemon_cert_root/$domain.cert" \ - "$daemon_cert_root/$domain.key" - - service some-daemon restart >/dev/null - ;; - esac - done - You can also specify hooks by placing files in subdirectories of Certbot's configuration directory. Assuming your configuration directory is ``/etc/letsencrypt``, any executable files found in @@ -686,6 +649,17 @@ your (web) server configuration directly to those files (or create symlinks). During the renewal_, ``/etc/letsencrypt/live`` is updated with the latest necessary files. +For historical reasons, the containing directories are created with +permissions of ``0700`` meaning that certificates are accessible only +to servers that run as the root user. **If you will never downgrade +to an older version of Certbot**, then you can safely fix this using +``chmod 0755 /etc/letsencrypt/{live,archive}``. + +For servers that drop root privileges before attempting to read the +private key file, you will also need to use ``chgrp`` and ``chmod +0640`` to allow the server to read +``/etc/letsencrypt/live/$domain/privkey.pem``. + .. note:: ``/etc/letsencrypt/archive`` and ``/etc/letsencrypt/keys`` contain all previous keys and certificates, while ``/etc/letsencrypt/live`` symlinks to the latest versions. From 31470262110ab8e9a388d738461b08a4f489d430 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 3 Mar 2020 11:07:15 -0800 Subject: [PATCH 20/32] Check OCSP as part of determining if the certificate is due for renewal (#7829) Fixes #1028. Doing this now because of https://community.letsencrypt.org/t/revoking-certain-certificates-on-march-4/. The new `ocsp_revoked_by_paths` function is taken from https://github.com/certbot/certbot/pull/7649 with the optional argument removed for now because it is unused. This function was added in this PR because `storage.py` uses `self.latest_common_version()` to determine which certificate should be looked at for determining renewal status at https://github.com/certbot/certbot/blob/9f8e4507ad0cb3dbedb726dda4c46affb1eb7ad3/certbot/certbot/_internal/storage.py#L939-L947 I think this is unnecessary and you can just look at the currently linked certificate, but I don't think we should be changing the logic that code has always had now. * Check OCSP status as part of determining to renew * add integration tests * add ocsp_revoked_by_paths --- .../certbot_tests/test_main.py | 17 ++++++++++ certbot/CHANGELOG.md | 2 ++ certbot/certbot/_internal/storage.py | 33 +++++++++++-------- certbot/certbot/ocsp.py | 13 +++++++- certbot/tests/storage_test.py | 33 ++++++++++++++++--- 5 files changed, 80 insertions(+), 18 deletions(-) diff --git a/certbot-ci/certbot_integration_tests/certbot_tests/test_main.py b/certbot-ci/certbot_integration_tests/certbot_tests/test_main.py index 94e76cf79..f0c5edd3f 100644 --- a/certbot-ci/certbot_integration_tests/certbot_tests/test_main.py +++ b/certbot-ci/certbot_integration_tests/certbot_tests/test_main.py @@ -595,6 +595,23 @@ def test_ocsp_status_live(context): assert output.count('REVOKED') == 1, 'Expected {0} to be REVOKED'.format(cert) +def test_ocsp_renew(context): + """Test that revoked certificates are renewed.""" + # Obtain a certificate + certname = context.get_domain('ocsp-renew') + context.certbot(['--domains', certname]) + + # Test that "certbot renew" does not renew the certificate + assert_cert_count_for_lineage(context.config_dir, certname, 1) + context.certbot(['renew'], force_renew=False) + assert_cert_count_for_lineage(context.config_dir, certname, 1) + + # Revoke the certificate and test that it does renew the certificate + context.certbot(['revoke', '--cert-name', certname, '--no-delete-after-revoke']) + context.certbot(['renew'], force_renew=False) + assert_cert_count_for_lineage(context.config_dir, certname, 2) + + def test_dry_run_deactivate_authzs(context): """Test that Certbot deactivates authorizations when performing a dry run""" diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md index 6c1b112d7..2a934ee5b 100644 --- a/certbot/CHANGELOG.md +++ b/certbot/CHANGELOG.md @@ -13,6 +13,8 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). ### Changed +* Certbot will now renew certificates early if they have been revoked according + to OCSP. * Fix acme module warnings when response Content-Type includes params (e.g. charset). * Fixed issue where webroot plugin would incorrectly raise `Read-only file system` error when creating challenge directories (issue #7165). diff --git a/certbot/certbot/_internal/storage.py b/certbot/certbot/_internal/storage.py index 6a34355a8..2dac163e2 100644 --- a/certbot/certbot/_internal/storage.py +++ b/certbot/certbot/_internal/storage.py @@ -15,6 +15,7 @@ import certbot from certbot import crypto_util from certbot import errors from certbot import interfaces +from certbot import ocsp from certbot import util from certbot._internal import cli from certbot._internal import constants @@ -882,27 +883,33 @@ class RenewableCert(interfaces.RenewableCert): with open(target) as f: return crypto_util.get_names_from_cert(f.read()) - def ocsp_revoked(self, version=None): - # pylint: disable=unused-argument + def ocsp_revoked(self, version): """Is the specified cert version revoked according to OCSP? - Also returns True if the cert version is declared as intended - to be revoked according to Let's Encrypt OCSP extensions. - (If no version is specified, uses the current version.) - - This method is not yet implemented and currently always returns - False. + Also returns True if the cert version is declared as revoked + according to OCSP. If OCSP status could not be determined, False + is returned. :param int version: the desired version number - :returns: whether the certificate is or will be revoked + :returns: True if the certificate is revoked, otherwise, False :rtype: bool """ - # XXX: This query and its associated network service aren't - # implemented yet, so we currently return False (indicating that the - # certificate is not revoked). - return False + cert_path = self.version("cert", version) + chain_path = self.version("chain", version) + # While the RevocationChecker should return False if it failed to + # determine the OCSP status, let's ensure we don't crash Certbot by + # catching all exceptions here. + try: + return ocsp.RevocationChecker().ocsp_revoked_by_paths(cert_path, + chain_path) + except Exception as e: # pylint: disable=broad-except + logger.warning( + "An error occurred determining the OCSP status of %s.", + cert_path) + logger.debug(str(e)) + return False def autorenewal_is_enabled(self): """Is automatic renewal enabled for this cert? diff --git a/certbot/certbot/ocsp.py b/certbot/certbot/ocsp.py index 6a95f26fa..9799c675c 100644 --- a/certbot/certbot/ocsp.py +++ b/certbot/certbot/ocsp.py @@ -68,8 +68,19 @@ class RevocationChecker(object): :rtype: bool """ - cert_path, chain_path = cert.cert_path, cert.chain_path + return self.ocsp_revoked_by_paths(cert.cert_path, cert.chain_path) + def ocsp_revoked_by_paths(self, cert_path, chain_path): + # type: (str, str) -> bool + """Performs the OCSP revocation check + + :param str cert_path: Certificate filepath + :param str chain_path: Certificate chain filepath + + :returns: True if revoked; False if valid or the check failed or cert is expired. + :rtype: bool + + """ if self.broken: return False diff --git a/certbot/tests/storage_test.py b/certbot/tests/storage_test.py index 6208974ec..0f7620b78 100644 --- a/certbot/tests/storage_test.py +++ b/certbot/tests/storage_test.py @@ -672,10 +672,35 @@ class RenewableCertTests(BaseRenewableCertTest): errors.CertStorageError, self.test_rc._update_link_to, "elephant", 17) - def test_ocsp_revoked(self): - # XXX: This is currently hardcoded to False due to a lack of an - # OCSP server to test against. - self.assertFalse(self.test_rc.ocsp_revoked()) + @mock.patch("certbot.ocsp.RevocationChecker.ocsp_revoked_by_paths") + def test_ocsp_revoked(self, mock_checker): + # Write out test files + for kind in ALL_FOUR: + self._write_out_kind(kind, 1) + version = self.test_rc.latest_common_version() + expected_cert_path = self.test_rc.version("cert", version) + expected_chain_path = self.test_rc.version("chain", version) + + # Test with cert revoked + mock_checker.return_value = True + self.assertTrue(self.test_rc.ocsp_revoked(version)) + self.assertEqual(mock_checker.call_args[0][0], expected_cert_path) + self.assertEqual(mock_checker.call_args[0][1], expected_chain_path) + + # Test with cert not revoked + mock_checker.return_value = False + self.assertFalse(self.test_rc.ocsp_revoked(version)) + self.assertEqual(mock_checker.call_args[0][0], expected_cert_path) + self.assertEqual(mock_checker.call_args[0][1], expected_chain_path) + + # Test with error + mock_checker.side_effect = ValueError + with mock.patch("certbot._internal.storage.logger.warning") as logger: + self.assertFalse(self.test_rc.ocsp_revoked(version)) + self.assertEqual(mock_checker.call_args[0][0], expected_cert_path) + self.assertEqual(mock_checker.call_args[0][1], expected_chain_path) + log_msg = logger.call_args[0][0] + self.assertIn("An error occurred determining the OCSP status", log_msg) def test_add_time_interval(self): from certbot._internal import storage From b1fb3296e949c5ce5175321328a89a41b7dd3d12 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 3 Mar 2020 12:36:36 -0800 Subject: [PATCH 21/32] Update changelog for 1.3.0 release --- certbot/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md index 2a934ee5b..493e4d5c1 100644 --- a/certbot/CHANGELOG.md +++ b/certbot/CHANGELOG.md @@ -2,7 +2,7 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). -## 1.3.0 - master +## 1.3.0 - 2020-03-03 ### Added From 6edb4e1a3924821316b9344adb9c533937426fa7 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 3 Mar 2020 12:43:02 -0800 Subject: [PATCH 22/32] Release 1.3.0 --- acme/setup.py | 2 +- certbot-apache/setup.py | 2 +- certbot-auto | 26 +++++++++--------- certbot-compatibility-test/setup.py | 2 +- certbot-dns-cloudflare/setup.py | 2 +- certbot-dns-cloudxns/setup.py | 2 +- certbot-dns-digitalocean/setup.py | 2 +- certbot-dns-dnsimple/setup.py | 2 +- certbot-dns-dnsmadeeasy/setup.py | 2 +- certbot-dns-gehirn/setup.py | 2 +- certbot-dns-google/setup.py | 2 +- certbot-dns-linode/setup.py | 2 +- certbot-dns-luadns/setup.py | 2 +- certbot-dns-nsone/setup.py | 2 +- certbot-dns-ovh/setup.py | 2 +- certbot-dns-rfc2136/setup.py | 2 +- certbot-dns-route53/setup.py | 2 +- certbot-dns-sakuracloud/setup.py | 2 +- certbot-nginx/setup.py | 2 +- certbot/certbot/__init__.py | 2 +- certbot/docs/cli-help.txt | 2 +- letsencrypt-auto | 26 +++++++++--------- letsencrypt-auto-source/certbot-auto.asc | 16 +++++------ letsencrypt-auto-source/letsencrypt-auto | 26 +++++++++--------- letsencrypt-auto-source/letsencrypt-auto.sig | Bin 256 -> 256 bytes .../pieces/certbot-requirements.txt | 24 ++++++++-------- 26 files changed, 79 insertions(+), 79 deletions(-) diff --git a/acme/setup.py b/acme/setup.py index 0e11779ba..922cae26c 100644 --- a/acme/setup.py +++ b/acme/setup.py @@ -4,7 +4,7 @@ from setuptools import find_packages from setuptools import setup from setuptools.command.test import test as TestCommand -version = '1.3.0.dev0' +version = '1.3.0' # Please update tox.ini when modifying dependency version requirements install_requires = [ diff --git a/certbot-apache/setup.py b/certbot-apache/setup.py index b37ee3972..6483313b8 100644 --- a/certbot-apache/setup.py +++ b/certbot-apache/setup.py @@ -4,7 +4,7 @@ from setuptools import find_packages from setuptools import setup from setuptools.command.test import test as TestCommand -version = '1.3.0.dev0' +version = '1.3.0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-auto b/certbot-auto index cea58e2cb..0ea3275c3 100755 --- a/certbot-auto +++ b/certbot-auto @@ -31,7 +31,7 @@ if [ -z "$VENV_PATH" ]; then fi VENV_BIN="$VENV_PATH/bin" BOOTSTRAP_VERSION_PATH="$VENV_PATH/certbot-auto-bootstrap-version.txt" -LE_AUTO_VERSION="1.2.0" +LE_AUTO_VERSION="1.3.0" BASENAME=$(basename $0) USAGE="Usage: $BASENAME [OPTIONS] A self-updating wrapper script for the Certbot ACME client. When run, updates @@ -1540,18 +1540,18 @@ letsencrypt==0.7.0 \ --hash=sha256:105a5fb107e45bcd0722eb89696986dcf5f08a86a321d6aef25a0c7c63375ade \ --hash=sha256:c36e532c486a7e92155ee09da54b436a3c420813ec1c590b98f635d924720de9 -certbot==1.2.0 \ - --hash=sha256:e25c17125c00b3398c8e9b9d54ef473c0e8f5aff53389f313a51b06cf472d335 \ - --hash=sha256:95dcbae085f8e4eb18442fe7b12994b08964a9a6e8e352e556cdb4a8a625373c -acme==1.2.0 \ - --hash=sha256:284d22fde75687a8ea72d737cac6bcbdc91f3c796221aa25378b8732ba6f6875 \ - --hash=sha256:0630c740d49bda945e97bd35fc8d6f02d082c8cb9e18f8fec0dbb3d395ac26ab -certbot-apache==1.2.0 \ - --hash=sha256:3f7493918353d3bd6067d446a2cf263e03831c4c10ec685b83d644b47767090d \ - --hash=sha256:b46e9def272103a68108e48bf7e410ea46801529b1ea6954f6506b14dd9df9b3 -certbot-nginx==1.2.0 \ - --hash=sha256:efd32a2b32f2439279da446b6bf67684f591f289323c5f494ebfd86a566a28fd \ - --hash=sha256:6fd7cf4f2545ad66e57000343227df9ccccaf04420e835e05cb3250fac1fa6db +certbot==1.3.0 \ + --hash=sha256:979793b36151be26c159f1946d065a0cbbcaed3e9ac452c19a142b0d2d2b42e3 \ + --hash=sha256:bc2091cbbc2f432872ed69309046e79771d9c81cd441bde3e6a6553ecd04b1d8 +acme==1.3.0 \ + --hash=sha256:b888757c750e393407a3cdf0eb5c2d06036951e10c41db4c83537617568561b6 \ + --hash=sha256:c0de9e1fbcb4a28509825a4d19ab5455910862b23fa338acebc7bbe7c0abd20d +certbot-apache==1.3.0 \ + --hash=sha256:1050cd262bcc598957c45a6fa1febdf5e41e87176c0aebad3a1ab7268b0d82d9 \ + --hash=sha256:4a6bb818a7a70803127590a54bb25c1e79810761c9d4c92cf9f16a56b518bd52 +certbot-nginx==1.3.0 \ + --hash=sha256:46106b96429d1aaf3765635056352d2372941027a3bc26bbf964e4329202adc7 \ + --hash=sha256:9aa0869c1250b7ea0a1eb1df6bdb5d0d6190d6ca0400da1033a8decc0df6f65b UNLIKELY_EOF # ------------------------------------------------------------------------- diff --git a/certbot-compatibility-test/setup.py b/certbot-compatibility-test/setup.py index 1dbcefa75..f85311999 100644 --- a/certbot-compatibility-test/setup.py +++ b/certbot-compatibility-test/setup.py @@ -3,7 +3,7 @@ import sys from setuptools import find_packages from setuptools import setup -version = '1.3.0.dev0' +version = '1.3.0' install_requires = [ 'certbot', diff --git a/certbot-dns-cloudflare/setup.py b/certbot-dns-cloudflare/setup.py index 9376bc1c4..d5da3ed95 100644 --- a/certbot-dns-cloudflare/setup.py +++ b/certbot-dns-cloudflare/setup.py @@ -4,7 +4,7 @@ from setuptools import find_packages from setuptools import setup from setuptools.command.test import test as TestCommand -version = '1.3.0.dev0' +version = '1.3.0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-dns-cloudxns/setup.py b/certbot-dns-cloudxns/setup.py index 4e99ff5ff..491096d70 100644 --- a/certbot-dns-cloudxns/setup.py +++ b/certbot-dns-cloudxns/setup.py @@ -4,7 +4,7 @@ from setuptools import find_packages from setuptools import setup from setuptools.command.test import test as TestCommand -version = '1.3.0.dev0' +version = '1.3.0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-dns-digitalocean/setup.py b/certbot-dns-digitalocean/setup.py index 9c9d1717c..23c2903f4 100644 --- a/certbot-dns-digitalocean/setup.py +++ b/certbot-dns-digitalocean/setup.py @@ -4,7 +4,7 @@ from setuptools import find_packages from setuptools import setup from setuptools.command.test import test as TestCommand -version = '1.3.0.dev0' +version = '1.3.0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-dns-dnsimple/setup.py b/certbot-dns-dnsimple/setup.py index 9cde6214c..197873733 100644 --- a/certbot-dns-dnsimple/setup.py +++ b/certbot-dns-dnsimple/setup.py @@ -5,7 +5,7 @@ from setuptools import find_packages from setuptools import setup from setuptools.command.test import test as TestCommand -version = '1.3.0.dev0' +version = '1.3.0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-dns-dnsmadeeasy/setup.py b/certbot-dns-dnsmadeeasy/setup.py index adaba6851..b0a016441 100644 --- a/certbot-dns-dnsmadeeasy/setup.py +++ b/certbot-dns-dnsmadeeasy/setup.py @@ -4,7 +4,7 @@ from setuptools import find_packages from setuptools import setup from setuptools.command.test import test as TestCommand -version = '1.3.0.dev0' +version = '1.3.0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-dns-gehirn/setup.py b/certbot-dns-gehirn/setup.py index a849cef45..2c76869bc 100644 --- a/certbot-dns-gehirn/setup.py +++ b/certbot-dns-gehirn/setup.py @@ -4,7 +4,7 @@ from setuptools import find_packages from setuptools import setup from setuptools.command.test import test as TestCommand -version = '1.3.0.dev0' +version = '1.3.0' # Please update tox.ini when modifying dependency version requirements install_requires = [ diff --git a/certbot-dns-google/setup.py b/certbot-dns-google/setup.py index 51d5b8a3f..61528b3a0 100644 --- a/certbot-dns-google/setup.py +++ b/certbot-dns-google/setup.py @@ -4,7 +4,7 @@ from setuptools import find_packages from setuptools import setup from setuptools.command.test import test as TestCommand -version = '1.3.0.dev0' +version = '1.3.0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-dns-linode/setup.py b/certbot-dns-linode/setup.py index e7e91b929..1e7829588 100644 --- a/certbot-dns-linode/setup.py +++ b/certbot-dns-linode/setup.py @@ -4,7 +4,7 @@ from setuptools import find_packages from setuptools import setup from setuptools.command.test import test as TestCommand -version = '1.3.0.dev0' +version = '1.3.0' # Please update tox.ini when modifying dependency version requirements install_requires = [ diff --git a/certbot-dns-luadns/setup.py b/certbot-dns-luadns/setup.py index ea64f79a2..418c635b9 100644 --- a/certbot-dns-luadns/setup.py +++ b/certbot-dns-luadns/setup.py @@ -4,7 +4,7 @@ from setuptools import find_packages from setuptools import setup from setuptools.command.test import test as TestCommand -version = '1.3.0.dev0' +version = '1.3.0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-dns-nsone/setup.py b/certbot-dns-nsone/setup.py index d6bedca1c..99bc1022e 100644 --- a/certbot-dns-nsone/setup.py +++ b/certbot-dns-nsone/setup.py @@ -4,7 +4,7 @@ from setuptools import find_packages from setuptools import setup from setuptools.command.test import test as TestCommand -version = '1.3.0.dev0' +version = '1.3.0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-dns-ovh/setup.py b/certbot-dns-ovh/setup.py index 8f5b052a2..9f5974bcb 100644 --- a/certbot-dns-ovh/setup.py +++ b/certbot-dns-ovh/setup.py @@ -4,7 +4,7 @@ from setuptools import find_packages from setuptools import setup from setuptools.command.test import test as TestCommand -version = '1.3.0.dev0' +version = '1.3.0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-dns-rfc2136/setup.py b/certbot-dns-rfc2136/setup.py index fa51c2108..42c6f11bd 100644 --- a/certbot-dns-rfc2136/setup.py +++ b/certbot-dns-rfc2136/setup.py @@ -4,7 +4,7 @@ from setuptools import find_packages from setuptools import setup from setuptools.command.test import test as TestCommand -version = '1.3.0.dev0' +version = '1.3.0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-dns-route53/setup.py b/certbot-dns-route53/setup.py index f25e348ff..64e307b52 100644 --- a/certbot-dns-route53/setup.py +++ b/certbot-dns-route53/setup.py @@ -4,7 +4,7 @@ from setuptools import find_packages from setuptools import setup from setuptools.command.test import test as TestCommand -version = '1.3.0.dev0' +version = '1.3.0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-dns-sakuracloud/setup.py b/certbot-dns-sakuracloud/setup.py index 8df2320ba..b18da9d97 100644 --- a/certbot-dns-sakuracloud/setup.py +++ b/certbot-dns-sakuracloud/setup.py @@ -4,7 +4,7 @@ from setuptools import find_packages from setuptools import setup from setuptools.command.test import test as TestCommand -version = '1.3.0.dev0' +version = '1.3.0' # Please update tox.ini when modifying dependency version requirements install_requires = [ diff --git a/certbot-nginx/setup.py b/certbot-nginx/setup.py index b180fe06a..fe707fb09 100644 --- a/certbot-nginx/setup.py +++ b/certbot-nginx/setup.py @@ -4,7 +4,7 @@ from setuptools import find_packages from setuptools import setup from setuptools.command.test import test as TestCommand -version = '1.3.0.dev0' +version = '1.3.0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot/certbot/__init__.py b/certbot/certbot/__init__.py index 84ade6b08..39171bad2 100644 --- a/certbot/certbot/__init__.py +++ b/certbot/certbot/__init__.py @@ -1,4 +1,4 @@ """Certbot client.""" # version number like 1.2.3a0, must have at least 2 parts, like 1.2 -__version__ = '1.3.0.dev0' +__version__ = '1.3.0' diff --git a/certbot/docs/cli-help.txt b/certbot/docs/cli-help.txt index ff49609c4..3c2289030 100644 --- a/certbot/docs/cli-help.txt +++ b/certbot/docs/cli-help.txt @@ -113,7 +113,7 @@ optional arguments: case, and to know when to deprecate support for past Python versions and flags. If you wish to hide this information from the Let's Encrypt server, set this to - "". (default: CertbotACMEClient/1.2.0 (certbot(-auto); + "". (default: CertbotACMEClient/1.3.0 (certbot(-auto); OS_NAME OS_VERSION) Authenticator/XXX Installer/YYY (SUBCOMMAND; flags: FLAGS) Py/major.minor.patchlevel). The flags encoded in the user agent are: --duplicate, diff --git a/letsencrypt-auto b/letsencrypt-auto index cea58e2cb..0ea3275c3 100755 --- a/letsencrypt-auto +++ b/letsencrypt-auto @@ -31,7 +31,7 @@ if [ -z "$VENV_PATH" ]; then fi VENV_BIN="$VENV_PATH/bin" BOOTSTRAP_VERSION_PATH="$VENV_PATH/certbot-auto-bootstrap-version.txt" -LE_AUTO_VERSION="1.2.0" +LE_AUTO_VERSION="1.3.0" BASENAME=$(basename $0) USAGE="Usage: $BASENAME [OPTIONS] A self-updating wrapper script for the Certbot ACME client. When run, updates @@ -1540,18 +1540,18 @@ letsencrypt==0.7.0 \ --hash=sha256:105a5fb107e45bcd0722eb89696986dcf5f08a86a321d6aef25a0c7c63375ade \ --hash=sha256:c36e532c486a7e92155ee09da54b436a3c420813ec1c590b98f635d924720de9 -certbot==1.2.0 \ - --hash=sha256:e25c17125c00b3398c8e9b9d54ef473c0e8f5aff53389f313a51b06cf472d335 \ - --hash=sha256:95dcbae085f8e4eb18442fe7b12994b08964a9a6e8e352e556cdb4a8a625373c -acme==1.2.0 \ - --hash=sha256:284d22fde75687a8ea72d737cac6bcbdc91f3c796221aa25378b8732ba6f6875 \ - --hash=sha256:0630c740d49bda945e97bd35fc8d6f02d082c8cb9e18f8fec0dbb3d395ac26ab -certbot-apache==1.2.0 \ - --hash=sha256:3f7493918353d3bd6067d446a2cf263e03831c4c10ec685b83d644b47767090d \ - --hash=sha256:b46e9def272103a68108e48bf7e410ea46801529b1ea6954f6506b14dd9df9b3 -certbot-nginx==1.2.0 \ - --hash=sha256:efd32a2b32f2439279da446b6bf67684f591f289323c5f494ebfd86a566a28fd \ - --hash=sha256:6fd7cf4f2545ad66e57000343227df9ccccaf04420e835e05cb3250fac1fa6db +certbot==1.3.0 \ + --hash=sha256:979793b36151be26c159f1946d065a0cbbcaed3e9ac452c19a142b0d2d2b42e3 \ + --hash=sha256:bc2091cbbc2f432872ed69309046e79771d9c81cd441bde3e6a6553ecd04b1d8 +acme==1.3.0 \ + --hash=sha256:b888757c750e393407a3cdf0eb5c2d06036951e10c41db4c83537617568561b6 \ + --hash=sha256:c0de9e1fbcb4a28509825a4d19ab5455910862b23fa338acebc7bbe7c0abd20d +certbot-apache==1.3.0 \ + --hash=sha256:1050cd262bcc598957c45a6fa1febdf5e41e87176c0aebad3a1ab7268b0d82d9 \ + --hash=sha256:4a6bb818a7a70803127590a54bb25c1e79810761c9d4c92cf9f16a56b518bd52 +certbot-nginx==1.3.0 \ + --hash=sha256:46106b96429d1aaf3765635056352d2372941027a3bc26bbf964e4329202adc7 \ + --hash=sha256:9aa0869c1250b7ea0a1eb1df6bdb5d0d6190d6ca0400da1033a8decc0df6f65b UNLIKELY_EOF # ------------------------------------------------------------------------- diff --git a/letsencrypt-auto-source/certbot-auto.asc b/letsencrypt-auto-source/certbot-auto.asc index 488d0bf2e..84473dc30 100644 --- a/letsencrypt-auto-source/certbot-auto.asc +++ b/letsencrypt-auto-source/certbot-auto.asc @@ -1,11 +1,11 @@ -----BEGIN PGP SIGNATURE----- -iQEzBAABCAAdFiEEos+1H6J1pyhiNOeyTRfJlc2XdfIFAl456ZoACgkQTRfJlc2X -dfJx8wf/addMw4kUlwu6poHqLvsifZzHAESgvq+qybgFvl5yTh2U+99PGBgxRYx+ -bENIWBi6+XB+CiVuLzIXWw/VkXh+za99orRkkVK9PI33Xr7jBMZo5Oa3JviYjl3X -PcfjioRQCD+a9Tf9RO25LXQmxn87Ql9x3nxJuk//YeSpuImFmYjIBPE4n/LPEf7z -8WHU4oxxa/bgqGCPgv6O7ZBw7ipd3g+VHcDZcNQMP4tWYb6m7x/nN61yirid7q3M -uqQ1lbitN48ISyru6xPyE6WGTvfl1SIQd21FNRETpcoesx+MTv3ApWT4dqXjZvaX -FeM55IS65e7ci6yLV9qdAbqGKzhX0Q== -=uLcV +iQEzBAABCAAdFiEEos+1H6J1pyhiNOeyTRfJlc2XdfIFAl5ewVUACgkQTRfJlc2X +dfJnZAf+KmxYl1YoP/FlTG5Npb64qaDdxm59SeEVJez6fZh15xq71tRPYR+4xszE +XTeyGt7uAxjYqeiBJU5xBvGC1Veprhj5AbflVOTP+5yiBr9iNWC35zmgaE63UlZ/ +V94sfL0pkax7wLngil7a0OuzUjikzK3gXOqrY8LoUdr4mAA9AhSjajWHmyY3tpDR +84GKrVhybIt0sjy/172VuPPbXZKno/clztkKMZHXNrDeL5jgJ15Va4Ts5FK0j9VT +HQvuazbGkYVCuvlp8Np5ESDje69LCJfPZxl34htoa8WNJoVIOsQWZpoXp5B5huSP +vGrh4LabZ5UDsl+k11ikHBRUpO7E5w== +=IgRH -----END PGP SIGNATURE----- diff --git a/letsencrypt-auto-source/letsencrypt-auto b/letsencrypt-auto-source/letsencrypt-auto index e2813853b..0ea3275c3 100755 --- a/letsencrypt-auto-source/letsencrypt-auto +++ b/letsencrypt-auto-source/letsencrypt-auto @@ -31,7 +31,7 @@ if [ -z "$VENV_PATH" ]; then fi VENV_BIN="$VENV_PATH/bin" BOOTSTRAP_VERSION_PATH="$VENV_PATH/certbot-auto-bootstrap-version.txt" -LE_AUTO_VERSION="1.3.0.dev0" +LE_AUTO_VERSION="1.3.0" BASENAME=$(basename $0) USAGE="Usage: $BASENAME [OPTIONS] A self-updating wrapper script for the Certbot ACME client. When run, updates @@ -1540,18 +1540,18 @@ letsencrypt==0.7.0 \ --hash=sha256:105a5fb107e45bcd0722eb89696986dcf5f08a86a321d6aef25a0c7c63375ade \ --hash=sha256:c36e532c486a7e92155ee09da54b436a3c420813ec1c590b98f635d924720de9 -certbot==1.2.0 \ - --hash=sha256:e25c17125c00b3398c8e9b9d54ef473c0e8f5aff53389f313a51b06cf472d335 \ - --hash=sha256:95dcbae085f8e4eb18442fe7b12994b08964a9a6e8e352e556cdb4a8a625373c -acme==1.2.0 \ - --hash=sha256:284d22fde75687a8ea72d737cac6bcbdc91f3c796221aa25378b8732ba6f6875 \ - --hash=sha256:0630c740d49bda945e97bd35fc8d6f02d082c8cb9e18f8fec0dbb3d395ac26ab -certbot-apache==1.2.0 \ - --hash=sha256:3f7493918353d3bd6067d446a2cf263e03831c4c10ec685b83d644b47767090d \ - --hash=sha256:b46e9def272103a68108e48bf7e410ea46801529b1ea6954f6506b14dd9df9b3 -certbot-nginx==1.2.0 \ - --hash=sha256:efd32a2b32f2439279da446b6bf67684f591f289323c5f494ebfd86a566a28fd \ - --hash=sha256:6fd7cf4f2545ad66e57000343227df9ccccaf04420e835e05cb3250fac1fa6db +certbot==1.3.0 \ + --hash=sha256:979793b36151be26c159f1946d065a0cbbcaed3e9ac452c19a142b0d2d2b42e3 \ + --hash=sha256:bc2091cbbc2f432872ed69309046e79771d9c81cd441bde3e6a6553ecd04b1d8 +acme==1.3.0 \ + --hash=sha256:b888757c750e393407a3cdf0eb5c2d06036951e10c41db4c83537617568561b6 \ + --hash=sha256:c0de9e1fbcb4a28509825a4d19ab5455910862b23fa338acebc7bbe7c0abd20d +certbot-apache==1.3.0 \ + --hash=sha256:1050cd262bcc598957c45a6fa1febdf5e41e87176c0aebad3a1ab7268b0d82d9 \ + --hash=sha256:4a6bb818a7a70803127590a54bb25c1e79810761c9d4c92cf9f16a56b518bd52 +certbot-nginx==1.3.0 \ + --hash=sha256:46106b96429d1aaf3765635056352d2372941027a3bc26bbf964e4329202adc7 \ + --hash=sha256:9aa0869c1250b7ea0a1eb1df6bdb5d0d6190d6ca0400da1033a8decc0df6f65b UNLIKELY_EOF # ------------------------------------------------------------------------- diff --git a/letsencrypt-auto-source/letsencrypt-auto.sig b/letsencrypt-auto-source/letsencrypt-auto.sig index fefc81b37796cdecd066b9bb212d8e285fffd4d0..8c4f52d6e2992d76012ed4c5e8b65c366e53cf8c 100644 GIT binary patch literal 256 zcmV+b0ssEUOnRo|t9S`PT@Q6pdEx5)TLaw|tXK(Czj^_>8PhYd%g<>HQ?bGidlv-5 zE}9A&SzM6uK*eeB!}iF{EuInepHwl!!N=ypH>Giu0~gNuz3H_G-xK$*4GGCrIi8a} zc|vYH^EA>Vt1;uU?ETR7;K}DFE1nDBiU-xb=ODbJ#yW{#gBQmTy?oXA)L zay!@Hc;qXdB0+{NAC2*m>{YND9$B?_{$Q;DGoOCVmMhh`)kRLNXuPfjru6g4?PpB5 zE=0kjQwl0Y1m+t}(iNS8e1OH8)e$OZ#|vb^J4)NEd(wxnV%*phX+ZJIy2kBPS7%B+ G&t7>eJc8N) literal 256 zcmV+b0ssCyy6+%Byu~4aOZx+sGM?uWzM?nz3Jfz_Cb^!H(J$u3EbYx+Wmt@nUdJdC z5^ed_c;i%mwYmiD>ud)5-Lk6YWkh7j-?{0%L?KJZd%TEUG+|;|-*#FI(%jlkvk;lO zO~Yh(rw(~6mZiF)`O}!O(DYn@gPBFqESJ@M(%s)|7%F?plRgOU1Sm{BW(R9Bfx|N( zKD3y8K_?ev`Hu@xHbPDe3A34WZ3?o_F0i8u)P>>`q{3hr9CRb?E6FEIK^btkZ@Qg6 zt+z9SQGsl`GlM4_;VnLX96Xg&)Kv+HS#^V0%aEYWG9N+LKk6WSdEBew*pUM_Rk_eZ GIvjh!?|N Date: Tue, 3 Mar 2020 12:43:03 -0800 Subject: [PATCH 23/32] Add contents to certbot/CHANGELOG.md for next version --- certbot/CHANGELOG.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md index 493e4d5c1..cb1f2968c 100644 --- a/certbot/CHANGELOG.md +++ b/certbot/CHANGELOG.md @@ -2,6 +2,22 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). +## 1.4.0 - master + +### Added + +* + +### Changed + +* + +### Fixed + +* + +More details about these changes can be found on our GitHub repo. + ## 1.3.0 - 2020-03-03 ### Added From 144d4f2b446a5659e713133868eb921885ce105b Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 3 Mar 2020 12:43:04 -0800 Subject: [PATCH 24/32] Bump version to 1.4.0 --- acme/setup.py | 2 +- certbot-apache/setup.py | 2 +- certbot-compatibility-test/setup.py | 2 +- certbot-dns-cloudflare/setup.py | 2 +- certbot-dns-cloudxns/setup.py | 2 +- certbot-dns-digitalocean/setup.py | 2 +- certbot-dns-dnsimple/setup.py | 2 +- certbot-dns-dnsmadeeasy/setup.py | 2 +- certbot-dns-gehirn/setup.py | 2 +- certbot-dns-google/setup.py | 2 +- certbot-dns-linode/setup.py | 2 +- certbot-dns-luadns/setup.py | 2 +- certbot-dns-nsone/setup.py | 2 +- certbot-dns-ovh/setup.py | 2 +- certbot-dns-rfc2136/setup.py | 2 +- certbot-dns-route53/setup.py | 2 +- certbot-dns-sakuracloud/setup.py | 2 +- certbot-nginx/setup.py | 2 +- certbot/certbot/__init__.py | 2 +- letsencrypt-auto-source/letsencrypt-auto | 2 +- 20 files changed, 20 insertions(+), 20 deletions(-) diff --git a/acme/setup.py b/acme/setup.py index 922cae26c..0527b3fb5 100644 --- a/acme/setup.py +++ b/acme/setup.py @@ -4,7 +4,7 @@ from setuptools import find_packages from setuptools import setup from setuptools.command.test import test as TestCommand -version = '1.3.0' +version = '1.4.0.dev0' # Please update tox.ini when modifying dependency version requirements install_requires = [ diff --git a/certbot-apache/setup.py b/certbot-apache/setup.py index 6483313b8..4ec1d0a9c 100644 --- a/certbot-apache/setup.py +++ b/certbot-apache/setup.py @@ -4,7 +4,7 @@ from setuptools import find_packages from setuptools import setup from setuptools.command.test import test as TestCommand -version = '1.3.0' +version = '1.4.0.dev0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-compatibility-test/setup.py b/certbot-compatibility-test/setup.py index f85311999..d6760576a 100644 --- a/certbot-compatibility-test/setup.py +++ b/certbot-compatibility-test/setup.py @@ -3,7 +3,7 @@ import sys from setuptools import find_packages from setuptools import setup -version = '1.3.0' +version = '1.4.0.dev0' install_requires = [ 'certbot', diff --git a/certbot-dns-cloudflare/setup.py b/certbot-dns-cloudflare/setup.py index d5da3ed95..67aac3231 100644 --- a/certbot-dns-cloudflare/setup.py +++ b/certbot-dns-cloudflare/setup.py @@ -4,7 +4,7 @@ from setuptools import find_packages from setuptools import setup from setuptools.command.test import test as TestCommand -version = '1.3.0' +version = '1.4.0.dev0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-dns-cloudxns/setup.py b/certbot-dns-cloudxns/setup.py index 491096d70..6c653967d 100644 --- a/certbot-dns-cloudxns/setup.py +++ b/certbot-dns-cloudxns/setup.py @@ -4,7 +4,7 @@ from setuptools import find_packages from setuptools import setup from setuptools.command.test import test as TestCommand -version = '1.3.0' +version = '1.4.0.dev0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-dns-digitalocean/setup.py b/certbot-dns-digitalocean/setup.py index 23c2903f4..c45fc8d03 100644 --- a/certbot-dns-digitalocean/setup.py +++ b/certbot-dns-digitalocean/setup.py @@ -4,7 +4,7 @@ from setuptools import find_packages from setuptools import setup from setuptools.command.test import test as TestCommand -version = '1.3.0' +version = '1.4.0.dev0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-dns-dnsimple/setup.py b/certbot-dns-dnsimple/setup.py index 197873733..9124f0552 100644 --- a/certbot-dns-dnsimple/setup.py +++ b/certbot-dns-dnsimple/setup.py @@ -5,7 +5,7 @@ from setuptools import find_packages from setuptools import setup from setuptools.command.test import test as TestCommand -version = '1.3.0' +version = '1.4.0.dev0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-dns-dnsmadeeasy/setup.py b/certbot-dns-dnsmadeeasy/setup.py index b0a016441..2a4fd92b0 100644 --- a/certbot-dns-dnsmadeeasy/setup.py +++ b/certbot-dns-dnsmadeeasy/setup.py @@ -4,7 +4,7 @@ from setuptools import find_packages from setuptools import setup from setuptools.command.test import test as TestCommand -version = '1.3.0' +version = '1.4.0.dev0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-dns-gehirn/setup.py b/certbot-dns-gehirn/setup.py index 2c76869bc..2198fdd3e 100644 --- a/certbot-dns-gehirn/setup.py +++ b/certbot-dns-gehirn/setup.py @@ -4,7 +4,7 @@ from setuptools import find_packages from setuptools import setup from setuptools.command.test import test as TestCommand -version = '1.3.0' +version = '1.4.0.dev0' # Please update tox.ini when modifying dependency version requirements install_requires = [ diff --git a/certbot-dns-google/setup.py b/certbot-dns-google/setup.py index 61528b3a0..087766edd 100644 --- a/certbot-dns-google/setup.py +++ b/certbot-dns-google/setup.py @@ -4,7 +4,7 @@ from setuptools import find_packages from setuptools import setup from setuptools.command.test import test as TestCommand -version = '1.3.0' +version = '1.4.0.dev0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-dns-linode/setup.py b/certbot-dns-linode/setup.py index 1e7829588..1e6b96b71 100644 --- a/certbot-dns-linode/setup.py +++ b/certbot-dns-linode/setup.py @@ -4,7 +4,7 @@ from setuptools import find_packages from setuptools import setup from setuptools.command.test import test as TestCommand -version = '1.3.0' +version = '1.4.0.dev0' # Please update tox.ini when modifying dependency version requirements install_requires = [ diff --git a/certbot-dns-luadns/setup.py b/certbot-dns-luadns/setup.py index 418c635b9..4db50b56c 100644 --- a/certbot-dns-luadns/setup.py +++ b/certbot-dns-luadns/setup.py @@ -4,7 +4,7 @@ from setuptools import find_packages from setuptools import setup from setuptools.command.test import test as TestCommand -version = '1.3.0' +version = '1.4.0.dev0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-dns-nsone/setup.py b/certbot-dns-nsone/setup.py index 99bc1022e..49e5e3bcf 100644 --- a/certbot-dns-nsone/setup.py +++ b/certbot-dns-nsone/setup.py @@ -4,7 +4,7 @@ from setuptools import find_packages from setuptools import setup from setuptools.command.test import test as TestCommand -version = '1.3.0' +version = '1.4.0.dev0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-dns-ovh/setup.py b/certbot-dns-ovh/setup.py index 9f5974bcb..6c66b39dc 100644 --- a/certbot-dns-ovh/setup.py +++ b/certbot-dns-ovh/setup.py @@ -4,7 +4,7 @@ from setuptools import find_packages from setuptools import setup from setuptools.command.test import test as TestCommand -version = '1.3.0' +version = '1.4.0.dev0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-dns-rfc2136/setup.py b/certbot-dns-rfc2136/setup.py index 42c6f11bd..5e0900e4d 100644 --- a/certbot-dns-rfc2136/setup.py +++ b/certbot-dns-rfc2136/setup.py @@ -4,7 +4,7 @@ from setuptools import find_packages from setuptools import setup from setuptools.command.test import test as TestCommand -version = '1.3.0' +version = '1.4.0.dev0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-dns-route53/setup.py b/certbot-dns-route53/setup.py index 64e307b52..98455c362 100644 --- a/certbot-dns-route53/setup.py +++ b/certbot-dns-route53/setup.py @@ -4,7 +4,7 @@ from setuptools import find_packages from setuptools import setup from setuptools.command.test import test as TestCommand -version = '1.3.0' +version = '1.4.0.dev0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot-dns-sakuracloud/setup.py b/certbot-dns-sakuracloud/setup.py index b18da9d97..16990a4d3 100644 --- a/certbot-dns-sakuracloud/setup.py +++ b/certbot-dns-sakuracloud/setup.py @@ -4,7 +4,7 @@ from setuptools import find_packages from setuptools import setup from setuptools.command.test import test as TestCommand -version = '1.3.0' +version = '1.4.0.dev0' # Please update tox.ini when modifying dependency version requirements install_requires = [ diff --git a/certbot-nginx/setup.py b/certbot-nginx/setup.py index fe707fb09..34785f963 100644 --- a/certbot-nginx/setup.py +++ b/certbot-nginx/setup.py @@ -4,7 +4,7 @@ from setuptools import find_packages from setuptools import setup from setuptools.command.test import test as TestCommand -version = '1.3.0' +version = '1.4.0.dev0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. diff --git a/certbot/certbot/__init__.py b/certbot/certbot/__init__.py index 39171bad2..0ce7ff6b7 100644 --- a/certbot/certbot/__init__.py +++ b/certbot/certbot/__init__.py @@ -1,4 +1,4 @@ """Certbot client.""" # version number like 1.2.3a0, must have at least 2 parts, like 1.2 -__version__ = '1.3.0' +__version__ = '1.4.0.dev0' diff --git a/letsencrypt-auto-source/letsencrypt-auto b/letsencrypt-auto-source/letsencrypt-auto index 0ea3275c3..ca0bda2d5 100755 --- a/letsencrypt-auto-source/letsencrypt-auto +++ b/letsencrypt-auto-source/letsencrypt-auto @@ -31,7 +31,7 @@ if [ -z "$VENV_PATH" ]; then fi VENV_BIN="$VENV_PATH/bin" BOOTSTRAP_VERSION_PATH="$VENV_PATH/certbot-auto-bootstrap-version.txt" -LE_AUTO_VERSION="1.3.0" +LE_AUTO_VERSION="1.4.0.dev0" BASENAME=$(basename $0) USAGE="Usage: $BASENAME [OPTIONS] A self-updating wrapper script for the Certbot ACME client. When run, updates From d72a1a71d22792cecf66a8d636705b9319e8fca3 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 5 Mar 2020 11:50:52 -0800 Subject: [PATCH 25/32] Fix issues with Azure Pipelines (#7838) This PR fixes two issues. First, it fixes #7814 by removing our tests on Windows Server 2012. I also added the sentence "Certbot supports Windows Server 2016 and Windows Server 2019." to https://community.letsencrypt.org/t/beta-phase-of-certbot-for-windows/105822. Second, it fixes the test failures which can be seen at https://dev.azure.com/certbot/certbot/_build/results?buildId=1309&view=results by no longer manually installing our own version of Python and instead using the one provided by Azure. These small changes are in the same PR because I wanted to fix test failures ASAP and `UsePythonVersion` is not available on Windows 2012. See https://github.com/certbot/certbot/pull/7641#discussion_r358510854. You can see tests passing with this change at https://dev.azure.com/certbot/certbot/_build/results?buildId=1311&view=results. * stop testing on win2012 * switch to UsePythonVersion --- .azure-pipelines/templates/installer-tests.yml | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/.azure-pipelines/templates/installer-tests.yml b/.azure-pipelines/templates/installer-tests.yml index 6d5672339..ea2101792 100644 --- a/.azure-pipelines/templates/installer-tests.yml +++ b/.azure-pipelines/templates/installer-tests.yml @@ -28,15 +28,13 @@ jobs: imageName: windows-2019 win2016: imageName: vs2017-win2016 - win2012r2: - imageName: vs2015-win2012r2 pool: vmImage: $(imageName) steps: - - powershell: Invoke-WebRequest https://www.python.org/ftp/python/3.8.1/python-3.8.1-amd64-webinstall.exe -OutFile C:\py3-setup.exe - displayName: Get Python - - script: C:\py3-setup.exe /quiet PrependPath=1 InstallAllUsers=1 Include_launcher=1 InstallLauncherAllUsers=1 Include_test=0 Include_doc=0 Include_dev=1 Include_debug=0 Include_tcltk=0 TargetDir=C:\py3 - displayName: Install Python + - task: UsePythonVersion@0 + inputs: + versionSpec: 3.8 + addToPath: true - task: DownloadPipelineArtifact@2 inputs: artifact: windows-installer From 7f63141e410ae7ce200c7c7408d05c87eae0c5f5 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 6 Mar 2020 09:46:30 -0800 Subject: [PATCH 26/32] Add changes to the correct changelog entry (#7833) https://github.com/certbot/certbot/pull/7742 and https://github.com/certbot/certbot/pull/7738 landed after our 1.2.0 release, but the 1.2.0 changelog entry was modified instead of the one for master/1.3.0. This PR moves the changelog entries to the 1.3.0 section. --- certbot/CHANGELOG.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md index cb1f2968c..868f3c8be 100644 --- a/certbot/CHANGELOG.md +++ b/certbot/CHANGELOG.md @@ -26,6 +26,7 @@ More details about these changes can be found on our GitHub repo. determine the OCSP status of certificates. * Don't verify the existing certificate in HTTP01Response.simple_verify, for compatibility with the real-world ACME challenge checks. +* Added support for `$hostname` in nginx `server_name` directive ### Changed @@ -37,7 +38,7 @@ More details about these changes can be found on our GitHub repo. ### Fixed -* +* Fix Apache plugin to use less restrictive umask for making the challenge directory when a restrictive umask was set when certbot was started. More details about these changes can be found on our GitHub repo. @@ -46,7 +47,6 @@ More details about these changes can be found on our GitHub repo. ### Added * Added support for Cloudflare's limited-scope API Tokens -* Added support for `$hostname` in nginx `server_name` directive ### Changed @@ -59,7 +59,6 @@ More details about these changes can be found on our GitHub repo. ### Fixed * Fix collections.abc imports for Python 3.9. -* Fix Apache plugin to use less restrictive umask for making the challenge directory when a restrictive umask was set when certbot was started. More details about these changes can be found on our GitHub repo. From 69aec55ead42c9ef9231602d002825e838e9dafb Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 9 Mar 2020 13:05:35 -0700 Subject: [PATCH 27/32] Remove --no-site-packages outside of certbot-auto. (#7832) --- certbot-compatibility-test/Dockerfile | 2 +- tools/_release.sh | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/certbot-compatibility-test/Dockerfile b/certbot-compatibility-test/Dockerfile index a9996f779..a6a0c93db 100644 --- a/certbot-compatibility-test/Dockerfile +++ b/certbot-compatibility-test/Dockerfile @@ -31,7 +31,7 @@ COPY certbot-nginx /opt/certbot/src/certbot-nginx/ COPY certbot-compatibility-test /opt/certbot/src/certbot-compatibility-test/ COPY tools /opt/certbot/src/tools -RUN VIRTUALENV_NO_DOWNLOAD=1 virtualenv --no-site-packages -p python2 /opt/certbot/venv && \ +RUN VIRTUALENV_NO_DOWNLOAD=1 virtualenv -p python2 /opt/certbot/venv && \ /opt/certbot/venv/bin/pip install -U setuptools && \ /opt/certbot/venv/bin/pip install -U pip ENV PATH /opt/certbot/venv/bin:$PATH diff --git a/tools/_release.sh b/tools/_release.sh index 1819adad2..97d5f5eb8 100755 --- a/tools/_release.sh +++ b/tools/_release.sh @@ -59,7 +59,7 @@ mv "dist.$version" "dist.$version.$(date +%s).bak" || true git tag --delete "$tag" || true tmpvenv=$(mktemp -d) -VIRTUALENV_NO_DOWNLOAD=1 virtualenv --no-site-packages -p python2 $tmpvenv +VIRTUALENV_NO_DOWNLOAD=1 virtualenv -p python2 $tmpvenv . $tmpvenv/bin/activate # update setuptools/pip just like in other places in the repo pip install -U setuptools @@ -160,7 +160,7 @@ cd "dist.$version" python -m SimpleHTTPServer $PORT & # cd .. is NOT done on purpose: we make sure that all subpackages are # installed from local PyPI rather than current directory (repo root) -VIRTUALENV_NO_DOWNLOAD=1 virtualenv --no-site-packages ../venv +VIRTUALENV_NO_DOWNLOAD=1 virtualenv ../venv . ../venv/bin/activate pip install -U setuptools pip install -U pip From 78168a5248cf119289053ffcf048c4be9b2af9d6 Mon Sep 17 00:00:00 2001 From: radek-sprta Date: Wed, 11 Mar 2020 21:27:19 +0100 Subject: [PATCH 28/32] Add CloudDNS to third-party plugins (#7840) --- certbot/docs/using.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/certbot/docs/using.rst b/certbot/docs/using.rst index 8ec172c24..3c3ef6fad 100644 --- a/certbot/docs/using.rst +++ b/certbot/docs/using.rst @@ -280,6 +280,7 @@ pritunl_ N Y Install certificates in pritunl distributed OpenVPN proxmox_ N Y Install certificates in Proxmox Virtualization servers dns-standalone_ Y N Obtain certificates via an integrated DNS server dns-ispconfig_ Y N DNS Authentication using ISPConfig as DNS server +dns-clouddns_ Y N DNS Authentication using CloudDNS API ================== ==== ==== =============================================================== .. _haproxy: https://github.com/greenhost/certbot-haproxy @@ -291,6 +292,7 @@ dns-ispconfig_ Y N DNS Authentication using ISPConfig as DNS server .. _external-auth: https://github.com/EnigmaBridge/certbot-external-auth .. _dns-standalone: https://github.com/siilike/certbot-dns-standalone .. _dns-ispconfig: https://github.com/m42e/certbot-dns-ispconfig +.. _dns-clouddns: https://github.com/vshosting/certbot-dns-clouddns If you're interested, you can also :ref:`write your own plugin `. From 44b97df4e91a3d228bf933ee169e964070f96dd3 Mon Sep 17 00:00:00 2001 From: Adrien Ferrand Date: Thu, 12 Mar 2020 17:29:03 +0100 Subject: [PATCH 29/32] Exposes environment variable to let hooks scripts know when the last challenge is handled (#7837) Fixes #5484 This PRs makes Certbot expose two new environment variables in the auth and cleanup hooks of the `manual` plugin: * `CERTBOT_REMAINING_CHALLENGES` contains the number of challenges that remain after the current one (so it equals to 0 when the script is called for the last challenge) * `CERTBOT_ALL_DOMAINS` contains a comma-separated list of all domains concerned by a challenge for the current certificate With these variables, an hook script can know when it is run for the last time, and then trigger appropriate finalizers for all challenges that have been executed. This will be particularly useful for certificates with a lot of domains validated with DNS-01 challenges: instead of waiting on each hook execution to check that the relevant DNS TXT entry has been inserted, these waits can be avoided thanks to the latest hook verifying all domains in one run. * Inject environment variables in manual scripts about remaining challenges * Adapt tests * Less variables and less lines * Update manual.py * Update manual_test.py * Add documentation * Add changelog --- certbot/CHANGELOG.md | 5 ++++- certbot/certbot/_internal/plugins/manual.py | 21 +++++++++++++-------- certbot/docs/using.rst | 4 +++- certbot/tests/plugins/manual_test.py | 19 +++++++++++++------ 4 files changed, 33 insertions(+), 16 deletions(-) diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md index 868f3c8be..903af0610 100644 --- a/certbot/CHANGELOG.md +++ b/certbot/CHANGELOG.md @@ -6,7 +6,10 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). ### Added -* +* Expose two new environment variables in the authenticator and cleanup scripts used by + the `manual` plugin: `CERTBOT_REMAINING_CHALLENGES` is equal to the number of challenges + remaining after the current challenge, `CERTBOT_ALL_DOMAINS` is a comma-separated list + of all domains challenged for the current certificate. ### Changed diff --git a/certbot/certbot/_internal/plugins/manual.py b/certbot/certbot/_internal/plugins/manual.py index 3204fe1da..87ccdbd7e 100644 --- a/certbot/certbot/_internal/plugins/manual.py +++ b/certbot/certbot/_internal/plugins/manual.py @@ -35,7 +35,11 @@ class Authenticator(common.Plugin): 'is the validation string, and $CERTBOT_TOKEN is the filename of the ' 'resource requested when performing an HTTP-01 challenge. An additional ' 'cleanup script can also be provided and can use the additional variable ' - '$CERTBOT_AUTH_OUTPUT which contains the stdout output from the auth script.') + '$CERTBOT_AUTH_OUTPUT which contains the stdout output from the auth script.' + 'For both authenticator and cleanup script, on HTTP-01 and DNS-01 challenges,' + '$CERTBOT_REMAINING_CHALLENGES will be equal to the number of challenges that ' + 'remain after the current one, and $CERTBOT_ALL_DOMAINS contains a comma-separated ' + 'list of all domains that are challenged for the current certificate.') _DNS_INSTRUCTIONS = """\ Please deploy a DNS TXT record under the name {domain} with the following value: @@ -109,14 +113,13 @@ permitted by DNS standards.) def perform(self, achalls): # pylint: disable=missing-function-docstring self._verify_ip_logging_ok() - if self.conf('auth-hook'): - perform_achall = self._perform_achall_with_script - else: - perform_achall = self._perform_achall_manually responses = [] for achall in achalls: - perform_achall(achall) + if self.conf('auth-hook'): + self._perform_achall_with_script(achall, achalls) + else: + self._perform_achall_manually(achall) responses.append(achall.response(achall.account_key)) return responses @@ -134,9 +137,11 @@ permitted by DNS standards.) else: raise errors.PluginError('Must agree to IP logging to proceed') - def _perform_achall_with_script(self, achall): + def _perform_achall_with_script(self, achall, achalls): env = dict(CERTBOT_DOMAIN=achall.domain, - CERTBOT_VALIDATION=achall.validation(achall.account_key)) + CERTBOT_VALIDATION=achall.validation(achall.account_key), + CERTBOT_ALL_DOMAINS=','.join(one_achall.domain for one_achall in achalls), + CERTBOT_REMAINING_CHALLENGES=str(len(achalls) - achalls.index(achall) - 1)) if isinstance(achall.chall, challenges.HTTP01): env['CERTBOT_TOKEN'] = achall.chall.encode('token') else: diff --git a/certbot/docs/using.rst b/certbot/docs/using.rst index 3c3ef6fad..d3c2d1582 100644 --- a/certbot/docs/using.rst +++ b/certbot/docs/using.rst @@ -738,8 +738,10 @@ the ``cleanup.sh`` script. Additionally certbot will pass relevant environment variables to these scripts: - ``CERTBOT_DOMAIN``: The domain being authenticated -- ``CERTBOT_VALIDATION``: The validation string (HTTP-01 and DNS-01 only) +- ``CERTBOT_VALIDATION``: The validation string - ``CERTBOT_TOKEN``: Resource name part of the HTTP-01 challenge (HTTP-01 only) +- ``CERTBOT_REMAINING_CHALLENGES``: Number of challenges remaining after the current challenge +- ``CERTBOT_ALL_DOMAINS``: A comma-separated list of all domains challenged for the current certificate Additionally for cleanup: diff --git a/certbot/tests/plugins/manual_test.py b/certbot/tests/plugins/manual_test.py index bd11a9538..6cdef148a 100644 --- a/certbot/tests/plugins/manual_test.py +++ b/certbot/tests/plugins/manual_test.py @@ -72,16 +72,23 @@ class AuthenticatorTest(test_util.TempDirTestCase): self.config.manual_public_ip_logging_ok = True self.config.manual_auth_hook = ( '{0} -c "from __future__ import print_function;' - 'from certbot.compat import os; print(os.environ.get(\'CERTBOT_DOMAIN\'));' + 'from certbot.compat import os;' + 'print(os.environ.get(\'CERTBOT_DOMAIN\'));' 'print(os.environ.get(\'CERTBOT_TOKEN\', \'notoken\'));' - 'print(os.environ.get(\'CERTBOT_VALIDATION\', \'novalidation\'));"' + 'print(os.environ.get(\'CERTBOT_VALIDATION\', \'novalidation\'));' + 'print(os.environ.get(\'CERTBOT_ALL_DOMAINS\'));' + 'print(os.environ.get(\'CERTBOT_REMAINING_CHALLENGES\'));"' .format(sys.executable)) - dns_expected = '{0}\n{1}\n{2}'.format( + dns_expected = '{0}\n{1}\n{2}\n{3}\n{4}'.format( self.dns_achall.domain, 'notoken', - self.dns_achall.validation(self.dns_achall.account_key)) - http_expected = '{0}\n{1}\n{2}'.format( + self.dns_achall.validation(self.dns_achall.account_key), + ','.join(achall.domain for achall in self.achalls), + len(self.achalls) - self.achalls.index(self.dns_achall) - 1) + http_expected = '{0}\n{1}\n{2}\n{3}\n{4}'.format( self.http_achall.domain, self.http_achall.chall.encode('token'), - self.http_achall.validation(self.http_achall.account_key)) + self.http_achall.validation(self.http_achall.account_key), + ','.join(achall.domain for achall in self.achalls), + len(self.achalls) - self.achalls.index(self.http_achall) - 1) self.assertEqual( self.auth.perform(self.achalls), From 2fd85a4f36c37cd7dfa96f129338c2b6d95dd0d8 Mon Sep 17 00:00:00 2001 From: osirisinferi Date: Thu, 12 Mar 2020 17:37:49 +0100 Subject: [PATCH 30/32] Add serial number to certificates output (#7842) Fixes #7835 I had to mock out `get_serial_from_cert` to keep a test from failing, because `cert_path` was mocked itself in `test_report_human_readable`. Also, I kept the same style for the serial number as the recent Let's Encrypt e-mail: lowercase hexadecimal without a `0x` prefix and without colons every 2 chars. Shouldn't be a problem to change the format if required. --- certbot/CHANGELOG.md | 1 + certbot/certbot/_internal/cert_manager.py | 11 +++++++---- certbot/certbot/crypto_util.py | 14 ++++++++++++++ certbot/tests/cert_manager_test.py | 4 +++- 4 files changed, 25 insertions(+), 5 deletions(-) diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md index 903af0610..2f22b5204 100644 --- a/certbot/CHANGELOG.md +++ b/certbot/CHANGELOG.md @@ -6,6 +6,7 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). ### Added +* Added serial number of certificate to the output of `certbot certificates` * Expose two new environment variables in the authenticator and cleanup scripts used by the `manual` plugin: `CERTBOT_REMAINING_CHALLENGES` is equal to the number of challenges remaining after the current challenge, `CERTBOT_ALL_DOMAINS` is a comma-separated list diff --git a/certbot/certbot/_internal/cert_manager.py b/certbot/certbot/_internal/cert_manager.py index e6cbd5c2c..2652b3d2c 100644 --- a/certbot/certbot/_internal/cert_manager.py +++ b/certbot/certbot/_internal/cert_manager.py @@ -276,12 +276,15 @@ def human_readable_cert_info(config, cert, skip_filter_checks=False): status = "VALID: {0} days".format(diff.days) valid_string = "{0} ({1})".format(cert.target_expiry, status) + serial = format(crypto_util.get_serial_from_cert(cert.cert_path), 'x') certinfo.append(" Certificate Name: {0}\n" - " Domains: {1}\n" - " Expiry Date: {2}\n" - " Certificate Path: {3}\n" - " Private Key Path: {4}".format( + " Serial Number: {1}\n" + " Domains: {2}\n" + " Expiry Date: {3}\n" + " Certificate Path: {4}\n" + " Private Key Path: {5}".format( cert.lineagename, + serial, " ".join(cert.names()), valid_string, cert.fullchain, diff --git a/certbot/certbot/crypto_util.py b/certbot/certbot/crypto_util.py index 9136445bc..adb972f24 100644 --- a/certbot/certbot/crypto_util.py +++ b/certbot/certbot/crypto_util.py @@ -491,3 +491,17 @@ def cert_and_chain_from_fullchain(fullchain_pem): crypto.load_certificate(crypto.FILETYPE_PEM, fullchain_pem)).decode() chain = fullchain_pem[len(cert):].lstrip() return (cert, chain) + +def get_serial_from_cert(cert_path): + """Retrieve the serial number of a certificate from certificate path + + :param str cert_path: path to a cert in PEM format + + :returns: serial number of the certificate + :rtype: int + """ + # pylint: disable=redefined-outer-name + with open(cert_path) as f: + x509 = crypto.load_certificate(crypto.FILETYPE_PEM, + f.read()) + return x509.get_serial_number() diff --git a/certbot/tests/cert_manager_test.py b/certbot/tests/cert_manager_test.py index eb8005b2b..bea64f09c 100644 --- a/certbot/tests/cert_manager_test.py +++ b/certbot/tests/cert_manager_test.py @@ -200,9 +200,11 @@ class CertificatesTest(BaseCertManagerTest): self.assertTrue(mock_utility.called) shutil.rmtree(empty_tempdir) + @mock.patch('certbot.crypto_util.get_serial_from_cert') @mock.patch('certbot._internal.cert_manager.ocsp.RevocationChecker.ocsp_revoked') - def test_report_human_readable(self, mock_revoked): + def test_report_human_readable(self, mock_revoked, mock_serial): mock_revoked.return_value = None + mock_serial.return_value = 1234567890 from certbot._internal import cert_manager import datetime import pytz From 07abe7a8d68961042ee301039dd4da87306cb1a0 Mon Sep 17 00:00:00 2001 From: Adrien Ferrand Date: Thu, 12 Mar 2020 21:53:19 +0100 Subject: [PATCH 31/32] Reimplement tls-alpn-01 in acme (#6886) This PR is the first part of work described in #6724. It reintroduces the tls-alpn-01 challenge in `acme` module, that was introduced by #5894 and reverted by #6100. The reason it was removed in the past is because some tests showed that with `1.0.2` branch of OpenSSL, the self-signed certificate containing the authorization key is sent to the requester even if the ALPN protocol `acme-tls/1` was not declared as supported by the requester during the TLS handshake. However recent discussions lead to the conclusion that this behavior was not a security issue, because first it is coherent with the behavior with servers that do not support ALPN at all, and second it cannot make a tls-alpn-01 challenge be validated in this kind of corner case. On top of the original modifications given by #5894, I merged the code to be up-to-date with our `master`, and fixed tests to match recent evolution about not displaying the `keyAuthorization` in the deserialized JSON form of an ACME challenge. I also move the logic to verify if ALPN is available on the current system, and so that the tls-alpn-01 challenge can be used, to a dedicated static function `is_available` in `acme.challenge.TLSALPN01`. This function is used in the related tests to skip them, and will be used in the future from Certbot plugins to trigger or not the logic related to tls-alpn-01, depending on the OpenSSL version available to Python. * Reimplement TLS-ALPN-01 challenge and standalone TLS-ALPN server from #5894. * Setup a class method to check if tls-alpn-01 is supported. * Add potential missing parameter in validation for tls-alpn * Improve comments * Make a class private * Handle old versions of openssl that do not terminate the handshake when they should do. * Add changelog * Explicitly close the TLS connection by the book. * Remove unused exception * Fix lint --- acme/acme/challenges.py | 172 ++++++++++++++++++++++++--- acme/acme/crypto_util.py | 63 +++++++--- acme/acme/standalone.py | 56 ++++++++- acme/tests/challenges_test.py | 87 ++++++++++++-- acme/tests/crypto_util_test.py | 16 ++- acme/tests/standalone_test.py | 57 ++++++++- acme/tests/testdata/README | 6 +- acme/tests/testdata/rsa1024_cert.pem | 13 ++ certbot/CHANGELOG.md | 2 + 9 files changed, 425 insertions(+), 47 deletions(-) create mode 100644 acme/tests/testdata/rsa1024_cert.pem diff --git a/acme/acme/challenges.py b/acme/acme/challenges.py index 39c8d6269..0b112be00 100644 --- a/acme/acme/challenges.py +++ b/acme/acme/challenges.py @@ -1,14 +1,20 @@ """ACME Identifier Validation Challenges.""" import abc +import codecs import functools import hashlib import logging +import socket from cryptography.hazmat.primitives import hashes # type: ignore import josepy as jose import requests import six +from OpenSSL import SSL # type: ignore # https://github.com/python/typeshed/issues/2052 +from OpenSSL import crypto +from acme import crypto_util +from acme import errors from acme import fields logger = logging.getLogger(__name__) @@ -362,29 +368,163 @@ class HTTP01(KeyAuthorizationChallenge): @ChallengeResponse.register class TLSALPN01Response(KeyAuthorizationChallengeResponse): - """ACME TLS-ALPN-01 challenge response. - - This class only allows initiating a TLS-ALPN-01 challenge returned from the - CA. Full support for responding to TLS-ALPN-01 challenges by generating and - serving the expected response certificate is not currently provided. - """ + """ACME tls-alpn-01 challenge response.""" typ = "tls-alpn-01" + PORT = 443 + """Verification port as defined by the protocol. -@Challenge.register + You can override it (e.g. for testing) by passing ``port`` to + `simple_verify`. + + """ + + ID_PE_ACME_IDENTIFIER_V1 = b"1.3.6.1.5.5.7.1.30.1" + ACME_TLS_1_PROTOCOL = "acme-tls/1" + + @property + def h(self): + """Hash value stored in challenge certificate""" + return hashlib.sha256(self.key_authorization.encode('utf-8')).digest() + + def gen_cert(self, domain, key=None, bits=2048): + """Generate tls-alpn-01 certificate. + + :param unicode domain: Domain verified by the challenge. + :param OpenSSL.crypto.PKey key: Optional private key used in + certificate generation. If not provided (``None``), then + fresh key will be generated. + :param int bits: Number of bits for newly generated key. + + :rtype: `tuple` of `OpenSSL.crypto.X509` and `OpenSSL.crypto.PKey` + + """ + if key is None: + key = crypto.PKey() + key.generate_key(crypto.TYPE_RSA, bits) + + + der_value = b"DER:" + codecs.encode(self.h, 'hex') + acme_extension = crypto.X509Extension(self.ID_PE_ACME_IDENTIFIER_V1, + critical=True, value=der_value) + + return crypto_util.gen_ss_cert(key, [domain], force_san=True, + extensions=[acme_extension]), key + + def probe_cert(self, domain, host=None, port=None): + """Probe tls-alpn-01 challenge certificate. + + :param unicode domain: domain being validated, required. + :param string host: IP address used to probe the certificate. + :param int port: Port used to probe the certificate. + + """ + if host is None: + host = socket.gethostbyname(domain) + logger.debug('%s resolved to %s', domain, host) + if port is None: + port = self.PORT + + return crypto_util.probe_sni(host=host, port=port, name=domain, + alpn_protocols=[self.ACME_TLS_1_PROTOCOL]) + + def verify_cert(self, domain, cert): + """Verify tls-alpn-01 challenge certificate. + + :param unicode domain: Domain name being validated. + :param OpensSSL.crypto.X509 cert: Challenge certificate. + + :returns: Whether the certificate was successfully verified. + :rtype: bool + + """ + # pylint: disable=protected-access + names = crypto_util._pyopenssl_cert_or_req_all_names(cert) + logger.debug('Certificate %s. SANs: %s', cert.digest('sha256'), names) + if len(names) != 1 or names[0].lower() != domain.lower(): + return False + + for i in range(cert.get_extension_count()): + ext = cert.get_extension(i) + # FIXME: assume this is the ACME extension. Currently there is no + # way to get full OID of an unknown extension from pyopenssl. + if ext.get_short_name() == b'UNDEF': + data = ext.get_data() + return data == self.h + + return False + + # pylint: disable=too-many-arguments + def simple_verify(self, chall, domain, account_public_key, + cert=None, host=None, port=None): + """Simple verify. + + Verify ``validation`` using ``account_public_key``, optionally + probe tls-alpn-01 certificate and check using `verify_cert`. + + :param .challenges.TLSALPN01 chall: Corresponding challenge. + :param str domain: Domain name being validated. + :param JWK account_public_key: + :param OpenSSL.crypto.X509 cert: Optional certificate. If not + provided (``None``) certificate will be retrieved using + `probe_cert`. + :param string host: IP address used to probe the certificate. + :param int port: Port used to probe the certificate. + + + :returns: ``True`` if and only if client's control of the domain has been verified. + :rtype: bool + + """ + if not self.verify(chall, account_public_key): + logger.debug("Verification of key authorization in response failed") + return False + + if cert is None: + try: + cert = self.probe_cert(domain=domain, host=host, port=port) + except errors.Error as error: + logger.debug(str(error), exc_info=True) + return False + + return self.verify_cert(domain, cert) + + +@Challenge.register # pylint: disable=too-many-ancestors class TLSALPN01(KeyAuthorizationChallenge): - """ACME tls-alpn-01 challenge. - - This class simply allows parsing the TLS-ALPN-01 challenge returned from - the CA. Full TLS-ALPN-01 support is not currently provided. - - """ - typ = "tls-alpn-01" + """ACME tls-alpn-01 challenge.""" response_cls = TLSALPN01Response + typ = response_cls.typ def validation(self, account_key, **kwargs): - """Generate validation for the challenge.""" - raise NotImplementedError() + """Generate validation. + + :param JWK account_key: + :param unicode domain: Domain verified by the challenge. + :param OpenSSL.crypto.PKey cert_key: Optional private key used + in certificate generation. If not provided (``None``), then + fresh key will be generated. + + :rtype: `tuple` of `OpenSSL.crypto.X509` and `OpenSSL.crypto.PKey` + + """ + return self.response(account_key).gen_cert( + key=kwargs.get('cert_key'), + domain=kwargs.get('domain')) + + @staticmethod + def is_supported(): + """ + Check if TLS-ALPN-01 challenge is supported on this machine. + This implies that a recent version of OpenSSL is installed (>= 1.0.2), + or a recent cryptography version shipped with the OpenSSL library is installed. + + :returns: ``True`` if TLS-ALPN-01 is supported on this machine, ``False`` otherwise. + :rtype: bool + + """ + return (hasattr(SSL.Connection, "set_alpn_protos") + and hasattr(SSL.Context, "set_alpn_select_callback")) @Challenge.register diff --git a/acme/acme/crypto_util.py b/acme/acme/crypto_util.py index dc8fedad0..f8b7e2b30 100644 --- a/acme/acme/crypto_util.py +++ b/acme/acme/crypto_util.py @@ -27,19 +27,41 @@ logger = logging.getLogger(__name__) _DEFAULT_SSL_METHOD = SSL.SSLv23_METHOD # type: ignore -class SSLSocket(object): +class _DefaultCertSelection(object): + def __init__(self, certs): + self.certs = certs + + def __call__(self, connection): + server_name = connection.get_servername() + return self.certs.get(server_name, None) + + +class SSLSocket(object): # pylint: disable=too-few-public-methods """SSL wrapper for sockets. :ivar socket sock: Original wrapped socket. :ivar dict certs: Mapping from domain names (`bytes`) to `OpenSSL.crypto.X509`. :ivar method: See `OpenSSL.SSL.Context` for allowed values. + :ivar alpn_selection: Hook to select negotiated ALPN protocol for + connection. + :ivar cert_selection: Hook to select certificate for connection. If given, + `certs` parameter would be ignored, and therefore must be empty. """ - def __init__(self, sock, certs, method=_DEFAULT_SSL_METHOD): + def __init__(self, sock, certs=None, + method=_DEFAULT_SSL_METHOD, alpn_selection=None, + cert_selection=None): self.sock = sock - self.certs = certs + self.alpn_selection = alpn_selection self.method = method + if not cert_selection and not certs: + raise ValueError("Neither cert_selection or certs specified.") + if cert_selection and certs: + raise ValueError("Both cert_selection and certs specified.") + if cert_selection is None: + cert_selection = _DefaultCertSelection(certs) + self.cert_selection = cert_selection def __getattr__(self, name): return getattr(self.sock, name) @@ -56,18 +78,19 @@ class SSLSocket(object): :type connection: :class:`OpenSSL.Connection` """ - server_name = connection.get_servername() - try: - key, cert = self.certs[server_name] - except KeyError: - logger.debug("Server name (%s) not recognized, dropping SSL", - server_name) + pair = self.cert_selection(connection) + if pair is None: + logger.debug("Certificate selection for server name %s failed, dropping SSL", + connection.get_servername()) return + key, cert = pair new_context = SSL.Context(self.method) new_context.set_options(SSL.OP_NO_SSLv2) new_context.set_options(SSL.OP_NO_SSLv3) new_context.use_privatekey(key) new_context.use_certificate(cert) + if self.alpn_selection is not None: + new_context.set_alpn_select_callback(self.alpn_selection) connection.set_context(new_context) class FakeConnection(object): @@ -92,6 +115,8 @@ class SSLSocket(object): context.set_options(SSL.OP_NO_SSLv2) context.set_options(SSL.OP_NO_SSLv3) context.set_tlsext_servername_callback(self._pick_certificate_cb) + if self.alpn_selection is not None: + context.set_alpn_select_callback(self.alpn_selection) ssl_sock = self.FakeConnection(SSL.Connection(context, sock)) ssl_sock.set_accept_state() @@ -107,8 +132,9 @@ class SSLSocket(object): return ssl_sock, addr -def probe_sni(name, host, port=443, timeout=300, - method=_DEFAULT_SSL_METHOD, source_address=('', 0)): +def probe_sni(name, host, port=443, timeout=300, # pylint: disable=too-many-arguments + method=_DEFAULT_SSL_METHOD, source_address=('', 0), + alpn_protocols=None): """Probe SNI server for SSL certificate. :param bytes name: Byte string to send as the server name in the @@ -120,6 +146,8 @@ def probe_sni(name, host, port=443, timeout=300, :param tuple source_address: Enables multi-path probing (selection of source interface). See `socket.creation_connection` for more info. Available only in Python 2.7+. + :param alpn_protocols: Protocols to request using ALPN. + :type alpn_protocols: `list` of `bytes` :raises acme.errors.Error: In case of any problems. @@ -149,6 +177,8 @@ def probe_sni(name, host, port=443, timeout=300, client_ssl = SSL.Connection(context, client) client_ssl.set_connect_state() client_ssl.set_tlsext_host_name(name) # pyOpenSSL>=0.13 + if alpn_protocols is not None: + client_ssl.set_alpn_protos(alpn_protocols) try: client_ssl.do_handshake() client_ssl.shutdown() @@ -239,12 +269,14 @@ def _pyopenssl_cert_or_req_san(cert_or_req): def gen_ss_cert(key, domains, not_before=None, - validity=(7 * 24 * 60 * 60), force_san=True): + validity=(7 * 24 * 60 * 60), force_san=True, extensions=None): """Generate new self-signed certificate. :type domains: `list` of `unicode` :param OpenSSL.crypto.PKey key: :param bool force_san: + :param extensions: List of additional extensions to include in the cert. + :type extensions: `list` of `OpenSSL.crypto.X509Extension` If more than one domain is provided, all of the domains are put into ``subjectAltName`` X.509 extension and first domain is set as the @@ -257,10 +289,13 @@ def gen_ss_cert(key, domains, not_before=None, cert.set_serial_number(int(binascii.hexlify(os.urandom(16)), 16)) cert.set_version(2) - extensions = [ + if extensions is None: + extensions = [] + + extensions.append( crypto.X509Extension( b"basicConstraints", True, b"CA:TRUE, pathlen:0"), - ] + ) cert.get_subject().CN = domains[0] # TODO: what to put into cert.get_subject()? diff --git a/acme/acme/standalone.py b/acme/acme/standalone.py index 236f2c234..52ac07915 100644 --- a/acme/acme/standalone.py +++ b/acme/acme/standalone.py @@ -33,7 +33,14 @@ class TLSServer(socketserver.TCPServer): def _wrap_sock(self): self.socket = crypto_util.SSLSocket( - self.socket, certs=self.certs, method=self.method) + self.socket, cert_selection=self._cert_selection, + alpn_selection=getattr(self, '_alpn_selection', None), + method=self.method) + + def _cert_selection(self, connection): # pragma: no cover + """Callback selecting certificate for connection.""" + server_name = connection.get_servername() + return self.certs.get(server_name, None) def server_bind(self): self._wrap_sock() @@ -120,6 +127,40 @@ class BaseDualNetworkedServers(object): self.threads = [] +class TLSALPN01Server(TLSServer, ACMEServerMixin): + """TLSALPN01 Server.""" + + ACME_TLS_1_PROTOCOL = b"acme-tls/1" + + def __init__(self, server_address, certs, challenge_certs, ipv6=False): + TLSServer.__init__( + self, server_address, _BaseRequestHandlerWithLogging, certs=certs, + ipv6=ipv6) + self.challenge_certs = challenge_certs + + def _cert_selection(self, connection): + # TODO: We would like to serve challenge cert only if asked for it via + # ALPN. To do this, we need to retrieve the list of protos from client + # hello, but this is currently impossible with openssl [0], and ALPN + # negotiation is done after cert selection. + # Therefore, currently we always return challenge cert, and terminate + # handshake in alpn_selection() if ALPN protos are not what we expect. + # [0] https://github.com/openssl/openssl/issues/4952 + server_name = connection.get_servername() + logger.debug("Serving challenge cert for server name %s", server_name) + return self.challenge_certs.get(server_name, None) + + def _alpn_selection(self, _connection, alpn_protos): + """Callback to select alpn protocol.""" + if len(alpn_protos) == 1 and alpn_protos[0] == self.ACME_TLS_1_PROTOCOL: + logger.debug("Agreed on %s ALPN", self.ACME_TLS_1_PROTOCOL) + return self.ACME_TLS_1_PROTOCOL + logger.debug("Cannot agree on ALPN proto. Got: %s", str(alpn_protos)) + # Explicitly close the connection now, by returning an empty string. + # See https://www.pyopenssl.org/en/stable/api/ssl.html#OpenSSL.SSL.Context.set_alpn_select_callback # pylint: disable=line-too-long + return b"" + + class HTTPServer(BaseHTTPServer.HTTPServer): """Generic HTTP Server.""" @@ -222,3 +263,16 @@ class HTTP01RequestHandler(BaseHTTPServer.BaseHTTPRequestHandler): """ return functools.partial( cls, simple_http_resources=simple_http_resources) + + +class _BaseRequestHandlerWithLogging(socketserver.BaseRequestHandler): + """BaseRequestHandler with logging.""" + + def log_message(self, format, *args): # pylint: disable=redefined-builtin + """Log arbitrary message.""" + logger.debug("%s - - %s", self.client_address[0], format % args) + + def handle(self): + """Handle request.""" + self.log_message("Incoming request") + socketserver.BaseRequestHandler.handle(self) diff --git a/acme/tests/challenges_test.py b/acme/tests/challenges_test.py index adebaffc5..2b44d677d 100644 --- a/acme/tests/challenges_test.py +++ b/acme/tests/challenges_test.py @@ -2,10 +2,13 @@ import unittest import josepy as jose +import OpenSSL import mock import requests from six.moves.urllib import parse as urllib_parse +from acme import errors + import test_util CERT = test_util.load_comparable_cert('cert.pem') @@ -256,30 +259,87 @@ class HTTP01Test(unittest.TestCase): class TLSALPN01ResponseTest(unittest.TestCase): def setUp(self): - from acme.challenges import TLSALPN01Response - self.msg = TLSALPN01Response(key_authorization=u'foo') + from acme.challenges import TLSALPN01 + self.chall = TLSALPN01( + token=jose.b64decode(b'a82d5ff8ef740d12881f6d3c2277ab2e')) + self.domain = u'example.com' + self.domain2 = u'example2.com' + + self.response = self.chall.response(KEY) self.jmsg = { 'resource': 'challenge', 'type': 'tls-alpn-01', - 'keyAuthorization': u'foo', + 'keyAuthorization': self.response.key_authorization, } - from acme.challenges import TLSALPN01 - self.chall = TLSALPN01(token=(b'x' * 16)) - self.response = self.chall.response(KEY) - def test_to_partial_json(self): self.assertEqual({k: v for k, v in self.jmsg.items() if k != 'keyAuthorization'}, - self.msg.to_partial_json()) + self.response.to_partial_json()) def test_from_json(self): from acme.challenges import TLSALPN01Response - self.assertEqual(self.msg, TLSALPN01Response.from_json(self.jmsg)) + self.assertEqual(self.response, TLSALPN01Response.from_json(self.jmsg)) def test_from_json_hashable(self): from acme.challenges import TLSALPN01Response hash(TLSALPN01Response.from_json(self.jmsg)) + def test_gen_verify_cert(self): + key1 = test_util.load_pyopenssl_private_key('rsa512_key.pem') + cert, key2 = self.response.gen_cert(self.domain, key1) + self.assertEqual(key1, key2) + self.assertTrue(self.response.verify_cert(self.domain, cert)) + + def test_gen_verify_cert_gen_key(self): + cert, key = self.response.gen_cert(self.domain) + self.assertTrue(isinstance(key, OpenSSL.crypto.PKey)) + self.assertTrue(self.response.verify_cert(self.domain, cert)) + + def test_verify_bad_cert(self): + self.assertFalse(self.response.verify_cert(self.domain, + test_util.load_cert('cert.pem'))) + + def test_verify_bad_domain(self): + key1 = test_util.load_pyopenssl_private_key('rsa512_key.pem') + cert, key2 = self.response.gen_cert(self.domain, key1) + self.assertEqual(key1, key2) + self.assertFalse(self.response.verify_cert(self.domain2, cert)) + + def test_simple_verify_bad_key_authorization(self): + key2 = jose.JWKRSA.load(test_util.load_vector('rsa256_key.pem')) + self.response.simple_verify(self.chall, "local", key2.public_key()) + + @mock.patch('acme.challenges.TLSALPN01Response.verify_cert', autospec=True) + def test_simple_verify(self, mock_verify_cert): + mock_verify_cert.return_value = mock.sentinel.verification + self.assertEqual( + mock.sentinel.verification, self.response.simple_verify( + self.chall, self.domain, KEY.public_key(), + cert=mock.sentinel.cert)) + mock_verify_cert.assert_called_once_with( + self.response, self.domain, mock.sentinel.cert) + + @mock.patch('acme.challenges.socket.gethostbyname') + @mock.patch('acme.challenges.crypto_util.probe_sni') + def test_probe_cert(self, mock_probe_sni, mock_gethostbyname): + mock_gethostbyname.return_value = '127.0.0.1' + self.response.probe_cert('foo.com') + mock_gethostbyname.assert_called_once_with('foo.com') + mock_probe_sni.assert_called_once_with( + host='127.0.0.1', port=self.response.PORT, name='foo.com', + alpn_protocols=['acme-tls/1']) + + self.response.probe_cert('foo.com', host='8.8.8.8') + mock_probe_sni.assert_called_with( + host='8.8.8.8', port=mock.ANY, name='foo.com', + alpn_protocols=['acme-tls/1']) + + @mock.patch('acme.challenges.TLSALPN01Response.probe_cert') + def test_simple_verify_false_on_probe_error(self, mock_probe_cert): + mock_probe_cert.side_effect = errors.Error + self.assertFalse(self.response.simple_verify( + self.chall, self.domain, KEY.public_key())) + class TLSALPN01Test(unittest.TestCase): @@ -309,8 +369,13 @@ class TLSALPN01Test(unittest.TestCase): self.assertRaises( jose.DeserializationError, TLSALPN01.from_json, self.jmsg) - def test_validation(self): - self.assertRaises(NotImplementedError, self.msg.validation, KEY) + @mock.patch('acme.challenges.TLSALPN01Response.gen_cert') + def test_validation(self, mock_gen_cert): + mock_gen_cert.return_value = ('cert', 'key') + self.assertEqual(('cert', 'key'), self.msg.validation( + KEY, cert_key=mock.sentinel.cert_key, domain=mock.sentinel.domain)) + mock_gen_cert.assert_called_once_with(key=mock.sentinel.cert_key, + domain=mock.sentinel.domain) class DNSTest(unittest.TestCase): diff --git a/acme/tests/crypto_util_test.py b/acme/tests/crypto_util_test.py index 41640ed60..ff08a5405 100644 --- a/acme/tests/crypto_util_test.py +++ b/acme/tests/crypto_util_test.py @@ -18,7 +18,6 @@ import test_util class SSLSocketAndProbeSNITest(unittest.TestCase): """Tests for acme.crypto_util.SSLSocket/probe_sni.""" - def setUp(self): self.cert = test_util.load_comparable_cert('rsa2048_cert.pem') key = test_util.load_pyopenssl_private_key('rsa2048_key.pem') @@ -32,7 +31,8 @@ class SSLSocketAndProbeSNITest(unittest.TestCase): # six.moves.* | pylint: disable=attribute-defined-outside-init,no-init def server_bind(self): # pylint: disable=missing-docstring - self.socket = SSLSocket(socket.socket(), certs=certs) + self.socket = SSLSocket(socket.socket(), + certs) socketserver.TCPServer.server_bind(self) self.server = _TestServer(('', 0), socketserver.BaseRequestHandler) @@ -73,6 +73,18 @@ class SSLSocketAndProbeSNITest(unittest.TestCase): socket.setdefaulttimeout(original_timeout) +class SSLSocketTest(unittest.TestCase): + """Tests for acme.crypto_util.SSLSocket.""" + + def test_ssl_socket_invalid_arguments(self): + from acme.crypto_util import SSLSocket + with self.assertRaises(ValueError): + _ = SSLSocket(None, {'sni': ('key', 'cert')}, + cert_selection=lambda _: None) + with self.assertRaises(ValueError): + _ = SSLSocket(None) + + class PyOpenSSLCertOrReqAllNamesTest(unittest.TestCase): """Test for acme.crypto_util._pyopenssl_cert_or_req_all_names.""" diff --git a/acme/tests/standalone_test.py b/acme/tests/standalone_test.py index 83ced12b0..e2817b29c 100644 --- a/acme/tests/standalone_test.py +++ b/acme/tests/standalone_test.py @@ -10,7 +10,10 @@ from six.moves import http_client # pylint: disable=import-error from six.moves import socketserver # type: ignore # pylint: disable=import-error from acme import challenges +from acme import crypto_util +from acme import errors from acme.magic_typing import Set # pylint: disable=unused-import, no-name-in-module + import test_util @@ -84,6 +87,59 @@ class HTTP01ServerTest(unittest.TestCase): self.assertFalse(self._test_http01(add=False)) +@unittest.skipIf(not challenges.TLSALPN01.is_supported(), "pyOpenSSL too old") +class TLSALPN01ServerTest(unittest.TestCase): + """Test for acme.standalone.TLSALPN01Server.""" + + def setUp(self): + self.certs = {b'localhost': ( + test_util.load_pyopenssl_private_key('rsa2048_key.pem'), + test_util.load_cert('rsa2048_cert.pem'), + )} + # Use different certificate for challenge. + self.challenge_certs = {b'localhost': ( + test_util.load_pyopenssl_private_key('rsa1024_key.pem'), + test_util.load_cert('rsa1024_cert.pem'), + )} + from acme.standalone import TLSALPN01Server + self.server = TLSALPN01Server(("localhost", 0), certs=self.certs, + challenge_certs=self.challenge_certs) + # pylint: disable=no-member + self.thread = threading.Thread(target=self.server.serve_forever) + self.thread.start() + + def tearDown(self): + self.server.shutdown() # pylint: disable=no-member + self.thread.join() + + # TODO: This is not implemented yet, see comments in standalone.py + # def test_certs(self): + # host, port = self.server.socket.getsockname()[:2] + # cert = crypto_util.probe_sni( + # b'localhost', host=host, port=port, timeout=1) + # # Expect normal cert when connecting without ALPN. + # self.assertEqual(jose.ComparableX509(cert), + # jose.ComparableX509(self.certs[b'localhost'][1])) + + def test_challenge_certs(self): + host, port = self.server.socket.getsockname()[:2] + cert = crypto_util.probe_sni( + b'localhost', host=host, port=port, timeout=1, + alpn_protocols=[b"acme-tls/1"]) + # Expect challenge cert when connecting with ALPN. + self.assertEqual( + jose.ComparableX509(cert), + jose.ComparableX509(self.challenge_certs[b'localhost'][1]) + ) + + def test_bad_alpn(self): + host, port = self.server.socket.getsockname()[:2] + with self.assertRaises(errors.Error): + crypto_util.probe_sni( + b'localhost', host=host, port=port, timeout=1, + alpn_protocols=[b"bad-alpn"]) + + class BaseDualNetworkedServersTest(unittest.TestCase): """Test for acme.standalone.BaseDualNetworkedServers.""" @@ -138,7 +194,6 @@ class BaseDualNetworkedServersTest(unittest.TestCase): class HTTP01DualNetworkedServersTest(unittest.TestCase): """Tests for acme.standalone.HTTP01DualNetworkedServers.""" - def setUp(self): self.account_key = jose.JWK.load( test_util.load_vector('rsa1024_key.pem')) diff --git a/acme/tests/testdata/README b/acme/tests/testdata/README index dfe3f5405..d65cc3018 100644 --- a/acme/tests/testdata/README +++ b/acme/tests/testdata/README @@ -10,6 +10,8 @@ and for the CSR: openssl req -key rsa2048_key.pem -new -subj '/CN=example.com' -outform DER > csr.der -and for the certificate: +and for the certificates: - openssl req -key rsa2047_key.pem -new -subj '/CN=example.com' -x509 -outform DER > cert.der + openssl req -key rsa2048_key.pem -new -subj '/CN=example.com' -x509 -outform DER > cert.der + openssl req -key rsa2048_key.pem -new -subj '/CN=example.com' -x509 > rsa2048_cert.pem + openssl req -key rsa1024_key.pem -new -subj '/CN=example.com' -x509 > rsa1024_cert.pem diff --git a/acme/tests/testdata/rsa1024_cert.pem b/acme/tests/testdata/rsa1024_cert.pem new file mode 100644 index 000000000..1b7912181 --- /dev/null +++ b/acme/tests/testdata/rsa1024_cert.pem @@ -0,0 +1,13 @@ +-----BEGIN CERTIFICATE----- +MIIB/TCCAWagAwIBAgIJAOyRIBs3QT8QMA0GCSqGSIb3DQEBCwUAMBYxFDASBgNV +BAMMC2V4YW1wbGUuY29tMB4XDTE4MDQyMzEwMzE0NFoXDTE4MDUyMzEwMzE0NFow +FjEUMBIGA1UEAwwLZXhhbXBsZS5jb20wgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJ +AoGBAJqJ87R8aVwByONxgQA9hwgvQd/QqI1r1UInXhEF2VnEtZGtUWLi100IpIqr +Mq4qusDwNZ3g8cUPtSkvJGs89djoajMDIJP7lQUEKUYnYrI0q755Tr/DgLWSk7iW +l5ezym0VzWUD0/xXUz8yRbNMTjTac80rS5SZk2ja2wWkYlRJAgMBAAGjUzBRMB0G +A1UdDgQWBBSsaX0IVZ4XXwdeffVAbG7gnxSYjTAfBgNVHSMEGDAWgBSsaX0IVZ4X +XwdeffVAbG7gnxSYjTAPBgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3DQEBCwUAA4GB +ADe7SVmvGH2nkwVfONk8TauRUDkePN1CJZKFb2zW1uO9ANJ2v5Arm/OQp0BG/xnI +Djw/aLTNVESF89oe15dkrUErtcaF413MC1Ld5lTCaJLHLGqDKY69e02YwRuxW7jY +qarpt7k7aR5FbcfO5r4V/FK/Gvp4Dmoky8uap7SJIW6x +-----END CERTIFICATE----- diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md index 2f22b5204..36547fdd1 100644 --- a/certbot/CHANGELOG.md +++ b/certbot/CHANGELOG.md @@ -11,6 +11,8 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). the `manual` plugin: `CERTBOT_REMAINING_CHALLENGES` is equal to the number of challenges remaining after the current challenge, `CERTBOT_ALL_DOMAINS` is a comma-separated list of all domains challenged for the current certificate. +* Added TLS-ALPN-01 challenge support in the `acme` library. Support of this + challenge in the Certbot client is planned to be added in a future release. ### Changed From 809cb516c918575bc1688141dfe9b4da001d6570 Mon Sep 17 00:00:00 2001 From: Adrien Ferrand Date: Fri, 13 Mar 2020 17:56:35 +0100 Subject: [PATCH 32/32] Fix acme compliance to RFC 8555 (#7176) This PR is an alternative to #7125. Instead of disabling the strict mode on Pebble, this PR fixes the JWS payloads regarding RFC 8555 to be compliant, and allow certbot to work with Pebble v2.1.0+. * Fix acme compliance to RFC 8555. * Working mixin * Activate back pebble strict mode * Use mixin for type * Update dependencies * Fix also in fields_to_partial_json * Update pebble * Add changelog --- acme/acme/challenges.py | 3 +- acme/acme/client.py | 3 + acme/acme/messages.py | 13 ++-- acme/acme/mixins.py | 65 +++++++++++++++++++ acme/tests/challenges_test.py | 13 ++++ acme/tests/client_test.py | 3 +- acme/tests/messages_test.py | 14 ++++ .../utils/acme_server.py | 2 +- .../utils/pebble_artifacts.py | 2 +- certbot-nginx/local-oldest-requirements.txt | 4 +- certbot-nginx/setup.py | 4 +- certbot/CHANGELOG.md | 3 +- certbot/local-oldest-requirements.txt | 2 +- certbot/setup.py | 2 +- 14 files changed, 116 insertions(+), 17 deletions(-) create mode 100644 acme/acme/mixins.py diff --git a/acme/acme/challenges.py b/acme/acme/challenges.py index 0b112be00..b9c6b7eb2 100644 --- a/acme/acme/challenges.py +++ b/acme/acme/challenges.py @@ -16,6 +16,7 @@ from OpenSSL import crypto from acme import crypto_util from acme import errors from acme import fields +from acme.mixins import ResourceMixin, TypeMixin logger = logging.getLogger(__name__) @@ -34,7 +35,7 @@ class Challenge(jose.TypedJSONObjectWithFields): return UnrecognizedChallenge.from_json(jobj) -class ChallengeResponse(jose.TypedJSONObjectWithFields): +class ChallengeResponse(ResourceMixin, TypeMixin, jose.TypedJSONObjectWithFields): # _fields_to_partial_json """ACME challenge response.""" TYPES = {} # type: dict diff --git a/acme/acme/client.py b/acme/acme/client.py index cecb727c7..cbe543f91 100644 --- a/acme/acme/client.py +++ b/acme/acme/client.py @@ -25,6 +25,7 @@ from acme.magic_typing import Dict from acme.magic_typing import List from acme.magic_typing import Set from acme.magic_typing import Text +from acme.mixins import VersionedLEACMEMixin logger = logging.getLogger(__name__) @@ -987,6 +988,8 @@ class ClientNetwork(object): :rtype: `josepy.JWS` """ + if isinstance(obj, VersionedLEACMEMixin): + obj.le_acme_version = acme_version jobj = obj.json_dumps(indent=2).encode() if obj else b'' logger.debug('JWS payload:\n%s', jobj) kwargs = { diff --git a/acme/acme/messages.py b/acme/acme/messages.py index f8f4bfbe7..90059a6fb 100644 --- a/acme/acme/messages.py +++ b/acme/acme/messages.py @@ -9,6 +9,7 @@ from acme import errors from acme import fields from acme import jws from acme import util +from acme.mixins import ResourceMixin try: from collections.abc import Hashable @@ -356,13 +357,13 @@ class Registration(ResourceBody): @Directory.register -class NewRegistration(Registration): +class NewRegistration(ResourceMixin, Registration): """New registration.""" resource_type = 'new-reg' resource = fields.Resource(resource_type) -class UpdateRegistration(Registration): +class UpdateRegistration(ResourceMixin, Registration): """Update registration.""" resource_type = 'reg' resource = fields.Resource(resource_type) @@ -498,13 +499,13 @@ class Authorization(ResourceBody): @Directory.register -class NewAuthorization(Authorization): +class NewAuthorization(ResourceMixin, Authorization): """New authorization.""" resource_type = 'new-authz' resource = fields.Resource(resource_type) -class UpdateAuthorization(Authorization): +class UpdateAuthorization(ResourceMixin, Authorization): """Update authorization.""" resource_type = 'authz' resource = fields.Resource(resource_type) @@ -522,7 +523,7 @@ class AuthorizationResource(ResourceWithURI): @Directory.register -class CertificateRequest(jose.JSONObjectWithFields): +class CertificateRequest(ResourceMixin, jose.JSONObjectWithFields): """ACME new-cert request. :ivar josepy.util.ComparableX509 csr: @@ -548,7 +549,7 @@ class CertificateResource(ResourceWithURI): @Directory.register -class Revocation(jose.JSONObjectWithFields): +class Revocation(ResourceMixin, jose.JSONObjectWithFields): """Revocation message. :ivar .ComparableX509 certificate: `OpenSSL.crypto.X509` wrapped in diff --git a/acme/acme/mixins.py b/acme/acme/mixins.py new file mode 100644 index 000000000..1cd050ccc --- /dev/null +++ b/acme/acme/mixins.py @@ -0,0 +1,65 @@ +"""Useful mixins for Challenge and Resource objects""" + + +class VersionedLEACMEMixin(object): + """This mixin stores the version of Let's Encrypt's endpoint being used.""" + @property + def le_acme_version(self): + """Define the version of ACME protocol to use""" + return getattr(self, '_le_acme_version', 1) + + @le_acme_version.setter + def le_acme_version(self, version): + # We need to use object.__setattr__ to not depend on the specific implementation of + # __setattr__ in current class (eg. jose.TypedJSONObjectWithFields raises AttributeError + # for any attempt to set an attribute to make objects immutable). + object.__setattr__(self, '_le_acme_version', version) + + def __setattr__(self, key, value): + if key == 'le_acme_version': + # Required for @property to operate properly. See comment above. + object.__setattr__(self, key, value) + else: + super(VersionedLEACMEMixin, self).__setattr__(key, value) # pragma: no cover + + +class ResourceMixin(VersionedLEACMEMixin): + """ + This mixin generates a RFC8555 compliant JWS payload + by removing the `resource` field if needed (eg. ACME v2 protocol). + """ + def to_partial_json(self): + """See josepy.JSONDeserializable.to_partial_json()""" + return _safe_jobj_compliance(super(ResourceMixin, self), + 'to_partial_json', 'resource') + + def fields_to_partial_json(self): + """See josepy.JSONObjectWithFields.fields_to_partial_json()""" + return _safe_jobj_compliance(super(ResourceMixin, self), + 'fields_to_partial_json', 'resource') + + +class TypeMixin(VersionedLEACMEMixin): + """ + This mixin allows generation of a RFC8555 compliant JWS payload + by removing the `type` field if needed (eg. ACME v2 protocol). + """ + def to_partial_json(self): + """See josepy.JSONDeserializable.to_partial_json()""" + return _safe_jobj_compliance(super(TypeMixin, self), + 'to_partial_json', 'type') + + def fields_to_partial_json(self): + """See josepy.JSONObjectWithFields.fields_to_partial_json()""" + return _safe_jobj_compliance(super(TypeMixin, self), + 'fields_to_partial_json', 'type') + + +def _safe_jobj_compliance(instance, jobj_method, uncompliant_field): + if hasattr(instance, jobj_method): + jobj = getattr(instance, jobj_method)() + if instance.le_acme_version == 2: + jobj.pop(uncompliant_field, None) + return jobj + + raise AttributeError('Method {0}() is not implemented.'.format(jobj_method)) # pragma: no cover diff --git a/acme/tests/challenges_test.py b/acme/tests/challenges_test.py index 2b44d677d..433c7bb82 100644 --- a/acme/tests/challenges_test.py +++ b/acme/tests/challenges_test.py @@ -478,5 +478,18 @@ class DNSResponseTest(unittest.TestCase): self.msg.check_validation(self.chall, KEY.public_key())) +class JWSPayloadRFC8555Compliant(unittest.TestCase): + """Test for RFC8555 compliance of JWS generated from resources/challenges""" + def test_challenge_payload(self): + from acme.challenges import HTTP01Response + + challenge_body = HTTP01Response() + challenge_body.le_acme_version = 2 + + jobj = challenge_body.json_dumps(indent=2).encode() + # RFC8555 states that challenge responses must have an empty payload. + self.assertEqual(jobj, b'{}') + + if __name__ == '__main__': unittest.main() # pragma: no cover diff --git a/acme/tests/client_test.py b/acme/tests/client_test.py index a4966140f..1e132d79f 100644 --- a/acme/tests/client_test.py +++ b/acme/tests/client_test.py @@ -16,6 +16,7 @@ from acme import errors from acme import jws as acme_jws from acme import messages from acme.magic_typing import Dict # pylint: disable=unused-import, no-name-in-module +from acme.mixins import VersionedLEACMEMixin import messages_test import test_util @@ -886,7 +887,7 @@ class ClientV2Test(ClientTestBase): self.client.net.get.assert_not_called() -class MockJSONDeSerializable(jose.JSONDeSerializable): +class MockJSONDeSerializable(VersionedLEACMEMixin, jose.JSONDeSerializable): # pylint: disable=missing-docstring def __init__(self, value): self.value = value diff --git a/acme/tests/messages_test.py b/acme/tests/messages_test.py index b9b70266b..d53fb764c 100644 --- a/acme/tests/messages_test.py +++ b/acme/tests/messages_test.py @@ -453,6 +453,7 @@ class OrderResourceTest(unittest.TestCase): 'authorizations': None, }) + class NewOrderTest(unittest.TestCase): """Tests for acme.messages.NewOrder.""" @@ -467,5 +468,18 @@ class NewOrderTest(unittest.TestCase): }) +class JWSPayloadRFC8555Compliant(unittest.TestCase): + """Test for RFC8555 compliance of JWS generated from resources/challenges""" + def test_message_payload(self): + from acme.messages import NewAuthorization + + new_order = NewAuthorization() + new_order.le_acme_version = 2 + + jobj = new_order.json_dumps(indent=2).encode() + # RFC8555 states that JWS bodies must not have a resource field. + self.assertEqual(jobj, b'{}') + + if __name__ == '__main__': unittest.main() # pragma: no cover diff --git a/certbot-ci/certbot_integration_tests/utils/acme_server.py b/certbot-ci/certbot_integration_tests/utils/acme_server.py index 5483251e6..b14bd4a32 100755 --- a/certbot-ci/certbot_integration_tests/utils/acme_server.py +++ b/certbot-ci/certbot_integration_tests/utils/acme_server.py @@ -131,7 +131,7 @@ class ACMEServer(object): environ['PEBBLE_AUTHZREUSE'] = '100' self._launch_process( - [pebble_path, '-config', pebble_config_path, '-dnsserver', '127.0.0.1:8053'], + [pebble_path, '-config', pebble_config_path, '-dnsserver', '127.0.0.1:8053', '-strict'], env=environ) self._launch_process( diff --git a/certbot-ci/certbot_integration_tests/utils/pebble_artifacts.py b/certbot-ci/certbot_integration_tests/utils/pebble_artifacts.py index 2b1557928..7fe03b990 100644 --- a/certbot-ci/certbot_integration_tests/utils/pebble_artifacts.py +++ b/certbot-ci/certbot_integration_tests/utils/pebble_artifacts.py @@ -7,7 +7,7 @@ import requests from certbot_integration_tests.utils.constants import MOCK_OCSP_SERVER_PORT -PEBBLE_VERSION = 'v2.2.1' +PEBBLE_VERSION = 'v2.3.0' ASSETS_PATH = pkg_resources.resource_filename('certbot_integration_tests', 'assets') diff --git a/certbot-nginx/local-oldest-requirements.txt b/certbot-nginx/local-oldest-requirements.txt index cee142934..1782f15ba 100644 --- a/certbot-nginx/local-oldest-requirements.txt +++ b/certbot-nginx/local-oldest-requirements.txt @@ -1,3 +1,3 @@ # Remember to update setup.py to match the package versions below. -acme[dev]==1.0.0 -certbot[dev]==1.1.0 +-e acme[dev] +-e certbot[dev] diff --git a/certbot-nginx/setup.py b/certbot-nginx/setup.py index 34785f963..0d62e7d55 100644 --- a/certbot-nginx/setup.py +++ b/certbot-nginx/setup.py @@ -9,8 +9,8 @@ version = '1.4.0.dev0' # Remember to update local-oldest-requirements.txt when changing the minimum # acme/certbot version. install_requires = [ - 'acme>=1.0.0', - 'certbot>=1.1.0', + 'acme>=1.4.0.dev0', + 'certbot>=1.4.0.dev0', 'mock', 'PyOpenSSL', 'pyparsing>=1.5.5', # Python3 support diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md index 36547fdd1..0bbc9b7ff 100644 --- a/certbot/CHANGELOG.md +++ b/certbot/CHANGELOG.md @@ -20,7 +20,8 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). ### Fixed -* +* When using an RFC 8555 compliant endpoint, the `acme` library no longer sends the + `resource` field in any requests or the `type` field when responding to challenges. More details about these changes can be found on our GitHub repo. diff --git a/certbot/local-oldest-requirements.txt b/certbot/local-oldest-requirements.txt index f6d158890..0acc68652 100644 --- a/certbot/local-oldest-requirements.txt +++ b/certbot/local-oldest-requirements.txt @@ -1,2 +1,2 @@ # Remember to update setup.py to match the package versions below. -acme[dev]==0.40.0 +-e acme[dev] diff --git a/certbot/setup.py b/certbot/setup.py index d19327e5e..514aec8c5 100644 --- a/certbot/setup.py +++ b/certbot/setup.py @@ -36,7 +36,7 @@ version = meta['version'] # specified here to avoid masking the more specific request requirements in # acme. See https://github.com/pypa/pip/issues/988 for more info. install_requires = [ - 'acme>=0.40.0', + 'acme>=1.4.0.dev0', # We technically need ConfigArgParse 0.10.0 for Python 2.6 support, but # saying so here causes a runtime error against our temporary fork of 0.9.3 # in which we added 2.6 support (see #2243), so we relax the requirement.