feature: Add DNS-01 challenge support for certificate requests#429
Merged
MrMasterbay merged 1 commit intoMay 30, 2026
Merged
Conversation
Review Summary by Qodo(Agentic_describe updated until commit 1c1b2c1)Add DNS-01 ACME challenge support with RFC 2136 dynamic DNS
WalkthroughsDescription• Add DNS-01 challenge support alongside HTTP-01 for ACME certificate requests • Support RFC 2136 dynamic DNS updates for automated DNS record management • Add manual DNS-01 mode for users to create TXT records manually • Implement two-step DNS-01 flow: prepare challenge, then complete after propagation • Add comprehensive UI controls for DNS provider configuration and TSIG settings • Add dnspython dependency for RFC 2136 DNS update operations Diagramflowchart LR
User["User selects<br/>DNS-01 challenge"] -->|Choose provider| Manual["Manual TXT<br/>Record"]
User -->|Choose provider| RFC2136["RFC 2136<br/>Dynamic DNS"]
Manual -->|Prepare challenge| PrepDNS["Prepare DNS<br/>Challenge"]
RFC2136 -->|Auto-update DNS| PrepDNS
PrepDNS -->|Show TXT record| Display["Display dns_name<br/>& dns_value"]
Display -->|Admin creates record| Wait["Wait for<br/>propagation"]
Wait -->|Complete validation| Validate["Validate challenge<br/>& issue cert"]
Validate -->|Success| Cert["Certificate<br/>issued"]
File Changes1. web/src/settings_modal.js
|
Code Review by Qodo
Context used✅ Tickets:
🎫 Add DNS-01 challenge type to Letsencrypt 1. TSIG secret stored plaintext
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
339
to
376
| def prepare_dns01_challenge(domain, email, ssl_dir, staging=False, directory_url=None): | ||
| """Prepare a DNS-01 challenge and return the TXT record the admin must create.""" | ||
| try: | ||
| context = _create_order(domain, email, ssl_dir, staging=staging, directory_url=directory_url) | ||
| if not context.get('success'): | ||
| return context | ||
|
|
||
| dns_challenge = None | ||
| for ch in context['authz'].get('challenges', []): | ||
| if ch['type'] == 'dns-01': | ||
| dns_challenge = ch | ||
| break | ||
| time.sleep(2) | ||
| poll_resp = _signed_request(order_url, None, account_key, nonce, kid) | ||
| nonce = poll_resp.headers.get('Replay-Nonce', nonce) | ||
| order_data = poll_resp.json() | ||
| else: | ||
| return {'success': False, 'message': 'Timed out waiting for certificate'} | ||
|
|
||
| # Step 12: Download certificate | ||
| cert_url = order_data['certificate'] | ||
| cert_resp = _signed_request(cert_url, None, account_key, nonce, kid) | ||
| cert_pem = cert_resp.text # fullchain PEM | ||
|
|
||
| # Step 13: Save cert + key | ||
| os.makedirs(ssl_dir, exist_ok=True) | ||
| cert_path = os.path.join(ssl_dir, 'cert.pem') | ||
| key_path = os.path.join(ssl_dir, 'key.pem') | ||
|
|
||
| with open(cert_path, 'w') as f: | ||
| f.write(cert_pem) | ||
| os.chmod(cert_path, 0o644) | ||
|
|
||
| with open(key_path, 'wb') as f: | ||
| f.write(domain_key.private_bytes( | ||
| serialization.Encoding.PEM, | ||
| serialization.PrivateFormat.PKCS8, | ||
| serialization.NoEncryption() | ||
| )) | ||
| os.chmod(key_path, 0o600) | ||
|
|
||
| # Parse expiry from cert | ||
| cert_obj = x509.load_pem_x509_certificate(cert_pem.encode()) | ||
| expires = cert_obj.not_valid_after_utc.isoformat() | ||
|
|
||
| logging.info(f"[ACME] Certificate saved! Expires: {expires}") | ||
|
|
||
| if not dns_challenge: | ||
| return {'success': False, 'message': 'No DNS-01 challenge offered by ACME server'} | ||
|
|
||
| token = dns_challenge['token'] | ||
| key_authorization = f"{token}.{_jwk_thumbprint(context['account_key'])}" | ||
| state_id = secrets.token_urlsafe(24) | ||
| dns_name = f"_acme-challenge.{domain}".rstrip('.') | ||
| dns_value = _dns01_value(key_authorization) | ||
|
|
||
| _pending_dns_challenges[state_id] = { | ||
| **context, | ||
| 'domain': domain, | ||
| 'challenge_url': dns_challenge['url'], | ||
| 'expires_at': time.time() + 3600, | ||
| } | ||
|
|
||
| logging.info(f"[ACME] DNS-01 challenge prepared for {domain}") | ||
| return { | ||
| 'success': True, | ||
| 'message': f"Certificate issued for {domain}", | ||
| 'cert_path': cert_path, | ||
| 'key_path': key_path, | ||
| 'expires': expires, | ||
| 'success': False, | ||
| 'pending_dns': True, | ||
| 'message': 'Create the DNS TXT record, wait for propagation, then continue validation.', | ||
| 'challenge_id': state_id, | ||
| 'dns_name': dns_name, | ||
| 'dns_value': dns_value, | ||
| } |
There was a problem hiding this comment.
1. Dns-01 lacks provider automation 📎 Requirement gap ⚙ Maintainability
The DNS-01 flow requires an admin to manually create the TXT record and the code does not provide any pluggable DNS provider model or webhook/script hook to create/update/remove _acme-challenge records. This fails the requirement for an integration mechanism that can manage DNS records throughout the DNS-01 lifecycle.
Agent Prompt
## Issue description
DNS-01 currently requires manual DNS TXT record management and does not provide a pluggable provider model or a generic webhook/script hook to create/update/remove ACME TXT records.
## Issue Context
Compliance requires an extension/integration mechanism for DNS-01 that can reliably manage TXT records during issuance/renewal (create/update and cleanup/remove).
## Fix Focus Areas
- pegaprox/core/acme.py[339-376]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+481
to
+483
| if challenge_type == 'dns-01': | ||
| logging.info("[ACME] Skipping automatic renewal for manual DNS-01 challenge") | ||
| return False |
There was a problem hiding this comment.
2. Dns-01 renewal explicitly skipped 📎 Requirement gap ☼ Reliability
Automatic renewal is explicitly disabled when challenge_type is dns-01, so DNS-01 certificates will not renew via the existing renewal workflow. This violates the requirement that issuance and renewal workflows support DNS-01.
Agent Prompt
## Issue description
The renewal path returns early for `dns-01`, preventing DNS-01 certificates from being renewed by the renewal loop.
## Issue Context
Compliance requires DNS-01 to work for both issuance and renewal workflows.
## Fix Focus Areas
- pegaprox/core/acme.py[466-486]
- pegaprox/app.py[863-868]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+358
to
+359
| dns_name = f"_acme-challenge.{domain}".rstrip('.') | ||
| dns_value = _dns01_value(key_authorization) |
There was a problem hiding this comment.
3. Wildcard dns_name includes asterisk 📎 Requirement gap ≡ Correctness
For wildcard requests like *.example.com, prepare_dns01_challenge() builds the DNS-01 TXT record name as _acme-challenge.*.example.com, which is not a valid/correct ACME DNS-01 record name and will cause wildcard validation (and thus wildcard certificate issuance) to fail. Wildcard DNS-01 challenges must publish the TXT record under _acme-challenge.example.com (without the *.).
Agent Prompt
## Issue description
`prepare_dns01_challenge()` constructs the DNS-01 TXT record name as `_acme-challenge.{domain}` using `domain` verbatim; for wildcard identifiers (where `domain` starts with `*.`), this produces an invalid/incorrect TXT record name (`_acme-challenge.*.example.com`) and prevents wildcard certificate issuance.
## Issue Context
Wildcard certificates require DNS-01, and ACME orders may use identifiers like `*.example.com`, but the TXT record for DNS-01 must be published at `_acme-challenge.<base-domain>` (e.g., `_acme-challenge.example.com`) without the `*.` label.
## Fix Focus Areas
- pegaprox/core/acme.py[355-360]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+382
to
+399
| def complete_dns01_challenge(challenge_id, ssl_dir): | ||
| """Continue a pending DNS-01 challenge after the TXT record has propagated.""" | ||
| context = _pending_dns_challenges.get(challenge_id) | ||
| if not context: | ||
| return {'success': False, 'message': 'DNS-01 challenge was not found or has expired'} | ||
| if context.get('expires_at', 0) < time.time(): | ||
| _pending_dns_challenges.pop(challenge_id, None) | ||
| return {'success': False, 'message': 'DNS-01 challenge expired; request a new challenge'} | ||
|
|
||
| try: | ||
| result = _validate_challenge( | ||
| context['authz_url'], | ||
| context['challenge_url'], | ||
| context['account_key'], | ||
| context['nonce'], | ||
| context['kid'], | ||
| ) | ||
| if not result.get('success'): |
There was a problem hiding this comment.
4. No dns propagation wait setting 📎 Requirement gap ☼ Reliability
The DNS-01 flow provides no default propagation wait and no user-configurable override; validation is triggered immediately when the admin clicks continue. This makes DNS-01 predictably flaky on slower DNS providers and does not meet the propagation-wait requirement.
Agent Prompt
## Issue description
DNS-01 completion immediately notifies ACME without any built-in propagation wait or configurable override.
## Issue Context
Compliance requires a sensible default propagation wait and a user override, applied during both issuance and renewal.
## Fix Focus Areas
- pegaprox/core/acme.py[339-376]
- pegaprox/core/acme.py[382-399]
- pegaprox/api/helpers.py[31-35]
- pegaprox/api/settings.py[1006-1011]
- web/src/settings_modal.js[5833-5884]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+384
to
+386
| context = _pending_dns_challenges.get(challenge_id) | ||
| if not context: | ||
| return {'success': False, 'message': 'DNS-01 challenge was not found or has expired'} |
There was a problem hiding this comment.
5. Dns challenge state volatile 🐞 Bug ☼ Reliability
DNS-01 pending challenges are stored only in the process-global _pending_dns_challenges dict, so restarting the server or running multiple workers will lose the stored ACME order context and make /api/settings/acme/dns/complete fail. This makes DNS-01 completion unreliable in production deployments.
Agent Prompt
## Issue description
DNS-01 flow stores the entire pending order context in a module-level dict. That state is lost on process restart and is not shared across workers, so the follow-up completion request cannot succeed reliably.
## Issue Context
`prepare_dns01_challenge()` returns a `challenge_id` to the UI, and the UI later calls `/api/settings/acme/dns/complete` with that id. The backend currently requires the original in-memory context to exist.
## Fix Focus Areas
- pegaprox/core/acme.py[26-28]
- pegaprox/core/acme.py[339-367]
- pegaprox/core/acme.py[382-411]
- pegaprox/api/settings.py[1646-1674]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
gyptazy
force-pushed
the
feature/420-add-letsecnrypt-dns-01-support
branch
2 times, most recently
from
8ea009e to
c58528d
Compare
1 task
Contributor
Author
|
/review (still WIP) |
|
Persistent review updated to latest commit c58528d |
Comment on lines
+427
to
+438
| propagation_seconds = max(0, min(600, int(dns_config.get('propagation_seconds') or 30))) | ||
| if propagation_seconds: | ||
| logging.info(f"[ACME] Waiting {propagation_seconds}s for RFC 2136 DNS propagation") | ||
| time.sleep(propagation_seconds) | ||
|
|
||
| result = _validate_challenge( | ||
| context['authz_url'], | ||
| dns_challenge['url'], | ||
| context['account_key'], | ||
| context['nonce'], | ||
| context['kid'], | ||
| ) |
There was a problem hiding this comment.
2. Rfc2136 blocks request thread 🐞 Bug ☼ Reliability
The RFC2136 DNS-01 path sleeps for propagation and then synchronously polls ACME validation, causing /api/settings/acme/request to block for up to ~11 minutes and tie up a worker while the client/proxy may time out. Users can see a failure/timeout even if the certificate issuance eventually succeeds, and busy servers can suffer worker starvation.
Agent Prompt
### Issue description
The RFC2136 DNS-01 issuance path performs `time.sleep(propagation_seconds)` and then `_validate_challenge()` polling inside the HTTP request handler call chain. This can make the request take minutes.
### Issue Context
`request_acme_certificate()` calls `request_certificate()` inline and returns only when it finishes.
### Fix Focus Areas
- pegaprox/core/acme.py[427-438]
- pegaprox/api/settings.py[1703-1707]
### Implementation notes
Choose one:
1) **Async/background job**: enqueue RFC2136 issuance and return a job id/status endpoint.
2) **Two-step flow** (like manual): return `pending_dns` immediately after creating TXT; let a separate endpoint trigger validation/finalization.
Also consider hard upper bounds that keep request durations below typical reverse proxy timeouts.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
gyptazy
force-pushed
the
feature/420-add-letsecnrypt-dns-01-support
branch
from
c58528d to
1c1b2c1
Compare
Contributor
Author
|
/review |
|
Persistent review updated to latest commit 1c1b2c1 |
Comment on lines
+1652
to
1681
| challenge_type = str(data.get('challenge_type') or settings.get('acme_challenge_type', 'http-01')).strip() | ||
| acme_provider = str(data.get('provider') or settings.get('acme_provider', 'letsencrypt')).strip() or 'letsencrypt' | ||
| directory_url = str(data.get('directory_url') or settings.get('acme_directory_url', '')).strip() | ||
| if data.get('dns_provider') and not data.get('acme_dns_provider'): | ||
| data['acme_dns_provider'] = data.get('dns_provider') | ||
| settings = _sanitize_acme_dns_settings(settings, data) | ||
| dns_provider = settings.get('acme_dns_provider', 'manual') | ||
|
|
||
| if not domain: | ||
| return jsonify({'error': 'Domain is required'}), 400 | ||
| if acme_provider not in ('letsencrypt', 'custom'): | ||
| return jsonify({'error': 'Invalid ACME provider'}), 400 | ||
| if challenge_type not in ('http-01', 'dns-01'): | ||
| return jsonify({'error': 'Invalid ACME challenge type'}), 400 | ||
| if dns_provider not in ('manual', 'rfc2136'): | ||
| return jsonify({'error': 'Invalid DNS-01 provider'}), 400 | ||
| if challenge_type == 'dns-01' and dns_provider == 'rfc2136': | ||
| missing = [ | ||
| label for label, value in ( | ||
| ('RFC 2136 nameserver', settings.get('acme_dns_rfc2136_nameserver')), | ||
| ('RFC 2136 zone', settings.get('acme_dns_rfc2136_zone')), | ||
| ('RFC 2136 key name', settings.get('acme_dns_rfc2136_key_name')), | ||
| ('RFC 2136 secret', settings.get('acme_dns_rfc2136_secret')), | ||
| ) if not value | ||
| ] | ||
| if missing: | ||
| return jsonify({'error': ', '.join(missing) + ' required'}), 400 | ||
| if acme_provider == 'custom': | ||
| if not directory_url: | ||
| return jsonify({'error': 'Custom ACME directory URL is required'}), 400 |
There was a problem hiding this comment.
1. Global acme_challenge_type setting 📎 Requirement gap ⚙ Maintainability
The PR stores acme_challenge_type as a single server-wide setting and uses it for renewal, so the ACME challenge type cannot be selected per certificate. This violates the requirement for per-certificate challenge selection in mixed environments.
Agent Prompt
## Issue description
`acme_challenge_type` (and DNS provider config) is persisted as a single global server setting and then used by the renewal loop, so it cannot vary per certificate/domain. The compliance requirement expects per-certificate selection (HTTP-01 vs DNS-01), not a global-only toggle.
## Issue Context
- `request_acme_certificate()` persists the chosen challenge type into server settings.
- `acme_renewal_loop()` reads that single value for renewal.
- There is no per-certificate configuration object/record that would allow different domains/certs to use different challenge types.
## Fix Focus Areas
- pegaprox/api/settings.py[1649-1707]
- pegaprox/app.py[852-881]
- pegaprox/api/helpers.py[26-43]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Persistent review updated to latest commit 1c1b2c1 |
Contributor
Author
|
Switched to target branch |
gyptazy
force-pushed
the
feature/420-add-letsecnrypt-dns-01-support
branch
3 times, most recently
from
7e234d6 to
a2b004b
Compare
Fixes: PegaProx#420 Sponsored-by: credativ GmbH <https://credativ.de>
gyptazy
force-pushed
the
feature/420-add-letsecnrypt-dns-01-support
branch
from
a2b004b to
a7df7d1
Compare
Contributor
Author
|
/review |
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
Contributor
Author
|
Adjusted the plain text sig handling and the frontend part. Cheers, |
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
Contributor
|
Hey @gyptazy — reviewed the diff end-to-end. Verdict: looks clean. Highlights from the read-through:
One housekeeping bit: GH shows mergeable=DIRTY against Testing — looks like my changes from earlier this morning ( opencollective.com/pegaprox if PegaProx earns the ACME-DNS work back. — Marcus |
Contributor
Author
|
Hey @mkellermann97, thanks so far. Regarding the conflicting file: it’s luckily just affecting the index.html file and this gets recreated by running web/Dev/build.sh and is not really human friendly editable. so, force merge and recreating the file could lead into a solution or I revert the changes on that file, push again and we rebuild the html afterwards on a second branch again. If you prefer the second solution, just let me know. cheers, |
Contributor
|
Merged. Conflict resolution kept Testing's Thanks gyptazy. Testing tip is |
MrMasterbay
added a commit
that referenced
this pull request
May 30, 2026
All five surfaces aligned per the version-bumping checklist: pegaprox/constants.py PEGAPROX_VERSION = "Beta 0.9.12.0" / PEGAPROX_BUILD = "2026.05.30" web/src/constants.js PEGAPROX_VERSION = "Beta 0.9.12.0" version.json version=0.9.12.0 build=2026.05.30 release_date=2026-05-30 README.md version badge → 0.9.12.0-beta web/index.html rebuilt — PEGAPROX_VERSION="Beta 0.9.12.0" version.json changelog gets a new top entry summarising the v0.9.12.0 train (vacation R3 wrap-up): #509 davinkevin SQLCipher mlock heuristic, #510 davinkevin per-node-status info→debug, gyptazy PRs #429 (ACME DNS-01) + #508 (PVE subscription management with key-mask), #413 blackshocks SR layer 5, the CodeAnt batch (API-token role-refresh, WS cluster scope, SSH-WS SSRF, vmware path-traversal, OIDC discovery pre-validation, subscription-key mask, useEffect clusterId dep), the Aikido May-30 batch (3rd-party action SHA pinning, template-injection, Debian cloud-image SHA512 verify), plain-JSON config fallback removal, CPU dropdown sort, worldmap wheel passive-listener fix, first-run setup wizard, alert-email html-escape, audit-log CRLF strip, ACME directory SSRF guard, version.json worldmap-files manifest gap. No tag, no GitHub release cut — that decision is yours. Testing-only push so the build line on dev installs flips to 0.9.12.0 today; the mirror upload + docs.pegaprox.com pass still pending until you give the release-cut OK.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
feature: Add DNS-01 challenge support for certificate requests
Fixes: #420
Sponsored-by: credativ GmbH https://credativ.de