Fix user_interactive multi-threaded state update crash

This commit is contained in:
Tak Wai Wong 2022-08-10 18:29:52 -07:00
parent 934c021cd3
commit 7681ae93e8

View file

@ -18,6 +18,7 @@ import (
"context" "context"
"encoding/json" "encoding/json"
"net/http" "net/http"
"sync"
"github.com/matrix-org/dendrite/clientapi/jsonerror" "github.com/matrix-org/dendrite/clientapi/jsonerror"
"github.com/matrix-org/dendrite/internal/mapsutil" "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 // the user already has a valid access token, but we want to double-check
// that it isn't stolen by re-authenticating them. // that it isn't stolen by re-authenticating them.
type UserInteractive struct { type UserInteractive struct {
sync.RWMutex
Flows []userInteractiveFlow Flows []userInteractiveFlow
// Map of login type to implementation // Map of login type to implementation
Types map[string]Type Types map[string]Type
@ -144,6 +146,8 @@ func NewUserInteractive(
} }
func (u *UserInteractive) IsSingleStageFlow(authType string) bool { func (u *UserInteractive) IsSingleStageFlow(authType string) bool {
u.RLock()
defer u.RUnlock()
for _, f := range u.Flows { for _, f := range u.Flows {
if len(f.Stages) == 1 && f.Stages[0] == authType { if len(f.Stages) == 1 && f.Stages[0] == authType {
return true return true
@ -153,12 +157,16 @@ func (u *UserInteractive) IsSingleStageFlow(authType string) bool {
} }
func (u *UserInteractive) AddCompletedStage(sessionID, authType string) { func (u *UserInteractive) AddCompletedStage(sessionID, authType string) {
u.Lock()
// TODO: Handle multi-stage flows // TODO: Handle multi-stage flows
delete(u.Sessions, sessionID) delete(u.Sessions, sessionID)
u.Unlock()
} }
func (u *UserInteractive) DeleteSession(sessionID string) { func (u *UserInteractive) DeleteSession(sessionID string) {
u.Lock()
delete(u.Sessions, sessionID) delete(u.Sessions, sessionID)
u.Unlock()
} }
type Challenge struct { type Challenge struct {
@ -170,8 +178,13 @@ type Challenge struct {
} }
// Challenge returns an HTTP 401 with the supported flows for authenticating // 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) paramsCopy := mapsutil.MapCopy(u.Params)
completed := u.Sessions[sessionID]
flows := u.Flows
u.RUnlock()
for key, element := range paramsCopy { for key, element := range paramsCopy {
p := GetAuthParams(element) p := GetAuthParams(element)
if p != nil { if p != nil {
@ -184,8 +197,8 @@ func (u *UserInteractive) Challenge(sessionID string) *util.JSONResponse {
return &util.JSONResponse{ return &util.JSONResponse{
Code: 401, Code: 401,
JSON: Challenge{ JSON: Challenge{
Completed: u.Sessions[sessionID], Completed: completed,
Flows: u.Flows, Flows: flows,
Session: sessionID, Session: sessionID,
Params: paramsCopy, Params: paramsCopy,
}, },
@ -200,8 +213,10 @@ func (u *UserInteractive) NewSession() *util.JSONResponse {
res := jsonerror.InternalServerError() res := jsonerror.InternalServerError()
return &res return &res
} }
u.Lock()
u.Sessions[sessionID] = []string{} 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 // 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 return &ise
} }
_ = json.Unmarshal(b, &mixedObjects) _ = json.Unmarshal(b, &mixedObjects)
challenge := u.Challenge(sessionID) challenge := u.challenge(sessionID)
b, err = json.Marshal(challenge.JSON) b, err = json.Marshal(challenge.JSON)
if err != nil { if err != nil {
ise := jsonerror.InternalServerError() 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 // extract the type so we know which login type to use
authType := gjson.GetBytes(bodyBytes, "auth.type").Str authType := gjson.GetBytes(bodyBytes, "auth.type").Str
u.RLock()
loginType, ok := u.Types[authType] loginType, ok := u.Types[authType]
u.RUnlock()
if !ok { if !ok {
return nil, &util.JSONResponse{ return nil, &util.JSONResponse{
Code: http.StatusBadRequest, Code: http.StatusBadRequest,
@ -253,7 +272,12 @@ func (u *UserInteractive) Verify(ctx context.Context, bodyBytes []byte, device *
// retrieve the session // retrieve the session
sessionID := gjson.GetBytes(bodyBytes, "auth.session").Str 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 the login type is part of a single stage flow then allow them to omit the session ID
if !u.IsSingleStageFlow(authType) { if !u.IsSingleStageFlow(authType) {
return nil, &util.JSONResponse{ return nil, &util.JSONResponse{