From 05b2e384040d049fd331a8886ad43ab352fccfc7 Mon Sep 17 00:00:00 2001 From: Andrew Martinez Date: Fri, 15 Sep 2023 14:13:26 -0400 Subject: [PATCH] fixes openziti/ziti#1310 enrollment extension waits for controller For router certificate extension, routers now wait for a control channel for 10s then check again every one minute till it is available or the router is terminated or self terminates due to a lack of a control channel. --- router/internal/edgerouter/config.go | 4 +- router/xgress_edge/certchecker.go | 95 +++++++++++++++++--------- router/xgress_edge/certchecker_test.go | 26 ------- 3 files changed, 64 insertions(+), 61 deletions(-) diff --git a/router/internal/edgerouter/config.go b/router/internal/edgerouter/config.go index c1dab6cfc..0fd7ab54c 100644 --- a/router/internal/edgerouter/config.go +++ b/router/internal/edgerouter/config.go @@ -55,7 +55,7 @@ type Config struct { SessionValidateMinInterval time.Duration SessionValidateMaxInterval time.Duration Tcfg transport.Configuration - ExtendEnrollment bool + ForceExtendEnrollment bool RouterConfig *router.Config EnrollmentIdentityConfig *identity.Config @@ -113,7 +113,7 @@ func (config *Config) LoadConfigFromMap(configMap map[interface{}]interface{}) e if val, ok := configMap[FlagsCfgMapKey]; ok { if flags, ok := val.(map[string]*pflag.Flag); ok { if flag, ok := flags["extend"]; ok { - config.ExtendEnrollment = flag.Value.String() == "true" + config.ForceExtendEnrollment = flag.Value.String() == "true" } } } diff --git a/router/xgress_edge/certchecker.go b/router/xgress_edge/certchecker.go index 6869caea3..5649f820f 100644 --- a/router/xgress_edge/certchecker.go +++ b/router/xgress_edge/certchecker.go @@ -109,10 +109,16 @@ func (self *CertExpirationChecker) Run() error { } } - durationToWait, err := self.getWaitTime() + durationToWait := 0 * time.Second + if self.edgeConfig.ForceExtendEnrollment { + self.edgeConfig.ForceExtendEnrollment = false + } else { + var err error + durationToWait, err = self.getWaitTime() - if err != nil { - return fmt.Errorf("could not extend certificates: %v", err) + if err != nil { + return fmt.Errorf("could not determine enrollent extension wait time: %v", err) + } } pfxlog.Logger().Infof("waiting %s to renew certificates", durationToWait) @@ -130,18 +136,50 @@ func (self *CertExpirationChecker) Run() error { } } +func (self *CertExpirationChecker) AnyCtrlChannelWithTimeout(timeout time.Duration) (channel.Channel, bool) { + startTime := time.Now() + interval := 500 * time.Millisecond + for { + ctrlCh := self.ctrls.AnyCtrlChannel() + + if ctrlCh != nil && !ctrlCh.IsClosed() { + return ctrlCh, false + } + + select { + case <-time.After(interval): + break + case <-self.closeNotify: + return nil, true + } + + if time.Since(startTime) >= timeout { + break + } + } + + return nil, false +} + func (self *CertExpirationChecker) ExtendEnrollment() error { if !self.extender.IsRequestingCompareAndSwap(false, true) { return fmt.Errorf("could not send enrollment extension request, a request has already been sent") } - ctrlCh := self.ctrls.AnyCtrlChannel() - if ctrlCh == nil { - return errors.New("cannot request updates, no controller is available") - } + var ctrlCh channel.Channel + timeout := 10 * time.Second + intervalWait := 1 * time.Minute + for ctrlCh == nil { + var closeNotified bool + ctrlCh, closeNotified = self.AnyCtrlChannelWithTimeout(timeout) - if ctrlCh.IsClosed() { - return errors.New("cannot request updates, control channel has closed") + if closeNotified { + return errors.New("router shutting down") + } + + if ctrlCh == nil { + pfxlog.Logger().Errorf("cannot send enrollment extension request, no controller is available, waited %s for a control channel, waiting %s to try again", timeout, intervalWait) + } } self.requestSentAt = time.Now() @@ -180,36 +218,27 @@ func (self *CertExpirationChecker) ExtendEnrollment() error { } func (self *CertExpirationChecker) getWaitTime() (time.Duration, error) { - var durationToWait time.Duration + now := time.Now() - if self.edgeConfig.ExtendEnrollment { - self.edgeConfig.ExtendEnrollment = false - durationToWait = 0 + if self.id.Cert().Leaf.NotAfter.Before(now) || self.id.Cert().Leaf.NotAfter == now { + return 0, fmt.Errorf("client certificate has expired") + } - pfxlog.Logger().Info("enrollment extension was forced") - } else { - now := time.Now() + clientExpirationDuration := self.id.Cert().Leaf.NotAfter.Add(-7 * 24 * time.Hour).Sub(now) //1 week before client cert expires - if self.id.Cert().Leaf.NotAfter.Before(now) || self.id.Cert().Leaf.NotAfter == now { - return 0, fmt.Errorf("client certificate has expired") - } + if clientExpirationDuration < 0 { + return 0, nil + } - clientExpirationDuration := self.id.Cert().Leaf.NotAfter.Add(-7 * 24 * time.Hour).Sub(now) //1 week before client cert expires + serverExpirationDuration := self.id.ServerCert()[0].Leaf.NotAfter.Add(-7 * 24 * time.Hour).Sub(now) // 1 week before server cert expires - if clientExpirationDuration < 0 { - return 0, nil - } + if serverExpirationDuration < 0 { + return 0, nil + } - serverExpirationDuration := self.id.ServerCert()[0].Leaf.NotAfter.Add(-7 * 24 * time.Hour).Sub(now) // 1 week before server cert expires - - if serverExpirationDuration < 0 { - return 0, nil - } - - durationToWait = serverExpirationDuration - if clientExpirationDuration < serverExpirationDuration { - durationToWait = clientExpirationDuration - } + durationToWait := serverExpirationDuration + if clientExpirationDuration < serverExpirationDuration { + durationToWait = clientExpirationDuration } return durationToWait, nil diff --git a/router/xgress_edge/certchecker_test.go b/router/xgress_edge/certchecker_test.go index 809187149..186e76b14 100644 --- a/router/xgress_edge/certchecker_test.go +++ b/router/xgress_edge/certchecker_test.go @@ -236,19 +236,6 @@ func Test_CertExpirationChecker(t *testing.T) { req.GreaterOrEqual(waitTime, 20*time.Second) req.LessOrEqual(waitTime, 30*time.Second) }) - - t.Run("force returns 0", func(t *testing.T) { - req := require.New(t) - certChecker, _ := newCertChecker() - - certChecker.edgeConfig.ExtendEnrollment = true - - waitTime, err := certChecker.getWaitTime() - - req.NoError(err) - req.Equal(time.Duration(0), waitTime) - req.False(certChecker.edgeConfig.ExtendEnrollment) - }) }) t.Run("Run", func(t *testing.T) { @@ -370,19 +357,6 @@ func Test_CertExpirationChecker(t *testing.T) { }) t.Run("ExtendEnrollment", func(t *testing.T) { - t.Run("errors if control channel is closed", func(t *testing.T) { - req := require.New(t) - certChecker, _ := newCertChecker() - - testChannel := certChecker.ctrls.AnyCtrlChannel().(*simpleTestChannel) - req.NotNil(testChannel) - testChannel.isClosed = true - - err := certChecker.ExtendEnrollment() - - req.Error(err) - req.True(certChecker.isRequesting.Load()) - }) t.Run("errors if isRequesting = true", func(t *testing.T) { req := require.New(t)