Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

show_account broken in combination with ACMEv1 account reuse using symbolic links #9306

Closed
osirisinferi opened this issue May 26, 2022 · 1 comment · Fixed by #9307
Closed
Labels
area: ui / ux bug has pr priority: significant Issues with higher than average priority that do not need to be in the current milestone.

Comments

@osirisinferi
Copy link
Collaborator

osirisinferi commented May 26, 2022

Reported on the Community here: https://community.letsencrypt.org/t/certbot-show-account-fails-with-acme-v01-api-letsencrypt-org-dns-lookup-failure/178367

Since #6156 ACMEv1 account reuse is done using symbolic links.

In #9127 the show_account subcommand was introduced, making use of the query_registration function of the acme library, passing the regr object from the account to that function.

However, when symbolic link account reuse is in place, the regr object contains an account uri of the ACMEv1 protocol, including the now-nonexisting acme-v01 API URL. That URI is directly used in _send_recv_regr(), triggering a DNS error.

Possible fixes:

  • Detect ACMEv1 account URI and translate it into an ACMEv2 account URI in Certbots main.show_account and keep using query_registration, or
  • Detect ACMEv1 account URI and translate it into an ACMEv2 account URI in acme's client.query_registration, or
  • Rewrite Certbots main.show_account() to not use query_registration but build a signed request for newAccount on its own.
@osirisinferi osirisinferi added bug area: ui / ux priority: unplanned Work that we believe should be done, but does not have a higher priority. labels May 26, 2022
@alexzorin
Copy link
Collaborator

Assuming that the acme module makes it possible, I prefer the newAccount option.

However, in the past I noticed that the possibility to do this is not part of the public API: see acme.ClientV2._get_v2_account. Neither new_account nor query_registration fits the bill for what we want to do.

I am wondering out loud: why we can't change the implementation of query_registration to call _get_v2_account?

@alexzorin alexzorin added priority: significant Issues with higher than average priority that do not need to be in the current milestone. and removed priority: unplanned Work that we believe should be done, but does not have a higher priority. labels May 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: ui / ux bug has pr priority: significant Issues with higher than average priority that do not need to be in the current milestone.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants