diff --git a/CHANGELOG.md b/CHANGELOG.md index 724820356..07d00e3fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,8 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). ### Added -* +* Avoid to process again challenges that are already validated + when a certificate is issued. ### Changed @@ -22,6 +23,7 @@ all Certbot components during releases for the time being, however, the only package with changes other than its version number was: * acme +* certbot More details about these changes can be found on our GitHub repo. diff --git a/certbot-nginx/tests/boulder-integration.conf.sh b/certbot-nginx/tests/boulder-integration.conf.sh index 4374f9094..470eab28e 100755 --- a/certbot-nginx/tests/boulder-integration.conf.sh +++ b/certbot-nginx/tests/boulder-integration.conf.sh @@ -1,3 +1,4 @@ +#!/usr/bin/env bash # Based on # https://www.exratione.com/2014/03/running-nginx-as-a-non-root-user/ # https://github.com/exratione/non-root-nginx/blob/9a77f62e5d5cb9c9026fd62eece76b9514011019/nginx.conf @@ -52,7 +53,7 @@ http { listen 5002 $default_server; # IPv6. listen [::]:5002 $default_server; - server_name nginx.wtf nginx2.wtf; + server_name nginx.wtf nginx-tls.wtf nginx2.wtf; root $root/webroot; diff --git a/certbot-nginx/tests/boulder-integration.sh b/certbot-nginx/tests/boulder-integration.sh index 2a24e645f..03425734d 100755 --- a/certbot-nginx/tests/boulder-integration.sh +++ b/certbot-nginx/tests/boulder-integration.sh @@ -39,8 +39,8 @@ nginx -v reload_nginx certbot_test_nginx --domains nginx.wtf run test_deployment_and_rollback nginx.wtf -certbot_test_nginx --domains nginx.wtf run --preferred-challenges tls-sni -test_deployment_and_rollback nginx.wtf +certbot_test_nginx --domains nginx-tls.wtf run --preferred-challenges tls-sni +test_deployment_and_rollback nginx-tls.wtf certbot_test_nginx --domains nginx2.wtf --preferred-challenges http test_deployment_and_rollback nginx2.wtf # Overlapping location block and server-block-level return 301 diff --git a/certbot/auth_handler.py b/certbot/auth_handler.py index efee49143..3dfaaf26f 100644 --- a/certbot/auth_handler.py +++ b/certbot/auth_handler.py @@ -31,7 +31,7 @@ class AuthHandler(object): :class:`~acme.challenges.Challenge` types :type auth: :class:`certbot.interfaces.IAuthenticator` - :ivar acme.client.BackwardsCompatibleClientV2 acme: ACME client API. + :ivar acme.client.BackwardsCompatibleClientV2 acme_client: ACME client API. :ivar account: Client's Account :type account: :class:`certbot.account.Account` @@ -40,9 +40,9 @@ class AuthHandler(object): type strings with the most preferred challenge listed first """ - def __init__(self, auth, acme, account, pref_challs): + def __init__(self, auth, acme_client, account, pref_challs): self.auth = auth - self.acme = acme + self.acme = acme_client self.account = account self.pref_challs = pref_challs @@ -85,19 +85,26 @@ class AuthHandler(object): self.verify_authzr_complete(aauthzrs) # Only return valid authorizations - retVal = [aauthzr.authzr for aauthzr in aauthzrs - if aauthzr.authzr.body.status == messages.STATUS_VALID] + ret_val = [aauthzr.authzr for aauthzr in aauthzrs + if aauthzr.authzr.body.status == messages.STATUS_VALID] - if not retVal: + if not ret_val: raise errors.AuthorizationError( "Challenges failed for all domains") - return retVal + return ret_val def _choose_challenges(self, aauthzrs): - """Retrieve necessary challenges to satisfy server.""" - logger.info("Performing the following challenges:") - for aauthzr in aauthzrs: + """ + Retrieve necessary and pending challenges to satisfy server. + NB: Necessary and already validated challenges are not retrieved, + as they can be reused for a certificate issuance. + """ + pending_authzrs = [aauthzr for aauthzr in aauthzrs + if aauthzr.authzr.body.status != messages.STATUS_VALID] + if pending_authzrs: + logger.info("Performing the following challenges:") + for aauthzr in pending_authzrs: aauthzr_challenges = aauthzr.authzr.body.challenges if self.acme.acme_version == 1: combinations = aauthzr.authzr.body.combinations @@ -125,7 +132,7 @@ class AuthHandler(object): def _solve_challenges(self, aauthzrs): """Get Responses for challenges from authenticators.""" - resp = [] # type: Collection[acme.challenges.ChallengeResponse] + resp = [] # type: Collection[challenges.ChallengeResponse] all_achalls = self._get_all_achalls(aauthzrs) try: if all_achalls: @@ -531,7 +538,7 @@ def _report_failed_challs(failed_achalls): """ problems = collections.defaultdict(list)\ - # type: DefaultDict[str, List[achallenges.KeyAuthorizationAnnotatedChallenge]] + # type: DefaultDict[str, List[achallenges.KeyAuthorizationAnnotatedChallenge]] for achall in failed_achalls: if achall.error: problems[achall.error.typ].append(achall) diff --git a/certbot/tests/auth_handler_test.py b/certbot/tests/auth_handler_test.py index e1319b614..fe0ece12e 100644 --- a/certbot/tests/auth_handler_test.py +++ b/certbot/tests/auth_handler_test.py @@ -57,7 +57,7 @@ class ChallengeFactoryTest(unittest.TestCase): errors.Error, self.handler._challenge_factory, authzr, [0]) -class HandleAuthorizationsTest(unittest.TestCase): +class HandleAuthorizationsTest(unittest.TestCase): # pylint: disable=too-many-public-methods """handle_authorizations test. This tests everything except for all functions under _poll_challenges. @@ -316,6 +316,24 @@ class HandleAuthorizationsTest(unittest.TestCase): self.assertEqual( self.mock_auth.cleanup.call_args[0][0][0].typ, "tls-sni-01") + def test_validated_challenge_not_rerun(self): + # With pending challenge, we expect the challenge to be tried, and fail. + authzr = acme_util.gen_authzr( + messages.STATUS_PENDING, "0", + [acme_util.HTTP01], + [messages.STATUS_PENDING], False) + mock_order = mock.MagicMock(authorizations=[authzr]) + self.assertRaises( + errors.AuthorizationError, self.handler.handle_authorizations, mock_order) + + # With validated challenge; we expect the challenge not be tried again, and succeed. + authzr = acme_util.gen_authzr( + messages.STATUS_VALID, "0", + [acme_util.HTTP01], + [messages.STATUS_VALID], False) + mock_order = mock.MagicMock(authorizations=[authzr]) + self.handler.handle_authorizations(mock_order) + def _validate_all(self, aauthzrs, unused_1, unused_2): for i, aauthzr in enumerate(aauthzrs): azr = aauthzr.authzr