vault: implement authentication token renewal (#428)

This commit re-implements token renewal after it got removed by 13cee22. Now, the token renewal process works as following: - If the token is about to expire (max. 10s before expiry) we try to renew the token if renewable. - If the renewal fails, or the token is not renewable, KES tries to re-authenticate with the configured auth method. - If re-authenticating also fails, we retry after 3s.
minio · Jan 10, 2024 · 877a8ae · 877a8ae
1 parent a9b155d
commit 877a8ae
Show file tree

Hide file tree

Showing 4 changed files with 49 additions and 80 deletions.
diff --git a/internal/keystore/vault/client.go b/internal/keystore/vault/client.go
@@ -70,70 +70,45 @@ func (c *client) CheckStatus(ctx context.Context, delay time.Duration) {
 //
 // To renew the auth. token see: client.RenewToken(...).
 func (c *client) AuthenticateWithAppRole(login *AppRole) authFunc {
-	return func() (token string, ttl time.Duration, err error) {
+	return func() (*vaultapi.Secret, error) {
 		secret, err := c.Logical().Write(path.Join("auth", login.Engine, "login"), map[string]interface{}{
 			"role_id":   login.ID,
 			"secret_id": login.Secret,
 		})
-		if err != nil || secret == nil {
+		if secret == nil {
 			// The Vault SDK eventually returns no error but also no
 			// secret. In this case have to return a (not very helpful)
 			// error to signal that the authentication failed - for some
 			// (unknown) reason.
-			if err == nil {
-				err = errors.New("vault: authentication failed: SDK returned no error but also no token")
-			}
-			return token, ttl, err
-		}
-
-		token, err = secret.TokenID()
-		if err != nil {
-			return token, ttl, err
-		}
-
-		ttl, err = secret.TokenTTL()
-		if err != nil {
-			return token, ttl, err
+			return nil, errors.New("vault: authentication failed: SDK returned no error but also no token")
 		}
-		return token, ttl, nil
+		return secret, err
 	}
 }
 
 func (c *client) AuthenticateWithK8S(login *Kubernetes) authFunc {
-	return func() (token string, ttl time.Duration, err error) {
+	return func() (*vaultapi.Secret, error) {
 		secret, err := c.Logical().Write(path.Join("auth", login.Engine, "login"), map[string]interface{}{
 			"role": login.Role,
 			"jwt":  login.JWT,
 		})
-		if err != nil || secret == nil {
+		if secret == nil {
 			// The Vault SDK eventually returns no error but also no
 			// secret. In this case have to return a (not very helpful)
 			// error to signal that the authentication failed - for some
 			// (unknown) reason.
-			if err == nil {
-				err = errors.New("vault: authentication failed: SDK returned no error but also no token")
-			}
-			return token, ttl, err
-		}
-		token, err = secret.TokenID()
-		if err != nil {
-			return token, ttl, err
-		}
-
-		ttl, err = secret.TokenTTL()
-		if err != nil {
-			return token, ttl, err
+			return nil, errors.New("vault: authentication failed: SDK returned no error but also no token")
 		}
-		return token, ttl, nil
+		return secret, err
 	}
 }
 
 // authFunc implements a Vault authentication method.
 //
-// It returns a Vault authentication token and its
-// time-to-live (TTL) or an error explaining why
+// It returns a secret with a Vault authentication token
+// and its time-to-live (TTL) or an error explaining why
 // the authentication attempt failed.
-type authFunc func() (token string, ttl time.Duration, err error)
+type authFunc func() (*vaultapi.Secret, error)
 
 // RenewToken tries to renew the Vault auth token periodically
 // based on its TTL. If TTL is zero, RenewToken returns early
@@ -149,14 +124,13 @@ type authFunc func() (token string, ttl time.Duration, err error)
 // usually invoke CheckStatus in a separate go routine:
 //
 //	go client.RenewToken(ctx, login, ttl)
-func (c *client) RenewToken(ctx context.Context, authenticate authFunc, ttl, retry time.Duration) {
+func (c *client) RenewToken(ctx context.Context, authenticate authFunc, secret *vaultapi.Secret) {
+	ttl, _ := secret.TokenTTL()
 	if ttl == 0 {
 		return // Token has no TTL. Hence, we do not need to renew it. (long-lived)
 	}
-	if retry == 0 {
-		retry = 5 * time.Second
-	}
 
+	const Retry = 3 // Retry token renewal N times before re-authenticating.
 	for {
 		// If Vault is sealed we have to wait
 		// until it is unsealed again.
@@ -177,30 +151,43 @@ func (c *client) RenewToken(ctx context.Context, authenticate authFunc, ttl, ret
 			continue
 		}
 
-		// We don't use TTL / 2 to avoid loosing access
-		// if the renewal process fails once.
-		timer := time.NewTimer(ttl / 3)
+		// We renew the token right before it expires.
+		renewIn := ttl
+		if renewIn > 10*time.Second {
+			renewIn = ttl - 10*time.Second
+		}
+
+		timer := time.NewTimer(renewIn)
 		select {
 		case <-ctx.Done():
 			if !timer.Stop() {
 				<-timer.C
 			}
 			return
 		case <-timer.C:
-			token, newTTL, err := authenticate()
-			if err != nil || newTTL == 0 {
-				timer := time.NewTimer(retry)
-				select {
-				case <-ctx.Done():
-					if !timer.Stop() {
-						<-timer.C
+			// Try to renew token, if renewable. Otherwise, or if renewal
+			// fails try to re-authenticate.
+			if ok, _ := secret.TokenIsRenewable(); ok {
+				for i := 0; i < Retry; i++ {
+					var err error
+					secret, err = c.Auth().Token().RenewSelfWithContext(ctx, 0)
+					if err == nil {
+						break
 					}
-					return
-				case <-timer.C:
+				}
+				if secret == nil {
+					secret, _ = authenticate()
 				}
 			} else {
-				ttl = newTTL
+				secret, _ = authenticate()
+			}
+
+			if secret != nil {
+				ttl, _ = secret.TokenTTL()
+				token, _ := secret.TokenID()
 				c.SetToken(token) // SetToken is safe to call from different go routines
+			} else {
+				ttl = 3 * time.Second // In case of renew/auth failure, retry in 3s.
 			}
 		}
 	}

diff --git a/internal/keystore/vault/config.go b/internal/keystore/vault/config.go
@@ -54,11 +54,6 @@ type AppRole struct {
 
 	// Secret is the AppRole authentication secret.
 	Secret string
-
-	// Retry is the duration after which another
-	// authentication attempt is performed once
-	// an authentication attempt failed.
-	Retry time.Duration
 }
 
 // Clone returns a copy of the AppRole auth.
@@ -70,7 +65,6 @@ func (a *AppRole) Clone() *AppRole {
 		Engine: a.Engine,
 		ID:     a.ID,
 		Secret: a.Secret,
-		Retry:  a.Retry,
 	}
 }
 
@@ -92,11 +86,6 @@ type Kubernetes struct {
 
 	// JWT is the issued authentication token.
 	JWT string
-
-	// Retry is the duration after which another
-	// authentication attempt is performed once
-	// an authentication attempt failed.
-	Retry time.Duration
 }
 
 // Clone returns a copy of the Kubernetes auth.
@@ -108,7 +97,6 @@ func (k *Kubernetes) Clone() *Kubernetes {
 		Engine: k.Engine,
 		Role:   k.Role,
 		JWT:    k.JWT,
-		Retry:  k.Retry,
 	}
 }
 

diff --git a/internal/keystore/vault/config_test.go b/internal/keystore/vault/config_test.go
@@ -29,7 +29,6 @@ var cloneConfigTests = []*Config{
 			Engine: "auth",
 			ID:     "be7f3c83-9733-4d65-adaa-7eeb6e14e922",
 			Secret: "ba8d68af-23c4-4199-a516-e37cebdaab48",
-			Retry:  30 * time.Second,
 		},
 		K8S: &Kubernetes{
 			Engine: "auth",

diff --git a/internal/keystore/vault/vault.go b/internal/keystore/vault/vault.go
@@ -49,9 +49,6 @@ func Connect(ctx context.Context, c *Config) (*Store, error) {
 		c.APIVersion = APIv1
 	}
 	if c.AppRole != nil {
-		if c.AppRole.Retry == 0 {
-			c.AppRole.Retry = 5 * time.Second
-		}
 		if c.AppRole.Engine == "" {
 			c.AppRole.Engine = EngineAppRole
 		}
@@ -60,9 +57,6 @@ func Connect(ctx context.Context, c *Config) (*Store, error) {
 		if c.K8S.Engine == "" {
 			c.K8S.Engine = EngineKubernetes
 		}
-		if c.K8S.Retry == 0 {
-			c.K8S.Retry = 5 * time.Second
-		}
 	}
 	if c.Transit != nil {
 		if c.Transit.Engine == "" {
@@ -127,26 +121,27 @@ func Connect(ctx context.Context, c *Config) (*Store, error) {
 		client.SetNamespace(c.Namespace)
 	}
 
-	var (
-		authenticate authFunc
-		retry        time.Duration
-	)
+	var authenticate authFunc
 	switch {
 	case c.AppRole != nil && (c.AppRole.ID != "" || c.AppRole.Secret != ""):
-		authenticate, retry = client.AuthenticateWithAppRole(c.AppRole), c.AppRole.Retry
+		authenticate = client.AuthenticateWithAppRole(c.AppRole)
 	case c.K8S != nil && (c.K8S.Role != "" || c.K8S.JWT != ""):
-		authenticate, retry = client.AuthenticateWithK8S(c.K8S), c.K8S.Retry
+		authenticate = client.AuthenticateWithK8S(c.K8S)
 	}
 
-	token, ttl, err := authenticate()
+	auth, err := authenticate()
+	if err != nil {
+		return nil, err
+	}
+	token, err := auth.TokenID()
 	if err != nil {
 		return nil, err
 	}
 	client.SetToken(token)
 
 	ctx, cancel := context.WithCancel(ctx)
 	go client.CheckStatus(ctx, c.StatusPingAfter)
-	go client.RenewToken(ctx, authenticate, ttl, retry)
+	go client.RenewToken(ctx, authenticate, auth)
 	return &Store{
 		config: c,
 		client: client,