Skip to content
New issue

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

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

Already on GitHub? # to your account

Two-pronged fix for renew policy checking #4960

Merged
merged 1 commit into from
Jul 24, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion builtin/credential/app-id/path_login.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ func (b *backend) pathLoginRenew(ctx context.Context, req *logical.Request, d *f
if err != nil {
return nil, err
}
if !policyutil.EquivalentPolicies(mapPolicies, req.Auth.Policies) {
if !policyutil.EquivalentPolicies(mapPolicies, req.Auth.TokenPolicies) {
return nil, fmt.Errorf("policies do not match")
}

Expand Down
1 change: 1 addition & 0 deletions builtin/credential/cert/backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1672,6 +1672,7 @@ func Test_Renew(t *testing.T) {
req.Auth.Metadata = resp.Auth.Metadata
req.Auth.LeaseOptions = resp.Auth.LeaseOptions
req.Auth.Policies = resp.Auth.Policies
req.Auth.TokenPolicies = req.Auth.Policies
req.Auth.Period = resp.Auth.Period

// Normal renewal
Expand Down
2 changes: 1 addition & 1 deletion builtin/credential/cert/path_login.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ func (b *backend) pathLoginRenew(ctx context.Context, req *logical.Request, d *f
return nil, nil
}

if !policyutil.EquivalentPolicies(cert.Policies, req.Auth.Policies) {
if !policyutil.EquivalentPolicies(cert.Policies, req.Auth.TokenPolicies) {
return nil, fmt.Errorf("policies have changed, not renewing")
}

Expand Down
2 changes: 1 addition & 1 deletion builtin/credential/github/path_login.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ func (b *backend) pathLoginRenew(ctx context.Context, req *logical.Request, d *f
} else {
verifyResp = verifyResponse
}
if !policyutil.EquivalentPolicies(verifyResp.Policies, req.Auth.Policies) {
if !policyutil.EquivalentPolicies(verifyResp.Policies, req.Auth.TokenPolicies) {
return nil, fmt.Errorf("policies do not match")
}

Expand Down
2 changes: 1 addition & 1 deletion builtin/credential/ldap/path_login.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ func (b *backend) pathLoginRenew(ctx context.Context, req *logical.Request, d *f
return resp, err
}

if !policyutil.EquivalentPolicies(loginPolicies, req.Auth.Policies) {
if !policyutil.EquivalentPolicies(loginPolicies, req.Auth.TokenPolicies) {
return nil, fmt.Errorf("policies have changed, not renewing")
}

Expand Down
2 changes: 1 addition & 1 deletion builtin/credential/okta/path_login.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func (b *backend) pathLoginRenew(ctx context.Context, req *logical.Request, d *f
return resp, err
}

if !policyutil.EquivalentPolicies(loginPolicies, req.Auth.Policies) {
if !policyutil.EquivalentPolicies(loginPolicies, req.Auth.TokenPolicies) {
return nil, fmt.Errorf("policies have changed, not renewing")
}

Expand Down
2 changes: 1 addition & 1 deletion builtin/credential/radius/path_login.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ func (b *backend) pathLoginRenew(ctx context.Context, req *logical.Request, d *f
return resp, err
}

if !policyutil.EquivalentPolicies(loginPolicies, req.Auth.Policies) {
if !policyutil.EquivalentPolicies(loginPolicies, req.Auth.TokenPolicies) {
return nil, fmt.Errorf("policies have changed, not renewing")
}

Expand Down
2 changes: 1 addition & 1 deletion builtin/credential/userpass/path_login.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ func (b *backend) pathLoginRenew(ctx context.Context, req *logical.Request, d *f
return nil, nil
}

if !policyutil.EquivalentPolicies(user.Policies, req.Auth.Policies) {
if !policyutil.EquivalentPolicies(user.Policies, req.Auth.TokenPolicies) {
return nil, fmt.Errorf("policies have changed, not renewing")
}

Expand Down
2 changes: 1 addition & 1 deletion plugins/database/cassandra/test-fixtures/cassandra.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -572,7 +572,7 @@ ssl_storage_port: 7001
#
# Setting listen_address to 0.0.0.0 is always wrong.
#
listen_address: 172.17.0.4
listen_address: 172.17.0.2

# Set listen_address OR listen_interface, not both. Interfaces must correspond
# to a single address, IP aliasing is not supported.
Expand Down
18 changes: 12 additions & 6 deletions vault/request_handling.go
Original file line number Diff line number Diff line change
Expand Up @@ -639,15 +639,19 @@ func (c *Core) handleRequest(ctx context.Context, req *logical.Request) (retResp
}

resp.Auth.TokenPolicies = policyutil.SanitizePolicies(resp.Auth.Policies, policyutil.DoNotAddDefaultPolicy)
resp.Auth.IdentityPolicies = policyutil.SanitizePolicies(identityPolicies, policyutil.DoNotAddDefaultPolicy)
resp.Auth.Policies = policyutil.SanitizePolicies(append(resp.Auth.Policies, identityPolicies...), policyutil.DoNotAddDefaultPolicy)

if err := c.expiration.RegisterAuth(resp.Auth.CreationPath, resp.Auth); err != nil {
c.tokenStore.revokeOrphan(ctx, te.ID)
c.logger.Error("failed to register token lease", "request_path", req.Path, "error", err)
retErr = multierror.Append(retErr, ErrInternalError)
return nil, auth, retErr
}

// We do these later since it's not meaningful for backends/expmgr to
// have what is purely a snapshot of current identity policies, and
// plugins can be confused if they are checking contents of
// Auth.Policies instead of Auth.TokenPolicies
resp.Auth.IdentityPolicies = policyutil.SanitizePolicies(identityPolicies, policyutil.DoNotAddDefaultPolicy)
resp.Auth.Policies = policyutil.SanitizePolicies(append(resp.Auth.Policies, identityPolicies...), policyutil.DoNotAddDefaultPolicy)
}

if resp != nil &&
Expand Down Expand Up @@ -836,13 +840,12 @@ func (c *Core) handleLoginRequest(ctx context.Context, req *logical.Request) (re
}

auth.TokenPolicies = te.Policies
auth.IdentityPolicies = policyutil.SanitizePolicies(identityPolicies, policyutil.DoNotAddDefaultPolicy)
auth.Policies = policyutil.SanitizePolicies(append(te.Policies, identityPolicies...), policyutil.DoNotAddDefaultPolicy)
allPolicies := policyutil.SanitizePolicies(append(te.Policies, identityPolicies...), policyutil.DoNotAddDefaultPolicy)

// Prevent internal policies from being assigned to tokens. We check
// this on auth.Policies including derived ones from Identity before
// actually making the token.
for _, policy := range auth.Policies {
for _, policy := range allPolicies {
if policy == "root" {
return logical.ErrorResponse("auth methods cannot create root tokens"), nil, logical.ErrInvalidRequest
}
Expand All @@ -868,6 +871,9 @@ func (c *Core) handleLoginRequest(ctx context.Context, req *logical.Request) (re
return nil, auth, ErrInternalError
}

auth.IdentityPolicies = policyutil.SanitizePolicies(identityPolicies, policyutil.DoNotAddDefaultPolicy)
auth.Policies = allPolicies

// Attach the display name, might be used by audit backends
req.DisplayName = auth.DisplayName
}
Expand Down
22 changes: 21 additions & 1 deletion vault/token_store_ext_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,14 @@ func TestTokenStore_IdentityPolicies(t *testing.T) {
}
ldapClientToken := secret.Auth.ClientToken

expectedPolicies := []string{
"default",
"testgroup1-policy",
}
if !reflect.DeepEqual(expectedPolicies, secret.Auth.Policies) {
t.Fatalf("bad: identity policies; expected: %#v\nactual: %#v", expectedPolicies, secret.Auth.Policies)
}

// At this point there shouldn't be any identity policy on the token
secret, err = client.Logical().Read("auth/token/lookup/" + ldapClientToken)
if err != nil {
Expand Down Expand Up @@ -175,7 +183,7 @@ func TestTokenStore_IdentityPolicies(t *testing.T) {
}
sort.Strings(actualPolicies)

expectedPolicies := []string{
expectedPolicies = []string{
"entity_policy_1",
"entity_policy_2",
}
Expand Down Expand Up @@ -283,6 +291,18 @@ func TestTokenStore_IdentityPolicies(t *testing.T) {
if !reflect.DeepEqual(expectedPolicies, actualPolicies) {
t.Fatalf("bad: identity policies; expected: %#v\nactual: %#v", expectedPolicies, actualPolicies)
}

// Log in and get a new token, then renew it. See issue #4829
secret, err = client.Logical().Write("auth/ldap/#/tesla", map[string]interface{}{
"password": "password",
})
if err != nil {
t.Fatal(err)
}
secret, err = client.Auth().Token().Renew(secret.Auth.ClientToken, 10)
if err != nil {
t.Fatal(err)
}
}

func TestTokenStore_CIDRBlocks(t *testing.T) {
Expand Down