Merge pull request abbot#43 from vania-pooh/master

Fixed concurrent map read and map write in default secret providers (issue crashes web server)

Author: abbot

users.go

@@ -1,7 +1,10 @@
 package auth
 
-import "encoding/csv"
-import "os"
+import (
+	"encoding/csv"
+	"os"
+	"sync"
+)
 
 /* 
  SecretProvider is used by authenticators. Takes user name and realm
@@ -20,13 +23,16 @@ type File struct {
 	Info os.FileInfo
 	/* must be set in inherited types during initialization */
 	Reload func()
+	mu     sync.Mutex
 }
 
 func (f *File) ReloadIfNeeded() {
 	info, err := os.Stat(f.Path)
 	if err != nil {
 		panic(err)
 	}
+	f.mu.Lock()
+	defer f.mu.Unlock()
 	if f.Info == nil || f.Info.ModTime() != info.ModTime() {
 		f.Info = info
 		f.Reload()
@@ -40,6 +46,7 @@ func (f *File) ReloadIfNeeded() {
 type HtdigestFile struct {
 	File
 	Users map[string]map[string]string
+	mu    sync.RWMutex
 }
 
 func reload_htdigest(hf *HtdigestFile) {
@@ -57,6 +64,8 @@ func reload_htdigest(hf *HtdigestFile) {
 		panic(err)
 	}
 
+	hf.mu.Lock()
+	defer hf.mu.Unlock()
 	hf.Users = make(map[string]map[string]string)
 	for _, record := range records {
 		_, exists := hf.Users[record[1]]
@@ -77,6 +86,8 @@ func HtdigestFileProvider(filename string) SecretProvider {
 	hf.Reload = func() { reload_htdigest(hf) }
 	return func(user, realm string) string {
 		hf.ReloadIfNeeded()
+		hf.mu.RLock()
+		defer hf.mu.RUnlock()
 		_, exists := hf.Users[realm]
 		if !exists {
 			return ""
@@ -96,6 +107,7 @@ func HtdigestFileProvider(filename string) SecretProvider {
 type HtpasswdFile struct {
 	File
 	Users map[string]string
+	mu    sync.RWMutex
 }
 
 func reload_htpasswd(h *HtpasswdFile) {
@@ -113,6 +125,8 @@ func reload_htpasswd(h *HtpasswdFile) {
 		panic(err)
 	}
 
+	h.mu.Lock()
+	defer h.mu.Unlock()
 	h.Users = make(map[string]string)
 	for _, record := range records {
 		h.Users[record[0]] = record[1]
@@ -129,7 +143,9 @@ func HtpasswdFileProvider(filename string) SecretProvider {
 	h.Reload = func() { reload_htpasswd(h) }
 	return func(user, realm string) string {
 		h.ReloadIfNeeded()
+		h.mu.RLock()
 		password, exists := h.Users[user]
+		h.mu.RUnlock()
 		if !exists {
 			return ""
 		}

users_test.go

@@ -1,7 +1,9 @@
 package auth
 
 import (
+	"os"
 	"testing"
+	"time"
 )
 
 func TestHtdigestFile(t *testing.T) {
@@ -31,3 +33,13 @@ func TestHtpasswdFile(t *testing.T) {
 		t.Fatal("Got passwd for non-existant user:", passwd)
 	}
 }
+
+// TestConcurrent verifies potential race condition in users reading logic
+func TestConcurrent(t *testing.T) {
+	secrets := HtpasswdFileProvider("test.htpasswd")
+	os.Chtimes("test.htpasswd", time.Now(), time.Now())
+	go func() {
+		secrets("test", "blah")
+	}()
+	secrets("test", "blah")
+}