mirror
/
gitea
mirror of https://github.com/go-gitea/gitea
2
0
Fork 0
Code Issues Releases Wiki Activity

Refactor CSRF protector (#32057)

Browse Source 
Remove unused CSRF options, decouple "new csrf protector" and "prepare"
logic, do not redirect to home page if CSRF validation falis (it
shouldn't happen in daily usage, if it happens, redirecting to home
doesn't help either but just makes the problem more complex for "fetch")
pull/32073/head^2
wxiaoguang 2 months ago committed by GitHub
parent 55f1fcf0ad
commit 1fede04b83
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 71 additions and 172 deletions
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Show Stats Download Patch File Download Diff File
  1. 2
      options/locale/locale_en-US.ini
  2. 2
      routers/web/web.go
  3. 6
      services/context/context.go
  4. 182
      services/context/csrf.go
  5. 3
      tests/integration/attachment_test.go
  6. 26
      tests/integration/csrf_test.go
  7. 12
      tests/integration/repo_branch_test.go

2
options/locale/locale_en-US.ini
Unescape View File

@ -222,8 +222,6 @@ string.desc = Z - A
[error]
occurred = An error occurred
report_message = If you believe that this is a Gitea bug, please search for issues on <a href="%s" target="_blank">GitHub</a> or open a new issue if necessary.
missing_csrf = Bad Request: no CSRF token present
invalid_csrf = Bad Request: invalid CSRF token
not_found = The target couldn't be found.
network_error = Network error

2
routers/web/web.go
Unescape View File

@ -129,6 +129,8 @@ func webAuth(authMethod auth_service.Method) func(*context.Context) {
// ensure the session uid is deleted
_ = ctx.Session.Delete("uid")
}
ctx.Csrf.PrepareForSessionUser(ctx)
}
}

6
services/context/context.go
Unescape View File

