Skip to content

Commit

Permalink
BUG: ALIAS target not properly canonicalized (#2899)
Browse files Browse the repository at this point in the history
  • Loading branch information
tlimoncelli authored Apr 1, 2024
1 parent eb19b31 commit a9a4725
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 10 deletions.
15 changes: 15 additions & 0 deletions integrationTest/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 (@)
Expand Down
11 changes: 6 additions & 5 deletions models/record.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand Down Expand Up @@ -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." {
Expand All @@ -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." {
Expand Down
2 changes: 1 addition & 1 deletion pkg/normalize/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 7 additions & 4 deletions providers/cloudflare/cloudflareProvider.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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.
Expand Down Expand Up @@ -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 + "."
}
Expand Down

0 comments on commit a9a4725

Please # to comment.