diff --git a/api/auth_token_test.go b/api/auth_token_test.go index 4e92f1043f..a4ca5f843e 100644 --- a/api/auth_token_test.go +++ b/api/auth_token_test.go @@ -130,13 +130,13 @@ func TestAuthTokenRenew(t *testing.T) { client.SetToken(secret.Auth.ClientToken) // Now attempt a renew with the new token - secret, err = client.Auth().Token().Renew(secret.Auth.ClientToken, 0) + secret, err = client.Auth().Token().Renew(secret.Auth.ClientToken, 3600) if err != nil { t.Fatal(err) } if secret.Auth.LeaseDuration != 3600 { - t.Errorf("expected 1h, got %q", secret.Auth.LeaseDuration) + t.Errorf("expected 1h, got %v", secret.Auth.LeaseDuration) } if secret.Auth.Renewable != true { @@ -144,13 +144,13 @@ func TestAuthTokenRenew(t *testing.T) { } // Do the same thing with the self variant - secret, err = client.Auth().Token().RenewSelf(0) + secret, err = client.Auth().Token().RenewSelf(3600) if err != nil { t.Fatal(err) } if secret.Auth.LeaseDuration != 3600 { - t.Errorf("expected 1h, got %q", secret.Auth.LeaseDuration) + t.Errorf("expected 1h, got %v", secret.Auth.LeaseDuration) } if secret.Auth.Renewable != true { diff --git a/http/logical_test.go b/http/logical_test.go index d38b616588..7e2387f3cf 100644 --- a/http/logical_test.go +++ b/http/logical_test.go @@ -168,7 +168,7 @@ func TestLogical_CreateToken(t *testing.T) { "policies": []interface{}{"root"}, "metadata": nil, "lease_duration": float64(0), - "renewable": false, + "renewable": true, }, "warnings": nilWarnings, } diff --git a/logical/framework/backend_test.go b/logical/framework/backend_test.go index e956f5ff50..946c7a90a0 100644 --- a/logical/framework/backend_test.go +++ b/logical/framework/backend_test.go @@ -248,9 +248,14 @@ func TestBackendHandleRequest_renew(t *testing.T) { } func TestBackendHandleRequest_renewExtend(t *testing.T) { + sysView := logical.StaticSystemView{ + DefaultLeaseTTLVal: 5 * time.Minute, + MaxLeaseTTLVal: 30 * time.Hour, + } + secret := &Secret{ Type: "foo", - Renew: LeaseExtend(0, 0, false), + Renew: LeaseExtend(0, 0, sysView), DefaultDuration: 5 * time.Minute, } b := &Backend{ @@ -268,7 +273,7 @@ func TestBackendHandleRequest_renewExtend(t *testing.T) { t.Fatal("should have secret") } - if resp.Secret.TTL < 60*time.Minute || resp.Secret.TTL > 70*time.Minute { + if resp.Secret.TTL < 59*time.Minute || resp.Secret.TTL > 61*time.Minute { t.Fatalf("bad: %s", resp.Secret.TTL) } } diff --git a/logical/framework/lease.go b/logical/framework/lease.go index 353962648f..3e5ebe154d 100644 --- a/logical/framework/lease.go +++ b/logical/framework/lease.go @@ -7,77 +7,79 @@ import ( "github.com/hashicorp/vault/logical" ) -// LeaseExtend returns an OperationFunc that can be used to simply extend -// the lease of the auth/secret for the duration that was requested. Max -// is the max time past the _current_ time that a lease can be extended. i.e. -// setting it to 2 hours forces a renewal within the next 2 hours again. +// LeaseExtend returns an OperationFunc that can be used to simply extend the +// lease of the auth/secret for the duration that was requested. // -// maxSession is the maximum session length allowed since the original -// issue time. If this is zero, it is ignored. +// backendIncrement is the backend's requested increment -- perhaps from a user +// request, perhaps from a role/config value. If not set, uses the mount/system +// value. // -// maxFromLease controls if the maximum renewal period comes from the existing -// lease. This means the value of `max` will be replaced with the existing -// lease duration. -func LeaseExtend(max, maxSession time.Duration, maxFromLease bool) OperationFunc { +// backendMax is the backend's requested increment -- this can be more +// restrictive than the mount/system value but not less. +// +// systemView is the system view from the calling backend, used to determine +// and/or correct default/max times. +func LeaseExtend(backendIncrement, backendMax time.Duration, systemView logical.SystemView) OperationFunc { return func(req *logical.Request, data *FieldData) (*logical.Response, error) { - leaseOpts := detectLease(req) - if leaseOpts == nil { + var leaseOpts *logical.LeaseOptions + switch { + case req.Auth != nil: + leaseOpts = &req.Auth.LeaseOptions + case req.Secret != nil: + leaseOpts = &req.Secret.LeaseOptions + default: return nil, fmt.Errorf("no lease options for request") } - // Check if we should limit max - if maxFromLease { - max = leaseOpts.TTL + // Use the mount's configured max unless the backend specifies + // something more restrictive (perhaps from a role configuration + // parameter) + max := systemView.MaxLeaseTTL() + if backendMax > 0 && backendMax < max { + max = backendMax } - // Sanity check the desired increment - switch { - // Protect against negative leases - case leaseOpts.Increment < 0: - return logical.ErrorResponse( - "increment must be greater than 0"), logical.ErrInvalidRequest - - // If no lease increment, or too large of an increment, use the max - case max > 0 && leaseOpts.Increment == 0, max > 0 && leaseOpts.Increment > max: - leaseOpts.Increment = max + // Should never happen, but guard anyways + if max < 0 { + return nil, fmt.Errorf("max TTL is negative") } + // We cannot go past this time + maxValidTime := leaseOpts.IssueTime.UTC().Add(max) + // Get the current time now := time.Now().UTC() - // Check if we're passed the issue limit - var maxSessionTime time.Time - if maxSession > 0 { - maxSessionTime = leaseOpts.IssueTime.Add(maxSession) - if maxSessionTime.Before(now) { - return logical.ErrorResponse(fmt.Sprintf( - "lease can only be renewed up to %s past original issue", - maxSession)), logical.ErrInvalidRequest + // If we are past the max TTL, we shouldn't be in this function...but + // fast path out if we are + if maxValidTime.Before(now) { + return nil, fmt.Errorf("past the max TTL, cannot renew") + } + + // Basic max safety checks have passed, now let's figure out our + // increment. We'll use the user-supplied value first, then backend-provided default if possible, or the + // mount/system default if not. + increment := leaseOpts.Increment + if increment <= 0 { + if backendIncrement > 0 { + increment = backendIncrement + } else { + increment = systemView.DefaultLeaseTTL() } } - // The new lease is the minimum of the requested Increment - // or the maxSessionTime - requestedLease := now.Add(leaseOpts.Increment) - if !maxSessionTime.IsZero() && requestedLease.After(maxSessionTime) { - requestedLease = maxSessionTime + // We are proposing a time of the current time plus the increment + proposedExpiration := now.Add(increment) + + // If the proposed expiration is after the maximum TTL of the lease, + // cap the increment to whatever is left + if maxValidTime.Before(proposedExpiration) { + increment = maxValidTime.Sub(now) } - // Determine the requested lease - newLeaseDuration := requestedLease.Sub(now) - // Set the lease - leaseOpts.TTL = newLeaseDuration + leaseOpts.TTL = increment return &logical.Response{Auth: req.Auth, Secret: req.Secret}, nil } } - -func detectLease(req *logical.Request) *logical.LeaseOptions { - if req.Auth != nil { - return &req.Auth.LeaseOptions - } else if req.Secret != nil { - return &req.Secret.LeaseOptions - } - return nil -} diff --git a/logical/framework/lease_test.go b/logical/framework/lease_test.go index d7c40c12ce..0ab036a61a 100644 --- a/logical/framework/lease_test.go +++ b/logical/framework/lease_test.go @@ -8,66 +8,80 @@ import ( ) func TestLeaseExtend(t *testing.T) { + + testSysView := logical.StaticSystemView{ + DefaultLeaseTTLVal: 5 * time.Hour, + MaxLeaseTTLVal: 30 * time.Hour, + } + now := time.Now().UTC().Round(time.Hour) cases := map[string]struct { - Max time.Duration - MaxSession time.Duration - Request time.Duration - Result time.Duration - MaxFromLease bool - Error bool + BackendDefault time.Duration + BackendMax time.Duration + Increment time.Duration + Result time.Duration + Error bool }{ - "valid request, good bounds": { - Max: 30 * time.Hour, - Request: 1 * time.Hour, - Result: 1 * time.Hour, + "valid request, good bounds, increment is preferred": { + BackendDefault: 30 * time.Hour, + Increment: 1 * time.Hour, + Result: 1 * time.Hour, }, - "valid request, zero max": { - Max: 0, - Request: 1 * time.Hour, - Result: 1 * time.Hour, + "valid request, zero backend default, uses increment": { + BackendDefault: 0, + Increment: 1 * time.Hour, + Result: 1 * time.Hour, }, - "request is zero": { - Max: 30 * time.Hour, - Request: 0, - Result: 30 * time.Hour, + "lease increment is zero, uses backend default": { + BackendDefault: 30 * time.Hour, + Increment: 0, + Result: 30 * time.Hour, }, - "request is too long": { - Max: 3 * time.Hour, - Request: 7 * time.Hour, - Result: 3 * time.Hour, + "lease increment and default are zero, uses systemview": { + BackendDefault: 0, + Increment: 0, + Result: 5 * time.Hour, }, - "request would go past max session": { - Max: 9 * time.Hour, - MaxSession: 5 * time.Hour, - Request: 7 * time.Hour, - Result: 5 * time.Hour, + "backend max and associated request are too long": { + BackendDefault: 40 * time.Hour, + BackendMax: 45 * time.Hour, + Result: 30 * time.Hour, }, - "request within max session": { - Max: 9 * time.Hour, - MaxSession: 5 * time.Hour, - Request: 4 * time.Hour, - Result: 4 * time.Hour, + "all request values are larger than the system view, so the system view limits": { + BackendDefault: 40 * time.Hour, + BackendMax: 50 * time.Hour, + Increment: 40 * time.Hour, + Result: 30 * time.Hour, }, - // Don't think core will allow this, but let's protect against - // it at multiple layers anyways. - "request is negative": { - Max: 3 * time.Hour, - Request: -7 * time.Hour, - Error: true, + "request within backend max": { + BackendDefault: 9 * time.Hour, + BackendMax: 5 * time.Hour, + Increment: 4 * time.Hour, + Result: 4 * time.Hour, }, - "max form lease, request too large": { - Request: 10 * time.Hour, - MaxFromLease: true, - Result: time.Hour, + "request outside backend max": { + BackendDefault: 9 * time.Hour, + BackendMax: 4 * time.Hour, + Increment: 5 * time.Hour, + Result: 4 * time.Hour, + }, + + "request is negative, no backend default, use sysview": { + Increment: -7 * time.Hour, + Result: 5 * time.Hour, + }, + + "lease increment too large": { + Increment: 40 * time.Hour, + Result: 30 * time.Hour, }, } @@ -77,12 +91,12 @@ func TestLeaseExtend(t *testing.T) { LeaseOptions: logical.LeaseOptions{ TTL: 1 * time.Hour, IssueTime: now, - Increment: tc.Request, + Increment: tc.Increment, }, }, } - callback := LeaseExtend(tc.Max, tc.MaxSession, tc.MaxFromLease) + callback := LeaseExtend(tc.BackendDefault, tc.BackendMax, testSysView) resp, err := callback(req, nil) if (err != nil) != tc.Error { t.Fatalf("bad: %s\nerr: %s", name, err)