Additional fixes and tweaks

CHANGELOG.md

@@ -3,6 +3,7 @@
 ## What's New
 
 * Auth Rate Limiter
+* Link Management Fixes
 * ziti edge quickstart command deprecates redundant --already-initialized flag. The identical behavior is implied by --home.
 
 ## Backwards compatibility
@@ -11,6 +12,9 @@ This release includes new response types from the REST authentication APIS. They
 `429` (server too busy) responses to auth requests. As this is an API change, the version number is 
 being bumped to 0.32.
 
+If controller and router are both v0.32 or later, only the router which dialed a link will report it to the controller. 
+If the controller is older, newer routers will report links from both the dialing and listening side of the link.
+
 ## Auth Rate Limiter
 
 In order to prevent clients from overwhelming the server with auth requests, an auth rate limiter has been introduced.
@@ -37,15 +41,46 @@ New metrics:
 * `auth.limiter.window_size`  - current size at which new auth attempts will be rejected
 * `auth.limiter.work_timer`   - tracks the rate at which api sessions are being created and how long it's taking to create them
 
+## Link Management Fixes
+
+With long lived link ids, there was potential for link control message to be ambiguous, as the link id wasn't enough to identify
+a specific iteration of that link. An iteration field has been added to links so that messaging is unambiguous. 
+Links will also only be reported from the dialing router now to reduce ambiguouity and race condition in link control channel
+messaging.
+
+## Router SSL Handshake Timeout Config
+
+There is a new router config setting which allows setting the SSL handshake timeout for TLS connections, when using ALPN for listeners.
+
+```
+tls:
+  handshakeTimeout: 15s
+```
+
 ## Component Updates and Bug Fixes
 
