From fb120074ac73d109586c886368b37b61469c5ec9 Mon Sep 17 00:00:00 2001 From: naiba Date: Mon, 18 May 2026 15:17:59 +0000 Subject: [PATCH] chore(security): pin JWT algorithm + SameSite and harden OAuth2 state cookie MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Set GinJWTMiddleware.SigningAlgorithm to "HS256" explicitly so a future library default change (or an alg:none confusion attempt) cannot weaken token validation. This matches the current gin-jwt default, so behaviour is unchanged. - Set CookieSameSite to Lax: same as the modern-browser default, but pinned so server-side intent is clear and CSRF on cross-site POST is blocked while top-level GET (OAuth callback) still works. - Move the nz-o2s OAuth2 state cookie into writeOauth2StateCookie and set HttpOnly=true. The frontend does not read this cookie, so HttpOnly is strictly an XSS-hardening win with no behaviour change. JWT Cookie HttpOnly/Secure are intentionally left default for now: the frontend reads \`!!document.cookie\` to display login state and many deployments terminate TLS at an upstream proxy — flipping those would require a coordinated frontend change. Co-authored-by: naiba/CloudCode --- cmd/dashboard/controller/jwt.go | 26 ++++++++--- cmd/dashboard/controller/oauth2.go | 13 +++++- .../controller/stream_ownership_test.go | 46 +++++++++++++++++++ 3 files changed, 76 insertions(+), 9 deletions(-) diff --git a/cmd/dashboard/controller/jwt.go b/cmd/dashboard/controller/jwt.go index 17702c21..102e39dc 100644 --- a/cmd/dashboard/controller/jwt.go +++ b/cmd/dashboard/controller/jwt.go @@ -22,18 +22,30 @@ func initParams() *jwt.GinJWTMiddleware { Key: []byte(singleton.Conf.JWTSecretKey), CookieName: "nz-jwt", SendCookie: true, - Timeout: time.Hour * time.Duration(singleton.Conf.JWTTimeout), - MaxRefresh: time.Hour * time.Duration(singleton.Conf.JWTTimeout), - IdentityKey: model.CtxKeyAuthorizedUser, - PayloadFunc: payloadFunc(), + // Pin the signing algorithm so a future library default change (or an + // `alg: none` confusion attempt) cannot weaken token validation. + SigningAlgorithm: "HS256", + // Lax keeps OAuth callback redirects (top-level GET navigations from + // the provider domain) working while blocking cross-site POST CSRF. + // HttpOnly/Secure are intentionally left default: the frontend reads + // `!!document.cookie` for login-state display and many deployments + // terminate TLS at a proxy upstream — both warrant a separate change. + CookieSameSite: http.SameSiteLaxMode, + Timeout: time.Hour * time.Duration(singleton.Conf.JWTTimeout), + MaxRefresh: time.Hour * time.Duration(singleton.Conf.JWTTimeout), + IdentityKey: model.CtxKeyAuthorizedUser, + PayloadFunc: payloadFunc(), IdentityHandler: identityHandler(), Authenticator: authenticator(), Authorizator: authorizator(), Unauthorized: unauthorized(), - TokenLookup: "header: Authorization, query: token, cookie: nz-jwt", - TokenHeadName: "Bearer", - TimeFunc: time.Now, + // query: token still accepted because the WebSocket browser API + // cannot set Authorization headers; removing it would break the + // /ws/* routes until the frontend migrates to cookie auth. + TokenLookup: "header: Authorization, query: token, cookie: nz-jwt", + TokenHeadName: "Bearer", + TimeFunc: time.Now, LoginResponse: func(c *gin.Context, code int, token string, expire time.Time) { c.JSON(http.StatusOK, model.CommonResponse[model.LoginResponse]{ diff --git a/cmd/dashboard/controller/oauth2.go b/cmd/dashboard/controller/oauth2.go index acb02188..31d23477 100644 --- a/cmd/dashboard/controller/oauth2.go +++ b/cmd/dashboard/controller/oauth2.go @@ -66,12 +66,21 @@ func oauth2redirect(c *gin.Context) (*model.Oauth2LoginResponse, error) { }, cache.DefaultExpiration) url := o2conf.AuthCodeURL(state, oauth2.AccessTypeOnline) - // CodeQL go/cookie-secure-not-set: 根据请求协议动态设置 Secure 属性，避免 HTTP 环境下 Cookie 无法使用 - c.SetCookie("nz-o2s", stateKey, 60*5, "", "", c.Request.URL.Scheme == "https" || c.Request.TLS != nil, false) + writeOauth2StateCookie(c, stateKey) return &model.Oauth2LoginResponse{Redirect: url}, nil } +// writeOauth2StateCookie sets the nz-o2s cookie used to authenticate the +// OAuth2 callback. Secure is set when the request arrives over HTTPS; +// HttpOnly is enabled unconditionally — the frontend does not read this +// cookie, only the dashboard's callback handler does, so HTTP-only access +// is strictly an XSS-hardening win. +func writeOauth2StateCookie(c *gin.Context, stateKey string) { + secure := c.Request.URL.Scheme == "https" || c.Request.TLS != nil + c.SetCookie("nz-o2s", stateKey, 60*5, "", "", secure, true) +} + // @Summary Unbind Oauth2 // @Description Unbind Oauth2 // @Accept json diff --git a/cmd/dashboard/controller/stream_ownership_test.go b/cmd/dashboard/controller/stream_ownership_test.go index ae88f233..f000468d 100644 --- a/cmd/dashboard/controller/stream_ownership_test.go +++ b/cmd/dashboard/controller/stream_ownership_test.go @@ -4,6 +4,7 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "strings" "testing" "github.com/gin-gonic/gin" @@ -115,3 +116,48 @@ func TestTerminalStreamRejectsUnknownStreamID(t *testing.T) { success, _ := decodeCommonResponseError(t, w.Body.Bytes()) assert.False(t, success, "unknown stream id must produce an error response") } + +// JWT cookie security: SigningAlgorithm must be pinned to HS256 (defense +// against future algorithm-confusion regressions in the library) and the +// JWT cookie must use SameSite=Lax so cross-site GET navigations don't +// silently mint requests with the user's session. HttpOnly/Secure are NOT +// asserted here because the frontend currently reads `!!document.cookie` +// to display login state and many deployments terminate TLS at a proxy — +// flipping those would break user-visible behaviour and is tracked +// separately. +func TestJWTInitParamsPinsAlgorithmAndSameSite(t *testing.T) { + ensureLocalizerForStreamTests(t) + if singleton.Conf == nil { + singleton.Conf = &singleton.ConfigClass{ + Config: &model.Config{JWTSecretKey: "test-secret-for-jwt-config-assertions"}, + } + } + params := initParams() + if params.SigningAlgorithm != "HS256" { + t.Fatalf("SigningAlgorithm must be pinned to HS256, got %q", params.SigningAlgorithm) + } + if params.CookieSameSite != http.SameSiteLaxMode { + t.Fatalf("CookieSameSite must be Lax for OAuth-callback compatibility + CSRF safety, got %v", params.CookieSameSite) + } +} + +// nz-o2s carries the OAuth2 state binding that authenticates the callback. +// The frontend never reads it, so HttpOnly is safe to enable and shuts the +// door on XSS attempting to steal the state. +func TestWriteOauth2StateCookieIsHttpOnly(t *testing.T) { + gin.SetMode(gin.TestMode) + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest(http.MethodGet, "/", nil) + + writeOauth2StateCookie(c, "test-key") + + header := w.Header().Get("Set-Cookie") + if !strings.Contains(header, "nz-o2s=test-key") { + t.Fatalf("expected nz-o2s cookie in response, got %q", header) + } + if !strings.Contains(header, "HttpOnly") { + t.Fatalf("nz-o2s must be HttpOnly to prevent XSS reading OAuth state, got %q", header) + } +} +