From 7a91838d59482aa1a64371be04d4dbfd3187658c Mon Sep 17 00:00:00 2001 From: jackHay22 Date: Wed, 6 Dec 2023 10:27:50 -0500 Subject: [PATCH 1/5] add setting to disable query auth --- modules/setting/security.go | 4 ++++ services/auth/oauth2.go | 17 ++++++++++------- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/modules/setting/security.go b/modules/setting/security.go index 92caa05fad174..0fb7e17f65b4d 100644 --- a/modules/setting/security.go +++ b/modules/setting/security.go @@ -34,6 +34,7 @@ var ( PasswordHashAlgo string PasswordCheckPwn bool SuccessfulTokensCacheSize int + DisableQueryAuthToken bool CSRFCookieName = "_csrf" CSRFCookieHTTPOnly = true ) @@ -157,4 +158,7 @@ func loadSecurityFrom(rootCfg ConfigProvider) { PasswordComplexity = append(PasswordComplexity, name) } } + + // TODO: default value should be true in future releases + DisableQueryAuthToken = sec.Key("DISABLE_QUERY_AUTH_TOKEN").MustBool(false) } diff --git a/services/auth/oauth2.go b/services/auth/oauth2.go index 08a2a05539b5a..ce63df94b30ab 100644 --- a/services/auth/oauth2.go +++ b/services/auth/oauth2.go @@ -14,6 +14,7 @@ import ( auth_model "code.gitea.io/gitea/models/auth" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/web/middleware" "code.gitea.io/gitea/services/auth/source/oauth2" @@ -62,13 +63,15 @@ func (o *OAuth2) Name() string { // representing whether the token exists or not func parseToken(req *http.Request) (string, bool) { _ = req.ParseForm() - // Check token. - if token := req.Form.Get("token"); token != "" { - return token, true - } - // Check access token. - if token := req.Form.Get("access_token"); token != "" { - return token, true + if !setting.DisableQueryAuthToken { + // Check token. + if token := req.Form.Get("token"); token != "" { + return token, true + } + // Check access token. + if token := req.Form.Get("access_token"); token != "" { + return token, true + } } // check header token if auHead := req.Header.Get("Authorization"); auHead != "" { From 89615f33c60230b2aa2efadf15121673a56e2c67 Mon Sep 17 00:00:00 2001 From: jackHay22 Date: Thu, 7 Dec 2023 12:07:28 -0500 Subject: [PATCH 2/5] add api deprecation warning --- custom/conf/app.example.ini | 4 ++++ routers/api/v1/api.go | 11 +++++++++++ services/auth/oauth2.go | 3 +++ templates/swagger/v1_json.tmpl | 2 ++ 4 files changed, 20 insertions(+) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index 18d6fe37a8ba2..ac03797f3b3b2 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -492,6 +492,10 @@ INTERNAL_TOKEN= ;; Cache successful token hashes. API tokens are stored in the DB as pbkdf2 hashes however, this means that there is a potentially significant hashing load when there are multiple API operations. ;; This cache will store the successfully hashed tokens in a LRU cache as a balance between performance and security. ;SUCCESSFUL_TOKENS_CACHE_SIZE = 20 +;; +;; Reject API tokens sent in URL query string (Accept Header-based API tokens only). This avoids security vulnerabilities +;; stemming from cached/logged plain-text API tokens. +;DISABLE_QUERY_AUTH_TOKEN = false ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 623c798feea68..7e3c25122125c 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -35,10 +35,12 @@ // type: apiKey // name: token // in: query +// description: This authentication option is deprecated. Please use AuthorizationHeaderToken instead. // AccessToken: // type: apiKey // name: access_token // in: query +// description: This authentication option is deprecated. Please use AuthorizationHeaderToken instead. // AuthorizationHeaderToken: // type: apiKey // name: Authorization @@ -788,6 +790,13 @@ func verifyAuthWithOptions(options *common.VerifyOptions) func(ctx *context.APIC } } +// check for and warn against deprecated authentication options +func checkDeprecatedAuthMethods(ctx *context.APIContext) { + if ctx.FormString("token") != "" || ctx.FormString("access_token") != "" { + ctx.Resp.Header().Set("Warning", "token and access_token API authentication is deprecated") + } +} + // Routes registers all v1 APIs routes to web application. func Routes() *web.Route { m := web.NewRoute() @@ -806,6 +815,8 @@ func Routes() *web.Route { } m.Use(context.APIContexter()) + m.Use(checkDeprecatedAuthMethods) + // Get user from session if logged in. m.Use(apiAuth(buildAuthGroup())) diff --git a/services/auth/oauth2.go b/services/auth/oauth2.go index ce63df94b30ab..f2f7858a850c1 100644 --- a/services/auth/oauth2.go +++ b/services/auth/oauth2.go @@ -72,7 +72,10 @@ func parseToken(req *http.Request) (string, bool) { if token := req.Form.Get("access_token"); token != "" { return token, true } + } else if req.Form.Get("token") != "" || req.Form.Get("access_token") != "" { + log.Warn("API token sent in query string but DISABLE_QUERY_AUTH_TOKEN=true") } + // check header token if auHead := req.Header.Get("Authorization"); auHead != "" { auths := strings.Fields(auHead) diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 2541726a64b81..31420787482dd 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -24046,6 +24046,7 @@ }, "securityDefinitions": { "AccessToken": { + "description": "This authentication option is deprecated. Please use AuthorizationHeaderToken instead.", "type": "apiKey", "name": "access_token", "in": "query" @@ -24078,6 +24079,7 @@ "in": "header" }, "Token": { + "description": "This authentication option is deprecated. Please use AuthorizationHeaderToken instead.", "type": "apiKey", "name": "token", "in": "query" From acd7a63400179c382c2be0e18587fe7241bdb72c Mon Sep 17 00:00:00 2001 From: Jack Hay Date: Fri, 8 Dec 2023 13:56:40 -0500 Subject: [PATCH 3/5] Apply suggestions from code review Co-authored-by: delvh --- custom/conf/app.example.ini | 1 + modules/setting/security.go | 2 +- routers/api/v1/api.go | 6 +++--- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index ac03797f3b3b2..e10c4f7582e91 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -495,6 +495,7 @@ INTERNAL_TOKEN= ;; ;; Reject API tokens sent in URL query string (Accept Header-based API tokens only). This avoids security vulnerabilities ;; stemming from cached/logged plain-text API tokens. +;; In future releases, this will become the default behavior ;DISABLE_QUERY_AUTH_TOKEN = false ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; diff --git a/modules/setting/security.go b/modules/setting/security.go index 0fb7e17f65b4d..aa3cd47203446 100644 --- a/modules/setting/security.go +++ b/modules/setting/security.go @@ -159,6 +159,6 @@ func loadSecurityFrom(rootCfg ConfigProvider) { } } - // TODO: default value should be true in future releases + // TODO: default value should be true immediately after 1.22.0 has been released, so that 1.23.0 ships with the change DisableQueryAuthToken = sec.Key("DISABLE_QUERY_AUTH_TOKEN").MustBool(false) } diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 7e3c25122125c..13c6762b54d74 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -35,12 +35,12 @@ // type: apiKey // name: token // in: query -// description: This authentication option is deprecated. Please use AuthorizationHeaderToken instead. +// description: This authentication option is deprecated for removal in Gitea 1.23. Please use AuthorizationHeaderToken instead. // AccessToken: // type: apiKey // name: access_token // in: query -// description: This authentication option is deprecated. Please use AuthorizationHeaderToken instead. +// description: This authentication option is deprecated for removal in Gitea 1.23. Please use AuthorizationHeaderToken instead. // AuthorizationHeaderToken: // type: apiKey // name: Authorization @@ -793,7 +793,7 @@ func verifyAuthWithOptions(options *common.VerifyOptions) func(ctx *context.APIC // check for and warn against deprecated authentication options func checkDeprecatedAuthMethods(ctx *context.APIContext) { if ctx.FormString("token") != "" || ctx.FormString("access_token") != "" { - ctx.Resp.Header().Set("Warning", "token and access_token API authentication is deprecated") + ctx.Resp.Header().Set("Warning", "token and access_token API authentication is deprecated and will be removed in gitea 1.23. Please use AuthorizationHeaderToken instead. Existing queries will continue to work but without authorization.") } } From 1a43f7c126537cfdc1e41a1aad095295da03727d Mon Sep 17 00:00:00 2001 From: jackHay22 Date: Fri, 8 Dec 2023 14:20:46 -0500 Subject: [PATCH 4/5] swagger generate --- templates/swagger/v1_json.tmpl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 31420787482dd..6cf2beafec6e8 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -24046,7 +24046,7 @@ }, "securityDefinitions": { "AccessToken": { - "description": "This authentication option is deprecated. Please use AuthorizationHeaderToken instead.", + "description": "This authentication option is deprecated for removal in Gitea 1.23. Please use AuthorizationHeaderToken instead.", "type": "apiKey", "name": "access_token", "in": "query" @@ -24079,7 +24079,7 @@ "in": "header" }, "Token": { - "description": "This authentication option is deprecated. Please use AuthorizationHeaderToken instead.", + "description": "This authentication option is deprecated for removal in Gitea 1.23. Please use AuthorizationHeaderToken instead.", "type": "apiKey", "name": "token", "in": "query" From 078748e229968776f99e61bd9f163e4bf68ed67b Mon Sep 17 00:00:00 2001 From: jackHay22 Date: Mon, 11 Dec 2023 09:43:51 -0500 Subject: [PATCH 5/5] add setting warning --- modules/setting/security.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/modules/setting/security.go b/modules/setting/security.go index aa3cd47203446..4adfe20635e77 100644 --- a/modules/setting/security.go +++ b/modules/setting/security.go @@ -159,6 +159,10 @@ func loadSecurityFrom(rootCfg ConfigProvider) { } } - // TODO: default value should be true immediately after 1.22.0 has been released, so that 1.23.0 ships with the change + // TODO: default value should be true in future releases DisableQueryAuthToken = sec.Key("DISABLE_QUERY_AUTH_TOKEN").MustBool(false) + + if !DisableQueryAuthToken { + log.Warn("Enabling Query API Auth tokens is not recommended. DISABLE_QUERY_AUTH_TOKEN will default to true in gitea 1.23 and will be removed in gitea 1.24.") + } }