From 9be7fc5320043c299fe4e39b650b9edf1f619cb4 Mon Sep 17 00:00:00 2001
From: Anthony Romano <anthony.romano@coreos.com>
Date: Wed, 12 Apr 2017 13:35:00 -0700
Subject: [PATCH] auth: protect simpleToken with single mutex and check if
 enabled

Dual locking doesn't really give a convincing performance improvement and
the lock ordering makes it impossible to safely check if the TTL keeper
is enabled or not.

Fixes #7722
---
 auth/simple_token.go | 37 +++++++++++++++++++------------------
 auth/store.go        |  9 +++------
 2 files changed, 22 insertions(+), 24 deletions(-)

diff --git a/auth/simple_token.go b/auth/simple_token.go
index 16bb85b1946..3a60fc44a2d 100644
--- a/auth/simple_token.go
+++ b/auth/simple_token.go
@@ -37,20 +37,10 @@ var (
 )
 
 type simpleTokenTTLKeeper struct {
-	tokensMu        sync.Mutex
 	tokens          map[string]time.Time
 	stopCh          chan chan struct{}
 	deleteTokenFunc func(string)
-}
-
-func NewSimpleTokenTTLKeeper(deletefunc func(string)) *simpleTokenTTLKeeper {
-	stk := &simpleTokenTTLKeeper{
-		tokens:          make(map[string]time.Time),
-		stopCh:          make(chan chan struct{}),
-		deleteTokenFunc: deletefunc,
-	}
-	go stk.run()
-	return stk
+	mu              *sync.Mutex
 }
 
 func (tm *simpleTokenTTLKeeper) stop() {
@@ -81,14 +71,14 @@ func (tm *simpleTokenTTLKeeper) run() {
 		select {
 		case <-tokenTicker.C:
 			nowtime := time.Now()
-			tm.tokensMu.Lock()
+			tm.mu.Lock()
 			for t, tokenendtime := range tm.tokens {
 				if nowtime.After(tokenendtime) {
 					tm.deleteTokenFunc(t)
 					delete(tm.tokens, t)
 				}
 			}
-			tm.tokensMu.Unlock()
+			tm.mu.Unlock()
 		case waitCh := <-tm.stopCh:
 			tm.tokens = make(map[string]time.Time)
 			waitCh <- struct{}{}
@@ -97,6 +87,22 @@ func (tm *simpleTokenTTLKeeper) run() {
 	}
 }
 
+func (as *authStore) enable() {
+	delf := func(tk string) {
+		if username, ok := as.simpleTokens[tk]; ok {
+			plog.Infof("deleting token %s for user %s", tk, username)
+			delete(as.simpleTokens, tk)
+		}
+	}
+	as.simpleTokenKeeper = &simpleTokenTTLKeeper{
+		tokens:          make(map[string]time.Time),
+		stopCh:          make(chan chan struct{}),
+		deleteTokenFunc: delf,
+		mu:              &as.simpleTokensMu,
+	}
+	go as.simpleTokenKeeper.run()
+}
+
 func (as *authStore) GenSimpleToken() (string, error) {
 	ret := make([]byte, defaultSimpleTokenLength)
 
@@ -113,9 +119,7 @@ func (as *authStore) GenSimpleToken() (string, error) {
 }
 
 func (as *authStore) assignSimpleTokenToUser(username, token string) {
-	as.simpleTokenKeeper.tokensMu.Lock()
 	as.simpleTokensMu.Lock()
-
 	_, ok := as.simpleTokens[token]
 	if ok {
 		plog.Panicf("token %s is alredy used", token)
@@ -124,14 +128,12 @@ func (as *authStore) assignSimpleTokenToUser(username, token string) {
 	as.simpleTokens[token] = username
 	as.simpleTokenKeeper.addSimpleToken(token)
 	as.simpleTokensMu.Unlock()
-	as.simpleTokenKeeper.tokensMu.Unlock()
 }
 
 func (as *authStore) invalidateUser(username string) {
 	if as.simpleTokenKeeper == nil {
 		return
 	}
-	as.simpleTokenKeeper.tokensMu.Lock()
 	as.simpleTokensMu.Lock()
 	for token, name := range as.simpleTokens {
 		if strings.Compare(name, username) == 0 {
@@ -140,5 +142,4 @@ func (as *authStore) invalidateUser(username string) {
 		}
 	}
 	as.simpleTokensMu.Unlock()
-	as.simpleTokenKeeper.tokensMu.Unlock()
 }
diff --git a/auth/store.go b/auth/store.go
index 5f97db84e6a..221d5cb7af9 100644
--- a/auth/store.go
+++ b/auth/store.go
@@ -215,8 +215,7 @@ func (as *authStore) AuthEnable() error {
 	tx.UnsafePut(authBucketName, enableFlagKey, authEnabled)
 
 	as.enabled = true
-
-	as.simpleTokenKeeper = NewSimpleTokenTTLKeeper(newDeleterFunc(as))
+	as.enable()
 
 	as.rangePermCache = make(map[string]*unifiedRangePermissions)
 
@@ -647,14 +646,12 @@ func (as *authStore) RoleAdd(r *pb.AuthRoleAddRequest) (*pb.AuthRoleAddResponse,
 
 func (as *authStore) AuthInfoFromToken(token string) (*AuthInfo, bool) {
 	// same as '(t *tokenSimple) info' in v3.2+
-	as.simpleTokenKeeper.tokensMu.Lock()
 	as.simpleTokensMu.Lock()
 	username, ok := as.simpleTokens[token]
-	if ok {
+	if ok && as.simpleTokenKeeper != nil {
 		as.simpleTokenKeeper.resetSimpleToken(token)
 	}
 	as.simpleTokensMu.Unlock()
-	as.simpleTokenKeeper.tokensMu.Unlock()
 	return &AuthInfo{Username: username, Revision: as.revision}, ok
 }
 
@@ -914,7 +911,7 @@ func NewAuthStore(be backend.Backend, indexWaiter func(uint64) <-chan struct{})
 	}
 
 	if enabled {
-		as.simpleTokenKeeper = NewSimpleTokenTTLKeeper(newDeleterFunc(as))
+		as.enable()
 	}
 
 	if as.revision == 0 {