Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

VAULT-8337 Revert "VAULT-8337 OSS changes" #19700

Merged
merged 1 commit into from
Mar 23, 2023
Merged

VAULT-8337 Revert "VAULT-8337 OSS changes" #19700

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 3 additions & 10 deletions vault/request_handling.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2206,9 +2206,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.
Expand All @@ -2219,18 +2218,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)
}
}
Expand Down
15 changes: 0 additions & 15 deletions vault/token_store.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1560,10 +1560,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.
Expand Down Expand Up @@ -1707,17 +1703,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")
}

Expand Down