From e664daea5369c74a42794bffbabe1c4a5d6f5426 Mon Sep 17 00:00:00 2001 From: Max Chernoff Date: Fri, 3 Jan 2025 08:08:35 -0700 Subject: [PATCH] AXFRDDNS: Modernize, Update supported capabilities (#3294) --- .github/workflows/pr_integration_tests.yml | 4 + documentation/provider/axfrddns.md | 12 +-- documentation/providers.md | 2 +- integrationTest/integration_test.go | 2 - integrationTest/providers.json | 10 +- providers/axfrddns/axfrddnsProvider.go | 116 +++++++++------------ 6 files changed, 63 insertions(+), 83 deletions(-) diff --git a/.github/workflows/pr_integration_tests.yml b/.github/workflows/pr_integration_tests.yml index 8af0f7d14..36b7433fa 100644 --- a/.github/workflows/pr_integration_tests.yml +++ b/.github/workflows/pr_integration_tests.yml @@ -103,12 +103,16 @@ jobs: AXFRDDNS_MASTER: ${{ secrets.AXFRDDNS_MASTER }} AXFRDDNS_NAMESERVERS: ${{ secrets.AXFRDDNS_NAMESERVERS }} AXFRDDNS_TRANSFER_KEY: ${{ secrets.AXFRDDNS_TRANSFER_KEY }} + AXFRDDNS_TRANSFER_MODE: ${{ secrets.AXFRDDNS_TRANSFER_MODE }} AXFRDDNS_UPDATE_KEY: ${{ secrets.AXFRDDNS_UPDATE_KEY }} + AXFRDDNS_UPDATE_MODE: ${{ secrets.AXFRDDNS_UPDATE_MODE }} # AXFRDDNS_DNSSEC_MASTER: ${{ secrets.AXFRDDNS_DNSSEC_MASTER }} AXFRDDNS_DNSSEC_NAMESERVERS: ${{ secrets.AXFRDDNS_DNSSEC_NAMESERVERS }} AXFRDDNS_DNSSEC_TRANSFER_KEY: ${{ secrets.AXFRDDNS_DNSSEC_TRANSFER_KEY }} + AXFRDDNS_DNSSEC_TRANSFER_MODE: ${{ secrets.AXFRDDNS_DNSSEC_TRANSFER_MODE }} AXFRDDNS_DNSSEC_UPDATE_KEY: ${{ secrets.AXFRDDNS_DNSSEC_UPDATE_KEY }} + AXFRDDNS_DNSSEC_UPDATE_MODE: ${{ secrets.AXFRDDNS_DNSSEC_UPDATE_MODE }} # AZURE_DNS_CLIENT_ID: ${{ secrets.AZURE_DNS_CLIENT_ID }} AZURE_DNS_CLIENT_SECRET: ${{ secrets.AZURE_DNS_CLIENT_SECRET }} diff --git a/documentation/provider/axfrddns.md b/documentation/provider/axfrddns.md index ca182e276..36e785bc7 100644 --- a/documentation/provider/axfrddns.md +++ b/documentation/provider/axfrddns.md @@ -20,7 +20,7 @@ using this provider. The following two parameters in `creds.json` allow switching to TCP or TCP over TLS. -* `update-mode`: May contain `udp` (the default), `tcp`, or `tcp-tls`. +* `update-mode`: May contain `tcp` (the default), `udp`, or `tcp-tls`. * `transfer-mode`: May contain `tcp` (the default), or `tcp-tls`. ### Authentication @@ -162,16 +162,6 @@ by a separate server to DDNS requests. Use the `transfer-server` option in ``` {% endcode %} -### Buggy DNS servers regarding CNAME updates - -When modifying a CNAME record, or when replacing an A record by a -CNAME one in a single batched DDNS update, some DNS servers -(e.g. Knot) will incorrectly reject the update. For this particular -case, you might set the option `buggy-cname = "yes"` in `creds.json`. -The changes will then be split in two DDNS updates, applied -successively by the server. This will allow Knot to successfully apply -the changes, but you will loose the atomic-update property. - ### Example: local testing When testing `dnscontrol` against a local nameserver, you might use diff --git a/documentation/providers.md b/documentation/providers.md index 23d9862c1..380564f21 100644 --- a/documentation/providers.md +++ b/documentation/providers.md @@ -16,7 +16,7 @@ If a feature is definitively not supported for whatever reason, we would also li | ------------- | ---------------- | ------------ | --------- | -------------------- | ------------------------------------------------------- | --------------------------------------------------- | -------------------------------------------------------------------- | ------------------------------------------------------- | --------------------------------------------------- | ------------------------------------------------------- | --------------------------------------------------- | --------------------------------------------------- | --------------------------------------------------- | ------------------------------------------------------- | ----------------------------------------------------- | ----------------------------------------------------- | ------------------------------------------------- | ------------------------------------------------------- | ------------------------------------------------------- | --------------------------------------------------------- | --------- | -------------- | --------- | | [`AKAMAIEDGEDNS`](provider/akamaiedgedns.md) | ❌ | ✅ | ❌ | ❌ | ❌ | ✅ | ✅ | ❔ | ✅ | ✅ | ✅ | ❌ | ✅ | ✅ | ❔ | ✅ | ❌ | ❔ | ❔ | ❔ | ✅ | ✅ | ✅ | | [`AUTODNS`](provider/autodns.md) | ❌ | ✅ | ❌ | ❌ | ✅ | ✅ | ❔ | ❔ | ❔ | ❔ | ✅ | ❔ | ✅ | ❌ | ❔ | ❌ | ❌ | ❔ | ❔ | ❔ | ❌ | ❌ | ✅ | -| [`AXFRDDNS`](provider/axfrddns.md) | ❌ | ✅ | ❌ | ❌ | ❔ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ❔ | ✅ | ✅ | ✅ | ✅ | ❔ | ✅ | ❔ | ❔ | ❌ | ❌ | ❌ | +| [`AXFRDDNS`](provider/axfrddns.md) | ❌ | ✅ | ❌ | ✅ | ❌ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ❌ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ❌ | ❌ | ❌ | ❌ | | [`AZURE_DNS`](provider/azure_dns.md) | ✅ | ✅ | ❌ | ✅ | ❌ | ✅ | ❔ | ❔ | ❌ | ❌ | ✅ | ❔ | ✅ | ❌ | ❔ | ❌ | ❔ | ❔ | ❔ | ❔ | ✅ | ✅ | ✅ | | [`AZURE_PRIVATE_DNS`](provider/azure_private_dns.md) | ✅ | ✅ | ❌ | ❌ | ❌ | ❌ | ❔ | ❔ | ❌ | ❌ | ✅ | ❔ | ✅ | ❌ | ❔ | ❌ | ❔ | ❔ | ❔ | ❔ | ✅ | ✅ | ✅ | | [`BIND`](provider/bind.md) | ✅ | ✅ | ❌ | ✅ | ❔ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | diff --git a/integrationTest/integration_test.go b/integrationTest/integration_test.go index d77440829..47ea169c2 100644 --- a/integrationTest/integration_test.go +++ b/integrationTest/integration_test.go @@ -983,7 +983,6 @@ func makeTests() []*TestGroup { // CNAME testgroup("CNAME", - not("AXFRDDNS_DNSSEC"), tc("Create a CNAME", cname("testcname", "www.google.com.")), tc("Change CNAME target", cname("testcname", "www.yahoo.com.")), ), @@ -1116,7 +1115,6 @@ func makeTests() []*TestGroup { // changes properly for you. Let's verify that we got it right! testgroup("TypeChangeHard", - not("AXFRDDNS_DNSSEC"), tc("Create a CNAME", cname("foo", "google.com.")), tc("Change to A record", a("foo", "1.2.3.4")), tc("Change back to CNAME", cname("foo", "google2.com.")), diff --git a/integrationTest/providers.json b/integrationTest/providers.json index b25d2cf1e..555dff041 100644 --- a/integrationTest/providers.json +++ b/integrationTest/providers.json @@ -18,21 +18,23 @@ }, "AXFRDDNS": { "TYPE": "AXFRDDNS", - "buggy-cname": "$AXFRDDNS_BUGGY_CNAME", "domain": "$AXFRDDNS_DOMAIN", "master": "$AXFRDDNS_MASTER", "nameservers": "ns.example.com", "transfer-key": "$AXFRDDNS_TRANSFER_KEY", - "update-key": "$AXFRDDNS_UPDATE_KEY" + "transfer-mode": "AXFRDDNS_TRANSFER_MODE", + "update-key": "$AXFRDDNS_UPDATE_KEY", + "update-mode": "$AXFRDDNS_UPDATE_MODE" }, "AXFRDDNS_DNSSEC": { "TYPE": "AXFRDDNS", - "buggy-cname": "$AXFRDDNS_DNSSEC_BUGGY_CNAME", "domain": "$AXFRDDNS_DNSSEC_DOMAIN", "master": "$AXFRDDNS_DNSSEC_MASTER", "nameservers": "ns.example.com", "transfer-key": "$AXFRDDNS_DNSSEC_TRANSFER_KEY", - "update-key": "$AXFRDDNS_DNSSEC_UPDATE_KEY" + "transfer-mode": "AXFRDDNS_DNSSEC_TRANSFER_MODE", + "update-key": "$AXFRDDNS_DNSSEC_UPDATE_KEY", + "update-mode": "$AXFRDDNS_DNSSEC_UPDATE_MODE" }, "AZURE_DNS": { "ClientID": "$AZURE_DNS_CLIENT_ID", diff --git a/providers/axfrddns/axfrddnsProvider.go b/providers/axfrddns/axfrddnsProvider.go index 25ecd2e32..2183306ab 100644 --- a/providers/axfrddns/axfrddnsProvider.go +++ b/providers/axfrddns/axfrddnsProvider.go @@ -16,8 +16,6 @@ import ( "encoding/base64" "encoding/json" "fmt" - "math" - "math/rand" "net" "strings" "time" @@ -26,7 +24,6 @@ import ( "github.com/StackExchange/dnscontrol/v4/pkg/diff2" "github.com/StackExchange/dnscontrol/v4/pkg/printer" "github.com/StackExchange/dnscontrol/v4/providers" - "github.com/fatih/color" "github.com/miekg/dns" ) @@ -40,10 +37,11 @@ var features = providers.DocumentationNotes{ // The default for unlisted capabilities is 'Cannot'. // See providers/capabilities.go for the entire list of capabilities. providers.CanAutoDNSSEC: providers.Can("Just warn when DNSSEC is requested but no RRSIG is found in the AXFR or warn when DNSSEC is not requested but RRSIG are found in the AXFR."), - providers.CanGetZones: providers.Cannot(), - providers.CanConcur: providers.Cannot(), + providers.CanConcur: providers.Can(), providers.CanUseCAA: providers.Can(), providers.CanUseDHCID: providers.Can(), + providers.CanUseDNAME: providers.Can(), + providers.CanUseDS: providers.Can(), providers.CanUseHTTPS: providers.Can(), providers.CanUseLOC: providers.Can(), providers.CanUseNAPTR: providers.Can(), @@ -52,32 +50,39 @@ var features = providers.DocumentationNotes{ providers.CanUseSSHFP: providers.Can(), providers.CanUseSVCB: providers.Can(), providers.CanUseTLSA: providers.Can(), - providers.DocCreateDomains: providers.Cannot(), providers.DocDualHost: providers.Cannot(), providers.DocOfficiallySupported: providers.Cannot(), + // Possible to support via catalog zones (RFC 9432), but those are not + // directly supported by DNSControl right now (although nothing is stopping + // you from manually updating a catalog zone using DNSControl if you wish). + providers.CanGetZones: providers.Cannot(), + providers.DocCreateDomains: providers.Cannot(), + // Not a valid RR type, so impossible to encode in an RFC-compliant DNS + // packet. + providers.CanUseAlias: providers.Cannot(), + // These are both supported by RFC 2136 (DDNS), but neither work with + // DNSControl right now. + providers.CanUseSOA: providers.Cannot(), + providers.CanUseDNSKEY: providers.Cannot(), } // axfrddnsProvider stores the client info for the provider. type axfrddnsProvider struct { - rand *rand.Rand - master string - updateMode string - transferServer string - transferMode string - nameservers []*models.Nameserver - transferKey *Key - updateKey *Key - hasDnssecRecords bool - serverHasBuggyCNAME bool + master string + updateMode string + transferServer string + transferMode string + nameservers []*models.Nameserver + transferKey *Key + updateKey *Key + hasDnssecRecords bool } func initAxfrDdns(config map[string]string, providermeta json.RawMessage) (providers.DNSServiceProvider, error) { // config -- the key/values from creds.json // providermeta -- the json blob from NewReq('name', 'TYPE', providermeta) var err error - api := &axfrddnsProvider{ - rand: rand.New(rand.NewSource(int64(time.Now().Nanosecond()))), - } + api := &axfrddnsProvider{} param := &Param{} if len(providermeta) != 0 { err := json.Unmarshal(providermeta, param) @@ -107,7 +112,7 @@ func initAxfrDdns(config map[string]string, providermeta json.RawMessage) (provi printer.Printf("[Warning] AXFRDDNS: Unknown update-mode in `creds.json` (%s)\n", config["update-mode"]) } } else { - api.updateMode = "" + api.updateMode = "tcp" } if config["transfer-mode"] != "" { switch config["transfer-mode"] { @@ -148,9 +153,7 @@ func initAxfrDdns(config map[string]string, providermeta json.RawMessage) (provi } switch strings.ToLower(strings.TrimSpace(config["buggy-cname"])) { case "yes", "true": - api.serverHasBuggyCNAME = true - default: - api.serverHasBuggyCNAME = false + printer.Warnf("'buggy-cname' is deprecated as it is no longer necessary.\n") } for key := range config { switch key { @@ -209,8 +212,12 @@ func readKey(raw string, kind string) (*Key, error) { algo = dns.HmacMD5 case "hmac-sha1", "sha1": algo = dns.HmacSHA1 + case "hmac-sha224", "sha224": + algo = dns.HmacSHA224 case "hmac-sha256", "sha256": algo = dns.HmacSHA256 + case "hmac-sha384", "sha384": + algo = dns.HmacSHA384 case "hmac-sha512", "sha512": algo = dns.HmacSHA512 default: @@ -394,19 +401,6 @@ func (c *axfrddnsProvider) BuildCorrection(dc *models.DomainConfig, msgs []strin } } -// hasDeletionForName returns true if there exist a corrections for [name] which is a deletion -func hasDeletionForName(changes diff2.ChangeList, name string) bool { - for _, change := range changes { - switch change.Type { - case diff2.DELETE: - if change.Old[0].Name == name { - return true - } - } - } - return false -} - // hasNSDeletion returns true if there exist a correction that deletes or changes an NS record func hasNSDeletion(changes diff2.ChangeList) bool { for _, change := range changes { @@ -456,20 +450,14 @@ func (c *axfrddnsProvider) GetZoneRecordsCorrections(dc *models.DomainConfig, fo var msgs []string var reports []string - var msgs2 []string update := new(dns.Msg) update.SetUpdate(dc.Name + ".") - update.Id = uint16(c.rand.Intn(math.MaxUint16)) - update2 := new(dns.Msg) - update2.SetUpdate(dc.Name + ".") - update2.Id = uint16(c.rand.Intn(math.MaxUint16)) - hasTwoCorrections := false - dummyNs1, err := dns.NewRR(dc.Name + ". IN NS 255.255.255.255") + dummyNs1, err := dns.NewRR(dc.Name + ". IN NS dnscontrol.invalid.") if err != nil { return nil, 0, err } - dummyNs2, err := dns.NewRR(dc.Name + ". IN NS 255.255.255.255") + dummyNs2, err := dns.NewRR(dc.Name + ". IN NS dnscontrol.invalid.") if err != nil { return nil, 0, err } @@ -504,30 +492,31 @@ func (c *axfrddnsProvider) GetZoneRecordsCorrections(dc *models.DomainConfig, fo switch change.Type { case diff2.DELETE: msgs = append(msgs, change.Msgs[0]) - update.Remove([]dns.RR{change.Old[0].ToRR()}) + // It's semantically invalid for any RRs to exist alongside a + // CNAME RR + if change.Old[0].Type == "CNAME" { + update.RemoveName([]dns.RR{change.Old[0].ToRR()}) + } else { + update.Remove([]dns.RR{change.Old[0].ToRR()}) + } case diff2.CREATE: - if c.serverHasBuggyCNAME && - change.New[0].Type == "CNAME" && - hasDeletionForName(changes, change.New[0].Name) { - hasTwoCorrections = true - msgs2 = append(msgs2, change.Msgs[0]) - update2.Insert([]dns.RR{change.New[0].ToRR()}) - } else { - msgs = append(msgs, change.Msgs[0]) - update.Insert([]dns.RR{change.New[0].ToRR()}) + msgs = append(msgs, change.Msgs[0]) + // It's semantically invalid for any RRs to exist alongside a + // CNAME RR + if change.New[0].Type == "CNAME" { + update.RemoveName([]dns.RR{change.New[0].ToRR()}) } + update.Insert([]dns.RR{change.New[0].ToRR()}) case diff2.CHANGE: - if c.serverHasBuggyCNAME && change.New[0].Type == "CNAME" { - msgs = append(msgs, change.Msgs[0]+color.RedString(" (delete)")) - update.Remove([]dns.RR{change.Old[0].ToRR()}) - hasTwoCorrections = true - msgs2 = append(msgs2, change.Msgs[0]+color.GreenString(" (create)")) - update2.Insert([]dns.RR{change.New[0].ToRR()}) + msgs = append(msgs, change.Msgs[0]) + // It's semantically invalid for any RRs to exist alongside a + // CNAME RR + if (change.New[0].Type == "CNAME") || (change.Old[0].Type == "CNAME") { + update.RemoveName([]dns.RR{change.Old[0].ToRR()}) } else { - msgs = append(msgs, change.Msgs[0]) update.Remove([]dns.RR{change.Old[0].ToRR()}) - update.Insert([]dns.RR{change.New[0].ToRR()}) } + update.Insert([]dns.RR{change.New[0].ToRR()}) case diff2.REPORT: reports = append(reports, change.Msgs...) } @@ -542,9 +531,6 @@ func (c *axfrddnsProvider) GetZoneRecordsCorrections(dc *models.DomainConfig, fo if len(msgs) > 0 { returnValue = append(returnValue, c.BuildCorrection(dc, msgs, update)) } - if hasTwoCorrections && len(msgs2) > 0 { - returnValue = append(returnValue, c.BuildCorrection(dc, msgs2, update2)) - } if len(reports) > 0 { returnValue = append(returnValue, c.BuildCorrection(dc, reports, nil)) }