Skip to content

Commit

Permalink
VAULT-5935 agent: redact renew-self if using auto auth (#15380)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
swenson authored and Christopher Swenson committed May 12, 2022
1 parent d46f40c commit b1cdf3a
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 18 deletions.
3 changes: 3 additions & 0 deletions changelog/15380.txt
@@ -0,0 +1,3 @@
```release-note:bug
agent: Redact auto auth token from renew endpoints
```
31 changes: 30 additions & 1 deletion command/agent/cache/cache_test.go
Expand Up @@ -2,6 +2,7 @@ package cache

import (
"context"
"encoding/json"
"fmt"
"io"
"math/rand"
Expand All @@ -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"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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) {
Expand Down
40 changes: 25 additions & 15 deletions command/agent/cache/handler.go
Expand Up @@ -12,7 +12,7 @@ import (
"net/http"
"time"

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"
Expand Down Expand Up @@ -64,7 +64,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
Expand Down Expand Up @@ -108,11 +108,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
Expand All @@ -126,11 +125,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
Expand Down Expand Up @@ -158,16 +157,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
Expand Down
6 changes: 4 additions & 2 deletions command/agent/cache/lease_cache.go
Expand Up @@ -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"
Expand All @@ -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"
Expand Down

0 comments on commit b1cdf3a

Please sign in to comment.