Issue #274: Avoid premature vault token renewals

cert/source_test.go

@@ -140,6 +140,8 @@ func TestNewSource(t *testing.T) {
 			desc: "vault",
 			cfg:  certsource("vault"),
 			src: &VaultSource{
+				Addr:         os.Getenv("VAULT_ADDR"),
+				vaultToken:   os.Getenv("VAULT_TOKEN"),
 				CertPath:     "cert",
 				ClientCAPath: "clientca",
 				CAUpgradeCN:  "upgcn",
@@ -391,39 +393,39 @@ func TestVaultSource(t *testing.T) {
 
 	// run tests
 	tests := []struct {
-		desc    string
-		wrapTTL string
-		req     *vaultapi.TokenCreateRequest
-		dropErr bool
+		desc     string
+		wrapTTL  string
+		req      *vaultapi.TokenCreateRequest
+		dropWarn bool
 	}{
 		{
 			desc: "renewable token",
 			req:  &vaultapi.TokenCreateRequest{Lease: "1m", TTL: "1m", Policies: []string{"fabio"}},
 		},
 		{
-			desc:    "non-renewable token",
-			req:     &vaultapi.TokenCreateRequest{Lease: "1m", TTL: "1m", Renewable: newBool(false), Policies: []string{"fabio"}},
-			dropErr: true,
+			desc:     "non-renewable token",
+			req:      &vaultapi.TokenCreateRequest{Lease: "1m", TTL: "1m", Renewable: newBool(false), Policies: []string{"fabio"}},
+			dropWarn: true,
 		},
 		{
 			desc: "renewable orphan token",
 			req:  &vaultapi.TokenCreateRequest{Lease: "1m", TTL: "1m", NoParent: true, Policies: []string{"fabio"}},
 		},
 		{
-			desc:    "non-renewable orphan token",
-			req:     &vaultapi.TokenCreateRequest{Lease: "1m", TTL: "1m", NoParent: true, Renewable: newBool(false), Policies: []string{"fabio"}},
-			dropErr: true,
+			desc:     "non-renewable orphan token",
+			req:      &vaultapi.TokenCreateRequest{Lease: "1m", TTL: "1m", NoParent: true, Renewable: newBool(false), Policies: []string{"fabio"}},
+			dropWarn: true,
 		},
 		{
 			desc:    "renewable wrapped token",
 			wrapTTL: "10s",
 			req:     &vaultapi.TokenCreateRequest{Lease: "1m", TTL: "1m", Policies: []string{"fabio"}},
 		},
 		{
-			desc:    "non-renewable wrapped token",
-			wrapTTL: "10s",
-			req:     &vaultapi.TokenCreateRequest{Lease: "1m", TTL: "1m", Renewable: newBool(false), Policies: []string{"fabio"}},
-			dropErr: true,
+			desc:     "non-renewable wrapped token",
+			wrapTTL:  "10s",
+			req:      &vaultapi.TokenCreateRequest{Lease: "1m", TTL: "1m", Renewable: newBool(false), Policies: []string{"fabio"}},
+			dropWarn: true,
 		},
 	}
 
@@ -438,11 +440,11 @@ func TestVaultSource(t *testing.T) {
 				vaultToken: makeToken(t, client, tt.wrapTTL, tt.req),
 			}
 
-			// suppress the log warning about a non-renewable lease
+			// suppress the log warning about a non-renewable token
 			// since this is the expected behavior.
-			dropNotRenewableError = tt.dropErr
+			dropNotRenewableWarning = tt.dropWarn
 			testSource(t, src, pool, timeout)
-			dropNotRenewableError = false
+			dropNotRenewableWarning = false
 		})
 	}
 }

cert/vault_source.go

