diff --git a/.github/workflows/binaries-publish.yaml b/.github/workflows/binaries-publish.yaml index a9feb376..82912ef8 100644 --- a/.github/workflows/binaries-publish.yaml +++ b/.github/workflows/binaries-publish.yaml @@ -16,6 +16,9 @@ jobs: - name: Set up Go uses: actions/setup-go@v5 + with: + go-version: "1.23" + cache-dependency-path: backend/go.mod - uses: pnpm/action-setup@v2 with: diff --git a/.github/workflows/e2e-partial.yaml b/.github/workflows/e2e-partial.yaml index 838da8f3..f6ee9213 100644 --- a/.github/workflows/e2e-partial.yaml +++ b/.github/workflows/e2e-partial.yaml @@ -27,7 +27,8 @@ jobs: - name: Set up Go uses: actions/setup-go@v5 with: - go-version: "1.21" + go-version: "1.23" + cache-dependency-path: backend/go.mod - uses: actions/setup-node@v4 with: diff --git a/.github/workflows/partial-backend.yaml b/.github/workflows/partial-backend.yaml index e645a762..b8d7064f 100644 --- a/.github/workflows/partial-backend.yaml +++ b/.github/workflows/partial-backend.yaml @@ -12,7 +12,8 @@ jobs: - name: Set up Go uses: actions/setup-go@v5 with: - go-version: "1.21" + go-version: "1.23" + cache-dependency-path: backend/go.mod - name: Install Task uses: arduino/setup-task@v1 diff --git a/.github/workflows/partial-frontend.yaml b/.github/workflows/partial-frontend.yaml index baca779a..b219fed3 100644 --- a/.github/workflows/partial-frontend.yaml +++ b/.github/workflows/partial-frontend.yaml @@ -60,7 +60,8 @@ jobs: - name: Set up Go uses: actions/setup-go@v5 with: - go-version: "1.21" + go-version: "1.23" + cache-dependency-path: backend/go.mod - uses: actions/setup-node@v4 with: @@ -110,7 +111,8 @@ jobs: - name: Set up Go uses: actions/setup-go@v5 with: - go-version: "1.21" + go-version: "1.23" + cache-dependency-path: backend/go.mod - uses: actions/setup-node@v4 with: diff --git a/Taskfile.yml b/Taskfile.yml index 13ec614c..c8f49973 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -95,7 +95,7 @@ tasks: desc: Runs all go tests using gotestsum - supports passing gotestsum args dir: backend cmds: - - gotestsum {{ .CLI_ARGS }} ./... + - go test {{ .CLI_ARGS }} ./... go:coverage: desc: Runs all go tests with -race flag and generates a coverage report diff --git a/backend/internal/core/services/service_user.go b/backend/internal/core/services/service_user.go index 0b67cb31..b3397527 100644 --- a/backend/internal/core/services/service_user.go +++ b/backend/internal/core/services/service_user.go @@ -190,10 +190,23 @@ func (svc *UserService) Login(ctx context.Context, username, password string, ex return UserAuthTokenDetail{}, ErrorInvalidLogin } - if !hasher.CheckPasswordHash(password, usr.PasswordHash) { + check, rehash := hasher.CheckPasswordHash(password, usr.PasswordHash) + + if !check { return UserAuthTokenDetail{}, ErrorInvalidLogin } + if rehash { + hash, err := hasher.HashPassword(password) + if err != nil { + log.Err(err).Msg("Failed to hash password") + return UserAuthTokenDetail{}, err + } + err = svc.repos.Users.ChangePassword(ctx, usr.ID, hash) + if err != nil { + return UserAuthTokenDetail{}, err + } + } return svc.createSessionToken(ctx, usr.ID, extendedSession) } @@ -227,7 +240,8 @@ func (svc *UserService) ChangePassword(ctx Context, current string, new string) return false } - if !hasher.CheckPasswordHash(current, usr.PasswordHash) { + match, _ := hasher.CheckPasswordHash(current, usr.PasswordHash) + if !match { log.Err(errors.New("current password is incorrect")).Msg("Failed to change password") return false } diff --git a/backend/pkgs/hasher/password.go b/backend/pkgs/hasher/password.go index a68c8689..796cf201 100644 --- a/backend/pkgs/hasher/password.go +++ b/backend/pkgs/hasher/password.go @@ -1,14 +1,35 @@ package hasher import ( + "crypto/rand" + "crypto/subtle" + "encoding/base64" "fmt" "os" + "strings" + "golang.org/x/crypto/argon2" "golang.org/x/crypto/bcrypt" ) var enabled = true +type params struct { + memory uint32 + iterations uint32 + parallelism uint8 + saltLength uint32 + keyLength uint32 +} + +var p = ¶ms{ + memory: 64 * 1024, + iterations: 3, + parallelism: 2, + saltLength: 16, + keyLength: 32, +} + func init() { // nolint: gochecknoinits disableHas := os.Getenv("UNSAFE_DISABLE_PASSWORD_PROJECTION") == "yes_i_am_sure" @@ -18,20 +39,108 @@ func init() { // nolint: gochecknoinits } } +func GenerateRandomBytes(n uint32) ([]byte, error) { + b := make([]byte, n) + _, err := rand.Read(b) + if err != nil { + return nil, err + } + return b, nil +} + func HashPassword(password string) (string, error) { if !enabled { return password, nil } - bytes, err := bcrypt.GenerateFromPassword([]byte(password), 14) - return string(bytes), err + salt, err := GenerateRandomBytes(p.saltLength) + if err != nil { + return "", err + } + hash := argon2.IDKey([]byte(password), salt, p.iterations, p.memory, p.parallelism, p.keyLength) + + b64Salt := base64.RawStdEncoding.EncodeToString(salt) + b64Hash := base64.RawStdEncoding.EncodeToString(hash) + + encodedHash := fmt.Sprintf("$argon2id$v=%d$m=%d,t=%d,p=%d$%s$%s", argon2.Version, 64*1024, 3, 2, b64Salt, b64Hash) + return encodedHash, err } -func CheckPasswordHash(password, hash string) bool { +// CheckPasswordHash checks if the provided password matches the hash. +// Additionally, it returns a boolean indicating whether the password should be rehashed. +func CheckPasswordHash(password, hash string) (bool, bool) { if !enabled { - return password == hash + return password == hash, false } - err := bcrypt.CompareHashAndPassword([]byte(hash), []byte(password)) - return err == nil + // Compare Argon2id hash first + match, err := comparePasswordAndHash(password, hash) + if err != nil || !match { + // If argon2id hash fails or doesn't match, try bcrypt + err := bcrypt.CompareHashAndPassword([]byte(hash), []byte(password)) + if err == nil { + // If bcrypt hash matches, return true and indicate rehashing + return true, true + } else { + // If both fail, return false and indicate no rehashing + return false, false + } + } + return match, false +} + +func comparePasswordAndHash(password, encodedHash string) (match bool, err error) { + // Extract the parameters, salt and derived key from the encoded password + // hash. + p, salt, hash, err := decodeHash(encodedHash) + if err != nil { + return false, err + } + + // Derive the key from the other password using the same parameters. + otherHash := argon2.IDKey([]byte(password), salt, p.iterations, p.memory, p.parallelism, p.keyLength) + + // Check that the contents of the hashed passwords are identical. Note + // that we are using the subtle.ConstantTimeCompare() function for this + // to help prevent timing attacks. + if subtle.ConstantTimeCompare(hash, otherHash) == 1 { + return true, nil + } + return false, nil +} + +func decodeHash(encodedHash string) (p *params, salt, hash []byte, err error) { + vals := strings.Split(encodedHash, "$") + if len(vals) != 6 { + return nil, nil, nil, fmt.Errorf("invalid hash format") + } + + var version int + _, err = fmt.Sscanf(vals[2], "v=%d", &version) + if err != nil { + return nil, nil, nil, err + } + if version != argon2.Version { + return nil, nil, nil, fmt.Errorf("unsupported argon2 version: %d", version) + } + + p = ¶ms{} + _, err = fmt.Sscanf(vals[3], "m=%d,t=%d,p=%d", &p.memory, &p.iterations, &p.parallelism) + if err != nil { + return nil, nil, nil, err + } + + salt, err = base64.RawStdEncoding.Strict().DecodeString(vals[4]) + if err != nil { + return nil, nil, nil, err + } + p.saltLength = uint32(len(salt)) + + hash, err = base64.RawStdEncoding.Strict().DecodeString(vals[5]) + if err != nil { + return nil, nil, nil, err + } + p.keyLength = uint32(len(hash)) + + return p, salt, hash, nil } diff --git a/backend/pkgs/hasher/password_test.go b/backend/pkgs/hasher/password_test.go index 6f9128ef..1585ace5 100644 --- a/backend/pkgs/hasher/password_test.go +++ b/backend/pkgs/hasher/password_test.go @@ -1,11 +1,14 @@ package hasher -import "testing" +import ( + "testing" +) func TestHashPassword(t *testing.T) { t.Parallel() type args struct { - password string + password string + invalidInputs []string } tests := []struct { name string @@ -15,13 +18,29 @@ func TestHashPassword(t *testing.T) { { name: "letters_and_numbers", args: args{ - password: "password123456788", + password: "password123456788", + invalidInputs: []string{"testPassword", "AnotherBadPassword", "ThisShouldNeverWork", "1234567890"}, }, }, { name: "letters_number_and_special", args: args{ - password: "!2afj3214pofajip3142j;fa", + password: "!2afj3214pofajip3142j;fa", + invalidInputs: []string{"testPassword", "AnotherBadPassword", "ThisShouldNeverWork", "1234567890"}, + }, + }, + { + name: "extra_long_password", + args: args{ + password: "this_is_a_very_long_password_that_should_be_hashed_properly_and_still_work_with_the_check_function", + invalidInputs: []string{"testPassword", "AnotherBadPassword", "ThisShouldNeverWork", "1234567890"}, + }, + }, + { + name: "empty_password", + args: args{ + password: "", + invalidInputs: []string{"testPassword", "AnotherBadPassword", "ThisShouldNeverWork", "1234567890"}, }, }, } @@ -32,9 +51,17 @@ func TestHashPassword(t *testing.T) { t.Errorf("HashPassword() error = %v, wantErr %v", err, tt.wantErr) return } - if !CheckPasswordHash(tt.args.password, got) { + check, _ := CheckPasswordHash(tt.args.password, got) + if !check { t.Errorf("CheckPasswordHash() failed to validate password=%v against hash=%v", tt.args.password, got) } + + for _, invalid := range tt.args.invalidInputs { + check, _ := CheckPasswordHash(invalid, got) + if check { + t.Errorf("CheckPasswordHash() improperly validated password=%v against hash=%v", invalid, got) + } + } }) } }