J3vb/OwnCord
1
0
Fork 0
mirror of https://github.com/J3vb/OwnCord.git synced 2026-09-03 03:50:00 +03:00
Code Issues Packages Projects Releases Wiki Activity

fix: AdminIPRestrict uses trusted_proxies for real client IP (BUG-116)

Browse Source 
AdminIPRestrict now accepts trustedProxyCIDRs and resolves the real
client IP from X-Real-IP/X-Forwarded-For when connecting through a
trusted reverse proxy. Without trusted_proxies configured, behavior
is unchanged (RemoteAddr only). Prevents admin panel exposure when
OwnCord is deployed behind nginx/caddy/traefik.
This commit is contained in:
J3vb
2026-04-02 12:15:47 +02:00
parent 5edebb43b3
commit d912a5a87d
4 changed files with 110 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Server/api/metrics_handler_test.go
+1 -1
View File
@@ -13,7 +13,7 @@ import (
// buildMetricsRouter creates a chi router with the metrics endpoint behind AdminIPRestrict.
func buildMetricsRouter(allowedCIDRs []string) http.Handler {
r := chi.NewRouter()
r.With(api.AdminIPRestrict(allowedCIDRs)).
r.With(api.AdminIPRestrict(allowedCIDRs, nil)).
Get("/api/v1/metrics", api.HandleMetricsForTest(
func() int { return 5 },
func() int { return 2 },
Server/api/middleware.go
+6 -2
View File
@@ -249,7 +249,11 @@ func isTrustedProxy(remoteIP string, cidrList []string) (bool, error) {
// AdminIPRestrict returns middleware that blocks requests from IPs not in the
// allowed CIDR list. Returns 403 Forbidden for disallowed IPs. If the CIDR
// list is empty, all requests are allowed (no restriction).
func AdminIPRestrict(allowedCIDRs []string) func(http.Handler) http.Handler {
//
// trustedProxyCIDRs specifies which connecting IPs are trusted reverse proxies.
// When the connecting IP matches a trusted proxy, the real client IP is read
// from X-Real-IP or X-Forwarded-For headers (BUG-116).
func AdminIPRestrict(allowedCIDRs, trustedProxyCIDRs []string) func(http.Handler) http.Handler {
return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if len(allowedCIDRs) == 0 {
@@ -257,7 +261,7 @@ func AdminIPRestrict(allowedCIDRs []string) func(http.Handler) http.Handler {
return
}
ip := clientIP(r)
ip := clientIPWithProxies(r, trustedProxyCIDRs)
allowed, _ := isTrustedProxy(ip, allowedCIDRs)
if !allowed {
writeJSON(w, http.StatusForbidden, errorResponse{
Server/api/middleware_test.go
+99 -5
View File
@@ -581,7 +581,7 @@ func TestMaxBodySize_PassesThrough(t *testing.T) {
// ─── AdminIPRestrict tests ──────────────────────────────────────────────────
func TestAdminIPRestrict_AllowedCIDR(t *testing.T) {
h := api.AdminIPRestrict([]string{"127.0.0.0/8"})(http.HandlerFunc(ok))
h := api.AdminIPRestrict([]string{"127.0.0.0/8"}, nil)(http.HandlerFunc(ok))
req := httptest.NewRequest(http.MethodGet, "/", nil)
req.RemoteAddr = "127.0.0.1:9999"
@@ -594,7 +594,7 @@ func TestAdminIPRestrict_AllowedCIDR(t *testing.T) {
}
func TestAdminIPRestrict_BlockedCIDR(t *testing.T) {
h := api.AdminIPRestrict([]string{"10.0.0.0/8"})(http.HandlerFunc(ok))
h := api.AdminIPRestrict([]string{"10.0.0.0/8"}, nil)(http.HandlerFunc(ok))
req := httptest.NewRequest(http.MethodGet, "/", nil)
req.RemoteAddr = "192.168.1.1:9999" // not in 10.0.0.0/8
@@ -607,7 +607,7 @@ func TestAdminIPRestrict_BlockedCIDR(t *testing.T) {
}
func TestAdminIPRestrict_EmptyAllowsAll(t *testing.T) {
h := api.AdminIPRestrict(nil)(http.HandlerFunc(ok))
h := api.AdminIPRestrict(nil, nil)(http.HandlerFunc(ok))
req := httptest.NewRequest(http.MethodGet, "/", nil)
req.RemoteAddr = "203.0.113.1:9999"
@@ -622,7 +622,7 @@ func TestAdminIPRestrict_EmptyAllowsAll(t *testing.T) {
func TestAdminIPRestrict_InvalidCIDR(t *testing.T) {
// Invalid CIDR should fail closed (deny access since isTrustedProxy
// returns false on parse error).
h := api.AdminIPRestrict([]string{"not-a-cidr"})(http.HandlerFunc(ok))
h := api.AdminIPRestrict([]string{"not-a-cidr"}, nil)(http.HandlerFunc(ok))
req := httptest.NewRequest(http.MethodGet, "/", nil)
req.RemoteAddr = "127.0.0.1:9999"
@@ -635,7 +635,7 @@ func TestAdminIPRestrict_InvalidCIDR(t *testing.T) {
}
func TestAdminIPRestrict_MultipleCIDRs(t *testing.T) {
h := api.AdminIPRestrict([]string{"10.0.0.0/8", "192.168.0.0/16"})(http.HandlerFunc(ok))
h := api.AdminIPRestrict([]string{"10.0.0.0/8", "192.168.0.0/16"}, nil)(http.HandlerFunc(ok))
// First CIDR matches.
req := httptest.NewRequest(http.MethodGet, "/", nil)
@@ -665,6 +665,100 @@ func TestAdminIPRestrict_MultipleCIDRs(t *testing.T) {
}
}
// ─── AdminIPRestrict proxy-aware tests (BUG-116) ─────────────────────────────
// TestAdminIPRestrict_TrustedProxy_UsesXForwardedFor verifies that when the
// connecting IP is a trusted proxy, the real client IP is extracted from
// X-Forwarded-For and checked against admin CIDRs.
func TestAdminIPRestrict_TrustedProxy_UsesXForwardedFor(t *testing.T) {
// Admin allowed: only 203.0.113.0/24. Trusted proxy: 127.0.0.1.
h := api.AdminIPRestrict(
[]string{"203.0.113.0/24"},
[]string{"127.0.0.0/8"},
)(http.HandlerFunc(ok))
// Request from proxy (127.0.0.1) with real client in XFF → allowed.
req := httptest.NewRequest(http.MethodGet, "/", nil)
req.RemoteAddr = "127.0.0.1:9999"
req.Header.Set("X-Forwarded-For", "203.0.113.50")
rr := httptest.NewRecorder()
h.ServeHTTP(rr, req)
if rr.Code != http.StatusOK {
t.Errorf("trusted proxy + allowed XFF status = %d, want 200", rr.Code)
}
// Request from proxy with disallowed real client → blocked.
req = httptest.NewRequest(http.MethodGet, "/", nil)
req.RemoteAddr = "127.0.0.1:9999"
req.Header.Set("X-Forwarded-For", "198.51.100.1")
rr = httptest.NewRecorder()
h.ServeHTTP(rr, req)
if rr.Code != http.StatusForbidden {
t.Errorf("trusted proxy + blocked XFF status = %d, want 403", rr.Code)
}
}
// TestAdminIPRestrict_TrustedProxy_UsesXRealIP verifies X-Real-IP is preferred
// over X-Forwarded-For when both are present from a trusted proxy.
func TestAdminIPRestrict_TrustedProxy_UsesXRealIP(t *testing.T) {
h := api.AdminIPRestrict(
[]string{"203.0.113.0/24"},
[]string{"127.0.0.0/8"},
)(http.HandlerFunc(ok))
req := httptest.NewRequest(http.MethodGet, "/", nil)
req.RemoteAddr = "127.0.0.1:9999"
req.Header.Set("X-Real-IP", "203.0.113.50")
rr := httptest.NewRecorder()
h.ServeHTTP(rr, req)
if rr.Code != http.StatusOK {
t.Errorf("trusted proxy + X-Real-IP status = %d, want 200", rr.Code)
}
}
// TestAdminIPRestrict_UntrustedProxy_IgnoresHeaders verifies that proxy headers
// are ignored when the connecting IP is NOT a trusted proxy.
func TestAdminIPRestrict_UntrustedProxy_IgnoresHeaders(t *testing.T) {
h := api.AdminIPRestrict(
[]string{"203.0.113.0/24"},
[]string{"10.0.0.0/8"}, // only 10.x is trusted
)(http.HandlerFunc(ok))
// Untrusted proxy at 192.168.1.1 tries to spoof XFF.
req := httptest.NewRequest(http.MethodGet, "/", nil)
req.RemoteAddr = "192.168.1.1:9999"
req.Header.Set("X-Forwarded-For", "203.0.113.50") // spoofed
rr := httptest.NewRecorder()
h.ServeHTTP(rr, req)
if rr.Code != http.StatusForbidden {
t.Errorf("untrusted proxy spoofed XFF status = %d, want 403 (should use RemoteAddr)", rr.Code)
}
}
// TestAdminIPRestrict_ProxyCollapse_WithoutTrusted verifies the original bug:
// without trusted proxies, a proxy on localhost makes everything appear local.
func TestAdminIPRestrict_ProxyCollapse_WithoutTrusted(t *testing.T) {
// Admin CIDR: private networks. No trusted proxies.
h := api.AdminIPRestrict(
[]string{"127.0.0.0/8", "10.0.0.0/8"},
nil, // no trusted proxies
)(http.HandlerFunc(ok))
// External client behind nginx on localhost — RemoteAddr is 127.0.0.1.
// XFF has the real external IP, but it's ignored (no trusted proxies).
req := httptest.NewRequest(http.MethodGet, "/", nil)
req.RemoteAddr = "127.0.0.1:9999"
req.Header.Set("X-Forwarded-For", "198.51.100.1") // real external IP
rr := httptest.NewRecorder()
h.ServeHTTP(rr, req)
// Without trusted proxies, 127.0.0.1 is used — passes the private CIDR check.
// This is the documented limitation: operators MUST configure trusted_proxies.
if rr.Code != http.StatusOK {
t.Errorf("no trusted proxies, proxy on localhost status = %d, want 200 (known limitation)", rr.Code)
}
}
// ─── SecurityHeadersWithTLS tests ───────────────────────────────────────────
func TestSecurityHeadersWithTLS_HSTS(t *testing.T) {
Server/api/router.go
+4 -4
View File
@@ -127,12 +127,12 @@ func NewRouter(cfg *config.Config, database *db.DB, ver string, logBuf *admin.Ri
// LiveKit webhook endpoint (no auth middleware — uses LiveKit JWT verification).
if lkErr == nil {
r.With(AdminIPRestrict(cfg.Server.AdminAllowedCIDRs)).
r.With(AdminIPRestrict(cfg.Server.AdminAllowedCIDRs, cfg.Server.TrustedProxies)).
Post("/api/v1/livekit/webhook",
ws.MountWebhookRoute(hub, cfg.Voice.LiveKitAPIKey, cfg.Voice.LiveKitAPISecret))
// LiveKit health check — admin-IP-restricted.
r.With(AdminIPRestrict(cfg.Server.AdminAllowedCIDRs)).
r.With(AdminIPRestrict(cfg.Server.AdminAllowedCIDRs, cfg.Server.TrustedProxies)).
Get("/api/v1/livekit/health", handleLiveKitHealth(hub))
// Reverse proxy LiveKit signaling through OwnCord's HTTPS server.
@@ -160,7 +160,7 @@ func NewRouter(cfg *config.Config, database *db.DB, ver string, logBuf *admin.Ri
r.Get("/api/v1/ws", ws.ServeWS(hub, database, cfg.Server.AllowedOrigins))
// Metrics endpoint — admin-IP-restricted, returns runtime stats as JSON.
r.With(AdminIPRestrict(cfg.Server.AdminAllowedCIDRs)).
r.With(AdminIPRestrict(cfg.Server.AdminAllowedCIDRs, cfg.Server.TrustedProxies)).
Get("/api/v1/metrics", handleMetrics(
func() int { return hub.ClientCount() },
func() int { return hub.VoiceSessionCount() },
@@ -172,7 +172,7 @@ func NewRouter(cfg *config.Config, database *db.DB, ver string, logBuf *admin.Ri
u := updater.NewUpdater(ver, cfg.GitHub.Token, "J3vb", "OwnCord")
adminHandler := admin.NewHandler(database, ver, hub, u, logBuf, cfg.Server.AllowedOrigins)
r.Group(func(r chi.Router) {
r.Use(AdminIPRestrict(cfg.Server.AdminAllowedCIDRs))
r.Use(AdminIPRestrict(cfg.Server.AdminAllowedCIDRs, cfg.Server.TrustedProxies))
r.Mount("/admin", adminHandler)
})