diff --git a/checker/collect.go b/checker/collect.go index c30da19..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 @@ -400,29 +401,21 @@ 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, - }) } } -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} @@ -430,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 @@ -447,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/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: 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_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_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..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() @@ -290,24 +322,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() 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"`