crypto: fix nil pointer dereference of vault secret (#7241)

This commit fixes a nil pointer dereference issue
that can occur when the Vault KMS returns e.g. a 404
with an empty HTTP response. The Vault client SDK
does not treat that as error and returns nil for
the error and the secret.

Further it simplifies the token renewal and
re-authentication mechanism by using a single
background go-routine.

The control-flow of Vault authentications looks
like this:
1. `authenticate()`: Initial login and start of background job
2. Background job starts a `vault.Renewer` to renew the token
3. a) If this succeeds the token gets updated
   b) If this fails the background job tries to login again
4. If the login in 3b. succeeded goto 2. If it fails
   goto 3b.

cmd/crypto/vault.go

@@ -16,6 +16,7 @@ package crypto
 
 import (
 	"bytes"
+	"context"
 	"encoding/base64"
 	"errors"
 	"fmt"
@@ -64,7 +65,6 @@ type vaultService struct {
 	config        *VaultConfig
 	client        *vault.Client
 	leaseDuration time.Duration
-	tokenRenewer  *vault.Renewer
 }
 
 var _ KMS = (*vaultService)(nil) // compiler check that *vaultService implements KMS
@@ -130,31 +130,15 @@ func NewVault(config VaultConfig) (KMS, error) {
 	return v, nil
 }
 
-// reauthenticate() tries to login in 1 minute
-// intervals until successful.
-func (v *vaultService) reauthenticate() {
-	retryDelay := 1 * time.Minute
-	go func() {
-		for {
-			if err := v.authenticate(); err != nil {
-				time.Sleep(retryDelay)
-				continue
-			}
-			return
-		}
-	}()
-}
-
-// renewer calls vault client's renewer that automatically
-// renews secret periodically
-func (v *vaultService) renewer(secret *vault.Secret) {
+// renewSecret tries to renew the given secret. It blocks
+// until it receives either the new secret or encounters an error.
+func (v *vaultService) renewSecret(secret *vault.Secret) (*vault.Secret, error) {
 	renewer, err := v.client.NewRenewer(&vault.RenewerInput{
 		Secret: secret,
 	})
 	if err != nil {
-		logger.FatalIf(err, "crypto: hashicorp vault token renewer could not be started")
+		logger.CriticalIf(context.Background(), fmt.Errorf("crypto: failed to create hashicorp vault renewer: %s", err))
 	}
-	v.tokenRenewer = renewer
 	go renewer.Renew()
 	defer renewer.Stop()
 
@@ -162,38 +146,63 @@ func (v *vaultService) renewer(secret *vault.Secret) {
 		select {
 		case err := <-renewer.DoneCh():
 			if err != nil {
-				v.reauthenticate()
-				renewer.Stop()
-				return
+				return nil, err
 			}
-
-			// Renewal is now over
-		case renewal := <-renewer.RenewCh():
-			v.leaseDuration = time.Duration(renewal.Secret.Auth.LeaseDuration)
+		case renew := <-renewer.RenewCh():
+			if renew.Secret == nil || renew.Secret.Auth == nil {
+				return nil, ErrKMSAuthLogin
+			}
+			return renew.Secret, nil
 		}
 	}
 }
 
-// authenticate logs the app to vault, and starts the auto renewer
-// before secret expires
-func (v *vaultService) authenticate() (err error) {
+// login tries to authenticate the minio server to
+// the Vault KMS using the approle ID and secret.
+func (v *vaultService) login() (*vault.Secret, error) {
 	payload := map[string]interface{}{
 		"role_id":   v.config.Auth.AppRole.ID,
 		"secret_id": v.config.Auth.AppRole.Secret,
 	}
-	var secret *vault.Secret
-	secret, err = v.client.Logical().Write("auth/approle/login", payload)
+	secret, err := v.client.Logical().Write("auth/approle/login", payload)
 	if err != nil {
-		return
+		return nil, err
 	}
-	if secret.Auth == nil {
-		err = ErrKMSAuthLogin
-		return
+	if secret == nil || secret.Auth == nil {
+		return nil, ErrKMSAuthLogin
+	}
+	return secret, nil
+}
+
+// authenticate tries to authenticate the minio server
+// to the Vault KMS and starts a background job to renew
+// the login.
+func (v *vaultService) authenticate() error {
+	secret, err := v.login()
+	if err != nil {
+		return err
 	}
 	v.client.SetToken(secret.Auth.ClientToken)
 	v.leaseDuration = time.Duration(secret.Auth.LeaseDuration)
-	go v.renewer(secret)
-	return
+
+	// Start background job trying to renew the token
+	// or (if this fails) try to login again with app-ID and app-Secret.
+	go func(secret *vault.Secret) {
+		for {
+			newSecret, err := v.renewSecret(secret) // try to renew the secret (blocking)
+			if err != nil {
+				// Try to login again with app-ID and app-Secret
+				if newSecret, err = v.login(); err != nil { // failed -> try again
+					time.Sleep(1 * time.Minute) // retry delay
+					continue
+				}
+			}
+			secret = newSecret // Now newSecret contains a valid, non-nil *vault.Secret
+			v.client.SetToken(secret.Auth.ClientToken)
+			v.leaseDuration = time.Duration(secret.Auth.LeaseDuration)
+		}
+	}(secret)
+	return nil
 }
 
 // GenerateKey returns a new plaintext key, generated by the KMS,