From bc0918a3502afc5a3f81b2a0b0be97a39bd62360 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Sat, 21 Apr 2018 10:49:16 -0400 Subject: [PATCH] Add the ability to restrict token usage by IP. Add to token roles. (#4412) Fixes #815 --- vault/core.go | 34 +++++- vault/logical_system.go | 2 +- vault/token_store.go | 45 +++++++- vault/token_store_ext_test.go | 119 ++++++++++++++++++++ website/source/api/auth/token/index.html.md | 7 ++ website/source/docs/concepts/tokens.html.md | 13 ++- 6 files changed, 210 insertions(+), 10 deletions(-) diff --git a/vault/core.go b/vault/core.go index 409a672b9ed1..01adf9e8a9bd 100644 --- a/vault/core.go +++ b/vault/core.go @@ -17,6 +17,7 @@ import ( "github.com/armon/go-metrics" log "github.com/hashicorp/go-hclog" + sockaddr "github.com/hashicorp/go-sockaddr" "google.golang.org/grpc" @@ -740,11 +741,11 @@ func (c *Core) fetchEntityAndDerivedPolicies(entityID string) (*identity.Entity, return entity, policies, err } -func (c *Core) fetchACLTokenEntryAndEntity(clientToken string) (*ACL, *TokenEntry, *identity.Entity, error) { +func (c *Core) fetchACLTokenEntryAndEntity(req *logical.Request) (*ACL, *TokenEntry, *identity.Entity, error) { defer metrics.MeasureSince([]string{"core", "fetch_acl_and_token"}, time.Now()) // Ensure there is a client token - if clientToken == "" { + if req.ClientToken == "" { return nil, nil, nil, fmt.Errorf("missing client token") } @@ -754,7 +755,7 @@ func (c *Core) fetchACLTokenEntryAndEntity(clientToken string) (*ACL, *TokenEntr } // Resolve the token policy - te, err := c.tokenStore.Lookup(c.activeContext, clientToken) + te, err := c.tokenStore.Lookup(c.activeContext, req.ClientToken) if err != nil { c.logger.Error("failed to lookup token", "error", err) return nil, nil, nil, ErrInternalError @@ -765,6 +766,27 @@ func (c *Core) fetchACLTokenEntryAndEntity(clientToken string) (*ACL, *TokenEntr return nil, nil, nil, logical.ErrPermissionDenied } + // CIDR checks bind all tokens except non-expiring root tokens + if te.TTL != 0 && len(te.BoundCIDRs) > 0 { + var valid bool + remoteSockAddr, err := sockaddr.NewSockAddr(req.Connection.RemoteAddr) + if err != nil { + if c.Logger().IsDebug() { + c.Logger().Debug("could not parse remote addr into sockaddr", "error", err, "remote_addr", req.Connection.RemoteAddr) + } + return nil, nil, nil, logical.ErrPermissionDenied + } + for _, cidr := range te.BoundCIDRs { + if cidr.Contains(remoteSockAddr) { + valid = true + break + } + } + if !valid { + return nil, nil, nil, logical.ErrPermissionDenied + } + } + tokenPolicies := te.Policies entity, derivedPolicies, err := c.fetchEntityAndDerivedPolicies(te.EntityID) @@ -796,7 +818,7 @@ func (c *Core) checkToken(ctx context.Context, req *logical.Request, unauth bool // gather as much info as possible for the audit log and to e.g. control // trace mode for EGPs. if !unauth || (unauth && req.ClientToken != "") { - acl, te, entity, err = c.fetchACLTokenEntryAndEntity(req.ClientToken) + acl, te, entity, err = c.fetchACLTokenEntryAndEntity(req) // In the unauth case we don't want to fail the command, since it's // unauth, we just have no information to attach to the request, so // ignore errors...this was best-effort anyways @@ -1339,7 +1361,7 @@ func (c *Core) sealInitCommon(ctx context.Context, req *logical.Request) (retErr } // Validate the token is a root token - acl, te, entity, err := c.fetchACLTokenEntryAndEntity(req.ClientToken) + acl, te, entity, err := c.fetchACLTokenEntryAndEntity(req) if err != nil { retErr = multierror.Append(retErr, err) c.stateLock.RUnlock() @@ -1457,7 +1479,7 @@ func (c *Core) StepDown(req *logical.Request) (retErr error) { ctx := c.activeContext - acl, te, entity, err := c.fetchACLTokenEntryAndEntity(req.ClientToken) + acl, te, entity, err := c.fetchACLTokenEntryAndEntity(req) if err != nil { retErr = multierror.Append(retErr, err) return retErr diff --git a/vault/logical_system.go b/vault/logical_system.go index 951ed5714ea2..83ec07b5f91c 100644 --- a/vault/logical_system.go +++ b/vault/logical_system.go @@ -3444,7 +3444,7 @@ func (b *SystemBackend) pathInternalUIResultantACL(ctx context.Context, req *log return nil, nil } - acl, _, entity, err := b.Core.fetchACLTokenEntryAndEntity(req.ClientToken) + acl, _, entity, err := b.Core.fetchACLTokenEntryAndEntity(req) if err != nil { return nil, err } diff --git a/vault/token_store.go b/vault/token_store.go index 4760cf9c714b..ab85283791ce 100644 --- a/vault/token_store.go +++ b/vault/token_store.go @@ -13,6 +13,7 @@ import ( "github.com/hashicorp/errwrap" log "github.com/hashicorp/go-hclog" + sockaddr "github.com/hashicorp/go-sockaddr" "github.com/armon/go-metrics" "github.com/hashicorp/go-multierror" @@ -221,6 +222,11 @@ func NewTokenStore(ctx context.Context, logger log.Logger, c *Core, config *logi Default: true, Description: tokenRenewableHelp, }, + + "bound_cidrs": &framework.FieldSchema{ + Type: framework.TypeCommaStringSlice, + Description: `Comma separated string or JSON list of CIDR blocks. If set, specifies the blocks of IP addresses which are allowed to use the generated token.`, + }, }, Callbacks: map[logical.Operation]framework.OperationFunc{ @@ -575,6 +581,9 @@ type TokenEntry struct { ExplicitMaxTTLDeprecated time.Duration `json:"ExplicitMaxTTL" mapstructure:"ExplicitMaxTTL" structs:"ExplicitMaxTTL" sentinel:""` EntityID string `json:"entity_id" mapstructure:"entity_id" structs:"entity_id"` + + // The set of CIDRs that this token can be used with + BoundCIDRs []*sockaddr.SockAddrMarshaler `json:"bound_cidrs"` } func (te *TokenEntry) SentinelGet(key string) (interface{}, error) { @@ -657,6 +666,9 @@ type tsRoleEntry struct { // If set, the token entry will have an explicit maximum TTL set, rather // than deferring to role/mount values ExplicitMaxTTL time.Duration `json:"explicit_max_ttl" mapstructure:"explicit_max_ttl" structs:"explicit_max_ttl"` + + // The set of CIDRs that tokens generated using this role will be bound to + BoundCIDRs []*sockaddr.SockAddrMarshaler `json:"bound_cidrs"` } type accessorEntry struct { @@ -1863,6 +1875,10 @@ func (ts *TokenStore) handleCreateCommon(ctx context.Context, req *logical.Reque te.Parent = "" } + if len(role.BoundCIDRs) > 0 { + te.BoundCIDRs = role.BoundCIDRs + } + case data.NoParent: // Only allow an orphan token if the client has sudo policy if !isSudo { @@ -1987,12 +2003,14 @@ func (ts *TokenStore) handleCreateCommon(ctx context.Context, req *logical.Reque te.TTL = explicitMaxTTLToUse } - // Don't advertise non-expiring root tokens as renewable, as attempts to renew them are denied + // Don't advertise non-expiring root tokens as renewable, as attempts to + // renew them are denied. Don't CIDR-restrict these either. if te.TTL == 0 { if parent.TTL != 0 { return logical.ErrorResponse("expiring root tokens cannot create non-expiring root tokens"), logical.ErrInvalidRequest } renewable = false + te.BoundCIDRs = nil } // Create the token @@ -2185,10 +2203,15 @@ func (ts *TokenStore) handleLookup(ctx context.Context, req *logical.Request, da if out.Role != "" { resp.Data["role"] = out.Role } + if out.Period != 0 { resp.Data["period"] = int64(out.Period.Seconds()) } + if len(out.BoundCIDRs) > 0 { + resp.Data["bound_cidrs"] = out.BoundCIDRs + } + // Fetch the last renewal time leaseTimes, err := ts.expiration.FetchLeaseTimesByToken(out.Path, out.ID) if err != nil { @@ -2370,6 +2393,10 @@ func (ts *TokenStore) tokenStoreRoleRead(ctx context.Context, req *logical.Reque }, } + if len(role.BoundCIDRs) > 0 { + resp.Data["bound_cidrs"] = role.BoundCIDRs + } + return resp, nil } @@ -2429,6 +2456,22 @@ func (ts *TokenStore) tokenStoreRoleCreateUpdate(ctx context.Context, req *logic entry.Renewable = data.Get("renewable").(bool) } + boundCIDRsRaw, ok := data.GetOk("bound_cidrs") + if ok { + boundCIDRs := boundCIDRsRaw.([]string) + if len(boundCIDRs) > 0 { + var parsedCIDRs []*sockaddr.SockAddrMarshaler + for _, v := range boundCIDRs { + parsedCIDR, err := sockaddr.NewSockAddr(v) + if err != nil { + return logical.ErrorResponse(errwrap.Wrapf(fmt.Sprintf("invalid value %q when parsing bound cidrs: {{err}}", v), err).Error()), nil + } + parsedCIDRs = append(parsedCIDRs, &sockaddr.SockAddrMarshaler{parsedCIDR}) + } + entry.BoundCIDRs = parsedCIDRs + } + } + var resp *logical.Response explicitMaxTTLInt, ok := data.GetOk("explicit_max_ttl") diff --git a/vault/token_store_ext_test.go b/vault/token_store_ext_test.go index 3ebc1928e2eb..1ff6abd14a4f 100644 --- a/vault/token_store_ext_test.go +++ b/vault/token_store_ext_test.go @@ -3,6 +3,7 @@ package vault_test import ( "reflect" "sort" + "strings" "testing" "github.com/hashicorp/vault/api" @@ -217,3 +218,121 @@ func TestTokenStore_IdentityPolicies(t *testing.T) { t.Fatalf("bad: identity policies; expected: %#v\nactual: %#v", expectedPolicies, actualPolicies) } } + +func TestTokenStore_CIDRBlocks(t *testing.T) { + cluster := vault.NewTestCluster(t, nil, &vault.TestClusterOptions{ + HandlerFunc: vaulthttp.Handler, + }) + cluster.Start() + defer cluster.Cleanup() + + core := cluster.Cores[0].Core + vault.TestWaitActive(t, core) + client := cluster.Cores[0].Client + rootToken := client.Token() + + var err error + var secret *api.Secret + + // Test normally + _, err = client.Logical().Write("auth/token/roles/testrole", map[string]interface{}{ + "bound_cidrs": []string{}, + }) + if err != nil { + t.Fatal(err) + } + secret, err = client.Auth().Token().CreateWithRole(&api.TokenCreateRequest{ + Policies: []string{"default"}, + }, "testrole") + if err != nil { + t.Fatal(err) + } + client.SetToken(secret.Auth.ClientToken) + _, err = client.Auth().Token().LookupSelf() + if err != nil { + t.Fatal(err) + } + + // CIDR blocks, containing localhost + client.SetToken(rootToken) + _, err = client.Logical().Write("auth/token/roles/testrole", map[string]interface{}{ + "bound_cidrs": []string{"127.0.0.1/32", "1.2.3.4/8", "5.6.7.8/24"}, + }) + if err != nil { + t.Fatal(err) + } + secret, err = client.Auth().Token().CreateWithRole(&api.TokenCreateRequest{ + Policies: []string{"default"}, + }, "testrole") + if err != nil { + t.Fatal(err) + } + client.SetToken(secret.Auth.ClientToken) + _, err = client.Auth().Token().LookupSelf() + if err != nil { + t.Fatal(err) + } + + // CIDR blocks, not containing localhost (should fail) + client.SetToken(rootToken) + _, err = client.Logical().Write("auth/token/roles/testrole", map[string]interface{}{ + "bound_cidrs": []string{"1.2.3.4/8", "5.6.7.8/24"}, + }) + if err != nil { + t.Fatal(err) + } + secret, err = client.Auth().Token().CreateWithRole(&api.TokenCreateRequest{ + Policies: []string{"default"}, + }, "testrole") + if err != nil { + t.Fatal(err) + } + client.SetToken(secret.Auth.ClientToken) + _, err = client.Auth().Token().LookupSelf() + if err == nil { + t.Fatal("expected error") + } + if !strings.Contains(err.Error(), "permission denied") { + t.Fatalf("unexpected error: %v", err) + } + + // Root token, no ttl, should work + client.SetToken(rootToken) + _, err = client.Logical().Write("auth/token/roles/testrole", map[string]interface{}{ + "bound_cidrs": []string{"1.2.3.4/8", "5.6.7.8/24"}, + }) + if err != nil { + t.Fatal(err) + } + secret, err = client.Auth().Token().CreateWithRole(&api.TokenCreateRequest{}, "testrole") + if err != nil { + t.Fatal(err) + } + client.SetToken(secret.Auth.ClientToken) + _, err = client.Auth().Token().LookupSelf() + if err != nil { + t.Fatal(err) + } + + // Root token, ttl, should not work + client.SetToken(rootToken) + _, err = client.Logical().Write("auth/token/roles/testrole", map[string]interface{}{ + "bound_cidrs": []string{"1.2.3.4/8", "5.6.7.8/24"}, + "period": 3600, + }) + if err != nil { + t.Fatal(err) + } + secret, err = client.Auth().Token().CreateWithRole(&api.TokenCreateRequest{}, "testrole") + if err != nil { + t.Fatal(err) + } + client.SetToken(secret.Auth.ClientToken) + _, err = client.Auth().Token().LookupSelf() + if err == nil { + t.Fatal("expected error") + } + if !strings.Contains(err.Error(), "permission denied") { + t.Fatalf("unexpected error: %v", err) + } +} diff --git a/website/source/api/auth/token/index.html.md b/website/source/api/auth/token/index.html.md index 9ffff1c5750f..06d13ea1d24e 100644 --- a/website/source/api/auth/token/index.html.md +++ b/website/source/api/auth/token/index.html.md @@ -667,6 +667,12 @@ tokens created against a role to be revoked using the The suffix can be changed, allowing new callers to have the new suffix as part of their path, and then tokens with the old suffix can be revoked via `/sys/leases/revoke-prefix`. +- `bound_cidrs` `(string: "", or list: [])` – If set, restricts usage of the + generated token to client IPs falling within the range of the specified + CIDR(s). Unlike most other role parameters, this is not reevaluated from the + current role value at each usage; it is set on the token itself. Root tokens + with no TTL will not be bound by these CIDRs; root tokens with TTLs will be + bound by these CIDRs. ### Sample Payload @@ -676,6 +682,7 @@ tokens created against a role to be revoked using the ], "name": "nomad", "orphan": false, + "bound_cidrs": ["127.0.0.1/32", "128.252.0.0/16"], "renewable": true ``` diff --git a/website/source/docs/concepts/tokens.html.md b/website/source/docs/concepts/tokens.html.md index 52a8414fcc13..cfb420a45f81 100644 --- a/website/source/docs/concepts/tokens.html.md +++ b/website/source/docs/concepts/tokens.html.md @@ -204,5 +204,14 @@ to be given periodic tokens. There are a few important things to know when using periodic tokens: -* When a periodic token is created via a token store role, the _current_ value of the role's period setting will be used at renewal time -* A token with both a period and an explicit max TTL will act like a periodic token but will be revoked when the explicit max TTL is reached +* When a periodic token is created via a token store role, the _current_ value + of the role's period setting will be used at renewal time +* A token with both a period and an explicit max TTL will act like a periodic + token but will be revoked when the explicit max TTL is reached + +### CIDR-Bound Tokens + +Some tokens are able to be bound to CIDR(s) that restrict the range of client +IPs allowed to use them. These affect all tokens except for non-expiring root +tokens (those with a TTL of zero). If a root token has an expiration, it also +is affected by CIDR-binding.