From 8a15bd7927e2b8956bb1f4d062423e471e473ccf Mon Sep 17 00:00:00 2001 From: Alex Zorin Date: Thu, 21 May 2020 22:58:40 +1000 Subject: [PATCH 1/2] renewal: disregard acme-v01 in renewal configs Fixes #7979 --- certbot/_internal/constants.py | 2 ++ certbot/_internal/renewal.py | 17 +++++++++++++++-- certbot/tests/renewal_test.py | 8 ++++++++ 3 files changed, 25 insertions(+), 2 deletions(-) Index: python-certbot/certbot/constants.py =================================================================== --- python-certbot.orig/certbot/constants.py +++ python-certbot/certbot/constants.py @@ -120,6 +120,8 @@ CLI_DEFAULTS = dict( ) STAGING_URI = "https://acme-staging-v02.api.letsencrypt.org/directory" +V1_URI = "https://acme-v01.api.letsencrypt.org/directory" + # The set of reasons for revoking a certificate is defined in RFC 5280 in # section 5.3.1. The reasons that users are allowed to submit are restricted to # those accepted by the ACME server implementation. They are listed in Index: python-certbot/certbot/renewal.py =================================================================== --- python-certbot.orig/certbot/renewal.py +++ python-certbot/certbot/renewal.py @@ -17,6 +17,7 @@ import OpenSSL from acme.magic_typing import List # pylint: disable=unused-import, no-name-in-module from certbot import cli +from certbot import constants from certbot import crypto_util from certbot import errors from certbot import interfaces @@ -247,16 +248,28 @@ def _restore_int(name, value): raise errors.Error("Expected a numeric value for {0}".format(name)) -def _restore_str(unused_name, value): +def _restore_str(name, value): """Restores an string key-value pair from a renewal config file. - :param str unused_name: option name + :param str name: option name :param str value: option value :returns: converted option value to be stored in the runtime config :rtype: str or None """ + # Previous to v0.5.0, Certbot always stored the `server` URL in the renewal config, + # resulting in configs which explicitly use the deprecated ACMEv1 URL, today + # preventing an automatic transition to the default modern ACME URL. + # (https://github.com/certbot/certbot/issues/7978#issuecomment-625442870) + # As a mitigation, this function reinterprets the value of the `server` parameter if + # necessary, replacing the ACMEv1 URL with the default ACME URL. It is still possible + # to override this choice with the explicit `--server` CLI flag. + if name == "server" and value == constants.V1_URI: + logger.info("Using server %s instead of legacy %s", + constants.CLI_DEFAULTS["server"], value) + return constants.CLI_DEFAULTS["server"] + return None if value == "None" else value Index: python-certbot/certbot/tests/renewal_test.py =================================================================== --- python-certbot.orig/certbot/tests/renewal_test.py +++ python-certbot/certbot/tests/renewal_test.py @@ -31,6 +31,15 @@ class RenewalTest(test_util.ConfigTestCa renewal._restore_webroot_config(config, renewalparams) self.assertEqual(config.webroot_path, ['/var/www/']) + @mock.patch('certbot.renewal.cli.set_by_cli') + def test_ancient_server_renewal_conf(self, mock_set_by_cli): + from certbot import constants + self.config.server = None + mock_set_by_cli.return_value = False + from certbot.renewal import restore_required_config_elements + restore_required_config_elements(self.config, {'server': constants.V1_URI}) + self.assertEqual(self.config.server, constants.CLI_DEFAULTS['server']) + class RestoreRequiredConfigElementsTest(test_util.ConfigTestCase): """Tests for certbot.renewal.restore_required_config_elements."""