@ -138,10 +138,8 @@ func Contexter() func(next http.Handler) http.Handler {
csrfOpts := CsrfOptions{
Secret: hex.EncodeToString(setting.GetGeneralTokenSigningSecret()),
Cookie: setting.CSRFCookieName,
SetCookie: true,
Secure: setting.SessionConfig.Secure,
CookieHTTPOnly: setting.CSRFCookieHTTPOnly,
Header: "X-Csrf-Token",
CookieDomain: setting.SessionConfig.Domain,
CookiePath: setting.SessionConfig.CookiePath,
SameSite: setting.SessionConfig.SameSite,
@ -167,7 +165,7 @@ func Contexter() func(next http.Handler) http.Handler {
ctx.Base.AppendContextValue(WebContextKey, ctx)
ctx.Base.AppendContextValueFunc(gitrepo.RepositoryContextKey, func() any { return ctx.Repo.GitRepo })
ctx.Csrf = PrepareCSRFProtector(csrfOpts, ctx)
ctx.Csrf = NewCSRFProtector(csrfOpts)
// Get the last flash message from cookie
lastFlashCookie := middleware.GetSiteCookie(ctx.Req, CookieNameFlash)
@ -204,8 +202,6 @@ func Contexter() func(next http.Handler) http.Handler {
ctx.Resp.Header().Set(`X-Frame-Options`, setting.CORSConfig.XFrameOptions)
ctx.Data["SystemConfig"] = setting.Config()
ctx.Data["CsrfToken"] = ctx.Csrf.GetToken()
ctx.Data["CsrfTokenHtml"] = template.HTML(`<input type="hidden" name="_csrf" value="` + ctx.Data["CsrfToken"].(string) + `">`)
// FIXME: do we really always need these setting? There should be someway to have to avoid having to always set these
ctx.Data["DisableMigrations"] = setting.Repository.DisableMigrations

182
services/context/csrf.go
Unescape View File

@ -20,64 +20,42 @@
package context
import (
"encoding/base32"
"fmt"
"html/template"
"net/http"
"strconv"
"time"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/util"
"code.gitea.io/gitea/modules/web/middleware"
)
const (
CsrfHeaderName = "X-Csrf-Token"
CsrfFormName = "_csrf"
)
// CSRFProtector represents a CSRF protector and is used to get the current token and validate the token.
type CSRFProtector interface {
// GetHeaderName returns HTTP header to search for token.
GetHeaderName() string
// GetFormName returns form value to search for token.
GetFormName() string
// GetToken returns the token.
GetToken() string
// Validate validates the token in http context.
// PrepareForSessionUser prepares the csrf protector for the current session user.
PrepareForSessionUser(ctx *Context)
// Validate validates the csrf token in http context.
Validate(ctx *Context)
// DeleteCookie deletes the cookie
// DeleteCookie deletes the csrf cookie
DeleteCookie(ctx *Context)
}
type csrfProtector struct {
opt CsrfOptions
// Token generated to pass via header, cookie, or hidden form value.
Token string
// This value must be unique per user.
ID string
}
// GetHeaderName returns the name of the HTTP header for csrf token.
func (c *csrfProtector) GetHeaderName() string {
return c.opt.Header
}
// GetFormName returns the name of the form value for csrf token.
func (c *csrfProtector) GetFormName() string {
return c.opt.Form
}
// GetToken returns the current token. This is typically used
// to populate a hidden form in an HTML template.
func (c *csrfProtector) GetToken() string {
return c.Token
// id must be unique per user.
id string
// token is the valid one which wil be used by end user and passed via header, cookie, or hidden form value.
token string
}
// CsrfOptions maintains options to manage behavior of Generate.
type CsrfOptions struct {
// The global secret value used to generate Tokens.
Secret string
// HTTP header used to set and get token.
Header string
// Form value used to set and get token.
Form string
// Cookie value used to set and get token.
Cookie string
// Cookie domain.
@ -87,103 +65,64 @@ type CsrfOptions struct {
CookieHTTPOnly bool
// SameSite set the cookie SameSite type
SameSite http.SameSite
// Key used for getting the unique ID per user.
SessionKey string
// oldSessionKey saves old value corresponding to SessionKey.
oldSessionKey string
// If true, send token via X-Csrf-Token header.
SetHeader bool
// If true, send token via _csrf cookie.
SetCookie bool
// Set the Secure flag to true on the cookie.
Secure bool
// Disallow Origin appear in request header.
Origin bool
// Cookie lifetime. Default is 0
CookieLifeTime int
}
func prepareDefaultCsrfOptions(opt CsrfOptions) CsrfOptions {
if opt.Secret == "" {
randBytes, err := util.CryptoRandomBytes(8)
if err != nil {
// this panic can be handled by the recover() in http handlers
panic(fmt.Errorf("failed to generate random bytes: %w", err))
}
opt.Secret = base32.StdEncoding.EncodeToString(randBytes)
}
if opt.Header == "" {
opt.Header = "X-Csrf-Token"
}
if opt.Form == "" {
opt.Form = "_csrf"
}
if opt.Cookie == "" {
opt.Cookie = "_csrf"
}
if opt.CookiePath == "" {
opt.CookiePath = "/"
}
if opt.SessionKey == "" {
opt.SessionKey = "uid"
}
if opt.CookieLifeTime == 0 {
opt.CookieLifeTime = int(CsrfTokenTimeout.Seconds())
}
opt.oldSessionKey = "_old_" + opt.SessionKey
return opt
// sessionKey is the key used for getting the unique ID per user.
sessionKey string
// oldSessionKey saves old value corresponding to sessionKey.
oldSessionKey string
}
func newCsrfCookie(c *csrfProtector, value string) *http.Cookie {
func newCsrfCookie(opt *CsrfOptions, value string) *http.Cookie {
return &http.Cookie{
Name: c.opt.Cookie,
Name: opt.Cookie,
Value: value,
Path: c.opt.CookiePath,
Domain: c.opt.CookieDomain,
MaxAge: c.opt.CookieLifeTime,
Secure: c.opt.Secure,
HttpOnly: c.opt.CookieHTTPOnly,
SameSite: c.opt.SameSite,
Path: opt.CookiePath,
Domain: opt.CookieDomain,
MaxAge: int(CsrfTokenTimeout.Seconds()),
Secure: opt.Secure,
HttpOnly: opt.CookieHTTPOnly,
SameSite: opt.SameSite,
}
}
// PrepareCSRFProtector returns a CSRFProtector to be used for every request.
// Additionally, depending on options set, generated tokens will be sent via Header and/or Cookie.
func PrepareCSRFProtector(opt CsrfOptions, ctx *Context) CSRFProtector {
opt = prepareDefaultCsrfOptions(opt)
x := &csrfProtector{opt: opt}
if opt.Origin && len(ctx.Req.Header.Get("Origin")) > 0 {
return x
func NewCSRFProtector(opt CsrfOptions) CSRFProtector {
if opt.Secret == "" {
panic("CSRF secret is empty but it must be set") // it shouldn't happen because it is always set in code
}
opt.Cookie = util.IfZero(opt.Cookie, "_csrf")
opt.CookiePath = util.IfZero(opt.CookiePath, "/")
opt.sessionKey = "uid"
opt.oldSessionKey = "_old_" + opt.sessionKey
return &csrfProtector{opt: opt}
}
x.ID = "0"
uidAny := ctx.Session.Get(opt.SessionKey)
if uidAny != nil {
func (c *csrfProtector) PrepareForSessionUser(ctx *Context) {
c.id = "0"
if uidAny := ctx.Session.Get(c.opt.sessionKey); uidAny != nil {
switch uidVal := uidAny.(type) {
case string:
x.ID = uidVal
c.id = uidVal
case int64:
x.ID = strconv.FormatInt(uidVal, 10)
c.id = strconv.FormatInt(uidVal, 10)
default:
log.Error("invalid uid type in session: %T", uidAny)
}
}
oldUID := ctx.Session.Get(opt.oldSessionKey)
uidChanged := oldUID == nil || oldUID.(string) != x.ID
cookieToken := ctx.GetSiteCookie(opt.Cookie)
oldUID := ctx.Session.Get(c.opt.oldSessionKey)
uidChanged := oldUID == nil || oldUID.(string) != c.id
cookieToken := ctx.GetSiteCookie(c.opt.Cookie)
needsNew := true
if uidChanged {
_ = ctx.Session.Set(opt.oldSessionKey, x.ID)
_ = ctx.Session.Set(c.opt.oldSessionKey, c.id)
} else if cookieToken != "" {
// If cookie token presents, re-use existing unexpired token, else generate a new one.
if issueTime, ok := ParseCsrfToken(cookieToken); ok {
dur := time.Since(issueTime) // issueTime is not a monotonic-clock, the server time may change a lot to an early time.
if dur >= -CsrfTokenRegenerationInterval && dur <= CsrfTokenRegenerationInterval {
x.Token = cookieToken
c.token = cookieToken
needsNew = false
}
}
@ -191,42 +130,33 @@ func PrepareCSRFProtector(opt CsrfOptions, ctx *Context) CSRFProtector {
if needsNew {
// FIXME: actionId.
x.Token = GenerateCsrfToken(x.opt.Secret, x.ID, "POST", time.Now())
if opt.SetCookie {
cookie := newCsrfCookie(x, x.Token)
c.token = GenerateCsrfToken(c.opt.Secret, c.id, "POST", time.Now())
cookie := newCsrfCookie(&c.opt, c.token)
ctx.Resp.Header().Add("Set-Cookie", cookie.String())
}
}
if opt.SetHeader {
ctx.Resp.Header().Add(opt.Header, x.Token)
}
return x
ctx.Data["CsrfToken"] = c.token
ctx.Data["CsrfTokenHtml"] = template.HTML(`<input type="hidden" name="_csrf" value="` + template.HTMLEscapeString(c.token) + `">`)
}
func (c *csrfProtector) validateToken(ctx *Context, token string) {
if !ValidCsrfToken(token, c.opt.Secret, c.ID, "POST", time.Now()) {
if !ValidCsrfToken(token, c.opt.Secret, c.id, "POST", time.Now()) {
c.DeleteCookie(ctx)
if middleware.IsAPIPath(ctx.Req) {
// currently, there should be no access to the APIPath with CSRF token. because templates shouldn't use the `/api/` endpoints.
// FIXME: distinguish what the response is for: HTML (web page) or JSON (fetch)
http.Error(ctx.Resp, "Invalid CSRF token.", http.StatusBadRequest)
} else {
ctx.Flash.Error(ctx.Tr("error.invalid_csrf"))
ctx.Redirect(setting.AppSubURL + "/")
}
}
}
// Validate should be used as a per route middleware. It attempts to get a token from an "X-Csrf-Token"
// HTTP header and then a "_csrf" form value. If one of these is found, the token will be validated.
// If this validation fails, custom Error is sent in the reply.
// If neither a header nor form value is found, http.StatusBadRequest is sent.
// If this validation fails, http.StatusBadRequest is sent.
func (c *csrfProtector) Validate(ctx *Context) {
if token := ctx.Req.Header.Get(c.GetHeaderName()); token != "" {
if token := ctx.Req.Header.Get(CsrfHeaderName); token != "" {
c.validateToken(ctx, token)
return
}
if token := ctx.Req.FormValue(c.GetFormName()); token != "" {
if token := ctx.Req.FormValue(CsrfFormName); token != "" {
c.validateToken(ctx, token)
return
}
@ -234,9 +164,7 @@ func (c *csrfProtector) Validate(ctx *Context) {
}
func (c *csrfProtector) DeleteCookie(ctx *Context) {
if c.opt.SetCookie {
cookie := newCsrfCookie(c, "")
cookie := newCsrfCookie(&c.opt, "")
cookie.MaxAge = -1
ctx.Resp.Header().Add("Set-Cookie", cookie.String())
}
}

3
tests/integration/attachment_test.go
Unescape View File

@ -59,7 +59,8 @@ func createAttachment(t *testing.T, session *TestSession, repoURL, filename stri
func TestCreateAnonymousAttachment(t *testing.T) {
defer tests.PrepareTestEnv(t)()
session := emptyTestSession(t)
createAttachment(t, session, "user2/repo1", "image.png", generateImg(), http.StatusSeeOther)
// this test is not right because it just doesn't pass the CSRF validation
createAttachment(t, session, "user2/repo1", "image.png", generateImg(), http.StatusBadRequest)
}
func TestCreateIssueAttachment(t *testing.T) {

26
tests/integration/csrf_test.go
Unescape View File

@ -5,12 +5,10 @@ package integration
import (
"net/http"
"strings"
"testing"
"code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/tests"
"github.com/stretchr/testify/assert"
@ -25,28 +23,12 @@ func TestCsrfProtection(t *testing.T) {
req := NewRequestWithValues(t, "POST", "/user/settings", map[string]string{
"_csrf": "fake_csrf",
})
session.MakeRequest(t, req, http.StatusSeeOther)
resp := session.MakeRequest(t, req, http.StatusSeeOther)
loc := resp.Header().Get("Location")
assert.Equal(t, setting.AppSubURL+"/", loc)
resp = session.MakeRequest(t, NewRequest(t, "GET", loc), http.StatusOK)
htmlDoc := NewHTMLParser(t, resp.Body)
assert.Equal(t, "Bad Request: invalid CSRF token",
strings.TrimSpace(htmlDoc.doc.Find(".ui.message").Text()),
)
resp := session.MakeRequest(t, req, http.StatusBadRequest)
assert.Contains(t, resp.Body.String(), "Invalid CSRF token")
// test web form csrf via header. TODO: should use an UI api to test
req = NewRequest(t, "POST", "/user/settings")
req.Header.Add("X-Csrf-Token", "fake_csrf")
session.MakeRequest(t, req, http.StatusSeeOther)
resp = session.MakeRequest(t, req, http.StatusSeeOther)
loc = resp.Header().Get("Location")
assert.Equal(t, setting.AppSubURL+"/", loc)
resp = session.MakeRequest(t, NewRequest(t, "GET", loc), http.StatusOK)
htmlDoc = NewHTMLParser(t, resp.Body)
assert.Equal(t, "Bad Request: invalid CSRF token",
strings.TrimSpace(htmlDoc.doc.Find(".ui.message").Text()),
)
resp = session.MakeRequest(t, req, http.StatusBadRequest)
assert.Contains(t, resp.Body.String(), "Invalid CSRF token")
}

12
tests/integration/repo_branch_test.go
Unescape View File

@ -17,7 +17,6 @@ import (
repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unit"
"code.gitea.io/gitea/models/unittest"
"code.gitea.io/gitea/modules/setting"
api "code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/test"
"code.gitea.io/gitea/modules/translation"
@ -146,15 +145,8 @@ func TestCreateBranchInvalidCSRF(t *testing.T) {
"_csrf": "fake_csrf",
"new_branch_name": "test",
})
resp := session.MakeRequest(t, req, http.StatusSeeOther)
loc := resp.Header().Get("Location")
assert.Equal(t, setting.AppSubURL+"/", loc)
resp = session.MakeRequest(t, NewRequest(t, "GET", loc), http.StatusOK)
htmlDoc := NewHTMLParser(t, resp.Body)
assert.Equal(t,
"Bad Request: invalid CSRF token",
strings.TrimSpace(htmlDoc.doc.Find(".ui.message").Text()),
)
resp := session.MakeRequest(t, req, http.StatusBadRequest)
assert.Contains(t, resp.Body.String(), "Invalid CSRF token")
}
func prepareBranch(t *testing.T, session *TestSession, repo *repo_model.Repository) {

Write Preview
Loading…
Cancel
Save