vault: improve usage of time.Timer

This commit improves the usage of `time.Timer`
esp. within the Vault token renew and re-auth
logic.

It follows some recommendations from:
https://go101.org/article/memory-leaking.html

Ref: Real Memory Leaking Caused by Not Stopping
time.Ticker Values Which Are Not Used Any More

Further, this commit updates the Vault SDK dep
from 1.5 to 1.9.2 and removes some deprecated API
usages.

Signed-off-by: Andreas Auernhammer <[email protected]>

cmd/kes/gateway.go

@@ -12,7 +12,7 @@ import (
 	"encoding/hex"
 	"errors"
 	"fmt"
-	"io/ioutil"
+	"io"
 	"net/http"
 	"os"
 	"os/signal"
@@ -136,13 +136,11 @@ func startGateway(cliConfig gatewayConfig) {
 	}(ctx)
 
 	go func(ctx context.Context) {
-		ticker := time.NewTicker(15 * time.Minute)
-		defer ticker.Stop()
-
 		for {
 			select {
 			case <-ctx.Done():
-			case <-ticker.C:
+				return
+			case <-time.NewTicker(15 * time.Minute).C:
 				tlsConfig, err := newTLSConfig(config, cliConfig.TLSAuth)
 				if err != nil {
 					log.Printf("failed to reload TLS configuration: %v", err)
@@ -526,12 +524,12 @@ func newGatewayConfig(ctx context.Context, config *edge.ServerConfig, tlsConfig
 	if config.Log.Error {
 		rConfig.ErrorLog = log.New(os.Stderr, "Error: ", log.Ldate|log.Ltime|log.Lmsgprefix)
 	} else {
-		rConfig.ErrorLog = log.New(ioutil.Discard, "Error: ", log.Ldate|log.Ltime|log.Lmsgprefix)
+		rConfig.ErrorLog = log.New(io.Discard, "Error: ", log.Ldate|log.Ltime|log.Lmsgprefix)
 	}
 	if config.Log.Audit {
 		rConfig.AuditLog = log.New(os.Stdout, "", 0)
 	} else {
-		rConfig.AuditLog = log.New(ioutil.Discard, "", 0)
+		rConfig.AuditLog = log.New(io.Discard, "", 0)
 	}
 
 	if len(config.TLS.Proxies) != 0 {

internal/http/retry.go

@@ -186,9 +186,6 @@ func (r *Retry) Do(req *http.Request) (*http.Response, error) {
 		}
 	}
 
-	timer := time.NewTimer(0)
-	defer timer.Stop()
-
 	resp, err := r.Client.Do(req)
 	for N > 0 && (isTemporary(err) || (resp != nil && resp.StatusCode >= http.StatusInternalServerError)) {
 		N--
@@ -202,9 +199,10 @@ func (r *Retry) Do(req *http.Request) (*http.Response, error) {
 			delay = Delay + time.Duration(rand.Int63n(Jitter.Milliseconds()))*time.Millisecond
 		}
 
-		timer.Reset(delay)
+		timer := time.NewTimer(delay)
 		select {
 		case <-req.Context().Done():
+			timer.Stop()
 			return nil, &url.Error{
 				Op:  req.Method,
 				URL: req.URL.String(),

internal/https/server.go

@@ -113,7 +113,7 @@ func (s *Server) Start(ctx context.Context) error {
 		GetConfigForClient: func(*tls.ClientHelloInfo) (*tls.Config, error) {
 			s.lock.RLock()
 			defer s.lock.RUnlock()
-			return s.tlsConfig, nil
+			return s.tlsConfig.Clone(), nil
 		},
 	})
 	if err != nil {

internal/keystore/cache.go

@@ -233,7 +233,7 @@ func (c *Cache) Get(ctx context.Context, name string) (key.Key, error) {
 
 // gc executes f periodically until the ctx.Done() channel returns.
 func (c *Cache) gc(ctx context.Context, interval time.Duration, f func()) {
-	if interval == 0 {
+	if interval <= 0 {
 		return
 	}
 

internal/keystore/vault/client.go

@@ -20,7 +20,7 @@ import (
 type client struct {
 	*vaultapi.Client
 
-	sealed uint32 // Atomic bool: sealed == 0 is false, sealed == 1 is true
+	sealed atomic.Bool
 }
 
 // Sealed returns true if the most recently fetched vault
@@ -30,9 +30,9 @@ type client struct {
 // reflect the current status of the vault server - because it
 // may have changed in the meantime.
 //
-// If the vault health status hasn't been queried every then
+// If the vault health status hasn't been queried ever then
 // Sealed returns false.
-func (c *client) Sealed() bool { return atomic.LoadUint32(&c.sealed) == 1 }
+func (c *client) Sealed() bool { return c.sealed.Load() }
 
 // CheckStatus keeps fetching the vault health status every delay
 // unit of time until  <-ctx.Done() returns.
@@ -48,26 +48,16 @@ func (c *client) CheckStatus(ctx context.Context, delay time.Duration) {
 		delay = 10 * time.Second
 	}
 
-	timer := time.NewTimer(0)
-	defer timer.Stop()
-
 	for {
 		status, err := c.Sys().Health()
 		if err == nil {
-			if status.Sealed {
-				atomic.StoreUint32(&c.sealed, 1)
-			} else {
-				atomic.StoreUint32(&c.sealed, 0)
-			}
+			c.sealed.Store(status.Sealed)
 		}
 
-		// Add the delay to wait before next health check.
-		timer.Reset(delay)
-
 		select {
 		case <-ctx.Done():
 			return
-		case <-timer.C:
+		case <-time.NewTimer(delay).C:
 		}
 	}
 }
@@ -171,75 +161,64 @@ func (c *client) RenewToken(ctx context.Context, authenticate authFunc, ttl, ret
 		retry = 5 * time.Second
 	}
 
-	timer := time.NewTimer(0)
-	defer timer.Stop()
-
 	for {
 		// If Vault is sealed we have to wait
 		// until it is unsealed again.
 		//
 		// Users should start client.CheckStatus() in
 		// another go routine to unblock this for-loop
 		// once vault becomes unsealed again.
-		for c.Sealed() {
-			timer.Reset(time.Second)
-
+		if c.Sealed() {
 			select {
 			case <-ctx.Done():
 				return
-			case <-timer.C:
+			case <-time.NewTimer(1 * time.Second).C:
 			}
+			continue
 		}
 
 		// If the TTL is 0 we cannot renew the token.
 		// Therefore, we try to re-authenticate and
 		// get a new token. We repeat that until we
 		// successfully authenticate and got a token.
 		if ttl == 0 {
-			var (
-				token string
-				err   error
-			)
-			token, ttl, err = authenticate()
-			if err != nil {
-				ttl = 0 // On error, set the TTL again to 0 to re-auth. again.
-				timer.Reset(retry)
+			token, newTTL, err := authenticate()
+			if err != nil || newTTL == 0 {
 				select {
 				case <-ctx.Done():
 					return
-				case <-timer.C:
+				case <-time.NewTimer(retry).C:
 				}
-				continue
+			} else {
+				ttl = newTTL
+				c.SetToken(token) // SetToken is safe to call from different go routines
 			}
-			c.SetToken(token) // SetToken is safe to call from different go routines
+			continue
 		}
 
 		// Now the client has a token with a non-zero TTL
 		// such tht we can renew it. We repeat that until
 		// the renewable process fails once. In this case
 		// we try to re-authenticate again.
-		for {
-			timer.Reset(ttl / 2)
+		select {
+		case <-ctx.Done():
+			return
+		case <-time.NewTimer(ttl / 2).C:
+		}
 
-			select {
-			case <-ctx.Done():
-				return
-			case <-timer.C:
-			}
-			secret, err := c.Auth().Token().RenewSelf(int(ttl.Seconds()))
-			if err != nil || secret == nil {
-				break
-			}
-			if ok, err := secret.TokenIsRenewable(); !ok || err != nil {
-				break
-			}
-			ttl, err := secret.TokenTTL()
-			if err != nil || ttl == 0 {
-				break
-			}
+		secret, err := c.Auth().Token().RenewSelfWithContext(ctx, int(ttl.Seconds()))
+		if err != nil || secret == nil {
+			ttl = 0
+			continue
+		}
+		if ok, err := secret.TokenIsRenewable(); !ok || err != nil {
+			ttl = 0
+			continue
+		}
+		ttl, err = secret.TokenTTL()
+		if err != nil || ttl == 0 {
+			ttl = 0
+			continue
 		}
-		// If we exit the for-loop above set the TTL to 0 to trigger
-		// a re-authentication.
-		ttl = 0
 	}
 }

internal/keystore/vault/vault.go

@@ -149,10 +149,6 @@ func (s *Store) Status(ctx context.Context) (kv.State, error) {
 	}
 	client.ClearNamespace()
 
-	// First, we try to fetch the Vault health information.
-	// Only if this fails we try to dial Vault directly over
-	// a TCP connection. See: https://github.com/minio/kes-go/issues/230
-
 	start := time.Now()
 	health, err := client.Sys().HealthWithContext(ctx)
 	if err == nil {
@@ -168,15 +164,7 @@ func (s *Store) Status(ctx context.Context) (kv.State, error) {
 	if errors.Is(err, context.Canceled) && errors.Is(err, context.DeadlineExceeded) {
 		return kv.State{}, &kv.Unreachable{Err: err}
 	}
-
-	start = time.Now()
-	req := s.client.Client.NewRequest(http.MethodGet, "")
-	if _, err = s.client.Client.RawRequestWithContext(ctx, req); err != nil {
-		return kv.State{}, &kv.Unreachable{Err: err}
-	}
-	return kv.State{
-		Latency: time.Since(start),
-	}, nil
+	return kv.State{}, err
 }
 
 // Create creates the given key-value pair at Vault if and only