diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a16e5c..93a451f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -171,6 +171,14 @@ APIs change between minors without a deprecation period. torchaudio; every other package comes from PyPI alone, and the lock is PEP 751's (`pylock.llmgw.toml`), pinning each file's address as well as its hash. Checked against vLLM 0.30.0: the same 195 packages resolve. +- **The control plane's own fetches stay off link-local addresses.** + Checking a model's source and reading a GGUF's header went wherever the + URL said, cloud instance metadata (169.254.169.254) included. They now + refuse link-local, multicast and unspecified addresses, while still + reaching the cluster's network, where the mirror is. The mirror's fetches, + which refuse private addresses too, now also see through IPv6 addresses + carrying an IPv4 one (NAT64, 6to4, IPv4-compatible), and through a proxy: + the target's addresses are checked before the request is handed to it. ### Everything that can be changed on a running cluster, from the console diff --git a/internal/control/adminapi/api.go b/internal/control/adminapi/api.go index 4373ff3..9b04dab 100644 --- a/internal/control/adminapi/api.go +++ b/internal/control/adminapi/api.go @@ -24,6 +24,7 @@ import ( "gitea.mynodes.xyz/ilker/llmgw/internal/gguf" "gitea.mynodes.xyz/ilker/llmgw/internal/modelplan" "gitea.mynodes.xyz/ilker/llmgw/internal/release" + "gitea.mynodes.xyz/ilker/llmgw/internal/safedial" "gitea.mynodes.xyz/ilker/llmgw/internal/agent/discovery" "gitea.mynodes.xyz/ilker/llmgw/internal/bench" @@ -1715,7 +1716,7 @@ func (a *API) setAdapters(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusBadRequest, "no model "+name) return } - base, baseErr := gguf.KindFromURL(r.Context(), nil, m.Source) + base, baseErr := gguf.KindFromURL(r.Context(), safedial.Client(safedial.ControlPlane, 30*time.Second), m.Source) have := map[string]bool{} for _, ad := range m.Adapters { have[ad.Source] = true @@ -1724,7 +1725,7 @@ func (a *API) setAdapters(w http.ResponseWriter, r *http.Request) { if have[ad.Source] { continue // already checked when it was attached } - h, err := gguf.KindFromURL(r.Context(), nil, ad.Source) + h, err := gguf.KindFromURL(r.Context(), safedial.Client(safedial.ControlPlane, 30*time.Second), ad.Source) switch { case err != nil: writeError(w, http.StatusBadRequest, fmt.Sprintf("adapter %s: %v", ad.Name, err)) diff --git a/internal/mirror/api_test.go b/internal/mirror/api_test.go index dbf41d4..79ce4b1 100644 --- a/internal/mirror/api_test.go +++ b/internal/mirror/api_test.go @@ -446,6 +446,8 @@ func TestTheAPIWillNotFetchFromThePrivateNetwork(t *testing.T) { for _, src := range []string{ "http://127.0.0.1:1/x.gguf", "http://169.254.169.254/latest/meta-data", "http://10.0.0.140:9090/api/v1/nodes", "http://[::1]:1/x", + // The metadata address inside NAT64 and 6to4 ones. + "http://[64:ff9b::a9fe:a9fe]/latest", "http://[2002:a9fe:a9fe::1]/latest", } { st, err := m.Start(src, Entry{Model: "m", Version: "v", File: "x.gguf"}) if err != nil { @@ -456,7 +458,7 @@ func TestTheAPIWillNotFetchFromThePrivateNetwork(t *testing.T) { time.Sleep(10 * time.Millisecond) st, _ = m.Status("m", "v") } - if st.State != Failed || !strings.Contains(st.Err, "refusing to fetch") { + if st.State != Failed || !strings.Contains(st.Err, "refusing to reach") { t.Errorf("%s: %+v", src, st) } m.Forget("m", "v") diff --git a/internal/mirror/safefetch.go b/internal/mirror/safefetch.go index 290b2a3..d2f918c 100644 --- a/internal/mirror/safefetch.go +++ b/internal/mirror/safefetch.go @@ -1,80 +1,35 @@ package mirror import ( - "errors" "fmt" - "net" "net/http" "net/netip" "net/url" - "syscall" - "time" + + "gitea.mynodes.xyz/ilker/llmgw/internal/safedial" ) // errPrivateSource is a fetch refused because it would reach this machine or // the network behind it rather than the internet. -var errPrivateSource = errors.New("refusing to fetch from a loopback, private or link-local address") +var errPrivateSource = safedial.ErrBlocked // fetchClient downloads sources for the management API. // // The API fetches whatever URL it is given and the file server then hands the // result to anyone. Without this, "fetch http://169.254.169.254/..." was a way // to read a cloud's instance metadata — or anything else on the network the -// mirror sits in — through the mirror. The check is made on the address -// actually dialled, after resolution and on every redirect, so a name that -// resolves somewhere public and then somewhere private is caught too. +// mirror sits in — through the mirror. No overall timeout: a model is tens of +// gigabytes, and after the first byte a stall is the context's to end. func fetchClient(allowPrivate bool) *http.Client { - d := &net.Dialer{Timeout: 30 * time.Second, KeepAlive: 30 * time.Second} - if !allowPrivate { - d.Control = func(_, address string, _ syscall.RawConn) error { - host, _, err := net.SplitHostPort(address) - if err != nil { - return err - } - a, err := netip.ParseAddr(host) - if err != nil { - return err - } - if blockedAddr(a.Unmap()) { - return fmt.Errorf("%w (%s)", errPrivateSource, a) - } - return nil - } - } - return &http.Client{ - Transport: &http.Transport{ - Proxy: http.ProxyFromEnvironment, - DialContext: d.DialContext, - TLSHandshakeTimeout: 30 * time.Second, - ResponseHeaderTimeout: 2 * time.Minute, - ForceAttemptHTTP2: true, - }, - // No overall timeout: a model is tens of gigabytes. The dial, the - // handshake and the first byte are bounded above; after that a stall - // is the context's to end. - CheckRedirect: func(req *http.Request, via []*http.Request) error { - if len(via) >= 10 { - return errors.New("too many redirects") - } - return checkScheme(req.URL) - }, + if allowPrivate { + return safedial.Client(safedial.Policy{Private: true, Loopback: true, LinkLocal: true}, 0) } + return safedial.Client(safedial.Policy{}, 0) } -func blockedAddr(a netip.Addr) bool { - cgnat := netip.MustParsePrefix("100.64.0.0/10") - return a.IsLoopback() || a.IsPrivate() || a.IsLinkLocalUnicast() || - a.IsLinkLocalMulticast() || a.IsMulticast() || a.IsUnspecified() || cgnat.Contains(a) -} +func blockedAddr(a netip.Addr) bool { return safedial.Blocked(a, safedial.Policy{}) } -// checkScheme allows the two schemes a model is published over. file:// and -// the rest would read this machine's own disk. -func checkScheme(u *url.URL) error { - if u.Scheme != "http" && u.Scheme != "https" { - return fmt.Errorf("%q is not an http or https source", u.Redacted()) - } - return nil -} +func checkScheme(u *url.URL) error { return safedial.CheckScheme(u) } // validSource checks a source before a download is started for it. func validSource(source string) error { diff --git a/internal/safedial/safedial.go b/internal/safedial/safedial.go new file mode 100644 index 0000000..1f73f22 --- /dev/null +++ b/internal/safedial/safedial.go @@ -0,0 +1,160 @@ +// Package safedial makes HTTP clients that refuse to reach addresses a +// fetch on someone else's behalf has no business reaching: a cloud's +// instance metadata, this machine, the network behind it. +// +// The check is made on the address actually dialled, after resolution and on +// every redirect, so a name that resolves somewhere public and then somewhere +// private is caught too. IPv6 addresses that carry an IPv4 one — NAT64, +// 6to4, IPv4-mapped and -compatible — are judged by the IPv4 address inside. +package safedial + +import ( + "errors" + "fmt" + "net" + "net/http" + "net/netip" + "net/url" + "syscall" + "time" +) + +// Policy is what may be reached besides the internet. +type Policy struct { + // Private allows RFC 1918, unique-local and carrier-grade NAT addresses: + // a control plane's own mirror lives on one. + Private bool + // Loopback allows this machine. + Loopback bool + // LinkLocal allows link-local addresses, instance metadata's included: + // only for a mirror told to with --allow-private-sources. + LinkLocal bool +} + +// ErrBlocked is a fetch refused for where it would go. +var ErrBlocked = errors.New("refusing to reach a loopback, private or link-local address") + +var ( + cgnat = netip.MustParsePrefix("100.64.0.0/10") + nat64 = netip.MustParsePrefix("64:ff9b::/96") + nat64l = netip.MustParsePrefix("64:ff9b:1::/48") + sixTo4 = netip.MustParsePrefix("2002::/16") + compat = netip.MustParsePrefix("::/96") +) + +// Blocked reports whether p refuses an address. +func Blocked(a netip.Addr, p Policy) bool { + a = a.Unmap() + if v4, ok := embedded(a); ok { + a = v4 + } + switch { + case a.IsLinkLocalUnicast(), a.IsLinkLocalMulticast(): + // Instance metadata lives at 169.254.169.254, and nothing a model is + // published from does. + return !p.LinkLocal + case a.IsMulticast(), a.IsUnspecified(), a.IsInterfaceLocalMulticast(): + return !p.LinkLocal + case a.IsLoopback(): + return !p.Loopback + case a.IsPrivate(), cgnat.Contains(a): + return !p.Private + } + return false +} + +// embedded is the IPv4 address an IPv6 one carries, if it carries one. +func embedded(a netip.Addr) (netip.Addr, bool) { + if !a.Is6() { + return netip.Addr{}, false + } + b := a.As16() + switch { + case nat64.Contains(a): + return netip.AddrFrom4([4]byte(b[12:16])), true + case nat64l.Contains(a): + // RFC 8215's local-use prefix: the address in the last 32 bits too. + return netip.AddrFrom4([4]byte(b[12:16])), true + case sixTo4.Contains(a): + return netip.AddrFrom4([4]byte(b[2:6])), true + case compat.Contains(a) && !a.IsLoopback() && !a.IsUnspecified(): + return netip.AddrFrom4([4]byte(b[12:16])), true + } + return netip.Addr{}, false +} + +// Client is an HTTP client under a policy. timeout bounds a whole request; +// zero leaves it to the context, for downloads tens of gigabytes long. +// +// A proxy from the environment is used, since sites reach the internet +// through one; the dial check then sees only the proxy, so the target's own +// addresses are checked before the request is handed to it. +func Client(p Policy, timeout time.Duration) *http.Client { + d := &net.Dialer{Timeout: 30 * time.Second, KeepAlive: 30 * time.Second} + d.Control = func(_, address string, _ syscall.RawConn) error { + host, _, err := net.SplitHostPort(address) + if err != nil { + return err + } + a, err := netip.ParseAddr(host) + if err != nil { + return err + } + if Blocked(a, p) { + return fmt.Errorf("%w (%s)", ErrBlocked, a) + } + return nil + } + return &http.Client{ + Timeout: timeout, + Transport: &http.Transport{ + Proxy: proxy(p), + DialContext: d.DialContext, + TLSHandshakeTimeout: 30 * time.Second, + ResponseHeaderTimeout: 2 * time.Minute, + ForceAttemptHTTP2: true, + }, + CheckRedirect: func(req *http.Request, via []*http.Request) error { + if len(via) >= 10 { + return errors.New("too many redirects") + } + return CheckScheme(req.URL) + }, + } +} + +// proxy is the environment's proxy, for targets the policy allows. +func proxy(p Policy) func(*http.Request) (*url.URL, error) { + return func(req *http.Request) (*url.URL, error) { + u, err := http.ProxyFromEnvironment(req) + if err != nil || u == nil { + return u, err + } + addrs, err := net.DefaultResolver.LookupNetIP(req.Context(), "ip", req.URL.Hostname()) + if err != nil { + return nil, err + } + for _, a := range addrs { + if Blocked(a, p) { + return nil, fmt.Errorf("%w (%s)", ErrBlocked, a) + } + } + return u, nil + } +} + +// CheckScheme allows the two schemes a model is published over. file:// and +// the rest would read this machine's own disk. +func CheckScheme(u *url.URL) error { + if u.Scheme != "http" && u.Scheme != "https" { + return fmt.Errorf("%q is not an http or https source", u.Redacted()) + } + return nil +} + +// ControlPlane is the policy for fetches a control plane makes on an +// operator's word — checking that a model's source is there, reading a +// GGUF's header: the internet, and the cluster's own network, where its +// mirror is, but not instance metadata or anything link-local. +var ControlPlane = Policy{Private: true, Loopback: true} + diff --git a/internal/safedial/safedial_test.go b/internal/safedial/safedial_test.go new file mode 100644 index 0000000..0bc0ba2 --- /dev/null +++ b/internal/safedial/safedial_test.go @@ -0,0 +1,69 @@ +package safedial + +import ( + "errors" + "net/http" + "net/http/httptest" + "net/netip" + "testing" +) + +func TestWhatEachPolicyRefuses(t *testing.T) { + cases := []struct { + addr string + internet, control, private bool // blocked under Policy{}, ControlPlane, all allowed + }{ + {"203.0.113.7", false, false, false}, + {"2606:4700::1111", false, false, false}, + {"169.254.169.254", true, true, false}, + {"fe80::1", true, true, false}, + {"127.0.0.1", true, false, false}, + {"::1", true, false, false}, + {"10.0.0.146", true, false, false}, + {"fd00::5", true, false, false}, + {"100.64.1.1", true, false, false}, + {"0.0.0.0", true, true, false}, + // An IPv4 address carried in an IPv6 one is judged as itself. + {"::ffff:169.254.169.254", true, true, false}, + {"64:ff9b::a9fe:a9fe", true, true, false}, + {"64:ff9b::cb00:7107", false, false, false}, + {"2002:a9fe:a9fe::1", true, true, false}, + {"2002:0a00:0001::1", true, false, false}, + {"::a9fe:a9fe", true, true, false}, + } + all := Policy{Private: true, Loopback: true, LinkLocal: true} + for _, c := range cases { + a := netip.MustParseAddr(c.addr) + if got := Blocked(a, Policy{}); got != c.internet { + t.Errorf("%s under the internet-only policy: blocked=%v", c.addr, got) + } + if got := Blocked(a, ControlPlane); got != c.control { + t.Errorf("%s under the control plane's: blocked=%v", c.addr, got) + } + if got := Blocked(a, all); got != c.private { + t.Errorf("%s with everything allowed: blocked=%v", c.addr, got) + } + } +} + +func TestTheAddressDialledIsWhatIsChecked(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {})) + defer srv.Close() + if _, err := Client(Policy{}, 0).Get(srv.URL); !errors.Is(err, ErrBlocked) { + t.Errorf("loopback reached under the internet-only policy: %v", err) + } + resp, err := Client(ControlPlane, 0).Get(srv.URL) + if err != nil { + t.Fatalf("the control plane's own machine: %v", err) + } + resp.Body.Close() + + // A redirect to a scheme that reads the disk is refused. + redir := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, "file:///etc/passwd", http.StatusFound) + })) + defer redir.Close() + if _, err := Client(ControlPlane, 0).Get(redir.URL); err == nil { + t.Error("followed a redirect to file://") + } +} diff --git a/internal/sourcecheck/sourcecheck.go b/internal/sourcecheck/sourcecheck.go index ac8e48b..c387578 100644 --- a/internal/sourcecheck/sourcecheck.go +++ b/internal/sourcecheck/sourcecheck.go @@ -6,6 +6,8 @@ import ( "net/http" "strings" "time" + + "gitea.mynodes.xyz/ilker/llmgw/internal/safedial" ) // Check asks whether a model's weights are actually there. @@ -36,7 +38,9 @@ func Check(ctx context.Context, source string) (size int64, err error) { if err != nil { return 0, err } - resp, err := http.DefaultClient.Do(req) + // Not anything link-local: instance metadata answering a HEAD would be + // read through this, by whoever can add a model. + resp, err := safedial.Client(safedial.ControlPlane, 0).Do(req) if err != nil { return 0, fmt.Errorf("cannot reach %s: %w", url, err) } diff --git a/internal/web/admin_mirror.go b/internal/web/admin_mirror.go index 4df233d..4fd8727 100644 --- a/internal/web/admin_mirror.go +++ b/internal/web/admin_mirror.go @@ -19,6 +19,7 @@ import ( "gitea.mynodes.xyz/ilker/llmgw/internal/gguf" "gitea.mynodes.xyz/ilker/llmgw/internal/hf" "gitea.mynodes.xyz/ilker/llmgw/internal/mirror" + "gitea.mynodes.xyz/ilker/llmgw/internal/safedial" "gitea.mynodes.xyz/ilker/llmgw/internal/sourcecheck" "gitea.mynodes.xyz/ilker/llmgw/internal/store" ) @@ -799,7 +800,7 @@ func (p *AccountPages) openRepo(r *http.Request, d *adminMirrorData, repo string // kindOf reads what a GGUF file is; a variable so tests need no registry. var kindOf = func(ctx context.Context, url string) (gguf.Header, error) { - return gguf.KindFromURL(ctx, nil, url) + return gguf.KindFromURL(ctx, safedial.Client(safedial.ControlPlane, 30*time.Second), url) } // probeKinds reads the start of each single-file GGUF set, to tell LoRA