From 9afb6415b81ef5087ed79bfa1de75be8a6da331c Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 12 Jun 2025 11:09:12 -0700 Subject: [PATCH] fix up acme & certbot standalone code (#10293) certbot's standalone code contains confusing references to things like `SSLSocket` which we were hoping to deprecate in https://github.com/certbot/certbot/issues/10284. are they relevant? they're sure not! certbot's standalone plugin only supports HTTP-01 so comments about things like `ACMETLSServer` and the completely unused `certs` variable can be deleted furthermore, the type of the different variables named things like `http_01_resources` were wrong in multiple places. as can be seen in certbot's standalone code, the type is `Set[acme_standalone.HTTP01RequestHandler.HTTP01Resource]`. this is also [the type used in acme.standalone's tests](https://github.com/certbot/certbot/blob/723fe64d4dbc778f1e8fd0b93320c1a30dc6fa95/acme/src/acme/_internal/tests/standalone_test.py#L78-L81) despite the file's type annotations saying it takes a different type. i think the incorrect type annotations were never caught because mypy can't fully make sense of our overly complex server classes here finally, `from __future__ import annotations` was added to make [forward references in type annotations](https://mypy.readthedocs.io/en/stable/cheat_sheet_py3.html#forward-references) easier --- acme/src/acme/standalone.py | 9 ++++-- .../certbot/_internal/plugins/standalone.py | 29 ++----------------- .../tests/plugins/standalone_test.py | 6 ++-- 3 files changed, 11 insertions(+), 33 deletions(-) diff --git a/acme/src/acme/standalone.py b/acme/src/acme/standalone.py index b056c7916..f1be55186 100644 --- a/acme/src/acme/standalone.py +++ b/acme/src/acme/standalone.py @@ -1,4 +1,6 @@ """Support for standalone client challenge solvers. """ +from __future__ import annotations + import collections import functools import http.client as http_client @@ -219,7 +221,8 @@ class HTTPServer(BaseHTTPServer.HTTPServer): class HTTP01Server(HTTPServer, ACMEServerMixin): """HTTP01 Server.""" - def __init__(self, server_address: Tuple[str, int], resources: Set[challenges.HTTP01], + def __init__(self, server_address: Tuple[str, int], + resources: Set[HTTP01RequestHandler.HTTP01Resource], ipv6: bool = False, timeout: int = 30) -> None: super().__init__( server_address, HTTP01RequestHandler.partial_init( @@ -314,8 +317,8 @@ class HTTP01RequestHandler(BaseHTTPServer.BaseHTTPRequestHandler): self.path) @classmethod - def partial_init(cls, simple_http_resources: Set[challenges.HTTP01], - timeout: int) -> 'functools.partial[HTTP01RequestHandler]': + def partial_init(cls, simple_http_resources: Set[HTTP01RequestHandler.HTTP01Resource], + timeout: int) -> functools.partial[HTTP01RequestHandler]: """Partially initialize this handler. This is useful because `socketserver.BaseServer` takes diff --git a/certbot/src/certbot/_internal/plugins/standalone.py b/certbot/src/certbot/_internal/plugins/standalone.py index 0f59ef3b9..5173a0cb3 100644 --- a/certbot/src/certbot/_internal/plugins/standalone.py +++ b/certbot/src/certbot/_internal/plugins/standalone.py @@ -8,11 +8,9 @@ from typing import DefaultDict from typing import Dict from typing import Iterable from typing import List -from typing import Mapping from typing import Set from typing import Tuple from typing import Type -from typing import Union from typing import TYPE_CHECKING from acme import challenges @@ -23,10 +21,6 @@ from certbot import interfaces from certbot.display import util as display_util from certbot.plugins import common -from cryptography import x509 -from cryptography.hazmat.primitives.asymmetric import types -from OpenSSL import crypto - logger = logging.getLogger(__name__) if TYPE_CHECKING: @@ -35,30 +29,14 @@ if TYPE_CHECKING: Set[achallenges.AnnotatedChallenge] ] -_KeyAndCert = Union[ - Tuple[crypto.PKey, crypto.X509], - Tuple[types.CertificateIssuerPrivateKeyTypes, x509.Certificate], -] - class ServerManager: - """Standalone servers manager. + """Manager for HTTP-01 standalone server instances.""" - Manager for `ACMEServer` and `ACMETLSServer` instances. - - `certs` and `http_01_resources` correspond to - `acme.crypto_util.SSLSocket.certs` and - `acme.crypto_util.SSLSocket.http_01_resources` respectively. All - created servers share the same certificates and resources, so if - you're running both TLS and non-TLS instances, HTTP01 handlers - will serve the same URLs! - - """ - def __init__(self, certs: Mapping[bytes, _KeyAndCert], + def __init__(self, http_01_resources: Set[acme_standalone.HTTP01RequestHandler.HTTP01Resource] ) -> None: self._instances: Dict[int, acme_standalone.HTTP01DualNetworkedServers] = {} - self.certs = certs self.http_01_resources = http_01_resources def run(self, port: int, challenge_type: Type[challenges.Challenge], @@ -144,10 +122,9 @@ running. HTTP challenge only (wildcards not supported).""" # values, main thread writes). Due to the nature of CPython's # GIL, the operations are safe, c.f. # https://docs.python.org/2/faq/library.html#what-kinds-of-global-value-mutation-are-thread-safe - self.certs: Mapping[bytes, _KeyAndCert] = {} self.http_01_resources: Set[acme_standalone.HTTP01RequestHandler.HTTP01Resource] = set() - self.servers = ServerManager(self.certs, self.http_01_resources) + self.servers = ServerManager(self.http_01_resources) @classmethod def add_parser_arguments(cls, add: Callable[..., None]) -> None: diff --git a/certbot/src/certbot/_internal/tests/plugins/standalone_test.py b/certbot/src/certbot/_internal/tests/plugins/standalone_test.py index 018249394..9e998013e 100644 --- a/certbot/src/certbot/_internal/tests/plugins/standalone_test.py +++ b/certbot/src/certbot/_internal/tests/plugins/standalone_test.py @@ -23,13 +23,11 @@ class ServerManagerTest(unittest.TestCase): """Tests for certbot._internal.plugins.standalone.ServerManager.""" def setUp(self): - from certbot._internal.plugins.standalone import ServerManager, _KeyAndCert - self.certs: Dict[bytes, _KeyAndCert] = {} + from certbot._internal.plugins.standalone import ServerManager self.http_01_resources: Set[acme_standalone.HTTP01RequestHandler.HTTP01Resource] = {} - self.mgr = ServerManager(self.certs, self.http_01_resources) + self.mgr = ServerManager(self.http_01_resources) def test_init(self): - assert self.mgr.certs is self.certs assert self.mgr.http_01_resources is self.http_01_resources def _test_run_stop(self, challenge_type):