From d5efd31fadf6d33884f601e0843deee009e0eb26 Mon Sep 17 00:00:00 2001 From: Philip O'Toole Date: Sun, 17 Dec 2023 15:52:03 -0500 Subject: [PATCH] Remove faulty bcrypt support See https://github.com/rqlite/rqlite/issues/1488 --- auth/credential_store.go | 72 ++--------------------------- auth/credential_store_bench_test.go | 30 ------------ auth/credential_store_test.go | 49 +------------------- 3 files changed, 4 insertions(+), 147 deletions(-) diff --git a/auth/credential_store.go b/auth/credential_store.go index 0d1de7f8..e33a0960 100644 --- a/auth/credential_store.go +++ b/auth/credential_store.go @@ -6,9 +6,6 @@ import ( "encoding/json" "io" "os" - "sync" - - "golang.org/x/crypto/bcrypt" ) const ( @@ -43,43 +40,6 @@ type BasicAuther interface { BasicAuth() (string, string, bool) } -// HashCache store hash values for users. Safe for use from multiple goroutines. -type HashCache struct { - mu sync.RWMutex - m map[string]map[string]struct{} -} - -// NewHashCache returns a instantiated HashCache -func NewHashCache() *HashCache { - return &HashCache{ - m: make(map[string]map[string]struct{}), - } -} - -// Check returns whether hash is valid for username. -func (h *HashCache) Check(username, hash string) bool { - h.mu.RLock() - defer h.mu.RUnlock() - m, ok := h.m[username] - if !ok { - return false - } - - _, ok = m[hash] - return ok -} - -// Store stores the given hash as a valid hash for username. -func (h *HashCache) Store(username, hash string) { - h.mu.Lock() - defer h.mu.Unlock() - _, ok := h.m[username] - if !ok { - h.m[username] = make(map[string]struct{}) - } - h.m[username][hash] = struct{}{} -} - // Credential represents authentication and authorization configuration for a single user. type Credential struct { Username string `json:"username,omitempty"` @@ -91,18 +51,13 @@ type Credential struct { type CredentialsStore struct { store map[string]string perms map[string]map[string]bool - - UseCache bool - hashCache *HashCache } // NewCredentialsStore returns a new instance of a CredentialStore. func NewCredentialsStore() *CredentialsStore { return &CredentialsStore{ - store: make(map[string]string), - perms: make(map[string]map[string]bool), - hashCache: NewHashCache(), - UseCache: true, + store: make(map[string]string), + perms: make(map[string]map[string]bool), } } @@ -155,28 +110,7 @@ func (c *CredentialsStore) Check(username, password string) bool { if !ok { return false } - - // Simple match with plaintext password in creds? - if password == pw { - return true - } - - // Maybe the given password is a hash -- check if the hash is good - // for the given user. We use a cache to avoid recomputing a value we - // previously computed (at substantial compute cost). - if c.UseCache && c.hashCache.Check(username, password) { - return true - } - - // Next, what's in the file may be hashed, so hash the given password - // and compare. - if bcrypt.CompareHashAndPassword([]byte(pw), []byte(password)) != nil { - return false - } - - // It's good -- cache that result for this user. - c.hashCache.Store(username, password) - return true + return password == pw } // Password returns the password for the given user. diff --git a/auth/credential_store_bench_test.go b/auth/credential_store_bench_test.go index f9c8bad2..16620ca3 100644 --- a/auth/credential_store_bench_test.go +++ b/auth/credential_store_bench_test.go @@ -33,33 +33,3 @@ func BenchmarkCredentialStore(b *testing.B) { store.CheckRequest(b1) } } - -func BenchmarkCredentialStoreNoCache(b *testing.B) { - const jsonStream = ` - [ - { - "username": "username1", - "password": "$2a$10$fKRHxrEuyDTP6tXIiDycr.nyC8Q7UMIfc31YMyXHDLgRDyhLK3VFS" - }, - { "username": "username2", - "password": "password2" - } - ] - ` - - store := NewCredentialsStore() - store.UseCache = false - if err := store.Load(strings.NewReader(jsonStream)); err != nil { - panic("failed to load multiple credentials") - } - - b1 := &testBasicAuther{ - username: "username1", - password: "password1", - ok: true, - } - - for n := 0; n < b.N; n++ { - store.CheckRequest(b1) - } -} diff --git a/auth/credential_store_test.go b/auth/credential_store_test.go index f86a4945..8590ab19 100644 --- a/auth/credential_store_test.go +++ b/auth/credential_store_test.go @@ -16,53 +16,6 @@ func (t *testBasicAuther) BasicAuth() (string, string, bool) { return t.username, t.password, t.ok } -func Test_HashCache(t *testing.T) { - hc := NewHashCache() - - if hc.Check("user", "hash1") { - t.Fatalf("hash cache check OK for empty cache") - } - if hc.Check("user", "") { - t.Fatalf("hash cache check OK for empty cache") - } - if hc.Check("", "") { - t.Fatalf("hash cache check OK for empty cache") - } - - hc.Store("user1", "hash1") - if !hc.Check("user1", "hash1") { - t.Fatalf("hash cache check not OK for user1") - } - if hc.Check("user", "hash1") { - t.Fatalf("hash cache check OK for bad user") - } - - hc.Store("user1", "hash2") - if !hc.Check("user1", "hash1") { - t.Fatalf("hash cache check not OK for user1") - } - if !hc.Check("user1", "hash2") { - t.Fatalf("hash cache check not OK for user1") - } - - hc.Store("user3", "hash3") - if !hc.Check("user1", "hash1") { - t.Fatalf("hash cache check not OK for user1") - } - if !hc.Check("user1", "hash2") { - t.Fatalf("hash cache check not OK for user1") - } - if hc.Check("user", "hash1") { - t.Fatalf("hash cache check OK for bad user") - } - if !hc.Check("user3", "hash3") { - t.Fatalf("hash cache check not OK for user3") - } - if hc.Check("user3", "hash1") { - t.Fatalf("hash cache check OK for user3, with bad hash") - } -} - func Test_AuthLoadEmpty(t *testing.T) { const jsonStream = `[]` @@ -349,7 +302,7 @@ func Test_AuthLoadHashedSingleRequest(t *testing.T) { [ { "username": "username1", - "password": "$2a$10$fKRHxrEuyDTP6tXIiDycr.nyC8Q7UMIfc31YMyXHDLgRDyhLK3VFS" + "password": "password1" }, { "username": "username2", "password": "password2"