Don't allow in use CSRF tokens to expire (fixes #1008)

This commit is contained in:
Jakob Borg 2016-01-03 21:56:19 +01:00
parent adb1227b2e
commit 6e1d364d60
2 changed files with 68 additions and 5 deletions

View File

@ -17,9 +17,16 @@ import (
"github.com/syncthing/syncthing/lib/sync" "github.com/syncthing/syncthing/lib/sync"
) )
// csrfTokens is a list of valid tokens. It is sorted so that the most
// recently used token is first in the list. New tokens are added to the front
// of the list (as it is the most recently used at that time). The list is
// pruned to a maximum of maxCsrfTokens, throwing away the least recently used
// tokens.
var csrfTokens []string var csrfTokens []string
var csrfMut = sync.NewMutex() var csrfMut = sync.NewMutex()
const maxCsrfTokens = 25
// Check for CSRF token on /rest/ URLs. If a correct one is not given, reject // Check for CSRF token on /rest/ URLs. If a correct one is not given, reject
// the request with 403. For / and /index.html, set a new CSRF cookie if none // the request with 403. For / and /index.html, set a new CSRF cookie if none
// is currently set. // is currently set.
@ -36,6 +43,7 @@ func csrfMiddleware(unique, prefix, apiKey string, next http.Handler) http.Handl
if !strings.HasPrefix(r.URL.Path, prefix) { if !strings.HasPrefix(r.URL.Path, prefix) {
cookie, err := r.Cookie("CSRF-Token-" + unique) cookie, err := r.Cookie("CSRF-Token-" + unique)
if err != nil || !validCsrfToken(cookie.Value) { if err != nil || !validCsrfToken(cookie.Value) {
httpl.Debugln("new CSRF cookie in response to request for", r.URL)
cookie = &http.Cookie{ cookie = &http.Cookie{
Name: "CSRF-Token-" + unique, Name: "CSRF-Token-" + unique,
Value: newCsrfToken(), Value: newCsrfToken(),
@ -47,7 +55,13 @@ func csrfMiddleware(unique, prefix, apiKey string, next http.Handler) http.Handl
} }
if r.Method == "GET" { if r.Method == "GET" {
// Allow GET requests unconditionally // Allow GET requests unconditionally, but if we got the CSRF
// token cookie do the verification anyway so we keep the
// csrfTokens list sorted by recent usage. We don't care about the
// outcome of the validity check.
if cookie, err := r.Cookie("CSRF-Token-" + unique); err == nil {
validCsrfToken(cookie.Value)
}
next.ServeHTTP(w, r) next.ServeHTTP(w, r)
return return
} }
@ -66,8 +80,15 @@ func csrfMiddleware(unique, prefix, apiKey string, next http.Handler) http.Handl
func validCsrfToken(token string) bool { func validCsrfToken(token string) bool {
csrfMut.Lock() csrfMut.Lock()
defer csrfMut.Unlock() defer csrfMut.Unlock()
for _, t := range csrfTokens { for i, t := range csrfTokens {
if t == token { if t == token {
if i > 0 {
// Move this token to the head of the list. Copy the tokens at
// the front one step to the right and then replace the token
// at the head.
copy(csrfTokens[1:], csrfTokens[:i+1])
csrfTokens[0] = token
}
return true return true
} }
} }
@ -78,9 +99,9 @@ func newCsrfToken() string {
token := randomString(32) token := randomString(32)
csrfMut.Lock() csrfMut.Lock()
csrfTokens = append(csrfTokens, token) csrfTokens = append([]string{token}, csrfTokens...)
if len(csrfTokens) > 10 { if len(csrfTokens) > maxCsrfTokens {
csrfTokens = csrfTokens[len(csrfTokens)-10:] csrfTokens = csrfTokens[:maxCsrfTokens]
} }
defer csrfMut.Unlock() defer csrfMut.Unlock()

42
cmd/syncthing/gui_test.go Normal file
View File

@ -0,0 +1,42 @@
// Copyright (C) 2016 The Syncthing Authors.
//
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this file,
// You can obtain one at http://mozilla.org/MPL/2.0/.
package main
import "testing"
func TestCSRFToken(t *testing.T) {
t1 := newCsrfToken()
t2 := newCsrfToken()
t3 := newCsrfToken()
if !validCsrfToken(t3) {
t.Fatal("t3 should be valid")
}
for i := 0; i < 250; i++ {
if i%5 == 0 {
// t1 and t2 should remain valid by virtue of us checking them now
// and then.
if !validCsrfToken(t1) {
t.Fatal("t1 should be valid at iteration", i)
}
if !validCsrfToken(t2) {
t.Fatal("t2 should be valid at iteration", i)
}
}
// The newly generated token is always valid
t4 := newCsrfToken()
if !validCsrfToken(t4) {
t.Fatal("t4 should be valid at iteration", i)
}
}
if validCsrfToken(t3) {
t.Fatal("t3 should have expired by now")
}
}