From 3e72c764433b4ea15c822377a33ed073098c4568 Mon Sep 17 00:00:00 2001 From: Violet Hynes Date: Mon, 20 Mar 2023 09:04:55 -0400 Subject: [PATCH] VAULT-8337 OSS changes (#19580) --- vault/request_handling.go | 13 ++++++++++--- vault/token_store.go | 15 +++++++++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/vault/request_handling.go b/vault/request_handling.go index 24f23927aa42..06830a7ac9de 100644 --- a/vault/request_handling.go +++ b/vault/request_handling.go @@ -2206,8 +2206,9 @@ 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) { - token, err = c.CheckSSCToken(ctx, token, c.isLoginRequest(ctx, req), c.perfStandby) + decodedToken, 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. @@ -2218,12 +2219,18 @@ func (c *Core) PopulateTokenEntry(ctx context.Context, req *logical.Request) err return logical.ErrPermissionDenied } } - req.ClientToken = token + 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. 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(token) { + if !IsJWT(decodedToken) { return fmt.Errorf("error performing token check: %w", err) } } diff --git a/vault/token_store.go b/vault/token_store.go index 818932e9018e..f9a03d0554f4 100644 --- a/vault/token_store.go +++ b/vault/token_store.go @@ -1560,6 +1560,10 @@ 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. @@ -1703,6 +1707,17 @@ 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") }