diff --git a/dns/cache.go b/dns/cache.go index 374595c..4074738 100644 --- a/dns/cache.go +++ b/dns/cache.go @@ -90,12 +90,38 @@ func (c *LruCache) Set(k string, v []byte, ttl int) { } } +// remove deletes a cached response without changing custom records. +func (c *LruCache) remove(k string) { + c.mu.Lock() + defer c.mu.Unlock() + + it, ok := c.cache[k] + if !ok { + return + } + if it.prev != nil { + it.prev.next = it.next + } else { + c.head = it.next + } + if it.next != nil { + it.next.prev = it.prev + } else { + c.tail = it.prev + } + delete(c.cache, k) +} + // putToHead puts a new item to cache's head. func (c *LruCache) putToHead(k string, v []byte, exp int64) { it := &item{key: k, val: v, exp: exp, prev: nil, next: c.head} it.prev = nil it.next = c.head - c.head.prev = it + if c.head != nil { + c.head.prev = it + } else { + c.tail = it + } c.head = it c.cache[k] = it @@ -122,6 +148,10 @@ func (c *LruCache) moveToHead(it *item) { func (c *LruCache) removeTail() { delete(c.cache, c.tail.key) - c.tail.prev.next = nil c.tail = c.tail.prev + if c.tail != nil { + c.tail.next = nil + } else { + c.head = nil + } } diff --git a/dns/client.go b/dns/client.go index 4e24546..87b6ae0 100644 --- a/dns/client.go +++ b/dns/client.go @@ -125,8 +125,11 @@ func (c *Client) handleAnswer(respBytes []byte, clientAddr, dnsServer, network, ttl = c.config.MinTTL } - if ttl <= 0 { // we got a null result - ttl = 1800 + if ttl <= 0 { + // A zero TTL must not become a permanent cache entry. Responses + // without answers have no usable cache lifetime either. + c.cache.remove(qKey(resp.Question)) + return nil } c.cache.Set(qKey(resp.Question), valCopy(respBytes), ttl) @@ -138,8 +141,13 @@ func (c *Client) handleAnswer(respBytes []byte, clientAddr, dnsServer, network, func (c *Client) extractAnswer(resp *Message) ([]string, int) { var ips []string - ttl := c.config.MinTTL - for _, answer := range resp.Answers { + ttl := 0 + for i, answer := range resp.Answers { + // The entire response is cached, so it must expire with its + // shortest-lived answer, including CNAMEs and zero TTLs. + if i == 0 || int(answer.TTL) < ttl { + ttl = int(answer.TTL) + } if answer.TYPE == QTypeA || answer.TYPE == QTypeAAAA { if answer.IP.IsValid() && !answer.IP.IsUnspecified() { for _, h := range c.handlers { @@ -149,9 +157,6 @@ func (c *Client) extractAnswer(resp *Message) ([]string, int) { } ips = append(ips, answer.IP.String()) } - if answer.TTL != 0 { - ttl = int(answer.TTL) - } } } diff --git a/dns/client_test.go b/dns/client_test.go new file mode 100644 index 0000000..1b190eb --- /dev/null +++ b/dns/client_test.go @@ -0,0 +1,101 @@ +package dns + +import ( + "bytes" + "net/netip" + "testing" + "time" +) + +func TestHandleAnswerCacheTTL(t *testing.T) { + cname := func(ttl uint32) *RR { + var target bytes.Buffer + if _, err := MarshalDomainTo(&target, "target.example"); err != nil { + t.Fatal(err) + } + return &RR{NAME: "alias.example", TYPE: 5, CLASS: ClassINET, TTL: ttl, RDLENGTH: uint16(target.Len()), RDATA: target.Bytes()} + } + address := func(ttl uint32) *RR { + ip := netip.MustParseAddr("192.0.2.1") + return &RR{NAME: "target.example", TYPE: QTypeA, CLASS: ClassINET, TTL: ttl, RDLENGTH: 4, RDATA: ip.AsSlice()} + } + tests := []struct { + name string + answers []*RR + minTTL, maxTTL, wantTTL int + }{ + {"CNAME only", []*RR{cname(30)}, 0, 1800, 30}, + {"CNAME before longer address", []*RR{cname(10), address(300)}, 0, 1800, 10}, + {"CNAME after longer address", []*RR{address(300), cname(10)}, 0, 1800, 10}, + {"address expires first", []*RR{cname(300), address(10)}, 0, 1800, 10}, + {"zero TTL CNAME", []*RR{cname(0), address(300)}, 0, 1800, 0}, + {"zero TTL address", []*RR{cname(300), address(0)}, 0, 1800, 0}, + {"no answers", nil, 0, 1800, 0}, + {"minimum override", []*RR{cname(10)}, 60, 1800, 60}, + {"maximum override", []*RR{cname(300)}, 0, 60, 60}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := &Client{config: &Config{MinTTL: tt.minTTL, MaxTTL: tt.maxTTL}, cache: NewLruCache(10)} + resp := NewMessage(1, ResponseMsg) + resp.SetQuestion(&Question{QNAME: "alias.example", QTYPE: QTypeA, QCLASS: ClassINET}) + resp.Answers = tt.answers + wire, err := resp.Marshal() + if err != nil { + t.Fatal(err) + } + // A refresh without a cache lifetime must also discard a stale entry. + c.cache.Set(qKey(resp.Question), []byte("stale response"), 60) + before := time.Now().Unix() + if err := c.handleAnswer(wire, "fixture", "fixture", "udp", "DIRECT"); err != nil { + t.Fatal(err) + } + after := time.Now().Unix() + key := qKey(resp.Question) + entry, ok := c.cache.cache[key] + if tt.wantTTL == 0 { + if ok { + t.Fatalf("response with no positive TTL was cached until %d", entry.exp) + } + if _, ok := c.cache.store[key]; ok { + t.Fatal("response was cached permanently") + } + return + } + if !ok { + t.Fatal("response was not cached") + } + if entry.exp < before+int64(tt.wantTTL) || entry.exp > after+int64(tt.wantTTL) { + t.Fatalf("cache expiration = %d, want %d..%d", entry.exp, before+int64(tt.wantTTL), after+int64(tt.wantTTL)) + } + got, expired := c.cache.Get(key) + if expired || !bytes.Equal(got, wire) { + t.Fatalf("cached response = %x, expired = %v", got, expired) + } + }) + } +} + +func TestUncacheableAnswersLeaveCacheUsable(t *testing.T) { + c := &Client{config: &Config{MaxTTL: 1800}, cache: NewLruCache(2)} + var responses [][]byte + for _, name := range []string{"first.example", "second.example"} { + resp := NewMessage(1, ResponseMsg) + resp.SetQuestion(&Question{QNAME: name, QTYPE: QTypeA, QCLASS: ClassINET}) + wire, err := resp.Marshal() + if err != nil { + t.Fatal(err) + } + responses = append(responses, wire) + c.cache.Set(qKey(resp.Question), []byte("stale"), 60) + } + for _, wire := range responses { + if err := c.handleAnswer(wire, "fixture", "fixture", "udp", "DIRECT"); err != nil { + t.Fatal(err) + } + } + c.cache.Set("next", []byte("response"), 30) + if got, expired := c.cache.Get("next"); expired || string(got) != "response" { + t.Fatalf("cache after removing all responses: %q, expired = %v", got, expired) + } +}