Skip to content

Commit 9c5b27b

Browse files
committed
Refactor CSRF
1 parent 1843026 commit 9c5b27b

File tree

10 files changed

+124
-184
lines changed

10 files changed

+124
-184
lines changed

modules/context/api.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ package context
88
import (
99
"context"
1010
"fmt"
11-
"html"
1211
"net/http"
1312
"net/url"
1413
"strings"
@@ -212,7 +211,7 @@ func (ctx *APIContext) RequireCSRF() {
212211
headerToken := ctx.Req.Header.Get(ctx.csrf.GetHeaderName())
213212
formValueToken := ctx.Req.FormValue(ctx.csrf.GetFormName())
214213
if len(headerToken) > 0 || len(formValueToken) > 0 {
215-
Validate(ctx.Context, ctx.csrf)
214+
ctx.csrf.Validate(ctx.Context)
216215
} else {
217216
ctx.Context.Error(http.StatusUnauthorized, "Missing CSRF token.")
218217
}
@@ -289,7 +288,7 @@ func APIContexter() func(http.Handler) http.Handler {
289288
}
290289

291290
ctx.Req = WithAPIContext(WithContext(req, ctx.Context), &ctx)
292-
ctx.csrf = Csrfer(csrfOpts, ctx.Context)
291+
ctx.csrf = NewCSRFProtector(csrfOpts, ctx.Context)
293292

294293
// If request sends files, parse them here otherwise the Query() can't be parsed and the CsrfToken will be invalid.
295294
if ctx.Req.Method == "POST" && strings.Contains(ctx.Req.Header.Get("Content-Type"), "multipart/form-data") {
@@ -301,7 +300,7 @@ func APIContexter() func(http.Handler) http.Handler {
301300

302301
ctx.Resp.Header().Set(`X-Frame-Options`, setting.CORSConfig.XFrameOptions)
303302

304-
ctx.Data["CsrfToken"] = html.EscapeString(ctx.csrf.GetToken())
303+
ctx.Data["CsrfToken"] = ctx.csrf.GetToken()
305304
ctx.Data["Context"] = &ctx
306305

307306
next.ServeHTTP(ctx.Resp, ctx.Req)

modules/context/auth.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ func Toggle(options *ToggleOptions) func(ctx *Context) {
6363
}
6464

6565
if !options.SignOutRequired && !options.DisableCSRF && ctx.Req.Method == "POST" {
66-
Validate(ctx, ctx.csrf)
66+
ctx.csrf.Validate(ctx)
6767
if ctx.Written() {
6868
return
6969
}

modules/context/context.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ type Context struct {
5757
Render Render
5858
translation.Locale
5959
Cache cache.Cache
60-
csrf CSRF
60+
csrf CSRFProtector
6161
Flash *middleware.Flash
6262
Session session.Store
6363

@@ -679,7 +679,7 @@ func Contexter() func(next http.Handler) http.Handler {
679679
ctx.Data["Context"] = &ctx
680680

681681
ctx.Req = WithContext(req, &ctx)
682-
ctx.csrf = Csrfer(csrfOpts, &ctx)
682+
ctx.csrf = NewCSRFProtector(csrfOpts, &ctx)
683683

684684
// Get flash.
685685
flashCookie := ctx.GetCookie("macaron_flash")
@@ -737,7 +737,7 @@ func Contexter() func(next http.Handler) http.Handler {
737737

738738
ctx.Resp.Header().Set(`X-Frame-Options`, setting.CORSConfig.XFrameOptions)
739739

740-
ctx.Data["CsrfToken"] = html.EscapeString(ctx.csrf.GetToken())
740+
ctx.Data["CsrfToken"] = ctx.csrf.GetToken()
741741
ctx.Data["CsrfTokenHtml"] = template.HTML(`<input type="hidden" name="_csrf" value="` + ctx.Data["CsrfToken"].(string) + `">`)
742742

743743
// FIXME: do we really always need these setting? There should be someway to have to avoid having to always set these

modules/context/csrf.go

Lines changed: 71 additions & 128 deletions
Original file line numberDiff line numberDiff line change
@@ -31,97 +31,55 @@ import (
3131
"code.gitea.io/gitea/modules/web/middleware"
3232
)
3333

34-
// CSRF represents a CSRF service and is used to get the current token and validate a suspect token.
35-
type CSRF interface {
36-
// Return HTTP header to search for token.
34+
// CSRFProtector represents a CSRF protector and is used to get the current token and validate the token.
35+
type CSRFProtector interface {
36+
// GetHeaderName returns HTTP header to search for token.
3737
GetHeaderName() string
38-
// Return form value to search for token.
38+
// GetFormName returns form value to search for token.
3939
GetFormName() string
40-
// Return cookie name to search for token.
41-
GetCookieName() string
42-
// Return cookie path
43-
GetCookiePath() string
44-
// Return the flag value used for the csrf token.
45-
GetCookieHTTPOnly() bool
46-
// Return cookie domain
47-
GetCookieDomain() string
48-
// Return the token.
40+
// GetToken returns the token.
4941
GetToken() string
50-
// Validate by token.
51-
ValidToken(t string) bool
52-
// Error replies to the request with a custom function when ValidToken fails.
53-
Error(w http.ResponseWriter)
42+
// Validate validates the token in http context.
43+
Validate(ctx *Context)
5444
}
5545

56-
type csrf struct {
57-
// Header name value for setting and getting csrf token.
46+
type csrfProtector struct {
47+
// Header name value for setting and getting CSRF token.
5848
Header string
59-
// Form name value for setting and getting csrf token.
49+
// Form name value for setting and getting CSRF token.
6050
Form string
61-
// Cookie name value for setting and getting csrf token.
51+
// Cookie name value for setting and getting CSRF token.
6252
Cookie string
6353
// Cookie domain
6454
CookieDomain string
6555
// Cookie path
6656
CookiePath string
67-
// Cookie HttpOnly flag value used for the csrf token.
57+
// Cookie HttpOnly flag value used for the CSRF token.
6858
CookieHTTPOnly bool
6959
// Token generated to pass via header, cookie, or hidden form value.
7060
Token string
7161
// This value must be unique per user.
7262
ID string
7363
// Secret used along with the unique id above to generate the Token.
7464
Secret string
75-
// ErrorFunc is the custom function that replies to the request when ValidToken fails.
76-
ErrorFunc func(w http.ResponseWriter)
7765
}
7866

79-
// GetHeaderName returns the name of the HTTP header for csrf token.
80-
func (c *csrf) GetHeaderName() string {
67+
// GetHeaderName returns the name of the HTTP header for CSRF token.
68+
func (c *csrfProtector) GetHeaderName() string {
8169
return c.Header
8270
}
8371

84-
// GetFormName returns the name of the form value for csrf token.
85-
func (c *csrf) GetFormName() string {
72+
// GetFormName returns the name of the form value for CSRF token.
73+
func (c *csrfProtector) GetFormName() string {
8674
return c.Form
8775
}
8876

89-
// GetCookieName returns the name of the cookie for csrf token.
90-
func (c *csrf) GetCookieName() string {
91-
return c.Cookie
92-
}
93-
94-
// GetCookiePath returns the path of the cookie for csrf token.
95-
func (c *csrf) GetCookiePath() string {
96-
return c.CookiePath
97-
}
98-
99-
// GetCookieHTTPOnly returns the flag value used for the csrf token.
100-
func (c *csrf) GetCookieHTTPOnly() bool {
101-
return c.CookieHTTPOnly
102-
}
103-
104-
// GetCookieDomain returns the flag value used for the csrf token.
105-
func (c *csrf) GetCookieDomain() string {
106-
return c.CookieDomain
107-
}
108-
10977
// GetToken returns the current token. This is typically used
11078
// to populate a hidden form in an HTML template.
111-
func (c *csrf) GetToken() string {
79+
func (c *csrfProtector) GetToken() string {
11280
return c.Token
11381
}
11482

115-
// ValidToken validates the passed token against the existing Secret and ID.
116-
func (c *csrf) ValidToken(t string) bool {
117-
return ValidToken(t, c.Secret, c.ID, "POST")
118-
}
119-
120-
// Error replies to the request when ValidToken fails.
121-
func (c *csrf) Error(w http.ResponseWriter) {
122-
c.ErrorFunc(w)
123-
}
124-
12583
// CsrfOptions maintains options to manage behavior of Generate.
12684
type CsrfOptions struct {
12785
// The global secret value used to generate Tokens.
@@ -143,73 +101,58 @@ type CsrfOptions struct {
143101
SessionKey string
144102
// oldSessionKey saves old value corresponding to SessionKey.
145103
oldSessionKey string
146-
// If true, send token via X-CSRFToken header.
104+
// If true, send token via X-Csrf-Token header.
147105
SetHeader bool
148106
// If true, send token via _csrf cookie.
149107
SetCookie bool
150108
// Set the Secure flag to true on the cookie.
151109
Secure bool
152110
// Disallow Origin appear in request header.
153111
Origin bool
154-
// The function called when Validate fails.
155-
ErrorFunc func(w http.ResponseWriter)
156-
// Cookie life time. Default is 0
112+
// Cookie lifetime. Default is 0
157113
CookieLifeTime int
158114
}
159115

160-
func prepareOptions(options []CsrfOptions) CsrfOptions {
161-
var opt CsrfOptions
162-
if len(options) > 0 {
163-
opt = options[0]
164-
}
165-
166-
// Defaults.
167-
if len(opt.Secret) == 0 {
116+
func prepareDefaultCsrfOptions(opt CsrfOptions) CsrfOptions {
117+
if opt.Secret == "" {
168118
randBytes, err := util.CryptoRandomBytes(8)
169119
if err != nil {
170120
// this panic can be handled by the recover() in http handlers
171121
panic(fmt.Errorf("failed to generate random bytes: %w", err))
172122
}
173123
opt.Secret = base32.StdEncoding.EncodeToString(randBytes)
174124
}
175-
if len(opt.Header) == 0 {
176-
opt.Header = "X-CSRFToken"
125+
if opt.Header == "" {
126+
opt.Header = "X-Csrf-Token"
177127
}
178-
if len(opt.Form) == 0 {
128+
if opt.Form == "" {
179129
opt.Form = "_csrf"
180130
}
181-
if len(opt.Cookie) == 0 {
131+
if opt.Cookie == "" {
182132
opt.Cookie = "_csrf"
183133
}
184-
if len(opt.CookiePath) == 0 {
134+
if opt.CookiePath == "" {
185135
opt.CookiePath = "/"
186136
}
187-
if len(opt.SessionKey) == 0 {
137+
if opt.SessionKey == "" {
188138
opt.SessionKey = "uid"
189139
}
190140
opt.oldSessionKey = "_old_" + opt.SessionKey
191-
if opt.ErrorFunc == nil {
192-
opt.ErrorFunc = func(w http.ResponseWriter) {
193-
http.Error(w, "Invalid csrf token.", http.StatusBadRequest)
194-
}
195-
}
196-
197141
return opt
198142
}
199143

200-
// Csrfer maps CSRF to each request. If this request is a Get request, it will generate a new token.
144+
// NewCSRFProtector returns a CSRFProtector to be used for every request.
201145
// Additionally, depending on options set, generated tokens will be sent via Header and/or Cookie.
202-
func Csrfer(opt CsrfOptions, ctx *Context) CSRF {
203-
opt = prepareOptions([]CsrfOptions{opt})
204-
x := &csrf{
146+
func NewCSRFProtector(opt CsrfOptions, ctx *Context) CSRFProtector {
147+
opt = prepareDefaultCsrfOptions(opt)
148+
x := &csrfProtector{
205149
Secret: opt.Secret,
206150
Header: opt.Header,
207151
Form: opt.Form,
208152
Cookie: opt.Cookie,
209153
CookieDomain: opt.CookieDomain,
210154
CookiePath: opt.CookiePath,
211155
CookieHTTPOnly: opt.CookieHTTPOnly,
212-
ErrorFunc: opt.ErrorFunc,
213156
}
214157

215158
if opt.Origin && len(ctx.Req.Header.Get("Origin")) > 0 {
@@ -236,17 +179,25 @@ func Csrfer(opt CsrfOptions, ctx *Context) CSRF {
236179
_ = ctx.Session.Set(opt.oldSessionKey, x.ID)
237180
} else {
238181
// If cookie present, map existing token, else generate a new one.
239-
if val := ctx.GetCookie(opt.Cookie); len(val) > 0 {
240-
// FIXME: test coverage.
241-
x.Token = val
182+
if val := ctx.GetCookie(opt.Cookie); val != "" {
183+
x.Token = val // FIXME: test coverage.
242184
} else {
243185
needsNew = true
244186
}
245187
}
246188

189+
if !needsNew {
190+
if issueTime, ok := ParseCsrfToken(x.Token); ok {
191+
dur := time.Since(issueTime)
192+
if dur < -CsrfTokenRegenerationDuration || dur > CsrfTokenRegenerationDuration {
193+
needsNew = true
194+
}
195+
}
196+
}
197+
247198
if needsNew {
248199
// FIXME: actionId.
249-
x.Token = GenerateToken(x.Secret, x.ID, "POST")
200+
x.Token = GenerateCsrfToken(x.Secret, x.ID, "POST", time.Now())
250201
if opt.SetCookie {
251202
var expires interface{}
252203
if opt.CookieLifeTime == 0 {
@@ -270,47 +221,39 @@ func Csrfer(opt CsrfOptions, ctx *Context) CSRF {
270221
return x
271222
}
272223

273-
// Validate should be used as a per route middleware. It attempts to get a token from a "X-CSRFToken"
274-
// HTTP header and then a "_csrf" form value. If one of these is found, the token will be validated
275-
// using ValidToken. If this validation fails, custom Error is sent in the reply.
276-
// If neither a header or form value is found, http.StatusBadRequest is sent.
277-
func Validate(ctx *Context, x CSRF) {
278-
if token := ctx.Req.Header.Get(x.GetHeaderName()); len(token) > 0 {
279-
if !x.ValidToken(token) {
280-
// Delete the cookie
281-
middleware.SetCookie(ctx.Resp, x.GetCookieName(), "",
282-
-1,
283-
x.GetCookiePath(),
284-
x.GetCookieDomain()) // FIXME: Do we need to set the Secure, httpOnly and SameSite values too?
285-
if middleware.IsAPIPath(ctx.Req) {
286-
x.Error(ctx.Resp)
287-
return
288-
}
224+
func (c *csrfProtector) validateToken(ctx *Context, token string) bool {
225+
if !ValidCsrfToken(token, c.Secret, c.ID, "POST", time.Now()) {
226+
// Delete the cookie
227+
middleware.SetCookie(ctx.Resp, c.Cookie, "",
228+
-1,
229+
c.CookiePath,
230+
c.CookieDomain) // FIXME: Do we need to set the Secure, httpOnly and SameSite values too?
231+
232+
if middleware.IsAPIPath(ctx.Req) {
233+
http.Error(ctx.Resp, "Invalid CSRF token.", http.StatusBadRequest)
234+
} else {
289235
ctx.Flash.Error(ctx.Tr("error.invalid_csrf"))
290236
ctx.Redirect(setting.AppSubURL + "/")
291237
}
292-
return
238+
return false
293239
}
294-
if token := ctx.Req.FormValue(x.GetFormName()); len(token) > 0 {
295-
if !x.ValidToken(token) {
296-
// Delete the cookie
297-
middleware.SetCookie(ctx.Resp, x.GetCookieName(), "",
298-
-1,
299-
x.GetCookiePath(),
300-
x.GetCookieDomain()) // FIXME: Do we need to set the Secure, httpOnly and SameSite values too?
301-
if middleware.IsAPIPath(ctx.Req) {
302-
x.Error(ctx.Resp)
303-
return
304-
}
305-
ctx.Flash.Error(ctx.Tr("error.invalid_csrf"))
306-
ctx.Redirect(setting.AppSubURL + "/")
240+
return true
241+
}
242+
243+
// Validate should be used as a per route middleware. It attempts to get a token from an "X-Csrf-Token"
244+
// HTTP header and then a "_csrf" form value. If one of these is found, the token will be validated
245+
// using ValidToken. If this validation fails, custom Error is sent in the reply.
246+
// If neither a header nor form value is found, http.StatusBadRequest is sent.
247+
func (c *csrfProtector) Validate(ctx *Context) {
248+
if token := ctx.Req.Header.Get(c.GetHeaderName()); token != "" {
249+
if c.validateToken(ctx, token) {
250+
return
307251
}
308-
return
309252
}
310-
if middleware.IsAPIPath(ctx.Req) {
311-
http.Error(ctx.Resp, "Bad Request: no CSRF token present", http.StatusBadRequest)
312-
return
253+
if token := ctx.Req.FormValue(c.GetFormName()); token != "" {
254+
if c.validateToken(ctx, token) {
255+
return
256+
}
313257
}
314-
ctx.Flash.Error(ctx.Tr("error.missing_csrf"))
315-
ctx.Redirect(setting.AppSubURL + "/")
258+
c.validateToken(ctx, "") // no csrf token, use an empty token to respond error
316259
}

0 commit comments

Comments
 (0)