From 52a3e56c4f69d44b0b77c2de2597693eaf26b71d Mon Sep 17 00:00:00 2001 From: Pierre-Olivier Mercier Date: Fri, 15 May 2026 17:30:15 +0800 Subject: [PATCH 1/3] checker: rework target_resolvable to check existence (NOERROR) instead of A/AAAA --- checker/rules_chain.go | 36 +++++++----------------------------- checker/rules_common.go | 7 +++---- checker/rules_test.go | 13 +++++++------ 3 files changed, 17 insertions(+), 39 deletions(-) diff --git a/checker/rules_chain.go b/checker/rules_chain.go index 12e91b6..e3da350 100644 --- a/checker/rules_chain.go +++ b/checker/rules_chain.go @@ -189,24 +189,10 @@ type targetResolvableRule struct{} func (targetResolvableRule) Name() string { return "target_resolvable" } func (targetResolvableRule) Description() string { - return "Verifies that the final target of the alias chain publishes at least one A or AAAA record." + return "Verifies that the final target of the alias chain exists in DNS (returns NOERROR, not NXDOMAIN)." } -func (targetResolvableRule) Options() sdk.CheckerOptionsDocumentation { - return sdk.CheckerOptionsDocumentation{ - UserOpts: []sdk.CheckerOptionDocumentation{ - { - Id: "requireResolvableTarget", - Type: "bool", - Label: "Require resolvable target", - Description: "When enabled, a chain whose final target returns no A/AAAA is reported as critical (otherwise a warning).", - Default: defaultRequireResolvableTarget, - }, - }, - } -} - -func (targetResolvableRule) Evaluate(ctx context.Context, obs sdk.ObservationGetter, opts sdk.CheckerOptions) []sdk.CheckState { +func (targetResolvableRule) Evaluate(ctx context.Context, obs sdk.ObservationGetter, _ sdk.CheckerOptions) []sdk.CheckState { data, errState := loadAlias(ctx, obs) if errState != nil { return errState @@ -217,22 +203,14 @@ func (targetResolvableRule) Evaluate(ctx context.Context, obs sdk.ObservationGet if data.ChainTerminated.Reason != TermOK { return skipped("chain did not terminate normally") } - if len(data.FinalA) > 0 || len(data.FinalAAAA) > 0 { - return okState(data.FinalTarget, fmt.Sprintf("target %s resolves to %d address(es)", data.FinalTarget, len(data.FinalA)+len(data.FinalAAAA))) - } - status := sdk.StatusWarn - if sdk.GetBoolOption(opts, "requireResolvableTarget", defaultRequireResolvableTarget) { - status = sdk.StatusCrit - } - rcode := data.FinalRcode - if rcode == "" { - rcode = "no A/AAAA" + if data.FinalRcode != "NXDOMAIN" { + return okState(data.FinalTarget, fmt.Sprintf("target %s exists in DNS", data.FinalTarget)) } return []sdk.CheckState{withHint(sdk.CheckState{ - Status: status, + Status: sdk.StatusCrit, Subject: data.FinalTarget, - Message: fmt.Sprintf("final target %s does not resolve to an address (%s)", data.FinalTarget, rcode), - }, "Point the alias at a name that publishes at least one A or AAAA record, or fix the upstream zone.")} + Message: fmt.Sprintf("final target %s does not exist (NXDOMAIN)", data.FinalTarget), + }, "The alias points at a name that does not exist; create the missing record or update the alias target.")} } type multipleRecordsRule struct{} diff --git a/checker/rules_common.go b/checker/rules_common.go index b267fba..345cfc7 100644 --- a/checker/rules_common.go +++ b/checker/rules_common.go @@ -9,10 +9,9 @@ import ( // Defaults are centralised so Definition's docs and runtime reads cannot drift. const ( - defaultMaxChainLength = 8 - defaultMinTargetTTL = 60 - defaultRequireResolvableTarget = true - defaultAllowApexCNAME = false + defaultMaxChainLength = 8 + defaultMinTargetTTL = 60 + defaultAllowApexCNAME = false defaultRecognizeApexFlattening = true // hintKey is the CheckState.Meta key the HTML report reads to render the diff --git a/checker/rules_test.go b/checker/rules_test.go index 95a1c28..2c8452f 100644 --- a/checker/rules_test.go +++ b/checker/rules_test.go @@ -290,24 +290,25 @@ func TestCnameDnssecRule(t *testing.T) { } func TestTargetResolvableRule(t *testing.T) { - t.Run("ok", func(t *testing.T) { + t.Run("ok when NOERROR with A record", func(t *testing.T) { d := apexKnownData() d.ChainTerminated.Reason = TermOK d.FinalTarget = "target." d.FinalA = []string{"1.2.3.4"} assertSingle(t, run(targetResolvableRule{}, d, nil), sdk.StatusOK) }) - t.Run("crit by default", func(t *testing.T) { + t.Run("ok when NOERROR with no A/AAAA (e.g. service label)", func(t *testing.T) { d := apexKnownData() d.ChainTerminated.Reason = TermOK - d.FinalTarget = "target." - assertSingle(t, run(targetResolvableRule{}, d, nil), sdk.StatusCrit) + d.FinalTarget = "_2772._tcp.znc.example." + assertSingle(t, run(targetResolvableRule{}, d, nil), sdk.StatusOK) }) - t.Run("warn when requireResolvableTarget=false", func(t *testing.T) { + t.Run("crit when NXDOMAIN", func(t *testing.T) { d := apexKnownData() d.ChainTerminated.Reason = TermOK d.FinalTarget = "target." - assertSingle(t, run(targetResolvableRule{}, d, sdk.CheckerOptions{"requireResolvableTarget": false}), sdk.StatusWarn) + d.FinalRcode = "NXDOMAIN" + assertSingle(t, run(targetResolvableRule{}, d, nil), sdk.StatusCrit) }) t.Run("skip when chain did not terminate normally", func(t *testing.T) { d := apexKnownData() From 1493ef4d3f8fb2d5b535f0a0c1887dd7c7b13f31 Mon Sep 17 00:00:00 2001 From: Pierre-Olivier Mercier Date: Fri, 15 May 2026 17:37:10 +0800 Subject: [PATCH 2/3] report: move synthetic ALIAS hop from collector to report view --- checker/collect.go | 6 ------ checker/report.go | 12 ++++++++++-- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/checker/collect.go b/checker/collect.go index c30da19..5658218 100644 --- a/checker/collect.go +++ b/checker/collect.go @@ -400,12 +400,6 @@ func observeApex(ctx context.Context, data *AliasData, servers []string, apex st if (hasA || hasAAAA) && !data.ApexHasCNAME { data.ApexFlattening = true - // Synthesize a pseudo-hop so the report's chain view shows the ALIAS - // indirection that would otherwise be invisible from the wire. - data.Chain = append(data.Chain, ChainHop{ - Owner: lowerFQDN(apex), - Kind: KindALIAS, - }) } } diff --git a/checker/report.go b/checker/report.go index b8c58e7..3f98385 100644 --- a/checker/report.go +++ b/checker/report.go @@ -104,7 +104,15 @@ func buildReportView(data *AliasData, states []sdk.CheckState) *reportView { v.FinalAddresses = append(v.FinalAddresses, data.FinalA...) v.FinalAddresses = append(v.FinalAddresses, data.FinalAAAA...) - for i, h := range data.Chain { + chain := data.Chain + if data.ApexFlattening { + chain = append(chain, ChainHop{ + Owner: data.Apex, + Kind: KindALIAS, + }) + } + + for i, h := range chain { step := chainStep{ Index: i + 1, Owner: h.Owner, @@ -112,7 +120,7 @@ func buildReportView(data *AliasData, states []sdk.CheckState) *reportView { Target: h.Target, TTL: h.TTL, Server: h.Server, - IsLast: i == len(data.Chain)-1, + IsLast: i == len(chain)-1, } switch h.Kind { case KindCNAME: From c5c13960d52d4b398d3529046c3c45021fbd8fdd Mon Sep 17 00:00:00 2001 From: Pierre-Olivier Mercier Date: Sat, 16 May 2026 21:35:53 +0800 Subject: [PATCH 3/3] checker: add dname_coexistence rule and refactor sibling probing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extract querySiblings from observeCoexistence so both CNAME and DNAME coexistence checks share the same parallel RRset scan. Add observeDNAMECoexistence (called from Collect) that populates AliasData.DNAMECoexistence for each DNAME node in DNAMESubstitutions. Add the dname_coexistence rule (RFC 6672 §2.3) that flags any sibling RRsets at a DNAME owner as CRIT, with matching tests. --- checker/collect.go | 54 ++++++++++++++++++++++++++++-------- checker/definition.go | 1 + checker/rules_coexistence.go | 35 +++++++++++++++++++++++ checker/rules_test.go | 32 +++++++++++++++++++++ checker/types.go | 2 ++ 5 files changed, 112 insertions(+), 12 deletions(-) diff --git a/checker/collect.go b/checker/collect.go index 5658218..c3f2a15 100644 --- a/checker/collect.go +++ b/checker/collect.go @@ -49,6 +49,7 @@ func (p *aliasProvider) Collect(ctx context.Context, opts sdk.CheckerOptions) (a } observeCoexistence(ctx, data, servers, owner) + observeDNAMECoexistence(ctx, data, servers) observeDNSSEC(ctx, data, servers, apex, owner) return data, nil @@ -403,20 +404,18 @@ func observeApex(ctx context.Context, data *AliasData, servers []string, apex st } } -func observeCoexistence(ctx context.Context, data *AliasData, servers []string, owner string) { - if !data.OwnerHasCNAME { - return - } - - siblings := []uint16{ +// querySiblings returns RRsets of common types that sit alongside a CNAME or DNAME at owner. +// Filter on owner+type: a DNAME-synthesized CNAME would otherwise count as a sibling. +func querySiblings(ctx context.Context, servers []string, owner string) []CoexistingRRset { + candidates := []uint16{ dns.TypeA, dns.TypeAAAA, dns.TypeMX, dns.TypeTXT, dns.TypeNS, dns.TypeSRV, dns.TypeCAA, } seen := map[string]uint32{} var mu sync.Mutex var wg sync.WaitGroup - wg.Add(len(siblings)) - for _, qt := range siblings { + wg.Add(len(candidates)) + for _, qt := range candidates { go func() { defer wg.Done() q := dns.Question{Name: owner, Qtype: qt, Qclass: dns.ClassINET} @@ -424,8 +423,6 @@ func observeCoexistence(ctx context.Context, data *AliasData, servers []string, if err != nil || r == nil { return } - // Filter on owner+type because a DNAME-synthesized CNAME would - // otherwise count as a sibling of every queried type. for _, rr := range r.Answer { if rr.Header().Rrtype != qt { continue @@ -441,9 +438,42 @@ func observeCoexistence(ctx context.Context, data *AliasData, servers []string, }() } wg.Wait() - + var out []CoexistingRRset for t, ttl := range seen { - data.Coexisting = append(data.Coexisting, CoexistingRRset{Type: t, TTL: ttl}) + out = append(out, CoexistingRRset{Type: t, TTL: ttl}) + } + return out +} + +func observeCoexistence(ctx context.Context, data *AliasData, servers []string, owner string) { + if !data.OwnerHasCNAME { + return + } + data.Coexisting = querySiblings(ctx, servers, owner) +} + +func observeDNAMECoexistence(ctx context.Context, data *AliasData, servers []string) { + if len(data.DNAMESubstitutions) == 0 { + return + } + results := make(map[string][]CoexistingRRset, len(data.DNAMESubstitutions)) + var mu sync.Mutex + var wg sync.WaitGroup + wg.Add(len(data.DNAMESubstitutions)) + for _, hop := range data.DNAMESubstitutions { + go func() { + defer wg.Done() + siblings := querySiblings(ctx, servers, hop.Owner) + if len(siblings) > 0 { + mu.Lock() + results[hop.Owner] = siblings + mu.Unlock() + } + }() + } + wg.Wait() + if len(results) > 0 { + data.DNAMECoexistence = results } } diff --git a/checker/definition.go b/checker/definition.go index 772d966..38af530 100644 --- a/checker/definition.go +++ b/checker/definition.go @@ -80,6 +80,7 @@ func Definition() *sdk.CheckerDefinition { cnameAtApexRule{}, apexFlatteningRule{}, cnameCoexistenceRule{}, + dnameCoexistenceRule{}, cnameDnssecRule{}, targetResolvableRule{}, multipleRecordsRule{}, diff --git a/checker/rules_coexistence.go b/checker/rules_coexistence.go index 252c987..d305031 100644 --- a/checker/rules_coexistence.go +++ b/checker/rules_coexistence.go @@ -7,6 +7,41 @@ import ( sdk "git.happydns.org/checker-sdk-go/checker" ) +type dnameCoexistenceRule struct{} + +func (dnameCoexistenceRule) Name() string { return "dname_coexistence" } +func (dnameCoexistenceRule) Description() string { + return "Flags RRsets that sit at the same owner as a DNAME (RFC 6672 §2.3)." +} + +func (dnameCoexistenceRule) Evaluate(ctx context.Context, obs sdk.ObservationGetter, _ sdk.CheckerOptions) []sdk.CheckState { + data, errState := loadAlias(ctx, obs) + if errState != nil { + return errState + } + if !apexKnown(data) { + return skipped("apex lookup failed") + } + if len(data.DNAMESubstitutions) == 0 { + return skipped("no DNAME in chain") + } + if len(data.DNAMECoexistence) == 0 { + return okState(data.Owner, "all DNAME nodes have no sibling records") + } + var out []sdk.CheckState + for owner, coexisting := range data.DNAMECoexistence { + for _, rr := range coexisting { + out = append(out, withHint(sdk.CheckState{ + Status: sdk.StatusCrit, + Subject: owner, + Message: fmt.Sprintf("%s and DNAME both exist at %s (RFC 6672 §2.3)", rr.Type, owner), + Code: rr.Type, + }, "Remove the sibling record or move it under a different label; a DNAME owner must not carry other data.")) + } + } + return out +} + type cnameCoexistenceRule struct{} func (cnameCoexistenceRule) Name() string { return "cname_coexistence" } diff --git a/checker/rules_test.go b/checker/rules_test.go index 2c8452f..1d2d92f 100644 --- a/checker/rules_test.go +++ b/checker/rules_test.go @@ -266,6 +266,38 @@ func TestCnameCoexistenceRule(t *testing.T) { }) } +func TestDnameCoexistenceRule(t *testing.T) { + t.Run("skip when no DNAME in chain", func(t *testing.T) { + d := apexKnownData() + assertSkipped(t, run(dnameCoexistenceRule{}, d, nil), "no DNAME in chain") + }) + t.Run("ok when DNAME has no siblings", func(t *testing.T) { + d := apexKnownData() + d.DNAMESubstitutions = []ChainHop{{Owner: "old.example.com.", Kind: KindDNAME, Target: "new.example.com."}} + assertSingle(t, run(dnameCoexistenceRule{}, d, nil), sdk.StatusOK) + }) + t.Run("crit when DNAME has siblings", func(t *testing.T) { + d := apexKnownData() + d.DNAMESubstitutions = []ChainHop{{Owner: "old.example.com.", Kind: KindDNAME, Target: "new.example.com."}} + d.DNAMECoexistence = map[string][]CoexistingRRset{ + "old.example.com.": {{Type: "MX"}, {Type: "A"}}, + } + states := run(dnameCoexistenceRule{}, d, nil) + if len(states) != 2 { + t.Fatalf("want 2 states, got %d: %+v", len(states), states) + } + for _, s := range states { + if s.Status != sdk.StatusCrit { + t.Fatalf("want CRIT, got %v", s.Status) + } + } + }) + t.Run("skip when apex unknown", func(t *testing.T) { + d := &AliasData{Owner: "x.", ApexLookupError: "boom"} + assertSkipped(t, run(dnameCoexistenceRule{}, d, nil), "apex") + }) +} + func TestCnameDnssecRule(t *testing.T) { t.Run("skip unsigned zone", func(t *testing.T) { d := apexKnownData() diff --git a/checker/types.go b/checker/types.go index 724ec27..ecfa074 100644 --- a/checker/types.go +++ b/checker/types.go @@ -70,6 +70,8 @@ type AliasData struct { // Coexisting is populated only when Owner has a CNAME. Coexisting []CoexistingRRset `json:"coexisting,omitempty"` + // DNAMECoexistence maps each DNAME owner (from DNAMESubstitutions) to its sibling RRsets. + DNAMECoexistence map[string][]CoexistingRRset `json:"dname_coexistence,omitempty"` OwnerIsApex bool `json:"owner_is_apex,omitempty"` OwnerHasCNAME bool `json:"owner_has_cname,omitempty"`