From 5bce0cb5bd1178ca8cee801e5527eec818a32c4e Mon Sep 17 00:00:00 2001 From: Christopher Swenson Date: Wed, 11 May 2022 14:23:29 -0700 Subject: [PATCH] VAULT-5935 agent: redact renew-self if using auto auth Vault agent redacts the token and accessor for `/auth/token/lookup-self` (and `lookup`) if the token is the auto auth token to prevent it from leaking. Similarly, we need to redact the token and accessor from `renew-self` and `renew`, which also leak the token and accessor. I tested this locally by starting up a Vault agent and querying the agent endpoints, and ensuring that the accessor and token were set to the empty string in the response. --- changelog/15380.txt | 3 +++ command/agent/cache/cache_test.go | 31 ++++++++++++++++++++++- command/agent/cache/handler.go | 40 +++++++++++++++++++----------- command/agent/cache/lease_cache.go | 6 +++-- 4 files changed, 62 insertions(+), 18 deletions(-) create mode 100644 changelog/15380.txt diff --git a/changelog/15380.txt b/changelog/15380.txt new file mode 100644 index 0000000000000..d895f8afed327 --- /dev/null +++ b/changelog/15380.txt @@ -0,0 +1,3 @@ +```release-note:bug +agent: Redact auto auth token from renew endpoints +``` diff --git a/command/agent/cache/cache_test.go b/command/agent/cache/cache_test.go index bee5fc0e87f76..8b2e1caa7155f 100644 --- a/command/agent/cache/cache_test.go +++ b/command/agent/cache/cache_test.go @@ -2,6 +2,7 @@ package cache import ( "context" + "encoding/json" "fmt" "io" "math/rand" @@ -13,7 +14,7 @@ import ( "time" "github.com/go-test/deep" - hclog "github.com/hashicorp/go-hclog" + "github.com/hashicorp/go-hclog" kv "github.com/hashicorp/vault-plugin-secrets-kv" "github.com/hashicorp/vault/api" "github.com/hashicorp/vault/builtin/credential/userpass" @@ -214,9 +215,13 @@ func tokenRevocationValidation(t *testing.T, sampleSpace map[string]string, expe func TestCache_AutoAuthTokenStripping(t *testing.T) { response1 := `{"data": {"id": "testid", "accessor": "testaccessor", "request": "lookup-self"}}` response2 := `{"data": {"id": "testid", "accessor": "testaccessor", "request": "lookup"}}` + response3 := `{"auth": {"client_token": "testid", "accessor": "testaccessor"}}` + response4 := `{"auth": {"client_token": "testid", "accessor": "testaccessor"}}` responses := []*SendResponse{ newTestSendResponse(http.StatusOK, response1), newTestSendResponse(http.StatusOK, response2), + newTestSendResponse(http.StatusOK, response3), + newTestSendResponse(http.StatusOK, response4), } leaseCache := testNewLeaseCache(t, responses) @@ -279,6 +284,30 @@ func TestCache_AutoAuthTokenStripping(t *testing.T) { if secret.Data["id"] != nil || secret.Data["accessor"] != nil || secret.Data["request"].(string) != "lookup" { t.Fatalf("failed to strip off auto-auth token on lookup") } + + secret, err = testClient.Auth().Token().RenewSelf(1) + if err != nil { + t.Fatal(err) + } + if secret.Auth == nil { + secretJson, _ := json.Marshal(secret) + t.Fatalf("Expected secret to have Auth but was %s", secretJson) + } + if secret.Auth.ClientToken != "" || secret.Auth.Accessor != "" { + t.Fatalf("failed to strip off auto-auth token on renew-self") + } + + secret, err = testClient.Auth().Token().Renew("testid", 1) + if err != nil { + t.Fatal(err) + } + if secret.Auth == nil { + secretJson, _ := json.Marshal(secret) + t.Fatalf("Expected secret to have Auth but was %s", secretJson) + } + if secret.Auth.ClientToken != "" || secret.Auth.Accessor != "" { + t.Fatalf("failed to strip off auto-auth token on renew") + } } func TestCache_AutoAuthClientTokenProxyStripping(t *testing.T) { diff --git a/command/agent/cache/handler.go b/command/agent/cache/handler.go index c8c60ddbf4b92..2beea6cc213d8 100644 --- a/command/agent/cache/handler.go +++ b/command/agent/cache/handler.go @@ -13,7 +13,7 @@ import ( "time" "github.com/armon/go-metrics" - hclog "github.com/hashicorp/go-hclog" + "github.com/hashicorp/go-hclog" "github.com/hashicorp/vault/api" "github.com/hashicorp/vault/command/agent/sink" "github.com/hashicorp/vault/sdk/helper/consts" @@ -67,7 +67,7 @@ func Handler(ctx context.Context, logger hclog.Logger, proxier Proxier, inmemSin return } - err = processTokenLookupResponse(ctx, logger, inmemSink, req, resp) + err = sanitizeAutoAuthTokenResponse(ctx, logger, inmemSink, req, resp) if err != nil { logical.RespondError(w, http.StatusInternalServerError, fmt.Errorf("failed to process token lookup response: %w", err)) return @@ -120,11 +120,10 @@ func setHeaders(w http.ResponseWriter, resp *SendResponse) { w.WriteHeader(resp.Response.StatusCode) } -// processTokenLookupResponse checks if the request was one of token -// lookup-self. If the auto-auth token was used to perform lookup-self, the -// identifier of the token and its accessor same will be stripped off of the -// response. -func processTokenLookupResponse(ctx context.Context, logger hclog.Logger, inmemSink sink.Sink, req *SendRequest, resp *SendResponse) error { +// sanitizeAutoAuthTokenResponse checks if the request was a lookup or renew +// and if the auto-auth token was used to perform lookup-self, the identifier +// of the token and its accessor same will be stripped off of the response. +func sanitizeAutoAuthTokenResponse(ctx context.Context, logger hclog.Logger, inmemSink sink.Sink, req *SendRequest, resp *SendResponse) error { // If auto-auth token is not being used, there is nothing to do. if inmemSink == nil { return nil @@ -138,11 +137,11 @@ func processTokenLookupResponse(ctx context.Context, logger hclog.Logger, inmemS _, path := deriveNamespaceAndRevocationPath(req) switch path { - case vaultPathTokenLookupSelf: + case vaultPathTokenLookupSelf, vaultPathTokenRenewSelf: if req.Token != autoAuthToken { return nil } - case vaultPathTokenLookup: + case vaultPathTokenLookup, vaultPathTokenRenew: jsonBody := map[string]interface{}{} if err := json.Unmarshal(req.RequestBody, &jsonBody); err != nil { return err @@ -170,16 +169,27 @@ func processTokenLookupResponse(ctx context.Context, logger hclog.Logger, inmemS if err != nil { return fmt.Errorf("failed to parse token lookup response: %v", err) } - if secret == nil || secret.Data == nil { + if secret == nil { return nil - } - if secret.Data["id"] == nil && secret.Data["accessor"] == nil { + } else if secret.Data != nil { + // lookup endpoints + if secret.Data["id"] == nil && secret.Data["accessor"] == nil { + return nil + } + delete(secret.Data, "id") + delete(secret.Data, "accessor") + } else if secret.Auth != nil { + // renew endpoints + if secret.Auth.Accessor == "" && secret.Auth.ClientToken == "" { + return nil + } + secret.Auth.Accessor = "" + secret.Auth.ClientToken = "" + } else { + // nothing to redact return nil } - delete(secret.Data, "id") - delete(secret.Data, "accessor") - bodyBytes, err := json.Marshal(secret) if err != nil { return err diff --git a/command/agent/cache/lease_cache.go b/command/agent/cache/lease_cache.go index 8ecea7d93aeb6..1f7224691493b 100644 --- a/command/agent/cache/lease_cache.go +++ b/command/agent/cache/lease_cache.go @@ -16,12 +16,12 @@ import ( "sync" "time" - hclog "github.com/hashicorp/go-hclog" + "github.com/hashicorp/go-hclog" "github.com/hashicorp/go-multierror" "github.com/hashicorp/go-secure-stdlib/base62" "github.com/hashicorp/vault/api" "github.com/hashicorp/vault/command/agent/cache/cacheboltdb" - cachememdb "github.com/hashicorp/vault/command/agent/cache/cachememdb" + "github.com/hashicorp/vault/command/agent/cache/cachememdb" "github.com/hashicorp/vault/helper/namespace" nshelper "github.com/hashicorp/vault/helper/namespace" vaulthttp "github.com/hashicorp/vault/http" @@ -42,6 +42,8 @@ const ( vaultPathTokenRevokeOrphan = "/v1/auth/token/revoke-orphan" vaultPathTokenLookup = "/v1/auth/token/lookup" vaultPathTokenLookupSelf = "/v1/auth/token/lookup-self" + vaultPathTokenRenew = "/v1/auth/token/renew" + vaultPathTokenRenewSelf = "/v1/auth/token/renew-self" vaultPathLeaseRevoke = "/v1/sys/leases/revoke" vaultPathLeaseRevokeForce = "/v1/sys/leases/revoke-force" vaultPathLeaseRevokePrefix = "/v1/sys/leases/revoke-prefix"