Fix the concurrent map write error. (#177)

+ Add a lock to the keycache.
+ Ensure that all instantiations of keycaches use New, rather
  than the old keycache.Cache{make()} construct. This no longer
  works with the lock in place.
+ Update travis to run the race detector on a few specific packages
  that should help identify this type of problem in the future.

.travis.yml

@@ -9,6 +9,9 @@ script:
   - go list github.com/cloudflare/redoctober/... | grep -v vendor | xargs go vet
   - ./scripts/validate-html-generation
   - go list -f '{{if len .TestGoFiles}}"go test -coverprofile={{.Dir}}/.coverprofile {{.ImportPath}}"{{end}}' ./... | xargs -i sh -c {}
+  - go test -race github.com/cloudflare/redoctober/keycache
+  - go test -race github.com/cloudflare/redoctober/cryptor
+  - go test -race github.com/cloudflare/redoctober/core
   - gover . coverprofile.txt
 after_success:
   - bash <(curl -s https://codecov.io/bash) -f coverprofile.txt

core/core.go

@@ -262,7 +262,6 @@ func Init(path string, config *config.Config) error {
 
 	orders = order.NewOrderer(hipchatClient)
 	crypt, err = cryptor.New(&records, nil, config)
-
 	return err
 }
 

cryptor/cryptor.go

@@ -37,7 +37,8 @@ type Cryptor struct {
 
 func New(records *passvault.Records, cache *keycache.Cache, config *config.Config) (*Cryptor, error) {
 	if cache == nil {
-		cache = &keycache.Cache{UserKeys: make(map[keycache.DelegateIndex]keycache.ActiveUser)}
+		c := keycache.NewCache()
+		cache = &c
 	}
 
 	store, err := persist.New(config.Delegations)

keycache/keycache.go

@@ -15,6 +15,7 @@ import (
 	"fmt"
 	"log"
 	"strings"
+	"sync"
 	"time"
 
 	"github.com/cloudflare/redoctober/ecdh"
@@ -52,6 +53,7 @@ type ActiveUser struct {
 
 // Cache represents the current list of delegated keys in memory
 type Cache struct {
+	lock     *sync.Mutex
 	UserKeys map[DelegateIndex]ActiveUser
 }
 
@@ -93,12 +95,19 @@ func (usage Usage) matches(user string, labels []string) bool {
 
 // NewCache initalizes a new cache.
 func NewCache() Cache {
-	return Cache{make(map[DelegateIndex]ActiveUser)}
+	return Cache{
+		UserKeys: make(map[DelegateIndex]ActiveUser),
+		lock:     new(sync.Mutex),
+	}
 }
 
 // NewFrom takes the output of GetSummary and returns a new keycache.
 func NewFrom(summary map[string]ActiveUser) *Cache {
-	cache := &Cache{make(map[DelegateIndex]ActiveUser)}
+	cache := &Cache{
+		UserKeys: make(map[DelegateIndex]ActiveUser),
+		lock:     new(sync.Mutex),
+	}
+
 	for di, user := range summary {
 		diSplit := strings.SplitN(di, "-", 2)
 		index := DelegateIndex{
@@ -116,7 +125,9 @@ func NewFrom(summary map[string]ActiveUser) *Cache {
 
 // setUser takes an ActiveUser and adds it to the cache.
 func (cache *Cache) setUser(in ActiveUser, name, slot string) {
+	cache.lock.Lock()
 	cache.UserKeys[DelegateIndex{Name: name, Slot: slot}] = in
+	cache.lock.Unlock()
 }
 
 // Valid returns true if matching active user is present.
@@ -153,7 +164,7 @@ func (cache *Cache) MatchUser(name, user string, labels []string) (ActiveUser, s
 // for decryption or symmetric encryption
 func (cache *Cache) useKey(name, user, slot string, labels []string) {
 	if val, slot, present := cache.MatchUser(name, user, labels); present {
-		val.Usage.Uses -= 1
+		val.Usage.Uses--
 		if val.Usage.Uses <= 0 {
 			delete(cache.UserKeys, DelegateIndex{name, slot})
 		} else {
@@ -175,7 +186,7 @@ func (cache *Cache) GetSummary() map[string]ActiveUser {
 	return summaryData
 }
 
-// FlushCache removes all delegated keys. It returns true if the cache
+// Flush removes all delegated keys. It returns true if the cache
 // wasn't empty (i.e. there were active users removed), and false if
 // the cache was empty.
 func (cache *Cache) Flush() bool {
@@ -183,9 +194,11 @@ func (cache *Cache) Flush() bool {
 		return false
 	}
 
+	cache.lock.Lock()
 	for d := range cache.UserKeys {
 		delete(cache.UserKeys, d)
 	}
+	cache.lock.Unlock()
 
 	return true
 }

keycache/keycache_test.go

@@ -94,7 +94,7 @@ func TestTimeFlush(t *testing.T) {
 
 	cache := NewCache()
 
-	err = cache.AddKeyFromRecord(pr, "user", "weakpassword", nil, nil, 10, "", "1s")
+	err = cache.AddKeyFromRecord(pr, "user", "weakpassword", nil, nil, 10, "", "10s")
 	if err != nil {
 		t.Fatalf("%v", err)
 	}
@@ -104,7 +104,7 @@ func TestTimeFlush(t *testing.T) {
 		t.Fatalf("Error in number of live keys")
 	}
 
-	time.Sleep(time.Second)
+	time.Sleep(11 * time.Second)
 
 	dummy := make([]byte, 16)
 	pubEncryptedKey, err := pr.EncryptKey(dummy)
@@ -312,7 +312,7 @@ func TestRefresh(t *testing.T) {
 		pr, "user", "weakpassword",
 		[]string{"ci", "buildeng", "user"},
 		[]string{"red", "blue"},
-		1, "", "1s",
+		1, "", "10s",
 	)
 	if err != nil {
 		t.Fatalf("%v", err)
@@ -323,7 +323,7 @@ func TestRefresh(t *testing.T) {
 		t.Fatalf("Refresh should not have removed any active users.")
 	}
 
-	time.Sleep(2 * time.Second)
+	time.Sleep(10 * time.Second)
 
 	removed = cache.Refresh()
 	if removed != 1 {