From 695c379a6cbb6e1be59bc12719cb40fcc0b483b1 Mon Sep 17 00:00:00 2001 From: Juliusz Chroboczek Date: Fri, 12 Apr 2024 01:05:48 +0200 Subject: [PATCH] Distinguish empty key in password from absence of key. --- .../galene-password-generator.go | 6 +++-- group/client.go | 27 ++++++++++++------- group/client_test.go | 20 +++++++++++--- group/description.go | 2 +- group/description_test.go | 5 ++-- webserver/api.go | 3 ++- webserver/api_test.go | 2 +- 7 files changed, 46 insertions(+), 19 deletions(-) diff --git a/galene-password-generator/galene-password-generator.go b/galene-password-generator/galene-password-generator.go index d64aa09..60be619 100644 --- a/galene-password-generator/galene-password-generator.go +++ b/galene-password-generator/galene-password-generator.go @@ -60,10 +60,11 @@ func main() { key := pbkdf2.Key( []byte(pw), salt, iterations, length, sha256.New, ) + encoded := hex.EncodeToString(key) p = group.Password{ Type: "pbkdf2", Hash: "sha-256", - Key: hex.EncodeToString(key), + Key: &encoded, Salt: hex.EncodeToString(salt), Iterations: iterations, } @@ -75,9 +76,10 @@ func main() { log.Fatalf("Couldn't hash password: %v", err) } + k := string(key) p = group.Password{ Type: "bcrypt", - Key: string(key), + Key: &k, } } else { log.Fatalf("Unknown hash type %v", algorithm) diff --git a/group/client.go b/group/client.go index b9e83e7..43dfe10 100644 --- a/group/client.go +++ b/group/client.go @@ -15,11 +15,11 @@ import ( ) type RawPassword struct { - Type string `json:"type,omitempty"` - Hash string `json:"hash,omitempty"` - Key string `json:"key"` - Salt string `json:"salt,omitempty"` - Iterations int `json:"iterations,omitempty"` + Type string `json:"type,omitempty"` + Hash string `json:"hash,omitempty"` + Key *string `json:"key,omitempty"` + Salt string `json:"salt,omitempty"` + Iterations int `json:"iterations,omitempty"` } type Password RawPassword @@ -29,11 +29,17 @@ func (p Password) Match(pw string) (bool, error) { case "": return false, errors.New("missing password") case "plain": - return p.Key == pw, nil + if p.Key == nil { + return false, errors.New("missing key") + } + return *p.Key == pw, nil case "wildcard": return true, nil case "pbkdf2": - key, err := hex.DecodeString(p.Key) + if p.Key == nil { + return false, errors.New("missing key") + } + key, err := hex.DecodeString(*p.Key) if err != nil { return false, err } @@ -53,7 +59,10 @@ func (p Password) Match(pw string) (bool, error) { ) return bytes.Equal(key, theirKey), nil case "bcrypt": - err := bcrypt.CompareHashAndPassword([]byte(p.Key), []byte(pw)) + if p.Key == nil { + return false, errors.New("missing key") + } + err := bcrypt.CompareHashAndPassword([]byte(*p.Key), []byte(pw)) if err == bcrypt.ErrMismatchedHashAndPassword { return false, nil } @@ -69,7 +78,7 @@ func (p *Password) UnmarshalJSON(b []byte) error { if err == nil { *p = Password{ Type: "plain", - Key: k, + Key: &k, } return nil } diff --git a/group/client_test.go b/group/client_test.go index b2332ab..6d3d7d6 100644 --- a/group/client_test.go +++ b/group/client_test.go @@ -7,23 +7,28 @@ import ( "testing" ) +var key1 = "" var pw1 = Password{ Type: "plain", + Key: &key1, } +var key2 = "pass" var pw2 = Password{ Type: "plain", - Key: "pass", + Key: &key2, } +var key3 = "fe499504e8f144693fae828e8e371d50e019d0e4c84994fa03f7f445bd8a570a" var pw3 = Password{ Type: "pbkdf2", Hash: "sha-256", - Key: "fe499504e8f144693fae828e8e371d50e019d0e4c84994fa03f7f445bd8a570a", + Key: &key3, Salt: "bcc1717851030776", Iterations: 4096, } +var key4 = "$2a$10$afOr2f33onT/nDFFyT3mbOq5FMSw1wWXfyTXQTBMbKvZpBkoD3Qwu" var pw4 = Password{ Type: "bcrypt", - Key: "$2a$10$afOr2f33onT/nDFFyT3mbOq5FMSw1wWXfyTXQTBMbKvZpBkoD3Qwu", + Key: &key4, } var pw5 = Password{} var pw6 = Password{ @@ -66,6 +71,15 @@ func TestBad(t *testing.T) { } } +func TestEmptyKey(t *testing.T) { + for _, tpe := range []string{"", "plain", "pbkdf2", "bcrypt", "bad"} { + pw := Password{Type: tpe} + if match, err := pw.Match(""); err == nil || match { + t.Errorf("empty password of type %v didn't error", tpe) + } + } +} + func TestJSON(t *testing.T) { plain, err := json.Marshal(pw2) if err != nil || string(plain) != `"pass"` { diff --git a/group/description.go b/group/description.go index 81e85bd..2b8c4e6 100644 --- a/group/description.go +++ b/group/description.go @@ -629,7 +629,7 @@ func DeleteUser(group, username, etag string) error { } func UpdateUser(group, username, etag string, user *UserDescription) error { - if user.Password.Type != "" || user.Password.Key != "" { + if user.Password.Type != "" || user.Password.Key != nil { return errors.New("user description is not sanitised") } groups.mu.Lock() diff --git a/group/description_test.go b/group/description_test.go index 45ab770..1c71edf 100644 --- a/group/description_test.go +++ b/group/description_test.go @@ -216,16 +216,17 @@ func TestWritableGroups(t *testing.T) { t.Errorf("UpdateUser: got %v", err) } + pw := "pw" err = SetUserPassword("test", "jch", Password{ Type: "", - Key: "pw", + Key: &pw, }) if err != nil { t.Errorf("SetUserPassword: got %v", err) } desc, err = GetDescription("test") - if err != nil || desc.Users["jch"].Password.Key != "pw" { + if err != nil || *desc.Users["jch"].Password.Key != "pw" { t.Errorf("GetDescription: got %v %v, expected %v %v", err, desc.Users["jch"].Password.Key, nil, "pw", ) diff --git a/webserver/api.go b/webserver/api.go index d83c773..f7bba59 100644 --- a/webserver/api.go +++ b/webserver/api.go @@ -408,10 +408,11 @@ func passwordHandler(w http.ResponseWriter, r *http.Request, g, user string) { } iterations := 4096 key := pbkdf2.Key(body, salt, iterations, 32, sha256.New) + encoded := hex.EncodeToString(key) pw := group.Password{ Type: "pbkdf2", Hash: "sha-256", - Key: hex.EncodeToString(key), + Key: &encoded, Salt: hex.EncodeToString(salt), Iterations: iterations, } diff --git a/webserver/api_test.go b/webserver/api_test.go index db2f99d..f7aae5a 100644 --- a/webserver/api_test.go +++ b/webserver/api_test.go @@ -236,7 +236,7 @@ func TestApi(t *testing.T) { if err != nil { t.Errorf("Get user: %v", err) } - if user.Password.Type != "" && user.Password.Key != "" { + if user.Password.Type != "" && user.Password.Key != nil { t.Errorf("User not sanitised properly") }