Skip to content
Merged

Csrf #1003

Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions auth/authentication.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package auth

import (
"errors"
"net/http"
"strings"
"time"

Expand Down Expand Up @@ -40,6 +41,7 @@ type Database interface {
type Auth struct {
DB Database
SecureCookie bool
CrossOrigin *http.CrossOriginProtection
}

// RequireAdmin requires an elevated client token or basic auth, the user must be an admin.
Expand Down Expand Up @@ -87,6 +89,9 @@ func (a *Auth) Optional(ctx *gin.Context) {
}

func (a *Auth) evaluate(ctx *gin.Context, funcs ...func(ctx *gin.Context) (authState, error)) bool {
if a.rejectForeignOrigin(ctx) {
return true
}
for _, fn := range funcs {
state, err := fn(ctx)
if err != nil {
Expand Down Expand Up @@ -128,6 +133,17 @@ func (a *Auth) abort403(ctx *gin.Context) {
ctx.AbortWithError(403, errors.New("you are not allowed to access this api"))
}

func (a *Auth) rejectForeignOrigin(ctx *gin.Context) bool {
if _, isCookie := a.readTokenFromRequest(ctx); !isCookie {
return false
}
if err := a.CrossOrigin.Check(ctx.Request); err != nil {
ctx.AbortWithError(403, err)
return true
}
return false
}

func (a *Auth) handleUser(checks ...func(*model.User) (authState, error)) func(ctx *gin.Context) (authState, error) {
return func(ctx *gin.Context) (authState, error) {
if name, pass, ok := ctx.Request.BasicAuth(); ok {
Expand Down
63 changes: 62 additions & 1 deletion auth/authentication_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ type AuthenticationSuite struct {
func (s *AuthenticationSuite) SetupSuite() {
mode.Set(mode.TestDev)
s.DB = testdb.NewDB(s.T())
s.auth = &Auth{DB: s.DB}
s.auth = &Auth{DB: s.DB, CrossOrigin: http.NewCrossOriginProtection()}

now := time.Date(2025, 1, 1, 12, 0, 0, 0, time.UTC)
timeNow = func() time.Time { return now }
Expand Down Expand Up @@ -354,4 +354,65 @@ func (s *AuthenticationSuite) assertHeaderRequest(key, value string, f fMiddlewa
return ctx
}

func (s *AuthenticationSuite) TestCookieCrossOriginProtection() {
// httptest sets the request host to example.com.
s.assertCsrfRequest(map[string]string{"Origin": "http://example.com"}, "clienttoken", s.auth.RequireClient, 200)
s.assertCsrfRequest(map[string]string{"Origin": "https://example.com"}, "clienttoken", s.auth.RequireClient, 200)

s.assertCsrfRequest(map[string]string{"Origin": "http://evil.com"}, "clienttoken", s.auth.RequireClient, 403)
s.assertCsrfRequest(map[string]string{"Origin": "https://example.com.evil.com"}, "clienttoken", s.auth.RequireClient, 403)
s.assertCsrfRequest(map[string]string{"Origin": "null"}, "clienttoken", s.auth.RequireClient, 403)

s.assertCsrfRequest(nil, "clienttoken", s.auth.RequireClient, 200)

s.assertCsrfRequest(map[string]string{"Sec-Fetch-Site": "same-origin"}, "clienttoken", s.auth.RequireClient, 200)
s.assertCsrfRequest(map[string]string{"Sec-Fetch-Site": "none"}, "clienttoken", s.auth.RequireClient, 200)
s.assertCsrfRequest(map[string]string{"Sec-Fetch-Site": "cross-site"}, "clienttoken", s.auth.RequireClient, 403)
s.assertCsrfRequest(map[string]string{"Sec-Fetch-Site": "same-site"}, "clienttoken", s.auth.RequireClient, 403)

s.assertCsrfRequest(map[string]string{"Sec-Fetch-Site": "cross-site"}, "clienttoken_admin_elevated", s.auth.RequireElevatedClient, 403)
s.assertCsrfRequest(map[string]string{"Sec-Fetch-Site": "same-origin"}, "clienttoken_admin_elevated", s.auth.RequireElevatedClient, 200)
}

func (s *AuthenticationSuite) TestCrossOriginProtectionIgnoredForTokenAuth() {
recorder := httptest.NewRecorder()
ctx, _ := gin.CreateTestContext(recorder)
ctx.Request = httptest.NewRequest("POST", "/", nil)
ctx.Request.Header.Set("X-Gotify-Key", "clienttoken")
ctx.Request.Header.Set("Sec-Fetch-Site", "cross-site")
s.auth.RequireClient(ctx)
assert.Equal(s.T(), 200, recorder.Code)

recorder = httptest.NewRecorder()
ctx, _ = gin.CreateTestContext(recorder)
ctx.Request = httptest.NewRequest("POST", "/?token=clienttoken", nil)
ctx.Request.Header.Set("Sec-Fetch-Site", "cross-site")
s.auth.RequireClient(ctx)
assert.Equal(s.T(), 200, recorder.Code)
}

func (s *AuthenticationSuite) TestCrossOriginProtectionAllowsSafeMethods() {
recorder := httptest.NewRecorder()
ctx, _ := gin.CreateTestContext(recorder)
ctx.Request = httptest.NewRequest("GET", "/", nil)
ctx.Request.AddCookie(&http.Cookie{Name: cookieName, Value: "clienttoken"})
ctx.Request.Header.Set("Sec-Fetch-Site", "cross-site")
s.auth.RequireClient(ctx)
assert.Equal(s.T(), 200, recorder.Code)
}

func (s *AuthenticationSuite) assertCsrfRequest(headers map[string]string, cookie string, f fMiddleware, code int) {
recorder := httptest.NewRecorder()
ctx, _ := gin.CreateTestContext(recorder)
ctx.Request = httptest.NewRequest("POST", "/", nil)
if cookie != "" {
ctx.Request.AddCookie(&http.Cookie{Name: cookieName, Value: cookie})
}
for k, v := range headers {
ctx.Request.Header.Set(k, v)
}
f(ctx)
assert.Equal(s.T(), code, recorder.Code)
}

type fMiddleware gin.HandlerFunc
4 changes: 2 additions & 2 deletions model/message.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ type MessageExternal struct {
// read only: true
// required: true
// example: 5
ApplicationID uint `json:"appid"`
ApplicationID uint `form:"appid" query:"appid" json:"appid"`
// The message. Markdown (excluding html) is allowed.
//
// required: true
Expand Down Expand Up @@ -74,7 +74,7 @@ type CreateMessage struct {
// The application id that send this message. Always set when returned via the API.
//
// example: 5
ApplicationID uint `json:"appid"`
ApplicationID uint `form:"appid" query:"appid" json:"appid"`
// The message. Markdown (excluding html) is allowed.
//
// required: true
Expand Down
6 changes: 5 additions & 1 deletion router/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,11 @@ func Create(db *database.GormDatabase, vInfo *model.VersionInfo, conf *config.Co
}
}
}()
authentication := auth.Auth{DB: db, SecureCookie: conf.Server.SecureCookie}
authentication := auth.Auth{
DB: db,
SecureCookie: conf.Server.SecureCookie,
CrossOrigin: http.NewCrossOriginProtection(),
}
messageHandler := api.MessageAPI{Notifier: streamHandler, DB: db}
healthHandler := api.HealthAPI{DB: db}
clientHandler := api.ClientAPI{
Expand Down
Loading