fix(EVBL-47): fail fast on terminal pending-cert failures to avoid CA rate limit

lemur/common/celery.py

@@ -360,7 +360,25 @@ def fetch_acme_cert(id, notify_reissue_cert_id=None):
             error_log["last_error"] = cert.get("last_error")
             error_log["cn"] = pending_cert.cn
 
-            if pending_cert.number_attempts > ACME_ADDITIONAL_ATTEMPTS:
+            last_error = cert.get("last_error")
+            if pending_certificate_service.is_terminal_failure(last_error):
+                # Config / DNS-delegation / credential failure — retrying can never
+                # succeed and only burns the CA's rate limit. Fail fast.
+                error_log["message"] = "Terminal failure, resolving pending certificate"
+                # Persist the terminal state before notifying so a notification
+                # delivery failure can't leave the pending cert unresolved (and
+                # retried, re-exhausting the CA rate limit).
+                pending_certificate_service.update(
+                    pending_cert.id,
+                    status=str(last_error),
+                    resolved=True,
+                )
+                send_pending_failure_notification(
+                    pending_cert, notify_owner=pending_cert.notify
+                )
+                if notify_reissue_cert_id is not None:
+                    send_reissue_failed_notification(pending_cert)
+            elif pending_cert.number_attempts >= ACME_ADDITIONAL_ATTEMPTS:
                 error_log["message"] = "Deleting pending certificate"
                 send_pending_failure_notification(
                     pending_cert, notify_owner=pending_cert.notify
@@ -369,12 +387,12 @@ def fetch_acme_cert(id, notify_reissue_cert_id=None):
                     send_reissue_failed_notification(pending_cert)
                 # Mark the pending cert as resolved
                 pending_certificate_service.update(
-                    cert.get("pending_cert").id, resolved=True
+                    pending_cert.id, resolved=True
                 )
             else:
                 pending_certificate_service.increment_attempt(pending_cert)
                 pending_certificate_service.update(
-                    cert.get("pending_cert").id, status=str(cert.get("last_error"))
+                    pending_cert.id, status=str(last_error)
                 )
                 # Add failed pending cert task back to queue
                 fetch_acme_cert.delay(id, notify_reissue_cert_id)

lemur/exceptions.py

@@ -45,7 +45,32 @@ def __str__(self):
         return repr("The field '{0}' is not sortable or filterable".format(self.field))
 
 
-class InvalidConfiguration(Exception):
+class PendingCertificateTerminalError(Exception):
+    """
+    Base class for pending-certificate failures that retrying can never resolve.
+
+    Terminal failures are configuration, DNS-delegation, or credential problems
+    (e.g. no DNS provider configured for a domain, broken ACME CNAME delegation,
+    or invalid ACME account credentials). Re-queueing these only re-runs the ACME
+    order/challenge and burns the CA's rate limit (Let's Encrypt: 5 duplicate
+    certificates / failed validations per week per domain), so they must fail fast
+    and mark the pending certificate resolved instead of retrying.
+    """
+
+
+class ACMEAuthenticationError(PendingCertificateTerminalError):
+    """The ACME CA rejected our credentials (upstream HTTP 401/403)."""
+
+
+class NoDNSProviderError(PendingCertificateTerminalError):
+    """No DNS provider is authoritative for the validation domain."""
+
+
+class DNSChallengeSetupError(PendingCertificateTerminalError):
+    """The DNS-01 challenge could not be set up (delegation/config problem)."""
+
+
+class InvalidConfiguration(PendingCertificateTerminalError):
     pass
 
 

lemur/pending_certificates/cli.py

@@ -109,17 +109,30 @@ def fetch_all_acme():
             error_log["last_error"] = cert.get("last_error")
             error_log["cn"] = pending_cert.cn
 
-            if pending_cert.number_attempts > ACME_ADDITIONAL_ATTEMPTS:
+            last_error = cert.get("last_error")
+            if pending_certificate_service.is_terminal_failure(last_error):
+                error_log["message"] = "Terminal failure, marking pending certificate as resolved"
+                # Persist the terminal state before notifying so a notification
+                # delivery failure can't leave the pending cert unresolved.
+                pending_certificate_service.update(
+                    pending_cert.id,
+                    status=str(last_error),
+                    resolved=True,
+                )
+                send_pending_failure_notification(
+                    pending_cert, notify_owner=pending_cert.notify
+                )
+            elif pending_cert.number_attempts > ACME_ADDITIONAL_ATTEMPTS:
                 error_log["message"] = "Marking pending certificate as resolved"
                 send_pending_failure_notification(
                     pending_cert, notify_owner=pending_cert.notify
                 )
                 # Mark "resolved" as True
-                pending_certificate_service.update(cert.id, resolved=True)
+                pending_certificate_service.update(pending_cert.id, resolved=True)
             else:
                 pending_certificate_service.increment_attempt(pending_cert)
                 pending_certificate_service.update(
-                    cert.get("pending_cert").id, status=str(cert.get("last_error"))
+                    pending_cert.id, status=str(last_error)
                 )
             current_app.logger.error(error_log)
     log_data["message"] = "Complete"

lemur/pending_certificates/service.py

@@ -17,6 +17,7 @@
 from lemur.common import validators
 from lemur.destinations.models import Destination
 from lemur.domains.models import Domain
+from lemur.exceptions import PendingCertificateTerminalError
 from lemur.extensions import metrics
 from lemur.notifications.models import Notification
 from lemur.pending_certificates.models import PendingCertificate
@@ -180,6 +181,26 @@ def increment_attempt(pending_certificate):
     return pending_certificate.number_attempts
 
 
+# Failures that retrying can never resolve: configuration, DNS-delegation, or
+# credential problems (e.g. no DNS provider configured for a domain, broken ACME
+# CNAME delegation, or invalid ACME account credentials). Re-queueing these only
+# re-runs the ACME order/challenge and burns the CA's rate limit (Let's Encrypt:
+# 5 duplicate certificates / failed validations per week per domain), so they must
+# fail fast and mark the pending certificate resolved instead of retrying.
+
+
+def is_terminal_failure(error):
+    """
+    Return True if a pending-certificate failure is terminal (a configuration,
+    DNS-delegation, or credential problem that retrying will never resolve).
+
+    Classification is by explicit exception type (``PendingCertificateTerminalError``
+    and subclasses) raised at the ACME/DNS boundaries, never by matching on
+    mutable exception text.
+    """
+    return isinstance(error, PendingCertificateTerminalError)
+
+
 def update(pending_cert_id, **kwargs):
     """
     Updates a pending certificate.  The allowed fields are validated by

lemur/plugins/lemur_acme/acme_handlers.py

@@ -33,7 +33,13 @@
 from lemur.common.utils import data_encrypt, data_decrypt, is_json
 from lemur.common.utils import generate_private_key, key_to_alg
 from lemur.dns_providers import service as dns_provider_service
-from lemur.exceptions import InvalidAuthority, UnknownProvider, InvalidConfiguration
+from lemur.exceptions import (
+    DNSChallengeSetupError,
+    InvalidAuthority,
+    InvalidConfiguration,
+    NoDNSProviderError,
+    UnknownProvider,
+)
 from lemur.extensions import metrics
 from lemur.plugins.lemur_acme import cloudflare, dyn, route53, ultradns, powerdns, nsone
 
@@ -425,7 +431,9 @@ def start_dns_challenge(
         if not dns_challenges:
             capture_exception()
             metrics.send("start_dns_challenge_error_no_dns_challenges", "counter", 1)
-            raise Exception("Unable to determine DNS challenges from authorizations")
+            raise DNSChallengeSetupError(
+                "Unable to determine DNS challenges from authorizations"
+            )
 
         for dns_challenge in dns_challenges:
             if not cname_delegation:
@@ -458,7 +466,7 @@ def complete_dns_challenge(self, acme_client, authz_record):
         dns_providers = self.dns_providers_for_domain.get(authz_record.target_domain)
         if not dns_providers:
             metrics.send("complete_dns_challenge_error_no_dnsproviders", "counter", 1)
-            raise Exception(
+            raise NoDNSProviderError(
                 "No DNS providers found for domain: {}".format(
                     authz_record.target_domain
                 )
@@ -535,7 +543,7 @@ def get_authorizations(self, acme_client, order, order_info):
                 metrics.send(
                     "get_authorizations_no_dns_provider_for_domain", "counter", 1
                 )
-                raise Exception(
+                raise NoDNSProviderError(
                     "No DNS providers found for domain: {}".format(target_domain)
                 )
 

lemur/plugins/lemur_acme/plugin.py

@@ -22,7 +22,11 @@
 from lemur.common.utils import check_validation, drop_last_cert_from_chain, csr_to_string
 from lemur.constants import CRLReason, EMAIL_RE
 from lemur.dns_providers import service as dns_provider_service
-from lemur.exceptions import InvalidConfiguration
+from lemur.exceptions import (
+    ACMEAuthenticationError,
+    InvalidConfiguration,
+    PendingCertificateTerminalError,
+)
 from lemur.extensions import metrics
 
 from lemur.plugins import lemur_acme as acme
@@ -31,6 +35,36 @@
 from lemur.plugins.lemur_acme.challenge_types import AcmeHttpChallenge, AcmeDnsChallenge
 
 
+def _extract_http_status(e):
+    """Best-effort extraction of an upstream HTTP status code from an exception."""
+    # requests.HTTPError
+    resp = getattr(e, "response", None)
+    if resp is not None and hasattr(resp, "status_code"):
+        return resp.status_code
+    # botocore ClientError
+    if isinstance(e, ClientError):
+        try:
+            return e.response["ResponseMetadata"]["HTTPStatusCode"]
+        except (KeyError, TypeError):
+            return None
+    # acme.messages.Error exposes a `status` attribute
+    status = getattr(e, "status", None)
+    if isinstance(status, int):
+        return status
+    return None
+
+
+def _classify_pending_error(e):
+    """Wrap a raised error in a terminal exception type when it is known to be a
+    configuration/DNS-delegation/credential failure that retrying cannot resolve.
+    Returns the original error unchanged for transient failures."""
+    if isinstance(e, PendingCertificateTerminalError):
+        return e
+    if _extract_http_status(e) in (401, 403):
+        return ACMEAuthenticationError(e)
+    return e
+
+
 class ACMEIssuerPlugin(IssuerPlugin):
     title = "Acme"
     slug = "acme-issuer"
@@ -228,12 +262,12 @@ def get_ordered_certificates(self, pending_certs):
                 current_app.logger.error(
                     f"Unable to resolve pending cert: {pending_cert}", exc_info=True
                 )
-
-                error = e
-                if globals().get("order") and order:
-                    error += f" Order uri: {order.uri}"
                 certs.append(
-                    {"cert": False, "pending_cert": pending_cert, "last_error": e}
+                    {
+                        "cert": False,
+                        "pending_cert": pending_cert,
+                        "last_error": _classify_pending_error(e),
+                    }
                 )
 
         for entry in pending:
@@ -263,7 +297,6 @@ def get_ordered_certificates(self, pending_certs):
                 capture_exception()
                 metrics.send("get_ordered_certificates_resolution_error", "counter", 1)
                 order_url = order.uri
-                error = f"{e}. Order URI: {order_url}"
                 current_app.logger.error(
                     f"Unable to resolve pending cert: {pending_cert}. "
                     f"Check out {order_url} for more information.",
@@ -273,7 +306,7 @@ def get_ordered_certificates(self, pending_certs):
                     {
                         "cert": False,
                         "pending_cert": entry["pending_cert"],
-                        "last_error": error,
+                        "last_error": _classify_pending_error(e),
                     }
                 )
                 # Ensure DNS records get deleted

lemur/plugins/lemur_acme/tests/test_acme_dns.py

@@ -415,6 +415,35 @@ def test_get_authorizations(self, mock_start_dns_challenge):
         )
         self.assertEqual(result, ["test"])
 
+    def test_get_authorizations_delegated_cname_no_provider_terminal(self):
+        """A missing provider is terminal even when delegated CNAME is enabled."""
+        from lemur.exceptions import NoDNSProviderError
+
+        current_app.config["ACME_ENABLE_DELEGATED_CNAME"] = True
+        self.acme.get_cname = Mock(return_value=False)
+        self.acme.dns_providers_for_domain = {}
+
+        mock_order_info = Mock()
+        mock_order_info.domains = ["test.fakedomain.net"]
+
+        # No provider (and no resolvable delegation) is a terminal config error —
+        # retrying would only exhaust the CA rate limit, so fail fast.
+        with self.assertRaises(NoDNSProviderError):
+            self.acme.get_authorizations("acme_client", Mock(), mock_order_info)
+
+    def test_get_authorizations_no_provider_terminal(self):
+        """Without delegated CNAME, a missing provider is a terminal NoDNSProviderError."""
+        from lemur.exceptions import NoDNSProviderError
+
+        current_app.config["ACME_ENABLE_DELEGATED_CNAME"] = False
+        self.acme.dns_providers_for_domain = {}
+
+        mock_order_info = Mock()
+        mock_order_info.domains = ["test.fakedomain.net"]
+
+        with self.assertRaises(NoDNSProviderError):
+            self.acme.get_authorizations("acme_client", Mock(), mock_order_info)
+
     @patch(
         "lemur.plugins.lemur_acme.plugin.AcmeDnsHandler.complete_dns_challenge",
         return_value="test",

lemur/tests/conftest.py

@@ -5,7 +5,7 @@
 from cryptography import x509
 from cryptography.hazmat.backends import default_backend
 from cryptography.hazmat.primitives import hashes
-from flask import current_app
+from flask import current_app, g
 from flask_principal import identity_changed, Identity
 from sqlalchemy.sql import text
 
@@ -104,6 +104,11 @@ def session(db, request):
     db.session.begin_nested()
     yield db.session
     db.session.rollback()
+    # g is app-context scoped and persists across tests. Clear request-scoped
+    # state so a stale, detached User from an earlier test isn't reused by a
+    # later test (which raises DetachedInstanceError when its attributes are
+    # expired).
+    g.pop("current_user", None)
 
 
 @pytest.fixture(scope="function")