From aadf82911ac0dfe73a84219228c5a876c57d0566 Mon Sep 17 00:00:00 2001 From: Jacob Shandling <61553566+jacobshandling@users.noreply.github.com> Date: Wed, 28 Feb 2024 11:16:02 -0800 Subject: [PATCH] Cap salt length before concatenating with plaintext for password updates (#17068) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## –> #16487 - Ensures length of salt is no larger than the key size. This ensures the remaining space within the 72-byte limit for salt+hash is the advertised 48 bytes, for the default salt size. - Per [this discussion](https://github.com/fleetdm/fleet/pull/16524#discussion_r1474675857), we are assuming ASCII characters, and therefore 1byte:1char, for now – unicode support may come later. - Updated tests to follow. ## Checklist for submitter - [x] Changes file added for user-visible changes in `changes/` - [x] Manual QA for all new/changed functionality --------- Co-authored-by: Jacob Shandling --- server/fleet/users.go | 1 + server/fleet/users_test.go | 12 ++++++++++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/server/fleet/users.go b/server/fleet/users.go index 0549b86416..94f22292bd 100644 --- a/server/fleet/users.go +++ b/server/fleet/users.go @@ -404,6 +404,7 @@ func saltAndHashPassword(keySize int, plaintext string, cost int) (hashed []byte return nil, "", err } + salt = salt[:keySize] withSalt := []byte(fmt.Sprintf("%s%s", plaintext, salt)) hashed, err = bcrypt.GenerateFromPassword(withSalt, cost) if err != nil { diff --git a/server/fleet/users_test.go b/server/fleet/users_test.go index a1c9c12c78..1c4e45a655 100644 --- a/server/fleet/users_test.go +++ b/server/fleet/users_test.go @@ -164,11 +164,11 @@ func TestUserPasswordRequirements(t *testing.T) { } func TestSaltAndHashPassword(t *testing.T) { - passwordTests := []string{"foobar!!", "bazbing!!"} + goodTests := []string{"foobar!!", "bazbing!!", "foobarbaz!!!foobarbaz!!!foobarbaz!!!foobarbaz!!", "foobarbaz!!!foobarbaz!!!foobarbaz!!!foobarbaz!!!"} keySize := 24 cost := 10 - for _, pwd := range passwordTests { + for _, pwd := range goodTests { hashed, salt, err := saltAndHashPassword(keySize, pwd, cost) require.NoError(t, err) @@ -178,6 +178,14 @@ func TestSaltAndHashPassword(t *testing.T) { err = bcrypt.CompareHashAndPassword(hashed, []byte(fmt.Sprint("invalidpassword", salt))) require.Error(t, err) + + // too long + badTests := []string{"foobarbaz!!!foobarbaz!!!foobarbaz!!!foobarbaz!!!!"} + for _, pwd := range badTests { + _, _, err := saltAndHashPassword(keySize, pwd, cost) + require.Error(t, err) + + } } }