summaryrefslogtreecommitdiffstats
path: root/modules
diff options
context:
space:
mode:
authorwxiaoguang <wxiaoguang@gmail.com>2022-04-08 13:21:05 +0800
committerGitHub <noreply@github.com>2022-04-08 13:21:05 +0800
commit84ceaa98bd731431c7d3a7f65e59e7ad076a540f (patch)
treefc2743a69cde4e46c3a55796e2ab1541269b6c65 /modules
parent3c3d49899f0f7206e190bdeecdc4da248cc7e686 (diff)
downloadgitea-84ceaa98bd731431c7d3a7f65e59e7ad076a540f.tar.gz
gitea-84ceaa98bd731431c7d3a7f65e59e7ad076a540f.zip
Refactor CSRF protection modules, make sure CSRF tokens can be up-to-date. (#19337)
Do a refactoring to the CSRF related code, remove most unnecessary functions. Parse the generated token's issue time, regenerate the token every a few minutes.
Diffstat (limited to 'modules')
-rw-r--r--modules/context/auth.go2
-rw-r--r--modules/context/context.go10
-rw-r--r--modules/context/csrf.go188
-rw-r--r--modules/context/xsrf.go68
-rw-r--r--modules/context/xsrf_test.go20
-rw-r--r--modules/web/middleware/cookie.go13
6 files changed, 111 insertions, 190 deletions
diff --git a/modules/context/auth.go b/modules/context/auth.go
index 1a46ab586a..09c2295455 100644
--- a/modules/context/auth.go
+++ b/modules/context/auth.go
@@ -63,7 +63,7 @@ func Toggle(options *ToggleOptions) func(ctx *Context) {
}
if !options.SignOutRequired && !options.DisableCSRF && ctx.Req.Method == "POST" {
- Validate(ctx, ctx.csrf)
+ ctx.csrf.Validate(ctx)
if ctx.Written() {
return
}
diff --git a/modules/context/context.go b/modules/context/context.go
index 8ede3646a4..17e1673a36 100644
--- a/modules/context/context.go
+++ b/modules/context/context.go
@@ -59,7 +59,7 @@ type Context struct {
Render Render
translation.Locale
Cache cache.Cache
- csrf CSRF
+ csrf CSRFProtector
Flash *middleware.Flash
Session session.Store
@@ -666,7 +666,9 @@ func Auth(authMethod auth.Method) func(*Context) {
func Contexter() func(next http.Handler) http.Handler {
rnd := templates.HTMLRenderer()
csrfOpts := getCsrfOpts()
-
+ if !setting.IsProd {
+ CsrfTokenRegenerationInterval = 5 * time.Second // in dev, re-generate the tokens more aggressively for debug purpose
+ }
return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) {
locale := middleware.Locale(resp, req)
@@ -697,7 +699,7 @@ func Contexter() func(next http.Handler) http.Handler {
ctx.Data["Context"] = &ctx
ctx.Req = WithContext(req, &ctx)
- ctx.csrf = Csrfer(csrfOpts, &ctx)
+ ctx.csrf = PrepareCSRFProtector(csrfOpts, &ctx)
// Get flash.
flashCookie := ctx.GetCookie("macaron_flash")
@@ -755,7 +757,7 @@ func Contexter() func(next http.Handler) http.Handler {
ctx.Resp.Header().Set(`X-Frame-Options`, setting.CORSConfig.XFrameOptions)
- ctx.Data["CsrfToken"] = html.EscapeString(ctx.csrf.GetToken())
+ 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
diff --git a/modules/context/csrf.go b/modules/context/csrf.go
index 4fc9270504..df775048cb 100644
--- a/modules/context/csrf.go
+++ b/modules/context/csrf.go
@@ -31,29 +31,19 @@ import (
"code.gitea.io/gitea/modules/web/middleware"
)
-// CSRF represents a CSRF service and is used to get the current token and validate a suspect token.
-type CSRF interface {
- // Return HTTP header to search for token.
+// 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
- // Return form value to search for token.
+ // GetFormName returns form value to search for token.
GetFormName() string
- // Return cookie name to search for token.
- GetCookieName() string
- // Return cookie path
- GetCookiePath() string
- // Return the flag value used for the csrf token.
- GetCookieHTTPOnly() bool
- // Return cookie domain
- GetCookieDomain() string
- // Return the token.
+ // GetToken returns the token.
GetToken() string
- // Validate by token.
- ValidToken(t string) bool
- // Error replies to the request with a custom function when ValidToken fails.
- Error(w http.ResponseWriter)
+ // Validate validates the token in http context.
+ Validate(ctx *Context)
}
-type csrf struct {
+type csrfProtector struct {
// Header name value for setting and getting csrf token.
Header string
// Form name value for setting and getting csrf token.
@@ -72,56 +62,24 @@ type csrf struct {
ID string
// Secret used along with the unique id above to generate the Token.
Secret string
- // ErrorFunc is the custom function that replies to the request when ValidToken fails.
- ErrorFunc func(w http.ResponseWriter)
}
// GetHeaderName returns the name of the HTTP header for csrf token.
-func (c *csrf) GetHeaderName() string {
+func (c *csrfProtector) GetHeaderName() string {
return c.Header
}
// GetFormName returns the name of the form value for csrf token.
-func (c *csrf) GetFormName() string {
+func (c *csrfProtector) GetFormName() string {
return c.Form
}
-// GetCookieName returns the name of the cookie for csrf token.
-func (c *csrf) GetCookieName() string {
- return c.Cookie
-}
-
-// GetCookiePath returns the path of the cookie for csrf token.
-func (c *csrf) GetCookiePath() string {
- return c.CookiePath
-}
-
-// GetCookieHTTPOnly returns the flag value used for the csrf token.
-func (c *csrf) GetCookieHTTPOnly() bool {
- return c.CookieHTTPOnly
-}
-
-// GetCookieDomain returns the flag value used for the csrf token.
-func (c *csrf) GetCookieDomain() string {
- return c.CookieDomain
-}
-
// GetToken returns the current token. This is typically used
// to populate a hidden form in an HTML template.
-func (c *csrf) GetToken() string {
+func (c *csrfProtector) GetToken() string {
return c.Token
}
-// ValidToken validates the passed token against the existing Secret and ID.
-func (c *csrf) ValidToken(t string) bool {
- return ValidToken(t, c.Secret, c.ID, "POST")
-}
-
-// Error replies to the request when ValidToken fails.
-func (c *csrf) Error(w http.ResponseWriter) {
- c.ErrorFunc(w)
-}
-
// CsrfOptions maintains options to manage behavior of Generate.
type CsrfOptions struct {
// The global secret value used to generate Tokens.
@@ -143,7 +101,7 @@ type CsrfOptions struct {
SessionKey string
// oldSessionKey saves old value corresponding to SessionKey.
oldSessionKey string
- // If true, send token via X-CSRFToken header.
+ // If true, send token via X-Csrf-Token header.
SetHeader bool
// If true, send token via _csrf cookie.
SetCookie bool
@@ -151,20 +109,12 @@ type CsrfOptions struct {
Secure bool
// Disallow Origin appear in request header.
Origin bool
- // The function called when Validate fails.
- ErrorFunc func(w http.ResponseWriter)
- // Cookie life time. Default is 0
+ // Cookie lifetime. Default is 0
CookieLifeTime int
}
-func prepareOptions(options []CsrfOptions) CsrfOptions {
- var opt CsrfOptions
- if len(options) > 0 {
- opt = options[0]
- }
-
- // Defaults.
- if len(opt.Secret) == 0 {
+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
@@ -172,36 +122,30 @@ func prepareOptions(options []CsrfOptions) CsrfOptions {
}
opt.Secret = base32.StdEncoding.EncodeToString(randBytes)
}
- if len(opt.Header) == 0 {
- opt.Header = "X-CSRFToken"
+ if opt.Header == "" {
+ opt.Header = "X-Csrf-Token"
}
- if len(opt.Form) == 0 {
+ if opt.Form == "" {
opt.Form = "_csrf"
}
- if len(opt.Cookie) == 0 {
+ if opt.Cookie == "" {
opt.Cookie = "_csrf"
}
- if len(opt.CookiePath) == 0 {
+ if opt.CookiePath == "" {
opt.CookiePath = "/"
}
- if len(opt.SessionKey) == 0 {
+ if opt.SessionKey == "" {
opt.SessionKey = "uid"
}
opt.oldSessionKey = "_old_" + opt.SessionKey
- if opt.ErrorFunc == nil {
- opt.ErrorFunc = func(w http.ResponseWriter) {
- http.Error(w, "Invalid csrf token.", http.StatusBadRequest)
- }
- }
-
return opt
}
-// Csrfer maps CSRF to each request. If this request is a Get request, it will generate a new token.
+// 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 Csrfer(opt CsrfOptions, ctx *Context) CSRF {
- opt = prepareOptions([]CsrfOptions{opt})
- x := &csrf{
+func PrepareCSRFProtector(opt CsrfOptions, ctx *Context) CSRFProtector {
+ opt = prepareDefaultCsrfOptions(opt)
+ x := &csrfProtector{
Secret: opt.Secret,
Header: opt.Header,
Form: opt.Form,
@@ -209,7 +153,6 @@ func Csrfer(opt CsrfOptions, ctx *Context) CSRF {
CookieDomain: opt.CookieDomain,
CookiePath: opt.CookiePath,
CookieHTTPOnly: opt.CookieHTTPOnly,
- ErrorFunc: opt.ErrorFunc,
}
if opt.Origin && len(ctx.Req.Header.Get("Origin")) > 0 {
@@ -229,28 +172,31 @@ func Csrfer(opt CsrfOptions, ctx *Context) CSRF {
}
}
- needsNew := false
oldUID := ctx.Session.Get(opt.oldSessionKey)
- if oldUID == nil || oldUID.(string) != x.ID {
- needsNew = true
+ uidChanged := oldUID == nil || oldUID.(string) != x.ID
+ cookieToken := ctx.GetCookie(opt.Cookie)
+
+ needsNew := true
+ if uidChanged {
_ = ctx.Session.Set(opt.oldSessionKey, x.ID)
- } else {
- // If cookie present, map existing token, else generate a new one.
- if val := ctx.GetCookie(opt.Cookie); len(val) > 0 {
- // FIXME: test coverage.
- x.Token = val
- } else {
- needsNew = true
+ } 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
+ needsNew = false
+ }
}
}
if needsNew {
// FIXME: actionId.
- x.Token = GenerateToken(x.Secret, x.ID, "POST")
+ x.Token = GenerateCsrfToken(x.Secret, x.ID, "POST", time.Now())
if opt.SetCookie {
var expires interface{}
if opt.CookieLifeTime == 0 {
- expires = time.Now().AddDate(0, 0, 1)
+ expires = time.Now().Add(CsrfTokenTimeout)
}
middleware.SetCookie(ctx.Resp, opt.Cookie, x.Token,
opt.CookieLifeTime,
@@ -270,47 +216,31 @@ func Csrfer(opt CsrfOptions, ctx *Context) CSRF {
return x
}
-// Validate should be used as a per route middleware. It attempts to get a token from a "X-CSRFToken"
-// HTTP header and then a "_csrf" form value. If one of these is found, the token will be validated
-// using ValidToken. If this validation fails, custom Error is sent in the reply.
-// If neither a header or form value is found, http.StatusBadRequest is sent.
-func Validate(ctx *Context, x CSRF) {
- if token := ctx.Req.Header.Get(x.GetHeaderName()); len(token) > 0 {
- if !x.ValidToken(token) {
- // Delete the cookie
- middleware.SetCookie(ctx.Resp, x.GetCookieName(), "",
- -1,
- x.GetCookiePath(),
- x.GetCookieDomain()) // FIXME: Do we need to set the Secure, httpOnly and SameSite values too?
- if middleware.IsAPIPath(ctx.Req) {
- x.Error(ctx.Resp)
- return
- }
+func (c *csrfProtector) validateToken(ctx *Context, token string) {
+ if !ValidCsrfToken(token, c.Secret, c.ID, "POST", time.Now()) {
+ middleware.DeleteCSRFCookie(ctx.Resp)
+ 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.
+ http.Error(ctx.Resp, "Invalid CSRF token.", http.StatusBadRequest)
+ } else {
ctx.Flash.Error(ctx.Tr("error.invalid_csrf"))
ctx.Redirect(setting.AppSubURL + "/")
}
- return
}
- if token := ctx.Req.FormValue(x.GetFormName()); len(token) > 0 {
- if !x.ValidToken(token) {
- // Delete the cookie
- middleware.SetCookie(ctx.Resp, x.GetCookieName(), "",
- -1,
- x.GetCookiePath(),
- x.GetCookieDomain()) // FIXME: Do we need to set the Secure, httpOnly and SameSite values too?
- if middleware.IsAPIPath(ctx.Req) {
- x.Error(ctx.Resp)
- return
- }
- 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.
+func (c *csrfProtector) Validate(ctx *Context) {
+ if token := ctx.Req.Header.Get(c.GetHeaderName()); token != "" {
+ c.validateToken(ctx, token)
return
}
- if middleware.IsAPIPath(ctx.Req) {
- http.Error(ctx.Resp, "Bad Request: no CSRF token present", http.StatusBadRequest)
+ if token := ctx.Req.FormValue(c.GetFormName()); token != "" {
+ c.validateToken(ctx, token)
return
}
- ctx.Flash.Error(ctx.Tr("error.missing_csrf"))
- ctx.Redirect(setting.AppSubURL + "/")
+ c.validateToken(ctx, "") // no csrf token, use an empty token to respond error
}
diff --git a/modules/context/xsrf.go b/modules/context/xsrf.go
index 10e63a4180..e3ecc82f6d 100644
--- a/modules/context/xsrf.go
+++ b/modules/context/xsrf.go
@@ -28,69 +28,69 @@ import (
"time"
)
-// Timeout represents the duration that XSRF tokens are valid.
+// CsrfTokenTimeout represents the duration that XSRF tokens are valid.
// It is exported so clients may set cookie timeouts that match generated tokens.
-const Timeout = 24 * time.Hour
+const CsrfTokenTimeout = 24 * time.Hour
-// clean sanitizes a string for inclusion in a token by replacing all ":"s.
-func clean(s string) string {
- return strings.ReplaceAll(s, ":", "_")
-}
+// CsrfTokenRegenerationInterval is the interval between token generations, old tokens are still valid before CsrfTokenTimeout
+var CsrfTokenRegenerationInterval = 10 * time.Minute
-// GenerateToken returns a URL-safe secure XSRF token that expires in 24 hours.
-//
+var csrfTokenSep = []byte(":")
+
+// GenerateCsrfToken returns a URL-safe secure XSRF token that expires in CsrfTokenTimeout hours.
// key is a secret key for your application.
// userID is a unique identifier for the user.
// actionID is the action the user is taking (e.g. POSTing to a particular path).
-func GenerateToken(key, userID, actionID string) string {
- return generateTokenAtTime(key, userID, actionID, time.Now())
-}
-
-// generateTokenAtTime is like Generate, but returns a token that expires 24 hours from now.
-func generateTokenAtTime(key, userID, actionID string, now time.Time) string {
+func GenerateCsrfToken(key, userID, actionID string, now time.Time) string {
+ nowUnixNano := now.UnixNano()
+ nowUnixNanoStr := strconv.FormatInt(nowUnixNano, 10)
h := hmac.New(sha1.New, []byte(key))
- fmt.Fprintf(h, "%s:%s:%d", clean(userID), clean(actionID), now.UnixNano())
- tok := fmt.Sprintf("%s:%d", h.Sum(nil), now.UnixNano())
+ h.Write([]byte(strings.ReplaceAll(userID, ":", "_")))
+ h.Write(csrfTokenSep)
+ h.Write([]byte(strings.ReplaceAll(actionID, ":", "_")))
+ h.Write(csrfTokenSep)
+ h.Write([]byte(nowUnixNanoStr))
+ tok := fmt.Sprintf("%s:%s", h.Sum(nil), nowUnixNanoStr)
return base64.RawURLEncoding.EncodeToString([]byte(tok))
}
-// ValidToken returns true if token is a valid, unexpired token returned by Generate.
-func ValidToken(token, key, userID, actionID string) bool {
- return validTokenAtTime(token, key, userID, actionID, time.Now())
-}
-
-// validTokenAtTime is like Valid, but it uses now to check if the token is expired.
-func validTokenAtTime(token, key, userID, actionID string, now time.Time) bool {
- // Decode the token.
+func ParseCsrfToken(token string) (issueTime time.Time, ok bool) {
data, err := base64.RawURLEncoding.DecodeString(token)
if err != nil {
- return false
+ return time.Time{}, false
}
- // Extract the issue time of the token.
- sep := bytes.LastIndex(data, []byte{':'})
- if sep < 0 {
- return false
+ pos := bytes.LastIndex(data, csrfTokenSep)
+ if pos == -1 {
+ return time.Time{}, false
}
- nanos, err := strconv.ParseInt(string(data[sep+1:]), 10, 64)
+ nanos, err := strconv.ParseInt(string(data[pos+1:]), 10, 64)
if err != nil {
+ return time.Time{}, false
+ }
+ return time.Unix(0, nanos), true
+}
+
+// ValidCsrfToken returns true if token is a valid and unexpired token returned by Generate.
+func ValidCsrfToken(token, key, userID, actionID string, now time.Time) bool {
+ issueTime, ok := ParseCsrfToken(token)
+ if !ok {
return false
}
- issueTime := time.Unix(0, nanos)
// Check that the token is not expired.
- if now.Sub(issueTime) >= Timeout {
+ if now.Sub(issueTime) >= CsrfTokenTimeout {
return false
}
// Check that the token is not from the future.
- // Allow 1 minute grace period in case the token is being verified on a
+ // Allow 1-minute grace period in case the token is being verified on a
// machine whose clock is behind the machine that issued the token.
if issueTime.After(now.Add(1 * time.Minute)) {
return false
}
- expected := generateTokenAtTime(key, userID, actionID, issueTime)
+ expected := GenerateCsrfToken(key, userID, actionID, issueTime)
// Check that the token matches the expected value.
// Use constant time comparison to avoid timing attacks.
diff --git a/modules/context/xsrf_test.go b/modules/context/xsrf_test.go
index c0c711bf07..ef42d61d5a 100644
--- a/modules/context/xsrf_test.go
+++ b/modules/context/xsrf_test.go
@@ -37,18 +37,18 @@ var (
func Test_ValidToken(t *testing.T) {
t.Run("Validate token", func(t *testing.T) {
- tok := generateTokenAtTime(key, userID, actionID, now)
- assert.True(t, validTokenAtTime(tok, key, userID, actionID, oneMinuteFromNow))
- assert.True(t, validTokenAtTime(tok, key, userID, actionID, now.Add(Timeout-1*time.Nanosecond)))
- assert.True(t, validTokenAtTime(tok, key, userID, actionID, now.Add(-1*time.Minute)))
+ tok := GenerateCsrfToken(key, userID, actionID, now)
+ assert.True(t, ValidCsrfToken(tok, key, userID, actionID, oneMinuteFromNow))
+ assert.True(t, ValidCsrfToken(tok, key, userID, actionID, now.Add(CsrfTokenTimeout-1*time.Nanosecond)))
+ assert.True(t, ValidCsrfToken(tok, key, userID, actionID, now.Add(-1*time.Minute)))
})
}
// Test_SeparatorReplacement tests that separators are being correctly substituted
func Test_SeparatorReplacement(t *testing.T) {
t.Run("Test two separator replacements", func(t *testing.T) {
- assert.NotEqual(t, generateTokenAtTime("foo:bar", "baz", "wah", now),
- generateTokenAtTime("foo", "bar:baz", "wah", now))
+ assert.NotEqual(t, GenerateCsrfToken("foo:bar", "baz", "wah", now),
+ GenerateCsrfToken("foo", "bar:baz", "wah", now))
})
}
@@ -61,13 +61,13 @@ func Test_InvalidToken(t *testing.T) {
{"Bad key", "foobar", userID, actionID, oneMinuteFromNow},
{"Bad userID", key, "foobar", actionID, oneMinuteFromNow},
{"Bad actionID", key, userID, "foobar", oneMinuteFromNow},
- {"Expired", key, userID, actionID, now.Add(Timeout)},
+ {"Expired", key, userID, actionID, now.Add(CsrfTokenTimeout)},
{"More than 1 minute from the future", key, userID, actionID, now.Add(-1*time.Nanosecond - 1*time.Minute)},
}
- tok := generateTokenAtTime(key, userID, actionID, now)
+ tok := GenerateCsrfToken(key, userID, actionID, now)
for _, itt := range invalidTokenTests {
- assert.False(t, validTokenAtTime(tok, itt.key, itt.userID, itt.actionID, itt.t))
+ assert.False(t, ValidCsrfToken(tok, itt.key, itt.userID, itt.actionID, itt.t))
}
})
}
@@ -84,7 +84,7 @@ func Test_ValidateBadData(t *testing.T) {
}
for _, bdt := range badDataTests {
- assert.False(t, validTokenAtTime(bdt.tok, key, userID, actionID, oneMinuteFromNow))
+ assert.False(t, ValidCsrfToken(bdt.tok, key, userID, actionID, oneMinuteFromNow))
}
})
}
diff --git a/modules/web/middleware/cookie.go b/modules/web/middleware/cookie.go
index 80fe302137..b5904d6713 100644
--- a/modules/web/middleware/cookie.go
+++ b/modules/web/middleware/cookie.go
@@ -98,17 +98,6 @@ func DeleteRedirectToCookie(resp http.ResponseWriter) {
SameSite(setting.SessionConfig.SameSite))
}
-// DeleteSesionConfigPathCookie convenience function to delete SessionConfigPath cookies consistently
-func DeleteSesionConfigPathCookie(resp http.ResponseWriter, name string) {
- SetCookie(resp, name, "",
- -1,
- setting.SessionConfig.CookiePath,
- setting.SessionConfig.Domain,
- setting.SessionConfig.Secure,
- true,
- SameSite(setting.SessionConfig.SameSite))
-}
-
// DeleteCSRFCookie convenience function to delete SessionConfigPath cookies consistently
func DeleteCSRFCookie(resp http.ResponseWriter) {
SetCookie(resp, setting.CSRFCookieName, "",
@@ -117,7 +106,7 @@ func DeleteCSRFCookie(resp http.ResponseWriter) {
setting.SessionConfig.Domain) // FIXME: Do we need to set the Secure, httpOnly and SameSite values too?
}
-// SetCookie set the cookies
+// SetCookie set the cookies. (name, value, lifetime, path, domain, secure, httponly, expires, {sameSite, ...})
// TODO: Copied from gitea.com/macaron/macaron and should be improved after macaron removed.
func SetCookie(resp http.ResponseWriter, name, value string, others ...interface{}) {
cookie := http.Cookie{}