From 96fdbbee90d55788f262233f2277e55e5caa794f Mon Sep 17 00:00:00 2001 From: Jo Date: Mon, 3 Apr 2023 12:49:11 +0100 Subject: [PATCH] AuthJWT: Fix JWT query param leak (CVE-2023-1387) (#825) fix JWT query param leak Co-authored-by: Gabriel MABILLE Co-authored-by: Kalle Persson --- pkg/services/authn/clients/jwt.go | 15 +++++++++ pkg/services/authn/clients/jwt_test.go | 44 +++++++++++++++++++++++++ pkg/services/contexthandler/auth_jwt.go | 24 +++++++++++--- 3 files changed, 79 insertions(+), 4 deletions(-) diff --git a/pkg/services/authn/clients/jwt.go b/pkg/services/authn/clients/jwt.go index a6474ba7176..cc65b7e4ed2 100644 --- a/pkg/services/authn/clients/jwt.go +++ b/pkg/services/authn/clients/jwt.go @@ -19,6 +19,8 @@ import ( "github.com/grafana/grafana/pkg/util/errutil" ) +const authQueryParamName = "auth_token" + var _ authn.ContextAwareClient = new(JWT) var ( @@ -50,6 +52,7 @@ func (s *JWT) Name() string { func (s *JWT) Authenticate(ctx context.Context, r *authn.Request) (*authn.Identity, error) { jwtToken := s.retrieveToken(r.HTTPRequest) + s.stripSensitiveParam(r.HTTPRequest) claims, err := s.jwtService.Verify(ctx, jwtToken) if err != nil { @@ -120,6 +123,18 @@ func (s *JWT) Authenticate(ctx context.Context, r *authn.Request) (*authn.Identi return id, nil } +// remove sensitive query param +// avoid JWT URL login passing auth_token in URL +func (s *JWT) stripSensitiveParam(httpRequest *http.Request) { + if s.cfg.JWTAuthURLLogin { + params := httpRequest.URL.Query() + if params.Has(authQueryParamName) { + params.Del(authQueryParamName) + httpRequest.URL.RawQuery = params.Encode() + } + } +} + // retrieveToken retrieves the JWT token from the request. func (s *JWT) retrieveToken(httpRequest *http.Request) string { jwtToken := httpRequest.Header.Get(s.cfg.JWTAuthHeaderName) diff --git a/pkg/services/authn/clients/jwt_test.go b/pkg/services/authn/clients/jwt_test.go index 04a4c837bfe..f59aac88d8e 100644 --- a/pkg/services/authn/clients/jwt_test.go +++ b/pkg/services/authn/clients/jwt_test.go @@ -307,3 +307,47 @@ func TestJWTTest(t *testing.T) { }) } } + +func TestJWTStripParam(t *testing.T) { + jwtService := &jwt.FakeJWTService{ + VerifyProvider: func(context.Context, string) (jwt.JWTClaims, error) { + return jwt.JWTClaims{ + "sub": "1234567890", + "email": "eai.doe@cor.po", + "preferred_username": "eai-doe", + "name": "Eai Doe", + "roles": "Admin", + }, nil + }, + } + + jwtHeaderName := "X-Forwarded-User" + + cfg := &setting.Cfg{ + JWTAuthEnabled: true, + JWTAuthHeaderName: jwtHeaderName, + JWTAuthAutoSignUp: true, + JWTAuthAllowAssignGrafanaAdmin: true, + JWTAuthURLLogin: true, + JWTAuthRoleAttributeStrict: false, + JWTAuthRoleAttributePath: "roles", + JWTAuthEmailClaim: "email", + JWTAuthUsernameClaim: "preferred_username", + } + + // #nosec G101 -- This is a dummy/test token + token := "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.XbPfbIHMI6arZ3Y922BhjWgQzWXcXNrz0ogtVhfEd2o" + + httpReq := &http.Request{ + URL: &url.URL{RawQuery: "auth_token=" + token + "&other_param=other_value"}, + } + jwtClient := ProvideJWT(jwtService, cfg) + _, err := jwtClient.Authenticate(context.Background(), &authn.Request{ + OrgID: 1, + HTTPRequest: httpReq, + Resp: nil, + }) + require.NoError(t, err) + // auth_token should be removed from the query string + assert.Equal(t, "other_param=other_value", httpReq.URL.RawQuery) +} diff --git a/pkg/services/contexthandler/auth_jwt.go b/pkg/services/contexthandler/auth_jwt.go index 9936d956c96..1d3d79640c3 100644 --- a/pkg/services/contexthandler/auth_jwt.go +++ b/pkg/services/contexthandler/auth_jwt.go @@ -15,12 +15,14 @@ import ( loginsvc "github.com/grafana/grafana/pkg/services/login" "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/user" + "github.com/grafana/grafana/pkg/setting" ) const ( - InvalidJWT = "Invalid JWT" - InvalidRole = "Invalid Role" - UserNotFound = "User not found" + InvalidJWT = "Invalid JWT" + InvalidRole = "Invalid Role" + UserNotFound = "User not found" + authQueryParamName = "auth_token" ) func (h *ContextHandler) initContextWithJWT(ctx *contextmodel.ReqContext, orgId int64) bool { @@ -30,13 +32,15 @@ func (h *ContextHandler) initContextWithJWT(ctx *contextmodel.ReqContext, orgId jwtToken := ctx.Req.Header.Get(h.Cfg.JWTAuthHeaderName) if jwtToken == "" && h.Cfg.JWTAuthURLLogin { - jwtToken = ctx.Req.URL.Query().Get("auth_token") + jwtToken = ctx.Req.URL.Query().Get(authQueryParamName) } if jwtToken == "" { return false } + stripSensitiveParam(h.Cfg, ctx.Req) + // Strip the 'Bearer' prefix if it exists. jwtToken = strings.TrimPrefix(jwtToken, "Bearer ") @@ -204,3 +208,15 @@ func searchClaimsForStringAttr(attributePath string, claims map[string]interface return "", nil } + +// remove sensitive query params +// avoid JWT URL login passing auth_token in URL +func stripSensitiveParam(cfg *setting.Cfg, httpRequest *http.Request) { + if cfg.JWTAuthURLLogin { + params := httpRequest.URL.Query() + if params.Has(authQueryParamName) { + params.Del(authQueryParamName) + httpRequest.URL.RawQuery = params.Encode() + } + } +}