Skip to content

Commit

Permalink
Don't allow registering a non-root zero TTL token lease (#7524)
Browse files Browse the repository at this point in the history 
* Don't allow registering a non-root zero TTL token lease

This is defense-in-depth in that such a token was not allowed to be
used; however it's also a bug fix in that this would then cause no lease
to be generated but the token entry to be written, meaning the token
entry would stick around until it was attempted to be used or tidied (in
both cases the internal lookup would see that this was invalid and do a
revoke on the spot).

* Fix tests

* tidy
  • Loading branch information
@jefferai
jefferai authored Nov 5, 2019
1 parent 9f6ae4f commit 986a88b
Show file tree
Hide file tree
Showing 4 changed files with 86 additions and 12 deletions.
8 changes: 7 additions & 1 deletion vault/expiration.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1147,6 +1147,12 @@ func (m *ExpirationManager) Register(ctx context.Context, req *logical.Request,
func (m *ExpirationManager) RegisterAuth(ctx context.Context, te *logical.TokenEntry, auth *logical.Auth) error {
defer metrics.MeasureSince([]string{"expire", "register-auth"}, time.Now())

authExpirationTime := auth.ExpirationTime()

if te.TTL == 0 && authExpirationTime.IsZero() && (len(te.Policies) != 1 || te.Policies[0] != "root") {
return errors.New("refusing to register a lease for a non-root token with no TTL")
}

if te.Type == logical.TokenTypeBatch {
return errors.New("cannot register a lease for a batch token")
}
Expand Down Expand Up @@ -1185,7 +1191,7 @@ func (m *ExpirationManager) RegisterAuth(ctx context.Context, te *logical.TokenE
Auth: auth,
Path: te.Path,
IssueTime: time.Now(),
ExpireTime: auth.ExpirationTime(),
ExpireTime: authExpirationTime,
namespace: tokenNS,
}

Expand Down
50 changes: 50 additions & 0 deletions vault/expiration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -528,6 +528,7 @@ func TestExpiration_RegisterAuth_NoLease(t *testing.T) {
te := &logical.TokenEntry{
ID: root.ID,
Path: "auth/github/login",
Policies: []string{"root"},
NamespaceID: namespace.RootNamespaceID,
}
err = exp.RegisterAuth(namespace.RootContext(nil), te, auth)
Expand Down Expand Up @@ -562,6 +563,55 @@ func TestExpiration_RegisterAuth_NoLease(t *testing.T) {
}
}

// Tests both the expiration function and the core function
func TestExpiration_RegisterAuth_NoTTL(t *testing.T) {
c, _, _ := TestCoreUnsealed(t)
exp := c.expiration
ctx := namespace.RootContext(nil)

root, err := exp.tokenStore.rootToken(context.Background())
if err != nil {
t.Fatalf("err: %v", err)
}

auth := &logical.Auth{
ClientToken: root.ID,
TokenPolicies: []string{"root"},
}

// First on core
err = c.RegisterAuth(ctx, 0, "auth/github/login", auth)
if err != nil {
t.Fatal(err)
}

auth.TokenPolicies[0] = "default"
err = c.RegisterAuth(ctx, 0, "auth/github/login", auth)
if err == nil {
t.Fatal("expected error")
}

// Now expiration
// Should work, root token with zero TTL
te := &logical.TokenEntry{
ID: root.ID,
Path: "auth/github/login",
Policies: []string{"root"},
NamespaceID: namespace.RootNamespaceID,
}
err = exp.RegisterAuth(ctx, te, auth)
if err != nil {
t.Fatalf("err: %v", err)
}

// Test non-root token with zero TTL
te.Policies = []string{"default"}
err = exp.RegisterAuth(ctx, te, auth)
if err == nil {
t.Fatal("expected error")
}
}

func TestExpiration_Revoke(t *testing.T) {
exp := mockExpiration(t)
noop := &NoopBackend{}
Expand Down
7 changes: 7 additions & 0 deletions vault/request_handling.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -869,6 +869,8 @@ func (c *Core) handleRequest(ctx context.Context, req *logical.Request) (retResp
case logical.TokenTypeBatch:
case logical.TokenTypeService:
if err := c.expiration.RegisterAuth(ctx, &logical.TokenEntry{
TTL: auth.TTL,
Policies: auth.TokenPolicies,
Path: resp.Auth.CreationPath,
NamespaceID: ns.ID,
}, resp.Auth); err != nil {
Expand Down Expand Up @@ -1184,6 +1186,11 @@ func (c *Core) RegisterAuth(ctx context.Context, tokenTTL time.Duration, path st
Type: auth.TokenType,
}

if te.TTL == 0 && (len(te.Policies) != 1 || te.Policies[0] != "root") {
c.logger.Error("refusing to create a non-root zero TTL token")
return ErrInternalError
}

if err := c.tokenStore.create(ctx, &te); err != nil {
c.logger.Error("failed to create token", "error", err)
return ErrInternalError
Expand Down
33 changes: 22 additions & 11 deletions vault/token_store_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,15 @@ func TestTokenStore_CubbyholeDeletion(t *testing.T) {
Operation: logical.UpdateOperation,
Path: "create",
ClientToken: root,
Data: map[string]interface{}{
"ttl": "600s",
},
}
// Supplying token ID forces SHA1 hashing to be used
if i%2 == 0 {
tokenReq.Data = map[string]interface{}{
"id": "testroot",
"id": "testroot",
"ttl": "600s",
}
}
resp := testMakeTokenViaRequest(t, ts, tokenReq)
Expand Down Expand Up @@ -111,6 +115,9 @@ func TestTokenStore_CubbyholeTidy(t *testing.T) {
Operation: logical.UpdateOperation,
Path: "create",
ClientToken: root,
Data: map[string]interface{}{
"ttl": "600s",
},
}

resp := testMakeTokenViaRequest(t, ts, tokenReq)
Expand All @@ -119,7 +126,8 @@ func TestTokenStore_CubbyholeTidy(t *testing.T) {
// Supplying token ID forces SHA1 hashing to be used
if i%3 == 0 {
tokenReq.Data = map[string]interface{}{
"id": "testroot",
"id": "testroot",
"ttl": "600s",
}
}

Expand Down Expand Up @@ -541,10 +549,12 @@ func getBackendConfig(c *Core) *logical.BackendConfig {
}

func testMakeServiceTokenViaBackend(t testing.TB, ts *TokenStore, root, client, ttl string, policy []string) {
t.Helper()
testMakeTokenViaBackend(t, ts, root, client, ttl, policy, false)
}

func testMakeTokenViaBackend(t testing.TB, ts *TokenStore, root, client, ttl string, policy []string, batch bool) {
t.Helper()
req := logical.TestRequest(t, logical.UpdateOperation, "create")
req.ClientToken = root
if batch {
Expand All @@ -562,6 +572,7 @@ func testMakeTokenViaBackend(t testing.TB, ts *TokenStore, root, client, ttl str
}

func testMakeTokenViaRequest(t testing.TB, ts *TokenStore, req *logical.Request) *logical.Response {
t.Helper()
resp, err := ts.HandleRequest(namespace.RootContext(nil), req)
if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -719,7 +730,7 @@ func TestTokenStore_HandleRequest_LookupAccessor(t *testing.T) {
c, _, root := TestCoreUnsealed(t)
ts := c.tokenStore

testMakeServiceTokenViaBackend(t, ts, root, "tokenid", "", []string{"foo"})
testMakeServiceTokenViaBackend(t, ts, root, "tokenid", "60s", []string{"foo"})
out, err := ts.Lookup(namespace.RootContext(nil), "tokenid")
if err != nil {
t.Fatalf("err: %s", err)
Expand Down Expand Up @@ -757,7 +768,7 @@ func TestTokenStore_HandleRequest_ListAccessors(t *testing.T) {

testKeys := []string{"token1", "token2", "token3", "token4"}
for _, key := range testKeys {
testMakeServiceTokenViaBackend(t, ts, root, key, "", []string{"foo"})
testMakeServiceTokenViaBackend(t, ts, root, key, "60s", []string{"foo"})
}

// Revoke root to make the number of accessors match
Expand Down Expand Up @@ -2117,7 +2128,7 @@ func TestTokenStore_HandleRequest_Revoke(t *testing.T) {
}
root := rootToken.ID

testMakeServiceTokenViaBackend(t, ts, root, "child", "", []string{"root", "foo"})
testMakeServiceTokenViaBackend(t, ts, root, "child", "60s", []string{"root", "foo"})

te, err := ts.Lookup(namespace.RootContext(nil), "child")
if err != nil {
Expand All @@ -2139,7 +2150,7 @@ func TestTokenStore_HandleRequest_Revoke(t *testing.T) {
t.Fatalf("err: %v", err)
}

testMakeServiceTokenViaBackend(t, ts, "child", "sub-child", "", []string{"foo"})
testMakeServiceTokenViaBackend(t, ts, "child", "sub-child", "50s", []string{"foo"})

te, err = ts.Lookup(namespace.RootContext(nil), "sub-child")
if err != nil {
Expand Down Expand Up @@ -2193,8 +2204,8 @@ func TestTokenStore_HandleRequest_Revoke(t *testing.T) {
}

// Now test without registering the tokens through the expiration manager
testMakeServiceTokenViaBackend(t, ts, root, "child", "", []string{"root", "foo"})
testMakeServiceTokenViaBackend(t, ts, "child", "sub-child", "", []string{"foo"})
testMakeServiceTokenViaBackend(t, ts, root, "child", "60s", []string{"root", "foo"})
testMakeServiceTokenViaBackend(t, ts, "child", "sub-child", "50s", []string{"foo"})

req = logical.TestRequest(t, logical.UpdateOperation, "revoke")
req.Data = map[string]interface{}{
Expand Down Expand Up @@ -2231,8 +2242,8 @@ func TestTokenStore_HandleRequest_Revoke(t *testing.T) {
func TestTokenStore_HandleRequest_RevokeOrphan(t *testing.T) {
c, _, root := TestCoreUnsealed(t)
ts := c.tokenStore
testMakeServiceTokenViaBackend(t, ts, root, "child", "", []string{"root", "foo"})
testMakeServiceTokenViaBackend(t, ts, "child", "sub-child", "", []string{"foo"})
testMakeServiceTokenViaBackend(t, ts, root, "child", "60s", []string{"root", "foo"})
testMakeServiceTokenViaBackend(t, ts, "child", "sub-child", "50s", []string{"foo"})

req := logical.TestRequest(t, logical.UpdateOperation, "revoke-orphan")
req.Data = map[string]interface{}{
Expand Down Expand Up @@ -2283,7 +2294,7 @@ func TestTokenStore_HandleRequest_RevokeOrphan(t *testing.T) {
func TestTokenStore_HandleRequest_RevokeOrphan_NonRoot(t *testing.T) {
c, _, root := TestCoreUnsealed(t)
ts := c.tokenStore
testMakeServiceTokenViaBackend(t, ts, root, "child", "", []string{"foo"})
testMakeServiceTokenViaBackend(t, ts, root, "child", "60s", []string{"foo"})

out, err := ts.Lookup(namespace.RootContext(nil), "child")
if err != nil {
Expand Down

0 comments on commit 986a88b

Please sign in to comment.