Offline token revocation fix

CHANGELOG.md

@@ -1,5 +1,18 @@
 ## 0.10.2 (Unreleased)
 
+SECURITY:
+
+ * The Vault team identified a race condition that could occur if a token's
+   lease expired while Vault was not running. In this case, when Vault came
+   back online, sometimes it would properly revoke the lease but other times it
+   would not, leading to a Vault token that no longer had an expiration and had
+   essentially unlimited lifetime. This race was per-token, not all-or-nothing
+   for all tokens that may have expired during Vault's downtime. We have fixed
+   the behavior and put extra checks in place to help prevent any similar
+   future issues. In addition, the logic we have put in place ensures that such
+   lease-less tokens can no longer be used (unless they are root tokens that
+   never had an expiration to begin with).
+
 DEPRECATIONS/CHANGES:
 
  * PKI duration return types: The PKI backend now returns durations (e.g. when

logical/auth.go

@@ -74,6 +74,12 @@ type Auth struct {
 
 	// The set of CIDRs that this token can be used with
 	BoundCIDRs []*sockaddr.SockAddrMarshaler `json:"bound_cidrs"`
+
+	// CreationPath is a path that the backend can return to use in the lease.
+	// This is currently only supported for the token store where roles may
+	// change the perceived path of the lease, even though they don't change
+	// the request path itself.
+	CreationPath string `json:"creation_path"`
 }
 
 func (a *Auth) GoString() string {

logical/testing.go

@@ -3,15 +3,15 @@ package logical
 import (
 	"context"
 	"reflect"
-	"testing"
 	"time"
 
 	log "github.com/hashicorp/go-hclog"
 	"github.com/hashicorp/vault/helper/logging"
+	"github.com/mitchellh/go-testing-interface"
 )
 
 // TestRequest is a helper to create a purely in-memory Request struct.
-func TestRequest(t *testing.T, op Operation, path string) *Request {
+func TestRequest(t testing.T, op Operation, path string) *Request {
 	return &Request{
 		Operation: op,
 		Path:      path,
@@ -22,7 +22,7 @@ func TestRequest(t *testing.T, op Operation, path string) *Request {
 
 // TestStorage is a helper that can be used from unit tests to verify
 // the behavior of a Storage impl.
-func TestStorage(t *testing.T, s Storage) {
+func TestStorage(t testing.T, s Storage) {
 	keys, err := s.List(context.Background(), "")
 	if err != nil {
 		t.Fatalf("list error: %s", err)

vault/capabilities_test.go

@@ -5,6 +5,7 @@ import (
 	"reflect"
 	"sort"
 	"testing"
+	"time"
 
 	"github.com/hashicorp/vault/logical"
 )
@@ -73,10 +74,9 @@ path "secret/sample" {
 		Path:     "secret/sample",
 		Policies: []string{"policy2"},
 		EntityID: entityID,
+		TTL:      time.Hour,
 	}
-	if err := c.tokenStore.create(context.Background(), ent); err != nil {
-		t.Fatalf("err: %v", err)
-	}
+	testMakeTokenDirectly(t, c.tokenStore, ent)
 
 	actual, err := c.Capabilities(context.Background(), "capabilitiestoken", "secret/sample")
 	if err != nil {
@@ -139,10 +139,9 @@ func TestCapabilities(t *testing.T) {
 		ID:       "capabilitiestoken",
 		Path:     "testpath",
 		Policies: []string{"dev"},
+		TTL:      time.Hour,
 	}
-	if err := c.tokenStore.create(context.Background(), ent); err != nil {
-		t.Fatalf("err: %v", err)
-	}
+	testMakeTokenDirectly(t, c.tokenStore, ent)
 
 	actual, err = c.Capabilities(context.Background(), "capabilitiestoken", "foo/bar")
 	if err != nil {

vault/core_test.go

@@ -494,7 +494,7 @@ func TestCore_HandleRequest_NoSlash(t *testing.T) {
 // Test a root path is denied if non-root
 func TestCore_HandleRequest_RootPath(t *testing.T) {
 	c, _, root := TestCoreUnsealed(t)
-	testCoreMakeToken(t, c, root, "child", "", []string{"test"})
+	testMakeTokenViaCore(t, c, root, "child", "", []string{"test"})
 
 	req := &logical.Request{
 		Operation:   logical.ReadOperation,
@@ -529,7 +529,7 @@ func TestCore_HandleRequest_RootPath_WithSudo(t *testing.T) {
 	}
 
 	// Child token (non-root) but with 'test' policy should have access
-	testCoreMakeToken(t, c, root, "child", "", []string{"test"})
+	testMakeTokenViaCore(t, c, root, "child", "", []string{"test"})
 	req = &logical.Request{
 		Operation:   logical.ReadOperation,
 		Path:        "sys/policy", // root protected!
@@ -547,7 +547,7 @@ func TestCore_HandleRequest_RootPath_WithSudo(t *testing.T) {
 // Check that standard permissions work
 func TestCore_HandleRequest_PermissionDenied(t *testing.T) {
 	c, _, root := TestCoreUnsealed(t)
-	testCoreMakeToken(t, c, root, "child", "", []string{"test"})
+	testMakeTokenViaCore(t, c, root, "child", "", []string{"test"})
 
 	req := &logical.Request{
 		Operation: logical.UpdateOperation,
@@ -567,7 +567,7 @@ func TestCore_HandleRequest_PermissionDenied(t *testing.T) {
 // Check that standard permissions work
 func TestCore_HandleRequest_PermissionAllowed(t *testing.T) {
 	c, _, root := TestCoreUnsealed(t)
-	testCoreMakeToken(t, c, root, "child", "", []string{"test"})
+	testMakeTokenViaCore(t, c, root, "child", "", []string{"test"})
 
 	// Set the 'test' policy object to permit access to secret/
 	req := &logical.Request{

vault/expiration.go

@@ -624,9 +624,8 @@ func (m *ExpirationManager) revokePrefixCommon(prefix string, force bool) error
 				return errwrap.Wrapf(fmt.Sprintf("failed to revoke %q: {{err}}", prefix), err)
 			}
 			return nil
-		} else {
-			prefix = prefix + "/"
 		}
+		prefix = prefix + "/"
 	}
 
 	// Accumulate existing leases
@@ -719,45 +718,6 @@ func (m *ExpirationManager) Renew(leaseID string, increment time.Duration) (*log
 	return resp, nil
 }
 
-// RestoreSaltedTokenCheck verifies that the token is not expired while running
-// in restore mode.  If we are not in restore mode, the lease has already been
-// restored or the lease still has time left, it returns true.
-func (m *ExpirationManager) RestoreSaltedTokenCheck(source string, saltedID string) (bool, error) {
-	defer metrics.MeasureSince([]string{"expire", "restore-token-check"}, time.Now())
-
-	// Return immediately if we are not in restore mode, expiration manager is
-	// already loaded
-	if !m.inRestoreMode() {
-		return true, nil
-	}
-
-	m.restoreModeLock.RLock()
-	defer m.restoreModeLock.RUnlock()
-
-	// Check again after we obtain the lock
-	if !m.inRestoreMode() {
-		return true, nil
-	}
-
-	leaseID := path.Join(source, saltedID)
-
-	m.lockLease(leaseID)
-	defer m.unlockLease(leaseID)
-
-	le, err := m.loadEntryInternal(leaseID, true, true)
-	if err != nil {
-		return false, err
-	}
-	if le != nil && !le.ExpireTime.IsZero() {
-		expires := le.ExpireTime.Sub(time.Now())
-		if expires <= 0 {
-			return false, nil
-		}
-	}
-
-	return true, nil
-}
-
 // RenewToken is used to renew a token which does not need to
 // invoke a logical backend.
 func (m *ExpirationManager) RenewToken(req *logical.Request, source string, token string,
@@ -947,6 +907,7 @@ func (m *ExpirationManager) RegisterAuth(source string, auth *logical.Auth) erro
 
 	// Setup revocation timer
 	m.updatePending(&le, auth.LeaseTotal())
+
 	return nil
 }
 

vault/expiration_test.go

@@ -25,13 +25,13 @@ var (
 
 // mockExpiration returns a mock expiration manager
 func mockExpiration(t testing.TB) *ExpirationManager {
-	_, ts, _, _ := TestCoreWithTokenStore(t)
-	return ts.expiration
+	c, _, _ := TestCoreUnsealed(t)
+	return c.expiration
 }
 
 func mockBackendExpiration(t testing.TB, backend physical.Backend) (*Core, *ExpirationManager) {
-	c, ts, _, _ := TestCoreWithBackendTokenStore(t, backend)
-	return c, ts.expiration
+	c, _, _ := TestCoreUnsealedBackend(t, backend)
+	return c, c.expiration
 }
 
 func TestExpiration_Tidy(t *testing.T) {
@@ -293,7 +293,8 @@ func BenchmarkExpiration_Restore_InMem(b *testing.B) {
 }
 
 func benchmarkExpirationBackend(b *testing.B, physicalBackend physical.Backend, numLeases int) {
-	c, exp := mockBackendExpiration(b, physicalBackend)
+	c, _, _ := TestCoreUnsealedBackend(b, physicalBackend)
+	exp := c.expiration
 	noop := &NoopBackend{}
 	view := NewBarrierView(c.barrier, "logical/")
 	meUUID, err := uuid.GenerateUUID()
@@ -1603,7 +1604,7 @@ func TestLeaseEntry(t *testing.T) {
 }
 
 func TestExpiration_RevokeForce(t *testing.T) {
-	core, _, _, root := TestCoreWithTokenStore(t)
+	core, _, root := TestCoreUnsealed(t)
 
 	core.logicalBackends["badrenew"] = badRenewFactory
 	me := &MountEntry{
@@ -1651,7 +1652,7 @@ func TestExpiration_RevokeForce(t *testing.T) {
 }
 
 func TestExpiration_RevokeForceSingle(t *testing.T) {
-	core, _, _, root := TestCoreWithTokenStore(t)
+	core, _, root := TestCoreUnsealed(t)
 
 	core.logicalBackends["badrenew"] = badRenewFactory
 	me := &MountEntry{

vault/identity_store_test.go

@@ -225,11 +225,9 @@ func TestIdentityStore_WrapInfoInheritance(t *testing.T) {
 		Path:     "test",
 		Policies: []string{"default", responseWrappingPolicyName},
 		EntityID: entityID,
+		TTL:      time.Hour,
 	}
-
-	if err := ts.create(context.Background(), te); err != nil {
-		t.Fatal(err)
-	}
+	testMakeTokenDirectly(t, ts, te)
 
 	wrapReq := &logical.Request{
 		Path:        "sys/wrapping/wrap",
@@ -258,18 +256,17 @@ func TestIdentityStore_WrapInfoInheritance(t *testing.T) {
 }
 
 func TestIdentityStore_TokenEntityInheritance(t *testing.T) {
-	_, ts, _, _ := TestCoreWithTokenStore(t)
+	c, _, _ := TestCoreUnsealed(t)
+	ts := c.tokenStore
 
 	// Create a token which has EntityID set
 	te := &TokenEntry{
 		Path:     "test",
 		Policies: []string{"dev", "prod"},
 		EntityID: "testentityid",
+		TTL:      time.Hour,
 	}
-
-	if err := ts.create(context.Background(), te); err != nil {
-		t.Fatal(err)
-	}
+	testMakeTokenDirectly(t, ts, te)
 
 	// Create a child token; this should inherit the EntityID
 	tokenReq := &logical.Request{
@@ -301,14 +298,12 @@ func TestIdentityStore_TokenEntityInheritance(t *testing.T) {
 
 func testCoreWithIdentityTokenGithub(t *testing.T) (*Core, *IdentityStore, *TokenStore, string) {
 	is, ghAccessor, core := testIdentityStoreWithGithubAuth(t)
-	ts := testTokenStore(t, core)
-	return core, is, ts, ghAccessor
+	return core, is, core.tokenStore, ghAccessor
 }
 
 func testCoreWithIdentityTokenGithubRoot(t *testing.T) (*Core, *IdentityStore, *TokenStore, string, string) {
 	is, ghAccessor, core, root := testIdentityStoreWithGithubAuthRoot(t)
-	ts := testTokenStore(t, core)
-	return core, is, ts, ghAccessor, root
+	return core, is, core.tokenStore, ghAccessor, root
 }
 
 func testIdentityStoreWithGithubAuth(t *testing.T) (*IdentityStore, string, *Core) {

vault/logical_system_test.go

@@ -442,7 +442,7 @@ func TestSystemBackend_PathCapabilities(t *testing.T) {
 	rootCheckFunc(t, resp)
 
 	// Create a non-root token
-	testMakeToken(t, core.tokenStore, rootToken, "tokenid", "", []string{"test"})
+	testMakeTokenViaBackend(t, core.tokenStore, rootToken, "tokenid", "", []string{"test"})
 
 	nonRootCheckFunc := func(t *testing.T, resp *logical.Response) {
 		expected1 := []string{"create", "sudo", "update"}
@@ -544,7 +544,7 @@ func testCapabilities(t *testing.T, endpoint string) {
 		t.Fatalf("err: %v", err)
 	}
 
-	testMakeToken(t, core.tokenStore, rootToken, "tokenid", "", []string{"test"})
+	testMakeTokenViaBackend(t, core.tokenStore, rootToken, "tokenid", "", []string{"test"})
 	req = logical.TestRequest(t, logical.UpdateOperation, endpoint)
 	if endpoint == "capabilities-self" {
 		req.ClientToken = "tokenid"
@@ -600,7 +600,7 @@ func TestSystemBackend_CapabilitiesAccessor_BC(t *testing.T) {
 		t.Fatalf("err: %v", err)
 	}
 
-	testMakeToken(t, core.tokenStore, rootToken, "tokenid", "", []string{"test"})
+	testMakeTokenViaBackend(t, core.tokenStore, rootToken, "tokenid", "", []string{"test"})
 
 	te, err = core.tokenStore.Lookup(context.Background(), "tokenid")
 	if err != nil {
@@ -1264,8 +1264,10 @@ func TestSystemBackend_revokePrefix_origUrl(t *testing.T) {
 	}
 }
 
-func TestSystemBackend_revokePrefixAuth(t *testing.T) {
-	core, ts, _, _ := TestCoreWithTokenStore(t)
+func TestSystemBackend_revokePrefixAuth_newUrl(t *testing.T) {
+	core, _, _ := TestCoreUnsealed(t)
+
+	ts := core.tokenStore
 	bc := &logical.BackendConfig{
 		Logger: core.logger,
 		System: logical.StaticSystemView{
@@ -1284,11 +1286,9 @@ func TestSystemBackend_revokePrefixAuth(t *testing.T) {
 	te := &TokenEntry{
 		ID:   "foo",
 		Path: "auth/github/login/bar",
+		TTL:  time.Hour,
 	}
-	err = ts.create(context.Background(), te)
-	if err != nil {
-		t.Fatal(err)
-	}
+	testMakeTokenDirectly(t, ts, te)
 
 	te, err = ts.Lookup(context.Background(), "foo")
 	if err != nil {
@@ -1329,7 +1329,8 @@ func TestSystemBackend_revokePrefixAuth(t *testing.T) {
 }
 
 func TestSystemBackend_revokePrefixAuth_origUrl(t *testing.T) {
-	core, ts, _, _ := TestCoreWithTokenStore(t)
+	core, _, _ := TestCoreUnsealed(t)
+	ts := core.tokenStore
 	bc := &logical.BackendConfig{
 		Logger: core.logger,
 		System: logical.StaticSystemView{
@@ -1348,11 +1349,9 @@ func TestSystemBackend_revokePrefixAuth_origUrl(t *testing.T) {
 	te := &TokenEntry{
 		ID:   "foo",
 		Path: "auth/github/login/bar",
+		TTL:  time.Hour,
 	}
-	err = ts.create(context.Background(), te)
-	if err != nil {
-		t.Fatal(err)
-	}
+	testMakeTokenDirectly(t, ts, te)
 
 	te, err = ts.Lookup(context.Background(), "foo")
 	if err != nil {
@@ -2396,7 +2395,7 @@ func TestSystemBackend_InternalUIMount(t *testing.T) {
 		t.Fatalf("Bad Response: %#v", resp)
 	}
 
-	testMakeToken(t, core.tokenStore, rootToken, "tokenid", "", []string{"secret"})
+	testMakeTokenViaBackend(t, core.tokenStore, rootToken, "tokenid", "", []string{"secret"})
 
 	req = logical.TestRequest(t, logical.ReadOperation, "internal/ui/mounts/kv")
 	req.ClientToken = "tokenid"

vault/request_handling.go

@@ -610,16 +610,7 @@ func (c *Core) handleRequest(ctx context.Context, req *logical.Request) (retResp
 			return nil, auth, retErr
 		}
 
-		// Register with the expiration manager. We use the token's actual path
-		// here because roles allow suffixes.
-		te, err := c.tokenStore.Lookup(ctx, resp.Auth.ClientToken)
-		if err != nil {
-			c.logger.Error("failed to look up token", "error", err)
-			retErr = multierror.Append(retErr, ErrInternalError)
-			return nil, auth, retErr
-		}
-
-		if err := c.expiration.RegisterAuth(te.Path, resp.Auth); err != nil {
+		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)