diff --git a/checker/collect.go b/checker/collect.go index c3f2a15..c30da19 100644 --- a/checker/collect.go +++ b/checker/collect.go @@ -49,7 +49,6 @@ 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 @@ -401,21 +400,29 @@ 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, + }) } } -// 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{ +func observeCoexistence(ctx context.Context, data *AliasData, servers []string, owner string) { + if !data.OwnerHasCNAME { + return + } + + siblings := []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(candidates)) - for _, qt := range candidates { + wg.Add(len(siblings)) + for _, qt := range siblings { go func() { defer wg.Done() q := dns.Question{Name: owner, Qtype: qt, Qclass: dns.ClassINET} @@ -423,6 +430,8 @@ func querySiblings(ctx context.Context, servers []string, owner string) []Coexis 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 @@ -438,42 +447,9 @@ func querySiblings(ctx context.Context, servers []string, owner string) []Coexis }() } wg.Wait() - var out []CoexistingRRset + for t, ttl := range seen { - 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 + data.Coexisting = append(data.Coexisting, CoexistingRRset{Type: t, TTL: ttl}) } } diff --git a/checker/definition.go b/checker/definition.go index 38af530..772d966 100644 --- a/checker/definition.go +++ b/checker/definition.go @@ -80,7 +80,6 @@ func Definition() *sdk.CheckerDefinition { cnameAtApexRule{}, apexFlatteningRule{}, cnameCoexistenceRule{}, - dnameCoexistenceRule{}, cnameDnssecRule{}, targetResolvableRule{}, multipleRecordsRule{}, diff --git a/checker/report.go b/checker/report.go index 3f98385..b8c58e7 100644 --- a/checker/report.go +++ b/checker/report.go @@ -104,15 +104,7 @@ func buildReportView(data *AliasData, states []sdk.CheckState) *reportView { v.FinalAddresses = append(v.FinalAddresses, data.FinalA...) v.FinalAddresses = append(v.FinalAddresses, data.FinalAAAA...) - chain := data.Chain - if data.ApexFlattening { - chain = append(chain, ChainHop{ - Owner: data.Apex, - Kind: KindALIAS, - }) - } - - for i, h := range chain { + for i, h := range data.Chain { step := chainStep{ Index: i + 1, Owner: h.Owner, @@ -120,7 +112,7 @@ func buildReportView(data *AliasData, states []sdk.CheckState) *reportView { Target: h.Target, TTL: h.TTL, Server: h.Server, - IsLast: i == len(chain)-1, + IsLast: i == len(data.Chain)-1, } switch h.Kind { case KindCNAME: diff --git a/checker/rules_chain.go b/checker/rules_chain.go index e3da350..12e91b6 100644 --- a/checker/rules_chain.go +++ b/checker/rules_chain.go @@ -189,10 +189,24 @@ type targetResolvableRule struct{} func (targetResolvableRule) Name() string { return "target_resolvable" } func (targetResolvableRule) Description() string { - return "Verifies that the final target of the alias chain exists in DNS (returns NOERROR, not NXDOMAIN)." + return "Verifies that the final target of the alias chain publishes at least one A or AAAA record." } -func (targetResolvableRule) Evaluate(ctx context.Context, obs sdk.ObservationGetter, _ sdk.CheckerOptions) []sdk.CheckState { +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 { data, errState := loadAlias(ctx, obs) if errState != nil { return errState @@ -203,14 +217,22 @@ func (targetResolvableRule) Evaluate(ctx context.Context, obs sdk.ObservationGet if data.ChainTerminated.Reason != TermOK { return skipped("chain did not terminate normally") } - if data.FinalRcode != "NXDOMAIN" { - return okState(data.FinalTarget, fmt.Sprintf("target %s exists in DNS", data.FinalTarget)) + 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" } return []sdk.CheckState{withHint(sdk.CheckState{ - Status: sdk.StatusCrit, + Status: status, Subject: data.FinalTarget, - 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.")} + 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.")} } type multipleRecordsRule struct{} diff --git a/checker/rules_coexistence.go b/checker/rules_coexistence.go index d305031..252c987 100644 --- a/checker/rules_coexistence.go +++ b/checker/rules_coexistence.go @@ -7,41 +7,6 @@ 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 345cfc7..b267fba 100644 --- a/checker/rules_common.go +++ b/checker/rules_common.go @@ -9,9 +9,10 @@ import ( // Defaults are centralised so Definition's docs and runtime reads cannot drift. const ( - defaultMaxChainLength = 8 - defaultMinTargetTTL = 60 - defaultAllowApexCNAME = false + defaultMaxChainLength = 8 + defaultMinTargetTTL = 60 + defaultRequireResolvableTarget = true + 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 1d2d92f..95a1c28 100644 --- a/checker/rules_test.go +++ b/checker/rules_test.go @@ -266,38 +266,6 @@ 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() @@ -322,26 +290,25 @@ func TestCnameDnssecRule(t *testing.T) { } func TestTargetResolvableRule(t *testing.T) { - t.Run("ok when NOERROR with A record", func(t *testing.T) { + t.Run("ok", 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("ok when NOERROR with no A/AAAA (e.g. service label)", func(t *testing.T) { - d := apexKnownData() - d.ChainTerminated.Reason = TermOK - d.FinalTarget = "_2772._tcp.znc.example." - assertSingle(t, run(targetResolvableRule{}, d, nil), sdk.StatusOK) - }) - t.Run("crit when NXDOMAIN", func(t *testing.T) { + t.Run("crit by default", func(t *testing.T) { d := apexKnownData() d.ChainTerminated.Reason = TermOK d.FinalTarget = "target." - d.FinalRcode = "NXDOMAIN" assertSingle(t, run(targetResolvableRule{}, d, nil), sdk.StatusCrit) }) + t.Run("warn when requireResolvableTarget=false", func(t *testing.T) { + d := apexKnownData() + d.ChainTerminated.Reason = TermOK + d.FinalTarget = "target." + assertSingle(t, run(targetResolvableRule{}, d, sdk.CheckerOptions{"requireResolvableTarget": false}), sdk.StatusWarn) + }) t.Run("skip when chain did not terminate normally", func(t *testing.T) { d := apexKnownData() d.ChainTerminated.Reason = TermLoop diff --git a/checker/types.go b/checker/types.go index ecfa074..724ec27 100644 --- a/checker/types.go +++ b/checker/types.go @@ -70,8 +70,6 @@ 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"`