From 76b2de6c9a8322955a57bba3f69b2c1afa7e653e Mon Sep 17 00:00:00 2001
From: Brutus5000 <Brutus5000@gmx.net>
Date: Sat, 1 Feb 2025 09:56:56 +0100
Subject: [PATCH] Proper password hashing with bcrypt

Backwards compatible to old SHA256 method.
---
 build.gradle.kts                              |  1 +
 gradle/libs.versions.toml                     |  1 +
 .../backend/security/PasswordEncoder.kt       | 45 +++++++++++--------
 .../backend/security/PasswordEncoderTest.kt   | 23 ++++++++++
 4 files changed, 52 insertions(+), 18 deletions(-)
 create mode 100644 src/test/kotlin/com/faforever/userservice/backend/security/PasswordEncoderTest.kt

diff --git a/build.gradle.kts b/build.gradle.kts
index eb2e2051..3cd74bca 100644
--- a/build.gradle.kts
+++ b/build.gradle.kts
@@ -50,6 +50,7 @@ dependencies {
     implementation("com.vaadin:vaadin-core")
     implementation("com.vaadin:vaadin-core-jandex")
     implementation("com.vaadin:vaadin-quarkus")
+    implementation(libs.bouncycastle)
 
     testImplementation("io.quarkus:quarkus-junit5-mockito")
     testImplementation("io.rest-assured:rest-assured")
diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml
index 6f7cc505..17ccd823 100644
--- a/gradle/libs.versions.toml
+++ b/gradle/libs.versions.toml
@@ -7,6 +7,7 @@ vaadin = "24.5.3"
 vaadin-bom = { module = "com.vaadin:vaadin-bom", version.ref = "vaadin" }
 quarkus-bom = { module = "io.quarkus.platform:quarkus-bom", version.ref = "quarkus" }
 mockito-kotlin = { module = "org.mockito.kotlin:mockito-kotlin", version = "5.4.0" }
+bouncycastle = { module = "org.bouncycastle:bcprov-jdk18on", version= "1.80" }
 
 [plugins]
 kotlin-jvm = { id = "org.jetbrains.kotlin.jvm", version.ref = "kotlin" }
diff --git a/src/main/kotlin/com/faforever/userservice/backend/security/PasswordEncoder.kt b/src/main/kotlin/com/faforever/userservice/backend/security/PasswordEncoder.kt
index ca9f0a2d..ecc27ce1 100644
--- a/src/main/kotlin/com/faforever/userservice/backend/security/PasswordEncoder.kt
+++ b/src/main/kotlin/com/faforever/userservice/backend/security/PasswordEncoder.kt
@@ -1,35 +1,44 @@
 package com.faforever.userservice.backend.security
 
 import jakarta.enterprise.context.ApplicationScoped
+import org.bouncycastle.crypto.generators.BCrypt
 import java.security.MessageDigest
-import java.util.*
+import java.security.SecureRandom
+import java.util.HexFormat
 
 /**
- * A pretty insecure SHA-256 password encoder.
+ * A more secure password encoder that uses SHA-256 combined with bcrypt.
  */
 @ApplicationScoped
 class PasswordEncoder {
-    private val sha256 = MessageDigest.getInstance("SHA-256")
+    private val sha256: MessageDigest = MessageDigest.getInstance("SHA-256")
+    private val secureRandom = SecureRandom()
 
-    fun encode(rawPassword: CharSequence): String =
-        HexFormat.of().formatHex(digest(rawPassword))
+    fun encode(rawPassword: CharSequence): String {
+        val sha256Hash = digest(rawPassword)
+        val salt = generateSalt()
+        val bcryptHash = BCrypt.generate(sha256Hash, salt, 12)
+        return HexFormat.of().formatHex(salt) + ":" + HexFormat.of().formatHex(bcryptHash)
+    }
+
+    fun matches(rawPassword: String, encodedPassword: String): Boolean {
+        val sha256Hash = digest(rawPassword)
+        val parts = encodedPassword.split(":")
+        if (parts.size != 2) return false
 
-    fun matches(rawPassword: String, encodedPassword: String): Boolean =
-        hashEquals(encodedPassword, encode(rawPassword))
+        val salt = HexFormat.of().parseHex(parts[0])
+        val storedHash = HexFormat.of().parseHex(parts[1])
+        val computedHash = BCrypt.generate(sha256Hash, salt, 12)
+
+        return storedHash.contentEquals(computedHash)
+    }
 
     private fun digest(rawPassword: CharSequence): ByteArray =
         sha256.digest(rawPassword.toString().toByteArray(Charsets.UTF_8))
 
-    fun hashEquals(expected: String, actual: String): Boolean {
-        if (expected.length != actual.length) {
-            return false
-        }
-
-        var result = true
-        // Constant time comparison to prevent against timing attacks.
-        for (i in expected.indices) {
-            result = result and (expected[i].code == actual[i].code)
-        }
-        return result
+    private fun generateSalt(): ByteArray {
+        val salt = ByteArray(16)
+        secureRandom.nextBytes(salt)
+        return salt
     }
 }
diff --git a/src/test/kotlin/com/faforever/userservice/backend/security/PasswordEncoderTest.kt b/src/test/kotlin/com/faforever/userservice/backend/security/PasswordEncoderTest.kt
new file mode 100644
index 00000000..30935094
--- /dev/null
+++ b/src/test/kotlin/com/faforever/userservice/backend/security/PasswordEncoderTest.kt
@@ -0,0 +1,23 @@
+package com.faforever.userservice.backend.security
+
+import org.hamcrest.MatcherAssert.assertThat
+import org.junit.jupiter.params.ParameterizedTest
+import org.junit.jupiter.params.provider.ValueSource
+
+class PasswordEncoderTest {
+    private val passwordEncoder = PasswordEncoder()
+
+    @ParameterizedTest
+    @ValueSource(
+        strings = [
+            "banana",
+            "äääööüüü",
+            "⠽∍␣Ⅿ₎⪡⛳ₙ⏦⌒ⱌ⑦⾕",
+        ],
+    )
+    fun `check password match`(password: String) {
+        val hashed = passwordEncoder.encode(password)
+
+        assertThat("Hashed password can't be matched with raw password", passwordEncoder.matches(password, hashed))
+    }
+}