From 146cdc69eb675d54676d50524fa36060bd405528 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Fri, 12 Aug 2016 21:01:30 -0400 Subject: [PATCH] Add periodic support for root/sudo tokens to auth/token/create --- api/auth_token.go | 1 + command/token_create.go | 13 ++- command/token_renew.go | 3 +- http/logical_test.go | 1 - http/sys_generate_root_test.go | 2 - vault/token_store.go | 173 +++++++++++++++++++++------------ vault/token_store_test.go | 16 --- 7 files changed, 125 insertions(+), 84 deletions(-) diff --git a/api/auth_token.go b/api/auth_token.go index 2dae4df62a..1901ea1106 100644 --- a/api/auth_token.go +++ b/api/auth_token.go @@ -170,6 +170,7 @@ type TokenCreateRequest struct { Lease string `json:"lease,omitempty"` TTL string `json:"ttl,omitempty"` ExplicitMaxTTL string `json:"explicit_max_ttl,omitempty"` + Period string `json:"period,omitempty"` NoParent bool `json:"no_parent,omitempty"` NoDefaultPolicy bool `json:"no_default_policy,omitempty"` DisplayName string `json:"display_name"` diff --git a/command/token_create.go b/command/token_create.go index a035abe27a..4c17b83718 100644 --- a/command/token_create.go +++ b/command/token_create.go @@ -17,7 +17,7 @@ type TokenCreateCommand struct { func (c *TokenCreateCommand) Run(args []string) int { var format string - var id, displayName, lease, ttl, explicitMaxTTL, role string + var id, displayName, lease, ttl, explicitMaxTTL, period, role string var orphan, noDefaultPolicy, renewable bool var metadata map[string]string var numUses int @@ -29,6 +29,7 @@ func (c *TokenCreateCommand) Run(args []string) int { flags.StringVar(&lease, "lease", "", "") flags.StringVar(&ttl, "ttl", "", "") flags.StringVar(&explicitMaxTTL, "explicit-max-ttl", "", "") + flags.StringVar(&period, "period", "", "") flags.StringVar(&role, "role", "", "") flags.BoolVar(&orphan, "orphan", false, "") flags.BoolVar(&renewable, "renewable", true, "") @@ -71,6 +72,7 @@ func (c *TokenCreateCommand) Run(args []string) int { NumUses: numUses, Renewable: new(bool), ExplicitMaxTTL: explicitMaxTTL, + Period: period, } *tcr.Renewable = renewable @@ -135,6 +137,11 @@ Token Options: configuration file, this lifetime is a hard limit set on the token itself and cannot be exceeded. + -period="1h" If specified, the token will be periodic; it will + have no maximum TTL (unless an "explicit-max-ttl" is + also set) but every renewal will use the given + period. Requires a root/sudo token to use. + -renewable=true Whether or not the token is renewable to extend its TTL up to Vault's configured maximum TTL for tokens. This defaults to true; set to false to disable @@ -145,8 +152,8 @@ Token Options: times. -orphan If specified, the token will have no parent. Only - root tokens can create orphan tokens. This prevents - the new token from being revoked with your token. + This prevents the new token from being revoked with + your token. Requires a root/sudo token to use. -no-default-policy If specified, the token will not have the "default" policy included in its policy set. diff --git a/command/token_renew.go b/command/token_renew.go index 3f78cb37a8..9f64263a89 100644 --- a/command/token_renew.go +++ b/command/token_renew.go @@ -6,6 +6,7 @@ import ( "time" "github.com/hashicorp/vault/api" + "github.com/hashicorp/vault/helper/duration" "github.com/hashicorp/vault/meta" ) @@ -43,7 +44,7 @@ func (c *TokenRenewCommand) Run(args []string) int { increment = args[1] } if increment != "" { - dur, err := time.ParseDuration(increment) + dur, err := duration.ParseDurationSecond(increment) if err != nil { c.Ui.Error(fmt.Sprintf("Invalid increment: %s", err)) return 1 diff --git a/http/logical_test.go b/http/logical_test.go index ebb8f82f0d..18d9f046b7 100644 --- a/http/logical_test.go +++ b/http/logical_test.go @@ -144,7 +144,6 @@ func TestLogical_StandbyRedirect(t *testing.T) { "id": root, "ttl": json.Number("0"), "creation_ttl": json.Number("0"), - "role": "", "explicit_max_ttl": json.Number("0"), }, "warnings": nilWarnings, diff --git a/http/sys_generate_root_test.go b/http/sys_generate_root_test.go index 24f212c2dc..6d7a287a6f 100644 --- a/http/sys_generate_root_test.go +++ b/http/sys_generate_root_test.go @@ -303,7 +303,6 @@ func TestSysGenerateRoot_Update_OTP(t *testing.T) { "creation_ttl": json.Number("0"), "ttl": json.Number("0"), "path": "auth/token/root", - "role": "", "explicit_max_ttl": json.Number("0"), } @@ -386,7 +385,6 @@ func TestSysGenerateRoot_Update_PGP(t *testing.T) { "creation_ttl": json.Number("0"), "ttl": json.Number("0"), "path": "auth/token/root", - "role": "", "explicit_max_ttl": json.Number("0"), } diff --git a/vault/token_store.go b/vault/token_store.go index bd02fa625a..1d3205889c 100644 --- a/vault/token_store.go +++ b/vault/token_store.go @@ -473,6 +473,11 @@ type TokenEntry struct { // If set, the role that was used for parameters at creation time Role string `json:"role" mapstructure:"role" structs:"role"` + + // If set, the period of the token. This is only used when created directly + // through the create endpoint; periods managed by roles or other auth + // backends are subject to those renewal rules. + Period time.Duration `json:"period" mapstructure:"period" structs:"period"` } // tsRoleEntry contains token store role information @@ -1064,6 +1069,7 @@ func (ts *TokenStore) handleCreateCommon( ExplicitMaxTTL string `mapstructure:"explicit_max_ttl"` DisplayName string `mapstructure:"display_name"` NumUses int `mapstructure:"num_uses"` + Period string } if err := mapstructure.WeakDecode(req.Data, &data); err != nil { return logical.ErrorResponse(fmt.Sprintf( @@ -1186,6 +1192,24 @@ func (ts *TokenStore) handleCreateCommon( // Do not add the 'default' policy if requested not to. te.Policies = policyutil.SanitizePolicies(data.Policies, !data.NoDefaultPolicy) + // Prevent internal policies from being assigned to tokens + for _, policy := range te.Policies { + if strutil.StrListContains(nonAssignablePolicies, policy) { + return logical.ErrorResponse(fmt.Sprintf("cannot assign %s policy", policy)), nil + } + } + + // Prevent attempts to create a root token without an actual root token as parent. + // This is to thwart privilege escalation by tokens having 'sudo' privileges. + if strutil.StrListContains(data.Policies, "root") && !strutil.StrListContains(parent.Policies, "root") { + return logical.ErrorResponse("root tokens may not be created without parent token being root"), logical.ErrInvalidRequest + } + + // + // NOTE: Do not modify policies below this line. We need the checks above + // to be the last checks as they must look at the final policy set. + // + switch { case role != nil: if role.Orphan { @@ -1219,6 +1243,21 @@ func (ts *TokenStore) handleCreateCommon( te.ExplicitMaxTTL = dur } + if data.Period != "" { + if !isSudo { + return logical.ErrorResponse("root or sudo privileges required to create periodic token"), + logical.ErrInvalidRequest + } + dur, err := duration.ParseDurationSecond(data.Period) + if err != nil { + return logical.ErrorResponse(err.Error()), logical.ErrInvalidRequest + } + if dur < 0 { + return logical.ErrorResponse("period must be positive"), logical.ErrInvalidRequest + } + te.Period = dur + } + // Parse the TTL/lease if any if data.TTL != "" { dur, err := duration.ParseDurationSecond(data.TTL) @@ -1241,28 +1280,52 @@ func (ts *TokenStore) handleCreateCommon( te.TTL = dur } - // Prevent attempts to create a root token without an actual root token as parent. - // This is to thwart privilege escalation by tokens having 'sudo' privileges. - if strutil.StrListContains(data.Policies, "root") && !strutil.StrListContains(parent.Policies, "root") { - return logical.ErrorResponse("root tokens may not be created without parent token being root"), logical.ErrInvalidRequest - } - - // Set the lesser explicit max TTL if defined - if role != nil && role.ExplicitMaxTTL != 0 { - switch { - case te.ExplicitMaxTTL == 0: - te.ExplicitMaxTTL = role.ExplicitMaxTTL - default: - if role.ExplicitMaxTTL < te.ExplicitMaxTTL { + // Set the lesser period/explicit max TTL if defined both in arguments and in role + if role != nil { + if role.ExplicitMaxTTL != 0 { + switch { + case te.ExplicitMaxTTL == 0: te.ExplicitMaxTTL = role.ExplicitMaxTTL + default: + if role.ExplicitMaxTTL < te.ExplicitMaxTTL { + te.ExplicitMaxTTL = role.ExplicitMaxTTL + } + resp.AddWarning(fmt.Sprintf("Explicit max TTL specified both during creation call and in role; using the lesser value of %d seconds", int64(te.ExplicitMaxTTL.Seconds()))) + } + } + if role.Period != 0 { + switch { + case te.Period == 0: + te.Period = role.Period + default: + if role.Period < te.Period { + te.Period = role.Period + } + resp.AddWarning(fmt.Sprintf("Period specified both during creation call and in role; using the lesser value of %d seconds", int64(te.Period.Seconds()))) } - resp.AddWarning(fmt.Sprintf("Explicit max TTL specified both during creation call and in role; using the lesser value of %d seconds", int64(te.ExplicitMaxTTL.Seconds()))) } } sysView := ts.System() - // Run some bounding checks if the explicit max TTL is set + if role != nil && role.Period > 0 { + te.TTL = role.Period + } else if te.Period > 0 { + te.TTL = te.Period + } else { + // Set the default lease if not provided, root tokens are exempt + if te.TTL == 0 && !strutil.StrListContains(te.Policies, "root") { + te.TTL = sysView.DefaultLeaseTTL() + } + + // Limit the lease duration + if te.TTL > sysView.MaxLeaseTTL() && sysView.MaxLeaseTTL() != 0 { + te.TTL = sysView.MaxLeaseTTL() + } + } + + // Run some bounding checks if the explicit max TTL is set; we do not check + // period as it's defined to escape the max TTL if te.ExplicitMaxTTL > 0 { // Limit the lease duration if sysView.MaxLeaseTTL() != 0 && te.ExplicitMaxTTL > sysView.MaxLeaseTTL() { @@ -1284,24 +1347,6 @@ func (ts *TokenStore) handleCreateCommon( } } - if role != nil && role.Period > 0 { - // Periodic tokens are allowed to escape max TTL confines so don't check limits - if te.ExplicitMaxTTL > 0 { - return logical.ErrorResponse("using an explicit max TTL not supported when using periodic token roles"), nil - } - te.TTL = role.Period - } else { - // Set the default lease if not provided, root tokens are exempt - if te.TTL == 0 && !strutil.StrListContains(te.Policies, "root") { - te.TTL = sysView.DefaultLeaseTTL() - } - - // Limit the lease duration - if te.TTL > sysView.MaxLeaseTTL() && sysView.MaxLeaseTTL() != 0 { - te.TTL = sysView.MaxLeaseTTL() - } - } - // Don't advertise non-expiring root tokens as renewable, as attempts to renew them are denied if te.TTL == 0 { if parent.TTL != 0 { @@ -1310,13 +1355,6 @@ func (ts *TokenStore) handleCreateCommon( renewable = false } - // Prevent internal policies from being assigned to tokens - for _, policy := range te.Policies { - if strutil.StrListContains(nonAssignablePolicies, policy) { - return logical.ErrorResponse(fmt.Sprintf("cannot assign %s policy", policy)), nil - } - } - // Create the token if err := ts.create(&te); err != nil { return logical.ErrorResponse(err.Error()), logical.ErrInvalidRequest @@ -1466,7 +1504,6 @@ func (ts *TokenStore) handleLookup( "creation_time": int64(out.CreationTime), "creation_ttl": int64(out.TTL.Seconds()), "ttl": int64(0), - "role": out.Role, "explicit_max_ttl": int64(out.ExplicitMaxTTL.Seconds()), }, } @@ -1475,6 +1512,12 @@ func (ts *TokenStore) handleLookup( resp.Data["orphan"] = true } + if out.Role != "" { + resp.Data["role"] = out.Role + } else if out.Period != 0 { + resp.Data["period"] = int64(out.Period.Seconds()) + } + // Fetch the last renewal time leaseTimes, err := ts.expiration.FetchLeaseTimesByToken(out.Path, out.ID) if err != nil { @@ -1558,8 +1601,27 @@ func (ts *TokenStore) authRenew( f := framework.LeaseExtend(req.Auth.Increment, te.ExplicitMaxTTL, ts.System()) - // No role? Use normal LeaseExtend semantics + // If (te/role).Period is not zero, this is a periodic token. The TTL for a + // periodic token is always the same (the period value). It is not subject + // to normal maximum TTL checks that would come from calling LeaseExtend, + // so we fast path it. + // + // The one wrinkle here is if the token has an explicit max TTL. If both + // are set, we treat it as a regular token and use the periodic value as + // the increment. + + // No role? Use normal LeaseExtend semantics, taking into account + // TokenEntry properties if te.Role == "" { + //Explicit max TTL overrides the period, if both are set + if te.Period != 0 { + if te.ExplicitMaxTTL == 0 { + req.Auth.TTL = te.Period + return &logical.Response{Auth: req.Auth}, nil + } else { + f = framework.LeaseExtend(te.Period, te.ExplicitMaxTTL, ts.System()) + } + } return f(req, d) } @@ -1572,19 +1634,14 @@ func (ts *TokenStore) authRenew( return nil, fmt.Errorf("original token role (%s) could not be found, not renewing", te.Role) } - // If role.Period is not zero, this is a periodic token. The TTL for a - // periodic token is always the same (the role's period value). It is not - // subject to normal maximum TTL checks that would come from calling - // LeaseExtend, so we fast path it. - // - // The one wrinkle here is if the token has an explicit max TTL. Roles - // don't support having both configured, but they could be changed. We - // don't support tokens that are both periodic and have an explicit max - // TTL, so if the token has one, we treat it as a regular token even if the - // role is periodic. - if role.Period != 0 && te.ExplicitMaxTTL == 0 { - req.Auth.TTL = role.Period - return &logical.Response{Auth: req.Auth}, nil + // Same deal here, but using the role period + if role.Period != 0 { + if te.ExplicitMaxTTL == 0 { + req.Auth.TTL = role.Period + return &logical.Response{Auth: req.Auth}, nil + } else { + f = framework.LeaseExtend(role.Period, te.ExplicitMaxTTL, ts.System()) + } } return f(req, d) @@ -1773,12 +1830,6 @@ func (ts *TokenStore) tokenStoreRoleCreateUpdate( resp.AddWarning("Both 'allowed_policies' and 'disallowed_policies' are set; only 'allowed_policies' will take effect") } - // Explicit max TTLs and periods cannot be used at the same time since the - // purpose of a periodic token is to escape max TTL semantics - if entry.Period > 0 && entry.ExplicitMaxTTL > 0 { - return logical.ErrorResponse("a role cannot be used to issue both periodic tokens and tokens with explicit max TTLs"), logical.ErrInvalidRequest - } - // Store it jsonEntry, err := logical.StorageEntryJSON(fmt.Sprintf("%s%s", rolesPrefix, name), entry) if err != nil { diff --git a/vault/token_store_test.go b/vault/token_store_test.go index d40c1073b2..54435414d3 100644 --- a/vault/token_store_test.go +++ b/vault/token_store_test.go @@ -1161,7 +1161,6 @@ func TestTokenStore_HandleRequest_Lookup(t *testing.T) { "num_uses": 0, "creation_ttl": int64(0), "ttl": int64(0), - "role": "", "explicit_max_ttl": int64(0), } @@ -1197,7 +1196,6 @@ func TestTokenStore_HandleRequest_Lookup(t *testing.T) { "num_uses": 0, "creation_ttl": int64(3600), "ttl": int64(3600), - "role": "", "explicit_max_ttl": int64(0), "renewable": true, } @@ -1240,7 +1238,6 @@ func TestTokenStore_HandleRequest_Lookup(t *testing.T) { "num_uses": 0, "creation_ttl": int64(3600), "ttl": int64(3600), - "role": "", "explicit_max_ttl": int64(0), "renewable": true, } @@ -1306,7 +1303,6 @@ func TestTokenStore_HandleRequest_LookupSelf(t *testing.T) { "num_uses": 0, "creation_ttl": int64(0), "ttl": int64(0), - "role": "", "explicit_max_ttl": int64(0), } @@ -1510,18 +1506,6 @@ func TestTokenStore_RoleCRUD(t *testing.T) { t.Fatalf("bad: expected:%#v\nactual:%#v", expected, resp.Data) } - // Now test setting explicit max ttl at the same time as period, which - // should be an error - req.Operation = logical.CreateOperation - req.Data = map[string]interface{}{ - "explicit_max_ttl": "5", - } - - resp, err = core.HandleRequest(req) - if err == nil { - t.Fatalf("expected error") - } - // Now set explicit max ttl and clear the period req.Operation = logical.CreateOperation req.Data = map[string]interface{}{