certbot icon indicating copy to clipboard operation
certbot copied to clipboard

Certbot-dns-cloudflare 1.30.0 doesn't work with cloudflare 2.10.1

Open Phoenix591 opened this issue 2 years ago • 9 comments

If you're having trouble using Certbot and aren't sure you've found a bug or request for a new feature, please first try asking for help at https://community.letsencrypt.org/. There is a much larger community there of people familiar with the project who will be able to more quickly answer your questions.

My operating system is (include version): Gentoo Linux

I installed Certbot with (snap, OS package manager, pip, certbot-auto, etc):

package manager

I ran this command and it produced this output:

certbot renew --dry-run -v Saving debug log to /var/log/letsencrypt/letsencrypt.log


Processing /etc/letsencrypt/renewal/phoenix591.com.conf


Certificate not due for renewal, but simulating renewal for dry run Plugins selected: Authenticator dns-cloudflare, Installer None Simulating renewal of an existing certificate for *.phoenix591.com and phoenix591.com Performing the following challenges: dns-01 challenge for phoenix591.com dns-01 challenge for phoenix591.com Cleaning up challenges Failed to renew certificate phoenix591.com with error: Unable to determine zone_id for phoenix591.com using zone names: ['phoenix591.com', 'com']. The error from Cloudflare was: 0 confused info - both key and token defined.


All simulated renewals failed. The following certificates could not be renewed: /etc/letsencrypt/live/phoenix591.com/fullchain.pem (failure)


1 renew failure(s), 0 parse failure(s) Ask for help or search for solutions at https://community.letsencrypt.org. See the logfile /var/log/letsencrypt/letsencrypt.log or re-run Certbot with -v for more details.

Downgrading cloudflare to 2.9.10 fixes the error.

Certbot's behavior differed from what I expected because: it threw out this error when the credential file is simply dns_cloudflare_api_token = (token)

Here is a Certbot log showing the issue (if available):

letsencrypt.log

Logs are stored in /var/log/letsencrypt by default. Feel free to redact domains, e-mail and IP addresses as you see fit.

Here is the relevant nginx server block or Apache virtualhost for the domain I am configuring: N/A

Phoenix591 avatar Sep 17 '22 07:09 Phoenix591

I'm sorry, but I'm unable to reproduce this error on my Gentoo system. I'm running latest Certbot from git and the Python Cloudflare library 2.10.1 and certbot-dns-cloudflare plugin 1.30.0 from my overlay.

Your error log shows "both key and token defined", but I'm unable to actually do that in the CF credentials file: Certbot complains if I use a token and a key. And with either a token or a key, it just works? So I don't know why Cloudflare would complain about your token and for me a token with the versions you're using works just fine.

osirisinferi avatar Sep 17 '22 18:09 osirisinferi

I don't have a key actually defined, my whole credential file is just dns_cloudflare_api_token = (token)

Phoenix591 avatar Sep 17 '22 19:09 Phoenix591

I found why its happening: with cloudflare 2.10.1 ONLY its failing when I also have the normal config file for cloudflare itself present, its reading the normal cloudflare config file that specifies the same token

Phoenix591 avatar Sep 17 '22 19:09 Phoenix591

when I also have the normal config file for cloudflare itself present, its reading the normal cloudflare config file that specifies the same token

You mean ~/.cloudflare/cloudflare.cfg?

osirisinferi avatar Sep 17 '22 19:09 osirisinferi

Yes

On Sat, Sep 17, 2022, 14:22 osirisinferi @.***> wrote:

when I also have the normal config file for cloudflare itself present, its reading the normal cloudflare config file that specifies the same token

You mean ~/.cloudflare/cloudflare.cfg?

— Reply to this email directly, view it on GitHub https://github.com/certbot/certbot/issues/9407#issuecomment-1250127553, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAACZXQOQRWDNJLQNNC3JOTV6YLAZANCNFSM6AAAAAAQO4KSBU . You are receiving this because you authored the thread.Message ID: @.***>

Phoenix591 avatar Sep 17 '22 19:09 Phoenix591

I can reproduce your error indeed with Cloudflare 2.10.1 (when putting a token and a key in cloudflare.cfg), but for some reason I cannot get it working when downgrading to 2.9.10. When I have a token and a key in cloudflare.cfg, I'm getting the error:

