fix(proxy): resolve edge client IP through the trusted-proxy boundary
The edge recorded the connecting peer's address (`RemoteAddr`) for every monitoring record, ignoring forwarding headers. Behind an ingress, reverse proxy or load balancer that is the *same address for every viewer*, so the admin session list showed one indistinguishable client per node and any per-viewer analysis built on those records was reading the ingress address. The native API surface has resolved this correctly for a while via `internal/clientip`, which walks `X-Forwarded-For` right-to-left and returns the first untrusted hop. The edge simply never used it. This wires the same resolver into proxy mode so both surfaces share one trust model. The trust boundary is what makes this safe to believe: forwarding headers are consulted only when the connecting peer is itself in `clientip.trusted_proxies`, so an ordinary client cannot choose the address it is recorded under. If the trust list cannot be loaded the resolver keeps an empty list, which ignores forwarding headers entirely and falls back to the peer — failing closed on trust rather than failing startup. A directly-exposed edge with no resolver wired keeps the previous behavior. The list is reloaded on the node config watcher's change hook, so the edge follows a hot-reloaded setting without a restart, matching central. Only proxy mode is affected; `internal/transcodenode` records no client address. This was found while scoping re-stream detection (now issue #522) — an IP-based signal is meaningless if every viewer presents as the ingress — but the defect is independent of that feature and is worth fixing on its own. Part of #305
This commit is contained in:
@@ -682,6 +682,33 @@ func main() {
|
||||
if mode == "proxy" {
|
||||
srv := proxy.NewServer(watcher, tracker)
|
||||
srv.SetRevocationStore(revStore)
|
||||
// Resolve the viewer's address through the operator's trusted-proxy
|
||||
// boundary, as the native surface does. Without this the edge records
|
||||
// the connecting peer, which behind an ingress or load balancer is the
|
||||
// same address for every viewer — collapsing the admin session view and
|
||||
// any per-viewer analysis to one indistinguishable client.
|
||||
edgeResolver := clientip.NewResolver(nil)
|
||||
if cidrs, cidrErr := clientip.LoadTrustedCIDRs(appCtx, settingsRepo); cidrErr != nil {
|
||||
// Fail closed on trust, not on startup: an empty trusted list means
|
||||
// forwarding headers are ignored entirely, so a spoofed header can
|
||||
// never be believed. The edge falls back to the connecting peer.
|
||||
slog.Warn("edge client-IP trust list unavailable; ignoring forwarding headers",
|
||||
"component", "proxy", "error", cidrErr)
|
||||
} else {
|
||||
edgeResolver.UpdateTrustedCIDRs(cidrs)
|
||||
}
|
||||
// Keep the boundary current: the setting is hot-reloadable on central,
|
||||
// so the edge must not need a restart to follow it.
|
||||
watcher.OnChange(func(_, _ *config.Config) {
|
||||
cidrs, loadErr := clientip.LoadTrustedCIDRs(context.Background(), settingsRepo)
|
||||
if loadErr != nil {
|
||||
slog.Warn("edge client-IP trust list reload failed",
|
||||
"component", "proxy", "error", loadErr)
|
||||
return
|
||||
}
|
||||
edgeResolver.UpdateTrustedCIDRs(cidrs)
|
||||
})
|
||||
srv.SetClientIPResolver(edgeResolver)
|
||||
handler = srv.Handler()
|
||||
} else {
|
||||
srv := transcodenode.NewServer(watcher, tracker)
|
||||
|
||||
@@ -341,8 +341,11 @@ one session still present as one address, and a polling consumer cannot see fan-
|
||||
all. Building the heuristic requires collecting bounded viewer observations **at
|
||||
authenticated media-serve time**, not consuming the existing snapshot. Note also that C14
|
||||
proper — a downstream proxy re-broadcasting one pulled stream — is invisible to any
|
||||
IP-based signal by construction: Silo sees exactly one address. **Detection ❌ /
|
||||
Enforcement ❌.**
|
||||
IP-based signal by construction: Silo sees exactly one address. (The edge's own address
|
||||
resolution was separately blind behind an ingress and is now fixed — `edgeClientIP`
|
||||
resolves through the `clientip.trusted_proxies` boundary — but that only makes viewers
|
||||
distinguishable; it does not create the per-session address *set* the heuristic needs.)
|
||||
**Detection ❌ / Enforcement ❌.**
|
||||
|
||||
**C15. Many concurrent streams feeding a restream service (over cap).**
|
||||
This *is* caught — but only as a raw over-count, not labeled as re-streaming.
|
||||
@@ -625,10 +628,11 @@ Then, as originally scoped:
|
||||
(E29).
|
||||
5. **Implement the restream heuristic** (C16 fan-out; **not** C14): the fingerprints do
|
||||
**not** already flow in usable form — see correction #9. A distinct-viewer-IP rule
|
||||
needs per-request observation at media-serve time plus a trust-boundary-correct client
|
||||
address at the edge (`internal/proxy/server.go`'s `edgeClientIP` reads `RemoteAddr`
|
||||
and ignores `X-Forwarded-For`, so behind ingress every viewer collapses to one
|
||||
address).
|
||||
needs per-request observation at media-serve time. The edge trust-boundary half is
|
||||
**done**: `internal/proxy/server.go`'s `edgeClientIP` now resolves through the
|
||||
operator's `clientip.trusted_proxies` boundary like the native surface, so viewers
|
||||
behind an ingress are distinguishable and a forged `X-Forwarded-For` from an untrusted
|
||||
peer is ignored. Tracked as issue #522.
|
||||
Decisions are now settled: distinct viewer IPs per session as the signal, Redis
|
||||
with a ~24h TTL when configured and in-memory otherwise, detection **on by
|
||||
default and alert-only**, with auto-kill behind an operator setting that defaults
|
||||
|
||||
@@ -0,0 +1,111 @@
|
||||
package proxy
|
||||
|
||||
import (
|
||||
"net"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
|
||||
"github.com/Silo-Server/silo-server/internal/clientip"
|
||||
)
|
||||
|
||||
func mustCIDRs(t *testing.T, raw ...string) []*net.IPNet {
|
||||
t.Helper()
|
||||
out := make([]*net.IPNet, 0, len(raw))
|
||||
for _, entry := range raw {
|
||||
_, network, err := net.ParseCIDR(entry)
|
||||
if err != nil {
|
||||
t.Fatalf("ParseCIDR(%q): %v", entry, err)
|
||||
}
|
||||
out = append(out, network)
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// The edge attributes monitoring records to a viewer address. Behind an ingress
|
||||
// or load balancer every viewer shares the connecting peer address, so without
|
||||
// forwarded-header resolution the admin session view — and any per-viewer
|
||||
// analysis built on it — collapses to one indistinguishable client.
|
||||
func TestEdgeClientIPResolvesViewerBehindTrustedIngress(t *testing.T) {
|
||||
srv := &Server{}
|
||||
srv.SetClientIPResolver(clientip.NewResolver(mustCIDRs(t, "10.0.0.0/8")))
|
||||
|
||||
first := httptest.NewRequest(http.MethodGet, "/stream/direct/tok", nil)
|
||||
first.RemoteAddr = "10.10.10.100:54321"
|
||||
first.Header.Set("X-Forwarded-For", "203.0.113.7")
|
||||
|
||||
second := httptest.NewRequest(http.MethodGet, "/stream/direct/tok", nil)
|
||||
second.RemoteAddr = "10.10.10.100:54322"
|
||||
second.Header.Set("X-Forwarded-For", "198.51.100.42")
|
||||
|
||||
firstIP := srv.edgeClientIP(first)
|
||||
secondIP := srv.edgeClientIP(second)
|
||||
|
||||
if firstIP != "203.0.113.7" {
|
||||
t.Errorf("first viewer = %q, want 203.0.113.7", firstIP)
|
||||
}
|
||||
if secondIP != "198.51.100.42" {
|
||||
t.Errorf("second viewer = %q, want 198.51.100.42", secondIP)
|
||||
}
|
||||
if firstIP == secondIP {
|
||||
t.Fatal("two viewers behind one ingress collapsed to a single address")
|
||||
}
|
||||
}
|
||||
|
||||
// A client that is not itself a trusted proxy must not be able to choose its
|
||||
// own recorded address: with auto-enforcement built on these records, a
|
||||
// believed forged header would be a spoofable primitive.
|
||||
func TestEdgeClientIPIgnoresForwardedHeaderFromUntrustedPeer(t *testing.T) {
|
||||
srv := &Server{}
|
||||
srv.SetClientIPResolver(clientip.NewResolver(mustCIDRs(t, "10.0.0.0/8")))
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, "/stream/direct/tok", nil)
|
||||
req.RemoteAddr = "203.0.113.9:44444"
|
||||
req.Header.Set("X-Forwarded-For", "198.51.100.1")
|
||||
req.Header.Set("X-Real-IP", "198.51.100.2")
|
||||
|
||||
if got := srv.edgeClientIP(req); got != "203.0.113.9" {
|
||||
t.Fatalf("edgeClientIP = %q, want the connecting peer 203.0.113.9", got)
|
||||
}
|
||||
}
|
||||
|
||||
// With no trusted proxies configured, forwarding headers are never consulted —
|
||||
// the fail-closed direction for the trust boundary.
|
||||
func TestEdgeClientIPWithEmptyTrustListIgnoresForwardedHeader(t *testing.T) {
|
||||
srv := &Server{}
|
||||
srv.SetClientIPResolver(clientip.NewResolver(nil))
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, "/stream/direct/tok", nil)
|
||||
req.RemoteAddr = "10.10.10.100:54321"
|
||||
req.Header.Set("X-Forwarded-For", "203.0.113.7")
|
||||
|
||||
if got := srv.edgeClientIP(req); got != "10.10.10.100" {
|
||||
t.Fatalf("edgeClientIP = %q, want the connecting peer 10.10.10.100", got)
|
||||
}
|
||||
}
|
||||
|
||||
// A directly-exposed edge with no resolver wired keeps the previous behavior.
|
||||
func TestEdgeClientIPWithoutResolverUsesRemoteAddr(t *testing.T) {
|
||||
srv := &Server{}
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, "/stream/direct/tok", nil)
|
||||
req.RemoteAddr = "203.0.113.5:12345"
|
||||
req.Header.Set("X-Forwarded-For", "198.51.100.1")
|
||||
|
||||
if got := srv.edgeClientIP(req); got != "203.0.113.5" {
|
||||
t.Fatalf("edgeClientIP = %q, want 203.0.113.5", got)
|
||||
}
|
||||
}
|
||||
|
||||
// IPv6 peers carry bracketed host:port in RemoteAddr; the fallback must not
|
||||
// mangle them.
|
||||
func TestEdgeClientIPHandlesIPv6RemoteAddr(t *testing.T) {
|
||||
srv := &Server{}
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, "/stream/direct/tok", nil)
|
||||
req.RemoteAddr = "[2001:db8::1]:9999"
|
||||
|
||||
if got := srv.edgeClientIP(req); got != "2001:db8::1" {
|
||||
t.Fatalf("edgeClientIP = %q, want 2001:db8::1", got)
|
||||
}
|
||||
}
|
||||
@@ -15,6 +15,7 @@ import (
|
||||
"github.com/go-chi/chi/v5"
|
||||
"github.com/go-chi/cors"
|
||||
|
||||
"github.com/Silo-Server/silo-server/internal/clientip"
|
||||
"github.com/Silo-Server/silo-server/internal/httpstream"
|
||||
"github.com/Silo-Server/silo-server/internal/nodeconfig"
|
||||
"github.com/Silo-Server/silo-server/internal/nodesessions"
|
||||
@@ -42,6 +43,7 @@ type Server struct {
|
||||
subCache *playback.SubtitleCache
|
||||
|
||||
revocation revocationStore
|
||||
ipResolver *clientip.Resolver
|
||||
}
|
||||
|
||||
// SetRevocationStore wires the kill-switch the edge consults per request. The
|
||||
@@ -51,6 +53,16 @@ func (s *Server) SetRevocationStore(store revocationStore) {
|
||||
s.revocation = store
|
||||
}
|
||||
|
||||
// SetClientIPResolver wires the trusted-proxy-aware client address resolver so
|
||||
// edge monitoring records the real viewer rather than the ingress that fronts
|
||||
// this node. Without one the edge falls back to the connecting address, which
|
||||
// behind a reverse proxy or load balancer is the same value for every viewer.
|
||||
// The resolver's trusted list is updated in place on setting changes, so this
|
||||
// is wired once at startup.
|
||||
func (s *Server) SetClientIPResolver(resolver *clientip.Resolver) {
|
||||
s.ipResolver = resolver
|
||||
}
|
||||
|
||||
// NewServer creates a new proxy server backed by a config watcher and session
|
||||
// tracker.
|
||||
func NewServer(watcher *nodeconfig.Watcher, tracker *nodesessions.Tracker) *Server {
|
||||
@@ -198,7 +210,7 @@ func (s *Server) handleDirectPlay(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
info := sessionInfo(s.tracker, claims, "direct_play", edgeClientIP(r))
|
||||
info := sessionInfo(s.tracker, claims, "direct_play", s.edgeClientIP(r))
|
||||
lease := s.tracker.Track(r.Context(), info)
|
||||
defer s.releaseTracked(lease)
|
||||
|
||||
@@ -291,7 +303,7 @@ func (s *Server) handleRemux(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
info := sessionInfo(s.tracker, claims, "remux", edgeClientIP(r))
|
||||
info := sessionInfo(s.tracker, claims, "remux", s.edgeClientIP(r))
|
||||
lease := s.tracker.Track(r.Context(), info)
|
||||
defer s.releaseTracked(lease)
|
||||
|
||||
@@ -349,7 +361,7 @@ func transcodeTransportIDFromClaims(claims *streamtoken.Claims) string {
|
||||
// requests instead of request lifetime. Visibility is separate from served
|
||||
// liveness: only a successful upstream media response advances LastServedAt.
|
||||
func (s *Server) ensureTranscodeSession(r *http.Request, claims *streamtoken.Claims) {
|
||||
s.tracker.EnsureEphemeral(r.Context(), sessionInfo(s.tracker, claims, "transcode", edgeClientIP(r)))
|
||||
s.tracker.EnsureEphemeral(r.Context(), sessionInfo(s.tracker, claims, "transcode", s.edgeClientIP(r)))
|
||||
}
|
||||
|
||||
// sessionInfo builds the node-session tracker record for a verified token,
|
||||
@@ -374,8 +386,24 @@ func sessionInfo(tr *nodesessions.Tracker, claims *streamtoken.Claims, kind, cli
|
||||
}
|
||||
}
|
||||
|
||||
// edgeClientIP extracts the connecting client's IP from the request, best-effort.
|
||||
func edgeClientIP(r *http.Request) string {
|
||||
// edgeClientIP resolves the viewer's address for monitoring attribution.
|
||||
//
|
||||
// When a trusted-proxy resolver is wired it walks the forwarding headers the
|
||||
// same way the native API surface does, honouring the operator's
|
||||
// `clientip.trusted_proxies` boundary — headers are consulted only when the
|
||||
// connecting peer is itself trusted, so an untrusted client cannot forge its
|
||||
// own address. Without a resolver (or when the peer is untrusted) this is the
|
||||
// connecting address, which is correct for a directly-exposed edge but
|
||||
// collapses every viewer to one value behind an ingress or load balancer.
|
||||
func (s *Server) edgeClientIP(r *http.Request) string {
|
||||
if s != nil && s.ipResolver != nil {
|
||||
return s.ipResolver.ClientIP(r)
|
||||
}
|
||||
return remoteAddrIP(r)
|
||||
}
|
||||
|
||||
// remoteAddrIP extracts the connecting peer's IP, best-effort.
|
||||
func remoteAddrIP(r *http.Request) string {
|
||||
host, _, err := net.SplitHostPort(r.RemoteAddr)
|
||||
if err != nil {
|
||||
return r.RemoteAddr
|
||||
|
||||
Reference in New Issue
Block a user