From fe4840d1912439d1fe893d67380622be0ac9de15 Mon Sep 17 00:00:00 2001 From: ericanannn-blip Date: Wed, 30 Sep 2026 09:41:26 +0800 Subject: [PATCH 1/2] log CSRF rejections in CSRFMiddleware for observability --- internal/web/middleware/security.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/internal/web/middleware/security.go b/internal/web/middleware/security.go index 920afb4e8..807642189 100644 --- a/internal/web/middleware/security.go +++ b/internal/web/middleware/security.go @@ -5,6 +5,7 @@ import ( "encoding/base64" "net/http" + "github.com/mhsanaei/3x-ui/v3/internal/logger" "github.com/mhsanaei/3x-ui/v3/internal/web/session" "github.com/gin-gonic/gin" @@ -49,6 +50,10 @@ func CSRFMiddleware() gin.HandlerFunc { return } if !session.ValidateCSRFToken(c) { + // Rejected requests are otherwise completely invisible to operators, + // which makes diagnosing failed logins (e.g. a missing/expired CSRF + // token) very confusing. Log the rejection without any token material. + logger.Warning("CSRF validation failed, request rejected: method=", c.Request.Method, " path=", c.Request.URL.Path, " IP=", c.ClientIP()) c.AbortWithStatus(http.StatusForbidden) return } From b3ec003b21a3d1ad5407ffaa3e2a9c1aea60a09b Mon Sep 17 00:00:00 2001 From: MHSanaei Date: Wed, 7 Oct 2026 11:05:01 +0200 Subject: [PATCH 2/2] fix(middleware): log the trusted-proxy client IP on CSRF rejection The rejection log used c.ClientIP(), and nothing calls SetTrustedProxies, so Gin read X-Forwarded-For from any peer: an unauthenticated client could write an arbitrary IP into the panel log and the web-UI log buffer. CSRFMiddleware now takes the IP resolver from its caller, and every route passes getRemoteIp, which only honours forwarded headers from the configured trusted proxies. The comment blocks are trimmed to two lines. --- internal/web/controller/api.go | 2 +- internal/web/controller/csrf_log_test.go | 41 ++++++++++++++++++++++++ internal/web/controller/index.go | 6 ++-- internal/web/controller/spa.go | 2 +- internal/web/middleware/security.go | 13 +++----- internal/web/middleware/security_test.go | 4 +-- 6 files changed, 52 insertions(+), 16 deletions(-) create mode 100644 internal/web/controller/csrf_log_test.go diff --git a/internal/web/controller/api.go b/internal/web/controller/api.go index c02eb2cd1..28070082c 100644 --- a/internal/web/controller/api.go +++ b/internal/web/controller/api.go @@ -175,7 +175,7 @@ func (a *APIController) initRouter(g *gin.RouterGroup) { // Decode + verify the node config envelope (zstd + X-Config-Sha256) and // advertise support, before CSRF/handlers read the body. api.Use(middleware.ConfigEnvelopeMiddleware()) - api.Use(middleware.CSRFMiddleware()) + api.Use(middleware.CSRFMiddleware(getRemoteIp)) api.GET("/openapi.json", ServeOpenAPISpec) diff --git a/internal/web/controller/csrf_log_test.go b/internal/web/controller/csrf_log_test.go new file mode 100644 index 000000000..e436bcf39 --- /dev/null +++ b/internal/web/controller/csrf_log_test.go @@ -0,0 +1,41 @@ +package controller + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/gin-contrib/sessions" + "github.com/gin-contrib/sessions/cookie" + "github.com/gin-gonic/gin" + + "github.com/mhsanaei/3x-ui/v3/internal/logger" +) + +func TestLoginCSRFRejectionLogsPeerIPNotForwardedHeader(t *testing.T) { + gin.SetMode(gin.TestMode) + engine := gin.New() + engine.Use(sessions.Sessions("3x-ui", cookie.NewStore([]byte("csrf-log-test-secret")))) + NewIndexController(engine.Group("/")) + + req := httptest.NewRequest(http.MethodPost, "/login", nil) + req.RemoteAddr = "198.51.100.9:40000" + req.Header.Set("X-Forwarded-For", "203.0.113.66") + rec := httptest.NewRecorder() + engine.ServeHTTP(rec, req) + if rec.Code != http.StatusForbidden { + t.Fatalf("status = %d, want %d", rec.Code, http.StatusForbidden) + } + + var line string + for _, l := range logger.GetLogs(50, "WARNING") { + if strings.Contains(l, "CSRF validation failed") { + line = l + break + } + } + if !strings.Contains(line, "IP=198.51.100.9") || strings.Contains(line, "203.0.113.66") { + t.Fatalf("CSRF rejection log = %q, want the untrusted peer 198.51.100.9 and not the forwarded header", line) + } +} diff --git a/internal/web/controller/index.go b/internal/web/controller/index.go index a5ad9afc9..8f8e32fd0 100644 --- a/internal/web/controller/index.go +++ b/internal/web/controller/index.go @@ -43,9 +43,9 @@ func (a *IndexController) initRouter(g *gin.RouterGroup) { g.GET("/", a.index) g.GET("/csrf-token", a.csrfToken) - g.POST("/login", middleware.CSRFMiddleware(), a.login) - g.POST("/logout", middleware.CSRFMiddleware(), a.logout) - g.POST("/getTwoFactorEnable", middleware.CSRFMiddleware(), a.getTwoFactorEnable) + g.POST("/login", middleware.CSRFMiddleware(getRemoteIp), a.login) + g.POST("/logout", middleware.CSRFMiddleware(getRemoteIp), a.logout) + g.POST("/getTwoFactorEnable", middleware.CSRFMiddleware(getRemoteIp), a.getTwoFactorEnable) } // index handles the root route, redirecting logged-in users to the panel or showing the login page. diff --git a/internal/web/controller/spa.go b/internal/web/controller/spa.go index 913dfc2e5..a43e22f01 100644 --- a/internal/web/controller/spa.go +++ b/internal/web/controller/spa.go @@ -33,7 +33,7 @@ func NewXUIController(g *gin.RouterGroup) *XUIController { func (a *XUIController) initRouter(g *gin.RouterGroup) { g = g.Group("/panel") g.Use(a.checkLogin) - g.Use(middleware.CSRFMiddleware()) + g.Use(middleware.CSRFMiddleware(getRemoteIp)) g.GET("/", a.panelSPA) g.GET("/inbounds", a.panelSPA) diff --git a/internal/web/middleware/security.go b/internal/web/middleware/security.go index 807642189..daa1232fc 100644 --- a/internal/web/middleware/security.go +++ b/internal/web/middleware/security.go @@ -35,11 +35,9 @@ func newCSPNonce() string { return base64.RawStdEncoding.EncodeToString(b[:]) } -// CSRFMiddleware rejects unsafe requests that do not include the session CSRF token. -// Bearer-token-authenticated callers (api_authed flag set by APIController.checkAPIAuth) -// short-circuit the CSRF check — they are not browser sessions, so the -// cross-site request forgery threat model doesn't apply to them. -func CSRFMiddleware() gin.HandlerFunc { +// CSRFMiddleware rejects unsafe session requests lacking the CSRF token; bearer (api_authed) callers skip it. +// clientIP must honour the trusted-proxy list: c.ClientIP trusts any peer's X-Forwarded-For. +func CSRFMiddleware(clientIP func(*gin.Context) string) gin.HandlerFunc { return func(c *gin.Context) { if c.GetBool("api_authed") { c.Next() @@ -50,10 +48,7 @@ func CSRFMiddleware() gin.HandlerFunc { return } if !session.ValidateCSRFToken(c) { - // Rejected requests are otherwise completely invisible to operators, - // which makes diagnosing failed logins (e.g. a missing/expired CSRF - // token) very confusing. Log the rejection without any token material. - logger.Warning("CSRF validation failed, request rejected: method=", c.Request.Method, " path=", c.Request.URL.Path, " IP=", c.ClientIP()) + logger.Warningf("CSRF validation failed, request rejected: method=%s path=%s IP=%s", c.Request.Method, c.Request.URL.Path, clientIP(c)) c.AbortWithStatus(http.StatusForbidden) return } diff --git a/internal/web/middleware/security_test.go b/internal/web/middleware/security_test.go index f9fa19186..ac24c38a6 100644 --- a/internal/web/middleware/security_test.go +++ b/internal/web/middleware/security_test.go @@ -16,7 +16,7 @@ import ( func TestCSRFMiddlewareAllowsSafeMethods(t *testing.T) { gin.SetMode(gin.TestMode) router := gin.New() - router.Use(CSRFMiddleware()) + router.Use(CSRFMiddleware(func(*gin.Context) string { return "" })) router.GET("/safe", func(c *gin.Context) { c.String(http.StatusOK, "ok") }) @@ -42,7 +42,7 @@ func TestCSRFMiddlewareRejectsMissingTokenAndAcceptsValidToken(t *testing.T) { } c.String(http.StatusOK, token) }) - router.POST("/submit", CSRFMiddleware(), func(c *gin.Context) { + router.POST("/submit", CSRFMiddleware(func(*gin.Context) string { return "" }), func(c *gin.Context) { c.String(http.StatusOK, "ok") })