From 487fd7ed37343b907695527dc15ecb451ef798a2 Mon Sep 17 00:00:00 2001 From: hc-github-team-secure-vault-core <82990506+hc-github-team-secure-vault-core@users.noreply.github.com> Date: Thu, 23 Mar 2023 12:17:25 -0400 Subject: [PATCH] backport of commit 0c69cf10488c1b36ea112a98a2ea156e7eec8d74 (#19710) Co-authored-by: Violet Hynes --- vault/request_handling.go | 13 +++---------- vault/token_store.go | 15 --------------- 2 files changed, 3 insertions(+), 25 deletions(-) diff --git a/vault/request_handling.go b/vault/request_handling.go index 4b06fa97ee03..ff041838db2a 100644 --- a/vault/request_handling.go +++ b/vault/request_handling.go @@ -2199,9 +2199,8 @@ func (c *Core) PopulateTokenEntry(ctx context.Context, req *logical.Request) err token := req.ClientToken var err error req.InboundSSCToken = token - decodedToken := token if IsSSCToken(token) { - decodedToken, err = c.CheckSSCToken(ctx, token, c.isLoginRequest(ctx, req), c.perfStandby) + token, err = c.CheckSSCToken(ctx, token, c.isLoginRequest(ctx, req), c.perfStandby) // If we receive an error from CheckSSCToken, we can assume the token is bad somehow, and the client // should receive a 403 bad token error like they do for all other invalid tokens, unless the error // specifies that we should forward the request or retry the request. @@ -2212,18 +2211,12 @@ func (c *Core) PopulateTokenEntry(ctx context.Context, req *logical.Request) err return logical.ErrPermissionDenied } } - req.ClientToken = decodedToken - // We ignore the token returned from CheckSSCToken here as Lookup also decodes the SSCT, and - // it may need the original SSCT to check state. + req.ClientToken = token te, err := c.LookupToken(ctx, token) if err != nil { - // If we're missing required state, return that error as-is to the client - if errors.Is(err, logical.ErrPerfStandbyPleaseForward) || errors.Is(err, logical.ErrMissingRequiredState) { - return err - } // If we have two dots but the second char is a dot it's a vault // token of the form s.SOMETHING.nsid, not a JWT - if !IsJWT(decodedToken) { + if !IsJWT(token) { return fmt.Errorf("error performing token check: %w", err) } } diff --git a/vault/token_store.go b/vault/token_store.go index 9e6c1461226e..8edd0e37ff63 100644 --- a/vault/token_store.go +++ b/vault/token_store.go @@ -1557,10 +1557,6 @@ func (ts *TokenStore) lookupInternal(ctx context.Context, id string, salted, tai return ts.lookupBatchToken(ctx, id) } - // Before we check to see if this is an SSCT, keep the old value in case - // we need to check the full SSCT flow later. - originalToken := id - // lookupInternal is called internally with tokens that oftentimes come from request // parameters that we cannot really guess. Most notably, these calls come from either // validateWrappedToken and/or lookupTokenTainted, used in the wrapping token logic. @@ -1704,17 +1700,6 @@ func (ts *TokenStore) lookupInternal(ctx context.Context, id string, salted, tai // It's any kind of expiring token with no lease, immediately delete it case le == nil: if ts.core.perfStandby { - // If we're a perf standby with a token but without the lease entry, then - // we have the WALs for the token but not the lease entry. We should check - // the SSCToken again to validate our state. We will receive a 412 if we - // don't have the requisite state. - // We set unauth to 'false' here as we want to validate the full SSCT flow - // and if we're at this point in the method, we have reason to need the token. - _, err = ts.core.CheckSSCToken(ctx, originalToken, false, ts.core.perfStandby) - if err != nil { - return nil, err - } - // If we don't have a state error, and we're still here, return a 500. return nil, fmt.Errorf("no lease entry found for token that ought to have one, possible eventual consistency issue") }