Refuse loopback, unspecified and link-local addresses in netcheck_request, on the address dialled, and use no proxy

2026-10-21 10:27:44 +00:00
parent ff849d9f62
commit e9e85365b5
4 changed files with 93 additions and 3 deletions
@@ -1,3 +1,3 @@
 # netcheck

-The machine's own network checks as tools: a TCP connect (`netcheck_tcp`), a DNS lookup of the machine's resolver or of one named (`netcheck_dns`), what listens here (`netcheck_listening`), the private network's peers (`netcheck_tunnel`), and whether a URL answers (`netcheck_http`). `netcheck_request` makes one HTTP request (GET, HEAD or POST, with headers and a body of at most 256 KiB) and answers the status line, every response header and the body (at most 1 MiB, with a flag saying when it was cut), reporting redirects without following them and speaking only http and https, so a service's live answers can be captured (hq issue 1643).
+The machine's own network checks as tools: a TCP connect (`netcheck_tcp`), a DNS lookup of the machine's resolver or of one named (`netcheck_dns`), what listens here (`netcheck_listening`), the private network's peers (`netcheck_tunnel`), and whether a URL answers (`netcheck_http`). `netcheck_request` makes one HTTP request (GET, HEAD or POST, with headers and a body of at most 256 KiB) and answers the status line, every response header and the body (at most 1 MiB, with a flag saying when it was cut), reporting redirects without following them and speaking only http and https, refusing any connection to this machine's own loopback, unspecified or link-local addresses (checked on the address dialled, so names that resolve there are refused too) and using no proxy, so a service's live answers can be captured (hq issue 1643).
@@ -93,7 +93,7 @@ func tools() []stdio.Tool {
 			Description: "Make one HTTP request from this machine and answer what comes back: the status line, every response " +
 				"header and the body as text (base64 when it is not UTF-8), cut at 1 MiB with body_cut saying so. method is GET, " +
 				"HEAD or POST; headers and a body (at most 256 KiB) are sent as given. A redirect is reported (status and redirect), " +
-				"never followed. Only http and https; TLS is verified against this machine's trust store. For what a service " +
+				"never followed. Only http and https; never this machine's own loopback, unspecified or link-local addresses (refused on the address dialled, so names and rebinding are caught too), and no proxy; TLS is verified against this machine's trust store. For what a service " +
 				"answers, not only whether it is up (that is netcheck_http).",
 			Input: map[string]any{
 				"url":        map[string]any{"type": "string", "description": "http:// or https:// URL"},
@@ -13,9 +13,12 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"net"
 	"net/http"
+	"net/netip"
 	"net/url"
 	"strings"
+	"syscall"
 	"time"
 	"unicode/utf8"
 )
@@ -43,6 +46,42 @@ type RequestResult struct {
 	ElapsedMS  int64               `json:"elapsed_ms"`
 }

+// allowLoopback is a seam for the tests only (their servers listen on loopback); nothing sets it otherwise.
+var allowLoopback = false
+
+// refusedAddress says why an address may not be dialled, or "". The node's own loopback carries
+// unauthenticated consoles (the tool console, the bus monitor), and link-local carries instance metadata;
+// a caller on another node must not reach them through this tool.
+func refusedAddress(ip netip.Addr) string {
+	ip = ip.Unmap()
+	switch {
+	case ip.IsLoopback() && !allowLoopback:
+		return "loopback"
+	case ip.IsUnspecified():
+		return "unspecified"
+	case ip.IsLinkLocalUnicast(), ip.IsLinkLocalMulticast():
+		return "link-local"
+	}
+	return ""
+}
+
+// guardDial runs on the address actually about to be connected, after name resolution, so a name that
+// resolves to a refused address (or is rebound to one) is refused as well as an address in the URL.
+func guardDial(_, address string, _ syscall.RawConn) error {
+	host, _, err := net.SplitHostPort(address)
+	if err != nil {
+		return fmt.Errorf("refused: %q is not an address", address)
+	}
+	ip, err := netip.ParseAddr(strings.SplitN(host, "%", 2)[0])
+	if err != nil {
+		return fmt.Errorf("refused: %q is not an address", host)
+	}
+	if why := refusedAddress(ip); why != "" {
+		return fmt.Errorf("refused: %s is a %s address; this tool does not reach this machine's own services", host, why)
+	}
+	return nil
+}
+
 func runRequest(args map[string]any) (any, error) {
 	raw, err := text(args, "url")
 	if err != nil {
@@ -112,7 +151,13 @@ func runRequest(args map[string]any) (any, error) {
 		}
 		req.Header.Set(name, s)
 	}
+	transport := &http.Transport{
+		Proxy:       nil,
+		DialContext: (&net.Dialer{Timeout: timeout, Control: guardDial}).DialContext,
+	}
+	defer transport.CloseIdleConnections()
 	client := &http.Client{
+		Transport:     transport,
 		Timeout:       timeout,
 		CheckRedirect: func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse },
 	}
@@ -3,13 +3,58 @@ package main
 import (
 	"encoding/base64"
 	"io"
+	"net"
 	"net/http"
 	"net/http/httptest"
+	"os"
+	"strconv"
 	"strings"
 	"testing"
 	"time"
 )

+// The httptest servers listen on loopback, which the tool refuses; the tests allow it, and the refusal
+// tests put the refusal back.
+func TestMain(m *testing.M) {
+	allowLoopback = true
+	os.Exit(m.Run())
+}
+
+func refusing(t *testing.T) {
+	t.Helper()
+	allowLoopback = false
+	t.Cleanup(func() { allowLoopback = true })
+}
+
+func TestRequestRefusesLocalAddresses(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		t.Error("the server was reached")
+	}))
+	defer srv.Close()
+	port := srv.Listener.Addr().(*net.TCPAddr).Port
+	refusing(t)
+	for _, host := range []string{
+		"127.0.0.1", "localhost", "127.1", "[::1]", "[::ffff:127.0.0.1]", "0.0.0.0", "[::]",
+		"169.254.169.254", "[fe80::1]",
+	} {
+		u := "http://" + host + ":" + strconv.Itoa(port) + "/"
+		_, err := asked(t, map[string]any{"url": u, "timeout_ms": float64(2000)})
+		if err == nil || !strings.Contains(err.Error(), "refus") {
+			t.Errorf("%s: want a refusal, got %v", u, err)
+		}
+	}
+}
+
+func TestRequestIgnoresProxyEnvironment(t *testing.T) {
+	t.Setenv("HTTP_PROXY", "http://127.0.0.1:1")
+	t.Setenv("http_proxy", "http://127.0.0.1:1")
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}))
+	defer srv.Close()
+	if _, err := asked(t, map[string]any{"url": "http://example.invalid:80/"}); err == nil || strings.Contains(err.Error(), "127.0.0.1:1") {
+		t.Fatalf("a proxy seems to have been used: %v", err)
+	}
+}
+
 func asked(t *testing.T, args map[string]any) (RequestResult, error) {
 	t.Helper()
 	got, err := runRequest(args)
@@ -146,7 +191,7 @@ func TestRequestTimeout(t *testing.T) {
 	defer srv.Close()
 	defer close(release)
 	start := time.Now()
-	_, err := asked(t, map[string]any{"url": srv.URL, "timeout_ms": 100})
+	_, err := asked(t, map[string]any{"url": srv.URL, "timeout_ms": float64(100)})
 	if err == nil || time.Since(start) > 3*time.Second {
 		t.Fatalf("want a timeout error quickly, got %v after %v", err, time.Since(start))
 	}