From de9eaa4fad37322ff67b8b58e802749665ec0895 Mon Sep 17 00:00:00 2001 From: VioletHynes Date: Mon, 16 Jan 2023 16:28:27 -0500 Subject: [PATCH 01/14] VAULT-12564 Work so far on token file auto-auth --- api/lifetime_watcher.go | 53 ++++-- command/agent.go | 4 + command/agent/auth/auth.go | 105 ++++++++--- command/agent/auth/kubernetes/kubernetes.go | 7 + command/agent/auth/token-file/token-file.go | 117 ++++++++++++ command/agent/token_file_end_to_end_test.go | 193 ++++++++++++++++++++ 6 files changed, 440 insertions(+), 39 deletions(-) create mode 100644 command/agent/auth/token-file/token-file.go create mode 100644 command/agent/token_file_end_to_end_test.go diff --git a/api/lifetime_watcher.go b/api/lifetime_watcher.go index 5f3eadbffdd8..c3fd8b6dccc9 100644 --- a/api/lifetime_watcher.go +++ b/api/lifetime_watcher.go @@ -77,14 +77,15 @@ const ( type LifetimeWatcher struct { l sync.Mutex - client *Client - secret *Secret - grace time.Duration - random *rand.Rand - increment int - doneCh chan error - renewCh chan *RenewOutput - renewBehavior RenewBehavior + client *Client + secret *Secret + tokenRenewInfo *TokenRenewInfo + grace time.Duration + random *rand.Rand + increment int + doneCh chan error + renewCh chan *RenewOutput + renewBehavior RenewBehavior stopped bool stopCh chan struct{} @@ -93,11 +94,24 @@ type LifetimeWatcher struct { errLifetimeWatcherNoSecretData error } +// TokenRenewInfo is the information to be passed to the LifetimeWatcher +// for token_file auth +type TokenRenewInfo struct { + Renewable bool + LeaseDuration int + Token string +} + // LifetimeWatcherInput is used as input to the renew function. type LifetimeWatcherInput struct { // Secret is the secret to renew Secret *Secret + // TokenRenewInfo is for directly renewing a token that was + // not the product of an authentication method + // This will take precedence over secret, if provided. + TokenRenewInfo *TokenRenewInfo + // DEPRECATED: this does not do anything. Grace time.Duration @@ -156,13 +170,14 @@ func (c *Client) NewLifetimeWatcher(i *LifetimeWatcherInput) (*LifetimeWatcher, } return &LifetimeWatcher{ - client: c, - secret: secret, - increment: i.Increment, - random: random, - doneCh: make(chan error, 1), - renewCh: make(chan *RenewOutput, renewBuffer), - renewBehavior: i.RenewBehavior, + client: c, + secret: secret, + tokenRenewInfo: i.TokenRenewInfo, + increment: i.Increment, + random: random, + doneCh: make(chan error, 1), + renewCh: make(chan *RenewOutput, renewBuffer), + renewBehavior: i.RenewBehavior, stopped: false, stopCh: make(chan struct{}), @@ -221,7 +236,8 @@ func (r *LifetimeWatcher) Stop() { // Start starts a background process for watching the lifetime of this secret. // If renewal is enabled, when the secret has auth data, this attempts to renew // the auth (token); When the secret has a lease, this attempts to renew the -// lease. +// lease. If the token duration is 0 (infinite) we won't start the lifetime +// watcher, as there's nothing to renew. func (r *LifetimeWatcher) Start() { r.doneCh <- r.doRenew() } @@ -238,6 +254,10 @@ type renewFunc func(string, int) (*Secret, error) func (r *LifetimeWatcher) doRenew() error { defaultInitialRetryInterval := 10 * time.Second switch { + case r.tokenRenewInfo != nil: + return r.doRenewWithOptions(true, !r.tokenRenewInfo.Renewable, + r.tokenRenewInfo.LeaseDuration, r.tokenRenewInfo.Token, + r.client.Auth().Token().RenewTokenAsSelf, defaultInitialRetryInterval) case r.secret.Auth != nil: return r.doRenewWithOptions(true, !r.secret.Auth.Renewable, r.secret.Auth.LeaseDuration, r.secret.Auth.ClientToken, @@ -280,7 +300,6 @@ func (r *LifetimeWatcher) doRenewWithOptions(tokenMode bool, nonRenewable bool, // Can't or won't renew, just keep the same expiration so we exit // when it's reauthentication time remainingLeaseDuration = fallbackLeaseDuration - default: // Renew the token renewal, err = renew(credString, r.increment) diff --git a/command/agent.go b/command/agent.go index 4fbcd6ba278b..4afeefd3a505 100644 --- a/command/agent.go +++ b/command/agent.go @@ -16,6 +16,8 @@ import ( "sync" "time" + token_file "github.com/hashicorp/vault/command/agent/auth/token-file" + ctconfig "github.com/hashicorp/consul-template/config" "github.com/hashicorp/go-multierror" @@ -368,6 +370,8 @@ func (c *AgentCommand) Run(args []string) int { method, err = kubernetes.NewKubernetesAuthMethod(authConfig) case "approle": method, err = approle.NewApproleAuthMethod(authConfig) + case "token_file": + method, err = token_file.NewTokenFileAuthMethod(authConfig) case "pcf": // Deprecated. method, err = cf.NewCFAuthMethod(authConfig) default: diff --git a/command/agent/auth/auth.go b/command/agent/auth/auth.go index 854052adc5cb..6d1e86d8b7bc 100644 --- a/command/agent/auth/auth.go +++ b/command/agent/auth/auth.go @@ -254,9 +254,18 @@ func (ah *AuthHandler) Run(ctx context.Context, am AuthMethod) error { } // This should only happen if there's no preloaded token (regular auto-auth login) - // or if a preloaded token has expired and is now switching to auto-auth. + // or if a preloaded token has expired and is now switching to auto-auth. if secret.Auth == nil { - secret, err = clientToUse.Logical().WriteWithContext(ctx, path, data) + // TODO: Why do I keep getting 403s ;o; + isTokenFileMethod := path == "auth/token/lookup" + if isTokenFileMethod { + token, _ := data["token"].(string) + clientToUse.SetToken(token) + secret, err = clientToUse.Auth().Token().Lookup(token) + } else { + secret, err = clientToUse.Logical().WriteWithContext(ctx, path, data) + } + // Check errors/sanity if err != nil { ah.logger.Error("error authenticating", "error", err, "backoff", backoffCfg) @@ -269,6 +278,8 @@ func (ah *AuthHandler) Run(ctx context.Context, am AuthMethod) error { } } + var tokenRenewInfo *api.TokenRenewInfo + switch { case ah.wrapTTL > 0: if secret.WrapInfo == nil { @@ -319,28 +330,64 @@ func (ah *AuthHandler) Run(ctx context.Context, am AuthMethod) error { } default: - if secret == nil || secret.Auth == nil { - ah.logger.Error("authentication returned nil auth info", "backoff", backoffCfg) - metrics.IncrCounter([]string{"agent", "auth", "failure"}, 1) + // We handle the token_file method specially, as it's the only + // auth method that isn't actually authenticating, i.e. the secret + // returned does not have an Auth struct attached + isTokenFileMethod := path == "auth/token/lookup" + if isTokenFileMethod { + // We still check the response of the request to ensure the token is valid + // i.e. if the token is invalid, we will fail in the authentication step + if secret == nil || secret.Data == nil { + ah.logger.Error("token file validation failed, token may be invalid", "backoff", backoffCfg) + metrics.IncrCounter([]string{"agent", "auth", "failure"}, 1) - if backoff(ctx, backoffCfg) { - continue + if backoff(ctx, backoffCfg) { + continue + } + return err } - return err - } - if secret.Auth.ClientToken == "" { - ah.logger.Error("authentication returned empty client token", "backoff", backoffCfg) - metrics.IncrCounter([]string{"agent", "auth", "failure"}, 1) + token, ok := secret.Data["id"].(string) + if !ok || token == "" { + ah.logger.Error("token file validation returned empty client token", "backoff", backoffCfg) + metrics.IncrCounter([]string{"agent", "auth", "failure"}, 1) - if backoff(ctx, backoffCfg) { - continue + if backoff(ctx, backoffCfg) { + continue + } + return err + } + + tokenRenewInfo = getTokenRenewInfoFromTokenLookupResponse(secret) + ah.logger.Info("authentication successful, sending token to sinks") + ah.OutputCh <- token + if ah.enableTemplateTokenCh { + ah.TemplateTokenCh <- token + } + } else { + if secret == nil || secret.Auth == nil { + ah.logger.Error("authentication returned nil auth info", "backoff", backoffCfg) + metrics.IncrCounter([]string{"agent", "auth", "failure"}, 1) + + if backoff(ctx, backoffCfg) { + continue + } + return err + } + if secret.Auth.ClientToken == "" { + ah.logger.Error("authentication returned empty client token", "backoff", backoffCfg) + metrics.IncrCounter([]string{"agent", "auth", "failure"}, 1) + + if backoff(ctx, backoffCfg) { + continue + } + return err + } + + ah.logger.Info("authentication successful, sending token to sinks") + ah.OutputCh <- secret.Auth.ClientToken + if ah.enableTemplateTokenCh { + ah.TemplateTokenCh <- secret.Auth.ClientToken } - return err - } - ah.logger.Info("authentication successful, sending token to sinks") - ah.OutputCh <- secret.Auth.ClientToken - if ah.enableTemplateTokenCh { - ah.TemplateTokenCh <- secret.Auth.ClientToken } am.CredSuccess() @@ -352,7 +399,8 @@ func (ah *AuthHandler) Run(ctx context.Context, am AuthMethod) error { } watcher, err = clientToUse.NewLifetimeWatcher(&api.LifetimeWatcherInput{ - Secret: secret, + Secret: secret, + TokenRenewInfo: tokenRenewInfo, }) if err != nil { ah.logger.Error("error creating lifetime watcher", "error", err, "backoff", backoffCfg) @@ -364,7 +412,6 @@ func (ah *AuthHandler) Run(ctx context.Context, am AuthMethod) error { return err } - // Start the renewal process ah.logger.Info("starting renewal process") metrics.IncrCounter([]string{"agent", "auth", "success"}, 1) go watcher.Renew() @@ -397,6 +444,20 @@ func (ah *AuthHandler) Run(ctx context.Context, am AuthMethod) error { } } +func getTokenRenewInfoFromTokenLookupResponse(secret *api.Secret) *api.TokenRenewInfo { + if secret == nil || secret.Data == nil { + return nil + } + renewable, _ := secret.Data["renewable"].(bool) + leaseDuration, _ := secret.Data["ttl"].(int) + token, _ := secret.Data["id"].(string) + return &api.TokenRenewInfo{ + Renewable: renewable, + LeaseDuration: leaseDuration, + Token: token, + } +} + // agentBackoff tracks exponential backoff state. type agentBackoff struct { min time.Duration diff --git a/command/agent/auth/kubernetes/kubernetes.go b/command/agent/auth/kubernetes/kubernetes.go index c30f3cb5a68b..3e41f4353696 100644 --- a/command/agent/auth/kubernetes/kubernetes.go +++ b/command/agent/auth/kubernetes/kubernetes.go @@ -127,3 +127,10 @@ func (k *kubernetesMethod) readJWT() (string, error) { return strings.TrimSpace(string(contentBytes)), nil } + +func (k *kubernetesMethod) extractTokenFromRequest(secret *api.Secret) string { + if secret == nil || secret.Auth == nil { + return secret.Auth.ClientToken + } + return "" +} diff --git a/command/agent/auth/token-file/token-file.go b/command/agent/auth/token-file/token-file.go new file mode 100644 index 000000000000..31070456c1e6 --- /dev/null +++ b/command/agent/auth/token-file/token-file.go @@ -0,0 +1,117 @@ +package token_file + +import ( + "context" + "errors" + "fmt" + "net/http" + "os" + "strings" + + "github.com/hashicorp/go-secure-stdlib/parseutil" + + "github.com/hashicorp/vault/api" + + "github.com/hashicorp/vault/command/agent/auth" + + "github.com/hashicorp/go-hclog" +) + +type TokenFileMethod struct { + logger hclog.Logger + mountPath string + + cachedToken string + tokenFilePath string + followSymlinks bool + removeTokenFileAfterReading bool +} + +func NewTokenFileAuthMethod(conf *auth.AuthConfig) (auth.AuthMethod, error) { + if conf == nil { + return nil, errors.New("empty config") + } + if conf.Config == nil { + return nil, errors.New("empty config data") + } + + a := &TokenFileMethod{ + logger: conf.Logger, + mountPath: "auth/token", + } + + // TODO should we default to ~/.vault-token? + tokenFilePathRaw, ok := conf.Config["token_file_path"] + if !ok { + return nil, errors.New("missing 'token_file_path' value") + } + a.tokenFilePath, ok = tokenFilePathRaw.(string) + if !ok { + return nil, errors.New("could not convert 'token_file_path' config value to string") + } + if a.tokenFilePath == "" { + return nil, errors.New("'token_file_path' value is empty") + } + + removeTokenFileAfterReadingRaw, ok := conf.Config["remove_token_file_after_reading"] + if ok { + removeTokenFileAfterReading, err := parseutil.ParseBool(removeTokenFileAfterReadingRaw) + if err != nil { + return nil, fmt.Errorf("error parsing 'remove_token_file_after_reading' value: %w", err) + } + a.removeTokenFileAfterReading = removeTokenFileAfterReading + } + + followSymlinksRaw, ok := conf.Config["follow_symlinks"] + if ok { + followSymlinks, err := parseutil.ParseBool(followSymlinksRaw) + if err != nil { + return nil, fmt.Errorf("error parsing 'follow_symlinks' value: %w", err) + } + a.followSymlinks = followSymlinks + } + + return a, nil +} + +func (a *TokenFileMethod) Authenticate(ctx context.Context, client *api.Client) (string, http.Header, map[string]interface{}, error) { + // Lstat = symlink safe + if _, err := os.Stat(a.tokenFilePath); err == nil { + // filepath.evalsymlinks on the path + token, err := os.ReadFile(a.tokenFilePath) + if err != nil { + if a.cachedToken == "" { + return "", nil, nil, fmt.Errorf("error reading token file and no cached role ID known: %w", err) + } + a.logger.Warn("error reading token file", "error", err) + } + if len(token) == 0 { + if a.cachedToken == "" { + return "", nil, nil, errors.New("token file empty and no cached token known") + } + a.logger.Warn("role ID file exists but read empty value, re-using cached value") + } else { + a.cachedToken = strings.TrimSpace(string(token)) + } + } + + // i.e. auth/token/lookup + return fmt.Sprintf("%s/lookup", a.mountPath), nil, map[string]interface{}{ + "token": a.cachedToken, + }, nil +} + +func (a *TokenFileMethod) NewCreds() chan struct{} { + return nil +} + +func (a *TokenFileMethod) CredSuccess() { +} + +func (a *TokenFileMethod) Shutdown() { +} + +func IsAuthMethodTokenFile(am auth.AuthMethod) bool { + _, ok := am.(*TokenFileMethod) + return ok +} diff --git a/command/agent/token_file_end_to_end_test.go b/command/agent/token_file_end_to_end_test.go new file mode 100644 index 000000000000..e95da864b287 --- /dev/null +++ b/command/agent/token_file_end_to_end_test.go @@ -0,0 +1,193 @@ +package agent + +import ( + "context" + "fmt" + "os" + "testing" + "time" + + "github.com/hashicorp/vault/command/agent/sink" + "github.com/hashicorp/vault/command/agent/sink/file" + + token_file "github.com/hashicorp/vault/command/agent/auth/token-file" + + log "github.com/hashicorp/go-hclog" + "github.com/hashicorp/vault/command/agent/auth" + vaulthttp "github.com/hashicorp/vault/http" + "github.com/hashicorp/vault/sdk/helper/logging" + "github.com/hashicorp/vault/vault" +) + +func TestTokenFileEndToEnd(t *testing.T) { + t.Parallel() + + testCases := []struct { + removeTokenFile bool + expectToken bool + }{ + // default behaviour => token expected + {false, true}, + {true, true}, + } + + for _, tc := range testCases { + secretFileAction := "preserve" + if tc.removeTokenFile { + secretFileAction = "remove" + } + tc := tc // capture range variable + t.Run(fmt.Sprintf("%s_removeTokenFile, expectToken=%v", secretFileAction, tc.expectToken), func(t *testing.T) { + t.Parallel() + testTokenFileEndToEnd(t, tc.removeTokenFile, tc.expectToken) + }) + } +} + +func testTokenFileEndToEnd(t *testing.T, removeTokenFile bool, expectToken bool) { + var err error + logger := logging.NewVaultLogger(log.Trace) + coreConfig := &vault.CoreConfig{ + DisableMlock: true, + DisableCache: true, + Logger: log.NewNullLogger(), + } + + cluster := vault.NewTestCluster(t, coreConfig, &vault.TestClusterOptions{ + HandlerFunc: vaulthttp.Handler, + }) + + cluster.Start() + defer cluster.Cleanup() + + cores := cluster.Cores + + vault.TestWaitActive(t, cores[0].Core) + + client := cores[0].Client + + secret, err := client.Auth().Token().Create(nil) + if err != nil || secret == nil { + t.Fatal(err) + } + + tokenFile, err := os.CreateTemp("", "token_file") + if err != nil { + t.Fatal(err) + } + tokenFileName := tokenFile.Name() + tokenFile.Close() // WriteFile doesn't need it open + os.WriteFile(tokenFileName, []byte(secret.Auth.ClientToken), 0o666) + // os.WriteFile(tokenFileName, []byte(client.Token()), 0o666) + defer os.Remove(tokenFileName) + + t.Logf("input token_file_path: %s", tokenFileName) + + ahConfig := &auth.AuthHandlerConfig{ + Logger: logger.Named("auth.handler"), + Client: client, + } + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + + am, err := token_file.NewTokenFileAuthMethod(&auth.AuthConfig{ + Logger: logger.Named("auth.method"), + Config: map[string]interface{}{ + "token_file_path": tokenFileName, + }, + }) + if err != nil { + t.Fatal(err) + } + + ah := auth.NewAuthHandler(ahConfig) + errCh := make(chan error) + go func() { + errCh <- ah.Run(ctx, am) + }() + defer func() { + select { + case <-ctx.Done(): + case err := <-errCh: + if err != nil { + t.Fatal(err) + } + } + }() + + // We close these right away because we're just basically testing + // permissions and finding a usable file name + sinkFile, err := os.CreateTemp("", "auth.tokensink.test.") + if err != nil { + t.Fatal(err) + } + tokenSinkFileName := sinkFile.Name() + sinkFile.Close() + os.Remove(tokenSinkFileName) + t.Logf("output: %s", tokenSinkFileName) + + config := &sink.SinkConfig{ + Logger: logger.Named("sink.file"), + Config: map[string]interface{}{ + "path": tokenSinkFileName, + }, + WrapTTL: 10 * time.Second, + } + + fs, err := file.NewFileSink(config) + if err != nil { + t.Fatal(err) + } + config.Sink = fs + + ss := sink.NewSinkServer(&sink.SinkServerConfig{ + Logger: logger.Named("sink.server"), + Client: client, + }) + go func() { + errCh <- ss.Run(ctx, ah.OutputCh, []*sink.SinkConfig{config}) + }() + defer func() { + select { + case <-ctx.Done(): + case err := <-errCh: + if err != nil { + t.Fatal(err) + } + } + }() + + // This has to be after the other defers, so it happens first. It allows + // successful test runs to immediately cancel all of the runner goroutines + // and unblock any of the blocking defer calls by the runner's DoneCh that + // comes before this and avoid successful tests from taking the entire + // timeout duration. + defer cancel() + + if stat, err := os.Lstat(tokenSinkFileName); err == nil { + t.Fatalf("expected err but got %s", stat) + } else if !os.IsNotExist(err) { + t.Fatal("expected notexist err") + } + + if expectToken { + // Wait 2 seconds for the env variables to be detected and an auth to be generated. + time.Sleep(time.Second * 2) + + token, err := readToken(tokenSinkFileName) + if err != nil { + t.Fatal(err) + } + + if token.Token == "" { + t.Fatal("expected token but didn't receive it") + } + } + + if !removeTokenFile { + _, err := os.Stat(tokenFileName) + if err != nil { + t.Fatal("Token file removed despite remove token file being set to false") + } + } +} From a5cb0bfdb021e7460addaeff96f6436b9a29ae1a Mon Sep 17 00:00:00 2001 From: VioletHynes Date: Mon, 16 Jan 2023 16:36:17 -0500 Subject: [PATCH 02/14] VAULT-12564 remove lifetime watcher struct modifications --- api/lifetime_watcher.go | 41 ++++++++++++++------------------------ command/agent/auth/auth.go | 17 ++++++++++------ 2 files changed, 26 insertions(+), 32 deletions(-) diff --git a/api/lifetime_watcher.go b/api/lifetime_watcher.go index c3fd8b6dccc9..8d300602fc40 100644 --- a/api/lifetime_watcher.go +++ b/api/lifetime_watcher.go @@ -77,15 +77,14 @@ const ( type LifetimeWatcher struct { l sync.Mutex - client *Client - secret *Secret - tokenRenewInfo *TokenRenewInfo - grace time.Duration - random *rand.Rand - increment int - doneCh chan error - renewCh chan *RenewOutput - renewBehavior RenewBehavior + client *Client + secret *Secret + grace time.Duration + random *rand.Rand + increment int + doneCh chan error + renewCh chan *RenewOutput + renewBehavior RenewBehavior stopped bool stopCh chan struct{} @@ -107,11 +106,6 @@ type LifetimeWatcherInput struct { // Secret is the secret to renew Secret *Secret - // TokenRenewInfo is for directly renewing a token that was - // not the product of an authentication method - // This will take precedence over secret, if provided. - TokenRenewInfo *TokenRenewInfo - // DEPRECATED: this does not do anything. Grace time.Duration @@ -170,14 +164,13 @@ func (c *Client) NewLifetimeWatcher(i *LifetimeWatcherInput) (*LifetimeWatcher, } return &LifetimeWatcher{ - client: c, - secret: secret, - tokenRenewInfo: i.TokenRenewInfo, - increment: i.Increment, - random: random, - doneCh: make(chan error, 1), - renewCh: make(chan *RenewOutput, renewBuffer), - renewBehavior: i.RenewBehavior, + client: c, + secret: secret, + increment: i.Increment, + random: random, + doneCh: make(chan error, 1), + renewCh: make(chan *RenewOutput, renewBuffer), + renewBehavior: i.RenewBehavior, stopped: false, stopCh: make(chan struct{}), @@ -254,10 +247,6 @@ type renewFunc func(string, int) (*Secret, error) func (r *LifetimeWatcher) doRenew() error { defaultInitialRetryInterval := 10 * time.Second switch { - case r.tokenRenewInfo != nil: - return r.doRenewWithOptions(true, !r.tokenRenewInfo.Renewable, - r.tokenRenewInfo.LeaseDuration, r.tokenRenewInfo.Token, - r.client.Auth().Token().RenewTokenAsSelf, defaultInitialRetryInterval) case r.secret.Auth != nil: return r.doRenewWithOptions(true, !r.secret.Auth.Renewable, r.secret.Auth.LeaseDuration, r.secret.Auth.ClientToken, diff --git a/command/agent/auth/auth.go b/command/agent/auth/auth.go index 6d1e86d8b7bc..16c837622f39 100644 --- a/command/agent/auth/auth.go +++ b/command/agent/auth/auth.go @@ -256,7 +256,8 @@ func (ah *AuthHandler) Run(ctx context.Context, am AuthMethod) error { // This should only happen if there's no preloaded token (regular auto-auth login) // or if a preloaded token has expired and is now switching to auto-auth. if secret.Auth == nil { - // TODO: Why do I keep getting 403s ;o; + // TODO: Sometimes we get 404s if the file in ~/.vault-token + // uses an out of date token isTokenFileMethod := path == "auth/token/lookup" if isTokenFileMethod { token, _ := data["token"].(string) @@ -278,8 +279,6 @@ func (ah *AuthHandler) Run(ctx context.Context, am AuthMethod) error { } } - var tokenRenewInfo *api.TokenRenewInfo - switch { case ah.wrapTTL > 0: if secret.WrapInfo == nil { @@ -357,7 +356,13 @@ func (ah *AuthHandler) Run(ctx context.Context, am AuthMethod) error { return err } - tokenRenewInfo = getTokenRenewInfoFromTokenLookupResponse(secret) + duration, _ := secret.Data["ttl"].(json.Number).Int64() + renewable, _ := secret.Data["renewable"].(bool) + secret.Auth = &api.SecretAuth{ + ClientToken: token, + LeaseDuration: int(duration), + Renewable: renewable, + } ah.logger.Info("authentication successful, sending token to sinks") ah.OutputCh <- token if ah.enableTemplateTokenCh { @@ -399,8 +404,8 @@ func (ah *AuthHandler) Run(ctx context.Context, am AuthMethod) error { } watcher, err = clientToUse.NewLifetimeWatcher(&api.LifetimeWatcherInput{ - Secret: secret, - TokenRenewInfo: tokenRenewInfo, + Secret: secret, + // TokenRenewInfo: tokenRenewInfo, }) if err != nil { ah.logger.Error("error creating lifetime watcher", "error", err, "backoff", backoffCfg) From 270d349f5a92166194010b4558a209b0a8e8d6d8 Mon Sep 17 00:00:00 2001 From: VioletHynes Date: Tue, 17 Jan 2023 14:14:56 -0500 Subject: [PATCH 03/14] VAULT-12564 add other config items, and clean up --- command/agent/auth/auth.go | 32 ++++++-------- command/agent/auth/token-file/token-file.go | 49 ++++++++++++--------- command/agent/token_file_end_to_end_test.go | 11 ++++- 3 files changed, 51 insertions(+), 41 deletions(-) diff --git a/command/agent/auth/auth.go b/command/agent/auth/auth.go index 16c837622f39..cf9a53b6ecaf 100644 --- a/command/agent/auth/auth.go +++ b/command/agent/auth/auth.go @@ -169,6 +169,7 @@ func (ah *AuthHandler) Run(ctx context.Context, am AuthMethod) error { var path string var data map[string]interface{} var header http.Header + var isTokenFileMethod bool switch am.(type) { case AuthMethodWithClient: @@ -258,7 +259,7 @@ func (ah *AuthHandler) Run(ctx context.Context, am AuthMethod) error { if secret.Auth == nil { // TODO: Sometimes we get 404s if the file in ~/.vault-token // uses an out of date token - isTokenFileMethod := path == "auth/token/lookup" + isTokenFileMethod = path == "auth/token/lookup" if isTokenFileMethod { token, _ := data["token"].(string) clientToUse.SetToken(token) @@ -279,6 +280,8 @@ func (ah *AuthHandler) Run(ctx context.Context, am AuthMethod) error { } } + var leaseDuration int + switch { case ah.wrapTTL > 0: if secret.WrapInfo == nil { @@ -357,6 +360,7 @@ func (ah *AuthHandler) Run(ctx context.Context, am AuthMethod) error { } duration, _ := secret.Data["ttl"].(json.Number).Int64() + leaseDuration = int(duration) renewable, _ := secret.Data["renewable"].(bool) secret.Auth = &api.SecretAuth{ ClientToken: token, @@ -388,6 +392,7 @@ func (ah *AuthHandler) Run(ctx context.Context, am AuthMethod) error { return err } + leaseDuration = secret.LeaseDuration ah.logger.Info("authentication successful, sending token to sinks") ah.OutputCh <- secret.Auth.ClientToken if ah.enableTemplateTokenCh { @@ -405,7 +410,6 @@ func (ah *AuthHandler) Run(ctx context.Context, am AuthMethod) error { watcher, err = clientToUse.NewLifetimeWatcher(&api.LifetimeWatcherInput{ Secret: secret, - // TokenRenewInfo: tokenRenewInfo, }) if err != nil { ah.logger.Error("error creating lifetime watcher", "error", err, "backoff", backoffCfg) @@ -417,9 +421,15 @@ func (ah *AuthHandler) Run(ctx context.Context, am AuthMethod) error { return err } - ah.logger.Info("starting renewal process") metrics.IncrCounter([]string{"agent", "auth", "success"}, 1) - go watcher.Renew() + // We don't want to trigger the renewal process for tokens with + // unlimited TTL, such as the root token. + if leaseDuration == 0 && isTokenFileMethod { + ah.logger.Info("not starting token renewal process, as token has unlimited TTL") + } else { + ah.logger.Info("starting renewal process") + go watcher.Renew() + } LifetimeWatcherLoop: for { @@ -449,20 +459,6 @@ func (ah *AuthHandler) Run(ctx context.Context, am AuthMethod) error { } } -func getTokenRenewInfoFromTokenLookupResponse(secret *api.Secret) *api.TokenRenewInfo { - if secret == nil || secret.Data == nil { - return nil - } - renewable, _ := secret.Data["renewable"].(bool) - leaseDuration, _ := secret.Data["ttl"].(int) - token, _ := secret.Data["id"].(string) - return &api.TokenRenewInfo{ - Renewable: renewable, - LeaseDuration: leaseDuration, - Token: token, - } -} - // agentBackoff tracks exponential backoff state. type agentBackoff struct { min time.Duration diff --git a/command/agent/auth/token-file/token-file.go b/command/agent/auth/token-file/token-file.go index 31070456c1e6..c96e184c9273 100644 --- a/command/agent/auth/token-file/token-file.go +++ b/command/agent/auth/token-file/token-file.go @@ -40,7 +40,6 @@ func NewTokenFileAuthMethod(conf *auth.AuthConfig) (auth.AuthMethod, error) { mountPath: "auth/token", } - // TODO should we default to ~/.vault-token? tokenFilePathRaw, ok := conf.Config["token_file_path"] if !ok { return nil, errors.New("missing 'token_file_path' value") @@ -75,23 +74,34 @@ func NewTokenFileAuthMethod(conf *auth.AuthConfig) (auth.AuthMethod, error) { } func (a *TokenFileMethod) Authenticate(ctx context.Context, client *api.Client) (string, http.Header, map[string]interface{}, error) { - // Lstat = symlink safe - if _, err := os.Stat(a.tokenFilePath); err == nil { - // filepath.evalsymlinks on the path - token, err := os.ReadFile(a.tokenFilePath) - if err != nil { - if a.cachedToken == "" { - return "", nil, nil, fmt.Errorf("error reading token file and no cached role ID known: %w", err) - } - a.logger.Warn("error reading token file", "error", err) + _, err := os.Stat(a.tokenFilePath) + if err != nil { + if a.cachedToken == "" { + return "", nil, nil, fmt.Errorf("token file not found and no cached token known: %w", err) + } + a.logger.Warn("token file not found", "error", err) + } + + token, err := os.ReadFile(a.tokenFilePath) + if err != nil { + if a.cachedToken == "" { + return "", nil, nil, fmt.Errorf("error reading token file and no cached token known: %w", err) + } + a.logger.Warn("error reading token file", "error", err) + } + if len(token) == 0 { + if a.cachedToken == "" { + return "", nil, nil, errors.New("token file empty and no cached token known") } - if len(token) == 0 { - if a.cachedToken == "" { - return "", nil, nil, errors.New("token file empty and no cached token known") - } - a.logger.Warn("role ID file exists but read empty value, re-using cached value") - } else { - a.cachedToken = strings.TrimSpace(string(token)) + a.logger.Warn("role ID file exists but read empty value, re-using cached value") + } else { + a.cachedToken = strings.TrimSpace(string(token)) + } + + if a.removeTokenFileAfterReading { + a.logger.Info("removing token file after reading, because 'remove_token_file_after_reading' is true") + if err := os.Remove(a.tokenFilePath); err != nil { + a.logger.Error("error removing token file after reading", "error", err) } } @@ -110,8 +120,3 @@ func (a *TokenFileMethod) CredSuccess() { func (a *TokenFileMethod) Shutdown() { } - -func IsAuthMethodTokenFile(am auth.AuthMethod) bool { - _, ok := am.(*TokenFileMethod) - return ok -} diff --git a/command/agent/token_file_end_to_end_test.go b/command/agent/token_file_end_to_end_test.go index e95da864b287..0696ed0642a0 100644 --- a/command/agent/token_file_end_to_end_test.go +++ b/command/agent/token_file_end_to_end_test.go @@ -93,7 +93,8 @@ func testTokenFileEndToEnd(t *testing.T, removeTokenFile bool, expectToken bool) am, err := token_file.NewTokenFileAuthMethod(&auth.AuthConfig{ Logger: logger.Named("auth.method"), Config: map[string]interface{}{ - "token_file_path": tokenFileName, + "token_file_path": tokenFileName, + "remove_token_file_after_reading": removeTokenFile, }, }) if err != nil { @@ -189,5 +190,13 @@ func testTokenFileEndToEnd(t *testing.T, removeTokenFile bool, expectToken bool) if err != nil { t.Fatal("Token file removed despite remove token file being set to false") } + } else { + _, err := os.Stat(tokenFileName) + if err == nil { + t.Fatal("no error returned from stat, indicating the file is still present") + } + if !os.IsNotExist(err) { + t.Fatalf("unexpected error: %v", err) + } } } From f70b1b39b411dc272aa554b63c2f53d1b3baac86 Mon Sep 17 00:00:00 2001 From: VioletHynes Date: Tue, 17 Jan 2023 15:27:58 -0500 Subject: [PATCH 04/14] VAULT-12564 clean-up and more tests --- command/agent/auth/auth.go | 2 - .../{token-file.go => token_file.go} | 0 .../agent/auth/token-file/token_file_test.go | 146 ++++++++++++++++++ command/agent/token_file_end_to_end_test.go | 1 - 4 files changed, 146 insertions(+), 3 deletions(-) rename command/agent/auth/token-file/{token-file.go => token_file.go} (100%) create mode 100644 command/agent/auth/token-file/token_file_test.go diff --git a/command/agent/auth/auth.go b/command/agent/auth/auth.go index cf9a53b6ecaf..3fa6016008c6 100644 --- a/command/agent/auth/auth.go +++ b/command/agent/auth/auth.go @@ -257,8 +257,6 @@ func (ah *AuthHandler) Run(ctx context.Context, am AuthMethod) error { // This should only happen if there's no preloaded token (regular auto-auth login) // or if a preloaded token has expired and is now switching to auto-auth. if secret.Auth == nil { - // TODO: Sometimes we get 404s if the file in ~/.vault-token - // uses an out of date token isTokenFileMethod = path == "auth/token/lookup" if isTokenFileMethod { token, _ := data["token"].(string) diff --git a/command/agent/auth/token-file/token-file.go b/command/agent/auth/token-file/token_file.go similarity index 100% rename from command/agent/auth/token-file/token-file.go rename to command/agent/auth/token-file/token_file.go diff --git a/command/agent/auth/token-file/token_file_test.go b/command/agent/auth/token-file/token_file_test.go new file mode 100644 index 000000000000..2c1398210198 --- /dev/null +++ b/command/agent/auth/token-file/token_file_test.go @@ -0,0 +1,146 @@ +package token_file + +import ( + "os" + "testing" + + log "github.com/hashicorp/go-hclog" + "github.com/hashicorp/vault/sdk/helper/logging" + + "github.com/hashicorp/vault/command/agent/auth" +) + +func TestNewTokenFileAuthMethodEmptyConfig(t *testing.T) { + logger := logging.NewVaultLogger(log.Trace) + _, err := NewTokenFileAuthMethod(&auth.AuthConfig{ + Logger: logger.Named("auth.method"), + Config: map[string]interface{}{}, + }) + if err == nil { + t.Fatal("Expected error due to empty config") + } +} + +func TestNewTokenFileRemoveTokenString(t *testing.T) { + logger := logging.NewVaultLogger(log.Trace) + _, err := NewTokenFileAuthMethod(&auth.AuthConfig{ + Logger: logger.Named("auth.method"), + Config: map[string]interface{}{ + "token_file_path": "/some/path", + "remove_token_file_after_reading": "string", + }, + }) + if err == nil { + t.Fatalf("Expected error when giving string for bool") + } +} + +func TestNewTokenFileEmptyFilePath(t *testing.T) { + logger := logging.NewVaultLogger(log.Trace) + _, err := NewTokenFileAuthMethod(&auth.AuthConfig{ + Logger: logger.Named("auth.method"), + Config: map[string]interface{}{ + "token_file_path": "", + }, + }) + if err == nil { + t.Fatalf("Expected error when giving empty file path") + } +} + +func TestNewTokenFileAuthenticate(t *testing.T) { + tokenFile, err := os.CreateTemp("", "token_file") + tokenFileContents := "super-secret-token" + if err != nil { + t.Fatal(err) + } + tokenFileName := tokenFile.Name() + tokenFile.Close() // WriteFile doesn't need it open + os.WriteFile(tokenFileName, []byte(tokenFileContents), 0o666) + defer os.Remove(tokenFileName) + + logger := logging.NewVaultLogger(log.Trace) + am, err := NewTokenFileAuthMethod(&auth.AuthConfig{ + Logger: logger.Named("auth.method"), + Config: map[string]interface{}{ + "token_file_path": tokenFileName, + "remove_token_file_after_reading": false, + }, + }) + if err != nil { + t.Fatal(err) + } + + path, headers, data, err := am.Authenticate(nil, nil) + if err != nil { + t.Fatal(err) + } + if path != "auth/token/lookup" { + t.Fatalf("Incorrect path, was %s", path) + } + if headers != nil { + t.Fatalf("Expected no headers, instead got %v", headers) + } + if data == nil { + t.Fatal("Data was nil") + } + tokenDataFromAuthMethod := data["token"].(string) + if tokenDataFromAuthMethod != tokenFileContents { + t.Fatalf("Incorrect token file contents return by auth method, expected %s, got %s", tokenFileContents, tokenDataFromAuthMethod) + } + + _, err = os.Stat(tokenFileName) + if err != nil { + t.Fatal("Token file removed despite remove token file being set to false") + } +} + +func TestNewTokenFileAuthenticateRemoveAfterReading(t *testing.T) { + tokenFile, err := os.CreateTemp("", "token_file") + tokenFileContents := "super-secret-token" + if err != nil { + t.Fatal(err) + } + tokenFileName := tokenFile.Name() + tokenFile.Close() // WriteFile doesn't need it open + os.WriteFile(tokenFileName, []byte(tokenFileContents), 0o666) + defer os.Remove(tokenFileName) + + logger := logging.NewVaultLogger(log.Trace) + am, err := NewTokenFileAuthMethod(&auth.AuthConfig{ + Logger: logger.Named("auth.method"), + Config: map[string]interface{}{ + "token_file_path": tokenFileName, + "remove_token_file_after_reading": true, + }, + }) + if err != nil { + t.Fatal(err) + } + + path, headers, data, err := am.Authenticate(nil, nil) + if err != nil { + t.Fatal(err) + } + if path != "auth/token/lookup" { + t.Fatalf("Incorrect path, was %s", path) + } + if headers != nil { + t.Fatalf("Expected no headers, instead got %v", headers) + } + if data == nil { + t.Fatal("Data was nil") + } + tokenDataFromAuthMethod := data["token"].(string) + if tokenDataFromAuthMethod != tokenFileContents { + t.Fatalf("Incorrect token file contents return by auth method, expected %s, got %s", tokenFileContents, tokenDataFromAuthMethod) + } + + _, err = os.Stat(tokenFileName) + if err == nil { + t.Fatal("no error returned from stat, indicating the file is still present") + } + if !os.IsNotExist(err) { + t.Fatalf("unexpected error: %v", err) + } +} diff --git a/command/agent/token_file_end_to_end_test.go b/command/agent/token_file_end_to_end_test.go index 0696ed0642a0..43f836f6121e 100644 --- a/command/agent/token_file_end_to_end_test.go +++ b/command/agent/token_file_end_to_end_test.go @@ -78,7 +78,6 @@ func testTokenFileEndToEnd(t *testing.T, removeTokenFile bool, expectToken bool) tokenFileName := tokenFile.Name() tokenFile.Close() // WriteFile doesn't need it open os.WriteFile(tokenFileName, []byte(secret.Auth.ClientToken), 0o666) - // os.WriteFile(tokenFileName, []byte(client.Token()), 0o666) defer os.Remove(tokenFileName) t.Logf("input token_file_path: %s", tokenFileName) From 3f3f25e9e3909d3c82f268d5ff883b062e3a0e4e Mon Sep 17 00:00:00 2001 From: VioletHynes Date: Tue, 17 Jan 2023 15:32:34 -0500 Subject: [PATCH 05/14] VAULT-12564 clean-up --- api/lifetime_watcher.go | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/api/lifetime_watcher.go b/api/lifetime_watcher.go index 8d300602fc40..5f3eadbffdd8 100644 --- a/api/lifetime_watcher.go +++ b/api/lifetime_watcher.go @@ -93,14 +93,6 @@ type LifetimeWatcher struct { errLifetimeWatcherNoSecretData error } -// TokenRenewInfo is the information to be passed to the LifetimeWatcher -// for token_file auth -type TokenRenewInfo struct { - Renewable bool - LeaseDuration int - Token string -} - // LifetimeWatcherInput is used as input to the renew function. type LifetimeWatcherInput struct { // Secret is the secret to renew @@ -229,8 +221,7 @@ func (r *LifetimeWatcher) Stop() { // Start starts a background process for watching the lifetime of this secret. // If renewal is enabled, when the secret has auth data, this attempts to renew // the auth (token); When the secret has a lease, this attempts to renew the -// lease. If the token duration is 0 (infinite) we won't start the lifetime -// watcher, as there's nothing to renew. +// lease. func (r *LifetimeWatcher) Start() { r.doneCh <- r.doRenew() } @@ -289,6 +280,7 @@ func (r *LifetimeWatcher) doRenewWithOptions(tokenMode bool, nonRenewable bool, // Can't or won't renew, just keep the same expiration so we exit // when it's reauthentication time remainingLeaseDuration = fallbackLeaseDuration + default: // Renew the token renewal, err = renew(credString, r.increment) From 1ad92d413c7b2cd93385ab6d29144f77004758d9 Mon Sep 17 00:00:00 2001 From: VioletHynes Date: Tue, 17 Jan 2023 15:41:58 -0500 Subject: [PATCH 06/14] VAULT-12564 lookup-self and some clean-up --- command/agent/auth/auth.go | 6 +++--- command/agent/auth/token-file/token_file.go | 9 +++------ command/agent/auth/token-file/token_file_test.go | 7 +++---- command/agent/token_file_end_to_end_test.go | 8 +++----- 4 files changed, 12 insertions(+), 18 deletions(-) diff --git a/command/agent/auth/auth.go b/command/agent/auth/auth.go index 3fa6016008c6..52332e6854f0 100644 --- a/command/agent/auth/auth.go +++ b/command/agent/auth/auth.go @@ -257,11 +257,11 @@ func (ah *AuthHandler) Run(ctx context.Context, am AuthMethod) error { // This should only happen if there's no preloaded token (regular auto-auth login) // or if a preloaded token has expired and is now switching to auto-auth. if secret.Auth == nil { - isTokenFileMethod = path == "auth/token/lookup" + isTokenFileMethod = path == "auth/token/lookup-self" if isTokenFileMethod { token, _ := data["token"].(string) clientToUse.SetToken(token) - secret, err = clientToUse.Auth().Token().Lookup(token) + secret, err = clientToUse.Auth().Token().LookupSelf() } else { secret, err = clientToUse.Logical().WriteWithContext(ctx, path, data) } @@ -333,7 +333,7 @@ func (ah *AuthHandler) Run(ctx context.Context, am AuthMethod) error { // We handle the token_file method specially, as it's the only // auth method that isn't actually authenticating, i.e. the secret // returned does not have an Auth struct attached - isTokenFileMethod := path == "auth/token/lookup" + isTokenFileMethod := path == "auth/token/lookup-self" if isTokenFileMethod { // We still check the response of the request to ensure the token is valid // i.e. if the token is invalid, we will fail in the authentication step diff --git a/command/agent/auth/token-file/token_file.go b/command/agent/auth/token-file/token_file.go index c96e184c9273..2d5328330c66 100644 --- a/command/agent/auth/token-file/token_file.go +++ b/command/agent/auth/token-file/token_file.go @@ -8,13 +8,10 @@ import ( "os" "strings" + "github.com/hashicorp/go-hclog" "github.com/hashicorp/go-secure-stdlib/parseutil" - "github.com/hashicorp/vault/api" - "github.com/hashicorp/vault/command/agent/auth" - - "github.com/hashicorp/go-hclog" ) type TokenFileMethod struct { @@ -105,8 +102,8 @@ func (a *TokenFileMethod) Authenticate(ctx context.Context, client *api.Client) } } - // i.e. auth/token/lookup - return fmt.Sprintf("%s/lookup", a.mountPath), nil, map[string]interface{}{ + // i.e. auth/token/lookup-self + return fmt.Sprintf("%s/lookup-self", a.mountPath), nil, map[string]interface{}{ "token": a.cachedToken, }, nil } diff --git a/command/agent/auth/token-file/token_file_test.go b/command/agent/auth/token-file/token_file_test.go index 2c1398210198..0d1b781984cd 100644 --- a/command/agent/auth/token-file/token_file_test.go +++ b/command/agent/auth/token-file/token_file_test.go @@ -5,9 +5,8 @@ import ( "testing" log "github.com/hashicorp/go-hclog" - "github.com/hashicorp/vault/sdk/helper/logging" - "github.com/hashicorp/vault/command/agent/auth" + "github.com/hashicorp/vault/sdk/helper/logging" ) func TestNewTokenFileAuthMethodEmptyConfig(t *testing.T) { @@ -75,7 +74,7 @@ func TestNewTokenFileAuthenticate(t *testing.T) { if err != nil { t.Fatal(err) } - if path != "auth/token/lookup" { + if path != "auth/token/lookup-self" { t.Fatalf("Incorrect path, was %s", path) } if headers != nil { @@ -122,7 +121,7 @@ func TestNewTokenFileAuthenticateRemoveAfterReading(t *testing.T) { if err != nil { t.Fatal(err) } - if path != "auth/token/lookup" { + if path != "auth/token/lookup-self" { t.Fatalf("Incorrect path, was %s", path) } if headers != nil { diff --git a/command/agent/token_file_end_to_end_test.go b/command/agent/token_file_end_to_end_test.go index 43f836f6121e..e5cc561985a1 100644 --- a/command/agent/token_file_end_to_end_test.go +++ b/command/agent/token_file_end_to_end_test.go @@ -7,13 +7,11 @@ import ( "testing" "time" - "github.com/hashicorp/vault/command/agent/sink" - "github.com/hashicorp/vault/command/agent/sink/file" - - token_file "github.com/hashicorp/vault/command/agent/auth/token-file" - log "github.com/hashicorp/go-hclog" "github.com/hashicorp/vault/command/agent/auth" + token_file "github.com/hashicorp/vault/command/agent/auth/token-file" + "github.com/hashicorp/vault/command/agent/sink" + "github.com/hashicorp/vault/command/agent/sink/file" vaulthttp "github.com/hashicorp/vault/http" "github.com/hashicorp/vault/sdk/helper/logging" "github.com/hashicorp/vault/vault" From 8609049b01c54534f09b68846774d9c647c9c6ef Mon Sep 17 00:00:00 2001 From: VioletHynes Date: Tue, 17 Jan 2023 15:56:47 -0500 Subject: [PATCH 07/14] VAULT-12564 safer client usage --- command/agent/auth/auth.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/command/agent/auth/auth.go b/command/agent/auth/auth.go index 52332e6854f0..01189cff9b9c 100644 --- a/command/agent/auth/auth.go +++ b/command/agent/auth/auth.go @@ -260,8 +260,13 @@ func (ah *AuthHandler) Run(ctx context.Context, am AuthMethod) error { isTokenFileMethod = path == "auth/token/lookup-self" if isTokenFileMethod { token, _ := data["token"].(string) - clientToUse.SetToken(token) - secret, err = clientToUse.Auth().Token().LookupSelf() + lookupSelfClient, err := clientToUse.Clone() + if err != nil { + ah.logger.Error("failed to clone client to perform token lookup") + return err + } + lookupSelfClient.SetToken(token) + secret, err = lookupSelfClient.Auth().Token().LookupSelf() } else { secret, err = clientToUse.Logical().WriteWithContext(ctx, path, data) } From d95c0723ac9fcd8673d552d440cc62cb20aeb58e Mon Sep 17 00:00:00 2001 From: VioletHynes Date: Wed, 18 Jan 2023 09:13:16 -0500 Subject: [PATCH 08/14] VAULT-12564 some clean-up --- command/agent/auth/kubernetes/kubernetes.go | 7 ------- command/agent/auth/token-file/token_file.go | 12 ++++++------ 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/command/agent/auth/kubernetes/kubernetes.go b/command/agent/auth/kubernetes/kubernetes.go index 3e41f4353696..c30f3cb5a68b 100644 --- a/command/agent/auth/kubernetes/kubernetes.go +++ b/command/agent/auth/kubernetes/kubernetes.go @@ -127,10 +127,3 @@ func (k *kubernetesMethod) readJWT() (string, error) { return strings.TrimSpace(string(contentBytes)), nil } - -func (k *kubernetesMethod) extractTokenFromRequest(secret *api.Secret) string { - if secret == nil || secret.Auth == nil { - return secret.Auth.ClientToken - } - return "" -} diff --git a/command/agent/auth/token-file/token_file.go b/command/agent/auth/token-file/token_file.go index 2d5328330c66..13cec628e572 100644 --- a/command/agent/auth/token-file/token_file.go +++ b/command/agent/auth/token-file/token_file.go @@ -14,7 +14,7 @@ import ( "github.com/hashicorp/vault/command/agent/auth" ) -type TokenFileMethod struct { +type tokenFileMethod struct { logger hclog.Logger mountPath string @@ -32,7 +32,7 @@ func NewTokenFileAuthMethod(conf *auth.AuthConfig) (auth.AuthMethod, error) { return nil, errors.New("empty config data") } - a := &TokenFileMethod{ + a := &tokenFileMethod{ logger: conf.Logger, mountPath: "auth/token", } @@ -70,7 +70,7 @@ func NewTokenFileAuthMethod(conf *auth.AuthConfig) (auth.AuthMethod, error) { return a, nil } -func (a *TokenFileMethod) Authenticate(ctx context.Context, client *api.Client) (string, http.Header, map[string]interface{}, error) { +func (a *tokenFileMethod) Authenticate(ctx context.Context, client *api.Client) (string, http.Header, map[string]interface{}, error) { _, err := os.Stat(a.tokenFilePath) if err != nil { if a.cachedToken == "" { @@ -108,12 +108,12 @@ func (a *TokenFileMethod) Authenticate(ctx context.Context, client *api.Client) }, nil } -func (a *TokenFileMethod) NewCreds() chan struct{} { +func (a *tokenFileMethod) NewCreds() chan struct{} { return nil } -func (a *TokenFileMethod) CredSuccess() { +func (a *tokenFileMethod) CredSuccess() { } -func (a *TokenFileMethod) Shutdown() { +func (a *tokenFileMethod) Shutdown() { } From 74e415e52b4e756f4893440919e4ec1f4ec4a998 Mon Sep 17 00:00:00 2001 From: VioletHynes Date: Wed, 18 Jan 2023 09:21:07 -0500 Subject: [PATCH 09/14] VAULT-12564 changelog --- changelog/18740.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog/18740.txt diff --git a/changelog/18740.txt b/changelog/18740.txt new file mode 100644 index 000000000000..f493995d48a6 --- /dev/null +++ b/changelog/18740.txt @@ -0,0 +1,3 @@ +```release-note:improvement +agent: Added `token_file` auto-auth configuration to allow using a pre-existing token for Vault Agent. +``` From 0ec11a011b6c222abb0043e215bd2b3d8fd763be Mon Sep 17 00:00:00 2001 From: VioletHynes Date: Wed, 18 Jan 2023 11:21:56 -0500 Subject: [PATCH 10/14] VAULT-12564 some clean-ups --- command/agent/auth/token-file/token_file.go | 2 +- command/agent/auth/token-file/token_file_test.go | 4 ++-- command/agent/token_file_end_to_end_test.go | 6 ++---- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/command/agent/auth/token-file/token_file.go b/command/agent/auth/token-file/token_file.go index 13cec628e572..46a93c5d7b6d 100644 --- a/command/agent/auth/token-file/token_file.go +++ b/command/agent/auth/token-file/token_file.go @@ -90,7 +90,7 @@ func (a *tokenFileMethod) Authenticate(ctx context.Context, client *api.Client) if a.cachedToken == "" { return "", nil, nil, errors.New("token file empty and no cached token known") } - a.logger.Warn("role ID file exists but read empty value, re-using cached value") + a.logger.Warn("token file exists but read empty value, re-using cached value") } else { a.cachedToken = strings.TrimSpace(string(token)) } diff --git a/command/agent/auth/token-file/token_file_test.go b/command/agent/auth/token-file/token_file_test.go index 0d1b781984cd..a7b055f78e1f 100644 --- a/command/agent/auth/token-file/token_file_test.go +++ b/command/agent/auth/token-file/token_file_test.go @@ -48,7 +48,7 @@ func TestNewTokenFileEmptyFilePath(t *testing.T) { } func TestNewTokenFileAuthenticate(t *testing.T) { - tokenFile, err := os.CreateTemp("", "token_file") + tokenFile, err := os.CreateTemp(t.TempDir(), "token_file") tokenFileContents := "super-secret-token" if err != nil { t.Fatal(err) @@ -95,7 +95,7 @@ func TestNewTokenFileAuthenticate(t *testing.T) { } func TestNewTokenFileAuthenticateRemoveAfterReading(t *testing.T) { - tokenFile, err := os.CreateTemp("", "token_file") + tokenFile, err := os.CreateTemp(t.TempDir(), "token_file") tokenFileContents := "super-secret-token" if err != nil { t.Fatal(err) diff --git a/command/agent/token_file_end_to_end_test.go b/command/agent/token_file_end_to_end_test.go index e5cc561985a1..01c27cbaa75b 100644 --- a/command/agent/token_file_end_to_end_test.go +++ b/command/agent/token_file_end_to_end_test.go @@ -69,7 +69,7 @@ func testTokenFileEndToEnd(t *testing.T, removeTokenFile bool, expectToken bool) t.Fatal(err) } - tokenFile, err := os.CreateTemp("", "token_file") + tokenFile, err := os.CreateTemp(t.TempDir(), "token_file") if err != nil { t.Fatal(err) } @@ -78,8 +78,6 @@ func testTokenFileEndToEnd(t *testing.T, removeTokenFile bool, expectToken bool) os.WriteFile(tokenFileName, []byte(secret.Auth.ClientToken), 0o666) defer os.Remove(tokenFileName) - t.Logf("input token_file_path: %s", tokenFileName) - ahConfig := &auth.AuthHandlerConfig{ Logger: logger.Named("auth.handler"), Client: client, @@ -115,7 +113,7 @@ func testTokenFileEndToEnd(t *testing.T, removeTokenFile bool, expectToken bool) // We close these right away because we're just basically testing // permissions and finding a usable file name - sinkFile, err := os.CreateTemp("", "auth.tokensink.test.") + sinkFile, err := os.CreateTemp(t.TempDir(), "auth.tokensink.test.") if err != nil { t.Fatal(err) } From 6ab0d52c283f3ea342ccbc461f73d4ff2d4c2c1e Mon Sep 17 00:00:00 2001 From: VioletHynes Date: Wed, 18 Jan 2023 11:29:31 -0500 Subject: [PATCH 11/14] VAULT-12564 batch token warning --- command/agent/auth/auth.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/command/agent/auth/auth.go b/command/agent/auth/auth.go index 01189cff9b9c..3be7951e0c43 100644 --- a/command/agent/auth/auth.go +++ b/command/agent/auth/auth.go @@ -375,6 +375,11 @@ func (ah *AuthHandler) Run(ctx context.Context, am AuthMethod) error { if ah.enableTemplateTokenCh { ah.TemplateTokenCh <- token } + + tokenType := secret.Data["type"].(string) + if tokenType == "batch" { + ah.logger.Info("note that this token type is batch, and batch tokens cannot be renewed", "ttl", leaseDuration) + } } else { if secret == nil || secret.Auth == nil { ah.logger.Error("authentication returned nil auth info", "backoff", backoffCfg) From cbbda1ca17656047c9c0ba44fe0fabb214d5b7a0 Mon Sep 17 00:00:00 2001 From: VioletHynes Date: Fri, 20 Jan 2023 15:57:37 -0500 Subject: [PATCH 12/14] VAULT-12564 remove follow_symlink reference --- command/agent/auth/token-file/token_file.go | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/command/agent/auth/token-file/token_file.go b/command/agent/auth/token-file/token_file.go index 46a93c5d7b6d..3c6c88f96cff 100644 --- a/command/agent/auth/token-file/token_file.go +++ b/command/agent/auth/token-file/token_file.go @@ -20,7 +20,6 @@ type tokenFileMethod struct { cachedToken string tokenFilePath string - followSymlinks bool removeTokenFileAfterReading bool } @@ -58,15 +57,6 @@ func NewTokenFileAuthMethod(conf *auth.AuthConfig) (auth.AuthMethod, error) { a.removeTokenFileAfterReading = removeTokenFileAfterReading } - followSymlinksRaw, ok := conf.Config["follow_symlinks"] - if ok { - followSymlinks, err := parseutil.ParseBool(followSymlinksRaw) - if err != nil { - return nil, fmt.Errorf("error parsing 'follow_symlinks' value: %w", err) - } - a.followSymlinks = followSymlinks - } - return a, nil } From 2ed06c499136b97b3db5095b38ba3612c256adb9 Mon Sep 17 00:00:00 2001 From: VioletHynes Date: Tue, 24 Jan 2023 09:12:49 -0500 Subject: [PATCH 13/14] VAULT-12564 Remove redundant stat, change temp file creation --- command/agent/auth/token-file/token_file.go | 8 -------- command/agent/auth/token-file/token_file_test.go | 5 +++-- command/agent/token_file_end_to_end_test.go | 5 +++-- 3 files changed, 6 insertions(+), 12 deletions(-) diff --git a/command/agent/auth/token-file/token_file.go b/command/agent/auth/token-file/token_file.go index 3c6c88f96cff..1c4d72c3ca58 100644 --- a/command/agent/auth/token-file/token_file.go +++ b/command/agent/auth/token-file/token_file.go @@ -61,14 +61,6 @@ func NewTokenFileAuthMethod(conf *auth.AuthConfig) (auth.AuthMethod, error) { } func (a *tokenFileMethod) Authenticate(ctx context.Context, client *api.Client) (string, http.Header, map[string]interface{}, error) { - _, err := os.Stat(a.tokenFilePath) - if err != nil { - if a.cachedToken == "" { - return "", nil, nil, fmt.Errorf("token file not found and no cached token known: %w", err) - } - a.logger.Warn("token file not found", "error", err) - } - token, err := os.ReadFile(a.tokenFilePath) if err != nil { if a.cachedToken == "" { diff --git a/command/agent/auth/token-file/token_file_test.go b/command/agent/auth/token-file/token_file_test.go index a7b055f78e1f..4c85ac64bbd9 100644 --- a/command/agent/auth/token-file/token_file_test.go +++ b/command/agent/auth/token-file/token_file_test.go @@ -2,6 +2,7 @@ package token_file import ( "os" + "path/filepath" "testing" log "github.com/hashicorp/go-hclog" @@ -48,7 +49,7 @@ func TestNewTokenFileEmptyFilePath(t *testing.T) { } func TestNewTokenFileAuthenticate(t *testing.T) { - tokenFile, err := os.CreateTemp(t.TempDir(), "token_file") + tokenFile, err := os.Create(filepath.Join(t.TempDir(), "token_file")) tokenFileContents := "super-secret-token" if err != nil { t.Fatal(err) @@ -95,7 +96,7 @@ func TestNewTokenFileAuthenticate(t *testing.T) { } func TestNewTokenFileAuthenticateRemoveAfterReading(t *testing.T) { - tokenFile, err := os.CreateTemp(t.TempDir(), "token_file") + tokenFile, err := os.Create(filepath.Join(t.TempDir(), "token_file")) tokenFileContents := "super-secret-token" if err != nil { t.Fatal(err) diff --git a/command/agent/token_file_end_to_end_test.go b/command/agent/token_file_end_to_end_test.go index 01c27cbaa75b..b7aa28605d4d 100644 --- a/command/agent/token_file_end_to_end_test.go +++ b/command/agent/token_file_end_to_end_test.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "os" + "path/filepath" "testing" "time" @@ -69,7 +70,7 @@ func testTokenFileEndToEnd(t *testing.T, removeTokenFile bool, expectToken bool) t.Fatal(err) } - tokenFile, err := os.CreateTemp(t.TempDir(), "token_file") + tokenFile, err := os.Create(filepath.Join(t.TempDir(), "token_file")) if err != nil { t.Fatal(err) } @@ -113,7 +114,7 @@ func testTokenFileEndToEnd(t *testing.T, removeTokenFile bool, expectToken bool) // We close these right away because we're just basically testing // permissions and finding a usable file name - sinkFile, err := os.CreateTemp(t.TempDir(), "auth.tokensink.test.") + sinkFile, err := os.Create(filepath.Join(t.TempDir(), "auth.tokensink.test.")) if err != nil { t.Fatal(err) } From 177c156d61ac5eadc6cf18afe876d24e5bc32b98 Mon Sep 17 00:00:00 2001 From: VioletHynes Date: Tue, 24 Jan 2023 15:40:53 -0500 Subject: [PATCH 14/14] VAULT-12564 Remove ability to delete token after auth --- command/agent/auth/token-file/token_file.go | 22 +----- .../agent/auth/token-file/token_file_test.go | 69 +------------------ command/agent/token_file_end_to_end_test.go | 63 ++++------------- 3 files changed, 16 insertions(+), 138 deletions(-) diff --git a/command/agent/auth/token-file/token_file.go b/command/agent/auth/token-file/token_file.go index 1c4d72c3ca58..c5a8579371f4 100644 --- a/command/agent/auth/token-file/token_file.go +++ b/command/agent/auth/token-file/token_file.go @@ -9,7 +9,6 @@ import ( "strings" "github.com/hashicorp/go-hclog" - "github.com/hashicorp/go-secure-stdlib/parseutil" "github.com/hashicorp/vault/api" "github.com/hashicorp/vault/command/agent/auth" ) @@ -18,9 +17,8 @@ type tokenFileMethod struct { logger hclog.Logger mountPath string - cachedToken string - tokenFilePath string - removeTokenFileAfterReading bool + cachedToken string + tokenFilePath string } func NewTokenFileAuthMethod(conf *auth.AuthConfig) (auth.AuthMethod, error) { @@ -48,15 +46,6 @@ func NewTokenFileAuthMethod(conf *auth.AuthConfig) (auth.AuthMethod, error) { return nil, errors.New("'token_file_path' value is empty") } - removeTokenFileAfterReadingRaw, ok := conf.Config["remove_token_file_after_reading"] - if ok { - removeTokenFileAfterReading, err := parseutil.ParseBool(removeTokenFileAfterReadingRaw) - if err != nil { - return nil, fmt.Errorf("error parsing 'remove_token_file_after_reading' value: %w", err) - } - a.removeTokenFileAfterReading = removeTokenFileAfterReading - } - return a, nil } @@ -77,13 +66,6 @@ func (a *tokenFileMethod) Authenticate(ctx context.Context, client *api.Client) a.cachedToken = strings.TrimSpace(string(token)) } - if a.removeTokenFileAfterReading { - a.logger.Info("removing token file after reading, because 'remove_token_file_after_reading' is true") - if err := os.Remove(a.tokenFilePath); err != nil { - a.logger.Error("error removing token file after reading", "error", err) - } - } - // i.e. auth/token/lookup-self return fmt.Sprintf("%s/lookup-self", a.mountPath), nil, map[string]interface{}{ "token": a.cachedToken, diff --git a/command/agent/auth/token-file/token_file_test.go b/command/agent/auth/token-file/token_file_test.go index 4c85ac64bbd9..0dd737671449 100644 --- a/command/agent/auth/token-file/token_file_test.go +++ b/command/agent/auth/token-file/token_file_test.go @@ -21,20 +21,6 @@ func TestNewTokenFileAuthMethodEmptyConfig(t *testing.T) { } } -func TestNewTokenFileRemoveTokenString(t *testing.T) { - logger := logging.NewVaultLogger(log.Trace) - _, err := NewTokenFileAuthMethod(&auth.AuthConfig{ - Logger: logger.Named("auth.method"), - Config: map[string]interface{}{ - "token_file_path": "/some/path", - "remove_token_file_after_reading": "string", - }, - }) - if err == nil { - t.Fatalf("Expected error when giving string for bool") - } -} - func TestNewTokenFileEmptyFilePath(t *testing.T) { logger := logging.NewVaultLogger(log.Trace) _, err := NewTokenFileAuthMethod(&auth.AuthConfig{ @@ -63,8 +49,7 @@ func TestNewTokenFileAuthenticate(t *testing.T) { am, err := NewTokenFileAuthMethod(&auth.AuthConfig{ Logger: logger.Named("auth.method"), Config: map[string]interface{}{ - "token_file_path": tokenFileName, - "remove_token_file_after_reading": false, + "token_file_path": tokenFileName, }, }) if err != nil { @@ -91,56 +76,6 @@ func TestNewTokenFileAuthenticate(t *testing.T) { _, err = os.Stat(tokenFileName) if err != nil { - t.Fatal("Token file removed despite remove token file being set to false") - } -} - -func TestNewTokenFileAuthenticateRemoveAfterReading(t *testing.T) { - tokenFile, err := os.Create(filepath.Join(t.TempDir(), "token_file")) - tokenFileContents := "super-secret-token" - if err != nil { - t.Fatal(err) - } - tokenFileName := tokenFile.Name() - tokenFile.Close() // WriteFile doesn't need it open - os.WriteFile(tokenFileName, []byte(tokenFileContents), 0o666) - defer os.Remove(tokenFileName) - - logger := logging.NewVaultLogger(log.Trace) - am, err := NewTokenFileAuthMethod(&auth.AuthConfig{ - Logger: logger.Named("auth.method"), - Config: map[string]interface{}{ - "token_file_path": tokenFileName, - "remove_token_file_after_reading": true, - }, - }) - if err != nil { - t.Fatal(err) - } - - path, headers, data, err := am.Authenticate(nil, nil) - if err != nil { - t.Fatal(err) - } - if path != "auth/token/lookup-self" { - t.Fatalf("Incorrect path, was %s", path) - } - if headers != nil { - t.Fatalf("Expected no headers, instead got %v", headers) - } - if data == nil { - t.Fatal("Data was nil") - } - tokenDataFromAuthMethod := data["token"].(string) - if tokenDataFromAuthMethod != tokenFileContents { - t.Fatalf("Incorrect token file contents return by auth method, expected %s, got %s", tokenFileContents, tokenDataFromAuthMethod) - } - - _, err = os.Stat(tokenFileName) - if err == nil { - t.Fatal("no error returned from stat, indicating the file is still present") - } - if !os.IsNotExist(err) { - t.Fatalf("unexpected error: %v", err) + t.Fatal("Token file removed") } } diff --git a/command/agent/token_file_end_to_end_test.go b/command/agent/token_file_end_to_end_test.go index b7aa28605d4d..d9819ada0260 100644 --- a/command/agent/token_file_end_to_end_test.go +++ b/command/agent/token_file_end_to_end_test.go @@ -2,7 +2,6 @@ package agent import ( "context" - "fmt" "os" "path/filepath" "testing" @@ -19,31 +18,6 @@ import ( ) func TestTokenFileEndToEnd(t *testing.T) { - t.Parallel() - - testCases := []struct { - removeTokenFile bool - expectToken bool - }{ - // default behaviour => token expected - {false, true}, - {true, true}, - } - - for _, tc := range testCases { - secretFileAction := "preserve" - if tc.removeTokenFile { - secretFileAction = "remove" - } - tc := tc // capture range variable - t.Run(fmt.Sprintf("%s_removeTokenFile, expectToken=%v", secretFileAction, tc.expectToken), func(t *testing.T) { - t.Parallel() - testTokenFileEndToEnd(t, tc.removeTokenFile, tc.expectToken) - }) - } -} - -func testTokenFileEndToEnd(t *testing.T, removeTokenFile bool, expectToken bool) { var err error logger := logging.NewVaultLogger(log.Trace) coreConfig := &vault.CoreConfig{ @@ -89,8 +63,7 @@ func testTokenFileEndToEnd(t *testing.T, removeTokenFile bool, expectToken bool) am, err := token_file.NewTokenFileAuthMethod(&auth.AuthConfig{ Logger: logger.Named("auth.method"), Config: map[string]interface{}{ - "token_file_path": tokenFileName, - "remove_token_file_after_reading": removeTokenFile, + "token_file_path": tokenFileName, }, }) if err != nil { @@ -167,32 +140,20 @@ func testTokenFileEndToEnd(t *testing.T, removeTokenFile bool, expectToken bool) t.Fatal("expected notexist err") } - if expectToken { - // Wait 2 seconds for the env variables to be detected and an auth to be generated. - time.Sleep(time.Second * 2) + // Wait 2 seconds for the env variables to be detected and an auth to be generated. + time.Sleep(time.Second * 2) - token, err := readToken(tokenSinkFileName) - if err != nil { - t.Fatal(err) - } + token, err := readToken(tokenSinkFileName) + if err != nil { + t.Fatal(err) + } - if token.Token == "" { - t.Fatal("expected token but didn't receive it") - } + if token.Token == "" { + t.Fatal("expected token but didn't receive it") } - if !removeTokenFile { - _, err := os.Stat(tokenFileName) - if err != nil { - t.Fatal("Token file removed despite remove token file being set to false") - } - } else { - _, err := os.Stat(tokenFileName) - if err == nil { - t.Fatal("no error returned from stat, indicating the file is still present") - } - if !os.IsNotExist(err) { - t.Fatalf("unexpected error: %v", err) - } + _, err = os.Stat(tokenFileName) + if err != nil { + t.Fatal("Token file removed") } }