From ff77723622abcc3979575b54d51d3382bf5034bb Mon Sep 17 00:00:00 2001 From: mkalkere <14184493+mkalkere@users.noreply.github.com> Date: Mon, 14 Sep 2026 20:01:53 +0000 Subject: [PATCH 1/3] Harden ollama-proxy request handling Cap proxied request bodies at 32 MiB (413 before routing), enforce the loopback-only local engine invariant, and fail closed when a peer certificate pin is missing. Signed-off-by: mkalkere <14184493+mkalkere@users.noreply.github.com> --- services/ollama-proxy/ingress.go | 9 +- services/ollama-proxy/proxy.go | 71 ++++++++-- services/ollama-proxy/proxy_hardening_test.go | 133 ++++++++++++++++++ services/versions.json | 2 +- 4 files changed, 204 insertions(+), 11 deletions(-) create mode 100644 services/ollama-proxy/proxy_hardening_test.go diff --git a/services/ollama-proxy/ingress.go b/services/ollama-proxy/ingress.go index 984b7b20..be46d5b7 100644 --- a/services/ollama-proxy/ingress.go +++ b/services/ollama-proxy/ingress.go @@ -40,7 +40,10 @@ func (p *Proxy) setLocalBackend(b localBackend) { // localBackendTarget returns the loopback URL of the current local engine, and // false when none is set/healthy (the ingress then answers 503 rather than -// forwarding). The host defaults to 127.0.0.1 and is always loopback. +// forwarding). The host defaults to 127.0.0.1 and is always loopback: a +// non-loopback host is refused rather than dialed, so a compromised or buggy +// broker can never turn the cluster mTLS ingress into a forwarder to an +// arbitrary LAN address. func (p *Proxy) localBackendTarget() (*url.URL, bool) { p.backendMu.RLock() b := p.backend @@ -52,6 +55,10 @@ func (p *Proxy) localBackendTarget() (*url.URL, bool) { if host == "" { host = "127.0.0.1" } + if ip := net.ParseIP(host); ip == nil || !ip.IsLoopback() { + slog.Warn("refusing non-loopback local backend", "host", host, "port", b.Port) + return nil, false + } return &url.URL{Scheme: "http", Host: net.JoinHostPort(host, strconv.Itoa(b.Port))}, true } diff --git a/services/ollama-proxy/proxy.go b/services/ollama-proxy/proxy.go index ad4ac7a2..55673020 100644 --- a/services/ollama-proxy/proxy.go +++ b/services/ollama-proxy/proxy.go @@ -195,28 +195,36 @@ type workloadParams struct { WorkloadInfo Workload `json:"workloadInfo"` } +// errBodyTooLarge is returned by bufferBodyAndModel when the request body +// exceeds maxInferenceBodyBytes. +var errBodyTooLarge = stderrors.New("request body exceeds 32 MiB limit") + // bufferBodyAndModel reads the request body once and returns the raw bytes // (so each failover attempt can replay it — see the loop in handleHTTP) along // with the JSON "model" field for workload tracking. Inference bodies are // small (prompt + model), so full buffering is cheap. Returns (nil, "") when // the body is absent and an empty model when none is parseable. The caller // restores r.Body from the returned bytes before each forward attempt. -func bufferBodyAndModel(r *http.Request) ([]byte, string) { +// Bodies over maxInferenceBodyBytes are rejected with errBodyTooLarge. +func bufferBodyAndModel(r *http.Request) ([]byte, string, error) { if r.Body == nil { - return nil, "" + return nil, "", nil } - body, err := io.ReadAll(r.Body) + body, err := io.ReadAll(io.LimitReader(r.Body, maxInferenceBodyBytes+1)) _ = r.Body.Close() if err != nil { - return body, "" + return body, "", err + } + if len(body) > maxInferenceBodyBytes { + return nil, "", errBodyTooLarge } var probe struct { Model string `json:"model"` } if err := json.Unmarshal(body, &probe); err != nil { - return body, "" + return body, "", nil } - return body, probe.Model + return body, probe.Model, nil } type statusCapture struct { @@ -536,6 +544,12 @@ const ( proxyReadHeaderTimeout = 10 * time.Second proxyServerIdleTimeout = 90 * time.Second maxModelListBytes = 16 << 20 + // maxInferenceBodyBytes caps a single proxied request body. handleHTTP + // buffers the whole body to replay it across failover attempts, so an + // uncapped read lets any loopback client (or any visited web page, given + // the loopback CORS policy) grow the proxy until it OOMs. Bodies over the + // cap are rejected with 413 before any routing work happens. + maxInferenceBodyBytes = 32 << 20 ) // idleClientWriteTimeout bounds how long a single write of streamed response @@ -903,12 +917,17 @@ func (p *Proxy) peerHTTPTransport(peerUUID string) *http.Transport { tr.CloseIdleConnections() delete(p.peerTransports, peerUUID) } + // Fail closed: a peer whose certificate pin is gone (or was never there) + // must never be dialed with an unpinned transport, even if the pin + // vanished between candidate selection and dial time. The returned + // transport refuses every dial, so the request fails over to the next + // candidate (or 502s) instead of reaching the peer unauthenticated. if p.mesh == nil { - return newProxyTransport(nil) + return unpinnedPeerTransport() } cfg, ok := p.mesh.ClientTLSConfig(peerUUID) if !ok { - return newProxyTransport(nil) + return unpinnedPeerTransport() } tr := newProxyTransport(cfg) if p.peerTransports == nil { @@ -918,6 +937,22 @@ func (p *Proxy) peerHTTPTransport(peerUUID string) *http.Transport { return tr } +// errPeerUnpinned is the dial error of a fail-closed peer transport: the +// peer's certificate pin was absent when the transport was built. +var errPeerUnpinned = stderrors.New("peer is not a pinned cluster member") + +// unpinnedPeerTransport returns a transport that fails every dial. It is the +// fail-closed answer when a peer has no live certificate pin — used instead +// of an unpinned (plaintext-auth) transport so a vanished pin can never +// silently downgrade a peer connection. +func unpinnedPeerTransport() *http.Transport { + return &http.Transport{ + DialContext: func(_ context.Context, _, _ string) (net.Conn, error) { + return nil, errPeerUnpinned + }, + } +} + // dropUnpinnedPeerTransports closes idle conns for peer Transports whose pins // are gone. Safe to call from the mesh Watch callback. func (p *Proxy) dropUnpinnedPeerTransports() { @@ -1140,7 +1175,25 @@ func (p *Proxy) handleHTTP(w http.ResponseWriter, r *http.Request) { // Parse the request's model before choosing a node. Model eligibility only // applies to inference routes; control endpoints retain their existing // routing behavior even when their JSON happens to contain a model field. - bodyBytes, model := bufferBodyAndModel(r) + bodyBytes, model, bodyErr := bufferBodyAndModel(r) + if bodyErr != nil { + // errBodyTooLarge (413) is answered here so the oversized body never + // reaches candidate selection or an engine. Any other read error + // leaves bodyBytes as whatever was read before the failure. + if stderrors.Is(bodyErr, errBodyTooLarge) { + cors.Apply(w.Header()) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusRequestEntityTooLarge) + _, _ = w.Write([]byte(`{"error":"request body exceeds 32 MiB limit"}`)) + p.codec.Notify("proxy/request", RequestEvent{ + ID: reqID, Method: r.Method, Path: r.URL.Path, Target: "rejected", + Status: http.StatusRequestEntityTooLarge, Duration: time.Since(start).Milliseconds(), + Error: bodyErr.Error(), + }) + return + } + slog.Warn("request body read error", "id", reqID, "err", bodyErr) + } isInf := isInferenceRequest(r.Method, r.URL.Path) routingModel := "" if isInf { diff --git a/services/ollama-proxy/proxy_hardening_test.go b/services/ollama-proxy/proxy_hardening_test.go new file mode 100644 index 00000000..bc182f41 --- /dev/null +++ b/services/ollama-proxy/proxy_hardening_test.go @@ -0,0 +1,133 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +// TestBufferBodyAndModelRejectsOversizedBody: the inbound body cap is the +// fix for the unauthenticated OOM — handleHTTP buffers the whole body for +// failover replay, so an uncapped read lets any loopback client (or any +// visited web page) grow the proxy until it dies. +func TestBufferBodyAndModelRejectsOversizedBody(t *testing.T) { + big := strings.NewReader(strings.Repeat("x", maxInferenceBodyBytes+1)) + req := httptest.NewRequest(http.MethodPost, "/api/chat", big) + if _, _, err := bufferBodyAndModel(req); err != errBodyTooLarge { + t.Fatalf("oversized body err = %v, want errBodyTooLarge", err) + } + + // Exactly at the cap still passes. + exact := strings.NewReader(strings.Repeat("x", maxInferenceBodyBytes)) + req = httptest.NewRequest(http.MethodPost, "/api/chat", exact) + body, _, err := bufferBodyAndModel(req) + if err != nil { + t.Fatalf("at-cap body err = %v, want nil", err) + } + if len(body) != maxInferenceBodyBytes { + t.Fatalf("at-cap body len = %d, want %d", len(body), maxInferenceBodyBytes) + } + + // Small bodies still parse the model field. + req = httptest.NewRequest(http.MethodPost, "/api/chat", strings.NewReader(`{"model":"llama3"}`)) + _, model, err := bufferBodyAndModel(req) + if err != nil || model != "llama3" { + t.Fatalf("small body model = %q, err = %v; want %q, nil", model, err, "llama3") + } +} + +// TestHandleHTTPRejectsOversizedBody: an over-cap POST is answered 413 with +// a JSON error before any routing or engine work, and the over-cap read +// consumes at most cap+1 bytes from the client. +func TestHandleHTTPRejectsOversizedBody(t *testing.T) { + p := testProxy(NewDiscovery(), 11435) + + pr, pw := io.Pipe() + go func() { + _, _ = pw.Write([]byte(strings.Repeat("x", maxInferenceBodyBytes+1024))) + _ = pw.Close() + }() + req := httptest.NewRequest(http.MethodPost, "/api/chat", pr) + rec := httptest.NewRecorder() + + p.handleHTTP(rec, req) + + if rec.Code != http.StatusRequestEntityTooLarge { + t.Fatalf("oversized POST status = %d, want %d", rec.Code, http.StatusRequestEntityTooLarge) + } + if ct := rec.Header().Get("Content-Type"); ct != "application/json" { + t.Errorf("Content-Type = %q, want application/json", ct) + } + if !strings.Contains(rec.Body.String(), "request body exceeds") { + t.Errorf("413 body = %q, want it to name the limit", rec.Body.String()) + } +} + +// TestHandleHTTPAcceptsNormalBody: a small body is not 413'd (it takes the +// normal no-candidate rejection path in this fixture). +func TestHandleHTTPAcceptsNormalBody(t *testing.T) { + p := testProxy(NewDiscovery(), 11435) + req := httptest.NewRequest(http.MethodPost, "/api/chat", strings.NewReader(`{"model":"llama3"}`)) + rec := httptest.NewRecorder() + + p.handleHTTP(rec, req) + + if rec.Code == http.StatusRequestEntityTooLarge { + t.Fatalf("normal POST status = 413, want the ordinary rejection path") + } +} + +// TestLocalBackendTargetRejectsNonLoopback: the documented "always loopback" +// invariant is now enforced — a non-loopback host from node/set-local-backend +// is refused instead of dialed, so the mTLS ingress can never be turned into +// a forwarder to an arbitrary LAN address. +func TestLocalBackendTargetRejectsNonLoopback(t *testing.T) { + p := testProxy(NewDiscovery(), 11435) + + for _, host := range []string{"192.168.1.5", "10.0.0.2", "example.com", "::ffff:192.168.1.5"} { + p.setLocalBackend(localBackend{Engine: "ollama", Host: host, Port: 11436, Healthy: true}) + if _, ok := p.localBackendTarget(); ok { + t.Errorf("localBackendTarget accepted non-loopback host %q", host) + } + } + for _, host := range []string{"", "127.0.0.1", "127.0.0.2", "::1"} { + p.setLocalBackend(localBackend{Engine: "ollama", Host: host, Port: 11436, Healthy: true}) + u, ok := p.localBackendTarget() + if !ok { + t.Errorf("localBackendTarget rejected loopback host %q", host) + continue + } + if u.Scheme != "http" { + t.Errorf("localBackendTarget scheme = %q, want http", u.Scheme) + } + } +} + +// TestPeerHTTPTransportFailsClosedWithoutPin: with no live certificate pin +// the transport refuses every dial instead of falling back to an unpinned +// transport — a pin that vanishes between candidate selection and dial time +// can never silently downgrade to an unauthenticated connection. +func TestPeerHTTPTransportFailsClosedWithoutPin(t *testing.T) { + p := testProxy(NewDiscovery(), 11435) // mesh nil => unclustered + + tr := p.peerHTTPTransport("no-such-peer") + if tr == nil { + t.Fatal("peerHTTPTransport returned nil; want a fail-closed transport") + } + if tr.TLSClientConfig != nil { + t.Error("fail-closed transport must not carry a TLS config") + } + conn, err := tr.DialContext(t.Context(), "tcp", "192.0.2.10:443") + if err != errPeerUnpinned { + t.Errorf("dial err = %v, want errPeerUnpinned", err) + } + if conn != nil { + _ = conn.Close() + t.Error("fail-closed transport dialed successfully") + } +} diff --git a/services/versions.json b/services/versions.json index 29d8c230..16002c2d 100644 --- a/services/versions.json +++ b/services/versions.json @@ -3,7 +3,7 @@ "product": "0.91.7", "installer": "0.91.7", "components": { - "ollama-proxy": "0.26.2", + "ollama-proxy": "0.26.3", "lmstudio-proxy": "0.16.2", "nvpair-node-info": "0.13.3", "nvpair-node-scanner": "0.20.3", From 41f4d2e200873e9d12f202c290571084cd4b7859 Mon Sep 17 00:00:00 2001 From: mkalkere <14184493+mkalkere@users.noreply.github.com> Date: Mon, 14 Sep 2026 20:01:54 +0000 Subject: [PATCH 2/3] Harden lmstudio-proxy request handling Same three fixes as the Ollama proxy: 32 MiB request body cap with a 413 before routing, loopback-only local engine gate, and fail-closed peer transports when the certificate pin is absent. Signed-off-by: mkalkere <14184493+mkalkere@users.noreply.github.com> --- services/lmstudio-proxy/ingress.go | 9 +- services/lmstudio-proxy/proxy.go | 71 ++++++++-- .../lmstudio-proxy/proxy_hardening_test.go | 133 ++++++++++++++++++ services/versions.json | 2 +- 4 files changed, 204 insertions(+), 11 deletions(-) create mode 100644 services/lmstudio-proxy/proxy_hardening_test.go diff --git a/services/lmstudio-proxy/ingress.go b/services/lmstudio-proxy/ingress.go index 2b70e697..e5496a89 100644 --- a/services/lmstudio-proxy/ingress.go +++ b/services/lmstudio-proxy/ingress.go @@ -40,7 +40,10 @@ func (p *Proxy) setLocalBackend(b localBackend) { // localBackendTarget returns the loopback URL of the current local engine, and // false when none is set/healthy (the ingress then answers 503 rather than -// forwarding). The host defaults to 127.0.0.1 and is always loopback. +// forwarding). The host defaults to 127.0.0.1 and is always loopback: a +// non-loopback host is refused rather than dialed, so a compromised or buggy +// broker can never turn the cluster mTLS ingress into a forwarder to an +// arbitrary LAN address. func (p *Proxy) localBackendTarget() (*url.URL, bool) { p.backendMu.RLock() b := p.backend @@ -52,6 +55,10 @@ func (p *Proxy) localBackendTarget() (*url.URL, bool) { if host == "" { host = "127.0.0.1" } + if ip := net.ParseIP(host); ip == nil || !ip.IsLoopback() { + slog.Warn("refusing non-loopback local backend", "host", host, "port", b.Port) + return nil, false + } return &url.URL{Scheme: "http", Host: net.JoinHostPort(host, strconv.Itoa(b.Port))}, true } diff --git a/services/lmstudio-proxy/proxy.go b/services/lmstudio-proxy/proxy.go index 6e619e77..09b576b0 100644 --- a/services/lmstudio-proxy/proxy.go +++ b/services/lmstudio-proxy/proxy.go @@ -190,28 +190,36 @@ type workloadParams struct { WorkloadInfo Workload `json:"workloadInfo"` } +// errBodyTooLarge is returned by bufferBodyAndModel when the request body +// exceeds maxInferenceBodyBytes. +var errBodyTooLarge = stderrors.New("request body exceeds 32 MiB limit") + // bufferBodyAndModel reads the request body once and returns the raw bytes // (so each failover attempt can replay it — see the loop in handleHTTP) along // with the JSON "model" field for workload tracking. Inference bodies are // small (prompt + model), so full buffering is cheap. Returns (nil, "") when // the body is absent and an empty model when none is parseable. The caller // restores r.Body from the returned bytes before each forward attempt. -func bufferBodyAndModel(r *http.Request) ([]byte, string) { +// Bodies over maxInferenceBodyBytes are rejected with errBodyTooLarge. +func bufferBodyAndModel(r *http.Request) ([]byte, string, error) { if r.Body == nil { - return nil, "" + return nil, "", nil } - body, err := io.ReadAll(r.Body) + body, err := io.ReadAll(io.LimitReader(r.Body, maxInferenceBodyBytes+1)) _ = r.Body.Close() if err != nil { - return body, "" + return body, "", err + } + if len(body) > maxInferenceBodyBytes { + return nil, "", errBodyTooLarge } var probe struct { Model string `json:"model"` } if err := json.Unmarshal(body, &probe); err != nil { - return body, "" + return body, "", nil } - return body, probe.Model + return body, probe.Model, nil } type statusCapture struct { @@ -520,6 +528,12 @@ const ( proxyReadHeaderTimeout = 10 * time.Second proxyServerIdleTimeout = 90 * time.Second maxModelListBytes = 16 << 20 + // maxInferenceBodyBytes caps a single proxied request body. handleHTTP + // buffers the whole body to replay it across failover attempts, so an + // uncapped read lets any loopback client (or any visited web page, given + // the loopback CORS policy) grow the proxy until it OOMs. Bodies over the + // cap are rejected with 413 before any routing work happens. + maxInferenceBodyBytes = 32 << 20 ) // idleClientWriteTimeout bounds how long a single write of streamed response @@ -749,12 +763,17 @@ func (p *Proxy) peerHTTPTransport(peerUUID string) *http.Transport { tr.CloseIdleConnections() delete(p.peerTransports, peerUUID) } + // Fail closed: a peer whose certificate pin is gone (or was never there) + // must never be dialed with an unpinned transport, even if the pin + // vanished between candidate selection and dial time. The returned + // transport refuses every dial, so the request fails over to the next + // candidate (or 502s) instead of reaching the peer unauthenticated. if p.mesh == nil { - return newProxyTransport(nil) + return unpinnedPeerTransport() } cfg, ok := p.mesh.ClientTLSConfig(peerUUID) if !ok { - return newProxyTransport(nil) + return unpinnedPeerTransport() } tr := newProxyTransport(cfg) if p.peerTransports == nil { @@ -764,6 +783,22 @@ func (p *Proxy) peerHTTPTransport(peerUUID string) *http.Transport { return tr } +// errPeerUnpinned is the dial error of a fail-closed peer transport: the +// peer's certificate pin was absent when the transport was built. +var errPeerUnpinned = stderrors.New("peer is not a pinned cluster member") + +// unpinnedPeerTransport returns a transport that fails every dial. It is the +// fail-closed answer when a peer has no live certificate pin — used instead +// of an unpinned (plaintext-auth) transport so a vanished pin can never +// silently downgrade a peer connection. +func unpinnedPeerTransport() *http.Transport { + return &http.Transport{ + DialContext: func(_ context.Context, _, _ string) (net.Conn, error) { + return nil, errPeerUnpinned + }, + } +} + // dropUnpinnedPeerTransports closes idle conns for peer Transports whose pins // are gone. Safe to call from the mesh Watch callback. func (p *Proxy) dropUnpinnedPeerTransports() { @@ -945,7 +980,25 @@ func (p *Proxy) handleHTTP(w http.ResponseWriter, r *http.Request) { // Parse the request's model before choosing a node. Model eligibility only // applies to inference routes; control endpoints retain their existing // routing behavior even when their JSON happens to contain a model field. - bodyBytes, model := bufferBodyAndModel(r) + bodyBytes, model, bodyErr := bufferBodyAndModel(r) + if bodyErr != nil { + // errBodyTooLarge (413) is answered here so the oversized body never + // reaches candidate selection or an engine. Any other read error + // leaves bodyBytes as whatever was read before the failure. + if stderrors.Is(bodyErr, errBodyTooLarge) { + cors.Apply(w.Header()) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusRequestEntityTooLarge) + _, _ = w.Write([]byte(`{"error":"request body exceeds 32 MiB limit"}`)) + p.codec.Notify("proxy/request", RequestEvent{ + ID: reqID, Method: r.Method, Path: r.URL.Path, Target: "rejected", + Status: http.StatusRequestEntityTooLarge, Duration: time.Since(start).Milliseconds(), + Error: bodyErr.Error(), + }) + return + } + slog.Warn("request body read error", "id", reqID, "err", bodyErr) + } isInf := isInferenceRequest(r.Method, r.URL.Path) routingModel := "" if isInf { diff --git a/services/lmstudio-proxy/proxy_hardening_test.go b/services/lmstudio-proxy/proxy_hardening_test.go new file mode 100644 index 00000000..c976922d --- /dev/null +++ b/services/lmstudio-proxy/proxy_hardening_test.go @@ -0,0 +1,133 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +// TestBufferBodyAndModelRejectsOversizedBody: the inbound body cap is the +// fix for the unauthenticated OOM — handleHTTP buffers the whole body for +// failover replay, so an uncapped read lets any loopback client (or any +// visited web page) grow the proxy until it dies. +func TestBufferBodyAndModelRejectsOversizedBody(t *testing.T) { + big := strings.NewReader(strings.Repeat("x", maxInferenceBodyBytes+1)) + req := httptest.NewRequest(http.MethodPost, "/api/chat", big) + if _, _, err := bufferBodyAndModel(req); err != errBodyTooLarge { + t.Fatalf("oversized body err = %v, want errBodyTooLarge", err) + } + + // Exactly at the cap still passes. + exact := strings.NewReader(strings.Repeat("x", maxInferenceBodyBytes)) + req = httptest.NewRequest(http.MethodPost, "/api/chat", exact) + body, _, err := bufferBodyAndModel(req) + if err != nil { + t.Fatalf("at-cap body err = %v, want nil", err) + } + if len(body) != maxInferenceBodyBytes { + t.Fatalf("at-cap body len = %d, want %d", len(body), maxInferenceBodyBytes) + } + + // Small bodies still parse the model field. + req = httptest.NewRequest(http.MethodPost, "/api/chat", strings.NewReader(`{"model":"llama3"}`)) + _, model, err := bufferBodyAndModel(req) + if err != nil || model != "llama3" { + t.Fatalf("small body model = %q, err = %v; want %q, nil", model, err, "llama3") + } +} + +// TestHandleHTTPRejectsOversizedBody: an over-cap POST is answered 413 with +// a JSON error before any routing or engine work, and the over-cap read +// consumes at most cap+1 bytes from the client. +func TestHandleHTTPRejectsOversizedBody(t *testing.T) { + p := testProxy(NewDiscovery(), 11435) + + pr, pw := io.Pipe() + go func() { + _, _ = pw.Write([]byte(strings.Repeat("x", maxInferenceBodyBytes+1024))) + _ = pw.Close() + }() + req := httptest.NewRequest(http.MethodPost, "/api/chat", pr) + rec := httptest.NewRecorder() + + p.handleHTTP(rec, req) + + if rec.Code != http.StatusRequestEntityTooLarge { + t.Fatalf("oversized POST status = %d, want %d", rec.Code, http.StatusRequestEntityTooLarge) + } + if ct := rec.Header().Get("Content-Type"); ct != "application/json" { + t.Errorf("Content-Type = %q, want application/json", ct) + } + if !strings.Contains(rec.Body.String(), "request body exceeds") { + t.Errorf("413 body = %q, want it to name the limit", rec.Body.String()) + } +} + +// TestHandleHTTPAcceptsNormalBody: a small body is not 413'd (it takes the +// normal no-candidate rejection path in this fixture). +func TestHandleHTTPAcceptsNormalBody(t *testing.T) { + p := testProxy(NewDiscovery(), 11435) + req := httptest.NewRequest(http.MethodPost, "/api/chat", strings.NewReader(`{"model":"llama3"}`)) + rec := httptest.NewRecorder() + + p.handleHTTP(rec, req) + + if rec.Code == http.StatusRequestEntityTooLarge { + t.Fatalf("normal POST status = 413, want the ordinary rejection path") + } +} + +// TestLocalBackendTargetRejectsNonLoopback: the documented "always loopback" +// invariant is now enforced — a non-loopback host from node/set-local-backend +// is refused instead of dialed, so the mTLS ingress can never be turned into +// a forwarder to an arbitrary LAN address. +func TestLocalBackendTargetRejectsNonLoopback(t *testing.T) { + p := testProxy(NewDiscovery(), 11435) + + for _, host := range []string{"192.168.1.5", "10.0.0.2", "example.com", "::ffff:192.168.1.5"} { + p.setLocalBackend(localBackend{Engine: "lmstudio", Host: host, Port: 11436, Healthy: true}) + if _, ok := p.localBackendTarget(); ok { + t.Errorf("localBackendTarget accepted non-loopback host %q", host) + } + } + for _, host := range []string{"", "127.0.0.1", "127.0.0.2", "::1"} { + p.setLocalBackend(localBackend{Engine: "lmstudio", Host: host, Port: 11436, Healthy: true}) + u, ok := p.localBackendTarget() + if !ok { + t.Errorf("localBackendTarget rejected loopback host %q", host) + continue + } + if u.Scheme != "http" { + t.Errorf("localBackendTarget scheme = %q, want http", u.Scheme) + } + } +} + +// TestPeerHTTPTransportFailsClosedWithoutPin: with no live certificate pin +// the transport refuses every dial instead of falling back to an unpinned +// transport — a pin that vanishes between candidate selection and dial time +// can never silently downgrade to an unauthenticated connection. +func TestPeerHTTPTransportFailsClosedWithoutPin(t *testing.T) { + p := testProxy(NewDiscovery(), 11435) // mesh nil => unclustered + + tr := p.peerHTTPTransport("no-such-peer") + if tr == nil { + t.Fatal("peerHTTPTransport returned nil; want a fail-closed transport") + } + if tr.TLSClientConfig != nil { + t.Error("fail-closed transport must not carry a TLS config") + } + conn, err := tr.DialContext(t.Context(), "tcp", "192.0.2.10:443") + if err != errPeerUnpinned { + t.Errorf("dial err = %v, want errPeerUnpinned", err) + } + if conn != nil { + _ = conn.Close() + t.Error("fail-closed transport dialed successfully") + } +} diff --git a/services/versions.json b/services/versions.json index 16002c2d..eca45782 100644 --- a/services/versions.json +++ b/services/versions.json @@ -4,7 +4,7 @@ "installer": "0.91.7", "components": { "ollama-proxy": "0.26.3", - "lmstudio-proxy": "0.16.2", + "lmstudio-proxy": "0.16.3", "nvpair-node-info": "0.13.3", "nvpair-node-scanner": "0.20.3", "nvpair-manual-nodes": "0.11.1", From c3244d84e0cf448b444c53ef55918848c5ced291 Mon Sep 17 00:00:00 2001 From: mkalkere <14184493+mkalkere@users.noreply.github.com> Date: Mon, 14 Sep 2026 20:01:54 +0000 Subject: [PATCH 3/3] Document proxy request hardening Sequence and flow diagrams for the body cap, the loopback backend gate, and fail-closed peer TLS, plus a reading-order entry in the README. Signed-off-by: mkalkere <14184493+mkalkere@users.noreply.github.com> --- README.md | 3 + docs/proxy-request-hardening.mdx | 137 +++++++++++++++++++++++++++++++ 2 files changed, 140 insertions(+) create mode 100644 docs/proxy-request-hardening.mdx diff --git a/README.md b/README.md index 0f0a7242..5eaafd95 100644 --- a/README.md +++ b/README.md @@ -239,6 +239,9 @@ Each entry assumes the ones before it. 8. **[Developer guide](docs/developing.mdx)** — read this before contributing: where the code lives, how a change travels through the layers, and the conventions the project enforces. +9. **[Proxy request hardening](docs/proxy-request-hardening.mdx)** — request + body limits, the loopback-only local engine invariant, and fail-closed peer + TLS in the inference proxies. Component references, for when you already know what you are looking for: diff --git a/docs/proxy-request-hardening.mdx b/docs/proxy-request-hardening.mdx new file mode 100644 index 00000000..8190919a --- /dev/null +++ b/docs/proxy-request-hardening.mdx @@ -0,0 +1,137 @@ +{/* +SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +SPDX-License-Identifier: Apache-2.0 +*/} + +# Proxy request hardening + +Both inference proxies — `services/ollama-proxy` and `services/lmstudio-proxy` +— share the same request path: a loopback HTTP ingress plus a cluster mTLS +ingress on one port, ordered failover across eligible node candidates, and +model eligibility checks before routing. This document covers three hardening +fixes to that path. Nothing here changes the JSON-RPC surface or the proxy +endpoints clients use. + +## 1. Request bodies are capped at 32 MiB + +### The problem + +`handleHTTP` buffers the entire request body (`io.ReadAll(r.Body)`) so each +failover attempt can replay it. The read was uncapped: any loopback client — +and, given the proxies' permissive loopback CORS posture, any web page the +user visits — could grow the proxy process until it OOMed. No authentication +stands in front of the loopback ingress, so this was remotely triggerable by +anything able to reach the loopback port. + +### The fix + +`maxInferenceBodyBytes = 32 << 20` caps a single proxied request body. +`bufferBodyAndModel` now reads through `io.LimitReader(r.Body, maxInferenceBodyBytes+1)`: + +- Bodies over the cap return `errBodyTooLarge`. `handleHTTP` answers `413` + with a JSON error **before** candidate selection or any engine work, and + emits a `proxy/request` notification with the target marked `rejected`. +- Bodies at or under the cap behave exactly as before, including model-field + extraction for workload tracking. +- Any other read error leaves the partially read bytes in place, as before. + +```mermaid +sequenceDiagram + participant Client + participant handleHTTP + participant bufferBodyAndModel + participant Scheduler + + Client->>handleHTTP: POST /api/chat (body) + handleHTTP->>bufferBodyAndModel: read via LimitReader(cap+1) + alt body > 32 MiB + bufferBodyAndModel-->>handleHTTP: errBodyTooLarge + handleHTTP->>Client: 413 {"error":"request body exceeds 32 MiB limit"} + else body within cap + bufferBodyAndModel-->>handleHTTP: body, model + handleHTTP->>Scheduler: candidate selection, failover + end +``` + +The proxy never holds more than cap+1 bytes per in-flight request body, so a +malicious or buggy client can no longer exhaust proxy memory. + +## 2. The local backend must be loopback + +### The problem + +The proxies document a "the host is always loopback" invariant for the local +engine, but `localBackendTarget()` in `ingress.go` never enforced it: it built +a dial URL from whatever host the broker supplied via `set-local-backend`. +A compromised or buggy broker could have pointed the cluster mTLS ingress at +an arbitrary LAN address, turning the proxy into a forwarder for plaintext +inference traffic to hosts the operator never approved. + +### The fix + +`localBackendTarget()` now gates the host with `net.ParseIP(host).IsLoopback()`: + +- A non-loopback host (or an unparseable one — hostnames included) is refused + with a warning log, and the ingress answers `503` rather than forwarding. +- `127.0.0.1`, `::1`, other `127.x` addresses, and the empty default (which + resolves to `127.0.0.1`) are accepted as before. + +```mermaid +flowchart TD + A[broker: set-local-backend] --> B[localBackendTarget] + B --> C{host empty?} + C -->|yes| D[default 127.0.0.1] + C -->|no| E{ParseIP → IsLoopback?} + D --> F[dial local engine] + E -->|yes| F + E -->|no| G[warn + refuse
ingress answers 503] +``` + +The broker always advertises `127.0.0.1` (`nvpair-ui-broker/advertiser.go`), +so legitimate traffic is unaffected; the gate only bites when the supplied +host deviates from the documented invariant. + +## 3. Unpinned peers fail closed + +### The problem + +`peerHTTPTransport()` built the per-peer TLS transport from the cluster's +certificate pin store. When the pin was absent — no cluster mesh, or the pin +vanished between candidate selection and dial time — it returned an **unpinned** +transport. A request could then silently reach the peer over a connection with +no mutual-TLS authentication, exactly in the window where the operator had +reason to believe the peer was gone. + +### The fix + +The function now returns a fail-closed transport when no live pin exists: + +- The transport's `DialContext` always fails with `errPeerUnpinned`; it + carries no `TLSClientConfig` and never opens a connection. +- Dial errors flow through the existing failover path: the request tries the + next candidate, or the proxy answers `502` when none remain. + +```mermaid +flowchart TD + A[candidate selection] --> B[peerHTTPTransport] + B --> C{live pin for peer?} + C -->|yes| D[pinned mTLS transport
cached per peer] + C -->|no| E[fail-closed transport
every dial → errPeerUnpinned] + D --> F[forward to peer] + E --> G[fail over to next candidate
or 502] +``` + +Pins are still dropped from the cache when they disappear +(`dropUnpinnedPeerTransports`); the difference is that a missing pin can no +longer downgrade to an unauthenticated connection in the meantime. + +## Validation + +- `proxy_hardening_test.go` in each proxy covers all three fixes: over-cap and + at-cap bodies, the 413 response shape, loopback/non-loopback backend hosts, + and the fail-closed dial error. Run with `go test -race ./...` from the + component directory. +- No JSON-RPC method or payload changed, so no contract regeneration was + needed. +- Component versions bumped per `services/VERSIONING.md`: `ollama-proxy` + 0.26.2 → 0.26.3, `lmstudio-proxy` 0.16.2 → 0.16.3.