From 3c73afe0e1daf5db2a92c6ae8441b90ad591eebd Mon Sep 17 00:00:00 2001 From: abrown38 Date: Wed, 7 Oct 2026 15:29:05 +0000 Subject: [PATCH] Fix the negative-caching warning and show warnings in the CLI The soa_warning for negative caching fired on the SOA minimum alone and blamed BIND's resolver cap (max-ncache-ttl), which doesn't apply to an authoritative zone. Resolvers cache negative answers for the smaller of the SOA minimum and the SOA record's TTL (RFC 2308 section 5), and the SOA record uses the zone's default TTL. The warning now uses that value, explains the effect, and suggests the fix command (with --view when the zone is in a view). Also: - Problem.String shows zone names without the trailing dot (jarld check). - jarl prints the warnings a change introduces when it is applied or staged, not only on dry runs. - jarl check/resync print only named-checkconf errors, not its "loaded serial" lines. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 13 +++++++++++ cmd/jarl/admin_cmds.go | 8 +------ cmd/jarl/main.go | 9 ++++++++ docs/validation.md | 2 +- internal/validate/validate.go | 3 ++- internal/validate/validate_test.go | 36 ++++++++++++++++++++++++++++++ internal/validate/zones.go | 29 ++++++++++++++++++++++-- 7 files changed, 89 insertions(+), 11 deletions(-) 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) +}