diff --git a/.drone-manifest.yml b/.drone-manifest.yml new file mode 100644 index 0000000..e7df71b --- /dev/null +++ b/.drone-manifest.yml @@ -0,0 +1,22 @@ +image: happydomain/checker-dane:{{#if build.tag}}{{trimPrefix "v" build.tag}}{{else}}latest{{/if}} +{{#if build.tags}} +tags: +{{#each build.tags}} + - {{this}} +{{/each}} +{{/if}} +manifests: + - image: happydomain/checker-dane:{{#if build.tag}}{{trimPrefix "v" build.tag}}-{{/if}}linux-amd64 + platform: + architecture: amd64 + os: linux + - image: happydomain/checker-dane:{{#if build.tag}}{{trimPrefix "v" build.tag}}-{{/if}}linux-arm64 + platform: + architecture: arm64 + os: linux + variant: v8 + - image: happydomain/checker-dane:{{#if build.tag}}{{trimPrefix "v" build.tag}}-{{/if}}linux-arm + platform: + architecture: arm + os: linux + variant: v7 diff --git a/.drone.yml b/.drone.yml new file mode 100644 index 0000000..e0bce12 --- /dev/null +++ b/.drone.yml @@ -0,0 +1,187 @@ +--- +kind: pipeline +type: docker +name: build-amd64 + +platform: + os: linux + arch: amd64 + +steps: + - name: checker build + image: golang:1-alpine + commands: + - apk add --no-cache git make + - make + environment: + CHECKER_VERSION: "${DRONE_BRANCH}-${DRONE_COMMIT}" + CGO_ENABLED: 0 + when: + event: + exclude: + - tag + + - name: checker build tag + image: golang:1-alpine + commands: + - apk add --no-cache git make + - make + environment: + CHECKER_VERSION: "${DRONE_SEMVER}" + CGO_ENABLED: 0 + when: + event: + - tag + + - name: publish on Docker Hub + image: plugins/docker + settings: + repo: happydomain/checker-dane + auto_tag: true + auto_tag_suffix: ${DRONE_STAGE_OS}-${DRONE_STAGE_ARCH} + dockerfile: Dockerfile + build_args: + - CHECKER_VERSION=${DRONE_BRANCH}-${DRONE_COMMIT} + username: + from_secret: docker_username + password: + from_secret: docker_password + when: + event: + exclude: + - tag + + - name: publish on Docker Hub (tag) + image: plugins/docker + settings: + repo: happydomain/checker-dane + auto_tag: true + auto_tag_suffix: ${DRONE_STAGE_OS}-${DRONE_STAGE_ARCH} + dockerfile: Dockerfile + build_args: + - CHECKER_VERSION=${DRONE_SEMVER} + username: + from_secret: docker_username + password: + from_secret: docker_password + when: + event: + - tag + +trigger: + branch: + exclude: + - renovate/* + event: + - cron + - push + - tag + +--- +kind: pipeline +type: docker +name: build-arm64 + +platform: + os: linux + arch: arm64 + +steps: + - name: checker build + image: golang:1-alpine + commands: + - apk add --no-cache git make + - make + environment: + CHECKER_VERSION: "${DRONE_BRANCH}-${DRONE_COMMIT}" + CGO_ENABLED: 0 + when: + event: + exclude: + - tag + + - name: checker build tag + image: golang:1-alpine + commands: + - apk add --no-cache git make + - make + environment: + CHECKER_VERSION: "${DRONE_SEMVER}" + CGO_ENABLED: 0 + when: + event: + - tag + + - name: publish on Docker Hub + image: plugins/docker + settings: + repo: happydomain/checker-dane + auto_tag: true + auto_tag_suffix: ${DRONE_STAGE_OS}-${DRONE_STAGE_ARCH} + dockerfile: Dockerfile + build_args: + - CHECKER_VERSION=${DRONE_BRANCH}-${DRONE_COMMIT} + username: + from_secret: docker_username + password: + from_secret: docker_password + when: + event: + exclude: + - tag + + - name: publish on Docker Hub (tag) + image: plugins/docker + settings: + repo: happydomain/checker-dane + auto_tag: true + auto_tag_suffix: ${DRONE_STAGE_OS}-${DRONE_STAGE_ARCH} + dockerfile: Dockerfile + build_args: + - CHECKER_VERSION=${DRONE_SEMVER} + username: + from_secret: docker_username + password: + from_secret: docker_password + when: + event: + - tag + +trigger: + event: + - cron + - push + - tag + +--- +kind: pipeline +name: docker-manifest + +platform: + os: linux + arch: arm64 + +steps: + - name: publish on Docker Hub + image: plugins/manifest + settings: + auto_tag: true + ignore_missing: true + spec: .drone-manifest.yml + username: + from_secret: docker_username + password: + from_secret: docker_password + +trigger: + branch: + exclude: + - renovate/* + event: + - cron + - push + - tag + +depends_on: + - build-amd64 + - build-arm64 diff --git a/Dockerfile b/Dockerfile index d407a62..35a3be4 100644 --- a/Dockerfile +++ b/Dockerfile @@ -10,5 +10,8 @@ RUN CGO_ENABLED=0 go build -tags standalone -ldflags "-X main.Version=${CHECKER_ FROM scratch COPY --from=builder /checker-dane /checker-dane +USER 65534:65534 EXPOSE 8080 +HEALTHCHECK --interval=30s --timeout=3s --start-period=5s --retries=3 \ + CMD ["/checker-dane", "-healthcheck"] ENTRYPOINT ["/checker-dane"] diff --git a/checker/collect.go b/checker/collect.go index 31a399e..e444d65 100644 --- a/checker/collect.go +++ b/checker/collect.go @@ -20,7 +20,14 @@ import ( var tlsaOwner = regexp.MustCompile(`^_(\d+)\._(tcp|udp)(?:\.(.*))?$`) // tlsaOwnerName builds the canonical "_._." owner name. +// When base is empty (TLSA records sit directly at the zone apex of an +// otherwise-unspecified host), the trailing label is omitted so the result +// is still a syntactically valid relative name rather than "_443._tcp.". func tlsaOwnerName(port uint16, proto, base string) string { + base = strings.TrimSuffix(base, ".") + if base == "" { + return fmt.Sprintf("_%d._%s", port, proto) + } return fmt.Sprintf("_%d._%s.%s", port, proto, base) } @@ -75,6 +82,9 @@ var defaultSTARTTLS = map[uint16]string{ // No TLSA matching happens here; that's the rule's job: it reads the TLS // chain via obs.GetRelated on the next evaluation. func (p *daneProvider) Collect(ctx context.Context, opts sdk.CheckerOptions) (any, error) { + if err := ctx.Err(); err != nil { + return nil, err + } svc, err := serviceFromOptions(opts) if err != nil { return nil, err @@ -107,13 +117,23 @@ func (p *daneProvider) Collect(ctx context.Context, opts sdk.CheckerOptions) (an Base string // base host, fully-qualified without trailing dot } groups := map[key][]TLSARecord{} + var invalid []InvalidRecord for _, r := range pl.Records { - m := tlsaOwner.FindStringSubmatch(strings.TrimSuffix(r.Hdr.Name, ".")) + owner := strings.TrimSuffix(r.Hdr.Name, ".") + m := tlsaOwner.FindStringSubmatch(owner) if len(m) != 4 { + invalid = append(invalid, InvalidRecord{ + Owner: owner, + Reason: "owner name does not match _._[.]", + }) continue } port64, err := strconv.ParseUint(m[1], 10, 16) - if err != nil { + if err != nil || port64 == 0 { + invalid = append(invalid, InvalidRecord{ + Owner: owner, + Reason: fmt.Sprintf("port %q out of range (1-65535)", m[1]), + }) continue } base := m[3] @@ -121,6 +141,13 @@ func (p *daneProvider) Collect(ctx context.Context, opts sdk.CheckerOptions) (an // are typically stored relative to the service's subdomain // bucket. Fall back to the apex when unspecified. base = joinName(base, subdomain, apex) + if base == "" { + invalid = append(invalid, InvalidRecord{ + Owner: owner, + Reason: "could not resolve a host name (apex and subdomain both empty)", + }) + continue + } k := key{Port: uint16(port64), Proto: m[2], Base: base} groups[k] = append(groups[k], TLSARecord{ @@ -164,20 +191,32 @@ func (p *daneProvider) Collect(ctx context.Context, opts sdk.CheckerOptions) (an targets = append(targets, t) } - return &DANEData{ + data := &DANEData{ Targets: targets, + Invalid: invalid, CollectedAt: time.Now().UTC(), - }, nil + } + if v, ok := opts[OptionDNSSECValidated]; ok { + if b, ok := v.(bool); ok { + data.DNSSECValidated = &b + } + } + return data, nil } // endpointFromTarget builds the TLSEndpoint for a collected target. func endpointFromTarget(t TargetResult) tlscontract.TLSEndpoint { return tlscontract.TLSEndpoint{ - Host: t.Host, - Port: t.Port, - SNI: t.Host, - STARTTLS: t.STARTTLS, - RequireSTARTTLS: t.STARTTLS != "" && t.Port != 25, // SMTP on 25 stays opportunistic + Host: t.Host, + Port: t.Port, + SNI: t.Host, + STARTTLS: t.STARTTLS, + // RFC 7672 §2.2: when a TLSA record exists for an SMTP service, the + // receiving MTA MUST use STARTTLS. The whole point of DANE on port 25 + // is to defeat STARTTLS-stripping downgrade attacks, so the presence + // of TLSA records here flips the connection from opportunistic to + // mandatory. + RequireSTARTTLS: t.STARTTLS != "", } } diff --git a/checker/collect_test.go b/checker/collect_test.go new file mode 100644 index 0000000..72c732d --- /dev/null +++ b/checker/collect_test.go @@ -0,0 +1,226 @@ +package checker + +import ( + "context" + "encoding/json" + "testing" + + sdk "git.happydns.org/checker-sdk-go/checker" +) + +func makeOpts(t *testing.T, apex, subdomain string, records []map[string]any, starttls map[string]string) sdk.CheckerOptions { + t.Helper() + svc := map[string]any{ + "_svctype": serviceType, + "_domain": apex, + "Service": map[string]any{"tlsa": records}, + } + opts := sdk.CheckerOptions{ + OptionDomain: apex, + OptionService: svc, + } + if subdomain != "" { + opts[OptionSubdomain] = subdomain + } + if starttls != nil { + opts[OptionSTARTTLS] = starttls + } + return opts +} + +func tlsaRR(owner string, usage, selector, mtype int, cert string) map[string]any { + return map[string]any{ + "Hdr": map[string]any{"Name": owner}, + "Usage": usage, + "Selector": selector, + "MatchingType": mtype, + "Certificate": cert, + } +} + +func TestCollect_GroupsByEndpoint(t *testing.T) { + t.Parallel() + opts := makeOpts(t, "example.com.", "", []map[string]any{ + tlsaRR("_443._tcp.example.com.", 3, 1, 1, "AABB"), + tlsaRR("_443._tcp.example.com.", 3, 1, 1, "CCDD"), + tlsaRR("_25._tcp.mail.example.com.", 3, 1, 1, "EEFF"), + }, nil) + p := &daneProvider{} + out, err := p.Collect(context.Background(), opts) + if err != nil { + t.Fatalf("err=%v", err) + } + d := out.(*DANEData) + if len(d.Targets) != 2 { + t.Fatalf("targets=%d want 2", len(d.Targets)) + } + // Sorted by base alphabetically: example.com < mail.example.com. + if d.Targets[0].Host != "example.com" || d.Targets[0].Port != 443 { + t.Errorf("sort[0]: %+v", d.Targets[0]) + } + if d.Targets[1].Host != "mail.example.com" || d.Targets[1].Port != 25 { + t.Errorf("sort[1]: %+v", d.Targets[1]) + } + // Two records on the 443 endpoint + if len(d.Targets[0].Records) != 2 { + t.Errorf("443 records=%d want 2", len(d.Targets[0].Records)) + } + // Certificate hex was lowercased + if d.Targets[0].Records[0].Certificate != "aabb" { + t.Errorf("expected lowercased cert, got %q", d.Targets[0].Records[0].Certificate) + } +} + +func TestCollect_DefaultSTARTTLS(t *testing.T) { + t.Parallel() + opts := makeOpts(t, "example.com", "", []map[string]any{ + tlsaRR("_25._tcp.mail.example.com", 3, 1, 1, "00"), + tlsaRR("_443._tcp.example.com", 3, 1, 1, "00"), + tlsaRR("_587._tcp.mail.example.com", 3, 1, 1, "00"), + }, nil) + out, err := (&daneProvider{}).Collect(context.Background(), opts) + if err != nil { + t.Fatal(err) + } + d := out.(*DANEData) + got := map[uint16]string{} + for _, t := range d.Targets { + got[t.Port] = t.STARTTLS + } + if got[25] != "smtp" { + t.Errorf("port 25 starttls=%q want smtp", got[25]) + } + if got[443] != "" { + t.Errorf("port 443 starttls=%q want empty (direct TLS)", got[443]) + } + if got[587] != "submission" { + t.Errorf("port 587 starttls=%q want submission", got[587]) + } +} + +func TestCollect_STARTTLSOverride(t *testing.T) { + t.Parallel() + opts := makeOpts(t, "example.com", "", []map[string]any{ + tlsaRR("_25._tcp.mail.example.com", 3, 1, 1, "00"), + }, map[string]string{"25/tcp": "lmtp"}) + out, err := (&daneProvider{}).Collect(context.Background(), opts) + if err != nil { + t.Fatal(err) + } + d := out.(*DANEData) + if d.Targets[0].STARTTLS != "lmtp" { + t.Errorf("override: starttls=%q want lmtp", d.Targets[0].STARTTLS) + } +} + +func TestCollect_MalformedOwnerSurfaced(t *testing.T) { + t.Parallel() + opts := makeOpts(t, "example.com", "", []map[string]any{ + tlsaRR("totally-invalid", 3, 1, 1, "00"), + tlsaRR("_99999._tcp.example.com", 3, 1, 1, "00"), // port > 65535 + tlsaRR("_443._tcp.example.com", 3, 1, 1, "AA"), + }, nil) + out, err := (&daneProvider{}).Collect(context.Background(), opts) + if err != nil { + t.Fatal(err) + } + d := out.(*DANEData) + if len(d.Targets) != 1 { + t.Errorf("expected one well-formed target, got %d", len(d.Targets)) + } + if len(d.Invalid) != 2 { + t.Errorf("expected 2 invalid entries, got %d (%+v)", len(d.Invalid), d.Invalid) + } +} + +func TestCollect_BaseRelativeToSubdomain(t *testing.T) { + t.Parallel() + opts := makeOpts(t, "example.com", "mail", []map[string]any{ + // Owner has no base, so the records live on the subdomain itself. + tlsaRR("_25._tcp", 3, 1, 1, "AA"), + }, nil) + out, err := (&daneProvider{}).Collect(context.Background(), opts) + if err != nil { + t.Fatal(err) + } + d := out.(*DANEData) + if len(d.Targets) != 1 { + t.Fatalf("targets=%d", len(d.Targets)) + } + if d.Targets[0].Host != "mail.example.com" { + t.Errorf("host=%q want mail.example.com", d.Targets[0].Host) + } + if d.Targets[0].Owner != "_25._tcp.mail.example.com" { + t.Errorf("owner=%q", d.Targets[0].Owner) + } +} + +func TestCollect_WrongServiceType(t *testing.T) { + t.Parallel() + svc := map[string]any{ + "_svctype": "svcs.NotTLSAs", + "Service": map[string]any{"tlsa": []any{}}, + } + opts := sdk.CheckerOptions{OptionDomain: "example.com", OptionService: svc} + if _, err := (&daneProvider{}).Collect(context.Background(), opts); err == nil { + t.Error("expected error on wrong service type") + } +} + +func TestCollect_MissingService(t *testing.T) { + t.Parallel() + opts := sdk.CheckerOptions{OptionDomain: "example.com"} + if _, err := (&daneProvider{}).Collect(context.Background(), opts); err == nil { + t.Error("expected error on missing service") + } +} + +func TestCollect_DiscoverEntries(t *testing.T) { + t.Parallel() + opts := makeOpts(t, "example.com", "", []map[string]any{ + tlsaRR("_443._tcp.example.com", 3, 1, 1, "AA"), + tlsaRR("_25._tcp.mail.example.com", 3, 1, 1, "BB"), + }, nil) + p := &daneProvider{} + data, err := p.Collect(context.Background(), opts) + if err != nil { + t.Fatal(err) + } + entries, err := p.DiscoverEntries(data) + if err != nil { + t.Fatalf("err=%v", err) + } + if len(entries) != 2 { + t.Errorf("entries=%d want 2", len(entries)) + } + + // Nil/wrong type returns nil, nil (defensive). + if got, err := p.DiscoverEntries(nil); err != nil || got != nil { + t.Errorf("nil: got=%v err=%v", got, err) + } + if got, err := p.DiscoverEntries("not a *DANEData"); err != nil || got != nil { + t.Errorf("wrong type: got=%v err=%v", got, err) + } +} + +func TestCollect_DeterministicOutput(t *testing.T) { + t.Parallel() + opts := makeOpts(t, "example.com", "", []map[string]any{ + tlsaRR("_25._tcp.b.example.com", 3, 1, 1, "AA"), + tlsaRR("_25._tcp.a.example.com", 3, 1, 1, "BB"), + tlsaRR("_443._tcp.a.example.com", 3, 1, 1, "CC"), + }, nil) + var prev []byte + for i := range 3 { + out, err := (&daneProvider{}).Collect(context.Background(), opts) + if err != nil { + t.Fatal(err) + } + // Compare only Targets: CollectedAt is a wall-clock timestamp. + b, _ := json.Marshal(out.(*DANEData).Targets) + if i > 0 && string(b) != string(prev) { + t.Errorf("non-deterministic targets:\n%s\nvs\n%s", prev, b) + } + prev = b + } +} diff --git a/checker/definition.go b/checker/definition.go index 5ef2c21..9dba3cb 100644 --- a/checker/definition.go +++ b/checker/definition.go @@ -14,11 +14,6 @@ var Version = "built-in" // serviceType is the happyDomain service type this checker binds to. const serviceType = "svcs.TLSAs" -// Definition is the package-level helper exposed to the plugin entrypoint. -func Definition() *sdk.CheckerDefinition { - return (&daneProvider{}).Definition() -} - // Definition satisfies sdk.CheckerDefinitionProvider. func (p *daneProvider) Definition() *sdk.CheckerDefinition { return &sdk.CheckerDefinition{ diff --git a/checker/interactive.go b/checker/interactive.go index 47cf8d2..1a15f96 100644 --- a/checker/interactive.go +++ b/checker/interactive.go @@ -3,13 +3,16 @@ package checker import ( + "context" "encoding/json" "errors" "fmt" "net" "net/http" + "os" "strconv" "strings" + "time" "github.com/miekg/dns" @@ -17,8 +20,25 @@ import ( tls "git.happydns.org/checker-tls/checker" ) -// tlsaLookup fetches TLSA records for owner via the system resolver. -// It is a package variable so tests can swap it for a fixture. +// resolverEnvVar names the environment variable that points at the +// DNSSEC-validating resolver this checker queries. The operator MUST point +// this at a trusted, validating resolver (typically 127.0.0.1:53 backed by +// Unbound, BIND, or Knot Resolver). DANE without DNSSEC validation is a +// downgrade primitive: an on-path attacker can forge TLSA responses. To +// fail loudly rather than silently insecure, lookupTLSA returns an error +// when no validating resolver is configured. +const resolverEnvVar = "DANE_CHECKER_RESOLVER" + +// dnsClientTimeout bounds each TLSA exchange so a black-holing resolver +// cannot tie up server goroutines indefinitely on the public listener. +const dnsClientTimeout = 5 * time.Second + +// tlsaLookup fetches TLSA records for owner via the system resolver and +// reports whether the resolver cryptographically validated the answer +// (AD bit set). It is a package variable so tests can swap it for a +// fixture. The context bounds the underlying DNS exchange so a slow or +// hung resolver cannot outlive the originating HTTP request on the +// public listener. var tlsaLookup = lookupTLSA // RenderForm lets a human run this checker standalone. The form only @@ -74,7 +94,7 @@ func (p *daneProvider) ParseForm(r *http.Request) (sdk.CheckerOptions, error) { } owner := tlsaOwnerName(port, proto, domain) - records, err := tlsaLookup(owner) + records, validated, err := tlsaLookup(r.Context(), owner) if err != nil { return nil, fmt.Errorf("TLSA lookup for %s: %w", owner, err) } @@ -116,6 +136,7 @@ func (p *daneProvider) ParseForm(r *http.Request) (sdk.CheckerOptions, error) { opts[OptionProbeTimeoutMs] = float64(n) } } + opts[OptionDNSSECValidated] = validated return opts, nil } @@ -129,25 +150,34 @@ func (p *daneProvider) RelatedProviders() []sdk.ObservationProvider { return []sdk.ObservationProvider{tls.Provider()} } -// lookupTLSA queries the system resolver for TLSA records at owner. -// Falls back to 1.1.1.1 when /etc/resolv.conf is unreadable. -func lookupTLSA(owner string) ([]*dns.TLSA, error) { +// lookupTLSA queries the configured DNSSEC-validating resolver for TLSA +// records at owner. The second return reports whether the resolver +// cryptographically validated the response (AD bit set). Callers must +// treat unvalidated answers as untrusted: a DANE "match" against +// records that lack DNSSEC protection is meaningless because an on-path +// attacker could have injected them. The records are still returned so +// the absence of validation surfaces as a check rule failure rather +// than a hard error that aborts the whole evaluation. +func lookupTLSA(ctx context.Context, owner string) ([]*dns.TLSA, bool, error) { resolver, err := interactiveResolver() if err != nil { - return nil, err + return nil, false, err } msg := new(dns.Msg) msg.SetQuestion(dns.Fqdn(owner), dns.TypeTLSA) msg.RecursionDesired = true + // AuthenticDataRequired = true asks the resolver to set AD on validated + // answers; SetEdns0 with do=true requests DNSSEC RRs. + msg.AuthenticatedData = true msg.SetEdns0(4096, true) - c := new(dns.Client) - in, _, err := c.Exchange(msg, resolver) + c := &dns.Client{Timeout: dnsClientTimeout} + in, _, err := c.ExchangeContext(ctx, msg, resolver) if err != nil { - return nil, err + return nil, false, err } if in.Rcode != dns.RcodeSuccess && in.Rcode != dns.RcodeNameError { - return nil, fmt.Errorf("rcode %s", dns.RcodeToString[in.Rcode]) + return nil, false, fmt.Errorf("rcode %s", dns.RcodeToString[in.Rcode]) } var out []*dns.TLSA for _, rr := range in.Answer { @@ -155,13 +185,27 @@ func lookupTLSA(owner string) ([]*dns.TLSA, error) { out = append(out, t) } } - return out, nil + return out, in.AuthenticatedData, nil } +// interactiveResolver returns the address of the trusted, DNSSEC-validating +// resolver this checker should use. It refuses to silently fall back to a +// public plaintext resolver: that path is a downgrade vector and would make +// every "validation" trivially spoofable on a hostile network. The operator +// must opt in by setting DANE_CHECKER_RESOLVER (e.g. "127.0.0.1:53") or +// providing an /etc/resolv.conf entry that explicitly points at a local +// validating resolver. func interactiveResolver() (string, error) { + if v := strings.TrimSpace(os.Getenv(resolverEnvVar)); v != "" { + // Accept either "host" (port defaults to 53) or "host:port". + if _, _, err := net.SplitHostPort(v); err != nil { + v = net.JoinHostPort(v, "53") + } + return v, nil + } cfg, err := dns.ClientConfigFromFile("/etc/resolv.conf") if err != nil || len(cfg.Servers) == 0 { - return net.JoinHostPort("1.1.1.1", "53"), nil + return "", fmt.Errorf("no DNSSEC-validating resolver configured: set %s to a trusted validator (e.g. 127.0.0.1:53)", resolverEnvVar) } return net.JoinHostPort(cfg.Servers[0], cfg.Port), nil } diff --git a/checker/interactive_test.go b/checker/interactive_test.go index 4f97732..496a58c 100644 --- a/checker/interactive_test.go +++ b/checker/interactive_test.go @@ -3,6 +3,7 @@ package checker import ( + "context" "encoding/json" "net/http" "net/http/httptest" @@ -26,10 +27,15 @@ func stubTLSA(owner string, usage, selector, matching uint8, cert string) *dns.T } func withStubLookup(t *testing.T, records []*dns.TLSA, err error) { + t.Helper() + withStubLookupValidated(t, records, true, err) +} + +func withStubLookupValidated(t *testing.T, records []*dns.TLSA, validated bool, err error) { t.Helper() prev := tlsaLookup - tlsaLookup = func(owner string) ([]*dns.TLSA, error) { - return records, err + tlsaLookup = func(_ context.Context, _ string) ([]*dns.TLSA, bool, error) { + return records, validated, err } t.Cleanup(func() { tlsaLookup = prev }) } diff --git a/checker/match_test.go b/checker/match_test.go new file mode 100644 index 0000000..281d203 --- /dev/null +++ b/checker/match_test.go @@ -0,0 +1,417 @@ +package checker + +import ( + "crypto/sha256" + "crypto/sha512" + "encoding/base64" + "encoding/hex" + "strings" + "testing" + + tls "git.happydns.org/checker-tls/checker" +) + +// fakeCert builds a CertInfo whose hashes are precomputed from given +// pseudo-DER and pseudo-SPKI byte slices. Real DER is unnecessary: the +// matching logic only operates on bytes/hex. +func fakeCert(der, spki []byte) tls.CertInfo { + cs256 := sha256.Sum256(der) + cs512 := sha512.Sum512(der) + ss256 := sha256.Sum256(spki) + ss512 := sha512.Sum512(spki) + return tls.CertInfo{ + DERBase64: base64.StdEncoding.EncodeToString(der), + SPKIDERBase64: base64.StdEncoding.EncodeToString(spki), + CertSHA256: hex.EncodeToString(cs256[:]), + CertSHA512: hex.EncodeToString(cs512[:]), + SPKISHA256: hex.EncodeToString(ss256[:]), + SPKISHA512: hex.EncodeToString(ss512[:]), + } +} + +func TestTLSAOwnerRegex(t *testing.T) { + t.Parallel() + cases := []struct { + in string + ok bool + port, proto, bs string + }{ + {"_443._tcp.example.com", true, "443", "tcp", "example.com"}, + {"_25._tcp.mail.example.com", true, "25", "tcp", "mail.example.com"}, + {"_853._udp", true, "853", "udp", ""}, + {"_443._sctp.example.com", false, "", "", ""}, + {"443._tcp.example.com", false, "", "", ""}, + {"_abc._tcp.example.com", false, "", "", ""}, + {"_443.tcp.example.com", false, "", "", ""}, + } + for _, tc := range cases { + m := tlsaOwner.FindStringSubmatch(tc.in) + if (m != nil) != tc.ok { + t.Errorf("%q: match=%v want=%v", tc.in, m != nil, tc.ok) + continue + } + if !tc.ok { + continue + } + if m[1] != tc.port || m[2] != tc.proto || m[3] != tc.bs { + t.Errorf("%q: got (%q,%q,%q) want (%q,%q,%q)", tc.in, m[1], m[2], m[3], tc.port, tc.proto, tc.bs) + } + } +} + +func TestTLSAOwnerName(t *testing.T) { + t.Parallel() + cases := []struct { + port uint16 + proto string + base string + want string + }{ + {443, "tcp", "example.com", "_443._tcp.example.com"}, + {25, "tcp", "mail.example.com", "_25._tcp.mail.example.com"}, + } + for _, tc := range cases { + got := tlsaOwnerName(tc.port, tc.proto, tc.base) + if got != tc.want { + t.Errorf("tlsaOwnerName(%d,%q,%q)=%q want %q", tc.port, tc.proto, tc.base, got, tc.want) + } + } + + // Empty base: trailing label is omitted so the result is still a + // syntactically valid relative name rather than "_443._tcp.". + if got := tlsaOwnerName(443, "tcp", ""); got != "_443._tcp" { + t.Errorf("empty base: got %q want %q", got, "_443._tcp") + } + if got := tlsaOwnerName(443, "tcp", "example.com."); got != "_443._tcp.example.com" { + t.Errorf("trailing dot stripped: got %q", got) + } +} + +func TestStarttlsKey(t *testing.T) { + t.Parallel() + if got := starttlsKey(25, "tcp"); got != "25/tcp" { + t.Errorf("got %q want 25/tcp", got) + } +} + +func TestJoinName(t *testing.T) { + t.Parallel() + cases := []struct { + name string + base, sub, apex string + want string + }{ + {"empty base, no sub", "", "", "example.com", "example.com"}, + {"empty base with sub", "", "mail", "example.com", "mail.example.com"}, + {"absolute base equal apex", "example.com", "", "example.com", "example.com"}, + {"absolute base ending in apex", "mail.example.com", "", "example.com", "mail.example.com"}, + {"absolute base ending in apex with sub", "host.sub.example.com", "sub", "example.com", "host.sub.example.com"}, + {"relative base with sub", "host", "sub", "example.com", "host.sub.example.com"}, + {"relative base no sub", "host", "", "example.com", "host.example.com"}, + {"trailing dot", "host.", "", "example.com", "host.example.com"}, + {"empty everything", "", "", "", ""}, + // Brittle short-apex case (the "com" apex). Pinned to current + // behaviour: HasSuffix(".com") makes "example.com" already + // fully-qualified, so it is returned unchanged. + {"short apex collision", "example.com", "", "com", "example.com"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := joinName(tc.base, tc.sub, tc.apex) + if got != tc.want { + t.Errorf("got %q want %q", got, tc.want) + } + }) + } +} + +func TestRecordCandidate_Selectors(t *testing.T) { + t.Parallel() + der := []byte("der-bytes") + spki := []byte("spki-bytes") + c := fakeCert(der, spki) + + derHex := hex.EncodeToString(der) + spkiHex := hex.EncodeToString(spki) + + cases := []struct { + name string + rec TLSARecord + want string + }{ + {"cert/full", TLSARecord{Selector: SelectorCert, MatchingType: MatchingFull}, derHex}, + {"cert/sha256", TLSARecord{Selector: SelectorCert, MatchingType: MatchingSHA256}, c.CertSHA256}, + {"cert/sha512", TLSARecord{Selector: SelectorCert, MatchingType: MatchingSHA512}, c.CertSHA512}, + {"spki/full", TLSARecord{Selector: SelectorSPKI, MatchingType: MatchingFull}, spkiHex}, + {"spki/sha256", TLSARecord{Selector: SelectorSPKI, MatchingType: MatchingSHA256}, c.SPKISHA256}, + {"spki/sha512", TLSARecord{Selector: SelectorSPKI, MatchingType: MatchingSHA512}, c.SPKISHA512}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got, err := recordCandidate(tc.rec, c) + if err != nil { + t.Fatalf("err=%v", err) + } + if got != tc.want { + t.Errorf("got %q want %q", got, tc.want) + } + }) + } +} + +func TestRecordCandidate_Errors(t *testing.T) { + t.Parallel() + c := fakeCert([]byte("d"), []byte("s")) + if _, err := recordCandidate(TLSARecord{Selector: 9, MatchingType: MatchingSHA256}, c); err == nil { + t.Error("expected error on unknown selector") + } + if _, err := recordCandidate(TLSARecord{Selector: SelectorCert, MatchingType: 9}, c); err == nil { + t.Error("expected error on unknown matching type for cert") + } + if _, err := recordCandidate(TLSARecord{Selector: SelectorSPKI, MatchingType: 9}, c); err == nil { + t.Error("expected error on unknown matching type for spki") + } + bad := tls.CertInfo{DERBase64: "!!!not-base64!!!"} + if _, err := recordCandidate(TLSARecord{Selector: SelectorCert, MatchingType: MatchingFull}, bad); err == nil { + t.Error("expected base64 decode error") + } +} + +func TestDecodeFullDER_SizeLimit(t *testing.T) { + t.Parallel() + huge := strings.Repeat("A", maxFullDERBytes+10) // base64; decoded is ~3/4 of len + if _, err := decodeFullDER(huge, "test"); err == nil { + t.Error("expected size-limit error") + } + small := base64.StdEncoding.EncodeToString([]byte("hello")) + got, err := decodeFullDER(small, "test") + if err != nil { + t.Fatalf("err=%v", err) + } + if string(got) != "hello" { + t.Errorf("got %q want hello", got) + } +} + +func TestMatchRecord_LeafSelectsByUsage(t *testing.T) { + t.Parallel() + leaf := fakeCert([]byte("leaf-der"), []byte("leaf-spki")) + mid := fakeCert([]byte("mid-der"), []byte("mid-spki")) + probe := &tls.TLSProbe{Chain: []tls.CertInfo{leaf, mid}} + + // usage 3 (DANE-EE) matches leaf SHA-256 SPKI + rec := TLSARecord{Usage: UsageDANEEE, Selector: SelectorSPKI, MatchingType: MatchingSHA256, Certificate: leaf.SPKISHA256} + if ok, why := matchRecord(rec, probe); !ok { + t.Errorf("DANE-EE leaf SPKI sha256: ok=false reason=%q", why) + } + // usage 3 with intermediate hash should NOT match (wrong slot) + rec.Certificate = mid.SPKISHA256 + if ok, _ := matchRecord(rec, probe); ok { + t.Error("DANE-EE matching against intermediate SPKI should fail") + } + + // usage 2 (DANE-TA) matches intermediate + rec = TLSARecord{Usage: UsageDANETA, Selector: SelectorCert, MatchingType: MatchingSHA256, Certificate: mid.CertSHA256} + if ok, why := matchRecord(rec, probe); !ok { + t.Errorf("DANE-TA intermediate cert sha256: ok=false reason=%q", why) + } + + // usage 1 (PKIX-EE) matches leaf cert hash + rec = TLSARecord{Usage: UsagePKIXEE, Selector: SelectorCert, MatchingType: MatchingSHA256, Certificate: leaf.CertSHA256} + if ok, why := matchRecord(rec, probe); !ok { + t.Errorf("PKIX-EE leaf cert sha256: ok=false reason=%q", why) + } + + // usage 0 (PKIX-TA) matches intermediate + rec = TLSARecord{Usage: UsagePKIXTA, Selector: SelectorSPKI, MatchingType: MatchingSHA256, Certificate: mid.SPKISHA256} + if ok, why := matchRecord(rec, probe); !ok { + t.Errorf("PKIX-TA intermediate spki sha256: ok=false reason=%q", why) + } +} + +func TestMatchRecord_NoChain(t *testing.T) { + t.Parallel() + if ok, why := matchRecord(TLSARecord{Usage: UsageDANEEE}, &tls.TLSProbe{}); ok || why == "" { + t.Errorf("empty chain: ok=%v reason=%q", ok, why) + } +} + +func TestMatchRecord_TASelfSignedFallback(t *testing.T) { + t.Parallel() + // When the chain has only a leaf, usage 0/2 falls back to matching the + // leaf as a degenerate TA so the user gets feedback. + leaf := fakeCert([]byte("leaf"), []byte("spki")) + probe := &tls.TLSProbe{Chain: []tls.CertInfo{leaf}} + rec := TLSARecord{Usage: UsageDANETA, Selector: SelectorSPKI, MatchingType: MatchingSHA256, Certificate: leaf.SPKISHA256} + if ok, why := matchRecord(rec, probe); !ok { + t.Errorf("self-signed TA fallback: ok=false reason=%q", why) + } +} + +func TestMatchRecord_UnsupportedUsage(t *testing.T) { + t.Parallel() + leaf := fakeCert([]byte("leaf"), []byte("spki")) + probe := &tls.TLSProbe{Chain: []tls.CertInfo{leaf}} + if ok, why := matchRecord(TLSARecord{Usage: 9}, probe); ok || !strings.Contains(why, "unsupported") { + t.Errorf("usage 9: ok=%v reason=%q", ok, why) + } +} + +func TestMatchRecord_FullDER(t *testing.T) { + t.Parallel() + der := []byte("the-actual-cert-der") + leaf := fakeCert(der, []byte("ignored")) + probe := &tls.TLSProbe{Chain: []tls.CertInfo{leaf}} + rec := TLSARecord{ + Usage: UsageDANEEE, + Selector: SelectorCert, + MatchingType: MatchingFull, + Certificate: hex.EncodeToString(der), + } + if ok, why := matchRecord(rec, probe); !ok { + t.Errorf("Full DER match failed: %q", why) + } +} + +func TestSummarizeMatches(t *testing.T) { + t.Parallel() + leaf := fakeCert([]byte("leaf"), []byte("ls")) + probe := &tls.TLSProbe{Chain: []tls.CertInfo{leaf}} + t1 := TargetResult{Records: []TLSARecord{ + {Usage: UsageDANEEE, Selector: SelectorSPKI, MatchingType: MatchingSHA256, Certificate: leaf.SPKISHA256}, // ok + {Usage: UsageDANEEE, Selector: SelectorSPKI, MatchingType: MatchingSHA256, Certificate: "deadbeef"}, // miss + {Usage: UsageDANEEE, Selector: SelectorCert, MatchingType: MatchingSHA256, Certificate: leaf.CertSHA256}, // ok + }} + s := summarizeMatches(t1, probe) + if s.matched != 2 || s.unmatched != 1 || s.firstUnmatchedIdx != 1 { + t.Errorf("got matched=%d unmatched=%d firstIdx=%d", s.matched, s.unmatched, s.firstUnmatchedIdx) + } + + if got := summarizeMatches(t1, nil); got.matched != 0 || got.firstUnmatchedIdx != -1 { + t.Errorf("nil probe: %+v", got) + } +} + +func TestSummarizeMatches_BadFirstSlotDoesNotAbort(t *testing.T) { + t.Parallel() + // An undecodable Full record at slot 0 shouldn't prevent later valid + // records from matching: regression test for the per-slot continue. + leaf := fakeCert([]byte("leaf"), []byte("spki")) + probe := &tls.TLSProbe{Chain: []tls.CertInfo{leaf}} + bad := TargetResult{Records: []TLSARecord{ + {Usage: UsageDANEEE, Selector: SelectorCert, MatchingType: MatchingFull, Certificate: "00"}, // hex won't match decoded DER + {Usage: UsageDANEEE, Selector: SelectorSPKI, MatchingType: MatchingSHA256, Certificate: leaf.SPKISHA256}, + }} + s := summarizeMatches(bad, probe) + if s.matched != 1 { + t.Errorf("expected 1 match (the second record), got %d (unmatched=%d)", s.matched, s.unmatched) + } +} + +func TestHasPKIXUsage(t *testing.T) { + t.Parallel() + if hasPKIXUsage(TargetResult{Records: []TLSARecord{{Usage: UsageDANEEE}}}) { + t.Error("DANE-EE only: expected false") + } + if !hasPKIXUsage(TargetResult{Records: []TLSARecord{{Usage: UsagePKIXEE}}}) { + t.Error("PKIX-EE: expected true") + } + if !hasPKIXUsage(TargetResult{Records: []TLSARecord{{Usage: UsageDANETA}, {Usage: UsagePKIXTA}}}) { + t.Error("contains PKIX-TA: expected true") + } + if hasPKIXUsage(TargetResult{}) { + t.Error("empty: expected false") + } +} + +func TestSuspiciousUsage(t *testing.T) { + t.Parallel() + leaf := fakeCert([]byte("leaf"), []byte("ls")) + mid := fakeCert([]byte("mid"), []byte("ms")) + probe := &tls.TLSProbe{Chain: []tls.CertInfo{leaf, mid}} + + // Record declared as EE but hash matches intermediate => suspicious. + tgt := TargetResult{Records: []TLSARecord{{ + Usage: UsageDANEEE, Selector: SelectorSPKI, MatchingType: MatchingSHA256, + Certificate: mid.SPKISHA256, + }}} + if got := suspiciousUsage(tgt, probe); got == "" { + t.Error("expected suspicious-usage warning") + } + + // Record declared as EE matching the leaf is fine. + tgt.Records[0].Certificate = leaf.SPKISHA256 + if got := suspiciousUsage(tgt, probe); got != "" { + t.Errorf("unexpected warning: %q", got) + } + + // Single-cert chain: rule is silent. + if got := suspiciousUsage(tgt, &tls.TLSProbe{Chain: []tls.CertInfo{leaf}}); got != "" { + t.Errorf("single-cert chain should be silent, got %q", got) + } +} + +func TestProposedTLSA(t *testing.T) { + t.Parallel() + leaf := fakeCert([]byte("leaf"), []byte("spki")) + probe := &tls.TLSProbe{Chain: []tls.CertInfo{leaf}} + + // No record published: defaults to 3 1 1. + t1 := TargetResult{Owner: "_443._tcp.example.com", Records: nil} + got := proposedTLSA(t1, probe) + if !strings.Contains(got, "TLSA 3 1 1 ") || !strings.Contains(got, leaf.SPKISHA256) { + t.Errorf("default proposal: %q", got) + } + + // Existing record uses Full → suggestion downgrades to SHA-256. + t2 := TargetResult{Owner: "_443._tcp.example.com", Records: []TLSARecord{{Usage: UsageDANEEE, Selector: SelectorCert, MatchingType: MatchingFull}}} + got = proposedTLSA(t2, probe) + if !strings.Contains(got, "TLSA 3 0 1 ") { + t.Errorf("Full→SHA256 collapse: %q", got) + } + + // No probe: empty. + if got := proposedTLSA(t1, nil); got != "" { + t.Errorf("no probe: got %q", got) + } +} + +func TestHandshakeFix(t *testing.T) { + t.Parallel() + got := handshakeFix(TargetResult{Host: "mail.example.com", Port: 25, STARTTLS: "smtp"}) + if !strings.Contains(got, "-starttls smtp") || !strings.Contains(got, "-connect mail.example.com:25") { + t.Errorf("smtp fix: %q", got) + } + got = handshakeFix(TargetResult{Host: "example.com", Port: 443}) + if strings.Contains(got, "-starttls") || !strings.Contains(got, "-connect example.com:443") { + t.Errorf("direct fix: %q", got) + } +} + +func TestTruncHex(t *testing.T) { + t.Parallel() + if truncHex("abc") != "abc" { + t.Error("short") + } + long := strings.Repeat("a", 20) + if got := truncHex(long); got != "aaaaaaaaaaaa…" { + t.Errorf("long: %q", got) + } +} + +func TestProbeUsable(t *testing.T) { + t.Parallel() + leaf := fakeCert([]byte("l"), []byte("s")) + if probeUsable(nil) { + t.Error("nil") + } + if probeUsable(&tls.TLSProbe{}) { + t.Error("empty chain") + } + if probeUsable(&tls.TLSProbe{Chain: []tls.CertInfo{leaf}, Error: "boom"}) { + t.Error("error set") + } + if !probeUsable(&tls.TLSProbe{Chain: []tls.CertInfo{leaf}}) { + t.Error("good probe") + } +} diff --git a/checker/report.go b/checker/report.go index 27efbd6..675388e 100644 --- a/checker/report.go +++ b/checker/report.go @@ -177,15 +177,39 @@ func hasPKIXUsage(t TargetResult) bool { return false } -// proposedTLSA renders a ready-to-paste replacement RR using the most common -// DANE-EE + SPKI + SHA-256 triplet computed from the live leaf. This is the -// profile Let's Encrypt users are pushed towards because it survives any -// cert rotation that keeps the same key pair. +// proposedTLSA renders a ready-to-paste replacement RR computed from the +// live chain. The (usage, selector, matching) triplet is taken from the +// user's first existing record so the suggestion stays consistent with +// their published profile (e.g. a deployment standardised on usage 2 keeps +// usage 2). When no record is published yet, fall back to the DANE-EE + +// SPKI + SHA-256 triplet most Let's Encrypt deployers settle on. func proposedTLSA(t TargetResult, p *tls.TLSProbe) string { if p == nil || len(p.Chain) == 0 { return "" } - return fmt.Sprintf("%s IN TLSA 3 1 1 %s", t.Owner, p.Chain[0].SPKISHA256) + tmpl := TLSARecord{Usage: UsageDANEEE, Selector: SelectorSPKI, MatchingType: MatchingSHA256} + if len(t.Records) > 0 { + r := t.Records[0] + tmpl.Usage = r.Usage + tmpl.Selector = r.Selector + tmpl.MatchingType = r.MatchingType + // Suggesting Full (matching type 0) inline as a zone fragment is + // not useful: collapse to SHA-256 of the same selector, which is + // what operators publish in practice. + if tmpl.MatchingType == MatchingFull { + tmpl.MatchingType = MatchingSHA256 + } + } + + slot := p.Chain[0] + if (tmpl.Usage == UsagePKIXTA || tmpl.Usage == UsageDANETA) && len(p.Chain) > 1 { + slot = p.Chain[1] + } + hex, err := recordCandidate(tmpl, slot) + if err != nil || hex == "" { + return "" + } + return fmt.Sprintf("%s IN TLSA %d %d %d %s", t.Owner, tmpl.Usage, tmpl.Selector, tmpl.MatchingType, hex) } // handshakeFix proposes a STARTTLS-aware first step when the probe failed. diff --git a/checker/rule.go b/checker/rule.go index f2078ba..c4d63a4 100644 --- a/checker/rule.go +++ b/checker/rule.go @@ -18,6 +18,7 @@ import ( func Rules() []sdk.CheckRule { return []sdk.CheckRule{ &hasRecordsRule{}, + &dnssecValidatedRule{}, &probeAvailableRule{}, &handshakeOKRule{}, &recordsMatchChainRule{}, @@ -31,10 +32,10 @@ func Rules() []sdk.CheckRule { type ruleContext struct { data DANEData probes map[string]*tls.TLSProbe - // warn is a non-fatal issue encountered while loading related probes - // (e.g. the cross-checker lineage was unreachable). Rules surface it - // as an error state so operators can spot misconfiguration. - warn string + // relatedErr is a non-fatal error encountered while loading related + // probes (e.g. the cross-checker lineage was unreachable). Rules + // surface it as an error state so operators can spot misconfiguration. + relatedErr error // err is a fatal error loading the checker's own observation. err error } @@ -47,7 +48,7 @@ func loadRuleContext(ctx context.Context, obs sdk.ObservationGetter) *ruleContex rc.err = err return rc } - rc.probes, rc.warn = relatedTLSProbes(ctx, obs) + rc.probes, rc.relatedErr = relatedTLSProbes(ctx, obs) return rc } @@ -148,18 +149,49 @@ func matchRecord(rec TLSARecord, p *tls.TLSProbe) (bool, string) { return false, fmt.Sprintf("unsupported TLSA usage %d", rec.Usage) } + var lastErr string for _, c := range slots { got, err := recordCandidate(rec, c) if err != nil { - return false, err.Error() + lastErr = err.Error() + continue } if strings.EqualFold(got, rec.Certificate) { return true, "" } } + if lastErr != "" { + return false, lastErr + } return false, fmt.Sprintf("expected %s, got none matching in chain", truncHex(rec.Certificate)) } +// maxFullDERBytes caps the size of a "Full" (MatchingType 0) DER payload +// that this checker is willing to base64-decode and hex-encode. Real X.509 +// certificates rarely exceed 8 KiB; 64 KiB leaves comfortable headroom for +// pathological-but-legitimate chains while preventing a hostile probe +// payload from forcing arbitrary heap allocations during evaluation. +const maxFullDERBytes = 64 * 1024 + +// decodeFullDER base64-decodes b after rejecting payloads whose decoded size +// would exceed maxFullDERBytes, so an attacker-controlled probe cannot make +// the rule allocate unbounded memory before the hex comparison. +func decodeFullDER(b string, what string) ([]byte, error) { + // base64 decoded length is at most ceil(len(b)/4)*3; bail out cheaply + // before allocating the destination buffer. + if len(b)/4*3 > maxFullDERBytes { + return nil, fmt.Errorf("%s exceeds %d bytes", what, maxFullDERBytes) + } + der, err := base64.StdEncoding.DecodeString(b) + if err != nil { + return nil, fmt.Errorf("decode %s: %w", what, err) + } + if len(der) > maxFullDERBytes { + return nil, fmt.Errorf("%s exceeds %d bytes", what, maxFullDERBytes) + } + return der, nil +} + // recordCandidate returns the hex value the TLSA record should match for // the (selector, matching_type) pair against this certificate slot. For // matching_type 0 (Full), both sides are compared as hex-encoded DER. @@ -169,9 +201,9 @@ func recordCandidate(rec TLSARecord, c tls.CertInfo) (string, error) { case SelectorCert: switch rec.MatchingType { case MatchingFull: - der, err := base64.StdEncoding.DecodeString(c.DERBase64) + der, err := decodeFullDER(c.DERBase64, "cert DER") if err != nil { - return "", fmt.Errorf("decode cert DER: %w", err) + return "", err } source = hex.EncodeToString(der) case MatchingSHA256: @@ -184,9 +216,9 @@ func recordCandidate(rec TLSARecord, c tls.CertInfo) (string, error) { case SelectorSPKI: switch rec.MatchingType { case MatchingFull: - spki, err := base64.StdEncoding.DecodeString(c.SPKIDERBase64) + spki, err := decodeFullDER(c.SPKIDERBase64, "SPKI DER") if err != nil { - return "", fmt.Errorf("decode SPKI DER: %w", err) + return "", err } source = hex.EncodeToString(spki) case MatchingSHA256: @@ -215,12 +247,12 @@ func parseTLSProbeMap(data []byte) map[string]tls.TLSProbe { } // relatedTLSProbes indexes TLS probes fetched via GetRelated by endpoint Ref. -func relatedTLSProbes(ctx context.Context, obs sdk.ObservationGetter) (map[string]*tls.TLSProbe, string) { +func relatedTLSProbes(ctx context.Context, obs sdk.ObservationGetter) (map[string]*tls.TLSProbe, error) { related, err := obs.GetRelated(ctx, tls.ObservationKeyTLSProbes) if err != nil { - return nil, "related TLS probes unavailable: " + err.Error() + return nil, fmt.Errorf("related TLS probes unavailable: %w", err) } - return indexProbes(related), "" + return indexProbes(related), nil } // indexProbes flattens a slice of related TLS-probe observations into a probe diff --git a/checker/rules_probe.go b/checker/rules_probe.go index c9b322c..4e35333 100644 --- a/checker/rules_probe.go +++ b/checker/rules_probe.go @@ -21,16 +21,16 @@ func (r *probeAvailableRule) Evaluate(ctx context.Context, obs sdk.ObservationGe if rc.err != nil { return []sdk.CheckState{observationErrorState(rc.err)} } - if rc.warn != "" { + if rc.relatedErr != nil { return []sdk.CheckState{{ Status: sdk.StatusError, Code: "dane_observation_warning", - Message: rc.warn, + Message: rc.relatedErr.Error(), }} } if len(rc.data.Targets) == 0 { return []sdk.CheckState{{ - Status: sdk.StatusOK, + Status: sdk.StatusUnknown, Code: "dane_probe_available_skipped", Message: "No DANE endpoints to probe.", }} diff --git a/checker/rules_records.go b/checker/rules_records.go index d84c039..0a4ba39 100644 --- a/checker/rules_records.go +++ b/checker/rules_records.go @@ -2,6 +2,8 @@ package checker import ( "context" + "fmt" + "strings" sdk "git.happydns.org/checker-sdk-go/checker" ) @@ -20,17 +22,78 @@ func (r *hasRecordsRule) Evaluate(ctx context.Context, obs sdk.ObservationGetter if rc.err != nil { return []sdk.CheckState{observationErrorState(rc.err)} } + var states []sdk.CheckState + for _, inv := range rc.data.Invalid { + states = append(states, sdk.CheckState{ + Status: sdk.StatusError, + Code: "dane_invalid_owner", + Subject: inv.Owner, + Message: fmt.Sprintf("TLSA record %q is unusable: %s", inv.Owner, inv.Reason), + Meta: map[string]any{"owner": inv.Owner, "reason": inv.Reason}, + }) + } if len(rc.data.Targets) == 0 { + if len(states) > 0 { + // Records exist but none are usable; flag the aggregate too so + // the UI doesn't only show per-record errors. + owners := make([]string, 0, len(rc.data.Invalid)) + for _, inv := range rc.data.Invalid { + owners = append(owners, inv.Owner) + } + states = append(states, sdk.CheckState{ + Status: sdk.StatusError, + Code: "dane_no_usable_records", + Message: fmt.Sprintf("No usable TLSA records (all %d declared records are malformed: %s).", len(rc.data.Invalid), strings.Join(owners, ", ")), + }) + return states + } return []sdk.CheckState{{ Status: sdk.StatusUnknown, Code: "dane_no_records", Message: "No TLSA records declared on this service.", }} } - return []sdk.CheckState{{ + states = append(states, sdk.CheckState{ Status: sdk.StatusOK, Code: "dane_has_records_ok", Message: "TLSA records are declared for all bound endpoints.", Meta: map[string]any{"endpoints": len(rc.data.Targets)}, + }) + return states +} + +// dnssecValidatedRule reports whether the TLSA records this checker is +// evaluating were fetched over a DNSSEC-validated path. Without DNSSEC, +// DANE is a downgrade primitive: an on-path attacker can forge TLSA +// answers and any "match" the rest of the rules report is meaningless. +// The rule only emits when the collector recorded a validation status: +// in managed mode the records come from the user's authoritative zone +// config and DNSSEC posture is checked by a different checker. +type dnssecValidatedRule struct{} + +func (r *dnssecValidatedRule) Name() string { return "dane.dnssec_validated" } +func (r *dnssecValidatedRule) Description() string { + return "Verifies the TLSA records were fetched via a DNSSEC-validating resolver (AD bit set)." +} + +func (r *dnssecValidatedRule) Evaluate(ctx context.Context, obs sdk.ObservationGetter, _ sdk.CheckerOptions) []sdk.CheckState { + rc := loadRuleContext(ctx, obs) + if rc.err != nil { + return []sdk.CheckState{observationErrorState(rc.err)} + } + if rc.data.DNSSECValidated == nil { + return nil + } + if *rc.data.DNSSECValidated { + return []sdk.CheckState{{ + Status: sdk.StatusOK, + Code: "dane_dnssec_validated", + Message: "TLSA records were fetched over a DNSSEC-validated path (AD bit set).", + }} + } + return []sdk.CheckState{{ + Status: sdk.StatusError, + Code: "dane_dnssec_unvalidated", + Message: "TLSA records were fetched without DNSSEC validation (resolver did not set the AD bit). DANE matches are not trustworthy without DNSSEC.", }} } diff --git a/checker/rules_test.go b/checker/rules_test.go new file mode 100644 index 0000000..4e05b37 --- /dev/null +++ b/checker/rules_test.go @@ -0,0 +1,276 @@ +package checker + +import ( + "context" + "encoding/json" + "errors" + "testing" + + sdk "git.happydns.org/checker-sdk-go/checker" + tls "git.happydns.org/checker-tls/checker" + tlscontract "git.happydns.org/checker-tls/contract" +) + +// mockObs is a lightweight ObservationGetter for rule unit tests. +type mockObs struct { + dane *DANEData + daneErr error + probes map[string]tls.TLSProbe + relatedErr error +} + +func (m *mockObs) Get(_ context.Context, key sdk.ObservationKey, dest any) error { + if m.daneErr != nil { + return m.daneErr + } + if key != ObservationKeyDANE || m.dane == nil { + return errors.New("not found") + } + b, err := json.Marshal(m.dane) + if err != nil { + return err + } + return json.Unmarshal(b, dest) +} + +func (m *mockObs) GetRelated(_ context.Context, key sdk.ObservationKey) ([]sdk.RelatedObservation, error) { + if m.relatedErr != nil { + return nil, m.relatedErr + } + if key != tls.ObservationKeyTLSProbes || m.probes == nil { + return nil, nil + } + payload := struct { + Probes map[string]tls.TLSProbe `json:"probes"` + }{Probes: m.probes} + b, _ := json.Marshal(payload) + return []sdk.RelatedObservation{{ + CheckerID: "tls", + Key: tls.ObservationKeyTLSProbes, + Data: b, + }}, nil +} + +func makeTarget(host string, port uint16, recs []TLSARecord) TargetResult { + t := TargetResult{ + Owner: tlsaOwnerName(port, "tcp", host), + Host: host, + Port: port, + Proto: "tcp", + Records: recs, + } + t.Ref = tlscontract.Ref(tlscontract.TLSEndpoint{Host: host, Port: port, SNI: host}) + return t +} + +func TestHasRecordsRule(t *testing.T) { + t.Parallel() + r := &hasRecordsRule{} + + // No records, no invalid → unknown + obs := &mockObs{dane: &DANEData{}} + st := r.Evaluate(context.Background(), obs, nil) + if len(st) != 1 || st[0].Code != "dane_no_records" { + t.Errorf("no records: %+v", st) + } + + // Records present → ok + obs = &mockObs{dane: &DANEData{Targets: []TargetResult{makeTarget("a.example.com", 443, []TLSARecord{{}})}}} + st = r.Evaluate(context.Background(), obs, nil) + if len(st) != 1 || st[0].Code != "dane_has_records_ok" { + t.Errorf("ok: %+v", st) + } + + // Invalid records, no targets → error states + obs = &mockObs{dane: &DANEData{Invalid: []InvalidRecord{{Owner: "_x._tcp", Reason: "bad port"}}}} + st = r.Evaluate(context.Background(), obs, nil) + if len(st) < 2 { + t.Fatalf("expected per-record + aggregate, got %+v", st) + } + if st[0].Code != "dane_invalid_owner" || st[len(st)-1].Code != "dane_no_usable_records" { + t.Errorf("invalid only: %+v", st) + } + + // Observation read error + obs = &mockObs{daneErr: errors.New("boom")} + st = r.Evaluate(context.Background(), obs, nil) + if len(st) != 1 || st[0].Code != "dane_observation_error" { + t.Errorf("err: %+v", st) + } +} + +func TestProbeAvailableRule(t *testing.T) { + t.Parallel() + r := &probeAvailableRule{} + tgt := makeTarget("a.example.com", 443, []TLSARecord{{Usage: UsageDANEEE}}) + + // Probe present + leaf := fakeCert([]byte("l"), []byte("s")) + obs := &mockObs{ + dane: &DANEData{Targets: []TargetResult{tgt}}, + probes: map[string]tls.TLSProbe{tgt.Ref: {Chain: []tls.CertInfo{leaf}}}, + } + st := r.Evaluate(context.Background(), obs, nil) + if len(st) != 1 || st[0].Code != "dane_probe_available_ok" { + t.Errorf("ok: %+v", st) + } + + // Probe absent + obs.probes = map[string]tls.TLSProbe{} + st = r.Evaluate(context.Background(), obs, nil) + if len(st) != 1 || st[0].Code != "dane_no_probe" { + t.Errorf("missing: %+v", st) + } + + // No targets at all + obs = &mockObs{dane: &DANEData{}} + st = r.Evaluate(context.Background(), obs, nil) + if len(st) != 1 || st[0].Code != "dane_probe_available_skipped" { + t.Errorf("empty: %+v", st) + } + + // Related-fetch error surfaces as warning state. + obs = &mockObs{dane: &DANEData{Targets: []TargetResult{tgt}}, relatedErr: errors.New("upstream down")} + st = r.Evaluate(context.Background(), obs, nil) + if len(st) != 1 || st[0].Code != "dane_observation_warning" { + t.Errorf("relatedErr: %+v", st) + } +} + +func TestHandshakeOKRule(t *testing.T) { + t.Parallel() + r := &handshakeOKRule{} + tgt := makeTarget("a.example.com", 443, []TLSARecord{{Usage: UsageDANEEE}}) + leaf := fakeCert([]byte("l"), []byte("s")) + + // All good. + obs := &mockObs{ + dane: &DANEData{Targets: []TargetResult{tgt}}, + probes: map[string]tls.TLSProbe{tgt.Ref: {Chain: []tls.CertInfo{leaf}}}, + } + st := r.Evaluate(context.Background(), obs, nil) + if len(st) != 1 || st[0].Code != "dane_handshake_ok" { + t.Errorf("ok: %+v", st) + } + + // Handshake failed. + obs.probes = map[string]tls.TLSProbe{tgt.Ref: {Error: "tls: bad cert"}} + st = r.Evaluate(context.Background(), obs, nil) + if len(st) != 1 || st[0].Code != "dane_handshake_failed" { + t.Errorf("failed: %+v", st) + } +} + +func TestRecordsMatchChainRule(t *testing.T) { + t.Parallel() + r := &recordsMatchChainRule{} + leaf := fakeCert([]byte("leaf"), []byte("ls")) + + tgt := makeTarget("a.example.com", 443, []TLSARecord{ + {Usage: UsageDANEEE, Selector: SelectorSPKI, MatchingType: MatchingSHA256, Certificate: leaf.SPKISHA256}, + }) + obs := &mockObs{ + dane: &DANEData{Targets: []TargetResult{tgt}}, + probes: map[string]tls.TLSProbe{tgt.Ref: {Chain: []tls.CertInfo{leaf}}}, + } + st := r.Evaluate(context.Background(), obs, nil) + if len(st) != 1 || st[0].Code != "dane_match_ok" { + t.Errorf("match ok: %+v", st) + } + + // Same target, wrong cert hash → no match (crit). + tgt.Records[0].Certificate = "deadbeef" + obs.dane = &DANEData{Targets: []TargetResult{tgt}} + st = r.Evaluate(context.Background(), obs, nil) + if len(st) != 1 || st[0].Code != "dane_no_match" { + t.Errorf("no match: %+v", st) + } + + // No probe usable → skipped. + obs.probes = map[string]tls.TLSProbe{} + st = r.Evaluate(context.Background(), obs, nil) + if len(st) != 1 || st[0].Code != "dane_records_match_chain_skipped" { + t.Errorf("skipped: %+v", st) + } +} + +func TestPKIXChainValidRule(t *testing.T) { + t.Parallel() + r := &pkixChainValidRule{} + leaf := fakeCert([]byte("l"), []byte("s")) + bTrue, bFalse := true, false + + // PKIX usage + valid chain → ok. + tgt := makeTarget("a.example.com", 443, []TLSARecord{{Usage: UsagePKIXEE}}) + obs := &mockObs{ + dane: &DANEData{Targets: []TargetResult{tgt}}, + probes: map[string]tls.TLSProbe{tgt.Ref: {Chain: []tls.CertInfo{leaf}, ChainValid: &bTrue}}, + } + st := r.Evaluate(context.Background(), obs, nil) + if len(st) != 1 || st[0].Code != "dane_pkix_chain_valid_ok" { + t.Errorf("ok: %+v", st) + } + + // PKIX usage + invalid chain → crit. + obs.probes = map[string]tls.TLSProbe{tgt.Ref: {Chain: []tls.CertInfo{leaf}, ChainValid: &bFalse}} + st = r.Evaluate(context.Background(), obs, nil) + if len(st) != 1 || st[0].Code != "dane_pkix_chain_invalid" { + t.Errorf("invalid: %+v", st) + } + + // DANE-only usages → skipped (rule does not apply). + tgt.Records = []TLSARecord{{Usage: UsageDANEEE}} + obs.dane = &DANEData{Targets: []TargetResult{tgt}} + obs.probes = map[string]tls.TLSProbe{tgt.Ref: {Chain: []tls.CertInfo{leaf}}} + st = r.Evaluate(context.Background(), obs, nil) + if len(st) != 1 || st[0].Code != "dane_pkix_chain_valid_skipped" { + t.Errorf("skipped: %+v", st) + } +} + +func TestUsageCoherentRule(t *testing.T) { + t.Parallel() + r := &usageCoherentRule{} + leaf := fakeCert([]byte("l"), []byte("ls")) + mid := fakeCert([]byte("m"), []byte("ms")) + + // EE record whose hash matches the intermediate → warn. + tgt := makeTarget("a.example.com", 443, []TLSARecord{{ + Usage: UsageDANEEE, Selector: SelectorSPKI, MatchingType: MatchingSHA256, + Certificate: mid.SPKISHA256, + }}) + obs := &mockObs{ + dane: &DANEData{Targets: []TargetResult{tgt}}, + probes: map[string]tls.TLSProbe{tgt.Ref: {Chain: []tls.CertInfo{leaf, mid}}}, + } + st := r.Evaluate(context.Background(), obs, nil) + if len(st) != 1 || st[0].Code != "dane_usage_incoherent" { + t.Errorf("incoherent: %+v", st) + } + + // EE matching leaf → ok. + tgt.Records[0].Certificate = leaf.SPKISHA256 + obs.dane = &DANEData{Targets: []TargetResult{tgt}} + st = r.Evaluate(context.Background(), obs, nil) + if len(st) != 1 || st[0].Code != "dane_usage_coherent_ok" { + t.Errorf("coherent ok: %+v", st) + } + + // Single-cert chain → skipped. + obs.probes = map[string]tls.TLSProbe{tgt.Ref: {Chain: []tls.CertInfo{leaf}}} + st = r.Evaluate(context.Background(), obs, nil) + if len(st) != 1 || st[0].Code != "dane_usage_coherent_skipped" { + t.Errorf("skipped: %+v", st) + } +} + +func TestRules_ObservationError(t *testing.T) { + t.Parallel() + obs := &mockObs{daneErr: errors.New("read failed")} + for _, rule := range Rules() { + st := rule.Evaluate(context.Background(), obs, nil) + if len(st) == 0 || st[0].Code != "dane_observation_error" { + t.Errorf("%s: expected observation_error, got %+v", rule.Name(), st) + } + } +} diff --git a/checker/types.go b/checker/types.go index 75bfcbc..2190b2b 100644 --- a/checker/types.go +++ b/checker/types.go @@ -45,6 +45,12 @@ const ( // "/" → RFC 6335 service name (e.g. "25/tcp" → "smtp", // "587/tcp" → "submission"). Common ports auto-map via a built-in table. OptionSTARTTLS = "starttls" + + // OptionDNSSECValidated reports whether the TLSA records the host + // submitted to this checker came from a DNSSEC-validated lookup. + // Only set by the standalone interactive flow; absent in managed mode + // where TLSA records come from the user's authoritative zone config. + OptionDNSSECValidated = "dnssec_validated" ) // Severity constants mirror checker-tls. @@ -73,8 +79,24 @@ const ( type DANEData struct { // Targets is one entry per (port, proto, basename) triplet extracted // from the TLSAs service. - Targets []TargetResult `json:"targets"` - CollectedAt time.Time `json:"collected_at"` + Targets []TargetResult `json:"targets"` + // Invalid lists TLSA records that could not be parsed into a usable + // endpoint (malformed owner name, out-of-range port, etc.). They are + // surfaced by hasRecordsRule so a misconfigured zone fails loudly + // instead of silently passing as "no records". + Invalid []InvalidRecord `json:"invalid,omitempty"` + // DNSSECValidated reflects whether the resolver that fetched the TLSA + // records set the AD bit. Only populated by the standalone interactive + // flow (lookupTLSA); nil in managed mode where records come from the + // user's zone config and DNSSEC posture is checked elsewhere. + DNSSECValidated *bool `json:"dnssec_validated,omitempty"` + CollectedAt time.Time `json:"collected_at"` +} + +// InvalidRecord describes a TLSA record dropped during Collect. +type InvalidRecord struct { + Owner string `json:"owner"` + Reason string `json:"reason"` } // TargetResult groups all TLSA records declared on a single endpoint and diff --git a/go.mod b/go.mod index 8ab349c..329a14e 100644 --- a/go.mod +++ b/go.mod @@ -3,8 +3,8 @@ module git.happydns.org/checker-dane go 1.25.0 require ( - git.happydns.org/checker-sdk-go v1.3.0 - git.happydns.org/checker-tls v0.3.0 + git.happydns.org/checker-sdk-go v1.5.0 + git.happydns.org/checker-tls v0.6.2 github.com/miekg/dns v1.1.72 ) diff --git a/go.sum b/go.sum index 902a866..d4ff867 100644 --- a/go.sum +++ b/go.sum @@ -1,7 +1,7 @@ -git.happydns.org/checker-sdk-go v1.3.0 h1:FG2kIhlJCzI0m35EhxSgn4UWc9M4ha6aZTeoChu4l7A= -git.happydns.org/checker-sdk-go v1.3.0/go.mod h1:aNAcfYFfbhvH9kJhE0Njp5GX0dQbxdRB0rJ0KvSC5nI= -git.happydns.org/checker-tls v0.3.0 h1:hg9fNtT5l/0UJAuUakOwCuT1MFiyp4lDFaZKpgdzPoo= -git.happydns.org/checker-tls v0.3.0/go.mod h1:0ZSG0CTP007SHBPE7qInESVIOcW+xgucHUhHgj6MeZ8= +git.happydns.org/checker-sdk-go v1.5.0 h1:5uD5Cm6xJ+lwnhbJ09iCXGHbYS9zRh+Yh0NeBHkAPBY= +git.happydns.org/checker-sdk-go v1.5.0/go.mod h1:aNAcfYFfbhvH9kJhE0Njp5GX0dQbxdRB0rJ0KvSC5nI= +git.happydns.org/checker-tls v0.6.2 h1:8oKia1XlD+tklyqrwzmUgFH1Kw8VLSLLF9suZ7Qr14E= +git.happydns.org/checker-tls v0.6.2/go.mod h1:9tpnxg0iOwS+7If64DRG1jqYonUAgxOBuxwfF5mVkL4= github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/miekg/dns v1.1.72 h1:vhmr+TF2A3tuoGNkLDFK9zi36F2LS+hKTRW0Uf8kbzI= diff --git a/plugin/plugin.go b/plugin/plugin.go index be0e771..71afa4f 100644 --- a/plugin/plugin.go +++ b/plugin/plugin.go @@ -11,5 +11,6 @@ var Version = "custom-build" func NewCheckerPlugin() (*sdk.CheckerDefinition, sdk.ObservationProvider, error) { dane.Version = Version - return dane.Definition(), dane.Provider(), nil + prvd := dane.Provider() + return prvd.(sdk.CheckerDefinitionProvider).Definition(), prvd, nil }