From be5d421e00368b07318f514723c4902adf02ccae Mon Sep 17 00:00:00 2001 From: Paul Lorenz Date: Tue, 16 Jan 2024 14:35:06 -0500 Subject: [PATCH] Fix terminator id race condition. Fixes #1685 --- controller/handler_edge_ctrl/common.go | 20 +++++++++++++------ .../handler_edge_ctrl/create_terminator_v2.go | 1 + router/xgress_edge/fabric.go | 3 ++- router/xgress_edge/hosted.go | 11 +++++++--- 4 files changed, 25 insertions(+), 10 deletions(-) diff --git a/controller/handler_edge_ctrl/common.go b/controller/handler_edge_ctrl/common.go index 3b82dd831..ea93243f4 100644 --- a/controller/handler_edge_ctrl/common.go +++ b/controller/handler_edge_ctrl/common.go @@ -10,16 +10,16 @@ import ( "github.com/michaelquigley/pfxlog" "github.com/openziti/channel/v2" - "github.com/openziti/ziti/controller/env" - "github.com/openziti/ziti/controller/model" - "github.com/openziti/ziti/controller/db" - "github.com/openziti/ziti/controller/network" - "github.com/openziti/ziti/controller/xt" - "github.com/openziti/ziti/common/logcontext" "github.com/openziti/foundation/v2/stringz" "github.com/openziti/identity" "github.com/openziti/sdk-golang/ziti/edge" "github.com/openziti/storage/boltz" + "github.com/openziti/ziti/common/logcontext" + "github.com/openziti/ziti/controller/db" + "github.com/openziti/ziti/controller/env" + "github.com/openziti/ziti/controller/model" + "github.com/openziti/ziti/controller/network" + "github.com/openziti/ziti/controller/xt" "github.com/sirupsen/logrus" ) @@ -367,6 +367,14 @@ func (self *baseSessionRequestContext) verifyTerminator(terminatorId string, bin return nil } +func (self *baseSessionRequestContext) verifyTerminatorId(id string) { + if self.err == nil { + if id == "" { + self.err = invalidTerminator("provided terminator id is blank") + } + } +} + func (self *baseSessionRequestContext) updateTerminator(terminator *network.Terminator, request UpdateTerminatorRequest, ctx *change.Context) { if self.err == nil { checker := fields.UpdatedFieldsMap{} diff --git a/controller/handler_edge_ctrl/create_terminator_v2.go b/controller/handler_edge_ctrl/create_terminator_v2.go index be396e450..119ef87e1 100644 --- a/controller/handler_edge_ctrl/create_terminator_v2.go +++ b/controller/handler_edge_ctrl/create_terminator_v2.go @@ -80,6 +80,7 @@ func (self *createTerminatorV2Handler) CreateTerminatorV2(ctx *CreateTerminatorV if !ctx.loadRouter() { return } + ctx.verifyTerminatorId(ctx.req.Address) ctx.loadSession(ctx.req.SessionToken) ctx.checkSessionType(db.SessionTypeBind) ctx.checkSessionFingerprints(ctx.req.Fingerprints) diff --git a/router/xgress_edge/fabric.go b/router/xgress_edge/fabric.go index 43e2e115b..4d5c7c006 100644 --- a/router/xgress_edge/fabric.go +++ b/router/xgress_edge/fabric.go @@ -125,9 +125,10 @@ func (self *edgeTerminator) close(notify bool, reason string) { if terminatorId := self.terminatorId.Load(); terminatorId != "" { if self.terminatorId.CompareAndSwap(terminatorId, "") { logger.Debug("removing terminator on router") - self.edgeClientConn.listener.factory.hostedServices.Delete(terminatorId) self.state.Store(TerminatorStateDeleting) + self.edgeClientConn.listener.factory.hostedServices.Delete(terminatorId) + logger.Info("removing terminator on controller") ctrlCh := self.edgeClientConn.listener.factory.ctrls.AnyCtrlChannel() if ctrlCh == nil { diff --git a/router/xgress_edge/hosted.go b/router/xgress_edge/hosted.go index 15e88d554..0a1c4b482 100644 --- a/router/xgress_edge/hosted.go +++ b/router/xgress_edge/hosted.go @@ -262,8 +262,13 @@ func (self *hostedServiceRegistry) establishTerminator(terminator *edgeTerminato WithField("routerId", factory.env.GetRouterId().Token). WithField("terminatorId", terminator.terminatorId.Load()) + terminatorId := terminator.terminatorId.Load() + if terminatorId == "" { + return fmt.Errorf("edge link is closed, stopping terminator creation for terminator %s", terminatorId) + } + request := &edge_ctrl_pb.CreateTerminatorV2Request{ - Address: terminator.terminatorId.Load(), + Address: terminatorId, SessionToken: terminator.token, Fingerprints: terminator.edgeClientConn.fingerprints.Prints(), PeerData: terminator.hostData, @@ -286,7 +291,7 @@ func (self *hostedServiceRegistry) establishTerminator(terminator *edgeTerminato return err } - if self.waitForTerminatorCreated(terminator.terminatorId.Load(), 10*time.Second) { + if self.waitForTerminatorCreated(terminatorId, 10*time.Second) { return nil } @@ -295,7 +300,7 @@ func (self *hostedServiceRegistry) establishTerminator(terminator *edgeTerminato return errors.Errorf("timeout waiting for response to create terminator request for terminator %v", terminator.terminatorId.Load()) } -func (self *hostedServiceRegistry) HandleCreateTerminatorResponse(msg *channel.Message, ctrlCh channel.Channel) { +func (self *hostedServiceRegistry) HandleCreateTerminatorResponse(msg *channel.Message, _ channel.Channel) { log := pfxlog.Logger().WithField("routerId", self.env.GetRouterId().Token) response := &edge_ctrl_pb.CreateTerminatorV2Response{}