-* github.com/openziti/channel/v2: [v2.0.111 -> v2.0.113](https://github.com/openziti/channel/compare/v2.0.111...v2.0.113)
+* github.com/openziti/channel/v2: [v2.0.111 -> v2.0.116](https://github.com/openziti/channel/compare/v2.0.111...v2.0.116)
+    * [Issue #123](https://github.com/openziti/channel/issues/123) - Ensure hello messages respect connect timeout
     * [Issue #120](https://github.com/openziti/channel/issues/120) - Allow handling new underlay instances with function instead of channel 
 
-* github.com/openziti/edge-api: [v0.26.6 -> v0.26.7](https://github.com/openziti/edge-api/compare/v0.26.6...v0.26.7)
-* github.com/openziti/sdk-golang: [v0.22.0 -> v0.22.6](https://github.com/openziti/sdk-golang/compare/v0.22.0...v0.22.6)
-* github.com/openziti/secretstream: [v0.1.14 -> v0.1.15](https://github.com/openziti/secretstream/compare/v0.1.14...v0.1.15)
+* github.com/openziti/edge-api: [v0.26.6 -> v0.26.8](https://github.com/openziti/edge-api/compare/v0.26.6...v0.26.8)
+* github.com/openziti/foundation/v2: [v2.0.35 -> v2.0.36](https://github.com/openziti/foundation/compare/v2.0.35...v2.0.36)
+    * [Issue #391](https://github.com/openziti/foundation/issues/391) - goroutine pool can stall if configured for 0 min workers and with single producer
+
+* github.com/openziti/identity: [v1.0.68 -> v1.0.69](https://github.com/openziti/identity/compare/v1.0.68...v1.0.69)
+* github.com/openziti/metrics: [v1.2.41 -> v1.2.43](https://github.com/openziti/metrics/compare/v1.2.41...v1.2.43)
+* github.com/openziti/runzmd: [v1.0.36 -> v1.0.37](https://github.com/openziti/runzmd/compare/v1.0.36...v1.0.37)
+* github.com/openziti/sdk-golang: [v0.22.0 -> v0.22.17](https://github.com/openziti/sdk-golang/compare/v0.22.0...v0.22.17)
+    * [Issue #482](https://github.com/openziti/sdk-golang/issues/482) - Deprecate ListenOptions.MaxConnections in favor of MaxTerminators
+
+* github.com/openziti/secretstream: [v0.1.14 -> v0.1.16](https://github.com/openziti/secretstream/compare/v0.1.14...v0.1.16)
+* github.com/openziti/storage: [v0.2.27 -> v0.2.28](https://github.com/openziti/storage/compare/v0.2.27...v0.2.28)
+* github.com/openziti/transport/v2: [v2.0.119 -> v2.0.121](https://github.com/openziti/transport/compare/v2.0.119...v2.0.121)
+    * [Issue #73](https://github.com/openziti/transport/issues/73) - Allow overriding shared TLS/ALPN listener SSL handshake timeout
+
 * github.com/openziti/ziti: [v0.31.4 -> v0.32.0](https://github.com/openziti/ziti/compare/v0.31.4...v0.32.0)
+    * [Issue #1692](https://github.com/openziti/ziti/issues/1692) - Improve link stability with long lived link ids
+    * [Issue #1693](https://github.com/openziti/ziti/issues/1693) - Make links owned by the dialing router
     * [Issue #1685](https://github.com/openziti/ziti/issues/1685) - Race condition where we try to create terminator after client connection is closed
     * [Issue #1678](https://github.com/openziti/ziti/issues/1678) - Add link validation utility
     * [Issue #1673](https://github.com/openziti/ziti/issues/1673) - xgress dialers not getting passed xgress config

controller/handler_ctrl/fault.go

@@ -121,7 +121,7 @@ func (h *faultHandler) handleFaultedLink(log *logrus.Entry, fault *ctrl_pb.Fault
 		}
 
 		wasConnected := link.IsUsable()
-		if err := h.network.LinkFaulted(linkId, fault.Subject == ctrl_pb.FaultSubject_LinkDuplicate); err == nil {
+		if err := h.network.LinkFaulted(link, fault.Subject == ctrl_pb.FaultSubject_LinkDuplicate); err == nil {
 			h.network.LinkChanged(link)
 			otherRouter := link.Src
 			if link.Src.Id == h.r.Id {

controller/network/link_controller.go

@@ -19,6 +19,7 @@ package network
 import (
 	"encoding/json"
 	"errors"
+	"github.com/michaelquigley/pfxlog"
 	"github.com/openziti/channel/v2/protobufs"
 	"github.com/openziti/foundation/v2/info"
 	"github.com/openziti/storage/objectz"
@@ -129,11 +130,24 @@ func (linkController *linkController) routerReportedLink(linkId string, iteratio
 	linkController.lock.Lock()
 	defer linkController.lock.Unlock()
 
-	if link, found := linkController.get(linkId); found {
+	link, _ := linkController.get(linkId)
+	if link != nil && link.Iteration >= iteration {
 		return link, false
 	}
 
-	link := newLink(linkId, linkProtocol, dialAddress, linkController.initialLatency)
+	// remove the older link before adding the new one
+	if link != nil {
+		log := pfxlog.Logger().
+			WithField("routerId", src.Id).
+			WithField("linkId", linkId).
+			WithField("destRouterId", dstId).
+			WithField("iteration", iteration)
+
+		linkController.remove(link)
+		log.Infof("replaced link with newer iteration %v => %v", link.Iteration, iteration)
+	}
+
+	link = newLink(linkId, linkProtocol, dialAddress, linkController.initialLatency)
 	link.Iteration = iteration
 	link.Src = src
 	link.Dst.Store(dst)
@@ -152,10 +166,11 @@ func (linkController *linkController) all() []*Link {
 }
 
 func (linkController *linkController) remove(link *Link) {
-	linkController.linkTable.remove(link)
-	link.Src.routerLinks.Remove(link, link.DstId)
-	if dest := link.GetDest(); dest != nil {
-		dest.routerLinks.Remove(link, link.Src.Id)
+	if linkController.linkTable.remove(link) {
+		link.Src.routerLinks.Remove(link, link.DstId)
+		if dest := link.GetDest(); dest != nil {
+			dest.routerLinks.Remove(link, link.Src.Id)
+		}
 	}
 }
 
@@ -432,6 +447,8 @@ func (lt *linkTable) matching(f func(*Link) bool) []*Link {
 	return links
 }
 
-func (lt *linkTable) remove(link *Link) {
-	lt.links.Remove(link.Id)
+func (lt *linkTable) remove(link *Link) bool {
+	return lt.links.RemoveCb(link.Id, func(key string, v *Link, exists bool) bool {
+		return v != nil && v.Iteration == link.Iteration
+	})
 }

controller/network/network.go

@@ -445,19 +445,16 @@ func (network *Network) LinkConnected(msg *ctrl_pb.LinkConnected) error {
 	return errors.Errorf("no such link [l/%s]", msg.Id)
 }
 
-func (network *Network) LinkFaulted(id string, dupe bool) error {
-	if l, found := network.linkController.get(id); found {
-		l.addState(newLinkState(Failed))
-		if dupe {
-			network.NotifyLinkEvent(l, event.LinkDuplicate)
-		} else {
-			network.NotifyLinkEvent(l, event.LinkFault)
-		}
-		pfxlog.Logger().WithField("linkId", id).Info("removing failed link")
-		network.linkController.remove(l)
-		return nil
+func (network *Network) LinkFaulted(l *Link, dupe bool) error {
+	l.addState(newLinkState(Failed))
+	if dupe {
+		network.NotifyLinkEvent(l, event.LinkDuplicate)
+	} else {
+		network.NotifyLinkEvent(l, event.LinkFault)
 	}
-	return errors.Errorf("no such link [l/%s]", id)
+	pfxlog.Logger().WithField("linkId", l.Id).Info("removing failed link")
+	network.linkController.remove(l)
+	return nil
 }
 
 func (network *Network) VerifyRouter(routerId string, fingerprints []string) error {
@@ -982,22 +979,31 @@ func (network *Network) RemoveLink(linkId string) {
 	log := pfxlog.Logger().WithField("linkId", linkId)
 
 	link, _ := network.linkController.get(linkId)
+	var iteration uint32
 
 	var routerList []*Router
 	if link != nil {
+		iteration = link.Iteration
 		routerList = []*Router{link.Src}
 		if dst := link.GetDest(); dst != nil {
 			routerList = append(routerList, dst)
 		}
-		log = log.WithField("srcRouterId", link.Src.Id).WithField("dstRouterId", link.DstId)
+		log = log.WithField("srcRouterId", link.Src.Id).
+			WithField("dstRouterId", link.DstId).
+			WithField("iteration", iteration)
 		log.Info("deleting known link")
 	} else {
 		routerList = network.AllConnectedRouters()
 		log.Info("deleting unknown link (sending link fault to all connected routers)")
 	}
 
 	for _, router := range routerList {
-		fault := &ctrl_pb.Fault{Subject: ctrl_pb.FaultSubject_LinkFault, Id: linkId}
+		fault := &ctrl_pb.Fault{
+			Subject:   ctrl_pb.FaultSubject_LinkFault,
+			Id:        linkId,
+			Iteration: iteration,
+		}
+
 		if ctrl := router.Control; ctrl != nil {
 			if err := protobufs.MarshalTyped(fault).WithTimeout(15 * time.Second).Send(ctrl); err != nil {
 				log.WithField("faultDestRouterId", router.Id).WithError(err).

go.mod

@@ -52,10 +52,10 @@ require (
 	github.com/openziti/foundation/v2 v2.0.36
 	github.com/openziti/identity v1.0.69
 	github.com/openziti/jwks v1.0.3
-	github.com/openziti/metrics v1.2.42
+	github.com/openziti/metrics v1.2.43
 	github.com/openziti/runzmd v1.0.37
-	github.com/openziti/sdk-golang v0.22.11
-	github.com/openziti/secretstream v0.1.15
+	github.com/openziti/sdk-golang v0.22.17
+	github.com/openziti/secretstream v0.1.16
 	github.com/openziti/storage v0.2.28
 	github.com/openziti/transport/v2 v2.0.121
 	github.com/openziti/x509-claims v1.0.3
@@ -183,7 +183,7 @@ require (
 	golang.org/x/exp v0.0.0-20231006140011-7918f672742d // indirect
 	golang.org/x/image v0.13.0 // indirect
 	golang.org/x/mod v0.13.0 // indirect
-	golang.org/x/oauth2 v0.15.0 // indirect
+	golang.org/x/oauth2 v0.16.0 // indirect
 	golang.org/x/term v0.16.0 // indirect
 	golang.org/x/tools v0.14.0 // indirect
 	google.golang.org/appengine v1.6.7 // indirect

go.sum

@@ -578,14 +578,14 @@ github.com/openziti/identity v1.0.69 h1:wNgQomnv8ar2S1wge9jQK1jpqE2virOKKG8GyfTi
 github.com/openziti/identity v1.0.69/go.mod h1:+7hQNAG5nUUdz0165OubWpMHkP/ZVJB3nv9HUXXi9H8=
 github.com/openziti/jwks v1.0.3 h1:hf8wkb+Cg4nH/HM0KROFd7u+C3DkRVcFZJ7tDV+4icc=
 github.com/openziti/jwks v1.0.3/go.mod h1:t4xxq8vlXGsPn29kiQVnZBBDDnEoOFqtJoHibkJunQQ=
-github.com/openziti/metrics v1.2.42 h1:JRRwIIRkcMj/a03lRM1xej+DZ44NNQde/2YU3juySSo=
-github.com/openziti/metrics v1.2.42/go.mod h1:4f31ogWZzkHieDbpWeVhKiADa2GZeVlHILA0cqDbljw=
+github.com/openziti/metrics v1.2.43 h1:DSrmpLhoA45DlLVNdKOn2lBfCM0/r6wKz+3SDXe8X7Y=
+github.com/openziti/metrics v1.2.43/go.mod h1:+RY4avT60Vbxb9wyfvRD0msrARyYCB5+heb8VIZzCm8=
 github.com/openziti/runzmd v1.0.37 h1:qj2r9z4t7OAdmIXMdGbP9Su6TqA0bLdD2RMjJ71LRS0=
 github.com/openziti/runzmd v1.0.37/go.mod h1:eKhqJsGoLeDHex/o5Mw6TcNJxlVljafSVm7ZU+bX5G8=
-github.com/openziti/sdk-golang v0.22.11 h1:KB1Z1IdF4dQTruOUiOQG7+urwEIl+bfauyq83bzEmtQ=
-github.com/openziti/sdk-golang v0.22.11/go.mod h1:5PfOi3Bv7CXx88NGxbW8k7rXOWOuV7WeI07TidBvpcs=
-github.com/openziti/secretstream v0.1.15 h1:bGoPlT5zmZ+BiLKFMlaARG3gfiUzuhX/kmK6OInaghU=
-github.com/openziti/secretstream v0.1.15/go.mod h1:LyghB5JOlgvFASkLYPiBgjj5rcFXKiLD4qwHYRfBxnU=
+github.com/openziti/sdk-golang v0.22.17 h1:taywYpWpWBtZUj6KewMScYXgPe8TWz2nWNl96/y/IZ4=
+github.com/openziti/sdk-golang v0.22.17/go.mod h1:t0sT5N1Q/LdAd54Dxz274sQ9vJo8/B5Q0jn+VZ9vFuw=
+github.com/openziti/secretstream v0.1.16 h1:tVanF7OpJL1MJ1gvWaRlR2i+kAbrGsxr3q6EXFOS08U=
+github.com/openziti/secretstream v0.1.16/go.mod h1:bvjGBUW/0e5MzD5S3FW3rhGASRNWAi+kTkTENZ9qRDE=
 github.com/openziti/storage v0.2.28 h1:qHnsSF4RgQpT23hOXlwkAvJ0gO5PLmvTqyBy13dm3Rc=
 github.com/openziti/storage v0.2.28/go.mod h1:ahdvsmmdQWFsZXeExvXeA8MHUXPLrwuDFBJDTztLY5E=
 github.com/openziti/transport/v2 v2.0.121 h1:9WB1/9YMFed2UuefqZq+odUYYkoFOr+8aBIAZEC1MNE=
@@ -983,8 +983,8 @@ golang.org/x/oauth2 v0.0.0-20210218202405-ba52d332ba99/go.mod h1:KelEdhl1UZF7XfJ
 golang.org/x/oauth2 v0.0.0-20210220000619-9bb904979d93/go.mod h1:KelEdhl1UZF7XfJ4dDtk6s++YSgaE7mD/BuKKDLBl4A=
 golang.org/x/oauth2 v0.0.0-20210313182246-cd4f82c27b84/go.mod h1:KelEdhl1UZF7XfJ4dDtk6s++YSgaE7mD/BuKKDLBl4A=
 golang.org/x/oauth2 v0.0.0-20210402161424-2e8d93401602/go.mod h1:KelEdhl1UZF7XfJ4dDtk6s++YSgaE7mD/BuKKDLBl4A=
-golang.org/x/oauth2 v0.15.0 h1:s8pnnxNVzjWyrvYdFUQq5llS1PX2zhPXmccZv99h7uQ=
-golang.org/x/oauth2 v0.15.0/go.mod h1:q48ptWNTY5XWf+JNten23lcvHpLJ0ZSxF5ttTHKVCAM=
+golang.org/x/oauth2 v0.16.0 h1:aDkGMBSYxElaoP81NpoUoz2oo2R2wHdZpGToUxfyQrQ=
+golang.org/x/oauth2 v0.16.0/go.mod h1:hqZ+0LWXsiVoZpeld6jVt06P3adbS2Uu911W1SsJv2o=
 golang.org/x/perf v0.0.0-20180704124530-6e6d33e29852/go.mod h1:JLpeXjPJfIyPr5TlbXLkXWLhP8nz10XfvxElABhCtcw=
 golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=

router/link/link_events.go

@@ -237,6 +237,7 @@ func (self *updateLinkStatusForLink) Handle(registry *linkRegistryImpl) {
 		state.connectedCount++
 		state.retryDelay = time.Duration(0)
 		state.ctrlsNotified = false
+		registry.triggerNotify()
 	}
 
 	if state.status == StatusLinkFailed {

router/link/link_registry.go

@@ -53,6 +53,7 @@ func NewLinkRegistry(routerEnv Env) xlink.Registry {
 		env:            routerEnv,
 		destinations:   map[string]*linkDest{},
 		linkStateQueue: &linkStateHeap{},
+		triggerNotifyC: make(chan struct{}, 1),
 	}
 
 	go result.run()
@@ -71,6 +72,7 @@ type linkRegistryImpl struct {
 	destinations     map[string]*linkDest
 	linkStateQueue   *linkStateHeap
 	events           chan event
+	triggerNotifyC   chan struct{}
 	notifyInProgress atomic.Bool
 }
 
@@ -365,6 +367,8 @@ func (self *linkRegistryImpl) run() {
 		select {
 		case evt := <-self.events:
 			evt.Handle(self)
+		case <-self.triggerNotifyC:
+			self.notifyControllersOfLinks()
 		case <-queueCheckTicker.C:
 			self.evaluateLinkStateQueue()
 			self.notifyControllersOfLinks()
@@ -376,6 +380,13 @@ func (self *linkRegistryImpl) run() {
 	}
 }
 
+func (self *linkRegistryImpl) triggerNotify() {
+	select {
+	case self.triggerNotifyC <- struct{}{}:
+	default:
+	}
+}
+
 func (self *linkRegistryImpl) evaluateLinkStateQueue() {
 	now := time.Now()
 	for len(*self.linkStateQueue) > 0 {

tests/control.go

@@ -4,7 +4,10 @@ import (
 	"github.com/openziti/channel/v2"
 	"github.com/openziti/foundation/v2/versions"
 	"github.com/openziti/transport/v2"
+	"github.com/openziti/ziti/common/capabilities"
+	"github.com/openziti/ziti/common/pb/ctrl_pb"
 	"github.com/openziti/ziti/controller"
+	"math/big"
 )
 
 func (ctx *FabricTestContext) NewControlChannelListener() channel.UnderlayListener {
@@ -14,8 +17,13 @@ func (ctx *FabricTestContext) NewControlChannelListener() channel.UnderlayListen
 
 	versionHeader, err := versions.StdVersionEncDec.Encode(versions.NewDefaultVersionProvider().AsVersionInfo())
 	ctx.Req.NoError(err)
+
+	capabilityMask := &big.Int{}
+	capabilityMask.SetBit(capabilityMask, capabilities.ControllerCreateTerminatorV2, 1)
+	capabilityMask.SetBit(capabilityMask, capabilities.ControllerSingleRouterLinkSource, 1)
 	headers := map[int32][]byte{
-		channel.HelloVersionHeader: versionHeader,
+		channel.HelloVersionHeader:                       versionHeader,
+		int32(ctrl_pb.ControlHeaders_CapabilitiesHeader): capabilityMask.Bytes(),
 	}
 
 	ctrlChannelListenerConfig := channel.ListenerConfig{