Skip to content

Commit

Permalink
Support unknown ACME challenge types (#9680)
Browse files Browse the repository at this point in the history
This is, to my knowledge, an entirely inconsequential PR to add support for entirely novel challenge types.

Presently in the [`challb_to_achall` function](https://github.com/certbot/certbot/blob/399b932a869403b142fb0271aec510614085d6ed/certbot/certbot/_internal/auth_handler.py#L367) if the challenge type is not of a type known to certbot an error is thrown. This check is mostly pointless as an authenticator would not request a challenge unknown to it. This check does however forbid any plugins from supporting entirely novel challenges not of the key authorisation form.

* support unknown ACME challenge types

* add to changelog

* update tests

---------

Co-authored-by: Brad Warren <bmw@eff.org>
  • Loading branch information
TheEnbyperor and bmw committed Apr 26, 2023
1 parent 10fba2e commit 8a0b0f6
Show file tree
Hide file tree
Showing 5 changed files with 15 additions and 7 deletions.
1 change: 1 addition & 0 deletions AUTHORS.md
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ Authors
* [Piotr Kasprzyk](https://github.com/kwadrat)
* [Prayag Verma](https://github.com/pra85)
* [Preston Locke](https://github.com/Preston12321)
* [Q Misell][https://magicalcodewit.ch]
* [Rasesh Patel](https://github.com/raspat1)
* [Reinaldo de Souza Jr](https://github.com/juniorz)
* [Remi Rampin](https://github.com/remram44)
Expand Down
2 changes: 1 addition & 1 deletion certbot/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ Certbot adheres to [Semantic Versioning](https://semver.org/).
* `--dns-google-project` optionally allows for specifying the project that the DNS zone(s) reside in,
which allows for Certbot usage in scenarios where the auth credentials reside in a different
project to the zone(s) that are being managed.
*
* There is now a new `Other` annotated challenge object to allow plugins to support entirely novel challenges.

### Changed

Expand Down
3 changes: 2 additions & 1 deletion certbot/certbot/_internal/auth_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,8 @@ def challb_to_achall(challb: messages.ChallengeBody, account_key: josepy.JWK,
challb=challb, domain=domain, account_key=account_key)
elif isinstance(chall, challenges.DNS):
return achallenges.DNS(challb=challb, domain=domain)
raise errors.Error(f"Received unsupported challenge of type: {chall.typ}")
else:
return achallenges.Other(challb=challb, domain=domain)


def gen_challenge_path(challbs: List[messages.ChallengeBody],
Expand Down
10 changes: 5 additions & 5 deletions certbot/certbot/_internal/tests/auth_handler_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,12 @@ def test_one_http(self):

def test_unrecognized(self):
authzr = acme_util.gen_authzr(
messages.STATUS_PENDING, "test",
[mock.Mock(chall="chall", typ="unrecognized")],
[messages.STATUS_PENDING])
messages.STATUS_PENDING, "test",
[mock.Mock(chall="chall", typ="unrecognized")],
[messages.STATUS_PENDING])

with pytest.raises(errors.Error):
self.handler._challenge_factory(authzr, [0])
achalls = self.handler._challenge_factory(authzr, [0])
assert type(achalls[0]) == achallenges.Other


class HandleAuthorizationsTest(unittest.TestCase):
Expand Down
6 changes: 6 additions & 0 deletions certbot/certbot/achallenges.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,3 +59,9 @@ class DNS(AnnotatedChallenge):
"""Client annotated "dns" ACME challenge."""
__slots__ = ('challb', 'domain') # pylint: disable=redefined-slots-in-subclass
acme_type = challenges.DNS


class Other(AnnotatedChallenge):
"""Client annotated ACME challenge of an unknown type."""
__slots__ = ('challb', 'domain') # pylint: disable=redefined-slots-in-subclass
acme_type = challenges.Challenge

0 comments on commit 8a0b0f6

Please sign in to comment.