renewal: use lineage-specific server for ARI (#10307)

Previously, we were constructing an ACME client for ARI checking that
used the global value for `server`, not the one recorded in a lineage's
renewal file.

This resulted in errors in the logs and failure to observe ARI for
lineages that used a non-default `--server` (e.g. staging or non-Let's
Encrypt CAs).

---------

Co-authored-by: ohemorange <ebportnoy@gmail.com>

certbot-ci/src/certbot_integration_tests/certbot_tests/test_main.py

@@ -329,6 +329,13 @@ def test_renew_when_ari_says_its_time(context: IntegrationTestsContext) -> None:
         }
     }))
 
+    # For the renew call only, avoid passing `--server` to the `certbot` command, so
+    # we fall back on the hardcoded default of `https://acme-v02.api.letsencrypt.org`.
+    # No requests should be made to that URL because the lineage has a baked-in Pebble
+    # URL in its config from the issuance earlier in this test case. If there's a bug
+    # an ARI _is_ called against that URL it will fail because Let's Encrypt doesn't
+    # know about certificates issued by Pebble.
+    context.directory_url = None
     context.certbot(['renew', '--deploy-hook', misc.echo('deploy', context.hook_probe)],
                     force_renew=False)
 

certbot-ci/src/certbot_integration_tests/utils/certbot_call.py

@@ -6,6 +6,7 @@ import subprocess
 import sys
 from typing import Dict
 from typing import List
+from typing import Optional
 from typing import Tuple
 
 import certbot_integration_tests
@@ -13,7 +14,7 @@ import certbot_integration_tests
 from certbot_integration_tests.utils.constants import *
 
 
-def certbot_test(certbot_args: List[str], directory_url: str, http_01_port: int,
+def certbot_test(certbot_args: List[str], directory_url: Optional[str], http_01_port: int,
                  tls_alpn_01_port: int, config_dir: str, workspace: str,
                  force_renew: bool = True) -> Tuple[str, str]:
     """
@@ -81,7 +82,7 @@ def _prepare_environ(workspace: str) -> Dict[str, str]:
     return new_environ
 
 
-def _prepare_args_env(certbot_args: List[str], directory_url: str, http_01_port: int,
+def _prepare_args_env(certbot_args: List[str], directory_url: Optional[str], http_01_port: int,
                       tls_alpn_01_port: int, config_dir: str, workspace: str,
                       force_renew: bool) -> Tuple[List[str], Dict[str, str]]:
 
@@ -90,9 +91,12 @@ def _prepare_args_env(certbot_args: List[str], directory_url: str, http_01_port:
     if force_renew:
         additional_args.append('--renew-by-default')
 
+    if directory_url:
+        additional_args.extend(['--server', directory_url])
+
+
     command = [
         'certbot',
-        '--server', directory_url,
         '--no-verify-ssl',
         '--http-01-port', str(http_01_port),
         '--https-port', str(tls_alpn_01_port),

certbot/CHANGELOG.md

@@ -6,7 +6,11 @@ Certbot adheres to [Semantic Versioning](https://semver.org/).
 
 ### Added
 
-*
+* ACME Renewal Info (ARI) support. https://datatracker.ietf.org/doc/draft-ietf-acme-ari/
+  `certbot renew` will automatically check ARI when using an ACME server that supports it,
+  and may renew early based on the ARI information. For Let's Encrypt certificates this
+  will typically cause renewal at around 2/3rds of the certificate's lifetime, even if
+  the renew_before_expiry field of a lineage renewal config is set a later date.
 
 ### Changed
 

certbot/src/certbot/_internal/renewal.py

@@ -610,11 +610,15 @@ def handle_renewal_request(config: configuration.NamespaceConfig) -> Tuple[list,
             if not renewal_candidate:
                 parse_failures.append(renewal_file)
             else:
+                # We check ARI against the server stored in the lineage config even if the user
+                # specified a different `--server` on the command line. That's the server that
+                # issued the existing certificate, so it's the only server that can respond to
+                # ARI requests for it.
                 server = lineage_config.server
                 if not server:
                     raise errors.Error(f"Renewal config for {lineage_config.names} has no server.")
                 if server not in acme_clients:
-                    acme_clients[server] = client.acme_from_config_key(config)
+                    acme_clients[server] = client.acme_from_config_key(lineage_config)
 
                 renewal_candidate.ensure_deployed()
                 from certbot._internal import main

certbot/src/certbot/_internal/tests/renewal_test.py

@@ -216,10 +216,12 @@ class RenewalTest(test_util.ConfigTestCase):
         assert lineage_config.key_type == 'rsa'
 
     @test_util.patch_display_util()
+    @mock.patch.object(configuration.NamespaceConfig, 'set_by_user')
     @mock.patch('certbot._internal.client.acme_from_config_key')
     @mock.patch('certbot._internal.main.renew_cert')
     @mock.patch("certbot._internal.renewal.datetime")
-    def test_renewal_via_ari(self, mock_datetime, mock_renew_cert, mock_acme_from_config, unused_mock_display):
+    def test_renewal_via_ari(self, mock_datetime, mock_renew_cert, mock_acme_from_config, mock_set_by_user, unused_mock_display):
+        mock_set_by_user.return_value = False
         from certbot._internal import renewal
         acme_client = mock.MagicMock()
         mock_acme_from_config.return_value = acme_client
@@ -230,12 +232,17 @@ class RenewalTest(test_util.ConfigTestCase):
         acme_client.renewal_time.return_value = past, future
 
         test_util.make_lineage(self.config.config_dir, 'sample-renewal.conf', ec=False)
-        lineage_config = copy.deepcopy(self.config)
+        config = configuration.NamespaceConfig(self.config)
 
         with mock.patch('time.sleep') as sleep:
-            renewal.handle_renewal_request(lineage_config)
+            renewal.handle_renewal_request(config)
 
         mock_renew_cert.assert_called_once()
+        # This value comes from `sample-renewal.conf` and is different than
+        # the global default.
+        expected_server = "https://acme-staging-v02.api.letsencrypt.org/directory"
+        assert expected_server != config.server
+        assert mock_acme_from_config.call_args[0][0].server == expected_server
 
     def test_default_renewal_time(self):
         from certbot._internal import renewal