From a9a4725356e5dde69e3080ab00bf6ada95834b54 Mon Sep 17 00:00:00 2001 From: Tom Limoncelli Date: Mon, 1 Apr 2024 16:47:11 -0400 Subject: [PATCH] BUG: ALIAS target not properly canonicalized (#2899) --- integrationTest/integration_test.go | 15 +++++++++++++++ models/record.go | 11 ++++++----- pkg/normalize/validate.go | 2 +- providers/cloudflare/cloudflareProvider.go | 11 +++++++---- 4 files changed, 29 insertions(+), 10 deletions(-) diff --git a/integrationTest/integration_test.go b/integrationTest/integration_test.go index b12d19c2ce..5acba293c5 100644 --- a/integrationTest/integration_test.go +++ b/integrationTest/integration_test.go @@ -873,6 +873,13 @@ func makeTests() []*TestGroup { tc("Change CNAME target", cname("testcname", "www.yahoo.com.")), ), + testgroup("CNAME-short", + tc("Create a CNAME", + a("foo", "1.2.3.4"), + cname("testcname", "foo"), + ), + ), + // MX // Narrative: MX is the first record we're going to test with @@ -1607,6 +1614,14 @@ func makeTests() []*TestGroup { tc("change it", alias("@", "foo2.com.")), ), + testgroup("ALIAS to nonfqdn", + requires(providers.CanUseAlias), + tc("ALIAS at root", + a("foo", "1.2.3.4"), + alias("@", "foo"), + ), + ), + testgroup("ALIAS on subdomain", requires(providers.CanUseAlias), not("TRANSIP"), // TransIP does support ALIAS records, but only for apex records (@) diff --git a/models/record.go b/models/record.go index 4bc008c9d5..1c91a82f5c 100644 --- a/models/record.go +++ b/models/record.go @@ -429,7 +429,8 @@ func (rc *RecordConfig) ToRR() dns.RR { func (rc *RecordConfig) GetDependencies() []string { switch rc.Type { - case "NS", "SRV", "CNAME", "MX", "ALIAS", "AZURE_ALIAS", "R53_ALIAS": + // #rtype_variations + case "NS", "SRV", "CNAME", "DNAME", "MX", "ALIAS", "AZURE_ALIAS", "R53_ALIAS": return []string{ rc.target, } @@ -536,11 +537,11 @@ func Downcase(recs []*RecordConfig) { r.Name = strings.ToLower(r.Name) r.NameFQDN = strings.ToLower(r.NameFQDN) switch r.Type { // #rtype_variations - case "AKAMAICDN", "AAAA", "ANAME", "CNAME", "DS", "MX", "NS", "NAPTR", "PTR", "SRV", "TLSA": + case "AKAMAICDN", "ALIAS", "AAAA", "ANAME", "CNAME", "DS", "MX", "NS", "NAPTR", "PTR", "SRV", "TLSA": // Target is case insensitive. Downcase it. r.target = strings.ToLower(r.target) // BUGFIX(tlim): isn't ALIAS in the wrong case statement? - case "A", "ALIAS", "CAA", "CF_REDIRECT", "CF_TEMP_REDIRECT", "CF_WORKER_ROUTE", "DHCID", "IMPORT_TRANSFORM", "LOC", "SSHFP", "TXT": + case "A", "CAA", "CF_REDIRECT", "CF_TEMP_REDIRECT", "CF_WORKER_ROUTE", "DHCID", "IMPORT_TRANSFORM", "LOC", "SSHFP", "TXT": // Do nothing. (IP address or case sensitive target) case "SOA": if r.target != "DEFAULT_NOT_SET." { @@ -561,10 +562,10 @@ func CanonicalizeTargets(recs []*RecordConfig, origin string) { for _, r := range recs { switch r.Type { // #rtype_variations - case "ANAME", "CNAME", "DS", "MX", "NS", "NAPTR", "PTR", "SRV": + case "ALIAS", "ANAME", "CNAME", "DS", "MX", "NS", "NAPTR", "PTR", "SRV": // Target is a hostname that might be a shortname. Turn it into a FQDN. r.target = dnsutil.AddOrigin(r.target, originFQDN) - case "A", "AKAMAICDN", "ALIAS", "CAA", "DHCID", "CF_REDIRECT", "CF_TEMP_REDIRECT", "CF_WORKER_ROUTE", "IMPORT_TRANSFORM", "LOC", "SSHFP", "TLSA", "TXT": + case "A", "AKAMAICDN", "CAA", "DHCID", "CF_REDIRECT", "CF_TEMP_REDIRECT", "CF_WORKER_ROUTE", "IMPORT_TRANSFORM", "LOC", "SSHFP", "TLSA", "TXT": // Do nothing. case "SOA": if r.target != "DEFAULT_NOT_SET." { diff --git a/pkg/normalize/validate.go b/pkg/normalize/validate.go index 96095ddee4..1a7f735afe 100644 --- a/pkg/normalize/validate.go +++ b/pkg/normalize/validate.go @@ -382,7 +382,7 @@ func ValidateAndNormalizeConfig(config *models.DNSConfig) (errs []error) { } // Canonicalize Targets. - if rec.Type == "CNAME" || rec.Type == "MX" || rec.Type == "NS" || rec.Type == "SRV" { + if rec.Type == "ALIAS" || rec.Type == "CNAME" || rec.Type == "MX" || rec.Type == "NS" || rec.Type == "SRV" { // #rtype_variations // These record types have a target that is a hostname. // We normalize them to a FQDN so there is less variation to handle. If a diff --git a/providers/cloudflare/cloudflareProvider.go b/providers/cloudflare/cloudflareProvider.go index 13cc08a057..53134000b3 100644 --- a/providers/cloudflare/cloudflareProvider.go +++ b/providers/cloudflare/cloudflareProvider.go @@ -197,6 +197,12 @@ func (c *cloudflareProvider) getDomainID(name string) (string, error) { // GetZoneRecordsCorrections returns a list of corrections that will turn existing records into dc.Records. func (c *cloudflareProvider) GetZoneRecordsCorrections(dc *models.DomainConfig, records models.Records) ([]*models.Correction, error) { + for _, rec := range dc.Records { + if rec.Type == "ALIAS" { + rec.Type = "CNAME" + } + } + if err := c.preprocessConfig(dc); err != nil { return nil, err } @@ -209,9 +215,6 @@ func (c *cloudflareProvider) GetZoneRecordsCorrections(dc *models.DomainConfig, } for _, rec := range dc.Records { - if rec.Type == "ALIAS" { - rec.Type = "CNAME" - } // As per CF-API documentation proxied records are always forced to have a TTL of 1. // When not forcing this property change here, dnscontrol tries each time to update // the TTL of a record which simply cannot be changed anyway. @@ -735,7 +738,7 @@ func stringDefault(value interface{}, def string) string { func (c *cloudflareProvider) nativeToRecord(domain string, cr cloudflare.DNSRecord) (*models.RecordConfig, error) { // normalize cname,mx,ns records with dots to be consistent with our config format. - if cr.Type == "CNAME" || cr.Type == "MX" || cr.Type == "NS" || cr.Type == "PTR" { + if cr.Type == "ALIAS" || cr.Type == "CNAME" || cr.Type == "MX" || cr.Type == "NS" || cr.Type == "PTR" { if cr.Content != "." { cr.Content = cr.Content + "." }