Error determining zone_id: 6003 Invalid request headers. Please confirm that you have supplied valid Cloudflare API credentials. (Did you copy your entire API token/key? To use Cloudflare tokens, you'll need the python package cloudflare>=2.3.1. This certbot is running cloudflare 2.9.10)

That said, it looks like your issue comes from your cloudflare.cfg configuration file which is, I think, outside of the scope of Certbot. I'm not sure if Certbot can interact with that part of the Cloudflare library.

I believe this is not as much a problem Certbot can fix, but perhaps Certbot can relay the error better: now it looks like the error regarding "confused info - both key and token defined" comes from the remote Cloudflare API endpoint, but it probably is just an error thrown by the Cloudflare library itself. I don't know if Certbot can detect the difference, but now it's confusing I think.

osirisinferi avatar Sep 17 '22 19:09 osirisinferi

I don't have a key defined anywhere, my cloudflare.cfg is simply

[Cloudflare]
token = (token)

Phoenix591 avatar Sep 17 '22 19:09 Phoenix591

Hm, that's indeed the issue it seems. Cloudflare 2.10.1 complains when there's a token configured in cloudflare.cfg as wel as in the Certbot credentials. Weird.

Edit: I believe this comes from the way Certbot is introducing the token to the Cloudflare library: as a positional argument which was the token earlier, but the api_key with the most recent version. The way the library handles the initiation of the Cloudflare() class has changed due to issue 114 in the following PR: https://github.com/cloudflare/python-cloudflare/commit/a28a13e8312c98c3824dc2c50c551ebacb563d25 They claim it to be backwards compatible, but it seems not to be.

I think the way certbot-dns-cloudflare initiates the Cloudflare() class needs to change to either use the email and key arguments or the token argument, all named. This would be compatible with Cloudflare 2.10.1. Unfortunately, simply passing email, key and token to the class isn't backwards compatible with the Cloudflare library before 2.10.1... Fixed by only using token as a named argument if email is None, which is backwards compatible and using the current code if email is not None. See my next post.

I strongly believe this is a bug in the Cloudflare library, them not being backwards compatible (even though they're claiming to be just that) when the cloudflare.cfg configuration file comes into play.

osirisinferi avatar Sep 17 '22 19:09 osirisinferi

https://github.com/osirisinferi/certbot/commit/bd68b4a86d24bde03d633ec56ae6092fae6de0bchttps://github.com/osirisinferi/certbot/commit/f21cb60a0d0690bfb7767913fcd01571e7456108 should fix this issue. At least, with these modifications, Certbot works with either Cloudflare-2.9.12 or Cloudflare-2.10.1.

It's up to the Certbot team to make the call if they want to fix this or not to begin with :slightly_smiling_face:

osirisinferi avatar Sep 18 '22 14:09 osirisinferi

This fix sounds good to me, with additions:

  1. Let's use the named arguments for both instances. Looking at the oldest version we support (cloudflare==1.5.1), it should work on every version.
  2. Let's rename that api_key constructor argument to api_key_or_token or something. Or better yet, add another argument so they don't use the same variable. This code is incredibly confusing right now.

alexzorin avatar Sep 26 '22 10:09 alexzorin

  • Let's use the named arguments for both instances. Looking at the oldest version we support (cloudflare==1.5.1), it should work on every version.

Unfortunately, that's not as simple to introduce compared to my attempt @f21cb60, unless we include some way to branch based on the CF version. This is due to the fact the argument where one would include the key is named token before CF v2.10. Yes, the library would have its key in the variable token. And the variable key wasn't introduced yet before 2.10.1. You can see that approach in my first commit which I didn't find as elegant as the second one.

By relying on the fact the new key argument was introduced on the position of the previously (erroneously) named token argument, we can prevent the whole checking of the library version, which for me, is more elegant than to include the version checking code, just so we can name all the arguments. 🙂

But we can discuss that further once I've opened a PR later this afternoon.

osirisinferi avatar Sep 26 '22 11:09 osirisinferi

🤦

alexzorin avatar Sep 26 '22 11:09 alexzorin