luxolus
/
open-vault
2
0
Fork 0
Code Issues 1 Pull Requests Releases Activity

Patch expiration fix over from ENT (#11650) 

* Patch expiration fix over from ENT

* Rename changelog
This commit is contained in:
Scott Miller 2021-05-18 16:55:38 -05:00 committed by GitHub
parent ec5a0e96d4
commit 6b8d7fe2e6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 169 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
changelog/11650.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:bug
core: correct logic for renewal of leases nearing their expiration time.
```

2
sdk/framework/lease.go
View File

@ -91,7 +91,7 @@ func CalculateTTL(sysView logical.SystemView, increment, backendTTL, period, bac
// If we are past the max TTL, we shouldn't be in this function...but
// fast path out if we are
if maxValidTTL < 0 {
if maxValidTTL <= 0 {
return 0, nil, fmt.Errorf("past the max TTL, cannot renew")
}

28
vault/expiration.go
View File

@ -629,6 +629,7 @@ func (m *ExpirationManager) Tidy(ctx context.Context) error {
} else {
tokenCache[le.ClientToken] = true
}
goto REVOKE_CHECK
} else {
if isValid {
@ -844,6 +845,7 @@ func (m *ExpirationManager) processRestore(ctx context.Context, leaseID string)
if err != nil {
return err
}
return nil
}
@ -1001,7 +1003,13 @@ func (m *ExpirationManager) revokeCommon(ctx context.Context, leaseID string, fo
if m.logger.IsInfo() && !skipToken && m.logLeaseExpirations {
m.logger.Info("revoked lease", "lease_id", leaseID)
}
if m.logger.IsWarn() && !skipToken && le.isIncorrectlyNonExpiring() {
var accessor string
if le.Auth != nil {
accessor = le.Auth.Accessor
}
m.logger.Warn("finished revoking incorrectly non-expiring lease", "leaseID", le.LeaseID, "accessor", accessor)
}
return nil
}
@ -1727,15 +1735,14 @@ func (m *ExpirationManager) updatePendingInternal(le *leaseEntry) {
info, leaseInPending := m.pending.Load(le.LeaseID)
var pending pendingInfo
if le.ExpireTime.IsZero() {
if le.nonexpiringToken() {
if le.ExpireTime.IsZero() && le.nonexpiringToken() {
// Store this in the nonexpiring map instead of pending.
// There does not appear to be any cases where a token that had
// a nonzero can be can be assigned a zero TTL, but we can handle that
// anyway by falling through to the next check.
// a nonzero can be can be assigned a zero TTL, but that can be
// handled by the next check
pending.cachedLeaseInfo = m.inMemoryLeaseInfo(le)
m.nonexpiring.Store(le.LeaseID, pending)
}
// if the timer happened to exist, stop the time and delete it from the
// pending timers.
@ -2482,7 +2489,10 @@ func (le *leaseEntry) nonexpiringToken() bool {
if le.Auth == nil {
return false
}
return !le.Auth.LeaseEnabled()
// Note that at this time the only non-expiring tokens are root tokens, this test is more involved as it is trying
// to catch tokens created by the VAULT-1949 non-expiring tokens bug and ensure they become expiring.
return !le.Auth.LeaseEnabled() && len(le.Auth.Policies) == 1 && le.Auth.Policies[0] == "root" && le.namespace != nil &&
le.namespace.ID == namespace.RootNamespaceID
}
// TODO maybe lock RevokeErr once this goes in: https://github.com/hashicorp/vault/pull/11122
@ -2490,6 +2500,10 @@ func (le *leaseEntry) isIrrevocable() bool {
return le.RevokeErr != ""
}
func (le *leaseEntry) isIncorrectlyNonExpiring() bool {
return le.ExpireTime.IsZero() && !le.nonexpiringToken()
}
// decodeLeaseEntry is used to reverse encode and return a new entry
func decodeLeaseEntry(buf []byte) (*leaseEntry, error) {
out := new(leaseEntry)

138
vault/expiration_test.go
View File

@ -917,6 +917,7 @@ func TestExpiration_RegisterAuth_NoLease(t *testing.T) {
auth := &logical.Auth{
ClientToken: root.ID,
Policies: []string{"root"},
}
te := &logical.TokenEntry{
@ -1706,7 +1707,6 @@ func TestExpiration_Renew_NotRenewable(t *testing.T) {
t.Fatalf("Bad: %#v", noop.Requests)
}
}
func TestExpiration_Renew_RevokeOnExpire(t *testing.T) {
exp := mockExpiration(t)
noop := &NoopBackend{}
@ -1783,6 +1783,142 @@ func TestExpiration_Renew_RevokeOnExpire(t *testing.T) {
}
}
func TestExpiration_Renew_FinalSecond(t *testing.T) {
exp := mockExpiration(t)
noop := &NoopBackend{}
_, barrier, _ := mockBarrier(t)
view := NewBarrierView(barrier, "logical/")
meUUID, err := uuid.GenerateUUID()
if err != nil {
t.Fatal(err)
}
err = exp.router.Mount(noop, "prod/aws/", &MountEntry{Path: "prod/aws/", Type: "noop", UUID: meUUID, Accessor: "noop-accessor", namespace: namespace.RootNamespace}, view)
if err != nil {
t.Fatal(err)
}
req := &logical.Request{
Operation: logical.ReadOperation,
Path: "prod/aws/foo",
ClientToken: "foobar",
}
req.SetTokenEntry(&logical.TokenEntry{ID: "foobar", NamespaceID: "root"})
resp := &logical.Response{
Secret: &logical.Secret{
LeaseOptions: logical.LeaseOptions{
TTL: 2 * time.Second,
Renewable: true,
},
},
Data: map[string]interface{}{
"access_key": "xyz",
"secret_key": "abcd",
},
}
ctx := namespace.RootContext(nil)
id, err := exp.Register(ctx, req, resp)
if err != nil {
t.Fatalf("err: %v", err)
}
le, err := exp.loadEntry(ctx, id)
if err != nil {
t.Fatalf("err: %v", err)
}
// Give it an auth section to emulate the real world bug
le.Auth = &logical.Auth{
LeaseOptions: logical.LeaseOptions{
Renewable: true,
},
}
exp.persistEntry(ctx, le)
noop.Response = &logical.Response{
Secret: &logical.Secret{
LeaseOptions: logical.LeaseOptions{
TTL: 2 * time.Second,
MaxTTL: 2 * time.Second,
},
},
Data: map[string]interface{}{
"access_key": "123",
"secret_key": "abcd",
},
}
time.Sleep(1000 * time.Millisecond)
_, err = exp.Renew(ctx, id, 0)
if err != nil {
t.Fatalf("err: %v", err)
}
if _, ok := exp.nonexpiring.Load(id); ok {
t.Fatalf("expirable lease became nonexpiring")
}
}
func TestExpiration_Renew_FinalSecond_Lease(t *testing.T) {
exp := mockExpiration(t)
noop := &NoopBackend{}
_, barrier, _ := mockBarrier(t)
view := NewBarrierView(barrier, "logical/")
meUUID, err := uuid.GenerateUUID()
if err != nil {
t.Fatal(err)
}
err = exp.router.Mount(noop, "prod/aws/", &MountEntry{Path: "prod/aws/", Type: "noop", UUID: meUUID, Accessor: "noop-accessor", namespace: namespace.RootNamespace}, view)
if err != nil {
t.Fatal(err)
}
req := &logical.Request{
Operation: logical.ReadOperation,
Path: "prod/aws/foo",
ClientToken: "foobar",
}
req.SetTokenEntry(&logical.TokenEntry{ID: "foobar", NamespaceID: "root"})
resp := &logical.Response{
Secret: &logical.Secret{
LeaseOptions: logical.LeaseOptions{
TTL: 2 * time.Second,
Renewable: true,
},
LeaseID: "abcde",
},
Data: map[string]interface{}{
"access_key": "xyz",
"secret_key": "abcd",
},
}
ctx := namespace.RootContext(nil)
id, err := exp.Register(ctx, req, resp)
if err != nil {
t.Fatalf("err: %v", err)
}
le, err := exp.loadEntry(ctx, id)
if err != nil {
t.Fatalf("err: %v", err)
}
// Give it an auth section to emulate the real world bug
le.Auth = &logical.Auth{
LeaseOptions: logical.LeaseOptions{
Renewable: true,
},
}
exp.persistEntry(ctx, le)
time.Sleep(1000 * time.Millisecond)
_, err = exp.Renew(ctx, id, 0)
if err != nil {
t.Fatalf("err: %v", err)
}
if _, ok := exp.nonexpiring.Load(id); ok {
t.Fatalf("expirable lease became nonexpiring")
}
}
func TestExpiration_revokeEntry(t *testing.T) {
exp := mockExpiration(t)

4
vault/token_store_test.go
View File

@ -3788,7 +3788,7 @@ func TestTokenStore_RolePeriod(t *testing.T) {
req.Operation = logical.UpdateOperation
req.Path = "auth/token/renew-self"
req.Data = map[string]interface{}{
"increment": 1,
"increment": 2,
}
resp, err = core.HandleRequest(namespace.RootContext(nil), req)
if err != nil || (resp != nil && resp.IsError()) {
@ -3845,7 +3845,7 @@ func TestTokenStore_RolePeriod(t *testing.T) {
req.Operation = logical.UpdateOperation
req.Path = "auth/token/renew-self"
req.Data = map[string]interface{}{
"increment": 1,
"increment": 2,
}
resp, err = core.HandleRequest(namespace.RootContext(nil), req)
if err != nil || (resp != nil && resp.IsError()) {

2
vendor/github.com/hashicorp/vault/sdk/framework/lease.go generated vendored
View File

@ -91,7 +91,7 @@ func CalculateTTL(sysView logical.SystemView, increment, backendTTL, period, bac
// If we are past the max TTL, we shouldn't be in this function...but
// fast path out if we are
if maxValidTTL < 0 {
if maxValidTTL <= 0 {
return 0, nil, fmt.Errorf("past the max TTL, cannot renew")
}