Resolve PTR names over DNS-over-HTTPS and cache them for the record TTL (closes #238)

`internal/ptr` asks Google, Cloudflare, AdGuard and Quad9 in turn, 2 seconds in all per address. It keeps answers in memory and writes them through to `cache.db`, opened by the new `internal/cachedb`.

Judgement calls:
- Each resolver gets an equal share of the time left, so a hung resolver does not use up the 2 seconds.
- The negative TTL is the smaller of the SOA minimum and the SOA's own TTL (RFC 2308).
- Names that are not host names are dropped, as `net.Resolver.LookupAddr` does.

Model: opus-5-5

README.md

@@ -458,26 +458,21 @@ One address's lookup gets 2 seconds across all four. Each resolver gets an equal
 share of the time left, so one that does not answer leaves time for the next.
 When none answers in time, the address's `ptr` is an empty list.
 
-A reserved address, one whose `is_bogon` flag is true, is not sent to the
-resolvers. Its `ptr` is an empty list.
-
 What goes outside: each resolver asked receives the address being looked up. For
 `/ip/` and the home page, that address is the visitor's own. A lookup answered
 from the cache sends nothing.
 
 How RouteWatch keeps the answers:
 
-- It holds answers in memory. An answer that has not expired is served from
+- It holds every answer in memory. An answer that has not expired is served from
   there.
 - It writes new answers every 5 seconds to `cache.db` in the state directory
   (see Configuration), and loads them from there at startup.
 - An answer expires after the TTL of its records, the shortest one when there
   are several. RouteWatch keeps it for at least 60 seconds and at most 24 hours.
-- When an address has no PTR names, that answer expires after the SOA record's
+- An answer that an address has no PTR names expires after the SOA record's
   minimum field, or the record's own TTL when that is shorter. RouteWatch keeps
   it for at most 1 hour.
-- An answer with no PTR names and no SOA record is not kept. Neither is one
-  whose SOA record has a TTL or minimum field of 0.
 - A failed lookup is not kept, so the next lookup of the address asks again.
 - Every 10 minutes it deletes expired answers from memory and from `cache.db`.
 

internal/ptr/cache.go

@@ -16,7 +16,6 @@ import (
 	"sync"
 	"time"
 
-	"git.eeqj.de/sneak/routewatch/internal/bogon"
 	"github.com/miekg/dns"
 )
 
@@ -54,7 +53,6 @@ type entry struct {
 type Cache struct {
 	db        *sql.DB
 	resolvers []string
-	timeLimit time.Duration
 	logger    *slog.Logger
 	client    *http.Client
 	now       func() time.Time
@@ -75,7 +73,6 @@ func New(params Params) *Cache {
 	return &Cache{
 		db:        params.DB,
 		resolvers: params.Resolvers,
-		timeLimit: LookupTimeout,
 		logger:    params.Logger.With("component", "ptr"),
 		client:    &http.Client{},
 		now:       time.Now,
@@ -86,23 +83,16 @@ func New(params Params) *Cache {
 }
 
 // LookupAddr returns the PTR names of addr, fully qualified, or none when it
-// has none. A reserved address has none, and is not sent to the resolvers. A
-// lookup takes at most LookupTimeout. A cached answer makes no outside request.
-// It returns an error when no resolver answered, and nothing is cached then.
+// has none. It takes at most LookupTimeout. A cached answer makes no outside
+// request. It returns an error when no resolver answered, and nothing is
+// cached then.
 func (c *Cache) LookupAddr(ctx context.Context, addr string) ([]string, error) {
 	parsed, err := netip.ParseAddr(addr)
 	if err != nil {
 		return nil, fmt.Errorf("parsing the address: %w", err)
 	}
 
-	parsed = parsed.Unmap()
-
-	_, reserved := bogon.Prefix(netip.PrefixFrom(parsed, parsed.BitLen()))
-	if reserved {
-		return []string{}, nil
-	}
-
-	address := parsed.String()
+	address := parsed.Unmap().String()
 
 	cached, ok := c.current(address)
 	if ok {

internal/ptr/cache_internal_test.go

@@ -1,7 +1,6 @@
 package ptr
 
 import (
-	"context"
 	"database/sql"
 	"errors"
 	"io"
@@ -9,40 +8,30 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"slices"
-	"strconv"
 	"sync"
 	"testing"
-	"testing/synctest"
 	"time"
 
 	"git.eeqj.de/sneak/routewatch/internal/cachedb"
 	"github.com/miekg/dns"
 )
 
-// These addresses are public, because a reserved address is not sent to the
-// resolvers. That rules out the documentation ranges.
 const (
-	testAddress   = "8.8.4.4"
-	testReverse   = "4.4.8.8.in-addr.arpa."
-	mappedAddress = "::ffff:" + testAddress
-	otherAddress  = "8.8.8.8"
-	testV6Address = "2606:4700:4700::1111"
-	testV6Reverse = "1.1.1.1.0.0.0.0.0.0.0.0.0.0.0.0." +
-		"0.0.0.0.0.0.7.4.0.0.7.4.6.0.6.2.ip6.arpa."
-)
+	testAddress   = "192.0.2.1"
+	testReverse   = "1.2.0.192.in-addr.arpa."
+	testName      = "host.example.com."
+	otherName     = "other.example.com."
+	testV6Address = "2001:db8::1"
+	testV6Reverse = "1.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0." +
+		"0.0.0.0.0.0.0.0.8.b.d.0.1.0.0.2.ip6.arpa."
 
-const (
-	testName  = "host.example.com."
-	otherName = "other.example.com."
-
-	// testTimeLimit replaces LookupTimeout in the tests that use a real
-	// server. A stall on a busy host cannot use it up, and no test waits for
-	// it.
-	testTimeLimit = 2 * time.Minute
+	// timingAllowance is how far past its limit a loaded machine may run a
+	// lookup that has a time limit.
+	timingAllowance = 8 * time.Second
 )
 
 // classlessOwner is the name that holds the PTR records of a CNAME chain.
-const classlessOwner = "4.0-25.4.8.8.in-addr.arpa."
+const classlessOwner = "1.0-25.2.0.192.in-addr.arpa."
 
 // fixedTime returns the time a test's clock starts at.
 func fixedTime() time.Time {
@@ -242,7 +231,7 @@ func (c *testClock) advance(d time.Duration) {
 }
 
 // newTestCache returns a Cache that asks the resolvers in order, reads the
-// time from a clock the test holds, has testTimeLimit, and is not started.
+// time from a clock the test holds, and is not started.
 func newTestCache(
 	t *testing.T, db *sql.DB, resolvers ...*fakeResolver,
 ) (*Cache, *testClock) {
@@ -256,57 +245,10 @@ func newTestCache(
 	cache := New(Params{DB: db, Resolvers: urls, Logger: slog.Default()})
 	clock := &testClock{now: fixedTime()}
 	cache.now = clock.time
-	cache.timeLimit = testTimeLimit
 
 	return cache, clock
 }
 
-// handlerTransport hands each request to the fake resolver for its host, with
-// no network between them. A test in a synctest bubble needs this: the bubble's
-// clock stands still while one of its goroutines waits on a real connection. As
-// over a network, a request whose time is up is not sent.
-type handlerTransport map[string]*fakeResolver
-
-func (h handlerTransport) RoundTrip(r *http.Request) (*http.Response, error) {
-	recorder := httptest.NewRecorder()
-
-	if r.Context().Err() == nil {
-		h[r.URL.Host].serve(recorder, r)
-	}
-
-	err := r.Context().Err()
-	if err != nil {
-		return nil, err
-	}
-
-	return recorder.Result(), nil
-}
-
-// newBubbleCache returns a Cache with the real time limit, for a test in a
-// synctest bubble. It has one fake resolver for each reply, in order, reached
-// through a handlerTransport.
-func newBubbleCache(
-	replies ...func(http.ResponseWriter, *http.Request, *dns.Msg),
-) (*Cache, []*fakeResolver) {
-	transport := make(handlerTransport, len(replies))
-	resolvers := make([]*fakeResolver, 0, len(replies))
-	urls := make([]string, 0, len(replies))
-
-	for i, reply := range replies {
-		host := "resolver" + strconv.Itoa(i) + ".example.com"
-		resolver := &fakeResolver{reply: reply}
-
-		transport[host] = resolver
-		resolvers = append(resolvers, resolver)
-		urls = append(urls, "https://"+host+"/dns-query")
-	}
-
-	cache := New(Params{Resolvers: urls, Logger: slog.Default()})
-	cache.client = &http.Client{Transport: transport}
-
-	return cache, resolvers
-}
-
 // lookupOK looks up address and fails the test on an error.
 func lookupOK(t *testing.T, cache *Cache, address string) []string {
 	t.Helper()
@@ -341,7 +283,7 @@ func TestLookupAsksForTheReverseName(t *testing.T) {
 		want    string
 	}{
 		{testAddress, testReverse},
-		{mappedAddress, testReverse},
+		{"::ffff:" + testAddress, testReverse},
 		{testV6Address, testV6Reverse},
 	}
 
@@ -370,42 +312,6 @@ func TestLookupAsksForTheReverseName(t *testing.T) {
 	}
 }
 
-// TestOnlyPublicAddressesAreSentToTheResolvers gives every resolver an answer
-// to send, which a reserved address must not get.
-func TestOnlyPublicAddressesAreSentToTheResolvers(t *testing.T) {
-	t.Parallel()
-
-	tests := []struct {
-		name     string
-		address  string
-		want     []string
-		requests int
-	}{
-		{"private IPv4", "10.0.0.1", []string{}, 0},
-		{"documentation IPv4", "192.0.2.1", []string{}, 0},
-		{"private IPv4 in IPv6 form", "::ffff:10.0.0.1", []string{}, 0},
-		{"unique-local IPv6", "fd00::1", []string{}, 0},
-		{"documentation IPv6", "2001:db8::1", []string{}, 0},
-		{"public IPv4", testAddress, []string{testName}, 1},
-		{"public IPv4 in IPv6 form", mappedAddress, []string{testName}, 1},
-		{"public IPv6", testV6Address, []string{testName}, 1},
-	}
-
-	for _, tt := range tests {
-		resolver := newFakeResolver(t, ptrReply(300, testName))
-		cache, _ := newTestCache(t, openTestDB(t, t.TempDir()), resolver)
-
-		names := lookupOK(t, cache, tt.address)
-		if names == nil || !slices.Equal(names, tt.want) {
-			t.Errorf("%s: names %#v, want %#v", tt.name, names, tt.want)
-		}
-
-		if got := resolver.requestCount(); got != tt.requests {
-			t.Errorf("%s: %d requests, want %d", tt.name, got, tt.requests)
-		}
-	}
-}
-
 // TestResolversAreTriedInOrderUntilOneAnswers has the first two resolvers fail,
 // in different ways, and the third answer. The fourth is not asked.
 func TestResolversAreTriedInOrderUntilOneAnswers(t *testing.T) {
@@ -449,59 +355,51 @@ func TestResolversAreTriedInOrderUntilOneAnswers(t *testing.T) {
 }
 
 // TestResolverThatNeverAnswersLeavesTimeForTheNext would fail if the first
-// resolver could take the whole time limit. The first gets half of it, the
-// share of one resolver in two. The test runs in a synctest bubble, whose clock
-// is fake, so a stall on the host uses none of the limit.
+// resolver could take the whole time limit.
 func TestResolverThatNeverAnswersLeavesTimeForTheNext(t *testing.T) {
 	t.Parallel()
 
-	synctest.Test(t, func(t *testing.T) {
-		cache, _ := newBubbleCache(hangingReply, ptrReply(300, testName))
+	cache, _ := newTestCache(t, openTestDB(t, t.TempDir()),
+		newFakeResolver(t, hangingReply),
+		newFakeResolver(t, ptrReply(300, testName)))
 
-		start := time.Now()
-		names := lookupOK(t, cache, testAddress)
-		elapsed := time.Since(start)
-
-		if !slices.Equal(names, []string{testName}) {
-			t.Errorf("names %v, want %s", names, testName)
-		}
-
-		if elapsed != time.Second {
-			t.Errorf("took %s, want 1s", elapsed)
-		}
-	})
+	names := lookupOK(t, cache, testAddress)
+	if !slices.Equal(names, []string{testName}) {
+		t.Errorf("names %v, want %s", names, testName)
+	}
 }
 
-// TestLookupStopsAtTheTimeLimit has no resolver answer. The lookup fails when
-// 2 seconds have passed, having asked every resolver. The test runs in a
-// synctest bubble, whose clock is fake, so a stall on the host uses none of the
-// limit.
+// TestLookupStopsAtTheTimeLimit has no resolver answer. The lookup fails once
+// LookupTimeout has passed, having asked every resolver.
 func TestLookupStopsAtTheTimeLimit(t *testing.T) {
 	t.Parallel()
 
-	synctest.Test(t, func(t *testing.T) {
-		cache, resolvers := newBubbleCache(
-			hangingReply, hangingReply, hangingReply, hangingReply)
+	resolvers := []*fakeResolver{
+		newFakeResolver(t, hangingReply),
+		newFakeResolver(t, hangingReply),
+		newFakeResolver(t, hangingReply),
+		newFakeResolver(t, hangingReply),
+	}
+	cache, _ := newTestCache(t, openTestDB(t, t.TempDir()), resolvers...)
 
-		start := time.Now()
-		_, err := cache.LookupAddr(t.Context(), testAddress)
-		elapsed := time.Since(start)
+	start := time.Now()
+	_, err := cache.LookupAddr(t.Context(), testAddress)
+	elapsed := time.Since(start)
 
-		if !errors.Is(err, context.DeadlineExceeded) {
-			t.Errorf("error %v, want %v", err, context.DeadlineExceeded)
+	if err == nil {
+		t.Error("LookupAddr succeeded with no resolver answering")
+	}
+
+	if elapsed < LookupTimeout || elapsed > LookupTimeout+timingAllowance {
+		t.Errorf("took %s, want about %s", elapsed, LookupTimeout)
+	}
+
+	for i, resolver := range resolvers {
+		if resolver.requestCount() != 1 {
+			t.Errorf("resolver %d asked %d times, want once",
+				i, resolver.requestCount())
 		}
-
-		if elapsed != 2*time.Second {
-			t.Errorf("took %s, want 2s", elapsed)
-		}
-
-		for i, resolver := range resolvers {
-			if resolver.requestCount() != 1 {
-				t.Errorf("resolver %d asked %d times, want once",
-					i, resolver.requestCount())
-			}
-		}
-	})
+	}
 }
 
 // TestAnswerIsServedFromMemoryUntilItExpires makes a second lookup within the
@@ -539,7 +437,7 @@ func TestAnAddressHasOneCacheEntryHoweverItIsWritten(t *testing.T) {
 	resolver := newFakeResolver(t, ptrReply(300, testName))
 	cache, _ := newTestCache(t, openTestDB(t, t.TempDir()), resolver)
 
-	lookupOK(t, cache, mappedAddress)
+	lookupOK(t, cache, "::ffff:"+testAddress)
 	lookupOK(t, cache, testAddress)
 
 	if got := resolver.requestCount(); got != 1 {
@@ -547,17 +445,15 @@ func TestAnAddressHasOneCacheEntryHoweverItIsWritten(t *testing.T) {
 	}
 }
 
-// TestCallerCannotChangeACachedAnswer changes the list each lookup returned, as
-// the server does when it removes the trailing dots. The second lookup is the
-// first answered from the cache. The third shows that the cached list did not
-// change with the list the second returned.
+// TestCallerCannotChangeACachedAnswer changes the list a lookup returned, as
+// the server does when it removes the trailing dots.
 func TestCallerCannotChangeACachedAnswer(t *testing.T) {
 	t.Parallel()
 
 	resolver := newFakeResolver(t, ptrReply(300, testName))
 	cache, _ := newTestCache(t, openTestDB(t, t.TempDir()), resolver)
 
-	for range 3 {
+	for range 2 {
 		names := lookupOK(t, cache, testAddress)
 		if !slices.Equal(names, []string{testName}) {
 			t.Errorf("names %v, want %s", names, testName)
@@ -744,6 +640,8 @@ func TestCacheSurvivesARestart(t *testing.T) {
 	dir := t.TempDir()
 	db := openTestDB(t, dir)
 
+	const noNamesAddress = "192.0.2.2"
+
 	resolver := newFakeResolver(t,
 		func(w http.ResponseWriter, r *http.Request, query *dns.Msg) {
 			if query.Question[0].Name == testReverse {
@@ -763,7 +661,7 @@ func TestCacheSurvivesARestart(t *testing.T) {
 	}
 
 	lookupOK(t, cache, testAddress)
-	lookupOK(t, cache, otherAddress)
+	lookupOK(t, cache, noNamesAddress)
 	cache.Stop(t.Context())
 
 	err = db.Close()
@@ -787,7 +685,7 @@ func TestCacheSurvivesARestart(t *testing.T) {
 		t.Errorf("names after restart %v", names)
 	}
 
-	if names := lookupOK(t, restarted, otherAddress); len(names) != 0 {
+	if names := lookupOK(t, restarted, noNamesAddress); len(names) != 0 {
 		t.Errorf("names of the address with none after restart: %v", names)
 	}
 
@@ -815,7 +713,7 @@ func TestSweepDeletesExpiredRows(t *testing.T) {
 	cache, clock := newTestCache(t, db, resolver)
 
 	lookupOK(t, cache, testAddress)
-	lookupOK(t, cache, otherAddress)
+	lookupOK(t, cache, "192.0.2.2")
 	cache.flush(t.Context())
 
 	if got := countRows(t, db); got != 2 {

internal/ptr/doh.go

@@ -59,9 +59,9 @@ type answer struct {
 }
 
 // lookup asks the resolvers in turn for the PTR names of the reverse name
-// until one answers. It fails when none has answered within the time limit.
+// until one answers. It fails when none has answered within LookupTimeout.
 func (c *Cache) lookup(ctx context.Context, reverseName string) (answer, error) {
-	ctx, cancel := context.WithTimeout(ctx, c.timeLimit)
+	ctx, cancel := context.WithTimeout(ctx, LookupTimeout)
 	defer cancel()
 
 	query := new(dns.Msg).SetQuestion(reverseName, dns.TypePTR)