@@ -3,6 +3,7 @@ package cert
 import (
 	"crypto/tls"
 	"crypto/x509"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"log"
@@ -29,8 +30,21 @@ type VaultSource struct {
 	Refresh      time.Duration
 
 	mu         sync.Mutex
-	token      string // actual token
 	vaultToken string // VAULT_TOKEN env var. Might be wrapped.
+	auth       struct {
+		// token is the actual Vault token.
+		token string
+
+		// expireTime is the time at which the token expires (becomes useless).
+		// The zero value indicates that the token is not renewable or never
+		// expires.
+		expireTime time.Time
+
+		// renewTTL is the desired token lifetime after renewal, in seconds.
+		// This value is advisory and the Vault server may ignore or silently
+		// change it.
+		renewTTL int
+	}
 }
 
 func (s *VaultSource) client() (*api.Client, error) {
@@ -45,20 +59,20 @@ func (s *VaultSource) client() (*api.Client, error) {
 	if err != nil {
 		return nil, err
 	}
-	if err := s.setToken(c); err != nil {
+
+	if err := s.setAuth(c); err != nil {
 		return nil, err
 	}
+
 	return c, nil
 }
 
-func (s *VaultSource) setToken(c *api.Client) error {
+func (s *VaultSource) setAuth(c *api.Client) error {
 	s.mu.Lock()
-	defer func() {
-		c.SetToken(s.token)
-		s.mu.Unlock()
-	}()
+	defer s.mu.Unlock()
 
-	if s.token != "" {
+	if s.auth.token != "" {
+		c.SetToken(s.auth.token)
 		return nil
 	}
 
@@ -68,20 +82,66 @@ func (s *VaultSource) setToken(c *api.Client) error {
 
 	// did we get a wrapped token?
 	resp, err := c.Logical().Unwrap(s.vaultToken)
-	if err != nil {
-		// not a wrapped token?
-		if strings.HasPrefix(err.Error(), "no value found at") {
-			s.token = s.vaultToken
-			return nil
-		}
+	switch {
+	case err == nil:
+		log.Printf("[INFO] vault: Unwrapped token %s", s.vaultToken)
+		s.auth.token = resp.Auth.ClientToken
+	case strings.HasPrefix(err.Error(), "no value found at"):
+		// not a wrapped token
+		s.auth.token = s.vaultToken
+	default:
 		return err
 	}
-	log.Printf("[INFO] vault: Unwrapped token %s", s.vaultToken)
 
-	s.token = resp.Auth.ClientToken
+	c.SetToken(s.auth.token)
+	s.checkRenewal(c)
+
 	return nil
 }
 
+// dropNotRenewableWarning controls whether the 'Token is not renewable'
+// warning is logged. This is useful for testing where this is the expected
+// behavior. On production, this should always be set to false.
+var dropNotRenewableWarning bool
+
+// checkRenewal checks if the Vault token can be renewed, and if so when it
+// expires and how big the renewal increment should be.
+func (s *VaultSource) checkRenewal(c *api.Client) {
+	resp, err := c.Auth().Token().LookupSelf()
+	if err != nil {
+		log.Printf("[WARN] vault: lookup-self failed, token renewal is disabled: %s", err)
+		return
+	}
+
+	b, _ := json.Marshal(resp.Data)
+	var data struct {
+		CreationTTL int       `json:"creation_ttl"`
+		ExpireTime  time.Time `json:"expire_time"`
+		Renewable   bool      `json:"renewable"`
+	}
+	if err := json.Unmarshal(b, &data); err != nil {
+		log.Printf("[WARN] vault: lookup-self failed, token renewal is disabled: %s", err)
+		return
+	}
+
+	switch {
+	case data.Renewable:
+		s.auth.renewTTL = data.CreationTTL
+		s.auth.expireTime = data.ExpireTime
+	case data.ExpireTime.IsZero():
+		// token doesn't expire
+		return
+	case dropNotRenewableWarning:
+		return
+	default:
+		ttl := time.Until(data.ExpireTime)
+		ttl = ttl / time.Second * time.Second // truncate to seconds
+		log.Printf("[WARN] vault: Token is not renewable and will expire %s from now at %s",
+			ttl, data.ExpireTime.Format(time.RFC3339))
+	}
+
+}
+
 func (s *VaultSource) LoadClientCAs() (*x509.CertPool, error) {
 	return newCertPool(s.ClientCAPath, s.CAUpgradeCN, s.load)
 }
@@ -92,11 +152,6 @@ func (s *VaultSource) Certificates() chan []tls.Certificate {
 	return ch
 }
 
-// dropNotRenewableError controls whether the 'lease is not renewable'
-// error is logged. This is useful for testing where this is the expected
-// behavior. On production, this should always be set to false.
-var dropNotRenewableError bool
-
 func (s *VaultSource) load(path string) (pemBlocks map[string][]byte, err error) {
 	pemBlocks = map[string][]byte{}
 
@@ -130,15 +185,8 @@ func (s *VaultSource) load(path string) (pemBlocks map[string][]byte, err error)
 		return nil, fmt.Errorf("vault: client: %s", err)
 	}
 
-	// renew token
-	// TODO(fs): make configurable
-	const oneHour = 3600
-	_, err = c.Auth().Token().RenewSelf(oneHour)
-	if err != nil {
-		// TODO(fs): danger of filling up log since default refresh is 1s
-		if !dropNotRenewableError {
-			log.Printf("[WARN] vault: Failed to renew token. %s", err)
-		}
+	if err := s.renewToken(c); err != nil {
+		log.Printf("[WARN] vault: Failed to renew token: %s", err)
 	}
 
 	// get the subkeys under 'path'.
@@ -165,3 +213,37 @@ func (s *VaultSource) load(path string) (pemBlocks map[string][]byte, err error)
 
 	return pemBlocks, nil
 }
+
+func (s *VaultSource) renewToken(c *api.Client) error {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
+	ttl := time.Until(s.auth.expireTime)
+	switch {
+	case s.auth.expireTime.IsZero():
+		// Token isn't renewable.
+		return nil
+	case ttl < 2*s.Refresh:
+		// Renew the token if it isn't valid for two more refresh intervals.
+		break
+	case ttl < time.Minute:
+		// Renew the token if it isn't valid for one more minute. This happens
+		// if s.Refresh is small, say one second. It is risky to renew the
+		// token just one or two seconds before expiration; networks are
+		// unreliable, clocks can be skewed, etc.
+		break
+	default:
+		// Token doesn't need to be renewed yet.
+		return nil
+	}
+
+	resp, err := c.Auth().Token().RenewSelf(s.auth.renewTTL)
+	if err != nil {
+		return err
+	}
+
+	leaseDuration := time.Duration(resp.Auth.LeaseDuration) * time.Second
+	s.auth.expireTime = time.Now().Add(leaseDuration)
+
+	return nil
+}