search

obynio / certbot-plugin-gandi

Certbot plugin for authentication using Gandi LiveDNS
https://pypi.org/project/certbot-plugin-gandi/
MIT License
194 stars 27 forks source link

Add RRSet support for wildcards #5

Closed alexzorin closed 5 years ago

alexzorin commented 5 years ago

Previously, the plugin would blow away existing TXT records for the same DNS name when adding a TXT record. This means that issuing a certificate for both a base domain and its wildcard could not succeed.

I noticed that your README mentions wildcard support already, but as far as I can tell, it doesn't seem to actually work. For example, in a typical wildcard scenario:

  1. _acme-challenge.example.org 360 IN TXT "xxxx" is added via _add_txt_record (which calls _del_txt_record, wiping out all previous values).
  2. _acme-challenge.example.org 360 IN TXT "yyyy" is added via _add_txt_record (which calls _del_txt_record, wiping out all previous values).
  3. Certbot submits the challenges to the CA - the CA can only see yyyy because xxxx was deleted in (2). Challenge fails.

After this PR, existing TXT values are not removed in _add_txt_record.

Reported at https://community.letsencrypt.org/t/wildcard-certificate-not-renewed/87698

obynio commented 5 years ago

Thanks for the PR ! I will look at it asap 👍

rtwld commented 5 years ago

I originally reported the issue. Diff 313f948 does fix the main problem I had (the certificate is successfully renewed), but it still throws an error:

Unable to find or delete the DNS TXT record: Can't find the DNS record _acme-challenge/TXT in the zone

It seems that the plugin tries to delete the same record twice and it obviously fails the second time:

Cleaning up challenges Starting new HTTPS connection (1): dns.api.gandi.net:443 https://dns.api.gandi.net:443 "GET /api/v5/domains/forcing-project.com HTTP/1.1" 200 547 Starting new HTTPS connection (1): dns.api.gandi.net:443 https://dns.api.gandi.net:443 "DELETE /api/v5/zones/ede74274-195c-11e8-b081-00163e6dc886/records/_acme-challenge/TXT HTTP/1.1" 204 0 Starting new HTTPS connection (1): dns.api.gandi.net:443 https://dns.api.gandi.net:443 "GET /api/v5/domains/forcing-project.com HTTP/1.1" 200 547 Starting new HTTPS connection (1): dns.api.gandi.net:443 https://dns.api.gandi.net:443 "DELETE /api/v5/zones/ede74274-195c-11e8-b081-00163e6dc886/records/_acme-challenge/TXT HTTP/1.1" 404 131

Here's the full log.

alexzorin commented 5 years ago

The best fix would be for _cleanup to update the RRSet to remove the specific TXT value that Certbot is requesting, rather than deleting the entire RRSet.

I didn't want to change the behavior of the plugin too much for the sake of this fix.

alexzorin commented 5 years ago

On reflection that was pretty lazy from me - I've fixed it up from the cleanup side as well now.

obynio commented 5 years ago

Wow, you did a great job! Thank you for catching that :+1: Everything seems fine, let's merge it and update the package :slightly_smiling_face:

obynio commented 5 years ago

Your changes were added in the 1.1.0 release, thank you :+1: