diff --git a/CHANGELOG.md b/CHANGELOG.md index a63b3eb..56cc0b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,19 @@ All notable changes to Jarl are documented here. The format follows The first version, to be released as 0.1.0. +### Fixed + +- The negative-caching warning now uses the effective negative TTL, which + is the smaller of the SOA minimum and the zone's default TTL (RFC 2308). + It explains the real effect (new names can stay invisible to resolvers + that already asked) and gives the command that fixes it. Before, it + fired on the SOA minimum alone and wrongly blamed BIND's resolver cap. +- Problems printed by `jarld check` show zone names without the trailing dot. +- `jarl` now shows the warnings a change introduces when it is applied or + staged, not only on dry runs. +- `jarl check` and `jarl resync` print only named-checkconf errors, not its + "loaded serial" lines. + ### Added - **jarld daemon**: REST API (`/api/v1`) and embedded web UI, with SQLite as diff --git a/cmd/jarl/admin_cmds.go b/cmd/jarl/admin_cmds.go index a25514d..2e4de3e 100644 --- a/cmd/jarl/admin_cmds.go +++ b/cmd/jarl/admin_cmds.go @@ -206,13 +206,7 @@ func printResult(r *client.Result, ok string) error { fmt.Println("error:", r.Error.Message) } printProblems(os.Stdout, r.Problems) - for _, m := range r.Check { - obj := "" - if m.Object != "" { - obj = " [" + m.Object + "]" - } - fmt.Printf(" named-checkconf: %s%s\n", m.Text, obj) - } + printCheck(os.Stdout, r.Check) // errors only, not "loaded serial" lines for _, w := range r.Warnings { fmt.Println("warning:", w) } diff --git a/cmd/jarl/main.go b/cmd/jarl/main.go index 1b30892..dea6f78 100644 --- a/cmd/jarl/main.go +++ b/cmd/jarl/main.go @@ -343,6 +343,15 @@ func report(resp *api.MutationResponse) error { } fmt.Fprintf(w, "applied (changeset %d)\n", r.ChangesetID) } + // Warnings the change introduced (errors would have blocked it). + var introduced []client.Problem + switch { + case resp.Applied && r != nil: + introduced = r.Problems + case resp.Staged && len(resp.Problems) > 0: + json.Unmarshal(resp.Problems, &introduced) + } + printProblems(os.Stderr, introduced) for _, warn := range resp.Warnings { fmt.Fprintln(os.Stderr, "warning:", warn) } diff --git a/docs/validation.md b/docs/validation.md index 9705f5e..e03db5c 100644 --- a/docs/validation.md +++ b/docs/validation.md @@ -50,7 +50,7 @@ showing them. | `rrset_too_large`, `too_many_types` | error | BIND 9.20's limits: 100 records per RRset (`max-records-per-type`), 100 types per name (`max-types-per-name`) | | `spf_multiple`, `dmarc_multiple`, `dmarc_name` | warning | More than one SPF or DMARC policy (receivers then ignore them), or DMARC outside `_dmarc.` | | `wildcard_unusual` | warning | Wildcard NS records | -| `soa_invalid`, `soa_warning`, `ttl_invalid` | error / warning | SOA fields and TTLs in range and sensible | +| `soa_invalid`, `soa_warning`, `ttl_invalid` | error / warning | SOA fields and TTLs in range and sensible. One warning covers negative caching longer than 3 hours: resolvers cache "no such name" answers for the smaller of the SOA minimum and the zone's default TTL (RFC 2308), so a new name can stay invisible that long. 1 hour (`--minimum 3600`) is a good value | | `zone_name_invalid`, `zone_type_invalid`, `zone_duplicate` | error | Valid zone names and types. A zone exists once per view | | `zone_outside_view` | error | Once views exist, every zone must be in one (named requires this) | | `zone_missing_option` | error | Secondary, stub and mirror zones need `primaries` | diff --git a/internal/validate/validate.go b/internal/validate/validate.go index 987dd5b..e4fdd20 100644 --- a/internal/validate/validate.go +++ b/internal/validate/validate.go @@ -14,6 +14,7 @@ import ( "sort" "strings" + "jarl/internal/dnsname" "jarl/internal/model" ) @@ -47,7 +48,7 @@ func (p Problem) Key() string { func (p Problem) String() string { loc := "" if p.Zone != "" { - loc = "zone " + p.Zone + loc = "zone " + dnsname.Display(p.Zone) if p.View != "" { loc += " [view " + p.View + "]" } diff --git a/internal/validate/validate_test.go b/internal/validate/validate_test.go index 673ccfc..df74447 100644 --- a/internal/validate/validate_test.go +++ b/internal/validate/validate_test.go @@ -344,3 +344,39 @@ func TestRecordLimit(t *testing.T) { t.Error("max-records-per-type 0 (unlimited) not honoured") } } + +func TestNegativeCachingTTL(t *testing.T) { + for _, tc := range []struct { + defaultTTL, minimum uint32 + warn string // "" for no warning + }{ + {3600, 300, ""}, + {3600, 86400, ""}, // the SOA's own TTL (1h) limits negative caching + {86400, 10800, ""}, // 3 hours is fine + {86400, 86400, "1 day"}, // both long: negative answers live a day + {43200, 86400, "12 hours"}, + } { + b := newBuilder() + z := b.primary("example.com", nil) + z.DefaultTTL, z.SOA.Minimum = tc.defaultTTL, tc.minimum + var got *Problem + for _, p := range State(b.s, Options{}) { + if p.Code == "soa_warning" && strings.Contains(p.Message, "negative") { + got = &p + } + } + switch { + case tc.warn == "" && got != nil: + t.Errorf("TTL %d, minimum %d: unexpected warning %q", tc.defaultTTL, tc.minimum, got.Message) + case tc.warn != "" && (got == nil || !strings.Contains(got.Message, tc.warn)): + t.Errorf("TTL %d, minimum %d: want a warning mentioning %q, got %v", tc.defaultTTL, tc.minimum, tc.warn, got) + } + } +} + +func TestProblemStringDisplaysZone(t *testing.T) { + p := Problem{Severity: Warning, Message: "m", Zone: "10.0.10.in-addr.arpa.", View: "internal"} + if got, want := p.String(), "warning: zone 10.0.10.in-addr.arpa [view internal]: m"; got != want { + t.Errorf("String() = %q, want %q", got, want) + } +} diff --git a/internal/validate/zones.go b/internal/validate/zones.go index 87acb0d..995ec27 100644 --- a/internal/validate/zones.go +++ b/internal/validate/zones.go @@ -1,6 +1,7 @@ package validate import ( + "fmt" "path/filepath" "strings" @@ -164,8 +165,16 @@ func (c *checker) checkPrimary(z *model.Zone) { } else if soa.Expire < 604800 { c.zoneProblem(z, Warning, "soa_warning", "SOA expire is under a week (%d); 2-4 weeks is recommended so secondaries survive a primary outage", soa.Expire) } - if soa.Minimum > 10800 { - c.zoneProblem(z, Warning, "soa_warning", "SOA minimum (negative caching TTL) %d is more than 3 hours; BIND caps negative caching at 3 hours", soa.Minimum) + // Resolvers cache "no such name/type" answers for the smaller of the + // SOA minimum and the SOA record's own TTL (RFC 2308 section 5); the + // SOA record uses the zone's default TTL. + if neg := min(soa.Minimum, z.DefaultTTL); neg > 10800 { + cmd := "jarl zone update " + dnsname.Display(z.Name) + if v := c.s.ViewName(z); v != "" { + cmd += " --view " + v + } + c.zoneProblem(z, Warning, "soa_warning", "resolvers may cache negative answers (\"no such name\") from this zone for %s (the smaller of the SOA minimum, %d, and the zone's default TTL, %d), so a name you add can stay invisible that long to anyone who looked it up before; RFC 2308 recommends 1 to 3 hours (lower it with '%s --minimum 3600')", + humanSeconds(neg), soa.Minimum, z.DefaultTTL, cmd) } if z.DefaultTTL > maxTTL { c.zoneProblem(z, Error, "ttl_invalid", "default TTL %d is larger than 2147483647", z.DefaultTTL) @@ -180,3 +189,19 @@ func (c *checker) checkPrimary(z *model.Zone) { } c.checkZoneRecords(z) } + +// humanSeconds formats a duration in seconds as e.g. "1 day" or "12 hours". +func humanSeconds(n uint32) string { + for _, u := range []struct { + secs uint32 + name string + }{{604800, "week"}, {86400, "day"}, {3600, "hour"}, {60, "minute"}} { + if n >= u.secs && n%u.secs == 0 { + if v := n / u.secs; v != 1 { + return fmt.Sprintf("%d %ss", v, u.name) + } + return "1 " + u.name + } + } + return fmt.Sprintf("%d seconds", n) +}