diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 632ecc9..b71272f 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -210,15 +210,19 @@ automatic** (D-021). (`searxng` or `brave`, per `recipe_search_settings`). - Fetching is **SSRF-safe** (`recipes.Fetcher`): https:// only, at every redirect hop. Its dialer resolves the host once and refuses unless every - answer is public — loopback, RFC 1918, link-local, carrier-grade NAT - (100.64/10), multicast, unspecified, reserved, IPv6 unique-local, - link-local and multicast, and IPv4-mapped or IPv4-embedding (NAT64, 6to4, - Teredo) forms are refused — then dials the address it checked, so a + answer is public — every non-globally-reachable IPv4 special-purpose + range (loopback, RFC 1918, link-local, carrier-grade NAT, documentation + TEST-NETs, benchmarking 198.18/15, multicast, reserved and more) is + refused, IPv4-mapped forms included, and IPv6 must be global unicast + (2000::/3) outside documentation, 6to4, Teredo and other special ranges — + then dials the address it checked, so a rebinding DNS answer cannot swap in a private one. Keep-alives and environment proxies are off, so every request dials and checks afresh. At most 3 redirects; a body over 2 MB is an error, never truncated; 10 s for the whole fetch; a descriptive `User-Agent`; requests to one host at least - 1 s apart; 2xx pages cached by URL for an hour (64 pages, oldest evicted). + 1 s apart, counted from requests actually sent (an abandoned wait holds no + slot); 2xx pages cached by URL for an hour (64 pages, oldest evicted), and + callers get their own copy of a cached body. - Recipe data is parsed from schema.org `Recipe` JSON-LD. - Photos are cached locally (`photos`) and processed with `golang.org/x/image`. diff --git a/internal/recipes/fetcher.go b/internal/recipes/fetcher.go index 58e18dd..3c680a5 100644 --- a/internal/recipes/fetcher.go +++ b/internal/recipes/fetcher.go @@ -2,6 +2,7 @@ package recipes import ( + "bytes" "context" "crypto/tls" "errors" @@ -97,8 +98,10 @@ type Fetcher struct { // sleep waits d or until ctx is done; tests replace it. sleep func(ctx context.Context, d time.Duration) error - mu sync.Mutex - nextSlot map[string]time.Time + mu sync.Mutex + // lastSent is when each host's latest request was let through; only + // requests actually sent advance it. + lastSent map[string]time.Time cache map[string]cacheEntry } @@ -121,7 +124,7 @@ func newFetcher(resolver Resolver, dial dialFunc, tlsConfig *tls.Config) *Fetche timeout: Timeout, now: time.Now, sleep: sleep, - nextSlot: map[string]time.Time{}, + lastSent: map[string]time.Time{}, cache: map[string]cacheEntry{}, } transport := &http.Transport{ @@ -140,7 +143,8 @@ func newFetcher(resolver Resolver, dial dialFunc, tlsConfig *tls.Config) *Fetche } // Fetch returns the page at rawURL, from the cache when it was fetched -// within CacheTTL. Only 2xx responses are returned and cached. +// within CacheTTL. Only 2xx responses are returned and cached. The +// returned Body is the caller's own copy. func (f *Fetcher) Fetch(ctx context.Context, rawURL string) (Page, error) { u, err := url.Parse(rawURL) if err != nil { @@ -208,11 +212,15 @@ func (f *Fetcher) cached(key string) (Page, bool) { delete(f.cache, key) return Page{}, false } - return e.page, true + page := e.page + page.Body = bytes.Clone(page.Body) + return page, true } -// store caches page under key, evicting the oldest entry when full. +// store caches a copy of page under key, evicting the oldest entry when +// full. func (f *Fetcher) store(key string, page Page) { + page.Body = bytes.Clone(page.Body) f.mu.Lock() defer f.mu.Unlock() if _, ok := f.cache[key]; !ok && len(f.cache) >= CacheSize { @@ -228,21 +236,26 @@ func (f *Fetcher) store(key string, page Page) { f.cache[key] = cacheEntry{page: page, fetched: f.now()} } -// waitTurn blocks until host's next request slot, keeping requests to one -// host at least HostInterval apart. +// waitTurn blocks until a request to host may be sent, keeping requests to +// one host at least HostInterval apart. A wait holds no reservation: the +// host's slot advances only when a request is let through, so a wait that +// ends with ctx does not delay later requests. func (f *Fetcher) waitTurn(ctx context.Context, host string) error { - f.mu.Lock() - now := f.now() - slot := f.nextSlot[host] - if slot.Before(now) { - slot = now + for { + f.mu.Lock() + now := f.now() + last, sent := f.lastSent[host] + wait := last.Add(HostInterval).Sub(now) + if !sent || wait <= 0 { + f.lastSent[host] = now + f.mu.Unlock() + return nil + } + f.mu.Unlock() + if err := f.sleep(ctx, wait); err != nil { + return err + } } - f.nextSlot[host] = slot.Add(HostInterval) - f.mu.Unlock() - if wait := slot.Sub(now); wait > 0 { - return f.sleep(ctx, wait) - } - return nil } // dialPublic resolves address's host once, refuses unless every answer is @@ -321,38 +334,58 @@ func checkRedirect(req *http.Request, via []*http.Request) error { return checkURL(req.URL) } -// blockedPrefixes are the non-public ranges. IPv4-mapped IPv6 addresses -// are unmapped before the check, so the IPv4 entries cover them too. -var blockedPrefixes = []netip.Prefix{ - netip.MustParsePrefix("0.0.0.0/8"), // unspecified, "this network" +// blockedIPv4 are the IPv4 special-purpose ranges that are not globally +// reachable (IANA IPv4 Special-Purpose Address Registry). IPv4-mapped IPv6 +// addresses are unmapped before the check, so these cover them too. +var blockedIPv4 = []netip.Prefix{ + netip.MustParsePrefix("0.0.0.0/8"), // "this network" netip.MustParsePrefix("10.0.0.0/8"), // RFC 1918 netip.MustParsePrefix("100.64.0.0/10"), // carrier-grade NAT netip.MustParsePrefix("127.0.0.0/8"), // loopback netip.MustParsePrefix("169.254.0.0/16"), // link-local netip.MustParsePrefix("172.16.0.0/12"), // RFC 1918 + netip.MustParsePrefix("192.0.0.0/24"), // IETF protocol assignments + netip.MustParsePrefix("192.0.2.0/24"), // documentation, TEST-NET-1 + netip.MustParsePrefix("192.88.99.0/24"), // 6to4 relay anycast (deprecated) netip.MustParsePrefix("192.168.0.0/16"), // RFC 1918 + netip.MustParsePrefix("198.18.0.0/15"), // benchmarking + netip.MustParsePrefix("198.51.100.0/24"), // documentation, TEST-NET-2 + netip.MustParsePrefix("203.0.113.0/24"), // documentation, TEST-NET-3 netip.MustParsePrefix("224.0.0.0/4"), // multicast netip.MustParsePrefix("240.0.0.0/4"), // reserved, broadcast - netip.MustParsePrefix("::/96"), // unspecified, loopback, IPv4-compatible - netip.MustParsePrefix("fc00::/7"), // unique local - netip.MustParsePrefix("fe80::/10"), // link-local - netip.MustParsePrefix("fec0::/10"), // site-local (deprecated) - netip.MustParsePrefix("ff00::/8"), // multicast - netip.MustParsePrefix("64:ff9b::/96"), // NAT64, embeds any IPv4 - netip.MustParsePrefix("64:ff9b:1::/48"), // local-use NAT64 - netip.MustParsePrefix("2002::/16"), // 6to4, embeds any IPv4 - netip.MustParsePrefix("2001::/32"), // Teredo, embeds any IPv4 - netip.MustParsePrefix("::ffff:0:0:0/96"), // IPv4-translated +} + +// globalIPv6 is the only IPv6 range dialed: global unicast. Everything +// outside it is refused, including ::, ::1, IPv4-compatible and +// IPv4-translated forms, NAT64 (64:ff9b::/96), discard-only (100::/64), +// unique local (fc00::/7), link-local (fe80::/10) and multicast (ff00::/8). +var globalIPv6 = netip.MustParsePrefix("2000::/3") + +// blockedIPv6 are the special-purpose ranges inside global unicast that are +// not public or embed an arbitrary IPv4 address. +var blockedIPv6 = []netip.Prefix{ + netip.MustParsePrefix("2001::/23"), // IETF protocol assignments, Teredo, benchmarking + netip.MustParsePrefix("2001:db8::/32"), // documentation + netip.MustParsePrefix("2002::/16"), // 6to4, embeds any IPv4 + netip.MustParsePrefix("3fff::/20"), // documentation } // IsPublic reports whether ip is a public unicast address the Fetcher may -// dial. +// dial. IPv4 addresses are public unless in a special-purpose range; IPv6 +// addresses must be global unicast and outside its special-purpose ranges. func IsPublic(ip netip.Addr) bool { if !ip.IsValid() { return false } ip = ip.Unmap().WithZone("") - for _, p := range blockedPrefixes { + blocked := blockedIPv4 + if ip.Is6() { + if !globalIPv6.Contains(ip) { + return false + } + blocked = blockedIPv6 + } + for _, p := range blocked { if p.Contains(ip) { return false } diff --git a/internal/recipes/fetcher_test.go b/internal/recipes/fetcher_test.go index f1c2f53..de72215 100644 --- a/internal/recipes/fetcher_test.go +++ b/internal/recipes/fetcher_test.go @@ -60,8 +60,13 @@ type testEnv struct { dialed []string } -// publicIP is a documentation address the tests treat as a public host. -const publicIP = "203.0.113.10" +// publicIP, publicIP2 and publicIPv6 are public addresses; the test dialer +// routes them to the test server and never reaches the real hosts. +const ( + publicIP = "93.184.215.14" + publicIP2 = "93.184.215.20" + publicIPv6 = "2606:2800:21f::10" +) func newTestEnv(t *testing.T, handler http.Handler, answers map[string][][]string) *testEnv { t.Helper() @@ -81,7 +86,7 @@ func newTestEnv(t *testing.T, handler http.Handler, answers map[string][][]strin if err != nil { return nil, err } - if strings.HasPrefix(host, "203.0.113.") { + if host == publicIP || host == publicIP2 || host == publicIPv6 { var d net.Dialer return d.DialContext(ctx, network, server.Listener.Addr().String()) } @@ -128,6 +133,19 @@ func TestFetchReturnsPage(t *testing.T) { } } +func TestFetchDialsPublicIPv6(t *testing.T) { + t.Parallel() + env := newTestEnv(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, "ok") + }), map[string][][]string{"example.com": {{publicIPv6}}}) + if _, err := env.f.Fetch(t.Context(), "https://example.com/"); err != nil { + t.Fatalf("Fetch: %v", err) + } + if got := env.dials(); len(got) != 1 || got[0] != "["+publicIPv6+"]:443" { + t.Errorf("dialed %v, want [[%s]:443]", got, publicIPv6) + } +} + func TestFetchRejectsNonHTTPS(t *testing.T) { t.Parallel() env := newTestEnv(t, http.NotFoundHandler(), publicAnswers()) @@ -159,8 +177,15 @@ var blockedIPs = []string{ "224.0.0.1", "239.255.255.250", "ff02::1", "::ffff:224.0.0.251", // multicast "0.0.0.0", "0.1.2.3", "::", "::ffff:0.0.0.0", // unspecified "fc00::1", "fd12:3456:789a::1", // unique local - "255.255.255.255", "240.0.0.1", "fec0::1", - "64:ff9b::a00:1", "2002:a00:1::1", "2001:0:a00:1::1", "::a00:1", + "255.255.255.255", "240.0.0.1", "fec0::1", // reserved, site-local + "192.0.0.1", "192.0.0.255", // IETF protocol assignments + "192.0.2.1", "198.51.100.7", "203.0.113.10", "::ffff:203.0.113.10", // documentation + "192.88.99.1", // 6to4 relay anycast + "198.18.0.1", "198.19.255.255", "::ffff:198.18.0.1", // benchmarking + "2001:db8::1", "3fff::1", // IPv6 documentation + "2001:2::1", "100::1", // IPv6 benchmarking, discard-only + "::ffff:0:a00:1", // IPv4-translated + "64:ff9b::a00:1", "64:ff9b:1::a00:1", "2002:a00:1::1", "2001:0:a00:1::1", "::a00:1", } func TestIsPublic(t *testing.T) { @@ -170,7 +195,12 @@ func TestIsPublic(t *testing.T) { t.Errorf("IsPublic(%s) = true, want false", s) } } - for _, s := range []string{"8.8.8.8", "203.0.113.10", "100.63.255.255", "100.128.0.1", "172.32.0.1", "2606:4700::1111", "::ffff:8.8.8.8"} { + for _, s := range []string{ + "8.8.8.8", publicIP, "::ffff:8.8.8.8", "100.63.255.255", "100.128.0.1", "172.32.0.1", + "192.0.1.255", "192.0.3.1", "192.88.98.255", "198.17.255.255", "198.20.0.1", + "198.51.99.255", "203.0.112.255", "203.0.114.1", "223.255.255.254", + "2606:4700::1111", publicIPv6, "2001:4860:4860::8888", "2a00:1450::1", + } { if !IsPublic(netip.MustParseAddr(s)) { t.Errorf("IsPublic(%s) = false, want true", s) } @@ -269,7 +299,7 @@ func TestFetchFollowsAtMostThreeRedirects(t *testing.T) { t.Parallel() answers := map[string][][]string{ "example.com": {{publicIP}}, - "recipes.example.com": {{"203.0.113.20"}}, + "recipes.example.com": {{publicIP2}}, } env := newTestEnv(t, chainHandler, answers) page, err := env.f.Fetch(t.Context(), "https://example.com/hop/3") @@ -280,7 +310,7 @@ func TestFetchFollowsAtMostThreeRedirects(t *testing.T) { t.Errorf("page = %+v", page) } // Every hop is dialed, so every hop's address is checked. - if got := env.dials(); len(got) != 4 || got[0] != publicIP+":443" || got[3] != "203.0.113.20:443" { + if got := env.dials(); len(got) != 4 || got[0] != publicIP+":443" || got[3] != publicIP2+":443" { t.Errorf("dialed %v, want 4 checked hops", got) } @@ -494,7 +524,7 @@ func TestFetchRateLimitsPerHost(t *testing.T) { t.Parallel() answers := map[string][][]string{ "example.com": {{publicIP}}, - "recipes.example.com": {{"203.0.113.20"}}, + "recipes.example.com": {{publicIP2}}, } env := newTestEnv(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { fmt.Fprint(w, "ok") @@ -513,3 +543,52 @@ func TestFetchRateLimitsPerHost(t *testing.T) { t.Errorf("waits = %v, want [%s %s]", got, HostInterval, HostInterval) } } + +func TestFetchCacheIsNotSharedWithCallers(t *testing.T) { + t.Parallel() + env := newTestEnv(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, "tacos") + }), publicAnswers()) + + for range 2 { + page, err := env.f.Fetch(t.Context(), "https://example.com/tacos") + if err != nil { + t.Fatalf("Fetch: %v", err) + } + if string(page.Body) != "tacos" { + t.Fatalf("body = %q, want the cached page unchanged", page.Body) + } + // Overwrite the returned body: neither the fetched nor the cached + // copy may reach the cache. + copy(page.Body, "XXXXX") + } +} + +// TestFetchCanceledWaitHoldsNoSlot covers a burst of fetches whose +// rate-limit waits end with their context: they must not push back the +// next request that is allowed through. +func TestFetchCanceledWaitHoldsNoSlot(t *testing.T) { + t.Parallel() + env := newTestEnv(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, "ok") + }), publicAnswers()) + clock := installClock(env.f) + + if _, err := env.f.Fetch(t.Context(), "https://example.com/first"); err != nil { + t.Fatalf("first Fetch: %v", err) + } + env.f.sleep = func(context.Context, time.Duration) error { return context.DeadlineExceeded } + for i := range 5 { + if _, err := env.f.Fetch(t.Context(), fmt.Sprintf("https://example.com/burst/%d", i)); !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("burst Fetch %d error = %v, want context.DeadlineExceeded", i, err) + } + } + env.f.sleep = clock.sleep + clock.advance(HostInterval) + if _, err := env.f.Fetch(t.Context(), "https://example.com/after"); err != nil { + t.Fatalf("Fetch after the burst: %v", err) + } + if got := clock.waits(); len(got) != 0 { + t.Errorf("waits = %v, want none: canceled waits held slots", got) + } +}