starred/certbot-certbot
1
0
Fork 0
mirror of https://github.com/certbot/certbot.git synced 2026-07-27 16:30:31 +02:00
Code Issues Packages Projects Releases Wiki Activity

Add identifier field to AnnotatedChallenge subclasses (#10491)

Browse Source 
This field is optional to maintain backwards compatibility. Note that
`AnnotatedChallenge` inherits from `jose.ImmutableMap`, which has a
[check in
__init__](https://github.com/certbot/josepy/blob/4b747476703fe4fff1aaccd76ebe570698bbf4f0/src/josepy/util.py#L125-L131)
that all slots are provided. That check would not allow us to do a
backwards-compatible addition, so I implemented an `__init__` for each
of these subclasses that fills the fields without calling the parent
`__init__`, and so doesn't hit an error when `identifier` is absent.

I chose to use `acme.messages.Identifier` rather than
`certbot._internal.san.SAN` here because these are wrapped ACME types,
so they should use the ACME representation. Also, `AnnotatedChallenge`
is passed to plugins, so we need to pass a type that the plugins can
understand.

Additionally, `domain` is marked as deprecated.

Part of #10346

/cc @bmw, who noticed the issue with `AnnotatedChallenge`
[here](https://github.com/certbot/certbot/pull/10468#issuecomment-3403294394)
and provided additional feedback
[here](https://github.com/jsha/certbot/pull/2#issuecomment-3534895793).
Note that there's still some work to do to finish excising `domain`
assumptions from this portion of the code.

---------

Co-authored-by: ohemorange <ebportnoy@gmail.com>
This commit is contained in:
Jacob Hoffman-Andrews
2025-12-05 13:44:04 -08:00
committed by GitHub
co-authored by ohemorange
parent 9e7a98f4cd
commit b1cf53ff6b
25 changed files with 218 additions and 110 deletions
Download Patch File Download Diff File Expand all files Collapse all files
certbot-apache/src/certbot_apache/_internal/http_01.py
+5 -1
View File
@@ -4,6 +4,7 @@ import logging
from typing import TYPE_CHECKING
from acme.challenges import KeyAuthorizationChallengeResponse
from acme import messages
from certbot import errors
from certbot.achallenges import KeyAuthorizationAnnotatedChallenge
from certbot.compat import filesystem
@@ -52,6 +53,9 @@ class ApacheHttp01(common.ChallengePerformer):
"""Perform all HTTP-01 challenges."""
if not self.achalls:
return []
if any(achall.identifier.typ == messages.IDENTIFIER_IP for achall in self.achalls):
raise errors.ConfigurationError(
"Apache authenticator not supported for IP address certificates")
# Save any changes to the configuration as a precaution
# About to make temporary changes to the config
self.configurator.save("Changes before challenge setup", True)
@@ -82,7 +86,7 @@ class ApacheHttp01(common.ChallengePerformer):
# Search for VirtualHosts matching by name
for chall in self.achalls:
selected_vhosts += self._matching_vhosts(chall.domain)
selected_vhosts += self._matching_vhosts(chall.identifier.value)
# Ensure that we have one or more VirtualHosts that we can continue
# with. (one that listens to port configured with --http-01-port)
certbot-apache/src/certbot_apache/_internal/tests/configurator_test.py
+7 -4
View File
@@ -9,7 +9,7 @@ from unittest import mock
import pytest
from acme import challenges
from acme import challenges, messages
from certbot import achallenges
from certbot import crypto_util
from certbot import errors
@@ -1193,17 +1193,20 @@ class MultipleVhostsTest(util.ApacheTest):
challenges.HTTP01(
token=b"jIq_Xy1mXGN37tb4L6Xj_es58fW571ZNyXekdZzhh7Q"),
"pending"),
domain="encryption-example.demo", account_key=account_key)
identifier=messages.Identifier(typ=messages.IDENTIFIER_FQDN, value="encryption-example.demo"),
account_key=account_key)
achall2 = achallenges.KeyAuthorizationAnnotatedChallenge(
challb=acme_util.chall_to_challb(
challenges.HTTP01(
token=b"uqnaPzxtrndteOqtrXb0Asl5gOJfWAnnx6QJyvcmlDU"),
"pending"),
domain="certbot.demo", account_key=account_key)
identifier=messages.Identifier(typ=messages.IDENTIFIER_FQDN, value="certbot.demo"),
account_key=account_key)
achall3 = achallenges.KeyAuthorizationAnnotatedChallenge(
challb=acme_util.chall_to_challb(
challenges.HTTP01(token=(b'x' * 16)), "pending"),
domain="example.org", account_key=account_key)
identifier=messages.Identifier(typ=messages.IDENTIFIER_FQDN, value="example.org"),
account_key=account_key)
return account_key, (achall1, achall2, achall3)
certbot-apache/src/certbot_apache/_internal/tests/http_01_test.py
+25 -8
View File
@@ -5,7 +5,7 @@ from unittest import mock
import pytest
from acme import challenges
from acme import challenges, messages
from certbot import achallenges
from certbot import errors
from certbot.compat import filesystem
@@ -37,7 +37,8 @@ class ApacheHttp01Test(util.ApacheTest):
challb=acme_util.chall_to_challb(
challenges.HTTP01(token=((chr(ord('a') + i).encode() * 16))),
"pending"),
domain=self.vhosts[i].name, account_key=self.account_key))
identifier=messages.Identifier(typ=messages.IDENTIFIER_FQDN, value=self.vhosts[i].name),
account_key=self.account_key))
modules = ["ssl", "rewrite", "authz_core", "authz_host"]
for mod in modules:
@@ -50,6 +51,16 @@ class ApacheHttp01Test(util.ApacheTest):
def test_empty_perform(self):
assert len(self.http.perform()) == 0
def test_ip_address_perform(self):
self.http.achalls = [achallenges.KeyAuthorizationAnnotatedChallenge(
challb=acme_util.chall_to_challb(
challenges.HTTP01(token=((b'a' * 16))),
"pending"),
identifier=messages.Identifier(typ=messages.IDENTIFIER_IP, value="127.0.0.1"),
account_key=self.account_key)]
with pytest.raises(errors.ConfigurationError):
self.http.perform()
@mock.patch("certbot_apache._internal.configurator.ApacheConfigurator.enable_mod")
def test_enable_modules_apache_2_4(self, mock_enmod):
del self.config.parser.modules["authz_core_module"]
@@ -83,12 +94,14 @@ class ApacheHttp01Test(util.ApacheTest):
challb=acme_util.chall_to_challb(
challenges.HTTP01(token=((b'a' * 16))),
"pending"),
domain=vhost.name, account_key=self.account_key),
identifier=messages.Identifier(typ=messages.IDENTIFIER_FQDN, value=vhost.name),
account_key=self.account_key),
achallenges.KeyAuthorizationAnnotatedChallenge(
challb=acme_util.chall_to_challb(
challenges.HTTP01(token=((b'b' * 16))),
"pending"),
domain=next(iter(vhost.aliases)), account_key=self.account_key)
identifier=messages.Identifier(typ=messages.IDENTIFIER_FQDN, value=next(iter(vhost.aliases))),
account_key=self.account_key)
]
self.common_perform_test(achalls, [vhost])
@@ -99,7 +112,8 @@ class ApacheHttp01Test(util.ApacheTest):
challb=acme_util.chall_to_challb(
challenges.HTTP01(token=((b'a' * 16))),
"pending"),
domain="something.nonexistent", account_key=self.account_key)]
identifier=messages.Identifier(typ=messages.IDENTIFIER_FQDN, value="something.nonexistent"),
account_key=self.account_key)]
self.common_perform_test(achalls, vhosts)
def test_configure_multiple_vhosts(self):
@@ -110,7 +124,8 @@ class ApacheHttp01Test(util.ApacheTest):
challb=acme_util.chall_to_challb(
challenges.HTTP01(token=((b'a' * 16))),
"pending"),
domain="duplicate.example.com", account_key=self.account_key)]
identifier=messages.Identifier(typ=messages.IDENTIFIER_FQDN, value="duplicate.example.com"),
account_key=self.account_key)]
self.common_perform_test(achalls, vhosts)
def test_configure_name_and_blank(self):
@@ -121,7 +136,8 @@ class ApacheHttp01Test(util.ApacheTest):
challb=acme_util.chall_to_challb(
challenges.HTTP01(token=((b'a' * 16))),
"pending"),
domain=domain, account_key=self.account_key),
identifier=messages.Identifier(typ=messages.IDENTIFIER_FQDN, value=domain),
account_key=self.account_key),
]
self.common_perform_test(achalls, vhosts)
@@ -148,7 +164,8 @@ class ApacheHttp01Test(util.ApacheTest):
challb=acme_util.chall_to_challb(
challenges.HTTP01(token=((b'a' * 16))),
"pending"),
domain="certbot.demo", account_key=self.account_key)]
identifier=messages.Identifier(typ=messages.IDENTIFIER_FQDN, value="certbot.demo"),
account_key=self.account_key)]
vhosts[0].enabled = False
self.common_perform_test(achalls, vhosts)
matches = self.config.parser.find_dir(