From 3e3ffd43e0a0dd7b082adad51882517faf28f87a Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Fri, 20 Jul 2018 00:35:31 -0400 Subject: [PATCH] Two-pronged fix for renew policy checking 1) In backends, ensure they are now using TokenPolicies 2) Don't reassign auth.Policies until after expmgr registration as we don't need them at that point Fixes #4829 --- builtin/credential/app-id/path_login.go | 2 +- builtin/credential/cert/backend_test.go | 1 + builtin/credential/cert/path_login.go | 2 +- builtin/credential/github/path_login.go | 2 +- builtin/credential/ldap/path_login.go | 2 +- builtin/credential/okta/path_login.go | 2 +- builtin/credential/radius/path_login.go | 2 +- builtin/credential/userpass/path_login.go | 2 +- .../cassandra/test-fixtures/cassandra.yaml | 2 +- vault/request_handling.go | 18 ++++++++++----- vault/token_store_ext_test.go | 22 ++++++++++++++++++- 11 files changed, 42 insertions(+), 15 deletions(-) diff --git a/builtin/credential/app-id/path_login.go b/builtin/credential/app-id/path_login.go index 9fc86b48539d..ec744b283280 100644 --- a/builtin/credential/app-id/path_login.go +++ b/builtin/credential/app-id/path_login.go @@ -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") } diff --git a/builtin/credential/cert/backend_test.go b/builtin/credential/cert/backend_test.go index 4dd4e34951be..4c142712c79a 100644 --- a/builtin/credential/cert/backend_test.go +++ b/builtin/credential/cert/backend_test.go @@ -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 diff --git a/builtin/credential/cert/path_login.go b/builtin/credential/cert/path_login.go index b0e5eef7e37e..90dbacb44307 100644 --- a/builtin/credential/cert/path_login.go +++ b/builtin/credential/cert/path_login.go @@ -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") } diff --git a/builtin/credential/github/path_login.go b/builtin/credential/github/path_login.go index 34a0f617c211..216a159b1e88 100644 --- a/builtin/credential/github/path_login.go +++ b/builtin/credential/github/path_login.go @@ -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") } diff --git a/builtin/credential/ldap/path_login.go b/builtin/credential/ldap/path_login.go index 1950df1d6635..114f20a908e3 100644 --- a/builtin/credential/ldap/path_login.go +++ b/builtin/credential/ldap/path_login.go @@ -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") } diff --git a/builtin/credential/okta/path_login.go b/builtin/credential/okta/path_login.go index 9b6e7dfeb125..af784e015dee 100644 --- a/builtin/credential/okta/path_login.go +++ b/builtin/credential/okta/path_login.go @@ -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") } diff --git a/builtin/credential/radius/path_login.go b/builtin/credential/radius/path_login.go index 68e679908682..ef0c185d88f4 100644 --- a/builtin/credential/radius/path_login.go +++ b/builtin/credential/radius/path_login.go @@ -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") } diff --git a/builtin/credential/userpass/path_login.go b/builtin/credential/userpass/path_login.go index 89ee1d95042b..91fcdd1e11b8 100644 --- a/builtin/credential/userpass/path_login.go +++ b/builtin/credential/userpass/path_login.go @@ -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") } diff --git a/plugins/database/cassandra/test-fixtures/cassandra.yaml b/plugins/database/cassandra/test-fixtures/cassandra.yaml index 8351cf7a65f8..e8535107c873 100644 --- a/plugins/database/cassandra/test-fixtures/cassandra.yaml +++ b/plugins/database/cassandra/test-fixtures/cassandra.yaml @@ -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. diff --git a/vault/request_handling.go b/vault/request_handling.go index f474e062bae5..7115b64dd2bb 100644 --- a/vault/request_handling.go +++ b/vault/request_handling.go @@ -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 && @@ -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 } @@ -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 } diff --git a/vault/token_store_ext_test.go b/vault/token_store_ext_test.go index fa5db8742e2e..56b5efdded8d 100644 --- a/vault/token_store_ext_test.go +++ b/vault/token_store_ext_test.go @@ -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 { @@ -175,7 +183,7 @@ func TestTokenStore_IdentityPolicies(t *testing.T) { } sort.Strings(actualPolicies) - expectedPolicies := []string{ + expectedPolicies = []string{ "entity_policy_1", "entity_policy_2", } @@ -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/login/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) {