From 23f65163e8bb52c5b68f26f0710b3d71b8ad3a35 Mon Sep 17 00:00:00 2001 From: Tom Limoncelli Date: Sun, 24 Jan 2021 16:36:23 -0500 Subject: [PATCH] CLOUDFLAREAPI: Support Punycode for CF_REDIRECT/CF_TEMP_REDIRECT (with tests) (#1026) * CLOUDFLAREAPI: CF_REDIRECT should support Punycode * Add tests to CF_*REDIR * CLOUDFLARE: DS records only permitted on children --- integrationTest/integration_test.go | 95 +++++++++++++++++++++- models/domain.go | 12 +-- providers/cloudflare/cloudflareProvider.go | 25 +++++- 3 files changed, 120 insertions(+), 12 deletions(-) diff --git a/integrationTest/integration_test.go b/integrationTest/integration_test.go index 64c99aaa2..407605043 100644 --- a/integrationTest/integration_test.go +++ b/integrationTest/integration_test.go @@ -1,6 +1,7 @@ package main import ( + "encoding/json" "flag" "fmt" "os" @@ -41,7 +42,18 @@ func getProvider(t *testing.T) (providers.DNSServiceProvider, string, map[int]bo if *providerToRun != name { continue } - provider, err := providers.CreateDNSProvider(name, cfg, nil) + + var metadata json.RawMessage + // CLOUDFLAREAPI tests related to CF_REDIRECT/CF_TEMP_REDIRECT + // requires metadata to enable this feature. + // In hindsight, I have no idea why this metadata flag is required to + // use this feature. Maybe because we didn't have the capabilities + // feature at the time? + if name == "CLOUDFLAREAPI" { + metadata = []byte(`{ "manage_redirects": true }`) + } + + provider, err := providers.CreateDNSProvider(name, cfg, metadata) if err != nil { t.Fatal(err) } @@ -54,8 +66,10 @@ func getProvider(t *testing.T) (providers.DNSServiceProvider, string, map[int]bo fails[i] = true } } + return provider, cfg["domain"], fails, cfg } + t.Fatalf("Provider %s not found", *providerToRun) return nil, "", nil, nil } @@ -359,6 +373,18 @@ func azureAlias(name, aliasType, target string) *rec { return r } +func cfRedir(pattern, target string) *rec { + t := fmt.Sprintf("%s,%s", pattern, target) + r := makeRec("@", t, "CF_REDIRECT") + return r +} + +func cfRedirTemp(pattern, target string) *rec { + t := fmt.Sprintf("%s,%s", pattern, target) + r := makeRec("@", t, "CF_TEMP_REDIRECT") + return r +} + func ns(name, target string) *rec { return makeRec(name, target, "NS") } @@ -981,7 +1007,7 @@ func makeTests(t *testing.T) []*TestGroup { testgroup("DS (children only)", requires(providers.CanUseDSForChildren), - not("CLOUDNS"), + not("CLOUDNS", "CLOUDFLAREAPI"), // Use a valid digest value here, because GCLOUD (which implements this capability) verifies // the value passed in is a valid digest. RFC 4034, s5.1.4 specifies SHA1 as the only digest // algo at present, i.e. only hexadecimal values currently usable. @@ -1001,7 +1027,7 @@ func makeTests(t *testing.T) []*TestGroup { testgroup("DS (children only) CLOUDNS", requires(providers.CanUseDSForChildren), - only("CLOUDNS"), + only("CLOUDNS", "CLOUDFLAREAPI"), // Use a valid digest value here, because GCLOUD (which implements this capability) verifies // the value passed in is a valid digest. RFC 4034, s5.1.4 specifies SHA1 as the only digest // algo at present, i.e. only hexadecimal values currently usable. @@ -1146,6 +1172,69 @@ func makeTests(t *testing.T) []*TestGroup { cname("dev-system18", "ec2-54-91-33-155.compute-1.amazonaws.com."), ), ), + + testgroup("CF_REDIRECT", + only("CLOUDFLAREAPI"), + tc("redir", cfRedir("cnn.**current-domain-no-trailing**/*", "https://www.cnn.com/$1")), + tc("change", cfRedir("cnn.**current-domain-no-trailing**/*", "https://change.cnn.com/$1")), + tc("changelabel", cfRedir("cable.**current-domain-no-trailing**/*", "https://change.cnn.com/$1")), + clear(), + tc("multipleA", + cfRedir("cnn.**current-domain-no-trailing**/*", "https://www.cnn.com/$1"), + cfRedir("msnbc.**current-domain-no-trailing**/*", "https://msnbc.cnn.com/$1"), + ), + clear(), + tc("multipleB", + cfRedir("msnbc.**current-domain-no-trailing**/*", "https://msnbc.cnn.com/$1"), + cfRedir("cnn.**current-domain-no-trailing**/*", "https://www.cnn.com/$1"), + ), + tc("change1", + cfRedir("msnbc.**current-domain-no-trailing**/*", "https://msnbc.cnn.com/$1"), + cfRedir("cnn.**current-domain-no-trailing**/*", "https://change.cnn.com/$1"), + ), + tc("change1", + cfRedir("msnbc.**current-domain-no-trailing**/*", "https://msnbc.cnn.com/$1"), + cfRedir("cablenews.**current-domain-no-trailing**/*", "https://change.cnn.com/$1"), + ), + // TODO(tlim): Fix this test case: + //clear(), + //tc("multiple3", + // cfRedir("msnbc.**current-domain-no-trailing**/*", "https://msnbc.cnn.com/$1"), + // cfRedir("cnn.**current-domain-no-trailing**/*", "https://www.cnn.com/$1"), + // cfRedir("nytimes.**current-domain-no-trailing**/*", "https://www.nytimes.com/$1"), + //), + + // Repeat the above using CF_TEMP_REDIR instead + clear(), + tc("tempredir", cfRedirTemp("cnn.**current-domain-no-trailing**/*", "https://www.cnn.com/$1")), + tc("tempchange", cfRedirTemp("cnn.**current-domain-no-trailing**/*", "https://change.cnn.com/$1")), + tc("tempchangelabel", cfRedirTemp("cable.**current-domain-no-trailing**/*", "https://change.cnn.com/$1")), + clear(), + tc("tempmultipleA", + cfRedirTemp("cnn.**current-domain-no-trailing**/*", "https://www.cnn.com/$1"), + cfRedirTemp("msnbc.**current-domain-no-trailing**/*", "https://msnbc.cnn.com/$1"), + ), + clear(), + tc("tempmultipleB", + cfRedirTemp("msnbc.**current-domain-no-trailing**/*", "https://msnbc.cnn.com/$1"), + cfRedirTemp("cnn.**current-domain-no-trailing**/*", "https://www.cnn.com/$1"), + ), + tc("tempchange1", + cfRedirTemp("msnbc.**current-domain-no-trailing**/*", "https://msnbc.cnn.com/$1"), + cfRedirTemp("cnn.**current-domain-no-trailing**/*", "https://change.cnn.com/$1"), + ), + tc("tempchange1", + cfRedirTemp("msnbc.**current-domain-no-trailing**/*", "https://msnbc.cnn.com/$1"), + cfRedirTemp("cablenews.**current-domain-no-trailing**/*", "https://change.cnn.com/$1"), + ), + // TODO(tlim): Fix this test case: + //clear(), + //tc("tempmultiple3", + // cfRedirTemp("msnbc.**current-domain-no-trailing**/*", "https://msnbc.cnn.com/$1"), + // cfRedirTemp("cnn.**current-domain-no-trailing**/*", "https://www.cnn.com/$1"), + // cfRedirTemp("nytimes.**current-domain-no-trailing**/*", "https://www.nytimes.com/$1"), + //), + ), } return tests diff --git a/models/domain.go b/models/domain.go index eb547bfcb..b1a8699a1 100644 --- a/models/domain.go +++ b/models/domain.go @@ -67,26 +67,28 @@ func (dc *DomainConfig) Filter(f func(r *RecordConfig) bool) { // - Target (CNAME and MX only) func (dc *DomainConfig) Punycode() error { for _, rec := range dc.Records { + // Update the label: t, err := idna.ToASCII(rec.GetLabelFQDN()) if err != nil { return err } rec.SetLabelFromFQDN(t, dc.Name) + + // Set the target: switch rec.Type { // #rtype_variations case "ALIAS", "MX", "NS", "CNAME", "PTR", "SRV", "URL", "URL301", "FRAME", "R53_ALIAS": // These rtypes are hostnames, therefore need to be converted (unlike, for example, an AAAA record) t, err := idna.ToASCII(rec.GetTargetField()) - rec.SetTarget(t) if err != nil { return err } + rec.SetTarget(t) + case "CF_REDIRECT", "CF_TEMP_REDIRECT": + rec.SetTarget(rec.GetTargetField()) case "A", "AAAA", "CAA", "DS", "NAPTR", "SOA", "SSHFP", "TXT", "TLSA", "AZURE_ALIAS": // Nothing to do. default: - msg := fmt.Sprintf("Punycode rtype %v unimplemented", rec.Type) - panic(msg) - // We panic so that we quickly find any switch statements - // that have not been updated for a new RR type. + return fmt.Errorf("Punycode rtype %v unimplemented", rec.Type) } } return nil diff --git a/providers/cloudflare/cloudflareProvider.go b/providers/cloudflare/cloudflareProvider.go index 810f579c1..9afa0e9a8 100644 --- a/providers/cloudflare/cloudflareProvider.go +++ b/providers/cloudflare/cloudflareProvider.go @@ -44,7 +44,7 @@ var features = providers.DocumentationNotes{ providers.CanUseSRV: providers.Can(), providers.CanUseTLSA: providers.Can(), providers.CanUseSSHFP: providers.Can(), - providers.CanUseDS: providers.Can(), + providers.CanUseDSForChildren: providers.Can(), providers.CanUseTXTMulti: providers.Can(), providers.DocCreateDomains: providers.Can(), providers.DocDualHost: providers.Cannot("Cloudflare will not work well in situations where it is not the only DNS server"), @@ -177,6 +177,10 @@ func (c *cloudflareProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*m if c.manageRedirects { prs, err := c.getPageRules(id, dc.Name) + //fmt.Printf("GET PAGE RULES:\n") + //for i, p := range prs { + // fmt.Printf("%03d: %q\n", i, p.GetTargetField()) + //} if err != nil { return nil, err } @@ -218,9 +222,15 @@ func (c *cloudflareProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*m Msg: d.String(), F: func() error { return c.deletePageRule(ex.Original.(*pageRule).ID, id) }, }) - } else { - corrections = append(corrections, c.deleteRec(ex.Original.(*cfRecord), id)) + corr := c.deleteRec(ex.Original.(*cfRecord), id) + // DS records must always have a corresponding NS record. + // Therefore, we remove DS records before any NS records. + if d.Existing.Type == "DS" { + corrections = append([]*models.Correction{corr}, corrections...) + } else { + corrections = append(corrections, corr) + } } } for _, d := range create { @@ -231,7 +241,14 @@ func (c *cloudflareProvider) GetDomainCorrections(dc *models.DomainConfig) ([]*m F: func() error { return c.createPageRule(id, des.GetTargetField()) }, }) } else { - corrections = append(corrections, c.createRec(des, id)...) + corr := c.createRec(des, id) + // DS records must always have a corresponding NS record. + // Therefore, we create NS records before any DS records. + if d.Desired.Type == "NS" { + corrections = append(corr, corrections...) + } else { + corrections = append(corrections, corr...) + } } }