Skip to content

chore: add support for PKI DNS CNAME delegation#5501

Merged
carlosmonastyrski merged 4 commits intomainfrom
feat/PKI-125
Feb 17, 2026
Merged

chore: add support for PKI DNS CNAME delegation#5501
carlosmonastyrski merged 4 commits intomainfrom
feat/PKI-125

Conversation

@carlosmonastyrski
Copy link
Contributor

Context

Improve External CAs order flow to support DNS CNAME delegation, meaning a domain can have a CNAME record pointing to another domain, making all TXT writing attempts fall into that second one instead of itself.

Screenshots

Steps to verify the change

Type

  • Fix
  • Feature
  • Improvement
  • Breaking
  • Docs
  • Chore

Checklist

  • Title follows the conventional commit format: type(scope): short description (scope is optional, e.g., fix: prevent crash on sync or fix(api): handle null response).
  • Tested locally
  • Updated docs (if needed)
  • Read the contributing guide

@linear
Copy link

linear bot commented Feb 16, 2026

@maidul98
Copy link
Collaborator

maidul98 commented Feb 16, 2026

Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 16, 2026

Greptile Summary

Added DNS CNAME delegation support for ACME DNS-01 challenges, allowing _acme-challenge records to be delegated to different DNS zones via CNAME records. The implementation automatically follows CNAME chains (up to 10 levels) and includes DNS propagation verification.

  • Implemented resolveAcmeChallengeCname to follow CNAME chains for _acme-challenge records
  • Added waitForDnsPropagation to verify TXT record propagation before ACME validation (3 retries with 2s intervals)
  • Updated getAcmeChallengeRecord to resolve CNAMEs for all providers except DNS Made Easy
  • Used RE2 for regex operations to prevent ReDoS attacks (following Rule 1)
  • Added comprehensive documentation explaining CNAME delegation setup and use cases

The change follows RFC 8555 standard ACME CNAME delegation pattern used by certbot and acme.sh.

Confidence Score: 4/5

  • This PR is safe to merge with one minor logic issue that should be fixed
  • The implementation follows RFC 8555 ACME standards and adds legitimate CNAME delegation support. Uses RE2 for regex safety. One off-by-one error in CNAME depth limiting reduces effective max depth from 10 to 9, but doesn't create security issues. DNS propagation check is best-effort as explained in previous threads. Code is well-documented and follows existing patterns.
  • Fix the off-by-one error in resolveAcmeChallengeCname loop counter (backend/src/services/certificate-authority/acme/acme-certificate-authority-fns.ts:195-208)

Important Files Changed

Filename Overview
backend/src/services/certificate-authority/acme/acme-certificate-authority-fns.ts Added CNAME resolution and DNS propagation checking for ACME DNS-01 challenges with proper RE2 regex usage
docs/documentation/platform/pki/ca/acme-ca.mdx Added comprehensive documentation for DNS CNAME delegation feature with clear setup instructions

Last reviewed commit: 285ccfc

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

@carlosmonastyrski
Copy link
Contributor Author

@greptile, with the responses to your comments and my latest changes in mind, re-review my PR and update your summary when you are done.

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@carlosmonastyrski carlosmonastyrski merged commit 8a355ab into main Feb 17, 2026
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants