From 9706ee1da5b475ff81ad86e83503aaf182768309 Mon Sep 17 00:00:00 2001 From: lr00rl Date: Sat, 24 Oct 2026 17:41:37 -0700 Subject: [PATCH 01/10] feat: run dns checks over DoH, DoT, UDP and TCP A dns check (design 32 S5a) asks one resolver one question and judges the answer. The target's scheme picks the transport: https posts application/dns-message (RFC 8484, ID 0), tls frames the message behind a two-byte length inside TLS (RFC 7858), and udp and tcp are plain DNS. The resolver's host is resolved once and every address judged by the rule http and tcp checks follow, so a resolver that resolves to a private address is refused with destination_refused before anything is sent. One query per run, with recursion desired and an EDNS(0) record advertising 1232 bytes; a truncated UDP answer is asked again once over TCP. The check's timeout bounds the whole exchange, a DoH body over 64 KiB is refused unread, DoH and DoT verify the certificate against the host and record cert_not_after, and latency runs from the dial to the decoded answer. A response code above 15 is read through the EDNS record. The judgement follows the spec: NOERROR with an answer of the asked type, every expected value in the answer set in the SDK's canonical form, or NXDOMAIN when the check expects it. A failure's error starts with dns_rcode, dns_mismatch, dns_no_answer or dns_transport, and the result carries the response code and at most 16 answers of at most 255 bytes. The agent advertises dns in probe_methods, and a change to the question restarts the check. The SDK pin moves to the S5a branch commit (lattice-sdk #45); it is re-pinned after that merges. Tested: stub resolvers per transport covering pass, mismatch, NXDOMAIN expected and received, SERVFAIL, truncated UDP to TCP, a DoH 500, a certificate for another name, a slow resolver, a body over 64 KiB and a private resolver; go test -race on prober and cmd; gosec and govulncheck. --- cmd/lattice-agent/main.go | 4 +- cmd/lattice-agent/probe_facts_test.go | 10 +- go.mod | 2 +- go.sum | 2 + internal/prober/destination.go | 12 +- internal/prober/dns.go | 537 ++++++++++++++++++++++ internal/prober/dns_test.go | 635 ++++++++++++++++++++++++++ internal/prober/icmp.go | 2 +- internal/prober/icmp_test.go | 4 +- internal/prober/prober.go | 10 +- 10 files changed, 1201 insertions(+), 17 deletions(-) create mode 100644 internal/prober/dns.go create mode 100644 internal/prober/dns_test.go diff --git a/cmd/lattice-agent/main.go b/cmd/lattice-agent/main.go index b3c40e3..b13966e 100644 --- a/cmd/lattice-agent/main.go +++ b/cmd/lattice-agent/main.go @@ -783,7 +783,9 @@ func monitorChanged(a, b model.Monitor) bool { return a.Type != b.Type || a.Target != b.Target || a.TargetNode != b.TargetNode || a.IntervalSec != b.IntervalSec || a.TimeoutSec != b.TimeoutSec || a.Packets != b.Packets || a.ExpectStatus != b.ExpectStatus || - a.SecretsOrigin != b.SecretsOrigin || !slices.Equal(a.Headers, b.Headers) + a.SecretsOrigin != b.SecretsOrigin || !slices.Equal(a.Headers, b.Headers) || + a.DNSName != b.DNSName || a.DNSType != b.DNSType || a.DNSExpectRcode != b.DNSExpectRcode || + !slices.Equal(a.DNSExpect, b.DNSExpect) } // probe runs one probe and queues its result; flushLoop sends it, in a batch diff --git a/cmd/lattice-agent/probe_facts_test.go b/cmd/lattice-agent/probe_facts_test.go index 294ce30..fa648f9 100644 --- a/cmd/lattice-agent/probe_facts_test.go +++ b/cmd/lattice-agent/probe_facts_test.go @@ -43,7 +43,7 @@ func TestHelloAndHeartbeatCarryProbeFacts(t *testing.T) { if err := json.Unmarshal(data, &got); err != nil { t.Fatal(err) } - for _, kind := range []string{model.MonitorTypeTCP, model.MonitorTypeHTTP} { + for _, kind := range []string{model.MonitorTypeTCP, model.MonitorTypeHTTP, model.MonitorTypeDNS, model.MonitorTypeBeacon} { if !slices.Contains(got.ProbeMethods, kind) { t.Fatalf("%s probe_methods %v lacks %s", name, got.ProbeMethods, kind) } @@ -70,8 +70,12 @@ func TestMonitorChangedCoversProbeFields(t *testing.T) { "header value": func(m *model.Monitor) { m.Headers = []model.MonitorHeader{{Name: "X-Api-Key", Value: "two", Secret: true}} }, - "header removed": func(m *model.Monitor) { m.Headers = nil }, - "secrets_origin": func(m *model.Monitor) { m.SecretsOrigin = "https://b.example:443" }, + "header removed": func(m *model.Monitor) { m.Headers = nil }, + "secrets_origin": func(m *model.Monitor) { m.SecretsOrigin = "https://b.example:443" }, + "dns_name": func(m *model.Monitor) { m.DNSName = "example.net" }, + "dns_type": func(m *model.Monitor) { m.DNSType = model.DNSTypeAAAA }, + "dns_expect": func(m *model.Monitor) { m.DNSExpect = []string{"192.0.2.1"} }, + "dns_expect_rcode": func(m *model.Monitor) { m.DNSExpectRcode = model.DNSRcodeNXDomain }, } { changed := base changed.Headers = slices.Clone(base.Headers) diff --git a/go.mod b/go.mod index 2085716..4f69455 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,7 @@ go 1.26.0 toolchain go1.26.10 -require github.com/LatticeNet/lattice-sdk v0.2.24-0.20261017194014-a3de5f71b589 +require github.com/LatticeNet/lattice-sdk v0.2.24-0.20261025003326-6485e4abe7b9 require github.com/creack/pty v1.1.24 diff --git a/go.sum b/go.sum index 8cb8faa..7d4408f 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,7 @@ github.com/LatticeNet/lattice-sdk v0.2.24-0.20261017194014-a3de5f71b589 h1:B7EVUKUz5Bj9gKIt7H7n3YJfTBR0nGz5op6qMnoZEJE= github.com/LatticeNet/lattice-sdk v0.2.24-0.20261017194014-a3de5f71b589/go.mod h1:jx9PFwMH2zT993VRwecQZBo/rP9W1lgWcZYDdqr5D9M= +github.com/LatticeNet/lattice-sdk v0.2.24-0.20261025003326-6485e4abe7b9 h1:aKA/XfkcXfbYbGKSETmqwAUDVJ+EZMC68Oy8TMDgnG8= +github.com/LatticeNet/lattice-sdk v0.2.24-0.20261025003326-6485e4abe7b9/go.mod h1:jx9PFwMH2zT993VRwecQZBo/rP9W1lgWcZYDdqr5D9M= github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= github.com/creack/pty v1.1.24 h1:bJrF4RRfyJnbTJqzRLHzcGaZK1NeM5kTC9jGgovnR1s= diff --git a/internal/prober/destination.go b/internal/prober/destination.go index 6b11d51..21f7aa6 100644 --- a/internal/prober/destination.go +++ b/internal/prober/destination.go @@ -83,11 +83,11 @@ func asciiHost(host string) (string, error) { // net.Dialer gives each address of a serial fallback. const minDialSlice = 2 * time.Second -// dialJudged connects to the first of addrs that answers on port, in the -// order the resolver gave them, sharing the time left between the -// addresses not yet tried as net.Dialer does. addrs are IP literals, so the -// dial makes no lookup. -func (p *prober) dialJudged(ctx context.Context, addrs []netip.Addr, port string) (net.Conn, error) { +// dialJudged connects over network (tcp or udp) to the first of addrs that +// answers on port, in the order the resolver gave them, sharing the time +// left between the addresses not yet tried as net.Dialer does. addrs are +// IP literals, so the dial makes no lookup. +func (p *prober) dialJudged(ctx context.Context, network string, addrs []netip.Addr, port string) (net.Conn, error) { if len(addrs) == 0 { return nil, errors.New("no address to dial") } @@ -103,7 +103,7 @@ func (p *prober) dialJudged(ctx context.Context, addrs []netip.Addr, port string dctx, cancel = context.WithTimeout(ctx, slice) } } - conn, err := p.dial(dctx, "tcp", net.JoinHostPort(addr.String(), port)) + conn, err := p.dial(dctx, network, net.JoinHostPort(addr.String(), port)) cancel() if err == nil { return conn, nil diff --git a/internal/prober/dns.go b/internal/prober/dns.go new file mode 100644 index 0000000..f93601f --- /dev/null +++ b/internal/prober/dns.go @@ -0,0 +1,537 @@ +package prober + +import ( + "bytes" + "context" + "crypto/rand" + "crypto/tls" + "crypto/x509" + "encoding/binary" + "errors" + "fmt" + "io" + "mime" + "net" + "net/http" + "net/netip" + "net/url" + "slices" + "strconv" + "strings" + "time" + "unicode/utf8" + + "github.com/LatticeNet/lattice-sdk/model" + "golang.org/x/net/dns/dnsmessage" +) + +// A dns check (design 32 S5a) asks one resolver one question: DoH (RFC +// 8484, a POST of application/dns-message), DoT (RFC 7858, the message +// behind a two-byte length over TLS), or plain DNS over UDP or TCP. A run +// resolves the resolver's host once and judges every address as every +// other kind does, sends one query, asks once more over TCP only when a +// UDP answer comes back truncated, and judges the answer against the +// check's expectation. The timeout bounds the whole exchange. + +// dnsUDPPayload is the EDNS(0) payload size a query advertises, the size +// DNS Flag Day 2020 settled on to avoid fragmented UDP answers. +const dnsUDPPayload = 1232 + +// dnsMediaType is the DoH request and response body type. +const dnsMediaType = "application/dns-message" + +// dnsQuestion is a dns check's question and judgement, checked and in +// canonical form. +type dnsQuestion struct { + name string // without the trailing dot + typ string // a model.DNSTypes value + qtype dnsmessage.Type + rcode string // "" for NOERROR, or model.DNSRcodeNXDomain + expect []string +} + +var dnsQTypes = map[string]dnsmessage.Type{ + model.DNSTypeA: dnsmessage.TypeA, + model.DNSTypeAAAA: dnsmessage.TypeAAAA, + model.DNSTypeCNAME: dnsmessage.TypeCNAME, + model.DNSTypeTXT: dnsmessage.TypeTXT, + model.DNSTypeMX: dnsmessage.TypeMX, + model.DNSTypeNS: dnsmessage.TypeNS, +} + +// dnsQuestionOf reads m's question with the rules the control plane saved +// it under, so a check it would refuse is refused here too. +func dnsQuestionOf(m model.Monitor) (dnsQuestion, error) { + var q dnsQuestion + var err error + if q.name, err = model.NormalizeDNSName(m.DNSName); err != nil { + return q, fmt.Errorf("dns_name: %w", err) + } + if q.typ, err = model.NormalizeDNSType(m.DNSType); err != nil { + return q, err + } + q.qtype = dnsQTypes[q.typ] + if q.rcode, err = model.NormalizeDNSExpectRcode(m.DNSExpectRcode); err != nil { + return q, err + } + if len(m.DNSExpect) > model.MonitorDNSExpectMax { + return q, fmt.Errorf("dns_expect has %d values; at most %d", len(m.DNSExpect), model.MonitorDNSExpectMax) + } + for _, raw := range m.DNSExpect { + v, err := model.CanonicalDNSValue(q.typ, raw) + if err != nil { + return q, fmt.Errorf("dns_expect: %w", err) + } + q.expect = append(q.expect, v) + } + return q, nil +} + +// dnsFailure is a run that failed with a model.DNSFailure* code. +type dnsFailure struct { + code string + msg string +} + +func (e *dnsFailure) Error() string { return e.code + ": " + e.msg } + +func dnsTransportError(err error) *dnsFailure { + var failure *dnsFailure + if errors.As(err, &failure) { + return failure + } + if errors.Is(err, context.DeadlineExceeded) || errors.Is(err, context.Canceled) || isTimeout(err) { + return &dnsFailure{code: model.DNSFailureTransport, msg: "no answer before the check's timeout"} + } + return &dnsFailure{code: model.DNSFailureTransport, msg: err.Error()} +} + +func isTimeout(err error) bool { + var ne net.Error + return errors.As(err, &ne) && ne.Timeout() +} + +// dnsAnswer is a decoded response: its code and the values of the asked +// type, in canonical form and in the order they came. +type dnsAnswer struct { + rcode string + values []string + truncated bool +} + +func (p *prober) probeDNS(ctx context.Context, m model.Monitor, timeout time.Duration, res *model.MonitorResult) { + resolver, err := model.ParseDNSResolver(m.Target) + if err != nil { + res.Error = err.Error() + return + } + q, err := dnsQuestionOf(m) + if err != nil { + res.Error = err.Error() + return + } + ctx, cancel := context.WithTimeout(ctx, timeout) + defer cancel() + // dns never takes target_node, so the destination rules always apply. + addrs, err := p.resolveTarget(ctx, resolver.Host, false) + if err != nil { + var refused *destinationError + if errors.As(err, &refused) { + res.Reason = model.PathReasonDestinationRefused + res.Error = refused.Error() + return + } + res.Error = dnsTransportError(err).Error() + return + } + query, id, err := dnsQuery(q, resolver.Transport == model.DNSTransportDoH) + if err != nil { + res.Error = err.Error() + return + } + start := time.Now() + var raw []byte + switch resolver.Transport { + case model.DNSTransportDoH: + raw, err = p.exchangeDoH(ctx, resolver, addrs, query, res) + case model.DNSTransportDoT: + raw, err = p.exchangeDoT(ctx, resolver, addrs, query, res) + case model.DNSTransportTCP: + raw, err = p.exchangeTCP(ctx, addrs, strconv.Itoa(resolver.Port), query) + default: + raw, err = p.exchangeUDP(ctx, addrs, strconv.Itoa(resolver.Port), query, id) + } + var answer dnsAnswer + if err == nil { + answer, err = dnsDecode(raw, id, q) + } + if err == nil && answer.truncated && resolver.Transport == model.DNSTransportUDP { + // The one retry a run makes: the same question over TCP to the + // same resolver, as a stub resolver does with a truncated answer. + raw, err = p.exchangeTCP(ctx, addrs, strconv.Itoa(resolver.Port), query) + if err == nil { + answer, err = dnsDecode(raw, id, q) + } + } + res.LatencyMs = elapsedMs(time.Since(start)) + if err != nil { + var refused *destinationError + if errors.As(err, &refused) { + res.LatencyMs = 0 + res.Reason = model.PathReasonDestinationRefused + res.Error = refused.Error() + return + } + res.Error = dnsTransportError(err).Error() + return + } + res.DNSRcode = answer.rcode + res.DNSAnswers = dnsReported(answer.values) + if failure := q.judge(answer); failure != nil { + res.Error = failure.Error() + return + } + res.Success = true +} + +// judge is the check's verdict on a decoded answer, nil when it passes. +func (q dnsQuestion) judge(a dnsAnswer) *dnsFailure { + if q.rcode == model.DNSRcodeNXDomain { + if a.rcode != model.DNSRcodeNXDomain { + return &dnsFailure{code: model.DNSFailureRcode, msg: fmt.Sprintf("%s answered %s; the check expects NXDOMAIN", q.name, a.rcode)} + } + return nil + } + if a.rcode != model.DNSRcodeNoError { + return &dnsFailure{code: model.DNSFailureRcode, msg: fmt.Sprintf("%s answered %s", q.name, a.rcode)} + } + if len(a.values) == 0 { + return &dnsFailure{code: model.DNSFailureNoAnswer, msg: fmt.Sprintf("no %s answer for %s", q.typ, q.name)} + } + var missing []string + for _, want := range q.expect { + if !slices.Contains(a.values, want) { + missing = append(missing, want) + } + } + if len(missing) > 0 { + return &dnsFailure{code: model.DNSFailureMismatch, msg: fmt.Sprintf("the %s answer for %s lacks %s", q.typ, q.name, strings.Join(missing, ", "))} + } + return nil +} + +// dnsReported is what a result carries of the answer: at most +// model.MonitorDNSAnswersMax values of at most model.MonitorDNSValueMax +// bytes each, cut at a character boundary. +func dnsReported(values []string) []string { + if len(values) == 0 { + return nil + } + out := make([]string, 0, min(len(values), model.MonitorDNSAnswersMax)) + for _, v := range values[:min(len(values), model.MonitorDNSAnswersMax)] { + if len(v) > model.MonitorDNSValueMax { + v = v[:model.MonitorDNSValueMax] + } + out = append(out, strings.ToValidUTF8(v, string(utf8.RuneError))) + } + return out +} + +// dnsQuery packs the question with recursion desired and an EDNS(0) +// record. A DoH query carries ID 0, as RFC 8484 asks, so caches can share +// it; the others a random ID the answer must echo. +func dnsQuery(q dnsQuestion, doh bool) ([]byte, uint16, error) { + var id uint16 + if !doh { + var b [2]byte + if _, err := rand.Read(b[:]); err != nil { + return nil, 0, err + } + id = binary.BigEndian.Uint16(b[:]) + } + name, err := dnsmessage.NewName(q.name + ".") + if err != nil { + return nil, 0, fmt.Errorf("dns_name: %w", err) + } + b := dnsmessage.NewBuilder(make([]byte, 0, 512), dnsmessage.Header{ID: id, RecursionDesired: true}) + if err := b.StartQuestions(); err != nil { + return nil, 0, err + } + if err := b.Question(dnsmessage.Question{Name: name, Type: q.qtype, Class: dnsmessage.ClassINET}); err != nil { + return nil, 0, err + } + if err := b.StartAdditionals(); err != nil { + return nil, 0, err + } + var opt dnsmessage.ResourceHeader + if err := opt.SetEDNS0(dnsUDPPayload, dnsmessage.RCodeSuccess, false); err != nil { + return nil, 0, err + } + if err := b.OPTResource(opt, dnsmessage.OPTResource{}); err != nil { + return nil, 0, err + } + msg, err := b.Finish() + return msg, id, err +} + +// dnsDecode reads a response to the query with id: its response code, +// extended by an EDNS(0) record when there is one, and the values of +// answers of the asked type and class. +func dnsDecode(raw []byte, id uint16, q dnsQuestion) (dnsAnswer, error) { + var parser dnsmessage.Parser + h, err := parser.Start(raw) + if err != nil { + return dnsAnswer{}, &dnsFailure{code: model.DNSFailureTransport, msg: "the response is not a DNS message: " + err.Error()} + } + if !h.Response || h.ID != id { + return dnsAnswer{}, &dnsFailure{code: model.DNSFailureTransport, msg: "the response does not answer the query"} + } + a := dnsAnswer{truncated: h.Truncated} + if a.truncated { + return a, nil + } + malformed := func(err error) error { + return &dnsFailure{code: model.DNSFailureTransport, msg: "the response is malformed: " + err.Error()} + } + if err := parser.SkipAllQuestions(); err != nil { + return dnsAnswer{}, malformed(err) + } + for { + ah, err := parser.AnswerHeader() + if errors.Is(err, dnsmessage.ErrSectionDone) { + break + } + if err != nil { + return dnsAnswer{}, malformed(err) + } + if ah.Type != q.qtype || ah.Class != dnsmessage.ClassINET { + if err := parser.SkipAnswer(); err != nil { + return dnsAnswer{}, malformed(err) + } + continue + } + v, err := dnsValue(&parser, q.qtype) + if err != nil { + return dnsAnswer{}, malformed(err) + } + a.values = append(a.values, v) + } + rcode := h.RCode + // An EDNS(0) record carries the upper bits of the response code. A + // response that ends badly after its answers keeps the header's code. + if parser.SkipAllAuthorities() == nil { + for { + ah, err := parser.AdditionalHeader() + if err != nil { + break + } + if ah.Type == dnsmessage.TypeOPT { + rcode = ah.ExtendedRCode(h.RCode) + } + if parser.SkipAdditional() != nil { + break + } + } + } + a.rcode = dnsRcodeName(rcode) + return a, nil +} + +// dnsValue decodes the answer the parser is at, of type t, into the form +// model.CanonicalDNSValue gives an expected value. +func dnsValue(parser *dnsmessage.Parser, t dnsmessage.Type) (string, error) { + switch t { + case dnsmessage.TypeA: + r, err := parser.AResource() + return netip.AddrFrom4(r.A).String(), err + case dnsmessage.TypeAAAA: + r, err := parser.AAAAResource() + return netip.AddrFrom16(r.AAAA).String(), err + case dnsmessage.TypeCNAME: + r, err := parser.CNAMEResource() + return dnsName(r.CNAME), err + case dnsmessage.TypeNS: + r, err := parser.NSResource() + return dnsName(r.NS), err + case dnsmessage.TypeMX: + r, err := parser.MXResource() + return dnsName(r.MX), err + case dnsmessage.TypeTXT: + r, err := parser.TXTResource() + return strings.Join(r.TXT, ""), err + } + return "", fmt.Errorf("type %s is not asked", t) +} + +func dnsName(n dnsmessage.Name) string { + return strings.ToLower(strings.TrimSuffix(n.String(), ".")) +} + +// dnsRcodeNames are the IANA mnemonics of the response codes a resolver +// returns (RFC 6895 and its registry). +var dnsRcodeNames = map[dnsmessage.RCode]string{ + 0: model.DNSRcodeNoError, 1: "FORMERR", 2: "SERVFAIL", 3: model.DNSRcodeNXDomain, + 4: "NOTIMP", 5: "REFUSED", 6: "YXDOMAIN", 7: "YXRRSET", 8: "NXRRSET", + 9: "NOTAUTH", 10: "NOTZONE", 16: "BADVERS", 23: "BADCOOKIE", +} + +func dnsRcodeName(code dnsmessage.RCode) string { + if name, ok := dnsRcodeNames[code]; ok { + return name + } + return "RCODE" + strconv.Itoa(int(code)) +} + +// exchangeUDP sends the query to the first judged address and reads +// datagrams until one answers it. A connected socket takes datagrams from +// that address only; one that does not echo id is ignored. +func (p *prober) exchangeUDP(ctx context.Context, addrs []netip.Addr, port string, query []byte, id uint16) ([]byte, error) { + conn, err := p.dialJudged(ctx, "udp", addrs[:1], port) + if err != nil { + return nil, err + } + defer conn.Close() + setDeadline(ctx, conn) + if _, err := conn.Write(query); err != nil { + return nil, err + } + buf := make([]byte, model.MonitorDNSResponseMax) + for { + n, err := conn.Read(buf) + if err != nil { + return nil, err + } + if n >= 2 && binary.BigEndian.Uint16(buf[:2]) == id { + return slices.Clone(buf[:n]), nil + } + } +} + +// exchangeTCP sends the query over TCP to the first judged address that +// accepts, behind its two-byte length. +func (p *prober) exchangeTCP(ctx context.Context, addrs []netip.Addr, port string, query []byte) ([]byte, error) { + conn, err := p.dialJudged(ctx, "tcp", addrs, port) + if err != nil { + return nil, err + } + defer conn.Close() + setDeadline(ctx, conn) + return dnsStreamExchange(conn, query) +} + +// exchangeDoT is exchangeTCP inside TLS verified against the resolver's +// host. The certificate's expiry is recorded once the handshake completes. +func (p *prober) exchangeDoT(ctx context.Context, r model.DNSResolver, addrs []netip.Addr, query []byte, res *model.MonitorResult) ([]byte, error) { + raw, err := p.dialJudged(ctx, "tcp", addrs, strconv.Itoa(r.Port)) + if err != nil { + return nil, err + } + defer raw.Close() + setDeadline(ctx, raw) + conn := tls.Client(raw, &tls.Config{ServerName: r.Host, RootCAs: p.rootCAs, MinVersion: tls.VersionTLS12}) + if err := conn.HandshakeContext(ctx); err != nil { + if ctx.Err() != nil || isTimeout(err) { + return nil, err + } + return nil, &dnsFailure{code: model.DNSFailureTransport, msg: "tls: " + strings.TrimPrefix(err.Error(), "tls: ")} + } + recordCert(conn.ConnectionState().PeerCertificates, res) + return dnsStreamExchange(conn, query) +} + +// dnsStreamExchange writes one length-prefixed query and reads one +// length-prefixed response (RFC 1035 section 4.2.2, RFC 7858). The length +// is two bytes, so a response is never over model.MonitorDNSResponseMax. +func dnsStreamExchange(conn net.Conn, query []byte) ([]byte, error) { + frame := make([]byte, 2+len(query)) + binary.BigEndian.PutUint16(frame, uint16(len(query))) // #nosec G115 -- one question of a name of at most 255 bytes, far under 65535 + copy(frame[2:], query) + if _, err := conn.Write(frame); err != nil { + return nil, err + } + var size [2]byte + if _, err := io.ReadFull(conn, size[:]); err != nil { + return nil, err + } + out := make([]byte, binary.BigEndian.Uint16(size[:])) + if _, err := io.ReadFull(conn, out); err != nil { + return nil, err + } + return out, nil +} + +// exchangeDoH posts the query to the resolver's URL through the http +// check's transport: only the judged addresses are dialed, no proxy, no +// redirect, the certificate verified against the host. A body over +// model.MonitorDNSResponseMax is refused rather than read. +func (p *prober) exchangeDoH(ctx context.Context, r model.DNSResolver, addrs []netip.Addr, query []byte, res *model.MonitorResult) ([]byte, error) { + transport := p.httpTransport(r.Host, addrs) + defer transport.CloseIdleConnections() + client := &http.Client{ + Transport: transport, + CheckRedirect: func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse }, + } + req, err := http.NewRequestWithContext(ctx, http.MethodPost, r.URL(), bytes.NewReader(query)) + if err != nil { + return nil, err + } + req.Header.Set("Content-Type", dnsMediaType) + req.Header.Set("Accept", dnsMediaType) + resp, err := client.Do(req) + if err != nil { + var refused *destinationError + if errors.As(err, &refused) { + return nil, refused + } + var cert *tls.CertificateVerificationError + if errors.As(err, &cert) { + return nil, &dnsFailure{code: model.DNSFailureTransport, msg: "tls: " + cert.Err.Error()} + } + return nil, unwrapURLError(err) + } + defer resp.Body.Close() + if resp.TLS != nil { + recordCert(resp.TLS.PeerCertificates, res) + } + if resp.StatusCode != http.StatusOK { + return nil, &dnsFailure{code: model.DNSFailureTransport, msg: fmt.Sprintf("http status %d", resp.StatusCode)} + } + if media, _, _ := mime.ParseMediaType(resp.Header.Get("Content-Type")); media != dnsMediaType { + return nil, &dnsFailure{code: model.DNSFailureTransport, msg: fmt.Sprintf("http 200 with content type %q, not %s", resp.Header.Get("Content-Type"), dnsMediaType)} + } + tooLarge := &dnsFailure{code: model.DNSFailureTransport, msg: fmt.Sprintf("the response is over %d KiB", model.MonitorDNSResponseMax>>10)} + if resp.ContentLength > model.MonitorDNSResponseMax { + return nil, tooLarge + } + body, err := io.ReadAll(io.LimitReader(resp.Body, model.MonitorDNSResponseMax+1)) + if err != nil { + return nil, err + } + if len(body) > model.MonitorDNSResponseMax { + return nil, tooLarge + } + return body, nil +} + +// unwrapURLError drops the method and URL net/http puts before a request +// error; the check's target already names them. +func unwrapURLError(err error) error { + var uerr *url.Error + if errors.As(err, &uerr) && uerr.Err != nil { + return uerr.Err + } + return err +} + +func recordCert(chain []*x509.Certificate, res *model.MonitorResult) { + if len(chain) > 0 { + res.CertNotAfter = chain[0].NotAfter.UTC() + } +} + +// setDeadline bounds every read and write on conn by ctx's deadline. +func setDeadline(ctx context.Context, conn net.Conn) { + if deadline, ok := ctx.Deadline(); ok { + _ = conn.SetDeadline(deadline) + } +} diff --git a/internal/prober/dns_test.go b/internal/prober/dns_test.go new file mode 100644 index 0000000..5b4dea7 --- /dev/null +++ b/internal/prober/dns_test.go @@ -0,0 +1,635 @@ +package prober + +import ( + "context" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" + "encoding/binary" + "fmt" + "io" + "log" + "math/big" + "net" + "net/http" + "net/http/httptest" + "net/netip" + "slices" + "strconv" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/LatticeNet/lattice-sdk/model" + "golang.org/x/net/dns/dnsmessage" +) + +// dnsHandler answers one query; it returns the packed response, or nil to +// send nothing. network is "udp" or "tcp" (DoT and DoH answer as "tcp"). +type dnsHandler func(network string, q dnsmessage.Message) []byte + +// dnsStub is a resolver on loopback: UDP and TCP on their own ports, the +// TCP side optionally inside TLS. queries counts the queries it read. +type dnsStub struct { + udp net.PacketConn + tcp net.Listener + queries atomic.Int32 + handle dnsHandler +} + +func newDNSStub(t *testing.T, tlsConfig *tls.Config, handle dnsHandler) *dnsStub { + t.Helper() + udp, err := net.ListenPacket("udp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + tcp, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + if tlsConfig != nil { + tcp = tls.NewListener(tcp, tlsConfig) + } + s := &dnsStub{udp: udp, tcp: tcp, handle: handle} + t.Cleanup(func() { _ = udp.Close(); _ = tcp.Close() }) + go s.serveUDP() + go s.serveTCP() + return s +} + +func (s *dnsStub) serveUDP() { + buf := make([]byte, 65535) + for { + n, from, err := s.udp.ReadFrom(buf) + if err != nil { + return + } + var q dnsmessage.Message + if q.Unpack(buf[:n]) != nil { + continue + } + s.queries.Add(1) + if out := s.handle("udp", q); out != nil { + _, _ = s.udp.WriteTo(out, from) + } + } +} + +func (s *dnsStub) serveTCP() { + for { + conn, err := s.tcp.Accept() + if err != nil { + return + } + go func() { + defer conn.Close() + var size [2]byte + if _, err := io.ReadFull(conn, size[:]); err != nil { + return + } + raw := make([]byte, binary.BigEndian.Uint16(size[:])) + if _, err := io.ReadFull(conn, raw); err != nil { + return + } + var q dnsmessage.Message + if q.Unpack(raw) != nil { + return + } + s.queries.Add(1) + out := s.handle("tcp", q) + if out == nil { + return + } + frame := binary.BigEndian.AppendUint16(nil, uint16(len(out))) + _, _ = conn.Write(append(frame, out...)) + }() + } +} + +// dnsProber resolves testHost to testPublicAddr and carries a dial to that +// address to the stub's socket of the same network, so the destination +// rules run as on a node. roots verifies DoT and DoH. +func dnsProber(t *testing.T, udp, tcp net.Addr, roots *x509.CertPool) *prober { + t.Helper() + real := &net.Dialer{} + p := newProber() + p.resolver = &fakeResolver{answers: map[string][]string{testHost: {testPublicAddr}, "internal.example": {"10.0.0.5"}}} + p.rootCAs = roots + p.dial = func(ctx context.Context, network, address string) (net.Conn, error) { + host, _, err := net.SplitHostPort(address) + if err != nil || host != testPublicAddr { + return nil, fmt.Errorf("test dialer: unexpected address %s", address) + } + switch network { + case "udp": + return real.DialContext(ctx, network, udp.String()) + case "tcp": + return real.DialContext(ctx, network, tcp.String()) + } + return nil, fmt.Errorf("test dialer: unexpected network %s", network) + } + return p +} + +// testCert is a self-signed certificate for names, and a pool that trusts it. +func testCert(t *testing.T, names ...string) (tls.Certificate, *x509.CertPool, time.Time) { + t.Helper() + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatal(err) + } + notAfter := time.Now().Add(90 * 24 * time.Hour).Truncate(time.Second).UTC() + tmpl := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: names[0]}, + DNSNames: names, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: notAfter, + KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, + BasicConstraintsValid: true, + IsCA: true, + } + der, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &key.PublicKey, key) + if err != nil { + t.Fatal(err) + } + leaf, err := x509.ParseCertificate(der) + if err != nil { + t.Fatal(err) + } + pool := x509.NewCertPool() + pool.AddCert(leaf) + return tls.Certificate{Certificate: [][]byte{der}, PrivateKey: key, Leaf: leaf}, pool, notAfter +} + +// dnsReply builds the response to q. A code above 15 carries its upper +// bits in an EDNS(0) record, as a resolver sends it. +func dnsReply(q dnsmessage.Message, rcode dnsmessage.RCode, truncated bool, answers ...dnsmessage.Resource) []byte { + msg := dnsmessage.Message{ + Header: dnsmessage.Header{ID: q.ID, Response: true, RecursionAvailable: true, RCode: rcode & 0xF, Truncated: truncated}, + Questions: q.Questions, + Answers: answers, + } + if rcode > 0xF { + var opt dnsmessage.ResourceHeader + if err := opt.SetEDNS0(1232, rcode, false); err != nil { + panic(err) + } + msg.Additionals = []dnsmessage.Resource{{Header: opt, Body: &dnsmessage.OPTResource{}}} + } + out, err := msg.Pack() + if err != nil { + panic(err) + } + return out +} + +func rrHeader(q dnsmessage.Message, typ dnsmessage.Type) dnsmessage.ResourceHeader { + return dnsmessage.ResourceHeader{Name: q.Questions[0].Name, Type: typ, Class: dnsmessage.ClassINET, TTL: 60} +} + +func aRecord(q dnsmessage.Message, addr string) dnsmessage.Resource { + return dnsmessage.Resource{Header: rrHeader(q, dnsmessage.TypeA), Body: &dnsmessage.AResource{A: netip.MustParseAddr(addr).As4()}} +} + +// answerA answers every A question with addrs. +func answerA(addrs ...string) dnsHandler { + return func(_ string, q dnsmessage.Message) []byte { + var rrs []dnsmessage.Resource + for _, a := range addrs { + rrs = append(rrs, aRecord(q, a)) + } + return dnsReply(q, dnsmessage.RCodeSuccess, false, rrs...) + } +} + +func dnsMonitor(target string) model.Monitor { + return model.Monitor{ID: "mon_dns", Type: model.MonitorTypeDNS, Target: target, TimeoutSec: 2, DNSName: "www.example.org"} +} + +func failureCode(res model.MonitorResult) string { + code, _, _ := strings.Cut(res.Error, ":") + return code +} + +// Every transport: a pass with the default judgement (an answer of the +// asked type), a pass with the expected set, and a mismatch that reports +// what came back. +func TestDNSPassAndMismatchOverEveryTransport(t *testing.T) { + cert, roots, notAfter := testCert(t, testHost) + plain := newDNSStub(t, nil, answerA("192.0.2.1", "192.0.2.2")) + secure := newDNSStub(t, &tls.Config{Certificates: []tls.Certificate{cert}}, answerA("192.0.2.1", "192.0.2.2")) + doh := dohServer(t, cert, func(w http.ResponseWriter, q dnsmessage.Message) { + w.Header().Set("Content-Type", "application/dns-message") + _, _ = w.Write(answerA("192.0.2.1", "192.0.2.2")("tcp", q)) + }) + for _, tc := range []struct { + name string + p *prober + target string + cert bool + }{ + {"udp", dnsProber(t, plain.udp.LocalAddr(), plain.tcp.Addr(), nil), "udp://" + testHost, false}, + {"tcp", dnsProber(t, plain.udp.LocalAddr(), plain.tcp.Addr(), nil), "tcp://" + testHost, false}, + {"dot", dnsProber(t, plain.udp.LocalAddr(), secure.tcp.Addr(), roots), "tls://" + testHost, true}, + {"doh", dnsProber(t, plain.udp.LocalAddr(), doh.Listener.Addr(), roots), "https://" + testHost + "/dns-query", true}, + } { + t.Run(tc.name, func(t *testing.T) { + m := dnsMonitor(tc.target) + res := tc.p.probe(context.Background(), m) + if !res.Success || res.DNSRcode != "NOERROR" || !slices.Equal(res.DNSAnswers, []string{"192.0.2.1", "192.0.2.2"}) || res.Error != "" { + t.Fatalf("default judgement: %+v", res) + } + if res.LatencyMs <= 0 { + t.Fatalf("no latency recorded: %+v", res) + } + if tc.cert != !res.CertNotAfter.IsZero() || (tc.cert && !res.CertNotAfter.Equal(notAfter)) { + t.Fatalf("cert_not_after = %v, want %v (recorded: %v)", res.CertNotAfter, notAfter, tc.cert) + } + m.DNSExpect = []string{"192.0.2.2"} + if res := tc.p.probe(context.Background(), m); !res.Success { + t.Fatalf("expected value present: %+v", res) + } + m.DNSExpect = []string{"192.0.2.2", "198.51.100.7"} + res = tc.p.probe(context.Background(), m) + if res.Success || failureCode(res) != model.DNSFailureMismatch || !strings.Contains(res.Error, "198.51.100.7") || + !slices.Equal(res.DNSAnswers, []string{"192.0.2.1", "192.0.2.2"}) || res.Reason != "" { + t.Fatalf("mismatch: %+v", res) + } + }) + } +} + +// NXDOMAIN expected and received passes; NXDOMAIN when an answer is +// expected fails with the code; an answer when NXDOMAIN is expected fails +// too; SERVFAIL fails naming SERVFAIL. +func TestDNSResponseCodes(t *testing.T) { + var rcode atomic.Int32 + stub := newDNSStub(t, nil, func(_ string, q dnsmessage.Message) []byte { + code := dnsmessage.RCode(rcode.Load()) + if code == dnsmessage.RCodeSuccess { + return answerA("192.0.2.1")("udp", q) + } + return dnsReply(q, code, false) + }) + p := dnsProber(t, stub.udp.LocalAddr(), stub.tcp.Addr(), nil) + m := dnsMonitor("udp://" + testHost) + + rcode.Store(int32(dnsmessage.RCodeNameError)) + res := p.probe(context.Background(), m) + if res.Success || failureCode(res) != model.DNSFailureRcode || res.DNSRcode != "NXDOMAIN" { + t.Fatalf("NXDOMAIN for a name expected to resolve: %+v", res) + } + m.DNSExpectRcode = model.DNSRcodeNXDomain + if res := p.probe(context.Background(), m); !res.Success || res.DNSRcode != "NXDOMAIN" { + t.Fatalf("NXDOMAIN expected and received: %+v", res) + } + rcode.Store(int32(dnsmessage.RCodeSuccess)) + if res := p.probe(context.Background(), m); res.Success || failureCode(res) != model.DNSFailureRcode || res.DNSRcode != "NOERROR" { + t.Fatalf("a name that must not resolve resolved: %+v", res) + } + m.DNSExpectRcode = "" + rcode.Store(int32(dnsmessage.RCodeServerFailure)) + res = p.probe(context.Background(), m) + if res.Success || failureCode(res) != model.DNSFailureRcode || res.DNSRcode != "SERVFAIL" || !strings.Contains(res.Error, "SERVFAIL") || res.Reason != "" { + t.Fatalf("SERVFAIL: %+v", res) + } + rcode.Store(23) + if res := p.probe(context.Background(), m); res.DNSRcode != "BADCOOKIE" { + t.Fatalf("a code above 15 needs the EDNS record to read; got %+v", res) + } +} + +// NOERROR with no answer of the asked type (here only a CNAME) is +// dns_no_answer. +func TestDNSNoAnswerOfTheAskedType(t *testing.T) { + stub := newDNSStub(t, nil, func(_ string, q dnsmessage.Message) []byte { + target := dnsmessage.MustNewName("edge.example.net.") + return dnsReply(q, dnsmessage.RCodeSuccess, false, dnsmessage.Resource{Header: rrHeader(q, dnsmessage.TypeCNAME), Body: &dnsmessage.CNAMEResource{CNAME: target}}) + }) + p := dnsProber(t, stub.udp.LocalAddr(), stub.tcp.Addr(), nil) + res := p.probe(context.Background(), dnsMonitor("udp://"+testHost)) + if res.Success || failureCode(res) != model.DNSFailureNoAnswer || res.DNSRcode != "NOERROR" || len(res.DNSAnswers) != 0 { + t.Fatalf("CNAME only for an A question: %+v", res) + } + m := dnsMonitor("udp://" + testHost) + m.DNSType, m.DNSExpect = model.DNSTypeCNAME, []string{"EDGE.example.net."} + if res := p.probe(context.Background(), m); !res.Success || !slices.Equal(res.DNSAnswers, []string{"edge.example.net"}) { + t.Fatalf("CNAME compared without case or the trailing dot: %+v", res) + } +} + +// Values of the other types compare in the SDK's canonical form: AAAA as +// parsed addresses, MX as the exchange, TXT as the joined strings. +func TestDNSValueForms(t *testing.T) { + stub := newDNSStub(t, nil, func(_ string, q dnsmessage.Message) []byte { + switch q.Questions[0].Type { + case dnsmessage.TypeAAAA: + return dnsReply(q, dnsmessage.RCodeSuccess, false, dnsmessage.Resource{Header: rrHeader(q, dnsmessage.TypeAAAA), + Body: &dnsmessage.AAAAResource{AAAA: netip.MustParseAddr("2001:db8::1").As16()}}) + case dnsmessage.TypeMX: + return dnsReply(q, dnsmessage.RCodeSuccess, false, dnsmessage.Resource{Header: rrHeader(q, dnsmessage.TypeMX), + Body: &dnsmessage.MXResource{Pref: 10, MX: dnsmessage.MustNewName("Mx1.Example.org.")}}) + case dnsmessage.TypeTXT: + return dnsReply(q, dnsmessage.RCodeSuccess, false, dnsmessage.Resource{Header: rrHeader(q, dnsmessage.TypeTXT), + Body: &dnsmessage.TXTResource{TXT: []string{"v=spf1 ", "-all"}}}) + } + return dnsReply(q, dnsmessage.RCodeSuccess, false) + }) + p := dnsProber(t, stub.udp.LocalAddr(), stub.tcp.Addr(), nil) + for typ, expect := range map[string]string{ + model.DNSTypeAAAA: "2001:DB8:0::1", + model.DNSTypeMX: "mx1.example.org", + model.DNSTypeTXT: "v=spf1 -all", + } { + m := dnsMonitor("udp://" + testHost) + m.DNSType, m.DNSExpect = typ, []string{expect} + if res := p.probe(context.Background(), m); !res.Success { + t.Errorf("%s %q: %+v", typ, expect, res) + } + } +} + +// A truncated UDP answer is asked again once over TCP, and the TCP answer +// is judged. +func TestDNSTruncatedUDPRetriesOverTCP(t *testing.T) { + var udpQueries, tcpQueries atomic.Int32 + stub := newDNSStub(t, nil, func(network string, q dnsmessage.Message) []byte { + if network == "udp" { + udpQueries.Add(1) + return dnsReply(q, dnsmessage.RCodeSuccess, true) + } + tcpQueries.Add(1) + return answerA("192.0.2.9")(network, q) + }) + p := dnsProber(t, stub.udp.LocalAddr(), stub.tcp.Addr(), nil) + m := dnsMonitor("udp://" + testHost) + m.DNSExpect = []string{"192.0.2.9"} + res := p.probe(context.Background(), m) + if !res.Success || !slices.Equal(res.DNSAnswers, []string{"192.0.2.9"}) { + t.Fatalf("truncated answer not retried over TCP: %+v", res) + } + if udpQueries.Load() != 1 || tcpQueries.Load() != 1 { + t.Fatalf("queries udp=%d tcp=%d, want one each", udpQueries.Load(), tcpQueries.Load()) + } +} + +// A DoH status other than 200 is dns_transport naming the status, and the +// certificate is still recorded. +func TestDNSDoHServerError(t *testing.T) { + cert, roots, notAfter := testCert(t, testHost) + doh := dohServer(t, cert, func(w http.ResponseWriter, _ dnsmessage.Message) { + http.Error(w, "upstream down", http.StatusInternalServerError) + }) + p := dnsProber(t, nil, doh.Listener.Addr(), roots) + res := p.probe(context.Background(), dnsMonitor("https://"+testHost+"/dns-query")) + if res.Success || failureCode(res) != model.DNSFailureTransport || !strings.Contains(res.Error, "500") || res.Reason != "" || res.DNSRcode != "" { + t.Fatalf("DoH 500: %+v", res) + } + if !res.CertNotAfter.Equal(notAfter) { + t.Fatalf("the handshake completed; cert_not_after = %v", res.CertNotAfter) + } +} + +// A certificate for another name fails the handshake, over DoT and DoH, +// before any query is sent, and records no expiry. +func TestDNSCertificateForAnotherName(t *testing.T) { + cert, roots, _ := testCert(t, "other.example") + dot := newDNSStub(t, &tls.Config{Certificates: []tls.Certificate{cert}}, answerA("192.0.2.1")) + var dohQueries atomic.Int32 + doh := dohServer(t, cert, func(w http.ResponseWriter, q dnsmessage.Message) { + dohQueries.Add(1) + w.Header().Set("Content-Type", "application/dns-message") + _, _ = w.Write(answerA("192.0.2.1")("tcp", q)) + }) + for name, tc := range map[string]struct { + p *prober + target string + }{ + "dot": {dnsProber(t, nil, dot.tcp.Addr(), roots), "tls://" + testHost}, + "doh": {dnsProber(t, nil, doh.Listener.Addr(), roots), "https://" + testHost + "/dns-query"}, + } { + res := tc.p.probe(context.Background(), dnsMonitor(tc.target)) + if res.Success || failureCode(res) != model.DNSFailureTransport || !strings.Contains(res.Error, "certificate") || !res.CertNotAfter.IsZero() { + t.Fatalf("%s: a certificate for other.example was accepted for %s: %+v", name, testHost, res) + } + } + if dot.queries.Load() != 0 || dohQueries.Load() != 0 { + t.Fatalf("a query reached a resolver whose certificate failed: dot=%d doh=%d", dot.queries.Load(), dohQueries.Load()) + } +} + +// A resolver slower than the timeout fails within the timeout, over UDP +// and DoH alike. +func TestDNSSlowerThanTimeout(t *testing.T) { + release := make(chan struct{}) + t.Cleanup(func() { close(release) }) + slow := newDNSStub(t, nil, func(network string, q dnsmessage.Message) []byte { + <-release + return nil + }) + cert, roots, _ := testCert(t, testHost) + doh := dohServer(t, cert, func(w http.ResponseWriter, _ dnsmessage.Message) { + select { + case <-release: + case <-time.After(5 * time.Second): + } + }) + for name, tc := range map[string]struct { + p *prober + target string + }{ + "udp": {dnsProber(t, slow.udp.LocalAddr(), slow.tcp.Addr(), nil), "udp://" + testHost}, + "tcp": {dnsProber(t, slow.udp.LocalAddr(), slow.tcp.Addr(), nil), "tcp://" + testHost}, + "doh": {dnsProber(t, nil, doh.Listener.Addr(), roots), "https://" + testHost + "/dns-query"}, + } { + m := dnsMonitor(tc.target) + m.TimeoutSec = 1 + start := time.Now() + res := tc.p.probe(context.Background(), m) + if took := time.Since(start); took > 1500*time.Millisecond { + t.Fatalf("%s: the run took %s past a 1 s timeout", name, took) + } + if res.Success || failureCode(res) != model.DNSFailureTransport || !strings.Contains(res.Error, "timeout") { + t.Fatalf("%s: slow resolver: %+v", name, res) + } + } +} + +// A DoH body over 64 KiB is refused, whether or not it declares its +// length. +func TestDNSResponseOver64KiB(t *testing.T) { + cert, roots, _ := testCert(t, testHost) + for _, declared := range []bool{true, false} { + doh := dohServer(t, cert, func(w http.ResponseWriter, q dnsmessage.Message) { + body := append(answerA("192.0.2.1")("tcp", q), make([]byte, model.MonitorDNSResponseMax)...) + w.Header().Set("Content-Type", "application/dns-message") + if declared { + w.Header().Set("Content-Length", strconv.Itoa(len(body))) + } else { + w.(http.Flusher).Flush() + } + _, _ = w.Write(body) + }) + p := dnsProber(t, nil, doh.Listener.Addr(), roots) + res := p.probe(context.Background(), dnsMonitor("https://"+testHost+"/dns-query")) + if res.Success || failureCode(res) != model.DNSFailureTransport || !strings.Contains(res.Error, "64 KiB") { + t.Fatalf("declared=%v: oversized response: %+v", declared, res) + } + } +} + +// A DoH answer that is not a DNS message (a wrong path serving HTML) is a +// transport failure naming the content type. +func TestDNSDoHWrongContentType(t *testing.T) { + cert, roots, _ := testCert(t, testHost) + doh := dohServer(t, cert, func(w http.ResponseWriter, _ dnsmessage.Message) { + w.Header().Set("Content-Type", "text/html") + _, _ = w.Write([]byte("")) + }) + p := dnsProber(t, nil, doh.Listener.Addr(), roots) + res := p.probe(context.Background(), dnsMonitor("https://"+testHost+"/dns-query")) + if res.Success || failureCode(res) != model.DNSFailureTransport || !strings.Contains(res.Error, "text/html") { + t.Fatalf("html answer: %+v", res) + } +} + +// A resolver whose name resolves to a private address is refused before +// any query: the destination_refused reason, not a failure, and nothing +// dialed. +func TestDNSPrivateResolverRefusedBeforeQuery(t *testing.T) { + for _, target := range []string{"udp://internal.example", "tls://internal.example", "https://internal.example/dns-query", "tcp://10.0.0.5", "udp://169.254.169.254"} { + p, _, dialer := fakeNetProber(map[string][]string{"internal.example": {"10.0.0.5"}}) + res := p.probe(context.Background(), dnsMonitor(target)) + if res.Reason != model.PathReasonDestinationRefused || res.Success || res.DNSRcode != "" { + t.Fatalf("%s: %+v", target, res) + } + if got := dialer.addresses(); len(got) != 0 { + t.Fatalf("%s: dialed %v", target, got) + } + } +} + +// An answer that does not echo the query's ID is ignored, so a stray or +// spoofed datagram cannot pass the check. +func TestDNSUDPIgnoresAnswerToAnotherQuery(t *testing.T) { + stub := newDNSStub(t, nil, func(_ string, q dnsmessage.Message) []byte { + q.ID++ + return answerA("192.0.2.1")("udp", q) + }) + p := dnsProber(t, stub.udp.LocalAddr(), stub.tcp.Addr(), nil) + m := dnsMonitor("udp://" + testHost) + m.TimeoutSec = 1 + if res := p.probe(context.Background(), m); res.Success || failureCode(res) != model.DNSFailureTransport { + t.Fatalf("an answer with another ID was taken: %+v", res) + } +} + +// Answers are reported bounded: at most 16, each at most 255 bytes, while +// the judgement still sees every answer. +func TestDNSAnswersReportedBounded(t *testing.T) { + long := strings.Repeat("x", 250) + stub := newDNSStub(t, nil, func(network string, q dnsmessage.Message) []byte { + if q.Questions[0].Type == dnsmessage.TypeTXT { + return dnsReply(q, dnsmessage.RCodeSuccess, false, dnsmessage.Resource{Header: rrHeader(q, dnsmessage.TypeTXT), + Body: &dnsmessage.TXTResource{TXT: []string{long, long}}}) + } + var addrs []string + for i := 1; i <= 20; i++ { + addrs = append(addrs, "192.0.2."+strconv.Itoa(i)) + } + return answerA(addrs...)(network, q) + }) + p := dnsProber(t, stub.udp.LocalAddr(), stub.tcp.Addr(), nil) + m := dnsMonitor("tcp://" + testHost) + m.DNSExpect = []string{"192.0.2.20"} + res := p.probe(context.Background(), m) + if !res.Success || len(res.DNSAnswers) != model.MonitorDNSAnswersMax { + t.Fatalf("20 answers: %+v", res) + } + m.DNSType, m.DNSExpect = model.DNSTypeTXT, nil + res = p.probe(context.Background(), m) + if !res.Success || len(res.DNSAnswers) != 1 || len(res.DNSAnswers[0]) != model.MonitorDNSValueMax { + t.Fatalf("a 500-byte TXT value: %+v", res) + } +} + +// The query asks with recursion desired and an EDNS(0) record; a DoH query +// carries ID 0 (RFC 8484 section 4.1). +func TestDNSQueryShape(t *testing.T) { + q, err := dnsQuestionOf(model.Monitor{DNSName: "Example.org.", DNSType: "aaaa"}) + if err != nil { + t.Fatal(err) + } + for _, doh := range []bool{false, true} { + raw, id, err := dnsQuery(q, doh) + if err != nil { + t.Fatal(err) + } + var msg dnsmessage.Message + if err := msg.Unpack(raw); err != nil { + t.Fatal(err) + } + if !msg.RecursionDesired || msg.ID != id || (doh && id != 0) || len(msg.Questions) != 1 || + msg.Questions[0].Name.String() != "example.org." || msg.Questions[0].Type != dnsmessage.TypeAAAA || + len(msg.Additionals) != 1 || msg.Additionals[0].Header.Type != dnsmessage.TypeOPT { + t.Fatalf("doh=%v query = %+v", doh, msg) + } + } +} + +// A dns check the control plane would refuse is refused by the agent too, +// as a failure that names the field, before anything is resolved. +func TestDNSInvalidCheckFails(t *testing.T) { + p, resolver, _ := fakeNetProber(map[string][]string{testHost: {testPublicAddr}}) + for name, m := range map[string]model.Monitor{ + "scheme": dnsMonitor("quic://" + testHost), + "no path": dnsMonitor("https://" + testHost), + "empty name": {Type: model.MonitorTypeDNS, Target: "udp://" + testHost}, + "type": {Type: model.MonitorTypeDNS, Target: "udp://" + testHost, DNSName: "a.example", DNSType: "HTTPS"}, + "expect": {Type: model.MonitorTypeDNS, Target: "udp://" + testHost, DNSName: "a.example", DNSExpect: []string{"not-an-address"}}, + "rcode": {Type: model.MonitorTypeDNS, Target: "udp://" + testHost, DNSName: "a.example", DNSExpectRcode: "SERVFAIL"}, + } { + res := p.probe(context.Background(), m) + if res.Success || res.Error == "" || res.Reason != "" { + t.Fatalf("%s: %+v", name, res) + } + } + if len(resolver.lookups) != 0 { + t.Fatalf("an invalid check was resolved: %v", resolver.lookups) + } +} + +// dohServer is an https server for testHost on loopback whose handler gets +// the decoded query of every POST of application/dns-message. +func dohServer(t *testing.T, cert tls.Certificate, handle func(http.ResponseWriter, dnsmessage.Message)) *httptest.Server { + t.Helper() + srv := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost || r.URL.Path != "/dns-query" || r.Header.Get("Content-Type") != "application/dns-message" { + http.Error(w, "not a DoH query", http.StatusBadRequest) + return + } + raw, err := io.ReadAll(r.Body) + var q dnsmessage.Message + if err == nil { + err = q.Unpack(raw) + } + if err != nil { + http.Error(w, "bad query", http.StatusBadRequest) + return + } + handle(w, q) + })) + srv.EnableHTTP2 = true + // A handshake the client refuses is the point of some tests. + srv.Config.ErrorLog = log.New(io.Discard, "", 0) + srv.TLS = &tls.Config{Certificates: []tls.Certificate{cert}} + srv.StartTLS() + t.Cleanup(srv.Close) + return srv +} diff --git a/internal/prober/icmp.go b/internal/prober/icmp.go index f7cb3e8..4ac9673 100644 --- a/internal/prober/icmp.go +++ b/internal/prober/icmp.go @@ -326,7 +326,7 @@ func (c *capabilities) facts() ([]string, map[string]string) { if c.checkedAt.IsZero() || c.now().Sub(c.checkedAt) >= capabilityRecheck { c.checkICMPLocked() } - methods := []string{model.MonitorTypeHTTP, model.MonitorTypeTCP} + methods := []string{model.MonitorTypeDNS, model.MonitorTypeHTTP, model.MonitorTypeTCP} refusals := map[string]string{} if c.icmpRefusal == "" { methods = append(methods, model.MonitorTypeICMP) diff --git a/internal/prober/icmp_test.go b/internal/prober/icmp_test.go index 7b0b16c..dfcf874 100644 --- a/internal/prober/icmp_test.go +++ b/internal/prober/icmp_test.go @@ -293,8 +293,8 @@ func TestICMPPermissionFallback(t *testing.T) { if slices.Contains(methods, model.MonitorTypeICMP) || refusals[model.MonitorTypeICMP] != model.PathReasonICMPNotPermitted { t.Fatalf("%v: refused run did not move icmp to refusals: methods %v refusals %v", refusal, methods, refusals) } - if !slices.Equal(methods, []string{model.MonitorTypeHTTP, model.MonitorTypeTCP}) { - t.Fatalf("tcp and http always run: %v", methods) + if !slices.Equal(methods, []string{model.MonitorTypeDNS, model.MonitorTypeHTTP, model.MonitorTypeTCP}) { + t.Fatalf("dns, tcp and http always run: %v", methods) } } } diff --git a/internal/prober/prober.go b/internal/prober/prober.go index 3613481..71ec121 100644 --- a/internal/prober/prober.go +++ b/internal/prober/prober.go @@ -1,5 +1,6 @@ // Package prober runs one monitor probe and returns a timed result: a TCP -// connect, an HTTP GET or a run of ICMP echoes (design 29 M2a). Every kind +// connect, an HTTP GET, a run of ICMP echoes (design 29 M2a) or one DNS +// query over DoH, DoT, UDP or TCP (design 32 S5a). Every kind // resolves its target once, refuses private and local destinations unless // the control plane resolved a tcp or icmp target from a node, and dials // only an address it judged. A kind this agent does not run is reported with the @@ -111,6 +112,9 @@ func (p *prober) probe(ctx context.Context, m model.Monitor) model.MonitorResult case model.MonitorTypeICMP: p.probeICMP(ctx, m, timeout, &res) return res + case model.MonitorTypeDNS: + p.probeDNS(ctx, m, timeout, &res) + return res default: res.Reason = model.PathReasonAgentTooOld res.Error = fmt.Sprintf("this agent does not run %q checks", m.Type) @@ -146,7 +150,7 @@ func (p *prober) probeTCP(ctx context.Context, m model.Monitor, timeout time.Dur if err != nil { return err } - conn, err := p.dialJudged(ctx, addrs, port) + conn, err := p.dialJudged(ctx, "tcp", addrs, port) if err != nil { return err } @@ -246,7 +250,7 @@ func (p *prober) httpTransport(host string, addrs []netip.Addr) *http.Transport if !strings.EqualFold(dialHost, host) { return nil, &destinationError{msg: fmt.Sprintf("destination refused: a request left for %s, not the target", dialHost)} } - return p.dialJudged(ctx, addrs, port) + return p.dialJudged(ctx, "tcp", addrs, port) }, TLSClientConfig: &tls.Config{RootCAs: p.rootCAs, MinVersion: tls.VersionTLS12}, ForceAttemptHTTP2: true, From 380443c2161fa27f4930483e1189ba0401551501 Mon Sep 17 00:00:00 2001 From: lr00rl Date: Sat, 24 Oct 2026 18:14:34 -0700 Subject: [PATCH 02/10] chore: bump to 0.3.10-alpha.10 for the dns check method The release carries the dns check of design 32 S5a (lattice-node-agent #45): DoH, DoT, UDP and TCP resolvers, the destination rule on the resolved address, the certificate expiry of DoH and DoT, and the judgement and failure codes the control plane shows. No compatibility floor moves; a server before S5a never hands the agent a dns check. The SDK pin is the S5a branch commit; it moves to the merged lattice-sdk commit before this is tagged. --- CHANGELOG.md | 34 ++++++++++++++++++++++++++++++++++ cmd/lattice-agent/main.go | 2 +- cmd/lattice-agent/main_test.go | 4 ++-- 3 files changed, 37 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d29a3b..2759745 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,40 @@ is the constant in `cmd/lattice-agent/main.go`, prepared for the next tag but not yet tagged; the release workflow injects the tag at build time and must match it. +## 0.3.10-alpha.10 (unreleased) + +Design 32 S5a: the agent runs dns checks, which ask one resolver one question +from each source and judge the answer. This is the acceptance probe for an +encrypted resolver: one check definition from several vantage points, every +five minutes, judged on the answer and timed per source. Pins the lattice-sdk +commit that models the method (lattice-sdk #45); between the alpha.9 pin and +it the SDK adds types the agent does not read and omits zero times from task +records it sends, which every server reads as before. No compatibility floor +moves: a server before S5a drops dns from the agent's probe methods and never +hands it a dns check, and a source on alpha.9 or older reads agent_too_old for +a dns check until it updates through its own approval. + +What changes on a node: + +- dns appears in probe_methods. The check's target scheme picks the transport: + https:// posts application/dns-message (RFC 8484), tls:// frames the message + behind a two-byte length inside TLS (RFC 7858, port 853 by default), and + udp:// and tcp:// are plain DNS (port 53). A truncated UDP answer is asked + again once over TCP. +- The resolver's host is resolved once and every address judged by the rule + http and tcp checks follow, so a resolver that resolves to a loopback, + link-local, metadata, multicast or private address is refused with + destination_refused before anything is sent. A DoH request goes only to the + judged addresses, through no proxy, and follows no redirect. +- One query per run, with recursion desired and EDNS(0) at 1232 bytes. The + check's timeout bounds the whole exchange, a DoH body over 64 KiB is refused + unread, and latency runs from the dial to the decoded answer. +- DoH and DoT verify the resolver's certificate against its host name and + report its expiry, which the control plane lists under Upcoming. +- A failed run's error starts with dns_rcode, dns_mismatch, dns_no_answer or + dns_transport, and the result carries the response code and at most 16 + answers of at most 255 bytes. + ## 0.3.10-alpha.9 (2026-10-24) Security hardening found when CI's gosec step, which had been scanning no diff --git a/cmd/lattice-agent/main.go b/cmd/lattice-agent/main.go index b13966e..bddaf89 100644 --- a/cmd/lattice-agent/main.go +++ b/cmd/lattice-agent/main.go @@ -48,7 +48,7 @@ import ( "github.com/LatticeNet/lattice-sdk/model" ) -var version = "0.3.10-alpha.9" +var version = "0.3.10-alpha.10" var compatServerMin = "v0.2.2-alpha.19" var compatDashboardMin = "v0.2.2-alpha.7" var compatChannel = "alpha" diff --git a/cmd/lattice-agent/main_test.go b/cmd/lattice-agent/main_test.go index 8d9fad1..cc936cd 100644 --- a/cmd/lattice-agent/main_test.go +++ b/cmd/lattice-agent/main_test.go @@ -53,8 +53,8 @@ func (r *countingTaskRunner) Run(task model.Task) model.TaskResult { // node look like it never took the update it just took. Pinning it here means // the bump is a deliberate edit rather than something that drifts. func TestVersionMatchesCurrentRelease(t *testing.T) { - if version != "0.3.10-alpha.9" { - t.Fatalf("version = %q, want 0.3.10-alpha.9", version) + if version != "0.3.10-alpha.10" { + t.Fatalf("version = %q, want 0.3.10-alpha.10", version) } } From c5eac07740ec6a56e748b6d62725daa9ba2c0490 Mon Sep 17 00:00:00 2001 From: lr00rl Date: Sat, 24 Oct 2026 18:17:09 -0700 Subject: [PATCH 03/10] fix: read a truncated dns answer that cannot be retried as a transport failure A review found that only the UDP path acted on the truncation bit: a truncated answer over TCP, DoT or DoH, or a second one after the TCP retry, reached the judgement with an empty response code and failed as dns_rcode with no code. It now fails as dns_transport, "the response is truncated". The same pass tightens three smaller things. A response that echoes a question must echo the one asked, so an answer carrying the right ID for another name is refused. A DoH answer's ID is not judged, since HTTP binds it to its request and RFC 8484 sends 0. The TCP retry goes to the address the truncated UDP answer came from. A reported answer drops invalid UTF-8 before it is cut, at a character boundary, so it never grows past 255 bytes. Tested: the new cases fail before the change; go test -race ./internal/prober/; gosec. --- internal/prober/dns.go | 54 +++++++++++++++++++++++++---------- internal/prober/dns_test.go | 57 +++++++++++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+), 15 deletions(-) diff --git a/internal/prober/dns.go b/internal/prober/dns.go index f93601f..ba7a93b 100644 --- a/internal/prober/dns.go +++ b/internal/prober/dns.go @@ -161,18 +161,26 @@ func (p *prober) probeDNS(ctx context.Context, m model.Monitor, timeout time.Dur default: raw, err = p.exchangeUDP(ctx, addrs, strconv.Itoa(resolver.Port), query, id) } + // A DoH answer is bound to its request by HTTP, and its ID is 0. + checkID := resolver.Transport != model.DNSTransportDoH var answer dnsAnswer if err == nil { - answer, err = dnsDecode(raw, id, q) + answer, err = dnsDecode(raw, id, checkID, q) } if err == nil && answer.truncated && resolver.Transport == model.DNSTransportUDP { // The one retry a run makes: the same question over TCP to the - // same resolver, as a stub resolver does with a truncated answer. - raw, err = p.exchangeTCP(ctx, addrs, strconv.Itoa(resolver.Port), query) + // address the UDP answer came from, as a stub resolver does with a + // truncated answer. + raw, err = p.exchangeTCP(ctx, addrs[:1], strconv.Itoa(resolver.Port), query) if err == nil { - answer, err = dnsDecode(raw, id, q) + answer, err = dnsDecode(raw, id, checkID, q) } } + if err == nil && answer.truncated { + // Nothing is asked again: a truncated answer over TCP, DoT or DoH, + // or a second one after the retry, is not an answer to judge. + err = &dnsFailure{code: model.DNSFailureTransport, msg: "the response is truncated"} + } res.LatencyMs = elapsedMs(time.Since(start)) if err != nil { var refused *destinationError @@ -222,17 +230,22 @@ func (q dnsQuestion) judge(a dnsAnswer) *dnsFailure { // dnsReported is what a result carries of the answer: at most // model.MonitorDNSAnswersMax values of at most model.MonitorDNSValueMax -// bytes each, cut at a character boundary. +// bytes each, invalid UTF-8 dropped and cut at a character boundary. func dnsReported(values []string) []string { if len(values) == 0 { return nil } out := make([]string, 0, min(len(values), model.MonitorDNSAnswersMax)) for _, v := range values[:min(len(values), model.MonitorDNSAnswersMax)] { + v = strings.ToValidUTF8(v, "") if len(v) > model.MonitorDNSValueMax { - v = v[:model.MonitorDNSValueMax] + cut := model.MonitorDNSValueMax + for cut > 0 && !utf8.RuneStart(v[cut]) { + cut-- + } + v = v[:cut] } - out = append(out, strings.ToValidUTF8(v, string(utf8.RuneError))) + out = append(out, v) } return out } @@ -274,17 +287,19 @@ func dnsQuery(q dnsQuestion, doh bool) ([]byte, uint16, error) { return msg, id, err } -// dnsDecode reads a response to the query with id: its response code, -// extended by an EDNS(0) record when there is one, and the values of -// answers of the asked type and class. -func dnsDecode(raw []byte, id uint16, q dnsQuestion) (dnsAnswer, error) { +// dnsDecode reads a response to the query with id (judged when checkID): +// its response code, extended by an EDNS(0) record when there is one, and +// the values of answers of the asked type and class. A response that +// echoes a question must echo the one asked. +func dnsDecode(raw []byte, id uint16, checkID bool, q dnsQuestion) (dnsAnswer, error) { var parser dnsmessage.Parser h, err := parser.Start(raw) if err != nil { return dnsAnswer{}, &dnsFailure{code: model.DNSFailureTransport, msg: "the response is not a DNS message: " + err.Error()} } - if !h.Response || h.ID != id { - return dnsAnswer{}, &dnsFailure{code: model.DNSFailureTransport, msg: "the response does not answer the query"} + notOurs := &dnsFailure{code: model.DNSFailureTransport, msg: "the response does not answer the query"} + if !h.Response || (checkID && h.ID != id) { + return dnsAnswer{}, notOurs } a := dnsAnswer{truncated: h.Truncated} if a.truncated { @@ -293,8 +308,17 @@ func dnsDecode(raw []byte, id uint16, q dnsQuestion) (dnsAnswer, error) { malformed := func(err error) error { return &dnsFailure{code: model.DNSFailureTransport, msg: "the response is malformed: " + err.Error()} } - if err := parser.SkipAllQuestions(); err != nil { - return dnsAnswer{}, malformed(err) + for { + question, err := parser.Question() + if errors.Is(err, dnsmessage.ErrSectionDone) { + break + } + if err != nil { + return dnsAnswer{}, malformed(err) + } + if dnsName(question.Name) != q.name || question.Type != q.qtype || question.Class != dnsmessage.ClassINET { + return dnsAnswer{}, notOurs + } } for { ah, err := parser.AnswerHeader() diff --git a/internal/prober/dns_test.go b/internal/prober/dns_test.go index 5b4dea7..eecdf4b 100644 --- a/internal/prober/dns_test.go +++ b/internal/prober/dns_test.go @@ -23,6 +23,7 @@ import ( "sync/atomic" "testing" "time" + "unicode/utf8" "github.com/LatticeNet/lattice-sdk/model" "golang.org/x/net/dns/dnsmessage" @@ -633,3 +634,59 @@ func dohServer(t *testing.T, cert tls.Certificate, handle func(http.ResponseWrit t.Cleanup(srv.Close) return srv } + +// A truncated answer that cannot be asked again (TCP, DoT, DoH, or the TCP +// retry itself) is a transport failure, never a response code. +func TestDNSTruncatedWithoutRetryIsTransport(t *testing.T) { + stub := newDNSStub(t, nil, func(_ string, q dnsmessage.Message) []byte { + return dnsReply(q, dnsmessage.RCodeSuccess, true) + }) + p := dnsProber(t, stub.udp.LocalAddr(), stub.tcp.Addr(), nil) + for _, target := range []string{"tcp://" + testHost, "udp://" + testHost} { + m := dnsMonitor(target) + m.DNSExpectRcode = model.DNSRcodeNXDomain + res := p.probe(context.Background(), m) + if res.Success || failureCode(res) != model.DNSFailureTransport || !strings.Contains(res.Error, "truncated") || res.DNSRcode != "" { + t.Fatalf("%s: a truncated answer = %+v", target, res) + } + } +} + +// A response that echoes the ID but answers another question is refused. +func TestDNSAnswerToAnotherQuestionIsRefused(t *testing.T) { + stub := newDNSStub(t, nil, func(network string, q dnsmessage.Message) []byte { + q.Questions[0].Name = dnsmessage.MustNewName("other.example.") + return answerA("192.0.2.1")(network, q) + }) + p := dnsProber(t, stub.udp.LocalAddr(), stub.tcp.Addr(), nil) + res := p.probe(context.Background(), dnsMonitor("tcp://"+testHost)) + if res.Success || failureCode(res) != model.DNSFailureTransport { + t.Fatalf("an answer to another question was taken: %+v", res) + } +} + +// A DoH response is bound to its request by HTTP, so its ID is not judged. +func TestDNSDoHAcceptsAnyResponseID(t *testing.T) { + cert, roots, _ := testCert(t, testHost) + doh := dohServer(t, cert, func(w http.ResponseWriter, q dnsmessage.Message) { + q.ID = 4242 + w.Header().Set("Content-Type", "application/dns-message") + _, _ = w.Write(answerA("192.0.2.1")("tcp", q)) + }) + p := dnsProber(t, nil, doh.Listener.Addr(), roots) + if res := p.probe(context.Background(), dnsMonitor("https://"+testHost+"/dns-query")); !res.Success { + t.Fatalf("a DoH answer with another ID = %+v", res) + } +} + +// A reported value is cut at a character boundary and never grows past the bound. +func TestDNSReportedValuesKeepTheirBound(t *testing.T) { + v := strings.Repeat("a", model.MonitorDNSValueMax-1) + "é" + got := dnsReported([]string{v, "ok\xff"}) + if len(got[0]) > model.MonitorDNSValueMax || !utf8.ValidString(got[0]) || got[0] != strings.Repeat("a", model.MonitorDNSValueMax-1) { + t.Fatalf("cut value = %q (%d bytes)", got[0], len(got[0])) + } + if got[1] != "ok" { + t.Fatalf("invalid bytes = %q", got[1]) + } +} From 72cdeafeea6688f5f35ff7a27add8fcaba153fde Mon Sep 17 00:00:00 2001 From: lr00rl Date: Sat, 24 Oct 2026 18:45:43 -0700 Subject: [PATCH 04/10] refactor: name the per-address time slice and the DoH client dialJudged's share of the deadline per address becomes addrContext, so the UDP exchange can try addresses on the same terms, and exchangeDoH builds its client through dohClient, the seam the DNS tests use to pin that a DoH request dials only its target. No behavior changes. --- internal/prober/destination.go | 28 ++++++++++++++++++---------- internal/prober/dns.go | 18 ++++++++++++------ 2 files changed, 30 insertions(+), 16 deletions(-) diff --git a/internal/prober/destination.go b/internal/prober/destination.go index 21f7aa6..866ab4a 100644 --- a/internal/prober/destination.go +++ b/internal/prober/destination.go @@ -83,6 +83,23 @@ func asciiHost(host string) (string, error) { // net.Dialer gives each address of a serial fallback. const minDialSlice = 2 * time.Second +// addrContext is the share of ctx's time one address gets while left +// addresses, itself included, remain to be tried: an equal share of what is +// left, at least minDialSlice (or all of it when less remains), and all of +// it for the last address. +func addrContext(ctx context.Context, left int) (context.Context, context.CancelFunc) { + if left > 1 { + if deadline, ok := ctx.Deadline(); ok { + slice := time.Until(deadline) / time.Duration(left) + if slice < minDialSlice { + slice = min(minDialSlice, time.Until(deadline)) + } + return context.WithTimeout(ctx, slice) + } + } + return ctx, func() {} +} + // dialJudged connects over network (tcp or udp) to the first of addrs that // answers on port, in the order the resolver gave them, sharing the time // left between the addresses not yet tried as net.Dialer does. addrs are @@ -93,16 +110,7 @@ func (p *prober) dialJudged(ctx context.Context, network string, addrs []netip.A } var firstErr error for i, addr := range addrs { - dctx, cancel := ctx, context.CancelFunc(func() {}) - if left := len(addrs) - i; left > 1 { - if deadline, ok := ctx.Deadline(); ok { - slice := time.Until(deadline) / time.Duration(left) - if slice < minDialSlice { - slice = min(minDialSlice, time.Until(deadline)) - } - dctx, cancel = context.WithTimeout(ctx, slice) - } - } + dctx, cancel := addrContext(ctx, len(addrs)-i) conn, err := p.dial(dctx, network, net.JoinHostPort(addr.String(), port)) cancel() if err == nil { diff --git a/internal/prober/dns.go b/internal/prober/dns.go index ba7a93b..2f45f77 100644 --- a/internal/prober/dns.go +++ b/internal/prober/dns.go @@ -489,12 +489,8 @@ func dnsStreamExchange(conn net.Conn, query []byte) ([]byte, error) { // redirect, the certificate verified against the host. A body over // model.MonitorDNSResponseMax is refused rather than read. func (p *prober) exchangeDoH(ctx context.Context, r model.DNSResolver, addrs []netip.Addr, query []byte, res *model.MonitorResult) ([]byte, error) { - transport := p.httpTransport(r.Host, addrs) - defer transport.CloseIdleConnections() - client := &http.Client{ - Transport: transport, - CheckRedirect: func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse }, - } + client := p.dohClient(r.Host, addrs) + defer client.CloseIdleConnections() req, err := http.NewRequestWithContext(ctx, http.MethodPost, r.URL(), bytes.NewReader(query)) if err != nil { return nil, err @@ -537,6 +533,16 @@ func (p *prober) exchangeDoH(ctx context.Context, r model.DNSResolver, addrs []n return body, nil } +// dohClient is what a DoH exchange posts through: the http check's +// transport, which dials only the judged addresses of host and takes no +// proxy from the environment, and a redirect reported rather than followed. +func (p *prober) dohClient(host string, addrs []netip.Addr) *http.Client { + return &http.Client{ + Transport: p.httpTransport(host, addrs), + CheckRedirect: func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse }, + } +} + // unwrapURLError drops the method and URL net/http puts before a request // error; the check's target already names them. func unwrapURLError(err error) error { From c08bfd079fb3d9b728e55d6d12908cfe89021dab Mon Sep 17 00:00:00 2001 From: lr00rl Date: Sat, 24 Oct 2026 18:46:50 -0700 Subject: [PATCH 05/10] test: pin the dns probe's destination protections in its own tests The DoH and resolver protections were covered only through the http check's tests, which would not notice the DNS probe stepping around the shared transport. Four tests now pin them on the DNS path: a DoH 3xx to loopback, a private range or the metadata service is reported and not followed; a proxy in the environment is ignored (the transport is checked too, since net/http reads the environment once per process); a DoH request for any host but the target is refused before a dial; and a resolver name with one private address among public ones is refused before any query, in either order and over every transport. Tested: each test fails with its protection removed (CheckRedirect dropped, Proxy set to ProxyFromEnvironment, the host check disabled, only the first address judged) and passes with it in place. --- internal/prober/dns_test.go | 117 ++++++++++++++++++++++++++++++++++++ 1 file changed, 117 insertions(+) diff --git a/internal/prober/dns_test.go b/internal/prober/dns_test.go index eecdf4b..40ee4b4 100644 --- a/internal/prober/dns_test.go +++ b/internal/prober/dns_test.go @@ -9,6 +9,7 @@ import ( "crypto/x509" "crypto/x509/pkix" "encoding/binary" + "errors" "fmt" "io" "log" @@ -690,3 +691,119 @@ func TestDNSReportedValuesKeepTheirBound(t *testing.T) { t.Fatalf("invalid bytes = %q", got[1]) } } + +// A DoH 3xx is the run's failure and is never followed, wherever it +// points: loopback, a private range, the metadata service. Nothing reaches +// the address it names. +func TestDNSDoHRedirectIsReportedNotFollowed(t *testing.T) { + var landed atomic.Int32 + inside := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { landed.Add(1) })) + t.Cleanup(inside.Close) + cert, roots, _ := testCert(t, testHost) + for _, status := range []int{http.StatusMovedPermanently, http.StatusFound, http.StatusTemporaryRedirect, http.StatusPermanentRedirect} { + for _, location := range []string{inside.URL + "/dns-query", "http://10.0.0.5/dns-query", "http://169.254.169.254/latest/meta-data/"} { + doh := dohServer(t, cert, func(w http.ResponseWriter, _ dnsmessage.Message) { + w.Header().Set("Location", location) + w.WriteHeader(status) + }) + p := dnsProber(t, nil, doh.Listener.Addr(), roots) + res := p.probe(context.Background(), dnsMonitor("https://"+testHost+"/dns-query")) + if res.Success || res.Reason != "" || failureCode(res) != model.DNSFailureTransport || !strings.Contains(res.Error, "http status "+strconv.Itoa(status)) { + t.Fatalf("%d to %s: want a reported redirect, got %+v", status, location, res) + } + } + } + if n := landed.Load(); n != 0 { + t.Fatalf("a redirect was followed to loopback %d times", n) + } +} + +// A proxy named in the environment is never used for DoH: the query goes +// to the resolver's judged address and the proxy sees no connection. +func TestDNSDoHIgnoresEnvironmentProxy(t *testing.T) { + proxy, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = proxy.Close() }) + var proxied atomic.Int32 + go func() { + for { + conn, err := proxy.Accept() + if err != nil { + return + } + proxied.Add(1) + _ = conn.Close() + } + }() + for _, name := range []string{"HTTPS_PROXY", "https_proxy", "HTTP_PROXY", "http_proxy"} { + t.Setenv(name, "http://"+proxy.Addr().String()) + } + cert, roots, _ := testCert(t, testHost) + doh := dohServer(t, cert, func(w http.ResponseWriter, q dnsmessage.Message) { + w.Header().Set("Content-Type", "application/dns-message") + _, _ = w.Write(answerA("192.0.2.1")("tcp", q)) + }) + p := dnsProber(t, nil, doh.Listener.Addr(), roots) + // net/http reads the environment's proxy once per process, so the + // transport is checked as well as the run. + if tr, ok := p.dohClient(testHost, nil).Transport.(*http.Transport); !ok || tr.Proxy != nil { + t.Fatal("the DoH transport carries a proxy function") + } + res := p.probe(context.Background(), dnsMonitor("https://"+testHost+"/dns-query")) + if !res.Success { + t.Fatalf("DoH with a proxy in the environment: %+v", res) + } + if n := proxied.Load(); n != 0 { + t.Fatalf("the proxy saw %d connections", n) + } +} + +// A DoH request leaves only for the resolver's host. A request for any +// other host, which is what a followed redirect or a proxy would make, is +// refused before anything is dialed. +func TestDNSDoHDialsOnlyTheTarget(t *testing.T) { + p, _, d := fakeNetProber(nil) + client := p.dohClient(testHost, []netip.Addr{netip.MustParseAddr(testPublicAddr)}) + for _, u := range []string{"https://other.example/dns-query", "https://127.0.0.1/dns-query", "http://169.254.169.254/latest/meta-data/"} { + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + req, err := http.NewRequestWithContext(ctx, http.MethodPost, u, nil) + if err != nil { + t.Fatal(err) + } + resp, err := client.Do(req) + cancel() + if resp != nil { + _ = resp.Body.Close() + } + var refused *destinationError + if !errors.As(err, &refused) { + t.Fatalf("%s: want destination refused, got %v", u, err) + } + } + if got := d.addresses(); len(got) != 0 { + t.Fatalf("a request for another host dialed %v", got) + } +} + +// A resolver name with a private address among its public ones is refused +// before any query, whichever order the addresses come in and over every +// transport: one refused address refuses the run, and nothing is dialed. +func TestDNSResolverWithOnePrivateAddressRefused(t *testing.T) { + for _, addrs := range [][]string{{testPublicAddr, "10.0.0.5"}, {"10.0.0.5", testPublicAddr}} { + for _, target := range []string{"udp://mixed.example", "tcp://mixed.example", "tls://mixed.example", "https://mixed.example/dns-query"} { + p, r, d := fakeNetProber(map[string][]string{"mixed.example": addrs}) + res := p.probe(context.Background(), dnsMonitor(target)) + if res.Reason != model.PathReasonDestinationRefused || res.Success || !strings.Contains(res.Error, "10.0.0.5") { + t.Fatalf("%s resolving to %v: %+v", target, addrs, res) + } + if got := d.addresses(); len(got) != 0 { + t.Fatalf("%s resolving to %v: dialed %v", target, addrs, got) + } + if !slices.Equal(r.lookups, []string{"mixed.example"}) { + t.Fatalf("%s: lookups %v, want one", target, r.lookups) + } + } + } +} From e0a3c267ba32415f948c50db91eba6d82bfe1e1b Mon Sep 17 00:00:00 2001 From: lr00rl Date: Sat, 24 Oct 2026 18:48:27 -0700 Subject: [PATCH 06/10] fix: keep a resolver's own text in a dns error short A DoH answer with the wrong content type echoed the whole header, so a resolver could put up to 64 KiB of its own text into the result's error, which every node ships to the control plane on each run and the server stores. The error now names only the parsed media type, cut to 64 bytes. The other places a resolver's text reached the error are bounded the same way: a TLS failure names the host asked for and at most three of the certificate's names, 64 bytes each, instead of the x509 error's full list; a transport error, which can quote the resolver's bytes (a garbled HTTP status line, HTTP/2 GOAWAY debug data), is cut to 160 bytes; and the whole error is cut to 512 bytes, the server's bound on a stored error, so a node never sends text the server would cut. Reason only ever carries constants, decode errors are fixed strings, and answers already pass through dnsReported's bounds. Tested: TestDNSResolverTextInErrorsIsBounded failed before the fix (a 61513-byte error) and passes after. --- internal/prober/dns.go | 90 +++++++++++++++++++++++++++++++------ internal/prober/dns_test.go | 61 +++++++++++++++++++++++++ 2 files changed, 137 insertions(+), 14 deletions(-) diff --git a/internal/prober/dns.go b/internal/prober/dns.go index 2f45f77..f71248b 100644 --- a/internal/prober/dns.go +++ b/internal/prober/dns.go @@ -87,6 +87,30 @@ func dnsQuestionOf(m model.Monitor) (dnsQuestion, error) { return q, nil } +// Bounds on the text a result's error takes from the resolver's side: a +// header value or certificate name is quoted at most dnsQuotedMax bytes, a +// transport error (which can quote the resolver's bytes) at most +// dnsDetailMax, and the whole error at most dnsErrorMax, the server's bound +// on a stored error, so a node never sends text the server would cut. +const ( + dnsQuotedMax = 64 + dnsDetailMax = 160 + dnsErrorMax = 512 +) + +// cutUTF8 is s with invalid UTF-8 dropped, cut to at most n bytes at a +// character boundary. +func cutUTF8(s string, n int) string { + s = strings.ToValidUTF8(s, "") + if len(s) <= n { + return s + } + for n > 0 && !utf8.RuneStart(s[n]) { + n-- + } + return s[:n] +} + // dnsFailure is a run that failed with a model.DNSFailure* code. type dnsFailure struct { code string @@ -103,7 +127,42 @@ func dnsTransportError(err error) *dnsFailure { if errors.Is(err, context.DeadlineExceeded) || errors.Is(err, context.Canceled) || isTimeout(err) { return &dnsFailure{code: model.DNSFailureTransport, msg: "no answer before the check's timeout"} } - return &dnsFailure{code: model.DNSFailureTransport, msg: err.Error()} + return &dnsFailure{code: model.DNSFailureTransport, msg: cutUTF8(err.Error(), dnsDetailMax)} +} + +// dnsCertNamesShown is how many of a certificate's names a failure lists. +const dnsCertNamesShown = 3 + +// dnsTLSFailure names a refused handshake. A certificate's names are the +// resolver's to choose, so a few are listed, each cut short. +func dnsTLSFailure(err error) *dnsFailure { + var hostErr x509.HostnameError + var authErr x509.UnknownAuthorityError + var msg string + switch { + case errors.As(err, &hostErr) && hostErr.Certificate != nil: + msg = fmt.Sprintf("the certificate does not cover %s (it names %s)", hostErr.Host, dnsCertNames(hostErr.Certificate)) + case errors.As(err, &authErr): + msg = "the certificate is signed by an unknown authority" + default: + msg = cutUTF8(strings.TrimPrefix(err.Error(), "tls: "), dnsDetailMax) + } + return &dnsFailure{code: model.DNSFailureTransport, msg: "tls: " + msg} +} + +func dnsCertNames(c *x509.Certificate) string { + if len(c.DNSNames) == 0 { + return "no DNS names" + } + shown := make([]string, 0, dnsCertNamesShown) + for _, name := range c.DNSNames[:min(len(c.DNSNames), dnsCertNamesShown)] { + shown = append(shown, cutUTF8(name, dnsQuotedMax)) + } + out := strings.Join(shown, ", ") + if more := len(c.DNSNames) - len(shown); more > 0 { + out += fmt.Sprintf(" and %d more", more) + } + return out } func isTimeout(err error) bool { @@ -120,6 +179,7 @@ type dnsAnswer struct { } func (p *prober) probeDNS(ctx context.Context, m model.Monitor, timeout time.Duration, res *model.MonitorResult) { + defer func() { res.Error = cutUTF8(res.Error, dnsErrorMax) }() resolver, err := model.ParseDNSResolver(m.Target) if err != nil { res.Error = err.Error() @@ -237,15 +297,7 @@ func dnsReported(values []string) []string { } out := make([]string, 0, min(len(values), model.MonitorDNSAnswersMax)) for _, v := range values[:min(len(values), model.MonitorDNSAnswersMax)] { - v = strings.ToValidUTF8(v, "") - if len(v) > model.MonitorDNSValueMax { - cut := model.MonitorDNSValueMax - for cut > 0 && !utf8.RuneStart(v[cut]) { - cut-- - } - v = v[:cut] - } - out = append(out, v) + out = append(out, cutUTF8(v, model.MonitorDNSValueMax)) } return out } @@ -457,7 +509,7 @@ func (p *prober) exchangeDoT(ctx context.Context, r model.DNSResolver, addrs []n if ctx.Err() != nil || isTimeout(err) { return nil, err } - return nil, &dnsFailure{code: model.DNSFailureTransport, msg: "tls: " + strings.TrimPrefix(err.Error(), "tls: ")} + return nil, dnsTLSFailure(err) } recordCert(conn.ConnectionState().PeerCertificates, res) return dnsStreamExchange(conn, query) @@ -505,7 +557,7 @@ func (p *prober) exchangeDoH(ctx context.Context, r model.DNSResolver, addrs []n } var cert *tls.CertificateVerificationError if errors.As(err, &cert) { - return nil, &dnsFailure{code: model.DNSFailureTransport, msg: "tls: " + cert.Err.Error()} + return nil, dnsTLSFailure(cert) } return nil, unwrapURLError(err) } @@ -516,8 +568,18 @@ func (p *prober) exchangeDoH(ctx context.Context, r model.DNSResolver, addrs []n if resp.StatusCode != http.StatusOK { return nil, &dnsFailure{code: model.DNSFailureTransport, msg: fmt.Sprintf("http status %d", resp.StatusCode)} } - if media, _, _ := mime.ParseMediaType(resp.Header.Get("Content-Type")); media != dnsMediaType { - return nil, &dnsFailure{code: model.DNSFailureTransport, msg: fmt.Sprintf("http 200 with content type %q, not %s", resp.Header.Get("Content-Type"), dnsMediaType)} + // Only the parsed media type is named, cut short: the header is the + // resolver's text, up to the transport's 64 KiB header bound. + header := resp.Header.Get("Content-Type") + if media, _, _ := mime.ParseMediaType(header); media != dnsMediaType { + got := "no content type" + switch { + case media != "": + got = fmt.Sprintf("content type %q", cutUTF8(media, dnsQuotedMax)) + case header != "": + got = "a content type that does not parse" + } + return nil, &dnsFailure{code: model.DNSFailureTransport, msg: fmt.Sprintf("http 200 with %s, not %s", got, dnsMediaType)} } tooLarge := &dnsFailure{code: model.DNSFailureTransport, msg: fmt.Sprintf("the response is over %d KiB", model.MonitorDNSResponseMax>>10)} if resp.ContentLength > model.MonitorDNSResponseMax { diff --git a/internal/prober/dns_test.go b/internal/prober/dns_test.go index 40ee4b4..6c7bd78 100644 --- a/internal/prober/dns_test.go +++ b/internal/prober/dns_test.go @@ -807,3 +807,64 @@ func TestDNSResolverWithOnePrivateAddressRefused(t *testing.T) { } } } + +// Text the resolver chooses reaches a result's error only in short pieces: +// a content type, a certificate's names, an HTTP response it garbles. The +// error is shipped from every node and stored, so its size is the agent's +// to bound. +func TestDNSResolverTextInErrorsIsBounded(t *testing.T) { + long := strings.Repeat("a", 60<<10) + var names []string + for i := range 200 { + names = append(names, fmt.Sprintf("%s%d.example", strings.Repeat("n", 50), i)) + } + many, manyRoots, _ := testCert(t, names...) + cert, roots, _ := testCert(t, testHost) + contentType := func(value string) *httptest.Server { + return dohServer(t, cert, func(w http.ResponseWriter, q dnsmessage.Message) { + w.Header().Set("Content-Type", value) + _, _ = w.Write(answerA("192.0.2.1")("tcp", q)) + }) + } + garbled, err := tls.Listen("tcp", "127.0.0.1:0", &tls.Config{Certificates: []tls.Certificate{cert}}) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = garbled.Close() }) + go func() { + for { + conn, err := garbled.Accept() + if err != nil { + return + } + go func() { + defer conn.Close() + _, _ = conn.Read(make([]byte, 4096)) + _, _ = io.WriteString(conn, "XTTP/"+long[:8<<10]+"\r\n\r\n") + }() + } + }() + dot := newDNSStub(t, &tls.Config{Certificates: []tls.Certificate{many}}, answerA("192.0.2.1")) + manyDoH := dohServer(t, many, func(http.ResponseWriter, dnsmessage.Message) {}) + doh := "https://" + testHost + "/dns-query" + for name, tc := range map[string]struct { + p *prober + target string + want string + }{ + "long media type": {dnsProber(t, nil, contentType(long).Listener.Addr(), roots), doh, "content type"}, + "long parameter": {dnsProber(t, nil, contentType("text/html; x="+long).Listener.Addr(), roots), doh, `"text/html"`}, + "dot cert names": {dnsProber(t, nil, dot.tcp.Addr(), manyRoots), "tls://" + testHost, "does not cover " + testHost}, + "doh cert names": {dnsProber(t, nil, manyDoH.Listener.Addr(), manyRoots), doh, "does not cover " + testHost}, + "garbled http reply": {dnsProber(t, nil, garbled.Addr(), roots), doh, "XTTP/"}, + } { + res := tc.p.probe(context.Background(), dnsMonitor(tc.target)) + if res.Success || failureCode(res) != model.DNSFailureTransport || !strings.Contains(res.Error, tc.want) { + t.Fatalf("%s: %+v", name, res) + } + if len(res.Error) > dnsErrorMax || !utf8.ValidString(res.Error) || + strings.Contains(res.Error, strings.Repeat("a", dnsDetailMax+1)) || strings.Contains(res.Error, strings.Repeat("n", dnsQuotedMax+1)) { + t.Fatalf("%s: error of %d bytes carries the resolver's text unbounded: %.300q", name, len(res.Error), res.Error) + } + } +} From aef82438b23f7184a9e6662f8701af759e3b6998 Mon Sep 17 00:00:00 2001 From: lr00rl Date: Sat, 24 Oct 2026 18:50:04 -0700 Subject: [PATCH 07/10] fix: ask a resolver's next address over udp when one is unreachable or silent exchangeUDP dialed only the first judged address, so a healthy resolver whose first address is in a family the node cannot reach, or simply does not answer, read as a timeout. It now asks the addresses in turn, each for its share of the run's time on the same terms as dialJudged (addrContext), as TCP, DoT and DoH already fall through. An answer, right or wrong, ends the exchange at that address; a wrong answer is not a reason to ask another. A truncated answer is asked again over TCP of the address that sent it, which is no longer always the first. Tested: TestDNSUDPTriesTheNextAddress failed before the fix on the unreachable, silent and truncated cases and passes after; the wrong answer case pins that the second address is never asked. --- internal/prober/dns.go | 46 ++++++++++++---- internal/prober/dns_test.go | 107 ++++++++++++++++++++++++++++++++++++ 2 files changed, 143 insertions(+), 10 deletions(-) diff --git a/internal/prober/dns.go b/internal/prober/dns.go index f71248b..5afede1 100644 --- a/internal/prober/dns.go +++ b/internal/prober/dns.go @@ -29,9 +29,11 @@ import ( // 8484, a POST of application/dns-message), DoT (RFC 7858, the message // behind a two-byte length over TLS), or plain DNS over UDP or TCP. A run // resolves the resolver's host once and judges every address as every -// other kind does, sends one query, asks once more over TCP only when a -// UDP answer comes back truncated, and judges the answer against the -// check's expectation. The timeout bounds the whole exchange. +// other kind does, sends the query to the first address that answers (one +// that cannot be reached or stays silent passes it to the next), asks once +// more over TCP only when a UDP answer comes back truncated, and judges the +// answer against the check's expectation. The timeout bounds the whole +// exchange. // dnsUDPPayload is the EDNS(0) payload size a query advertises, the size // DNS Flag Day 2020 settled on to avoid fragmented UDP answers. @@ -211,6 +213,7 @@ func (p *prober) probeDNS(ctx context.Context, m model.Monitor, timeout time.Dur } start := time.Now() var raw []byte + var answeredBy netip.Addr switch resolver.Transport { case model.DNSTransportDoH: raw, err = p.exchangeDoH(ctx, resolver, addrs, query, res) @@ -219,7 +222,7 @@ func (p *prober) probeDNS(ctx context.Context, m model.Monitor, timeout time.Dur case model.DNSTransportTCP: raw, err = p.exchangeTCP(ctx, addrs, strconv.Itoa(resolver.Port), query) default: - raw, err = p.exchangeUDP(ctx, addrs, strconv.Itoa(resolver.Port), query, id) + raw, answeredBy, err = p.exchangeUDP(ctx, addrs, strconv.Itoa(resolver.Port), query, id) } // A DoH answer is bound to its request by HTTP, and its ID is 0. checkID := resolver.Transport != model.DNSTransportDoH @@ -231,7 +234,7 @@ func (p *prober) probeDNS(ctx context.Context, m model.Monitor, timeout time.Dur // The one retry a run makes: the same question over TCP to the // address the UDP answer came from, as a stub resolver does with a // truncated answer. - raw, err = p.exchangeTCP(ctx, addrs[:1], strconv.Itoa(resolver.Port), query) + raw, err = p.exchangeTCP(ctx, []netip.Addr{answeredBy}, strconv.Itoa(resolver.Port), query) if err == nil { answer, err = dnsDecode(raw, id, checkID, q) } @@ -458,11 +461,34 @@ func dnsRcodeName(code dnsmessage.RCode) string { return "RCODE" + strconv.Itoa(int(code)) } -// exchangeUDP sends the query to the first judged address and reads -// datagrams until one answers it. A connected socket takes datagrams from -// that address only; one that does not echo id is ignored. -func (p *prober) exchangeUDP(ctx context.Context, addrs []netip.Addr, port string, query []byte, id uint16) ([]byte, error) { - conn, err := p.dialJudged(ctx, "udp", addrs[:1], port) +// exchangeUDP asks the judged addresses in turn, each for its share of the +// run's time (addrContext), and returns the first answer and the address +// that sent it. An address that cannot be reached or stays silent passes +// the question to the next; an answer, right or wrong, ends the exchange. +func (p *prober) exchangeUDP(ctx context.Context, addrs []netip.Addr, port string, query []byte, id uint16) ([]byte, netip.Addr, error) { + var firstErr error + for i, addr := range addrs { + actx, cancel := addrContext(ctx, len(addrs)-i) + raw, err := p.askUDP(actx, addr, port, query, id) + cancel() + if err == nil { + return raw, addr, nil + } + if firstErr == nil { + firstErr = err + } + if ctx.Err() != nil { + break + } + } + return nil, netip.Addr{}, firstErr +} + +// askUDP sends the query to addr and reads datagrams until one answers it. +// A connected socket takes datagrams from that address only; one that does +// not echo id is ignored. +func (p *prober) askUDP(ctx context.Context, addr netip.Addr, port string, query []byte, id uint16) ([]byte, error) { + conn, err := p.dialJudged(ctx, "udp", []netip.Addr{addr}, port) if err != nil { return nil, err } diff --git a/internal/prober/dns_test.go b/internal/prober/dns_test.go index 6c7bd78..f770806 100644 --- a/internal/prober/dns_test.go +++ b/internal/prober/dns_test.go @@ -21,6 +21,7 @@ import ( "slices" "strconv" "strings" + "sync" "sync/atomic" "testing" "time" @@ -868,3 +869,109 @@ func TestDNSResolverTextInErrorsIsBounded(t *testing.T) { } } } + +// testFirstAddr is the address twoAddrProber gives a resolver ahead of +// testPublicAddr. +const testFirstAddr = "203.0.113.81" + +// twoAddrProber resolves testHost to testFirstAddr, then testPublicAddr, +// and carries a dial to each to its stub's socket of the same network. A +// nil stub refuses the dial, as a host without a route to that address's +// family does. dialed lists "network address" per dial, in order. +func twoAddrProber(t *testing.T, first, second *dnsStub) (p *prober, dialed func() []string) { + t.Helper() + real := &net.Dialer{} + stubs := map[string]*dnsStub{testFirstAddr: first, testPublicAddr: second} + var mu sync.Mutex + var log []string + p = newProber() + p.resolver = &fakeResolver{answers: map[string][]string{testHost: {testFirstAddr, testPublicAddr}}} + p.dial = func(ctx context.Context, network, address string) (net.Conn, error) { + host, _, err := net.SplitHostPort(address) + stub, ok := stubs[host] + if err != nil || !ok { + return nil, fmt.Errorf("test dialer: unexpected address %s", address) + } + mu.Lock() + log = append(log, network+" "+host) + mu.Unlock() + switch { + case stub == nil: + return nil, &net.OpError{Op: "dial", Net: network, Err: errors.New("network is unreachable")} + case network == "udp": + return real.DialContext(ctx, network, stub.udp.LocalAddr().String()) + } + return real.DialContext(ctx, network, stub.tcp.Addr().String()) + } + return p, func() []string { + mu.Lock() + defer mu.Unlock() + return slices.Clone(log) + } +} + +// UDP asks the resolver's next address when one cannot be reached or stays +// silent, within the run's timeout, as TCP, DoT and DoH do. An answer, even +// a wrong one, ends the run at that address, and a truncated answer is +// asked again over TCP of the address that sent it. +func TestDNSUDPTriesTheNextAddress(t *testing.T) { + good := newDNSStub(t, nil, answerA("192.0.2.1")) + silent := newDNSStub(t, nil, func(string, dnsmessage.Message) []byte { return nil }) + t.Run("unreachable", func(t *testing.T) { + p, dialed := twoAddrProber(t, nil, good) + res := p.probe(context.Background(), dnsMonitor("udp://"+testHost)) + if !res.Success { + t.Fatalf("first address unreachable: %+v", res) + } + if got, want := dialed(), []string{"udp " + testFirstAddr, "udp " + testPublicAddr}; !slices.Equal(got, want) { + t.Fatalf("dialed %v, want %v", got, want) + } + }) + t.Run("silent", func(t *testing.T) { + p, _ := twoAddrProber(t, silent, good) + m := dnsMonitor("udp://" + testHost) + m.TimeoutSec = 4 + before := silent.queries.Load() + start := time.Now() + res := p.probe(context.Background(), m) + if took := time.Since(start); !res.Success || took > 4*time.Second { + t.Fatalf("first address silent: %+v after %s", res, took) + } + if silent.queries.Load() == before { + t.Fatal("the first address was never asked") + } + }) + t.Run("wrong answer", func(t *testing.T) { + wrong := newDNSStub(t, nil, answerA("192.0.2.9")) + second := newDNSStub(t, nil, answerA("192.0.2.1")) + p, _ := twoAddrProber(t, wrong, second) + m := dnsMonitor("udp://" + testHost) + m.DNSExpect = []string{"192.0.2.1"} + res := p.probe(context.Background(), m) + if res.Success || failureCode(res) != model.DNSFailureMismatch { + t.Fatalf("a wrong answer from the first address: %+v", res) + } + if n := second.queries.Load(); n != 0 { + t.Fatalf("a wrong answer passed the question on; the second address was asked %d times", n) + } + }) + t.Run("truncated", func(t *testing.T) { + truncating := newDNSStub(t, nil, func(network string, q dnsmessage.Message) []byte { + if network == "udp" { + return dnsReply(q, dnsmessage.RCodeSuccess, true) + } + return answerA("192.0.2.1")(network, q) + }) + p, dialed := twoAddrProber(t, silent, truncating) + m := dnsMonitor("udp://" + testHost) + m.TimeoutSec = 4 + res := p.probe(context.Background(), m) + if !res.Success { + t.Fatalf("truncated answer from the second address: %+v", res) + } + got := dialed() + if !slices.Contains(got, "tcp "+testPublicAddr) || slices.Contains(got, "tcp "+testFirstAddr) { + t.Fatalf("the TCP retry did not go to the address that answered: %v", got) + } + }) +} From f4bb8c1a5ace3a087686e2c44cce24a235da7bf0 Mon Sep 17 00:00:00 2001 From: lr00rl Date: Sat, 24 Oct 2026 18:52:50 -0700 Subject: [PATCH 08/10] fix: wake a dns read in progress when its run is cancelled The UDP, TCP and DoT exchanges set the connection's deadline from the context but did not watch its cancellation, so a run cancelled at shutdown sat in its read until the check's timeout, up to the full timeout per check. bindConn replaces setDeadline: it keeps the deadline and closes the connection from context.AfterFunc when the context is cancelled. A deadline that passes is still left to the connection's own timeout, so a slow resolver reads as a timeout rather than a closed connection, and a cancelled run now says it was cancelled instead of timed out. DoH already followed its request's context. Tested: TestDNSCancelEndsTheRun failed before the fix (a TCP run cancelled after 200 ms took 20 s) and passes after over UDP, TCP, DoT and DoH; go test -race on internal/prober and cmd/lattice-agent, gosec (67 files, 0 issues) and govulncheck pass. --- internal/prober/dns.go | 29 +++++++++++++++++++------ internal/prober/dns_test.go | 42 +++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 6 deletions(-) diff --git a/internal/prober/dns.go b/internal/prober/dns.go index 5afede1..361d54e 100644 --- a/internal/prober/dns.go +++ b/internal/prober/dns.go @@ -126,7 +126,10 @@ func dnsTransportError(err error) *dnsFailure { if errors.As(err, &failure) { return failure } - if errors.Is(err, context.DeadlineExceeded) || errors.Is(err, context.Canceled) || isTimeout(err) { + if errors.Is(err, context.Canceled) { + return &dnsFailure{code: model.DNSFailureTransport, msg: "the run was cancelled before an answer"} + } + if errors.Is(err, context.DeadlineExceeded) || isTimeout(err) { return &dnsFailure{code: model.DNSFailureTransport, msg: "no answer before the check's timeout"} } return &dnsFailure{code: model.DNSFailureTransport, msg: cutUTF8(err.Error(), dnsDetailMax)} @@ -253,6 +256,11 @@ func (p *prober) probeDNS(ctx context.Context, m model.Monitor, timeout time.Dur res.Error = refused.Error() return } + // A read the run's cancellation woke fails on its closed + // connection; the result says what ended it. + if errors.Is(ctx.Err(), context.Canceled) { + err = ctx.Err() + } res.Error = dnsTransportError(err).Error() return } @@ -493,7 +501,7 @@ func (p *prober) askUDP(ctx context.Context, addr netip.Addr, port string, query return nil, err } defer conn.Close() - setDeadline(ctx, conn) + defer bindConn(ctx, conn)() if _, err := conn.Write(query); err != nil { return nil, err } @@ -517,7 +525,7 @@ func (p *prober) exchangeTCP(ctx context.Context, addrs []netip.Addr, port strin return nil, err } defer conn.Close() - setDeadline(ctx, conn) + defer bindConn(ctx, conn)() return dnsStreamExchange(conn, query) } @@ -529,7 +537,7 @@ func (p *prober) exchangeDoT(ctx context.Context, r model.DNSResolver, addrs []n return nil, err } defer raw.Close() - setDeadline(ctx, raw) + defer bindConn(ctx, raw)() conn := tls.Client(raw, &tls.Config{ServerName: r.Host, RootCAs: p.rootCAs, MinVersion: tls.VersionTLS12}) if err := conn.HandshakeContext(ctx); err != nil { if ctx.Err() != nil || isTimeout(err) { @@ -647,9 +655,18 @@ func recordCert(chain []*x509.Certificate, res *model.MonitorResult) { } } -// setDeadline bounds every read and write on conn by ctx's deadline. -func setDeadline(ctx context.Context, conn net.Conn) { +// bindConn bounds every read and write on conn by ctx: by its deadline, +// and by its cancellation, which closes conn so a read in progress wakes +// rather than waiting out the deadline. A deadline that passes is left to +// the connection's own timeout, so a slow resolver still reads as a +// timeout. The returned func releases ctx's hold on conn. +func bindConn(ctx context.Context, conn net.Conn) (stop func() bool) { if deadline, ok := ctx.Deadline(); ok { _ = conn.SetDeadline(deadline) } + return context.AfterFunc(ctx, func() { + if errors.Is(ctx.Err(), context.Canceled) { + _ = conn.Close() + } + }) } diff --git a/internal/prober/dns_test.go b/internal/prober/dns_test.go index f770806..8a87089 100644 --- a/internal/prober/dns_test.go +++ b/internal/prober/dns_test.go @@ -975,3 +975,45 @@ func TestDNSUDPTriesTheNextAddress(t *testing.T) { } }) } + +// Cancelling a run wakes a read in progress over UDP, TCP, DoT and DoH, so +// a shutdown does not wait out the check's timeout, and the result says +// the run was cancelled rather than timed out. +func TestDNSCancelEndsTheRun(t *testing.T) { + release := make(chan struct{}) + hold := func(string, dnsmessage.Message) []byte { + <-release + return nil + } + plain := newDNSStub(t, nil, hold) + cert, roots, _ := testCert(t, testHost) + dot := newDNSStub(t, &tls.Config{Certificates: []tls.Certificate{cert}}, hold) + doh := dohServer(t, cert, func(http.ResponseWriter, dnsmessage.Message) { <-release }) + // Registered after the servers, so it runs before they wait for their + // handlers to return. + t.Cleanup(func() { close(release) }) + for name, tc := range map[string]struct { + p *prober + target string + }{ + "udp": {dnsProber(t, plain.udp.LocalAddr(), plain.tcp.Addr(), nil), "udp://" + testHost}, + "tcp": {dnsProber(t, plain.udp.LocalAddr(), plain.tcp.Addr(), nil), "tcp://" + testHost}, + "dot": {dnsProber(t, nil, dot.tcp.Addr(), roots), "tls://" + testHost}, + "doh": {dnsProber(t, nil, doh.Listener.Addr(), roots), "https://" + testHost + "/dns-query"}, + } { + m := dnsMonitor(tc.target) + m.TimeoutSec = 20 + ctx, cancel := context.WithCancel(context.Background()) + time.AfterFunc(200*time.Millisecond, cancel) + start := time.Now() + res := tc.p.probe(ctx, m) + took := time.Since(start) + cancel() + if took > 2*time.Second { + t.Fatalf("%s: a run cancelled after 200 ms took %s", name, took) + } + if res.Success || failureCode(res) != model.DNSFailureTransport || !strings.Contains(res.Error, "cancelled") { + t.Fatalf("%s: cancelled run: %+v", name, res) + } + } +} From abbb7aab94a76364806f55de78e751eef96c214c Mon Sep 17 00:00:00 2001 From: lr00rl Date: Sat, 24 Oct 2026 18:53:33 -0700 Subject: [PATCH 09/10] docs: describe the dns probe's address fallthrough, cancellation and error bounds The alpha.10 entry said one query per run and nothing about error size or cancellation; agent #45's review fixes changed all three. --- CHANGELOG.md | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2759745..3489a69 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,20 +24,27 @@ What changes on a node: https:// posts application/dns-message (RFC 8484), tls:// frames the message behind a two-byte length inside TLS (RFC 7858, port 853 by default), and udp:// and tcp:// are plain DNS (port 53). A truncated UDP answer is asked - again once over TCP. + again once over TCP, of the address that sent it. - The resolver's host is resolved once and every address judged by the rule http and tcp checks follow, so a resolver that resolves to a loopback, link-local, metadata, multicast or private address is refused with destination_refused before anything is sent. A DoH request goes only to the judged addresses, through no proxy, and follows no redirect. -- One query per run, with recursion desired and EDNS(0) at 1232 bytes. The +- One query per run, with recursion desired and EDNS(0) at 1232 bytes, to the + first of the resolver's addresses that answers: over every transport, an + address that cannot be reached or stays silent passes the question to the + next within the timeout, and an answer, right or wrong, ends the run. The check's timeout bounds the whole exchange, a DoH body over 64 KiB is refused - unread, and latency runs from the dial to the decoded answer. + unread, latency runs from the dial to the decoded answer, and a run + cancelled at shutdown ends at once rather than at its timeout. - DoH and DoT verify the resolver's certificate against its host name and report its expiry, which the control plane lists under Upcoming. - A failed run's error starts with dns_rcode, dns_mismatch, dns_no_answer or dns_transport, and the result carries the response code and at most 16 - answers of at most 255 bytes. + answers of at most 255 bytes. The error is at most 512 bytes and quotes the + resolver's own text only in short pieces: a content type of at most 64 + bytes, at most three certificate names of 64 bytes each, a transport error + of at most 160 bytes. ## 0.3.10-alpha.9 (2026-10-24) From c8846409c7477c39338a2baf79a28730e021409c Mon Sep 17 00:00:00 2001 From: lr00rl Date: Sat, 24 Oct 2026 18:54:09 -0700 Subject: [PATCH 10/10] docs: say only udp passes a silent address on Over TCP, DoT and DoH only an address that cannot be connected to falls through; a connected one that stays silent ends the run at its timeout. --- CHANGELOG.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3489a69..681d776 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,9 +31,9 @@ What changes on a node: destination_refused before anything is sent. A DoH request goes only to the judged addresses, through no proxy, and follows no redirect. - One query per run, with recursion desired and EDNS(0) at 1232 bytes, to the - first of the resolver's addresses that answers: over every transport, an - address that cannot be reached or stays silent passes the question to the - next within the timeout, and an answer, right or wrong, ends the run. The + first of the resolver's addresses that answers: an address that cannot be + reached passes the question to the next within the timeout, over UDP so + does one that stays silent, and an answer, right or wrong, ends the run. The check's timeout bounds the whole exchange, a DoH body over 64 KiB is refused unread, latency runs from the dial to the decoded answer, and a run cancelled at shutdown ends at once rather than at its timeout.