diff --git a/.drone-manifest.yml b/.drone-manifest.yml deleted file mode 100644 index e7df71b..0000000 --- a/.drone-manifest.yml +++ /dev/null @@ -1,22 +0,0 @@ -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 deleted file mode 100644 index e0bce12..0000000 --- a/.drone.yml +++ /dev/null @@ -1,187 +0,0 @@ ---- -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 35a3be4..d407a62 100644 --- a/Dockerfile +++ b/Dockerfile @@ -10,8 +10,5 @@ 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 e444d65..31a399e 100644 --- a/checker/collect.go +++ b/checker/collect.go @@ -20,14 +20,7 @@ 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) } @@ -82,9 +75,6 @@ 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 @@ -117,23 +107,13 @@ 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 { - owner := strings.TrimSuffix(r.Hdr.Name, ".") - m := tlsaOwner.FindStringSubmatch(owner) + m := tlsaOwner.FindStringSubmatch(strings.TrimSuffix(r.Hdr.Name, ".")) 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 || port64 == 0 { - invalid = append(invalid, InvalidRecord{ - Owner: owner, - Reason: fmt.Sprintf("port %q out of range (1-65535)", m[1]), - }) + if err != nil { continue } base := m[3] @@ -141,13 +121,6 @@ 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{ @@ -191,32 +164,20 @@ func (p *daneProvider) Collect(ctx context.Context, opts sdk.CheckerOptions) (an targets = append(targets, t) } - data := &DANEData{ + return &DANEData{ Targets: targets, - Invalid: invalid, CollectedAt: time.Now().UTC(), - } - if v, ok := opts[OptionDNSSECValidated]; ok { - if b, ok := v.(bool); ok { - data.DNSSECValidated = &b - } - } - return data, nil + }, 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, - // 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 != "", + Host: t.Host, + Port: t.Port, + SNI: t.Host, + STARTTLS: t.STARTTLS, + RequireSTARTTLS: t.STARTTLS != "" && t.Port != 25, // SMTP on 25 stays opportunistic } } diff --git a/checker/collect_test.go b/checker/collect_test.go deleted file mode 100644 index 72c732d..0000000 --- a/checker/collect_test.go +++ /dev/null @@ -1,226 +0,0 @@ -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/interactive.go b/checker/interactive.go index 1a15f96..47cf8d2 100644 --- a/checker/interactive.go +++ b/checker/interactive.go @@ -3,16 +3,13 @@ package checker import ( - "context" "encoding/json" "errors" "fmt" "net" "net/http" - "os" "strconv" "strings" - "time" "github.com/miekg/dns" @@ -20,25 +17,8 @@ import ( tls "git.happydns.org/checker-tls/checker" ) -// 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. +// tlsaLookup fetches TLSA records for owner via the system resolver. +// It is a package variable so tests can swap it for a fixture. var tlsaLookup = lookupTLSA // RenderForm lets a human run this checker standalone. The form only @@ -94,7 +74,7 @@ func (p *daneProvider) ParseForm(r *http.Request) (sdk.CheckerOptions, error) { } owner := tlsaOwnerName(port, proto, domain) - records, validated, err := tlsaLookup(r.Context(), owner) + records, err := tlsaLookup(owner) if err != nil { return nil, fmt.Errorf("TLSA lookup for %s: %w", owner, err) } @@ -136,7 +116,6 @@ func (p *daneProvider) ParseForm(r *http.Request) (sdk.CheckerOptions, error) { opts[OptionProbeTimeoutMs] = float64(n) } } - opts[OptionDNSSECValidated] = validated return opts, nil } @@ -150,34 +129,25 @@ func (p *daneProvider) RelatedProviders() []sdk.ObservationProvider { return []sdk.ObservationProvider{tls.Provider()} } -// 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) { +// 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) { resolver, err := interactiveResolver() if err != nil { - return nil, false, err + return nil, 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 := &dns.Client{Timeout: dnsClientTimeout} - in, _, err := c.ExchangeContext(ctx, msg, resolver) + c := new(dns.Client) + in, _, err := c.Exchange(msg, resolver) if err != nil { - return nil, false, err + return nil, err } if in.Rcode != dns.RcodeSuccess && in.Rcode != dns.RcodeNameError { - return nil, false, fmt.Errorf("rcode %s", dns.RcodeToString[in.Rcode]) + return nil, fmt.Errorf("rcode %s", dns.RcodeToString[in.Rcode]) } var out []*dns.TLSA for _, rr := range in.Answer { @@ -185,27 +155,13 @@ func lookupTLSA(ctx context.Context, owner string) ([]*dns.TLSA, bool, error) { out = append(out, t) } } - return out, in.AuthenticatedData, nil + return out, 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 "", fmt.Errorf("no DNSSEC-validating resolver configured: set %s to a trusted validator (e.g. 127.0.0.1:53)", resolverEnvVar) + return net.JoinHostPort("1.1.1.1", "53"), nil } return net.JoinHostPort(cfg.Servers[0], cfg.Port), nil } diff --git a/checker/interactive_test.go b/checker/interactive_test.go index 496a58c..4f97732 100644 --- a/checker/interactive_test.go +++ b/checker/interactive_test.go @@ -3,7 +3,6 @@ package checker import ( - "context" "encoding/json" "net/http" "net/http/httptest" @@ -27,15 +26,10 @@ 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(_ context.Context, _ string) ([]*dns.TLSA, bool, error) { - return records, validated, err + tlsaLookup = func(owner string) ([]*dns.TLSA, error) { + return records, err } t.Cleanup(func() { tlsaLookup = prev }) } diff --git a/checker/match_test.go b/checker/match_test.go deleted file mode 100644 index 281d203..0000000 --- a/checker/match_test.go +++ /dev/null @@ -1,417 +0,0 @@ -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 675388e..27efbd6 100644 --- a/checker/report.go +++ b/checker/report.go @@ -177,39 +177,15 @@ func hasPKIXUsage(t TargetResult) bool { return false } -// 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. +// 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. func proposedTLSA(t TargetResult, p *tls.TLSProbe) string { if p == nil || len(p.Chain) == 0 { return "" } - 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) + return fmt.Sprintf("%s IN TLSA 3 1 1 %s", t.Owner, p.Chain[0].SPKISHA256) } // handshakeFix proposes a STARTTLS-aware first step when the probe failed. diff --git a/checker/rule.go b/checker/rule.go index c4d63a4..f2078ba 100644 --- a/checker/rule.go +++ b/checker/rule.go @@ -18,7 +18,6 @@ import ( func Rules() []sdk.CheckRule { return []sdk.CheckRule{ &hasRecordsRule{}, - &dnssecValidatedRule{}, &probeAvailableRule{}, &handshakeOKRule{}, &recordsMatchChainRule{}, @@ -32,10 +31,10 @@ func Rules() []sdk.CheckRule { type ruleContext struct { data DANEData probes map[string]*tls.TLSProbe - // 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 + // 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 // err is a fatal error loading the checker's own observation. err error } @@ -48,7 +47,7 @@ func loadRuleContext(ctx context.Context, obs sdk.ObservationGetter) *ruleContex rc.err = err return rc } - rc.probes, rc.relatedErr = relatedTLSProbes(ctx, obs) + rc.probes, rc.warn = relatedTLSProbes(ctx, obs) return rc } @@ -149,49 +148,18 @@ 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 { - lastErr = err.Error() - continue + return false, err.Error() } 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. @@ -201,9 +169,9 @@ func recordCandidate(rec TLSARecord, c tls.CertInfo) (string, error) { case SelectorCert: switch rec.MatchingType { case MatchingFull: - der, err := decodeFullDER(c.DERBase64, "cert DER") + der, err := base64.StdEncoding.DecodeString(c.DERBase64) if err != nil { - return "", err + return "", fmt.Errorf("decode cert DER: %w", err) } source = hex.EncodeToString(der) case MatchingSHA256: @@ -216,9 +184,9 @@ func recordCandidate(rec TLSARecord, c tls.CertInfo) (string, error) { case SelectorSPKI: switch rec.MatchingType { case MatchingFull: - spki, err := decodeFullDER(c.SPKIDERBase64, "SPKI DER") + spki, err := base64.StdEncoding.DecodeString(c.SPKIDERBase64) if err != nil { - return "", err + return "", fmt.Errorf("decode SPKI DER: %w", err) } source = hex.EncodeToString(spki) case MatchingSHA256: @@ -247,12 +215,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, error) { +func relatedTLSProbes(ctx context.Context, obs sdk.ObservationGetter) (map[string]*tls.TLSProbe, string) { related, err := obs.GetRelated(ctx, tls.ObservationKeyTLSProbes) if err != nil { - return nil, fmt.Errorf("related TLS probes unavailable: %w", err) + return nil, "related TLS probes unavailable: " + err.Error() } - return indexProbes(related), nil + return indexProbes(related), "" } // 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 4e35333..c9b322c 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.relatedErr != nil { + if rc.warn != "" { return []sdk.CheckState{{ Status: sdk.StatusError, Code: "dane_observation_warning", - Message: rc.relatedErr.Error(), + Message: rc.warn, }} } if len(rc.data.Targets) == 0 { return []sdk.CheckState{{ - Status: sdk.StatusUnknown, + Status: sdk.StatusOK, Code: "dane_probe_available_skipped", Message: "No DANE endpoints to probe.", }} diff --git a/checker/rules_records.go b/checker/rules_records.go index 0a4ba39..d84c039 100644 --- a/checker/rules_records.go +++ b/checker/rules_records.go @@ -2,8 +2,6 @@ package checker import ( "context" - "fmt" - "strings" sdk "git.happydns.org/checker-sdk-go/checker" ) @@ -22,78 +20,17 @@ 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.", }} } - states = append(states, sdk.CheckState{ + return []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 deleted file mode 100644 index 4e05b37..0000000 --- a/checker/rules_test.go +++ /dev/null @@ -1,276 +0,0 @@ -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 2190b2b..75bfcbc 100644 --- a/checker/types.go +++ b/checker/types.go @@ -45,12 +45,6 @@ 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. @@ -79,24 +73,8 @@ const ( type DANEData struct { // Targets is one entry per (port, proto, basename) triplet extracted // from the TLSAs service. - 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"` + Targets []TargetResult `json:"targets"` + CollectedAt time.Time `json:"collected_at"` } // TargetResult groups all TLSA records declared on a single endpoint and diff --git a/go.mod b/go.mod index 329a14e..318b0a6 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.5.0 - git.happydns.org/checker-tls v0.6.2 + git.happydns.org/checker-sdk-go v1.4.0 + git.happydns.org/checker-tls v0.6.1 github.com/miekg/dns v1.1.72 ) diff --git a/go.sum b/go.sum index d4ff867..f00160f 100644 --- a/go.sum +++ b/go.sum @@ -1,7 +1,7 @@ -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= +git.happydns.org/checker-sdk-go v1.4.0 h1:sO8EnF3suhNgYLRsbmCZWJOymH/oNMrOUqj3FEzJArs= +git.happydns.org/checker-sdk-go v1.4.0/go.mod h1:aNAcfYFfbhvH9kJhE0Njp5GX0dQbxdRB0rJ0KvSC5nI= +git.happydns.org/checker-tls v0.6.1 h1:YJp9Q+1aJZ6wATyUZbRh67ZtERN6Mp4Sje8ld2dNFuo= +git.happydns.org/checker-tls v0.6.1/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=