mirror of
https://github.com/MHSanaei/3x-ui.git
synced 2026-11-14 17:15:24 +03:00
fix(middleware): log rejected CSRF requests with the trusted client address (#6692)
A panel or API request rejected for a missing or wrong CSRF token got a bare 403 and left nothing in the log, so an operator chasing failed logins from a stale tab or a script had no trace. CSRFMiddleware now logs a warning with the method, path and client address. The address comes from getRemoteIp, which honours the trusted-proxy list, instead of gin's ClientIP, which trusts any peer's X-Forwarded-For.
This commit is contained in:
@@ -198,7 +198,7 @@ func (a *APIController) initRouter(g *gin.RouterGroup) {
|
|||||||
// Decode + verify the node config envelope (zstd + X-Config-Sha256) and
|
// Decode + verify the node config envelope (zstd + X-Config-Sha256) and
|
||||||
// advertise support, before CSRF/handlers read the body.
|
// advertise support, before CSRF/handlers read the body.
|
||||||
api.Use(middleware.ConfigEnvelopeMiddleware())
|
api.Use(middleware.ConfigEnvelopeMiddleware())
|
||||||
api.Use(middleware.CSRFMiddleware())
|
api.Use(middleware.CSRFMiddleware(getRemoteIp))
|
||||||
|
|
||||||
api.GET("/openapi.json", ServeOpenAPISpec)
|
api.GET("/openapi.json", ServeOpenAPISpec)
|
||||||
|
|
||||||
|
|||||||
@@ -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)
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -47,9 +47,9 @@ func (a *IndexController) initRouter(g *gin.RouterGroup) {
|
|||||||
g.GET("/sponsors", a.sponsors)
|
g.GET("/sponsors", a.sponsors)
|
||||||
g.GET("/sponsors/logo/:name", a.sponsorLogo)
|
g.GET("/sponsors/logo/:name", a.sponsorLogo)
|
||||||
|
|
||||||
g.POST("/login", middleware.CSRFMiddleware(), a.login)
|
g.POST("/login", middleware.CSRFMiddleware(getRemoteIp), a.login)
|
||||||
g.POST("/logout", middleware.CSRFMiddleware(), a.logout)
|
g.POST("/logout", middleware.CSRFMiddleware(getRemoteIp), a.logout)
|
||||||
g.POST("/getTwoFactorEnable", middleware.CSRFMiddleware(), a.getTwoFactorEnable)
|
g.POST("/getTwoFactorEnable", middleware.CSRFMiddleware(getRemoteIp), a.getTwoFactorEnable)
|
||||||
}
|
}
|
||||||
|
|
||||||
// sponsors is public so the login page can render its slot; failures stay silent.
|
// sponsors is public so the login page can render its slot; failures stay silent.
|
||||||
|
|||||||
@@ -33,7 +33,7 @@ func NewXUIController(g *gin.RouterGroup) *XUIController {
|
|||||||
func (a *XUIController) initRouter(g *gin.RouterGroup) {
|
func (a *XUIController) initRouter(g *gin.RouterGroup) {
|
||||||
g = g.Group("/panel")
|
g = g.Group("/panel")
|
||||||
g.Use(a.checkLogin)
|
g.Use(a.checkLogin)
|
||||||
g.Use(middleware.CSRFMiddleware())
|
g.Use(middleware.CSRFMiddleware(getRemoteIp))
|
||||||
|
|
||||||
g.GET("/", a.panelSPA)
|
g.GET("/", a.panelSPA)
|
||||||
g.GET("/inbounds", a.panelSPA)
|
g.GET("/inbounds", a.panelSPA)
|
||||||
|
|||||||
@@ -5,6 +5,7 @@ import (
|
|||||||
"encoding/base64"
|
"encoding/base64"
|
||||||
"net/http"
|
"net/http"
|
||||||
|
|
||||||
|
"github.com/mhsanaei/3x-ui/v3/internal/logger"
|
||||||
"github.com/mhsanaei/3x-ui/v3/internal/web/session"
|
"github.com/mhsanaei/3x-ui/v3/internal/web/session"
|
||||||
|
|
||||||
"github.com/gin-gonic/gin"
|
"github.com/gin-gonic/gin"
|
||||||
@@ -34,11 +35,9 @@ func newCSPNonce() string {
|
|||||||
return base64.RawStdEncoding.EncodeToString(b[:])
|
return base64.RawStdEncoding.EncodeToString(b[:])
|
||||||
}
|
}
|
||||||
|
|
||||||
// CSRFMiddleware rejects unsafe requests that do not include the session CSRF token.
|
// CSRFMiddleware rejects unsafe session requests lacking the CSRF token; bearer (api_authed) callers skip it.
|
||||||
// Bearer-token-authenticated callers (api_authed flag set by APIController.checkAPIAuth)
|
// clientIP must honour the trusted-proxy list: c.ClientIP trusts any peer's X-Forwarded-For.
|
||||||
// short-circuit the CSRF check — they are not browser sessions, so the
|
func CSRFMiddleware(clientIP func(*gin.Context) string) gin.HandlerFunc {
|
||||||
// cross-site request forgery threat model doesn't apply to them.
|
|
||||||
func CSRFMiddleware() gin.HandlerFunc {
|
|
||||||
return func(c *gin.Context) {
|
return func(c *gin.Context) {
|
||||||
if c.GetBool("api_authed") {
|
if c.GetBool("api_authed") {
|
||||||
c.Next()
|
c.Next()
|
||||||
@@ -49,6 +48,7 @@ func CSRFMiddleware() gin.HandlerFunc {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
if !session.ValidateCSRFToken(c) {
|
if !session.ValidateCSRFToken(c) {
|
||||||
|
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)
|
c.AbortWithStatus(http.StatusForbidden)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -16,7 +16,7 @@ import (
|
|||||||
func TestCSRFMiddlewareAllowsSafeMethods(t *testing.T) {
|
func TestCSRFMiddlewareAllowsSafeMethods(t *testing.T) {
|
||||||
gin.SetMode(gin.TestMode)
|
gin.SetMode(gin.TestMode)
|
||||||
router := gin.New()
|
router := gin.New()
|
||||||
router.Use(CSRFMiddleware())
|
router.Use(CSRFMiddleware(func(*gin.Context) string { return "" }))
|
||||||
router.GET("/safe", func(c *gin.Context) {
|
router.GET("/safe", func(c *gin.Context) {
|
||||||
c.String(http.StatusOK, "ok")
|
c.String(http.StatusOK, "ok")
|
||||||
})
|
})
|
||||||
@@ -42,7 +42,7 @@ func TestCSRFMiddlewareRejectsMissingTokenAndAcceptsValidToken(t *testing.T) {
|
|||||||
}
|
}
|
||||||
c.String(http.StatusOK, token)
|
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")
|
c.String(http.StatusOK, "ok")
|
||||||
})
|
})
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user