From 7681ae93e8cf273bf761a8acabf6178405c1e032 Mon Sep 17 00:00:00 2001 From: Tak Wai Wong Date: Wed, 10 Aug 2022 18:29:52 -0700 Subject: [PATCH] Fix user_interactive multi-threaded state update crash --- clientapi/auth/user_interactive.go | 36 +++++++++++++++++++++++++----- 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/clientapi/auth/user_interactive.go b/clientapi/auth/user_interactive.go index a6ee47454..9dad49a39 100644 --- a/clientapi/auth/user_interactive.go +++ b/clientapi/auth/user_interactive.go @@ -18,6 +18,7 @@ import ( "context" "encoding/json" "net/http" + "sync" "github.com/matrix-org/dendrite/clientapi/jsonerror" "github.com/matrix-org/dendrite/internal/mapsutil" @@ -103,6 +104,7 @@ type userInteractiveFlow struct { // the user already has a valid access token, but we want to double-check // that it isn't stolen by re-authenticating them. type UserInteractive struct { + sync.RWMutex Flows []userInteractiveFlow // Map of login type to implementation Types map[string]Type @@ -144,6 +146,8 @@ func NewUserInteractive( } func (u *UserInteractive) IsSingleStageFlow(authType string) bool { + u.RLock() + defer u.RUnlock() for _, f := range u.Flows { if len(f.Stages) == 1 && f.Stages[0] == authType { return true @@ -153,12 +157,16 @@ func (u *UserInteractive) IsSingleStageFlow(authType string) bool { } func (u *UserInteractive) AddCompletedStage(sessionID, authType string) { + u.Lock() // TODO: Handle multi-stage flows delete(u.Sessions, sessionID) + u.Unlock() } func (u *UserInteractive) DeleteSession(sessionID string) { + u.Lock() delete(u.Sessions, sessionID) + u.Unlock() } type Challenge struct { @@ -170,8 +178,13 @@ type Challenge struct { } // Challenge returns an HTTP 401 with the supported flows for authenticating -func (u *UserInteractive) Challenge(sessionID string) *util.JSONResponse { +func (u *UserInteractive) challenge(sessionID string) *util.JSONResponse { + u.RLock() paramsCopy := mapsutil.MapCopy(u.Params) + completed := u.Sessions[sessionID] + flows := u.Flows + u.RUnlock() + for key, element := range paramsCopy { p := GetAuthParams(element) if p != nil { @@ -184,8 +197,8 @@ func (u *UserInteractive) Challenge(sessionID string) *util.JSONResponse { return &util.JSONResponse{ Code: 401, JSON: Challenge{ - Completed: u.Sessions[sessionID], - Flows: u.Flows, + Completed: completed, + Flows: flows, Session: sessionID, Params: paramsCopy, }, @@ -200,8 +213,10 @@ func (u *UserInteractive) NewSession() *util.JSONResponse { res := jsonerror.InternalServerError() return &res } + u.Lock() u.Sessions[sessionID] = []string{} - return u.Challenge(sessionID) + u.Unlock() + return u.challenge(sessionID) } // ResponseWithChallenge mixes together a JSON body (e.g an error with errcode/message) with the @@ -214,7 +229,7 @@ func (u *UserInteractive) ResponseWithChallenge(sessionID string, response inter return &ise } _ = json.Unmarshal(b, &mixedObjects) - challenge := u.Challenge(sessionID) + challenge := u.challenge(sessionID) b, err = json.Marshal(challenge.JSON) if err != nil { ise := jsonerror.InternalServerError() @@ -243,7 +258,11 @@ func (u *UserInteractive) Verify(ctx context.Context, bodyBytes []byte, device * // extract the type so we know which login type to use authType := gjson.GetBytes(bodyBytes, "auth.type").Str + + u.RLock() loginType, ok := u.Types[authType] + u.RUnlock() + if !ok { return nil, &util.JSONResponse{ Code: http.StatusBadRequest, @@ -253,7 +272,12 @@ func (u *UserInteractive) Verify(ctx context.Context, bodyBytes []byte, device * // retrieve the session sessionID := gjson.GetBytes(bodyBytes, "auth.session").Str - if _, ok = u.Sessions[sessionID]; !ok { + + u.RLock() + _, ok = u.Sessions[sessionID] + u.RUnlock() + + if !ok { // if the login type is part of a single stage flow then allow them to omit the session ID if !u.IsSingleStageFlow(authType) { return nil, &util.JSONResponse{