From e098852a80db2b502d5c402b5b60c22781dde4f8 Mon Sep 17 00:00:00 2001
From: poornas <poornas@users.noreply.github.com>
Date: Wed, 20 Feb 2019 12:23:59 -0800
Subject: [PATCH] Revert PR #7241 to fix vault renewal (#7259)

- Current implementation was spawning renewer goroutines
without waiting for the lease duration to end. Remove vault renewer
and call vault.RenewToken directly and manage reauthentication if
lease expired.
---
 cmd/crypto/vault.go | 116 +++++++++++++++++++++-----------------------
 1 file changed, 55 insertions(+), 61 deletions(-)

diff --git a/cmd/crypto/vault.go b/cmd/crypto/vault.go
index f03312d76..565b247e4 100644
--- a/cmd/crypto/vault.go
+++ b/cmd/crypto/vault.go
@@ -16,7 +16,6 @@ package crypto
 
 import (
 	"bytes"
-	"context"
 	"encoding/base64"
 	"errors"
 	"fmt"
@@ -24,7 +23,6 @@ import (
 	"time"
 
 	vault "github.com/hashicorp/vault/api"
-	"github.com/minio/minio/cmd/logger"
 )
 
 var (
@@ -64,6 +62,7 @@ type VaultConfig struct {
 type vaultService struct {
 	config        *VaultConfig
 	client        *vault.Client
+	secret        *vault.Secret
 	leaseDuration time.Duration
 }
 
@@ -123,86 +122,81 @@ func NewVault(config VaultConfig) (KMS, error) {
 		client.SetNamespace(config.Namespace)
 	}
 	v := &vaultService{client: client, config: &config}
-
 	if err := v.authenticate(); err != nil {
 		return nil, err
 	}
+	v.renewToken()
 	return v, nil
 }
 
-// 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.CriticalIf(context.Background(), fmt.Errorf("crypto: failed to create hashicorp vault renewer: %s", err))
-	}
-	go renewer.Renew()
-	defer renewer.Stop()
-
-	for {
-		select {
-		case err := <-renewer.DoneCh():
-			if err != nil {
-				return nil, err
+// renewToken starts a new go-routine which renews
+// the vault authentication token periodically and re-authenticates
+// if the token renewal fails
+func (v *vaultService) renewToken() {
+	retryDelay := v.leaseDuration / 2
+	go func() {
+		for {
+			if v.secret == nil {
+				if err := v.authenticate(); err != nil {
+					time.Sleep(retryDelay)
+					continue
+				}
+			}
+			s, err := v.client.Auth().Token().RenewSelf(int(v.leaseDuration))
+			if err != nil || s == nil {
+				v.secret = nil
+				time.Sleep(retryDelay)
+				continue
 			}
-		case renew := <-renewer.RenewCh():
-			if renew.Secret == nil || renew.Secret.Auth == nil {
-				return nil, ErrKMSAuthLogin
+			if ok, err := s.TokenIsRenewable(); !ok || err != nil {
+				v.secret = nil
+				continue
 			}
-			return renew.Secret, nil
+			ttl, err := s.TokenTTL()
+			if err != nil {
+				v.secret = nil
+				continue
+			}
+			v.secret = s
+			retryDelay = ttl / 2
+			time.Sleep(retryDelay)
 		}
-	}
+	}()
 }
 
-// login tries to authenticate the minio server to
-// the Vault KMS using the approle ID and secret.
-func (v *vaultService) login() (*vault.Secret, error) {
+// authenticate logs the app to vault, and starts the auto renewer
+// before secret expires
+func (v *vaultService) authenticate() (err error) {
 	payload := map[string]interface{}{
 		"role_id":   v.config.Auth.AppRole.ID,
 		"secret_id": v.config.Auth.AppRole.Secret,
 	}
-	secret, err := v.client.Logical().Write("auth/approle/login", payload)
+	var tokenID string
+	var ttl time.Duration
+	var secret *vault.Secret
+	secret, err = v.client.Logical().Write("auth/approle/login", payload)
 	if err != nil {
-		return nil, err
+		return
 	}
-	if secret == nil || secret.Auth == nil {
-		return nil, ErrKMSAuthLogin
+	if secret == nil {
+		err = ErrKMSAuthLogin
+		return
 	}
-	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()
+	tokenID, err = secret.TokenID()
 	if err != nil {
-		return err
+		err = ErrKMSAuthLogin
+		return
 	}
-	v.client.SetToken(secret.Auth.ClientToken)
-	v.leaseDuration = time.Duration(secret.Auth.LeaseDuration)
-
-	// 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
+	ttl, err = secret.TokenTTL()
+	if err != nil {
+		err = ErrKMSAuthLogin
+		return
+	}
+	v.client.SetToken(tokenID)
+	v.secret = secret
+	v.leaseDuration = ttl
+	return
 }
 
 // GenerateKey returns a new plaintext key, generated by the KMS,