diff --git a/modules/netcheck/README.md b/modules/netcheck/README.md index 1e48e07f..76cd4994 100644 --- a/modules/netcheck/README.md +++ b/modules/netcheck/README.md @@ -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). diff --git a/modules/netcheck/cmd/netcheck/main.go b/modules/netcheck/cmd/netcheck/main.go index 0ed7362a..349f2aa8 100644 --- a/modules/netcheck/cmd/netcheck/main.go +++ b/modules/netcheck/cmd/netcheck/main.go @@ -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"}, diff --git a/modules/netcheck/cmd/netcheck/request.go b/modules/netcheck/cmd/netcheck/request.go index 49f34808..fb9287d3 100644 --- a/modules/netcheck/cmd/netcheck/request.go +++ b/modules/netcheck/cmd/netcheck/request.go @@ -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 }, } diff --git a/modules/netcheck/cmd/netcheck/request_test.go b/modules/netcheck/cmd/netcheck/request_test.go index b4e91740..15015a48 100644 --- a/modules/netcheck/cmd/netcheck/request_test.go +++ b/modules/netcheck/cmd/netcheck/request_test.go @@ -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)) }