diff --git a/test/automated/api/009_configmanagement.test.js b/test/automated/api/009_configmanagement.test.js index afd6443e34..72b0024ba8 100644 --- a/test/automated/api/009_configmanagement.test.js +++ b/test/automated/api/009_configmanagement.test.js @@ -1,5 +1,6 @@ var request = require('supertest'); var bcrypt = require('bcrypt'); +var shajs = require("sha.js"); const sendAdminRequest = require('./lib/admin').sendAdminRequest; const failAdminRequest = require('./lib/admin').failAdminRequest; @@ -356,6 +357,35 @@ test('reset admin password', async () => { ); }); +let newAdminPasswordLong = randomString(80); + +test('change admin password >72 bytes', async () => { + await sendAdminRequest('config/adminpass', newAdminPasswordLong); +}); + +test('verify admin password change (>72 bytes)', async () => { + const res = await getAdminResponse( + 'serverconfig', + (adminPassword = newAdminPasswordLong) + ); + + bcrypt.compare( + shajs('SHA512').update(newAdminPasswordLong).digest(), + res.body.adminPassword, + function (err, result) { + expect(result).toBe(true); + } + ); +}); + +test('reset admin password (>72)', async () => { + await sendAdminRequest( + 'config/adminpass', + defaultAdminPassword, + (adminPassword = newAdminPasswordLong) + ); +}); + test('set override websocket host', async () => { await sendAdminRequest('config/sockethostoverride', overriddenWebsocketHost); }); diff --git a/test/automated/api/package.json b/test/automated/api/package.json index 6e3a777bc3..31d98a5b9a 100644 --- a/test/automated/api/package.json +++ b/test/automated/api/package.json @@ -14,6 +14,7 @@ "bcrypt": "^5.1.1", "crypto-random": "^2.0.1", "jsonfile": "^6.1.0", + "sha.js": "^2.4.12", "supertest": "^6.3.2", "websocket": "^1.0.32" }, @@ -27,4 +28,4 @@ "maxWorkers": 1, "testRunner": "jest-jasmine2" } -} \ No newline at end of file +} diff --git a/utils/hashing.go b/utils/hashing.go index 1e3a56e981..1f5470c92b 100644 --- a/utils/hashing.go +++ b/utils/hashing.go @@ -1,15 +1,28 @@ package utils import ( + "crypto/sha512" + "golang.org/x/crypto/bcrypt" ) func HashPassword(password string) (string, error) { + password_bytes := compressPassword([]byte(password)) // 0 will use the default cost of 10 instead - hash, err := bcrypt.GenerateFromPassword([]byte(password), 0) + hash, err := bcrypt.GenerateFromPassword(password_bytes, 0) return string(hash), err } -func ComparseHash(hash string, password string) error { - return bcrypt.CompareHashAndPassword([]byte(hash), []byte(password)) +func CompareHash(hash string, password string) error { + password_bytes := compressPassword([]byte(password)) + return bcrypt.CompareHashAndPassword([]byte(hash), password_bytes) +} + +// Takes a password and computes a sha-512 hash of it if it is longer than 72 bytes, guaranteeing it is less than 72 bytes long. +func compressPassword(password []byte) []byte { + if len(password) > 72 { + sha512_hashed := sha512.Sum512(password) + password = sha512_hashed[:] + } + return password } diff --git a/utils/hashing_test.go b/utils/hashing_test.go new file mode 100644 index 0000000000..526af62a17 --- /dev/null +++ b/utils/hashing_test.go @@ -0,0 +1,320 @@ +package utils + +import ( + "strings" + "testing" + + "golang.org/x/crypto/bcrypt" +) + +func TestHashPassword_ShortPassword(t *testing.T) { + password := "short" + hash, err := HashPassword(password) + if err != nil { + t.Errorf("HashPassword failed for short password: %v", err) + } + + if hash == "" { + t.Error("HashPassword returned empty hash for short password") + } + + if hash == password { + t.Error("HashPassword returned unhashed password") + } +} + +func TestHashPassword_72BytePassword(t *testing.T) { + // Test with exactly 72 bytes (bcrypt's limit) + password := strings.Repeat("a", 72) + hash, err := HashPassword(password) + if err != nil { + t.Errorf("HashPassword failed for 72-byte password: %v", err) + } + + if hash == "" { + t.Error("HashPassword returned empty hash for 72-byte password") + } + + if hash == password { + t.Error("HashPassword returned unhashed password") + } +} + +func TestHashPassword_LongPassword(t *testing.T) { + // Test with password longer than 72 bytes + password := strings.Repeat("a", 100) + hash, err := HashPassword(password) + if err != nil { + t.Errorf("HashPassword failed for long password: %v", err) + } + + if hash == "" { + t.Error("HashPassword returned empty hash for long password") + } + + if hash == password { + t.Error("HashPassword returned unhashed password") + } +} + +func TestHashPassword_VeryLongPassword(t *testing.T) { + // Test with very long password (200 bytes) + password := strings.Repeat("x", 200) + hash, err := HashPassword(password) + if err != nil { + t.Errorf("HashPassword failed for very long password: %v", err) + } + + if hash == "" { + t.Error("HashPassword returned empty hash for very long password") + } +} + +func TestCompareHash_ShortPassword_Success(t *testing.T) { + password := "testpassword123" + hash, err := HashPassword(password) + if err != nil { + t.Fatalf("HashPassword failed: %v", err) + } + + err = CompareHash(hash, password) + if err != nil { + t.Errorf("CompareHash failed to match correct short password: %v", err) + } +} + +func TestCompareHash_ShortPassword_Failure(t *testing.T) { + password := "testpassword123" + hash, err := HashPassword(password) + if err != nil { + t.Fatalf("HashPassword failed: %v", err) + } + + err = CompareHash(hash, "wrongpassword") + if err == nil { + t.Error("CompareHash incorrectly matched wrong password") + } +} + +func TestCompareHash_LongPassword_Success(t *testing.T) { + // Test with password longer than 72 bytes + password := strings.Repeat("a", 100) + hash, err := HashPassword(password) + if err != nil { + t.Fatalf("HashPassword failed for long password: %v", err) + } + + err = CompareHash(hash, password) + if err != nil { + t.Errorf("CompareHash failed to match correct long password: %v", err) + } +} + +func TestCompareHash_LongPassword_Failure(t *testing.T) { + // Test with password longer than 72 bytes + password := strings.Repeat("a", 100) + hash, err := HashPassword(password) + if err != nil { + t.Fatalf("HashPassword failed: %v", err) + } + + // Wrong password also longer than 72 bytes + wrongPassword := strings.Repeat("b", 100) + err = CompareHash(hash, wrongPassword) + if err == nil { + t.Error("CompareHash incorrectly matched wrong long password") + } +} + +func TestCompareHash_LongPassword_SlightDifference(t *testing.T) { + // Test that even a slight difference in long passwords is detected + password := strings.Repeat("a", 100) + hash, err := HashPassword(password) + if err != nil { + t.Fatalf("HashPassword failed: %v", err) + } + + // Same length but one character different + wrongPassword := strings.Repeat("a", 99) + "b" + err = CompareHash(hash, wrongPassword) + if err == nil { + t.Error("CompareHash incorrectly matched password with slight difference") + } +} + +func TestCompareHash_73BytePassword(t *testing.T) { + // Test edge case: 73 bytes (just over bcrypt's limit) + password := strings.Repeat("a", 73) + hash, err := HashPassword(password) + if err != nil { + t.Fatalf("HashPassword failed for 73-byte password: %v", err) + } + + err = CompareHash(hash, password) + if err != nil { + t.Errorf("CompareHash failed to match correct 73-byte password: %v", err) + } + + // Ensure wrong password doesn't match + wrongPassword := strings.Repeat("b", 73) + err = CompareHash(hash, wrongPassword) + if err == nil { + t.Error("CompareHash incorrectly matched wrong 73-byte password") + } +} + +func TestCompressPassword_ShortPassword(t *testing.T) { + password := []byte("short") + compressed := compressPassword(password) + + if len(compressed) != len(password) { + t.Error("compressPassword should not modify passwords shorter than 72 bytes") + } + + if string(compressed) != string(password) { + t.Error("compressPassword modified short password content") + } +} + +func TestCompressPassword_72BytePassword(t *testing.T) { + password := []byte(strings.Repeat("a", 72)) + compressed := compressPassword(password) + + if len(compressed) != 72 { + t.Error("compressPassword should not modify passwords exactly 72 bytes") + } + + if string(compressed) != string(password) { + t.Error("compressPassword modified 72-byte password content") + } +} + +func TestCompressPassword_LongPassword(t *testing.T) { + password := []byte(strings.Repeat("a", 100)) + compressed := compressPassword(password) + + // SHA-512 produces 64 bytes + if len(compressed) != 64 { + t.Errorf("compressPassword should produce 64-byte hash, got %d bytes", len(compressed)) + } + + // Ensure it's different from original + if string(compressed) == string(password) { + t.Error("compressPassword did not hash long password") + } +} + +func TestCompressPassword_DifferentLongPasswords(t *testing.T) { + password1 := []byte(strings.Repeat("a", 100)) + password2 := []byte(strings.Repeat("b", 100)) + + compressed1 := compressPassword(password1) + compressed2 := compressPassword(password2) + + if string(compressed1) == string(compressed2) { + t.Error("compressPassword produced same hash for different long passwords") + } +} + +func TestHashPassword_UniqueHashes(t *testing.T) { + password := "testpassword" + + // Generate two hashes of the same password + hash1, err1 := HashPassword(password) + hash2, err2 := HashPassword(password) + + if err1 != nil || err2 != nil { + t.Fatalf("HashPassword failed: %v, %v", err1, err2) + } + + // bcrypt includes a salt, so hashes should be different + if hash1 == hash2 { + t.Error("HashPassword produced identical hashes (should use salt)") + } + + // But both should validate against the original password + if err := CompareHash(hash1, password); err != nil { + t.Error("First hash does not validate against password") + } + + if err := CompareHash(hash2, password); err != nil { + t.Error("Second hash does not validate against password") + } +} + +func TestHashPassword_EmptyPassword(t *testing.T) { + password := "" + hash, err := HashPassword(password) + if err != nil { + t.Errorf("HashPassword failed for empty password: %v", err) + } + + err = CompareHash(hash, password) + if err != nil { + t.Error("CompareHash failed to match empty password") + } +} + +func TestHashPassword_SpecialCharacters(t *testing.T) { + password := "p@ssw0rd!#$%^&*()" + hash, err := HashPassword(password) + if err != nil { + t.Errorf("HashPassword failed for password with special characters: %v", err) + } + + err = CompareHash(hash, password) + if err != nil { + t.Error("CompareHash failed to match password with special characters") + } +} + +func TestHashPassword_UnicodeCharacters(t *testing.T) { + password := "пароль密码🔒" + hash, err := HashPassword(password) + if err != nil { + t.Errorf("HashPassword failed for password with unicode characters: %v", err) + } + + err = CompareHash(hash, password) + if err != nil { + t.Error("CompareHash failed to match password with unicode characters") + } +} + +func TestHashPassword_LongUnicodePassword(t *testing.T) { + // Create a long password with unicode characters (over 72 bytes) + password := strings.Repeat("密码🔒", 30) // Each character is multiple bytes + + if len([]byte(password)) <= 72 { + t.Skip("Unicode password not long enough for test") + } + + hash, err := HashPassword(password) + if err != nil { + t.Errorf("HashPassword failed for long unicode password: %v", err) + } + + err = CompareHash(hash, password) + if err != nil { + t.Error("CompareHash failed to match long unicode password") + } +} + +func TestHashPassword_BcryptCostIsDefault(t *testing.T) { + password := "testpassword" + hash, err := HashPassword(password) + if err != nil { + t.Fatalf("HashPassword failed: %v", err) + } + + // Extract the cost from the hash + cost, err := bcrypt.Cost([]byte(hash)) + if err != nil { + t.Fatalf("Failed to extract cost from hash: %v", err) + } + + // bcrypt.DefaultCost is 10 + if cost != bcrypt.DefaultCost { + t.Errorf("Expected cost %d, got %d", bcrypt.DefaultCost, cost) + } +} diff --git a/webserver/router/middleware/auth.go b/webserver/router/middleware/auth.go index c0c5045e57..1c950f7282 100644 --- a/webserver/router/middleware/auth.go +++ b/webserver/router/middleware/auth.go @@ -43,7 +43,7 @@ func RequireAdminAuth(handler http.HandlerFunc) http.HandlerFunc { user, pass, ok := r.BasicAuth() // Failed - if !ok || subtle.ConstantTimeCompare([]byte(user), []byte(username)) != 1 || utils.ComparseHash(password, pass) != nil { + if !ok || subtle.ConstantTimeCompare([]byte(user), []byte(username)) != 1 || utils.CompareHash(password, pass) != nil { w.Header().Set("WWW-Authenticate", `Basic realm="`+realm+`"`) http.Error(w, "Unauthorized", http.StatusUnauthorized) log.Debugln("Failed admin authentication")