Make ACME directory fetches for ARI checks resilient (#10358)

Fixes https://github.com/certbot/certbot/issues/10342

When doing ARI checks in acme.renewal_time, we catch RequestException
and return a default value. That's so an unavailable ARI server doesn't
cause issues.

Before we get to acme.renewal_time, we have to create an ACME client,
and in the process fetch a directory. We should make the directory fetch
similarly resilient.

---------

Co-authored-by: Brad Warren <bmw@users.noreply.github.com>

certbot/CHANGELOG.md

@@ -10,13 +10,14 @@ Certbot adheres to [Semantic Versioning](https://semver.org/).
 
 ### Changed
 
-*
+* Catches and ignores errors during the directory fetch for ARI checking so that these
+  errors do not hinder the actual certificate issuance
 
 ### Fixed
 
-* Certbot now always uses the server value from the renewal configuration file                               
-  for ARI checks instead of the server value from the current invocation of                                                                                       
-  Certbot. This helps prevent ARI requests from going to the wrong server if        
+* Certbot now always uses the server value from the renewal configuration file
+  for ARI checks instead of the server value from the current invocation of
+  Certbot. This helps prevent ARI requests from going to the wrong server if
   the user changes CAs.
 * Previously, we claimed to set FAILED_DOMAINS and RENEWED_DOMAINS env variables for use by
   post-hooks when certificate renewals fail, but we were not actually setting them. Now, we are.

certbot/src/certbot/_internal/renewal.py

@@ -385,12 +385,17 @@ def should_autorenew(config: configuration.NamespaceConfig,
         # Creating a new ACME client makes a network request, so check if we have
         # one cached for this cert's server already
         if lineage.server not in acme_clients:
-            acme_clients[lineage.server] = \
-                client.create_acme_client(config, server_override=lineage.server)
-        acme = acme_clients[lineage.server]
+            try:
+                acme_clients[lineage.server] = \
+                    client.create_acme_client(config, server_override=lineage.server)
+            except Exception as error:  # pylint: disable=broad-except
+                logger.info("Unable to connect to %s to request ACME Renewal Information (ARI). "
+                            "Error was: %s", lineage.server, error)    
+        acme = acme_clients.get(lineage.server, None)
 
         # Attempt to get the ARI-defined renewal time
-        renewal_time, _ = acme.renewal_time(cert_pem)
+        if acme:
+            renewal_time, _ = acme.renewal_time(cert_pem)
     else:
         logger.info("Certificate has no 'server' field configured, unable to "
                     "perform ACME Renewal Information (ARI) request.")

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

@@ -420,6 +420,25 @@ class RenewalTest(test_util.ConfigTestCase):
             mock_rc.server = None
             assert renewal.should_autorenew(self.config, mock_rc, acme_clients)
 
+    @mock.patch('certbot._internal.client.create_acme_client')
+    @mock.patch('certbot._internal.storage.RenewableCert.ocsp_revoked')
+    @mock.patch('acme.client.ClientV2.renewal_time')
+    def test_resilient_ari_directory_fetches(self, mock_renewal_time, mock_ocsp, mock_create_acme):
+        from certbot._internal import renewal
+        from acme import messages
+
+        ari_server = 'http://ari'
+        acme_clients = {}
+        mock_rc = mock.MagicMock()
+        mock_rc.server = ari_server
+        mock_rc.autorenewal_is_enabled.return_value = True
+        mock_create_acme.side_effect = messages.Error()
+        mock_ocsp.return_value = True
+
+        with mock.patch('certbot._internal.renewal.open', mock.mock_open(read_data=b'')):
+            assert renewal.should_autorenew(self.config, mock_rc, acme_clients)
+            assert mock_renewal_time.call_count == 0
+
 
 class RestoreRequiredConfigElementsTest(test_util.ConfigTestCase):
     """Tests for certbot._internal.renewal.restore_required_config_elements."""