From 887e77c2ae21ba279d37a81f4c61fa8be7fe7935 Mon Sep 17 00:00:00 2001 From: tdsacilowski Date: Mon, 25 Jul 2022 09:42:09 -0400 Subject: [PATCH] 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 --- changelog/11969.txt | 3 + command/agent/auth/jwt/jwt.go | 76 ++++++++++++------- command/agent/auth/jwt/jwt_test.go | 67 +++++++++++++++- .../docs/agent/autoauth/methods/jwt.mdx | 4 + 4 files changed, 117 insertions(+), 33 deletions(-) create mode 100644 changelog/11969.txt diff --git a/changelog/11969.txt b/changelog/11969.txt new file mode 100644 index 000000000..668093565 --- /dev/null +++ b/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. +``` \ No newline at end of file diff --git a/command/agent/auth/jwt/jwt.go b/command/agent/auth/jwt/jwt.go index 0c97bee90..8f088eb19 100644 --- a/command/agent/auth/jwt/jwt.go +++ b/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 - path string - mountPath string - role string - credsFound chan struct{} - watchCh chan string - stopCh chan struct{} - doneCh chan struct{} - credSuccessGate chan struct{} - ticker *time.Ticker - once *sync.Once - latestToken *atomic.Value + logger hclog.Logger + path string + mountPath string + role string + removeJWTAfterReading bool + credsFound chan struct{} + watchCh chan string + stopCh chan struct{} + doneCh chan struct{} + credSuccessGate chan struct{} + ticker *time.Ticker + 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, - mountPath: conf.MountPath, - credsFound: make(chan struct{}), - watchCh: make(chan string), - stopCh: make(chan struct{}), - doneCh: make(chan struct{}), - credSuccessGate: make(chan struct{}), - once: new(sync.Once), - latestToken: new(atomic.Value), + logger: conf.Logger, + mountPath: conf.MountPath, + removeJWTAfterReading: true, + credsFound: make(chan struct{}), + watchCh: make(chan string), + stopCh: make(chan struct{}), + doneCh: make(chan struct{}), + credSuccessGate: make(chan struct{}), + 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 { - j.logger.Error("error removing jwt file", "error", err) + if j.removeJWTAfterReading { + if err := os.Remove(j.path); err != nil { + j.logger.Error("error removing jwt file", "error", err) + } } } diff --git a/command/agent/auth/jwt/jwt_test.go b/command/agent/auth/jwt/jwt_test.go index f912006b1..8e9a2ae86 100644 --- a/command/agent/auth/jwt/jwt_test.go +++ b/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) + } + } + } +} diff --git a/website/content/docs/agent/autoauth/methods/jwt.mdx b/website/content/docs/agent/autoauth/methods/jwt.mdx index db66af7b6..26400a219 100644 --- a/website/content/docs/agent/autoauth/methods/jwt.mdx +++ b/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.