diff --git a/plugin/auto/walk_test.go b/plugin/auto/walk_test.go index 6d67e6aee..caf4af5a3 100644 --- a/plugin/auto/walk_test.go +++ b/plugin/auto/walk_test.go @@ -49,6 +49,36 @@ func TestWalk(t *testing.T) { } } +func TestWalkSOAOrigin(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "db.example.org") + invalid := strings.Replace(zoneContent, "@", "example.net.", 1) + if err := os.WriteFile(path, []byte(invalid), 0644); err != nil { + t.Fatal(err) + } + a := Auto{ + loader: loader{directory: dir, re: regexp.MustCompile(`^db\.(.*)$`), template: `${1}`}, + Zones: &Zones{}, + } + if err := a.Walk(); err != nil { + t.Fatal(err) + } + if len(a.Names()) != 0 { + t.Fatal("loaded a zone with a mismatched SOA owner") + } + + if err := os.WriteFile(path, []byte(zoneContent), 0644); err != nil { + t.Fatal(err) + } + if err := a.Walk(); err != nil { + t.Fatal(err) + } + z := a.Zones.Zones("example.org.") + if z == nil || z.SOA == nil || z.SOA.Hdr.Name != "example.org." { + t.Fatal("did not load the corrected zone") + } +} + func TestWalkSymlinkedDirectory(t *testing.T) { t.Parallel() tempdir, err := createFiles(t) diff --git a/plugin/file/README.md b/plugin/file/README.md index ff26c8c7a..6ebfdf1f0 100644 --- a/plugin/file/README.md +++ b/plugin/file/README.md @@ -22,6 +22,12 @@ file DBFILE [ZONES...] * **ZONES** zones it should be authoritative for. If empty, the zones from the configuration block are used. +The SOA record's owner name must match the zone being loaded. Names without a final dot are +relative to the current origin; for example, `test` in a zone loaded as `test.` becomes +`test.test.`, not `test.`. Use `@` (when the current origin matches the zone) or the zone's +absolute name for the SOA owner. A mismatched SOA causes loading to fail; an invalid reload +leaves the last successfully loaded zone in service. + If you want to round-robin A and AAAA responses look at the *loadbalance* plugin. ~~~ @@ -72,17 +78,28 @@ www IN A 127.0.0.1 ~~~ -Or use a single zone file for multiple zones: +Or use a single zone file for multiple zones, with relative owner names and no fixed `$ORIGIN`: ~~~ corefile . { - file example.org.signed example.org example.net + file db.shared example.org example.net transfer example.org example.net { to * 10.240.1.1 } } ~~~ +For example, `db.shared` can contain: + +~~~ +@ 3600 IN SOA sns.dns.icann.org. noc.dns.icann.org. 2017042745 7200 3600 1209600 3600 +@ 3600 IN NS a.iana-servers.net. +www 3600 IN A 127.0.0.1 +~~~ + +Each configured zone is used as the initial origin when parsing this file, so `@` is its apex. +Signed zones require signatures for the actual owner names and must be signed separately. + Note that if you have a configuration like the following you may run into a problem of the origin not being correctly recognized: @@ -93,9 +110,9 @@ not being correctly recognized: ~~~ We omit the origin for the file `db.example.org`, so this references the zone in the server block, -which, in this case, is the root zone. Any contents of `db.example.org` will then read with that -origin set; this may or may not do what you want. -It's better to be explicit here and specify the correct origin. This can be done in two ways: +which, in this case, is the root zone. A file with an SOA for `example.org.` will be rejected +because it does not match the configured root zone, even if the file sets `$ORIGIN example.org.`. +Specify the correct zone in one of two ways: ~~~ corefile . { diff --git a/plugin/file/dname_test.go b/plugin/file/dname_test.go index cc70bb5ac..734f4e894 100644 --- a/plugin/file/dname_test.go +++ b/plugin/file/dname_test.go @@ -142,7 +142,7 @@ var dnameDnssecTestCases = []test.Case{ } func TestLookupDNAMEDNSSEC(t *testing.T) { - zone, err := Parse(strings.NewReader(dbExampleDNAMESigned), testzone, "stdin", 0) + zone, err := Parse(strings.NewReader(dbExampleDNAMESigned), "example.org.", "stdin", 0) if err != nil { t.Fatalf("Expect no error when reading zone, got %q", err) } diff --git a/plugin/file/file.go b/plugin/file/file.go index aa3827968..38d7461f0 100644 --- a/plugin/file/file.go +++ b/plugin/file/file.go @@ -192,8 +192,11 @@ func Parse(f io.Reader, origin, fileName string, serial int64) (*Zone, error) { seenSOA := false for rr, ok := zp.Next(); ok; rr, ok = zp.Next() { - if !seenSOA { - if s, ok := rr.(*dns.SOA); ok { + if s, ok := rr.(*dns.SOA); ok { + if dns.CanonicalName(canonicalEscape(s.Hdr.Name)) != dns.CanonicalName(canonicalEscape(z.origin)) { + return nil, fmt.Errorf("file %q has SOA owner %s that does not match origin %s", fileName, s.Hdr.Name, z.origin) + } + if !seenSOA { seenSOA = true // -1 is valid serial is we failed to load the file on startup. diff --git a/plugin/file/file_test.go b/plugin/file/file_test.go index dd3be334a..c919b0344 100644 --- a/plugin/file/file_test.go +++ b/plugin/file/file_test.go @@ -1,6 +1,7 @@ package file import ( + "fmt" "strings" "testing" ) @@ -21,6 +22,74 @@ func TestParseNoSOA(t *testing.T) { } } +func TestParseSOAOrigin(t *testing.T) { + tests := []struct { + name, origin, prefix, owner string + wantErr bool + }{ + {name: "at", origin: "example.org.", owner: "@"}, + {name: "absolute", origin: "example.org.", owner: "example.org."}, + {name: "mixed case", origin: "example.org.", owner: "ExAmPlE.OrG."}, + {name: "mixed case origin", origin: "ExAmPlE.OrG.", owner: "example.org."}, + {name: "origin without final dot", origin: "example.org", owner: "@"}, + {name: "relative to parent", origin: "example.org.", prefix: "$ORIGIN org.\n", owner: "example"}, + {name: "decimal escape", origin: "example.org.", owner: `\101xample.org.`}, + {name: "escaped dot", origin: `has\.dot.example.`, owner: `has\046dot.example.`}, + {name: "root", origin: ".", owner: "@"}, + {name: "relative owner", origin: "test", owner: "test", wantErr: true}, + {name: "child", origin: "example.org.", owner: "child.example.org.", wantErr: true}, + {name: "parent", origin: "example.org.", owner: "org.", wantErr: true}, + {name: "unrelated", origin: "example.org.", owner: "example.net.", wantErr: true}, + {name: "different ORIGIN", origin: "example.org.", prefix: "$ORIGIN example.net.\n", owner: "@", wantErr: true}, + {name: "not root", origin: ".", owner: "example.org.", wantErr: true}, + } + for _, tc := range tests { + for _, serial := range []int64{-1, 2, 3} { + t.Run(fmt.Sprintf("%s/serial=%d", tc.name, serial), func(t *testing.T) { + zone := tc.prefix + tc.owner + " 500 IN SOA ns.example. hostmaster.example. 3 3600 600 86400 300\n" + z, err := Parse(strings.NewReader(zone), tc.origin, "db.test", serial) + if tc.wantErr { + if err == nil { + t.Fatalf("accepted SOA owner %q for origin %q", z.SOA.Hdr.Name, tc.origin) + } + if !strings.Contains(err.Error(), "SOA owner") || !strings.Contains(err.Error(), "db.test") { + t.Fatalf("expected SOA owner error with file name, got %v", err) + } + if z != nil { + t.Fatal("invalid zone must not be returned") + } + return + } + if serial == 3 { + if _, ok := err.(*serialErr); !ok { + t.Fatalf("expected unchanged serial error, got %v", err) + } + return + } + if err != nil { + t.Fatal(err) + } + if z.SOA == nil { + t.Fatal("missing apex SOA") + } + }) + } + } +} + +func TestParseLaterSOAOrigin(t *testing.T) { + zone := `@ 500 IN SOA ns.example. hostmaster.example. 3 3600 600 86400 300 +child 500 IN SOA ns.example. hostmaster.example. 4 3600 600 86400 300 +` + z, err := Parse(strings.NewReader(zone), "example.org.", "db.test", -1) + if err == nil { + t.Fatalf("replaced apex SOA with %q", z.SOA.Hdr.Name) + } + if !strings.Contains(err.Error(), "SOA owner") { + t.Fatalf("expected SOA owner error, got %v", err) + } +} + const dbNoSOA = ` $TTL 1M $ORIGIN example.org. diff --git a/plugin/file/reload_test.go b/plugin/file/reload_test.go index 8ee42e388..d14f3552a 100644 --- a/plugin/file/reload_test.go +++ b/plugin/file/reload_test.go @@ -3,8 +3,10 @@ package file import ( "context" "os" + "path/filepath" "strings" "testing" + "testing/synctest" "time" "github.com/coredns/coredns/plugin/test" @@ -80,6 +82,52 @@ func TestZoneReloadSOAChange(t *testing.T) { } } +func TestZoneReloadSOAOrigin(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + fileName := filepath.Join(t.TempDir(), "db.example.org") + z, err := Parse(strings.NewReader(dbRelative), "example.org.", fileName, -1) + if err != nil { + t.Fatal(err) + } + beforeApex, beforeTree := z.snapshot() + updated := strings.Replace(dbRelative, " 3 3600", " 4 3600", 1) + updated = strings.Replace(updated, "192.0.2.1", "192.0.2.2", 1) + invalid := strings.Replace(updated, "@ 500 IN SOA", "child 500 IN SOA", 1) + if err := os.WriteFile(fileName, []byte(invalid), 0644); err != nil { + t.Fatal(err) + } + z.ReloadInterval = time.Second + if err := z.Reload(nil); err != nil { + t.Fatal(err) + } + defer z.OnShutdown() + + time.Sleep(2 * time.Second) + synctest.Wait() + apex, tree := z.snapshot() + if apex.SOA != beforeApex.SOA || tree != beforeTree { + t.Fatal("invalid reload replaced the last valid zone") + } + + if err := os.WriteFile(fileName, []byte(updated), 0644); err != nil { + t.Fatal(err) + } + time.Sleep(time.Second) + synctest.Wait() + apex, tree = z.snapshot() + if apex.SOA.Hdr.Name != "example.org." || apex.SOA.Serial != 4 || tree == beforeTree { + t.Fatalf("corrected zone was not reloaded: %v", apex.SOA) + } + r := new(dns.Msg) + r.SetQuestion("foo.example.org.", dns.TypeA) + state := request.Request{W: &test.ResponseWriter{}, Req: r} + answer, _, _, result := z.Lookup(context.Background(), state, state.Name()) + if result != Success || len(answer) != 1 || answer[0].String() != "foo.example.org.\t500\tIN\tA\t192.0.2.2" { + t.Fatalf("expected updated A record, got result %v, answer %v", result, answer) + } + }) +} + func TestZoneReloadByMtime(t *testing.T) { // Test 1: Basic mtime trigger - file modification should trigger reload t.Run("BasicMtimeTrigger", func(t *testing.T) { diff --git a/plugin/file/setup_test.go b/plugin/file/setup_test.go index d7c1c5887..f612140e2 100644 --- a/plugin/file/setup_test.go +++ b/plugin/file/setup_test.go @@ -1,14 +1,27 @@ package file import ( + "context" + "fmt" + "os" + "path/filepath" + "strings" "testing" "time" "github.com/coredns/caddy" + "github.com/coredns/coredns/plugin/pkg/dnstest" "github.com/coredns/coredns/plugin/pkg/fall" "github.com/coredns/coredns/plugin/test" + + "github.com/miekg/dns" ) +const dbRelative = `@ 500 IN SOA ns.example. hostmaster.example. 3 3600 600 86400 300 +@ 500 IN NS ns.example. +foo 500 IN A 192.0.2.1 +` + func TestFileParse(t *testing.T) { zoneFileName1, rm, err := test.TempFile(".", dbMiekNL) if err != nil { @@ -22,6 +35,12 @@ func TestFileParse(t *testing.T) { } defer rm() + zoneFileName3, rm, err := test.TempFile(".", dbRelative) + if err != nil { + t.Fatal(err) + } + defer rm() + tests := []struct { inputFileRules string shouldErr bool @@ -41,13 +60,13 @@ func TestFileParse(t *testing.T) { fall.Zero, }, { - `file ` + zoneFileName2 + ` 10.0.0.0/8`, + `file ` + zoneFileName3 + ` 10.0.0.0/8`, false, Zones{Names: []string{"10.in-addr.arpa."}}, fall.Zero, }, { - `file ` + zoneFileName2 + ` example.org. { + `file ` + zoneFileName3 + ` example.org. { fallthrough }`, false, @@ -55,7 +74,7 @@ func TestFileParse(t *testing.T) { fall.Root, }, { - `file ` + zoneFileName2 + ` example.org. { + `file ` + zoneFileName3 + ` example.org. { fallthrough www.example.org }`, false, @@ -78,7 +97,7 @@ func TestFileParse(t *testing.T) { fall.Zero, }, { - `file ` + zoneFileName1 + ` example.net. { + `file ` + zoneFileName3 + ` example.net. { no_reload }`, true, @@ -86,7 +105,7 @@ func TestFileParse(t *testing.T) { fall.Zero, }, { - `file ` + zoneFileName1 + ` example.net. { + `file ` + zoneFileName3 + ` example.net. { no_rebloat }`, true, @@ -120,7 +139,7 @@ func TestFileParse(t *testing.T) { } func TestParseReload(t *testing.T) { - name, rm, err := test.TempFile(".", dbMiekNL) + name, rm, err := test.TempFile(".", dbRelative) if err != nil { t.Fatal(err) } @@ -144,9 +163,87 @@ func TestParseReload(t *testing.T) { for i, test := range tests { c := caddy.NewTestController("dns", test.input) - z, _, _ := fileParse(c) + z, _, err := fileParse(c) + if err != nil { + t.Fatal(err) + } if x := z.Z["example.org."].ReloadInterval; x != test.reload { t.Errorf("Test %d expected reload to be %s, but got %s", i, test.reload, x) } } } + +func TestFileParseSOAOrigin(t *testing.T) { + for _, owner := range []string{"test", "@", "test."} { + for _, explicit := range []bool{false, true} { + t.Run(fmt.Sprintf("%s/explicit=%t", owner, explicit), func(t *testing.T) { + contents := fmt.Sprintf(`%s 500 IN SOA ns1.outside.com. root.test 3 604800 86400 2419200 604800 +%s 500 IN NS ns1.outside.com. +foo 500 IN A 1.1.1.1 +`, owner, owner) + fileName := filepath.Join(t.TempDir(), "db.test") + if err := os.WriteFile(fileName, []byte(contents), 0644); err != nil { + t.Fatal(err) + } + corefile := fmt.Sprintf("file %q", filepath.ToSlash(fileName)) + if explicit { + corefile += " test" + } + c := caddy.NewTestController("dns", corefile) + c.ServerBlockKeys = []string{"test:53"} + zones, _, err := fileParse(c) + if owner == "test" { + if err == nil || !strings.Contains(err.Error(), "SOA owner test.test. that does not match origin test.") { + t.Fatalf("expected mismatched SOA error, got %v", err) + } + return + } + if err != nil { + t.Fatal(err) + } + f := File{Zones: zones} + for _, tc := range []test.Case{ + { + Qname: "foo.test.", Qtype: dns.TypeA, Authoritative: true, + Answer: []dns.RR{test.A("foo.test. 500 IN A 1.1.1.1")}, + Ns: []dns.RR{test.NS("test. 500 IN NS ns1.outside.com.")}, + }, + { + Qname: "bar.test.", Qtype: dns.TypeA, Rcode: dns.RcodeNameError, Authoritative: true, + Ns: []dns.RR{test.SOA("test. 500 IN SOA ns1.outside.com. root.test.test. 3 604800 86400 2419200 604800")}, + }, + } { + rec := dnstest.NewRecorder(&test.ResponseWriter{}) + if _, err := f.ServeDNS(context.Background(), rec, tc.Msg()); err != nil { + t.Fatal(err) + } + if err := test.SortAndCheck(rec.Msg, tc); err != nil { + t.Error(err) + } + } + }) + } + } +} + +func TestFileParseRelativeZones(t *testing.T) { + fileName, rm, err := test.TempFile(".", dbRelative) + if err != nil { + t.Fatal(err) + } + defer rm() + c := caddy.NewTestController("dns", "file "+fileName+" example.org example.net") + zones, _, err := fileParse(c) + if err != nil { + t.Fatal(err) + } + for _, origin := range []string{"example.org.", "example.net."} { + z := zones.Z[origin] + if z == nil || z.SOA == nil || z.SOA.Hdr.Name != origin { + t.Fatalf("missing SOA at %s", origin) + } + if _, ok := z.Search("foo." + origin); !ok { + t.Errorf("missing relative A record in %s", origin) + } + } +} diff --git a/plugin/sign/nsec_test.go b/plugin/sign/nsec_test.go index f272651fc..3f36e44e4 100644 --- a/plugin/sign/nsec_test.go +++ b/plugin/sign/nsec_test.go @@ -10,11 +10,12 @@ import ( func TestNames(t *testing.T) { f, err := os.Open("testdata/db.miek.nl_ns") if err != nil { - t.Error(err) + t.Fatal(err) } - z, err := file.Parse(f, "db.miek.nl_ns", "miek.nl", 0) + defer f.Close() + z, err := file.Parse(f, "miek.nl.", "testdata/db.miek.nl_ns", 0) if err != nil { - t.Error(err) + t.Fatal(err) } names := names("miek.nl.", z) diff --git a/test/erratic_autopath_test.go b/test/erratic_autopath_test.go index 5f4a315be..7e5ee53d4 100644 --- a/test/erratic_autopath_test.go +++ b/test/erratic_autopath_test.go @@ -13,9 +13,7 @@ func setupProxyTargetCoreDNS(t *testing.T, fn func(string)) { tmpdir := t.TempDir() content := ` -example.org. IN SOA sns.dns.icann.org. noc.dns.icann.org. 1 3600 3600 3600 3600 - -google.com. IN SOA ns1.google.com. dns-admin.google.com. 1 3600 3600 3600 3600 +. IN SOA sns.dns.icann.org. noc.dns.icann.org. 1 3600 3600 3600 3600 google.com. IN A 172.217.25.110 ` diff --git a/test/file_reload_test.go b/test/file_reload_test.go index 254ef94bd..bbcd36d03 100644 --- a/test/file_reload_test.go +++ b/test/file_reload_test.go @@ -11,13 +11,13 @@ import ( ) func TestZoneReload(t *testing.T) { - name, rm, err := test.TempFile(".", exampleOrg) + name, rm, err := test.TempFile(".", relativeReloadZone) if err != nil { t.Fatalf("Failed to create zone: %s", err) } defer rm() - // Corefile with two stanzas + // Both stanzas load the same relative zone file with their own origin. corefile := ` example.org:0 { file ` + name + ` { @@ -45,7 +45,9 @@ func TestZoneReload(t *testing.T) { } // Remove RR from the Apex - os.WriteFile(name, []byte(exampleOrgUpdated), 0644) + if err := os.WriteFile(name, []byte(relativeReloadZoneUpdated), 0644); err != nil { + t.Fatal(err) + } time.Sleep(20 * time.Millisecond) // reload time, with some race insurance @@ -59,9 +61,17 @@ func TestZoneReload(t *testing.T) { } } -const exampleOrgUpdated = `; example.org test file -example.org. IN SOA sns.dns.icann.org. noc.dns.icann.org. 2016082541 7200 3600 1209600 3600 -example.org. IN NS b.iana-servers.net. -example.org. IN NS a.iana-servers.net. -example.org. IN A 127.0.0.2 +const relativeReloadZone = ` +@ IN SOA sns.dns.icann.org. noc.dns.icann.org. 2016082540 7200 3600 1209600 3600 +@ IN NS b.iana-servers.net. +@ IN NS a.iana-servers.net. +@ IN A 127.0.0.1 +@ IN A 127.0.0.2 +` + +const relativeReloadZoneUpdated = ` +@ IN SOA sns.dns.icann.org. noc.dns.icann.org. 2016082541 7200 3600 1209600 3600 +@ IN NS b.iana-servers.net. +@ IN NS a.iana-servers.net. +@ IN A 127.0.0.2 `