Agent JWT auto auth remove_jwt_after_reading config option (#11969)

Add a new config option for Vault Agent's JWT auto auth
`remove_jwt_after_reading`, which defaults to true. Can stop
Agent from attempting to delete the file, which is useful in k8s
where the service account JWT is mounted as a read-only file
and so any attempt to delete it generates spammy error logs.

When leaving the JWT file in place, the read period for new
tokens is 1 minute instead of 500ms to reflect the assumption
that there will always be a file there, so finding a file does not
provide any signal that it needs to be re-read. Kubernetes
has a minimum TTL of 10 minutes for tokens, so a period of
1 minute gives Agent plenty of time to detect new tokens,
without leaving it too unresponsive. We may want to add a
config option to override these default periods in the future.

Co-authored-by: Tom Proctor <tomhjp@users.noreply.github.com>

changelog/11969.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+agent: JWT auto auth now supports a `remove_jwt_after_reading` config option which defaults to true.
+```

command/agent/auth/jwt/jwt.go

@@ -5,7 +5,6 @@ import (
 	"errors"
 	"fmt"
 	"io/fs"
-	"io/ioutil"
 	"net/http"
 	"os"
 	"sync"
@@ -15,21 +14,23 @@ import (
 	hclog "github.com/hashicorp/go-hclog"
 	"github.com/hashicorp/vault/api"
 	"github.com/hashicorp/vault/command/agent/auth"
+	"github.com/hashicorp/vault/sdk/helper/parseutil"
 )
 
 type jwtMethod struct {
-	logger          hclog.Logger
+	logger                hclog.Logger
-	path            string
+	path                  string
-	mountPath       string
+	mountPath             string
-	role            string
+	role                  string
-	credsFound      chan struct{}
+	removeJWTAfterReading bool
-	watchCh         chan string
+	credsFound            chan struct{}
-	stopCh          chan struct{}
+	watchCh               chan string
-	doneCh          chan struct{}
+	stopCh                chan struct{}
-	credSuccessGate chan struct{}
+	doneCh                chan struct{}
-	ticker          *time.Ticker
+	credSuccessGate       chan struct{}
-	once            *sync.Once
+	ticker                *time.Ticker
-	latestToken     *atomic.Value
+	once                  *sync.Once
+	latestToken           *atomic.Value
 }
 
 // NewJWTAuthMethod returns an implementation of Agent's auth.AuthMethod
@@ -43,15 +44,16 @@ func NewJWTAuthMethod(conf *auth.AuthConfig) (auth.AuthMethod, error) {
 	}
 
 	j := &jwtMethod{
-		logger:          conf.Logger,
+		logger:                conf.Logger,
-		mountPath:       conf.MountPath,
+		mountPath:             conf.MountPath,
-		credsFound:      make(chan struct{}),
+		removeJWTAfterReading: true,
-		watchCh:         make(chan string),
+		credsFound:            make(chan struct{}),
-		stopCh:          make(chan struct{}),
+		watchCh:               make(chan string),
-		doneCh:          make(chan struct{}),
+		stopCh:                make(chan struct{}),
-		credSuccessGate: make(chan struct{}),
+		doneCh:                make(chan struct{}),
-		once:            new(sync.Once),
+		credSuccessGate:       make(chan struct{}),
-		latestToken:     new(atomic.Value),
+		once:                  new(sync.Once),
+		latestToken:           new(atomic.Value),
 	}
 	j.latestToken.Store("")
 
@@ -73,6 +75,14 @@ func NewJWTAuthMethod(conf *auth.AuthConfig) (auth.AuthMethod, error) {
 		return nil, errors.New("could not convert 'role' config value to string")
 	}
 
+	if removeJWTAfterReadingRaw, ok := conf.Config["remove_jwt_after_reading"]; ok {
+		removeJWTAfterReading, err := parseutil.ParseBool(removeJWTAfterReadingRaw)
+		if err != nil {
+			return nil, fmt.Errorf("error parsing 'remove_jwt_after_reading' value: %w", err)
+		}
+		j.removeJWTAfterReading = removeJWTAfterReading
+	}
+
 	switch {
 	case j.path == "":
 		return nil, errors.New("'path' value is empty")
@@ -80,7 +90,14 @@ func NewJWTAuthMethod(conf *auth.AuthConfig) (auth.AuthMethod, error) {
 		return nil, errors.New("'role' value is empty")
 	}
 
-	j.ticker = time.NewTicker(500 * time.Millisecond)
+	// If we don't delete the JWT after reading, use a slower reload period,
+	// otherwise we would re-read the whole file every 500ms, instead of just
+	// doing a stat on the file every 500ms.
+	readPeriod := 1 * time.Minute
+	if j.removeJWTAfterReading {
+		readPeriod = 500 * time.Millisecond
+	}
+	j.ticker = time.NewTicker(readPeriod)
 
 	go j.runWatcher()
 
@@ -145,6 +162,7 @@ func (j *jwtMethod) runWatcher() {
 			j.ingressToken()
 			newToken := j.latestToken.Load().(string)
 			if newToken != latestToken {
+				j.logger.Debug("new jwt file found")
 				j.credsFound <- struct{}{}
 			}
 		}
@@ -161,11 +179,9 @@ func (j *jwtMethod) ingressToken() {
 		return
 	}
 
-	j.logger.Debug("new jwt file found")
-
 	// Check that the path refers to a file.
 	// If it's a symlink, it could still be a symlink to a directory,
-	// but ioutil.ReadFile below will return a descriptive error.
+	// but os.ReadFile below will return a descriptive error.
 	switch mode := fi.Mode(); {
 	case mode.IsRegular():
 		// regular file
@@ -176,7 +192,7 @@ func (j *jwtMethod) ingressToken() {
 		return
 	}
 
-	token, err := ioutil.ReadFile(j.path)
+	token, err := os.ReadFile(j.path)
 	if err != nil {
 		j.logger.Error("failed to read jwt file", "error", err)
 		return
@@ -190,7 +206,9 @@ func (j *jwtMethod) ingressToken() {
 		j.latestToken.Store(string(token))
 	}
 
-	if err := os.Remove(j.path); err != nil {
+	if j.removeJWTAfterReading {
-		j.logger.Error("error removing jwt file", "error", err)
+		if err := os.Remove(j.path); err != nil {
+			j.logger.Error("error removing jwt file", "error", err)
+		}
 	}
 }

command/agent/auth/jwt/jwt_test.go

@@ -2,7 +2,6 @@ package jwt
 
 import (
 	"bytes"
-	"io/ioutil"
 	"os"
 	"path"
 	"strings"
@@ -10,6 +9,7 @@ import (
 	"testing"
 
 	"github.com/hashicorp/go-hclog"
+	"github.com/hashicorp/vault/command/agent/auth"
 )
 
 func TestIngressToken(t *testing.T) {
@@ -21,18 +21,18 @@ func TestIngressToken(t *testing.T) {
 		symlinked = "symlinked"
 	)
 
-	rootDir, err := ioutil.TempDir("", "vault-agent-jwt-auth-test")
+	rootDir, err := os.MkdirTemp("", "vault-agent-jwt-auth-test")
 	if err != nil {
 		t.Fatalf("failed to create temp dir: %s", err)
 	}
 	defer os.RemoveAll(rootDir)
 
 	setupTestDir := func() string {
-		testDir, err := ioutil.TempDir(rootDir, "")
+		testDir, err := os.MkdirTemp(rootDir, "")
 		if err != nil {
 			t.Fatal(err)
 		}
-		err = ioutil.WriteFile(path.Join(testDir, file), []byte("test"), 0o644)
+		err = os.WriteFile(path.Join(testDir, file), []byte("test"), 0o644)
 		if err != nil {
 			t.Fatal(err)
 		}
@@ -106,3 +106,62 @@ func TestIngressToken(t *testing.T) {
 		}
 	}
 }
+
+func TestDeleteAfterReading(t *testing.T) {
+	for _, tc := range map[string]struct {
+		configValue  string
+		shouldDelete bool
+	}{
+		"default": {
+			"",
+			true,
+		},
+		"explicit true": {
+			"true",
+			true,
+		},
+		"false": {
+			"false",
+			false,
+		},
+	} {
+		rootDir, err := os.MkdirTemp("", "vault-agent-jwt-auth-test")
+		if err != nil {
+			t.Fatalf("failed to create temp dir: %s", err)
+		}
+		defer os.RemoveAll(rootDir)
+		tokenPath := path.Join(rootDir, "token")
+		err = os.WriteFile(tokenPath, []byte("test"), 0o644)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		config := &auth.AuthConfig{
+			Config: map[string]interface{}{
+				"path": tokenPath,
+				"role": "unusedrole",
+			},
+			Logger: hclog.Default(),
+		}
+		if tc.configValue != "" {
+			config.Config["remove_jwt_after_reading"] = tc.configValue
+		}
+
+		jwtAuth, err := NewJWTAuthMethod(config)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		jwtAuth.(*jwtMethod).ingressToken()
+
+		if _, err := os.Lstat(tokenPath); tc.shouldDelete {
+			if err == nil || !os.IsNotExist(err) {
+				t.Fatal(err)
+			}
+		} else {
+			if err != nil {
+				t.Fatal(err)
+			}
+		}
+	}
+}

website/content/docs/agent/autoauth/methods/jwt.mdx

@@ -14,3 +14,7 @@ method](/docs/auth/jwt).
 - `path` `(string: required)` - The path to the JWT file
 
 - `role` `(string: required)` - The role to authenticate against on Vault
+
+- `remove_jwt_after_reading` `(bool: optional, defaults to true)` -
+  This can be set to `false` to disable the default behavior of removing the
+  JWT after it's been read.