From 7a021d0fe4ad267233442b5e14a5d66dc98b8a7f Mon Sep 17 00:00:00 2001 From: Brian Hill Date: Mon, 12 Oct 2026 17:27:50 -0400 Subject: [PATCH] fix: remove DNS-01 challenge TXT records after enrollment ProcessAuthorizations created challenge TXT records but nothing ever called DeleteRecordAsync, leaving orphaned _acme-challenge records in the DNS zone after every enrollment. Track created records and remove them in a finally block after order finalization (best effort, failures are logged and never fail the enrollment). Also fix Cloudflare deletion, which queried by the zone-relative name instead of the FQDN and removed only the first match. AB#111453 --- AcmeCaPlugin/AcmeCaPlugin.cs | 58 +++++++++++++++++-- .../Clients/DNS/CloudflareDnsProvider.cs | 30 +++++++--- CHANGELOG.md | 4 ++ 3 files changed, 78 insertions(+), 14 deletions(-) diff --git a/AcmeCaPlugin/AcmeCaPlugin.cs b/AcmeCaPlugin/AcmeCaPlugin.cs index 231047d..5b945b2 100644 --- a/AcmeCaPlugin/AcmeCaPlugin.cs +++ b/AcmeCaPlugin/AcmeCaPlugin.cs @@ -307,11 +307,20 @@ public async Task Enroll( // Store pending order immediately var accountId = accountDetails.Kid.Split('/').Last(); - // Process challenges - await ProcessAuthorizations(acmeClient, order, config); + // Process challenges, then finalize. Challenge TXT records are removed afterwards + // whether or not the order succeeded. + var createdRecords = new List(); + try + { + await ProcessAuthorizations(acmeClient, order, config, createdRecords); - // Finalize with original CSR bytes - order = await acmeClient.FinalizeOrderAsync(order, csrBytes); + // Finalize with original CSR bytes + order = await acmeClient.FinalizeOrderAsync(order, csrBytes); + } + finally + { + await CleanupDnsRecordsAsync(config, createdRecords); + } // If order is valid immediately, download cert if (order.Payload?.Status == "valid" && !string.IsNullOrEmpty(order.Payload.Certificate)) @@ -492,7 +501,7 @@ private List ExtractDomainsFromCsr(byte[] csrBytes) /// /// Processes ACME authorizations with DNS verification before challenge submission /// - private async Task ProcessAuthorizations(AcmeClient acmeClient, OrderDetails order, AcmeClientConfig config) + private async Task ProcessAuthorizations(AcmeClient acmeClient, OrderDetails order, AcmeClientConfig config, List createdRecords) { if (order?.Payload is not Order payload || payload.Authorizations == null) { @@ -527,6 +536,7 @@ private async Task ProcessAuthorizations(AcmeClient acmeClient, OrderDetails ord // Create DNS record (will throw exception with details if it fails) var dnsProvider = DnsProviderFactory.Create(config, _logger); await dnsProvider.CreateRecordAsync(validation.DnsRecordName, validation.DnsRecordValue); + createdRecords.Add(validation.DnsRecordName); _logger.LogInformation("Created DNS record {RecordName} for domain {Domain}", validation.DnsRecordName, authz.Identifier.Value); @@ -573,6 +583,44 @@ private async Task ProcessAuthorizations(AcmeClient acmeClient, OrderDetails ord } } + /// + /// Removes DNS-01 challenge TXT records. Best effort: failures are logged and never + /// affect the enrollment result. + /// + private async Task CleanupDnsRecordsAsync(AcmeClientConfig config, IEnumerable recordNames) + { + // Wildcard and base-domain authorizations share one record name, so delete each name once. + var names = recordNames.Distinct(StringComparer.OrdinalIgnoreCase).ToList(); + if (names.Count == 0) + return; + + IDnsProvider dnsProvider; + try + { + dnsProvider = DnsProviderFactory.Create(config, _logger); + } + catch (Exception ex) + { + _logger.LogWarning(ex, "Could not create DNS provider for challenge record cleanup"); + return; + } + + foreach (var name in names) + { + try + { + if (await dnsProvider.DeleteRecordAsync(name)) + _logger.LogInformation("Removed DNS record {RecordName}", name); + else + _logger.LogWarning("DNS record {RecordName} was not removed; it may need manual cleanup", name); + } + catch (Exception ex) + { + _logger.LogWarning(ex, "Failed to remove DNS record {RecordName}; it may need manual cleanup", name); + } + } + } + /// /// Encodes binary data to PEM format with specified label /// diff --git a/AcmeCaPlugin/Clients/DNS/CloudflareDnsProvider.cs b/AcmeCaPlugin/Clients/DNS/CloudflareDnsProvider.cs index d8b6611..c3c9f4e 100644 --- a/AcmeCaPlugin/Clients/DNS/CloudflareDnsProvider.cs +++ b/AcmeCaPlugin/Clients/DNS/CloudflareDnsProvider.cs @@ -59,25 +59,37 @@ public async Task DeleteRecordAsync(string recordName) var (zoneName, zoneId) = await FindZoneForRecordAsync(recordName); if (zoneId == null || zoneName == null) return false; - var relativeName = GetRelativeRecordName(recordName, zoneName); + // Cloudflare's name filter matches the fully-qualified record name, not the zone-relative one + var fullName = Uri.EscapeDataString(recordName.TrimEnd('.')); - var recordsResp = await _httpClient.GetAsync($"zones/{zoneId}/dns_records?type=TXT&name={relativeName}"); + var recordsResp = await _httpClient.GetAsync($"zones/{zoneId}/dns_records?type=TXT&name={fullName}"); if (!recordsResp.IsSuccessStatusCode) return false; var json = await recordsResp.Content.ReadAsStringAsync(); - var doc = JsonDocument.Parse(json); + using var doc = JsonDocument.Parse(json); var resultArray = doc.RootElement.GetProperty("result"); if (resultArray.GetArrayLength() == 0) return false; - var recordId = resultArray[0].GetProperty("id").GetString(); - if (recordId == null) return false; + // A wildcard and its base domain publish two TXT values under the same name; remove all of them + var allDeleted = true; + foreach (var record in resultArray.EnumerateArray()) + { + var recordId = record.GetProperty("id").GetString(); + if (recordId == null) + { + allDeleted = false; + continue; + } + + var deleteResp = await _httpClient.DeleteAsync($"zones/{zoneId}/dns_records/{recordId}"); + var result = await deleteResp.Content.ReadAsStringAsync(); - var deleteResp = await _httpClient.DeleteAsync($"zones/{zoneId}/dns_records/{recordId}"); - var result = await deleteResp.Content.ReadAsStringAsync(); + Console.WriteLine($"Delete TXT: {deleteResp.StatusCode} - {result}"); + allDeleted &= deleteResp.IsSuccessStatusCode; + } - Console.WriteLine($"Delete TXT: {deleteResp.StatusCode} - {result}"); - return deleteResp.IsSuccessStatusCode; + return allDeleted; } private async Task GetZoneIdAsync(string zoneName) diff --git a/CHANGELOG.md b/CHANGELOG.md index 97bf92e..c0244aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +# v1.3.2 +* Fixed DNS-01 challenge TXT records not being removed after certificate issuance +* Fixed Cloudflare DNS record deletion lookup and removal of all matching TXT records + # v1.3.0 * Containerization Changes for SaaS Environment * Fixed URL CaId Length issue