MM-55320 - Limit length of browser user agent version; ratelimit the /sessions endpoint (#25900)

* add ratelimit to /sessions; cap userAgent version length; tests

* add MaxSessionsLimit; remove oldest session first; tests

* can't use slices in 1.20; improve test

* nits

* add GetLRUSessions; move limiting to CreateSession; remove rate limiting

* use queryBuilder

* mysql needs a limit when using offset

* update i18n

* refactor into limitNumberOfSessions; protect createSessionForUserAccessToken

* add comment to GetLRUSessions

* add limit to oauth path; PR comments
Этот коммит содержится в:
Christopher Poile
2024-03-21 08:48:24 -04:00
коммит произвёл GitHub
родитель 9a2d96073e
Коммит 17d11db395
16 изменённых файлов: 321 добавлений и 4 удалений

Просмотреть файл

@@ -192,6 +192,9 @@ type AppIface interface {
// relationship with a user. That means any user sharing any channel, including
// direct and group channels.
GetKnownUsers(userID string) ([]string, *model.AppError)
// GetLRUSessions returns the Least Recently Used sessions for userID, skipping over the newest 'offset'
// number of sessions. E.g., if userID has 100 sessions, offset 98 will return the oldest 2 sessions.
GetLRUSessions(c request.CTX, userID string, limit uint64, offset uint64) ([]*model.Session, *model.AppError)
// GetLastAccessibleFileTime returns CreateAt time(from cache) of the last accessible post as per the cloud limit
GetLastAccessibleFileTime() (int64, *model.AppError)
// GetLastAccessiblePostTime returns CreateAt time(from cache) of the last accessible post as per the cloud limit

Просмотреть файл

@@ -383,6 +383,11 @@ func (a *App) GetOAuthAccessTokenForCodeFlow(c request.CTX, clientId, grantType,
}
func (a *App) newSession(c request.CTX, app *model.OAuthApp, user *model.User) (*model.Session, *model.AppError) {
if err := a.limitNumberOfSessions(c, user.Id); err != nil {
return nil, model.NewAppError("newSession", "api.oauth.get_access_token.internal_session.app_error", nil,
"", http.StatusInternalServerError).Wrap(err)
}
// Set new token an session
session := &model.Session{UserId: user.Id, Roles: user.Roles, IsOAuth: true}
session.GenerateCSRF()

Просмотреть файл

@@ -7308,6 +7308,28 @@ func (a *OpenTracingAppLayer) GetKnownUsers(userID string) ([]string, *model.App
return resultVar0, resultVar1
}
func (a *OpenTracingAppLayer) GetLRUSessions(c request.CTX, userID string, limit uint64, offset uint64) ([]*model.Session, *model.AppError) {
origCtx := a.ctx
span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetLRUSessions")
a.ctx = newCtx
a.app.Srv().Store().SetContext(newCtx)
defer func() {
a.app.Srv().Store().SetContext(origCtx)
a.ctx = origCtx
}()
defer span.Finish()
resultVar0, resultVar1 := a.app.GetLRUSessions(c, userID, limit, offset)
if resultVar1 != nil {
span.LogFields(spanlog.Error(resultVar1))
ext.Error.Set(span, true)
}
return resultVar0, resultVar1
}
func (a *OpenTracingAppLayer) GetLastAccessibleFileTime() (int64, *model.AppError) {
origCtx := a.ctx
span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetLastAccessibleFileTime")

Просмотреть файл

@@ -41,6 +41,10 @@ func (ps *PlatformService) GetSessions(c request.CTX, userID string) ([]*model.S
return ps.Store.Session().GetSessions(c, userID)
}
func (ps *PlatformService) GetLRUSessions(c request.CTX, userID string, limit uint64, offset uint64) ([]*model.Session, error) {
return ps.Store.Session().GetLRUSessions(c, userID, limit, offset)
}
func (ps *PlatformService) AddSessionToCache(session *model.Session) {
ps.sessionCache.SetWithExpiry(session.Token, session, time.Duration(int64(*ps.Config().ServiceSettings.SessionCacheInMinutes))*time.Minute)
}

Просмотреть файл

@@ -18,7 +18,14 @@ import (
"github.com/mattermost/mattermost/server/v8/channels/store"
)
// maxSessionsLimit prevents a potential DOS caused by creating an unbounded number of sessions; MM-55320
const maxSessionsLimit = 500
func (a *App) CreateSession(c request.CTX, session *model.Session) (*model.Session, *model.AppError) {
if appErr := a.limitNumberOfSessions(c, session.UserId); appErr != nil {
return nil, appErr
}
session, err := a.ch.srv.platform.CreateSession(c, session)
if err != nil {
var invErr *store.ErrInvalidInput
@@ -136,6 +143,40 @@ func (a *App) GetSessions(c request.CTX, userID string) ([]*model.Session, *mode
return sessions, nil
}
// limitNumberOfSessions revokes userId's least recently used sessions to keep the number below
// maxSessionsLimit; MM-55320
func (a *App) limitNumberOfSessions(c request.CTX, userId string) *model.AppError {
const returnLimit = 100
sessions, appErr := a.GetLRUSessions(c, userId, returnLimit, maxSessionsLimit-1)
if appErr != nil {
return model.NewAppError("limitNumberOfSessions", "app.session.save.app_error", nil, "", http.StatusInternalServerError).Wrap(appErr)
}
// Revoke any sessions over the limit to make room for new sessions
for _, sess := range sessions {
if err := a.RevokeSession(c, sess); err != nil {
return model.NewAppError("limitNumberOfSessions", "app.session.save.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
}
c.Logger().Debug("Session revoked; user's number of sessions were over the maxSessionsLimit",
mlog.String("user_id", userId),
mlog.String("session_id", sess.Id))
}
return nil
}
// GetLRUSessions returns the Least Recently Used sessions for userID, skipping over the newest 'offset'
// number of sessions. E.g., if userID has 100 sessions, offset 98 will return the oldest 2 sessions.
func (a *App) GetLRUSessions(c request.CTX, userID string, limit uint64, offset uint64) ([]*model.Session, *model.AppError) {
sessions, err := a.ch.srv.platform.GetLRUSessions(c, userID, limit, offset)
if err != nil {
return nil, model.NewAppError("GetLRUSessions", "app.session.get_lru_sessions.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
}
return sessions, nil
}
func (a *App) RevokeAllSessions(c request.CTX, userID string) *model.AppError {
if err := a.ch.srv.platform.RevokeAllSessions(c, userID); err != nil {
switch {
@@ -384,6 +425,10 @@ func (a *App) createSessionForUserAccessToken(c request.CTX, tokenString string)
return nil, model.NewAppError("createSessionForUserAccessToken", "app.user_access_token.invalid_or_missing", nil, "inactive_user_id="+user.Id, http.StatusUnauthorized)
}
if appErr := a.limitNumberOfSessions(c, user.Id); appErr != nil {
return nil, appErr
}
session := &model.Session{
Token: token.Token,
UserId: user.Id,

Просмотреть файл

@@ -5,8 +5,11 @@ package app
import (
"fmt"
"net/http"
"net/http/httptest"
"os"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
@@ -395,3 +398,57 @@ func TestGetRemoteClusterSession(t *testing.T) {
require.Nil(t, session)
})
}
func TestSessionsLimit(t *testing.T) {
th := Setup(t).InitBasic()
defer th.TearDown()
user := th.BasicUser
var sessions []*model.Session
r := &http.Request{}
w := httptest.NewRecorder()
for i := 0; i < maxSessionsLimit; i++ {
session, err := th.App.DoLogin(th.Context, w, r, th.BasicUser, "", false, false, false)
require.Nil(t, err)
sessions = append(sessions, session)
time.Sleep(1 * time.Millisecond)
}
gotSessions, _ := th.App.GetSessions(th.Context, user.Id)
require.Equal(t, maxSessionsLimit, len(gotSessions), "should have maxSessionsLimit number of sessions")
// Ensure we are retrieving the same sessions.
reverse(gotSessions)
for i, sess := range gotSessions {
require.Equal(t, sessions[i].Id, sess.Id)
}
// Now add 10 more.
for i := 0; i < 10; i++ {
session, err := th.App.DoLogin(th.Context, w, r, th.BasicUser, "", false, false, false)
require.Nil(t, err, "should not have an error creating user sessions")
// Remove oldest, append newest.
sessions = sessions[1:]
sessions = append(sessions, session)
time.Sleep(1 * time.Millisecond)
}
// Ensure that we still only have the max allowed.
gotSessions, _ = th.App.GetSessions(th.Context, user.Id)
require.Equal(t, maxSessionsLimit, len(gotSessions), "should have maxSessionsLimit number of sessions")
// Ensure the the oldest sessions were removed first.
reverse(gotSessions)
for i, sess := range gotSessions {
require.Equal(t, sessions[i].Id, sess.Id)
}
}
// reverse can be replaced by the slices version when we move to 1.21+
func reverse[S ~[]E, E any](s S) {
for i, j := 0, len(s)-1; i < j; i, j = i+1, j-1 {
s[i], s[j] = s[j], s[i]
}
}

Просмотреть файл

@@ -10,6 +10,8 @@ import (
"github.com/avct/uasurfer"
)
const maxUserAgentVersionLength = 128
var platformNames = map[uasurfer.Platform]string{
uasurfer.PlatformUnknown: "Windows",
uasurfer.PlatformWindows: "Windows",
@@ -86,27 +88,33 @@ func getOSName(ua *uasurfer.UserAgent) string {
func getBrowserVersion(ua *uasurfer.UserAgent, userAgentString string) string {
if index := strings.Index(userAgentString, "Mattermost Mobile/"); index != -1 {
afterVersion := userAgentString[index+len("Mattermost Mobile/"):]
return strings.Fields(afterVersion)[0]
// MM-55320: limitStringLength prevents potential DOS caused by filling an unbounded string with junk data
return limitStringLength(strings.Fields(afterVersion)[0], maxUserAgentVersionLength)
}
if index := strings.Index(userAgentString, "Mattermost/"); index != -1 {
afterVersion := userAgentString[index+len("Mattermost/"):]
return strings.Fields(afterVersion)[0]
return limitStringLength(strings.Fields(afterVersion)[0], maxUserAgentVersionLength)
}
if index := strings.Index(userAgentString, "mmctl/"); index != -1 {
afterVersion := userAgentString[index+len("mmctl/"):]
return strings.Fields(afterVersion)[0]
return limitStringLength(strings.Fields(afterVersion)[0], maxUserAgentVersionLength)
}
if index := strings.Index(userAgentString, "Franz/"); index != -1 {
afterVersion := userAgentString[index+len("Franz/"):]
return strings.Fields(afterVersion)[0]
return limitStringLength(strings.Fields(afterVersion)[0], maxUserAgentVersionLength)
}
return getUAVersion(ua.Browser.Version)
}
func limitStringLength(field string, limit int) string {
endPos := min(len(field), limit)
return field[:endPos]
}
func getUAVersion(version uasurfer.Version) string {
if version.Patch == 0 {
return fmt.Sprintf("%v.%v", version.Major, version.Minor)
@@ -151,3 +159,11 @@ func getBrowserName(ua *uasurfer.UserAgent, userAgentString string) string {
return browserNames[uasurfer.BrowserUnknown]
}
// min should be replaced by to go 1.21 built-in generic function, see MM-57356.
func min(a, b int) int {
if a < b {
return a
}
return b
}

Просмотреть файл

@@ -34,6 +34,7 @@ var testUserAgents = []testUserAgent{
{"Safari 8", "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_4) AppleWebKit/600.7.12 (KHTML, like Gecko) Version/8.0.7 Safari/600.7.12"},
{"Safari Mobile", "Mozilla/5.0 (iPhone; CPU iPhone OS 9_1 like Mac OS X) AppleWebKit/601.1.46 (KHTML, like Gecko) Version/9.0 Mobile/13B137 Safari/601.1"},
{"Mobile App", "Mattermost Mobile/2.7.0+482 (Android; 13; sdk_gphone64_arm64)"},
{"Mobile App", "Mattermost Mobile/233.234441.341234223421341234529099823109834440981234+abcdef3214eafeabc3242331129857301afesfffff1930a84e4bd2348fe129ac1309bd929dca3419af934bfe3089fcd (Android; 13; sdk_gphone64_arm64)"},
}
func TestGetPlatformName(t *testing.T) {
@@ -55,6 +56,7 @@ func TestGetPlatformName(t *testing.T) {
"Macintosh",
"iPhone",
"Linux",
"Linux",
}
for i, userAgent := range testUserAgents {
@@ -86,6 +88,7 @@ func TestGetOSName(t *testing.T) {
"Mac OS",
"iOS",
"Android",
"Android",
}
for i, userAgent := range testUserAgents {
@@ -117,6 +120,7 @@ func TestGetBrowserName(t *testing.T) {
"Safari",
"Safari",
"Mobile App",
"Mobile App",
}
for i, userAgent := range testUserAgents {
@@ -148,6 +152,7 @@ func TestGetBrowserVersion(t *testing.T) {
"8.0.7",
"9.0",
"2.7.0+482",
"233.234441.341234223421341234529099823109834440981234+abcdef3214eafeabc3242331129857301afesfffff1930a84e4bd2348fe129ac1309bd929d", // cut off at len 128
}
for i, userAgent := range testUserAgents {

Просмотреть файл

@@ -8648,6 +8648,24 @@ func (s *OpenTracingLayerSessionStore) Get(c request.CTX, sessionIDOrToken strin
return result, err
}
func (s *OpenTracingLayerSessionStore) GetLRUSessions(c request.CTX, userID string, limit uint64, offset uint64) ([]*model.Session, error) {
origCtx := s.Root.Store.Context()
span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "SessionStore.GetLRUSessions")
s.Root.Store.SetContext(newCtx)
defer func() {
s.Root.Store.SetContext(origCtx)
}()
defer span.Finish()
result, err := s.SessionStore.GetLRUSessions(c, userID, limit, offset)
if err != nil {
span.LogFields(spanlog.Error(err))
ext.Error.Set(span, true)
}
return result, err
}
func (s *OpenTracingLayerSessionStore) GetSessions(c request.CTX, userID string) ([]*model.Session, error) {
origCtx := s.Root.Store.Context()
span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "SessionStore.GetSessions")

Просмотреть файл

@@ -9864,6 +9864,27 @@ func (s *RetryLayerSessionStore) Get(c request.CTX, sessionIDOrToken string) (*m
}
func (s *RetryLayerSessionStore) GetLRUSessions(c request.CTX, userID string, limit uint64, offset uint64) ([]*model.Session, error) {
tries := 0
for {
result, err := s.SessionStore.GetLRUSessions(c, userID, limit, offset)
if err == nil {
return result, nil
}
if !isRepeatableError(err) {
return result, err
}
tries++
if tries >= 3 {
err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures")
return result, err
}
timepkg.Sleep(100 * timepkg.Millisecond)
}
}
func (s *RetryLayerSessionStore) GetSessions(c request.CTX, userID string) ([]*model.Session, error) {
tries := 0

Просмотреть файл

@@ -123,6 +123,28 @@ func (me SqlSessionStore) GetSessions(c request.CTX, userId string) ([]*model.Se
return sessions, nil
}
// GetLRUSessions gets the Least Recently Used sessions from the store. Note: the use of limit and offset
// are intentional; they are hardcoded from the app layer (i.e., will not result in a non-performant query).
func (me SqlSessionStore) GetLRUSessions(c request.CTX, userId string, limit uint64, offset uint64) ([]*model.Session, error) {
builder := me.getQueryBuilder().
Select("*").
From("Sessions").
Where(sq.Eq{"UserId": userId}).
OrderBy("LastActivityAt DESC").
Limit(limit).
Offset(offset)
query, args, err := builder.ToSql()
if err != nil {
return nil, errors.Wrap(err, "get_lru_sessions_tosql")
}
var sessions []*model.Session
if err := me.GetReplicaX().Select(&sessions, query, args...); err != nil {
return nil, errors.Wrapf(err, "failed to find Sessions with userId=%s", userId)
}
return sessions, nil
}
func (me SqlSessionStore) GetSessionsWithActiveDeviceIds(userId string) ([]*model.Session, error) {
query :=
`SELECT *

Просмотреть файл

@@ -497,6 +497,7 @@ type SessionStore interface {
Get(c request.CTX, sessionIDOrToken string) (*model.Session, error)
Save(c request.CTX, session *model.Session) (*model.Session, error)
GetSessions(c request.CTX, userID string) ([]*model.Session, error)
GetLRUSessions(c request.CTX, userID string, limit uint64, offset uint64) ([]*model.Session, error)
GetSessionsWithActiveDeviceIds(userID string) ([]*model.Session, error)
GetSessionsExpired(thresholdMillis int64, mobileOnly bool, unnotifiedOnly bool) ([]*model.Session, error)
UpdateExpiredNotify(sessionid string, notified bool) error

Просмотреть файл

@@ -79,6 +79,32 @@ func (_m *SessionStore) Get(c request.CTX, sessionIDOrToken string) (*model.Sess
return r0, r1
}
// GetLRUSessions provides a mock function with given fields: c, userID, limit, offset
func (_m *SessionStore) GetLRUSessions(c request.CTX, userID string, limit uint64, offset uint64) ([]*model.Session, error) {
ret := _m.Called(c, userID, limit, offset)
var r0 []*model.Session
var r1 error
if rf, ok := ret.Get(0).(func(request.CTX, string, uint64, uint64) ([]*model.Session, error)); ok {
return rf(c, userID, limit, offset)
}
if rf, ok := ret.Get(0).(func(request.CTX, string, uint64, uint64) []*model.Session); ok {
r0 = rf(c, userID, limit, offset)
} else {
if ret.Get(0) != nil {
r0 = ret.Get(0).([]*model.Session)
}
}
if rf, ok := ret.Get(1).(func(request.CTX, string, uint64, uint64) error); ok {
r1 = rf(c, userID, limit, offset)
} else {
r1 = ret.Error(1)
}
return r0, r1
}
// GetSessions provides a mock function with given fields: c, userID
func (_m *SessionStore) GetSessions(c request.CTX, userID string) ([]*model.Session, error) {
ret := _m.Called(c, userID)

Просмотреть файл

@@ -5,6 +5,7 @@ package storetest
import (
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
@@ -36,6 +37,7 @@ func TestSessionStore(t *testing.T, rctx request.CTX, ss store.Store) {
t.Run("SessionCount", func(t *testing.T) { testSessionCount(t, rctx, ss) })
t.Run("GetSessionsExpired", func(t *testing.T) { testGetSessionsExpired(t, rctx, ss) })
t.Run("UpdateExpiredNotify", func(t *testing.T) { testUpdateExpiredNotify(t, rctx, ss) })
t.Run("GetLRUSessions", func(t *testing.T) { testGetLRUSessions(t, rctx, ss) })
}
func testSessionStoreSave(t *testing.T, rctx request.CTX, ss store.Store) {
@@ -404,3 +406,53 @@ func testUpdateExpiredNotify(t *testing.T, rctx request.CTX, ss store.Store) {
require.NoError(t, err)
require.False(t, session.ExpiredNotify)
}
func testGetLRUSessions(t *testing.T, rctx request.CTX, ss store.Store) {
userId := model.NewId()
// Clear existing sessions.
err := ss.Session().RemoveAllSessions()
require.NoError(t, err)
s1 := &model.Session{}
s1.UserId = userId
s1.DeviceId = model.NewId()
_, err = ss.Session().Save(rctx, s1)
require.NoError(t, err)
time.Sleep(1 * time.Millisecond)
s2 := &model.Session{}
s2.UserId = userId
s2.DeviceId = model.NewId()
s2, err = ss.Session().Save(rctx, s2)
require.NoError(t, err)
time.Sleep(1 * time.Millisecond)
s3 := &model.Session{}
s3.UserId = userId
s3.DeviceId = model.NewId()
s3, err = ss.Session().Save(rctx, s3)
require.NoError(t, err)
sessions, err := ss.Session().GetLRUSessions(rctx, userId, 3, 3)
require.NoError(t, err)
require.Len(t, sessions, 0)
sessions, err = ss.Session().GetLRUSessions(rctx, userId, 3, 2)
require.NoError(t, err)
require.Len(t, sessions, 1)
require.Equal(t, s1.Id, sessions[0].Id)
sessions, err = ss.Session().GetLRUSessions(rctx, userId, 3, 1)
require.NoError(t, err)
require.Len(t, sessions, 2)
require.Equal(t, s2.Id, sessions[0].Id)
require.Equal(t, s1.Id, sessions[1].Id)
sessions, err = ss.Session().GetLRUSessions(rctx, userId, 3, 0)
require.NoError(t, err)
require.Len(t, sessions, 3)
require.Equal(t, s3.Id, sessions[0].Id)
require.Equal(t, s2.Id, sessions[1].Id)
require.Equal(t, s1.Id, sessions[2].Id)
}

Просмотреть файл

@@ -7795,6 +7795,22 @@ func (s *TimerLayerSessionStore) Get(c request.CTX, sessionIDOrToken string) (*m
return result, err
}
func (s *TimerLayerSessionStore) GetLRUSessions(c request.CTX, userID string, limit uint64, offset uint64) ([]*model.Session, error) {
start := time.Now()
result, err := s.SessionStore.GetLRUSessions(c, userID, limit, offset)
elapsed := float64(time.Since(start)) / float64(time.Second)
if s.Root.Metrics != nil {
success := "false"
if err == nil {
success = "true"
}
s.Root.Metrics.ObserveStoreMethodDuration("SessionStore.GetLRUSessions", success, elapsed)
}
return result, err
}
func (s *TimerLayerSessionStore) GetSessions(c request.CTX, userID string) ([]*model.Session, error) {
start := time.Now()

Просмотреть файл

@@ -6690,6 +6690,10 @@
"id": "app.session.get.app_error",
"translation": "We encountered an error finding the session."
},
{
"id": "app.session.get_lru_sessions.app_error",
"translation": "Unable to get least recently used sessions."
},
{
"id": "app.session.get_sessions.app_error",
"translation": "We encountered an error while finding user sessions."