From c9b5a918ceb4c93c85f2836307758ed67d09035d Mon Sep 17 00:00:00 2001 From: Michael Ben-Ami Date: Tue, 7 Jul 2026 17:05:09 -0400 Subject: [PATCH] feature/conn25: add client metrics for dns response rewrite errors and remove noisy logs Remove most logs in mapDNSResponse() that could potentially be spammed by a misbehaving or abusive DNS client or resolver. Keep logs, and complement with metrics, for failed rewrites, as they likely point to an internal error, e.g. ip pools exhausted. Metrics allow for potential alerting in the future. Updates tailscale/corp#40125 Updates tailscale/corp#40126 Signed-off-by: Michael Ben-Ami --- feature/conn25/conn25.go | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/feature/conn25/conn25.go b/feature/conn25/conn25.go index e5433cd54..33695ec12 100644 --- a/feature/conn25/conn25.go +++ b/feature/conn25/conn25.go @@ -39,6 +39,7 @@ import ( "tailscale.com/types/key" "tailscale.com/types/logger" "tailscale.com/types/views" + "tailscale.com/util/clientmetric" "tailscale.com/util/dnsname" "tailscale.com/util/mak" "tailscale.com/util/set" @@ -1089,6 +1090,20 @@ func makeServFail(logf logger.Logf, h dnsmessage.Header, q dnsmessage.Question) return bs } +var ( + // metricDNSResponseRewriteErrorServfail increments servfail returns + // on an error rewriting response for an app connector domain. + metricDNSResponseRewriteErrorServfail = clientmetric.NewCounter( + "conn25_map_dns_response_rewrite_error_servfail", + ) + + // metricDNSResponseRewriteUnsupportedQuestionTypeErrorServfail increments servfail returns + // on an error rewriting empty answers to an unsupported question type for an app connector domain. + metricDNSResponseRewriteUnsupportedQuestionTypeErrorServfail = clientmetric.NewCounter( + "conn25_map_dns_response_rewrite_unsupported_question_type_error_servfail", + ) +) + // mapDNSResponse parses and inspects the DNS response. If the domain // is determined to belong to app this node is client for, it assigns addresses // for connecting and rewrites the response to contain Magic IPs. @@ -1096,12 +1111,10 @@ func (c *Conn25) mapDNSResponse(buf []byte) []byte { var p dnsmessage.Parser hdr, err := p.Start(buf) if err != nil { - c.logf("error parsing dns response: %v", err) return buf } questions, err := p.AllQuestions() if err != nil { - c.logf("error parsing dns response: %v", err) return buf } // Any message we are interested in has one question (RFC 9619) @@ -1139,9 +1152,9 @@ func (c *Conn25) mapDNSResponse(buf []byte) []byte { var answers []dnsResponseRewrite var cnameChain map[dnsname.FQDN]dnsname.FQDN if question.Type != dnsmessage.TypeA && question.Type != dnsmessage.TypeAAAA { - c.logf("mapping dns response for connector domain, unsupported type: %v", question.Type) newBuf, err := c.client.rewriteDNSResponse(appName, hdr, questions, answers) if err != nil { + metricDNSResponseRewriteUnsupportedQuestionTypeErrorServfail.Add(1) c.logf("error writing empty response for unsupported type: %v", err) return makeServFail(c.logf, hdr, question) } @@ -1153,14 +1166,11 @@ func (c *Conn25) mapDNSResponse(buf []byte) []byte { break } if err != nil { - c.logf("error parsing dns response: %v", err) return makeServFail(c.logf, hdr, question) } // other classes are unsupported, and we checked the question was for ClassINET already if h.Class != dnsmessage.ClassINET { - c.logf("unexpected class for connector domain dns response: %v %v", queriedDomain, h.Class) if err := p.SkipAnswer(); err != nil { - c.logf("error parsing dns response: %v", err) return makeServFail(c.logf, hdr, question) } continue @@ -1179,17 +1189,14 @@ func (c *Conn25) mapDNSResponse(buf []byte) []byte { // a.example.com A (some magic IP that is associated with 1.1.1.1) r, err := p.CNAMEResource() if err != nil { - c.logf("error parsing dns response: %v", err) return makeServFail(c.logf, hdr, question) } src, err := normalizeDNSName(h.Name.String()) if err != nil { - c.logf("bad dnsname: %v", err) return makeServFail(c.logf, hdr, question) } target, err := normalizeDNSName(r.CNAME.String()) if err != nil { - c.logf("bad dnsname: %v", err) return makeServFail(c.logf, hdr, question) } mak.Set(&cnameChain, src, target) @@ -1197,14 +1204,12 @@ func (c *Conn25) mapDNSResponse(buf []byte) []byte { if h.Type != question.Type { // would not expect a v4 response to a v6 question or vice versa, don't add a rewrite for this. if err := p.SkipAnswer(); err != nil { - c.logf("error parsing dns response: %v", err) return makeServFail(c.logf, hdr, question) } continue } answerDomain, err := normalizeDNSName(h.Name.String()) if err != nil { - c.logf("bad dnsname: %v", err) return makeServFail(c.logf, hdr, question) } // If answerDomain is not the same domain as the domain that was queried for, @@ -1229,9 +1234,7 @@ func (c *Conn25) mapDNSResponse(buf []byte) []byte { d = target } if !found { - c.logf("unexpected domain for connector domain dns response: %v %v", queriedDomain, answerDomain) if err := p.SkipAnswer(); err != nil { - c.logf("error parsing dns response: %v", err) return makeServFail(c.logf, hdr, question) } continue @@ -1241,14 +1244,12 @@ func (c *Conn25) mapDNSResponse(buf []byte) []byte { if h.Type == dnsmessage.TypeA { r, err := p.AResource() if err != nil { - c.logf("error parsing dns response: %v", err) return makeServFail(c.logf, hdr, question) } dstAddr = netip.AddrFrom4(r.A) } else { r, err := p.AAAAResource() if err != nil { - c.logf("error parsing dns response: %v", err) return makeServFail(c.logf, hdr, question) } dstAddr = netip.AddrFrom16(r.AAAA) @@ -1256,15 +1257,14 @@ func (c *Conn25) mapDNSResponse(buf []byte) []byte { answers = append(answers, dnsResponseRewrite{domain: queriedDomain, dst: dstAddr, ttl: time.Second * time.Duration(h.TTL)}) default: // we already checked the question was for a supported type, this answer is unexpected - c.logf("unexpected type for connector domain dns response: %v %v", queriedDomain, h.Type) if err := p.SkipAnswer(); err != nil { - c.logf("error parsing dns response: %v", err) return makeServFail(c.logf, hdr, question) } } } newBuf, err := c.client.rewriteDNSResponse(appName, hdr, questions, answers) if err != nil { + metricDNSResponseRewriteErrorServfail.Add(1) c.logf("error rewriting dns response: %v", err) return makeServFail(c.logf, hdr, question) }